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. 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?
- 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.
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?
- Nest parent domain should never be pasid-compatible?
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?
Thoughts?
[1]
https://lore.kernel.org/linux-iommu/5a6c2676-256a-4fa5-b9a0-e433d4e933c9@xxxxxxxxx/
Regards,
Yi Liu
On 2024/11/4 21:25, Yi Liu wrote:
iommu hw may have special requirement on the domain attached to PASID
capable device. e.g. AMD IOMMU requires the domain allocated with the
IOMMU_HWPT_ALLOC_PASID flag. Hence, iommufd should enforce it when the
domain is used by PASID-capable device.
Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
---
drivers/iommu/intel/iommu.c | 6 ++++--
drivers/iommu/iommufd/hw_pagetable.c | 7 +++++--
drivers/iommu/iommufd/iommufd_private.h | 7 +++++++
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index a1341078b962..d24e21a757ff 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3545,13 +3545,15 @@ intel_iommu_domain_alloc_user(struct device *dev, u32 flags,
/* Must be NESTING domain */
if (parent) {
- if (!nested_supported(iommu) || flags)
+ if (!nested_supported(iommu) ||
+ flags & ~IOMMU_HWPT_ALLOC_PASID)
return ERR_PTR(-EOPNOTSUPP);
return intel_nested_domain_alloc(parent, user_data);
}
if (flags &
- (~(IOMMU_HWPT_ALLOC_NEST_PARENT | IOMMU_HWPT_ALLOC_DIRTY_TRACKING)))
+ (~(IOMMU_HWPT_ALLOC_NEST_PARENT | IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
+ IOMMU_HWPT_ALLOC_PASID)))
return ERR_PTR(-EOPNOTSUPP);
if (nested_parent && !nested_supported(iommu))
return ERR_PTR(-EOPNOTSUPP);
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 48639427749b..e4932a5a87ea 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -107,7 +107,8 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
const struct iommu_user_data *user_data)
{
const u32 valid_flags = IOMMU_HWPT_ALLOC_NEST_PARENT |
- IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
+ IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
+ IOMMU_HWPT_ALLOC_PASID;
const struct iommu_ops *ops = dev_iommu_ops(idev->dev);
struct iommufd_hwpt_paging *hwpt_paging;
struct iommufd_hw_pagetable *hwpt;
@@ -128,6 +129,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
if (IS_ERR(hwpt_paging))
return ERR_CAST(hwpt_paging);
hwpt = &hwpt_paging->common;
+ hwpt->pasid_compat = flags & IOMMU_HWPT_ALLOC_PASID;
INIT_LIST_HEAD(&hwpt_paging->hwpt_item);
/* Pairs with iommufd_hw_pagetable_destroy() */
@@ -223,7 +225,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
struct iommufd_hw_pagetable *hwpt;
int rc;
- if ((flags & ~IOMMU_HWPT_FAULT_ID_VALID) ||
+ if ((flags & ~(IOMMU_HWPT_FAULT_ID_VALID | IOMMU_HWPT_ALLOC_PASID)) ||
!user_data->len || !ops->domain_alloc_user)
return ERR_PTR(-EOPNOTSUPP);
if (parent->auto_domain || !parent->nest_parent ||
@@ -235,6 +237,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
if (IS_ERR(hwpt_nested))
return ERR_CAST(hwpt_nested);
hwpt = &hwpt_nested->common;
+ hwpt->pasid_compat = flags & IOMMU_HWPT_ALLOC_PASID;
refcount_inc(&parent->common.obj.users);
hwpt_nested->parent = parent;
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 11773cef5acc..81a95f869e10 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -296,6 +296,7 @@ struct iommufd_hw_pagetable {
struct iommufd_object obj;
struct iommu_domain *domain;
struct iommufd_fault *fault;
+ bool pasid_compat : 1;
};
struct iommufd_hwpt_paging {
@@ -531,6 +532,9 @@ static inline int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev,
ioasid_t pasid)
{
+ if (idev->dev->iommu->max_pasids && !hwpt->pasid_compat)
+ return -EINVAL;
+
if (hwpt->fault)
return iommufd_fault_domain_attach_dev(hwpt, idev, pasid);
@@ -564,6 +568,9 @@ static inline int iommufd_hwpt_replace_device(struct iommufd_device *idev,
struct iommufd_attach_handle *curr;
int ret;
+ if (idev->dev->iommu->max_pasids && !hwpt->pasid_compat)
+ return -EINVAL;
+
if (old->fault || hwpt->fault)
return iommufd_fault_domain_replace_dev(idev, pasid,
hwpt, old);
