| Message ID | 20240204080144.7977-2-xueshuai@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v11,1/3] ACPI: APEI: send SIGBUS to current task if synchronous memory error not recovered | expand |
On Sun, Feb 04, 2024 at 04:01:42PM +0800, 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> > --- > drivers/acpi/apei/ghes.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index 7b7c605166e0..0892550732d4 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -806,6 +806,15 @@ 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) { > + pr_err("Sending SIGBUS to current task due to memory error not recovered"); > + force_sig(SIGBUS); > + } Except that there are a bunch of CXL GUIDs being handled there too and this will sigbus those processes now automatically. Lemme add the whole bunch from 671a794c33c6 ("acpi/ghes: Process CXL Component Events") for comment to Cc.
On 2024/2/19 17:25, Borislav Petkov wrote: > On Sun, Feb 04, 2024 at 04:01:42PM +0800, 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> >> --- >> drivers/acpi/apei/ghes.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c >> index 7b7c605166e0..0892550732d4 100644 >> --- a/drivers/acpi/apei/ghes.c >> +++ b/drivers/acpi/apei/ghes.c >> @@ -806,6 +806,15 @@ 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) { >> + pr_err("Sending SIGBUS to current task due to memory error not recovered"); >> + force_sig(SIGBUS); >> + } > > Except that there are a bunch of CXL GUIDs being handled there too and > this will sigbus those processes now automatically. Before the CXL GUIDs added, @Tony confirmed that the HEST notifications are always asynchronous on x86 platform, so only Synchronous External Abort (SEA) on ARM is delivered as a synchronous notification. Will the CXL component trigger synchronous events for which we need to terminate the current process by sending sigbus to process? > > Lemme add the whole bunch from > > 671a794c33c6 ("acpi/ghes: Process CXL Component Events") > > for comment to Cc. > Thank you. Best Regards, Shuai
Shuai Xue wrote: > > > On 2024/2/19 17:25, Borislav Petkov wrote: > > On Sun, Feb 04, 2024 at 04:01:42PM +0800, 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> > >> --- > >> drivers/acpi/apei/ghes.c | 9 +++++++++ > >> 1 file changed, 9 insertions(+) > >> > >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > >> index 7b7c605166e0..0892550732d4 100644 > >> --- a/drivers/acpi/apei/ghes.c > >> +++ b/drivers/acpi/apei/ghes.c > >> @@ -806,6 +806,15 @@ 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) { > >> + pr_err("Sending SIGBUS to current task due to memory error not recovered"); > >> + force_sig(SIGBUS); > >> + } > > > > Except that there are a bunch of CXL GUIDs being handled there too and > > this will sigbus those processes now automatically. > > Before the CXL GUIDs added, @Tony confirmed that the HEST notifications are always > asynchronous on x86 platform, so only Synchronous External Abort (SEA) on ARM is > delivered as a synchronous notification. > > Will the CXL component trigger synchronous events for which we need to terminate the > current process by sending sigbus to process? None of the CXL component errors should be handled as synchronous events. They are either asynchronous protocol errors, or effectively equivalent to CPER_SEC_PLATFORM_MEM notifications.
On Thu, 22 Feb 2024 21:26:43 -0800 Dan Williams <dan.j.williams@intel.com> wrote: > Shuai Xue wrote: > > > > > > On 2024/2/19 17:25, Borislav Petkov wrote: > > > On Sun, Feb 04, 2024 at 04:01:42PM +0800, 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> > > >> --- > > >> drivers/acpi/apei/ghes.c | 9 +++++++++ > > >> 1 file changed, 9 insertions(+) > > >> > > >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > > >> index 7b7c605166e0..0892550732d4 100644 > > >> --- a/drivers/acpi/apei/ghes.c > > >> +++ b/drivers/acpi/apei/ghes.c > > >> @@ -806,6 +806,15 @@ 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) { > > >> + pr_err("Sending SIGBUS to current task due to memory error not recovered"); > > >> + force_sig(SIGBUS); > > >> + } > > > > > > Except that there are a bunch of CXL GUIDs being handled there too and > > > this will sigbus those processes now automatically. > > > > Before the CXL GUIDs added, @Tony confirmed that the HEST notifications are always > > asynchronous on x86 platform, so only Synchronous External Abort (SEA) on ARM is > > delivered as a synchronous notification. > > > > Will the CXL component trigger synchronous events for which we need to terminate the > > current process by sending sigbus to process? > > None of the CXL component errors should be handled as synchronous > events. They are either asynchronous protocol errors, or effectively > equivalent to CPER_SEC_PLATFORM_MEM notifications. Not a good example, CPER_SEC_PLATFORM_MEM is sometimes signaled via SEA.
On Fri, 23 Feb 2024 12:08:13 +0000 Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote: > On Thu, 22 Feb 2024 21:26:43 -0800 > Dan Williams <dan.j.williams@intel.com> wrote: > > > Shuai Xue wrote: > > > > > > > > > On 2024/2/19 17:25, Borislav Petkov wrote: > > > > On Sun, Feb 04, 2024 at 04:01:42PM +0800, 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> > > > >> --- > > > >> drivers/acpi/apei/ghes.c | 9 +++++++++ > > > >> 1 file changed, 9 insertions(+) > > > >> > > > >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > > > >> index 7b7c605166e0..0892550732d4 100644 > > > >> --- a/drivers/acpi/apei/ghes.c > > > >> +++ b/drivers/acpi/apei/ghes.c > > > >> @@ -806,6 +806,15 @@ 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) { > > > >> + pr_err("Sending SIGBUS to current task due to memory error not recovered"); > > > >> + force_sig(SIGBUS); > > > >> + } > > > > > > > > Except that there are a bunch of CXL GUIDs being handled there too and > > > > this will sigbus those processes now automatically. > > > > > > Before the CXL GUIDs added, @Tony confirmed that the HEST notifications are always > > > asynchronous on x86 platform, so only Synchronous External Abort (SEA) on ARM is > > > delivered as a synchronous notification. > > > > > > Will the CXL component trigger synchronous events for which we need to terminate the > > > current process by sending sigbus to process? > > > > None of the CXL component errors should be handled as synchronous > > events. They are either asynchronous protocol errors, or effectively > > equivalent to CPER_SEC_PLATFORM_MEM notifications. > > Not a good example, CPER_SEC_PLATFORM_MEM is sometimes signaled via SEA. > Premature send.:( One example I can point at is how we do signaling of memory errors detected by the host into a VM on arm64. https://elixir.bootlin.com/qemu/latest/source/hw/acpi/ghes.c#L391 CPER_SEC_PLATFORM_MEM via ARM Synchronous External Abort (SEA). Right now we've only used async in QEMU for proposed CXL error CPER records signalling but your reference to them being similar to CPER_SEC_PLATFORM_MEM is valid so 'maybe' they will be synchronous in some physical systems as it's one viable way to provide rich information for synchronous reception of poison. For the VM case my assumption today is we don't care about providing the VM with rich data, so CPER_SEC_PLATFORM_MEM is fine as a path for errors whether from CXL CPER records or not. Jonathan
On 2024/2/23 20:17, Jonathan Cameron wrote: > On Fri, 23 Feb 2024 12:08:13 +0000 > Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote: > >> On Thu, 22 Feb 2024 21:26:43 -0800 >> Dan Williams <dan.j.williams@intel.com> wrote: >> >>> Shuai Xue wrote: >>>> >>>> >>>> On 2024/2/19 17:25, Borislav Petkov wrote: >>>>> On Sun, Feb 04, 2024 at 04:01:42PM +0800, 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> >>>>>> --- >>>>>> drivers/acpi/apei/ghes.c | 9 +++++++++ >>>>>> 1 file changed, 9 insertions(+) >>>>>> >>>>>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c >>>>>> index 7b7c605166e0..0892550732d4 100644 >>>>>> --- a/drivers/acpi/apei/ghes.c >>>>>> +++ b/drivers/acpi/apei/ghes.c >>>>>> @@ -806,6 +806,15 @@ 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) { >>>>>> + pr_err("Sending SIGBUS to current task due to memory error not recovered"); >>>>>> + force_sig(SIGBUS); >>>>>> + } >>>>> >>>>> Except that there are a bunch of CXL GUIDs being handled there too and >>>>> this will sigbus those processes now automatically. >>>> >>>> Before the CXL GUIDs added, @Tony confirmed that the HEST notifications are always >>>> asynchronous on x86 platform, so only Synchronous External Abort (SEA) on ARM is >>>> delivered as a synchronous notification. >>>> >>>> Will the CXL component trigger synchronous events for which we need to terminate the >>>> current process by sending sigbus to process? >>> >>> None of the CXL component errors should be handled as synchronous >>> events. They are either asynchronous protocol errors, or effectively >>> equivalent to CPER_SEC_PLATFORM_MEM notifications. >> >> Not a good example, CPER_SEC_PLATFORM_MEM is sometimes signaled via SEA. >> > > Premature send.:( > > One example I can point at is how we do signaling of memory > errors detected by the host into a VM on arm64. > https://elixir.bootlin.com/qemu/latest/source/hw/acpi/ghes.c#L391 > CPER_SEC_PLATFORM_MEM via ARM Synchronous External Abort (SEA). > > Right now we've only used async in QEMU for proposed CXL error > CPER records signalling but your reference to them being similar > to CPER_SEC_PLATFORM_MEM is valid so 'maybe' they will be > synchronous in some physical systems as it's one viable way to > provide rich information for synchronous reception of poison. > For the VM case my assumption today is we don't care about providing the > VM with rich data, so CPER_SEC_PLATFORM_MEM is fine as a path for > errors whether from CXL CPER records or not. > > Jonathan Thank you for your confirmation and explanation. So I think the condition: - `sync` for synchronous event, - `!queued` for CPER_SEC_PLATFORM_MEM notifications which do not handle memory failures. is fine. @Borislav, do you have any other concerns? Best Regards, Shuai
Borislav Petkov wrote: > On Sun, Feb 04, 2024 at 04:01:42PM +0800, 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> > > --- > > drivers/acpi/apei/ghes.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > > index 7b7c605166e0..0892550732d4 100644 > > --- a/drivers/acpi/apei/ghes.c > > +++ b/drivers/acpi/apei/ghes.c > > @@ -806,6 +806,15 @@ 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) { > > + pr_err("Sending SIGBUS to current task due to memory error not recovered"); > > + force_sig(SIGBUS); > > + } > > Except that there are a bunch of CXL GUIDs being handled there too and > this will sigbus those processes now automatically. > > Lemme add the whole bunch from > > 671a794c33c6 ("acpi/ghes: Process CXL Component Events") > > for comment to Cc. BTW, I am about to revert all the CXL GUIDs for v6.8 to try again for v6.9: http://lore.kernel.org/r/170820177849.631006.8893584762602010898.stgit@dwillia2-xfh.jf.intel.com
Jonathan Cameron wrote: [..] > > > None of the CXL component errors should be handled as synchronous > > > events. They are either asynchronous protocol errors, or effectively > > > equivalent to CPER_SEC_PLATFORM_MEM notifications. > > > > Not a good example, CPER_SEC_PLATFORM_MEM is sometimes signaled via SEA. > > > > Premature send.:( > > One example I can point at is how we do signaling of memory > errors detected by the host into a VM on arm64. > https://elixir.bootlin.com/qemu/latest/source/hw/acpi/ghes.c#L391 > CPER_SEC_PLATFORM_MEM via ARM Synchronous External Abort (SEA). > > Right now we've only used async in QEMU for proposed CXL error > CPER records signalling but your reference to them being similar > to CPER_SEC_PLATFORM_MEM is valid so 'maybe' they will be > synchronous in some physical systems as it's one viable way to > provide rich information for synchronous reception of poison. > For the VM case my assumption today is we don't care about providing the > VM with rich data, so CPER_SEC_PLATFORM_MEM is fine as a path for > errors whether from CXL CPER records or not. Makes sense... and I was not precise when I mentioned the equivalency, I was only considering x86.
On Sat, Feb 24, 2024 at 02:08:42PM +0800, Shuai Xue wrote:
> @Borislav, do you have any other concerns?
Yes, this change needs to be further reviewed by an ARM person: I have
no clue what those "abnormal synchronous errors" on ARM are and how
they're supposed to be handled properly there:
- what happens if you get such an error when ghes is disabled there?
- is that even the right place to handle them?
James?
On 2024/2/26 18:29, Borislav Petkov wrote: > On Sat, Feb 24, 2024 at 02:08:42PM +0800, Shuai Xue wrote: >> @Borislav, do you have any other concerns? > > Yes, this change needs to be further reviewed by an ARM person: I have > no clue what those "abnormal synchronous errors" on ARM are Hi, Borislav, May the `abnormal` is not inaccurate and misled you. I mean the preconditions check before memory_failure_queue(): - `if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))` in ghes_handle_memory_failure() - `if (flags == -1)` in ghes_handle_memory_failure() - `if (!IS_ENABLED(CONFIG_ACPI_APEI_MEMORY_FAILURE))` in ghes_do_memory_failure() - `if (!pfn_valid(pfn) && !arch_is_platform_page(physical_addr)) ` in ghes_do_memory_failure() If the preconditions are not passed, 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. > and how > they're supposed to be handled properly there: > > - what happens if you get such an error when ghes is disabled there? If ghes_disable is set, the GHES driver will not be inited by acpi_ghes_init(), so none of error notifications will be handled. IMHO, it is expected. > > - is that even the right place to handle them? > > James? > Leave this to @James. Thank you. Best Regards, Shuai
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 7b7c605166e0..0892550732d4 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -806,6 +806,15 @@ 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) { + pr_err("Sending SIGBUS to current task due to memory error not recovered"); + force_sig(SIGBUS); + } + return queued; }
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> --- drivers/acpi/apei/ghes.c | 9 +++++++++ 1 file changed, 9 insertions(+)