Re: [Qemu-devel] [PATCH 9/9] KVM: PPC: Enable compatibility mode

[Alexander Graf]

On 16.05.14 16:27, Alexey Kardashevskiy wrote:
> On 05/17/2014 12:18 AM, Alexander Graf wrote:
> > On 15.05.14 13:28, Alexey Kardashevskiy wrote:
> > > The host kernel implements a KVM_REG_PPC_ARCH_COMPAT register which
> > > this uses to enable a compatibility mode if any chosen.
> > >
> > > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > > ---
> > >    hw/ppc/spapr.c       | 6 ++++++
> > >    hw/ppc/spapr_hcall.c | 4 ++++
> > >    target-ppc/kvm.c     | 5 +++++
> > >    target-ppc/kvm_ppc.h | 6 ++++++
> > >    4 files changed, 21 insertions(+)
> > >
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index a0882a1..f89be10 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1376,6 +1376,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> > >            /* Tell KVM that we're in PAPR mode */
> > >            if (kvm_enabled()) {
> > >                kvmppc_set_papr(cpu);
> > > +
> > > +            if (cpu->max_compat &&
> > > +                kvmppc_set_compat(cpu, cpu->max_compat) < 0) {
> > > +                fprintf(stderr, "Unable to set compatibility mode\n");
> > > +                exit(1);
> > Why is KVM special here?
> Sorry, I am not following you here. There is no SPR for that which guest
> kernel could change, this is a register from "kvm set/get one reg" interface.

So semantically, what is ppc_set_compat() then? Why would we have to set 
the KVM SPR that represents which compat mode we're in here, but not the 
TCG version of it?

> > > +            }
> > >            }
> > >              if (cpu->max_compat) {
> > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > > index cb815c3..2ab21d3 100644
> > > --- a/hw/ppc/spapr_hcall.c
> > > +++ b/hw/ppc/spapr_hcall.c
> > > @@ -834,6 +834,10 @@ static target_ulong
> > > h_client_architecture_support(PowerPCCPU *cpu_,
> > >            CPU_FOREACH(cs) {
> > >                PowerPCCPU *cpu = POWERPC_CPU(cs);
> > >    +            if (kvmppc_set_compat(cpu, cpu_version) < 0) {
> > > +                fprintf(stderr, "Unable to set compatibility mode\n");
> > > +                return H_HARDWARE;
> > > +            }
> > Just fold this into ppc_set_compat which will run from vcpu context.
>
> TCG version cannot fail and KVM's one can. Adding non-void return value
> would give impression that ppc_set_compat may fail while it cannot. Still fold?

Yes, please.


Alex