Re: [PATCH v2 5/6] ACPI: APEI: add separate Memory Error Section 2 handlers

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Mon, 19 Oct 2026 22:38:44 +0200
Pierre Gondois <pierre.gondois@arm.com> wrote:

> Add dedicated GHES and EDAC callbacks for Memory Error Section 2
> records. Use a separate GHES notifier chain so existing consumers
> continue to receive legacy memory error sections.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
I think this is is ok. I don't love the dual notifier chains but
right now I also don't have a better idea.

So a somewhat tentative
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Jonathan

> ---
>  drivers/acpi/apei/ghes.c |  49 +++++++++++++++-
>  drivers/edac/ghes_edac.c | 118 +++++++++++++++++++++++++++++++++++++++
>  include/acpi/ghes.h      |   2 +
>  3 files changed, 168 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index fe10ab0e02f6..c7fb15543b56 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -101,6 +101,7 @@
>  #endif
>  
>  static ATOMIC_NOTIFIER_HEAD(ghes_report_chain);
> +static ATOMIC_NOTIFIER_HEAD(ghes_report_mem2_chain);
>  
>  static inline bool is_hest_type_generic_v2(struct ghes *ghes)
>  {
> @@ -551,6 +552,29 @@ static bool ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata,
>  	return false;
>  }
>  
> +static bool ghes_handle_memory_failure2(struct acpi_hest_generic_data *gdata,
> +					int sev, bool sync)
> +{
> +	int flags = -1;
> +	int sec_sev = ghes_severity(gdata->error_severity);
> +	struct cper_sec_mem_err2 *mem_err = acpi_hest_get_payload(gdata);
> +
> +	if (!(mem_err->validation_bits & CPER_MEM2_VALID_PA))
> +		return false;
> +
> +	/* iff following two events can be handled properly by now */
> +	if (sec_sev == GHES_SEV_CORRECTED &&
> +	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED))
> +		flags = MF_SOFT_OFFLINE;
> +	if (sev == GHES_SEV_RECOVERABLE && sec_sev == GHES_SEV_RECOVERABLE)
> +		flags = sync ? MF_ACTION_REQUIRED : 0;
> +
> +	if (flags != -1)
> +		return ghes_do_memory_failure(mem_err->physical_addr, flags);
> +
> +	return false;
> +}
> +
>  static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
>  				     int sev, bool sync)
>  {
> @@ -915,7 +939,8 @@ static void ghes_log_hwerr(int sev, guid_t *sec_type)
>  		return;
>  	}
>  
> -	if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM)) {
> +	if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM) ||
> +	    guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM2)) {
>  		hwerr_log_error_type(HWERR_RECOV_MEMORY);
>  		return;
>  	}
> @@ -952,6 +977,16 @@ static void ghes_do_proc(struct ghes *ghes,
>  
>  			arch_apei_report_mem_error(sev, mem_err);
>  			queued = ghes_handle_memory_failure(gdata, sev, sync);
> +		} else if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM2)) {
> +			struct cper_sec_mem_err2 *mem_err = acpi_hest_get_payload(gdata);
> +
> +			if (gdata->error_data_length < sizeof(*mem_err)) {
> +				pr_warn(FW_WARN "Memory Error Section 2 length is too small\n");
> +				continue;
> +			}
> +
> +			atomic_notifier_call_chain(&ghes_report_mem2_chain, sev, mem_err);
> +			queued = ghes_handle_memory_failure2(gdata, sev, sync);
>  		} else if (guid_equal(sec_type, &CPER_SEC_PCIE)) {
>  			ghes_handle_aer(gdata);
>  		} else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) {
> @@ -1973,3 +2008,15 @@ void ghes_unregister_report_chain(struct notifier_block *nb)
>  	atomic_notifier_chain_unregister(&ghes_report_chain, nb);
>  }
>  EXPORT_SYMBOL_GPL(ghes_unregister_report_chain);
> +
> +void ghes_register_report_mem2_chain(struct notifier_block *nb)
> +{
> +	atomic_notifier_chain_register(&ghes_report_mem2_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(ghes_register_report_mem2_chain);
> +
> +void ghes_unregister_report_mem2_chain(struct notifier_block *nb)
> +{
> +	atomic_notifier_chain_unregister(&ghes_report_mem2_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(ghes_unregister_report_mem2_chain);
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
> index b90150c67f4b..01defc4e3bf1 100644
> --- a/drivers/edac/ghes_edac.c
> +++ b/drivers/edac/ghes_edac.c
> @@ -390,6 +390,122 @@ static struct notifier_block ghes_edac_mem_err_nb = {
>  	.priority	= 0,
>  };
>  
> +static int print_mem_error2_other_detail(const struct cper_sec_mem_err2 *mem, char *msg,
> +					 const char *location, unsigned int len)
> +{
> +	u32 n;
> +
> +	if (!msg)
> +		return 0;
> +
> +	n = 0;
> +	len -= 1;
> +
> +	n += scnprintf(msg + n, len - n, "APEI location: %s ", location);
> +
> +	if (mem->validation_bits & CPER_MEM2_VALID_ERROR_STATUS) {
> +		n += scnprintf(msg + n, len - n, "status(0x%016llx): ", mem->error_status);
> +		n += scnprintf(msg + n, len - n, "%s ",
> +			       cper_mem_err_status_str(mem->error_status));
> +	}
> +	if (mem->validation_bits & CPER_MEM2_VALID_STATUS)
> +		n += scnprintf(msg + n, len - n, "section_status:0x%02x ",
> +			       mem->status);
> +
> +	msg[n] = '\0';
> +
> +	return n;
> +}
> +
> +static int ghes_edac_report_mem_error2(struct notifier_block *nb,
> +				       unsigned long val, void *data)
> +{
> +	struct cper_sec_mem_err2 *mem_err = data;
> +	struct edac_raw_error_desc *e;
> +	struct mem_ctl_info *mci;
> +	struct ghes_pvt *pvt;
> +	char *p;
> +
> +	/*
> +	 * We can do the locking below because GHES defers error processing
> +	 * from NMI to IRQ context. Whenever that changes, we'd at least
> +	 * know.
> +	 */
> +	if (WARN_ON_ONCE(in_nmi()))
> +		return NOTIFY_OK;
> +
> +	guard(spinlock_irqsave)(&ghes_lock);
> +
> +	pvt = ghes_pvt;
> +	if (!pvt)
> +		return NOTIFY_OK;
> +
> +	mci = pvt->mci;
> +	e = ghes_edac_init_error(pvt, val);
> +
> +	edac_dbg(1, "error validation_bits: 0x%08llx\n",
> +		 (long long)mem_err->validation_bits);
> +
> +	/* Error type, mapped on e->msg */
> +	if (mem_err->validation_bits & CPER_MEM2_VALID_ERROR_TYPE) {
> +		u8 etype = mem_err->error_type;
> +
> +		p = pvt->msg;
> +		p += snprintf(p, sizeof(pvt->msg), "%s", cper_mem_err_type_str(etype));
> +	} else {
> +		strscpy(pvt->msg, "unknown error");
> +	}
> +
> +	/* Error address */
> +	if (mem_err->validation_bits & CPER_MEM2_VALID_PA) {
> +		e->page_frame_number = PHYS_PFN(mem_err->physical_addr);
> +		e->offset_in_page = offset_in_page(mem_err->physical_addr);
> +	}
> +
> +	/* Error grain */
> +	if (mem_err->validation_bits & CPER_MEM2_VALID_PA_MASK)
> +		e->grain = ~mem_err->physical_addr_mask + 1;
> +
> +	/* Memory error location, mapped on e->location */
> +	p = e->location;
> +	p += cper_mem_err2_location(mem_err, p);
> +
> +	if (mem_err->validation_bits & CPER_MEM2_VALID_MODULE_HANDLE) {
> +		struct dimm_info *dimm = NULL;
> +
> +		p += cper_dimm_err2_location(mem_err, p);
> +		if (mem_err->mem_dev_handle <= U16_MAX)
> +			dimm = find_dimm_by_handle(mci, mem_err->mem_dev_handle);
> +		else
> +			pr_warn(FW_WARN "Memory Error Section 2 module handle 0x%x exceeds SMBIOS range\n",
> +				mem_err->mem_dev_handle);
> +		if (dimm) {
> +			e->top_layer = dimm->idx;
> +			strscpy(e->label, dimm->label);
> +		}
> +	}
> +	if (p > e->location)
> +		*(p - 1) = '\0';
> +
> +	if (!*e->label)
> +		strscpy(e->label, "unknown memory");
> +
> +	/* All other fields are mapped on e->other_detail */
> +	p = pvt->other_detail;
> +	p += print_mem_error2_other_detail(mem_err, p, e->location, OTHER_DETAIL_LEN);
> +	if (p > pvt->other_detail)
> +		*(p - 1) = '\0';
> +
> +	edac_raw_mc_handle_error(e);
> +
> +	return NOTIFY_OK;
> +}