PPC: Correct the tophys/tovirt macros
PATCH 1/1 PPC: Correct the tophys/tovirt macros
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.
--- arch/powerpc/include/asm/ppc_asm.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
arch/powerpc/include/asm/ppc_asm.h +2 -2
@@ -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; \
Signature · 1 line
1.7.4.1
Quoted 9 lines · Jason Gunthorpe
asm/page.hdiscusses the calculation for v2p and p2v, it should be:
va = pa + KERNELBASE -PHYSICAL_START
which is the same as:
va = pa +LOAD_OFFSETtophys/tovirt were using
PAGE_OFFSET, which aspage.hsays, is almost always the same thing.Signed-off-by Jason Gunthorpe <[email protected]>
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 25 lines
--- arch/powerpc/include/asm/ppc_asm.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)arch/powerpc/include/asm/ppc_asm.h +2
-2@@ -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; \
Quoted 9 lines · Benjamin Herrenschmidt
Quoted 8 lines · Jason Gunthorpe
asm/page.hdiscusses the calculation for v2p and p2v, it should be:
va = pa + KERNELBASE -PHYSICAL_START
which is the same as:
va = pa +LOAD_OFFSETtophys/tovirt were using
PAGE_OFFSET, which aspage.hsays, is almost always the same thing.
Quoted 6 lines
Quoted 1 line
Signed-off-by Jason Gunthorpe <[email protected]>It's a bit gross tho in that
KERNEL_BASE,PHYSICAL_STARTandLOAD_OFFSETare about where the kernel is linked/running, and while the value ends up happening to also be thep->voffset (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.
Quoted 2 lines
I don't have the bandwidth to revisit all that, but I really think that whole are area where
PAGE_OFFSETdoesn'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