diff mbox series

[2/9] x86/virt/tdx: Unbind global metadata read with 'struct tdx_tdmr_sysinfo'

Message ID 43c646d35088a0bada9fbbf8b731a7e4a44b22c0.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

Commit Message

Kai Huang June 16, 2024, 12:01 p.m. UTC
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.

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, and the metadata reading code can only work with that structure.

Future changes will need to read other metadata fields that don't make
sense to populate to the "struct tdx_tdmr_sysinfo".  It's essential to
provide a generic metadata read infrastructure which is not bound to any
specific structure.

To start providing such infrastructure, unbind the metadata reading code
with the 'struct tdx_tdmr_sysinfo'.

Note the kernel has a helper macro, TD_SYSINFO_MAP(), for marshaling the
metadata into the 'struct tdx_tdmr_sysinfo', and currently the macro
hardcodes the structure name.  As part of unbinding the metadata reading
code with 'struct tdx_tdmr_sysinfo', it is extended to accept different
structures.

Unfortunately, this will result in the usage of TD_SYSINFO_MAP() for
populating 'struct tdx_tdmr_sysinfo' to be changed to use the structure
name explicitly for each structure member and make the code longer.  Add
a wrapper macro which hides the 'struct tdx_tdmr_sysinfo' internally to
make the code shorter thus better readability.

Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

Comments

Nikolay Borisov June 18, 2024, 11:23 a.m. UTC | #1
On 16.06.24 г. 15:01 ч., 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.
> 
> 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, and the metadata reading code can only work with that structure.
> 
> Future changes will need to read other metadata fields that don't make
> sense to populate to the "struct tdx_tdmr_sysinfo".  It's essential to
> provide a generic metadata read infrastructure which is not bound to any
> specific structure.
> 
> To start providing such infrastructure, unbind the metadata reading code
> with the 'struct tdx_tdmr_sysinfo'.
> 
> Note the kernel has a helper macro, TD_SYSINFO_MAP(), for marshaling the
> metadata into the 'struct tdx_tdmr_sysinfo', and currently the macro
> hardcodes the structure name.  As part of unbinding the metadata reading
> code with 'struct tdx_tdmr_sysinfo', it is extended to accept different
> structures.
> 
> Unfortunately, this will result in the usage of TD_SYSINFO_MAP() for
> populating 'struct tdx_tdmr_sysinfo' to be changed to use the structure
> name explicitly for each structure member and make the code longer.  Add
> a wrapper macro which hides the 'struct tdx_tdmr_sysinfo' internally to
> make the code shorter thus better readability.
> 
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> ---
>   arch/x86/virt/vmx/tdx/tdx.c | 25 ++++++++++++++-----------
>   1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index fbde24ea3b3e..854312e97eff 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -272,9 +272,9 @@ 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)
> +				     void *stbuf)
>   {
> -	u16 *ts_member = ((void *)ts) + offset;
> +	u16 *st_member = stbuf + offset;

This st_* prefix is completely arbitrary, Just name it "member" since 
this function can be used for any arbitrary member.

>   	u64 tmp;
>   	int ret;
>   

<snip>
Kai Huang June 18, 2024, 11:29 p.m. UTC | #2
> >   static int read_sys_metadata_field16(u64 field_id,
> >   				     int offset,
> > -				     struct tdx_tdmr_sysinfo *ts)
> > +				     void *stbuf)
> >   {
> > -	u16 *ts_member = ((void *)ts) + offset;
> > +	u16 *st_member = stbuf + offset;
> 
> This st_* prefix is completely arbitrary, Just name it "member" since 
> this function can be used for any arbitrary member.
> 
> 

OK fine to me.
diff mbox series

Patch

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index fbde24ea3b3e..854312e97eff 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -272,9 +272,9 @@  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)
+				     void *stbuf)
 {
-	u16 *ts_member = ((void *)ts) + offset;
+	u16 *st_member = stbuf + offset;
 	u64 tmp;
 	int ret;
 
@@ -286,7 +286,7 @@  static int read_sys_metadata_field16(u64 field_id,
 	if (ret)
 		return ret;
 
-	*ts_member = tmp;
+	*st_member = tmp;
 
 	return 0;
 }
@@ -296,17 +296,20 @@  struct field_mapping {
 	int offset;
 };
 
-#define TD_SYSINFO_MAP(_field_id, _member) \
-	{ .field_id = MD_FIELD_ID_##_field_id,	   \
-	  .offset   = offsetof(struct tdx_tdmr_sysinfo, _member) }
+#define TD_SYSINFO_MAP(_field_id, _struct, _member)	\
+	{ .field_id = MD_FIELD_ID_##_field_id,		\
+	  .offset   = offsetof(_struct, _member) }
+
+#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(MAX_TDMRS,	      max_tdmrs),
-	TD_SYSINFO_MAP(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr),
-	TD_SYSINFO_MAP(PAMT_4K_ENTRY_SIZE,    pamt_entry_size[TDX_PS_4K]),
-	TD_SYSINFO_MAP(PAMT_2M_ENTRY_SIZE,    pamt_entry_size[TDX_PS_2M]),
-	TD_SYSINFO_MAP(PAMT_1G_ENTRY_SIZE,    pamt_entry_size[TDX_PS_1G]),
+	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)