diff mbox series

[v4,07/13] powerpc/rtas: Warn if per-function lock isn't held

Message ID 20231117-papr-sys_rtas-vs-lockdown-v4-7-b794d8cb8502@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series powerpc/pseries: New character devices for system parameters and VPD | expand

Commit Message

Nathan Lynch via B4 Relay Nov. 18, 2023, 5:14 a.m. UTC
From: Nathan Lynch <nathanl@linux.ibm.com>

If the function descriptor has a populated lock member, then callers
are required to hold it across calls. Now that the firmware activation
sequence is appropriately guarded, we can warn when the requirement
isn't satisfied.

__do_enter_rtas_trace() gets reorganized a bit as a result of
performing the function descriptor lookup unconditionally now.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
 arch/powerpc/kernel/rtas.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Comments

Aneesh Kumar K.V Nov. 20, 2023, 8:13 a.m. UTC | #1
Nathan Lynch via B4 Relay <devnull+nathanl.linux.ibm.com@kernel.org>
writes:

> From: Nathan Lynch <nathanl@linux.ibm.com>
>
> If the function descriptor has a populated lock member, then callers
> are required to hold it across calls. Now that the firmware activation
> sequence is appropriately guarded, we can warn when the requirement
> isn't satisfied.
>
> __do_enter_rtas_trace() gets reorganized a bit as a result of
> performing the function descriptor lookup unconditionally now.
>

Reviewed-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org>

> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
> ---
>  arch/powerpc/kernel/rtas.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index e38ba05ad613..deb6289fcf9c 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -685,28 +685,25 @@ static void __do_enter_rtas(struct rtas_args *args)
>  
>  static void __do_enter_rtas_trace(struct rtas_args *args)
>  {
> -	const char *name = NULL;
> +	struct rtas_function *func = rtas_token_to_function(be32_to_cpu(args->token));
>  
> -	if (args == &rtas_args)
> -		lockdep_assert_held(&rtas_lock);
>  	/*
> -	 * If the tracepoints that consume the function name aren't
> -	 * active, avoid the lookup.
> +	 * If there is a per-function lock, it must be held by the
> +	 * caller.
>  	 */
> -	if ((trace_rtas_input_enabled() || trace_rtas_output_enabled())) {
> -		const s32 token = be32_to_cpu(args->token);
> -		const struct rtas_function *func = rtas_token_to_function(token);
> +	if (func->lock)
> +		WARN_ON(!mutex_is_locked(func->lock));
>  
> -		name = func->name;
> -	}
> +	if (args == &rtas_args)
> +		lockdep_assert_held(&rtas_lock);
>  
> -	trace_rtas_input(args, name);
> +	trace_rtas_input(args, func->name);
>  	trace_rtas_ll_entry(args);
>  
>  	__do_enter_rtas(args);
>  
>  	trace_rtas_ll_exit(args);
> -	trace_rtas_output(args, name);
> +	trace_rtas_output(args, func->name);
>  }
>  
>  static void do_enter_rtas(struct rtas_args *args)
>
> -- 
> 2.41.0
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index e38ba05ad613..deb6289fcf9c 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -685,28 +685,25 @@  static void __do_enter_rtas(struct rtas_args *args)
 
 static void __do_enter_rtas_trace(struct rtas_args *args)
 {
-	const char *name = NULL;
+	struct rtas_function *func = rtas_token_to_function(be32_to_cpu(args->token));
 
-	if (args == &rtas_args)
-		lockdep_assert_held(&rtas_lock);
 	/*
-	 * If the tracepoints that consume the function name aren't
-	 * active, avoid the lookup.
+	 * If there is a per-function lock, it must be held by the
+	 * caller.
 	 */
-	if ((trace_rtas_input_enabled() || trace_rtas_output_enabled())) {
-		const s32 token = be32_to_cpu(args->token);
-		const struct rtas_function *func = rtas_token_to_function(token);
+	if (func->lock)
+		WARN_ON(!mutex_is_locked(func->lock));
 
-		name = func->name;
-	}
+	if (args == &rtas_args)
+		lockdep_assert_held(&rtas_lock);
 
-	trace_rtas_input(args, name);
+	trace_rtas_input(args, func->name);
 	trace_rtas_ll_entry(args);
 
 	__do_enter_rtas(args);
 
 	trace_rtas_ll_exit(args);
-	trace_rtas_output(args, name);
+	trace_rtas_output(args, func->name);
 }
 
 static void do_enter_rtas(struct rtas_args *args)