Re: [PATCH] powerpc/64: option to force run-at-load to test relocation

[Nicholas Piggin <npiggin@gmail.com>]

On Wed, 12 Oct 2016 18:35:21 +1100
Balbir Singh <bsingharora@gmail.com> wrote:

> On 12/10/16 17:57, Nicholas Piggin wrote:
> > This adds a config option that can help exercise the case when
> > the kernel is not running at PAGE_OFFSET.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  arch/powerpc/Kconfig               | 9 +++++++++
> >  arch/powerpc/kernel/head_64.S      | 4 ++++
> >  arch/powerpc/kernel/setup-common.c | 3 +++
> >  3 files changed, 16 insertions(+)
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 65fba4c..5d43cb8 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -478,6 +478,15 @@ config RELOCATABLE
> >  	  setting can still be useful to bootwrappers that need to know the
> >  	  load address of the kernel (eg. u-boot/mkimage).
> >  
> > +config RELOCATABLE_TEST
> > +	bool "Test relocatable kernel"
> > +	depends on (PPC64 && RELOCATABLE)
> > +	default n
> > +	help
> > +	  This runs the relocatable kernel at the address it was initially
> > +	  loaded at, which tends to be non-zero and therefore test the
> > +	  relocation code.
> > +
> >  config CRASH_DUMP
> >  	bool "Build a kdump crash kernel"
> >  	depends on PPC64 || 6xx || FSL_BOOKE || (44x && !SMP)
> > diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> > index 79da0641..bc9ceac 100644
> > --- a/arch/powerpc/kernel/head_64.S
> > +++ b/arch/powerpc/kernel/head_64.S
> > @@ -111,8 +111,12 @@ __secondary_hold_acknowledge:
> >  	.globl	__run_at_load
> >  __run_at_load:
> >  DEFINE_FIXED_SYMBOL(__run_at_load)
> > +#ifdef CONFIG_RELOCATABLE_TEST
> > +	.long	0x1		/* Test relocation, do not relocate to 0 */
> > +#else
> >  	.long	0x72756e30	/* "run0" -- relocate to 0 by default */
> >  #endif
> > +#endif  
> 
> Could we do something like
> 
> config RELOCATION_VALUE
> 	default 0x72756e30
> 	default	1  if CONFIG_RELOCTABLE_TEST
> 
> and then get
> 
> 	.long CONFIG_RELOCATION_VALUE

Normally I'm up for reducing ifdefs in S and c files, but in this case
I'm not sure. I like being able to see the two possible values in the
source. I don't really mind though. If you or Michael feel strongly, I'm
happy to change it.


> >  	. = 0x60
> >  /*
> > diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> > index dba265c..18e0f19 100644
> > --- a/arch/powerpc/kernel/setup-common.c
> > +++ b/arch/powerpc/kernel/setup-common.c
> > @@ -795,6 +795,9 @@ static __init void print_system_info(void)
> >  	pr_info("mmu_features      = 0x%08x\n", cur_cpu_spec->mmu_features);
> >  #ifdef CONFIG_PPC64
> >  	pr_info("firmware_features = 0x%016lx\n", powerpc_firmware_features);
> > +
> > +	if (get_paca()->kernelbase != PAGE_OFFSET)
> > +		pr_info("kernelbase        = 0x%llx\n", get_paca()->kernelbase);
> >  #endif
> >    
> 
> Do we need this? We get physical_offset if we are relocated.

You're right, that hunk can go.

Thanks,
Nick