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