[PATCH v2 3/4] powerpc/rtas: remove lock and args fields from global rtas struct

[Laurent Dufour]

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 at fr.ibm.com>

> 
> Suggested-by: Laurent Dufour <ldufour at linux.ibm.com>
> Signed-off-by: Nathan Lynch <nathanl at 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. */