On Wed, May 17, 2017 at 05:56:57PM +0200, Greg Kurz wrote: > On Wed, 17 May 2017 16:35:46 +1000 > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > Guests of the qemu machine type go through a feature negotiation process > > known as "client architecture support" (CAS) during early boot. This does > > a number of things, one of which is finding a CPU compatibility mode which > > can be supported by both guest and host. > > > > In fact the CPU negotiation is probably the single most complex part of the > > CAS process, so this splits it out into a helper function. We've recently > > made some mistakes in maintaining backward compatibility for old machine > > types here. Splitting this out will also make it easier to fix this. > > > > This also adds a possibly useful error message if the negotiation fails > > (i.e. if there is CPU mode that's suitable for both guest and host). > > s/if there is/if there isn't a/ ?
Oops, I'll correct that. > > > > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > --- > > hw/ppc/spapr_hcall.c | 43 +++++++++++++++++++++++++++---------------- > > 1 file changed, 27 insertions(+), 16 deletions(-) > > > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > index 0d608d6..007ae8d 100644 > > --- a/hw/ppc/spapr_hcall.c > > +++ b/hw/ppc/spapr_hcall.c > > @@ -1047,19 +1047,13 @@ static target_ulong h_signal_sys_reset(PowerPCCPU > > *cpu, > > } > > } > > > > -static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > > - sPAPRMachineState *spapr, > > - target_ulong opcode, > > - target_ulong *args) > > +static uint32_t cas_check_pvr(PowerPCCPU *cpu, target_ulong list, > > + Error **errp) > > { > > - target_ulong list = ppc64_phys_to_real(args[0]); > > - target_ulong ov_table; > > bool explicit_match = false; /* Matched the CPU's real PVR */ > > uint32_t max_compat = cpu->max_compat; > > uint32_t best_compat = 0; > > int i; > > - sPAPROptionVector *ov1_guest, *ov5_guest, *ov5_cas_old, *ov5_updates; > > - bool guest_radix; > > > > /* > > * We scan the supplied table of PVRs looking for two things > > @@ -1090,26 +1084,43 @@ static target_ulong > > h_client_architecture_support(PowerPCCPU *cpu, > > /* We couldn't find a suitable compatibility mode, and either > > * the guest doesn't support "raw" mode for this CPU, or raw > > * mode is disabled because a maximum compat mode is set */ > > - return H_HARDWARE; > > + error_setg(errp, "Couldn't negotiate a suitable PVR during CAS"); > > + return 0; > > } > > > > /* Parsing finished */ > > trace_spapr_cas_pvr(cpu->compat_pvr, explicit_match, best_compat); > > > > - /* Update CPUs */ > > - if (cpu->compat_pvr != best_compat) { > > - Error *local_err = NULL; > > + return best_compat; > > +} > > + > > +static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > > + sPAPRMachineState *spapr, > > + target_ulong opcode, > > + target_ulong *args) > > +{ > > + /* @ov_table points to the first option vector */ > > + target_ulong ov_table = ppc64_phys_to_real(args[0]); > > + uint32_t cas_pvr; > > + sPAPROptionVector *ov1_guest, *ov5_guest, *ov5_cas_old, *ov5_updates; > > + bool guest_radix; > > + Error *local_err = NULL; > > > > - ppc_set_compat_all(best_compat, &local_err); > > + cas_pvr = cas_check_pvr(cpu, ov_table, &local_err); > > Shouldn't cas_check_pvr() take a target_ulong * so that we can get > the address of the first option vector... > > > + if (local_err) { > > + error_report_err(local_err); > > + return H_HARDWARE; > > + } > > + > > + /* Update CPUs */ > > + if (cpu->compat_pvr != cas_pvr) { > > + ppc_set_compat_all(cas_pvr, &local_err); > > if (local_err) { > > error_report_err(local_err); > > return H_HARDWARE; > > } > > } > > > > - /* For the future use: here @ov_table points to the first option > > vector */ > > - ov_table = list; > > - > > ... and address Laurent's comment ? Yes, I completely misread what was happening with the pvr_list vs. the option vectors. I'll correct that. > > > ov1_guest = spapr_ovec_parse_vector(ov_table, 1); > > ov5_guest = spapr_ovec_parse_vector(ov_table, 5); > > if (spapr_ovec_test(ov5_guest, OV5_MMU_BOTH)) { > -- 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