[9/9] KVM: PPC: Enable compatibility mode

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(+)

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?

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


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)
>   {
>   }

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)
>>   {
>>   }
>

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