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

[Alexander Graf <agraf@suse.de>]

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 <aik@ozlabs.ru>
>>> ---
>>>    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