| Message ID | 20250220224918.2520417-5-alex.williamson@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Implement basic PCI PM capability backing | expand |
>-----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 >
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.
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(-)