1. Patchew
  2. QEMU
  3. [PATCH 0/5] PCI: Implement basic PCI PM capability backing
  4. View patch

[PATCH 4/5] pcie, virtio: Remove redundant pm_cap

Alex Williamson posted 5 patches 2 days, 6 hours ago
[PATCH 4/5] pcie, virtio: Remove redundant pm_cap
Posted by Alex Williamson 2 days, 6 hours ago 
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
RE: [PATCH 4/5] pcie, virtio: Remove redundant pm_cap
Posted by Duan, Zhenzhong 1 day, 23 hours ago 

>-----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
Re: [PATCH 4/5] pcie, virtio: Remove redundant pm_cap
Posted by Cédric Le Goater 23 hours ago 
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
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.