All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI
@ 2020-08-12 17:51 Sean Christopherson
  2020-08-19 23:56 ` Sasha Levin
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sean Christopherson @ 2020-08-12 17:51 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Liran Alon

On successful nested VM-Enter, check for pending interrupts and convert
the highest priority interrupt to a pending posted interrupt if it
matches L2's notification vector.  If the vCPU receives a notification
interrupt before nested VM-Enter (assuming L1 disables IRQs before doing
VM-Enter), the pending interrupt (for L1) should be recognized and
processed as a posted interrupt when interrupts become unblocked after
VM-Enter to L2.

This fixes a bug where L1/L2 will get stuck in an infinite loop if L1 is
trying to inject an interrupt into L2 by setting the appropriate bit in
L2's PIR and sending a self-IPI prior to VM-Enter (as opposed to KVM's
method of manually moving the vector from PIR->vIRR/RVI).  KVM will
observe the IPI while the vCPU is in L1 context and so won't immediately
morph it to a posted interrupt for L2.  The pending interrupt will be
seen by vmx_check_nested_events(), cause KVM to force an immediate exit
after nested VM-Enter, and eventually be reflected to L1 as a VM-Exit.
After handling the VM-Exit, L1 will see that L2 has a pending interrupt
in PIR, send another IPI, and repeat until L2 is killed.

Note, posted interrupts require virtual interrupt deliveriy, and virtual
interrupt delivery requires exit-on-interrupt, ergo interrupts will be
unconditionally unmasked on VM-Enter if posted interrupts are enabled.

Fixes: 705699a13994 ("KVM: nVMX: Enable nested posted interrupt processing")
Cc: stable@vger.kernel.org
Cc: Liran Alon <liran.alon@oracle.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---

I am by no means 100% confident this the a complete, correct fix.  I also
don't like exporting two low level LAPIC functions.  But, the fix does
appear to work as intended.

 arch/x86/kvm/lapic.c      | 7 +++++++
 arch/x86/kvm/lapic.h      | 1 +
 arch/x86/kvm/vmx/nested.c | 8 ++++++++
 3 files changed, 16 insertions(+)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 5ccbee7165a21..ce37376bc96ec 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -488,6 +488,12 @@ static inline void apic_clear_irr(int vec, struct kvm_lapic *apic)
 	}
 }
 
+void kvm_apic_clear_irr(struct kvm_vcpu *vcpu, int vec)
+{
+	apic_clear_irr(vec, vcpu->arch.apic);
+}
+EXPORT_SYMBOL_GPL(kvm_apic_clear_irr);
+
 static inline void apic_set_isr(int vec, struct kvm_lapic *apic)
 {
 	struct kvm_vcpu *vcpu;
@@ -2461,6 +2467,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	__apic_update_ppr(apic, &ppr);
 	return apic_has_interrupt_for_ppr(apic, ppr);
 }
+EXPORT_SYMBOL_GPL(kvm_apic_has_interrupt);
 
 int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu)
 {
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 754f29beb83e3..4fb86e3a9dd3d 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -89,6 +89,7 @@ int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
 bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 			   int shorthand, unsigned int dest, int dest_mode);
 int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2);
+void kvm_apic_clear_irr(struct kvm_vcpu *vcpu, int vec);
 bool __kvm_apic_update_irr(u32 *pir, void *regs, int *max_irr);
 bool kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir, int *max_irr);
 void kvm_apic_update_ppr(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 23b58c28a1c92..2acf33b110b5c 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3528,6 +3528,14 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
 	if (unlikely(status != NVMX_VMENTRY_SUCCESS))
 		goto vmentry_failed;
 
+	/* Emulate processing of posted interrupts on VM-Enter. */
+	if (nested_cpu_has_posted_intr(vmcs12) &&
+	    kvm_apic_has_interrupt(vcpu) == vmx->nested.posted_intr_nv) {
+		vmx->nested.pi_pending = true;
+		kvm_make_request(KVM_REQ_EVENT, vcpu);
+		kvm_apic_clear_irr(vcpu, vmx->nested.posted_intr_nv);
+	}
+
 	/* Hide L1D cache contents from the nested guest.  */
 	vmx->vcpu.arch.l1tf_flush_l1d = true;
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI
  2020-08-12 17:51 [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI Sean Christopherson
@ 2020-08-19 23:56 ` Sasha Levin
  2020-08-26 13:54 ` Sasha Levin
  2020-10-06 17:36 ` Jim Mattson
  2 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2020-08-19 23:56 UTC (permalink / raw)
  To: Sasha Levin, Sean Christopherson, Paolo Bonzini
  Cc: Sean Christopherson, stable, Liran Alon, stable

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 705699a13994 ("KVM: nVMX: Enable nested posted interrupt processing").

The bot has tested the following trees: v5.8.1, v5.7.15, v5.4.58, v4.19.139, v4.14.193, v4.9.232, v4.4.232.

v5.8.1: Build OK!
v5.7.15: Build OK!
v5.4.58: Failed to apply! Possible dependencies:
    59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
    5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")

v4.19.139: Failed to apply! Possible dependencies:
    0b0a31badb2d ("KVM: x86: hyperv: valid_bank_mask should be 'u64'")
    214ff83d4473 ("KVM: x86: hyperv: implement PV IPI send hypercalls")
    2cefc5feb80c ("KVM: x86: hyperv: optimize kvm_hv_flush_tlb() for vp_index == vcpu_idx case")
    360cae313702 ("KVM: PPC: Book3S HV: Nested guest entry via hypercall")
    59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
    5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
    89329c0be8bd ("KVM: PPC: Book3S HV: Clear partition table entry on vm teardown")
    8e3f5fc1045d ("KVM: PPC: Book3S HV: Framework and hcall stubs for nested virtualization")
    a812297c4fd9 ("KVM: x86: hyperv: optimize 'all cpus' case in kvm_hv_flush_tlb()")
    aa069a996951 ("KVM: PPC: Book3S HV: Add a VM capability to enable nested virtualization")
    e6b6c483ebe9 ("KVM: x86: hyperv: fix 'tlb_lush' typo")
    f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")

v4.14.193: Failed to apply! Possible dependencies:
    0234bf885236 ("KVM: x86: introduce ISA specific SMM entry/exit callbacks")
    05cade71cf3b ("KVM: nSVM: fix SMI injection in guest mode")
    44883f01fe6a ("KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd")
    59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
    5acc5c063196 ("KVM: Introduce KVM_MEMORY_ENCRYPT_OP ioctl")
    5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
    69eaedee411c ("KVM: Introduce KVM_MEMORY_ENCRYPT_{UN,}REG_REGION ioctl")
    72d7b374b14d ("KVM: x86: introduce ISA specific smi_allowed callback")
    a2e164e7f45a ("x86/kvm/hyper-v: add reenlightenment MSRs support")
    cc3d967f7e32 ("KVM: SVM: detect opening of SMI window using STGI intercept")
    f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")
    faeb7833eee0 ("kvm: x86: hyperv: guest->host event signaling via eventfd")

v4.9.232: Failed to apply! Possible dependencies:
    004172bdad64 ("sched/core: Remove unnecessary #include headers")
    174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
    3aa53859d23e ("KVM: Documentation: document MCE ioctls")
    3ca0ff571b09 ("locking/mutex: Rework mutex::owner")
    4036e3874a1c ("KVM: s390: ioctls to get and set guest storage attributes")
    44883f01fe6a ("KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd")
    59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
    5acc5c063196 ("KVM: Introduce KVM_MEMORY_ENCRYPT_OP ioctl")
    5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
    69eaedee411c ("KVM: Introduce KVM_MEMORY_ENCRYPT_{UN,}REG_REGION ioctl")
    a2e164e7f45a ("x86/kvm/hyper-v: add reenlightenment MSRs support")
    ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
    b3c045d33218 ("KVM: lapic: remove unnecessary KVM_REQ_EVENT on PPR update")
    c92701322711 ("KVM: PPC: Book3S HV: Add userspace interfaces for POWER9 MMU")
    ea8b1c4a6019 ("drivers: psci: PSCI checker module")
    eb90f3417a0c ("KVM: vmx: speed up TPR below threshold vmexits")
    ef1ead0c3b1d ("KVM: PPC: Book3S HV: HPT resizing documentation and reserved numbers")
    f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")
    faeb7833eee0 ("kvm: x86: hyperv: guest->host event signaling via eventfd")

v4.4.232: Failed to apply! Possible dependencies:
    1e6e2755b635 ("KVM: x86: Misc LAPIC changes to expose helper functions")
    520040146a0a ("KVM: x86: Use vector-hashing to deliver lowest-priority interrupts")
    5c919412fe61 ("kvm/x86: Hyper-V synthetic interrupt controller")
    6308630bd3db ("kvm/x86: split ioapic-handled and EOI exit bitmaps")
    b3c045d33218 ("KVM: lapic: remove unnecessary KVM_REQ_EVENT on PPR update")
    d62caabb41f3 ("kvm/x86: per-vcpu apicv deactivation support")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI
  2020-08-12 17:51 [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI Sean Christopherson
  2020-08-19 23:56 ` Sasha Levin
@ 2020-08-26 13:54 ` Sasha Levin
  2020-09-23 14:44   ` Paolo Bonzini
  2020-10-06 17:36 ` Jim Mattson
  2 siblings, 1 reply; 8+ messages in thread
From: Sasha Levin @ 2020-08-26 13:54 UTC (permalink / raw)
  To: Sasha Levin, Sean Christopherson, Paolo Bonzini
  Cc: Sean Christopherson, stable, Liran Alon, stable

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 705699a13994 ("KVM: nVMX: Enable nested posted interrupt processing").

The bot has tested the following trees: v5.8.2, v5.7.16, v5.4.59, v4.19.140, v4.14.193, v4.9.232, v4.4.232.

v5.8.2: Build OK!
v5.7.16: Build OK!
v5.4.59: Failed to apply! Possible dependencies:
    59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
    5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")

v4.19.140: Failed to apply! Possible dependencies:
    0b0a31badb2d ("KVM: x86: hyperv: valid_bank_mask should be 'u64'")
    214ff83d4473 ("KVM: x86: hyperv: implement PV IPI send hypercalls")
    2cefc5feb80c ("KVM: x86: hyperv: optimize kvm_hv_flush_tlb() for vp_index == vcpu_idx case")
    360cae313702 ("KVM: PPC: Book3S HV: Nested guest entry via hypercall")
    59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
    5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
    89329c0be8bd ("KVM: PPC: Book3S HV: Clear partition table entry on vm teardown")
    8e3f5fc1045d ("KVM: PPC: Book3S HV: Framework and hcall stubs for nested virtualization")
    a812297c4fd9 ("KVM: x86: hyperv: optimize 'all cpus' case in kvm_hv_flush_tlb()")
    aa069a996951 ("KVM: PPC: Book3S HV: Add a VM capability to enable nested virtualization")
    e6b6c483ebe9 ("KVM: x86: hyperv: fix 'tlb_lush' typo")
    f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")

v4.14.193: Failed to apply! Possible dependencies:
    0234bf885236 ("KVM: x86: introduce ISA specific SMM entry/exit callbacks")
    05cade71cf3b ("KVM: nSVM: fix SMI injection in guest mode")
    44883f01fe6a ("KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd")
    59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
    5acc5c063196 ("KVM: Introduce KVM_MEMORY_ENCRYPT_OP ioctl")
    5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
    69eaedee411c ("KVM: Introduce KVM_MEMORY_ENCRYPT_{UN,}REG_REGION ioctl")
    72d7b374b14d ("KVM: x86: introduce ISA specific smi_allowed callback")
    a2e164e7f45a ("x86/kvm/hyper-v: add reenlightenment MSRs support")
    cc3d967f7e32 ("KVM: SVM: detect opening of SMI window using STGI intercept")
    f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")
    faeb7833eee0 ("kvm: x86: hyperv: guest->host event signaling via eventfd")

v4.9.232: Failed to apply! Possible dependencies:
    004172bdad64 ("sched/core: Remove unnecessary #include headers")
    174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
    3aa53859d23e ("KVM: Documentation: document MCE ioctls")
    3ca0ff571b09 ("locking/mutex: Rework mutex::owner")
    4036e3874a1c ("KVM: s390: ioctls to get and set guest storage attributes")
    44883f01fe6a ("KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd")
    59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
    5acc5c063196 ("KVM: Introduce KVM_MEMORY_ENCRYPT_OP ioctl")
    5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
    69eaedee411c ("KVM: Introduce KVM_MEMORY_ENCRYPT_{UN,}REG_REGION ioctl")
    a2e164e7f45a ("x86/kvm/hyper-v: add reenlightenment MSRs support")
    ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
    b3c045d33218 ("KVM: lapic: remove unnecessary KVM_REQ_EVENT on PPR update")
    c92701322711 ("KVM: PPC: Book3S HV: Add userspace interfaces for POWER9 MMU")
    ea8b1c4a6019 ("drivers: psci: PSCI checker module")
    eb90f3417a0c ("KVM: vmx: speed up TPR below threshold vmexits")
    ef1ead0c3b1d ("KVM: PPC: Book3S HV: HPT resizing documentation and reserved numbers")
    f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")
    faeb7833eee0 ("kvm: x86: hyperv: guest->host event signaling via eventfd")

v4.4.232: Failed to apply! Possible dependencies:
    1e6e2755b635 ("KVM: x86: Misc LAPIC changes to expose helper functions")
    520040146a0a ("KVM: x86: Use vector-hashing to deliver lowest-priority interrupts")
    5c919412fe61 ("kvm/x86: Hyper-V synthetic interrupt controller")
    6308630bd3db ("kvm/x86: split ioapic-handled and EOI exit bitmaps")
    b3c045d33218 ("KVM: lapic: remove unnecessary KVM_REQ_EVENT on PPR update")
    d62caabb41f3 ("kvm/x86: per-vcpu apicv deactivation support")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI
  2020-08-26 13:54 ` Sasha Levin
@ 2020-09-23 14:44   ` Paolo Bonzini
  2020-09-23 15:49     ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2020-09-23 14:44 UTC (permalink / raw)
  To: Sasha Levin, Sean Christopherson; +Cc: stable, Liran Alon

Queued, thanks.

I cannot think of a "nicer" way to do this, we could perhaps move

+		vmx->nested.pi_pending = true;
+		kvm_make_request(KVM_REQ_EVENT, vcpu);
+		kvm_apic_clear_irr(vcpu, vmx->nested.posted_intr_nv);

to a separate function (possibly with the IRR clear made conditional, so
that we can reuse the function for regular posted interrupt injection)
but that is it.

Paolo

On 26/08/20 15:54, Sasha Levin wrote:
> Hi
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag
> fixing commit: 705699a13994 ("KVM: nVMX: Enable nested posted interrupt processing").
> 
> The bot has tested the following trees: v5.8.2, v5.7.16, v5.4.59, v4.19.140, v4.14.193, v4.9.232, v4.4.232.
> 
> v5.8.2: Build OK!
> v5.7.16: Build OK!
> v5.4.59: Failed to apply! Possible dependencies:
>     59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
>     5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
> 
> v4.19.140: Failed to apply! Possible dependencies:
>     0b0a31badb2d ("KVM: x86: hyperv: valid_bank_mask should be 'u64'")
>     214ff83d4473 ("KVM: x86: hyperv: implement PV IPI send hypercalls")
>     2cefc5feb80c ("KVM: x86: hyperv: optimize kvm_hv_flush_tlb() for vp_index == vcpu_idx case")
>     360cae313702 ("KVM: PPC: Book3S HV: Nested guest entry via hypercall")
>     59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
>     5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
>     89329c0be8bd ("KVM: PPC: Book3S HV: Clear partition table entry on vm teardown")
>     8e3f5fc1045d ("KVM: PPC: Book3S HV: Framework and hcall stubs for nested virtualization")
>     a812297c4fd9 ("KVM: x86: hyperv: optimize 'all cpus' case in kvm_hv_flush_tlb()")
>     aa069a996951 ("KVM: PPC: Book3S HV: Add a VM capability to enable nested virtualization")
>     e6b6c483ebe9 ("KVM: x86: hyperv: fix 'tlb_lush' typo")
>     f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")
> 
> v4.14.193: Failed to apply! Possible dependencies:
>     0234bf885236 ("KVM: x86: introduce ISA specific SMM entry/exit callbacks")
>     05cade71cf3b ("KVM: nSVM: fix SMI injection in guest mode")
>     44883f01fe6a ("KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd")
>     59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
>     5acc5c063196 ("KVM: Introduce KVM_MEMORY_ENCRYPT_OP ioctl")
>     5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
>     69eaedee411c ("KVM: Introduce KVM_MEMORY_ENCRYPT_{UN,}REG_REGION ioctl")
>     72d7b374b14d ("KVM: x86: introduce ISA specific smi_allowed callback")
>     a2e164e7f45a ("x86/kvm/hyper-v: add reenlightenment MSRs support")
>     cc3d967f7e32 ("KVM: SVM: detect opening of SMI window using STGI intercept")
>     f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")
>     faeb7833eee0 ("kvm: x86: hyperv: guest->host event signaling via eventfd")
> 
> v4.9.232: Failed to apply! Possible dependencies:
>     004172bdad64 ("sched/core: Remove unnecessary #include headers")
>     174cd4b1e5fb ("sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>")
>     3aa53859d23e ("KVM: Documentation: document MCE ioctls")
>     3ca0ff571b09 ("locking/mutex: Rework mutex::owner")
>     4036e3874a1c ("KVM: s390: ioctls to get and set guest storage attributes")
>     44883f01fe6a ("KVM: x86: ensure all MSRs can always be KVM_GET/SET_MSR'd")
>     59508b303e4e ("KVM: X86: Move irrelevant declarations out of ioapic.h")
>     5acc5c063196 ("KVM: Introduce KVM_MEMORY_ENCRYPT_OP ioctl")
>     5c69d5c113f1 ("KVM: X86: Fix callers of kvm_apic_match_dest() to use correct macros")
>     69eaedee411c ("KVM: Introduce KVM_MEMORY_ENCRYPT_{UN,}REG_REGION ioctl")
>     a2e164e7f45a ("x86/kvm/hyper-v: add reenlightenment MSRs support")
>     ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
>     b3c045d33218 ("KVM: lapic: remove unnecessary KVM_REQ_EVENT on PPR update")
>     c92701322711 ("KVM: PPC: Book3S HV: Add userspace interfaces for POWER9 MMU")
>     ea8b1c4a6019 ("drivers: psci: PSCI checker module")
>     eb90f3417a0c ("KVM: vmx: speed up TPR below threshold vmexits")
>     ef1ead0c3b1d ("KVM: PPC: Book3S HV: HPT resizing documentation and reserved numbers")
>     f21dd494506a ("KVM: x86: hyperv: optimize sparse VP set processing")
>     faeb7833eee0 ("kvm: x86: hyperv: guest->host event signaling via eventfd")
> 
> v4.4.232: Failed to apply! Possible dependencies:
>     1e6e2755b635 ("KVM: x86: Misc LAPIC changes to expose helper functions")
>     520040146a0a ("KVM: x86: Use vector-hashing to deliver lowest-priority interrupts")
>     5c919412fe61 ("kvm/x86: Hyper-V synthetic interrupt controller")
>     6308630bd3db ("kvm/x86: split ioapic-handled and EOI exit bitmaps")
>     b3c045d33218 ("KVM: lapic: remove unnecessary KVM_REQ_EVENT on PPR update")
>     d62caabb41f3 ("kvm/x86: per-vcpu apicv deactivation support")
> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI
  2020-09-23 14:44   ` Paolo Bonzini
@ 2020-09-23 15:49     ` Sean Christopherson
  0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2020-09-23 15:49 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Sasha Levin, stable, Liran Alon

On Wed, Sep 23, 2020 at 04:44:01PM +0200, Paolo Bonzini wrote:
> Queued, thanks.
> 
> I cannot think of a "nicer" way to do this, we could perhaps move
> 
> +		vmx->nested.pi_pending = true;
> +		kvm_make_request(KVM_REQ_EVENT, vcpu);
> +		kvm_apic_clear_irr(vcpu, vmx->nested.posted_intr_nv);
> 
> to a separate function (possibly with the IRR clear made conditional, so
> that we can reuse the function for regular posted interrupt injection)
> but that is it.

Ya, I played around with similar approaches and didn't particular like any
of them :-/

For the record, I suspect there may be additional issues with a doubly nested
scenario, i.e. when running L3 and L2 is using the self-IPI method for
triggering posted interrupts.  I sort of tested once, and it appeared to be
broken, but it's entirely possible that there was an issue somewhere else in
my stack (L0, L1 and L2 all had non-trivial KVM changes), and I haven't yet
had time to dig in.  That, and I suspect I'm the only person that would care
about L3 functioning properly in this scenario :-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI
  2020-08-12 17:51 [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI Sean Christopherson
  2020-08-19 23:56 ` Sasha Levin
  2020-08-26 13:54 ` Sasha Levin
@ 2020-10-06 17:36 ` Jim Mattson
  2020-10-06 18:35   ` Sean Christopherson
  2 siblings, 1 reply; 8+ messages in thread
From: Jim Mattson @ 2020-10-06 17:36 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel,
	kvm list, LKML, Liran Alon

On Wed, Aug 12, 2020 at 10:51 AM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On successful nested VM-Enter, check for pending interrupts and convert
> the highest priority interrupt to a pending posted interrupt if it
> matches L2's notification vector.  If the vCPU receives a notification
> interrupt before nested VM-Enter (assuming L1 disables IRQs before doing
> VM-Enter), the pending interrupt (for L1) should be recognized and
> processed as a posted interrupt when interrupts become unblocked after
> VM-Enter to L2.
>
> This fixes a bug where L1/L2 will get stuck in an infinite loop if L1 is
> trying to inject an interrupt into L2 by setting the appropriate bit in
> L2's PIR and sending a self-IPI prior to VM-Enter (as opposed to KVM's
> method of manually moving the vector from PIR->vIRR/RVI).  KVM will
> observe the IPI while the vCPU is in L1 context and so won't immediately
> morph it to a posted interrupt for L2.  The pending interrupt will be
> seen by vmx_check_nested_events(), cause KVM to force an immediate exit
> after nested VM-Enter, and eventually be reflected to L1 as a VM-Exit.
> After handling the VM-Exit, L1 will see that L2 has a pending interrupt
> in PIR, send another IPI, and repeat until L2 is killed.
>
> Note, posted interrupts require virtual interrupt deliveriy, and virtual
> interrupt delivery requires exit-on-interrupt, ergo interrupts will be
> unconditionally unmasked on VM-Enter if posted interrupts are enabled.
>
> Fixes: 705699a13994 ("KVM: nVMX: Enable nested posted interrupt processing")
> Cc: stable@vger.kernel.org
> Cc: Liran Alon <liran.alon@oracle.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
I don't think this is the best fix.

I believe the real problem is the way that external and posted
interrupts are handled in vmx_check_nested_events().

First of all, I believe that the existing call to
vmx_complete_nested_posted_interrupt() at the end of
vmx_check_nested_events() is far too aggressive. Unless I am missing
something in the SDM, posted interrupt processing is *only* triggered
when the notification vector is received in VMX non-root mode. It is
not triggered on VM-entry.

Looking back one block, we have:

if (kvm_cpu_has_interrupt(vcpu) && !vmx_interrupt_blocked(vcpu)) {
    if (block_nested_events)
        return -EBUSY;
    if (!nested_exit_on_intr(vcpu))
        goto no_vmexit;
    nested_vmx_vmexit(vcpu, EXIT_REASON_EXTERNAL_INTERRUPT, 0, 0);
    return 0;
}

If nested_exit_on_intr() is true, we should first check to see if
"acknowledge interrupt on exit" is set. If so, we should acknowledge
the interrupt right here, with a call to kvm_cpu_get_interrupt(),
rather than deep in the guts of nested_vmx_vmexit(). If the vector we
get is the notification vector from VMCS12, then we should call
vmx_complete_nested_posted_interrupt(). Otherwise, we should call
nested_vmx_vmexit(EXIT_REASON_EXTERNAL_INTERRUPT) as we do now.

Furthermore, vmx_complete_nested_posted_interrupt() should write to
the L1 EOI register, as indicated in step 4 of the 7-step sequence
detailed in section 29.6 of the SDM, volume 3. It skips this step
today.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI
  2020-10-06 17:36 ` Jim Mattson
@ 2020-10-06 18:35   ` Sean Christopherson
  2020-10-06 21:22     ` Oliver Upton
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2020-10-06 18:35 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel,
	kvm list, LKML, Liran Alon

On Tue, Oct 06, 2020 at 10:36:09AM -0700, Jim Mattson wrote:
> On Wed, Aug 12, 2020 at 10:51 AM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > On successful nested VM-Enter, check for pending interrupts and convert
> > the highest priority interrupt to a pending posted interrupt if it
> > matches L2's notification vector.  If the vCPU receives a notification
> > interrupt before nested VM-Enter (assuming L1 disables IRQs before doing
> > VM-Enter), the pending interrupt (for L1) should be recognized and
> > processed as a posted interrupt when interrupts become unblocked after
> > VM-Enter to L2.
> >
> > This fixes a bug where L1/L2 will get stuck in an infinite loop if L1 is
> > trying to inject an interrupt into L2 by setting the appropriate bit in
> > L2's PIR and sending a self-IPI prior to VM-Enter (as opposed to KVM's
> > method of manually moving the vector from PIR->vIRR/RVI).  KVM will
> > observe the IPI while the vCPU is in L1 context and so won't immediately
> > morph it to a posted interrupt for L2.  The pending interrupt will be
> > seen by vmx_check_nested_events(), cause KVM to force an immediate exit
> > after nested VM-Enter, and eventually be reflected to L1 as a VM-Exit.
> > After handling the VM-Exit, L1 will see that L2 has a pending interrupt
> > in PIR, send another IPI, and repeat until L2 is killed.
> >
> > Note, posted interrupts require virtual interrupt deliveriy, and virtual
> > interrupt delivery requires exit-on-interrupt, ergo interrupts will be
> > unconditionally unmasked on VM-Enter if posted interrupts are enabled.
> >
> > Fixes: 705699a13994 ("KVM: nVMX: Enable nested posted interrupt processing")
> > Cc: stable@vger.kernel.org
> > Cc: Liran Alon <liran.alon@oracle.com>
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> I don't think this is the best fix.

I agree, even without any more explanantion :-)

> I believe the real problem is the way that external and posted
> interrupts are handled in vmx_check_nested_events().
> 
> First of all, I believe that the existing call to
> vmx_complete_nested_posted_interrupt() at the end of
> vmx_check_nested_events() is far too aggressive. Unless I am missing
> something in the SDM, posted interrupt processing is *only* triggered
> when the notification vector is received in VMX non-root mode. It is
> not triggered on VM-entry.

That's my understanding as well.  Virtual interrupt delivery is evaluated
on VM-Enter, but not posted interrupts.

  Evaluation of pending virtual interrupts is caused only by VM entry, TPR
  virtualization, EOI virtualization, self-IPI virtualization, and posted-
  interrupt processing. 

> Looking back one block, we have:
> 
> if (kvm_cpu_has_interrupt(vcpu) && !vmx_interrupt_blocked(vcpu)) {
>     if (block_nested_events)
>         return -EBUSY;
>     if (!nested_exit_on_intr(vcpu))
>         goto no_vmexit;
>     nested_vmx_vmexit(vcpu, EXIT_REASON_EXTERNAL_INTERRUPT, 0, 0);
>     return 0;
> }
> 
> If nested_exit_on_intr() is true, we should first check to see if
> "acknowledge interrupt on exit" is set. If so, we should acknowledge
> the interrupt right here, with a call to kvm_cpu_get_interrupt(),
> rather than deep in the guts of nested_vmx_vmexit(). If the vector we
> get is the notification vector from VMCS12, then we should call
> vmx_complete_nested_posted_interrupt(). Otherwise, we should call
> nested_vmx_vmexit(EXIT_REASON_EXTERNAL_INTERRUPT) as we do now.

That makes sense.  And we can pass in exit_intr_info instead of computing
it in nested_vmx_vmexit() since this is the only path that does a nested
exit with EXIT_REASON_EXTERNAL_INTERRUPT.

> Furthermore, vmx_complete_nested_posted_interrupt() should write to
> the L1 EOI register, as indicated in step 4 of the 7-step sequence
> detailed in section 29.6 of the SDM, volume 3. It skips this step
> today.

Yar.

Thanks Jim!  I'll get a series out.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI
  2020-10-06 18:35   ` Sean Christopherson
@ 2020-10-06 21:22     ` Oliver Upton
  0 siblings, 0 replies; 8+ messages in thread
From: Oliver Upton @ 2020-10-06 21:22 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Jim Mattson, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li,
	Joerg Roedel, kvm list, LKML, Liran Alon

On Tue, Oct 6, 2020 at 11:35 AM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Tue, Oct 06, 2020 at 10:36:09AM -0700, Jim Mattson wrote:
> > On Wed, Aug 12, 2020 at 10:51 AM Sean Christopherson
> > <sean.j.christopherson@intel.com> wrote:
> > >
> > > On successful nested VM-Enter, check for pending interrupts and convert
> > > the highest priority interrupt to a pending posted interrupt if it
> > > matches L2's notification vector.  If the vCPU receives a notification
> > > interrupt before nested VM-Enter (assuming L1 disables IRQs before doing
> > > VM-Enter), the pending interrupt (for L1) should be recognized and
> > > processed as a posted interrupt when interrupts become unblocked after
> > > VM-Enter to L2.
> > >
> > > This fixes a bug where L1/L2 will get stuck in an infinite loop if L1 is
> > > trying to inject an interrupt into L2 by setting the appropriate bit in
> > > L2's PIR and sending a self-IPI prior to VM-Enter (as opposed to KVM's
> > > method of manually moving the vector from PIR->vIRR/RVI).  KVM will
> > > observe the IPI while the vCPU is in L1 context and so won't immediately
> > > morph it to a posted interrupt for L2.  The pending interrupt will be
> > > seen by vmx_check_nested_events(), cause KVM to force an immediate exit
> > > after nested VM-Enter, and eventually be reflected to L1 as a VM-Exit.
> > > After handling the VM-Exit, L1 will see that L2 has a pending interrupt
> > > in PIR, send another IPI, and repeat until L2 is killed.
> > >
> > > Note, posted interrupts require virtual interrupt deliveriy, and virtual
> > > interrupt delivery requires exit-on-interrupt, ergo interrupts will be
> > > unconditionally unmasked on VM-Enter if posted interrupts are enabled.
> > >
> > > Fixes: 705699a13994 ("KVM: nVMX: Enable nested posted interrupt processing")
> > > Cc: stable@vger.kernel.org
> > > Cc: Liran Alon <liran.alon@oracle.com>
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > > ---
> > I don't think this is the best fix.
>
> I agree, even without any more explanantion :-)
>
> > I believe the real problem is the way that external and posted
> > interrupts are handled in vmx_check_nested_events().
> >
> > First of all, I believe that the existing call to
> > vmx_complete_nested_posted_interrupt() at the end of
> > vmx_check_nested_events() is far too aggressive. Unless I am missing
> > something in the SDM, posted interrupt processing is *only* triggered
> > when the notification vector is received in VMX non-root mode. It is
> > not triggered on VM-entry.
>
> That's my understanding as well.  Virtual interrupt delivery is evaluated
> on VM-Enter, but not posted interrupts.
>
>   Evaluation of pending virtual interrupts is caused only by VM entry, TPR
>   virtualization, EOI virtualization, self-IPI virtualization, and posted-
>   interrupt processing.
>
> > Looking back one block, we have:
> >
> > if (kvm_cpu_has_interrupt(vcpu) && !vmx_interrupt_blocked(vcpu)) {
> >     if (block_nested_events)
> >         return -EBUSY;
> >     if (!nested_exit_on_intr(vcpu))
> >         goto no_vmexit;
> >     nested_vmx_vmexit(vcpu, EXIT_REASON_EXTERNAL_INTERRUPT, 0, 0);
> >     return 0;
> > }
> >
> > If nested_exit_on_intr() is true, we should first check to see if
> > "acknowledge interrupt on exit" is set. If so, we should acknowledge
> > the interrupt right here, with a call to kvm_cpu_get_interrupt(),
> > rather than deep in the guts of nested_vmx_vmexit(). If the vector we
> > get is the notification vector from VMCS12, then we should call
> > vmx_complete_nested_posted_interrupt(). Otherwise, we should call
> > nested_vmx_vmexit(EXIT_REASON_EXTERNAL_INTERRUPT) as we do now.
>
> That makes sense.  And we can pass in exit_intr_info instead of computing
> it in nested_vmx_vmexit() since this is the only path that does a nested
> exit with EXIT_REASON_EXTERNAL_INTERRUPT.
>
> > Furthermore, vmx_complete_nested_posted_interrupt() should write to
> > the L1 EOI register, as indicated in step 4 of the 7-step sequence
> > detailed in section 29.6 of the SDM, volume 3. It skips this step
> > today.
>
> Yar.
>
> Thanks Jim!  I'll get a series out.

Hey Sean,

I actually ran into this issue as well before noticing your patch. I
have a repro kvm-unit-test that I'll send out shortly.

Thanks for looking into this!

--
Oliver

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-06 21:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-12 17:51 [PATCH] KVM: nVMX: Morph notification vector IRQ on nested VM-Enter to pending PI Sean Christopherson
2020-08-19 23:56 ` Sasha Levin
2020-08-26 13:54 ` Sasha Levin
2020-09-23 14:44   ` Paolo Bonzini
2020-09-23 15:49     ` Sean Christopherson
2020-10-06 17:36 ` Jim Mattson
2020-10-06 18:35   ` Sean Christopherson
2020-10-06 21:22     ` Oliver Upton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.