Messages in this thread Patch in this message | | | Date | Mon, 19 Jun 2023 15:29:47 +0200 | From | David Hildenbrand <> | Subject | Re: [PATCH v11 08/20] x86/virt/tdx: Get information about TDX module and TDX-capable memory |
| |
On 04.06.23 16:27, Kai Huang wrote: > Start to transit out the "multi-steps" to initialize the TDX module. > > TDX provides increased levels of memory confidentiality and integrity. > This requires special hardware support for features like memory > encryption and storage of memory integrity checksums. Not all memory > satisfies these requirements. > > As a result, TDX introduced the concept of a "Convertible Memory Region" > (CMR). During boot, the firmware builds a list of all of the memory > ranges which can provide the TDX security guarantees. > > CMRs tell the kernel which memory is TDX compatible. The kernel takes > CMRs (plus a little more metadata) and constructs "TD Memory Regions" > (TDMRs). TDMRs let the kernel grant TDX protections to some or all of > the CMR areas. > > The TDX module also reports necessary information to let the kernel > build TDMRs and run TDX guests in structure 'tdsysinfo_struct'. The > list of CMRs, along with the TDX module information, is available to > the kernel by querying the TDX module. > > As a preparation to construct TDMRs, get the TDX module information and > the list of CMRs. Print out CMRs to help user to decode which memory > regions are TDX convertible. > > The 'tdsysinfo_struct' is fairly large (1024 bytes) and contains a lot > of info about the TDX module. Fully define the entire structure, but > only use the fields necessary to build the TDMRs and pr_info() some > basics about the module. The rest of the fields will get used by KVM. > > For now both 'tdsysinfo_struct' and CMRs are only used during the module > initialization. But because they are both relatively big, declare them > inside the module initialization function but as static variables. > > Signed-off-by: Kai Huang <kai.huang@intel.com> > Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com> > ---
[...]
> --- > arch/x86/virt/vmx/tdx/tdx.c | 67 +++++++++++++++++++++++++++++++++- > arch/x86/virt/vmx/tdx/tdx.h | 72 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 138 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > index bcf2b2d15a2e..9fde0f71dd8b 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.c > +++ b/arch/x86/virt/vmx/tdx/tdx.c > @@ -20,6 +20,7 @@ > #include <asm/msr-index.h> > #include <asm/msr.h> > #include <asm/archrandom.h> > +#include <asm/page.h> > #include <asm/tdx.h> > #include "tdx.h" > > @@ -191,12 +192,76 @@ int tdx_cpu_enable(void) > } > EXPORT_SYMBOL_GPL(tdx_cpu_enable); > > +static inline bool is_cmr_empty(struct cmr_info *cmr) > +{ > + return !cmr->size; > +} > +
Nit: maybe it's just me, but this function seems unnecessary.
If "!cmr->size" is not expressive, then I don't know why "is_cmr_empty" should be. Just inline that into the single user.
.. after all the single caller also uses/prints cmr->size ...
> +static void print_cmrs(struct cmr_info *cmr_array, int nr_cmrs) > +{ > + int i; > + > + for (i = 0; i < nr_cmrs; i++) { > + struct cmr_info *cmr = &cmr_array[i]; > + > + /* > + * The array of CMRs reported via TDH.SYS.INFO can > + * contain tail empty CMRs. Don't print them. > + */ > + if (is_cmr_empty(cmr)) > + break; > + > + pr_info("CMR: [0x%llx, 0x%llx)\n", cmr->base, > + cmr->base + cmr->size); > + } > +} > + > +/* > + * Get the TDX module information (TDSYSINFO_STRUCT) and the array of > + * CMRs, and save them to @sysinfo and @cmr_array. @sysinfo must have > + * been padded to have enough room to save the TDSYSINFO_STRUCT. > + */ > +static int tdx_get_sysinfo(struct tdsysinfo_struct *sysinfo, > + struct cmr_info *cmr_array) > +{ > + struct tdx_module_output out; > + u64 sysinfo_pa, cmr_array_pa; > + int ret; > + > + sysinfo_pa = __pa(sysinfo); > + cmr_array_pa = __pa(cmr_array); > + ret = seamcall(TDH_SYS_INFO, sysinfo_pa, TDSYSINFO_STRUCT_SIZE, > + cmr_array_pa, MAX_CMRS, NULL, &out); > + if (ret) > + return ret; > + > + pr_info("TDX module: atributes 0x%x, vendor_id 0x%x, major_version %u, minor_version %u, build_date %u, build_num %u",
"attributes" ?
> + sysinfo->attributes, sysinfo->vendor_id, > + sysinfo->major_version, sysinfo->minor_version, > + sysinfo->build_date, sysinfo->build_num); > + > + /* R9 contains the actual entries written to the CMR array. */ > + print_cmrs(cmr_array, out.r9); > + > + return 0; > +} > + > static int init_tdx_module(void) > { > + static DECLARE_PADDED_STRUCT(tdsysinfo_struct, tdsysinfo, > + TDSYSINFO_STRUCT_SIZE, TDSYSINFO_STRUCT_ALIGNMENT); > + static struct cmr_info cmr_array[MAX_CMRS] > + __aligned(CMR_INFO_ARRAY_ALIGNMENT); > + struct tdsysinfo_struct *sysinfo = &PADDED_STRUCT(tdsysinfo); > + int ret; > + > + ret = tdx_get_sysinfo(sysinfo, cmr_array); > + if (ret) > + return ret; > + > /* > * TODO: > * > - * - Get TDX module information and TDX-capable memory regions. > * - Build the list of TDX-usable memory regions. > * - Construct a list of "TD Memory Regions" (TDMRs) to cover > * all TDX-usable memory regions. > diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h > index 9fb46033c852..97f4d7e7f1a4 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.h > +++ b/arch/x86/virt/vmx/tdx/tdx.h > @@ -3,6 +3,8 @@ > #define _X86_VIRT_TDX_H > > #include <linux/types.h> > +#include <linux/stddef.h> > +#include <linux/compiler_attributes.h> > > /* > * This file contains both macros and data structures defined by the TDX > @@ -21,6 +23,76 @@ > */ > #define TDH_SYS_INIT 33 > #define TDH_SYS_LP_INIT 35 > +#define TDH_SYS_INFO 32 > + > +struct cmr_info { > + u64 base; > + u64 size; > +} __packed; > + > +#define MAX_CMRS 32 > +#define CMR_INFO_ARRAY_ALIGNMENT 512 > + > +struct cpuid_config { > + u32 leaf; > + u32 sub_leaf; > + u32 eax; > + u32 ebx; > + u32 ecx; > + u32 edx; > +} __packed; > + > +#define DECLARE_PADDED_STRUCT(type, name, size, alignment) \ > + struct type##_padded { \ > + union { \ > + struct type name; \ > + u8 padding[size]; \ > + }; \ > + } name##_padded __aligned(alignment) > + > +#define PADDED_STRUCT(name) (name##_padded.name) > + > +#define TDSYSINFO_STRUCT_SIZE 1024
So, it can never be larger than 1024 bytes? Not even with many cpuid configs?
> +#define TDSYSINFO_STRUCT_ALIGNMENT 1024 > +
-- Cheers,
David / dhildenb
| |