Re: [PATCH 2/4] KVM: VMX: prepare sync_pir_to_irr for running with APICv disabled

David Matlack
2021-11-29T23:34:16Z

On Mon, Nov 22, 2021 at 07:43:09PM -0500, Paolo Bonzini wrote:
> If APICv is disabled for this vCPU, assigned devices may
> still attempt to post interrupts. In that case, we need
> to cancel the vmentry and deliver the interrupt with
> KVM_REQ_EVENT. Extend the existing code that handles
> injection of L1 interrupts into L2 to cover this case
> as well.
>
> vmx_hwapic_irr_update is only called when APICv is active
> so it would be confusing to add a check for
> vcpu->arch.apicv_active in there. Instead, just use
> vmx_set_rvi directly in vmx_sync_pir_to_irr.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: David Matlack <dmatlack@google.com>

(Although I agree with Sean's suggestions and 1 nit below.)

> ---
> arch/x86/kvm/vmx/vmx.c | 35 +++++++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index ba66c171d951..cccf1eab58ac 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6264,7 +6264,7 @@ static int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu)
> int max_irr;
> bool max_irr_updated;
>
> - if (KVM_BUG_ON(!vcpu->arch.apicv_active, vcpu->kvm))
> + if (KVM_BUG_ON(!enable_apicv, vcpu->kvm))
> return -EIO;
>
> if (pi_test_on(&vmx->pi_desc)) {
> @@ -6276,20 +6276,31 @@ static int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu)
> smp_mb__after_atomic();
> max_irr_updated =
> kvm_apic_update_irr(vcpu, vmx->pi_desc.pir, &max_irr);
> -
> - /*
> - * If we are running L2 and L1 has a new pending interrupt
> - * which can be injected, this may cause a vmexit or it may
> - * be injected into L2. Either way, this interrupt will be
> - * processed via KVM_REQ_EVENT, not RVI, because we do not use
> - * virtual interrupt delivery to inject L1 interrupts into L2.
> - */
> - if (is_guest_mode(vcpu) && max_irr_updated)
> - kvm_make_request(KVM_REQ_EVENT, vcpu);
> } else {
> max_irr = kvm_lapic_find_highest_irr(vcpu);
> + max_irr_updated = false;
> }
> - vmx_hwapic_irr_update(vcpu, max_irr);
> +
> + /*
> + * If virtual interrupt delivery is not in use, the interrupt
> + * will be processed via KVM_REQ_EVENT, not RVI. This can happen
> + * in two cases:
> + *
> + * 1) If we are running L2 and L1 has a new pending interrupt
> + * which can be injected, this may cause a vmexit or it may
> + * be injected into L2. We do not use virtual interrupt
> + * delivery to inject L1 interrupts into L2.
> + *
> + * 2) If APICv is disabled for this vCPU, assigned devices may
> + * still attempt to post interrupts. The posted interrupt
> + * vector will cause a vmexit and the subsequent entry will
> + * call sync_pir_to_irr.
> + */
> + if (!is_guest_mode(vcpu) && vcpu->arch.apicv_active)
> + vmx_set_rvi(max_irr);
> + else if (max_irr_updated)
> + kvm_make_request(KVM_REQ_EVENT, vcpu);

nit: In the version of this code that is currently in kvm/queue the
indentation of the previous 3 lines uses spaces instead of tabs. I see
tabs in this mail though.

> +
> return max_irr;
> }
>
> --
> 2.27.0
>
>