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

[Paolo Bonzini]

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;