[PATCH MANUALSEL 5.10 1/2] KVM: x86: lapic: don't touch irr_pending in kvm_apic_update_apicv when inhibiting it

[PATCH MANUALSEL 5.10 1/2] KVM: x86: lapic: don't touch irr_pending in kvm_apic_update_apicv when inhibiting it

[Sasha Levin]

From: Maxim Levitsky <mlevitsk@redhat.com>

[ Upstream commit 755c2bf878607dbddb1423df9abf16b82205896f ]

kvm_apic_update_apicv is called when AVIC is still active, thus IRR bits
can be set by the CPU after it is called, and don't cause the irr_pending
to be set to true.

Also logic in avic_kick_target_vcpu doesn't expect a race with this
function so to make it simple, just keep irr_pending set to true and
let the next interrupt injection to the guest clear it.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20220207155447.840194-9-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/x86/kvm/lapic.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 677d21082454f..d484269a390bc 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2292,7 +2292,12 @@ void kvm_apic_update_apicv(struct kvm_vcpu *vcpu)
 		apic->irr_pending = true;
 		apic->isr_count = 1;
 	} else {
-		apic->irr_pending = (apic_search_irr(apic) != -1);
+		/*
+		 * Don't clear irr_pending, searching the IRR can race with
+		 * updates from the CPU as APICv is still active from hardware's
+		 * perspective.  The flag will be cleared as appropriate when
+		 * KVM injects the interrupt.
+		 */
 		apic->isr_count = count_vectors(apic->regs + APIC_ISR);
 	}
 }
-- 
2.34.1

[PATCH MANUALSEL 5.10 2/2] KVM: x86: nSVM: deal with L1 hypervisor that intercepts interrupts but lets L2 control them

[Sasha Levin]

From: Maxim Levitsky <mlevitsk@redhat.com>

[ Upstream commit 2b0ecccb55310a4b8ad5d59c703cf8c821be6260 ]

Fix a corner case in which the L1 hypervisor intercepts
interrupts (INTERCEPT_INTR) and either doesn't set
virtual interrupt masking (V_INTR_MASKING) or enters a
nested guest with EFLAGS.IF disabled prior to the entry.

In this case, despite the fact that L1 intercepts the interrupts,
KVM still needs to set up an interrupt window to wait before
injecting the INTR vmexit.

Currently the KVM instead enters an endless loop of 'req_immediate_exit'.

Exactly the same issue also happens for SMIs and NMI.
Fix this as well.

Note that on VMX, this case is impossible as there is only
'vmexit on external interrupts' execution control which either set,
in which case both host and guest's EFLAGS.IF
are ignored, or not set, in which case no VMexits are delivered.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20220207155447.840194-8-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/x86/kvm/svm/svm.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d515c8e68314c..ec9586a30a50c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3237,11 +3237,13 @@ static int svm_nmi_allowed(struct kvm_vcpu *vcpu, bool for_injection)
 	if (svm->nested.nested_run_pending)
 		return -EBUSY;
 
+	if (svm_nmi_blocked(vcpu))
+		return 0;
+
 	/* An NMI must not be injected into L2 if it's supposed to VM-Exit.  */
 	if (for_injection && is_guest_mode(vcpu) && nested_exit_on_nmi(svm))
 		return -EBUSY;
-
-	return !svm_nmi_blocked(vcpu);
+	return 1;
 }
 
 static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu)
@@ -3293,9 +3295,13 @@ bool svm_interrupt_blocked(struct kvm_vcpu *vcpu)
 static int svm_interrupt_allowed(struct kvm_vcpu *vcpu, bool for_injection)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
+
 	if (svm->nested.nested_run_pending)
 		return -EBUSY;
 
+	if (svm_interrupt_blocked(vcpu))
+		return 0;
+
 	/*
 	 * An IRQ must not be injected into L2 if it's supposed to VM-Exit,
 	 * e.g. if the IRQ arrived asynchronously after checking nested events.
@@ -3303,7 +3309,7 @@ static int svm_interrupt_allowed(struct kvm_vcpu *vcpu, bool for_injection)
 	if (for_injection && is_guest_mode(vcpu) && nested_exit_on_intr(svm))
 		return -EBUSY;
 
-	return !svm_interrupt_blocked(vcpu);
+	return 1;
 }
 
 static void enable_irq_window(struct kvm_vcpu *vcpu)
@@ -4023,11 +4029,14 @@ static int svm_smi_allowed(struct kvm_vcpu *vcpu, bool for_injection)
 	if (svm->nested.nested_run_pending)
 		return -EBUSY;
 
+	if (svm_smi_blocked(vcpu))
+		return 0;
+
 	/* An SMI must not be injected into L2 if it's supposed to VM-Exit.  */
 	if (for_injection && is_guest_mode(vcpu) && nested_exit_on_smi(svm))
 		return -EBUSY;
 
-	return !svm_smi_blocked(vcpu);
+	return 1;
 }
 
 static int svm_pre_enter_smm(struct kvm_vcpu *vcpu, char *smstate)
-- 
2.34.1

Re: [PATCH MANUALSEL 5.10 2/2] KVM: x86: nSVM: deal with L1 hypervisor that intercepts interrupts but lets L2 control them

[Paolo Bonzini]

On 2/22/22 15:05, Sasha Levin wrote:
> From: Maxim Levitsky <mlevitsk@redhat.com>
> 
> [ Upstream commit 2b0ecccb55310a4b8ad5d59c703cf8c821be6260 ]
> 
> Fix a corner case in which the L1 hypervisor intercepts
> interrupts (INTERCEPT_INTR) and either doesn't set
> virtual interrupt masking (V_INTR_MASKING) or enters a
> nested guest with EFLAGS.IF disabled prior to the entry.
> 
> In this case, despite the fact that L1 intercepts the interrupts,
> KVM still needs to set up an interrupt window to wait before
> injecting the INTR vmexit.
> 
> Currently the KVM instead enters an endless loop of 'req_immediate_exit'.
> 
> Exactly the same issue also happens for SMIs and NMI.
> Fix this as well.
> 
> Note that on VMX, this case is impossible as there is only
> 'vmexit on external interrupts' execution control which either set,
> in which case both host and guest's EFLAGS.IF
> are ignored, or not set, in which case no VMexits are delivered.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> Message-Id: <20220207155447.840194-8-mlevitsk@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>   arch/x86/kvm/svm/svm.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d515c8e68314c..ec9586a30a50c 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3237,11 +3237,13 @@ static int svm_nmi_allowed(struct kvm_vcpu *vcpu, bool for_injection)
>   	if (svm->nested.nested_run_pending)
>   		return -EBUSY;
>   
> +	if (svm_nmi_blocked(vcpu))
> +		return 0;
> +
>   	/* An NMI must not be injected into L2 if it's supposed to VM-Exit.  */
>   	if (for_injection && is_guest_mode(vcpu) && nested_exit_on_nmi(svm))
>   		return -EBUSY;
> -
> -	return !svm_nmi_blocked(vcpu);
> +	return 1;
>   }
>   
>   static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu)
> @@ -3293,9 +3295,13 @@ bool svm_interrupt_blocked(struct kvm_vcpu *vcpu)
>   static int svm_interrupt_allowed(struct kvm_vcpu *vcpu, bool for_injection)
>   {
>   	struct vcpu_svm *svm = to_svm(vcpu);
> +
>   	if (svm->nested.nested_run_pending)
>   		return -EBUSY;
>   
> +	if (svm_interrupt_blocked(vcpu))
> +		return 0;
> +
>   	/*
>   	 * An IRQ must not be injected into L2 if it's supposed to VM-Exit,
>   	 * e.g. if the IRQ arrived asynchronously after checking nested events.
> @@ -3303,7 +3309,7 @@ static int svm_interrupt_allowed(struct kvm_vcpu *vcpu, bool for_injection)
>   	if (for_injection && is_guest_mode(vcpu) && nested_exit_on_intr(svm))
>   		return -EBUSY;
>   
> -	return !svm_interrupt_blocked(vcpu);
> +	return 1;
>   }
>   
>   static void enable_irq_window(struct kvm_vcpu *vcpu)
> @@ -4023,11 +4029,14 @@ static int svm_smi_allowed(struct kvm_vcpu *vcpu, bool for_injection)
>   	if (svm->nested.nested_run_pending)
>   		return -EBUSY;
>   
> +	if (svm_smi_blocked(vcpu))
> +		return 0;
> +
>   	/* An SMI must not be injected into L2 if it's supposed to VM-Exit.  */
>   	if (for_injection && is_guest_mode(vcpu) && nested_exit_on_smi(svm))
>   		return -EBUSY;
>   
> -	return !svm_smi_blocked(vcpu);
> +	return 1;
>   }
>   
>   static int svm_pre_enter_smm(struct kvm_vcpu *vcpu, char *smstate)


Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [PATCH MANUALSEL 5.10 1/2] KVM: x86: lapic: don't touch irr_pending in kvm_apic_update_apicv when inhibiting it

[Paolo Bonzini]

On 2/22/22 15:05, Sasha Levin wrote:
> From: Maxim Levitsky <mlevitsk@redhat.com>
> 
> [ Upstream commit 755c2bf878607dbddb1423df9abf16b82205896f ]
> 
> kvm_apic_update_apicv is called when AVIC is still active, thus IRR bits
> can be set by the CPU after it is called, and don't cause the irr_pending
> to be set to true.
> 
> Also logic in avic_kick_target_vcpu doesn't expect a race with this
> function so to make it simple, just keep irr_pending set to true and
> let the next interrupt injection to the guest clear it.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> Message-Id: <20220207155447.840194-9-mlevitsk@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>   arch/x86/kvm/lapic.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 677d21082454f..d484269a390bc 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2292,7 +2292,12 @@ void kvm_apic_update_apicv(struct kvm_vcpu *vcpu)
>   		apic->irr_pending = true;
>   		apic->isr_count = 1;
>   	} else {
> -		apic->irr_pending = (apic_search_irr(apic) != -1);
> +		/*
> +		 * Don't clear irr_pending, searching the IRR can race with
> +		 * updates from the CPU as APICv is still active from hardware's
> +		 * perspective.  The flag will be cleared as appropriate when
> +		 * KVM injects the interrupt.
> +		 */
>   		apic->isr_count = count_vectors(apic->regs + APIC_ISR);
>   	}
>   }


Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [PATCH MANUALSEL 5.10 1/2] KVM: x86: lapic: don't touch irr_pending in kvm_apic_update_apicv when inhibiting it

[Zenghui Yu]

Hi,

On 2022/3/2 1:10, Paolo Bonzini wrote:
> On 2/22/22 15:05, Sasha Levin wrote:
>> From: Maxim Levitsky <mlevitsk@redhat.com>
>>
>> [ Upstream commit 755c2bf878607dbddb1423df9abf16b82205896f ]
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>

What prevented it to be accepted into 5.10-stable? It can still be
applied cleanly on top of linux-5.10.y.

Re: [PATCH MANUALSEL 5.10 1/2] KVM: x86: lapic: don't touch irr_pending in kvm_apic_update_apicv when inhibiting it

[Sean Christopherson]

On Mon, Jul 25, 2022, Zenghui Yu wrote:
> Hi,
> 
> On 2022/3/2 1:10, Paolo Bonzini wrote:
> > On 2/22/22 15:05, Sasha Levin wrote:
> > > From: Maxim Levitsky <mlevitsk@redhat.com>
> > > 
> > > [ Upstream commit 755c2bf878607dbddb1423df9abf16b82205896f ]
> > 
> > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> What prevented it to be accepted into 5.10-stable? It can still be
> applied cleanly on top of linux-5.10.y.

KVM opts out of the AUTOSEL logic and instead uses MANUALSEL.  The basic idea is
the same, use scripts/magic to determine what commits that _aren't_ tagged with an
explicit "Cc: stable@vger.kernel.org" should be backported to stable trees, the
difference being that MANUALSEL requires an explicit Acked-by from the maintainer.

Many (most?) of the automatically selected patches for KVM are good stable candidates,
but there are enough selected patches that we _don't_ want backported that the "apply
unless there's an objection" model of AUTOSEL is a bit too risky for us. 

Re: [PATCH MANUALSEL 5.10 1/2] KVM: x86: lapic: don't touch irr_pending in kvm_apic_update_apicv when inhibiting it

[Paolo Bonzini]

On 7/26/22 01:49, Sean Christopherson wrote:
> On Mon, Jul 25, 2022, Zenghui Yu wrote:
>> Hi,
>>
>> On 2022/3/2 1:10, Paolo Bonzini wrote:
>>> On 2/22/22 15:05, Sasha Levin wrote:
>>>> From: Maxim Levitsky <mlevitsk@redhat.com>
>>>>
>>>> [ Upstream commit 755c2bf878607dbddb1423df9abf16b82205896f ]
>>>
>>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> What prevented it to be accepted into 5.10-stable? It can still be
>> applied cleanly on top of linux-5.10.y.
> 
> KVM opts out of the AUTOSEL logic and instead uses MANUALSEL.  The basic idea is
> the same, use scripts/magic to determine what commits that _aren't_ tagged with an
> explicit "Cc: stable@vger.kernel.org" should be backported to stable trees, the
> difference being that MANUALSEL requires an explicit Acked-by from the maintainer.

But as far as I understand it was not applied, and neither was "KVM: 
x86: nSVM: deal with L1 hypervisor that intercepts interrupts but lets 
L2 control them".

Paolo

Re: [PATCH MANUALSEL 5.10 1/2] KVM: x86: lapic: don't touch irr_pending in kvm_apic_update_apicv when inhibiting it

[Sean Christopherson]

On Tue, Jul 26, 2022, Paolo Bonzini wrote:
> On 7/26/22 01:49, Sean Christopherson wrote:
> > On Mon, Jul 25, 2022, Zenghui Yu wrote:
> > > Hi,
> > > 
> > > On 2022/3/2 1:10, Paolo Bonzini wrote:
> > > > On 2/22/22 15:05, Sasha Levin wrote:
> > > > > From: Maxim Levitsky <mlevitsk@redhat.com>
> > > > > 
> > > > > [ Upstream commit 755c2bf878607dbddb1423df9abf16b82205896f ]
> > > > 
> > > > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> > > 
> > > What prevented it to be accepted into 5.10-stable? It can still be
> > > applied cleanly on top of linux-5.10.y.
> > 
> > KVM opts out of the AUTOSEL logic and instead uses MANUALSEL.  The basic idea is
> > the same, use scripts/magic to determine what commits that _aren't_ tagged with an
> > explicit "Cc: stable@vger.kernel.org" should be backported to stable trees, the
> > difference being that MANUALSEL requires an explicit Acked-by from the maintainer.
> 
> But as far as I understand it was not applied, and neither was "KVM: x86:
> nSVM: deal with L1 hypervisor that intercepts interrupts but lets L2 control
> them".

Ah, I misunderstood the question.  I'll get out of the way.