Re: [PATCH v3 2/5] powerpc/rtas: Allocate ibm,errinjct buffer below RTAS limit

Hi Sourabh,

On 04/08/26 12:52 PM, Sourabh Jain wrote:
> Hello Narayana,
> 
> How about using the rtas_work_area_alloc() instead of allocating a new 
> dedicated buffer
> for errinjct only?
> 
> Checkout Commit 43033bc62d34 ("powerpc/pseries: add RTAS work area 
> allocator") for
> infrastructure to allocated working buffer.
> 
> Also checkout commit e27e14231eb5 ("powerpc/pseries/dlpar: use RTAS work 
> area API")
> on how to use work area API.
> 
Agreed. I reworked this in v4 to use the RTAS work area allocator 
instead of adding a dedicated global rtas_errinjct_buf.

The v3 approach allocated a separate error-injection buffer during
rtas_initialize(), but that is not necessary now that the RTAS work area
allocator already provides the right infrastructure for RTAS-accessible 
working buffers.

In v4, the pseries error-injection path allocates a work area for the 
RTAS parameter buffer, fills it using the mapped kernel address, passes 
the RTAS-visible physical address to firmware, and frees the work area 
after the open/inject/close sequence.

This also removes the extra global buffer only for error injection.

Thanks,
Narayana

> - Sourabh Jain
> 
> On 21/07/26 09:08, Narayana Murty N wrote:
>> ibm,errinjct requires a caller-provided work buffer whose physical
>> address is passed to RTAS firmware.
>>
>> A static C array such as:
>>
>>    char rtas_errinjct_buf[1024] __aligned(SZ_1K);
>>
>> only guarantees alignment, not physical placement.  If the array lands
>> above the RTAS-safe range (RTAS_INSTANTIATE_MAX, 1 GB) or above 4 GB,
>> RTAS receives a truncated or invalid address and the injection call
>> will fail silently or corrupt memory.
>>
>> Instead, introduce rtas_errinjct_buf as a global unsigned long storing
>> the physical address allocated during rtas_initialize() using
>> memblock_phys_alloc_range() with the same rtas_region upper bound used
>> for rtas_rmo_buf.  This matches the existing placement model for
>> RTAS-accessible buffers and guarantees the physical address fits in 32
>> bits.
>>
>>    Usage:
>>      void *buf    = __va(rtas_errinjct_buf);       /* kernel VA to 
>> fill */
>>      u32  buf_phys = lower_32_bits(rtas_errinjct_buf); /* PA for RTAS */
>>
>> Always check upper_32_bits(rtas_errinjct_buf) == 0 before passing the
>> lower 32 bits to RTAS.
>>
>> Add rtas_errinjct_mutex to serialise the complete open-session /
>> inject / close-session firmware call sequence.  A mutex is required
>> because the sequence involves multiple rtas_call() invocations with
>> possible busy/extended-delay retries that may sleep.
>>
>> Signed-off-by: Narayana Murty N <[email protected]>
>> ---
>>   arch/powerpc/include/asm/rtas.h | 26 ++++++++++++++++++++++++++
>>   arch/powerpc/kernel/rtas.c      | 17 +++++++++++++++++
>>   2 files changed, 43 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/ 
>> asm/rtas.h
>> index d046bbd5017d..59e3c1296018 100644
>> --- a/arch/powerpc/include/asm/rtas.h
>> +++ b/arch/powerpc/include/asm/rtas.h
>> @@ -4,6 +4,7 @@
>>   #ifdef __KERNEL__
>>   #include <linux/mutex.h>
>> +#include <linux/sizes.h>
>>   #include <linux/spinlock.h>
>>   #include <asm/page.h>
>>   #include <asm/rtas-types.h>
>> @@ -519,6 +520,31 @@ int rtas_get_error_log_max(void);
>>   extern spinlock_t rtas_data_buf_lock;
>>   extern char rtas_data_buf[RTAS_DATA_BUF_SIZE];
>> +/*
>> + * RTAS error-injection work buffer.
>> + *
>> + * ibm,errinjct requires a caller-provided work buffer whose physical
>> + * address is passed to firmware.  A static C array only guarantees
>> + * alignment, not physical placement; if it lands above the RTAS-safe
>> + * range or above 4 GB, RTAS receives a truncated or bogus address.
>> + *
>> + * rtas_errinjct_buf stores the physical address allocated during
>> + * rtas_initialize() using memblock_phys_alloc_range() with the same
>> + * rtas_region upper bound used for rtas_rmo_buf, matching the existing
>> + * placement model for RTAS-accessible buffers.
>> + *
>> + * Use __va(rtas_errinjct_buf) to obtain the kernel virtual address for
>> + * filling the buffer, and lower_32_bits(rtas_errinjct_buf) to pass the
>> + * physical address to RTAS (after checking upper_32_bits() == 0).
>> + *
>> + * rtas_errinjct_mutex must be held across the complete
>> + * ibm,open-errinjct / ibm,errinjct / ibm,close-errinjct sequence.
>> + */
>> +#define RTAS_ERRINJCT_BUF_SIZE    SZ_1K
>> +
>> +extern unsigned long rtas_errinjct_buf;
>> +extern struct mutex rtas_errinjct_mutex;
>> +
>>   /* RMO buffer reserved for user-space RTAS use */
>>   extern unsigned long rtas_rmo_buf;
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 27d53f34494d..7883f973e8c9 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -769,6 +769,17 @@ EXPORT_SYMBOL_GPL(rtas_data_buf);
>>   unsigned long rtas_rmo_buf;
>> +/*
>> + * Physical address of the ibm,errinjct work buffer.  Allocated during
>> + * rtas_initialize() using memblock_phys_alloc_range() below rtas_region
>> + * so the address fits in 32 bits and is safe to pass to RTAS firmware.
>> + */
>> +unsigned long rtas_errinjct_buf;
>> +EXPORT_SYMBOL_GPL(rtas_errinjct_buf);
>> +
>> +DEFINE_MUTEX(rtas_errinjct_mutex);
>> +EXPORT_SYMBOL_GPL(rtas_errinjct_mutex);
>> +
>>   /*
>>    * If non-NULL, this gets called when the kernel terminates.
>>    * This is done like this so rtas_flash can be a module.
>> @@ -2109,6 +2120,12 @@ void __init rtas_initialize(void)
>>           panic("ERROR: RTAS: Failed to allocate %lx bytes below %pa\n",
>>                 PAGE_SIZE, &rtas_region);
>> +    rtas_errinjct_buf = 
>> memblock_phys_alloc_range(RTAS_ERRINJCT_BUF_SIZE,
>> +                              SZ_1K, 0, rtas_region);
>> +    if (!rtas_errinjct_buf)
>> +        panic("ERROR: RTAS: Failed to allocate %lu bytes below %pa\n",
>> +              (unsigned long)RTAS_ERRINJCT_BUF_SIZE, &rtas_region);
>> +
>>       rtas_work_area_reserve_arena(rtas_region);
>>   }
>