>-----Original Message-----
>From: Cédric Le Goater <c...@redhat.com>
>Subject: Re: [PATCH v5 19/19] intel_iommu: Check compatibility with host
>IOMMU capabilities
>
>On 5/8/24 11:03, 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
>> before scalable modern mode is introduced.
>>
>> Signed-off-by: Yi Liu <yi.l....@intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>> hw/i386/intel_iommu.c | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 747c988bc4..07bfd4f99e 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -20,6 +20,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include CONFIG_DEVICES /* CONFIG_HOST_IOMMU_DEVICE */
>> #include "qemu/error-report.h"
>> #include "qemu/main-loop.h"
>> #include "qapi/error.h"
>> @@ -3819,6 +3820,26 @@ VTDAddressSpace
>*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>> return vtd_dev_as;
>> }
>>
>> +static bool vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice
>*vtd_hdev,
>> + Error **errp)
>> +{
>> +#ifdef CONFIG_HOST_IOMMU_DEVICE
>> + HostIOMMUDevice *hiod = vtd_hdev->dev;
>> + int ret;
>> +
>> + /* Common checks */
>> + ret = host_iommu_device_get_cap(hiod,
>HOST_IOMMU_DEVICE_CAP_AW_BITS, errp);
>
>To avoid CONFIG_HOST_IOMMU_DEVICE, host_iommu_device_get_cap()
>could be
>open coded.
Thanks for suggesting, it works for build on both windows and linux.
>
>> + 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;
>> + }
>> +#endif
>> + return true;
>> +}
>> +
>> static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
>devfn,
>> HostIOMMUDevice *hiod, Error **errp)
>> {
>> @@ -3848,6 +3869,12 @@ static bool
>vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>> vtd_hdev->iommu_state = s;
>> vtd_hdev->dev = hiod;
>>
>> + if (!vtd_check_hdev(s, vtd_hdev, errp)) {
>> + g_free(vtd_hdev);
>> + vtd_iommu_unlock(s);
>> + return false;
>> + }
>
>This check could be first done before allocating vtd_hdev.
OK, will do.
I made it that way to facilitate this patch:
https://github.com/yiliu1765/qemu/commit/d589a470002ccf607b5743b2951612f7b4790833
Thanks
Zhenzhong
