The pm_cap on the PCIExpressDevice object can be distilled down
to the new instance on the PCIDevice object.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci-bridge/pcie_pci_bridge.c | 1 -
 hw/virtio/virtio-pci.c          | 8 +++-----
 include/hw/pci/pcie.h           | 2 --
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index 9fa656b43b42..2429503cfbbf 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -56,7 +56,6 @@ static void pcie_pci_bridge_realize(PCIDevice *d, Error **errp)
     if (pos < 0) {
         goto pm_error;
     }
-    d->exp.pm_cap = pos;
     pci_set_word(d->config + pos + PCI_PM_PMC, 0x3);
 
     pcie_cap_arifwd_init(d);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index afe8b5551c5c..3ca3f849d391 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2209,8 +2209,6 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
             return;
         }
 
-        pci_dev->exp.pm_cap = pos;
-
         /*
          * Indicates that this function complies with revision 1.2 of the
          * PCI Power Management Interface Specification.
@@ -2309,11 +2307,11 @@ static bool virtio_pci_no_soft_reset(PCIDevice *dev)
 {
     uint16_t pmcsr;
 
-    if (!pci_is_express(dev) || !dev->exp.pm_cap) {
+    if (!pci_is_express(dev) || !(dev->cap_present & QEMU_PCI_CAP_PM)) {
         return false;
     }
 
-    pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
+    pmcsr = pci_get_word(dev->config + dev->pm_cap + PCI_PM_CTRL);
 
     /*
      * When No_Soft_Reset bit is set and the device
@@ -2342,7 +2340,7 @@ static void virtio_pci_bus_reset_hold(Object *obj, ResetType type)
 
         if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
             pci_word_test_and_clear_mask(
-                dev->config + dev->exp.pm_cap + PCI_PM_CTRL,
+                dev->config + dev->pm_cap + PCI_PM_CTRL,
                 PCI_PM_CTRL_STATE_MASK);
         }
     }
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index b8d59732bc63..70a5de09de39 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -58,8 +58,6 @@ typedef enum {
 struct PCIExpressDevice {
     /* Offset of express capability in config space */
     uint8_t exp_cap;
-    /* Offset of Power Management capability in config space */
-    uint8_t pm_cap;
 
     /* SLOT */
     bool hpev_notified; /* Logical AND of conditions for hot plug event.
-- 
2.48.1

On 2/20/25 11:48 PM, Alex Williamson wrote:
> The pm_cap on the PCIExpressDevice object can be distilled down
> to the new instance on the PCIDevice object.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> ---
>  hw/pci-bridge/pcie_pci_bridge.c | 1 -
>  hw/virtio/virtio-pci.c          | 8 +++-----
>  include/hw/pci/pcie.h           | 2 --
>  3 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
> index 9fa656b43b42..2429503cfbbf 100644
> --- a/hw/pci-bridge/pcie_pci_bridge.c
> +++ b/hw/pci-bridge/pcie_pci_bridge.c
> @@ -56,7 +56,6 @@ static void pcie_pci_bridge_realize(PCIDevice *d, Error **errp)
>      if (pos < 0) {
>          goto pm_error;
>      }
> -    d->exp.pm_cap = pos;
>      pci_set_word(d->config + pos + PCI_PM_PMC, 0x3);
>  
>      pcie_cap_arifwd_init(d);
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index afe8b5551c5c..3ca3f849d391 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2209,8 +2209,6 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
>              return;
>          }
>  
> -        pci_dev->exp.pm_cap = pos;
> -
>          /*
>           * Indicates that this function complies with revision 1.2 of the
>           * PCI Power Management Interface Specification.
> @@ -2309,11 +2307,11 @@ static bool virtio_pci_no_soft_reset(PCIDevice *dev)
>  {
>      uint16_t pmcsr;
>  
> -    if (!pci_is_express(dev) || !dev->exp.pm_cap) {
> +    if (!pci_is_express(dev) || !(dev->cap_present & QEMU_PCI_CAP_PM)) {
>          return false;
>      }
>  
> -    pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
> +    pmcsr = pci_get_word(dev->config + dev->pm_cap + PCI_PM_CTRL);
>  
>      /*
>       * When No_Soft_Reset bit is set and the device
> @@ -2342,7 +2340,7 @@ static void virtio_pci_bus_reset_hold(Object *obj, ResetType type)
>  
>          if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
>              pci_word_test_and_clear_mask(
> -                dev->config + dev->exp.pm_cap + PCI_PM_CTRL,
> +                dev->config + dev->pm_cap + PCI_PM_CTRL,
>                  PCI_PM_CTRL_STATE_MASK);
>          }
>      }
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index b8d59732bc63..70a5de09de39 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -58,8 +58,6 @@ typedef enum {
>  struct PCIExpressDevice {
>      /* Offset of express capability in config space */
>      uint8_t exp_cap;
> -    /* Offset of Power Management capability in config space */
> -    uint8_t pm_cap;
>  
>      /* SLOT */
>      bool hpev_notified; /* Logical AND of conditions for hot plug event.

>-----Original Message-----
>From: Alex Williamson <alex.williamson@redhat.com>
>Subject: [PATCH 4/5] pcie, virtio: Remove redundant pm_cap
>
>The pm_cap on the PCIExpressDevice object can be distilled down
>to the new instance on the PCIDevice object.
>
>Cc: Michael S. Tsirkin <mst@redhat.com>
>Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>---
> hw/pci-bridge/pcie_pci_bridge.c | 1 -
> hw/virtio/virtio-pci.c          | 8 +++-----
> include/hw/pci/pcie.h           | 2 --
> 3 files changed, 3 insertions(+), 8 deletions(-)
>
>diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
>index 9fa656b43b42..2429503cfbbf 100644
>--- a/hw/pci-bridge/pcie_pci_bridge.c
>+++ b/hw/pci-bridge/pcie_pci_bridge.c
>@@ -56,7 +56,6 @@ static void pcie_pci_bridge_realize(PCIDevice *d, Error
>**errp)
>     if (pos < 0) {
>         goto pm_error;
>     }
>-    d->exp.pm_cap = pos;
>     pci_set_word(d->config + pos + PCI_PM_PMC, 0x3);
>
>     pcie_cap_arifwd_init(d);
>diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>index afe8b5551c5c..3ca3f849d391 100644
>--- a/hw/virtio/virtio-pci.c
>+++ b/hw/virtio/virtio-pci.c
>@@ -2209,8 +2209,6 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error
>**errp)
>             return;
>         }
>
>-        pci_dev->exp.pm_cap = pos;
>-
>         /*
>          * Indicates that this function complies with revision 1.2 of the
>          * PCI Power Management Interface Specification.
>@@ -2309,11 +2307,11 @@ static bool virtio_pci_no_soft_reset(PCIDevice *dev)
> {
>     uint16_t pmcsr;
>
>-    if (!pci_is_express(dev) || !dev->exp.pm_cap) {
>+    if (!pci_is_express(dev) || !(dev->cap_present & QEMU_PCI_CAP_PM)) {

Maybe a bit more optimized by checking dev->pm_cap,
but it's also ok checking present bit. For the whole series,

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>         return false;
>     }
>
>-    pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
>+    pmcsr = pci_get_word(dev->config + dev->pm_cap + PCI_PM_CTRL);
>
>     /*
>      * When No_Soft_Reset bit is set and the device
>@@ -2342,7 +2340,7 @@ static void virtio_pci_bus_reset_hold(Object *obj,
>ResetType type)
>
>         if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
>             pci_word_test_and_clear_mask(
>-                dev->config + dev->exp.pm_cap + PCI_PM_CTRL,
>+                dev->config + dev->pm_cap + PCI_PM_CTRL,
>                 PCI_PM_CTRL_STATE_MASK);
>         }
>     }
>diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
>index b8d59732bc63..70a5de09de39 100644
>--- a/include/hw/pci/pcie.h
>+++ b/include/hw/pci/pcie.h
>@@ -58,8 +58,6 @@ typedef enum {
> struct PCIExpressDevice {
>     /* Offset of express capability in config space */
>     uint8_t exp_cap;
>-    /* Offset of Power Management capability in config space */
>-    uint8_t pm_cap;
>
>     /* SLOT */
>     bool hpev_notified; /* Logical AND of conditions for hot plug event.
>--
>2.48.1

Hello Zhenzhong,

On 2/21/25 07:12, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Alex Williamson <alex.williamson@redhat.com>
>> Subject: [PATCH 4/5] pcie, virtio: Remove redundant pm_cap
>>
>> The pm_cap on the PCIExpressDevice object can be distilled down
>> to the new instance on the PCIDevice object.
>>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>> ---
>> hw/pci-bridge/pcie_pci_bridge.c | 1 -
>> hw/virtio/virtio-pci.c          | 8 +++-----
>> include/hw/pci/pcie.h           | 2 --
>> 3 files changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
>> index 9fa656b43b42..2429503cfbbf 100644
>> --- a/hw/pci-bridge/pcie_pci_bridge.c
>> +++ b/hw/pci-bridge/pcie_pci_bridge.c
>> @@ -56,7 +56,6 @@ static void pcie_pci_bridge_realize(PCIDevice *d, Error
>> **errp)
>>      if (pos < 0) {
>>          goto pm_error;
>>      }
>> -    d->exp.pm_cap = pos;
>>      pci_set_word(d->config + pos + PCI_PM_PMC, 0x3);
>>
>>      pcie_cap_arifwd_init(d);
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index afe8b5551c5c..3ca3f849d391 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -2209,8 +2209,6 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error
>> **errp)
>>              return;
>>          }
>>
>> -        pci_dev->exp.pm_cap = pos;
>> -
>>          /*
>>           * Indicates that this function complies with revision 1.2 of the
>>           * PCI Power Management Interface Specification.
>> @@ -2309,11 +2307,11 @@ static bool virtio_pci_no_soft_reset(PCIDevice *dev)
>> {
>>      uint16_t pmcsr;
>>
>> -    if (!pci_is_express(dev) || !dev->exp.pm_cap) {
>> +    if (!pci_is_express(dev) || !(dev->cap_present & QEMU_PCI_CAP_PM)) {
> 
> Maybe a bit more optimized by checking dev->pm_cap,
> but it's also ok checking present bit. For the whole series,
> 
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Could you please reply to the cover letter instead ? Tools like b4
will apply the provided trailers to the whole series and not this
patch only.


Thanks,

C.




> 
> Thanks
> Zhenzhong
> 
>>          return false;
>>      }
>>
>> -    pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
>> +    pmcsr = pci_get_word(dev->config + dev->pm_cap + PCI_PM_CTRL);
>>
>>      /*
>>       * When No_Soft_Reset bit is set and the device
>> @@ -2342,7 +2340,7 @@ static void virtio_pci_bus_reset_hold(Object *obj,
>> ResetType type)
>>
>>          if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
>>              pci_word_test_and_clear_mask(
>> -                dev->config + dev->exp.pm_cap + PCI_PM_CTRL,
>> +                dev->config + dev->pm_cap + PCI_PM_CTRL,
>>                  PCI_PM_CTRL_STATE_MASK);
>>          }
>>      }
>> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
>> index b8d59732bc63..70a5de09de39 100644
>> --- a/include/hw/pci/pcie.h
>> +++ b/include/hw/pci/pcie.h
>> @@ -58,8 +58,6 @@ typedef enum {
>> struct PCIExpressDevice {
>>      /* Offset of express capability in config space */
>>      uint8_t exp_cap;
>> -    /* Offset of Power Management capability in config space */
>> -    uint8_t pm_cap;
>>
>>      /* SLOT */
>>      bool hpev_notified; /* Logical AND of conditions for hot plug event.
>> --
>> 2.48.1
>

Hi Cédric,

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH 4/5] pcie, virtio: Remove redundant pm_cap
>
>Hello Zhenzhong,
>
>On 2/21/25 07:12, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Alex Williamson <alex.williamson@redhat.com>
>>> Subject: [PATCH 4/5] pcie, virtio: Remove redundant pm_cap
>>>
>>> The pm_cap on the PCIExpressDevice object can be distilled down
>>> to the new instance on the PCIDevice object.
>>>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>>> ---
>>> hw/pci-bridge/pcie_pci_bridge.c | 1 -
>>> hw/virtio/virtio-pci.c          | 8 +++-----
>>> include/hw/pci/pcie.h           | 2 --
>>> 3 files changed, 3 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
>>> index 9fa656b43b42..2429503cfbbf 100644
>>> --- a/hw/pci-bridge/pcie_pci_bridge.c
>>> +++ b/hw/pci-bridge/pcie_pci_bridge.c
>>> @@ -56,7 +56,6 @@ static void pcie_pci_bridge_realize(PCIDevice *d, Error
>>> **errp)
>>>      if (pos < 0) {
>>>          goto pm_error;
>>>      }
>>> -    d->exp.pm_cap = pos;
>>>      pci_set_word(d->config + pos + PCI_PM_PMC, 0x3);
>>>
>>>      pcie_cap_arifwd_init(d);
>>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>>> index afe8b5551c5c..3ca3f849d391 100644
>>> --- a/hw/virtio/virtio-pci.c
>>> +++ b/hw/virtio/virtio-pci.c
>>> @@ -2209,8 +2209,6 @@ static void virtio_pci_realize(PCIDevice *pci_dev,
>Error
>>> **errp)
>>>              return;
>>>          }
>>>
>>> -        pci_dev->exp.pm_cap = pos;
>>> -
>>>          /*
>>>           * Indicates that this function complies with revision 1.2 of the
>>>           * PCI Power Management Interface Specification.
>>> @@ -2309,11 +2307,11 @@ static bool virtio_pci_no_soft_reset(PCIDevice
>*dev)
>>> {
>>>      uint16_t pmcsr;
>>>
>>> -    if (!pci_is_express(dev) || !dev->exp.pm_cap) {
>>> +    if (!pci_is_express(dev) || !(dev->cap_present & QEMU_PCI_CAP_PM)) {
>>
>> Maybe a bit more optimized by checking dev->pm_cap,
>> but it's also ok checking present bit. For the whole series,
>>
>> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>
>Could you please reply to the cover letter instead ? Tools like b4
>will apply the provided trailers to the whole series and not this
>patch only.

Got it, just done.

Thanks
Zhenzhong