Re: [PATCH v19 039/130] KVM: TDX: initialize VM with TDX specific parameters

Huang, Kai
2024-03-25T10:39:27Z

On Fri, 2024-03-22 at 18:22 -0700, Yamahata, Isaku wrote:
> On Fri, Mar 22, 2024 at 11:20:01AM +0000,
> "Huang, Kai" <kai.huang@intel.com> wrote:
>
> > On Mon, 2024-02-26 at 00:25 -0800, isaku.yamahata@intel.com wrote:
> > > +struct kvm_tdx_init_vm {
> > > + __u64 attributes;
> > > + __u64 mrconfigid[6]; /* sha384 digest */
> > > + __u64 mrowner[6]; /* sha384 digest */
> > > + __u64 mrownerconfig[6]; /* sha384 digest */
> > > + /*
> > > + * For future extensibility to make sizeof(struct kvm_tdx_init_vm) = 8KB.
> > > + * This should be enough given sizeof(TD_PARAMS) = 1024.
> > > + * 8KB was chosen given because
> > > + * sizeof(struct kvm_cpuid_entry2) * KVM_MAX_CPUID_ENTRIES(=256) = 8KB.
> > > + */
> > > + __u64 reserved[1004];
> >
> > This is insane.
> >
> > You said you want to reserve 8K for CPUID entries, but how can these 1004 * 8
> > bytes be used for CPUID entries since ...
>
> I tried to overestimate it. It's too much, how about to make it
> 1024, reserved[109]?
>

I am not sure why we need 1024B either.

IIUC, the inputs here in 'kvm_tdx_init_vm' should be a subset of the members in
TD_PARAMS. This IOCTL() isn't intended to carry any additional input besides
these defined in TD_PARAMS, right?

If so, then it seems to me you "at most" only need to reserve the space for the
members excluding the CPUID entries, because for the CPUID entries we will
always pass them as a flexible array at the end of the structure.

Based on the spec, the "non-CPUID-entry" part only occupies 256 bytes. To me it
seems we have no reason to reserve more space than 256 bytes.