diff mbox series

[v5,4/7] iommu/vt-d: Factoring out PASID set up helper function

Message ID 20230427174937.471668-5-jacob.jun.pan@linux.intel.com (mailing list archive)
State Superseded
Headers show
Series Re-enable IDXD kernel workqueue under DMA API | expand

Commit Message

Jacob Pan April 27, 2023, 5:49 p.m. UTC 
From: Lu Baolu <baolu.lu@linux.intel.com>

To prepare non-RID_PASID attachment, factoring out common code to be
reused. PASID entry set up is common for all DMA API PASIDs.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 42 ++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 17 deletions(-)

Comments

Tian, Kevin April 28, 2023, 9:47 a.m. UTC | #1 
> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Sent: Friday, April 28, 2023 1:50 AM
> 
> 
> +static int dmar_domain_attach_device_pasid(struct dmar_domain *domain,
> +					   struct intel_iommu *iommu,
> +					   struct device *dev, ioasid_t pasid)
> +{
> +	int ret;
> +
> +	/* PASID table is mandatory for a PCI device in scalable mode. */
> +	if (!sm_supported(iommu) && dev_is_real_dma_subdevice(dev))
> +		return -EOPNOTSUPP;

"&&" should be "||"
Baolu Lu May 3, 2023, 6:37 a.m. UTC | #2 
On 4/28/23 5:47 PM, Tian, Kevin wrote:
>> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Sent: Friday, April 28, 2023 1:50 AM
>>
>>
>> +static int dmar_domain_attach_device_pasid(struct dmar_domain *domain,
>> +					   struct intel_iommu *iommu,
>> +					   struct device *dev, ioasid_t pasid)
>> +{
>> +	int ret;
>> +
>> +	/* PASID table is mandatory for a PCI device in scalable mode. */
>> +	if (!sm_supported(iommu) && dev_is_real_dma_subdevice(dev))
>> +		return -EOPNOTSUPP;
> 
> "&&" should be "||"
> 

Also should return success instead if this is a RID case. Perhaps,

	if (!sm_supported(iommu) || dev_is_real_dma_subdevice(dev))
		return pasid == RID2PASID ? 0 : -EOPNOTSUPP;

Best regards,
baolu
Jacob Pan May 4, 2023, 9:27 p.m. UTC | #3 
Hi Kevin,

On Fri, 28 Apr 2023 09:47:45 +0000, "Tian, Kevin" <kevin.tian@intel.com>
wrote:

> > From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Sent: Friday, April 28, 2023 1:50 AM
> > 
> > 
> > +static int dmar_domain_attach_device_pasid(struct dmar_domain *domain,
> > +					   struct intel_iommu *iommu,
> > +					   struct device *dev,
> > ioasid_t pasid) +{
> > +	int ret;
> > +
> > +	/* PASID table is mandatory for a PCI device in scalable mode.
> > */
> > +	if (!sm_supported(iommu) && dev_is_real_dma_subdevice(dev))
> > +		return -EOPNOTSUPP;  
> 
> "&&" should be "||"
> 
good catch,

Thanks,

Jacob
Jacob Pan May 4, 2023, 9:39 p.m. UTC | #4 
Hi Baolu,

On Wed, 3 May 2023 14:37:16 +0800, Baolu Lu <baolu.lu@linux.intel.com>
wrote:

> On 4/28/23 5:47 PM, Tian, Kevin wrote:
> >> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> >> Sent: Friday, April 28, 2023 1:50 AM
> >>
> >>
> >> +static int dmar_domain_attach_device_pasid(struct dmar_domain *domain,
> >> +					   struct intel_iommu *iommu,
> >> +					   struct device *dev,
> >> ioasid_t pasid) +{
> >> +	int ret;
> >> +
> >> +	/* PASID table is mandatory for a PCI device in scalable
> >> mode. */
> >> +	if (!sm_supported(iommu) && dev_is_real_dma_subdevice(dev))
> >> +		return -EOPNOTSUPP;  
> > 
> > "&&" should be "||"
> >   
> 
> Also should return success instead if this is a RID case. Perhaps,
> 
> 	if (!sm_supported(iommu) || dev_is_real_dma_subdevice(dev))
> 		return pasid == RID2PASID ? 0 : -EOPNOTSUPP;
> 
Yeah,  I think this is better.  will do.

I was hoping not to treat RIDPASID special here. The caller of this
function does the check if that is RIDPASID but code is duplicated.

Thanks,

Jacob
diff mbox series

Patch

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 9ec45e0497cc..cb586849a1ee 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2429,6 +2429,26 @@  static int __init si_domain_init(int hw)
 	return 0;
 }
 
+static int dmar_domain_attach_device_pasid(struct dmar_domain *domain,
+					   struct intel_iommu *iommu,
+					   struct device *dev, ioasid_t pasid)
+{
+	int ret;
+
+	/* PASID table is mandatory for a PCI device in scalable mode. */
+	if (!sm_supported(iommu) && dev_is_real_dma_subdevice(dev))
+		return -EOPNOTSUPP;
+
+	if (hw_pass_through && domain_type_is_si(domain))
+		ret = intel_pasid_setup_pass_through(iommu, domain, dev, pasid);
+	else if (domain->use_first_level)
+		ret = domain_setup_first_level(iommu, domain, dev, pasid);
+	else
+		ret = intel_pasid_setup_second_level(iommu, domain, dev, pasid);
+
+	return 0;
+}
+
 static int dmar_domain_attach_device(struct dmar_domain *domain,
 				     struct device *dev)
 {
@@ -2450,23 +2470,11 @@  static int dmar_domain_attach_device(struct dmar_domain *domain,
 	list_add(&info->link, &domain->devices);
 	spin_unlock_irqrestore(&domain->lock, flags);
 
-	/* PASID table is mandatory for a PCI device in scalable mode. */
-	if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
-		/* Setup the PASID entry for requests without PASID: */
-		if (hw_pass_through && domain_type_is_si(domain))
-			ret = intel_pasid_setup_pass_through(iommu, domain,
-					dev, IOMMU_DEF_RID_PASID);
-		else if (domain->use_first_level)
-			ret = domain_setup_first_level(iommu, domain, dev,
-					IOMMU_DEF_RID_PASID);
-		else
-			ret = intel_pasid_setup_second_level(iommu, domain,
-					dev, IOMMU_DEF_RID_PASID);
-		if (ret) {
-			dev_err(dev, "Setup RID2PASID failed\n");
-			device_block_translation(dev);
-			return ret;
-		}
+	ret = dmar_domain_attach_device_pasid(domain, iommu, dev,
+					      IOMMU_DEF_RID_PASID);
+	if (ret) {
+		dev_err(dev, "Setup RID2PASID failed\n");
+		device_block_translation(dev);
 	}
 
 	ret = domain_context_mapping(domain, dev);