| Message ID | 20240109033851.1310455-1-guozihua@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc: Fix preserved memory size for int-vectors | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
Le 09/01/2024 à 04:38, GUO Zihua a écrit : > [Vous ne recevez pas souvent de courriers de guozihua@huawei.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > The first 32k of memory is reserved for interrupt vectors, however for > powerpc64 this might not be enough. Fix this by reserving the maximum > size between 32k and the real size of interrupt vectors. You say "this might not be enough". Do you have exemples of when it is not enough, or is it just hypothetycal ? > > Signed-off-by: GUO Zihua <guozihua@huawei.com> > --- > arch/powerpc/kernel/prom.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 0b5878c3125b..f374487513b3 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -758,6 +758,7 @@ static inline void save_fscr_to_task(void) {} > void __init early_init_devtree(void *params) > { > phys_addr_t limit; > + size_t int_vector_size; Why size_t ? memblock_reserve() takes a phys_addr_t > > DBG(" -> early_init_devtree(%px)\n", params); > > @@ -810,9 +811,17 @@ void __init early_init_devtree(void *params) > setup_initial_memory_limit(memstart_addr, first_memblock_size); > /* Reserve MEMBLOCK regions used by kernel, initrd, dt, etc... */ > memblock_reserve(PHYSICAL_START, __pa(_end) - PHYSICAL_START); > +#ifdef CONFIG_PPC64 > + /* If relocatable, reserve at least 32k for interrupt vectors etc. */ > + int_vector_size = (size_t)((uintptr_t)__end_interrupts - > + (uintptr_t)_stext); Why do you need so many casts ? When I look into function overlaps_interrupt_vector_text() it seems to work well without cast at all. > + int_vector_size = max_t(size_t, 0x8000, int_vector_size); Use SZ_32K instead of 0x8000 > +#else > /* If relocatable, reserve first 32k for interrupt vectors etc. */ > + int_vector_size = 0x8000; Use SZ_32K > +#endif > if (PHYSICAL_START > MEMORY_START) > - memblock_reserve(MEMORY_START, 0x8000); > + memblock_reserve(MEMORY_START, int_vector_size); > reserve_kdump_trampoline(); > #if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP) > /* > -- > 2.34.1 >
On 2024/1/9 17:59, Christophe Leroy wrote: > > > Le 09/01/2024 à 04:38, GUO Zihua a écrit : >> [Vous ne recevez pas souvent de courriers de guozihua@huawei.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] >> >> The first 32k of memory is reserved for interrupt vectors, however for >> powerpc64 this might not be enough. Fix this by reserving the maximum >> size between 32k and the real size of interrupt vectors. > > You say "this might not be enough". Do you have exemples of when it is > not enough, or is it just hypothetycal ? Hi Christophe, The problem is discovered when I am trying to test ppc64 kaslr (developed by Yan, ref: https://lwn.net/Articles/811686/) on QEMU. On QEMU it seems that the size of interrupt vectors is way larger than 0x8000, resulting in the interrupt vectors code overriding memory allocated for PACAs during do_final_fixups(). > >> >> Signed-off-by: GUO Zihua <guozihua@huawei.com> >> --- >> arch/powerpc/kernel/prom.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c >> index 0b5878c3125b..f374487513b3 100644 >> --- a/arch/powerpc/kernel/prom.c >> +++ b/arch/powerpc/kernel/prom.c >> @@ -758,6 +758,7 @@ static inline void save_fscr_to_task(void) {} >> void __init early_init_devtree(void *params) >> { >> phys_addr_t limit; >> + size_t int_vector_size; > > Why size_t ? > > memblock_reserve() takes a phys_addr_t Will fix that. > >> >> DBG(" -> early_init_devtree(%px)\n", params); >> >> @@ -810,9 +811,17 @@ void __init early_init_devtree(void *params) >> setup_initial_memory_limit(memstart_addr, first_memblock_size); >> /* Reserve MEMBLOCK regions used by kernel, initrd, dt, etc... */ >> memblock_reserve(PHYSICAL_START, __pa(_end) - PHYSICAL_START); >> +#ifdef CONFIG_PPC64 >> + /* If relocatable, reserve at least 32k for interrupt vectors etc. */ >> + int_vector_size = (size_t)((uintptr_t)__end_interrupts - >> + (uintptr_t)_stext); > > Why do you need so many casts ? When I look into function > overlaps_interrupt_vector_text() it seems to work well without cast at all. I'll look into it. > >> + int_vector_size = max_t(size_t, 0x8000, int_vector_size); > > Use SZ_32K instead of 0x8000 > >> +#else >> /* If relocatable, reserve first 32k for interrupt vectors etc. */ >> + int_vector_size = 0x8000; > > Use SZ_32K Sure. > >> +#endif >> if (PHYSICAL_START > MEMORY_START) >> - memblock_reserve(MEMORY_START, 0x8000); >> + memblock_reserve(MEMORY_START, int_vector_size); >> reserve_kdump_trampoline(); >> #if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP) >> /* >> -- >> 2.34.1 >>
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 0b5878c3125b..f374487513b3 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -758,6 +758,7 @@ static inline void save_fscr_to_task(void) {} void __init early_init_devtree(void *params) { phys_addr_t limit; + size_t int_vector_size; DBG(" -> early_init_devtree(%px)\n", params); @@ -810,9 +811,17 @@ void __init early_init_devtree(void *params) setup_initial_memory_limit(memstart_addr, first_memblock_size); /* Reserve MEMBLOCK regions used by kernel, initrd, dt, etc... */ memblock_reserve(PHYSICAL_START, __pa(_end) - PHYSICAL_START); +#ifdef CONFIG_PPC64 + /* If relocatable, reserve at least 32k for interrupt vectors etc. */ + int_vector_size = (size_t)((uintptr_t)__end_interrupts - + (uintptr_t)_stext); + int_vector_size = max_t(size_t, 0x8000, int_vector_size); +#else /* If relocatable, reserve first 32k for interrupt vectors etc. */ + int_vector_size = 0x8000; +#endif if (PHYSICAL_START > MEMORY_START) - memblock_reserve(MEMORY_START, 0x8000); + memblock_reserve(MEMORY_START, int_vector_size); reserve_kdump_trampoline(); #if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP) /*
The first 32k of memory is reserved for interrupt vectors, however for powerpc64 this might not be enough. Fix this by reserving the maximum size between 32k and the real size of interrupt vectors. Signed-off-by: GUO Zihua <guozihua@huawei.com> --- arch/powerpc/kernel/prom.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)