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

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

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.

- 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 <nnmlinux@linux.ibm.com>
> ---
>   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);
>   }
>