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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3517 bytes --]

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 <agraf@suse.de>
> CC: Jan Kiszka <jan.kiszka@siemens.com>
> 
> ---
> 
> 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).

> + */
> +#define kvm_halt_in_kernel() (kvm_halt_in_kernel_allowed)
> +
> +/**
>   * kvm_irqfds_enabled:
>   *
>   * Returns: true if we can use irqfds to inject interrupts into
> @@ -101,6 +110,7 @@ extern bool kvm_gsi_routing_allowed;
>  #define kvm_enabled()           (0)
>  #define kvm_irqchip_in_kernel() (false)
>  #define kvm_async_interrupts_enabled() (false)
> +#define kvm_halt_in_kernel() (false)
>  #define kvm_irqfds_enabled() (false)
>  #define kvm_msi_via_irqfd_enabled() (false)
>  #define kvm_gsi_routing_allowed() (false)
> diff --git a/kvm-all.c b/kvm-all.c
> index 130cdbc..2e301f5 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -106,6 +106,7 @@ struct KVMState
>  KVMState *kvm_state;
>  bool kvm_kernel_irqchip;
>  bool kvm_async_interrupts_allowed;
> +bool kvm_halt_in_kernel_allowed;
>  bool kvm_irqfds_allowed;
>  bool kvm_msi_via_irqfd_allowed;
>  bool kvm_gsi_routing_allowed;
> @@ -1219,6 +1220,7 @@ static int kvm_irqchip_create(KVMState *s)
>       * interrupt delivery (though the reverse is not necessarily true)
>       */
>      kvm_async_interrupts_allowed = true;
> +    kvm_halt_in_kernel_allowed = true;
>  
>      kvm_init_irq_routing(s);
>  
> 

The code looks good to me.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]