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

[Thomas Gleixner]

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?

> +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