On 2/28/25 9:29 AM, Duan, Zhenzhong wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.au...@redhat.com>
>> Subject: Re: [PATCH rfcv2 06/20] host_iommu_device: Define two new
>> capabilities HOST_IOMMU_DEVICE_CAP_[NESTING|FS1GP]
>>
>> Hi Zhenzhong,
>>
>>
>> On 2/19/25 9:22 AM, Zhenzhong Duan wrote:
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>> ---
>>> include/system/host_iommu_device.h | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/include/system/host_iommu_device.h
>> b/include/system/host_iommu_device.h
>>> index df782598f2..18f8b5e5cf 100644
>>> --- a/include/system/host_iommu_device.h
>>> +++ b/include/system/host_iommu_device.h
>>> @@ -22,10 +22,16 @@
>>> *
>>> * @hw_caps: host platform IOMMU capabilities (e.g. on IOMMUFD this
>> represents
>>> * the @out_capabilities value returned from IOMMU_GET_HW_INFO
>> ioctl)
>>> + *
>>> + * @nesting: nesting page table support.
>>> + *
>>> + * @fs1gp: first stage(a.k.a, Stage-1) 1GB huge page support.
>>> */
>>> typedef struct HostIOMMUDeviceCaps {
>>> uint32_t type;
>>> uint64_t hw_caps;
>>> + bool nesting;
>>> + bool fs1gp;
>> this looks quite vtd specific, isn't it? Shouldn't we hide this is a
>> vendor specific cap struct?
> Yes? I guess ARM hw could also provide nesting support at least
> There are some reasons I perfer a flatten struct even if some
> Elements may be vendor specific.
> 1. If a vendor doesn't support an capability for other vendor,
> corresponding element should be zero by default.
> 2. An element vendor specific may become generic in future
> and we don't need to update the structure when that happens.
> 3. vIOMMU calls get_cap() to query if a capability is supported,
> so a vIOMMU never query a vendor specific capability it doesn't
> recognize. Even if that happens, zero is returned hinting no support.
I will let others comment but in general this is frown upon and unions
are prefered at least.
Eric
>
> Thanks
> Zhenzhong
>
