On Tue, Oct 22, 2019 at 08:16:22AM -0700, Sean Christopherson wrote: > On Tue, Oct 22, 2019 at 12:51:01PM +0200, Paolo Bonzini wrote: > > On 22/10/19 02:08, Sean Christopherson wrote: > > > Remove the code to initialize IA32_FEATURE_CONTROL MSR when KVM is > > > loaded now that the MSR is initialized during boot on all CPUs that > > > support VMX, i.e. can possibly load kvm_intel. > > > > > > Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx> > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> > > > --- > > > arch/x86/kvm/vmx/vmx.c | 48 +++++++++++++++++------------------------- > > > 1 file changed, 19 insertions(+), 29 deletions(-) > > > > I am still not sure about this... Enabling VMX is adding a possible > > attack vector for the kernel, we should not do it unless we plan to do a > > VMXON. > > An attacker would need arbitrary cpl0 access to toggle CR4.VMXE and do > VMXON (and VMLAUNCH), would an extra WRMSR really slow them down? > > And practically speaking, how often do you encounter systems whose > firmware leaves IA32_FEATURE_CONTROL unlocked? > > > Why is it so important to operate with locked > > IA32_FEATURE_CONTROL (so that KVM can enable VMX and the kernel can > > still enable SGX if desired). > > For simplicity. The alternative that comes to mind is to compute the > desired MSR value and write/lock the MSR on demand, e.g. add a sequence > similar to KVM's hardware_enable_all() for SGX, but that's a fair amount > of complexity for marginal benefit (IMO). > > If a user really doesn't want VMX enabled, they can clear the feature bit > via the clearcpuid kernel param. > > That being said, enabling VMX in IA32_FEATURE_CONTROL if and only if > IS_ENABLED(CONFIG_KVM) is true would be an easy enhancement. Paolo, any follow up thoughts on this approach?
