linux-kernel - Re: [PATCH v19 039/130] 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] [thread-next>] [day] [month] [year] [list]

Date: Thu, 28 Mar 2024 09:36:21 +0800
From: Xiaoyao Li <xiaoyao.li@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
 "Gao, Chao" <chao.gao@...el.com>, "Yamahata, Isaku"
 <isaku.yamahata@...el.com>
Cc: "Zhang, Tina" <tina.zhang@...el.com>,
 "isaku.yamahata@...ux.intel.com" <isaku.yamahata@...ux.intel.com>,
 "seanjc@...gle.com" <seanjc@...gle.com>, "Huang, Kai" <kai.huang@...el.com>,
 "sagis@...gle.com" <sagis@...gle.com>, "Chen, Bo2" <chen.bo@...el.com>,
 "isaku.yamahata@...il.com" <isaku.yamahata@...il.com>,
 "Aktas, Erdem" <erdemaktas@...gle.com>,
 "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
 "pbonzini@...hat.com" <pbonzini@...hat.com>, "Yuan, Hang"
 <hang.yuan@...el.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v19 039/130] KVM: TDX: initialize VM with TDX specific
 parameters

On 3/28/2024 9:12 AM, Edgecombe, Rick P wrote:
> On Thu, 2024-03-21 at 08:55 -0700, Isaku Yamahata wrote:
>> On Wed, Mar 20, 2024 at 02:12:49PM +0800,
>> Chao Gao <chao.gao@...el.com> wrote:
>>
>>>> +static void setup_tdparams_cpuids(struct kvm_cpuid2 *cpuid,
>>>> +                                 struct td_params *td_params)
>>>> +{
>>>> +       int i;
>>>> +
>>>> +       /*
>>>> +        * td_params.cpuid_values: The number and the order of cpuid_value must
>>>> +        * be same to the one of struct tdsysinfo.{num_cpuid_config, cpuid_configs}
>>>> +        * It's assumed that td_params was zeroed.
>>>> +        */
>>>> +       for (i = 0; i < tdx_info->num_cpuid_config; i++) {
>>>> +               const struct kvm_tdx_cpuid_config *c = &tdx_info->cpuid_configs[i];
>>>> +               /* KVM_TDX_CPUID_NO_SUBLEAF means index = 0. */
>>>> +               u32 index = c->sub_leaf == KVM_TDX_CPUID_NO_SUBLEAF ? 0 : c->sub_leaf;
>>>> +               const struct kvm_cpuid_entry2 *entry =
>>>> +                       kvm_find_cpuid_entry2(cpuid->entries, cpuid->nent,
>>>> +                                             c->leaf, index);
>>>> +               struct tdx_cpuid_value *value = &td_params->cpuid_values[i];
>>>> +
>>>> +               if (!entry)
>>>> +                       continue;
>>>> +
>>>> +               /*
>>>> +                * tdsysinfo.cpuid_configs[].{eax, ebx, ecx, edx}
>>>> +                * bit 1 means it can be configured to zero or one.
>>>> +                * bit 0 means it must be zero.
>>>> +                * Mask out non-configurable bits.
>>>> +                */
>>>> +               value->eax = entry->eax & c->eax;
>>>> +               value->ebx = entry->ebx & c->ebx;
>>>> +               value->ecx = entry->ecx & c->ecx;
>>>> +               value->edx = entry->edx & c->edx;
>>>
>>> Any reason to mask off non-configurable bits rather than return an error? this
>>> is misleading to userspace because guest sees the values emulated by TDX module
>>> instead of the values passed from userspace (i.e., the request from userspace
>>> isn't done but there is no indication of that to userspace).
>>
>> Ok, I'll eliminate them.  If user space passes wrong cpuids, TDX module will
>> return error. I'll leave the error check to the TDX module.
> 
> I was just looking at this. Agreed. It breaks the selftests though.

If all you prefer to go this direction, then please update the error 
handling of this specific SEAMCALL.