Re: [PATCH 23/30] KVM: TDX: initialize VM with TDX specific parameters

[Paolo Bonzini <pbonzini@redhat.com>]

On 2/21/25 03:31, Xiaoyao Li wrote:
> On 2/21/2025 1:05 AM, Paolo Bonzini wrote:
> 
> ...
> 
>> @@ -403,8 +412,9 @@ int tdx_vm_init(struct kvm *kvm)
>>        */
>>       kvm->max_vcpus = min_t(int, kvm->max_vcpus, num_present_cpus());
>> -    /* Place holder for TDX specific logic. */
>> -    return __tdx_td_init(kvm);
> 
> it moves __tdx_td_init() from KVM_CRAETE_VM->tdx_vm_init() to ...
> 
>> +    kvm_tdx->state = TD_STATE_UNINITIALIZED;
>> +
>> +    return 0;
>>   }
> 
>> +static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
>> +{

...

>> +    ret = setup_tdparams(kvm, td_params, init_vm);
>> +    if (ret)
>> +        goto out;
>> +
>> +    ret = __tdx_td_init(kvm, td_params, &cmd->hw_error);
> 
> ... KVM_TDX_INIT_VM, which moves the keyid allocation, TDR/TDCS page 
> allocation and other works from KVM_CREATE_VM to KVM_TDX_INIT_VM as well.
> 
> I'm not sure if it is intentional, or by accident. There is no 
> mentioning and justfication of it.

Until this patch there was no KVM_TDX_INIT_VM ioctl, so there was no 
separation between uninitialized and initialized TDs - the TD was 
already initialized right after KVM_CREATE_VM.

I can adjust a bit the commit message.

Paolo

>> +    if (ret)
>> +        goto out;
>> +
>> +    kvm_tdx->tsc_offset = td_tdcs_exec_read64(kvm_tdx, 
>> TD_TDCS_EXEC_TSC_OFFSET);
>> +    kvm_tdx->attributes = td_params->attributes;
>> +    kvm_tdx->xfam = td_params->xfam;
>> +
>> +    kvm_tdx->state = TD_STATE_INITIALIZED;
>> +out:
>> +    /* kfree() accepts NULL. */
>> +    kfree(init_vm);
>> +    kfree(td_params);
>> +
>> +    return ret;
>> +}
>> +
>>   int tdx_vm_ioctl(struct kvm *kvm, void __user *argp)
>>   {
>>       struct kvm_tdx_cmd tdx_cmd;
>> @@ -647,6 +882,9 @@ int tdx_vm_ioctl(struct kvm *kvm, void __user *argp)
>>       case KVM_TDX_CAPABILITIES:
>>           r = tdx_get_capabilities(&tdx_cmd);
>>           break;
>> +    case KVM_TDX_INIT_VM:
>> +        r = tdx_td_init(kvm, &tdx_cmd);
>> +        break;
>>       default:
>>           r = -EINVAL;
>>           goto out;
> 
>