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

From: Andrew Donnellan <ajd@linux.ibm.com>
To: Nathan Lynch <nathanl@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org
Cc: tyreld@linux.ibm.com, nnac123@linux.ibm.com,
	ldufour@linux.ibm.com, npiggin@gmail.com
Subject: Re: [PATCH 01/13] powerpc/rtas: document rtas_call()
Date: Tue, 22 Nov 2022 13:46:00 +1100	[thread overview]
Message-ID: <b4fbedbbf2009438946b9af884b2363f7886c606.camel@linux.ibm.com> (raw)
In-Reply-To: <20221118150751.469393-2-nathanl@linux.ibm.com>

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 <nathanl@linux.ibm.com>

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 <ajd@linux.ibm.com>

> ---
>  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_ERROR            0x80000000 /* set bit 0 */
>  #define RTAS_EPOW_WARNING              0x40000000 /* 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.
> + * *                      -9000 - Multi-level isolation error.
> + * *              -9999...-9004 - Vendor-specific error codes.
> + * * Additional negative values - Function-specific error.
> + * * Additional positive values - Function-specific success.
> + */
>  int rtas_call(int token, int nargs, int nret, int *outputs, ...)
>  {
>         va_list list;

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited