Re: [PATCH v2 2/4] powerpc: move to 64-bit RTAS

[Sourabh Jain <sourabhjain@linux.ibm.com>]

On 26/07/26 16:25, Hari Bathini wrote:
>
>
> On 08/07/26 8:03 pm, Sourabh Jain wrote:
>> Kdump kernels loaded at high addresses (above 4G) could not boot
>> because the kernel used 32-bit RTAS.
>>
>> Until now, the kernel always used 32-bit RTAS, even for 64-bit kernels.
>> Before making an RTAS call, it clears the SF bit in MSR and uses LR as
>> the return address from RTAS. For kdump kernels loaded above 4G, RTAS
>> returns to a 32-bit truncated LR address, typically causing an exception
>> and kernel panic.
>>
>> Fix this by switching to 64-bit RTAS support on systems where firmware
>> advertises "ibm,rtas-64-capable". The kernel instantiates 64-bit RTAS,
>> passes RTAS arguments using a 64-bit argument structure, and sets the
>> SF bit in MSR before entering RTAS. This ensures RTAS can correctly
>> return to addresses above 4G and allows high-address kdump kernels to
>> boot successfully.
>>
>> If 64-bit RTAS is not supported or initialization fails, the kernel
>> continues to use 32-bit RTAS. In that case, high-address kdump kernels
>> will not be allowed (handled in upcoming patches), and RTAS calls will
>> continue to run with the SF bit cleared.
>>
>> Changes made to support 64-bit RTAS:
>> - Detect firmware support using "ibm,rtas-64-capable"
>> - Initialize 64-bit RTAS in prom_init and add a new FDT property
>>    "linux,rtas-64"
>> - Read "linux,rtas-64" during boot and track RTAS mode using rtas_64
>> - Add 64-bit RTAS argument handling and return value conversion
>> - Prepare MSR appropriately for 32-bit or 64-bit RTAS calls
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/include/asm/rtas-types.h |  8 +++++
>>   arch/powerpc/include/asm/rtas.h       |  2 ++
>>   arch/powerpc/kernel/prom_init.c       | 20 +++++++++--
>>   arch/powerpc/kernel/rtas.c            | 48 +++++++++++++++++++++++++--
>>   arch/powerpc/kernel/rtas_entry.S      | 17 +++++++++-
>>   5 files changed, 88 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/rtas-types.h 
>> b/arch/powerpc/include/asm/rtas-types.h
>> index 5d40d187b965..de794b3e0fbd 100644
>> --- a/arch/powerpc/include/asm/rtas-types.h
>> +++ b/arch/powerpc/include/asm/rtas-types.h
>> @@ -14,6 +14,14 @@ struct rtas_args {
>>       rtas_arg_t *rets;     /* Pointer to return values in args[]. */
>>   } __aligned(8);
>>   +struct rtas_args_64 {
>> +        __be64 token;
>> +        __be64 nargs;
>> +        __be64 nret;
>> +        __be64 args[16];
>> +        __be64 *rets;     /* Pointer to return values in args[]. */
>> +} __aligned(8);
>> +
>>   struct rtas_t {
>>       unsigned long entry;        /* physical address pointer */
>>       unsigned long base;        /* physical address pointer */
>> diff --git a/arch/powerpc/include/asm/rtas.h 
>> b/arch/powerpc/include/asm/rtas.h
>> index d046bbd5017d..aaa4c3bc1d61 100644
>> --- a/arch/powerpc/include/asm/rtas.h
>> +++ b/arch/powerpc/include/asm/rtas.h
>> @@ -10,6 +10,8 @@
>>   #include <linux/time.h>
>>   #include <linux/cpumask.h>
>>   +extern int rtas_64;
>> +
>>   /*
>>    * Definitions for talking to the RTAS on CHRP machines.
>>    *
>> diff --git a/arch/powerpc/kernel/prom_init.c 
>> b/arch/powerpc/kernel/prom_init.c
>> index 53503937de0e..be7c5ea0b75d 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -1841,6 +1841,7 @@ static void __init prom_instantiate_rtas(void)
>>       u32 base, entry = 0;
>>       __be32 val;
>>       u32 size = 0;
>> +    u32 rtas_64 = 0;
>>         prom_debug("prom_instantiate_rtas: start...\n");
>>   @@ -1865,15 +1866,24 @@ static void __init prom_instantiate_rtas(void)
>>           return;
>>       }
>>   +    /* Check for rtas 64-bit support */
>> +    if (prom_getprop(rtas_node, "ibm,rtas-64-capable",
>> +             &val, sizeof(val)) != PROM_ERROR) {
>> +        rtas_64 = 1;
>> +        prom_debug("Node ibm,rtas-64-capable: %x\n", val);
>> +    }
>> +
>>       prom_printf("instantiating rtas at 0x%x...", base);
>>   +    const char *method = rtas_64 ? "instantiate-rtas-64" : 
>> "instantiate-rtas";
>> +
>>       if (call_prom_ret("call-method", 3, 2, &entry,
>> -              ADDR("instantiate-rtas"),
>> -              rtas_inst, base) != 0
>> -        || entry == 0) {
>> +              ADDR(method), rtas_inst, base) != 0 ||
>> +              entry == 0) {
>>           prom_printf(" failed\n");
>>           return;
>>       }
>> +
>>       prom_printf(" done\n");
>>         reserve_mem(base, size);
>> @@ -1884,6 +1894,9 @@ static void __init prom_instantiate_rtas(void)
>>       val = cpu_to_be32(entry);
>>       prom_setprop(rtas_node, "/rtas", "linux,rtas-entry",
>>                &val, sizeof(val));
>> +    val = cpu_to_be32(rtas_64);
>> +    prom_setprop(rtas_node, "/rtas", "linux,rtas-64",
>> +             &val, sizeof(val));
>>         /* Check if it supports "query-cpu-stopped-state" */
>>       if (prom_getprop(rtas_node, "query-cpu-stopped-state",
>> @@ -1893,6 +1906,7 @@ static void __init prom_instantiate_rtas(void)
>>       prom_debug("rtas base     = 0x%x\n", base);
>>       prom_debug("rtas entry    = 0x%x\n", entry);
>>       prom_debug("rtas size     = 0x%x\n", size);
>> +    prom_debug("rtas 64-bit   = 0x%x\n", rtas_64);
>>         prom_debug("prom_instantiate_rtas: end...\n");
>>   }
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 8d81c1e7a8db..b7b9bd0b5b43 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -45,6 +45,8 @@
>>   #include <asm/trace.h>
>>   #include <asm/udbg.h>
>>   +int rtas_64;
>> +
>>   struct rtas_filter {
>>       /* Indexes into the args buffer, -1 if not used */
>>       const int buf_idx1;
>> @@ -569,6 +571,7 @@ static struct rtas_function rtas_function_table[] 
>> __ro_after_init = {
>>    */
>>   static DEFINE_RAW_SPINLOCK(rtas_lock);
>>   static struct rtas_args rtas_args;
>> +static struct rtas_args_64 rtas_args_64;
>>     /**
>>    * rtas_function_token() - RTAS function token lookup.
>> @@ -690,13 +693,48 @@ static const struct rtas_function 
>> *rtas_token_to_function(s32 token)
>>       return NULL;
>>   }
>>   +static void populate_rtas_args_64(struct rtas_args *args)
>> +{
>> +    int i;
>> +
>> +    rtas_args_64.token = 
>> cpu_to_be64((s64)(s32)be32_to_cpu(args->token));
>> +    rtas_args_64.nargs = 
>> cpu_to_be64((s64)(s32)be32_to_cpu(args->nargs));
>> +    rtas_args_64.nret  = 
>> cpu_to_be64((s64)(s32)be32_to_cpu(args->nret));
>> +    rtas_args_64.rets  = &rtas_args_64.args[be32_to_cpu(args->nargs)];
>> +
>> +    for (i = 0; i < be32_to_cpu(args->nargs); ++i)
>> +        rtas_args_64.args[i] = 
>> cpu_to_be64((s64)(s32)be32_to_cpu(args->args[i]));
>> +
>> +    for (i = 0; i < be32_to_cpu(args->nret); ++i)
>> +        rtas_args_64.rets[i] = 0;
>> +}
>> +
>> +static void rtas_args_copy_64_32(struct rtas_args *args)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < (s32)be64_to_cpu(rtas_args_64.nret); ++i)
>> +        args->rets[i] = 
>> cpu_to_be32((s32)be64_to_cpu(rtas_args_64.rets[i]));
>> +}
>> +
>>   /* This is here deliberately so it's only used in this file */
>>   void enter_rtas(unsigned long);
>>   +static void _do_enter_rtas_64(struct rtas_args *args)
>> +{
>> +    populate_rtas_args_64(args);
>> +    enter_rtas(__pa(&rtas_args_64));
>> +    rtas_args_copy_64_32(args);
>> +}
>> +
>>   static void __do_enter_rtas(struct rtas_args *args)
>>   {
>> -    enter_rtas(__pa(args));
>> -    srr_regs_clobbered(); /* rtas uses SRRs, invalidate */
>> +    if (rtas_64)
>> +        _do_enter_rtas_64(args);
>> +    else
>> +        enter_rtas(__pa(args));
>> +
>> +    srr_regs_clobbered();
>>   }
>>     static void __do_enter_rtas_trace(struct rtas_args *args)
>> @@ -2078,7 +2116,7 @@ void __init rtas_initialize(void)
>>   int __init early_init_dt_scan_rtas(unsigned long node,
>>           const char *uname, int depth, void *data)
>>   {
>> -    const u32 *basep, *entryp, *sizep;
>> +    const u32 *basep, *entryp, *sizep, *val;
>>         if (depth != 1 || strcmp(uname, "rtas") != 0)
>>           return 0;
>> @@ -2086,6 +2124,10 @@ int __init early_init_dt_scan_rtas(unsigned 
>> long node,
>>       basep  = of_get_flat_dt_prop(node, "linux,rtas-base", NULL);
>>       entryp = of_get_flat_dt_prop(node, "linux,rtas-entry", NULL);
>>       sizep  = of_get_flat_dt_prop(node, "rtas-size", NULL);
>> +    val    = of_get_flat_dt_prop(node, "linux,rtas-64", NULL);
>> +
>
>> +    if (*val)
>
> prom_init() is skipped during kexec boot. So, "val" cannot be
> dereferenced directly as it can be NULL when kexec booted into
> a newer kernel (kernel with these patches) from an older kernel...

Yes, `val` will be NULL if linux,rtas-64 prop is not present. I will 
validate
it before de-referencing it.

- Sourabh Jain


>
>> +        rtas_64 = 1;
>>     #ifdef CONFIG_PPC64
>>       /* need this feature to decide the crashkernel offset */
>> diff --git a/arch/powerpc/kernel/rtas_entry.S 
>> b/arch/powerpc/kernel/rtas_entry.S
>> index 6ce95ddadbcd..df776f0103c9 100644
>> --- a/arch/powerpc/kernel/rtas_entry.S
>> +++ b/arch/powerpc/kernel/rtas_entry.S
>> @@ -54,6 +54,10 @@ _ASM_NOKPROBE_SYMBOL(enter_rtas)
>>   /*
>>    * 32-bit rtas on 64-bit machines has the additional problem that 
>> RTAS may
>>    * not preserve the upper parts of registers it uses.
>> + *
>> + * Note: In 64-bit RTAS, the SF bit is set so that RTAS can return
>> + * correctly if the return address is above 4 GB. Everything else
>> + * works the same as in 32-bit RTAS.
>>    */
>>   _GLOBAL(enter_rtas)
>>       mflr    r0
>> @@ -113,7 +117,18 @@ __enter_rtas:
>>        * from the saved MSR value and insert into the value RTAS will 
>> use.
>>        */
>>       extrdi    r0, r6, 1, 63 - MSR_HV_LG
>> -    LOAD_REG_IMMEDIATE(r6, MSR_ME | MSR_RI)
>> +
>> +    LOAD_REG_ADDR(r7, rtas_64)    /* Load the address rtas_64 into 
>> r7 */
>> +    ld      r8, 0(r7)               /* Load the value of rtas_64 
>> from memory into r8 */
>> +    cmpdi   r8, 0                   /* Compare r8 with 0 (check if 
>> rtas_64 is zero) */
>> +    beq     no_sf_bit               /* Branch to no_sf_bit if 
>> rtas_64 is zero */
>> +    LOAD_REG_IMMEDIATE(r6, MSR_ME | MSR_RI | MSR_SF)    /* r6 = 
>> ME|RI|SF */
>> +    b       continue
>> +
>> +no_sf_bit:
>> +    LOAD_REG_IMMEDIATE(r6, MSR_ME | MSR_RI)        /* r6 = ME|RI (NO 
>> SF bit in MSR) */
>> +
>> +continue:
>>       insrdi    r6, r0, 1, 63 - MSR_HV_LG
>>         li      r0,0
>
> - Hari