diff mbox series

[topic/core-for-ci,v2] iommu/vt-d: Check domain flags before setting snp bit in page-control

Message ID 20230824224249.365665-1-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show
Series [topic/core-for-ci,v2] iommu/vt-d: Check domain flags before setting snp bit in page-control | expand

Commit Message

Sripada, Radhakrishna Aug. 24, 2023, 10:42 p.m. UTC 
From: Ashok Raj <ashok.raj@intel.com>

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/iommu/intel/iommu.c | 2 +-
 drivers/iommu/intel/pasid.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Jani Nikula Aug. 25, 2023, 6:54 a.m. UTC | #1 
On Thu, 24 Aug 2023, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote:
> From: Ashok Raj <ashok.raj@intel.com>
>

The *why* goes here, along with a link to a gitlab issue.

Please don't expect topic/core-for-ci to have lower standards than any
other branches. That's not the case. On the contrary, you'll need the
*additional* justification for the commit being in topic/core-for-ci,
and the gitlab issue.


BR,
Jani.


> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/iommu/intel/iommu.c | 2 +-
>  drivers/iommu/intel/pasid.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 5c8c5cdc36cf..71da6f818e96 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -2150,7 +2150,7 @@ __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
>  	if ((prot & (DMA_PTE_READ|DMA_PTE_WRITE)) == 0)
>  		return -EINVAL;
>  
> -	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
> +	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE);
>  	attr |= DMA_FL_PTE_PRESENT;
>  	if (domain->use_first_level) {
>  		attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US | DMA_FL_PTE_ACCESS;
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index c5d479770e12..a057ecf84d82 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -538,7 +538,7 @@ int intel_pasid_setup_first_level(struct intel_iommu *iommu,
>  	if (flags & PASID_FLAG_FL5LP)
>  		pasid_set_flpm(pte, 1);
>  
> -	if (flags & PASID_FLAG_PAGE_SNOOP)
> +	if ((flags & PASID_FLAG_PAGE_SNOOP) && ecap_sc_support(iommu->ecap))
>  		pasid_set_pgsnp(pte);
>  
>  	pasid_set_domain_id(pte, did);
Sripada, Radhakrishna Aug. 25, 2023, 2:15 p.m. UTC | #2 
I was trying this as a solution for the Pipe fault errors. However, I still see
The pipe fault errors which do not occur all the time.

Will update the explanation in my follow up patches.

Thanks,
Radhakrishna Sripada

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Thursday, August 24, 2023 11:54 PM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Raj, Ashok <ashok.raj@intel.com>
> Subject: Re: [Intel-gfx] [topic/core-for-ci v2] iommu/vt-d: Check domain flags
> before setting snp bit in page-control
> 
> On Thu, 24 Aug 2023, Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> wrote:
> > From: Ashok Raj <ashok.raj@intel.com>
> >
> 
> The *why* goes here, along with a link to a gitlab issue.
> 
> Please don't expect topic/core-for-ci to have lower standards than any
> other branches. That's not the case. On the contrary, you'll need the
> *additional* justification for the commit being in topic/core-for-ci,
> and the gitlab issue.
> 
> 
> BR,
> Jani.
> 
> 
> > Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  drivers/iommu/intel/iommu.c | 2 +-
> >  drivers/iommu/intel/pasid.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 5c8c5cdc36cf..71da6f818e96 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -2150,7 +2150,7 @@ __domain_mapping(struct dmar_domain *domain,
> unsigned long iov_pfn,
> >  	if ((prot & (DMA_PTE_READ|DMA_PTE_WRITE)) == 0)
> >  		return -EINVAL;
> >
> > -	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
> > +	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE);
> >  	attr |= DMA_FL_PTE_PRESENT;
> >  	if (domain->use_first_level) {
> >  		attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US |
> DMA_FL_PTE_ACCESS;
> > diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> > index c5d479770e12..a057ecf84d82 100644
> > --- a/drivers/iommu/intel/pasid.c
> > +++ b/drivers/iommu/intel/pasid.c
> > @@ -538,7 +538,7 @@ int intel_pasid_setup_first_level(struct intel_iommu
> *iommu,
> >  	if (flags & PASID_FLAG_FL5LP)
> >  		pasid_set_flpm(pte, 1);
> >
> > -	if (flags & PASID_FLAG_PAGE_SNOOP)
> > +	if ((flags & PASID_FLAG_PAGE_SNOOP) && ecap_sc_support(iommu-
> >ecap))
> >  		pasid_set_pgsnp(pte);
> >
> >  	pasid_set_domain_id(pte, did);
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
Rodrigo Vivi Aug. 25, 2023, 5:23 p.m. UTC | #3 
On Fri, Aug 25, 2023 at 02:15:27PM +0000, Sripada, Radhakrishna wrote:
> I was trying this as a solution for the Pipe fault errors. However, I still see
> The pipe fault errors which do not occur all the time.

We should avoid overloading CI with tests. But if needed because we cannot
reproduce locally but only on CI, please use the try-bot list instead of
this one.

And in the very last case where this list needs to be used, please use
another prefix like CI or HAX, but not the topic/core-for-ci, otherwise
we will think you are already asking to get that merged there.

> 
> Will update the explanation in my follow up patches.

> 
> Thanks,
> Radhakrishna Sripada
> 
> > -----Original Message-----
> > From: Jani Nikula <jani.nikula@linux.intel.com>
> > Sent: Thursday, August 24, 2023 11:54 PM
> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Raj, Ashok <ashok.raj@intel.com>
> > Subject: Re: [Intel-gfx] [topic/core-for-ci v2] iommu/vt-d: Check domain flags
> > before setting snp bit in page-control
> > 
> > On Thu, 24 Aug 2023, Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > wrote:
> > > From: Ashok Raj <ashok.raj@intel.com>
> > >
> > 
> > The *why* goes here, along with a link to a gitlab issue.
> > 
> > Please don't expect topic/core-for-ci to have lower standards than any
> > other branches. That's not the case. On the contrary, you'll need the
> > *additional* justification for the commit being in topic/core-for-ci,
> > and the gitlab issue.
> > 
> > 
> > BR,
> > Jani.
> > 
> > 
> > > Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > ---
> > >  drivers/iommu/intel/iommu.c | 2 +-
> > >  drivers/iommu/intel/pasid.c | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > > index 5c8c5cdc36cf..71da6f818e96 100644
> > > --- a/drivers/iommu/intel/iommu.c
> > > +++ b/drivers/iommu/intel/iommu.c
> > > @@ -2150,7 +2150,7 @@ __domain_mapping(struct dmar_domain *domain,
> > unsigned long iov_pfn,
> > >  	if ((prot & (DMA_PTE_READ|DMA_PTE_WRITE)) == 0)
> > >  		return -EINVAL;
> > >
> > > -	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
> > > +	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE);
> > >  	attr |= DMA_FL_PTE_PRESENT;
> > >  	if (domain->use_first_level) {
> > >  		attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US |
> > DMA_FL_PTE_ACCESS;
> > > diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> > > index c5d479770e12..a057ecf84d82 100644
> > > --- a/drivers/iommu/intel/pasid.c
> > > +++ b/drivers/iommu/intel/pasid.c
> > > @@ -538,7 +538,7 @@ int intel_pasid_setup_first_level(struct intel_iommu
> > *iommu,
> > >  	if (flags & PASID_FLAG_FL5LP)
> > >  		pasid_set_flpm(pte, 1);
> > >
> > > -	if (flags & PASID_FLAG_PAGE_SNOOP)
> > > +	if ((flags & PASID_FLAG_PAGE_SNOOP) && ecap_sc_support(iommu-
> > >ecap))
> > >  		pasid_set_pgsnp(pte);
> > >
> > >  	pasid_set_domain_id(pte, did);
> > 
> > --
> > Jani Nikula, Intel Open Source Graphics Center
Sripada, Radhakrishna Aug. 26, 2023, 12:47 a.m. UTC | #4 
> -----Original Message-----
> From: Rodrigo Vivi <rodrigo.vivi@kernel.org>
> Sent: Friday, August 25, 2023 10:24 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>; intel-gfx@lists.freedesktop.org;
> Raj, Ashok <ashok.raj@intel.com>
> Subject: Re: [Intel-gfx] [topic/core-for-ci v2] iommu/vt-d: Check domain flags
> before setting snp bit in page-control
> 
> On Fri, Aug 25, 2023 at 02:15:27PM +0000, Sripada, Radhakrishna wrote:
> > I was trying this as a solution for the Pipe fault errors. However, I still see
> > The pipe fault errors which do not occur all the time.
> 
> We should avoid overloading CI with tests. But if needed because we cannot
> reproduce locally but only on CI, please use the try-bot list instead of
> this one.
> 
> And in the very last case where this list needs to be used, please use
> another prefix like CI or HAX, but not the topic/core-for-ci, otherwise
> we will think you are already asking to get that merged there.

Sure will update that. This is a tricky bug that resurfaced after removing
Iommu=pt kernel command line. I already took down one CI resume system for
debug but the issue occurrence now seems to be intermittent and difficult to narrow down.

Anyways thank you Rodrigo will add the CI tag for later if try bot does not help.

Thanks,
Radhakrishna Sripada
> 
> >
> > Will update the explanation in my follow up patches.
> 
> >
> > Thanks,
> > Radhakrishna Sripada
> >
> > > -----Original Message-----
> > > From: Jani Nikula <jani.nikula@linux.intel.com>
> > > Sent: Thursday, August 24, 2023 11:54 PM
> > > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> > > gfx@lists.freedesktop.org
> > > Cc: Raj, Ashok <ashok.raj@intel.com>
> > > Subject: Re: [Intel-gfx] [topic/core-for-ci v2] iommu/vt-d: Check domain flags
> > > before setting snp bit in page-control
> > >
> > > On Thu, 24 Aug 2023, Radhakrishna Sripada
> <radhakrishna.sripada@intel.com>
> > > wrote:
> > > > From: Ashok Raj <ashok.raj@intel.com>
> > > >
> > >
> > > The *why* goes here, along with a link to a gitlab issue.
> > >
> > > Please don't expect topic/core-for-ci to have lower standards than any
> > > other branches. That's not the case. On the contrary, you'll need the
> > > *additional* justification for the commit being in topic/core-for-ci,
> > > and the gitlab issue.
> > >
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > > > Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> > > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > ---
> > > >  drivers/iommu/intel/iommu.c | 2 +-
> > > >  drivers/iommu/intel/pasid.c | 2 +-
> > > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > > > index 5c8c5cdc36cf..71da6f818e96 100644
> > > > --- a/drivers/iommu/intel/iommu.c
> > > > +++ b/drivers/iommu/intel/iommu.c
> > > > @@ -2150,7 +2150,7 @@ __domain_mapping(struct dmar_domain
> *domain,
> > > unsigned long iov_pfn,
> > > >  	if ((prot & (DMA_PTE_READ|DMA_PTE_WRITE)) == 0)
> > > >  		return -EINVAL;
> > > >
> > > > -	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
> > > > +	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE);
> > > >  	attr |= DMA_FL_PTE_PRESENT;
> > > >  	if (domain->use_first_level) {
> > > >  		attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US |
> > > DMA_FL_PTE_ACCESS;
> > > > diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> > > > index c5d479770e12..a057ecf84d82 100644
> > > > --- a/drivers/iommu/intel/pasid.c
> > > > +++ b/drivers/iommu/intel/pasid.c
> > > > @@ -538,7 +538,7 @@ int intel_pasid_setup_first_level(struct intel_iommu
> > > *iommu,
> > > >  	if (flags & PASID_FLAG_FL5LP)
> > > >  		pasid_set_flpm(pte, 1);
> > > >
> > > > -	if (flags & PASID_FLAG_PAGE_SNOOP)
> > > > +	if ((flags & PASID_FLAG_PAGE_SNOOP) && ecap_sc_support(iommu-
> > > >ecap))
> > > >  		pasid_set_pgsnp(pte);
> > > >
> > > >  	pasid_set_domain_id(pte, did);
> > >
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 5c8c5cdc36cf..71da6f818e96 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2150,7 +2150,7 @@  __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
 	if ((prot & (DMA_PTE_READ|DMA_PTE_WRITE)) == 0)
 		return -EINVAL;
 
-	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
+	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE);
 	attr |= DMA_FL_PTE_PRESENT;
 	if (domain->use_first_level) {
 		attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US | DMA_FL_PTE_ACCESS;
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index c5d479770e12..a057ecf84d82 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -538,7 +538,7 @@  int intel_pasid_setup_first_level(struct intel_iommu *iommu,
 	if (flags & PASID_FLAG_FL5LP)
 		pasid_set_flpm(pte, 1);
 
-	if (flags & PASID_FLAG_PAGE_SNOOP)
+	if ((flags & PASID_FLAG_PAGE_SNOOP) && ecap_sc_support(iommu->ecap))
 		pasid_set_pgsnp(pte);
 
 	pasid_set_domain_id(pte, did);