On Wed, Aug 13, 2025 at 7:49 PM Edgecombe, Rick P
<rick.p.edgeco...@intel.com> wrote:
>
> On Thu, 2025-08-07 at 13:16 -0700, Sagi Shahar wrote:
> > +#define XFEATURE_MASK_CET (XFEATURE_MASK_CET_USER | 
> > XFEATURE_MASK_CET_KERNEL)
> > +
> > +static void tdx_apply_cpuid_restrictions(struct kvm_cpuid2 *cpuid_data)
> > +{
> > +     for (int i = 0; i < cpuid_data->nent; i++) {
> > +             struct kvm_cpuid_entry2 *e = &cpuid_data->entries[i];
> > +
> > +             if (e->function == 0xd && e->index == 0) {
> > +                     /*
> > +                      * TDX module requires both XTILE_{CFG, DATA} to be 
> > set.
> > +                      * Both bits are required for AMX to be functional.
> > +                      */
> > +                     if ((e->eax & XFEATURE_MASK_XTILE) !=
> > +                         XFEATURE_MASK_XTILE) {
> > +                             e->eax &= ~XFEATURE_MASK_XTILE;
> > +                     }
> > +             }
> > +             if (e->function == 0xd && e->index == 1) {
> > +                     /*
> > +                      * TDX doesn't support LBR yet.
> > +                      * Disable bits from the XCR0 register.
> > +                      */
> > +                     e->ecx &= ~XFEATURE_MASK_LBR;
> > +                     /*
> > +                      * TDX modules requires both CET_{U, S} to be set even
> > +                      * if only one is supported.
> > +                      */
> > +                     if (e->ecx & XFEATURE_MASK_CET)
> > +                             e->ecx |= XFEATURE_MASK_CET;
> > +             }
> > +     }
> > +}
>
> Since this is only going to be used control the directly configurable bits, do
> we really need to do this? SET_CPUID2 will just get what comes out of
> KVM_TDX_GET_CPUID, so it should pick up the correct values.
>

This code is used before vcpus are created so KVM_TDX_GET_CPUID can't
be used here.

But either way, I removed this function in the next version.

>
> <snip>
>
> > +
> > +static void tdx_td_init(struct kvm_vm *vm, uint64_t attributes)
> > +{
> > +     struct kvm_tdx_init_vm *init_vm;
> > +     const struct kvm_cpuid2 *tmp;
> > +     struct kvm_cpuid2 *cpuid;
> > +
> > +     tmp = kvm_get_supported_cpuid();
> > +
> > +     cpuid = allocate_kvm_cpuid2(KVM_MAX_CPUID_ENTRIES);
> > +     memcpy(cpuid, tmp, kvm_cpuid2_size(tmp->nent));
> > +     tdx_mask_cpuid_features(cpuid);
> > +
> > +     init_vm = calloc(1, sizeof(*init_vm) +
> > +                      sizeof(init_vm->cpuid.entries[0]) * cpuid->nent);
> > +     TEST_ASSERT(init_vm, "vm allocation failed");
> > +
> > +     memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
> > +     free(cpuid);
> > +
> > +     init_vm->attributes = attributes;
> > +
> > +     tdx_apply_cpuid_restrictions(&init_vm->cpuid);
> > +     tdx_filter_cpuid(vm, &init_vm->cpuid);
> > +
> > +     tdx_ioctl(vm->fd, KVM_TDX_INIT_VM, 0, init_vm);
> > +     free(init_vm);
> > +}
>
> We should comment the CPUID twiddling that happens here. It masks, filters, 
> and
> applies restrictions. Sounds like all the same thing.
>
>
>

Reply via email to