xics/spapr: Register RTAS/hypercalls once at machine init

QEMU may crash when running a spapr machine in 'dual' interrupt controller
mode on some older (but not that old, eg. ubuntu 18.04.2) KVMs with partial
XIVE support:

qemu-system-ppc64: hw/ppc/spapr_rtas.c:411: spapr_rtas_register:
 Assertion `!name || !rtas_table[token].name' failed.

XICS is controlled by the guest thanks to a set of RTAS calls. Depending
on whether KVM XICS is used or not, the RTAS calls are handled by KVM or
QEMU. In both cases, QEMU needs to expose the RTAS calls to the guest
through the "rtas" node of the device tree.

The spapr_rtas_register() helper takes care of all of that: it adds the
RTAS call token to the "rtas" node and registers a QEMU callback to be
invoked when the guest issues the RTAS call. In the KVM XICS case, QEMU
registers a dummy callback that just prints an error since it isn't
supposed to be invoked, ever.

Historically, the XICS controller was setup during machine init and
released during final teardown. This changed when the 'dual' interrupt
controller mode was added to the spapr machine: in this case we need
to tear the XICS down and set it up again during machine reset. The
crash happens because we indeed have an incompatibility with older
KVMs that forces QEMU to fallback on emulated XICS, which tries to
re-registers the same RTAS calls.

This could be fixed by adding proper rollback that would unregister
RTAS calls on error. But since the emulated RTAS calls in QEMU can
now detect when they are mistakenly called while KVM XICS is in
use, it seems simpler to register them once and for all at machine
init. This fixes the crash and allows to remove some now useless
lines of code.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <156044429963.125694.13710679451927268758.stgit@bahia.lab.toulouse-stg.fr.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

hw/intc/xics_kvm.c

@@ -331,15 +331,6 @@ void ics_kvm_set_irq(ICSState *ics, int srcno, int val)
     }
 }
 
-static void rtas_dummy(PowerPCCPU *cpu, SpaprMachineState *spapr,
-                       uint32_t token,
-                       uint32_t nargs, target_ulong args,
-                       uint32_t nret, target_ulong rets)
-{
-    error_report("pseries: %s must never be called for in-kernel XICS",
-                 __func__);
-}
-
 int xics_kvm_init(SpaprMachineState *spapr, Error **errp)
 {
     int rc;
@@ -360,11 +351,6 @@ int xics_kvm_init(SpaprMachineState *spapr, Error **errp)
         goto fail;
     }
 
-    spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_dummy);
-    spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_dummy);
-    spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_dummy);
-    spapr_rtas_register(RTAS_IBM_INT_ON, "ibm,int-on", rtas_dummy);
-
     rc = kvmppc_define_rtas_kernel_token(RTAS_IBM_SET_XIVE, "ibm,set-xive");
     if (rc < 0) {
         error_setg(errp, "kvmppc_define_rtas_kernel_token: ibm,set-xive");
@@ -454,11 +440,6 @@ void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp)
     close(kernel_xics_fd);
     kernel_xics_fd = -1;
 
-    spapr_rtas_unregister(RTAS_IBM_SET_XIVE);
-    spapr_rtas_unregister(RTAS_IBM_GET_XIVE);
-    spapr_rtas_unregister(RTAS_IBM_INT_OFF);
-    spapr_rtas_unregister(RTAS_IBM_INT_ON);
-
     kvmppc_define_rtas_kernel_token(0, "ibm,set-xive");
     kvmppc_define_rtas_kernel_token(0, "ibm,get-xive");
     kvmppc_define_rtas_kernel_token(0, "ibm,int-on");

hw/intc/xics_spapr.c

@@ -285,14 +285,6 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr,
 
 void xics_spapr_init(SpaprMachineState *spapr)
 {
-    /* Emulated mode can only be initialized once. */
-    if (spapr->ics->init) {
-        return;
-    }
-
-    spapr->ics->init = true;
-
-    /* Registration of global state belongs into realize */
     spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive);
     spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive);
     spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off);

hw/ppc/spapr_irq.c

@@ -114,6 +114,8 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs,
     }
 
     spapr->ics = ICS_BASE(obj);
+
+    xics_spapr_init(spapr);
 }
 
 #define ICS_IRQ_FREE(ics, srcno)   \
@@ -236,7 +238,6 @@ static const char *spapr_irq_get_nodename_xics(SpaprMachineState *spapr)
 
 static void spapr_irq_init_emu_xics(SpaprMachineState *spapr, Error **errp)
 {
-    xics_spapr_init(spapr);
 }
 
 static void spapr_irq_init_kvm_xics(SpaprMachineState *spapr, Error **errp)

include/hw/ppc/spapr.h

@@ -676,10 +676,6 @@ typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, SpaprMachineState *sm,
                               uint32_t nargs, target_ulong args,
                               uint32_t nret, target_ulong rets);
 void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn);
-static inline void spapr_rtas_unregister(int token)
-{
-    spapr_rtas_register(token, NULL, NULL);
-}
 target_ulong spapr_rtas_call(PowerPCCPU *cpu, SpaprMachineState *sm,
                              uint32_t token, uint32_t nargs, target_ulong args,
                              uint32_t nret, target_ulong rets);

include/hw/ppc/xics.h

@@ -119,7 +119,6 @@ struct ICSState {
     uint32_t offset;
     ICSIRQState *irqs;
     XICSFabric *xics;
-    bool init; /* sPAPR ICS device initialized */
 };
 
 #define ICS_PROP_XICS "xics"

include/hw/ppc/xics_spapr.h

@@ -36,5 +36,6 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt,
 int xics_kvm_init(SpaprMachineState *spapr, Error **errp);
 void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp);
 void xics_spapr_init(SpaprMachineState *spapr);
+void xics_spapr_connect(SpaprMachineState *spapr);
 
 #endif /* XICS_SPAPR_H */