Messages in this thread | | | Subject | Re: [PATCH v8 03/11] x86/cpufeatures: Add TDX Guest CPU feature | From | "Kuppuswamy, Sathyanarayanan" <> | Date | Tue, 5 Oct 2021 14:41:35 -0700 |
| |
On 10/5/21 2:04 PM, Josh Poimboeuf wrote: > On Mon, Oct 04, 2021 at 07:51:57PM -0700, Kuppuswamy Sathyanarayanan wrote: >> @@ -495,6 +496,13 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data) >> >> copy_bootdata(__va(real_mode_data)); >> >> + /* >> + * tdx_early_init() has dependency on command line parameters. >> + * So the order of calling it should be after copy_bootdata() >> + * (in which command line parameter is initialized). >> + */ >> + tdx_early_init(); > > Which cmdline parameters are those?
We have few debug command line options like tdx_forced (force TDX initialization) and tdx_disable_filter (Disables TDX device filter support). Support for these options have not been posted out (waiting to merge the initial support patches first). Since we need to access command line options, we want to follow the above calling order.
> >> +/* >> + * Allocate it in the data region to avoid zeroing it during >> + * BSS initialization. It is mainly used in cc_platform_has() >> + * call during early boot call. >> + */ >> +u64 __section(".data") is_tdx_guest = 0; > > Or you could just give it a -1 value here to avoid the section > annotation. Not sure why it needs 64 bits, any reason it can't just be > bool?
It can be bool. I can fix this in next version.
> >> + >> +static void __init is_tdx_guest_init(void) >> +{ >> + u32 eax, sig[3]; >> + >> + if (cpuid_eax(0) < TDX_CPUID_LEAF_ID) { >> + is_tdx_guest = 0; >> + return; >> + } >> + >> + cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax, &sig[0], &sig[2], &sig[1]); >> + >> + is_tdx_guest = !memcmp("IntelTDX ", sig, 12); >> +} >> + >> +void __init tdx_early_init(void) >> +{ >> + is_tdx_guest_init(); >> + >> + if (!is_tdx_guest) >> + return; >> + >> + setup_force_cpu_cap(X86_FEATURE_TDX_GUEST); >> + >> + pr_info("Guest initialized\n"); >> +} > > What's the point of having both 'is_tdx_guest' and > X86_FEATURE_TDX_GUEST? Are they not redundant?
is_tdx_guest was mainly introduced to support cc_platform_has() API in early boot calls (similar to sme_me_mask in AMD code). Regarding FEATURE flag it will be useful for userspace tools to check the TDX feature support.
>
-- Sathyanarayanan Kuppuswamy Linux Kernel Developer
| |