[PATCH v2 0/3] target/arm/kvm: Improve memory error handling

[PATCH v2 0/3] target/arm/kvm: Improve memory error handling

[Gavin Shan]

Currently, there is only one CPER buffer (entry) can be delivered and
acknoledged at once. This conflicts to the scenario where the host and
guest has 64KB and 4KB page size. In this specific scenario, a problematic
host page can affect 16x guest pages, resulting in 16x memory errors
in the worst case. Unfortunately, qemu runs to core dump at (a) because
the previous error isn't acknoledged and current error is no way to be
delivered, shown in the following call trace

  kvm_vcpu_thread_fn
    kvm_cpu_exec
      kvm_arch_on_sigbus_vcpu
        kvm_cpu_synchronize_state
        acpi_ghes_memory_errors         (a)
        kvm_inject_arm_sea | abort

Fix the issue by sending 16x consecutive memory CPER entries for this
specific case in one shot. With the series applied on top of Mauro's
(v9) series [1], no qemu core dump is observed in the test where guest
memory access is triggered by 'victimd' and the recoverable memory error
is injected from the host.

[1] [PATCH v9 00/20] Change ghes to use HEST-based offsets and add support for error inject
    (https://lists.nongnu.org/archive/html/qemu-arm/2025-05/msg01104.html)

Changelog
=========
v2:
  * v1: https://lists.nongnu.org/archive/html/qemu-arm/2025-02/msg00897.html
  * Send 16x memory errors for the specific case                 (Jonathan)

Gavin Shan (3):
  acpi/ghes: Extend acpi_ghes_memory_errors() to support multiple CPERs
  kvm/arm/kvm: Introduce helper push_ghes_memory_errors()
  target/arm/kvm: Support multiple memory CPERs injection

 hw/acpi/ghes-stub.c    |  2 +-
 hw/acpi/ghes.c         | 29 ++++++++--------
 include/hw/acpi/ghes.h |  2 +-
 target/arm/kvm.c       | 77 +++++++++++++++++++++++++++++++++++++-----
 4 files changed, 86 insertions(+), 24 deletions(-)

-- 
2.49.0

[PATCH v2 1/3] acpi/ghes: Extend acpi_ghes_memory_errors() to support multiple CPERs

[Gavin Shan]

In the situation where host and guest has 64KB and 4KB page sizes, one
error on the host's page affects 16 guest's pages. we need to send 16
consective errors in this specific case.

Extend acpi_ghes_memory_errors() to support multiple CPERs after the
hunk of code to generate the GHES error status is pulled out from
ghes_gen_err_data_uncorrectable_recoverable().

No functional changes intended.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/acpi/ghes-stub.c    |  2 +-
 hw/acpi/ghes.c         | 27 ++++++++++++++-------------
 include/hw/acpi/ghes.h |  2 +-
 target/arm/kvm.c       |  7 ++++++-
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/hw/acpi/ghes-stub.c b/hw/acpi/ghes-stub.c
index 40f660c246..c1f8c9bec9 100644
--- a/hw/acpi/ghes-stub.c
+++ b/hw/acpi/ghes-stub.c
@@ -12,7 +12,7 @@
 #include "hw/acpi/ghes.h"
 
 int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
-                            uint64_t physical_address)
+                            GArray *addresses)
 {
     return -1;
 }
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
index 1d02ef6dcb..34ff682048 100644
--- a/hw/acpi/ghes.c
+++ b/hw/acpi/ghes.c
@@ -214,18 +214,13 @@ static void acpi_ghes_build_append_mem_cper(GArray *table,
 
 static void
 ghes_gen_err_data_uncorrectable_recoverable(GArray *block,
-                                            const uint8_t *section_type,
-                                            int data_length)
+                                            const uint8_t *section_type)
 {
     /* invalid fru id: ACPI 4.0: 17.3.2.6.1 Generic Error Data,
      * Table 17-13 Generic Error Data Entry
      */
     QemuUUID fru_id = {};
 
-    /* Build the new generic error status block header */
-    acpi_ghes_generic_error_status(block, ACPI_GEBS_UNCORRECTABLE,
-        0, 0, data_length, ACPI_CPER_SEV_RECOVERABLE);
-
     /* Build this new generic error data entry header */
     acpi_ghes_generic_error_data(block, section_type,
         ACPI_CPER_SEV_RECOVERABLE, 0, 0,
@@ -557,19 +552,20 @@ void ghes_record_cper_errors(AcpiGhesState *ags, const void *cper, size_t len,
 }
 
 int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
-                            uint64_t physical_address)
+                            GArray *addresses)
 {
     /* Memory Error Section Type */
     const uint8_t guid[] =
           UUID_LE(0xA5BC1114, 0x6F64, 0x4EDE, 0xB8, 0x63, 0x3E, 0x83, \
                   0xED, 0x7C, 0x83, 0xB1);
     Error *errp = NULL;
-    int data_length;
+    int data_length, i;
     GArray *block;
 
     block = g_array_new(false, true /* clear */, 1);
 
-    data_length = ACPI_GHES_DATA_LENGTH + ACPI_GHES_MEM_CPER_LENGTH;
+    data_length = addresses->len *
+                  (ACPI_GHES_DATA_LENGTH + ACPI_GHES_MEM_CPER_LENGTH);
     /*
      * It should not run out of the preallocated memory if adding a new generic
      * error data entry
@@ -577,10 +573,15 @@ int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
     assert((data_length + ACPI_GHES_GESB_SIZE) <=
             ACPI_GHES_MAX_RAW_DATA_LENGTH);
 
-    ghes_gen_err_data_uncorrectable_recoverable(block, guid, data_length);
-
-    /* Build the memory section CPER for above new generic error data entry */
-    acpi_ghes_build_append_mem_cper(block, physical_address);
+    /* Build the new generic error status block header */
+    acpi_ghes_generic_error_status(block, ACPI_GEBS_UNCORRECTABLE, 0, 0,
+                                   data_length, ACPI_CPER_SEV_RECOVERABLE);
+    for (i = 0; i < addresses->len; i++) {
+        ghes_gen_err_data_uncorrectable_recoverable(block, guid);
+        /* Memory section CPER on top of the generic error data entry */
+        acpi_ghes_build_append_mem_cper(block,
+                                        g_array_index(addresses, uint64_t, i));
+    }
 
     /* Report the error */
     ghes_record_cper_errors(ags, block->data, block->len, source_id, &errp);
diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
index df2ecbf6e4..a8cbc520d5 100644
--- a/include/hw/acpi/ghes.h
+++ b/include/hw/acpi/ghes.h
@@ -99,7 +99,7 @@ void acpi_build_hest(AcpiGhesState *ags, GArray *table_data,
 void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
                           GArray *hardware_errors);
 int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
-                            uint64_t error_physical_addr);
+                            GArray *addresses);
 void ghes_record_cper_errors(AcpiGhesState *ags, const void *cper, size_t len,
                              uint16_t source_id, Error **errp);
 
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index a1492bcaeb..97c1bce692 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -2338,6 +2338,7 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
     ram_addr_t ram_addr;
     hwaddr paddr;
     AcpiGhesState *ags;
+    GArray *addresses;
 
     assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
 
@@ -2346,6 +2347,7 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
         ram_addr = qemu_ram_addr_from_host(addr);
         if (ram_addr != RAM_ADDR_INVALID &&
             kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
+            addresses = g_array_new(false, false, sizeof(paddr));
             kvm_hwpoison_page_add(ram_addr);
             /*
              * If this is a BUS_MCEERR_AR, we know we have been called
@@ -2358,16 +2360,19 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
              * later from the main thread, so doing the injection of
              * the error would be more complicated.
              */
+            g_array_append_vals(addresses, &paddr, 1);
             if (code == BUS_MCEERR_AR) {
                 kvm_cpu_synchronize_state(c);
                 if (!acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC,
-                                             paddr)) {
+                                             addresses)) {
                     kvm_inject_arm_sea(c);
                 } else {
                     error_report("failed to record the error");
                     abort();
                 }
             }
+
+            g_array_free(addresses, true);
             return;
         }
         if (code == BUS_MCEERR_AO) {
-- 
2.49.0

[PATCH v2 2/3] kvm/arm/kvm: Introduce helper push_ghes_memory_errors()

[Gavin Shan]

Introduce helper push_ghes_memory_errors(), which sends ACPI GHES memory
errors, injects SEA exception or aborts on errors. This function will
be extended to support multiple ACPI GHES memory errors in the next
path.

No functional changes intended.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/arm/kvm.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 97c1bce692..e31fcde797 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -2333,12 +2333,34 @@ int kvm_arch_get_registers(CPUState *cs, Error **errp)
     return ret;
 }
 
+static void push_ghes_memory_errors(CPUState *c, AcpiGhesState *ags,
+                                    uint64_t paddr)
+{
+    GArray *addresses = g_array_new(false, false, sizeof(paddr));
+    int ret;
+
+    kvm_cpu_synchronize_state(c);
+    g_array_append_vals(addresses, &paddr, 1);
+    ret = acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC, addresses);
+    if (ret) {
+        goto error;
+    }
+
+    kvm_inject_arm_sea(c);
+
+    g_array_free(addresses, true);
+
+    return;
+error:
+    error_report("failed to record the error");
+    abort();
+}
+
 void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
 {
     ram_addr_t ram_addr;
     hwaddr paddr;
     AcpiGhesState *ags;
-    GArray *addresses;
 
     assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
 
@@ -2347,7 +2369,6 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
         ram_addr = qemu_ram_addr_from_host(addr);
         if (ram_addr != RAM_ADDR_INVALID &&
             kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
-            addresses = g_array_new(false, false, sizeof(paddr));
             kvm_hwpoison_page_add(ram_addr);
             /*
              * If this is a BUS_MCEERR_AR, we know we have been called
@@ -2360,19 +2381,10 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
              * later from the main thread, so doing the injection of
              * the error would be more complicated.
              */
-            g_array_append_vals(addresses, &paddr, 1);
             if (code == BUS_MCEERR_AR) {
-                kvm_cpu_synchronize_state(c);
-                if (!acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC,
-                                             addresses)) {
-                    kvm_inject_arm_sea(c);
-                } else {
-                    error_report("failed to record the error");
-                    abort();
-                }
+                push_ghes_memory_errors(c, ags, paddr);
             }
 
-            g_array_free(addresses, true);
             return;
         }
         if (code == BUS_MCEERR_AO) {
-- 
2.49.0

[PATCH v2 3/3] target/arm/kvm: Support multiple memory CPERs injection

[Gavin Shan]

In the combination of 64KB host and 4KB guest, a problematic host page
affects 16x guest pages. In this specific case, it's reasonable to
push 16 consecutive memory CPERs. Otherwise, QEMU can run into core
dump due to the current error can't be delivered as the previous error
isn't acknoledges. It's caused by the nature the host page can be
accessed in parallel due to the mismatched host and guest page sizes.

Imporve push_ghes_memory_errors() to push 16x consecutive memory CPERs
for this specific case. The maximal error block size is bumped to 4KB,
providing enough storage space for those 16x memory CPERs.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/acpi/ghes.c   |  2 +-
 target/arm/kvm.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
index 34ff682048..43d52f5e2e 100644
--- a/hw/acpi/ghes.c
+++ b/hw/acpi/ghes.c
@@ -33,7 +33,7 @@
 #define ACPI_HEST_ADDR_FW_CFG_FILE          "etc/acpi_table_hest_addr"
 
 /* The max size in bytes for one error block */
-#define ACPI_GHES_MAX_RAW_DATA_LENGTH   (1 * KiB)
+#define ACPI_GHES_MAX_RAW_DATA_LENGTH   (4 * KiB)
 
 /* Generic Hardware Error Source version 2 */
 #define ACPI_GHES_SOURCE_GENERIC_ERROR_V2   10
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index e31fcde797..c346bd7b49 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include <sys/ioctl.h>
 
 #include <linux/kvm.h>
@@ -2337,10 +2338,53 @@ static void push_ghes_memory_errors(CPUState *c, AcpiGhesState *ags,
                                     uint64_t paddr)
 {
     GArray *addresses = g_array_new(false, false, sizeof(paddr));
+    uint64_t val, start, end, guest_pgsz, host_pgsz;
     int ret;
 
     kvm_cpu_synchronize_state(c);
-    g_array_append_vals(addresses, &paddr, 1);
+
+    /*
+     * Sort out the guest page size from TCR_EL1, which can be modified
+     * by the guest from time to time. So we have to sort it out dynamically.
+     */
+    ret = read_sys_reg64(c->kvm_fd, &val, ARM64_SYS_REG(3, 0, 2, 0, 2));
+    if (ret) {
+        goto error;
+    }
+
+    switch (extract64(val, 14, 2)) {
+    case 0:
+        guest_pgsz = 4 * KiB;
+        break;
+    case 1:
+        guest_pgsz = 64 * KiB;
+        break;
+    case 2:
+        guest_pgsz = 16 * KiB;
+        break;
+    default:
+        error_report("unknown page size from TCR_EL1 (0x%" PRIx64 ")", val);
+        goto error;
+    }
+
+    host_pgsz = qemu_real_host_page_size();
+    start = paddr & ~(host_pgsz - 1);
+    end = start + host_pgsz;
+    while (start < end) {
+        /*
+         * The precise physical address is provided for the affected
+         * guest page that contains @paddr. Otherwise, the starting
+         * address of the guest page is provided.
+         */
+        if (paddr >= start && paddr < (start + guest_pgsz)) {
+            g_array_append_vals(addresses, &paddr, 1);
+        } else {
+            g_array_append_vals(addresses, &start, 1);
+        }
+
+        start += guest_pgsz;
+    }
+
     ret = acpi_ghes_memory_errors(ags, ACPI_HEST_SRC_ID_SYNC, addresses);
     if (ret) {
         goto error;
-- 
2.49.0