[PATCH] KVM: x86: Exempt pending triple fault from event injection sanity check

[PATCH] KVM: x86: Exempt pending triple fault from event injection sanity check

[Sean Christopherson]

Exempt pending triple faults, a.k.a. KVM_REQ_TRIPLE_FAULT, when asserting
that KVM didn't attempt to queue a new exception during event injection.
KVM needs to emulate the injection itself when emulating Real Mode due to
lack of unrestricted guest support (VMX) and will queue a triple fault if
that emulation fails.

Ideally the assertion would more precisely filter out the emulated Real
Mode triple fault case, but rmode.vm86_active is buried in vcpu_vmx and
can't be queried without a new kvm_x86_ops.  And unlike "regular"
exceptions, triple fault cannot put the vCPU into an infinite loop; the
triple fault will force either an exit to userspace or a nested VM-Exit,
and triple fault after nested VM-Exit will force an exit to userspace.
I.e. there is no functional issue, so just suppress the warning for
triple faults.

Opportunistically convert the warning to a one-time thing, when it
fires, it fires _a lot_, and is usually user triggerable, i.e. can be
used to spam the kernel log.

Fixes: 7055fb113116 ("KVM: x86: Treat pending TRIPLE_FAULT requests as pending exceptions")
Reported-by: kernel test robot <yujie.liu@intel.com>
Link: https://lore.kernel.org/r/202209301338.aca913c3-yujie.liu@intel.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/x86.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index eb9d2c23fb04..20497685e6d1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9972,7 +9972,20 @@ static int kvm_check_and_inject_events(struct kvm_vcpu *vcpu,
 	    kvm_x86_ops.nested_ops->has_events(vcpu))
 		*req_immediate_exit = true;
 
-	WARN_ON(kvm_is_exception_pending(vcpu));
+	/*
+	 * KVM must never queue a new exception while injecting an event; KVM
+	 * is done emulating and should only propagate the to-be-injected event
+	 * to the VMCS/VMCB.  Queueing a new exception can put the vCPU into an
+	 * infinite loop as KVM will bail from VM-Enter to inject the pending
+	 * exception and start the cycle all over.
+	 *
+	 * Exempt triple faults as they have special handling and won't put the
+	 * vCPU into an infinite loop.  Triple fault can be queued when running
+	 * VMX without unrestricted guest, as that requires KVM to emulate Real
+	 * Mode events (see kvm_inject_realmode_interrupt()).
+	 */
+	WARN_ON_ONCE(vcpu->arch.exception.pending ||
+		     vcpu->arch.exception_vmexit.pending);
 	return 0;
 
 out:

base-commit: c59fb127583869350256656b7ed848c398bef879
-- 
2.38.0.rc1.362.ged0d419d3c-goog

Re: [PATCH] KVM: x86: Exempt pending triple fault from event injection sanity check

[Yujie Liu]

Hi Sean and Paolo,

Thanks for posting this fix patch to address the issue we reported.

We noticed that commit 7055fb113116 has been merged to mainline, but seems
this patch is still under review, so the mmio_warning_test kvm selftests
still fails on the current head of mainline. Could you please help to
update the status that whether this patch will be applied? Thanks.

We tested this patch on the head of mainline, and confirmed it does fix
the problem:

commit:
   a970174d7a101 ("x86/mm: Do not verify W^X at boot up")
   1a98b7142ef26 ("KVM: x86: Exempt pending triple fault from event injection sanity check")

a970174d7a1010cb 1a98b7142ef2695ee3d522ab1de
---------------- ---------------------------
        fail:runs  %reproduction    fail:runs
            |             |             |
           3:3         -100%            :3     dmesg.RIP:kvm_check_and_inject_events[kvm]
           3:3         -100%            :3     dmesg.WARNING:at_arch/x86/kvm/x86.c:#kvm_check_and_inject_events[kvm]
           3:3         -100%            :3     kernel-selftests.kvm.mmio_warning_test.fail

--
Best Regards,
Yujie

On 10/1/2022 07:00, Sean Christopherson wrote:
> Exempt pending triple faults, a.k.a. KVM_REQ_TRIPLE_FAULT, when asserting
> that KVM didn't attempt to queue a new exception during event injection.
> KVM needs to emulate the injection itself when emulating Real Mode due to
> lack of unrestricted guest support (VMX) and will queue a triple fault if
> that emulation fails.
> 
> Ideally the assertion would more precisely filter out the emulated Real
> Mode triple fault case, but rmode.vm86_active is buried in vcpu_vmx and
> can't be queried without a new kvm_x86_ops.  And unlike "regular"
> exceptions, triple fault cannot put the vCPU into an infinite loop; the
> triple fault will force either an exit to userspace or a nested VM-Exit,
> and triple fault after nested VM-Exit will force an exit to userspace.
> I.e. there is no functional issue, so just suppress the warning for
> triple faults.
> 
> Opportunistically convert the warning to a one-time thing, when it
> fires, it fires _a lot_, and is usually user triggerable, i.e. can be
> used to spam the kernel log.
> 
> Fixes: 7055fb113116 ("KVM: x86: Treat pending TRIPLE_FAULT requests as pending exceptions")
> Reported-by: kernel test robot <yujie.liu@intel.com>
> Link: https://lore.kernel.org/r/202209301338.aca913c3-yujie.liu@intel.com
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/x86.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index eb9d2c23fb04..20497685e6d1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -9972,7 +9972,20 @@ static int kvm_check_and_inject_events(struct kvm_vcpu *vcpu,
>   	    kvm_x86_ops.nested_ops->has_events(vcpu))
>   		*req_immediate_exit = true;
>   
> -	WARN_ON(kvm_is_exception_pending(vcpu));
> +	/*
> +	 * KVM must never queue a new exception while injecting an event; KVM
> +	 * is done emulating and should only propagate the to-be-injected event
> +	 * to the VMCS/VMCB.  Queueing a new exception can put the vCPU into an
> +	 * infinite loop as KVM will bail from VM-Enter to inject the pending
> +	 * exception and start the cycle all over.
> +	 *
> +	 * Exempt triple faults as they have special handling and won't put the
> +	 * vCPU into an infinite loop.  Triple fault can be queued when running
> +	 * VMX without unrestricted guest, as that requires KVM to emulate Real
> +	 * Mode events (see kvm_inject_realmode_interrupt()).
> +	 */
> +	WARN_ON_ONCE(vcpu->arch.exception.pending ||
> +		     vcpu->arch.exception_vmexit.pending);
>   	return 0;
>   
>   out:
> 
> base-commit: c59fb127583869350256656b7ed848c398bef879

Re: [PATCH] KVM: x86: Exempt pending triple fault from event injection sanity check

[Sean Christopherson]

Paolo, I hope your bus factor is high, because I'm throwing you under one :-)

On Tue, Oct 25, 2022, Yujie Liu wrote:
> Hi Sean and Paolo,
> 
> Thanks for posting this fix patch to address the issue we reported.
> 
> We noticed that commit 7055fb113116 has been merged to mainline, but seems
> this patch is still under review, so the mmio_warning_test kvm selftests
> still fails on the current head of mainline. Could you please help to
> update the status that whether this patch will be applied? Thanks.

Last I heard, Paolo is planning on handling patches for this cycle (fixes for 6.1
and new features for 6.2), which is why I haven't been queuing anything.  Not sure
what Paolo's excuse is.  ;-)

Re: [PATCH] KVM: x86: Exempt pending triple fault from event injection sanity check

[Paolo Bonzini]

Queued, thanks.

Paolo