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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ae3eb42e-295f-420e-9857-013629d55aa6@redhat.com>
Date: Tue, 25 Feb 2025 18:28:15 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Xiaoyao Li <xiaoyao.li@...el.com>, linux-kernel@...r.kernel.org,
 kvm@...r.kernel.org
Cc: seanjc@...gle.com, Yan Zhao <yan.y.zhao@...el.com>,
 Rick Edgecombe <rick.p.edgecombe@...el.com>,
 Isaku Yamahata <isaku.yamahata@...el.com>
Subject: Re: [PATCH 23/30] KVM: TDX: initialize VM with TDX specific
 parameters

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;
> 
>