[PATCH 02/38] x86: Allow listing MTRRs in SPL

[Heinrich Schuchardt]

Am 30. März 2023 23:31:51 MESZ schrieb Simon Glass <sjg at chromium.org>:
>When using video in SPL it is useful to see the values of the MTRR
>registers. Move this code into a shared file, so it can be called from
>SPL easily.
>
>Signed-off-by: Simon Glass <sjg at chromium.org>
>---
>
> arch/x86/cpu/mtrr.c         | 61 +++++++++++++++++++++++++++++++++++++
> arch/x86/include/asm/mtrr.h | 20 ++++++++++++
> cmd/x86/mtrr.c              | 60 +++---------------------------------
> 3 files changed, 85 insertions(+), 56 deletions(-)
>
>diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
>index e69dfb552b16..1b5f24aab317 100644
>--- a/arch/x86/cpu/mtrr.c
>+++ b/arch/x86/cpu/mtrr.c
>@@ -30,6 +30,16 @@
> 
> DECLARE_GLOBAL_DATA_PTR;
> 
>+const char *const mtrr_type_name[MTRR_TYPE_COUNT] = {
>+	"Uncacheable",
>+	"Combine",
>+	"2",
>+	"3",
>+	"Through",
>+	"Protect",
>+	"Back",
>+};
>+
> /* Prepare to adjust MTRRs */
> void mtrr_open(struct mtrr_state *state, bool do_caches)
> {
>@@ -320,3 +330,54 @@ int mtrr_set(int cpu_select, int reg, u64 base, u64 mask)
> 
> 	return mtrr_start_op(cpu_select, &oper);
> }
>+
>+static void read_mtrrs_(void *arg)
>+{
>+	struct mtrr_info *info = arg;
>+
>+	mtrr_read_all(info);
>+}
>+
>+int mtrr_list(int reg_count, int cpu_select)
>+{
>+	struct mtrr_info info;
>+	int ret;
>+	int i;
>+
>+	printf("Reg Valid Write-type   %-16s %-16s %-16s\n", "Base   ||",
>+	       "Mask   ||", "Size   ||");
>+	memset(&info, '\0', sizeof(info));
>+	ret = mp_run_on_cpus(cpu_select, read_mtrrs_, &info);
>+	if (ret)
>+		return log_msg_ret("run", ret);
>+	for (i = 0; i < reg_count; i++) {
>+		const char *type = "Invalid";
>+		u64 base, mask, size;
>+		bool valid;
>+
>+		base = info.mtrr[i].base;
>+		mask = info.mtrr[i].mask;
>+		size = ~mask & ((1ULL << CONFIG_CPU_ADDR_BITS) - 1);
>+		size |= (1 << 12) - 1;
>+		size += 1;
>+		valid = mask & MTRR_PHYS_MASK_VALID;
>+		type = mtrr_type_name[base & MTRR_BASE_TYPE_MASK];
>+		printf("%d   %-5s %-12s %016llx %016llx %016llx\n", i,
>+		       valid ? "Y" : "N", type, base & ~MTRR_BASE_TYPE_MASK,
>+		       mask & ~MTRR_PHYS_MASK_VALID, size);
>+	}
>+
>+	return 0;
>+}
>+
>+int mtrr_get_type_by_name(const char *typename)
>+{
>+	int i;
>+
>+	for (i = 0; i < MTRR_TYPE_COUNT; i++) {
>+		if (*typename == *mtrr_type_name[i])
>+			return i;
>+	}
>+
>+	return -EINVAL;
>+};
>diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
>index ca2edc7878f7..2e995f540616 100644
>--- a/arch/x86/include/asm/mtrr.h
>+++ b/arch/x86/include/asm/mtrr.h
>@@ -190,6 +190,26 @@ int mtrr_set(int cpu_select, int reg, u64 base, u64 mask);
>  */
> int mtrr_get_var_count(void);
> 
>+/**
>+ * mtrr_list() - List the MTRRs
>+ *
>+ * Shows a list of all the MTRRs including their values
>+ *
>+ * @reg_count: Number of registers to show. You can use mtrr_get_var_count() for
>+ * this
>+ * @cpu_select: CPU to use. Use MP_SELECT_BSP for the boot CPU
>+ * Returns: 0 if OK, -ve if the CPU was not found
>+ */
>+int mtrr_list(int reg_count, int cpu_select);
>+
>+/**
>+ * mtrr_get_type_by_name() - Get the type of an MTRR given its type name
>+ *
>+ * @typename: Name to check
>+ * Returns: MTRR type (MTRR_TYPE_...) or -EINVAL if invalid
>+ */
>+int mtrr_get_type_by_name(const char *typename);
>+
> #endif
> 
> #if ((CONFIG_XIP_ROM_SIZE & (CONFIG_XIP_ROM_SIZE - 1)) != 0)
>diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
>index ff4be6b7bf5b..fd0e67e46eb3 100644
>--- a/cmd/x86/mtrr.c
>+++ b/cmd/x86/mtrr.c
>@@ -10,71 +10,19 @@
> #include <asm/mp.h>
> #include <asm/mtrr.h>
> 
>-static const char *const mtrr_type_name[MTRR_TYPE_COUNT] = {
>-	"Uncacheable",
>-	"Combine",
>-	"2",
>-	"3",
>-	"Through",
>-	"Protect",
>-	"Back",
>-};


Adding a "write-" prefix would make the meaning clearer

{
"uncachable", /* 0 */ 
"write-combining", /* 1 */ 
"2", /* 2 */
"3", /* 3 */ 
"write-through", /* 4 */
"write-protect", /* 5 */
"write-back", /* 6 */ 
};

Please, provide /doc/usage/cmd/mtrr.rst.

Best regards

Heinrich 

>-
>-static void read_mtrrs(void *arg)
>-{
>-	struct mtrr_info *info = arg;
>-
>-	mtrr_read_all(info);
>-}
>-
>-static int do_mtrr_list(int reg_count, int cpu_select)
>-{
>-	struct mtrr_info info;
>-	int ret;
>-	int i;
>-
>-	printf("Reg Valid Write-type   %-16s %-16s %-16s\n", "Base   ||",
>-	       "Mask   ||", "Size   ||");
>-	memset(&info, '\0', sizeof(info));
>-	ret = mp_run_on_cpus(cpu_select, read_mtrrs, &info);
>-	if (ret)
>-		return log_msg_ret("run", ret);
>-	for (i = 0; i < reg_count; i++) {
>-		const char *type = "Invalid";
>-		uint64_t base, mask, size;
>-		bool valid;
>-
>-		base = info.mtrr[i].base;
>-		mask = info.mtrr[i].mask;
>-		size = ~mask & ((1ULL << CONFIG_CPU_ADDR_BITS) - 1);
>-		size |= (1 << 12) - 1;
>-		size += 1;
>-		valid = mask & MTRR_PHYS_MASK_VALID;
>-		type = mtrr_type_name[base & MTRR_BASE_TYPE_MASK];
>-		printf("%d   %-5s %-12s %016llx %016llx %016llx\n", i,
>-		       valid ? "Y" : "N", type, base & ~MTRR_BASE_TYPE_MASK,
>-		       mask & ~MTRR_PHYS_MASK_VALID, size);
>-	}
>-
>-	return 0;
>-}
>-
> static int do_mtrr_set(int cpu_select, uint reg, int argc, char *const argv[])
> {
> 	const char *typename = argv[0];
> 	uint32_t start, size;
> 	uint64_t base, mask;
>-	int i, type = -1;
>+	int type = -1;
> 	bool valid;
> 	int ret;
> 
> 	if (argc < 3)
> 		return CMD_RET_USAGE;
>-	for (i = 0; i < MTRR_TYPE_COUNT; i++) {
>-		if (*typename == *mtrr_type_name[i])
>-			type = i;
>-	}
>-	if (type == -1) {
>+	type = mtrr_get_type_by_name(typename);
>+	if (type < 0) {
> 		printf("Invalid type name %s\n", typename);
> 		return CMD_RET_USAGE;
> 	}
>@@ -147,7 +95,7 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int argc,
> 				printf("\n");
> 			if (i < MP_SELECT_ALL)
> 				printf("CPU %d:\n", i);
>-			ret = do_mtrr_list(reg_count, i);
>+			ret = mtrr_list(reg_count, i);
> 			if (ret) {
> 				printf("Failed to read CPU %s (err=%d)\n",
> 				       i < MP_SELECT_ALL ? simple_itoa(i) : "",