Re: [PATCH v6 09/60] i386/tdx: Initialize TDX before creating TD vcpus

Xiaoyao Li
2025-01-14T12:39:45Z

On 12/17/2024 9:10 PM, Tony Lindgren wrote:
> On Thu, Dec 12, 2024 at 11:24:03AM -0600, Ira Weiny wrote:
>> On Wed, Nov 06, 2024 at 07:13:56AM +0200, Tony Lindgren wrote:
>>> On Wed, Nov 06, 2024 at 10:01:04AM +0800, Xiaoyao Li wrote:
>>>> On 11/6/2024 4:51 AM, Edgecombe, Rick P wrote:
>>>>> +Tony
>>>>>
>>>>> On Tue, 2024-11-05 at 01:23 -0500, Xiaoyao Li wrote:
>>>>>> +int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>>>>>> +{
>>>>>> +    X86CPU *x86cpu = X86_CPU(cpu);
>>>>>> +    CPUX86State *env = &x86cpu->env;
>>>>>> +    g_autofree struct kvm_tdx_init_vm *init_vm = NULL;
>>>>>> +    int r = 0;
>>>>>> +
>>>>>> +    QEMU_LOCK_GUARD(&tdx_guest->lock);
>>>>>> +    if (tdx_guest->initialized) {
>>>>>> +        return r;
>>>>>> +    }
>>>>>> +
>>>>>> +    init_vm = g_malloc0(sizeof(struct kvm_tdx_init_vm) +
>>>>>> +                        sizeof(struct kvm_cpuid_entry2) * KVM_MAX_CPUID_ENTRIES);
>>>>>> +
>>>>>> +    r = setup_td_xfam(x86cpu, errp);
>>>>>> +    if (r) {
>>>>>> +        return r;
>>>>>> +    }
>>>>>> +
>>>>>> +    init_vm->cpuid.nent = kvm_x86_build_cpuid(env, init_vm->cpuid.entries, 0);
>>>>>> +    tdx_filter_cpuid(&init_vm->cpuid);
>>>>>> +
>>>>>> +    init_vm->attributes = tdx_guest->attributes;
>>>>>> +    init_vm->xfam = tdx_guest->xfam;
>>>>>> +
>>>>>> +    do {
>>>>>> +        r = tdx_vm_ioctl(KVM_TDX_INIT_VM, 0, init_vm);
>>>>>> +    } while (r == -EAGAIN);
>>>>>
>>>>> KVM_TDX_INIT_VM can also return EBUSY. This should check for it, or KVM should
>>>>> standardize on one for both conditions. In KVM, both cases handle
>>>>> TDX_RND_NO_ENTROPY, but one tries to save some of the initialization for the
>>>>> next attempt. I don't know why userspace would need to differentiate between the
>>>>> two cases though, which makes me think we should just change the KVM side.
>>>>
>>>> I remember I tested retrying on the two cases and no surprise showed.
>>>>
>>>> I agree to change KVM side to return -EAGAIN for the two cases.
>>>
>>> OK yeah let's patch KVM for it.
>>
>> Will the patch to KVM converge such that it is ok for qemu to loop forever?
>
> Hmm I don't think we should loop forever anywhere, the retries needed should
> be only a few. Or what do you have in mind?

"A few" seems not accurate. It depends on how heavy the RDRAND/RDSEED
traffic from others are. IIRC, it gets > 10 0000 -EAGAIN before success
when all the LPs in the system are doing RDRAND/RDSEED.

Maybe a timeout? E.g., QEMU exits when it cannot move forward for a
certain period.

However, I'm not sure what value is reasonable for the timeout.

> Regards,
>
> Tony