On VT-d platforms, RID_PASID is used for DMA request without PASID. We
should not treat RID_PASID special instead let it be allocated from the
global SVA PASID number space.

Consequently, for devices also do DMA with PASID, drivers will not worry
about conflicts when it comes to allocating PASIDs for in-kernel DMA.

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

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 9f737ef55463..61c06f7ad8f7 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3956,6 +3956,10 @@ int __init intel_iommu_init(void)
 
 	intel_iommu_enabled = 1;
 
+#ifdef CONFIG_INTEL_IOMMU_SVM
+	/* Reserved RID_PASID from the global namespace for legacy DMA */
+	iommu_sva_reserve_pasid(PASID_RID2PASID, PASID_RID2PASID);
+#endif
 	return 0;
 
 out_free_dmar:
-- 
2.25.1

On 3/28/23 7:21 AM, Jacob Pan wrote:
> On VT-d platforms, RID_PASID is used for DMA request without PASID. We
> should not treat RID_PASID special instead let it be allocated from the
> global SVA PASID number space.

It's same to AMD and ARM SMMUv3, right? They also need an explicit
reservation of PASID 0.

> 
> Consequently, for devices also do DMA with PASID, drivers will not worry
> about conflicts when it comes to allocating PASIDs for in-kernel DMA.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
>   drivers/iommu/intel/iommu.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 9f737ef55463..61c06f7ad8f7 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -3956,6 +3956,10 @@ int __init intel_iommu_init(void)
>   
>   	intel_iommu_enabled = 1;
>   
> +#ifdef CONFIG_INTEL_IOMMU_SVM

Do we really need this #ifdef? IOMMU_SVA is selected by INTEL_IOMMU_SVM,
right? So if CONFIG_INTEL_IOMMU_SVM is not set,
iommu_sva_reserve_pasid() is just a dumb.

> +	/* Reserved RID_PASID from the global namespace for legacy DMA */
> +	iommu_sva_reserve_pasid(PASID_RID2PASID, PASID_RID2PASID);
> +#endif
>   	return 0;
>   
>   out_free_dmar:

Best regards,
baolu

On 3/28/2023 10:50 AM, Baolu Lu wrote:
> On 3/28/23 7:21 AM, Jacob Pan wrote:
>> On VT-d platforms, RID_PASID is used for DMA request without PASID. We
>> should not treat RID_PASID special instead let it be allocated from the
>> global SVA PASID number space.
> 
> It's same to AMD and ARM SMMUv3, right? They also need an explicit
> reservation of PASID 0.

Yes for AMD driver. (Requests from the I/O device without a PASID are treated as
if they have PASID of 0).


-Vasant

Hi Baolu,

On Tue, 28 Mar 2023 13:20:19 +0800, Baolu Lu <baolu.lu@linux.intel.com>
wrote:

> On 3/28/23 7:21 AM, Jacob Pan wrote:
> > On VT-d platforms, RID_PASID is used for DMA request without PASID. We
> > should not treat RID_PASID special instead let it be allocated from the
> > global SVA PASID number space.  
> 
> It's same to AMD and ARM SMMUv3, right? They also need an explicit
> reservation of PASID 0.
> 
yes, all IOMMU drivers need to do that. I will give it a try but might need
help to place the call.

> > 
> > Consequently, for devices also do DMA with PASID, drivers will not worry
> > about conflicts when it comes to allocating PASIDs for in-kernel DMA.
> > 
> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > ---
> >   drivers/iommu/intel/iommu.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 9f737ef55463..61c06f7ad8f7 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -3956,6 +3956,10 @@ int __init intel_iommu_init(void)
> >   
> >   	intel_iommu_enabled = 1;
> >   
> > +#ifdef CONFIG_INTEL_IOMMU_SVM  
> 
> Do we really need this #ifdef? IOMMU_SVA is selected by INTEL_IOMMU_SVM,
> right? So if CONFIG_INTEL_IOMMU_SVM is not set,
> iommu_sva_reserve_pasid() is just a dumb.
> 
good catch! will remove

> > +	/* Reserved RID_PASID from the global namespace for legacy DMA
> > */
> > +	iommu_sva_reserve_pasid(PASID_RID2PASID, PASID_RID2PASID);
> > +#endif
> >   	return 0;
> >   
> >   out_free_dmar:  
> 
> Best regards,
> baolu


Thanks,

Jacob

Hi Baolu,

On Tue, 28 Mar 2023 09:29:19 -0700, Jacob Pan
<jacob.jun.pan@linux.intel.com> wrote:

> > > On VT-d platforms, RID_PASID is used for DMA request without PASID. We
> > > should not treat RID_PASID special instead let it be allocated from
> > > the global SVA PASID number space.    
> > 
> > It's same to AMD and ARM SMMUv3, right? They also need an explicit
> > reservation of PASID 0.
> >   
> yes, all IOMMU drivers need to do that. I will give it a try but might
> need help to place the call.
It might be simpler to just let SVA code allocate from 1 up instead of 0
(as is in the current code). Global PASID allocator would still allow the
full range from 0 to max. Then there is no change to architectures that
don't support non-zero RID_PASID. For VT-d, it would still work in the
future when we have nonzero RID_PASID. is that reasonable?

Thanks,

Jacob

On 3/29/23 4:52 AM, Jacob Pan wrote:
> On Tue, 28 Mar 2023 09:29:19 -0700, Jacob Pan
> <jacob.jun.pan@linux.intel.com>  wrote:
> 
>>>> On VT-d platforms, RID_PASID is used for DMA request without PASID. We
>>>> should not treat RID_PASID special instead let it be allocated from
>>>> the global SVA PASID number space.
>>> It's same to AMD and ARM SMMUv3, right? They also need an explicit
>>> reservation of PASID 0.
>>>    
>> yes, all IOMMU drivers need to do that. I will give it a try but might
>> need help to place the call.
> It might be simpler to just let SVA code allocate from 1 up instead of 0
> (as is in the current code). Global PASID allocator would still allow the
> full range from 0 to max. Then there is no change to architectures that
> don't support non-zero RID_PASID. For VT-d, it would still work in the
> future when we have nonzero RID_PASID. is that reasonable?

Yes. It's reasonable from the status quo.

Best regards,
baolu