| Message ID | 20211009021236.4122790-41-seanjc@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | KVM: Halt-polling and x86 APICv overhaul | expand |
On 09/10/21 04:12, Sean Christopherson wrote: > > Lastly, this aligns the non-nested and nested usage of triggering posted > interrupts, and will allow for additional cleanups. It also aligns with SVM a little bit more (especially given patch 35), doesn't it? Paolo
On Mon, Oct 25, 2021, Paolo Bonzini wrote: > On 09/10/21 04:12, Sean Christopherson wrote: > > > > Lastly, this aligns the non-nested and nested usage of triggering posted > > interrupts, and will allow for additional cleanups. > > It also aligns with SVM a little bit more (especially given patch 35), > doesn't it? Yes, aligning VMX and SVM APICv behavior as much as possible is definitely a goal of this series, though I suspect I failed to state that anywhere.
On Wed, 2021-10-27 at 15:30 +0000, Sean Christopherson wrote: > On Mon, Oct 25, 2021, Paolo Bonzini wrote: > > On 09/10/21 04:12, Sean Christopherson wrote: > > > Lastly, this aligns the non-nested and nested usage of triggering posted > > > interrupts, and will allow for additional cleanups. > > > > It also aligns with SVM a little bit more (especially given patch 35), > > doesn't it? > > Yes, aligning VMX and SVM APICv behavior as much as possible is definitely a goal > of this series, though I suspect I failed to state that anywhere. > Looks reasonable to me. Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitky
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 44d760dde0f9..78c8bc7f1b3b 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -4024,8 +4024,7 @@ static int vmx_deliver_posted_interrupt(struct kvm_vcpu *vcpu, int vector) * guaranteed to see PID.ON=1 and sync the PIR to IRR if triggering a * posted interrupt "fails" because vcpu->mode != IN_GUEST_MODE. */ - if (vcpu != kvm_get_running_vcpu() && - !kvm_vcpu_trigger_posted_interrupt(vcpu, false)) + if (!kvm_vcpu_trigger_posted_interrupt(vcpu, false)) kvm_vcpu_wake_up(vcpu); return 0;
Drop a check that guards triggering a posted interrupt on the currently running vCPU, and more importantly guards waking the target vCPU if triggering a posted interrupt fails because the vCPU isn't IN_GUEST_MODE. The "do nothing" logic when "vcpu == running_vcpu" works only because KVM doesn't have a path to ->deliver_posted_interrupt() from asynchronous context, e.g. if apic_timer_expired() were changed to always go down the posted interrupt path for APICv, or if the IN_GUEST_MODE check in kvm_use_posted_timer_interrupt() were dropped, and the hrtimer fired in kvm_vcpu_block() after the final kvm_vcpu_check_block() check, the vCPU would be scheduled() out without being awakened, i.e. would "miss" the timer interrupt. One could argue that invoking kvm_apic_local_deliver() from (soft) IRQ context for the current running vCPU should be illegal, but nothing in KVM actually enforces that rules. There's also no strong obvious benefit to making such behavior illegal, e.g. checking IN_GUEST_MODE and calling kvm_vcpu_wake_up() is at worst marginally more costly than querying the current running vCPU. Lastly, this aligns the non-nested and nested usage of triggering posted interrupts, and will allow for additional cleanups. Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/vmx/vmx.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)