On 14/11/19 19:32, Sean Christopherson wrote: >>> + pr_err_once("x86/cpu: VMX disabled, IA32_FEATURE_CONTROL MSR unsupported\n"); > > My thought for having the print was to alert the user that something is > royally borked with their system. There's nothing the user can do to fix > it per se, but it does indicate that either their hardware or the VMM > hosting their virtual machine is broken. So maybe be more explicit about > it being a likely hardware/VMM issue? Yes, good idea. >>> +update_caps: >>> + if (!cpu_has(c, X86_FEATURE_VMX)) >>> + return; >> >> If this test is just so we can save us the below code, I'd say remove it >> for the sake of having less code in that function. The test is cheap and >> not on a fast path so who cares if we clear an alrady cleared bit. But >> maybe this evolves in the later patches... > > I didn't want to print the "VMX disabled by BIOS..." message if VMX isn't > supported in the first place. Later patches also add more code in this > flow, but avoiding the print message is the main motiviation. I agree on this too. Paolo >>> + >>> + if ((tboot && !(msr & FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX)) || >>> + (!tboot && !(msr & FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX))) { >>> + pr_err_once("x86/cpu: VMX disabled by BIOS (TXT %s)\n", >>> + tboot ? "enabled" : "disabled"); >>> + clear_cpu_cap(c, X86_FEATURE_VMX); >>> + } >>> } >> >> -- >> Regards/Gruss, >> Boris. >> >> https://people.kernel.org/tglx/notes-about-netiquette >