Re: [PATCH 01/13] powerpc/rtas: document rtas_call()

2022-11-21 Thread Andrew Donnellan
On Fri, 2022-11-18 at 09:07 -0600, Nathan Lynch wrote:
> rtas_call() has a complex calling convention, non-standard return
> values, and many users. Add kernel-doc for it and remove the less
> structured commentary from rtas.h.
> 
> Signed-off-by: Nathan Lynch 

Excellent work!

I'm not overly familiar with some of the RTAS internal stuff you
describe, but I've checked the status codes against my copy of PAPR and
they concur, so I'll give this a:

Reviewed-by: Andrew Donnellan 

> ---
>  arch/powerpc/include/asm/rtas.h | 15 -
>  arch/powerpc/kernel/rtas.c  | 58
> +
>  2 files changed, 58 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/rtas.h
> b/arch/powerpc/include/asm/rtas.h
> index 56319aea646e..479a95cb2770 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -33,21 +33,6 @@
>  #define RTAS_THREADS_ACTIVE -9005 /* Multiple processor threads
> active */
>  #define RTAS_OUTSTANDING_COPROC -9006 /* Outstanding coprocessor
> operations */
>  
> -/*
> - * In general to call RTAS use rtas_token("string") to lookup
> - * an RTAS token for the given string (e.g. "event-scan").
> - * To actually perform the call use
> - *    ret = rtas_call(token, n_in, n_out, ...)
> - * Where n_in is the number of input parameters and
> - *   n_out is the number of output parameters
> - *
> - * If the "string" is invalid on this system, RTAS_UNKNOWN_SERVICE
> - * will be returned as a token.  rtas_call() does look for this
> - * token and error out gracefully so rtas_call(rtas_token("str"),
> ...)
> - * may be safely used for one-shot calls to RTAS.
> - *
> - */
> -
>  /* RTAS event classes */
>  #define RTAS_INTERNAL_ERROR0x8000 /* set bit 0 */
>  #define RTAS_EPOW_WARNING  0x4000 /* set bit 1 */
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index e847f9b1c5b9..c12dd5ed5e00 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -467,6 +467,64 @@ void rtas_call_unlocked(struct rtas_args *args,
> int token, int nargs, int nret,
>  static int ibm_open_errinjct_token;
>  static int ibm_errinjct_token;
>  
> +/**
> + * rtas_call() - Invoke an RTAS firmware function.
> + * @token: Identifies the function being invoked.
> + * @nargs: Number of input parameters. Does not include token.
> + * @nret: Number of output parameters, including the call status.
> + * @outputs: Array of @nret output words.
> + * @: List of @nargs input parameters.
> + *
> + * Invokes the RTAS function indicated by @token, which the caller
> + * should obtain via rtas_token().
> + *
> + * The @nargs and @nret arguments must match the number of input and
> + * output parameters specified for the RTAS function.
> + *
> + * rtas_call() returns RTAS status codes, not conventional Linux
> errno
> + * values. Callers must translate any failure to an appropriate
> errno
> + * in syscall context. Most callers of RTAS functions that can
> return
> + * -2 or 990x should use rtas_busy_delay() to correctly handle those
> + * statuses before calling again.
> + *
> + * The return value descriptions are adapted from 7.2.8 [RTAS]
> Return
> + * Codes of the PAPR and CHRP specifications.
> + *
> + * Context: Process context preferably, interrupt context if
> + *  necessary.  Acquires an internal spinlock and may
> perform
> + *  GFP_ATOMIC slab allocation in error path. Unsafe for NMI
> + *  context.
> + * Return:
> + * *  0 - RTAS function call succeeded.
> + * * -1 - RTAS function encountered a
> hardware or
> + *    platform error, or the token is
> invalid,
> + *    or the function is restricted by
> kernel policy.
> + * * -2 - Specs say "A necessary hardware
> device was busy,
> + *    and the requested function could
> not be
> + *    performed. The operation should be
> retried at
> + *    a later time." This is misleading,
> at least with
> + *    respect to current RTAS
> implementations. What it
> + *    usually means in practice is that
> the function
> + *    could not be completed while
> meeting RTAS's
> + *    deadline for returning control to
> the OS (250us
> + *    for PAPR/PowerVM, typically), but
> the call may be
> + *    immediately reattempted to resume
> work on it.
> + * * -3 - Parameter error.
> + * * -7 - Unexpected state change.
> + * *    9000...9899 - Vendor-specific success codes.
> + * *    9900...9905 - Advisory extended delay. Caller
> should try
> + *  

[PATCH 01/13] powerpc/rtas: document rtas_call()

2022-11-18 Thread Nathan Lynch
rtas_call() has a complex calling convention, non-standard return
values, and many users. Add kernel-doc for it and remove the less
structured commentary from rtas.h.

Signed-off-by: Nathan Lynch 
---
 arch/powerpc/include/asm/rtas.h | 15 -
 arch/powerpc/kernel/rtas.c  | 58 +
 2 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 56319aea646e..479a95cb2770 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -33,21 +33,6 @@
 #define RTAS_THREADS_ACTIVE -9005 /* Multiple processor threads active */
 #define RTAS_OUTSTANDING_COPROC -9006 /* Outstanding coprocessor operations */
 
-/*
- * In general to call RTAS use rtas_token("string") to lookup
- * an RTAS token for the given string (e.g. "event-scan").
- * To actually perform the call use
- *ret = rtas_call(token, n_in, n_out, ...)
- * Where n_in is the number of input parameters and
- *   n_out is the number of output parameters
- *
- * If the "string" is invalid on this system, RTAS_UNKNOWN_SERVICE
- * will be returned as a token.  rtas_call() does look for this
- * token and error out gracefully so rtas_call(rtas_token("str"), ...)
- * may be safely used for one-shot calls to RTAS.
- *
- */
-
 /* RTAS event classes */
 #define RTAS_INTERNAL_ERROR0x8000 /* set bit 0 */
 #define RTAS_EPOW_WARNING  0x4000 /* set bit 1 */
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index e847f9b1c5b9..c12dd5ed5e00 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -467,6 +467,64 @@ void rtas_call_unlocked(struct rtas_args *args, int token, 
int nargs, int nret,
 static int ibm_open_errinjct_token;
 static int ibm_errinjct_token;
 
+/**
+ * rtas_call() - Invoke an RTAS firmware function.
+ * @token: Identifies the function being invoked.
+ * @nargs: Number of input parameters. Does not include token.
+ * @nret: Number of output parameters, including the call status.
+ * @outputs: Array of @nret output words.
+ * @: List of @nargs input parameters.
+ *
+ * Invokes the RTAS function indicated by @token, which the caller
+ * should obtain via rtas_token().
+ *
+ * The @nargs and @nret arguments must match the number of input and
+ * output parameters specified for the RTAS function.
+ *
+ * rtas_call() returns RTAS status codes, not conventional Linux errno
+ * values. Callers must translate any failure to an appropriate errno
+ * in syscall context. Most callers of RTAS functions that can return
+ * -2 or 990x should use rtas_busy_delay() to correctly handle those
+ * statuses before calling again.
+ *
+ * The return value descriptions are adapted from 7.2.8 [RTAS] Return
+ * Codes of the PAPR and CHRP specifications.
+ *
+ * Context: Process context preferably, interrupt context if
+ *  necessary.  Acquires an internal spinlock and may perform
+ *  GFP_ATOMIC slab allocation in error path. Unsafe for NMI
+ *  context.
+ * Return:
+ * *  0 - RTAS function call succeeded.
+ * * -1 - RTAS function encountered a hardware or
+ *platform error, or the token is invalid,
+ *or the function is restricted by kernel 
policy.
+ * * -2 - Specs say "A necessary hardware device was 
busy,
+ *and the requested function could not be
+ *performed. The operation should be retried at
+ *a later time." This is misleading, at least 
with
+ *respect to current RTAS implementations. 
What it
+ *usually means in practice is that the 
function
+ *could not be completed while meeting RTAS's
+ *deadline for returning control to the OS 
(250us
+ *for PAPR/PowerVM, typically), but the call 
may be
+ *immediately reattempted to resume work on it.
+ * * -3 - Parameter error.
+ * * -7 - Unexpected state change.
+ * *9000...9899 - Vendor-specific success codes.
+ * *9900...9905 - Advisory extended delay. Caller should try
+ *again after ~10^x ms has elapsed, where x is
+ *the last digit of the status [0-5]. Again 
going
+ *beyond the PAPR text, 990x on PowerVM 
indicates
+ *contention for RTAS-internal resources. Other
+ *RTAS call sequences in progress should be
+ *allowed to complete before reattempting the
+ *call.