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]

Message-ID: <5470570d804b52dcf24b454d5fdfc2320f735e80.camel@intel.com>
Date: Thu, 28 Mar 2024 01:12:50 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "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>, "Li,
 Xiaoyao" <xiaoyao.li@...el.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 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.