[PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Hoang-Nam Nguyen]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

Hello Roland!
This is another patch of ehca for 64k page support. It fixes a bug that maps
4k aligned addresses in 64k page mode in a wrong way.
Thanks!
Nam


Signed-off-by: Hoang-Nam Nguyen <hnguyen@de.ibm.com>
---


 hcp_phyp.c |    5 +++--0018_64kpage_ioremap.patch  
 1 files changed, 3 insertions(+), 2 deletions(-)


diff --git a/drivers/infiniband/hw/ehca/hcp_phyp.c b/drivers/infiniband/hw/ehca/hcp_phyp.c
index 0b1a477..6237252 100644
--- a/drivers/infiniband/hw/ehca/hcp_phyp.c
+++ b/drivers/infiniband/hw/ehca/hcp_phyp.c
@@ -44,13 +44,14 @@ #include "hipz_hw.h"
 
 int hcall_map_page(u64 physaddr, u64 *mapaddr)
 {
- *mapaddr = (u64)(ioremap(physaddr, EHCA_PAGESIZE));
+ *mapaddr = (u64)ioremap((physaddr & PAGE_MASK), PAGE_SIZE) +
+  (physaddr & (~PAGE_MASK));
  return 0;
 }
 
 int hcall_unmap_page(u64 mapaddr)
 {
- iounmap((volatile void __iomem*)mapaddr);
+ iounmap((void __iomem*)(mapaddr & PAGE_MASK));
  return 0;
 }
 

[-- Attachment #2: roland_svnehca_0018_64kpage_ioremap.patch --]
[-- Type: text/x-diff, Size: 615 bytes --]

diff --git a/drivers/infiniband/hw/ehca/hcp_phyp.c b/drivers/infiniband/hw/ehca/hcp_phyp.c
index 0b1a477..6237252 100644
--- a/drivers/infiniband/hw/ehca/hcp_phyp.c
+++ b/drivers/infiniband/hw/ehca/hcp_phyp.c
@@ -44,13 +44,14 @@ #include "hipz_hw.h"
 
 int hcall_map_page(u64 physaddr, u64 *mapaddr)
 {
-	*mapaddr = (u64)(ioremap(physaddr, EHCA_PAGESIZE));
+	*mapaddr = (u64)ioremap((physaddr & PAGE_MASK), PAGE_SIZE) +
+		(physaddr & (~PAGE_MASK));
 	return 0;
 }
 
 int hcall_unmap_page(u64 mapaddr)
 {
-	iounmap((volatile void __iomem*)mapaddr);
+	iounmap((void __iomem*)(mapaddr & PAGE_MASK));
 	return 0;
 }
 

Re: [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Roland Dreier]

 > -	*mapaddr = (u64)(ioremap(physaddr, EHCA_PAGESIZE));
 > +	*mapaddr = (u64)ioremap((physaddr & PAGE_MASK), PAGE_SIZE) +
 > +		(physaddr & (~PAGE_MASK));

I'm confused -- shouldn't ioremap() do the right thing even if
physaddr isn't page-aligned?  Why is this needed?

 - R.

Re: [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Christoph Raisch]

Roland Dreier wrote on 07.11.2006 20:25:12:

>  > -   *mapaddr = (u64)(ioremap(physaddr, EHCA_PAGESIZE));
>  > +   *mapaddr = (u64)ioremap((physaddr & PAGE_MASK), PAGE_SIZE) +
>  > +      (physaddr & (~PAGE_MASK));
>
> I'm confused -- shouldn't ioremap() do the right thing even if
> physaddr isn't page-aligned?  Why is this needed?
>
>  - R.

ioremap maps 4k pages on 4k kernels and on 64k pages on 64k kernels. So far
the theory.

This is true for memory.

For mapped PCI or ebus registers things are a bit different.
Some PCI adapters expect that every other 4k page is a new area with
different meaning starts
(some PCI adapters are definetly ehca and mellanox here). The consequence
is you have to map
only 4k instead of 64k, otherwise you'd map 15 other "access areas" are
also mapped.

On POWER the ebus memory is mapped by H_ENTER.
The hypervisor checks for 4k page size on H_ENTER, reason see above.

The nopage handler now does seperate 4k H_ENTERs even for 64k pages in the
ebus area,
therefore we have to register a 64k page on a 64k boundary, and the nopage
triggers the right H_ENTER
as soon as we access the page at the right offset.

We plan to change that as soon as the base kernel can handle mixed
pagesizes in a more official way.

Christop R.

Re: [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Roland Dreier]

 > We plan to change that as soon as the base kernel can handle mixed
 > pagesizes in a more official way.

OK, so this is just a temporary workaround for powerpc's broken ioremap()?

I'll apply for 2.6.19, and I hope we can back this out in 2.6.20.

 - R.

Re: [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Paul Mackerras]

Christoph Raisch writes:

> ioremap maps 4k pages on 4k kernels and on 64k pages on 64k kernels. So far
> the theory.
> 
> This is true for memory.

And for I/O. :)  ioremap updates the (Linux) page tables that map the
vmalloc/ioremap area, and that is at page granularity.  So there is in
fact no difference in the end result in the page tables whether you
ask to map a small amount inside a page, or the whole page.

> On POWER the ebus memory is mapped by H_ENTER.
> The hypervisor checks for 4k page size on H_ENTER, reason see above.

The next part of the story is that the low-level MMU code on System-P
(pSeries) machines only does the H_ENTER when you access an I/O
mapping.  It does H_ENTER for 4k pages for non-cacheable mappings,
and it only does the H_ENTER for the 4k subpages of a 64k page that
the kernel actually accesses.

So Roland is correct in his comment about how ioremap is called.

Regards,
Paul.

Re: [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Roland Dreier]

 > So Roland is correct in his comment about how ioremap is called.

Umm, so is this patch really needed?  Where did the patch come from --
is it needed to fix something actually seen, or was it written just
based on some theoretical understanding?

I'm confused...

 - R.

Re: [openib-general] [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Christoph Raisch]

> Umm, so is this patch really needed?  Where did the patch come from --
> is it needed to fix something actually seen, or was it written just
> based on some theoretical understanding?
>
> I'm confused...
>
>  - R.
The patch is needed. We've seen it on the real system. We did fix it on the
real system.
...and it conforms to theory... although theory is a bit confusing here.

let me try to summarize:
ioremap checks for 64k boundary (actually page boundary)
nopage does H_ENTER in 4k granularity if it's configured like that for a
certain type of POWER processor.

so you have to adjust the ioremap to page boundary, and THEN access at the
offset within the 64k.

Took quite a while until we understood that code path.... ;-)

Christoph R.

Re: [openib-general] [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Paul Mackerras]

Christoph Raisch writes:

> The patch is needed. We've seen it on the real system. We did fix it on the
> real system.

I disagree that the ioremap change is needed.

> ...and it conforms to theory... although theory is a bit confusing here.
> 
> let me try to summarize:
> ioremap checks for 64k boundary (actually page boundary)

Actually, ioremap itself already does the calculations that your patch
adds - that is, it generates the offset within the page and the
physical address of the start of the page, does the mapping using the
latter, then adds on the offset to the virtual address of the page and
returns that.

Paul.

Re: [openib-general] [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Roland Dreier]

 > > The patch is needed. We've seen it on the real system. We did fix it on the
 > > real system.
 > 
 > I disagree that the ioremap change is needed.

Hmm... Paul, what you say makes sense and is what I would have
thought, but Christoph says that the unpatched code really fails on a
real system.  So I'm still confused.

I think I'll merge this with a fat comment, with the hope that we can
drop it ASAP once everyone agrees on what's going on.

 - R.

Re: [openib-general] [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Christoph Raisch]

> Christoph Raisch writes:
>
> > The patch is needed. We've seen it on the real system. We did fix it on
the
> > real system.
>
> I disagree that the ioremap change is needed.
>
> > ...and it conforms to theory... although theory is a bit confusing
here.
> >
> > let me try to summarize:
> > ioremap checks for 64k boundary (actually page boundary)
>
> Actually, ioremap itself already does the calculations that your patch
> adds - that is, it generates the offset within the page and the
> physical address of the start of the page, does the mapping using the
> latter, then adds on the offset to the virtual address of the page and
> returns that.

Paul,
you are right. The calculation is done in your code already.
We can't reproduce the problem anymore on latest kernel.
Was this calculation there in ioremap right from the start with 64k on
POWER or added later on?

So Roland, feel free to ignore that line where we do the calculation.

>
> Paul.

Re: [openib-general] [PATCH 2.6.19 2/4] ehca: hcp_phyp.c: correct page mapping in 64k page mode

[Roland Dreier]

 > So Roland, feel free to ignore that line where we do the calculation.

OK, ignore the email I just sent.  I'll drop the patch.

thanks