On Tue, Dec 18, 2018 at 7:04 AM Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
>
> On Mon, Dec 17, 2018 at 03:14:14PM -0800, Jim Mattson wrote:
> > The virtual VMX preemption timer doesn't behave correctly when the
> > VMCS12 VMX-preemption timer value field is 0 and there is an injected
> > event in the VMCS12. The event should be vectored through the guest
> > IDT before the "VMX-preemption timer expired" VM-exit from L2 to L1 is
> > synthesized by L0, but it is not. Similarly, the virtual VMX
> > preemption timer doesn't behave correctly when the VMCS12
> > VMX-preemption timer value field is 0 and there are pending debug
> > exceptions in the VMCS12. The pending debug exceptions should be
> > delivered before the "VMX-preemption timer expired" VM-exit from L2 to
> > L1 is synthesized by L0, but they are not.
> >
> > The easiest way to fix this is to use the VMX-preemption timer in
> > VMCS02 whenever the VMCS12 VMX-preemption timer value field is 0.
> > Multiplexing with the existing usage of the VMCS02 VMX-preemption
> > timer is straightforward. However, this approach introduces a
> > dependency on the underlying hardware having VMX-preemption timer
> > support. (Even broken VMX-preemption timer support should be
> > sufficient. I know of no VMX preemption-timer errata that would impact
> > the case where the VMX-preemption timer value field is 0.)
> > Unfortunately, commit f4124500c2c13 ("KVM: nVMX: Fully emulate
> > preemption timer") removed the dependency of the virtual
> > VMX-preemption timer on a hardware VMX-preemption timer.
> >
> > I see at least the following three options:
> > 1) Require a hardware VMX-preemption timer before advertising a
> > virtual VMX-preemption timer.
> > 2) Only provide a working virtual VMX-preemption timer when there is a
> > hardware VMX-preemption timer, but continue to advertise the broken
> > VMX-preemption timer on platforms that don't support a hardware
> > VMX-preemption timer.
> > 3) Teach kvm how to do guest IDT-vectoring in software, so that a
> > hardware VMX-preemption timer isn't necessary.
> >
> > Thoughts? Other options?
>
> 4) Move the exception handling out of vmx_check_nested_events() and into
> a separate function, and reorder the flow of inject_pending_event()
> to prioritize VOE. kvm_vcpu_running() also uses .check_nested_events(),
> not sure what needs to be done there.
Unless I'm missing something, this reorganization seems orthogonal to
(1) or (2). That is, even if we fix the code that was causing us to
bypass the launch of vmcs02, how do we get a VM-exit after the event
injection if we don't set up a zero-valued VMX-preemption timer in
vmcs02?
> (1) and (2) seem reasonable.
>
> Anything but (3), far too many corner cases, i.e. you'll spend the next
> few years stabilizing the code :)
Thanks! I'm in complete agreement. I just wanted to put this out there.
> I think (4) is the most correct, e.g. NMI and INTR that VM-Exit have the
> same bug, and it would make the nested code more consistent in how it
> prioritizes events. inject_pending_event() even has a giant comment
> about not injecting NMI/INTR when there's a pending exception.
