Re: [RESEND PATCH v18 1/2] ACPI: APEI: send SIGBUS to current task if synchronous memory error not recovered

[Shuai Xue <xueshuai@linux.alibaba.com>]

在 2025/4/25 09:00, Hanjun Guo 写道:
> On 2025/4/18 20:35, Shuai Xue wrote:
>>
>>
>> 在 2025/4/18 15:48, Hanjun Guo 写道:
>>> On 2025/4/14 23:02, Shuai Xue wrote:
>>>>
>>>>
>>>> 在 2025/4/14 22:37, Hanjun Guo 写道:
>>>>> On 2025/4/4 19:20, Shuai Xue wrote:
>>>>>> Synchronous error was detected as a result of user-space process accessing
>>>>>> a 2-bit uncorrected error. The CPU will take a synchronous error exception
>>>>>> such as Synchronous External Abort (SEA) on Arm64. The kernel will queue a
>>>>>> memory_failure() work which poisons the related page, unmaps the page, and
>>>>>> then sends a SIGBUS to the process, so that a system wide panic can be
>>>>>> avoided.
>>>>>>
>>>>>> However, no memory_failure() work will be queued when abnormal synchronous
>>>>>> errors occur. These errors can include situations such as invalid PA,
>>>>>> unexpected severity, no memory failure config support, invalid GUID
>>>>>> section, etc. In such case, the user-space process will trigger SEA again.
>>>>>> This loop can potentially exceed the platform firmware threshold or even
>>>>>> trigger a kernel hard lockup, leading to a system reboot.
>>>>>>
>>>>>> Fix it by performing a force kill if no memory_failure() work is queued
>>>>>> for synchronous errors.
>>>>>>
>>>>>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>>>>>> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>>>>>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>>>>> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
>>>>>> Reviewed-by: Jane Chu <jane.chu@oracle.com>
>>>>>> ---
>>>>>>   drivers/acpi/apei/ghes.c | 11 +++++++++++
>>>>>>   1 file changed, 11 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>>>>>> index b72772494655..50e4d924aa8b 100644
>>>>>> --- a/drivers/acpi/apei/ghes.c
>>>>>> +++ b/drivers/acpi/apei/ghes.c
>>>>>> @@ -799,6 +799,17 @@ static bool ghes_do_proc(struct ghes *ghes,
>>>>>>           }
>>>>>>       }
>>>>>> +    /*
>>>>>> +     * If no memory failure work is queued for abnormal synchronous
>>>>>> +     * errors, do a force kill.
>>>>>> +     */
>>>>>> +    if (sync && !queued) {
>>>>>> +        dev_err(ghes->dev,
>>>>>> +            HW_ERR GHES_PFX "%s:%d: synchronous unrecoverable error (SIGBUS)\n",
>>>>>> +            current->comm, task_pid_nr(current));
>>>>>> +        force_sig(SIGBUS);
>>>>>> +    }
>>>>>
>>>>> I think it's reasonable to send a force kill to the task when the
>>>>> synchronous memory error is not recovered.
>>>>>
>>>>> But I hope this code will not trigger some legacy firmware issues,
>>>>> let's be careful for this, so can we just introduce arch specific
>>>>> callbacks for this?
>>>>
>>>> Sorry, can you give more details? I am not sure I got your point.
>>>>
>>>> For x86, Tony confirmed that ghes will not dispatch x86 synchronous errors
>>>> (a.k.a machine check exception), in previous vesion.
>>>> Sync is only used in arm64 platform, see is_hest_sync_notify().
>>>
>>> Sorry for the late reply, from the code I can see that x86 will reuse
>>> ghes_do_proc(), if Tony confirmed that x86 is OK, it's OK to me as well.
>>
>> Hi, Hanjun,
>>
>> Glad to hear that.
>>
>> I copy and paste in the original disscusion with @Tony from mailist.[1]
>>
>>> On x86 the "action required" cases are signaled by a synchronous machine check
>>> that is delivered before the instruction that is attempting to consume the uncorrected
>>> data retires. I.e., it is guaranteed that the uncorrected error has not been propagated
>>> because it is not visible in any architectural state.
>>
>>> APEI signaled errors don't fall into that category on x86 ... the uncorrected data
>>> could have been consumed and propagated long before the signaling used for
>>> APEI can alert the OS.
>>
>> I also add comments in the code.
>>
>> /*
>>   * A platform may describe one error source for the handling of synchronous
>>   * errors (e.g. MCE or SEA), or for handling asynchronous errors (e.g. SCI
>>   * or External Interrupt). On x86, the HEST notifications are always
>>   * asynchronous, so only SEA on ARM is delivered as a synchronous
>>   * notification.
>>   */
>> static inline bool is_hest_sync_notify(struct ghes *ghes)
>> {
>>      u8 notify_type = ghes->generic->notify.type;
>>
>>      return notify_type == ACPI_HEST_NOTIFY_SEA;
>> }
>>
>>
>> If you are happy with code, please explictly give me your reviewed-by tags :)
> 
> Call force_sig(SIGBUS) directly in ghes_do_proc() is not my favourite,
> but I can bear that, please add
> 
> Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
> 
> Thanks
> Hanjun

Thanks. Hanjun.

@Rafael, @Catalin,

Both patch 1 and 2 have reviewed-by tag from the arm64 ACPI maintainers, Hanjun,
now. Are you happpy to pick and queue this patch set to acpi tree or arm tree?

If you need me to send a new version to collect the reviewed-by tag, please
let me know.

Thanks.

Best Regards,
Shuai