Re: [PATCH v3 3/8] x86/virt/tdx: Prepare to support reading other global metadata fields

["Huang, Kai" <kai.huang@intel.com>]

> > 
> > For now the kernel only reads "TD Memory Region" (TDMR) related fields
> > for module initialization.  There are both immediate needs to read more
> > fields for module initialization and near-future needs for other kernel
> > components like KVM to run TDX guests.
> > will be organized in different structures depending on their meanings.
> 
> Line above seems lost

Hmm.. It should be removed.  I thought I have done the spell check for all the
patches :-(
> 
> > 
> > For now the kernel only reads TDMR related fields.  The TD_SYSINFO_MAP()
> > macro hard-codes the 'struct tdx_sys_info_tdmr' instance name.  To make
> > it work with different instances of different structures, extend it to
> > take the structure instance name as an argument.
> > 
> > This also means the current code which uses TD_SYSINFO_MAP() must type
> > 'struct tdx_sys_info_tdmr' instance name explicitly for each use.  To
> > make the code easier to read, add a wrapper TD_SYSINFO_MAP_TDMR_INFO()
> > which hides the instance name.
> 
> Note, were you to accept my suggestion for patch 2, TD_SYSINFO_MAP() would
> have gone away, and no changes would be needed to get_tdx_sys_info_tdmr().
> So the above 2 paragraphs could be dropped.

Yeah seems better to me.  I'll use your way unless someone objects.

> 
> > 
> > TDX also support 8/16/32/64 bits metadata field element sizes.  For now
> > all TDMR related fields are 16-bit long thus the kernel only has one
> > helper:
> > 
> >   static int read_sys_metadata_field16(u64 field_id, u16 *val) {}
> > 
> > Future changes will need to read more metadata fields with different
> > element sizes.  To make the code short, extend the helper to take a
> > 'void *' buffer and the buffer size so it can work with all element
> > sizes.
> > 
> > Note in this way the helper loses the type safety and the build-time
> > check inside the helper cannot work anymore because the compiler cannot
> > determine the exact value of the buffer size.
> > 
> > To resolve those, add a wrapper of the helper which only works with
> > u8/u16/u32/u64 directly and do build-time check, where the compiler
> > can easily know both the element size (from field ID) and the buffer
> > size(using sizeof()), before calling the helper.
> > 
> > An alternative option is to provide one helper for each element size:
> 
> IMHO, it is not necessary to describe alternative options. 
> 

I am not sure?  My understanding is we should mention those alternatives in
the changelog so that the reviewers can have a better view?

[...]