Re: [PATCH 13/13] powerpc/rtas: place tracepoints in do_enter_rtas()

From: Nathan Lynch <nathanl@linux.ibm.com>
To: Nicholas Piggin <npiggin@gmail.com>, linuxppc-dev@lists.ozlabs.org
Cc: tyreld@linux.ibm.com, nnac123@linux.ibm.com,
	ldufour@linux.ibm.com, ajd@linux.ibm.com
Subject: Re: [PATCH 13/13] powerpc/rtas: place tracepoints in do_enter_rtas()
Date: Mon, 28 Nov 2022 17:44:33 -0600	[thread overview]
Message-ID: <87tu2i8x0e.fsf@linux.ibm.com> (raw)
In-Reply-To: <CONLJ0VXR77H.JM4D75GQAF62@bobo>

"Nicholas Piggin" <npiggin@gmail.com> writes:

> On Sat Nov 19, 2022 at 1:07 AM AEST, Nathan Lynch wrote:
>> Call the just-added rtas tracepoints in do_enter_rtas(), taking care
>> to avoid function name lookups in the CPU offline path.
>>
>> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
>> ---
>>  arch/powerpc/kernel/rtas.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 198366d641d0..3487b42cfbf7 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -38,6 +38,7 @@
>>  #include <asm/page.h>
>>  #include <asm/rtas.h>
>>  #include <asm/time.h>
>> +#include <asm/trace.h>
>>  #include <asm/udbg.h>
>>  
>>  enum rtas_function_flags {
>> @@ -525,6 +526,7 @@ void enter_rtas(unsigned long);
>>  static void do_enter_rtas(struct rtas_args *args)
>>  {
>>  	unsigned long msr;
>> +	const char *name = NULL;
>>  
>>  	/*
>>  	 * Make sure MSR[RI] is currently enabled as it will be forced later
>> @@ -537,9 +539,30 @@ static void do_enter_rtas(struct rtas_args *args)
>>  
>>  	hard_irq_disable(); /* Ensure MSR[EE] is disabled on PPC64 */
>>  
>> +	if ((trace_rtas_input_enabled() || trace_rtas_output_enabled())) {
>> +		/*
>> +		 * rtas_token_to_function() uses xarray which uses RCU,
>> +		 * but this code can run in the CPU offline path
>> +		 * (e.g. stop-self), after it's become invalid to call
>> +		 * RCU APIs.
>> +		 */
>
> We can call this in real-mode via pseries_machine_check_realmode
> -> fwnmi_release_errinfo, so tracing should be disabled for that
> case too... Does this_cpu_set_ftrace_enabled(0) in the early
> machine check handler cover that sufficiently?

I suspect so, but I'd like to verify. Do you know how I could exercise
this path in qemu or LPAR?