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

[Pierre Gondois <pierre.gondois@arm.com>]

On 11/4/26 18:51, Mark Brown wrote:
> On Wed, Nov 04, 2026 at 12:03:14PM +0100, Pierre Gondois 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.
>> @@ -1008,6 +1021,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");
> The v1 code uses pr_warn_ratelimited() which seems sensible.
yes right
>
>> +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;
>> +	/* 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);
> Do we need to raise the size of e->location?  It's 256 bytes and
> there's an awful ot of potential fields that could be decoded here,
> including some with 64 bit IDs.

cper_mem_err2_location()
node:0000 card:0000 module:0000 rank:0000 bank:0000 bank_group:0000 
bank_address:0000 device:0000 row:0000 column:0000 bit_position:0000 
requestor_id:0x0000000000000000 responder_id:0x0000000000000000 
target_id:0x0000000000000000 chip_id:0000
~244 chars

cper_dimm_err2_location()
DIMM location: not present. DMI handle: 0x00000000
~51 chars
DIMM location: %s %s
~depends on string length

So indeed LOCATION_SIZE should be increased,
512 chars should be better.


>> +		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);
> There's an equivalent print in cper_dimm_err2_location().
Right, I ll remove this one