Re: [PATCH v8 03/11] x86/cpufeatures: Add TDX Guest CPU feature

["Kuppuswamy, Sathyanarayanan" <sathyanarayanan.kuppuswamy@linux.intel.com>]

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