Messages in this thread | | | Subject | Re: [PATCH v10 03/11] x86/cpufeatures: Add TDX Guest CPU feature | From | Sathyanarayanan Kuppuswamy <> | Date | Wed, 13 Oct 2021 14:05:42 -0700 |
| |
On 10/13/21 1:44 PM, Thomas Gleixner wrote: > On Fri, Oct 08 2021 at 22:37, Kuppuswamy Sathyanarayanan wrote: >> + >> +bool is_tdx_guest(void) >> +{ >> + static int tdx_guest = -1; >> + u32 eax, sig[3]; >> + >> + if (tdx_guest >= 0) >> + goto done; >> + >> + if (cpuid_eax(0) < TDX_CPUID_LEAF_ID) { >> + tdx_guest = 0; >> + goto done; >> + } >> + >> + cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax, &sig[0], &sig[2], &sig[1]); >> + >> + tdx_guest = !memcmp("IntelTDX ", sig, 12); >> + >> +done: >> + return !!tdx_guest; >> +} > No. This is tasteless garbage, really. > > tdx_early_init() is invoked from x86_64_start_kernel() very early in the > boot process __before__ is_tdx_guest() is invoked. > > So why on earth is it requried to keep those conditionals and cpuid() > muck around after init?
is_tdx_guest() is also used by cc_platform_has() API. Please check the following patch [1]. cc_platform_has() will be called much earlier than x86_64_start_kernel() (like __startup_64() [2]).
[1] - https://lore.kernel.org/lkml/20211009053747.1694419-5-sathyanarayanan.kuppuswamy@linux.intel.com/ [2] - https://lore.kernel.org/lkml/46a18427dc4e9dda985b10e472965e3e4c769f1d.1631141919.git.thomas.lendacky@amd.com/
> >> +void __init tdx_early_init(void) >> +{ >> + if (!is_tdx_guest()) >> + return; >> + >> + setup_force_cpu_cap(X86_FEATURE_TDX_GUEST); >> + >> + pr_info("Guest initialized\n"); >> +} > What's wrong with: > > static bool tdx_guest_detected __ro_after_init; > > void __init tdx_early_init(void) > { > u32 eax, sig[3]; > > if (cpuid_eax(0) < TDX_CPUID_LEAF_ID) > return; > > cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax, &sig[0], &sig[2], &sig[1]); > > if (memcmp("IntelTDX ", sig, 12)) > return; > > tdx_guest_detected = true; > setup_force_cpu_cap(X86_FEATURE_TDX_GUEST); > > pr_info("Guest initialized\n"); > } > > bool is_tdx_guest(void) > { > return tdx_guest_detected; > } > > That's simple and obvious and discards all the detection gunk completely > after init and uses the proper data type, right? > > Thanks, > > tglx
-- Sathyanarayanan Kuppuswamy Linux Kernel Developer
| |