Message ID | 20220308135047.478297-11-npiggin@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | powerpc/rtas: various cleanups and improvements | expand |
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 <npiggin@gmail.com> Despite a minor comment below Reviewed-by: Laurent Dufour <ldufour@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 */
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, ...); 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 */
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 <npiggin@gmail.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(-)