| Message ID | 20230124140448.45938-4-nathanl@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 599af49155467148afaf0bc3c0114bd80fd4491f |
| Headers | show |
| Series | powerpc/rtas: exports and locking | expand |
On 24/01/2023 15:04:47, Nathan Lynch wrote: > Only code internal to the RTAS subsystem needs access to the central > lock and parameter block. Remove these from the globally visible > 'rtas' struct and make them file-static in rtas.c. > > Some changed lines in rtas_call() lack appropriate spacing around > operators and cause checkpatch errors; fix these as well. Reviewed-by: Laurent Dufour <laurent.dufour@fr.ibm.com> > > Suggested-by: Laurent Dufour <ldufour@linux.ibm.com> > Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com> > --- > arch/powerpc/include/asm/rtas-types.h | 2 -- > arch/powerpc/kernel/rtas.c | 50 ++++++++++++++++----------- > 2 files changed, 29 insertions(+), 23 deletions(-) > > diff --git a/arch/powerpc/include/asm/rtas-types.h b/arch/powerpc/include/asm/rtas-types.h > index 8df6235d64d1..f2ad4a96cbc5 100644 > --- a/arch/powerpc/include/asm/rtas-types.h > +++ b/arch/powerpc/include/asm/rtas-types.h > @@ -18,8 +18,6 @@ struct rtas_t { > unsigned long entry; /* physical address pointer */ > unsigned long base; /* physical address pointer */ > unsigned long size; > - arch_spinlock_t lock; > - struct rtas_args args; > struct device_node *dev; /* virtual address pointer */ > }; > > diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c > index e60e2f5af7b9..0059bb2a8f04 100644 > --- a/arch/powerpc/kernel/rtas.c > +++ b/arch/powerpc/kernel/rtas.c > @@ -60,9 +60,17 @@ static inline void do_enter_rtas(unsigned long args) > srr_regs_clobbered(); /* rtas uses SRRs, invalidate */ > } > > -struct rtas_t rtas = { > - .lock = __ARCH_SPIN_LOCK_UNLOCKED > -}; > +struct rtas_t rtas; > + > +/* > + * Nearly all RTAS calls need to be serialized. All uses of the > + * default rtas_args block must hold rtas_lock. > + * > + * Exceptions to the RTAS serialization requirement (e.g. stop-self) > + * must use a separate rtas_args structure. > + */ > +static arch_spinlock_t rtas_lock = __ARCH_SPIN_LOCK_UNLOCKED; > +static struct rtas_args rtas_args; > > DEFINE_SPINLOCK(rtas_data_buf_lock); > EXPORT_SYMBOL_GPL(rtas_data_buf_lock); > @@ -90,13 +98,13 @@ static unsigned long lock_rtas(void) > > local_irq_save(flags); > preempt_disable(); > - arch_spin_lock(&rtas.lock); > + arch_spin_lock(&rtas_lock); > return flags; > } > > static void unlock_rtas(unsigned long flags) > { > - arch_spin_unlock(&rtas.lock); > + arch_spin_unlock(&rtas_lock); > local_irq_restore(flags); > preempt_enable(); > } > @@ -114,7 +122,7 @@ static void call_rtas_display_status(unsigned char c) > return; > > s = lock_rtas(); > - rtas_call_unlocked(&rtas.args, 10, 1, 1, NULL, c); > + rtas_call_unlocked(&rtas_args, 10, 1, 1, NULL, c); > unlock_rtas(s); > } > > @@ -386,7 +394,7 @@ static int rtas_last_error_token; > * most recent failed call to rtas. Because the error text > * might go stale if there are any other intervening rtas calls, > * this routine must be called atomically with whatever produced > - * the error (i.e. with rtas.lock still held from the previous call). > + * the error (i.e. with rtas_lock still held from the previous call). > */ > static char *__fetch_rtas_last_error(char *altbuf) > { > @@ -406,13 +414,13 @@ static char *__fetch_rtas_last_error(char *altbuf) > err_args.args[1] = cpu_to_be32(bufsz); > err_args.args[2] = 0; > > - save_args = rtas.args; > - rtas.args = err_args; > + save_args = rtas_args; > + rtas_args = err_args; > > - do_enter_rtas(__pa(&rtas.args)); > + do_enter_rtas(__pa(&rtas_args)); > > - err_args = rtas.args; > - rtas.args = save_args; > + err_args = rtas_args; > + rtas_args = save_args; > > /* Log the error in the unlikely case that there was one. */ > if (unlikely(err_args.args[2] == 0)) { > @@ -534,7 +542,7 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...) > va_list list; > int i; > unsigned long s; > - struct rtas_args *rtas_args; > + struct rtas_args *args; > char *buff_copy = NULL; > int ret; > > @@ -559,21 +567,21 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...) > s = lock_rtas(); > > /* We use the global rtas args buffer */ > - rtas_args = &rtas.args; > + args = &rtas_args; > > va_start(list, outputs); > - va_rtas_call_unlocked(rtas_args, token, nargs, nret, list); > + va_rtas_call_unlocked(args, token, nargs, nret, 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 (be32_to_cpu(args->rets[0]) == -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; > + outputs[i] = be32_to_cpu(args->rets[i + 1]); > + ret = (nret > 0) ? be32_to_cpu(args->rets[0]) : 0; > > unlock_rtas(s); > > @@ -1269,9 +1277,9 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs) > > flags = lock_rtas(); > > - rtas.args = args; > - do_enter_rtas(__pa(&rtas.args)); > - args = rtas.args; > + rtas_args = args; > + do_enter_rtas(__pa(&rtas_args)); > + args = rtas_args; > > /* A -1 return code indicates that the last command couldn't > be completed due to a hardware error. */
On Tue, 2023-01-24 at 08:04 -0600, Nathan Lynch wrote: > Only code internal to the RTAS subsystem needs access to the central > lock and parameter block. Remove these from the globally visible > 'rtas' struct and make them file-static in rtas.c. > > Some changed lines in rtas_call() lack appropriate spacing around > operators and cause checkpatch errors; fix these as well. > > Suggested-by: Laurent Dufour <ldufour@linux.ibm.com> > Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
diff --git a/arch/powerpc/include/asm/rtas-types.h b/arch/powerpc/include/asm/rtas-types.h index 8df6235d64d1..f2ad4a96cbc5 100644 --- a/arch/powerpc/include/asm/rtas-types.h +++ b/arch/powerpc/include/asm/rtas-types.h @@ -18,8 +18,6 @@ struct rtas_t { unsigned long entry; /* physical address pointer */ unsigned long base; /* physical address pointer */ unsigned long size; - arch_spinlock_t lock; - struct rtas_args args; struct device_node *dev; /* virtual address pointer */ }; diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c index e60e2f5af7b9..0059bb2a8f04 100644 --- a/arch/powerpc/kernel/rtas.c +++ b/arch/powerpc/kernel/rtas.c @@ -60,9 +60,17 @@ static inline void do_enter_rtas(unsigned long args) srr_regs_clobbered(); /* rtas uses SRRs, invalidate */ } -struct rtas_t rtas = { - .lock = __ARCH_SPIN_LOCK_UNLOCKED -}; +struct rtas_t rtas; + +/* + * Nearly all RTAS calls need to be serialized. All uses of the + * default rtas_args block must hold rtas_lock. + * + * Exceptions to the RTAS serialization requirement (e.g. stop-self) + * must use a separate rtas_args structure. + */ +static arch_spinlock_t rtas_lock = __ARCH_SPIN_LOCK_UNLOCKED; +static struct rtas_args rtas_args; DEFINE_SPINLOCK(rtas_data_buf_lock); EXPORT_SYMBOL_GPL(rtas_data_buf_lock); @@ -90,13 +98,13 @@ static unsigned long lock_rtas(void) local_irq_save(flags); preempt_disable(); - arch_spin_lock(&rtas.lock); + arch_spin_lock(&rtas_lock); return flags; } static void unlock_rtas(unsigned long flags) { - arch_spin_unlock(&rtas.lock); + arch_spin_unlock(&rtas_lock); local_irq_restore(flags); preempt_enable(); } @@ -114,7 +122,7 @@ static void call_rtas_display_status(unsigned char c) return; s = lock_rtas(); - rtas_call_unlocked(&rtas.args, 10, 1, 1, NULL, c); + rtas_call_unlocked(&rtas_args, 10, 1, 1, NULL, c); unlock_rtas(s); } @@ -386,7 +394,7 @@ static int rtas_last_error_token; * most recent failed call to rtas. Because the error text * might go stale if there are any other intervening rtas calls, * this routine must be called atomically with whatever produced - * the error (i.e. with rtas.lock still held from the previous call). + * the error (i.e. with rtas_lock still held from the previous call). */ static char *__fetch_rtas_last_error(char *altbuf) { @@ -406,13 +414,13 @@ static char *__fetch_rtas_last_error(char *altbuf) err_args.args[1] = cpu_to_be32(bufsz); err_args.args[2] = 0; - save_args = rtas.args; - rtas.args = err_args; + save_args = rtas_args; + rtas_args = err_args; - do_enter_rtas(__pa(&rtas.args)); + do_enter_rtas(__pa(&rtas_args)); - err_args = rtas.args; - rtas.args = save_args; + err_args = rtas_args; + rtas_args = save_args; /* Log the error in the unlikely case that there was one. */ if (unlikely(err_args.args[2] == 0)) { @@ -534,7 +542,7 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...) va_list list; int i; unsigned long s; - struct rtas_args *rtas_args; + struct rtas_args *args; char *buff_copy = NULL; int ret; @@ -559,21 +567,21 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...) s = lock_rtas(); /* We use the global rtas args buffer */ - rtas_args = &rtas.args; + args = &rtas_args; va_start(list, outputs); - va_rtas_call_unlocked(rtas_args, token, nargs, nret, list); + va_rtas_call_unlocked(args, token, nargs, nret, 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 (be32_to_cpu(args->rets[0]) == -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; + outputs[i] = be32_to_cpu(args->rets[i + 1]); + ret = (nret > 0) ? be32_to_cpu(args->rets[0]) : 0; unlock_rtas(s); @@ -1269,9 +1277,9 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs) flags = lock_rtas(); - rtas.args = args; - do_enter_rtas(__pa(&rtas.args)); - args = rtas.args; + rtas_args = args; + do_enter_rtas(__pa(&rtas_args)); + args = rtas_args; /* A -1 return code indicates that the last command couldn't be completed due to a hardware error. */
Only code internal to the RTAS subsystem needs access to the central lock and parameter block. Remove these from the globally visible 'rtas' struct and make them file-static in rtas.c. Some changed lines in rtas_call() lack appropriate spacing around operators and cause checkpatch errors; fix these as well. Suggested-by: Laurent Dufour <ldufour@linux.ibm.com> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com> --- arch/powerpc/include/asm/rtas-types.h | 2 -- arch/powerpc/kernel/rtas.c | 50 ++++++++++++++++----------- 2 files changed, 29 insertions(+), 23 deletions(-)