Re: [PATCH v6 18/22] iommufd: Introduce a helper function to extract vendor capabilities

[Nicolin Chen <nicolinc@nvidia.com>]

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 <nicolinc@nvidia.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Reviewed-by: Nicolin Chen <nicolinc@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)
    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