[PATCH 0/3] hw/ppc: reorg PowerPC RAS code for better maintainability

[PATCH 0/3] hw/ppc: reorg PowerPC RAS code for better maintainability

[Shivang Upadhyay]

Extracting RAS related code from spapr_rtas.c, to a newly created spapr_rtas_ras.c
and carving out a MAINTAINERS entry dedicated to PowerPC RAS. This will cover RAS
related functionalities for PowerPC platforms.

Also adding myself as a reviewer, to help share the review workload.

Shivang Upadhyay (3):
  hw/ppc: move RAS-specific RTAS handlers to spapr_rtas_ras.c
  MAINTAINERS: add dedicated PowerPC RAS section
  MAINTAINERS: add self as reviewer for PowerPC RAS

 MAINTAINERS             |  33 +++---
 hw/ppc/meson.build      |   1 +
 hw/ppc/spapr_rtas.c     | 189 --------------------------------
 hw/ppc/spapr_rtas_ras.c | 232 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+), 206 deletions(-)
 create mode 100644 hw/ppc/spapr_rtas_ras.c

-- 
2.54.0

[PATCH 1/3] hw/ppc: move RAS-specific RTAS handlers to spapr_rtas_ras.c

[Shivang Upadhyay]

Following RTAS calls are moved to spapr_rtas_ras.c file
- ibm,os-term
- ibm,nmi-register
- ibm,nmi-interlock
- ibm,configure-kernel-dump

The rtas calls are initialized with a new rtas_register_types
constructor.

No functional changes.

Signed-off-by: Shivang Upadhyay <shivanug@linux.ibm.com>
---
 hw/ppc/meson.build      |   1 +
 hw/ppc/spapr_rtas.c     | 189 --------------------------------
 hw/ppc/spapr_rtas_ras.c | 232 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 233 insertions(+), 189 deletions(-)
 create mode 100644 hw/ppc/spapr_rtas_ras.c

diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 37aa535db2..83edb3d852 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -14,6 +14,7 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
   'spapr_events.c',
   'spapr_hcall.c',
   'spapr_nested.c',
+  'spapr_rtas_ras.c',
   'spapr_iommu.c',
   'spapr_rtas.c',
   'spapr_pci.c',
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 328fc27c40..f8538735a4 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -361,96 +361,6 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
     rtas_st(rets, 0, ret);
 }
 
-/* Papr Section 7.4.9 ibm,configure-kernel-dump RTAS call */
-static void rtas_configure_kernel_dump(PowerPCCPU *cpu,
-                                   SpaprMachineState *spapr,
-                                   uint32_t token, uint32_t nargs,
-                                   target_ulong args,
-                                   uint32_t nret, target_ulong rets)
-{
-    target_ulong cmd = rtas_ld(args, 0);
-    uint32_t ret_val;
-
-    /* Number of outputs has to be 1 */
-    if (nret != 1) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-            "FADump: ibm,configure-kernel-dump called with nret != 1.\n");
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    /* Number of inputs has to be 3 */
-    if (nargs != 3) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-            "FADump: ibm,configure-kernel-dump called with nargs != 3.\n");
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    switch (cmd) {
-    case FADUMP_CMD_REGISTER:
-        ret_val = do_fadump_register(spapr, args);
-        if (ret_val != RTAS_OUT_SUCCESS) {
-            rtas_st(rets, 0, ret_val);
-            return;
-        }
-        break;
-    case FADUMP_CMD_UNREGISTER:
-        if (spapr->fadump_dump_active) {
-            rtas_st(rets, 0, RTAS_OUT_DUMP_ACTIVE);
-            return;
-        }
-
-        spapr->fadump_registered = false;
-        spapr->fadump_dump_active = false;
-        memset(&spapr->registered_fdm, 0, sizeof(spapr->registered_fdm));
-        break;
-    case FADUMP_CMD_INVALIDATE:
-        if (!spapr->fadump_dump_active) {
-            qemu_log_mask(LOG_GUEST_ERROR,
-                "FADump: Nothing to invalidate, no dump active\n");
-
-            rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        }
-
-        spapr->fadump_registered = false;
-        spapr->fadump_dump_active = false;
-        memset(&spapr->registered_fdm, 0, sizeof(spapr->registered_fdm));
-        break;
-    default:
-        qemu_log_mask(LOG_GUEST_ERROR,
-                "FADump: Unknown command: " TARGET_FMT_lu "\n", cmd);
-
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
-static void rtas_ibm_os_term(PowerPCCPU *cpu,
-                            SpaprMachineState *spapr,
-                            uint32_t token, uint32_t nargs,
-                            target_ulong args,
-                            uint32_t nret, target_ulong rets)
-{
-    target_ulong msgaddr = rtas_ld(args, 0);
-    char msg[512];
-
-    if (spapr->fadump_registered) {
-        /* If fadump boot works, control won't come back here */
-        return trigger_fadump_boot(spapr, rets);
-    }
-
-    physical_memory_read(msgaddr, msg, sizeof(msg) - 1);
-    msg[sizeof(msg) - 1] = 0;
-
-    error_report("OS terminated: %s", msg);
-    qemu_system_guest_panicked(NULL);
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
 static void rtas_set_power_level(PowerPCCPU *cpu, SpaprMachineState *spapr,
                                  uint32_t token, uint32_t nargs,
                                  target_ulong args, uint32_t nret,
@@ -501,95 +411,6 @@ static void rtas_get_power_level(PowerPCCPU *cpu, SpaprMachineState *spapr,
     rtas_st(rets, 1, 100);
 }
 
-static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
-                                  SpaprMachineState *spapr,
-                                  uint32_t token, uint32_t nargs,
-                                  target_ulong args,
-                                  uint32_t nret, target_ulong rets)
-{
-    hwaddr rtas_addr;
-    target_ulong sreset_addr, mce_addr;
-
-    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
-        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
-        return;
-    }
-
-    rtas_addr = spapr_get_rtas_addr();
-    if (!rtas_addr) {
-        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
-        return;
-    }
-
-    sreset_addr = rtas_ld(args, 0);
-    mce_addr = rtas_ld(args, 1);
-
-    /* PAPR requires these are in the first 32M of memory and within RMA */
-    if (sreset_addr >= 32 * MiB || sreset_addr >= spapr->rma_size ||
-           mce_addr >= 32 * MiB ||    mce_addr >= spapr->rma_size) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    if (kvm_enabled()) {
-        if (kvmppc_set_fwnmi(cpu) < 0) {
-            rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
-            return;
-        }
-    }
-
-    spapr->fwnmi_system_reset_addr = sreset_addr;
-    spapr->fwnmi_machine_check_addr = mce_addr;
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
-static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
-                                   SpaprMachineState *spapr,
-                                   uint32_t token, uint32_t nargs,
-                                   target_ulong args,
-                                   uint32_t nret, target_ulong rets)
-{
-    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
-        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
-        return;
-    }
-
-    if (spapr->fwnmi_machine_check_addr == -1) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-"FWNMI: ibm,nmi-interlock RTAS called with FWNMI not registered.\n");
-
-        /* NMI register not called */
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    if (spapr->fwnmi_machine_check_interlock != cpu->vcpu_id) {
-        /*
-         * The vCPU that hit the NMI should invoke "ibm,nmi-interlock"
-         * This should be PARAM_ERROR, but Linux calls "ibm,nmi-interlock"
-         * for system reset interrupts, despite them not being interlocked.
-         * PowerVM silently ignores this and returns success here. Returning
-         * failure causes Linux to print the error "FWNMI: nmi-interlock
-         * failed: -3", although no other apparent ill effects, this is a
-         * regression for the user when enabling FWNMI. So for now, match
-         * PowerVM. When most Linux clients are fixed, this could be
-         * changed.
-         */
-        rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-        return;
-    }
-
-    /*
-     * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
-     * hence unset fwnmi_machine_check_interlock.
-     */
-    spapr->fwnmi_machine_check_interlock = -1;
-    qemu_cond_signal(&spapr->fwnmi_machine_check_interlock_cond);
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-    migrate_del_blocker(&spapr->fwnmi_migration_blocker);
-}
-
 static struct rtas_call {
     const char *name;
     spapr_rtas_fn fn;
@@ -737,20 +558,10 @@ static void core_rtas_register_types(void)
     spapr_rtas_register(RTAS_IBM_SET_SYSTEM_PARAMETER,
                         "ibm,set-system-parameter",
                         rtas_ibm_set_system_parameter);
-    spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
-                        rtas_ibm_os_term);
     spapr_rtas_register(RTAS_SET_POWER_LEVEL, "set-power-level",
                         rtas_set_power_level);
     spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
                         rtas_get_power_level);
-    spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
-                        rtas_ibm_nmi_register);
-    spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
-                        rtas_ibm_nmi_interlock);
-
-    /* Register fadump rtas call */
-    spapr_rtas_register(RTAS_CONFIGURE_KERNEL_DUMP, "ibm,configure-kernel-dump",
-                        rtas_configure_kernel_dump);
 
     qtest_set_command_cb(spapr_qtest_callback);
 }
diff --git a/hw/ppc/spapr_rtas_ras.c b/hw/ppc/spapr_rtas_ras.c
new file mode 100644
index 0000000000..2f7ee3bb24
--- /dev/null
+++ b/hw/ppc/spapr_rtas_ras.c
@@ -0,0 +1,232 @@
+/*
+ * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
+ *
+ * RAS (Reliability, Availability and Serviceability) RTAS call handlers:
+ *   - ibm,configure-kernel-dump  (FADump)
+ *   - ibm,os-term                (FADump-aware OS termination)
+ *
+ * Copyright (c) 2010-2011 David Gibson, IBM Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "qemu/units.h"
+#include "system/physmem.h"
+#include "system/runstate.h"
+#include "kvm_ppc.h"
+#include "migration/blocker.h"
+
+#include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_fadump.h"
+
+/* PAPR Section 7.4.9 ibm,configure-kernel-dump RTAS call */
+static void rtas_configure_kernel_dump(PowerPCCPU *cpu,
+                                       SpaprMachineState *spapr,
+                                       uint32_t token, uint32_t nargs,
+                                       target_ulong args,
+                                       uint32_t nret, target_ulong rets)
+{
+    target_ulong cmd = rtas_ld(args, 0);
+    uint32_t ret_val;
+
+    /* Number of outputs has to be 1 */
+    if (nret != 1) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "FADump: ibm,configure-kernel-dump called with nret != 1.\n");
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    /* Number of inputs has to be 3 */
+    if (nargs != 3) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "FADump: ibm,configure-kernel-dump called with nargs != 3.\n");
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    switch (cmd) {
+    case FADUMP_CMD_REGISTER:
+        ret_val = do_fadump_register(spapr, args);
+        if (ret_val != RTAS_OUT_SUCCESS) {
+            rtas_st(rets, 0, ret_val);
+            return;
+        }
+        break;
+    case FADUMP_CMD_UNREGISTER:
+        if (spapr->fadump_dump_active) {
+            rtas_st(rets, 0, RTAS_OUT_DUMP_ACTIVE);
+            return;
+        }
+
+        spapr->fadump_registered = false;
+        spapr->fadump_dump_active = false;
+        memset(&spapr->registered_fdm, 0, sizeof(spapr->registered_fdm));
+        break;
+    case FADUMP_CMD_INVALIDATE:
+        if (!spapr->fadump_dump_active) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                "FADump: Nothing to invalidate, no dump active\n");
+
+            rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        }
+
+        spapr->fadump_registered = false;
+        spapr->fadump_dump_active = false;
+        memset(&spapr->registered_fdm, 0, sizeof(spapr->registered_fdm));
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                "FADump: Unknown command: " TARGET_FMT_lu "\n", cmd);
+
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_ibm_os_term(PowerPCCPU *cpu,
+                             SpaprMachineState *spapr,
+                             uint32_t token, uint32_t nargs,
+                             target_ulong args,
+                             uint32_t nret, target_ulong rets)
+{
+    target_ulong msgaddr = rtas_ld(args, 0);
+    char msg[512];
+
+    if (spapr->fadump_registered) {
+        /* If fadump boot works, control won't come back here */
+        return trigger_fadump_boot(spapr, rets);
+    }
+
+    physical_memory_read(msgaddr, msg, sizeof(msg) - 1);
+    msg[sizeof(msg) - 1] = 0;
+
+    error_report("OS terminated: %s", msg);
+    qemu_system_guest_panicked(NULL);
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
+                                  SpaprMachineState *spapr,
+                                  uint32_t token, uint32_t nargs,
+                                  target_ulong args,
+                                  uint32_t nret, target_ulong rets)
+{
+    hwaddr rtas_addr;
+    target_ulong sreset_addr, mce_addr;
+
+    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
+    rtas_addr = spapr_get_rtas_addr();
+    if (!rtas_addr) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
+    sreset_addr = rtas_ld(args, 0);
+    mce_addr = rtas_ld(args, 1);
+
+    /* PAPR requires these are in the first 32M of memory and within RMA */
+    if (sreset_addr >= 32 * MiB || sreset_addr >= spapr->rma_size ||
+           mce_addr >= 32 * MiB ||    mce_addr >= spapr->rma_size) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    if (kvm_enabled()) {
+        if (kvmppc_set_fwnmi(cpu) < 0) {
+            rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+            return;
+        }
+    }
+
+    spapr->fwnmi_system_reset_addr = sreset_addr;
+    spapr->fwnmi_machine_check_addr = mce_addr;
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
+                                   SpaprMachineState *spapr,
+                                   uint32_t token, uint32_t nargs,
+                                   target_ulong args,
+                                   uint32_t nret, target_ulong rets)
+{
+    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
+        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+        return;
+    }
+
+    if (spapr->fwnmi_machine_check_addr == -1) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+"FWNMI: ibm,nmi-interlock RTAS called with FWNMI not registered.\n");
+
+        /* NMI register not called */
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    if (spapr->fwnmi_machine_check_interlock != cpu->vcpu_id) {
+        /*
+         * The vCPU that hit the NMI should invoke "ibm,nmi-interlock"
+         * This should be PARAM_ERROR, but Linux calls "ibm,nmi-interlock"
+         * for system reset interrupts, despite them not being interlocked.
+         * PowerVM silently ignores this and returns success here. Returning
+         * failure causes Linux to print the error "FWNMI: nmi-interlock
+         * failed: -3", although no other apparent ill effects, this is a
+         * regression for the user when enabling FWNMI. So for now, match
+         * PowerVM. When most Linux clients are fixed, this could be
+         * changed.
+         */
+        rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+        return;
+    }
+
+    /*
+     * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
+     * hence unset fwnmi_machine_check_interlock.
+     */
+    spapr->fwnmi_machine_check_interlock = -1;
+    qemu_cond_signal(&spapr->fwnmi_machine_check_interlock_cond);
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+    migrate_del_blocker(&spapr->fwnmi_migration_blocker);
+}
+
+static void ras_rtas_register_types(void)
+{
+    spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
+                        rtas_ibm_os_term);
+    spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
+                        rtas_ibm_nmi_register);
+    spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
+                        rtas_ibm_nmi_interlock);
+    spapr_rtas_register(RTAS_CONFIGURE_KERNEL_DUMP, "ibm,configure-kernel-dump",
+                        rtas_configure_kernel_dump);
+}
+
+type_init(ras_rtas_register_types)
-- 
2.54.0

Re: [PATCH 1/3] hw/ppc: move RAS-specific RTAS handlers to spapr_rtas_ras.c

[Sourabh Jain]

On 14/07/26 22:17, Shivang Upadhyay wrote:
> Following RTAS calls are moved to spapr_rtas_ras.c file
> - ibm,os-term
> - ibm,nmi-register
> - ibm,nmi-interlock
> - ibm,configure-kernel-dump
>
> The rtas calls are initialized with a new rtas_register_types
> constructor.
>
> No functional changes.
>
> Signed-off-by: Shivang Upadhyay <shivanug@linux.ibm.com>
> ---
>   hw/ppc/meson.build      |   1 +
>   hw/ppc/spapr_rtas.c     | 189 --------------------------------
>   hw/ppc/spapr_rtas_ras.c | 232 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 233 insertions(+), 189 deletions(-)
>   create mode 100644 hw/ppc/spapr_rtas_ras.c
>
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index 37aa535db2..83edb3d852 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -14,6 +14,7 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
>     'spapr_events.c',
>     'spapr_hcall.c',
>     'spapr_nested.c',
> +  'spapr_rtas_ras.c',
>     'spapr_iommu.c',
>     'spapr_rtas.c',
>     'spapr_pci.c',
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 328fc27c40..f8538735a4 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -361,96 +361,6 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
>       rtas_st(rets, 0, ret);
>   }
>   
> -/* Papr Section 7.4.9 ibm,configure-kernel-dump RTAS call */
> -static void rtas_configure_kernel_dump(PowerPCCPU *cpu,
> -                                   SpaprMachineState *spapr,
> -                                   uint32_t token, uint32_t nargs,
> -                                   target_ulong args,
> -                                   uint32_t nret, target_ulong rets)
> -{
> -    target_ulong cmd = rtas_ld(args, 0);
> -    uint32_t ret_val;
> -
> -    /* Number of outputs has to be 1 */
> -    if (nret != 1) {
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -            "FADump: ibm,configure-kernel-dump called with nret != 1.\n");
> -        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> -        return;
> -    }
> -
> -    /* Number of inputs has to be 3 */
> -    if (nargs != 3) {
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -            "FADump: ibm,configure-kernel-dump called with nargs != 3.\n");
> -        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> -        return;
> -    }
> -
> -    switch (cmd) {
> -    case FADUMP_CMD_REGISTER:
> -        ret_val = do_fadump_register(spapr, args);
> -        if (ret_val != RTAS_OUT_SUCCESS) {
> -            rtas_st(rets, 0, ret_val);
> -            return;
> -        }
> -        break;
> -    case FADUMP_CMD_UNREGISTER:
> -        if (spapr->fadump_dump_active) {
> -            rtas_st(rets, 0, RTAS_OUT_DUMP_ACTIVE);
> -            return;
> -        }
> -
> -        spapr->fadump_registered = false;
> -        spapr->fadump_dump_active = false;
> -        memset(&spapr->registered_fdm, 0, sizeof(spapr->registered_fdm));
> -        break;
> -    case FADUMP_CMD_INVALIDATE:
> -        if (!spapr->fadump_dump_active) {
> -            qemu_log_mask(LOG_GUEST_ERROR,
> -                "FADump: Nothing to invalidate, no dump active\n");
> -
> -            rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> -        }
> -
> -        spapr->fadump_registered = false;
> -        spapr->fadump_dump_active = false;
> -        memset(&spapr->registered_fdm, 0, sizeof(spapr->registered_fdm));
> -        break;
> -    default:
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -                "FADump: Unknown command: " TARGET_FMT_lu "\n", cmd);
> -
> -        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> -        return;
> -    }
> -
> -    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> -}
> -
> -static void rtas_ibm_os_term(PowerPCCPU *cpu,
> -                            SpaprMachineState *spapr,
> -                            uint32_t token, uint32_t nargs,
> -                            target_ulong args,
> -                            uint32_t nret, target_ulong rets)
> -{
> -    target_ulong msgaddr = rtas_ld(args, 0);
> -    char msg[512];
> -
> -    if (spapr->fadump_registered) {
> -        /* If fadump boot works, control won't come back here */
> -        return trigger_fadump_boot(spapr, rets);
> -    }
> -
> -    physical_memory_read(msgaddr, msg, sizeof(msg) - 1);
> -    msg[sizeof(msg) - 1] = 0;
> -
> -    error_report("OS terminated: %s", msg);
> -    qemu_system_guest_panicked(NULL);
> -
> -    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> -}
> -
>   static void rtas_set_power_level(PowerPCCPU *cpu, SpaprMachineState *spapr,
>                                    uint32_t token, uint32_t nargs,
>                                    target_ulong args, uint32_t nret,
> @@ -501,95 +411,6 @@ static void rtas_get_power_level(PowerPCCPU *cpu, SpaprMachineState *spapr,
>       rtas_st(rets, 1, 100);
>   }
>   
> -static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> -                                  SpaprMachineState *spapr,
> -                                  uint32_t token, uint32_t nargs,
> -                                  target_ulong args,
> -                                  uint32_t nret, target_ulong rets)
> -{
> -    hwaddr rtas_addr;
> -    target_ulong sreset_addr, mce_addr;
> -
> -    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
> -        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> -        return;
> -    }
> -
> -    rtas_addr = spapr_get_rtas_addr();
> -    if (!rtas_addr) {
> -        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> -        return;
> -    }
> -
> -    sreset_addr = rtas_ld(args, 0);
> -    mce_addr = rtas_ld(args, 1);
> -
> -    /* PAPR requires these are in the first 32M of memory and within RMA */
> -    if (sreset_addr >= 32 * MiB || sreset_addr >= spapr->rma_size ||
> -           mce_addr >= 32 * MiB ||    mce_addr >= spapr->rma_size) {
> -        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> -        return;
> -    }
> -
> -    if (kvm_enabled()) {
> -        if (kvmppc_set_fwnmi(cpu) < 0) {
> -            rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> -            return;
> -        }
> -    }
> -
> -    spapr->fwnmi_system_reset_addr = sreset_addr;
> -    spapr->fwnmi_machine_check_addr = mce_addr;
> -
> -    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> -}
> -
> -static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
> -                                   SpaprMachineState *spapr,
> -                                   uint32_t token, uint32_t nargs,
> -                                   target_ulong args,
> -                                   uint32_t nret, target_ulong rets)
> -{
> -    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
> -        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> -        return;
> -    }
> -
> -    if (spapr->fwnmi_machine_check_addr == -1) {
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -"FWNMI: ibm,nmi-interlock RTAS called with FWNMI not registered.\n");
> -
> -        /* NMI register not called */
> -        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> -        return;
> -    }
> -
> -    if (spapr->fwnmi_machine_check_interlock != cpu->vcpu_id) {
> -        /*
> -         * The vCPU that hit the NMI should invoke "ibm,nmi-interlock"
> -         * This should be PARAM_ERROR, but Linux calls "ibm,nmi-interlock"
> -         * for system reset interrupts, despite them not being interlocked.
> -         * PowerVM silently ignores this and returns success here. Returning
> -         * failure causes Linux to print the error "FWNMI: nmi-interlock
> -         * failed: -3", although no other apparent ill effects, this is a
> -         * regression for the user when enabling FWNMI. So for now, match
> -         * PowerVM. When most Linux clients are fixed, this could be
> -         * changed.
> -         */
> -        rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> -        return;
> -    }
> -
> -    /*
> -     * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> -     * hence unset fwnmi_machine_check_interlock.
> -     */
> -    spapr->fwnmi_machine_check_interlock = -1;
> -    qemu_cond_signal(&spapr->fwnmi_machine_check_interlock_cond);
> -    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> -    migrate_del_blocker(&spapr->fwnmi_migration_blocker);

Nit: since |migrate_del_blocker| is being moved out of this file, we can 
remove
|#include "migration/blocker.h"| from here.

> -}
> -
>   static struct rtas_call {
>       const char *name;
>       spapr_rtas_fn fn;
> @@ -737,20 +558,10 @@ static void core_rtas_register_types(void)
>       spapr_rtas_register(RTAS_IBM_SET_SYSTEM_PARAMETER,
>                           "ibm,set-system-parameter",
>                           rtas_ibm_set_system_parameter);
> -    spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
> -                        rtas_ibm_os_term);
>       spapr_rtas_register(RTAS_SET_POWER_LEVEL, "set-power-level",
>                           rtas_set_power_level);
>       spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
>                           rtas_get_power_level);
> -    spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
> -                        rtas_ibm_nmi_register);
> -    spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
> -                        rtas_ibm_nmi_interlock);
> -
> -    /* Register fadump rtas call */
> -    spapr_rtas_register(RTAS_CONFIGURE_KERNEL_DUMP, "ibm,configure-kernel-dump",
> -                        rtas_configure_kernel_dump);
>   
>       qtest_set_command_cb(spapr_qtest_callback);
>   }
> diff --git a/hw/ppc/spapr_rtas_ras.c b/hw/ppc/spapr_rtas_ras.c
> new file mode 100644
> index 0000000000..2f7ee3bb24
> --- /dev/null
> +++ b/hw/ppc/spapr_rtas_ras.c
> @@ -0,0 +1,232 @@
> +/*
> + * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
> + *
> + * RAS (Reliability, Availability and Serviceability) RTAS call handlers:
> + *   - ibm,configure-kernel-dump  (FADump)
> + *   - ibm,os-term                (FADump-aware OS termination)
> + *
> + * Copyright (c) 2010-2011 David Gibson, IBM Corporation.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/error-report.h"
> +#include "qemu/units.h"
> +#include "system/physmem.h"
> +#include "system/runstate.h"
> +#include "kvm_ppc.h"
> +#include "migration/blocker.h"
> +
> +#include "hw/ppc/spapr.h"
> +#include "hw/ppc/spapr_fadump.h"
> +
> +/* PAPR Section 7.4.9 ibm,configure-kernel-dump RTAS call */
> +static void rtas_configure_kernel_dump(PowerPCCPU *cpu,
> +                                       SpaprMachineState *spapr,
> +                                       uint32_t token, uint32_t nargs,
> +                                       target_ulong args,
> +                                       uint32_t nret, target_ulong rets)
> +{
> +    target_ulong cmd = rtas_ld(args, 0);
> +    uint32_t ret_val;
> +
> +    /* Number of outputs has to be 1 */
> +    if (nret != 1) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "FADump: ibm,configure-kernel-dump called with nret != 1.\n");
> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        return;
> +    }
> +
> +    /* Number of inputs has to be 3 */
> +    if (nargs != 3) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "FADump: ibm,configure-kernel-dump called with nargs != 3.\n");
> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        return;
> +    }
> +
> +    switch (cmd) {
> +    case FADUMP_CMD_REGISTER:
> +        ret_val = do_fadump_register(spapr, args);
> +        if (ret_val != RTAS_OUT_SUCCESS) {
> +            rtas_st(rets, 0, ret_val);
> +            return;
> +        }
> +        break;
> +    case FADUMP_CMD_UNREGISTER:
> +        if (spapr->fadump_dump_active) {
> +            rtas_st(rets, 0, RTAS_OUT_DUMP_ACTIVE);
> +            return;
> +        }
> +
> +        spapr->fadump_registered = false;
> +        spapr->fadump_dump_active = false;
> +        memset(&spapr->registered_fdm, 0, sizeof(spapr->registered_fdm));
> +        break;
> +    case FADUMP_CMD_INVALIDATE:
> +        if (!spapr->fadump_dump_active) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                "FADump: Nothing to invalidate, no dump active\n");
> +
> +            rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        }
> +
> +        spapr->fadump_registered = false;
> +        spapr->fadump_dump_active = false;
> +        memset(&spapr->registered_fdm, 0, sizeof(spapr->registered_fdm));
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                "FADump: Unknown command: " TARGET_FMT_lu "\n", cmd);
> +
> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        return;
> +    }
> +
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +}
> +
> +static void rtas_ibm_os_term(PowerPCCPU *cpu,
> +                             SpaprMachineState *spapr,
> +                             uint32_t token, uint32_t nargs,
> +                             target_ulong args,
> +                             uint32_t nret, target_ulong rets)
> +{
> +    target_ulong msgaddr = rtas_ld(args, 0);
> +    char msg[512];
> +
> +    if (spapr->fadump_registered) {
> +        /* If fadump boot works, control won't come back here */
> +        return trigger_fadump_boot(spapr, rets);
> +    }
> +
> +    physical_memory_read(msgaddr, msg, sizeof(msg) - 1);
> +    msg[sizeof(msg) - 1] = 0;
> +
> +    error_report("OS terminated: %s", msg);
> +    qemu_system_guest_panicked(NULL);
> +
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +}
> +
> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> +                                  SpaprMachineState *spapr,
> +                                  uint32_t token, uint32_t nargs,
> +                                  target_ulong args,
> +                                  uint32_t nret, target_ulong rets)
> +{
> +    hwaddr rtas_addr;
> +    target_ulong sreset_addr, mce_addr;
> +
> +    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
> +        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> +        return;
> +    }
> +
> +    rtas_addr = spapr_get_rtas_addr();
> +    if (!rtas_addr) {
> +        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> +        return;
> +    }
> +
> +    sreset_addr = rtas_ld(args, 0);
> +    mce_addr = rtas_ld(args, 1);
> +
> +    /* PAPR requires these are in the first 32M of memory and within RMA */
> +    if (sreset_addr >= 32 * MiB || sreset_addr >= spapr->rma_size ||
> +           mce_addr >= 32 * MiB ||    mce_addr >= spapr->rma_size) {
> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        return;
> +    }
> +
> +    if (kvm_enabled()) {
> +        if (kvmppc_set_fwnmi(cpu) < 0) {
> +            rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> +            return;
> +        }
> +    }
> +
> +    spapr->fwnmi_system_reset_addr = sreset_addr;
> +    spapr->fwnmi_machine_check_addr = mce_addr;
> +
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +}
> +
> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
> +                                   SpaprMachineState *spapr,
> +                                   uint32_t token, uint32_t nargs,
> +                                   target_ulong args,
> +                                   uint32_t nret, target_ulong rets)
> +{
> +    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
> +        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> +        return;
> +    }
> +
> +    if (spapr->fwnmi_machine_check_addr == -1) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +"FWNMI: ibm,nmi-interlock RTAS called with FWNMI not registered.\n");
> +
> +        /* NMI register not called */
> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        return;
> +    }
> +
> +    if (spapr->fwnmi_machine_check_interlock != cpu->vcpu_id) {
> +        /*
> +         * The vCPU that hit the NMI should invoke "ibm,nmi-interlock"
> +         * This should be PARAM_ERROR, but Linux calls "ibm,nmi-interlock"
> +         * for system reset interrupts, despite them not being interlocked.
> +         * PowerVM silently ignores this and returns success here. Returning
> +         * failure causes Linux to print the error "FWNMI: nmi-interlock
> +         * failed: -3", although no other apparent ill effects, this is a
> +         * regression for the user when enabling FWNMI. So for now, match
> +         * PowerVM. When most Linux clients are fixed, this could be
> +         * changed.
> +         */
> +        rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +        return;
> +    }
> +
> +    /*
> +     * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> +     * hence unset fwnmi_machine_check_interlock.
> +     */
> +    spapr->fwnmi_machine_check_interlock = -1;
> +    qemu_cond_signal(&spapr->fwnmi_machine_check_interlock_cond);
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +    migrate_del_blocker(&spapr->fwnmi_migration_blocker);
> +}
> +
> +static void ras_rtas_register_types(void)
> +{
> +    spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
> +                        rtas_ibm_os_term);
> +    spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
> +                        rtas_ibm_nmi_register);
> +    spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
> +                        rtas_ibm_nmi_interlock);
> +    spapr_rtas_register(RTAS_CONFIGURE_KERNEL_DUMP, "ibm,configure-kernel-dump",
> +                        rtas_configure_kernel_dump);
> +}
> +
> +type_init(ras_rtas_register_types)

[PATCH 2/3] MAINTAINERS: add dedicated PowerPC RAS section

[Shivang Upadhyay]

Introduce a new "PowerPC RAS (Reliability, Availability and
Serviceability)" entry in the PowerPC Machines block, replacing the
existing Fadump/MPIPL sections. Retaining the maintainer and
reviewer entries from Fadump/MPIPL sections for this broader umbrella.

Additionally adding spapr_events.c, as it implements RTAS error logging
infrastructure and spapr_pci_vfio.c, since its all EEH related code.

Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
---
 MAINTAINERS | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e25df9493c..4a010d4807 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1745,6 +1745,21 @@ F: include/hw/ppc/vof*
 F: pc-bios/vof/*
 F: pc-bios/vof*
 
+PowerPC RAS (Reliability, Availability and Serviceability)
+M: Aditya Gupta <adityag@linux.ibm.com>
+R: Sourabh Jain <sourabhjain@linux.ibm.com>
+R: Hari Bathini <hbathini@linux.ibm.com>
+L: qemu-ppc@nongnu.org
+S: Maintained
+F: hw/ppc/spapr_events.c
+F: hw/ppc/spapr_fadump.c
+F: hw/ppc/spapr_pci_vfio.c
+F: hw/ppc/spapr_rtas_ras.c
+F: hw/ppc/pnv_mpipl.c
+F: include/hw/ppc/spapr_fadump.h
+F: include/hw/ppc/pnv_mpipl.h
+F: tests/functional/ppc64/test_fadump.py
+
 RISC-V Machines
 ---------------
 OpenTitan
@@ -3361,23 +3376,6 @@ F: scripts/coccinelle/remove_local_err.cocci
 F: scripts/coccinelle/use-error_fatal.cocci
 F: scripts/coccinelle/errp-guard.cocci
 
-Firmware Assisted Dump (fadump) for sPAPR (pseries)
-M: Aditya Gupta <adityag@linux.ibm.com>
-R: Sourabh Jain <sourabhjain@linux.ibm.com>
-S: Maintained
-F: include/hw/ppc/spapr_fadump.h
-F: hw/ppc/spapr_fadump.c
-F: tests/functional/ppc64/test_fadump.py
-
-Memory-Preserving Initial Program Load (MPIPL) for PowerNV
-M: Aditya Gupta <adityag@linux.ibm.com>
-R: Hari Bathini <hbathini@linux.ibm.com>
-R: Sourabh <sourabhjain@linux.ibm.com>
-S: Maintained
-F: include/hw/ppc/pnv_mpipl.h
-F: hw/ppc/pnv_mpipl.c
-F: tests/functional/ppc64/test_fadump.py
-
 GDB stub
 M: Alex Bennée <alex.bennee@linaro.org>
 R: Philippe Mathieu-Daudé <philmd@mailo.com>
-- 
2.54.0

Re: [PATCH 2/3] MAINTAINERS: add dedicated PowerPC RAS section

[Aditya Gupta]

On 14/07/26 22:17, Shivang Upadhyay wrote:

> Introduce a new "PowerPC RAS (Reliability, Availability and
> Serviceability)" entry in the PowerPC Machines block, replacing the
> existing Fadump/MPIPL sections. Retaining the maintainer and
> reviewer entries from Fadump/MPIPL sections for this broader umbrella.
>
> Additionally adding spapr_events.c, as it implements RTAS error logging
> infrastructure and spapr_pci_vfio.c, since its all EEH related code.
>
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>

Good to have a common entry for RAS. Looks good to me.

Acked-by: Aditya Gupta <adityag@linux.ibm.com>


Thanks,

- Aditya G

Re: [PATCH 2/3] MAINTAINERS: add dedicated PowerPC RAS section

[Sourabh Jain]

On 14/07/26 22:17, Shivang Upadhyay wrote:
> Introduce a new "PowerPC RAS (Reliability, Availability and
> Serviceability)" entry in the PowerPC Machines block, replacing the
> existing Fadump/MPIPL sections. Retaining the maintainer and
> reviewer entries from Fadump/MPIPL sections for this broader umbrella.
>
> Additionally adding spapr_events.c, as it implements RTAS error logging
> infrastructure and spapr_pci_vfio.c, since its all EEH related code.
>
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
>   MAINTAINERS | 32 +++++++++++++++-----------------
>   1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e25df9493c..4a010d4807 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1745,6 +1745,21 @@ F: include/hw/ppc/vof*
>   F: pc-bios/vof/*
>   F: pc-bios/vof*
>   
> +PowerPC RAS (Reliability, Availability and Serviceability)
> +M: Aditya Gupta <adityag@linux.ibm.com>
> +R: Sourabh Jain <sourabhjain@linux.ibm.com>
> +R: Hari Bathini <hbathini@linux.ibm.com>
> +L: qemu-ppc@nongnu.org
> +S: Maintained
> +F: hw/ppc/spapr_events.c
> +F: hw/ppc/spapr_fadump.c
> +F: hw/ppc/spapr_pci_vfio.c
> +F: hw/ppc/spapr_rtas_ras.c

As there is a suggestion to drop the 1/3, some of the above file names 
need to be
corrected accordingly. The rest of the changes look good to me.

Feel free to include in 2/3 and 3/3:

Acked-by: Sourabh Jain <sourabhjain@linux.ibm.com>


> +F: hw/ppc/pnv_mpipl.c
> +F: include/hw/ppc/spapr_fadump.h
> +F: include/hw/ppc/pnv_mpipl.h
> +F: tests/functional/ppc64/test_fadump.py
> +
>   RISC-V Machines
>   ---------------
>   OpenTitan
> @@ -3361,23 +3376,6 @@ F: scripts/coccinelle/remove_local_err.cocci
>   F: scripts/coccinelle/use-error_fatal.cocci
>   F: scripts/coccinelle/errp-guard.cocci
>   
> -Firmware Assisted Dump (fadump) for sPAPR (pseries)
> -M: Aditya Gupta <adityag@linux.ibm.com>
> -R: Sourabh Jain <sourabhjain@linux.ibm.com>
> -S: Maintained
> -F: include/hw/ppc/spapr_fadump.h
> -F: hw/ppc/spapr_fadump.c
> -F: tests/functional/ppc64/test_fadump.py
> -
> -Memory-Preserving Initial Program Load (MPIPL) for PowerNV
> -M: Aditya Gupta <adityag@linux.ibm.com>
> -R: Hari Bathini <hbathini@linux.ibm.com>
> -R: Sourabh <sourabhjain@linux.ibm.com>
> -S: Maintained
> -F: include/hw/ppc/pnv_mpipl.h
> -F: hw/ppc/pnv_mpipl.c
> -F: tests/functional/ppc64/test_fadump.py
> -
>   GDB stub
>   M: Alex Bennée <alex.bennee@linaro.org>
>   R: Philippe Mathieu-Daudé <philmd@mailo.com>

[PATCH 3/3] MAINTAINERS: add self as reviewer for PowerPC RAS

[Shivang Upadhyay]

I have been contributing to Fadump, MPIPL as well as PowerNV for quite
some time, and my daily work responsibilities includes taking care of
PowerPC RAS features. I, therefore would like to step up as a reviewer
to get notified of incoming changes in this area and help reviewing
them.

Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4a010d4807..6bbb725ec2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1749,6 +1749,7 @@ PowerPC RAS (Reliability, Availability and Serviceability)
 M: Aditya Gupta <adityag@linux.ibm.com>
 R: Sourabh Jain <sourabhjain@linux.ibm.com>
 R: Hari Bathini <hbathini@linux.ibm.com>
+R: Shivang Upadhyay <shivangu@linux.ibm.com>
 L: qemu-ppc@nongnu.org
 S: Maintained
 F: hw/ppc/spapr_events.c
-- 
2.54.0

Re: [PATCH 3/3] MAINTAINERS: add self as reviewer for PowerPC RAS

[Aditya Gupta]

On 14/07/26 22:17, Shivang Upadhyay wrote:

> I have been contributing to Fadump, MPIPL as well as PowerNV for quite
> some time, and my daily work responsibilities includes taking care of
> PowerPC RAS features. I, therefore would like to step up as a reviewer
> to get notified of incoming changes in this area and help reviewing
> them.
>
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4a010d4807..6bbb725ec2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1749,6 +1749,7 @@ PowerPC RAS (Reliability, Availability and Serviceability)
>   M: Aditya Gupta <adityag@linux.ibm.com>
>   R: Sourabh Jain <sourabhjain@linux.ibm.com>
>   R: Hari Bathini <hbathini@linux.ibm.com>
> +R: Shivang Upadhyay <shivangu@linux.ibm.com>
>   L: qemu-ppc@nongnu.org
>   S: Maintained
>   F: hw/ppc/spapr_events.c

Thanks for your helpful reviews Shivang !

Acked-by: Aditya Gupta <adityag@linux.ibm.com>


Thanks,

- Aditya G

Re: [PATCH 0/3] hw/ppc: reorg PowerPC RAS code for better maintainability

[Vaibhav Jain]

Hi Shivang,

Thanks for the patches. My review comments below:

Shivang Upadhyay <shivangu@linux.ibm.com> writes:

> Extracting RAS related code from spapr_rtas.c, to a newly created spapr_rtas_ras.c
> and carving out a MAINTAINERS entry dedicated to PowerPC RAS. This will cover RAS
> related functionalities for PowerPC platforms.
For a refactor-only change, this patch is adding 44 new lines to the
codebase which feels a bit heavy.

Just moving code from one file to a new file will reset the git blame
history of the code making future reviews difficult.

Such movement also makes code optimization difficult for compiler.

IMHO such code movement should only be done when:

* Its accompanied with patches for adding or fixing a functionality.
Or
* It clearly improves performance
Or
* Results in significant reduction in LOC

Since these patches doesnt seem to fit to any of the above criteria I
suggest you to reconsider these patches. 

>
> Also adding myself as a reviewer, to help share the review workload.
>
Thanks for volunteering as a reviwer for this code. Having more help
with reviews is always good :-)

> Shivang Upadhyay (3):
>   hw/ppc: move RAS-specific RTAS handlers to spapr_rtas_ras.c
>   MAINTAINERS: add dedicated PowerPC RAS section
>   MAINTAINERS: add self as reviewer for PowerPC RAS
>
>  MAINTAINERS             |  33 +++---
>  hw/ppc/meson.build      |   1 +
>  hw/ppc/spapr_rtas.c     | 189 --------------------------------
>  hw/ppc/spapr_rtas_ras.c | 232 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 249 insertions(+), 206 deletions(-)
>  create mode 100644 hw/ppc/spapr_rtas_ras.c
>
> -- 
> 2.54.0
>
>

-- 
Cheers
~ Vaibhav

Re: [PATCH 0/3] hw/ppc: reorg PowerPC RAS code for better maintainability

[Harsh Prateek Bora]

[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]

On Wed, 15 Jul, 2026, 8:10 pm Vaibhav Jain, <vaibhav@linux.ibm.com> wrote:

> Hi Shivang,
>
> Thanks for the patches. My review comments below:
>
> Shivang Upadhyay <shivangu@linux.ibm.com> writes:
>
> > Extracting RAS related code from spapr_rtas.c, to a newly created
> spapr_rtas_ras.c
> > and carving out a MAINTAINERS entry dedicated to PowerPC RAS. This will
> cover RAS
> > related functionalities for PowerPC platforms.
> For a refactor-only change, this patch is adding 44 new lines to the
> codebase which feels a bit heavy.
>
> Just moving code from one file to a new file will reset the git blame
> history of the code making future reviews difficult.
>
> Such movement also makes code optimization difficult for compiler.
>
> IMHO such code movement should only be done when:
>
> * Its accompanied with patches for adding or fixing a functionality.
> Or
> * It clearly improves performance
> Or
> * Results in significant reduction in LOC
>
> Since these patches doesnt seem to fit to any of the above criteria I
> suggest you to reconsider these patches.


I think the goal here is to ensure right people get notified to review RAS
related code changes. Other option would be to keep entire spapr_rtas.c
file listed under RAS and let it have non RAS related code also and do the
split later when it's bloated enough to be considered for split. Open to
suggestions.

Aditya, Sourabh, thoughts?


>
> >
> > Also adding myself as a reviewer, to help share the review workload.
> >
> Thanks for volunteering as a reviwer for this code. Having more help
> with reviews is always good :-)
>
> > Shivang Upadhyay (3):
> >   hw/ppc: move RAS-specific RTAS handlers to spapr_rtas_ras.c
> >   MAINTAINERS: add dedicated PowerPC RAS section
> >   MAINTAINERS: add self as reviewer for PowerPC RAS
> >
> >  MAINTAINERS             |  33 +++---
> >  hw/ppc/meson.build      |   1 +
> >  hw/ppc/spapr_rtas.c     | 189 --------------------------------
> >  hw/ppc/spapr_rtas_ras.c | 232 ++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 249 insertions(+), 206 deletions(-)
> >  create mode 100644 hw/ppc/spapr_rtas_ras.c
> >
> > --
> > 2.54.0
> >
> >
>
> --
> Cheers
> ~ Vaibhav
>
>

[-- Attachment #2: Type: text/html, Size: 3159 bytes --]

Re: [PATCH 0/3] hw/ppc: reorg PowerPC RAS code for better maintainability

[Sourabh Jain]

On 15/07/26 20:22, Harsh Prateek Bora wrote:
> On Wed, 15 Jul, 2026, 8: 10 pm Vaibhav Jain, 
> <vaibhav@ linux. ibm. com> wrote: Hi Shivang, Thanks for the patches. 
> My review comments below: Shivang Upadhyay <shivangu@ linux. ibm. com> 
> writes: > Extracting RAS related code from spapr_rtas. c,
> 
>
>
> On Wed, 15 Jul, 2026, 8:10 pm Vaibhav Jain, <vaibhav@linux.ibm.com> wrote:
>
>     Hi Shivang,
>
>     Thanks for the patches. My review comments below:
>
>     Shivang Upadhyay <shivangu@linux.ibm.com> writes:
>
>     > Extracting RAS related code from spapr_rtas.c, to a newly
>     created spapr_rtas_ras.c
>     > and carving out a MAINTAINERS entry dedicated to PowerPC RAS.
>     This will cover RAS
>     > related functionalities for PowerPC platforms.
>     For a refactor-only change, this patch is adding 44 new lines to the
>     codebase which feels a bit heavy.
>
>     Just moving code from one file to a new file will reset the git blame
>     history of the code making future reviews difficult.
>
>     Such movement also makes code optimization difficult for compiler.
>
>     IMHO such code movement should only be done when:
>
>     * Its accompanied with patches for adding or fixing a functionality.
>     Or
>     * It clearly improves performance
>     Or
>     * Results in significant reduction in LOC
>
>     Since these patches doesnt seem to fit to any of the above criteria I
>     suggest you to reconsider these patches.
>
>
> I think the goal here is to ensure right people get notified to review 
> RAS related code changes. Other option would be to keep entire 
> spapr_rtas.c file listed under RAS and let it have non RAS related 
> code also and do the split later when it's bloated enough to be 
> considered for split. Open to suggestions.
>
> Aditya, Sourabh, thoughts?

Yes, I think we can drop the code rearrangement done in 1/3 for now. 
And, as you said,
to bring the right people into the review, we can still consider the 2nd 
and 3rd patches.


- Sourabh Jain

>
>
>
>     >
>     > Also adding myself as a reviewer, to help share the review workload.
>     >
>     Thanks for volunteering as a reviwer for this code. Having more help
>     with reviews is always good :-)
>
>     > Shivang Upadhyay (3):
>     >   hw/ppc: move RAS-specific RTAS handlers to spapr_rtas_ras.c
>     >   MAINTAINERS: add dedicated PowerPC RAS section
>     >   MAINTAINERS: add self as reviewer for PowerPC RAS
>     >
>     >  MAINTAINERS             |  33 +++---
>     >  hw/ppc/meson.build      |   1 +
>     >  hw/ppc/spapr_rtas.c     | 189 --------------------------------
>     >  hw/ppc/spapr_rtas_ras.c | 232
>     ++++++++++++++++++++++++++++++++++++++++
>     >  4 files changed, 249 insertions(+), 206 deletions(-)
>     >  create mode 100644 hw/ppc/spapr_rtas_ras.c
>     >
>     > --
>     > 2.54.0
>     >
>     >
>
>     -- 
>     Cheers
>     ~ Vaibhav
>