Re: [PATCH v3 04/11] x86: Introduce generic protected guest abstraction

[Tom Lendacky]

On 6/28/21 12:52 PM, Tom Lendacky wrote:
> On 6/18/21 5:57 PM, Kuppuswamy Sathyanarayanan wrote:
>> +
>> +static inline bool prot_guest_has(unsigned long flag)
>> +{
>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
>> +		return tdx_protected_guest_has(flag);
>> +	else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>> +		return sev_protected_guest_has(flag);
> 
> So as I think about this, I don't think this will work if the hypervisor
> decides to change the vendor name, right?
> 
> And doesn't TDX supply "IntelTDX    " as a signature. I don't see where
> the signature is used to set the CPU vendor to X86_VENDOR_INTEL.
> 
> The current SEV checks to set sev_status, which is used by sme_active(),
> sev_active, etc.) are based on the max leaf and CPUID bits, but not a
> CPUID vendor check.
> 
> So maybe we can keep the prot_guest_has() but I think it will have to be a
> common routine, with a "switch" statement that has supporting case element
> that check for "sev_active() || static_cpu_has(X86_FEATURE_TDX_GUEST)", etc.
> 

Or keep the separate vendor routines for separation and easier testing
but, instead, they would have to key off of the support:

	if (static_cpu_has(X86_FEATURE_TDX_GUEST))
		return tdx_prot_guest_has(flag);
	else if (sme_active() || sev_active())
		return sev_prot_guest_has(flag);

Thanks,
Tom