linux-kernel - [PATCH 0/3] x86/cpu: KVM: Make SGX and VMX depend on FEAT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221203003745.1475584-1-seanjc@google.com>
Date:   Sat,  3 Dec 2022 00:37:42 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Cc:     "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: [PATCH 0/3] x86/cpu: KVM: Make SGX and VMX depend on FEAT_CTL

"Officially" make SGX and VMX depend on X86_FEATURE_MSR_IA32_FEAT_CTL,
and drop manual checks on X86_FEATURE_MSR_IA32_FEAT_CTL when querying
VMX support.

To make dependencies on MSR_IA32_FEAT_CTL work as expected, process all
CPUID dependencies at the end of CPU indentification.  Because
MSR_IA32_FEAT_CTL is a synthetic flag, it is effectively off-by-default,
and thus may never be unset via clear_cpu_cap(), i.e. never triggers
processing of its dependents.

The obvious alternative would be to explicitly clear MSR_IA32_FEAT_CTL if
the MSR is unsupported, but that ends up being rather ugly as it would
require clearing the flag in default_init() to handle the scenario where
hardware supports the MSR, but the kernel was built without support for the
CPU vendor.  E.g. running on an Intel CPU with CPU_SUP_INTEL=n.  This edge
case is also why the existing manual checks in KVM are necessary; KVM_INTEL
effectively depends on any of CPU_SUP_{INTEL,CENATUR,ZHAOXIN}.

Processing all dependencies also seems like the correct thing to do across
the board, e.g. if the kernel ends up with more synthetic features with
dependents.

The placement of the call to apply_cpuid_deps() isn't super scientific.  I
placed it after, AFAICT, the overwhelming majority of cpu cap updates had
already been done, but before anything was likely to want the dependencies
to be processed.  Specifically, I couldn't find any set_cpu_caps() in the
machine check code, but there are definitely cpu_has() calls under
mcheck_cpu_init().

Last thought, patch 3 will conflict with at least one in-flight KVM series[*].
The conflict should be straightfoward to resolve, but at the same time this
is far from urgent, i.e. kicking this series down the road until KVM settles
down is totally ok.

[*] https://lore.kernel.org/all/20221130230934.1014142-1-seanjc@google.com

Sean Christopherson (3):
  x86/cpu: Process all CPUID dependencies after identifying CPU info
  x86/cpu: Mark SGX and VMX as being dependent on MSR_IA32_FEAT_CTL
  KVM: VMX: Drop manual checks on X86_FEATURE_MSR_IA32_FEAT_CTL

 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/kernel/cpu/bugs.c        |  3 +--
 arch/x86/kernel/cpu/common.c      |  6 ++++++
 arch/x86/kernel/cpu/cpuid-deps.c  | 12 ++++++++++++
 arch/x86/kernel/cpu/feat_ctl.c    |  3 +--
 arch/x86/kvm/vmx/vmx.c            |  6 ++----
 6 files changed, 23 insertions(+), 8 deletions(-)

base-commit: d800169041c0e035160c8b81f30d4b7e8f8ef777
-- 
2.39.0.rc0.267.gcb52ba06e7-goog