Re: [RFC PATCH 1/2] kvm/x86: intercept guest changes to X86_CR4_LA57

[Lai Jiangshan]

On Tue, Sep 29, 2020 at 4:32 PM Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
>
> On Tue, Sep 29, 2020 at 01:32:45PM +0800, Lai Jiangshan wrote:
> > On Tue, Sep 29, 2020 at 12:24 AM Sean Christopherson
> > <sean.j.christopherson@xxxxxxxxx> wrote:
> > >
> > > On Mon, Sep 28, 2020 at 04:30:46PM +0800, Lai Jiangshan wrote:
> > > > From: Lai Jiangshan <laijs@xxxxxxxxxxxxxxxxx>
> > > >
> > > > When shadowpaping is enabled, guest should not be allowed
> > > > to toggle X86_CR4_LA57. And X86_CR4_LA57 is a rarely changed
> > > > bit, so we can just intercept all the attempts to toggle it
> > > > no matter shadowpaping is in used or not.
> > > >
> > > > Fixes: fd8cb433734ee ("KVM: MMU: Expose the LA57 feature to VM.")
> > > > Cc: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> > > > Cc: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> > > > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > > > Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxxxxx>
> > > > ---
> > > >   No test to toggle X86_CR4_LA57 in guest since I can't access to
> > > >   any CPU supports it. Maybe it is not a real problem.
> > >
> >
> >
> > Hello
> >
> > Thanks for reviewing.
> >
> > > LA57 doesn't need to be intercepted.  It can't be toggled in 64-bit mode
> > > (causes a #GP), and it's ignored in 32-bit mode.  That means LA57 can only
> > > take effect when 64-bit mode is enabled, at which time KVM will update its
> > > MMU context accordingly.
> > >
> >
> > Oh, I missed that part which is so obvious that the patch
> > seems impertinent.
> >
> > But X86_CR4_LA57 is so fundamental that it makes me afraid to
> > give it over to guests. And it is rarely changed too. At least,
> > there is no better reason to give it to the guest than
> > intercepting it.
> >
> > There might be another reason that this patch is still needed with
> > an updated changelog.
> >
> > When a user (via VMM such as qemu) launches a VM with LA57 disabled
> > in its cpuid on a LA57 enabled host. The hypervisor, IMO, needs to
> > intercept guest's changes to X86_CR4_LA57 even when the guest is still
> > in the non-paging mode. Otherwise the hypervisor failed to detective
> > such combination when the guest changes paging mode later.
> >
> > Anyway, maybe it is still not a real problem.
>
> Oof, the above is a KVM bug, though in a more generic manner.  All reserved
> bits should be intercepted, not just LA57.  LA57 is the only affected bit at
> the moment, but proper support is needed as the follow-on patch to let the
> guest toggle FSGSBASE would introduce the same bug.
>
> Sadly, fixing this is a bit of a mess.  Well, fixing LA57 is easy, e.g. this
> patch will do the trick.  But actually refreshing the CR4 guest/host mask when
> the guest's CPUID is updated is a pain, and that's what's needed for proper
> FSGSBASE support.
>
> I'll send a series, bookended by these two RFC patches, with patches to

Thanks for illustrating deep inside.
I'm looking forward to the series.

> intercept CR4 reserved bits smushed in between.  I agree there's no point in
> letting the guest write LA57 directly, it's almost literally a once-per-boot
> thing.  I wouldn't be surprised if intercepting it is a net win (but still
> inconsequential), e.g. due to the MMU having to grab CR4 out of the VMCS.