>-----Original Message-----
>From: Eric Auger <eric.au...@redhat.com>
>Subject: Re: [PATCH v6 09/19] vfio/iommufd: Implement
>HostIOMMUDeviceClass::realize() handler
>
>
>
>On 6/4/24 04:58, Duan, Zhenzhong wrote:
>>
>>> -----Original Message-----
>>> From: Eric Auger <eric.au...@redhat.com>
>>> Subject: Re: [PATCH v6 09/19] vfio/iommufd: Implement
>>> HostIOMMUDeviceClass::realize() handler
>>>
>>> Hi Zhenzhong,
>>>
>>> On 6/3/24 08:10, Zhenzhong Duan wrote:
>>>> It calls iommufd_backend_get_device_info() to get host IOMMU
>>>> related information and translate it into HostIOMMUDeviceCaps
>>>> for query with .get_cap().
>>>>
>>>> Introduce macro VTD_MGAW_FROM_CAP to get MGAW which equals
>to
>>>> (aw_bits - 1).
>>>>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>>> ---
>>>> include/hw/i386/intel_iommu.h | 1 +
>>>> hw/vfio/iommufd.c | 37
>>> +++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 38 insertions(+)
>>>>
>>>> diff --git a/include/hw/i386/intel_iommu.h
>>> b/include/hw/i386/intel_iommu.h
>>>> index 7fa0a695c8..7d694b0813 100644
>>>> --- a/include/hw/i386/intel_iommu.h
>>>> +++ b/include/hw/i386/intel_iommu.h
>>>> @@ -47,6 +47,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IntelIOMMUState,
>>> INTEL_IOMMU_DEVICE)
>>>> #define VTD_HOST_AW_48BIT 48
>>>> #define VTD_HOST_ADDRESS_WIDTH VTD_HOST_AW_39BIT
>>>> #define VTD_HAW_MASK(aw) ((1ULL << (aw)) - 1)
>>>> +#define VTD_MGAW_FROM_CAP(cap) ((cap >> 16) & 0x3fULL)
>>>>
>>>> #define DMAR_REPORT_F_INTR (1)
>>>>
>>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>>> index e4a507d55c..9d2e95e20e 100644
>>>> --- a/hw/vfio/iommufd.c
>>>> +++ b/hw/vfio/iommufd.c
>>>> @@ -25,6 +25,7 @@
>>>> #include "qemu/cutils.h"
>>>> #include "qemu/chardev_open.h"
>>>> #include "pci.h"
>>>> +#include "hw/i386/intel_iommu_internal.h"
>>>>
>>>> static int iommufd_cdev_map(const VFIOContainerBase *bcontainer,
>>> hwaddr iova,
>>>> ram_addr_t size, void *vaddr, bool readonly)
>>>> @@ -619,6 +620,41 @@ static void
>>> vfio_iommu_iommufd_class_init(ObjectClass *klass, void *data)
>>>> vioc->pci_hot_reset = iommufd_cdev_pci_hot_reset;
>>>> };
>>>>
>>>> +static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void
>>> *opaque,
>>>> + Error **errp)
>>>> +{
>>>> + VFIODevice *vdev = opaque;
>>>> + HostIOMMUDeviceCaps *caps = &hiod->caps;
>>>> + enum iommu_hw_info_type type;
>>>> + union {
>>>> + struct iommu_hw_info_vtd vtd;
>>>> + } data;
>>>> +
>>>> + if (!iommufd_backend_get_device_info(vdev->iommufd, vdev-
>>devid,
>>>> + &type, &data, sizeof(data),
>>>> errp)) {
>>>> + return false;
>>>> + }
>>>> +
>>>> + caps->type = type;
>>>> +
>>>> + switch (type) {
>>>> + case IOMMU_HW_INFO_TYPE_INTEL_VTD:
>>>> + caps->aw_bits = VTD_MGAW_FROM_CAP(data.vtd.cap_reg) + 1;
>>> Please can you remind me of why you can't reuse the iova_ranges
>method.
>>> isn't it generic enough?
>> Yes, iova_ranges method is only for iova_ranges, we want to make
>> HostIOMMUDevice.get_cap() a common interface.
>>
>> When we want to pass iova_ranges, we can add
>HOST_IOMMU_DEVICE_CAP_IOVA_RANGES
>> and HostIOMMUDevice.iova_ranges.
>
>I rather meant that iova_ranges is part of VFIOContainerBase and you
>could reuse the technics used in hiod_legacy_vfio_realize, relying on a
>common helper instead of using
>
>VTD_MGAW_FROM_CAP(data.vtd.cap_reg). Doesn't it work?
Get your point.
Yes, It does work and should have same result.
That means iommufd backend support two ways to get aw_bits.
Only reason is I feel VTD_MGAW_FROM_CAP(data.vtd.cap_reg) is a bit simpler
and there are other bits picked in nesting series, see:
case IOMMU_HW_INFO_TYPE_INTEL_VTD:
caps->aw_bits = VTD_MGAW_FROM_CAP(data.vtd.cap_reg) + 1;
caps->nesting = !!(data.vtd.ecap_reg & VTD_ECAP_NEST);
caps->fs1gp = !!(data.vtd.cap_reg & VTD_CAP_FS1GP);
caps->errata = data.vtd.flags & IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17;
I'm fine to use iova_ranges to calculate aw_bits for iommufd backend if you
prefer that.
Thanks
Zhenzhong
