All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-12.0 v2] pcie: Model DLLLA transitions on Secondary Bus Reset
@ 2026-11-10  6:02 Cédric Le Goater
  2026-11-11 12:48 ` Michael S. Tsirkin
  0 siblings, 1 reply; 4+ messages in thread
From: Cédric Le Goater @ 2026-11-10  6:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael S. Tsirkin, Alex Williamson, Cédric Le Goater

Per PCIe r6.5 sec 6.6.1 and 7.5.3.8, setting the Secondary Bus Reset
bit on a downstream port drives the Data Link Layer to DL_Down,
clearing the Data Link Layer Link Active bit (DLLLA). On de-assertion
the link retrains and DLLLA is re-asserted.

Currently DLLLA is only managed during plug/unplug events, so a
guest sees stale DLLLA=1 while SBR is in progress.

Handle both SBR transitions in pci_bridge_write_config(), the common
config write path for all bridge types, guarded by
pci_is_express_downstream_port(). Also restore DLLLA in
pcie_cap_slot_reset() so that a system reset while SBR is asserted
does not leave DLLLA stuck at 0.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/hw/pci/pcie.h |  1 +
 hw/pci/pci_bridge.c   | 13 ++++++++++++-
 hw/pci/pcie.c         | 20 ++++++++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

 Changes in v2 :

 - Introduced pcie_cap_update_slot_link() as suggested by Michael.

diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index ec25e7a7de99..50fbbacb2200 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -112,6 +112,7 @@ void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slt_ctl, uint16_t *slt_sta);
 void pcie_cap_slot_write_config(PCIDevice *dev,
                                 uint16_t old_slt_ctl, uint16_t old_slt_sta,
                                 uint32_t addr, uint32_t val, int len);
+void pcie_cap_update_slot_link(PCIDevice *dev, uint16_t bctl);
 int pcie_cap_slot_post_load(void *opaque, int version_id);
 void pcie_cap_slot_push_attention_button(PCIDevice *dev);
 void pcie_cap_slot_enable_power(PCIDevice *dev);
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index e85932e41a92..a11cf2f20afa 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -33,6 +33,7 @@
 #include "qemu/units.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_bus.h"
+#include "hw/pci/pcie.h"
 #include "qemu/module.h"
 #include "qemu/range.h"
 #include "qapi/error.h"
@@ -274,9 +275,19 @@ void pci_bridge_write_config(PCIDevice *d,
 
     newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
     if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
-        /* Trigger hot reset on 0->1 transition. */
+        /* SBR assertion (0->1): bring link down, then reset devices. */
+        if (pci_is_express_downstream_port(d)) {
+            pcie_cap_update_slot_link(d, newctl);
+        }
         bus_cold_reset(BUS(&s->sec_bus));
     }
+
+    if (oldctl & ~newctl & PCI_BRIDGE_CTL_BUS_RESET) {
+        /* SBR de-assertion (1->0): bring link back up. */
+        if (pci_is_express_downstream_port(d)) {
+            pcie_cap_update_slot_link(d, newctl);
+        }
+    }
 }
 
 void pci_bridge_disable_base_limit(PCIDevice *dev)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index bd00305ecca6..3b6dcd15aa44 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -803,6 +803,8 @@ void pcie_cap_slot_reset(PCIDevice *dev)
                                  PCI_EXP_SLTSTA_PDC |
                                  PCI_EXP_SLTSTA_ABP);
 
+    pcie_cap_update_slot_link(dev,
+                              pci_get_word(dev->config + PCI_BRIDGE_CONTROL));
     pcie_cap_update_power(dev);
     hotplug_event_update_event_status(dev);
 }
@@ -946,6 +948,24 @@ void pcie_cap_slot_push_attention_button(PCIDevice *dev)
     pcie_cap_slot_event(dev, PCI_EXP_HP_EV_ABP);
 }
 
+/*
+ * Set DLLLA to reflect the actual link state: up iff a device is
+ * present on the secondary bus and SBR is not asserted.
+ */
+void pcie_cap_update_slot_link(PCIDevice *dev, uint16_t bctl)
+{
+    PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(dev));
+    uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
+    uint32_t lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
+    bool link_up = sec_bus->devices[0] && !(bctl & PCI_BRIDGE_CTL_BUS_RESET);
+
+    if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA ||
+        (lnkcap & PCI_EXP_LNKCAP_DLLLARC)) {
+        pci_set_word_by_mask(exp_cap + PCI_EXP_LNKSTA,
+                             PCI_EXP_LNKSTA_DLLLA, link_up);
+    }
+}
+
 /* root control/capabilities/status. PME isn't emulated for now */
 void pcie_cap_root_init(PCIDevice *dev)
 {
-- 
2.56.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH for-12.0 v2] pcie: Model DLLLA transitions on Secondary Bus Reset
  2026-11-10  6:02 [PATCH for-12.0 v2] pcie: Model DLLLA transitions on Secondary Bus Reset Cédric Le Goater
@ 2026-11-11 12:48 ` Michael S. Tsirkin
  2026-11-12 10:28   ` Cédric Le Goater
  0 siblings, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2026-11-11 12:48 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-devel, Alex Williamson

On Tue, Nov 10, 2026 at 07:02:04AM +0100, Cédric Le Goater wrote:
> Per PCIe r6.5 sec 6.6.1 and 7.5.3.8, setting the Secondary Bus Reset
> bit on a downstream port drives the Data Link Layer to DL_Down,
> clearing the Data Link Layer Link Active bit (DLLLA). On de-assertion
> the link retrains and DLLLA is re-asserted.
> 
> Currently DLLLA is only managed during plug/unplug events, so a
> guest sees stale DLLLA=1 while SBR is in progress.
> 
> Handle both SBR transitions in pci_bridge_write_config(), the common
> config write path for all bridge types, guarded by
> pci_is_express_downstream_port(). Also restore DLLLA in
> pcie_cap_slot_reset() so that a system reset while SBR is asserted
> does not leave DLLLA stuck at 0.
> 
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>  include/hw/pci/pcie.h |  1 +
>  hw/pci/pci_bridge.c   | 13 ++++++++++++-
>  hw/pci/pcie.c         | 20 ++++++++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
>  Changes in v2 :
> 
>  - Introduced pcie_cap_update_slot_link() as suggested by Michael.
> 
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index ec25e7a7de99..50fbbacb2200 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -112,6 +112,7 @@ void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slt_ctl, uint16_t *slt_sta);
>  void pcie_cap_slot_write_config(PCIDevice *dev,
>                                  uint16_t old_slt_ctl, uint16_t old_slt_sta,
>                                  uint32_t addr, uint32_t val, int len);
> +void pcie_cap_update_slot_link(PCIDevice *dev, uint16_t bctl);
>  int pcie_cap_slot_post_load(void *opaque, int version_id);
>  void pcie_cap_slot_push_attention_button(PCIDevice *dev);
>  void pcie_cap_slot_enable_power(PCIDevice *dev);
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index e85932e41a92..a11cf2f20afa 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -33,6 +33,7 @@
>  #include "qemu/units.h"
>  #include "hw/pci/pci_bridge.h"
>  #include "hw/pci/pci_bus.h"
> +#include "hw/pci/pcie.h"
>  #include "qemu/module.h"
>  #include "qemu/range.h"
>  #include "qapi/error.h"
> @@ -274,9 +275,19 @@ void pci_bridge_write_config(PCIDevice *d,
>  
>      newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
>      if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
> -        /* Trigger hot reset on 0->1 transition. */
> +        /* SBR assertion (0->1): bring link down, then reset devices. */
> +        if (pci_is_express_downstream_port(d)) {
> +            pcie_cap_update_slot_link(d, newctl);
> +        }
>          bus_cold_reset(BUS(&s->sec_bus));
>      }
> +
> +    if (oldctl & ~newctl & PCI_BRIDGE_CTL_BUS_RESET) {
> +        /* SBR de-assertion (1->0): bring link back up. */
> +        if (pci_is_express_downstream_port(d)) {
> +            pcie_cap_update_slot_link(d, newctl);
> +        }
> +    }
>  }
>  
>  void pci_bridge_disable_base_limit(PCIDevice *dev)


I'd rather call it once, unconditionally: it's idempotent.

Or if you really want:
	if ((oldctl ^ newctl) & PCI_BRIDGE_CTL_BUS_RESET)


> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index bd00305ecca6..3b6dcd15aa44 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -803,6 +803,8 @@ void pcie_cap_slot_reset(PCIDevice *dev)
>                                   PCI_EXP_SLTSTA_PDC |
>                                   PCI_EXP_SLTSTA_ABP);
>  
> +    pcie_cap_update_slot_link(dev,
> +                              pci_get_word(dev->config + PCI_BRIDGE_CONTROL));
>      pcie_cap_update_power(dev);
>      hotplug_event_update_event_status(dev);
>  }
> @@ -946,6 +948,24 @@ void pcie_cap_slot_push_attention_button(PCIDevice *dev)
>      pcie_cap_slot_event(dev, PCI_EXP_HP_EV_ABP);
>  }
>  
> +/*
> + * Set DLLLA to reflect the actual link state: up iff a device is
> + * present on the secondary bus and SBR is not asserted.
> + */
> +void pcie_cap_update_slot_link(PCIDevice *dev, uint16_t bctl)
> +{
> +    PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(dev));
> +    uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
> +    uint32_t lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
> +    bool link_up = sec_bus->devices[0] && !(bctl & PCI_BRIDGE_CTL_BUS_RESET);
> +
> +    if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA ||
> +        (lnkcap & PCI_EXP_LNKCAP_DLLLARC)) {
> +        pci_set_word_by_mask(exp_cap + PCI_EXP_LNKSTA,
> +                             PCI_EXP_LNKSTA_DLLLA, link_up);
> +    }
> +}
> +
>  /* root control/capabilities/status. PME isn't emulated for now */
>  void pcie_cap_root_init(PCIDevice *dev)
>  {
> -- 
> 2.56.0



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH for-12.0 v2] pcie: Model DLLLA transitions on Secondary Bus Reset
  2026-11-11 12:48 ` Michael S. Tsirkin
@ 2026-11-12 10:28   ` Cédric Le Goater
  2026-11-12 10:41     ` Michael S. Tsirkin
  0 siblings, 1 reply; 4+ messages in thread
From: Cédric Le Goater @ 2026-11-12 10:28 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, Alex Williamson

On 11/11/26 13:48, Michael S. Tsirkin wrote:
> On Tue, Nov 10, 2026 at 07:02:04AM +0100, Cédric Le Goater wrote:
>> Per PCIe r6.5 sec 6.6.1 and 7.5.3.8, setting the Secondary Bus Reset
>> bit on a downstream port drives the Data Link Layer to DL_Down,
>> clearing the Data Link Layer Link Active bit (DLLLA). On de-assertion
>> the link retrains and DLLLA is re-asserted.
>>
>> Currently DLLLA is only managed during plug/unplug events, so a
>> guest sees stale DLLLA=1 while SBR is in progress.
>>
>> Handle both SBR transitions in pci_bridge_write_config(), the common
>> config write path for all bridge types, guarded by
>> pci_is_express_downstream_port(). Also restore DLLLA in
>> pcie_cap_slot_reset() so that a system reset while SBR is asserted
>> does not leave DLLLA stuck at 0.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   include/hw/pci/pcie.h |  1 +
>>   hw/pci/pci_bridge.c   | 13 ++++++++++++-
>>   hw/pci/pcie.c         | 20 ++++++++++++++++++++
>>   3 files changed, 33 insertions(+), 1 deletion(-)
>>
>>   Changes in v2 :
>>
>>   - Introduced pcie_cap_update_slot_link() as suggested by Michael.
>>
>> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
>> index ec25e7a7de99..50fbbacb2200 100644
>> --- a/include/hw/pci/pcie.h
>> +++ b/include/hw/pci/pcie.h
>> @@ -112,6 +112,7 @@ void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slt_ctl, uint16_t *slt_sta);
>>   void pcie_cap_slot_write_config(PCIDevice *dev,
>>                                   uint16_t old_slt_ctl, uint16_t old_slt_sta,
>>                                   uint32_t addr, uint32_t val, int len);
>> +void pcie_cap_update_slot_link(PCIDevice *dev, uint16_t bctl);
>>   int pcie_cap_slot_post_load(void *opaque, int version_id);
>>   void pcie_cap_slot_push_attention_button(PCIDevice *dev);
>>   void pcie_cap_slot_enable_power(PCIDevice *dev);
>> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
>> index e85932e41a92..a11cf2f20afa 100644
>> --- a/hw/pci/pci_bridge.c
>> +++ b/hw/pci/pci_bridge.c
>> @@ -33,6 +33,7 @@
>>   #include "qemu/units.h"
>>   #include "hw/pci/pci_bridge.h"
>>   #include "hw/pci/pci_bus.h"
>> +#include "hw/pci/pcie.h"
>>   #include "qemu/module.h"
>>   #include "qemu/range.h"
>>   #include "qapi/error.h"
>> @@ -274,9 +275,19 @@ void pci_bridge_write_config(PCIDevice *d,
>>   
>>       newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
>>       if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
>> -        /* Trigger hot reset on 0->1 transition. */
>> +        /* SBR assertion (0->1): bring link down, then reset devices. */
>> +        if (pci_is_express_downstream_port(d)) {
>> +            pcie_cap_update_slot_link(d, newctl);
>> +        }
>>           bus_cold_reset(BUS(&s->sec_bus));
>>       }
>> +
>> +    if (oldctl & ~newctl & PCI_BRIDGE_CTL_BUS_RESET) {
>> +        /* SBR de-assertion (1->0): bring link back up. */
>> +        if (pci_is_express_downstream_port(d)) {
>> +            pcie_cap_update_slot_link(d, newctl);
>> +        }
>> +    }
>>   }
>>   
>>   void pci_bridge_disable_base_limit(PCIDevice *dev)
> 
> 
> I'd rather call it once, unconditionally: it's idempotent.
> 
> Or if you really want:
> 	if ((oldctl ^ newctl) & PCI_BRIDGE_CTL_BUS_RESET)

yes. Looks better. How's that ?

@@ -273,6 +274,12 @@ void pci_bridge_write_config(PCIDevice *
      }
  
      newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
+    if ((oldctl ^ newctl) & PCI_BRIDGE_CTL_BUS_RESET) {
+        if (pci_is_express_downstream_port(d)) {
+            pcie_cap_update_slot_link(d, newctl);
+        }
+    }
+
      if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
          /* Trigger hot reset on 0->1 transition. */
          bus_cold_reset(BUS(&s->sec_bus));


C.

> 
> 
>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>> index bd00305ecca6..3b6dcd15aa44 100644
>> --- a/hw/pci/pcie.c
>> +++ b/hw/pci/pcie.c
>> @@ -803,6 +803,8 @@ void pcie_cap_slot_reset(PCIDevice *dev)
>>                                    PCI_EXP_SLTSTA_PDC |
>>                                    PCI_EXP_SLTSTA_ABP);
>>   
>> +    pcie_cap_update_slot_link(dev,
>> +                              pci_get_word(dev->config + PCI_BRIDGE_CONTROL));
>>       pcie_cap_update_power(dev);
>>       hotplug_event_update_event_status(dev);
>>   }
>> @@ -946,6 +948,24 @@ void pcie_cap_slot_push_attention_button(PCIDevice *dev)
>>       pcie_cap_slot_event(dev, PCI_EXP_HP_EV_ABP);
>>   }
>>   
>> +/*
>> + * Set DLLLA to reflect the actual link state: up iff a device is
>> + * present on the secondary bus and SBR is not asserted.
>> + */
>> +void pcie_cap_update_slot_link(PCIDevice *dev, uint16_t bctl)
>> +{
>> +    PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(dev));
>> +    uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
>> +    uint32_t lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
>> +    bool link_up = sec_bus->devices[0] && !(bctl & PCI_BRIDGE_CTL_BUS_RESET);
>> +
>> +    if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA ||
>> +        (lnkcap & PCI_EXP_LNKCAP_DLLLARC)) {
>> +        pci_set_word_by_mask(exp_cap + PCI_EXP_LNKSTA,
>> +                             PCI_EXP_LNKSTA_DLLLA, link_up);
>> +    }
>> +}
>> +
>>   /* root control/capabilities/status. PME isn't emulated for now */
>>   void pcie_cap_root_init(PCIDevice *dev)
>>   {
>> -- 
>> 2.56.0
> 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH for-12.0 v2] pcie: Model DLLLA transitions on Secondary Bus Reset
  2026-11-12 10:28   ` Cédric Le Goater
@ 2026-11-12 10:41     ` Michael S. Tsirkin
  0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2026-11-12 10:41 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-devel, Alex Williamson

On Thu, Nov 12, 2026 at 11:28:34AM +0100, Cédric Le Goater wrote:
> On 11/11/26 13:48, Michael S. Tsirkin wrote:
> > On Tue, Nov 10, 2026 at 07:02:04AM +0100, Cédric Le Goater wrote:
> > > Per PCIe r6.5 sec 6.6.1 and 7.5.3.8, setting the Secondary Bus Reset
> > > bit on a downstream port drives the Data Link Layer to DL_Down,
> > > clearing the Data Link Layer Link Active bit (DLLLA). On de-assertion
> > > the link retrains and DLLLA is re-asserted.
> > > 
> > > Currently DLLLA is only managed during plug/unplug events, so a
> > > guest sees stale DLLLA=1 while SBR is in progress.
> > > 
> > > Handle both SBR transitions in pci_bridge_write_config(), the common
> > > config write path for all bridge types, guarded by
> > > pci_is_express_downstream_port(). Also restore DLLLA in
> > > pcie_cap_slot_reset() so that a system reset while SBR is asserted
> > > does not leave DLLLA stuck at 0.
> > > 
> > > Signed-off-by: Cédric Le Goater <clg@redhat.com>
> > > ---
> > >   include/hw/pci/pcie.h |  1 +
> > >   hw/pci/pci_bridge.c   | 13 ++++++++++++-
> > >   hw/pci/pcie.c         | 20 ++++++++++++++++++++
> > >   3 files changed, 33 insertions(+), 1 deletion(-)
> > > 
> > >   Changes in v2 :
> > > 
> > >   - Introduced pcie_cap_update_slot_link() as suggested by Michael.
> > > 
> > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> > > index ec25e7a7de99..50fbbacb2200 100644
> > > --- a/include/hw/pci/pcie.h
> > > +++ b/include/hw/pci/pcie.h
> > > @@ -112,6 +112,7 @@ void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slt_ctl, uint16_t *slt_sta);
> > >   void pcie_cap_slot_write_config(PCIDevice *dev,
> > >                                   uint16_t old_slt_ctl, uint16_t old_slt_sta,
> > >                                   uint32_t addr, uint32_t val, int len);
> > > +void pcie_cap_update_slot_link(PCIDevice *dev, uint16_t bctl);
> > >   int pcie_cap_slot_post_load(void *opaque, int version_id);
> > >   void pcie_cap_slot_push_attention_button(PCIDevice *dev);
> > >   void pcie_cap_slot_enable_power(PCIDevice *dev);
> > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> > > index e85932e41a92..a11cf2f20afa 100644
> > > --- a/hw/pci/pci_bridge.c
> > > +++ b/hw/pci/pci_bridge.c
> > > @@ -33,6 +33,7 @@
> > >   #include "qemu/units.h"
> > >   #include "hw/pci/pci_bridge.h"
> > >   #include "hw/pci/pci_bus.h"
> > > +#include "hw/pci/pcie.h"
> > >   #include "qemu/module.h"
> > >   #include "qemu/range.h"
> > >   #include "qapi/error.h"
> > > @@ -274,9 +275,19 @@ void pci_bridge_write_config(PCIDevice *d,
> > >       newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
> > >       if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
> > > -        /* Trigger hot reset on 0->1 transition. */
> > > +        /* SBR assertion (0->1): bring link down, then reset devices. */
> > > +        if (pci_is_express_downstream_port(d)) {
> > > +            pcie_cap_update_slot_link(d, newctl);
> > > +        }
> > >           bus_cold_reset(BUS(&s->sec_bus));
> > >       }
> > > +
> > > +    if (oldctl & ~newctl & PCI_BRIDGE_CTL_BUS_RESET) {
> > > +        /* SBR de-assertion (1->0): bring link back up. */
> > > +        if (pci_is_express_downstream_port(d)) {
> > > +            pcie_cap_update_slot_link(d, newctl);
> > > +        }
> > > +    }
> > >   }
> > >   void pci_bridge_disable_base_limit(PCIDevice *dev)
> > 
> > 
> > I'd rather call it once, unconditionally: it's idempotent.
> > 
> > Or if you really want:
> > 	if ((oldctl ^ newctl) & PCI_BRIDGE_CTL_BUS_RESET)
> 
> yes. Looks better. How's that ?
> 
> @@ -273,6 +274,12 @@ void pci_bridge_write_config(PCIDevice *
>      }
>      newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
> +    if ((oldctl ^ newctl) & PCI_BRIDGE_CTL_BUS_RESET) {
> +        if (pci_is_express_downstream_port(d)) {
> +            pcie_cap_update_slot_link(d, newctl);
> +        }
> +    }
> +
>      if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
>          /* Trigger hot reset on 0->1 transition. */
>          bus_cold_reset(BUS(&s->sec_bus));
> 
> 
> C.

LGTM

> > 
> > 
> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index bd00305ecca6..3b6dcd15aa44 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -803,6 +803,8 @@ void pcie_cap_slot_reset(PCIDevice *dev)
> > >                                    PCI_EXP_SLTSTA_PDC |
> > >                                    PCI_EXP_SLTSTA_ABP);
> > > +    pcie_cap_update_slot_link(dev,
> > > +                              pci_get_word(dev->config + PCI_BRIDGE_CONTROL));
> > >       pcie_cap_update_power(dev);
> > >       hotplug_event_update_event_status(dev);
> > >   }
> > > @@ -946,6 +948,24 @@ void pcie_cap_slot_push_attention_button(PCIDevice *dev)
> > >       pcie_cap_slot_event(dev, PCI_EXP_HP_EV_ABP);
> > >   }
> > > +/*
> > > + * Set DLLLA to reflect the actual link state: up iff a device is
> > > + * present on the secondary bus and SBR is not asserted.
> > > + */
> > > +void pcie_cap_update_slot_link(PCIDevice *dev, uint16_t bctl)
> > > +{
> > > +    PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(dev));
> > > +    uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
> > > +    uint32_t lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
> > > +    bool link_up = sec_bus->devices[0] && !(bctl & PCI_BRIDGE_CTL_BUS_RESET);
> > > +
> > > +    if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA ||
> > > +        (lnkcap & PCI_EXP_LNKCAP_DLLLARC)) {
> > > +        pci_set_word_by_mask(exp_cap + PCI_EXP_LNKSTA,
> > > +                             PCI_EXP_LNKSTA_DLLLA, link_up);
> > > +    }
> > > +}
> > > +
> > >   /* root control/capabilities/status. PME isn't emulated for now */
> > >   void pcie_cap_root_init(PCIDevice *dev)
> > >   {
> > > -- 
> > > 2.56.0
> > 



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-12 10:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-10  6:02 [PATCH for-12.0 v2] pcie: Model DLLLA transitions on Secondary Bus Reset Cédric Le Goater
2026-11-11 12:48 ` Michael S. Tsirkin
2026-11-12 10:28   ` Cédric Le Goater
2026-11-12 10:41     ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.