[PATCH v2 6/8] iommu/vt-d: Implement set_dev_pasid domain op

Jacob Pan posted 8 patches 2 years, 10 months ago
There is a newer version of this series
[PATCH v2 6/8] iommu/vt-d: Implement set_dev_pasid domain op
Posted by Jacob Pan 2 years, 10 months ago
Devices that use ENQCMDS to submit work on buffers mapped by DMA API
must attach a PASID to the default domain of the device. In preparation
for this use case, this patch implements set_dev_pasid() for the
default_domain_ops. Besides PASID attachment, device will also be
attached to the domain as the result of this call if the device has not
been attached before.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 0a195136bac7..652ee0ca72da 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4782,6 +4782,26 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 	intel_iommu_detach_device_pasid(domain, dev, pasid);
 }
 
+static int intel_iommu_attach_device_pasid(struct iommu_domain *domain,
+					   struct device *dev, ioasid_t pasid)
+{
+	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;
+	int ret;
+
+	if (!pasid_supported(iommu))
+		return -ENODEV;
+
+	ret = prepare_domain_attach_device(domain, dev);
+	if (ret)
+		return ret;
+
+	return dmar_domain_attach_device_pasid(dmar_domain, dev, pasid);
+}
+
+
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -4801,6 +4821,7 @@ const struct iommu_ops intel_iommu_ops = {
 #endif
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev		= intel_iommu_attach_device,
+		.set_dev_pasid          = intel_iommu_attach_device_pasid,
 		.map_pages		= intel_iommu_map_pages,
 		.unmap_pages		= intel_iommu_unmap_pages,
 		.iotlb_sync_map		= intel_iommu_iotlb_sync_map,
-- 
2.25.1
RE: [PATCH v2 6/8] iommu/vt-d: Implement set_dev_pasid domain op
Posted by Tian, Kevin 2 years, 10 months ago
> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Sent: Tuesday, March 28, 2023 7:22 AM
> 
> Devices that use ENQCMDS to submit work on buffers mapped by DMA API
> must attach a PASID to the default domain of the device. In preparation
> for this use case, this patch implements set_dev_pasid() for the
> default_domain_ops. Besides PASID attachment, device will also be
> attached to the domain as the result of this call if the device has not
> been attached before.
> 

I didn't get the last point. PASID attach should only have the scope
for the pasid. RID of the device might be attached to another domain
which shouldn't be changed by this call.
Re: [PATCH v2 6/8] iommu/vt-d: Implement set_dev_pasid domain op
Posted by Jacob Pan 2 years, 10 months ago
Hi Kevin,

On Tue, 28 Mar 2023 07:47:45 +0000, "Tian, Kevin" <kevin.tian@intel.com>
wrote:

> > From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Sent: Tuesday, March 28, 2023 7:22 AM
> > 
> > Devices that use ENQCMDS to submit work on buffers mapped by DMA API
> > must attach a PASID to the default domain of the device. In preparation
> > for this use case, this patch implements set_dev_pasid() for the
> > default_domain_ops. Besides PASID attachment, device will also be
> > attached to the domain as the result of this call if the device has not
> > been attached before.
> >   
> 
> I didn't get the last point. PASID attach should only have the scope
> for the pasid. RID of the device might be attached to another domain
> which shouldn't be changed by this call.
I meant if the RID context has not been set up before attaching this PASID,
this call will also set up the context, PASID dir etc. In the end, we
eliminated ordering requirement of attaching device, RID_PASID first, then
other PASIDs.
How about:
"If the device context has not been set up prior to this call, this will
set up the device context in addition to PASID attachment."

Thanks,

Jacob
Re: [PATCH v2 6/8] iommu/vt-d: Implement set_dev_pasid domain op
Posted by Baolu Lu 2 years, 10 months ago
On 3/28/23 11:40 PM, Jacob Pan wrote:
> Hi Kevin,
> 
> On Tue, 28 Mar 2023 07:47:45 +0000, "Tian, Kevin" <kevin.tian@intel.com>
> wrote:
> 
>>> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>> Sent: Tuesday, March 28, 2023 7:22 AM
>>>
>>> Devices that use ENQCMDS to submit work on buffers mapped by DMA API
>>> must attach a PASID to the default domain of the device. In preparation
>>> for this use case, this patch implements set_dev_pasid() for the
>>> default_domain_ops. Besides PASID attachment, device will also be
>>> attached to the domain as the result of this call if the device has not
>>> been attached before.
>>>    
>>
>> I didn't get the last point. PASID attach should only have the scope
>> for the pasid. RID of the device might be attached to another domain
>> which shouldn't be changed by this call.
> I meant if the RID context has not been set up before attaching this PASID,
> this call will also set up the context, PASID dir etc. In the end, we
> eliminated ordering requirement of attaching device, RID_PASID first, then
> other PASIDs.

This occurs in default domain deferred attaching case.

> How about:
> "If the device context has not been set up prior to this call, this will
> set up the device context in addition to PASID attachment."

Best regards,
baolu
RE: [PATCH v2 6/8] iommu/vt-d: Implement set_dev_pasid domain op
Posted by Tian, Kevin 2 years, 10 months ago
> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Sent: Tuesday, March 28, 2023 11:40 PM
> 
> Hi Kevin,
> 
> On Tue, 28 Mar 2023 07:47:45 +0000, "Tian, Kevin" <kevin.tian@intel.com>
> wrote:
> 
> > > From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > > Sent: Tuesday, March 28, 2023 7:22 AM
> > >
> > > Devices that use ENQCMDS to submit work on buffers mapped by DMA
> API
> > > must attach a PASID to the default domain of the device. In preparation
> > > for this use case, this patch implements set_dev_pasid() for the
> > > default_domain_ops. Besides PASID attachment, device will also be
> > > attached to the domain as the result of this call if the device has not
> > > been attached before.
> > >
> >
> > I didn't get the last point. PASID attach should only have the scope
> > for the pasid. RID of the device might be attached to another domain
> > which shouldn't be changed by this call.
> I meant if the RID context has not been set up before attaching this PASID,
> this call will also set up the context, PASID dir etc. In the end, we
> eliminated ordering requirement of attaching device, RID_PASID first, then
> other PASIDs.
> How about:
> "If the device context has not been set up prior to this call, this will
> set up the device context in addition to PASID attachment."
> 

this is clearer.