All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATACH 1/1] iommu/vt-d: Use user privilege for RID2PASID translation
@ 2021-05-12  6:44 ` Lu Baolu
  0 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2021-05-12  6:44 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: ashok.raj, kevin.tian, jacob.jun.pan, yi.l.liu, sanjay.k.kumar,
	iommu, linux-kernel, Lu Baolu, Jacob Pan

When first-level page tables are used for IOVA translation, we use user
privilege by setting U/S bit in the page table entry. This is to make it
consistent with the second level translation, where the U/S enforcement
is not available. Clear the SRE (Supervisor Request Enable) field in the
pasid table entry of RID2PASID so that requests requesting the supervisor
privilege are blocked and treated as DMA remapping faults.

Suggested-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Fixes: b802d070a52a1 ("iommu/vt-d: Use iova over first level")
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 7 +++++--
 drivers/iommu/intel/pasid.c | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 708f430af1c4..f1742da42478 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2525,9 +2525,9 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
 				    struct device *dev,
 				    u32 pasid)
 {
-	int flags = PASID_FLAG_SUPERVISOR_MODE;
 	struct dma_pte *pgd = domain->pgd;
 	int agaw, level;
+	int flags = 0;
 
 	/*
 	 * Skip top levels of page tables for iommu which has
@@ -2543,7 +2543,10 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
 	if (level != 4 && level != 5)
 		return -EINVAL;
 
-	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
+	if (pasid != PASID_RID2PASID)
+		flags |= PASID_FLAG_SUPERVISOR_MODE;
+	if (level == 5)
+		flags |= PASID_FLAG_FL5LP;
 
 	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
 		flags |= PASID_FLAG_PAGE_SNOOP;
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 72646bafc52f..72dc84821dad 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -699,7 +699,8 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
 	 * Since it is a second level only translation setup, we should
 	 * set SRE bit as well (addresses are expected to be GPAs).
 	 */
-	pasid_set_sre(pte);
+	if (pasid != PASID_RID2PASID)
+		pasid_set_sre(pte);
 	pasid_set_present(pte);
 	pasid_flush_caches(iommu, pte, pasid, did);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [RESEND PATACH 1/1] iommu/vt-d: Use user privilege for RID2PASID translation
@ 2021-05-12  6:44 ` Lu Baolu
  0 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2021-05-12  6:44 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: kevin.tian, ashok.raj, sanjay.k.kumar, linux-kernel, iommu,
	jacob.jun.pan

When first-level page tables are used for IOVA translation, we use user
privilege by setting U/S bit in the page table entry. This is to make it
consistent with the second level translation, where the U/S enforcement
is not available. Clear the SRE (Supervisor Request Enable) field in the
pasid table entry of RID2PASID so that requests requesting the supervisor
privilege are blocked and treated as DMA remapping faults.

Suggested-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Fixes: b802d070a52a1 ("iommu/vt-d: Use iova over first level")
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 7 +++++--
 drivers/iommu/intel/pasid.c | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 708f430af1c4..f1742da42478 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2525,9 +2525,9 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
 				    struct device *dev,
 				    u32 pasid)
 {
-	int flags = PASID_FLAG_SUPERVISOR_MODE;
 	struct dma_pte *pgd = domain->pgd;
 	int agaw, level;
+	int flags = 0;
 
 	/*
 	 * Skip top levels of page tables for iommu which has
@@ -2543,7 +2543,10 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
 	if (level != 4 && level != 5)
 		return -EINVAL;
 
-	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
+	if (pasid != PASID_RID2PASID)
+		flags |= PASID_FLAG_SUPERVISOR_MODE;
+	if (level == 5)
+		flags |= PASID_FLAG_FL5LP;
 
 	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
 		flags |= PASID_FLAG_PAGE_SNOOP;
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 72646bafc52f..72dc84821dad 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -699,7 +699,8 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
 	 * Since it is a second level only translation setup, we should
 	 * set SRE bit as well (addresses are expected to be GPAs).
 	 */
-	pasid_set_sre(pte);
+	if (pasid != PASID_RID2PASID)
+		pasid_set_sre(pte);
 	pasid_set_present(pte);
 	pasid_flush_caches(iommu, pte, pasid, did);
 
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [RESEND PATACH 1/1] iommu/vt-d: Use user privilege for RID2PASID translation
  2021-05-12  6:44 ` Lu Baolu
@ 2021-05-12 17:03   ` Raj, Ashok
  -1 siblings, 0 replies; 6+ messages in thread
From: Raj, Ashok @ 2021-05-12 17:03 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Joerg Roedel, Will Deacon, kevin.tian, jacob.jun.pan, yi.l.liu,
	sanjay.k.kumar, iommu, linux-kernel, Jacob Pan, Ashok Raj

On Wed, May 12, 2021 at 02:44:26PM +0800, Lu Baolu wrote:
> When first-level page tables are used for IOVA translation, we use user
> privilege by setting U/S bit in the page table entry. This is to make it
> consistent with the second level translation, where the U/S enforcement
> is not available. Clear the SRE (Supervisor Request Enable) field in the
> pasid table entry of RID2PASID so that requests requesting the supervisor
> privilege are blocked and treated as DMA remapping faults.
> 
> Suggested-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Fixes: b802d070a52a1 ("iommu/vt-d: Use iova over first level")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/iommu.c | 7 +++++--
>  drivers/iommu/intel/pasid.c | 3 ++-
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 708f430af1c4..f1742da42478 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -2525,9 +2525,9 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
>  				    struct device *dev,
>  				    u32 pasid)
>  {
> -	int flags = PASID_FLAG_SUPERVISOR_MODE;
>  	struct dma_pte *pgd = domain->pgd;
>  	int agaw, level;
> +	int flags = 0;
>  
>  	/*
>  	 * Skip top levels of page tables for iommu which has
> @@ -2543,7 +2543,10 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
>  	if (level != 4 && level != 5)
>  		return -EINVAL;
>  
> -	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
> +	if (pasid != PASID_RID2PASID)
> +		flags |= PASID_FLAG_SUPERVISOR_MODE;
> +	if (level == 5)
> +		flags |= PASID_FLAG_FL5LP;

Given that we are still not bought into the supervisor PASID, should we make this an 
explicit request before turning on SUPERVISOR mode always? Since
pasid_set_sre() has no return, we have no way to fail it.



>  
>  	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
>  		flags |= PASID_FLAG_PAGE_SNOOP;
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index 72646bafc52f..72dc84821dad 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -699,7 +699,8 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
>  	 * Since it is a second level only translation setup, we should
>  	 * set SRE bit as well (addresses are expected to be GPAs).
>  	 */
> -	pasid_set_sre(pte);
> +	if (pasid != PASID_RID2PASID)
> +		pasid_set_sre(pte);
>  	pasid_set_present(pte);
>  	pasid_flush_caches(iommu, pte, pasid, did);
>  
> -- 
> 2.25.1
> 

-- 
Cheers,
Ashok

[Forgiveness is the attribute of the STRONG - Gandhi]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [RESEND PATACH 1/1] iommu/vt-d: Use user privilege for RID2PASID translation
@ 2021-05-12 17:03   ` Raj, Ashok
  0 siblings, 0 replies; 6+ messages in thread
From: Raj, Ashok @ 2021-05-12 17:03 UTC (permalink / raw)
  To: Lu Baolu
  Cc: kevin.tian, Ashok Raj, sanjay.k.kumar, linux-kernel, iommu,
	jacob.jun.pan, Will Deacon

On Wed, May 12, 2021 at 02:44:26PM +0800, Lu Baolu wrote:
> When first-level page tables are used for IOVA translation, we use user
> privilege by setting U/S bit in the page table entry. This is to make it
> consistent with the second level translation, where the U/S enforcement
> is not available. Clear the SRE (Supervisor Request Enable) field in the
> pasid table entry of RID2PASID so that requests requesting the supervisor
> privilege are blocked and treated as DMA remapping faults.
> 
> Suggested-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Fixes: b802d070a52a1 ("iommu/vt-d: Use iova over first level")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/iommu.c | 7 +++++--
>  drivers/iommu/intel/pasid.c | 3 ++-
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 708f430af1c4..f1742da42478 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -2525,9 +2525,9 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
>  				    struct device *dev,
>  				    u32 pasid)
>  {
> -	int flags = PASID_FLAG_SUPERVISOR_MODE;
>  	struct dma_pte *pgd = domain->pgd;
>  	int agaw, level;
> +	int flags = 0;
>  
>  	/*
>  	 * Skip top levels of page tables for iommu which has
> @@ -2543,7 +2543,10 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
>  	if (level != 4 && level != 5)
>  		return -EINVAL;
>  
> -	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
> +	if (pasid != PASID_RID2PASID)
> +		flags |= PASID_FLAG_SUPERVISOR_MODE;
> +	if (level == 5)
> +		flags |= PASID_FLAG_FL5LP;

Given that we are still not bought into the supervisor PASID, should we make this an 
explicit request before turning on SUPERVISOR mode always? Since
pasid_set_sre() has no return, we have no way to fail it.



>  
>  	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
>  		flags |= PASID_FLAG_PAGE_SNOOP;
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index 72646bafc52f..72dc84821dad 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -699,7 +699,8 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
>  	 * Since it is a second level only translation setup, we should
>  	 * set SRE bit as well (addresses are expected to be GPAs).
>  	 */
> -	pasid_set_sre(pte);
> +	if (pasid != PASID_RID2PASID)
> +		pasid_set_sre(pte);
>  	pasid_set_present(pte);
>  	pasid_flush_caches(iommu, pte, pasid, did);
>  
> -- 
> 2.25.1
> 

-- 
Cheers,
Ashok

[Forgiveness is the attribute of the STRONG - Gandhi]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [RESEND PATACH 1/1] iommu/vt-d: Use user privilege for RID2PASID translation
  2021-05-12 17:03   ` Raj, Ashok
@ 2021-05-13  3:06     ` Lu Baolu
  -1 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2021-05-13  3:06 UTC (permalink / raw)
  To: Raj, Ashok
  Cc: baolu.lu, Joerg Roedel, Will Deacon, kevin.tian, jacob.jun.pan,
	yi.l.liu, sanjay.k.kumar, iommu, linux-kernel, Jacob Pan

Hi Ashok,

On 5/13/21 1:03 AM, Raj, Ashok wrote:
> On Wed, May 12, 2021 at 02:44:26PM +0800, Lu Baolu wrote:
>> When first-level page tables are used for IOVA translation, we use user
>> privilege by setting U/S bit in the page table entry. This is to make it
>> consistent with the second level translation, where the U/S enforcement
>> is not available. Clear the SRE (Supervisor Request Enable) field in the
>> pasid table entry of RID2PASID so that requests requesting the supervisor
>> privilege are blocked and treated as DMA remapping faults.
>>
>> Suggested-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Fixes: b802d070a52a1 ("iommu/vt-d: Use iova over first level")
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   drivers/iommu/intel/iommu.c | 7 +++++--
>>   drivers/iommu/intel/pasid.c | 3 ++-
>>   2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 708f430af1c4..f1742da42478 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -2525,9 +2525,9 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
>>   				    struct device *dev,
>>   				    u32 pasid)
>>   {
>> -	int flags = PASID_FLAG_SUPERVISOR_MODE;
>>   	struct dma_pte *pgd = domain->pgd;
>>   	int agaw, level;
>> +	int flags = 0;
>>   
>>   	/*
>>   	 * Skip top levels of page tables for iommu which has
>> @@ -2543,7 +2543,10 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
>>   	if (level != 4 && level != 5)
>>   		return -EINVAL;
>>   
>> -	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
>> +	if (pasid != PASID_RID2PASID)
>> +		flags |= PASID_FLAG_SUPERVISOR_MODE;
>> +	if (level == 5)
>> +		flags |= PASID_FLAG_FL5LP;
> 
> Given that we are still not bought into the supervisor PASID, should we make this an
> explicit request before turning on SUPERVISOR mode always? Since
> pasid_set_sre() has no return, we have no way to fail it.
> 

The supervisor PASID is now supported in VT-d implementation. This patch
is only for RID2PASID. We need a separated patch to remove the superisor
pasid code once we reach a consensus.

Does this work for you?

Best regards,
baolu

> 
>>   
>>   	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
>>   		flags |= PASID_FLAG_PAGE_SNOOP;
>> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
>> index 72646bafc52f..72dc84821dad 100644
>> --- a/drivers/iommu/intel/pasid.c
>> +++ b/drivers/iommu/intel/pasid.c
>> @@ -699,7 +699,8 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
>>   	 * Since it is a second level only translation setup, we should
>>   	 * set SRE bit as well (addresses are expected to be GPAs).
>>   	 */
>> -	pasid_set_sre(pte);
>> +	if (pasid != PASID_RID2PASID)
>> +		pasid_set_sre(pte);
>>   	pasid_set_present(pte);
>>   	pasid_flush_caches(iommu, pte, pasid, did);
>>   
>> -- 
>> 2.25.1
>>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [RESEND PATACH 1/1] iommu/vt-d: Use user privilege for RID2PASID translation
@ 2021-05-13  3:06     ` Lu Baolu
  0 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2021-05-13  3:06 UTC (permalink / raw)
  To: Raj, Ashok
  Cc: kevin.tian, sanjay.k.kumar, linux-kernel, iommu, jacob.jun.pan,
	Will Deacon

Hi Ashok,

On 5/13/21 1:03 AM, Raj, Ashok wrote:
> On Wed, May 12, 2021 at 02:44:26PM +0800, Lu Baolu wrote:
>> When first-level page tables are used for IOVA translation, we use user
>> privilege by setting U/S bit in the page table entry. This is to make it
>> consistent with the second level translation, where the U/S enforcement
>> is not available. Clear the SRE (Supervisor Request Enable) field in the
>> pasid table entry of RID2PASID so that requests requesting the supervisor
>> privilege are blocked and treated as DMA remapping faults.
>>
>> Suggested-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Fixes: b802d070a52a1 ("iommu/vt-d: Use iova over first level")
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   drivers/iommu/intel/iommu.c | 7 +++++--
>>   drivers/iommu/intel/pasid.c | 3 ++-
>>   2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 708f430af1c4..f1742da42478 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -2525,9 +2525,9 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
>>   				    struct device *dev,
>>   				    u32 pasid)
>>   {
>> -	int flags = PASID_FLAG_SUPERVISOR_MODE;
>>   	struct dma_pte *pgd = domain->pgd;
>>   	int agaw, level;
>> +	int flags = 0;
>>   
>>   	/*
>>   	 * Skip top levels of page tables for iommu which has
>> @@ -2543,7 +2543,10 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
>>   	if (level != 4 && level != 5)
>>   		return -EINVAL;
>>   
>> -	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
>> +	if (pasid != PASID_RID2PASID)
>> +		flags |= PASID_FLAG_SUPERVISOR_MODE;
>> +	if (level == 5)
>> +		flags |= PASID_FLAG_FL5LP;
> 
> Given that we are still not bought into the supervisor PASID, should we make this an
> explicit request before turning on SUPERVISOR mode always? Since
> pasid_set_sre() has no return, we have no way to fail it.
> 

The supervisor PASID is now supported in VT-d implementation. This patch
is only for RID2PASID. We need a separated patch to remove the superisor
pasid code once we reach a consensus.

Does this work for you?

Best regards,
baolu

> 
>>   
>>   	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
>>   		flags |= PASID_FLAG_PAGE_SNOOP;
>> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
>> index 72646bafc52f..72dc84821dad 100644
>> --- a/drivers/iommu/intel/pasid.c
>> +++ b/drivers/iommu/intel/pasid.c
>> @@ -699,7 +699,8 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
>>   	 * Since it is a second level only translation setup, we should
>>   	 * set SRE bit as well (addresses are expected to be GPAs).
>>   	 */
>> -	pasid_set_sre(pte);
>> +	if (pasid != PASID_RID2PASID)
>> +		pasid_set_sre(pte);
>>   	pasid_set_present(pte);
>>   	pasid_flush_caches(iommu, pte, pasid, did);
>>   
>> -- 
>> 2.25.1
>>
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-13  3:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-12  6:44 [RESEND PATACH 1/1] iommu/vt-d: Use user privilege for RID2PASID translation Lu Baolu
2021-05-12  6:44 ` Lu Baolu
2021-05-12 17:03 ` Raj, Ashok
2021-05-12 17:03   ` Raj, Ashok
2021-05-13  3:06   ` Lu Baolu
2021-05-13  3:06     ` Lu Baolu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.