[v6] Add a host IOMMU device abstraction to check with vIOMMU

[PATCH v6 19/19] intel_iommu: Check compatibility with host IOMMU capabilities

Posted by Zhenzhong Duan 5 months, 3 weeks ago

If check fails, host device (either VFIO or VDPA device) is not
compatible with current vIOMMU config and should not be passed to
guest.

Only aw_bits is checked for now, we don't care other capabilities
before scalable modern mode is introduced.

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 747c988bc4..d8202a77dd 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3819,6 +3819,30 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
     return vtd_dev_as;
 }
 
+static bool vtd_check_hdev(IntelIOMMUState *s, HostIOMMUDevice *hiod,
+                           Error **errp)
+{
+    HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
+    int ret;
+
+    if (!hiodc->get_cap) {
+        error_setg(errp, ".get_cap() not implemented");
+        return false;
+    }
+
+    /* Common checks */
+    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_AW_BITS, errp);
+    if (ret < 0) {
+        return false;
+    }
+    if (s->aw_bits > ret) {
+        error_setg(errp, "aw-bits %d > host aw-bits %d", s->aw_bits, ret);
+        return false;
+    }
+
+    return true;
+}
+
 static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
                                      HostIOMMUDevice *hiod, Error **errp)
 {
@@ -3842,6 +3866,11 @@ static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
         return false;
     }
 
+    if (!vtd_check_hdev(s, hiod, errp)) {
+        vtd_iommu_unlock(s);
+        return false;
+    }
+
     vtd_hdev = g_malloc0(sizeof(VTDHostIOMMUDevice));
     vtd_hdev->bus = bus;
     vtd_hdev->devfn = (uint8_t)devfn;
-- 
2.34.1

Re: [PATCH v6 19/19] intel_iommu: Check compatibility with host IOMMU capabilities

Posted by Eric Auger 5 months, 3 weeks ago

Hi Zhenzhong,

On 6/3/24 08:10, Zhenzhong Duan wrote:
> If check fails, host device (either VFIO or VDPA device) is not
> compatible with current vIOMMU config and should not be passed to
> guest.
>
> Only aw_bits is checked for now, we don't care other capabilities
we don't care about other caps
> before scalable modern mode is introduced.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  hw/i386/intel_iommu.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 747c988bc4..d8202a77dd 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3819,6 +3819,30 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>      return vtd_dev_as;
>  }
>  
> +static bool vtd_check_hdev(IntelIOMMUState *s, HostIOMMUDevice *hiod,
> +                           Error **errp)
> +{
> +    HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
> +    int ret;
> +
> +    if (!hiodc->get_cap) {
> +        error_setg(errp, ".get_cap() not implemented");
> +        return false;
> +    }
> +
> +    /* Common checks */
> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_AW_BITS, errp);
> +    if (ret < 0) {
> +        return false;
> +    }
> +    if (s->aw_bits > ret) {
> +        error_setg(errp, "aw-bits %d > host aw-bits %d", s->aw_bits, ret);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>                                       HostIOMMUDevice *hiod, Error **errp)
>  {
> @@ -3842,6 +3866,11 @@ static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>          return false;
>      }
>  
> +    if (!vtd_check_hdev(s, hiod, errp)) {
> +        vtd_iommu_unlock(s);
> +        return false;
> +    }
> +
>      vtd_hdev = g_malloc0(sizeof(VTDHostIOMMUDevice));
>      vtd_hdev->bus = bus;
>      vtd_hdev->devfn = (uint8_t)devfn;
Eric

RE: [PATCH v6 19/19] intel_iommu: Check compatibility with host IOMMU capabilities

Posted by Duan, Zhenzhong 5 months, 3 weeks ago


>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH v6 19/19] intel_iommu: Check compatibility with host
>IOMMU capabilities
>
>Hi Zhenzhong,
>
>On 6/3/24 08:10, Zhenzhong Duan wrote:
>> If check fails, host device (either VFIO or VDPA device) is not
>> compatible with current vIOMMU config and should not be passed to
>> guest.
>>
>> Only aw_bits is checked for now, we don't care other capabilities
>we don't care about other caps

Will do.

Thanks
Zhenzhong

>> before scalable modern mode is introduced.
>>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  hw/i386/intel_iommu.c | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 747c988bc4..d8202a77dd 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3819,6 +3819,30 @@ VTDAddressSpace
>*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>>      return vtd_dev_as;
>>  }
>>
>> +static bool vtd_check_hdev(IntelIOMMUState *s, HostIOMMUDevice
>*hiod,
>> +                           Error **errp)
>> +{
>> +    HostIOMMUDeviceClass *hiodc =
>HOST_IOMMU_DEVICE_GET_CLASS(hiod);
>> +    int ret;
>> +
>> +    if (!hiodc->get_cap) {
>> +        error_setg(errp, ".get_cap() not implemented");
>> +        return false;
>> +    }
>> +
>> +    /* Common checks */
>> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_AW_BITS,
>errp);
>> +    if (ret < 0) {
>> +        return false;
>> +    }
>> +    if (s->aw_bits > ret) {
>> +        error_setg(errp, "aw-bits %d > host aw-bits %d", s->aw_bits, ret);
>> +        return false;
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>  static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
>devfn,
>>                                       HostIOMMUDevice *hiod, Error **errp)
>>  {
>> @@ -3842,6 +3866,11 @@ static bool
>vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>>          return false;
>>      }
>>
>> +    if (!vtd_check_hdev(s, hiod, errp)) {
>> +        vtd_iommu_unlock(s);
>> +        return false;
>> +    }
>> +
>>      vtd_hdev = g_malloc0(sizeof(VTDHostIOMMUDevice));
>>      vtd_hdev->bus = bus;
>>      vtd_hdev->devfn = (uint8_t)devfn;
>Eric