[v4,1/4] RAS/AMD/ATL: Read DRAM hole base early

Commit Message

John Allen May 6, 2024, 3:46 p.m. UTC

Read DRAM hole base when constructing the address map as the value will
not change during run time.

Reviewed-by: Yazen Ghannam <[e-mail]>
Signed-off-by: John Allen <[e-mail]>
---
v2:
  - Fix compilation error. ctx->addr should read ctx->ret_addr.
  - Improve commit description.
v3:
  - Include "0x" prefix for dram_hole_base print
---
 drivers/ras/amd/atl/core.c     | 15 ++-------------
 drivers/ras/amd/atl/internal.h |  2 ++
 drivers/ras/amd/atl/system.c   | 21 +++++++++++++++++++++
 3 files changed, 25 insertions(+), 13 deletions(-)

Comments

Borislav Petkov May 28, 2024, 8:16 a.m. UTC | #1

On Mon, May 06, 2024 at 03:46:02PM +0000, John Allen wrote:
> @@ -282,6 +298,11 @@ int get_df_system_info(void)
>  
>  	get_num_maps();
>  
> +	if (get_dram_hole_base()) {
> +		pr_warn("amd_atl: Failed to read DRAM hole base");

This thing with the printk prefix "amd_atl: ". Please do a pre-patch
which adds

#undef pr_fmt
#define pr_fmt(fmt) "amd_atl: " fmt

to ...atl/internal.h and remove all such string prefixes from the pr_*
statements in the driver.

> +		return -EINVAL;

So this basically says that the driver won't load if it can't read hole
base. But it did load before - on failure to read that reg, it would
simply fail translating.

So why is that failure so tragic? Or is it that we absolutely need hole
base in all circumstances so that loading it is pointless if not?

Patch

diff --git a/drivers/ras/amd/atl/core.c b/drivers/ras/amd/atl/core.c
index 6dc4e06305f7..63513d972c07 100644
--- a/drivers/ras/amd/atl/core.c
+++ b/drivers/ras/amd/atl/core.c
@@ -51,22 +51,11 @@  static bool legacy_hole_en(struct addr_ctx *ctx)
 
 static int add_legacy_hole(struct addr_ctx *ctx)
 {
-	u32 dram_hole_base;
-	u8 func = 0;
-
 	if (!legacy_hole_en(ctx))
 		return 0;
 
-	if (df_cfg.rev >= DF4)
-		func = 7;
-
-	if (df_indirect_read_broadcast(ctx->node_id, func, 0x104, &dram_hole_base))
-		return -EINVAL;
-
-	dram_hole_base &= DF_DRAM_HOLE_BASE_MASK;
-
-	if (ctx->ret_addr >= dram_hole_base)
-		ctx->ret_addr += (BIT_ULL(32) - dram_hole_base);
+	if (ctx->ret_addr >= df_cfg.dram_hole_base)
+		ctx->ret_addr += (BIT_ULL(32) - df_cfg.dram_hole_base);
 
 	return 0;
 }
diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
index 5de69e0bb0f9..1413c8ddc6c5 100644
--- a/drivers/ras/amd/atl/internal.h
+++ b/drivers/ras/amd/atl/internal.h
@@ -132,6 +132,8 @@  struct df_config {
 	/* Number of DRAM Address maps visible in a Coherent Station. */
 	u8 num_coh_st_maps;
 
+	u32 dram_hole_base;
+
 	/* Global flags to handle special cases. */
 	struct df_flags flags;
 };
diff --git a/drivers/ras/amd/atl/system.c b/drivers/ras/amd/atl/system.c
index 701349e84942..a4314f5073ab 100644
--- a/drivers/ras/amd/atl/system.c
+++ b/drivers/ras/amd/atl/system.c
@@ -223,6 +223,21 @@  static int determine_df_rev(void)
 	return -EINVAL;
 }
 
+static int get_dram_hole_base(void)
+{
+	u8 func = 0;
+
+	if (df_cfg.rev >= DF4)
+		func = 7;
+
+	if (df_indirect_read_broadcast(0, func, 0x104, &df_cfg.dram_hole_base))
+		return -EINVAL;
+
+	df_cfg.dram_hole_base &= DF_DRAM_HOLE_BASE_MASK;
+
+	return 0;
+}
+
 static void get_num_maps(void)
 {
 	switch (df_cfg.rev) {
@@ -266,6 +281,7 @@  static void dump_df_cfg(void)
 
 	pr_debug("num_coh_st_maps=%u",			df_cfg.num_coh_st_maps);
 
+	pr_debug("dram_hole_base=0x%x",			df_cfg.dram_hole_base);
 	pr_debug("flags.legacy_ficaa=%u",		df_cfg.flags.legacy_ficaa);
 	pr_debug("flags.socket_id_shift_quirk=%u",	df_cfg.flags.socket_id_shift_quirk);
 }
@@ -282,6 +298,11 @@  int get_df_system_info(void)
 
 	get_num_maps();
 
+	if (get_dram_hole_base()) {
+		pr_warn("amd_atl: Failed to read DRAM hole base");
+		return -EINVAL;
+	}
+
 	dump_df_cfg();
 
 	return 0;