>-----Original Message-----
>From: Nicolin Chen <nicol...@nvidia.com>
>Subject: Re: [PATCH v6 18/22] iommufd: Introduce a helper function to
>extract vendor capabilities
>
>On Thu, Sep 18, 2025 at 04:57:57AM -0400, Zhenzhong Duan wrote:
>> In VFIO core, we call iommufd_backend_get_device_info() to return vendor
>> specific hardware information data, but it's not good to extract this raw
>> data in VFIO core.
>>
>> Introduce host_iommu_extract_vendor_caps() to help extracting the raw
>> data and return a bitmap in iommufd.c because it's the place defining
>> iommufd_backend_get_device_info().
>>
>> The other choice is to put vendor data extracting code in vendor vIOMMU
>> emulation file, but that will make those files mixed with vIOMMU
>> emulation and host IOMMU extracting code, also need a new callback in
>> PCIIOMMUOps. So we choose a simpler way as above.
>>
>> Suggested-by: Nicolin Chen <nicol...@nvidia.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>
>Reviewed-by: Nicolin Chen <nicol...@nvidia.com>
>
>With some nits:
>
>> +enum {
>> + /* Nesting parent HWPT shouldn't have readonly mapping, due to
>errata */
>> + IOMMU_HW_NESTING_PARENT_BYPASS_RO = BIT_ULL(0),
>> +};
>
>I would put a name here too. And given this is defined generically:
>
>/* Host IOMMU quirks. Extracted from host IOMMU capabilities */
>enum host_iommu_quirks {
> HOST_IOMMU_QUIRK_NESTING_PARENT_BYPASS_RO = BIT_ULL(0),
>};
>
>> +/**
>> + * host_iommu_extract_vendor_caps: Extract vendor capabilities
>
>Then:
>
> * host_iommu_extract_quirks: Extract host IOMMU quirks
>
>> + * This function converts @type specific hardware information data
>> + * into a standard bitmap format.
>> + *
>> + * @type: IOMMU Hardware Info Types
>> + *
>> + * @VendorCaps: IOMMU @type specific hardware information data
>> + *
>> + * Returns: 64bit bitmap with each bit represents a capability of host
>> + * IOMMU that we want to expose. See IOMMU_HW_* in
>include/hw/iommu.h
>> + * for all possible capabilities currently exposed.
>
>And simplify this:
>
> * Returns: bitmap with each representing a host IOMMU quirk defined in
> * enum host_iommu_quirks
>
>> +uint64_t host_iommu_extract_vendor_caps(uint32_t type, VendorCaps
>*caps)
>> +{
>> + uint64_t vendor_caps = 0;
>> +
>> + if (type == IOMMU_HW_INFO_TYPE_INTEL_VTD &&
>> + caps->vtd.flags &
>IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17) {
>> + vendor_caps |= IOMMU_HW_NESTING_PARENT_BYPASS_RO;
>> + }
>> +
>> + return vendor_caps;
>> +}
>
>uint64_t host_iommu_extract_quirks(enum iommu_hw_info_type,
>VendorCaps *caps)
>{
> uint64_t quirks = 0;
>
>#if defined(CONFIG_VTD)
I have applied all suggested change except CONFIG_VTD here as it's a device
config and iommufd.c is device agnostic, it doesn't recognize CONFIG_VTD.
../backends/iommufd.c:419:13: error: attempt to use poisoned "CONFIG_VTD"
I thought this is trivial and OK for not having CONFIG_VTD?
Thanks
Zhenzhong
> if (type == IOMMU_HW_INFO_TYPE_INTEL_VTD) {
> if (caps->vtd.flags &
>IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17) {
> quirks |=
>HOST_IOMMU_QUIRK_NESTING_PARENT_BYPASS_RO;
> }
> }
>#endif
>
> return quirks;
>}
>
>Thanks
>Nicolin
