Re: [Qemu-devel] [PATCH 3/4] ppc: kvm: introduce a helper to update SDR1 for a single CPU

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

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

On Mon, Sep 04, 2017 at 11:47:14PM +0200, Greg Kurz wrote:
> When running with KVM PR, we hijack the SDR1 slot to pass the address of
> the HPT allocated by QEMU to KVM.  On pseries virtual machines, we have to
> do this when the guest calls the KVMPPC_H_CAS or the H_RESIZE_HPT_COMMIT
> hypercalls. This is currently handled by kvmppc_update_sdr1() which updates
> SDR1 for all CPUs. But we also need to update SDR1 at machine reset, and
> this is currently open-coded in spapr_cpu_reset() on a per-CPU basis.
> 
> This patch renames kvmppc_update_sdr1() to kvmppc_update_sdr1_all() and
> reuses the kvmppc_update_sdr1() function name to update a single CPU,
> like we already do with in the CPU compat mode code.
> 
> It finally converts the sPAPR code to use the all CPUs or single CPU helpers
> where appropriate.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Seems reasonable, but I'll wait for the next spin of the series, since
the previous patch wants some work.

> ---
>  hw/ppc/spapr_cpu_core.c |    6 +-----
>  hw/ppc/spapr_hcall.c    |    4 ++--
>  target/ppc/kvm.c        |   12 ++++++++----
>  target/ppc/kvm_ppc.h    |    3 ++-
>  4 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 581eb4d92de9..da81688b0f4d 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -95,11 +95,7 @@ static void spapr_cpu_reset(void *opaque)
>      if (kvm_enabled()) {
>          target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>          if (sdr1) {
> -            env->spr[SPR_SDR1] = sdr1;
> -            if (kvmppc_put_books_sregs(cpu) < 0) {
> -                error_report("Unable to update SDR1 in KVM");
> -                exit(1);
> -            }
> +            kvmppc_update_sdr1(cpu, sdr1);
>          }
>      }
>  }
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 06059b44ab40..e090b69efe7f 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -737,7 +737,7 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
>              /* For KVM PR, update the HPT pointer */
>              target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>              if (sdr1) {
> -                kvmppc_update_sdr1(sdr1);
> +                kvmppc_update_sdr1_all(sdr1);
>              }
>          }
>  
> @@ -1569,7 +1569,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>                  /* For KVM PR, update the HPT pointer */
>                  target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>                  if (sdr1) {
> -                    kvmppc_update_sdr1(sdr1);
> +                    kvmppc_update_sdr1_all(sdr1);
>                  }
>              }
>          }
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 6442dfcb95b3..e69366968f15 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2806,10 +2806,9 @@ int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift)
>      return kvm_vm_ioctl(cs->kvm_state, KVM_PPC_RESIZE_HPT_COMMIT, &rhpt);
>  }
>  
> -static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
> +void kvmppc_update_sdr1(PowerPCCPU *cpu, target_ulong sdr1)
>  {
> -    target_ulong sdr1 = arg.target_ptr;
> -    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
>  
>      /* This is just for the benefit of PR KVM */
> @@ -2821,7 +2820,12 @@ static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
>      }
>  }
>  
> -void kvmppc_update_sdr1(target_ulong sdr1)
> +static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
> +{
> +    kvmppc_update_sdr1(POWERPC_CPU(cs), arg.target_ptr);
> +}
> +
> +void kvmppc_update_sdr1_all(target_ulong sdr1)
>  {
>      CPUState *cs;
>  
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index f780e6ec7b72..9524a7a0c21c 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -68,7 +68,8 @@ PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
>  void kvmppc_check_papr_resize_hpt(Error **errp);
>  int kvmppc_resize_hpt_prepare(PowerPCCPU *cpu, target_ulong flags, int shift);
>  int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift);
> -void kvmppc_update_sdr1(target_ulong sdr1);
> +void kvmppc_update_sdr1(PowerPCCPU *cpu, target_ulong sdr1);
> +void kvmppc_update_sdr1_all(target_ulong sdr1);
>  bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
>  
>  bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path);
> 

-- 
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]