Re: [Qemu-devel] [PATCH v4 16/18] spapr_rtas_ddw: Workaround broken LE guests

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6661 bytes --]

On Thu, Jan 29, 2015 at 08:27:28PM +1100, Alexey Kardashevskiy wrote:
> Recent kernels do parse results of what DDW RTAS calls return incorrectly
> if compiled with LITTLE_ENDIAN=yes.
> 
> This adds special handling for such guests.

I don't really follow this commit message.  You need to justify
including this ugly workaround for incorrect guests.

What are the guests that are out in the field which will trigger this behaviour?

> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/ppc/spapr_rtas.c     | 29 +++++++++++++++++++++++++++++
>  hw/ppc/spapr_rtas_ddw.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h  |  2 ++
>  3 files changed, 71 insertions(+)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 2ec2a8e..c3dee94 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -293,12 +293,15 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
>  static struct rtas_call {
>      const char *name;
>      spapr_rtas_fn fn;
> +    spapr_rtas_fn fn_wa; /* workaround helper */
>  } rtas_table[RTAS_TOKEN_MAX - RTAS_TOKEN_BASE];
>  
>  target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>                               uint32_t token, uint32_t nargs, target_ulong args,
>                               uint32_t nret, target_ulong rets)
>  {
> +    uint32_t tokensw = bswap32(token);
> +
>      if ((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX)) {
>          struct rtas_call *call = rtas_table + (token - RTAS_TOKEN_BASE);
>  
> @@ -308,6 +311,16 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>          }
>      }
>  
> +    /* Workaround for LE guests */
> +    if ((tokensw >= RTAS_TOKEN_BASE) && (tokensw < RTAS_TOKEN_MAX)) {
> +        struct rtas_call *call = rtas_table + (tokensw - RTAS_TOKEN_BASE);
> +
> +        if (call->fn_wa) {
> +            call->fn_wa(cpu, spapr, tokensw, nargs, args, nret, rets);
> +            return H_SUCCESS;
> +        }
> +    }
> +
>      /* HACK: Some Linux early debug code uses RTAS display-character,
>       * but assumes the token value is 0xa (which it is on some real
>       * machines) without looking it up in the device tree.  This
> @@ -340,6 +353,22 @@ void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
>      rtas_table[token].fn = fn;
>  }
>  
> +void spapr_rtas_register_wrong_endian(int token, spapr_rtas_fn fn)
> +{
> +    if (!((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX))) {
> +        fprintf(stderr, "RTAS invalid token 0x%x\n", token);
> +        exit(1);
> +    }
> +
> +    token -= RTAS_TOKEN_BASE;
> +    if (!rtas_table[token].fn) {
> +        fprintf(stderr, "RTAS token %x must be initialized to allow workaround\n",
> +                token);
> +        exit(1);
> +    }
> +    rtas_table[token].fn_wa = fn;
> +}
> +
>  int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>                                   hwaddr rtas_size)
>  {
> diff --git a/hw/ppc/spapr_rtas_ddw.c b/hw/ppc/spapr_rtas_ddw.c
> index af70601..56eae9f 100644
> --- a/hw/ppc/spapr_rtas_ddw.c
> +++ b/hw/ppc/spapr_rtas_ddw.c
> @@ -278,6 +278,41 @@ param_error_exit:
>      rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>  }
>  
> +#define SPAPR_RTAS_DDW_SWAP(n) rtas_st(rets, (n), bswap32(rtas_ld(rets, (n))))
> +
> +static void rtas_ibm_query_pe_dma_window_wrong_endian(PowerPCCPU *cpu,
> +                                                      sPAPREnvironment *spapr,
> +                                                      uint32_t token,
> +                                                      uint32_t nargs,
> +                                                      target_ulong args,
> +                                                      uint32_t nret,
> +                                                      target_ulong rets)
> +{
> +    rtas_ibm_query_pe_dma_window(cpu, spapr, token, nargs, args, nret, rets);
> +
> +    SPAPR_RTAS_DDW_SWAP(0);
> +    SPAPR_RTAS_DDW_SWAP(1);
> +    SPAPR_RTAS_DDW_SWAP(2);
> +    SPAPR_RTAS_DDW_SWAP(3);
> +    SPAPR_RTAS_DDW_SWAP(4);
> +}
> +
> +static void rtas_ibm_create_pe_dma_window_wrong_endian(PowerPCCPU *cpu,
> +                                                       sPAPREnvironment *spapr,
> +                                                       uint32_t token,
> +                                                       uint32_t nargs,
> +                                                       target_ulong args,
> +                                                       uint32_t nret,
> +                                                       target_ulong rets)
> +{
> +    rtas_ibm_create_pe_dma_window(cpu, spapr, token, nargs, args, nret, rets);
> +
> +    SPAPR_RTAS_DDW_SWAP(0);
> +    SPAPR_RTAS_DDW_SWAP(1);
> +    SPAPR_RTAS_DDW_SWAP(2);
> +    SPAPR_RTAS_DDW_SWAP(3);
> +}
> +
>  static void spapr_rtas_ddw_init(void)
>  {
>      spapr_rtas_register(RTAS_IBM_QUERY_PE_DMA_WINDOW,
> @@ -292,6 +327,11 @@ static void spapr_rtas_ddw_init(void)
>      spapr_rtas_register(RTAS_IBM_RESET_PE_DMA_WINDOW,
>                          "ibm,reset-pe-dma-window",
>                          rtas_ibm_reset_pe_dma_window);
> +
> +    spapr_rtas_register_wrong_endian(RTAS_IBM_QUERY_PE_DMA_WINDOW,
> +                                     rtas_ibm_query_pe_dma_window_wrong_endian);
> +    spapr_rtas_register_wrong_endian(RTAS_IBM_CREATE_PE_DMA_WINDOW,
> +                                     rtas_ibm_create_pe_dma_window_wrong_endian);
>  }
>  
>  type_init(spapr_rtas_ddw_init)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5f4e137..bf8e4a6 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -435,6 +435,8 @@ typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>                                uint32_t nargs, target_ulong args,
>                                uint32_t nret, target_ulong rets);
>  void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn);
> +void spapr_rtas_register_wrong_endian(int token, spapr_rtas_fn fn);
> +
>  target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>                               uint32_t token, uint32_t nargs, target_ulong args,
>                               uint32_t nret, target_ulong rets);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]