On 4/5/22 9:49 PM, Kai Huang wrote:
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. The list of these
ranges, along with TDX module information, is available to the kernel by
querying the TDX module via TDH.SYS.INFO SEAMCALL.
Host kernel can choose whether or not to use all convertible memory
regions as TDX memory. Before TDX module is ready to create any TD
guests, all TDX memory regions that host kernel intends to use must be
configured to the TDX module, using specific data structures defined by
TDX architecture. Constructing those structures requires information of
both TDX module and the Convertible Memory Regions. Call TDH.SYS.INFO
to get this information as preparation to construct those structures.
Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx>
---
Looks good. Some minor comments.
arch/x86/virt/vmx/tdx/tdx.c | 131 ++++++++++++++++++++++++++++++++++++
arch/x86/virt/vmx/tdx/tdx.h | 61 +++++++++++++++++
2 files changed, 192 insertions(+)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index ef2718423f0f..482e6d858181 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -80,6 +80,11 @@ static DEFINE_MUTEX(tdx_module_lock);
static struct p_seamldr_info p_seamldr_info;
+/* Base address of CMR array needs to be 512 bytes aligned. */
+static struct cmr_info tdx_cmr_array[MAX_CMRS] __aligned(CMR_INFO_ARRAY_ALIGNMENT);
+static int tdx_cmr_num;
+static struct tdsysinfo_struct tdx_sysinfo;
+
static bool __seamrr_enabled(void)
{
return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
@@ -468,6 +473,127 @@ static int tdx_module_init_cpus(void)
return seamcall_on_each_cpu(&sc);
}
+static inline bool cmr_valid(struct cmr_info *cmr)
+{
+ return !!cmr->size;
+}
+
+static void print_cmrs(struct cmr_info *cmr_array, int cmr_num,
+ const char *name)
+{
+ int i;
+
+ for (i = 0; i < cmr_num; i++) {
+ struct cmr_info *cmr = &cmr_array[i];
+
+ pr_info("%s : [0x%llx, 0x%llx)\n", name,
+ cmr->base, cmr->base + cmr->size);
+ }
I am not sure if it is ok to print this info by default or pr_debug
would be better. I will let maintainers decide about it.
+}
+
+static int sanitize_cmrs(struct cmr_info *cmr_array, int cmr_num)
Since this function only deals with tdx_cmr_array, why pass it
as argument?
+{
+ int i, j;
+
+ /*
+ * Intel TDX module spec, 20.7.3 CMR_INFO:
+ *
+ * TDH.SYS.INFO leaf function returns a MAX_CMRS (32) entry
+ * array of CMR_INFO entries. The CMRs are sorted from the
+ * lowest base address to the highest base address, and they
+ * are non-overlapping.
+ *
+ * This implies that BIOS may generate invalid empty entries
+ * if total CMRs are less than 32. Skip them manually.
+ */
+ for (i = 0; i < cmr_num; i++) {
+ struct cmr_info *cmr = &cmr_array[i];
+ struct cmr_info *prev_cmr = NULL;
Why not keep declarations together at the top of the function?
+
+ /* Skip further invalid CMRs */
+ if (!cmr_valid(cmr))
+ break;
+
+ if (i > 0)
+ prev_cmr = &cmr_array[i - 1];
+
+ /*
+ * It is a TDX firmware bug if CMRs are not
+ * in address ascending order.
+ */
+ if (prev_cmr && ((prev_cmr->base + prev_cmr->size) >
+ cmr->base)) {
+ pr_err("Firmware bug: CMRs not in address ascending order.\n");
+ return -EFAULT;
+ }
Since above condition is only true for i > 0 case, why not combine them
together if (i > 0) {...}
+ }
+
+ /*
+ * Also a sane BIOS should never generate invalid CMR(s) between
+ * two valid CMRs. Sanity check this and simply return error in
+ * this case.
+ *
+ * By reaching here @i is the index of the first invalid CMR (or
+ * cmr_num). Starting with next entry of @i since it has already
+ * been checked.
+ */
+ for (j = i + 1; j < cmr_num; j++)
+ if (cmr_valid(&cmr_array[j])) {
+ pr_err("Firmware bug: invalid CMR(s) among valid CMRs.\n");
+ return -EFAULT;
+ }
+
+ /*
+ * Trim all tail invalid empty CMRs. BIOS should generate at
+ * least one valid CMR, otherwise it's a TDX firmware bug.
+ */
+ tdx_cmr_num = i;
+ if (!tdx_cmr_num) {
+ pr_err("Firmware bug: No valid CMR.\n");
+ return -EFAULT;
+ }
+
+ /* Print kernel sanitized CMRs */
+ print_cmrs(tdx_cmr_array, tdx_cmr_num, "Kernel-sanitized-CMR");
+
+ return 0;
+}
+
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
