On 27/08/24 10:14, Kai Huang wrote: "to reflect the spec better" is a bit vague. How about: x86/virt/tdx: Rename 'struct tdx_tdmr_sysinfo' to 'struct tdx_sys_info_tdmr' Rename 'struct tdx_tdmr_sysinfo' to 'struct tdx_sys_info_tdmr' to prepare for adding similar structures that will all be prefixed by 'tdx_sys_info_'. > The TDX module provides a set of "global metadata fields". They report Since it is a name of something, could capitalize "Global Metadata Fields" > things like TDX module version, supported features, and fields related > to create/run TDX guests and so on. > > TDX organizes those metadata fields by "Class"es based on the meaning of by "Class"es -> into "Classes" > those fields. E.g., for now the kernel only reads "TD Memory Region" > (TDMR) related fields for module initialization. Those fields are > defined under class "TDMR Info". > > There are both immediate needs to read more metadata fields for module > initialization and near-future needs for other kernel components like > KVM to run TDX guests. To meet all those requirements, the idea is the > TDX host core-kernel 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. > > More specifically, the target is to end up with something like: > > struct tdx_sys_info { > struct tdx_sys_info_classA a; > struct tdx_sys_info_classB b; > ... > }; > > Currently the kernel organizes all fields under "TDMR Info" class in > 'struct tdx_tdmr_sysinfo'. To prepare for the above target, rename the > structure to 'struct tdx_sys_info_tdmr' to follow the class name better. > > No functional change intended. > > Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx> Reviewed-by: Adrian Hunter <adrian.hunter@xxxxxxxxx> > --- > v2 -> v3: > - Split out as a separate patch and place it at beginning: > > https://lore.kernel.org/kvm/cover.1721186590.git.kai.huang@xxxxxxxxx/T/#m8fec7c429242d640cf5e756eb68e3b822e6dff8b > > - Rename to 'struct tdx_sys_info_tdmr': > > https://lore.kernel.org/kvm/cover.1721186590.git.kai.huang@xxxxxxxxx/T/#md73dd9b02a492acf4a6facae63e8d030e320967d > https://lore.kernel.org/kvm/cover.1721186590.git.kai.huang@xxxxxxxxx/T/#m8fec7c429242d640cf5e756eb68e3b822e6dff8b > > --- > arch/x86/virt/vmx/tdx/tdx.c | 36 ++++++++++++++++++------------------ > arch/x86/virt/vmx/tdx/tdx.h | 2 +- > 2 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > index 4e2b2e2ac9f9..e979bf442929 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.c > +++ b/arch/x86/virt/vmx/tdx/tdx.c > @@ -272,7 +272,7 @@ static int read_sys_metadata_field(u64 field_id, u64 *data) > > static int read_sys_metadata_field16(u64 field_id, > int offset, > - struct tdx_tdmr_sysinfo *ts) > + struct tdx_sys_info_tdmr *ts) > { > u16 *ts_member = ((void *)ts) + offset; > u64 tmp; > @@ -298,9 +298,9 @@ struct field_mapping { > > #define TD_SYSINFO_MAP(_field_id, _offset) \ > { .field_id = MD_FIELD_ID_##_field_id, \ > - .offset = offsetof(struct tdx_tdmr_sysinfo, _offset) } > + .offset = offsetof(struct tdx_sys_info_tdmr, _offset) } > > -/* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */ > +/* Map TD_SYSINFO fields into 'struct tdx_sys_info_tdmr': */ > static const struct field_mapping fields[] = { > TD_SYSINFO_MAP(MAX_TDMRS, max_tdmrs), > TD_SYSINFO_MAP(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr), > @@ -309,16 +309,16 @@ static const struct field_mapping fields[] = { > TD_SYSINFO_MAP(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]), > }; > > -static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo) > +static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr) > { > int ret; > int i; > > - /* Populate 'tdmr_sysinfo' fields using the mapping structure above: */ > + /* Populate 'sysinfo_tdmr' fields using the mapping structure above: */ > for (i = 0; i < ARRAY_SIZE(fields); i++) { > ret = read_sys_metadata_field16(fields[i].field_id, > fields[i].offset, > - tdmr_sysinfo); > + sysinfo_tdmr); > if (ret) > return ret; > } > @@ -342,13 +342,13 @@ static int tdmr_size_single(u16 max_reserved_per_tdmr) > } > > static int alloc_tdmr_list(struct tdmr_info_list *tdmr_list, > - struct tdx_tdmr_sysinfo *tdmr_sysinfo) > + struct tdx_sys_info_tdmr *sysinfo_tdmr) > { > size_t tdmr_sz, tdmr_array_sz; > void *tdmr_array; > > - tdmr_sz = tdmr_size_single(tdmr_sysinfo->max_reserved_per_tdmr); > - tdmr_array_sz = tdmr_sz * tdmr_sysinfo->max_tdmrs; > + tdmr_sz = tdmr_size_single(sysinfo_tdmr->max_reserved_per_tdmr); > + tdmr_array_sz = tdmr_sz * sysinfo_tdmr->max_tdmrs; > > /* > * To keep things simple, allocate all TDMRs together. > @@ -367,7 +367,7 @@ static int alloc_tdmr_list(struct tdmr_info_list *tdmr_list, > * at a given index in the TDMR list. > */ > tdmr_list->tdmr_sz = tdmr_sz; > - tdmr_list->max_tdmrs = tdmr_sysinfo->max_tdmrs; > + tdmr_list->max_tdmrs = sysinfo_tdmr->max_tdmrs; > tdmr_list->nr_consumed_tdmrs = 0; > > return 0; > @@ -921,11 +921,11 @@ static int tdmrs_populate_rsvd_areas_all(struct tdmr_info_list *tdmr_list, > /* > * Construct a list of TDMRs on the preallocated space in @tdmr_list > * to cover all TDX memory regions in @tmb_list based on the TDX module > - * TDMR global information in @tdmr_sysinfo. > + * TDMR global information in @sysinfo_tdmr. > */ > static int construct_tdmrs(struct list_head *tmb_list, > struct tdmr_info_list *tdmr_list, > - struct tdx_tdmr_sysinfo *tdmr_sysinfo) > + struct tdx_sys_info_tdmr *sysinfo_tdmr) > { > int ret; > > @@ -934,12 +934,12 @@ static int construct_tdmrs(struct list_head *tmb_list, > return ret; > > ret = tdmrs_set_up_pamt_all(tdmr_list, tmb_list, > - tdmr_sysinfo->pamt_entry_size); > + sysinfo_tdmr->pamt_entry_size); > if (ret) > return ret; > > ret = tdmrs_populate_rsvd_areas_all(tdmr_list, tmb_list, > - tdmr_sysinfo->max_reserved_per_tdmr); > + sysinfo_tdmr->max_reserved_per_tdmr); > if (ret) > tdmrs_free_pamt_all(tdmr_list); > > @@ -1098,7 +1098,7 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list) > > static int init_tdx_module(void) > { > - struct tdx_tdmr_sysinfo tdmr_sysinfo; > + struct tdx_sys_info_tdmr sysinfo_tdmr; > int ret; > > /* > @@ -1117,17 +1117,17 @@ static int init_tdx_module(void) > if (ret) > goto out_put_tdxmem; > > - ret = get_tdx_tdmr_sysinfo(&tdmr_sysinfo); > + ret = get_tdx_sys_info_tdmr(&sysinfo_tdmr); > if (ret) > goto err_free_tdxmem; > > /* Allocate enough space for constructing TDMRs */ > - ret = alloc_tdmr_list(&tdx_tdmr_list, &tdmr_sysinfo); > + ret = alloc_tdmr_list(&tdx_tdmr_list, &sysinfo_tdmr); > if (ret) > goto err_free_tdxmem; > > /* Cover all TDX-usable memory regions in TDMRs */ > - ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, &tdmr_sysinfo); > + ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, &sysinfo_tdmr); > if (ret) > goto err_free_tdmrs; > > diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h > index b701f69485d3..148f9b4d1140 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.h > +++ b/arch/x86/virt/vmx/tdx/tdx.h > @@ -100,7 +100,7 @@ struct tdx_memblock { > }; > > /* "TDMR info" part of "Global Scope Metadata" for constructing TDMRs */ > -struct tdx_tdmr_sysinfo { > +struct tdx_sys_info_tdmr { > u16 max_tdmrs; > u16 max_reserved_per_tdmr; > u16 pamt_entry_size[TDX_PS_NR];