| Message ID | 20250404112050.42040-2-xueshuai@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND,v18,1/2] ACPI: APEI: send SIGBUS to current task if synchronous memory error not recovered | expand |
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? Thanks Hanjun
在 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(). > > Thanks > Hanjun Thanks. Shuai
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. Thanks Hanjun
在 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 :) > > Thanks > Hanjun Thanks. Best Regards, Shuai [1] https://lore.kernel.org/lkml/CAJZ5v0hdgxsDiXqOmeqBQoZUQJ1RssM=3jpYpWt3qzy0n2eyaA@mail.gmail.com/t/#u
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); + } + return queued; }