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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

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.

> 
>> +            }
>>           }
>>             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?



> 
> 
> Alex
> 
>>               ppc_set_compat(cpu, cpu_version);
>>           }
>>       }
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index ff319fc..f8e8453 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1513,6 +1513,11 @@ void kvmppc_set_papr(PowerPCCPU *cpu)
>>       cap_papr = 1;
>>   }
>>   +int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>> +{
>> +    return kvm_set_one_reg(CPU(cpu), KVM_REG_PPC_ARCH_COMPAT,
>> &cpu_version);
>> +}
>> +
>>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>   {
>>       CPUState *cs = CPU(cpu);
>> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
>> index ff077ec..716c33d 100644
>> --- a/target-ppc/kvm_ppc.h
>> +++ b/target-ppc/kvm_ppc.h
>> @@ -23,6 +23,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
>>   int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
>>   int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
>>   void kvmppc_set_papr(PowerPCCPU *cpu);
>> +int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>>   int kvmppc_smt_threads(void);
>>   int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
>> @@ -95,6 +96,11 @@ static inline void kvmppc_set_papr(PowerPCCPU *cpu)
>>   {
>>   }
>>   +static inline int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t
>> cpu_version)
>> +{
>> +    return 0;
>> +}
>> +
>>   static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>   {
>>   }
> 


-- 
Alexey