Re: [PATCH v2 3/3] powerpc/mce: Handle memcpy_mcsafe

[Reza Arbab]

On Thu, Apr 05, 2018 at 05:15:00PM +1000, Balbir Singh wrote:

Add a blocking notifier callback to be called in real-mode
on machine check exceptions for UE (ld/st) errors only.


It's been a while, but is this patchset still being pursued?

This patch in particular (callbacks for MCE handling) has other device 
memory use cases and I'd like to move it along.



The patch registers a callback on boot to be notified
of machine check exceptions and returns a NOTIFY_STOP when
a page of interest is seen as the source of the machine
check exception. This page of interest is a ZONE_DEVICE
page and hence for now, for memcpy_mcsafe to work, the page
needs to belong to ZONE_DEVICE and memcpy_mcsafe should be
used to access the memory.

The patch also modifies the NIP of the exception context
to go back to the fixup handler (in memcpy_mcsafe) and does
not print any error message as the error is treated as
returned via a return value and handled.

Signed-off-by: Balbir Singh 
---
arch/powerpc/include/asm/mce.h |  3 +-
arch/powerpc/kernel/mce.c  | 77 --
2 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 3a1226e9b465..a76638e3e47e 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -125,7 +125,8 @@ struct machine_check_event {
enum MCE_UeErrorType ue_error_type:8;
uint8_t effective_address_provided;
uint8_t physical_address_provided;
-   uint8_t reserved_1[5];
+   uint8_t error_return;
+   uint8_t reserved_1[4];
uint64_teffective_address;
uint64_tphysical_address;
uint8_t reserved_2[8];
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index efdd16a79075..b9e4881fa8c5 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -28,7 +28,9 @@
#include 
#include 
#include 
+#include 

+#include 
#include 
#include 

@@ -54,6 +56,52 @@ static struct irq_work mce_event_process_work = {

DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);

+static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
+
+int register_mce_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_register(_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_mce_notifier);
+
+int unregister_mce_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_unregister(_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_mce_notifier);
+
+
+static int check_memcpy_mcsafe(struct notifier_block *nb,
+   unsigned long val, void *data)
+{
+   /*
+* val contains the physical_address of the bad address
+*/
+   unsigned long pfn = val >> PAGE_SHIFT;
+   struct page *page = realmode_pfn_to_page(pfn);
+   int rc = NOTIFY_DONE;
+
+   if (!page)
+   goto out;
+
+   if (is_zone_device_page(page))  /* for HMM and PMEM */
+   rc = NOTIFY_STOP;
+out:
+   return rc;
+}
+
+struct notifier_block memcpy_mcsafe_nb = {
+   .priority = 0,
+   .notifier_call = check_memcpy_mcsafe,
+};
+
+int  mce_mcsafe_register(void)
+{
+   register_mce_notifier(_mcsafe_nb);
+   return 0;
+}
+arch_initcall(mce_mcsafe_register);
+
static void mce_set_error_info(struct machine_check_event *mce,
   struct mce_error_info *mce_err)
{
@@ -151,9 +199,31 @@ void save_mce_event(struct pt_regs *regs, long handled,
mce->u.ue_error.effective_address_provided = true;
mce->u.ue_error.effective_address = addr;
if (phys_addr != ULONG_MAX) {
+   int rc;
+   const struct exception_table_entry *entry;
+
+   /*
+* Once we have the physical address, we check to
+* see if the current nip has a fixup entry.
+* Having a fixup entry plus the notifier stating
+* that it can handle the exception is an indication
+* that we should return to the fixup entry and
+* return an error from there
+*/
mce->u.ue_error.physical_address_provided = true;
mce->u.ue_error.physical_address = phys_addr;
-   machine_check_ue_event(mce);
+
+   rc = blocking_notifier_call_chain(_notifier_list,
+   phys_addr, NULL);


Could we pass mce entirely here instead of just phys_addr? It would 
allow the callback itself to set error_return if needed.



+   if (rc & NOTIFY_STOP_MASK) {
+

[PATCH v2 3/3] powerpc/mce: Handle memcpy_mcsafe

[Balbir Singh]

Add a blocking notifier callback to be called in real-mode
on machine check exceptions for UE (ld/st) errors only.
The patch registers a callback on boot to be notified
of machine check exceptions and returns a NOTIFY_STOP when
a page of interest is seen as the source of the machine
check exception. This page of interest is a ZONE_DEVICE
page and hence for now, for memcpy_mcsafe to work, the page
needs to belong to ZONE_DEVICE and memcpy_mcsafe should be
used to access the memory.

The patch also modifies the NIP of the exception context
to go back to the fixup handler (in memcpy_mcsafe) and does
not print any error message as the error is treated as
returned via a return value and handled.

Signed-off-by: Balbir Singh 
---
 arch/powerpc/include/asm/mce.h |  3 +-
 arch/powerpc/kernel/mce.c  | 77 --
 2 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 3a1226e9b465..a76638e3e47e 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -125,7 +125,8 @@ struct machine_check_event {
enum MCE_UeErrorType ue_error_type:8;
uint8_t effective_address_provided;
uint8_t physical_address_provided;
-   uint8_t reserved_1[5];
+   uint8_t error_return;
+   uint8_t reserved_1[4];
uint64_teffective_address;
uint64_tphysical_address;
uint8_t reserved_2[8];
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index efdd16a79075..b9e4881fa8c5 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -28,7 +28,9 @@
 #include 
 #include 
 #include 
+#include 
 
+#include 
 #include 
 #include 
 
@@ -54,6 +56,52 @@ static struct irq_work mce_event_process_work = {
 
 DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
 
+static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
+
+int register_mce_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_register(_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_mce_notifier);
+
+int unregister_mce_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_unregister(_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_mce_notifier);
+
+
+static int check_memcpy_mcsafe(struct notifier_block *nb,
+   unsigned long val, void *data)
+{
+   /*
+* val contains the physical_address of the bad address
+*/
+   unsigned long pfn = val >> PAGE_SHIFT;
+   struct page *page = realmode_pfn_to_page(pfn);
+   int rc = NOTIFY_DONE;
+
+   if (!page)
+   goto out;
+
+   if (is_zone_device_page(page))  /* for HMM and PMEM */
+   rc = NOTIFY_STOP;
+out:
+   return rc;
+}
+
+struct notifier_block memcpy_mcsafe_nb = {
+   .priority = 0,
+   .notifier_call = check_memcpy_mcsafe,
+};
+
+int  mce_mcsafe_register(void)
+{
+   register_mce_notifier(_mcsafe_nb);
+   return 0;
+}
+arch_initcall(mce_mcsafe_register);
+
 static void mce_set_error_info(struct machine_check_event *mce,
   struct mce_error_info *mce_err)
 {
@@ -151,9 +199,31 @@ void save_mce_event(struct pt_regs *regs, long handled,
mce->u.ue_error.effective_address_provided = true;
mce->u.ue_error.effective_address = addr;
if (phys_addr != ULONG_MAX) {
+   int rc;
+   const struct exception_table_entry *entry;
+
+   /*
+* Once we have the physical address, we check to
+* see if the current nip has a fixup entry.
+* Having a fixup entry plus the notifier stating
+* that it can handle the exception is an indication
+* that we should return to the fixup entry and
+* return an error from there
+*/
mce->u.ue_error.physical_address_provided = true;
mce->u.ue_error.physical_address = phys_addr;
-   machine_check_ue_event(mce);
+
+   rc = blocking_notifier_call_chain(_notifier_list,
+   phys_addr, NULL);
+   if (rc & NOTIFY_STOP_MASK) {
+   entry = search_exception_tables(regs->nip);
+   if (entry != NULL) {
+   mce->u.ue_error.error_return = 1;
+   regs->nip = extable_fixup(entry);
+   } else
+