>-----Original Message----- >From: Cédric Le Goater <[email protected]> >Subject: Re: [PATCH v7 05/23] hw/pci: Introduce >pci_device_get_viommu_flags() > >On 10/24/25 10:43, Zhenzhong Duan wrote: >> Introduce a new PCIIOMMUOps optional callback, get_viommu_flags() >which >> allows to retrieve flags exposed by a vIOMMU. The first planned vIOMMU >> device flag is VIOMMU_FLAG_WANT_NESTING_PARENT that advertises the >> support of HW nested stage translation scheme and wants other sub-system >> like VFIO's cooperation to create nesting parent HWPT. >> >> pci_device_get_viommu_flags() is a wrapper that can be called on a PCI >> device potentially protected by a vIOMMU. >> >> get_viommu_flags() is designed to return 64bit bitmap of purely vIOMMU >> flags which are only determined by user's configuration, no host >> capabilities involved. Reasons are: >> >> 1. host may has heterogeneous IOMMUs, each with different capabilities >> 2. this is migration friendly, return value is consistent between source >> and target. >> >> Note that this op will be invoked at the attach_device() stage, at which >> point host IOMMU capabilities are not yet forwarded to the vIOMMU >through >> the set_iommu_device() callback that will be after the attach_device(). >> >> See below sequence: >> >> vfio_device_attach(): >> iommufd_cdev_attach(): >> pci_device_get_viommu_flags() for HW nesting cap >> create a nesting parent HWPT >> attach device to the HWPT >> vfio_device_hiod_create_and_realize() creating hiod >> ... >> pci_device_set_iommu_device(hiod) >> >> Suggested-by: Yi Liu <[email protected]> >> Signed-off-by: Zhenzhong Duan <[email protected]> >> Reviewed-by: Nicolin Chen <[email protected]> >> Reviewed-by: Eric Auger <[email protected]> >> Reviewed-by: Yi Liu <[email protected]> >> --- >> MAINTAINERS | 1 + >> include/hw/iommu.h | 25 +++++++++++++++++++++++++ > > >Hmm, why not under include/hw/pci/ ?
I'm not sure if it's better to restrict iommu to pci subsystem. I have a vague memory there is iommu supporting non-PCI devices. > Was this discussed ? No. Thanks Zhenzhong > > >Reviewed-by: Cédric Le Goater <[email protected]> > >Thanks, > >C. > > > >> include/hw/pci/pci.h | 22 ++++++++++++++++++++++ >> hw/pci/pci.c | 11 +++++++++++ >> 4 files changed, 59 insertions(+) >> create mode 100644 include/hw/iommu.h >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 36eef27b41..d94fbcbdfb 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -2338,6 +2338,7 @@ F: include/system/iommufd.h >> F: backends/host_iommu_device.c >> F: include/system/host_iommu_device.h >> F: include/qemu/chardev_open.h >> +F: include/hw/iommu.h >> F: util/chardev_open.c >> F: docs/devel/vfio-iommufd.rst >> >> diff --git a/include/hw/iommu.h b/include/hw/iommu.h >> new file mode 100644 >> index 0000000000..9b8bb94fc2 >> --- /dev/null >> +++ b/include/hw/iommu.h >> @@ -0,0 +1,25 @@ >> +/* >> + * General vIOMMU flags >> + * >> + * Copyright (C) 2025 Intel Corporation. >> + * >> + * SPDX-License-Identifier: GPL-2.0-or-later >> + */ >> + >> +#ifndef HW_IOMMU_H >> +#define HW_IOMMU_H >> + >> +#include "qemu/bitops.h" >> + >> +/* >> + * Theoretical vIOMMU flags. Only determined by the vIOMMU device >properties and >> + * independent on the actual host IOMMU capabilities they may depend on. >Each >> + * flag can be an expectation or request to other sub-system or just a pure >> + * vIOMMU capability. vIOMMU can choose which flags to expose. >> + */ >> +enum viommu_flags { >> + /* vIOMMU needs nesting parent HWPT to create nested HWPT */ >> + VIOMMU_FLAG_WANT_NESTING_PARENT = BIT_ULL(0), >> +}; >> + >> +#endif /* HW_IOMMU_H */ >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index bde9dca8e2..cf99b5bb68 100644 >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -462,6 +462,18 @@ typedef struct PCIIOMMUOps { >> * @devfn: device and function number of the PCI device. >> */ >> void (*unset_iommu_device)(PCIBus *bus, void *opaque, int devfn); >> + /** >> + * @get_viommu_flags: get vIOMMU flags >> + * >> + * Optional callback, if not implemented, then vIOMMU doesn't >support >> + * exposing flags to other sub-system, e.g., VFIO. >> + * >> + * @opaque: the data passed to pci_setup_iommu(). >> + * >> + * Returns: bitmap with each representing a vIOMMU flag defined in >> + * enum viommu_flags. >> + */ >> + uint64_t (*get_viommu_flags)(void *opaque); >> /** >> * @get_iotlb_info: get properties required to initialize a device >IOTLB. >> * >> @@ -644,6 +656,16 @@ bool pci_device_set_iommu_device(PCIDevice >*dev, HostIOMMUDevice *hiod, >> Error **errp); >> void pci_device_unset_iommu_device(PCIDevice *dev); >> >> +/** >> + * pci_device_get_viommu_flags: get vIOMMU flags. >> + * >> + * Returns: bitmap with each representing a vIOMMU flag defined in >> + * enum viommu_flags. Or 0 if vIOMMU doesn't report any. >> + * >> + * @dev: PCI device pointer. >> + */ >> +uint64_t pci_device_get_viommu_flags(PCIDevice *dev); >> + >> /** >> * pci_iommu_get_iotlb_info: get properties required to initialize a >> * device IOTLB. >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index d0e81651aa..c9932c87e3 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -3010,6 +3010,17 @@ void >pci_device_unset_iommu_device(PCIDevice *dev) >> } >> } >> >> +uint64_t pci_device_get_viommu_flags(PCIDevice *dev) >> +{ >> + PCIBus *iommu_bus; >> + >> + pci_device_get_iommu_bus_devfn(dev, &iommu_bus, NULL, NULL); >> + if (iommu_bus && iommu_bus->iommu_ops->get_viommu_flags) { >> + return >iommu_bus->iommu_ops->get_viommu_flags(iommu_bus->iommu_opaque); >> + } >> + return 0; >> +} >> + >> int pci_pri_request_page(PCIDevice *dev, uint32_t pasid, bool priv_req, >> bool exec_req, hwaddr addr, bool lpig, >> uint16_t prgi, bool is_read, bool is_write)
