On Fri, 2024-08-30 at 14:01 +0300, Nikolay Borisov wrote: > > On 27.08.24 г. 10:14 ч., Kai Huang wrote: > > The TDX module provides a set of "global metadata fields". They report > > things like TDX module version, supported features, and fields related > > to create/run TDX guests and so on. > > > > Currently the kernel only reads "TD Memory Region" (TDMR) related fields > > for module initialization. There are immediate needs which require the > > TDX module initialization to read more global metadata including module > > version, supported features and "Convertible Memory Regions" (CMRs). > > > > Also, KVM will need to read more metadata fields to support baseline TDX > > guests. In the longer term, other TDX features like TDX Connect (which > > supports assigning trusted IO devices to TDX guest) may also require > > other kernel components such as pci/vt-d to access global metadata. > > > > To meet all those requirements, the idea is the TDX host core-kernel to > > to provide a centralized, canonical, and read-only structure for the > > global metadata that comes out from the TDX module for all kernel > > components to use. > > > > As the first step, introduce a new 'struct tdx_sys_info' to track all > > global metadata fields. > > > > TDX categories global metadata fields into different "Class"es. E.g., > > the TDMR related fields are under class "TDMR Info". Instead of making > > 'struct tdx_sys_info' a plain structure to contain all metadata fields, > > organize them in smaller structures based on the "Class". > > > > This allows those metadata fields to be used in finer granularity thus > > makes the code more clear. E.g., the construct_tdmr() can just take the > > structure which contains "TDMR Info" metadata fields. > > > > Add a new function get_tdx_sys_info() as the placeholder to read all > > metadata fields, and call it at the beginning of init_tdx_module(). For > > now it only calls get_tdx_sys_info_tdmr() to read TDMR related fields. > > > > Note there is a functional change: get_tdx_sys_info_tdmr() is moved from > > after build_tdx_memlist() to before it, but it is fine to do so. > > > > Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx> > > --- > > > > v2 -> v3: > > - Split out the part to rename 'struct tdx_tdmr_sysinfo' to 'struct > > tdx_sys_info_tdmr'. > > > > > > --- > > arch/x86/virt/vmx/tdx/tdx.c | 19 ++++++++++++------- > > arch/x86/virt/vmx/tdx/tdx.h | 36 +++++++++++++++++++++++++++++------- > > 2 files changed, 41 insertions(+), 14 deletions(-) > > > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > > index 1cd9035c783f..24eb289c80e8 100644 > > --- a/arch/x86/virt/vmx/tdx/tdx.c > > +++ b/arch/x86/virt/vmx/tdx/tdx.c > > @@ -318,6 +318,11 @@ static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr) > > return ret; > > } > > > > +static int get_tdx_sys_info(struct tdx_sys_info *sysinfo) > > A more apt name for this function would be init_tdx_sys_info, because it > will be executed only once during module initialization and it's really > initialising those values. > > Given how complex TDX turns out to be it will be best if one off init > functions are prefixed with 'init_'. > Since the function is only called once, I don't see big difference between get_xx() vs init_xx(). Is init_xx() slightly better? Maybe. But to me we are not at that point that get_xx() needs to be renamed. One reason I don't want to do such change is the current upstream code is already using get_tdx_tdmr_sysinfo(). I don't want to rename using init_xx().
