[Qemu-devel] [PATCH 3/3] pcie: work around for racy guest init

Michael S. Tsirkin posted 3 patches 6 years, 4 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
[Qemu-devel] [PATCH 3/3] pcie: work around for racy guest init
Posted by Michael S. Tsirkin 6 years, 4 months ago
During boot, linux guests tend to clear all bits in pcie slot status
register which is used for hotplug.
If they clear bits that weren't set this is racy and will lose events:
not a big problem for manual hotplug on bare-metal, but a problem for us.

For example, the following is broken ATM:

/x86_64-softmmu/qemu-system-x86_64 -enable-kvm -S -machine q35  \
    -device pcie-root-port,id=pcie_root_port_0,slot=2,chassis=2,addr=0x2,bus=pcie.0 \
    -device virtio-balloon-pci,id=balloon,bus=pcie_root_port_0 \
    -monitor stdio disk.qcow2
(qemu)device_del balloon
(qemu)cont

Balloon isn't deleted as it should.

As a work-around, detect this attempt to clear slot status and revert
status to what it was before the write.

Note: in theory this can be detected as a duplicate button press
which cancels the previous press. Does not seem to happen in
practice as guests seem to only have this bug during init.

Note2: the right thing to do is probably to fix Linux to
read status before clearing it, and act on the bits that are set.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pcie.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index f8490a00de..c605d32dd4 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -610,6 +610,25 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint16_t slt_ctl, uint16_t slt_s
     uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
 
     if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) {
+        /*
+         * Guests tend to clears all bits during init.
+         * If they clear bits that weren't set this is racy and will lose events:
+         * not a big problem for manual button presses, but a problem for us.
+         * As a work-around, detect this and revert status to what it was
+         * before the write.
+         *
+         * Note: in theory this can be detected as a duplicate button press
+         * which cancels the previous press. Does not seem to happen in
+         * practice as guests seem to only have this bug during init.
+         */
+#define PCIE_SLOT_EVENTS (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | \
+                          PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | \
+                          PCI_EXP_SLTSTA_CC)
+
+        if (val & ~slt_sta & PCIE_SLOT_EVENTS) {
+            sltsta = (sltsta & ~PCIE_SLOT_EVENTS) | (slt_sta & PCIE_SLOT_EVENTS);
+            pci_set_word(exp_cap + PCI_EXP_SLTSTA, sltsta);
+        }
         hotplug_event_clear(dev);
     }
 
-- 
MST


Re: [Qemu-devel] [PATCH 3/3] pcie: work around for racy guest init
Posted by Marcel Apfelbaum 6 years, 4 months ago

On 6/21/19 9:46 AM, Michael S. Tsirkin wrote:
> During boot, linux guests tend to clear all bits in pcie slot status
> register which is used for hotplug.
> If they clear bits that weren't set this is racy and will lose events:
> not a big problem for manual hotplug on bare-metal, but a problem for us.
>
> For example, the following is broken ATM:
>
> /x86_64-softmmu/qemu-system-x86_64 -enable-kvm -S -machine q35  \
>      -device pcie-root-port,id=pcie_root_port_0,slot=2,chassis=2,addr=0x2,bus=pcie.0 \
>      -device virtio-balloon-pci,id=balloon,bus=pcie_root_port_0 \
>      -monitor stdio disk.qcow2
> (qemu)device_del balloon
> (qemu)cont
>
> Balloon isn't deleted as it should.
>
> As a work-around, detect this attempt to clear slot status and revert
> status to what it was before the write.
>
> Note: in theory this can be detected as a duplicate button press
> which cancels the previous press. Does not seem to happen in
> practice as guests seem to only have this bug during init.
>
> Note2: the right thing to do is probably to fix Linux to
> read status before clearing it, and act on the bits that are set.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   hw/pci/pcie.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index f8490a00de..c605d32dd4 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -610,6 +610,25 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint16_t slt_ctl, uint16_t slt_s
>       uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
>   
>       if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) {
> +        /*
> +         * Guests tend to clears all bits during init.
> +         * If they clear bits that weren't set this is racy and will lose events:
> +         * not a big problem for manual button presses, but a problem for us.
> +         * As a work-around, detect this and revert status to what it was
> +         * before the write.
> +         *
> +         * Note: in theory this can be detected as a duplicate button press
> +         * which cancels the previous press. Does not seem to happen in
> +         * practice as guests seem to only have this bug during init.
> +         */
> +#define PCIE_SLOT_EVENTS (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | \
> +                          PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | \
> +                          PCI_EXP_SLTSTA_CC)
> +
> +        if (val & ~slt_sta & PCIE_SLOT_EVENTS) {
> +            sltsta = (sltsta & ~PCIE_SLOT_EVENTS) | (slt_sta & PCIE_SLOT_EVENTS);
> +            pci_set_word(exp_cap + PCI_EXP_SLTSTA, sltsta);
> +        }
>           hotplug_event_clear(dev);
>       }
>   

Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>

Thanks,
Marcel

Re: [Qemu-devel] [PATCH 3/3] pcie: work around for racy guest init
Posted by Igor Mammedov 6 years, 4 months ago
On Fri, 21 Jun 2019 02:46:50 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> During boot, linux guests tend to clear all bits in pcie slot status
> register which is used for hotplug.
> If they clear bits that weren't set this is racy and will lose events:
> not a big problem for manual hotplug on bare-metal, but a problem for us.
> 
> For example, the following is broken ATM:
> 
> /x86_64-softmmu/qemu-system-x86_64 -enable-kvm -S -machine q35  \
>     -device pcie-root-port,id=pcie_root_port_0,slot=2,chassis=2,addr=0x2,bus=pcie.0 \
>     -device virtio-balloon-pci,id=balloon,bus=pcie_root_port_0 \
>     -monitor stdio disk.qcow2
> (qemu)device_del balloon
> (qemu)cont
> 
> Balloon isn't deleted as it should.
> 
> As a work-around, detect this attempt to clear slot status and revert
> status to what it was before the write.
> 
> Note: in theory this can be detected as a duplicate button press
> which cancels the previous press. Does not seem to happen in
> practice as guests seem to only have this bug during init.
> 
> Note2: the right thing to do is probably to fix Linux to
> read status before clearing it, and act on the bits that are set.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>


had to change slot addr since #2 seems to be taken by default nic


> ---
>  hw/pci/pcie.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index f8490a00de..c605d32dd4 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -610,6 +610,25 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint16_t slt_ctl, uint16_t slt_s
>      uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
>  
>      if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) {
> +        /*
> +         * Guests tend to clears all bits during init.
> +         * If they clear bits that weren't set this is racy and will lose events:
> +         * not a big problem for manual button presses, but a problem for us.
> +         * As a work-around, detect this and revert status to what it was
> +         * before the write.
> +         *
> +         * Note: in theory this can be detected as a duplicate button press
> +         * which cancels the previous press. Does not seem to happen in
> +         * practice as guests seem to only have this bug during init.
> +         */
> +#define PCIE_SLOT_EVENTS (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | \
> +                          PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | \
> +                          PCI_EXP_SLTSTA_CC)
> +
> +        if (val & ~slt_sta & PCIE_SLOT_EVENTS) {
> +            sltsta = (sltsta & ~PCIE_SLOT_EVENTS) | (slt_sta & PCIE_SLOT_EVENTS);
> +            pci_set_word(exp_cap + PCI_EXP_SLTSTA, sltsta);
> +        }
>          hotplug_event_clear(dev);
>      }
>  


Re: [Qemu-devel] [PATCH 3/3] pcie: work around for racy guest init
Posted by Igor Mammedov 6 years, 4 months ago
On Fri, 21 Jun 2019 02:46:50 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> During boot, linux guests tend to clear all bits in pcie slot status
> register which is used for hotplug.
> If they clear bits that weren't set this is racy and will lose events:
> not a big problem for manual hotplug on bare-metal, but a problem for us.
> 
> For example, the following is broken ATM:
> 
> /x86_64-softmmu/qemu-system-x86_64 -enable-kvm -S -machine q35  \
>     -device pcie-root-port,id=pcie_root_port_0,slot=2,chassis=2,addr=0x2,bus=pcie.0 \
>     -device virtio-balloon-pci,id=balloon,bus=pcie_root_port_0 \
>     -monitor stdio disk.qcow2
> (qemu)device_del balloon
> (qemu)cont
> 
> Balloon isn't deleted as it should.
> 
> As a work-around, detect this attempt to clear slot status and revert
> status to what it was before the write.
> 
> Note: in theory this can be detected as a duplicate button press
> which cancels the previous press. Does not seem to happen in
> practice as guests seem to only have this bug during init.
> 
> Note2: the right thing to do is probably to fix Linux to
> read status before clearing it, and act on the bits that are set.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/pci/pcie.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index f8490a00de..c605d32dd4 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -610,6 +610,25 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint16_t slt_ctl, uint16_t slt_s
>      uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
>  
>      if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) {
> +        /*
> +         * Guests tend to clears all bits during init.
> +         * If they clear bits that weren't set this is racy and will lose events:
> +         * not a big problem for manual button presses, but a problem for us.
> +         * As a work-around, detect this and revert status to what it was
> +         * before the write.
> +         *
> +         * Note: in theory this can be detected as a duplicate button press
> +         * which cancels the previous press. Does not seem to happen in
> +         * practice as guests seem to only have this bug during init.
> +         */
> +#define PCIE_SLOT_EVENTS (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | \
> +                          PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | \
> +                          PCI_EXP_SLTSTA_CC)
> +
> +        if (val & ~slt_sta & PCIE_SLOT_EVENTS) {
> +            sltsta = (sltsta & ~PCIE_SLOT_EVENTS) | (slt_sta & PCIE_SLOT_EVENTS);
I'm reading it as:
  sltsta = LOWER_PART(sltsta) | UPPER_PART(sltsta)
which basically
  sltsta = sltsta
or am I missing something here?

> +            pci_set_word(exp_cap + PCI_EXP_SLTSTA, sltsta);
> +        }
>          hotplug_event_clear(dev);
>      }
>  


Re: [Qemu-devel] [PATCH 3/3] pcie: work around for racy guest init
Posted by Michael S. Tsirkin 6 years, 4 months ago
On Tue, Jun 25, 2019 at 03:07:30PM +0200, Igor Mammedov wrote:
> On Fri, 21 Jun 2019 02:46:50 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > During boot, linux guests tend to clear all bits in pcie slot status
> > register which is used for hotplug.
> > If they clear bits that weren't set this is racy and will lose events:
> > not a big problem for manual hotplug on bare-metal, but a problem for us.
> > 
> > For example, the following is broken ATM:
> > 
> > /x86_64-softmmu/qemu-system-x86_64 -enable-kvm -S -machine q35  \
> >     -device pcie-root-port,id=pcie_root_port_0,slot=2,chassis=2,addr=0x2,bus=pcie.0 \
> >     -device virtio-balloon-pci,id=balloon,bus=pcie_root_port_0 \
> >     -monitor stdio disk.qcow2
> > (qemu)device_del balloon
> > (qemu)cont
> > 
> > Balloon isn't deleted as it should.
> > 
> > As a work-around, detect this attempt to clear slot status and revert
> > status to what it was before the write.
> > 
> > Note: in theory this can be detected as a duplicate button press
> > which cancels the previous press. Does not seem to happen in
> > practice as guests seem to only have this bug during init.
> > 
> > Note2: the right thing to do is probably to fix Linux to
> > read status before clearing it, and act on the bits that are set.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/pci/pcie.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > index f8490a00de..c605d32dd4 100644
> > --- a/hw/pci/pcie.c
> > +++ b/hw/pci/pcie.c
> > @@ -610,6 +610,25 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint16_t slt_ctl, uint16_t slt_s
> >      uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
> >  
> >      if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) {
> > +        /*
> > +         * Guests tend to clears all bits during init.
> > +         * If they clear bits that weren't set this is racy and will lose events:
> > +         * not a big problem for manual button presses, but a problem for us.
> > +         * As a work-around, detect this and revert status to what it was
> > +         * before the write.
> > +         *
> > +         * Note: in theory this can be detected as a duplicate button press
> > +         * which cancels the previous press. Does not seem to happen in
> > +         * practice as guests seem to only have this bug during init.
> > +         */
> > +#define PCIE_SLOT_EVENTS (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | \
> > +                          PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | \
> > +                          PCI_EXP_SLTSTA_CC)
> > +
> > +        if (val & ~slt_sta & PCIE_SLOT_EVENTS) {
> > +            sltsta = (sltsta & ~PCIE_SLOT_EVENTS) | (slt_sta & PCIE_SLOT_EVENTS);
> I'm reading it as:
>   sltsta = LOWER_PART(sltsta) | UPPER_PART(sltsta)
> which basically
>   sltsta = sltsta
> or am I missing something here?

You are missing the underscore.

slt_sta is the old value.
sltsta is the new value.

> > +            pci_set_word(exp_cap + PCI_EXP_SLTSTA, sltsta);
> > +        }
> >          hotplug_event_clear(dev);
> >      }
> >  

Re: [Qemu-devel] [PATCH 3/3] pcie: work around for racy guest init
Posted by Igor Mammedov 6 years, 4 months ago
On Mon, 1 Jul 2019 05:20:41 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Jun 25, 2019 at 03:07:30PM +0200, Igor Mammedov wrote:
> > On Fri, 21 Jun 2019 02:46:50 -0400
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> > > During boot, linux guests tend to clear all bits in pcie slot status
> > > register which is used for hotplug.
> > > If they clear bits that weren't set this is racy and will lose events:
> > > not a big problem for manual hotplug on bare-metal, but a problem for us.
> > > 
> > > For example, the following is broken ATM:
> > > 
> > > /x86_64-softmmu/qemu-system-x86_64 -enable-kvm -S -machine q35  \
> > >     -device pcie-root-port,id=pcie_root_port_0,slot=2,chassis=2,addr=0x2,bus=pcie.0 \
> > >     -device virtio-balloon-pci,id=balloon,bus=pcie_root_port_0 \
> > >     -monitor stdio disk.qcow2
> > > (qemu)device_del balloon
> > > (qemu)cont
> > > 
> > > Balloon isn't deleted as it should.
> > > 
> > > As a work-around, detect this attempt to clear slot status and revert
> > > status to what it was before the write.
> > > 
> > > Note: in theory this can be detected as a duplicate button press
> > > which cancels the previous press. Does not seem to happen in
> > > practice as guests seem to only have this bug during init.
> > > 
> > > Note2: the right thing to do is probably to fix Linux to
> > > read status before clearing it, and act on the bits that are set.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  hw/pci/pcie.c | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > > 
> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index f8490a00de..c605d32dd4 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -610,6 +610,25 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint16_t slt_ctl, uint16_t slt_s
> > >      uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
> > >  
> > >      if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) {
> > > +        /*
> > > +         * Guests tend to clears all bits during init.
> > > +         * If they clear bits that weren't set this is racy and will lose events:
> > > +         * not a big problem for manual button presses, but a problem for us.
> > > +         * As a work-around, detect this and revert status to what it was
> > > +         * before the write.
> > > +         *
> > > +         * Note: in theory this can be detected as a duplicate button press
> > > +         * which cancels the previous press. Does not seem to happen in
> > > +         * practice as guests seem to only have this bug during init.
> > > +         */
> > > +#define PCIE_SLOT_EVENTS (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | \
> > > +                          PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | \
> > > +                          PCI_EXP_SLTSTA_CC)
> > > +
> > > +        if (val & ~slt_sta & PCIE_SLOT_EVENTS) {
> > > +            sltsta = (sltsta & ~PCIE_SLOT_EVENTS) | (slt_sta & PCIE_SLOT_EVENTS);  
> > I'm reading it as:
> >   sltsta = LOWER_PART(sltsta) | UPPER_PART(sltsta)
> > which basically
> >   sltsta = sltsta
> > or am I missing something here?  
> 
> You are missing the underscore.
> 
> slt_sta is the old value.
> sltsta is the new value.

I did notice it but still don't see where 
  exp_cap + PCI_EXP_SLTSTA
is being modified in call chain between
  pcie_cap_slot_get(d, &slt_ctl, &slt_sta)
  ...
  pcie_cap_slot_write_config():
      sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA)

> 
> > > +            pci_set_word(exp_cap + PCI_EXP_SLTSTA, sltsta);
> > > +        }
> > >          hotplug_event_clear(dev);
> > >      }
> > >    
> 


Re: [Qemu-devel] [PATCH 3/3] pcie: work around for racy guest init
Posted by Michael S. Tsirkin 6 years, 4 months ago
On Mon, Jul 01, 2019 at 02:04:02PM +0200, Igor Mammedov wrote:
> On Mon, 1 Jul 2019 05:20:41 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Tue, Jun 25, 2019 at 03:07:30PM +0200, Igor Mammedov wrote:
> > > On Fri, 21 Jun 2019 02:46:50 -0400
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >   
> > > > During boot, linux guests tend to clear all bits in pcie slot status
> > > > register which is used for hotplug.
> > > > If they clear bits that weren't set this is racy and will lose events:
> > > > not a big problem for manual hotplug on bare-metal, but a problem for us.
> > > > 
> > > > For example, the following is broken ATM:
> > > > 
> > > > /x86_64-softmmu/qemu-system-x86_64 -enable-kvm -S -machine q35  \
> > > >     -device pcie-root-port,id=pcie_root_port_0,slot=2,chassis=2,addr=0x2,bus=pcie.0 \
> > > >     -device virtio-balloon-pci,id=balloon,bus=pcie_root_port_0 \
> > > >     -monitor stdio disk.qcow2
> > > > (qemu)device_del balloon
> > > > (qemu)cont
> > > > 
> > > > Balloon isn't deleted as it should.
> > > > 
> > > > As a work-around, detect this attempt to clear slot status and revert
> > > > status to what it was before the write.
> > > > 
> > > > Note: in theory this can be detected as a duplicate button press
> > > > which cancels the previous press. Does not seem to happen in
> > > > practice as guests seem to only have this bug during init.
> > > > 
> > > > Note2: the right thing to do is probably to fix Linux to
> > > > read status before clearing it, and act on the bits that are set.
> > > > 
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >  hw/pci/pcie.c | 19 +++++++++++++++++++
> > > >  1 file changed, 19 insertions(+)
> > > > 
> > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > > index f8490a00de..c605d32dd4 100644
> > > > --- a/hw/pci/pcie.c
> > > > +++ b/hw/pci/pcie.c
> > > > @@ -610,6 +610,25 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint16_t slt_ctl, uint16_t slt_s
> > > >      uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
> > > >  
> > > >      if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) {
> > > > +        /*
> > > > +         * Guests tend to clears all bits during init.
> > > > +         * If they clear bits that weren't set this is racy and will lose events:
> > > > +         * not a big problem for manual button presses, but a problem for us.
> > > > +         * As a work-around, detect this and revert status to what it was
> > > > +         * before the write.
> > > > +         *
> > > > +         * Note: in theory this can be detected as a duplicate button press
> > > > +         * which cancels the previous press. Does not seem to happen in
> > > > +         * practice as guests seem to only have this bug during init.
> > > > +         */
> > > > +#define PCIE_SLOT_EVENTS (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | \
> > > > +                          PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | \
> > > > +                          PCI_EXP_SLTSTA_CC)
> > > > +
> > > > +        if (val & ~slt_sta & PCIE_SLOT_EVENTS) {
> > > > +            sltsta = (sltsta & ~PCIE_SLOT_EVENTS) | (slt_sta & PCIE_SLOT_EVENTS);  
> > > I'm reading it as:
> > >   sltsta = LOWER_PART(sltsta) | UPPER_PART(sltsta)
> > > which basically
> > >   sltsta = sltsta
> > > or am I missing something here?  
> > 
> > You are missing the underscore.
> > 
> > slt_sta is the old value.
> > sltsta is the new value.
> 
> I did notice it but still don't see where 
>   exp_cap + PCI_EXP_SLTSTA
> is being modified in call chain between
>   pcie_cap_slot_get(d, &slt_ctl, &slt_sta)
>   ...
>   pcie_cap_slot_write_config():
>       sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA)

It's modified by pci_bridge_write_config which calls pci_default_write_config.


> > 
> > > > +            pci_set_word(exp_cap + PCI_EXP_SLTSTA, sltsta);
> > > > +        }
> > > >          hotplug_event_clear(dev);
> > > >      }
> > > >    
> >