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

Sourabh Jain <[email protected]>
Newsgroups org.ozlabs.lists.linuxppc-dev,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[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);
>   }
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.