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.