On 2024/12/7 01:58, Jason Gunthorpe wrote:
On Fri, Dec 06, 2024 at 03:57:39PM +0800, Yi Liu wrote:
Hi Jason, Vasant,
When cooking new version, I got three opens on enforcing using
pasid-compatible domains to pasid-capable device in iommufd as suggested
in [1].
- Concept problem when considering nested domain
IIUC. pasid-compatible domain means the domain can be attached to PASID.
e.g. AMD requires using V2 page table hence it can be configed to GCR3.
However, the nested domain uses both V1 and V2 page table, I don't think
it can be attached to PASID on AMD. So nested domain can not be
considered as pasid-compatible.
Yes.
Based on this, this enforcement only
applies to paging-domains. If so, do we still need to enforce it in
iommufd? Will it simpler to let the AMD iommu driver to deal it?
I think driver should deal with it, Intel doesn't have that
limitation. I sent patches to fix that detection for AMD and ARM
already.
ok.
- PASID-capable device v.s. PASID-enabled device
We keep saying PASID-capable, but system may not enable it. Would it
better enforce the pasid-compatible domain for PASID-enabled device?
Seems all iommu vendor will enable PASID if it's supported. But
conceptly, it is be more accurate if only do it when PASID is
enabled.
If we want to do more here we should put the core code in charge of
deciding of a device will be PASID enabled and the IOMMU driver only
indicates if it can be PASID supported.
For PCI devices, we can check if the pasid cap is enabled from device
config space. But for non-PCI PASID support (e.g. some ARM platform), I
don't know if there is any way to check the PASID enabled or not. Or, to
cover both, we need an iommu API to check PASID enabled or not?
Yes, some iommu API, I suggest a flag in the common iommu_device. We
already have max_pasids there, it may already be nearly enough.
yeah, the dev->iommu->max_pasids indicates if a device can enable pasid
or not. It already counts in the iommu support. Since all known iommu
drivers will enable it once it is supported, can we say
dev->iommu->max_pasids also means enabled? If so, in the HW_INFO path[1],
we only need check it instead of checking pci config space.
[1] https://lore.kernel.org/kvm/20241108121742.18889-6-yi.l.liu@xxxxxxxxx/
- Nest parent domain should never be pasid-compatible?
Up to the driver.
I think the AMD iommu uses the V1 page table format for the parent
domain. Hence parent domain should not be allocated with the
IOMMU_HWPT_ALLOC_PASID flag. Otherwise, it does not work. Should this
be enforced in iommufd?
Enforced in the driver.
ok. BTW. Should we update the below description to be "the rule is only
applied to the domains that will be attached to pasid-capable device"?
Otherwise, a 'poor' userspace might consider any domains allocated for
pasid-capable device must use this flag.
* @IOMMU_HWPT_ALLOC_PASID: Requests a domain that can be used with PASID. The
* domain can be attached to any PASID on the device.
* Any domain attached to the non-PASID part of the
* device must also be flaged, otherwise attaching a
* PASID will blocked.
* If IOMMU does not support PASID it will return
* error (-EOPNOTSUPP).
iommufd should enforce that the domain was created with
IOMMU_HWPT_ALLOC_PASID before passing the HWPT to any pasid
attach/replace function.
This seems much simpler enforcement than I did in this patch. I even
enforced the domains used in the non-pasid path to be flagged with
_ALLOC_PASID. But just as the beginning of this email, the nested domains
on AMD is not able to be used by pasid, a sane userspace won't do it. So
such a domain won't appear in the pasid path on AMD platform. But it can be
on Intel as we support attaching nested domain to pasid. So we would need
the nested domain be flagged in order to pass this check. Looks like we
cannot do this enforcement in iommufd. Put it in the iommu drivers would be
better. Is it?
Regards,
Yi Liu
