Re: [PATCH 5.5 111/176] KVM: nVMX: Emulate MTF when performing instruction emulation

Greg Kroah-Hartman
2020-03-04T08:10:07Z

On Tue, Mar 03, 2020 at 11:39:35PM -0800, Oliver Upton wrote:
> On Tue, Mar 3, 2020 at 11:23 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 03/03/20 18:42, Greg Kroah-Hartman wrote:
> > > From: Oliver Upton <oupton@google.com>
> > >
> > > commit 5ef8acbdd687c9d72582e2c05c0b9756efb37863 upstream.
> > >
> > > Since commit 5f3d45e7f282 ("kvm/x86: add support for
> > > MONITOR_TRAP_FLAG"), KVM has allowed an L1 guest to use the monitor trap
> > > flag processor-based execution control for its L2 guest. KVM simply
> > > forwards any MTF VM-exits to the L1 guest, which works for normal
> > > instruction execution.
> > >
> > > However, when KVM needs to emulate an instruction on the behalf of an L2
> > > guest, the monitor trap flag is not emulated. Add the necessary logic to
> > > kvm_skip_emulated_instruction() to synthesize an MTF VM-exit to L1 upon
> > > instruction emulation for L2.
> > >
> > > Fixes: 5f3d45e7f282 ("kvm/x86: add support for MONITOR_TRAP_FLAG")
> > > Signed-off-by: Oliver Upton <oupton@google.com>
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > Why is this included in a stable release? It was part of a series of
> > four patches and the prerequisites as far as I can see are not part of 5.5.
>
> It looks like these commits were already picked from upstream:
>
> 684c0422da71 ("KVM: nVMX: Handle pending #DB when injecting INIT VM-exit")
> 307f1cfa2696 ("KVM: x86: Mask off reserved bit from #DB exception payload")
>
> Which were bug fixes in their own right and were sensible for
> backporting (though I didn't cc stable if I'm not mistaken).
>
> but not:
>
> a06230b62b89 ("KVM: x86: Deliver exception payload on KVM_GET_VCPU_EVENTS")
>
> which this patch absolutely depends on.
>
> Otherwise, I'll defer discussions regarding the suitability of this
> patch for stable to Paolo.

I picked this patch up solely because of the Fixes: tag showed that this
ommit resolved something from a previous commit. The interdependancies
were not obvious, especially as it all seemed to build just fine here.

> > I have already said half a dozen times that I don't want any of the
> > autopick stuff for KVM. Is a Fixes tag sufficient to get patches into
> > stable now?

Yes, it can happen that a Fixes tag does cause a patch to get into
stable because I look out for that. I do that because a number of
maintainers/developers only put that tag in there, and also to catch
patches for when we backport stuff and then need to take a fix for that
backport (not the case here though).

I'll be glad to just put KVM into the "never apply any patches to
stable unless you explicitly mark it as such", but the sad fact is that
many recent KVM fixes for reported CVEs never had any "Cc: stable@vger"
markings. They only had "Fixes:" tags and so I have had to dig them out
of the tree and backport them myself in order to resolve those very
public issues.

So can I ask that you always properly tag things for stable? If so, I
will be glad to ignore Fixes: tags for KVM patches in the future.

I'll go drop this patch as well. Note, there are other KVM patches in
this release cycle also, can someone verify that I did not overreach for
them as well?

thanks,

greg k-h