[PATCH 0/2] KVM: x86: Fix userspace API regarding latched init

[PATCH 0/2] KVM: x86: Fix userspace API regarding latched init

[Liran Alon]

Hi,
This patch series aims to fix 2 issue in KVM userspace API regarding latched init.

1st patch makes sure that userspace can get/set latched_init state
regardless of if vCPU is in SMM state.

2nd patch prevents userspace from setting vCPU in INIT_RECEIVED/SIPI_RECEIVED
state in case vCPU is in a state that latch INIT signals.

For further information, refer to patches commit messages.

Regards,
-Liran

[PATCH 1/2] KVM: x86: Evaluate latched_init in KVM_SET_VCPU_EVENTS when vCPU not in SMM

[Liran Alon]

Commit 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
fixed KVM to also latch pending LAPIC INIT event when vCPU is in VMX
operation.

However, current API of KVM_SET_VCPU_EVENTS defines this field as
part of SMM state and only set pending LAPIC INIT event if vCPU is
specified to be in SMM mode (events->smi.smm is set).

Change KVM_SET_VCPU_EVENTS handler to set pending LAPIC INIT event
by latched_init field regardless of if vCPU is in SMM mode or not.

Fixes: 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 arch/x86/kvm/x86.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ff395f812719..f41d5d05e9f2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3792,12 +3792,13 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 				vcpu->arch.hflags |= HF_SMM_INSIDE_NMI_MASK;
 			else
 				vcpu->arch.hflags &= ~HF_SMM_INSIDE_NMI_MASK;
-			if (lapic_in_kernel(vcpu)) {
-				if (events->smi.latched_init)
-					set_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
-				else
-					clear_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
-			}
+		}
+
+		if (lapic_in_kernel(vcpu)) {
+			if (events->smi.latched_init)
+				set_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
+			else
+				clear_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
 		}
 	}
 
-- 
2.20.1

[PATCH 2/2] KVM: x86: Prevent set vCPU into INIT/SIPI_RECEIVED state when INIT are latched

[Liran Alon]

Commit 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
fixed KVM to also latch pending LAPIC INIT event when vCPU is in VMX
operation.

However, current API of KVM_SET_MP_STATE allows userspace to put vCPU
into KVM_MP_STATE_SIPI_RECEIVED or KVM_MP_STATE_INIT_RECEIVED even when
vCPU is in VMX operation.

Fix this by introducing a util method to check if vCPU state latch INIT
signals and use it in KVM_SET_MP_STATE handler.

Fixes: 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
Reported-by: Sean Christopherson <sean.j.christopherson@intel.com>
Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 arch/x86/kvm/lapic.c | 5 ++---
 arch/x86/kvm/x86.c   | 4 ++--
 arch/x86/kvm/x86.h   | 5 +++++
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index b29d00b661ff..0df265248cae 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2702,14 +2702,13 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
 		return;
 
 	/*
-	 * INITs are latched while CPU is in specific states
-	 * (SMM, VMX non-root mode, SVM with GIF=0).
+	 * INITs are latched while CPU is in specific states.
 	 * Because a CPU cannot be in these states immediately
 	 * after it has processed an INIT signal (and thus in
 	 * KVM_MP_STATE_INIT_RECEIVED state), just eat SIPIs
 	 * and leave the INIT pending.
 	 */
-	if (is_smm(vcpu) || kvm_x86_ops->apic_init_signal_blocked(vcpu)) {
+	if (kvm_vcpu_latch_init(vcpu)) {
 		WARN_ON_ONCE(vcpu->arch.mp_state == KVM_MP_STATE_INIT_RECEIVED);
 		if (test_bit(KVM_APIC_SIPI, &apic->pending_events))
 			clear_bit(KVM_APIC_SIPI, &apic->pending_events);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f41d5d05e9f2..eb992f5d299f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8675,8 +8675,8 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 	    mp_state->mp_state != KVM_MP_STATE_RUNNABLE)
 		goto out;
 
-	/* INITs are latched while in SMM */
-	if ((is_smm(vcpu) || vcpu->arch.smi_pending) &&
+	/* INITs are latched while CPU is in specific states */
+	if ((kvm_vcpu_latch_init(vcpu) || vcpu->arch.smi_pending) &&
 	    (mp_state->mp_state == KVM_MP_STATE_SIPI_RECEIVED ||
 	     mp_state->mp_state == KVM_MP_STATE_INIT_RECEIVED))
 		goto out;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index dbf7442a822b..d40da892f889 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -260,6 +260,11 @@ static inline bool kvm_check_has_quirk(struct kvm *kvm, u64 quirk)
 	return !(kvm->arch.disabled_quirks & quirk);
 }
 
+static inline bool kvm_vcpu_latch_init(struct kvm_vcpu *vcpu)
+{
+	return is_smm(vcpu) || kvm_x86_ops->apic_init_signal_blocked(vcpu);
+}
+
 void kvm_set_pending_timer(struct kvm_vcpu *vcpu);
 void kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip);
 
-- 
2.20.1

Re: [PATCH 2/2] KVM: x86: Prevent set vCPU into INIT/SIPI_RECEIVED state when INIT are latched

[Paolo Bonzini]

On 11/11/19 10:16, Liran Alon wrote:
> -	/* INITs are latched while in SMM */
> -	if ((is_smm(vcpu) || vcpu->arch.smi_pending) &&
> +	/* INITs are latched while CPU is in specific states */
> +	if ((kvm_vcpu_latch_init(vcpu) || vcpu->arch.smi_pending) &&
>  	    (mp_state->mp_state == KVM_MP_STATE_SIPI_RECEIVED ||
>  	     mp_state->mp_state == KVM_MP_STATE_INIT_RECEIVED))
>  		goto out;

Just a small doc clarification:

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 318046647fda..cacfe14717d6 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2707,7 +2707,8 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
 		return;
 
 	/*
-	 * INITs are latched while CPU is in specific states.
+	 * INITs are latched while CPU is in specific states
+	 * (SMM, VMX non-root mode, SVM with GIF=0).
 	 * Because a CPU cannot be in these states immediately
 	 * after it has processed an INIT signal (and thus in
 	 * KVM_MP_STATE_INIT_RECEIVED state), just eat SIPIs
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 681544f8db31..11746534e209 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8706,7 +8706,11 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 	    mp_state->mp_state != KVM_MP_STATE_RUNNABLE)
 		goto out;
 
-	/* INITs are latched while CPU is in specific states */
+	/*
+	 * KVM_MP_STATE_INIT_RECEIVED means the processor is in
+	 * INIT state; latched init should be reported using
+	 * KVM_SET_VCPU_EVENTS, so reject it here.
+	 */
 	if ((kvm_vcpu_latch_init(vcpu) || vcpu->arch.smi_pending) &&
 	    (mp_state->mp_state == KVM_MP_STATE_SIPI_RECEIVED ||
 	     mp_state->mp_state == KVM_MP_STATE_INIT_RECEIVED))


I'm not sure why you're removing the first hunk, it's just meant to
explain why it needs to be a kvm_x86_ops in case the reader is not
thinking about nested virtualization.

Paolo

Re: [PATCH 0/2] KVM: x86: Fix userspace API regarding latched init

[Paolo Bonzini]

On 11/11/19 10:16, Liran Alon wrote:
> Hi,
> This patch series aims to fix 2 issue in KVM userspace API regarding latched init.
> 
> 1st patch makes sure that userspace can get/set latched_init state
> regardless of if vCPU is in SMM state.
> 
> 2nd patch prevents userspace from setting vCPU in INIT_RECEIVED/SIPI_RECEIVED
> state in case vCPU is in a state that latch INIT signals.
> 
> For further information, refer to patches commit messages.
> 
> Regards,
> -Liran
> 

Queued, thanks.

Paolo

Re: [PATCH 2/2] KVM: x86: Prevent set vCPU into INIT/SIPI_RECEIVED state when INIT are latched

[Liran Alon]

> On 11 Nov 2019, at 15:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 11/11/19 10:16, Liran Alon wrote:
>> -	/* INITs are latched while in SMM */
>> -	if ((is_smm(vcpu) || vcpu->arch.smi_pending) &&
>> +	/* INITs are latched while CPU is in specific states */
>> +	if ((kvm_vcpu_latch_init(vcpu) || vcpu->arch.smi_pending) &&
>> 	    (mp_state->mp_state == KVM_MP_STATE_SIPI_RECEIVED ||
>> 	     mp_state->mp_state == KVM_MP_STATE_INIT_RECEIVED))
>> 		goto out;
> 
> Just a small doc clarification:
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 318046647fda..cacfe14717d6 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2707,7 +2707,8 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
> 		return;
> 
> 	/*
> -	 * INITs are latched while CPU is in specific states.
> +	 * INITs are latched while CPU is in specific states
> +	 * (SMM, VMX non-root mode, SVM with GIF=0).

I didn’t want this line of comment as it may diverge from the implementation of kvm_vcpu_latch_init().
That’s why I removed it.

> 	 * Because a CPU cannot be in these states immediately
> 	 * after it has processed an INIT signal (and thus in
> 	 * KVM_MP_STATE_INIT_RECEIVED state), just eat SIPIs
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 681544f8db31..11746534e209 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8706,7 +8706,11 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
> 	    mp_state->mp_state != KVM_MP_STATE_RUNNABLE)
> 		goto out;
> 
> -	/* INITs are latched while CPU is in specific states */
> +	/*
> +	 * KVM_MP_STATE_INIT_RECEIVED means the processor is in
> +	 * INIT state; latched init should be reported using
> +	 * KVM_SET_VCPU_EVENTS, so reject it here.
> +	 */

Yes this is a good comment. Thanks for adding it.

> 	if ((kvm_vcpu_latch_init(vcpu) || vcpu->arch.smi_pending) &&
> 	    (mp_state->mp_state == KVM_MP_STATE_SIPI_RECEIVED ||
> 	     mp_state->mp_state == KVM_MP_STATE_INIT_RECEIVED))
> 
> 
> I'm not sure why you're removing the first hunk, it's just meant to
> explain why it needs to be a kvm_x86_ops in case the reader is not
> thinking about nested virtualization.
> 
> Paolo
> 

Re: [PATCH 2/2] KVM: x86: Prevent set vCPU into INIT/SIPI_RECEIVED state when INIT are latched

[Paolo Bonzini]

On 11/11/19 14:46, Liran Alon wrote:
>> +	 * INITs are latched while CPU is in specific states
>> +	 * (SMM, VMX non-root mode, SVM with GIF=0).
> I didn’t want this line of comment as it may diverge from the implementation of kvm_vcpu_latch_init().
> That’s why I removed it.
> 
>> 	 * Because a CPU cannot be in these states immediately
>> 	 * after it has processed an INIT signal (and thus in
>> 	 * KVM_MP_STATE_INIT_RECEIVED state), just eat SIPIs

Got it... on the other hand knowing the specific states clarifies why
they cannot be in that state immediately after processing INIT.  It's a
bit of a catch-22 indeed.

Paolo