diff mbox series

[1/5] KVM: x86: Re-enter guest if WRMSR(X2APIC_ICR) fastpath is successful

Message ID 20240802195120.325560-2-seanjc@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86: Fastpath cleanup, fix, and enhancement | expand

Commit Message

Sean Christopherson Aug. 2, 2024, 7:51 p.m. UTC
Re-enter the guest in the fastpath if WRMSR emulation for x2APIC's ICR is
successful, as no additional work is needed, i.e. there is no code unique
for WRMSR exits between the fastpath and the "!= EXIT_FASTPATH_NONE" check
in __vmx_handle_exit().

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paolo Bonzini Sept. 2, 2024, 9:58 a.m. UTC | #1
On Fri, Aug 2, 2024 at 9:51 PM Sean Christopherson <seanjc@google.com> wrote:
> Re-enter the guest in the fastpath if WRMSR emulation for x2APIC's ICR is
> successful, as no additional work is needed, i.e. there is no code unique
> for WRMSR exits between the fastpath and the "!= EXIT_FASTPATH_NONE" check
> in __vmx_handle_exit().

What about if you send an IPI to yourself?  Doesn't that return true
for kvm_vcpu_exit_request() if posted interrupts are disabled?

Paolo

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/x86.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index af6c8cf6a37a..cf397110953f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2173,7 +2173,7 @@ fastpath_t handle_fastpath_set_msr_irqoff(struct kvm_vcpu *vcpu)
>                 data = kvm_read_edx_eax(vcpu);
>                 if (!handle_fastpath_set_x2apic_icr_irqoff(vcpu, data)) {
>                         kvm_skip_emulated_instruction(vcpu);
> -                       ret = EXIT_FASTPATH_EXIT_HANDLED;
> +                       ret = EXIT_FASTPATH_REENTER_GUEST;
>                 }
>                 break;
>         case MSR_IA32_TSC_DEADLINE:
> --
> 2.46.0.rc2.264.g509ed76dc8-goog
>
Sean Christopherson Sept. 3, 2024, 3:09 p.m. UTC | #2
On Mon, Sep 02, 2024, Paolo Bonzini wrote:
> On Fri, Aug 2, 2024 at 9:51 PM Sean Christopherson <seanjc@google.com> wrote:
> > Re-enter the guest in the fastpath if WRMSR emulation for x2APIC's ICR is
> > successful, as no additional work is needed, i.e. there is no code unique
> > for WRMSR exits between the fastpath and the "!= EXIT_FASTPATH_NONE" check
> > in __vmx_handle_exit().
> 
> What about if you send an IPI to yourself?  Doesn't that return true
> for kvm_vcpu_exit_request() if posted interrupts are disabled?

Yes, but that doesn't have anything to do with WRMSR itself, as KVM needs to morph
EXIT_FASTPATH_EXIT_HANDLED => EXIT_FASTPATH_REENTER_GUEST if there's a pending
event that needs requires injection.

Given that kvm_x86_ops.sync_pir_to_irr is likely NULL if virtual interrupt delivery
is enabled, the overhead of the trying to re-enter the guest it essentially a few
cycles, e.g. check vcpu->mode and kvm_request_pending().
Paolo Bonzini Sept. 3, 2024, 4:49 p.m. UTC | #3
On Tue, Sep 3, 2024 at 5:09 PM Sean Christopherson <seanjc@google.com> wrote:
> On Mon, Sep 02, 2024, Paolo Bonzini wrote:
> > On Fri, Aug 2, 2024 at 9:51 PM Sean Christopherson <seanjc@google.com> wrote:
> > > Re-enter the guest in the fastpath if WRMSR emulation for x2APIC's ICR is
> > > successful, as no additional work is needed, i.e. there is no code unique
> > > for WRMSR exits between the fastpath and the "!= EXIT_FASTPATH_NONE" check
> > > in __vmx_handle_exit().
> >
> > What about if you send an IPI to yourself?  Doesn't that return true
> > for kvm_vcpu_exit_request() if posted interrupts are disabled?
>
> Yes, but that doesn't have anything to do with WRMSR itself, as KVM needs to morph
> EXIT_FASTPATH_EXIT_HANDLED => EXIT_FASTPATH_REENTER_GUEST if there's a pending
> event that needs requires injection.

The other way round? i.e. treat EXIT_FASTPATH_REENTER_GUEST as
EXIT_FASTPATH_EXIT_HANDLED to go through event injection.

> Given that kvm_x86_ops.sync_pir_to_irr is likely NULL if virtual interrupt delivery
> is enabled, the overhead of the trying to re-enter the guest it essentially a few
> cycles, e.g. check vcpu->mode and kvm_request_pending().

No, I wasn't worried about performance. Probably I misread

                if (likely(exit_fastpath != EXIT_FASTPATH_REENTER_GUEST))
                        break;

as something like

                if (likely(exit_fastpath == EXIT_FASTPATH_REENTER_GUEST))
                        continue;

EXIT_FASTPATH_REENTER_GUEST is exactly what's needed here.

Paolo
Sean Christopherson Sept. 3, 2024, 4:58 p.m. UTC | #4
On Tue, Sep 03, 2024, Paolo Bonzini wrote:
> On Tue, Sep 3, 2024 at 5:09 PM Sean Christopherson <seanjc@google.com> wrote:
> > On Mon, Sep 02, 2024, Paolo Bonzini wrote:
> > > On Fri, Aug 2, 2024 at 9:51 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > Re-enter the guest in the fastpath if WRMSR emulation for x2APIC's ICR is
> > > > successful, as no additional work is needed, i.e. there is no code unique
> > > > for WRMSR exits between the fastpath and the "!= EXIT_FASTPATH_NONE" check
> > > > in __vmx_handle_exit().
> > >
> > > What about if you send an IPI to yourself?  Doesn't that return true
> > > for kvm_vcpu_exit_request() if posted interrupts are disabled?
> >
> > Yes, but that doesn't have anything to do with WRMSR itself, as KVM needs to morph
> > EXIT_FASTPATH_EXIT_HANDLED => EXIT_FASTPATH_REENTER_GUEST if there's a pending
> > event that needs requires injection.
> 
> The other way round? i.e. treat EXIT_FASTPATH_REENTER_GUEST as
> EXIT_FASTPATH_EXIT_HANDLED to go through event injection.

Doh, yes.
diff mbox series

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index af6c8cf6a37a..cf397110953f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2173,7 +2173,7 @@  fastpath_t handle_fastpath_set_msr_irqoff(struct kvm_vcpu *vcpu)
 		data = kvm_read_edx_eax(vcpu);
 		if (!handle_fastpath_set_x2apic_icr_irqoff(vcpu, data)) {
 			kvm_skip_emulated_instruction(vcpu);
-			ret = EXIT_FASTPATH_EXIT_HANDLED;
+			ret = EXIT_FASTPATH_REENTER_GUEST;
 		}
 		break;
 	case MSR_IA32_TSC_DEADLINE: