Subject: Re: [PATCH] powerpc/rtas: use correct function name for resetting TCE tables

From: Michael Ellerman <[email-masked]>
Date: Fri, 23 Feb 2024 17:21:00 +1100

Nathan Lynch via B4 Relay <[email-masked]>
writes:
> From: Nathan Lynch <[email-masked]>
>
> The PAPR spec spells the function name as
>
>   "ibm,reset-pe-dma-windows"
>
> but in practice firmware uses the singular form:

Just to be clear, you're talking about IBM firmware on PowerVM machines.

>   "ibm,reset-pe-dma-window"
>
> in the device tree. Since we have the wrong spelling in the RTAS
> function table, reverse lookups (token -> name) fail and warn:
>
>   unexpected failed lookup for token 86
>   WARNING: CPU: 1 PID: 545 at arch/powerpc/kernel/rtas.c:659 __do_enter_rtas_trace+0x2a4/0x2b4
...
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 7e793b503e29..8064d9c3de86 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -375,8 +375,13 @@ static struct rtas_function rtas_function_table[] __ro_after_init = {
>  	[RTAS_FNIDX__IBM_REMOVE_PE_DMA_WINDOW] = {
>  		.name = "ibm,remove-pe-dma-window",
>  	},
> -	[RTAS_FNIDX__IBM_RESET_PE_DMA_WINDOWS] = {
> -		.name = "ibm,reset-pe-dma-windows",
> +	[RTAS_FNIDX__IBM_RESET_PE_DMA_WINDOW] = {
> +		/*
> +		 * Note: PAPR+ v2.13 7.3.31.4.1 spells this as
> +		 * "ibm,reset-pe-dma-windows" (plural), but RTAS
> +		 * implementations use the singular form in practice.
> +		 */
> +		.name = "ibm,reset-pe-dma-window",

Qemu also spells it that way:

$ grep -C 12 ibm,reset-pe-dma-window hw/ppc/spapr_rtas_ddw.c
static void spapr_rtas_ddw_init(void)
{
    spapr_rtas_register(RTAS_IBM_QUERY_PE_DMA_WINDOW,
                        "ibm,query-pe-dma-window",
                        rtas_ibm_query_pe_dma_window);
    spapr_rtas_register(RTAS_IBM_CREATE_PE_DMA_WINDOW,
                        "ibm,create-pe-dma-window",
                        rtas_ibm_create_pe_dma_window);
    spapr_rtas_register(RTAS_IBM_REMOVE_PE_DMA_WINDOW,
                        "ibm,remove-pe-dma-window",
                        rtas_ibm_remove_pe_dma_window);
    spapr_rtas_register(RTAS_IBM_RESET_PE_DMA_WINDOW,
                        "ibm,reset-pe-dma-window",
                        rtas_ibm_reset_pe_dma_window);
}

There's no version in SLOF, it delegates to Qemu.

The old platforms that use RTAS won't implement this call at all, so
there's no issue with the naming there.

So LGTM.

cheers