[PATCH 0/2] KVM: x86: fix deliver of SMIs to halted VCPUs

[PATCH 0/2] KVM: x86: fix deliver of SMIs to halted VCPUs

[Paolo Bonzini]

While SeaBIOS only uses synchronous SMI delivery through port 0xb2,
OVMF also uses APIC delivery, and it is buggy.

This is enough to run OVMF with SMM enabled and multiple processors,
up to a UEFI shell, but it literally takes minutes to get there.
However, the same behavior is visible with KVM disabled, so it may
be a firmware bug instead.

Paolo

Paolo Bonzini (2):
  KVM: x86: clean up kvm_arch_vcpu_runnable
  KVM: x86: fix SMI to halted VCPU

 arch/x86/kvm/x86.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] KVM: x86: clean up kvm_arch_vcpu_runnable

[Paolo Bonzini]

Split the huge conditional in two functions.

Fixes: 64d6067057d9658acb8675afcfba549abdb7fc16
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e1bc11d8e4bd..ad82420761dd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6453,6 +6453,12 @@ static inline int vcpu_block(struct kvm *kvm, struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+static inline bool kvm_vcpu_running(struct kvm_vcpu *vcpu)
+{
+	return (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&
+		!vcpu->arch.apf.halted);
+}
+
 static int vcpu_run(struct kvm_vcpu *vcpu)
 {
 	int r;
@@ -6461,8 +6467,7 @@ static int vcpu_run(struct kvm_vcpu *vcpu)
 	vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);
 
 	for (;;) {
-		if (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&
-		    !vcpu->arch.apf.halted)
+		if (kvm_vcpu_running(vcpu))
 			r = vcpu_enter_guest(vcpu);
 		else
 			r = vcpu_block(kvm, vcpu);
@@ -7762,19 +7767,33 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
 	kvm_mmu_invalidate_zap_all_pages(kvm);
 }
 
+static inline bool kvm_vcpu_has_events(struct kvm_vcpu *vcpu)
+{
+	if (!list_empty_careful(&vcpu->async_pf.done))
+		return true;
+
+	if (kvm_apic_has_events(vcpu))
+		return true;
+
+	if (vcpu->arch.pv.pv_unhalted)
+		return true;
+
+	if (atomic_read(&vcpu->arch.nmi_queued))
+		return true;
+
+	if (kvm_arch_interrupt_allowed(vcpu) &&
+	    kvm_cpu_has_interrupt(vcpu))
+		return true;
+
+	return false;
+}
+
 int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
 {
 	if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events)
 		kvm_x86_ops->check_nested_events(vcpu, false);
 
-	return (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&
-		!vcpu->arch.apf.halted)
-		|| !list_empty_careful(&vcpu->async_pf.done)
-		|| kvm_apic_has_events(vcpu)
-		|| vcpu->arch.pv.pv_unhalted
-		|| atomic_read(&vcpu->arch.nmi_queued) ||
-		(kvm_arch_interrupt_allowed(vcpu) &&
-		 kvm_cpu_has_interrupt(vcpu));
+	return kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu);
 }
 
 int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
-- 
1.8.3.1

Re: [PATCH 1/2] KVM: x86: clean up kvm_arch_vcpu_runnable

[Radim Krčmář]

2015-10-13 12:34+0200, Paolo Bonzini:
> Split the huge conditional in two functions.
> 
> Fixes: 64d6067057d9658acb8675afcfba549abdb7fc16
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Thanks!

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

[PATCH 2/2] KVM: x86: fix SMI to halted VCPU

[Paolo Bonzini]

An SMI to a halted VCPU must wake it up, hence a VCPU with a pending
SMI must be considered runnable.

Fixes: 64d6067057d9658acb8675afcfba549abdb7fc16
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ad82420761dd..7b481470602a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7781,6 +7781,9 @@ static inline bool kvm_vcpu_has_events(struct kvm_vcpu *vcpu)
 	if (atomic_read(&vcpu->arch.nmi_queued))
 		return true;
 
+	if (test_bit(KVM_REQ_SMI, &vcpu->requests))
+		return true;
+
 	if (kvm_arch_interrupt_allowed(vcpu) &&
 	    kvm_cpu_has_interrupt(vcpu))
 		return true;
-- 
1.8.3.1

Re: [PATCH 2/2] KVM: x86: fix SMI to halted VCPU

[Radim Krčmář]

2015-10-13 12:34+0200, Paolo Bonzini:
> An SMI to a halted VCPU must wake it up, hence a VCPU with a pending
> SMI must be considered runnable.
> 
> Fixes: 64d6067057d9658acb8675afcfba549abdb7fc16
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> @@ -7781,6 +7781,9 @@ static inline bool kvm_vcpu_has_events(struct kvm_vcpu *vcpu)
> +	if (test_bit(KVM_REQ_SMI, &vcpu->requests))

(Ah, and refactoring of this API is also on my new TODO.)

Re: [PATCH 0/2] KVM: x86: fix deliver of SMIs to halted VCPUs

[Paolo Bonzini]

On 13/10/2015 12:34, Paolo Bonzini wrote:
> While SeaBIOS only uses synchronous SMI delivery through port 0xb2,
> OVMF also uses APIC delivery, and it is buggy.
> 
> This is enough to run OVMF with SMM enabled and multiple processors,
> up to a UEFI shell, but it literally takes minutes to get there.
> However, the same behavior is visible with KVM disabled, so it may
> be a firmware bug instead.

Which it was. :)  http://permalink.gmane.org/gmane.comp.bios.edk2.devel/2899

Paolo