[PATCH] powerpc: Fix preserved memory size for int-vectors

[Guozihua (Scott)]

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 at 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 at 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
>>

-- 
Best
GUO Zihua