Message ID | 20120930232847.GG30637@obsidianresearch.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Benjamin Herrenschmidt |
Headers | show |
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 <jgunthorpe@obsidianresearch.com> 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. > --- > 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; \
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: > > 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 <jgunthorpe@obsidianresearch.com> > > 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
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; \
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 <jgunthorpe@obsidianresearch.com> --- arch/powerpc/include/asm/ppc_asm.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)