Re: [PATCH v3 20/62] KVM: SVM: Add a comment to explain why avic_vcpu_blocking() ignores IRQ blocking

Naveen N Rao
2025-06-23T15:58:37Z

On Wed, Jun 11, 2025 at 03:45:23PM -0700, Sean Christopherson wrote:
> Add a comment to explain why KVM clears IsRunning when putting a vCPU,
> even though leaving IsRunning=1 would be ok from a functional perspective.
> Per Maxim's experiments, a misbehaving VM could spam the AVIC doorbell so
> fast as to induce a 50%+ loss in performance.
>
> Link: https://lore.kernel.org/all/8d7e0d0391df4efc7cb28557297eb2ec9904f1e5.camel@redhat.com
> Cc: Maxim Levitsky <mlevitsk@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/svm/avic.c | 31 ++++++++++++++++++-------------
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index bf8b59556373..3cf929ac117f 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -1121,19 +1121,24 @@ void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
> if (!kvm_vcpu_apicv_active(vcpu))
> return;
>
> - /*
> - * Unload the AVIC when the vCPU is about to block, _before_
> - * the vCPU actually blocks.
> - *
> - * Any IRQs that arrive before IsRunning=0 will not cause an
> - * incomplete IPI vmexit on the source, therefore vIRR will also
> - * be checked by kvm_vcpu_check_block() before blocking. The
> - * memory barrier implicit in set_current_state orders writing
> - * IsRunning=0 before reading the vIRR. The processor needs a
> - * matching memory barrier on interrupt delivery between writing
> - * IRR and reading IsRunning; the lack of this barrier might be
> - * the cause of errata #1235).
> - */
> + /*
> + * Unload the AVIC when the vCPU is about to block, _before_ the vCPU
> + * actually blocks.
> + *
> + * Note, any IRQs that arrive before IsRunning=0 will not cause an
> + * incomplete IPI vmexit on the source; kvm_vcpu_check_block() handles
> + * this by checking vIRR one last time before blocking. The memory
> + * barrier implicit in set_current_state orders writing IsRunning=0
> + * before reading the vIRR. The processor needs a matching memory
> + * barrier on interrupt delivery between writing IRR and reading
> + * IsRunning; the lack of this barrier might be the cause of errata #1235).
> + *
> + * Clear IsRunning=0 even if guest IRQs are disabled, i.e. even if KVM
> + * doesn't need to detect events for scheduling purposes. The doorbell

Nit: just IsRunning (you can drop the =0 part).

Trying to understand the significance of IRQs being disabled here. Is
that a path KVM tries to optimize? Theoretically, it looks like we want
to clear IsRunning regardless of whether the vCPU is blocked so as to
prevent the guest from spamming the host with AVIC doorbells -- compared
to always keeping IsRunning set so as to speed up VM entry/exit.

> + * used to signal running vCPUs cannot be blocked, i.e. will perturb the
> + * CPU and cause noisy neighbor problems if the VM is sending interrupts
> + * to the vCPU while it's scheduled out.
> + */
> avic_vcpu_put(vcpu);
> }

Otherwise, this LGTM.
Acked-by: Naveen N Rao (AMD) <naveen@kernel.org>

Thanks,
Naveen