[PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded mapping on powerpc

[PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded mapping on powerpc

[Aneesh Kumar K.V]

The driver was requesting for a writethrough mapping. But with thoses
flags we will end up with a SAO mapping because we now have memory
conherence always enabled. ie, the existing mapping will end up with
a WIMG value 0b1110 which is Strong Access Order.

Update this to use cache inhibitted guarded mapping

Cc: Mike Marciniszyn <infinipath@intel.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
Changes from v1:
* resend because V1 never reached mailing list.

NOTE: This is only compile tested and I am also not sure why ppc64 needs special handling.
We need this patch because the series at http://mid.gmane.org/1460182444-2468-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com
will drop _PAGE_WRITETHRU

 drivers/infiniband/hw/qib/qib_file_ops.c | 5 +----
 drivers/infiniband/hw/qib/qib_pcie.c     | 6 ------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index e449e394963f..a3d593f546ec 100644
--- a/drivers/infiniband/hw/qib/qib_file_ops.c
+++ b/drivers/infiniband/hw/qib/qib_file_ops.c
@@ -822,10 +822,7 @@ static int mmap_piobufs(struct vm_area_struct *vma,
 	phys = dd->physaddr + piobufs;
 
 #if defined(__powerpc__)
-	/* There isn't a generic way to specify writethrough mappings */
-	pgprot_val(vma->vm_page_prot) |= _PAGE_NO_CACHE;
-	pgprot_val(vma->vm_page_prot) |= _PAGE_WRITETHRU;
-	pgprot_val(vma->vm_page_prot) &= ~_PAGE_GUARDED;
+	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
 #endif
 
 	/*
diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index 4758a3801ae8..6abe1c621aa4 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -144,13 +144,7 @@ int qib_pcie_ddinit(struct qib_devdata *dd, struct pci_dev *pdev,
 	addr = pci_resource_start(pdev, 0);
 	len = pci_resource_len(pdev, 0);
 
-#if defined(__powerpc__)
-	/* There isn't a generic way to specify writethrough mappings */
-	dd->kregbase = __ioremap(addr, len, _PAGE_NO_CACHE | _PAGE_WRITETHRU);
-#else
 	dd->kregbase = ioremap_nocache(addr, len);
-#endif
-
 	if (!dd->kregbase)
 		return -ENOMEM;
 
-- 
2.5.0

RE: [PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded mapping on powerpc

[Marciniszyn, Mike]

> From: Aneesh Kumar K.V [mailto:aneesh.kumar@linux.vnet.ibm.com]
> Sent: Wednesday, April 20, 2016 3:58 AM
> Subject: [PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded
> mapping on powerpc
>=20

Doug, can you fix up the summary to git the spelling error and powerpc/infi=
niband:  -> IB/qib:

Thanks for the patch!

Acked-by: Mike Marciniszyn <mike.marciniszyn@intel.com>

Re: [PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded mapping on powerpc

[Michael Ellerman]

On Wed, 2016-04-20 at 16:29 +0000, Marciniszyn, Mike wrote:
> > From: Aneesh Kumar K.V [mailto:aneesh.kumar@linux.vnet.ibm.com]
> > Sent: Wednesday, April 20, 2016 3:58 AM
> > Subject: [PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded
> > mapping on powerpc
> > 
> 
> Doug, can you fix up the summary to git the spelling error and powerpc/infiniband:  -> IB/qib:

We'll need to take this via the powerpc tree, we have a 68 patch series that
depends on it. I'm happy to update the subject.

cheers

Re: [PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded mapping on powerpc

[Benjamin Herrenschmidt]

On Wed, 2016-04-20 at 03:58 -0400, Aneesh Kumar K.V wrote:
> The driver was requesting for a writethrough mapping. But with thoses
> flags we will end up with a SAO mapping because we now have memory
> conherence always enabled. ie, the existing mapping will end up with
> a WIMG value 0b1110 which is Strong Access Order.
> 
> Update this to use cache inhibitted guarded mapping

Why guarded ? If it's performance sensitive (and the driver has
appropriate barriers where needed), you will get write combining
without guarded, you won't with it.

Cheers,
Ben.

> Cc: Mike Marciniszyn <infinipath@intel.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Sean Hefty <sean.hefty@intel.com>
> Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
> Cc: linux-rdma@vger.kernel.org
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> Changes from v1:
> * resend because V1 never reached mailing list.
> 
> NOTE: This is only compile tested and I am also not sure why ppc64
> needs special handling.
> We need this patch because the series at http://mid.gmane.org/1460182
> 444-2468-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com
> will drop _PAGE_WRITETHRU
> 
>  drivers/infiniband/hw/qib/qib_file_ops.c | 5 +----
>  drivers/infiniband/hw/qib/qib_pcie.c     | 6 ------
>  2 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c
> b/drivers/infiniband/hw/qib/qib_file_ops.c
> index e449e394963f..a3d593f546ec 100644
> --- a/drivers/infiniband/hw/qib/qib_file_ops.c
> +++ b/drivers/infiniband/hw/qib/qib_file_ops.c
> @@ -822,10 +822,7 @@ static int mmap_piobufs(struct vm_area_struct
> *vma,
>  	phys = dd->physaddr + piobufs;
>  
>  #if defined(__powerpc__)
> -	/* There isn't a generic way to specify writethrough
> mappings */
> -	pgprot_val(vma->vm_page_prot) |= _PAGE_NO_CACHE;
> -	pgprot_val(vma->vm_page_prot) |= _PAGE_WRITETHRU;
> -	pgprot_val(vma->vm_page_prot) &= ~_PAGE_GUARDED;
> +	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>  #endif
>  
>  	/*
> diff --git a/drivers/infiniband/hw/qib/qib_pcie.c
> b/drivers/infiniband/hw/qib/qib_pcie.c
> index 4758a3801ae8..6abe1c621aa4 100644
> --- a/drivers/infiniband/hw/qib/qib_pcie.c
> +++ b/drivers/infiniband/hw/qib/qib_pcie.c
> @@ -144,13 +144,7 @@ int qib_pcie_ddinit(struct qib_devdata *dd,
> struct pci_dev *pdev,
>  	addr = pci_resource_start(pdev, 0);
>  	len = pci_resource_len(pdev, 0);
>  
> -#if defined(__powerpc__)
> -	/* There isn't a generic way to specify writethrough
> mappings */
> -	dd->kregbase = __ioremap(addr, len, _PAGE_NO_CACHE |
> _PAGE_WRITETHRU);
> -#else
>  	dd->kregbase = ioremap_nocache(addr, len);
> -#endif
> -
>  	if (!dd->kregbase)
>  		return -ENOMEM;
>  

Re: [PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded mapping on powerpc

[Jason Gunthorpe]

On Fri, Apr 22, 2016 at 08:47:56AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2016-04-20 at 03:58 -0400, Aneesh Kumar K.V wrote:
> > The driver was requesting for a writethrough mapping. But with thoses
> > flags we will end up with a SAO mapping because we now have memory
> > conherence always enabled. ie, the existing mapping will end up with
> > a WIMG value 0b1110 which is Strong Access Order.
> > 
> > Update this to use cache inhibitted guarded mapping
> 
> Why guarded ? If it's performance sensitive (and the driver has
> appropriate barriers where needed), you will get write combining
> without guarded, you won't with it.

This driver uses uncached write combining on x86

Jason

Re: [PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded mapping on powerpc

[Benjamin Herrenschmidt]

On Fri, 2016-04-22 at 10:09 -0600, Jason Gunthorpe wrote:
> On Fri, Apr 22, 2016 at 08:47:56AM +1000, Benjamin Herrenschmidt
> wrote:
> > 
> > On Wed, 2016-04-20 at 03:58 -0400, Aneesh Kumar K.V wrote:
> > > 
> > > The driver was requesting for a writethrough mapping. But with
> > > thoses
> > > flags we will end up with a SAO mapping because we now have
> > > memory
> > > conherence always enabled. ie, the existing mapping will end up
> > > with
> > > a WIMG value 0b1110 which is Strong Access Order.
> > > 
> > > Update this to use cache inhibitted guarded mapping
> > Why guarded ? If it's performance sensitive (and the driver has
> > appropriate barriers where needed), you will get write combining
> > without guarded, you won't with it.
> This driver uses uncached write combining on x86

Right so if you want something similar on power, use the _wc variant,
and make sure you have the appropriate barriers when you need to
break combining.

Cheers,
Ben.

Re: [PATCH V2] powerpc/infiniband: Use cache inhibitted and guarded mapping on powerpc

[Aneesh Kumar K.V]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Fri, 2016-04-22 at 10:09 -0600, Jason Gunthorpe wrote:
>> On Fri, Apr 22, 2016 at 08:47:56AM +1000, Benjamin Herrenschmidt
>> wrote:
>> > 
>> > On Wed, 2016-04-20 at 03:58 -0400, Aneesh Kumar K.V wrote:
>> > > 
>> > > The driver was requesting for a writethrough mapping. But with
>> > > thoses
>> > > flags we will end up with a SAO mapping because we now have
>> > > memory
>> > > conherence always enabled. ie, the existing mapping will end up
>> > > with
>> > > a WIMG value 0b1110 which is Strong Access Order.
>> > > 
>> > > Update this to use cache inhibitted guarded mapping
>> > Why guarded ? If it's performance sensitive (and the driver has
>> > appropriate barriers where needed), you will get write combining
>> > without guarded, you won't with it.
>> This driver uses uncached write combining on x86
>
> Right so if you want something similar on power, use the _wc variant,
> and make sure you have the appropriate barriers when you need to
> break combining.
>
> Cheers,
> Ben.

The code after change looks like below

#if defined(__powerpc__)
	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
#endif

	/*
	 * don't allow them to later change to readable with mprotect (for when
	 * not initially mapped readable, as is normally the case)
	 */
	vma->vm_flags &= ~VM_MAYREAD;
	vma->vm_flags |= VM_DONTCOPY | VM_DONTEXPAND;

	/* We used PAT if wc_cookie == 0 */
	if (!dd->wc_cookie)
		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);

So it uses pgprot_noncached_wc if (!dd->wc_cookie). Hence I was thinking
it needs more stricter mapping otherwise.

-aneesh