[PATCH] PPC: Correct the tophys/tovirt macros

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE5055d

3 messages, 2 authors, 2012-10-04 · open the first message on its own page

[PATCH] PPC: Correct the tophys/tovirt macros

From: Jason Gunthorpe <hidden>
Date: 2012-09-30 23:48:53

asm/page.h discusses the calculation for v2p and p2v, it should be:
 va = pa + KERNELBASE - PHYSICAL_START
which is the same as:
 va = pa + LOAD_OFFSET

tophys/tovirt were using PAGE_OFFSET, which as page.h says, is almost
always the same thing.

Signed-off-by: Jason Gunthorpe <redacted>
---
 arch/powerpc/include/asm/ppc_asm.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index ea2a86e..44edc3a 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -461,14 +461,14 @@ END_FTR_SECTION_IFCLR(CPU_FTR_601)
 #define fromreal(rd)	tovirt(rd,rd)
 
 #define tophys(rd,rs)				\
-0:	addis	rd,rs,-PAGE_OFFSET@h;		\
+0:	addis	rd,rs,-LOAD_OFFSET@h;		\
 	.section ".vtop_fixup","aw";		\
 	.align  1;				\
 	.long   0b;				\
 	.previous
 
 #define tovirt(rd,rs)				\
-0:	addis	rd,rs,PAGE_OFFSET@h;		\
+0:	addis	rd,rs,LOAD_OFFSET@h;		\
 	.section ".ptov_fixup","aw";		\
 	.align  1;				\
 	.long   0b;				\
-- 
1.7.4.1

Re: [PATCH] PPC: Correct the tophys/tovirt macros

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2012-10-04 11:19:09

On Sun, 2012-09-30 at 17:28 -0600, Jason Gunthorpe wrote:
asm/page.h discusses the calculation for v2p and p2v, it should be:
 va = pa + KERNELBASE - PHYSICAL_START
which is the same as:
 va = pa + LOAD_OFFSET

tophys/tovirt were using PAGE_OFFSET, which as page.h says, is almost
always the same thing.

Signed-off-by: Jason Gunthorpe <redacted>
It's a bit gross tho in that KERNEL_BASE, PHYSICAL_START and LOAD_OFFSET
are about where the kernel is linked/running, and while the value ends
up happening to also be the p->v offset (and indeed not by accident), it
makes the code less clear and more confusing.

I think the main issue is that we did things wrong when implementing
non-0 based setups. PAGE_OFFSET should have remained what its name
implies which is the offset between p and v.

I don't have the bandwidth to revisit all that, but I really think that
whole are area where PAGE_OFFSET doesn't map 0 needs revisiting.

Ben.
quoted hunk
---
 arch/powerpc/include/asm/ppc_asm.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index ea2a86e..44edc3a 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -461,14 +461,14 @@ END_FTR_SECTION_IFCLR(CPU_FTR_601)
 #define fromreal(rd)	tovirt(rd,rd)
 
 #define tophys(rd,rs)				\
-0:	addis	rd,rs,-PAGE_OFFSET@h;		\
+0:	addis	rd,rs,-LOAD_OFFSET@h;		\
 	.section ".vtop_fixup","aw";		\
 	.align  1;				\
 	.long   0b;				\
 	.previous
 
 #define tovirt(rd,rs)				\
-0:	addis	rd,rs,PAGE_OFFSET@h;		\
+0:	addis	rd,rs,LOAD_OFFSET@h;		\
 	.section ".ptov_fixup","aw";		\
 	.align  1;				\
 	.long   0b;				\

Re: [PATCH] PPC: Correct the tophys/tovirt macros

From: Jason Gunthorpe <hidden>
Date: 2012-10-04 17:08:48

On Thu, Oct 04, 2012 at 09:19:04PM +1000, Benjamin Herrenschmidt wrote:
On Sun, 2012-09-30 at 17:28 -0600, Jason Gunthorpe wrote:
quoted
asm/page.h discusses the calculation for v2p and p2v, it should be:
 va = pa + KERNELBASE - PHYSICAL_START
which is the same as:
 va = pa + LOAD_OFFSET

tophys/tovirt were using PAGE_OFFSET, which as page.h says, is almost
always the same thing.

Signed-off-by: Jason Gunthorpe <redacted>
It's a bit gross tho in that KERNEL_BASE, PHYSICAL_START and LOAD_OFFSET
are about where the kernel is linked/running, and while the value ends
up happening to also be the p->v offset (and indeed not by accident), it
makes the code less clear and more confusing.
Yes, I found the three names confusing.. However LOAD_OFFSET is the
name that include/asm-generic/vmlinux.lds.h and many arch's use for
P2V translation in the linker script, so at least there is some
precedent.
I don't have the bandwidth to revisit all that, but I really think that
whole are area where PAGE_OFFSET doesn't map 0 needs revisiting.
Something like that is also outside my scope.. This patch does fix a
bug I hit when CONFIG_PHYSICAL_START is used, are there problems with
it in other cases?

FWIW, this is part of a larger work that makes CONFIG_PHYSICAL_START
work on PPC404, which is not yet clean enough to post:
https://github.com/jgunthorpe/linux/commit/90df2d247c9db5d39c32dbbada0815f15d7b3be7

Regards,
Jason
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help