On Wed, May 17, 2017 at 05:56:57PM +0200, Greg Kurz wrote:
> On Wed, 17 May 2017 16:35:46 +1000
> David Gibson 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
> > ---
> > 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
