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

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

Hi Ashok,

On 10/24/26 23:50, Ashok Raj wrote:
> Hi Pierre,
>
>
> On Mon, Oct 19, 2026 at 10:38:44PM +0200, 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.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   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
>>
> [snip]
>
>> @@ -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);
> One issue here.
>
>> +		} 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);
> could this accumulate queued with |= ?
>
> queued describes whether any section in this GHES status record queued
> memory-failure work. The synchronous GHES path sends SIGBUS after the
> section loop when `!queued`.
>
> For example, a synchronous record can contain an eligible legacy memory
> section followed by a Memory Error Section 2 record without a valid
> physical address. The legacy section queues memory-failure work and sets
> queued, but the Section 2 handler returns false and this assignment clears
> the previous result. ghes_do_proc() then reaches the `sync && !queued`
> path and sends SIGBUS despite having queued recovery work for the earlier
> section.
>
> This series appears to be based before the queued accumulation fix [1].
> Could the new Memory Error Section 2 branch follow the same pattern?
>
>> +                     queued = ghes_handle_memory_failure2(gdata, sev, sync);
> should be:
>
>          queued |= ghes_handle_memory_failure2(gdata, sev, sync);
>
> Otherwise, an earlier section can queue memory-failure work, while a later
> Section 2 record for which no work is queued clears `queued`. For a
> synchronous GHES record, ghes_do_proc() will then reach `sync && !queued`
> and send SIGBUS despite recovery work having been queued for the earlier
> section.
>
> Thoughts?
>
> [1] https://lore.kernel.org/r/20260925-ghes-queued-accumulate-v1-1-df50553957d4@debian.org/
>
Ok sure, I will rebase the serie and do similar changes