Re: [PATCH] powerpc/64: option to force run-at-load to test relocation
Balbir Singh writes: > On 12/10/16 17:57, Nicholas Piggin wrote: >> 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 I'm not a fan of using kconfig logic when plain #defines would achieve the same result, eg: #ifdef CONFIG_RELOCATABLE_TEST #define RUN_AT_LOAD_DEFAULT 1 /* Test relocation, do not relocate to 0 */ #else #define RUN_AT_LOAD_DEFAULT 0x72756e30 /* "run0" -- relocate to 0 by default */ #endif .globl __run_at_load __run_at_load: DEFINE_FIXED_SYMBOL(__run_at_load) .long RUN_AT_LOAD_DEFAULT Which is probably nicer to look at than Nick's version, but not by a huge margin. I'd merge either. cheers
Re: [PATCH] powerpc/64: option to force run-at-load to test relocation
On 13/10/16 13:25, Nicholas Piggin wrote: > On Wed, 12 Oct 2016 18:35:21 +1100 > Balbir Singh 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 >>> --- >>> 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. > I guess it depends, for me parsing two .long's under #ifdef seems a little more work than a .long with a constant depending on the configuration. I don't feel too strongly about it either Balbir Singh.
Re: [PATCH] powerpc/64: option to force run-at-load to test relocation
On Wed, 12 Oct 2016 18:35:21 +1100 Balbir Singh 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 > > --- > > 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
Re: [PATCH] powerpc/64: option to force run-at-load to test relocation
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 > --- > 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 > > . = 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. > #ifdef CONFIG_PPC_STD_MMU_64 > Balbir Singh.