* [PATCH v3 0/2] Incorporate DRAM address in EDAC messages
@ 2025-10-13 19:34 Avadhut Naik
2025-10-13 19:34 ` [PATCH v3 1/2] RAS/AMD/ATL: Translate UMC normalized address to DRAM address using PRM Avadhut Naik
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Avadhut Naik @ 2025-10-13 19:34 UTC (permalink / raw)
To: linux-edac; +Cc: bp, yazen.ghannam, john.allen, linux-kernel, avadhut.naik
Currently, the amd64_edac module only provides UMC normalized and system
physical address when a DRAM ECC error occurs. DRAM Address is neither
logged nor exported through tracepoint.
Modern AMD SOCs provide UEFI PRM module that implements various address
translation PRM handlers. These PRM handlers can be leveraged to convert
UMC normalized address into DRAM address at runtime on occurrence of a
DRAM ECC error. This translated DRAM address can then be logged and
exported through tracepoints. This set adds the required support to
accomplish the aforementioned.
The first patch adds support in the Address Translation Library to invoke
the appropriate PRM handler to perform the translation.
The second patch leverages the support added in the first patch to log
DRAM Address and export it through the RAS tracepoint on occurrence of a
DRAM ECC error.
Changes in v2:
- Modify commit messages per feedback received.
- Remove unnecessary variables.
- Rename struct dram_addr to atl_dram_addr.
- Replace sprintf call in __log_ecc_error() with scnprintf.
- Pass the DRAM Address to edac_mc_handle_error() through "other_detail"
parameter instead of "msg".
Changes in v3:
- Rebase on top of edac-for-next.
- Add Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Links:
v1: https://lore.kernel.org/all/20250717165622.1162091-1-avadhut.naik@amd.com/
v2: https://lore.kernel.org/all/20250915212244.886668-1-avadhut.naik@amd.com/
Avadhut Naik (2):
RAS/AMD/ATL: Translate UMC normalized address to DRAM address using
PRM
EDAC/amd64: Incorporate DRAM Address in EDAC message
drivers/edac/amd64_edac.c | 23 +++++++++++++++++++++-
drivers/edac/amd64_edac.h | 1 +
drivers/ras/amd/atl/core.c | 3 ++-
drivers/ras/amd/atl/internal.h | 9 +++++++++
drivers/ras/amd/atl/prm.c | 36 ++++++++++++++++++++++++++++++----
drivers/ras/amd/atl/umc.c | 9 +++++++++
drivers/ras/ras.c | 18 +++++++++++++++--
include/linux/ras.h | 19 +++++++++++++++++-
8 files changed, 109 insertions(+), 9 deletions(-)
base-commit: 79c0a2b7abc906c7cf3c793256c6b638d7dc477f
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] RAS/AMD/ATL: Translate UMC normalized address to DRAM address using PRM
2025-10-13 19:34 [PATCH v3 0/2] Incorporate DRAM address in EDAC messages Avadhut Naik
@ 2025-10-13 19:34 ` Avadhut Naik
2025-10-13 19:34 ` [PATCH v3 2/2] EDAC/amd64: Incorporate DRAM Address in EDAC message Avadhut Naik
2025-10-13 22:00 ` [PATCH v3 0/2] Incorporate DRAM address in EDAC messages Borislav Petkov
2 siblings, 0 replies; 8+ messages in thread
From: Avadhut Naik @ 2025-10-13 19:34 UTC (permalink / raw)
To: linux-edac; +Cc: bp, yazen.ghannam, john.allen, linux-kernel, avadhut.naik
Modern AMD SOCs provide UEFI PRM module that implements various address
translation PRM handlers.[1] These handlers can be invoked by the OS or
hypervisor at runtime to perform address translations.
On AMD's Zen-based SOCs, Unified Memory Controller (UMC) relative
"normalized" address is reported through MCA_ADDR of UMC SMCA bank type
on occurrence of a DRAM ECC error. This address must be converted into
system physical address and DRAM address to export additional information
about the error.
Add support to convert normalized address into DRAM address through the
appropriate PRM handler. Instead of logging the translated DRAM address
locally, register the translating function when the Address Translation
library is initialized. Modules like amd64_edac can then invoke the PRM
handler to add the DRAM address to their error records. Additionally, it
can also be exported through the RAS tracepont.
[1] https://bugzilla.kernel.org/show_bug.cgi?id=220577
Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
Changes in v2:
1. Modified the commit message and linked kernel bugzilla as reference.
2. Removed unnecessary variables.
3. Renamed struct dram_addr to atl_dram_addr.
Changes in v3:
1. Rebase on top of edac-for-next.
2. Add Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
drivers/ras/amd/atl/core.c | 3 ++-
drivers/ras/amd/atl/internal.h | 9 +++++++++
drivers/ras/amd/atl/prm.c | 36 ++++++++++++++++++++++++++++++----
drivers/ras/amd/atl/umc.c | 9 +++++++++
drivers/ras/ras.c | 18 +++++++++++++++--
include/linux/ras.h | 19 +++++++++++++++++-
6 files changed, 86 insertions(+), 8 deletions(-)
diff --git a/drivers/ras/amd/atl/core.c b/drivers/ras/amd/atl/core.c
index 4197e10993ac..ca1646d030ca 100644
--- a/drivers/ras/amd/atl/core.c
+++ b/drivers/ras/amd/atl/core.c
@@ -207,7 +207,8 @@ static int __init amd_atl_init(void)
/* Increment this module's recount so that it can't be easily unloaded. */
__module_get(THIS_MODULE);
- amd_atl_register_decoder(convert_umc_mca_addr_to_sys_addr);
+ amd_atl_register_decoder(convert_umc_mca_addr_to_sys_addr,
+ convert_umc_mca_addr_to_dram_addr);
pr_info("AMD Address Translation Library initialized\n");
return 0;
diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
index 2b6279d32774..3dad1a5860d6 100644
--- a/drivers/ras/amd/atl/internal.h
+++ b/drivers/ras/amd/atl/internal.h
@@ -279,18 +279,27 @@ int dehash_address(struct addr_ctx *ctx);
unsigned long norm_to_sys_addr(u8 socket_id, u8 die_id, u8 coh_st_inst_id, unsigned long addr);
unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err);
+int convert_umc_mca_addr_to_dram_addr(struct atl_err *err, struct atl_dram_addr *dram_addr);
u64 add_base_and_hole(struct addr_ctx *ctx, u64 addr);
u64 remove_base_and_hole(struct addr_ctx *ctx, u64 addr);
#ifdef CONFIG_AMD_ATL_PRM
unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id, unsigned long addr);
+int prm_umc_norm_to_dram_addr(u8 socket_id, u64 bank_id,
+ unsigned long addr, struct atl_dram_addr *dram_addr);
#else
static inline unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id,
unsigned long addr)
{
return -ENODEV;
}
+
+static inline int prm_umc_norm_to_dram_addr(u8 socket_id, u64 bank_id,
+ unsigned long addr, struct atl_dram_addr *dram_addr)
+{
+ return -ENODEV;
+}
#endif
/*
diff --git a/drivers/ras/amd/atl/prm.c b/drivers/ras/amd/atl/prm.c
index 0931a20d213b..02c47c27690b 100644
--- a/drivers/ras/amd/atl/prm.c
+++ b/drivers/ras/amd/atl/prm.c
@@ -19,10 +19,11 @@
#include <linux/prmt.h>
/*
- * PRM parameter buffer - normalized to system physical address, as described
- * in the "PRM Parameter Buffer" section of the AMD ACPI Porting Guide.
+ * PRM parameter buffer - normalized to system physical address and normalized
+ * to DRAM address, as described in the "PRM Parameter Buffer" section of the
+ * AMD ACPI Porting Guide.
*/
-struct norm_to_sys_param_buf {
+struct prm_parameter_buffer {
u64 norm_addr;
u8 socket;
u64 bank_id;
@@ -33,9 +34,13 @@ static const guid_t norm_to_sys_guid = GUID_INIT(0xE7180659, 0xA65D, 0x451D,
0x92, 0xCD, 0x2B, 0x56, 0xF1,
0x2B, 0xEB, 0xA6);
+static const guid_t norm_to_dram_guid = GUID_INIT(0x7626C6AE, 0xF973, 0x429C,
+ 0xA9, 0x1C, 0x10, 0x7D, 0x7B,
+ 0xE2, 0x98, 0xB0);
+
unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 bank_id, unsigned long addr)
{
- struct norm_to_sys_param_buf p_buf;
+ struct prm_parameter_buffer p_buf;
unsigned long ret_addr;
int ret;
@@ -55,3 +60,26 @@ unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 bank_id, unsigned long
return ret;
}
+
+int prm_umc_norm_to_dram_addr(u8 socket_id, u64 bank_id,
+ unsigned long addr, struct atl_dram_addr *dram_addr)
+{
+ struct prm_parameter_buffer p_buf;
+ int ret;
+
+ p_buf.norm_addr = addr;
+ p_buf.socket = socket_id;
+ p_buf.bank_id = bank_id;
+ p_buf.out_buf = dram_addr;
+
+ ret = acpi_call_prm_handler(norm_to_dram_guid, &p_buf);
+ if (!ret)
+ return ret;
+
+ if (ret == -ENODEV)
+ pr_debug("PRM module/handler not available.\n");
+ else
+ pr_notice_once("PRM DRAM Address Translation failed.\n");
+
+ return ret;
+}
diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
index 6e072b7667e9..3f53f90dadc0 100644
--- a/drivers/ras/amd/atl/umc.c
+++ b/drivers/ras/amd/atl/umc.c
@@ -427,3 +427,12 @@ unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err)
return norm_to_sys_addr(socket_id, die_id, coh_st_inst_id, addr);
}
+
+int convert_umc_mca_addr_to_dram_addr(struct atl_err *err, struct atl_dram_addr *dram_addr)
+{
+ u8 socket_id = topology_physical_package_id(err->cpu);
+ unsigned long addr = get_addr(err->addr);
+ u64 bank_id = err->ipid;
+
+ return prm_umc_norm_to_dram_addr(socket_id, bank_id, addr, dram_addr);
+}
diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
index ac0e132ccc3e..94f767be08ee 100644
--- a/drivers/ras/ras.c
+++ b/drivers/ras/ras.c
@@ -19,15 +19,20 @@
*/
static unsigned long (*amd_atl_umc_na_to_spa)(struct atl_err *err);
-void amd_atl_register_decoder(unsigned long (*f)(struct atl_err *))
+static int (*amd_atl_umc_na_to_dram_addr)(struct atl_err *err, struct atl_dram_addr *dram_addr);
+
+void amd_atl_register_decoder(unsigned long (*f1)(struct atl_err *),
+ int (*f2)(struct atl_err *, struct atl_dram_addr *))
{
- amd_atl_umc_na_to_spa = f;
+ amd_atl_umc_na_to_spa = f1;
+ amd_atl_umc_na_to_dram_addr = f2;
}
EXPORT_SYMBOL_GPL(amd_atl_register_decoder);
void amd_atl_unregister_decoder(void)
{
amd_atl_umc_na_to_spa = NULL;
+ amd_atl_umc_na_to_dram_addr = NULL;
}
EXPORT_SYMBOL_GPL(amd_atl_unregister_decoder);
@@ -39,6 +44,15 @@ unsigned long amd_convert_umc_mca_addr_to_sys_addr(struct atl_err *err)
return amd_atl_umc_na_to_spa(err);
}
EXPORT_SYMBOL_GPL(amd_convert_umc_mca_addr_to_sys_addr);
+
+int amd_convert_umc_mca_addr_to_dram_addr(struct atl_err *err, struct atl_dram_addr *dram_addr)
+{
+ if (!amd_atl_umc_na_to_dram_addr)
+ return -EINVAL;
+
+ return amd_atl_umc_na_to_dram_addr(err, dram_addr);
+}
+EXPORT_SYMBOL_GPL(amd_convert_umc_mca_addr_to_dram_addr);
#endif /* CONFIG_AMD_ATL */
#define CREATE_TRACE_POINTS
diff --git a/include/linux/ras.h b/include/linux/ras.h
index a64182bc72ad..f489da8b4722 100644
--- a/include/linux/ras.h
+++ b/include/linux/ras.h
@@ -42,15 +42,32 @@ struct atl_err {
u32 cpu;
};
+struct atl_dram_addr {
+ u8 chip_select;
+ u8 bank_group;
+ u8 bank_addr;
+ u32 row_addr;
+ u16 col_addr;
+ u8 rank_mul;
+ u8 sub_ch;
+} __packed;
+
#if IS_ENABLED(CONFIG_AMD_ATL)
-void amd_atl_register_decoder(unsigned long (*f)(struct atl_err *));
+void amd_atl_register_decoder(unsigned long (*f1)(struct atl_err *),
+ int (*f2)(struct atl_err *, struct atl_dram_addr *));
void amd_atl_unregister_decoder(void);
void amd_retire_dram_row(struct atl_err *err);
unsigned long amd_convert_umc_mca_addr_to_sys_addr(struct atl_err *err);
+int amd_convert_umc_mca_addr_to_dram_addr(struct atl_err *err, struct atl_dram_addr *dram_addr);
#else
static inline void amd_retire_dram_row(struct atl_err *err) { }
static inline unsigned long
amd_convert_umc_mca_addr_to_sys_addr(struct atl_err *err) { return -EINVAL; }
+static inline int amd_convert_umc_mca_addr_to_dram_addr(struct atl_err *err,
+ struct atl_dram_addr *dram_addr)
+{
+ return -EINVAL;
+}
#endif /* CONFIG_AMD_ATL */
#endif /* __RAS_H__ */
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] EDAC/amd64: Incorporate DRAM Address in EDAC message
2025-10-13 19:34 [PATCH v3 0/2] Incorporate DRAM address in EDAC messages Avadhut Naik
2025-10-13 19:34 ` [PATCH v3 1/2] RAS/AMD/ATL: Translate UMC normalized address to DRAM address using PRM Avadhut Naik
@ 2025-10-13 19:34 ` Avadhut Naik
2025-10-13 22:00 ` [PATCH v3 0/2] Incorporate DRAM address in EDAC messages Borislav Petkov
2 siblings, 0 replies; 8+ messages in thread
From: Avadhut Naik @ 2025-10-13 19:34 UTC (permalink / raw)
To: linux-edac; +Cc: bp, yazen.ghannam, john.allen, linux-kernel, avadhut.naik
Currently, the amd64_edac module provides decoded error data to the EDAC
interface. This data involves the system physical address (PFN + offset).
Furthermore, the UMC normalized address, gathered from MCA error decoding,
is also provided. The DRAM Address on which the error has occurred,
however, is not provided.
Use the new PRM call in the AMD Address Translation Library to gather the
DRAM address of an error. Include this data in the EDAC 'string' so it
is available in the kernel messages and the RAS tracepoint.
Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
Changes in v2:
1. Modify commit message per feedback received.
2. Pass the DRAM Address to edac_mc_handle_error() through "other_detail"
parameter instead of "msg".
3. Replace sprintf call with scnprintf in __log_ecc_error().
Changes in v3:
1. Rebase on top of edac-for-next.
2. Add Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
drivers/edac/amd64_edac.c | 23 ++++++++++++++++++++++-
drivers/edac/amd64_edac.h | 1 +
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 2f6ab783bf20..856a78175885 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2709,6 +2709,9 @@ static void __log_ecc_error(struct mem_ctl_info *mci, struct err_info *err,
{
enum hw_event_mc_err_type err_type;
const char *string;
+ char s[100];
+
+ memset(s, 0, sizeof(s));
if (ecc_type == 2)
err_type = HW_EVENT_ERR_CORRECTED;
@@ -2724,6 +2727,17 @@ static void __log_ecc_error(struct mem_ctl_info *mci, struct err_info *err,
switch (err->err_code) {
case DECODE_OK:
string = "";
+
+ if (err->dram_addr) {
+ scnprintf(s, sizeof(s), "Cs: 0x%x Bank Grp: 0x%x Bank Addr: 0x%x Row: 0x%x Column: 0x%x RankMul: 0x%x SubChannel: 0x%x",
+ err->dram_addr->chip_select,
+ err->dram_addr->bank_group,
+ err->dram_addr->bank_addr,
+ err->dram_addr->row_addr,
+ err->dram_addr->col_addr,
+ err->dram_addr->rank_mul,
+ err->dram_addr->sub_ch);
+ }
break;
case ERR_NODE:
string = "Failed to map error addr to a node";
@@ -2748,7 +2762,7 @@ static void __log_ecc_error(struct mem_ctl_info *mci, struct err_info *err,
edac_mc_handle_error(err_type, mci, 1,
err->page, err->offset, err->syndrome,
err->csrow, err->channel, -1,
- string, "");
+ string, s);
}
static inline void decode_bus_error(int node_id, struct mce *m)
@@ -2808,11 +2822,13 @@ static void umc_get_err_info(struct mce *m, struct err_info *err)
static void decode_umc_error(int node_id, struct mce *m)
{
u8 ecc_type = (m->status >> 45) & 0x3;
+ struct atl_dram_addr dram_addr;
struct mem_ctl_info *mci;
unsigned long sys_addr;
struct amd64_pvt *pvt;
struct atl_err a_err;
struct err_info err;
+ int ret;
node_id = fixup_node_id(node_id, m);
@@ -2822,6 +2838,7 @@ static void decode_umc_error(int node_id, struct mce *m)
pvt = mci->pvt_info;
+ memset(&dram_addr, 0, sizeof(dram_addr));
memset(&err, 0, sizeof(err));
if (m->status & MCI_STATUS_DEFERRED)
@@ -2853,6 +2870,10 @@ static void decode_umc_error(int node_id, struct mce *m)
goto log_error;
}
+ ret = amd_convert_umc_mca_addr_to_dram_addr(&a_err, &dram_addr);
+ if (!ret)
+ err.dram_addr = &dram_addr;
+
error_address_to_page_and_offset(sys_addr, &err);
log_error:
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index d70b8a8d0b09..5d82e052746d 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -399,6 +399,7 @@ struct err_info {
u16 syndrome;
u32 page;
u32 offset;
+ struct atl_dram_addr *dram_addr;
};
static inline u32 get_umc_base(u8 channel)
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Incorporate DRAM address in EDAC messages
2025-10-13 19:34 [PATCH v3 0/2] Incorporate DRAM address in EDAC messages Avadhut Naik
2025-10-13 19:34 ` [PATCH v3 1/2] RAS/AMD/ATL: Translate UMC normalized address to DRAM address using PRM Avadhut Naik
2025-10-13 19:34 ` [PATCH v3 2/2] EDAC/amd64: Incorporate DRAM Address in EDAC message Avadhut Naik
@ 2025-10-13 22:00 ` Borislav Petkov
2025-10-14 13:52 ` Yazen Ghannam
2 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2025-10-13 22:00 UTC (permalink / raw)
To: Avadhut Naik; +Cc: linux-edac, yazen.ghannam, john.allen, linux-kernel
On Mon, Oct 13, 2025 at 07:34:47PM +0000, Avadhut Naik wrote:
> Currently, the amd64_edac module only provides UMC normalized and system
> physical address when a DRAM ECC error occurs. DRAM Address is neither
> logged nor exported through tracepoint.
>
> Modern AMD SOCs provide UEFI PRM module that implements various address
> translation PRM handlers. These PRM handlers can be leveraged to convert
> UMC normalized address into DRAM address at runtime on occurrence of a
> DRAM ECC error. This translated DRAM address can then be logged and
> exported through tracepoints.
And?
I read all three commit messages to figure out *why* those DRAM addresses want
to be logged. But it seems they don't want to be logged. Because there's not
a single reason why they should be, AFAICT. Without a proper justification,
this looks like a bunch of unnecessary code to me...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Incorporate DRAM address in EDAC messages
2025-10-13 22:00 ` [PATCH v3 0/2] Incorporate DRAM address in EDAC messages Borislav Petkov
@ 2025-10-14 13:52 ` Yazen Ghannam
2025-10-14 17:13 ` Naik, Avadhut
0 siblings, 1 reply; 8+ messages in thread
From: Yazen Ghannam @ 2025-10-14 13:52 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Avadhut Naik, linux-edac, john.allen, linux-kernel
On Tue, Oct 14, 2025 at 12:00:19AM +0200, Borislav Petkov wrote:
> On Mon, Oct 13, 2025 at 07:34:47PM +0000, Avadhut Naik wrote:
> > Currently, the amd64_edac module only provides UMC normalized and system
> > physical address when a DRAM ECC error occurs. DRAM Address is neither
> > logged nor exported through tracepoint.
> >
> > Modern AMD SOCs provide UEFI PRM module that implements various address
> > translation PRM handlers. These PRM handlers can be leveraged to convert
> > UMC normalized address into DRAM address at runtime on occurrence of a
> > DRAM ECC error. This translated DRAM address can then be logged and
> > exported through tracepoints.
>
> And?
>
> I read all three commit messages to figure out *why* those DRAM addresses want
> to be logged. But it seems they don't want to be logged. Because there's not
> a single reason why they should be, AFAICT. Without a proper justification,
> this looks like a bunch of unnecessary code to me...
>
Good point. I overlooked this myself.
The "DRAM address" helps memory vendors analyze failures. System
builders want to collect this data and pass it along to the memory
vendors. The DRAM address is not contained in architectural data like
MCA info, and getting the address from MCA requires using additional
system-specific hardware info. It's much more reliable to get the DRAM
address from the system with the error rather than try to post-process
it later.
Thanks,
Yazen
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 0/2] Incorporate DRAM address in EDAC messages
2025-10-14 13:52 ` Yazen Ghannam
@ 2025-10-14 17:13 ` Naik, Avadhut
2025-10-14 17:52 ` Borislav Petkov
0 siblings, 1 reply; 8+ messages in thread
From: Naik, Avadhut @ 2025-10-14 17:13 UTC (permalink / raw)
To: Borislav Petkov
Cc: Avadhut Naik, linux-edac, john.allen, linux-kernel, Yazen Ghannam
On 10/14/2025 08:52, Yazen Ghannam wrote:
> On Tue, Oct 14, 2025 at 12:00:19AM +0200, Borislav Petkov wrote:
>> On Mon, Oct 13, 2025 at 07:34:47PM +0000, Avadhut Naik wrote:
>>> Currently, the amd64_edac module only provides UMC normalized and system
>>> physical address when a DRAM ECC error occurs. DRAM Address is neither
>>> logged nor exported through tracepoint.
>>>
>>> Modern AMD SOCs provide UEFI PRM module that implements various address
>>> translation PRM handlers. These PRM handlers can be leveraged to convert
>>> UMC normalized address into DRAM address at runtime on occurrence of a
>>> DRAM ECC error. This translated DRAM address can then be logged and
>>> exported through tracepoints.
>>
>> And?
>>
>> I read all three commit messages to figure out *why* those DRAM addresses want
>> to be logged. But it seems they don't want to be logged. Because there's not
>> a single reason why they should be, AFAICT. Without a proper justification,
>> this looks like a bunch of unnecessary code to me...
>>
>
> Good point. I overlooked this myself.
>
> The "DRAM address" helps memory vendors analyze failures. System
> builders want to collect this data and pass it along to the memory
> vendors. The DRAM address is not contained in architectural data like
> MCA info, and getting the address from MCA requires using additional
> system-specific hardware info. It's much more reliable to get the DRAM
> address from the system with the error rather than try to post-process
> it later.
>
Adding to what Yazen mentioned, the algorithm employed for translating
physical or normalized address into DRAM Address is somewhat complex.
As such, user space tools might not incorporate the support required
for translation.
Having DRAM Address as part of kernel messages and tracepoints ensures
that any memory error related information is not skipped.
Does this alleviate your concerns?
If yes, will add this information to commit messages and resend.
> Thanks,
> Yazen
--
Thanks,
Avadhut Naik
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Incorporate DRAM address in EDAC messages
2025-10-14 17:13 ` Naik, Avadhut
@ 2025-10-14 17:52 ` Borislav Petkov
2025-10-14 19:15 ` Naik, Avadhut
0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2025-10-14 17:52 UTC (permalink / raw)
To: Naik, Avadhut, Yazen Ghannam
Cc: Avadhut Naik, linux-edac, john.allen, linux-kernel
On Tue, Oct 14, 2025 at 12:13:36PM -0500, Naik, Avadhut wrote:
> > The "DRAM address" helps memory vendors analyze failures. System
> > builders want to collect this data and pass it along to the memory
> > vendors.
How real is such a use case? It sounds to me like wishful thinking and that no
one is going to use it in the end and we'll end up warming up the universe
with electrons needlessly...
> > The DRAM address is not contained in architectural data like
> > MCA info, and getting the address from MCA requires using additional
> > system-specific hardware info. It's much more reliable to get the DRAM
> > address from the system with the error rather than try to post-process
> > it later.
Ok, a bit better.
Now, why isn't that address part of the tracepoint so that system builders can
consume structured data instead of parsing scnprintf()-ed strings and trying
to guess what's there?
Also, some of the fields of TRACE_EVENT(mce_record already contain the fields
this set is adding - CS or so, for example. So there's redundancy already.
> If yes, will add this information to commit messages and resend.
When that happens, remove all text gunk which talks about what a patch does
- that should be visible from the diff.
And this is not the first time I'm saying this: folks, please stop explaining
the code.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 0/2] Incorporate DRAM address in EDAC messages
2025-10-14 17:52 ` Borislav Petkov
@ 2025-10-14 19:15 ` Naik, Avadhut
0 siblings, 0 replies; 8+ messages in thread
From: Naik, Avadhut @ 2025-10-14 19:15 UTC (permalink / raw)
To: Borislav Petkov
Cc: Avadhut Naik, linux-edac, john.allen, linux-kernel, Yazen Ghannam
On 10/14/2025 12:52, Borislav Petkov wrote:
> On Tue, Oct 14, 2025 at 12:13:36PM -0500, Naik, Avadhut wrote:
>>> The "DRAM address" helps memory vendors analyze failures. System
>>> builders want to collect this data and pass it along to the memory
>>> vendors.
>
> How real is such a use case? It sounds to me like wishful thinking and that no
> one is going to use it in the end and we'll end up warming up the universe
> with electrons needlessly...
>
>>> The DRAM address is not contained in architectural data like
>>> MCA info, and getting the address from MCA requires using additional
>>> system-specific hardware info. It's much more reliable to get the DRAM
>>> address from the system with the error rather than try to post-process
>>> it later.
>
> Ok, a bit better.
>
> Now, why isn't that address part of the tracepoint so that system builders can
> consume structured data instead of parsing scnprintf()-ed strings and trying
> to guess what's there?
>
> Also, some of the fields of TRACE_EVENT(mce_record already contain the fields
> this set is adding - CS or so, for example. So there's redundancy already.
>
Currently, it is being exported through the RAS tracepoint along with the physical
address. Example snippet below:
kworker/4:1-3950 [004] ..... 84373.064068: mc_event: 1 Corrected error: on mc#0csrow#0channel#9 (mc:0 location:0:9:-1 address:0x9ffff000 grain:64 syndrome:0x00000001 Cs: 0x0 Bank Grp: 0x0 Bank Addr: 0x1f Row: 0x27f Column: 0x7e0 RankMul: 0x0 SubChannel: 0x0)
Would you rather have it exported through the mce_record tracepoint?
>> If yes, will add this information to commit messages and resend.
>
> When that happens, remove all text gunk which talks about what a patch does
> - that should be visible from the diff.
>
> And this is not the first time I'm saying this: folks, please stop explaining
> the code.
>
Will do.
> Thx.
>
--
Thanks,
Avadhut Naik
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-10-14 19:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-13 19:34 [PATCH v3 0/2] Incorporate DRAM address in EDAC messages Avadhut Naik
2025-10-13 19:34 ` [PATCH v3 1/2] RAS/AMD/ATL: Translate UMC normalized address to DRAM address using PRM Avadhut Naik
2025-10-13 19:34 ` [PATCH v3 2/2] EDAC/amd64: Incorporate DRAM Address in EDAC message Avadhut Naik
2025-10-13 22:00 ` [PATCH v3 0/2] Incorporate DRAM address in EDAC messages Borislav Petkov
2025-10-14 13:52 ` Yazen Ghannam
2025-10-14 17:13 ` Naik, Avadhut
2025-10-14 17:52 ` Borislav Petkov
2025-10-14 19:15 ` Naik, Avadhut
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®