| Message ID | dd4ab4f97fc12780e4052f7ece94ceadffafd24d.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. Future changes will need to read other metadata fields that > don't make sense to populate to the "struct tdx_tdmr_sysinfo". > > Now the code in get_tdx_tdmr_sysinfo() to read multiple global metadata > fields is not bound to the 'struct tdx_tdmr_sysinfo', and can support > reading all metadata element sizes. Abstract this code as a helper for > future use. > > Signed-off-by: Kai Huang <kai.huang@intel.com> > --- > arch/x86/virt/vmx/tdx/tdx.c | 27 ++++++++++++++++----------- > 1 file changed, 16 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > index 4392e82a9bcb..c68fbaf4aa15 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.c > +++ b/arch/x86/virt/vmx/tdx/tdx.c > @@ -304,6 +304,21 @@ struct field_mapping { > .offset = offsetof(_struct, _member), \ > .size = sizeof(typeof(((_struct *)0)->_member)) } > > +static int stbuf_read_sysmd_multi(const struct field_mapping *fields, Rename it to read_system_metadata_fields i.e just use the plural form of the single field function. Whatever you choose to rename the singular form, just make this function be the plural. But as a general remark - I don't see what value the "stbuf" prefix brings. 'sysmd' is also somewhat unintuitive. Any of read_metadata_fields/read_sys_metadata_fields/read_system_metadata_fields seem better. > + int nr_fields, void *stbuf) > +{ > + int i, ret; > + > + for (i = 0; i < nr_fields; i++) { > + ret = stbuf_read_sysmd_field(fields[i].field_id, stbuf, > + fields[i].offset, fields[i].size); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > #define TD_SYSINFO_MAP_TDMR_INFO(_field_id, _member) \ > TD_SYSINFO_MAP(_field_id, struct tdx_tdmr_sysinfo, _member) > <snip>
On Tue, 2024-06-18 at 14:45 +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. Future changes will need to read other metadata fields that > > don't make sense to populate to the "struct tdx_tdmr_sysinfo". > > > > Now the code in get_tdx_tdmr_sysinfo() to read multiple global metadata > > fields is not bound to the 'struct tdx_tdmr_sysinfo', and can support > > reading all metadata element sizes. Abstract this code as a helper for > > future use. > > > > Signed-off-by: Kai Huang <kai.huang@intel.com> > > --- > > arch/x86/virt/vmx/tdx/tdx.c | 27 ++++++++++++++++----------- > > 1 file changed, 16 insertions(+), 11 deletions(-) > > > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > > index 4392e82a9bcb..c68fbaf4aa15 100644 > > --- a/arch/x86/virt/vmx/tdx/tdx.c > > +++ b/arch/x86/virt/vmx/tdx/tdx.c > > @@ -304,6 +304,21 @@ struct field_mapping { > > .offset = offsetof(_struct, _member), \ > > .size = sizeof(typeof(((_struct *)0)->_member)) } > > > > +static int stbuf_read_sysmd_multi(const struct field_mapping *fields, > > Rename it to read_system_metadata_fields i.e just use the plural form of > the single field function. Whatever you choose to rename the singular > form, just make this function be the plural. But as a general remark - I > don't see what value the "stbuf" prefix brings. 'sysmd' is also somewhat > unintuitive. Any of > read_metadata_fields/read_sys_metadata_fields/read_system_metadata_fields > seem better. read_sys_metadata_field() vs read_sys_metadata_fields() is hard to distinguish due to there's only one character difference. This is one reason I added "stbuf" prefix hoping to make it more clear that this function reads multiple fields to a buffer of a structure. In terms of "sysmd" I just wanted to make the function name shorter. This function is only used by tdx.c internally so I think it's fine. Anyway I am open on this. Let's wait for some more time to see whether other people have comments or not.
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c index 4392e82a9bcb..c68fbaf4aa15 100644 --- a/arch/x86/virt/vmx/tdx/tdx.c +++ b/arch/x86/virt/vmx/tdx/tdx.c @@ -304,6 +304,21 @@ struct field_mapping { .offset = offsetof(_struct, _member), \ .size = sizeof(typeof(((_struct *)0)->_member)) } +static int stbuf_read_sysmd_multi(const struct field_mapping *fields, + int nr_fields, void *stbuf) +{ + int i, ret; + + for (i = 0; i < nr_fields; i++) { + ret = stbuf_read_sysmd_field(fields[i].field_id, stbuf, + fields[i].offset, fields[i].size); + if (ret) + return ret; + } + + return 0; +} + #define TD_SYSINFO_MAP_TDMR_INFO(_field_id, _member) \ TD_SYSINFO_MAP(_field_id, struct tdx_tdmr_sysinfo, _member) @@ -318,18 +333,8 @@ static const struct field_mapping fields[] = { static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo) { - int ret; - int i; - /* Populate 'tdmr_sysinfo' fields using the mapping structure above: */ - for (i = 0; i < ARRAY_SIZE(fields); i++) { - ret = stbuf_read_sysmd_field(fields[i].field_id, tdmr_sysinfo, - fields[i].offset, fields[i].size); - if (ret) - return ret; - } - - return 0; + return stbuf_read_sysmd_multi(fields, ARRAY_SIZE(fields), tdmr_sysinfo); } /* Calculate the actual TDMR size */
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. Future changes will need to read other metadata fields that don't make sense to populate to the "struct tdx_tdmr_sysinfo". Now the code in get_tdx_tdmr_sysinfo() to read multiple global metadata fields is not bound to the 'struct tdx_tdmr_sysinfo', and can support reading all metadata element sizes. Abstract this code as a helper for future use. Signed-off-by: Kai Huang <kai.huang@intel.com> --- arch/x86/virt/vmx/tdx/tdx.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)