Re: [PATCH v5 2/2] powerpc/rtas: Implement reentrant rtas call

[Nicholas Piggin]

Excerpts from Leonardo Bras's message of May 16, 2020 3:21 pm:
> Implement rtas_call_reentrant() for reentrant rtas-calls:
> "ibm,int-on", "ibm,int-off",ibm,get-xive" and  "ibm,set-xive".
> 
> On LoPAPR Version 1.1 (March 24, 2016), from 7.3.10.1 to 7.3.10.4,
> items 2 and 3 say:
> 
> 2 - For the PowerPC External Interrupt option: The * call must be
> reentrant to the number of processors on the platform.
> 3 - For the PowerPC External Interrupt option: The * argument call
> buffer for each simultaneous call must be physically unique.
> 
> So, these rtas-calls can be called in a lockless way, if using
> a different buffer for each cpu doing such rtas call.
> 
> For this, it was suggested to add the buffer (struct rtas_args)
> in the PACA struct, so each cpu can have it's own buffer.
> The PACA struct received a pointer to rtas buffer, which is
> allocated in the memory range available to rtas 32-bit.
> 
> Reentrant rtas calls are useful to avoid deadlocks in crashing,
> where rtas-calls are needed, but some other thread crashed holding
> the rtas.lock.
> 
> This is a backtrace of a deadlock from a kdump testing environment:
> 
>   #0 arch_spin_lock
>   #1  lock_rtas ()
>   #2  rtas_call (token=8204, nargs=1, nret=1, outputs=0x0)
>   #3  ics_rtas_mask_real_irq (hw_irq=4100)
>   #4  machine_kexec_mask_interrupts
>   #5  default_machine_crash_shutdown
>   #6  machine_crash_shutdown
>   #7  __crash_kexec
>   #8  crash_kexec
>   #9  oops_end
> 
> Signed-off-by: Leonardo Bras <leobras.c@xxxxxxxxx>
> ---
>  arch/powerpc/include/asm/paca.h     |  2 ++
>  arch/powerpc/include/asm/rtas.h     |  1 +
>  arch/powerpc/kernel/paca.c          | 20 +++++++++++
>  arch/powerpc/kernel/rtas.c          | 52 +++++++++++++++++++++++++++++
>  arch/powerpc/sysdev/xics/ics-rtas.c | 22 ++++++------
>  5 files changed, 86 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index e3cc9eb9204d..87cd9c2220cc 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -29,6 +29,7 @@
>  #include <asm/hmi.h>
>  #include <asm/cpuidle.h>
>  #include <asm/atomic.h>
> +#include <asm/rtas-types.h>
>  
>  #include <asm-generic/mmiowb_types.h>
>  
> @@ -270,6 +271,7 @@ struct paca_struct {
>  #ifdef CONFIG_MMIOWB
>  	struct mmiowb_state mmiowb_state;
>  #endif
> +	struct rtas_args *reentrant_args;
>  } ____cacheline_aligned;
>  
>  extern void copy_mm_to_paca(struct mm_struct *mm);
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index c35c5350b7e4..fa7509c85881 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -236,6 +236,7 @@ extern struct rtas_t rtas;
>  extern int rtas_token(const char *service);
>  extern int rtas_service_present(const char *service);
>  extern int rtas_call(int token, int, int, int *, ...);
> +int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...);
>  void rtas_call_unlocked(struct rtas_args *args, int token, int nargs,
>  			int nret, ...);
>  extern void __noreturn rtas_restart(char *cmd);
> diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
> index 3f91ccaa9c74..88c9b61489fc 100644
> --- a/arch/powerpc/kernel/paca.c
> +++ b/arch/powerpc/kernel/paca.c
> @@ -16,6 +16,7 @@
>  #include <asm/kexec.h>
>  #include <asm/svm.h>
>  #include <asm/ultravisor.h>
> +#include <asm/rtas.h>
>  
>  #include "setup.h"
>  
> @@ -164,6 +165,23 @@ static struct slb_shadow * __init new_slb_shadow(int cpu, unsigned long limit)
>  
>  #endif /* CONFIG_PPC_BOOK3S_64 */
>  
> +/**
> + * new_rtas_args() - Allocates rtas args
> + * @cpu:	CPU number
> + * @limit:	Memory limit for this allocation
> + *
> + * Allocates a struct rtas_args and return it's pointer.
> + *
> + * Return:	Pointer to allocated rtas_args
> + */
> +static struct rtas_args * __init new_rtas_args(int cpu, unsigned long limit)
> +{
> +	limit = min_t(unsigned long, limit, RTAS_INSTANTIATE_MAX);
> +
> +	return alloc_paca_data(sizeof(struct rtas_args), L1_CACHE_BYTES,
> +			       limit, cpu);
> +}
> +
>  /* The Paca is an array with one entry per processor.  Each contains an
>   * lppaca, which contains the information shared between the
>   * hypervisor and Linux.
> @@ -202,6 +220,7 @@ void __init __nostackprotector initialise_paca(struct paca_struct *new_paca, int
>  	/* For now -- if we have threads this will be adjusted later */
>  	new_paca->tcd_ptr = &new_paca->tcd;
>  #endif
> +	new_paca->reentrant_args = NULL;
>  }
>  
>  /* Put the paca pointer into r13 and SPRG_PACA */
> @@ -274,6 +293,7 @@ void __init allocate_paca(int cpu)
>  #ifdef CONFIG_PPC_BOOK3S_64
>  	paca->slb_shadow_ptr = new_slb_shadow(cpu, limit);
>  #endif
> +	paca->reentrant_args = new_rtas_args(cpu, limit);

Good, I think tihs should work as you want now. Can you allocate it like 
lppacas? Put it under PSERIES (and in the paca) and check for !HV?

Thanks,
Nick