2022-08-31 00:38:53

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH 01/19] KVM: SVM: Process ICR on AVIC IPI delivery failure due to invalid target

Emulate ICR writes on AVIC IPI failures due to invalid targets using the
same logic as failures due to invalid types. AVIC acceleration fails if
_any_ of the targets are invalid, and crucially VM-Exits before sending
IPIs to targets that _are_ valid. In logical mode, the destination is a
bitmap, i.e. a single IPI can target multiple logical IDs. Doing nothing
causes KVM to drop IPIs if at least one target is valid and at least one
target is invalid.

Fixes: 18f40c53e10f ("svm: Add VMEXIT handlers for AVIC")
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/svm/avic.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 6919dee69f18..b1ade555e8d0 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -496,14 +496,18 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
trace_kvm_avic_incomplete_ipi(vcpu->vcpu_id, icrh, icrl, id, index);

switch (id) {
+ case AVIC_IPI_FAILURE_INVALID_TARGET:
case AVIC_IPI_FAILURE_INVALID_INT_TYPE:
/*
* Emulate IPIs that are not handled by AVIC hardware, which
- * only virtualizes Fixed, Edge-Triggered INTRs. The exit is
- * a trap, e.g. ICR holds the correct value and RIP has been
- * advanced, KVM is responsible only for emulating the IPI.
- * Sadly, hardware may sometimes leave the BUSY flag set, in
- * which case KVM needs to emulate the ICR write as well in
+ * only virtualizes Fixed, Edge-Triggered INTRs, and falls over
+ * if _any_ targets are invalid, e.g. if the logical mode mask
+ * is a superset of running vCPUs.
+ *
+ * The exit is a trap, e.g. ICR holds the correct value and RIP
+ * has been advanced, KVM is responsible only for emulating the
+ * IPI. Sadly, hardware may sometimes leave the BUSY flag set,
+ * in which case KVM needs to emulate the ICR write as well in
* order to clear the BUSY flag.
*/
if (icrl & APIC_ICR_BUSY)
@@ -519,8 +523,6 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
*/
avic_kick_target_vcpus(vcpu->kvm, apic, icrl, icrh, index);
break;
- case AVIC_IPI_FAILURE_INVALID_TARGET:
- break;
case AVIC_IPI_FAILURE_INVALID_BACKING_PAGE:
WARN_ONCE(1, "Invalid backing page\n");
break;
--
2.37.2.672.g94769d06f0-goog


2022-08-31 09:44:27

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [PATCH 01/19] KVM: SVM: Process ICR on AVIC IPI delivery failure due to invalid target

On Wed, 2022-08-31 at 00:34 +0000, Sean Christopherson wrote:
> Emulate ICR writes on AVIC IPI failures due to invalid targets using the
> same logic as failures due to invalid types. AVIC acceleration fails if
> _any_ of the targets are invalid
> , and crucially VM-Exits before sending
> IPIs to targets that _are_ valid. In logical mode, the destination is a
> bitmap, i.e. a single IPI can target multiple logical IDs. Doing nothing
> causes KVM to drop IPIs if at least one target is valid and at least one
> target is invalid.


This is a corner case, but it does makes sense to fix it.

Reviewed-by: Maxim Levitsky <[email protected]>

Best regards,
Maxim Levitsky

>
> Fixes: 18f40c53e10f ("svm: Add VMEXIT handlers for AVIC")
> Cc: [email protected]
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/svm/avic.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 6919dee69f18..b1ade555e8d0 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -496,14 +496,18 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
> trace_kvm_avic_incomplete_ipi(vcpu->vcpu_id, icrh, icrl, id, index);
>
> switch (id) {
> + case AVIC_IPI_FAILURE_INVALID_TARGET:
> case AVIC_IPI_FAILURE_INVALID_INT_TYPE:
> /*
> * Emulate IPIs that are not handled by AVIC hardware, which
> - * only virtualizes Fixed, Edge-Triggered INTRs. The exit is
> - * a trap, e.g. ICR holds the correct value and RIP has been
> - * advanced, KVM is responsible only for emulating the IPI.
> - * Sadly, hardware may sometimes leave the BUSY flag set, in
> - * which case KVM needs to emulate the ICR write as well in
> + * only virtualizes Fixed, Edge-Triggered INTRs, and falls over
> + * if _any_ targets are invalid, e.g. if the logical mode mask
> + * is a superset of running vCPUs.
> + *
> + * The exit is a trap, e.g. ICR holds the correct value and RIP
> + * has been advanced, KVM is responsible only for emulating the
> + * IPI. Sadly, hardware may sometimes leave the BUSY flag set,
> + * in which case KVM needs to emulate the ICR write as well in
> * order to clear the BUSY flag.
> */
> if (icrl & APIC_ICR_BUSY)
> @@ -519,8 +523,6 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
> */
> avic_kick_target_vcpus(vcpu->kvm, apic, icrl, icrh, index);
> break;
> - case AVIC_IPI_FAILURE_INVALID_TARGET:
> - break;
> case AVIC_IPI_FAILURE_INVALID_BACKING_PAGE:
> WARN_ONCE(1, "Invalid backing page\n");
> break;