Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] spapr-hcall: fix little-endian resource handling in H_SET_MODE

[Greg Kurz <gkurz@linux.vnet.ibm.com>]

On Fri,  7 Mar 2014 15:37:40 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> This changes resource code definitions to ones used in the host kernel.
> 
> This fixes H_SET_MODE_RESOURCE_LE (switch between big endian and
> little endian) to sync registers from KVM before changing LPCR value.
> 
> This adds a set_spr() helper to update an SPR in a CPU's context to avoid
> possible races and makes use of it to change LPCR.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---

Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

>  hw/ppc/spapr_hcall.c   | 38 ++++++++++++++++++++++++++++++--------
>  include/hw/ppc/spapr.h |  9 +++++++--
>  2 files changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index d918780..1bf19b2 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -4,6 +4,33 @@
>  #include "hw/ppc/spapr.h"
>  #include "mmu-hash64.h"
> 
> +struct spr_sync_struct {
> +    CPUState *cs;
> +    int spr;
> +    target_ulong value;
> +    target_ulong mask;
> +};
> +
> +static void do_spr_sync(void *arg)
> +{
> +    struct spr_sync_struct *s = arg;
> +    PowerPCCPU *cp = POWERPC_CPU(s->cs);
> +    CPUPPCState *env = &cp->env;
> +
> +    cpu_synchronize_state(s->cs);
> +    env->spr[s->spr] &= ~s->mask;
> +    env->spr[s->spr] |= s->value;
> +}
> +
> +static void set_spr(CPUState *cs, int spr, target_ulong value,
> +                    target_ulong mask)
> +{
> +    struct spr_sync_struct s = {
> +        .cs = cs, .spr = spr, .value = value, .mask = mask
> +    };
> +    run_on_cpu(cs, do_spr_sync, &s);
> +}
> +
>  static target_ulong compute_tlbie_rb(target_ulong v, target_ulong r,
>                                       target_ulong pte_index)
>  {
> @@ -690,7 +717,7 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>      target_ulong value2 = args[3];
>      target_ulong ret = H_P2;
> 
> -    if (resource == H_SET_MODE_ENDIAN) {
> +    if (resource == H_SET_MODE_RESOURCE_LE) {
>          if (value1) {
>              ret = H_P3;
>              goto out;
> @@ -699,22 +726,17 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>              ret = H_P4;
>              goto out;
>          }
> -
>          switch (mflags) {
>          case H_SET_MODE_ENDIAN_BIG:
>              CPU_FOREACH(cs) {
> -                PowerPCCPU *cp = POWERPC_CPU(cs);
> -                CPUPPCState *env = &cp->env;
> -                env->spr[SPR_LPCR] &= ~LPCR_ILE;
> +                set_spr(cs, SPR_LPCR, 0, LPCR_ILE);
>              }
>              ret = H_SUCCESS;
>              break;
> 
>          case H_SET_MODE_ENDIAN_LITTLE:
>              CPU_FOREACH(cs) {
> -                PowerPCCPU *cp = POWERPC_CPU(cs);
> -                CPUPPCState *env = &cp->env;
> -                env->spr[SPR_LPCR] |= LPCR_ILE;
> +                set_spr(cs, SPR_LPCR, LPCR_ILE, LPCR_ILE);
>              }
>              ret = H_SUCCESS;
>              break;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 449fc7c..5fdac1e 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -153,8 +153,13 @@ typedef struct sPAPREnvironment {
>  #define H_PP1             (1ULL<<(63-62))
>  #define H_PP2             (1ULL<<(63-63))
> 
> -/* H_SET_MODE flags */
> -#define H_SET_MODE_ENDIAN        4
> +/* Values for 2nd argument to H_SET_MODE */
> +#define H_SET_MODE_RESOURCE_SET_CIABR           1
> +#define H_SET_MODE_RESOURCE_SET_DAWR            2
> +#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE     3
> +#define H_SET_MODE_RESOURCE_LE                  4
> +
> +/* Flags for H_SET_MODE_RESOURCE_LE */
>  #define H_SET_MODE_ENDIAN_BIG    0
>  #define H_SET_MODE_ENDIAN_LITTLE 1
> 



-- 
Gregory Kurz                                     kurzgreg@fr.ibm.com
                                                 gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.