Re: [Qemu-ppc] [QEMU-ppc for-2.10][PATCH V5 1/6] target/ppc: Set UPRT and GTSE on all cpus in H_REGISTER_PROCESS_TABLE

[David Gibson]

On Fri, Apr 28, 2017 at 04:58:21PM +1000, Suraj Jitindar Singh wrote:
> The UPRT and GTSE bits are set when a guest calls H_REGISTER_PROCESS_TABLE
> to choose determine how address translation is performed. Currently these
> bits in the LPCR are only set for the cpu which handles the H_CALL, however
> they need to be set for all cpus for that guest as address translation
> cannot be performed differently on a per cpu basis.
> 
> Update the H_CALL handler to set these bits in the LPCR correctly for all
> cpus of the guest.
> 
> Note it is the reponsibility of the guest to ensure that any secondary cpus
> are suspended when the H_CALL is made and thus we can safely update these
> values here.
> 
> Signed-off-by: Suraj Jitindar Singh <address@hidden>
> 
> ---
> 
> V4 -> V5:
> - Rework spr setting code for extensibility
> 
> -> V4:
> - Added patch to series
> ---
>  hw/ppc/spapr_hcall.c | 51 +++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 44f4489..2e571cc 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -924,6 +924,38 @@ static void spapr_check_setup_free_hpt(sPAPRMachineState 
> *spapr,
>      return;
>  }
>  
> +typedef struct {
> +    int spr_n;
> +    target_ulong mask;
> +    target_ulong val;
> +} SetSPRState;
> +
> +static void do_set_spr(CPUState *cs, run_on_cpu_data arg)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +    SetSPRState *s = arg.host_ptr;
> +
> +    env->spr[s->spr_n] &= ~s->mask;
> +    env->spr[s->spr_n] |= (s->val & s->mask);

I guess since it's running on the cpu thread this will be atomic
enough.  Personally I'd use a local for the intermediate value,
though.

> +}
> +
> +static void ppc_set_spr_cpu_foreach(int spr_n, target_ulong mask,
> +                                    target_ulong val)
> +{
> +    CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        SetSPRState s = {
> +            .spr_n = spr_n,
> +            .mask = mask,
> +            .val = val
> +        };
> +
> +        run_on_cpu(cs, do_set_spr, RUN_ON_CPU_HOST_PTR(&s));
> +    }
> +}
> +
>  #define FLAGS_MASK              0x01FULL
>  #define FLAG_MODIFY             0x10
>  #define FLAG_REGISTER           0x08
> @@ -936,7 +968,6 @@ static target_ulong h_register_process_table(PowerPCCPU 
> *cpu,
>                                               target_ulong opcode,
>                                               target_ulong *args)
>  {
> -    CPUPPCState *env = &cpu->env;
>      target_ulong flags = args[0];
>      target_ulong proc_tbl = args[1];
>      target_ulong page_size = args[2];
> @@ -992,17 +1023,13 @@ static target_ulong 
> h_register_process_table(PowerPCCPU *cpu,
>      spapr_check_setup_free_hpt(spapr, spapr->patb_entry, cproc);
>  
>      spapr->patb_entry = cproc; /* Save new process table */
> -    if ((flags & FLAG_RADIX) || (flags & FLAG_HASH_PROC_TBL)) {
> -        /* Use Process TBL */
> -        env->spr[SPR_LPCR] |= LPCR_UPRT;
> -    } else {
> -        env->spr[SPR_LPCR] &= ~LPCR_UPRT;
> -    }
> -    if (flags & FLAG_GTSE) { /* Partition Uses Guest Translation Shootdwn */
> -        env->spr[SPR_LPCR] |= LPCR_GTSE;
> -    } else {
> -        env->spr[SPR_LPCR] &= ~LPCR_GTSE;
> -    }
> +
> +    /* Update the UPRT and GTSE bits in the LPCR for all cpus */
> +    ppc_set_spr_cpu_foreach(SPR_LPCR, LPCR_UPRT,
> +                            (flags & (FLAG_RADIX | FLAG_HASH_PROC_TBL)) ?
> +                            LPRC_UPRT : 0);
> +    ppc_set_spr_cpu_foreach(SPR_LPCR, LPCR_GTSE, (flags & FLAG_GTSE) ? 
> LPCR_GTSE
> +                                                                     : 0);

You should be able to set both flags with a single foreach, no?

>  
>      if (kvm_enabled()) {
>          return kvmppc_configure_v3_mmu(cpu, flags & FLAG_RADIX,

-- 
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

signature.asc
Description: PGP signature