On 2024/12/9 22:57, Jason Gunthorpe wrote:
On Sat, Dec 07, 2024 at 06:49:04PM +0800, Yi Liu wrote:
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.
That would be nice, and it is better than checking config space
- 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).
I'm not sure, I think we should not make it dependent on the device
capability. There may be multiple devices in the iommufd and some of
them may be PASID capable. The PASID capable domains should interwork
with all of the devices. Thus I'd also expect to be able to allocate a
PASID capable domain on a non-pasid capable device. Even though that
would be pointless on its own.
yes. I also had an offline email to confirm with Vasant, and he confirmed
a non-pasid capable device should be able to use pasid-capable domain (V2
page table.
As far as I know, allocating PASID cpable domain depends on whether
the underlying IOMMU hw supports the page table format (AMD IOMMU V2, Intel
VT-d Stage-1, ARM SMMUv3 Stage-1 page table?) or not. So even for a
non-pasid capable device, allocating pasid-capable domain should succeed if
its IOMMU supports the page table format.
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.
That seems good too, if it isn't too hard to do.
The non-pasid and pasid shares much code, I think it should be doable.
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?
Why can't it be in iommufd? A PASID domain should be a hwpt_paging
with the ALLOC_PASID flag, just put a bit in the hwpt_paging struct
and be done with it. That automatically rejects nested domains from
pasid.
The problem is Intel side, we allow attaching nested domains to pasid. :(
That's why I'm asking for updating the description of ALLOC_PASID and
the enforcement to be only applicable to paging domains, not applicable for
nested domains.
I would like to keep this detail out of the drivers as I think drivers
will have a hard time getting it right. Drivers should implement their
HW restrictions and if they are more permissive than the iommufd model
that is OK.
We want some reasonable compromise to encourage applications to use
IOMMU_HWPT_ALLOC_PASID properly, but not build too much complexity to
reject driver-specific behavior.
I'm ok to do it in iommufd as long as it is only applicable to hwpt_paging.
Otherwise, attaching nested domain to pasid would be failed according to
the aforementioned enforcement.
--
Regards,
Yi Liu
