On 30/10/16 22:12, David Gibson wrote:
> Once a compatiblity mode is negotiated with the guest,
> h_client_architecture_support() uses run_on_cpu() to update each CPU to
> the new mode. We're going to want this logic somewhere else shortly,
> so make a helper function to do this global update.
>
> We put it in target-ppc/compat.c - it makes as much sense at the CPU level
> as it does at the machine level. We also move the cpu_synchronize_state()
> into ppc_set_compat(), since it doesn't really make any sense to call that
> without synchronizing state.
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr_hcall.c | 31 +++++--------------------------
> target-ppc/compat.c | 36 ++++++++++++++++++++++++++++++++++++
> target-ppc/cpu.h | 3 +++
> 3 files changed, 44 insertions(+), 26 deletions(-)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 3bd6d06..4eaf9a6 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -881,20 +881,6 @@ static target_ulong h_set_mode(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> return ret;
> }
>
> -typedef struct {
> - uint32_t compat_pvr;
> - Error *err;
> -} SetCompatState;
> -
> -static void do_set_compat(CPUState *cs, void *arg)
> -{
> - PowerPCCPU *cpu = POWERPC_CPU(cs);
> - SetCompatState *s = arg;
> -
> - cpu_synchronize_state(cs);
> - ppc_set_compat(cpu, s->compat_pvr, &s->err);
> -}
> -
> static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> target_ulong opcode,
> @@ -902,7 +888,6 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
> {
> target_ulong list = ppc64_phys_to_real(args[0]);
> target_ulong ov_table;
> - CPUState *cs;
> bool explicit_match = false; /* Matched the CPU's real PVR */
> uint32_t max_compat = cpu->max_compat;
> uint32_t best_compat = 0;
> @@ -949,18 +934,12 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
>
> /* Update CPUs */
> if (cpu->compat_pvr != best_compat) {
> - CPU_FOREACH(cs) {
> - SetCompatState s = {
> - .compat_pvr = best_compat,
> - .err = NULL,
> - };
> + Error *local_err = NULL;
>
> - run_on_cpu(cs, do_set_compat, &s);
> -
> - if (s.err) {
> - error_report_err(s.err);
> - return H_HARDWARE;
> - }
> + ppc_set_compat_all(best_compat, &local_err);
> + if (local_err) {
> + error_report_err(local_err);
> + return H_HARDWARE;
> }
> }
>
> diff --git a/target-ppc/compat.c b/target-ppc/compat.c
> index 1059555..0b12b58 100644
> --- a/target-ppc/compat.c
> +++ b/target-ppc/compat.c
> @@ -124,6 +124,8 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr,
> Error **errp)
> pcr = compat->pcr;
> }
>
> + cpu_synchronize_state(CPU(cpu));
> +
> cpu->compat_pvr = compat_pvr;
> env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
>
> @@ -136,6 +138,40 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t
> compat_pvr, Error **errp)
> }
> }
>
> +#if !defined(CONFIG_USER_ONLY)
> +typedef struct {
> + uint32_t compat_pvr;
> + Error *err;
> +} SetCompatState;
> +
> +static void do_set_compat(CPUState *cs, void *arg)
> +{
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> + SetCompatState *s = arg;
> +
> + ppc_set_compat(cpu, s->compat_pvr, &s->err);
> +}
> +
> +void ppc_set_compat_all(uint32_t compat_pvr, Error **errp)
> +{
> + CPUState *cs;
> +
> + CPU_FOREACH(cs) {
> + SetCompatState s = {
> + .compat_pvr = compat_pvr,
> + .err = NULL,
> + };
> +
> + run_on_cpu(cs, do_set_compat, &s);
> +
> + if (s.err) {
> + error_propagate(errp, s.err);
> + return;
> + }
> + }
> +}
> +#endif
> +
> int ppc_compat_max_threads(PowerPCCPU *cpu)
> {
> const CompatInfo *compat = compat_by_pvr(cpu->compat_pvr);
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 91e8be8..201a655 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1317,6 +1317,9 @@ static inline int cpu_mmu_index (CPUPPCState *env, bool
> ifetch)
> bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr,
> uint32_t min_compat_pvr, uint32_t max_compat_pvr);
> void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
> +#if !defined(CONFIG_USER_ONLY)
> +void ppc_set_compat_all(uint32_t compat_pvr, Error **errp);
> +#endif
I would put all ppc*compat*() under #if defined(CONFIG_USER_ONLY) &&
defined(TARGET_PPC64) (or even moved this to target-ppc/Makefile.objs).
Otherwise, functions like ppc_check_compat() have #if
!defined(CONFIG_USER_ONLY) which suggests that the rest of
ppc_check_compat() can actually be executed in ppc64-linux-user (while it
cannot, can it?).
> int ppc_compat_max_threads(PowerPCCPU *cpu);
> #endif /* defined(TARGET_PPC64) */
>
>
--
Alexey
