Re: [Qemu-devel] [PATCH] KVM: Don't assume that mpstate exists with in-kernel PIC always

[Alexander Graf]

On 25.04.2013, at 18:13, Jan Kiszka wrote:

> On 2013-04-25 11:06, Alexander Graf wrote:
>> On PPC, we don't support MP state. So far it's not necessary and I'm
>> not convinced yet that we really need to support it ever.
>> 
>> However, the current idle logic in QEMU assumes that an in-kernel PIC
>> also means we support MP state. This assumption is not true anymore.
>> 
>> Let's split up the two cases into two different variables. That way
>> PPC can expose an in-kernel PIC, while not implementing MP state.
>> 
>> Signed-off-by: Alexander Graf <address@hidden>
>> CC: Jan Kiszka <address@hidden>
>> 
>> ---
>> 
>> v1 -> v2:
>> 
>>  - use kvm_halt_in_kernel() instead
>> ---
>> cpus.c               |    2 +-
>> include/sysemu/kvm.h |   10 ++++++++++
>> kvm-all.c            |    2 ++
>> 3 files changed, 13 insertions(+), 1 deletions(-)
>> 
>> diff --git a/cpus.c b/cpus.c
>> index e919dd7..e626f7e 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -73,7 +73,7 @@ static bool cpu_thread_is_idle(CPUArchState *env)
>>         return true;
>>     }
>>     if (!cpu->halted || qemu_cpu_has_work(cpu) ||
>> -        kvm_async_interrupts_enabled()) {
>> +        kvm_halt_in_kernel()) {
>>         return false;
>>     }
>>     return true;
>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>> index 8a9ebda..07375bf 100644
>> --- a/include/sysemu/kvm.h
>> +++ b/include/sysemu/kvm.h
>> @@ -42,6 +42,7 @@
>> extern bool kvm_allowed;
>> extern bool kvm_kernel_irqchip;
>> extern bool kvm_async_interrupts_allowed;
>> +extern bool kvm_halt_in_kernel_allowed;
>> extern bool kvm_irqfds_allowed;
>> extern bool kvm_msi_via_irqfd_allowed;
>> extern bool kvm_gsi_routing_allowed;
>> @@ -72,6 +73,14 @@ extern bool kvm_gsi_routing_allowed;
>> #define kvm_async_interrupts_enabled() (kvm_async_interrupts_allowed)
>> 
>> /**
>> + * kvm_supports_mpstate:
>> + *
>> + * Returns: true if the kvm side supports MP states which is
>> + * required to indicate to kvm that a vcpu is currently halted
> 
> "...and make KVM wait for resumption inside the kernel instead for
> reporting halt to userspace." Or so.
> 
> That's why I asked for calling this helper differently: to express the
> concept behind it, not the implementation detail "mpstate" (which is an
> x86-caused misnomer).

It's not exactly a misnomer. It's a concept that is required to implement the 
logic. I've updated the comment accordingly :).


Alex