Hi Cédric,
>-----Original Message-----
>From: Cédric Le Goater <c...@redhat.com>
>Subject: Re: [PATCH v2 3/5] intel_iommu: Add a framework to do
>compatibility check with host IOMMU cap/ecap
>
>On 4/8/24 10:44, Zhenzhong Duan wrote:
>> From: Yi Liu <yi.l....@intel.com>
>>
>> If check fails, the host side device(either vfio or vdpa device) should not
>> be passed to guest.
>>
>> Implementation details for different backends will be in following patches.
>>
>> Signed-off-by: Yi Liu <yi.l....@intel.com>
>> Signed-off-by: Yi Sun <yi.y....@linux.intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>> hw/i386/intel_iommu.c | 35
>+++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 4f84e2e801..a49b587c73 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -35,6 +35,7 @@
>> #include "sysemu/kvm.h"
>> #include "sysemu/dma.h"
>> #include "sysemu/sysemu.h"
>> +#include "sysemu/iommufd.h"
>> #include "hw/i386/apic_internal.h"
>> #include "kvm/kvm_i386.h"
>> #include "migration/vmstate.h"
>> @@ -3819,6 +3820,32 @@ VTDAddressSpace
>*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>> return vtd_dev_as;
>> }
>>
>> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>> + HostIOMMUDevice *hiod,
>> + Error **errp)
>> +{
>> + return 0;
>> +}
>> +
>> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>> + HostIOMMUDevice *hiod,
>> + Error **errp)
>> +{
>> + return 0;
>> +}
>> +
>> +static int vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice
>*vtd_hdev,
>> + Error **errp)
>> +{
>> + HostIOMMUDevice *hiod = vtd_hdev->dev;
>> +
>> + if (object_dynamic_cast(OBJECT(hiod), TYPE_HIOD_IOMMUFD)) {
>> + return vtd_check_iommufd_hdev(s, hiod, errp);
>> + }
>> +
>> + return vtd_check_legacy_hdev(s, hiod, errp);
>> +}
>
>
>I think we should be using the .get_host_iommu_info() class handler
>instead. Can we refactor the code slightly to avoid this check on
>the type ?
There is some difficulty ini avoiding this check, the behavior of
vtd_check_legacy_hdev
and vtd_check_iommufd_hdev are different especially after nesting support
introduced.
vtd_check_iommufd_hdev() has much wider check over cap/ecap bits besides
aw_bits.
That the reason I have two functions to do different thing.
See:
https://github.com/yiliu1765/qemu/blob/zhenzhong/iommufd_nesting_rfcv2/hw/i386/intel_iommu.c#L5472
Meanwhile in vtd_check_legacy_hdev(), when legacy VFIO device attaches to
modern vIOMMU,
this is unsupported and error out early, it will not call
.get_host_iommu_info().
I mean we don't need to unconditionally call .get_host_iommu_info() in some
cases.
Thanks
Zhenzhong
