On Wed, Aug 27, 2025 at 07:56:38PM +0800, Yi Liu wrote:
> On 2025/8/23 07:55, Nicolin Chen wrote:
> > > +    if (vtd.flags & IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17) {
> > > +        container->bcontainer.bypass_ro = true;
> > 
> > This circled back to checking a vendor specific flag in the core..
> > 
> > Perhaps we could upgrade the get_viommu_cap op and its API:
> > 
> > enum viommu_flags {
> >      VIOMMU_FLAG_HW_NESTED = BIT_ULL(0),
> >      VIOMMU_FLAG_BYPASS_RO = BIT_ULL(1),
> 
> hmmm. I'm not quite on this idea as the two flags have different sources.
> One determined by vIOMMU config, one by the hardware limit. Reporting
> them in one API is strange.

It's fair enough that we want to make such a clear boundary between
a vIOMMU flag and a HW IOMMU flag of the same vendor..

> I think the bypass RO can be determined in
> VFIO just like the patch has done. But it should check if vIOMMU has
> requested nested hwpt and also the reported hw_info::type is
> IOMMU_HW_INFO_TYPE_INTEL_VTD.
> 
>       if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT) &&
>             type == IOMMU_HW_INFO_TYPE_INTEL_VTD &&
>             vtd.flags & IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17) {
>             container->bcontainer.bypass_ro = true;
>          }

Then, it feels odd to me that we don't have a clear boundary between
a generic flag and a vendor flag :-/

It's fine if we want to keep all the host-level vendor flags outside
the vIOMMU code, but at least could we please have a generic looking
function outside this iommufd_cdev_autodomains_get() to translate a
vendor flag to a generic looking flag?

We could start with a function that loads the HostIOMMUDeviceCaps (or
just VendorCaps) dealing with vendor types and outputs generic ones:

        host_iommu_flags = host_iommu_decode_vendor_caps(&vendor_caps);

        if (hwpt_flags & IOMMU_HWPT_ALLOC_NEST_PARENT &&
            host_iommu_flags & HOST_IOMMU_FLAG_BYPASS_RO) {
             container->bcontainer.bypass_ro = true;
        }

Over time, it can even grow into a separate file, if there are more
vendor specific requirement.

Nicolin

Reply via email to