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@xxxxxxxxx> 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.
