The 2nd parameter of pci_device_get_iommu_bus_devfn() about root PCIBus
backed by an IOMMU for the PCI device, the 3rd is about aliased PCIBus
of the PCI device.
Meanwhile the 3rd and 4th parameters are optional, pass NULL if they
are not needed.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
hw/pci/pci.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index c3df9d6656..d5ed89aab7 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2967,7 +2967,7 @@ int pci_iommu_init_iotlb_notifier(PCIDevice *dev, IOMMUNotifier *n,
PCIBus *iommu_bus;
int devfn;
- pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
+ pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
if (iommu_bus && iommu_bus->iommu_ops->init_iotlb_notifier) {
iommu_bus->iommu_ops->init_iotlb_notifier(bus, iommu_bus->iommu_opaque,
devfn, n, fn, opaque);
@@ -3025,7 +3025,7 @@ int pci_pri_request_page(PCIDevice *dev, uint32_t pasid, bool priv_req,
return -EPERM;
}
- pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
+ pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
if (iommu_bus && iommu_bus->iommu_ops->pri_request_page) {
return iommu_bus->iommu_ops->pri_request_page(bus,
iommu_bus->iommu_opaque,
@@ -3049,7 +3049,7 @@ int pci_pri_register_notifier(PCIDevice *dev, uint32_t pasid,
return -EPERM;
}
- pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
+ pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
if (iommu_bus && iommu_bus->iommu_ops->pri_register_notifier) {
iommu_bus->iommu_ops->pri_register_notifier(bus,
iommu_bus->iommu_opaque,
@@ -3066,7 +3066,7 @@ void pci_pri_unregister_notifier(PCIDevice *dev, uint32_t pasid)
PCIBus *iommu_bus;
int devfn;
- pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
+ pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
if (iommu_bus && iommu_bus->iommu_ops->pri_unregister_notifier) {
iommu_bus->iommu_ops->pri_unregister_notifier(bus,
iommu_bus->iommu_opaque,
@@ -3098,7 +3098,7 @@ ssize_t pci_ats_request_translation(PCIDevice *dev, uint32_t pasid,
return -EPERM;
}
- pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
+ pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
if (iommu_bus && iommu_bus->iommu_ops->ats_request_translation) {
return iommu_bus->iommu_ops->ats_request_translation(bus,
iommu_bus->iommu_opaque,
@@ -3122,7 +3122,7 @@ int pci_iommu_register_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
return -EPERM;
}
- pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
+ pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
if (iommu_bus && iommu_bus->iommu_ops->register_iotlb_notifier) {
iommu_bus->iommu_ops->register_iotlb_notifier(bus,
iommu_bus->iommu_opaque, devfn,
@@ -3144,7 +3144,7 @@ int pci_iommu_unregister_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
return -EPERM;
}
- pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
+ pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
if (iommu_bus && iommu_bus->iommu_ops->unregister_iotlb_notifier) {
iommu_bus->iommu_ops->unregister_iotlb_notifier(bus,
iommu_bus->iommu_opaque,
@@ -3158,11 +3158,9 @@ int pci_iommu_unregister_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
int pci_iommu_get_iotlb_info(PCIDevice *dev, uint8_t *addr_width,
uint32_t *min_page_size)
{
- PCIBus *bus;
PCIBus *iommu_bus;
- int devfn;
- pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
+ pci_device_get_iommu_bus_devfn(dev, &iommu_bus, NULL, NULL);
if (iommu_bus && iommu_bus->iommu_ops->get_iotlb_info) {
iommu_bus->iommu_ops->get_iotlb_info(iommu_bus->iommu_opaque,
addr_width, min_page_size);
--
2.47.1On 9/29/25 05:42, Zhenzhong Duan wrote:
> The 2nd parameter of pci_device_get_iommu_bus_devfn() about root PCIBus
> backed by an IOMMU for the PCI device, the 3rd is about aliased PCIBus
> of the PCI device.
>
> Meanwhile the 3rd and 4th parameters are optional, pass NULL if they
> are not needed.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
The commit log should mention potential consequences of this change.
Will this fix need to be backported ? up to ~9.1
Thanks,
C.
> ---
> hw/pci/pci.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index c3df9d6656..d5ed89aab7 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2967,7 +2967,7 @@ int pci_iommu_init_iotlb_notifier(PCIDevice *dev, IOMMUNotifier *n,
> PCIBus *iommu_bus;
> int devfn;
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->init_iotlb_notifier) {
> iommu_bus->iommu_ops->init_iotlb_notifier(bus, iommu_bus->iommu_opaque,
> devfn, n, fn, opaque);
> @@ -3025,7 +3025,7 @@ int pci_pri_request_page(PCIDevice *dev, uint32_t pasid, bool priv_req,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->pri_request_page) {
> return iommu_bus->iommu_ops->pri_request_page(bus,
> iommu_bus->iommu_opaque,
> @@ -3049,7 +3049,7 @@ int pci_pri_register_notifier(PCIDevice *dev, uint32_t pasid,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->pri_register_notifier) {
> iommu_bus->iommu_ops->pri_register_notifier(bus,
> iommu_bus->iommu_opaque,
> @@ -3066,7 +3066,7 @@ void pci_pri_unregister_notifier(PCIDevice *dev, uint32_t pasid)
> PCIBus *iommu_bus;
> int devfn;
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->pri_unregister_notifier) {
> iommu_bus->iommu_ops->pri_unregister_notifier(bus,
> iommu_bus->iommu_opaque,
> @@ -3098,7 +3098,7 @@ ssize_t pci_ats_request_translation(PCIDevice *dev, uint32_t pasid,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->ats_request_translation) {
> return iommu_bus->iommu_ops->ats_request_translation(bus,
> iommu_bus->iommu_opaque,
> @@ -3122,7 +3122,7 @@ int pci_iommu_register_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->register_iotlb_notifier) {
> iommu_bus->iommu_ops->register_iotlb_notifier(bus,
> iommu_bus->iommu_opaque, devfn,
> @@ -3144,7 +3144,7 @@ int pci_iommu_unregister_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->unregister_iotlb_notifier) {
> iommu_bus->iommu_ops->unregister_iotlb_notifier(bus,
> iommu_bus->iommu_opaque,
> @@ -3158,11 +3158,9 @@ int pci_iommu_unregister_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
> int pci_iommu_get_iotlb_info(PCIDevice *dev, uint8_t *addr_width,
> uint32_t *min_page_size)
> {
> - PCIBus *bus;
> PCIBus *iommu_bus;
> - int devfn;
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, NULL, NULL);
> if (iommu_bus && iommu_bus->iommu_ops->get_iotlb_info) {
> iommu_bus->iommu_ops->get_iotlb_info(iommu_bus->iommu_opaque,
> addr_width, min_page_size);
>-----Original Message----- >From: Cédric Le Goater <clg@redhat.com> >Subject: Re: [PATCH v2 3/3] pci: Fix wrong parameter passing to >pci_device_get_iommu_bus_devfn() > >On 9/29/25 05:42, Zhenzhong Duan wrote: >> The 2nd parameter of pci_device_get_iommu_bus_devfn() about root >PCIBus >> backed by an IOMMU for the PCI device, the 3rd is about aliased PCIBus >> of the PCI device. >> >> Meanwhile the 3rd and 4th parameters are optional, pass NULL if they >> are not needed. >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > >The commit log should mention potential consequences of this change. Without fix, the callback function may not be called as iommu_ops is set for iommu_bus rather than bus. Luckily, there is no user of those functions yet, so no real issue currently. > >Will this fix need to be backported ? up to ~9.1 I think no need, Clement could correct me if above explanation is wrong. Thanks Zhenzhong
On Wed, 2025-10-08 at 10:15 +0000, Duan, Zhenzhong wrote: > > > > > -----Original Message----- > > From: Cédric Le Goater <[clg@redhat.com](mailto:clg@redhat.com)> > > Subject: Re: [PATCH v2 3/3] pci: Fix wrong parameter passing to > > pci_device_get_iommu_bus_devfn() > > > > On 9/29/25 05:42, Zhenzhong Duan wrote: > > > > > The 2nd parameter of pci_device_get_iommu_bus_devfn() about root > > > > PCIBus > > > > > backed by an IOMMU for the PCI device, the 3rd is about aliased PCIBus > > > of the PCI device. > > > > > > Meanwhile the 3rd and 4th parameters are optional, pass NULL if they > > > are not needed. > > > > > > Signed-off-by: Zhenzhong Duan <[zhenzhong.duan@intel.com](mailto:zhenzhong.duan@intel.com)> > > > > > > The commit log should mention potential consequences of this change. > > > Without fix, the callback function may not be called as iommu_ops is set > for iommu_bus rather than bus. Luckily, there is no user of those > functions yet, so no real issue currently. > > > > > Will this fix need to be backported ? up to ~9.1 > > > I think no need, Clement could correct me if above explanation is wrong. That's correct, thanks Zhenzhong > > Thanks > Zhenzhong >
Hi Zhenzhong
Uh, good catch O.o
Reviewed-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
On Sun, 2025-09-28 at 23:42 -0400, Zhenzhong Duan wrote:
> The 2nd parameter of pci_device_get_iommu_bus_devfn() about root PCIBus
> backed by an IOMMU for the PCI device, the 3rd is about aliased PCIBus
> of the PCI device.
>
> Meanwhile the 3rd and 4th parameters are optional, pass NULL if they
> are not needed.
>
> Signed-off-by: Zhenzhong Duan <[zhenzhong.duan@intel.com](mailto:zhenzhong.duan@intel.com)>
> ---
> hw/pci/pci.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index c3df9d6656..d5ed89aab7 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2967,7 +2967,7 @@ int pci_iommu_init_iotlb_notifier(PCIDevice *dev, IOMMUNotifier *n,
> PCIBus *iommu_bus;
> int devfn;
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->init_iotlb_notifier) {
> iommu_bus->iommu_ops->init_iotlb_notifier(bus, iommu_bus->iommu_opaque,
> devfn, n, fn, opaque);
> @@ -3025,7 +3025,7 @@ int pci_pri_request_page(PCIDevice *dev, uint32_t pasid, bool priv_req,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->pri_request_page) {
> return iommu_bus->iommu_ops->pri_request_page(bus,
> iommu_bus->iommu_opaque,
> @@ -3049,7 +3049,7 @@ int pci_pri_register_notifier(PCIDevice *dev, uint32_t pasid,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->pri_register_notifier) {
> iommu_bus->iommu_ops->pri_register_notifier(bus,
> iommu_bus->iommu_opaque,
> @@ -3066,7 +3066,7 @@ void pci_pri_unregister_notifier(PCIDevice *dev, uint32_t pasid)
> PCIBus *iommu_bus;
> int devfn;
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->pri_unregister_notifier) {
> iommu_bus->iommu_ops->pri_unregister_notifier(bus,
> iommu_bus->iommu_opaque,
> @@ -3098,7 +3098,7 @@ ssize_t pci_ats_request_translation(PCIDevice *dev, uint32_t pasid,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->ats_request_translation) {
> return iommu_bus->iommu_ops->ats_request_translation(bus,
> iommu_bus->iommu_opaque,
> @@ -3122,7 +3122,7 @@ int pci_iommu_register_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->register_iotlb_notifier) {
> iommu_bus->iommu_ops->register_iotlb_notifier(bus,
> iommu_bus->iommu_opaque, devfn,
> @@ -3144,7 +3144,7 @@ int pci_iommu_unregister_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
> return -EPERM;
> }
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, &bus, &devfn);
> if (iommu_bus && iommu_bus->iommu_ops->unregister_iotlb_notifier) {
> iommu_bus->iommu_ops->unregister_iotlb_notifier(bus,
> iommu_bus->iommu_opaque,
> @@ -3158,11 +3158,9 @@ int pci_iommu_unregister_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
> int pci_iommu_get_iotlb_info(PCIDevice *dev, uint8_t *addr_width,
> uint32_t *min_page_size)
> {
> - PCIBus *bus;
> PCIBus *iommu_bus;
> - int devfn;
>
> - pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn);
> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, NULL, NULL);
> if (iommu_bus && iommu_bus->iommu_ops->get_iotlb_info) {
> iommu_bus->iommu_ops->get_iotlb_info(iommu_bus->iommu_opaque,
> addr_width, min_page_size);
© 2016 - 2025 Red Hat, Inc.