Hi Clement, See inline.
From: CLEMENT MATHIEU--DRIF <[email protected]> Sent: Tuesday, May 7, 2024 7:40 PM To: Duan, Zhenzhong <[email protected]>; [email protected] Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Tian, Kevin <[email protected]>; Liu, Yi L <[email protected]>; Peng, Chao P <[email protected]>; Marcel Apfelbaum <[email protected]>; Paolo Bonzini <[email protected]>; Richard Henderson <[email protected]>; Eduardo Habkost <[email protected]> Subject: Re: [PATCH v4 19/19] intel_iommu: Check compatibility with host IOMMU capabilities Hi Zhenzhong, On 07/05/2024 11:20, Zhenzhong Duan wrote: Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe. 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 <[email protected]><mailto:[email protected]> Signed-off-by: Zhenzhong Duan <[email protected]><mailto:[email protected]> --- hw/i386/intel_iommu.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 747c988bc4..146fde23fc 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/host_iommu_device.h" #include "hw/i386/apic_internal.h" #include "kvm/kvm_i386.h" #include "migration/vmstate.h" @@ -3819,6 +3820,25 @@ 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) +{ + HostIOMMUDevice *hiod = vtd_hdev->dev; Why not passing the hiod pointer as parameter directly? Maybe you have something in mind for a future patch? [Zhenzhong] Yes, I have below change in commit https://github.com/yiliu1765/qemu/commit/7a8219b040b44efe7a828e130bdf5ccc2dddb4d0 ret = host_iommu_device_get_cap(hiod, HOST_IOMMU_DEVICE_CAP_ERRATA, errp); if (ret < 0) { return false; } vtd_hdev->errata = ret; Thanks Zhenzhong It would allow us to allocate the VTDHostIOMMUDevice later in vtd_dev_set_iommu_device. + int ret; + + /* Common checks */ + ret = host_iommu_device_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) { @@ -3848,6 +3868,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; + } + new_key = g_malloc(sizeof(*new_key)); new_key->bus = bus; new_key->devfn = devfn; -- 2.34.1
