Re: [PATCH 18/25] KVM: TDX: Do TDX specific vcpu initialization

[Adrian Hunter]

On 13/08/24 11:00, Yuan Yao wrote:
> On Mon, Aug 12, 2024 at 03:48:13PM -0700, Rick Edgecombe wrote:
>> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>>
>> TD guest vcpu needs TDX specific initialization before running.  Repurpose
>> KVM_MEMORY_ENCRYPT_OP to vcpu-scope, add a new sub-command
>> KVM_TDX_INIT_VCPU, and implement the callback for it.
>>
>> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
>> Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
>> ---

<SNIP>

>> @@ -884,6 +930,149 @@ int tdx_vm_ioctl(struct kvm *kvm, void __user *argp)
>>  	return r;
>>  }
>>
>> +/* VMM can pass one 64bit auxiliary data to vcpu via RCX for guest BIOS. */
>> +static int tdx_td_vcpu_init(struct kvm_vcpu *vcpu, u64 vcpu_rcx)
>> +{

<SNIP>

>> +	if (modinfo->tdx_features0 & MD_FIELD_ID_FEATURES0_TOPOLOGY_ENUM)
>> +		err = tdh_vp_init_apicid(tdx, vcpu_rcx, vcpu->vcpu_id);
>> +	else
>> +		err = tdh_vp_init(tdx, vcpu_rcx);
> 
> This can cause incorrect topology information to guest
> silently:
> 
> A user space VMM uses "-smp 8,threads=4,cores=2" but doesn't
> pass any 0x1f leaf data to KVM, means no 0x1f value to TDX
> module for this TD. The topology TD guest observed is:
> 
> Thread(s) per core:                 2
> Core(s) per socket:                 4
> 
> I suggest to use tdh_vp_init_apicid() only when 0x1f is
> valid. This will disable the 0x1f/0xb topology feature per
> the spec, but leaf 0x1/0x4 still are available to present
> right topology in this example. It presents correct topology
> information to guest if user space VMM doesn't use 0x1f for
> simple topology and run on TDX module w/ FEATURES0_TOPOLOGY.

tdh_vp_init_apicid() passes x2APIC ID to TDH.VP.INIT which
is one of the steps for the TDX Module to support topology
information for the guest i.e. CPUID leaf 0xB and CPUID leaf 0x1F.

If the host VMM does not provide CPUID leaf 0x1F values
(i.e. the values are 0), then the TDX Module will use native
values for both CPUID leaf 0x1F and CPUID leaf 0xB.

To get 0x1F/0xB the guest must also opt-in by setting
TDCS.TD_CTLS.ENUM_TOPOLOGY to 1.  AFAICT currently Linux
does not do that.

In the tdh_vp_init() case, topology information will not be
supported.

If topology information is not supported CPUID leaf 0xB and
CPUID leaf 0x1F will #VE, and a Linux guest will return zeros.

So, yes, it seems like tdh_vp_init_apicid() should only
be called if there is non-zero CPUID leaf 0x1F values provided
by host VMM. e.g. add a helper function

bool tdx_td_enum_topology(struct kvm_cpuid2 *cpuid)
{
	const struct tdx_sys_info_features *modinfo = &tdx_sysinfo->features;
	const struct kvm_cpuid_entry2 *entry;

	if (!(modinfo->tdx_features0 & MD_FIELD_ID_FEATURES0_TOPOLOGY_ENUM))
		return false;

	entry = kvm_find_cpuid_entry2(cpuid->entries, cpuid->nent, 0x1f, 0);
	if (!entry)
		return false;

	return entry->eax || entry->ebx || entry->ecx || entry->edx;
}