iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs
@ 2019-07-20  2:01 Lu Baolu
  2019-07-22  5:21 ` Prakhya, Sai Praneeth
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Lu Baolu @ 2019-07-20  2:01 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: kevin.tian, ashok.raj, linux-kernel, iommu, jacob.jun.pan

PASID support and enable bit in the context entry isn't the right
indicator for the type of tables (legacy or scalable mode). Check
the DMA_RTADDR_SMT bit in the root context pointer instead.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Sai Praneeth <sai.praneeth.prakhya@intel.com>
Fixes: dd5142ca5d24b ("iommu/vt-d: Add debugfs support to show scalable mode DMAR table internals")
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu-debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu-debugfs.c b/drivers/iommu/intel-iommu-debugfs.c
index 73a552914455..e31c3b416351 100644
--- a/drivers/iommu/intel-iommu-debugfs.c
+++ b/drivers/iommu/intel-iommu-debugfs.c
@@ -235,7 +235,7 @@ static void ctx_tbl_walk(struct seq_file *m, struct intel_iommu *iommu, u16 bus)
 		tbl_wlk.ctx_entry = context;
 		m->private = &tbl_wlk;
 
-		if (pasid_supported(iommu) && is_pasid_enabled(context)) {
+		if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) & DMA_RTADDR_SMT) {
 			pasid_dir_ptr = context->lo & VTD_PAGE_MASK;
 			pasid_dir_size = get_pasid_dir_size(context);
 			pasid_dir_walk(m, pasid_dir_ptr, pasid_dir_size);
-- 
2.17.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: [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs
  2019-07-20  2:01 [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs Lu Baolu
@ 2019-07-22  5:21 ` Prakhya, Sai Praneeth
  2019-07-25  1:40   ` Lu Baolu
  2019-08-09  5:36 ` Lu Baolu
  2019-08-09 15:29 ` Joerg Roedel
  2 siblings, 1 reply; 6+ messages in thread
From: Prakhya, Sai Praneeth @ 2019-07-22  5:21 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, David Woodhouse
  Cc: Tian, Kevin, Raj, Ashok, linux-kernel, iommu, Pan, Jacob jun

Hi Allen,

> diff --git a/drivers/iommu/intel-iommu-debugfs.c b/drivers/iommu/intel-
> iommu-debugfs.c
> index 73a552914455..e31c3b416351 100644
> --- a/drivers/iommu/intel-iommu-debugfs.c
> +++ b/drivers/iommu/intel-iommu-debugfs.c
> @@ -235,7 +235,7 @@ static void ctx_tbl_walk(struct seq_file *m, struct
> intel_iommu *iommu, u16 bus)
>  		tbl_wlk.ctx_entry = context;
>  		m->private = &tbl_wlk;
> 
> -		if (pasid_supported(iommu) && is_pasid_enabled(context)) {
> +		if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) &
> DMA_RTADDR_SMT) {

Thanks for adding this, I do believe this is a good addition but I also think that we might 
need "is_pasid_enabled()" as well. It checks if PASIDE bit in context entry is enabled or not.

I am thinking that even though DMAR might be using scalable root and context table, the entry 
itself should have PASIDE bit set. Did I miss something here?

And I also think a macro would be better so that it could reused elsewhere (if need be).

Regards,
Sai
_______________________________________________
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: [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs
  2019-07-22  5:21 ` Prakhya, Sai Praneeth
@ 2019-07-25  1:40   ` Lu Baolu
  2019-07-26 17:17     ` Prakhya, Sai Praneeth
  0 siblings, 1 reply; 6+ messages in thread
From: Lu Baolu @ 2019-07-25  1:40 UTC (permalink / raw)
  To: Prakhya, Sai Praneeth, Joerg Roedel, David Woodhouse
  Cc: Tian, Kevin, Raj, Ashok, linux-kernel, iommu, Pan, Jacob jun

Hi Sai,

On 7/22/19 1:21 PM, Prakhya, Sai Praneeth wrote:
> Hi Allen,
> 
>> diff --git a/drivers/iommu/intel-iommu-debugfs.c b/drivers/iommu/intel-
>> iommu-debugfs.c
>> index 73a552914455..e31c3b416351 100644
>> --- a/drivers/iommu/intel-iommu-debugfs.c
>> +++ b/drivers/iommu/intel-iommu-debugfs.c
>> @@ -235,7 +235,7 @@ static void ctx_tbl_walk(struct seq_file *m, struct
>> intel_iommu *iommu, u16 bus)
>>   		tbl_wlk.ctx_entry = context;
>>   		m->private = &tbl_wlk;
>>
>> -		if (pasid_supported(iommu) && is_pasid_enabled(context)) {
>> +		if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) &
>> DMA_RTADDR_SMT) {
> 
> Thanks for adding this, I do believe this is a good addition but I also think that we might
> need "is_pasid_enabled()" as well. It checks if PASIDE bit in context entry is enabled or not.
> 
> I am thinking that even though DMAR might be using scalable root and context table, the entry
> itself should have PASIDE bit set. Did I miss something here?

No matter the PASIDE bit set or not, IOMMU always uses the scalable mode
page table if scalable mode is enabled. If PASIDE is set, requests with
PASID will be handled. Otherwise, requests with PASID will be blocked
(but request without PASID will always be handled).

We are dumpling the page table of the IOMMU, so we only care about what
page table format it is using. Do I understand it right>

Best regards,
Baolu

> 
> And I also think a macro would be better so that it could reused elsewhere (if need be).
> 
> Regards,
> Sai
> 
_______________________________________________
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: [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs
  2019-07-25  1:40   ` Lu Baolu
@ 2019-07-26 17:17     ` Prakhya, Sai Praneeth
  0 siblings, 0 replies; 6+ messages in thread
From: Prakhya, Sai Praneeth @ 2019-07-26 17:17 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, David Woodhouse
  Cc: Tian, Kevin, Raj, Ashok, linux-kernel, iommu, Pan, Jacob jun

> On 7/22/19 1:21 PM, Prakhya, Sai Praneeth wrote:
> > Hi Allen,
> >
> >> diff --git a/drivers/iommu/intel-iommu-debugfs.c
> >> b/drivers/iommu/intel- iommu-debugfs.c index
> >> 73a552914455..e31c3b416351 100644
> >> --- a/drivers/iommu/intel-iommu-debugfs.c
> >> +++ b/drivers/iommu/intel-iommu-debugfs.c
> >> @@ -235,7 +235,7 @@ static void ctx_tbl_walk(struct seq_file *m,
> >> struct intel_iommu *iommu, u16 bus)
> >>   		tbl_wlk.ctx_entry = context;
> >>   		m->private = &tbl_wlk;
> >>
> >> -		if (pasid_supported(iommu) && is_pasid_enabled(context)) {
> >> +		if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) &
> >> DMA_RTADDR_SMT) {
> >
> > Thanks for adding this, I do believe this is a good addition but I
> > also think that we might need "is_pasid_enabled()" as well. It checks if PASIDE
> bit in context entry is enabled or not.
> >
> > I am thinking that even though DMAR might be using scalable root and
> > context table, the entry itself should have PASIDE bit set. Did I miss something
> here?
> 
> No matter the PASIDE bit set or not, IOMMU always uses the scalable mode
> page table if scalable mode is enabled. If PASIDE is set, requests with PASID will
> be handled. Otherwise, requests with PASID will be blocked (but request without
> PASID will always be handled).
> 
> We are dumpling the page table of the IOMMU, so we only care about what
> page table format it is using. Do I understand it right>

Thanks! Baolu, for the explanation. Yes, it makes sense and I agree that we don't need the extra check for PASIDE bit.

I have tested this change on scalable/legacy DMAR's with/without iommu=pt and it works :)

Regards,
Sai
_______________________________________________
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: [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs
  2019-07-20  2:01 [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs Lu Baolu
  2019-07-22  5:21 ` Prakhya, Sai Praneeth
@ 2019-08-09  5:36 ` Lu Baolu
  2019-08-09 15:29 ` Joerg Roedel
  2 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2019-08-09  5:36 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: kevin.tian, ashok.raj, linux-kernel, iommu, jacob.jun.pan

Hi Joerg,

Just a friendly reminder. What do you think of this fix?

Best regards,
Lu Baolu

On 7/20/19 10:01 AM, Lu Baolu wrote:
> PASID support and enable bit in the context entry isn't the right
> indicator for the type of tables (legacy or scalable mode). Check
> the DMA_RTADDR_SMT bit in the root context pointer instead.
> 
> Cc: Ashok Raj <ashok.raj@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> Fixes: dd5142ca5d24b ("iommu/vt-d: Add debugfs support to show scalable mode DMAR table internals")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>   drivers/iommu/intel-iommu-debugfs.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel-iommu-debugfs.c b/drivers/iommu/intel-iommu-debugfs.c
> index 73a552914455..e31c3b416351 100644
> --- a/drivers/iommu/intel-iommu-debugfs.c
> +++ b/drivers/iommu/intel-iommu-debugfs.c
> @@ -235,7 +235,7 @@ static void ctx_tbl_walk(struct seq_file *m, struct intel_iommu *iommu, u16 bus)
>   		tbl_wlk.ctx_entry = context;
>   		m->private = &tbl_wlk;
>   
> -		if (pasid_supported(iommu) && is_pasid_enabled(context)) {
> +		if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) & DMA_RTADDR_SMT) {
>   			pasid_dir_ptr = context->lo & VTD_PAGE_MASK;
>   			pasid_dir_size = get_pasid_dir_size(context);
>   			pasid_dir_walk(m, pasid_dir_ptr, pasid_dir_size);
> 
_______________________________________________
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: [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs
  2019-07-20  2:01 [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs Lu Baolu
  2019-07-22  5:21 ` Prakhya, Sai Praneeth
  2019-08-09  5:36 ` Lu Baolu
@ 2019-08-09 15:29 ` Joerg Roedel
  2 siblings, 0 replies; 6+ messages in thread
From: Joerg Roedel @ 2019-08-09 15:29 UTC (permalink / raw)
  To: Lu Baolu
  Cc: kevin.tian, ashok.raj, linux-kernel, iommu, jacob.jun.pan,
	David Woodhouse

On Sat, Jul 20, 2019 at 10:01:26AM +0800, Lu Baolu wrote:
>  drivers/iommu/intel-iommu-debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.
_______________________________________________
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:[~2019-08-09 15:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-20  2:01 [PATCH 1/1] iommu/vt-d: Correctly check format of page table in debugfs Lu Baolu
2019-07-22  5:21 ` Prakhya, Sai Praneeth
2019-07-25  1:40   ` Lu Baolu
2019-07-26 17:17     ` Prakhya, Sai Praneeth
2019-08-09  5:36 ` Lu Baolu
2019-08-09 15:29 ` Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).