On 08/03/2022, 14:50:43, Nicholas Piggin wrote:
> Use the same calling and rets return convention with the raw rtas
> call rather than requiring callers to load and byteswap return
> values out of rtas_args.
>
> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
Despite a minor comment below
Reviewed-by: Laurent Dufour <lduf...@linux.ibm.com>
> ---
> arch/powerpc/include/asm/rtas.h | 4 +-
> arch/powerpc/kernel/rtas.c | 53 +++++++++++---------
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +-
> arch/powerpc/platforms/pseries/ras.c | 7 +--
> arch/powerpc/xmon/xmon.c | 2 +-
> 5 files changed, 33 insertions(+), 35 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 82e5b055fa2a..1014ff140cf8 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -241,8 +241,8 @@ extern int rtas_token(const char *service);
> extern int rtas_service_present(const char *service);
> extern int rtas_call(int token, int, int, int *, ...);
> int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...);
> -void rtas_call_unlocked(struct rtas_args *args, int token, int nargs,
> - int nret, ...);
> +int raw_rtas_call(struct rtas_args *args, int token,
> + int nargs, int nret, int *outputs, ...);
Minor, would it be better to keep the "unlocked" suffix to advise that the
rtas lock is not held here?
> extern void __noreturn rtas_restart(char *cmd);
> extern void rtas_power_off(void);
> extern void __noreturn rtas_halt(void);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index fece066115f0..751a20669669 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -123,7 +123,7 @@ static void call_rtas_display_status(unsigned char c)
> return;
>
> s = lock_rtas();
> - rtas_call_unlocked(&rtas.args, 10, 1, 1, NULL, c);
> + raw_rtas_call(&rtas.args, 10, 1, 1, NULL, c);
> unlock_rtas(s);
> }
>
> @@ -434,10 +434,9 @@ static char *__fetch_rtas_last_error(char *altbuf)
> #define get_errorlog_buffer() NULL
> #endif
>
> -
> -static void
> -va_rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret,
> - va_list list)
> +static int notrace va_raw_rtas_call(struct rtas_args *args, int token,
> + int nargs, int nret, int *outputs,
> + va_list list)
> {
> int i;
>
> @@ -453,21 +452,37 @@ va_rtas_call_unlocked(struct rtas_args *args, int
> token, int nargs, int nret,
> args->rets[i] = 0;
>
> do_enter_rtas(__pa(args));
> +
> + if (nret > 1 && outputs != NULL) {
> + for (i = 0; i < nret-1; ++i)
> + outputs[i] = be32_to_cpu(args->rets[i+1]);
> + }
> +
> + return (nret > 0) ? be32_to_cpu(args->rets[0]) : 0;
> }
>
> -void rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int
> nret, ...)
> +/*
> + * Like rtas_call but no kmalloc or printk etc in error handling, so
> + * error won't go through log_error. No tracing, may be called in real mode.
> + * rtas_args must be supplied, and appropriate synchronization for the rtas
> + * call being made has to be performed by the caller.
> + */
> +int notrace raw_rtas_call(struct rtas_args *args, int token,
> + int nargs, int nret, int *outputs, ...)
> {
> va_list list;
> + int ret;
>
> - va_start(list, nret);
> - va_rtas_call_unlocked(args, token, nargs, nret, list);
> + va_start(list, outputs);
> + ret = va_raw_rtas_call(args, token, nargs, nret, outputs, list);
> va_end(list);
> +
> + return ret;
> }
>
> int rtas_call(int token, int nargs, int nret, int *outputs, ...)
> {
> va_list list;
> - int i;
> unsigned long s;
> struct rtas_args *rtas_args;
> char *buff_copy = NULL;
> @@ -482,19 +497,14 @@ int rtas_call(int token, int nargs, int nret, int
> *outputs, ...)
> rtas_args = &rtas.args;
>
> va_start(list, outputs);
> - va_rtas_call_unlocked(rtas_args, token, nargs, nret, list);
> + ret = va_raw_rtas_call(rtas_args, token, nargs, nret, outputs, list);
> va_end(list);
>
> /* A -1 return code indicates that the last command couldn't
> be completed due to a hardware error. */
> - if (be32_to_cpu(rtas_args->rets[0]) == -1)
> + if (ret == -1)
> buff_copy = __fetch_rtas_last_error(NULL);
>
> - if (nret > 1 && outputs != NULL)
> - for (i = 0; i < nret-1; ++i)
> - outputs[i] = be32_to_cpu(rtas_args->rets[i+1]);
> - ret = (nret > 0)? be32_to_cpu(rtas_args->rets[0]): 0;
> -
> unlock_rtas(s);
>
> if (buff_copy) {
> @@ -950,7 +960,7 @@ int rtas_call_reentrant(int token, int nargs, int nret,
> int *outputs, ...)
> va_list list;
> struct rtas_args *args;
> unsigned long flags;
> - int i, ret = 0;
> + int ret;
>
> if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
> return -1;
> @@ -962,16 +972,9 @@ int rtas_call_reentrant(int token, int nargs, int nret,
> int *outputs, ...)
> args = local_paca->rtas_args_reentrant;
>
> va_start(list, outputs);
> - va_rtas_call_unlocked(args, token, nargs, nret, list);
> + ret = va_raw_rtas_call(args, token, nargs, nret, outputs, list);
> va_end(list);
>
> - if (nret > 1 && outputs)
> - for (i = 0; i < nret - 1; ++i)
> - outputs[i] = be32_to_cpu(args->rets[i + 1]);
> -
> - if (nret > 0)
> - ret = be32_to_cpu(args->rets[0]);
> -
> local_irq_restore(flags);
> preempt_enable();
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index b81fc846d99c..17c05650b6b9 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -53,7 +53,7 @@ static void rtas_stop_self(void)
>
> BUG_ON(rtas_stop_self_token == RTAS_UNKNOWN_SERVICE);
>
> - rtas_call_unlocked(&args, rtas_stop_self_token, 0, 1, NULL);
> + raw_rtas_call(&args, rtas_stop_self_token, 0, 1, NULL);
>
> panic("Alas, I survived.\n");
> }
> diff --git a/arch/powerpc/platforms/pseries/ras.c
> b/arch/powerpc/platforms/pseries/ras.c
> index 74c9b1b5bc66..b009ed7de1ee 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -465,12 +465,7 @@ static void fwnmi_release_errinfo(void)
> struct rtas_args rtas_args;
> int ret;
>
> - /*
> - * On pseries, the machine check stack is limited to under 4GB, so
> - * args can be on-stack.
> - */
> - rtas_call_unlocked(&rtas_args, ibm_nmi_interlock_token, 0, 1, NULL);
> - ret = be32_to_cpu(rtas_args.rets[0]);
> + ret = raw_rtas_call(&rtas_args, ibm_nmi_interlock_token, 0, 1, NULL);
> if (ret != 0)
> printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
> }
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index fd72753e8ad5..6f53e8bccc33 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -410,7 +410,7 @@ static inline void disable_surveillance(void)
> if (set_indicator_token == RTAS_UNKNOWN_SERVICE)
> return;
>
> - rtas_call_unlocked(&args, set_indicator_token, 3, 1, NULL,
> + raw_rtas_call(&args, set_indicator_token, 3, 1, NULL,
> SURVEILLANCE_TOKEN, 0, 0);
>
> #endif /* CONFIG_PPC_PSERIES */
