[PATCH v2 02/19] hw/pci: Introduce pci_device_get_viommu_cap()

Zhenzhong Duan posted 19 patches 4 months, 4 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Yi Liu <yi.l.liu@intel.com>, "Clément Mathieu--Drif" <clement.mathieu--drif@eviden.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Eric Auger <eric.auger@redhat.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>
There is a newer version of this series
[PATCH v2 02/19] hw/pci: Introduce pci_device_get_viommu_cap()
Posted by Zhenzhong Duan 4 months, 4 weeks ago
pci_device_get_viommu_cap() call pci_device_get_iommu_bus_devfn()
to get iommu_bus->iommu_ops and call get_viommu_cap() callback to
get a bitmap with each bit represents a vIOMMU exposed capability.

Suggested-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/pci/pci.h | 22 ++++++++++++++++++++++
 hw/pci/pci.c         | 11 +++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index df3cc7b875..829757b2c2 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -453,6 +453,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_cap: get vIOMMU capabilities
+     *
+     * Optional callback, if not implemented, then vIOMMU doesn't
+     * support exposing capabilities to other subsystem, e.g., VFIO.
+     * vIOMMU can choose which capabilities to expose.
+     *
+     * @opaque: the data passed to pci_setup_iommu().
+     *
+     * Returns: 64bit bitmap with each bit represents a capability.
+     */
+    uint64_t (*get_viommu_cap)(void *opaque);
     /**
      * @get_iotlb_info: get properties required to initialize a device IOTLB.
      *
@@ -633,6 +645,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_cap: get vIOMMU capabilities.
+ *
+ * Returns a 64bit bitmap with each bit represents a vIOMMU exposed
+ * capability, 0 if vIOMMU doesn't support esposing capabilities.
+ *
+ * @dev: PCI device pointer.
+ */
+uint64_t pci_device_get_viommu_cap(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 c70b5ceeba..df1fb615a8 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2992,6 +2992,17 @@ void pci_device_unset_iommu_device(PCIDevice *dev)
     }
 }
 
+uint64_t pci_device_get_viommu_cap(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_cap) {
+        return iommu_bus->iommu_ops->get_viommu_cap(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)
-- 
2.34.1
Re: [PATCH v2 02/19] hw/pci: Introduce pci_device_get_viommu_cap()
Posted by Eric Auger 4 months, 4 weeks ago
Hi Zhenzhong,

On 6/20/25 9:17 AM, Zhenzhong Duan wrote:
> pci_device_get_viommu_cap() call pci_device_get_iommu_bus_devfn()
> to get iommu_bus->iommu_ops and call get_viommu_cap() callback to
> get a bitmap with each bit represents a vIOMMU exposed capability.
>
> Suggested-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  include/hw/pci/pci.h | 22 ++++++++++++++++++++++
>  hw/pci/pci.c         | 11 +++++++++++
>  2 files changed, 33 insertions(+)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index df3cc7b875..829757b2c2 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -453,6 +453,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_cap: get vIOMMU capabilities
> +     *
> +     * Optional callback, if not implemented, then vIOMMU doesn't
> +     * support exposing capabilities to other subsystem, e.g., VFIO.
> +     * vIOMMU can choose which capabilities to expose.
> +     *
> +     * @opaque: the data passed to pci_setup_iommu().
> +     *
> +     * Returns: 64bit bitmap with each bit represents a capability.
I think we need to clarify what this bitmap contains as capability bits
(enum type)

Thanks

Eric
> +     */
> +    uint64_t (*get_viommu_cap)(void *opaque);
>      /**
>       * @get_iotlb_info: get properties required to initialize a device IOTLB.
>       *
> @@ -633,6 +645,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_cap: get vIOMMU capabilities.
> + *
> + * Returns a 64bit bitmap with each bit represents a vIOMMU exposed
> + * capability, 0 if vIOMMU doesn't support esposing capabilities.
> + *
> + * @dev: PCI device pointer.
> + */
> +uint64_t pci_device_get_viommu_cap(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 c70b5ceeba..df1fb615a8 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2992,6 +2992,17 @@ void pci_device_unset_iommu_device(PCIDevice *dev)
>      }
>  }
>  
> +uint64_t pci_device_get_viommu_cap(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_cap) {
> +        return iommu_bus->iommu_ops->get_viommu_cap(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)
RE: [PATCH v2 02/19] hw/pci: Introduce pci_device_get_viommu_cap()
Posted by Duan, Zhenzhong 4 months, 3 weeks ago
Hi Eric,

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH v2 02/19] hw/pci: Introduce pci_device_get_viommu_cap()
>
>Hi Zhenzhong,
>
>On 6/20/25 9:17 AM, Zhenzhong Duan wrote:
>> pci_device_get_viommu_cap() call pci_device_get_iommu_bus_devfn()
>> to get iommu_bus->iommu_ops and call get_viommu_cap() callback to
>> get a bitmap with each bit represents a vIOMMU exposed capability.
>>
>> Suggested-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  include/hw/pci/pci.h | 22 ++++++++++++++++++++++
>>  hw/pci/pci.c         | 11 +++++++++++
>>  2 files changed, 33 insertions(+)
>>
>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>> index df3cc7b875..829757b2c2 100644
>> --- a/include/hw/pci/pci.h
>> +++ b/include/hw/pci/pci.h
>> @@ -453,6 +453,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_cap: get vIOMMU capabilities
>> +     *
>> +     * Optional callback, if not implemented, then vIOMMU doesn't
>> +     * support exposing capabilities to other subsystem, e.g., VFIO.
>> +     * vIOMMU can choose which capabilities to expose.
>> +     *
>> +     * @opaque: the data passed to pci_setup_iommu().
>> +     *
>> +     * Returns: 64bit bitmap with each bit represents a capability.
>I think we need to clarify what this bitmap contains as capability bits
>(enum type)

Sure, will be like:

     * Returns: 64bit bitmap with each bit represents a capability emulated
     * by VIOMMU_CAP_* in include/hw/iommu.h

enum {
    VIOMMU_CAP_STAGE1 = BIT_ULL(0),  /* stage1 page table supported */
};

Thanks
Zhenzhong
Re: [PATCH v2 02/19] hw/pci: Introduce pci_device_get_viommu_cap()
Posted by Eric Auger 4 months, 3 weeks ago
Hi Zhenzhong,

On 6/23/25 4:20 AM, Duan, Zhenzhong wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: Re: [PATCH v2 02/19] hw/pci: Introduce pci_device_get_viommu_cap()
>>
>> Hi Zhenzhong,
>>
>> On 6/20/25 9:17 AM, Zhenzhong Duan wrote:
>>> pci_device_get_viommu_cap() call pci_device_get_iommu_bus_devfn()
>>> to get iommu_bus->iommu_ops and call get_viommu_cap() callback to
>>> get a bitmap with each bit represents a vIOMMU exposed capability.
>>>
>>> Suggested-by: Yi Liu <yi.l.liu@intel.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>  include/hw/pci/pci.h | 22 ++++++++++++++++++++++
>>>  hw/pci/pci.c         | 11 +++++++++++
>>>  2 files changed, 33 insertions(+)
>>>
>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>>> index df3cc7b875..829757b2c2 100644
>>> --- a/include/hw/pci/pci.h
>>> +++ b/include/hw/pci/pci.h
>>> @@ -453,6 +453,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_cap: get vIOMMU capabilities
>>> +     *
>>> +     * Optional callback, if not implemented, then vIOMMU doesn't
>>> +     * support exposing capabilities to other subsystem, e.g., VFIO.
>>> +     * vIOMMU can choose which capabilities to expose.
>>> +     *
>>> +     * @opaque: the data passed to pci_setup_iommu().
>>> +     *
>>> +     * Returns: 64bit bitmap with each bit represents a capability.
>> I think we need to clarify what this bitmap contains as capability bits
>> (enum type)
> Sure, will be like:
>
>      * Returns: 64bit bitmap with each bit represents a capability emulated
>      * by VIOMMU_CAP_* in include/hw/iommu.h
>
> enum {
>     VIOMMU_CAP_STAGE1 = BIT_ULL(0),  /* stage1 page table supported */
> };
looks good to me

Eric
>
> Thanks
> Zhenzhong
>