On 2024/5/6 21:36, Jason Gunthorpe wrote:
On Mon, May 06, 2024 at 03:42:21PM +0800, Baolu Lu wrote:
On 2024/4/30 17:19, Yi Liu wrote:
On 2024/4/17 17:25, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Friday, April 12, 2024 4:15 PM
From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
This allows the upper layers to set a nested type domain to a PASID of a
device if the PASID feature is supported by the IOMMU hardware.
The set_dev_pasid callback for non-nested domain has already be
there, so
this only needs to add it for nested domains. Note that the S2
domain with
dirty tracking capability is not supported yet as no user for now.
S2 domain does support dirty tracking. Do you mean the specific
check in intel_iommu_set_dev_pasid() i.e. pasid-granular dirty
tracking is not supported yet?
yes. We may remove this check when real usage comes. e.g. SIOV.
+static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid,
+ struct iommu_domain *old)
+{
+ struct device_domain_info *info = dev_iommu_priv_get(dev);
+ struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+ struct intel_iommu *iommu = info->iommu;
+
+ if (iommu->agaw < dmar_domain->s2_domain->agaw)
+ return -EINVAL;
+
this check is covered by prepare_domain_attach_device() already.
This was added to avoid modifying the s2_domain's agaw. I'm fine to remove
it personally as the existing attach path also needs to update domain's
agaw per device attachment. @Baolu, how about your opinion?
We still need something to do before we can safely remove this check.
All the domain allocation interfaces should eventually have the device
pointer as the input, and all domain attributions could be initialized
during domain allocation. In the attach paths, it should return -EINVAL
directly if the domain is not compatible with the iommu for the device.
Yes, and this is already true for PASID.
I'm not quite get why it is already true for PASID. I think Baolu's remark
is general to domains attached to either RID or PASID.
I feel we could reasonably insist that domanis used with PASID are
allocated with a non-NULL dev.
Any special reason for this disclaim?
If so it means we need to fixup the domain allocation in iommufd as
part of the pasid series, and Intel will have to implement
alloc_domain_paging().
I agree implementing alloc_domain_paging() is the final solution to avoid
such dynamic modifications to domain's caps. If it's really needed for
PASID series now, I can add it in next version. :)
--
Regards,
Yi Liu
