| Message ID | c7d85f753172f16238b6b3c9cdb4acf8cbd7bfe6.1718538552.git.kai.huang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TDX host: metadata reading tweaks, bug fix and info dump | expand |
On 16.06.24 г. 15:01 ч., Kai Huang wrote: > For now the kernel only reads "TD Memory Region" (TDMR) related global > metadata fields to a 'struct tdx_tdmr_sysinfo' for initializing the TDX > module. The kernel populates the relevant metadata fields into the > structure using a "field mapping table" of metadata field IDs and the > structure members. > > Currently the scope of this "field mapping table" is the entire C file. > Future changes will need to read more global metadata fields that will > be organized in other structures and use this kind of field mapping > tables for other structures too. > > Move the field mapping table to the function local to limit its scope so > that the same name can also be used by other functions. nit: I think all of this could be condensed simply to : "The mapping table is only used by foo() so move it there, no functional changes". The consideration for future usage seems somewhat moot with respect to such a trivial change. In any case: Reviewed-by: Nikolay Borisov <nik.borisov@suse.com> > > Signed-off-by: Kai Huang <kai.huang@intel.com> > --- > arch/x86/virt/vmx/tdx/tdx.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > index c68fbaf4aa15..fad42014ca37 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.c > +++ b/arch/x86/virt/vmx/tdx/tdx.c > @@ -322,17 +322,17 @@ static int stbuf_read_sysmd_multi(const struct field_mapping *fields, > #define TD_SYSINFO_MAP_TDMR_INFO(_field_id, _member) \ > TD_SYSINFO_MAP(_field_id, struct tdx_tdmr_sysinfo, _member) > > -/* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */ > -static const struct field_mapping fields[] = { > - TD_SYSINFO_MAP_TDMR_INFO(MAX_TDMRS, max_tdmrs), > - TD_SYSINFO_MAP_TDMR_INFO(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr), > - TD_SYSINFO_MAP_TDMR_INFO(PAMT_4K_ENTRY_SIZE, pamt_entry_size[TDX_PS_4K]), > - TD_SYSINFO_MAP_TDMR_INFO(PAMT_2M_ENTRY_SIZE, pamt_entry_size[TDX_PS_2M]), > - TD_SYSINFO_MAP_TDMR_INFO(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]), > -}; > - > static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo) > { > + /* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */ > + static const struct field_mapping fields[] = { > + TD_SYSINFO_MAP_TDMR_INFO(MAX_TDMRS, max_tdmrs), > + TD_SYSINFO_MAP_TDMR_INFO(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr), > + TD_SYSINFO_MAP_TDMR_INFO(PAMT_4K_ENTRY_SIZE, pamt_entry_size[TDX_PS_4K]), > + TD_SYSINFO_MAP_TDMR_INFO(PAMT_2M_ENTRY_SIZE, pamt_entry_size[TDX_PS_2M]), > + TD_SYSINFO_MAP_TDMR_INFO(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]), > + }; > + > /* Populate 'tdmr_sysinfo' fields using the mapping structure above: */ > return stbuf_read_sysmd_multi(fields, ARRAY_SIZE(fields), tdmr_sysinfo); > }
On Tue, 2024-06-18 at 14:47 +0300, Nikolay Borisov wrote: > > On 16.06.24 г. 15:01 ч., Kai Huang wrote: > > For now the kernel only reads "TD Memory Region" (TDMR) related global > > metadata fields to a 'struct tdx_tdmr_sysinfo' for initializing the TDX > > module. The kernel populates the relevant metadata fields into the > > structure using a "field mapping table" of metadata field IDs and the > > structure members. > > > > Currently the scope of this "field mapping table" is the entire C file. > > Future changes will need to read more global metadata fields that will > > be organized in other structures and use this kind of field mapping > > tables for other structures too. > > > > Move the field mapping table to the function local to limit its scope so > > that the same name can also be used by other functions. > > nit: I think all of this could be condensed simply to : > > "The mapping table is only used by foo() so move it there, no functional > changes". The consideration for future usage seems somewhat moot with > respect to such a trivial change. > Without future usage I am not sure whether it is worth to do such change, so I would at least keep the future usage part. And to explain "future usage", I think it would be helpful to have some background text here. So I tend to keep the current way. But I am open on this if other people have comments. > In any case: > > Reviewed-by: Nikolay Borisov <nik.borisov@suse.com> Thanks! > > > > > > Signed-off-by: Kai Huang <kai.huang@intel.com> > > --- > > arch/x86/virt/vmx/tdx/tdx.c | 18 +++++++++--------- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > > index c68fbaf4aa15..fad42014ca37 100644 > > --- a/arch/x86/virt/vmx/tdx/tdx.c > > +++ b/arch/x86/virt/vmx/tdx/tdx.c > > @@ -322,17 +322,17 @@ static int stbuf_read_sysmd_multi(const struct field_mapping *fields, > > #define TD_SYSINFO_MAP_TDMR_INFO(_field_id, _member) \ > > TD_SYSINFO_MAP(_field_id, struct tdx_tdmr_sysinfo, _member) > > > > -/* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */ > > -static const struct field_mapping fields[] = { > > - TD_SYSINFO_MAP_TDMR_INFO(MAX_TDMRS, max_tdmrs), > > - TD_SYSINFO_MAP_TDMR_INFO(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr), > > - TD_SYSINFO_MAP_TDMR_INFO(PAMT_4K_ENTRY_SIZE, pamt_entry_size[TDX_PS_4K]), > > - TD_SYSINFO_MAP_TDMR_INFO(PAMT_2M_ENTRY_SIZE, pamt_entry_size[TDX_PS_2M]), > > - TD_SYSINFO_MAP_TDMR_INFO(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]), > > -}; > > - > > static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo) > > { > > + /* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */ > > + static const struct field_mapping fields[] = { > > + TD_SYSINFO_MAP_TDMR_INFO(MAX_TDMRS, max_tdmrs), > > + TD_SYSINFO_MAP_TDMR_INFO(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr), > > + TD_SYSINFO_MAP_TDMR_INFO(PAMT_4K_ENTRY_SIZE, pamt_entry_size[TDX_PS_4K]), > > + TD_SYSINFO_MAP_TDMR_INFO(PAMT_2M_ENTRY_SIZE, pamt_entry_size[TDX_PS_2M]), > > + TD_SYSINFO_MAP_TDMR_INFO(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]), > > + }; > > + > > /* Populate 'tdmr_sysinfo' fields using the mapping structure above: */ > > return stbuf_read_sysmd_multi(fields, ARRAY_SIZE(fields), tdmr_sysinfo); > > }
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c index c68fbaf4aa15..fad42014ca37 100644 --- a/arch/x86/virt/vmx/tdx/tdx.c +++ b/arch/x86/virt/vmx/tdx/tdx.c @@ -322,17 +322,17 @@ static int stbuf_read_sysmd_multi(const struct field_mapping *fields, #define TD_SYSINFO_MAP_TDMR_INFO(_field_id, _member) \ TD_SYSINFO_MAP(_field_id, struct tdx_tdmr_sysinfo, _member) -/* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */ -static const struct field_mapping fields[] = { - TD_SYSINFO_MAP_TDMR_INFO(MAX_TDMRS, max_tdmrs), - TD_SYSINFO_MAP_TDMR_INFO(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr), - TD_SYSINFO_MAP_TDMR_INFO(PAMT_4K_ENTRY_SIZE, pamt_entry_size[TDX_PS_4K]), - TD_SYSINFO_MAP_TDMR_INFO(PAMT_2M_ENTRY_SIZE, pamt_entry_size[TDX_PS_2M]), - TD_SYSINFO_MAP_TDMR_INFO(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]), -}; - static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo) { + /* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */ + static const struct field_mapping fields[] = { + TD_SYSINFO_MAP_TDMR_INFO(MAX_TDMRS, max_tdmrs), + TD_SYSINFO_MAP_TDMR_INFO(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr), + TD_SYSINFO_MAP_TDMR_INFO(PAMT_4K_ENTRY_SIZE, pamt_entry_size[TDX_PS_4K]), + TD_SYSINFO_MAP_TDMR_INFO(PAMT_2M_ENTRY_SIZE, pamt_entry_size[TDX_PS_2M]), + TD_SYSINFO_MAP_TDMR_INFO(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]), + }; + /* Populate 'tdmr_sysinfo' fields using the mapping structure above: */ return stbuf_read_sysmd_multi(fields, ARRAY_SIZE(fields), tdmr_sysinfo); }
For now the kernel only reads "TD Memory Region" (TDMR) related global metadata fields to a 'struct tdx_tdmr_sysinfo' for initializing the TDX module. The kernel populates the relevant metadata fields into the structure using a "field mapping table" of metadata field IDs and the structure members. Currently the scope of this "field mapping table" is the entire C file. Future changes will need to read more global metadata fields that will be organized in other structures and use this kind of field mapping tables for other structures too. Move the field mapping table to the function local to limit its scope so that the same name can also be used by other functions. Signed-off-by: Kai Huang <kai.huang@intel.com> --- arch/x86/virt/vmx/tdx/tdx.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)