Re: [PATCH v4 02/13] powerpc/rtas: Fall back to linear search on failed token->function lookup

[Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org>]

Nathan Lynch via B4 Relay <devnull+nathanl.linux.ibm.com@kernel.org>
writes:

> From: Nathan Lynch <nathanl@linux.ibm.com>
>
> Enabling any of the powerpc:rtas_* tracepoints at boot is likely to
> result in an oops on RTAS platforms. For example, booting a QEMU
> pseries model with 'trace_event=powerpc:rtas_input' in the command
> line leads to:
>
>   BUG: Kernel NULL pointer dereference on read at 0x00000008
>   Oops: Kernel access of bad area, sig: 7 [#1]
>   NIP [c00000000004231c] do_enter_rtas+0x1bc/0x460
>   LR [c00000000004231c] do_enter_rtas+0x1bc/0x460
>   Call Trace:
>     do_enter_rtas+0x1bc/0x460 (unreliable)
>     rtas_call+0x22c/0x4a0
>     rtas_get_boot_time+0x80/0x14c
>     read_persistent_clock64+0x124/0x150
>     read_persistent_wall_and_boot_offset+0x28/0x58
>     timekeeping_init+0x70/0x348
>     start_kernel+0xa0c/0xc1c
>     start_here_common+0x1c/0x20
>
> (This is preceded by a warning for the failed lookup in
> rtas_token_to_function().)
>
> This happens when __do_enter_rtas_trace() attempts a token to function
> descriptor lookup before the xarray containing the mappings has been
> set up.
>
> Fall back to linear scan of the table if rtas_token_to_function_xarray
> is empty.
>

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

> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
> Fixes: 24098f580e2b ("powerpc/rtas: add tracepoints around RTAS entry")
> ---
>  arch/powerpc/kernel/rtas.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 1ad1869e2e96..f0051881348a 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -557,11 +557,21 @@ static const struct rtas_function *rtas_token_to_function(s32 token)
>  		return NULL;
>  
>  	func = xa_load(&rtas_token_to_function_xarray, token);
> +	if (func)
> +		return func;
> +	/*
> +	 * Fall back to linear scan in case the reverse mapping hasn't
> +	 * been initialized yet.
> +	 */
> +	if (xa_empty(&rtas_token_to_function_xarray)) {
> +		for_each_rtas_function(func) {
> +			if (func->token == token)
> +				return func;
> +		}
> +	}
>  
> -	if (WARN_ONCE(!func, "unexpected failed lookup for token %d", token))
> -		return NULL;
> -
> -	return func;
> +	WARN_ONCE(true, "unexpected failed lookup for token %d", token);
> +	return NULL;
>  }
>  
>  /* This is here deliberately so it's only used in this file */
>
> -- 
> 2.41.0