| Message ID | 20230220-rtas-queue-for-6-4-v1-7-010e4416f13f@linux.ibm.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | RTAS changes for 6.4 | expand |
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote: > From: Nathan Lynch <nathanl@linux.ibm.com> > > Any caller of rtas_call_unlocked() must provide an rtas_args > parameter > block distinct from the core rtas_args buffer used by the rtas_call() > path. It's an unlikely error to make, but the potential consequences > are grim, and it's trivial to check. > > Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com> call_rtas_display_status() seems to do exactly this, or am I missing something? > --- > arch/powerpc/kernel/rtas.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c > index 633c925164e7..47a2aa43d7d4 100644 > --- a/arch/powerpc/kernel/rtas.c > +++ b/arch/powerpc/kernel/rtas.c > @@ -1042,6 +1042,13 @@ void rtas_call_unlocked(struct rtas_args > *args, int token, int nargs, int nret, > { > va_list list; > > + /* > + * Callers must not use rtas_args; otherwise they risk > + * corrupting the state of the rtas_call() path, which is > + * serialized by rtas_lock. > + */ > + WARN_ON(args == &rtas_args); > + > va_start(list, nret); > va_rtas_call(args, token, nargs, nret, list); > va_end(list); >
Andrew Donnellan <ajd@linux.ibm.com> writes: > On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote: >> From: Nathan Lynch <nathanl@linux.ibm.com> >> >> Any caller of rtas_call_unlocked() must provide an rtas_args >> parameter >> block distinct from the core rtas_args buffer used by the rtas_call() >> path. It's an unlikely error to make, but the potential consequences >> are grim, and it's trivial to check. >> >> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com> > > call_rtas_display_status() seems to do exactly this, or am I missing > something? No you're right, the warning would be spurious in that case. May need to drop this one, or refactor rtas_call(): 4456f4524604be2558e5f6a8e0f7cc9ed17c783e Author: Michael Ellerman <mpe@ellerman.id.au> AuthorDate: Tue Nov 24 22:26:11 2015 +1100 powerpc/rtas: Use rtas_call_unlocked() in call_rtas_display_status() Although call_rtas_display_status() does actually want to use the regular RTAS locking, it doesn't want the extra logic that is in rtas_call(), so currently it open codes the logic. Instead we can use rtas_call_unlocked(), after taking the RTAS lock. aside: does anyone know if the display_status() code is worth keeping? It looks like it is used to drive the 16-character wide physical LCD I remember seeing on P4-era and older machines. Is it a vestige of non-LPAR pseries that should be dropped, or is it perhaps useful for chrp or cell?
Nathan Lynch <nathanl@linux.ibm.com> writes: > > aside: does anyone know if the display_status() code is worth keeping? > It looks like it is used to drive the 16-character wide physical LCD I > remember seeing on P4-era and older machines. Is it a vestige of > non-LPAR pseries that should be dropped, or is it perhaps useful for > chrp or cell? Never mind, I see the display-character token and associated properties on a P8 LPAR and in a current PAPR.
Nathan Lynch <nathanl@linux.ibm.com> writes: > Nathan Lynch <nathanl@linux.ibm.com> writes: >> >> aside: does anyone know if the display_status() code is worth keeping? >> It looks like it is used to drive the 16-character wide physical LCD I >> remember seeing on P4-era and older machines. Is it a vestige of >> non-LPAR pseries that should be dropped, or is it perhaps useful for >> chrp or cell? > > Never mind, I see the display-character token and associated properties > on a P8 LPAR and in a current PAPR. Or a P10 LPAR even. The characters written using it are shown on the HMC, somewhere on the partition info page. cheers
Michael Ellerman <mpe@ellerman.id.au> writes: > Nathan Lynch <nathanl@linux.ibm.com> writes: >> Nathan Lynch <nathanl@linux.ibm.com> writes: >>> >>> aside: does anyone know if the display_status() code is worth keeping? >>> It looks like it is used to drive the 16-character wide physical LCD I >>> remember seeing on P4-era and older machines. Is it a vestige of >>> non-LPAR pseries that should be dropped, or is it perhaps useful for >>> chrp or cell? >> >> Never mind, I see the display-character token and associated properties >> on a P8 LPAR and in a current PAPR. > > Or a P10 LPAR even. > > The characters written using it are shown on the HMC, somewhere on the > partition info page. Yes, in the "Reference code" field. On the command line, you can see the history, too: hscroot@ltchmcv3:~> lsrefcode -r lpar -m ltczep4 --filter lpar_names=ltczep4-lp3 -n 10 -F 'time_stamp refcode' "03/28/2023 18:38:20" "Linux ppc64le" "03/28/2023 18:38:19" CA000093 "03/28/2023 18:38:19" CA00E891 "03/28/2023 18:38:19" CA260208 "03/28/2023 18:38:19" CA260208 "03/28/2023 18:38:19" CA00E890 "03/28/2023 18:38:19" CA000050 "03/28/2023 18:38:19" CA000040 "03/28/2023 18:38:19" CA00E880 "03/28/2023 18:38:19" CA00E879 https://www.ibm.com/docs/en/power10/000V-HMC?topic=commands-lsrefcode
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c index 633c925164e7..47a2aa43d7d4 100644 --- a/arch/powerpc/kernel/rtas.c +++ b/arch/powerpc/kernel/rtas.c @@ -1042,6 +1042,13 @@ void rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret, { va_list list; + /* + * Callers must not use rtas_args; otherwise they risk + * corrupting the state of the rtas_call() path, which is + * serialized by rtas_lock. + */ + WARN_ON(args == &rtas_args); + va_start(list, nret); va_rtas_call(args, token, nargs, nret, list); va_end(list);