qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH] spapr: Fix RTAS token numbers


From: Alexander Graf
Subject: Re: [Qemu-ppc] [PATCH] spapr: Fix RTAS token numbers
Date: Mon, 23 Jun 2014 14:50:31 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.6.0


On 23.06.14 11:27, Alexey Kardashevskiy wrote:
At the moment spapr_rtas_register() allocates a new token number for every
new RTAS callback so numbers are not fixed and depend on the number of
supported RTAS handlers and the exact order of spapr_rtas_register() calls.
These tokens are copied into the device tree and remain the same during
the guest lifetime.

When we start another guest to receive a migration, it calls
spapr_rtas_register() as well. If the number of RTAS handlers or their
order is different in QEMU on source and destination sides, the "/rtas"
node in the device tree will differ. Since migration overwrites the device
tree (as it overwrites the entire RAM), the actual RTAS config on
the destination side gets broken.

This defines global contant values for every RTAS token which QEMU
is using today.

This changes spapr_rtas_register() to accept a token number instead of
allocating one. This changes all users of spapr_rtas_register().

This changes XICS-KVM not to cache tokens registered with KVM as they
constant now.

This makes TOKEN_BASE global as RTAS_XXX use TOKEN_BASE as
a base. TOKEN_MAX is moved+renamed too to keep them together.

This reserves token numbers for "os-term" handlers and PCI hotplug
which we are working on.

Cc: Michael Roth <address@hidden>
Cc: Nikunj A Dadhania <address@hidden>
Signed-off-by: Alexey Kardashevskiy <address@hidden>
---
  hw/intc/xics.c         |  8 +++---
  hw/intc/xics_kvm.c     | 22 ++++++----------
  hw/nvram/spapr_nvram.c |  4 +--
  hw/ppc/spapr_events.c  |  3 ++-
  hw/ppc/spapr_pci.c     | 18 ++++++++-----
  hw/ppc/spapr_rtas.c    | 69 +++++++++++++++++++++++++-------------------------
  hw/ppc/spapr_vio.c     |  5 ++--
  include/hw/ppc/spapr.h | 40 ++++++++++++++++++++++++++++-
  8 files changed, 104 insertions(+), 65 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 76dd6f5..493a2a4 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -866,10 +866,10 @@ static void xics_realize(DeviceState *dev, Error **errp)
      }
/* Registration of global state belongs into realize */
-    spapr_rtas_register("ibm,set-xive", rtas_set_xive);
-    spapr_rtas_register("ibm,get-xive", rtas_get_xive);
-    spapr_rtas_register("ibm,int-off", rtas_int_off);
-    spapr_rtas_register("ibm,int-on", rtas_int_on);
+    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);
+    spapr_rtas_register(RTAS_IBM_INT_ON, "ibm,int-on", rtas_int_on);
spapr_register_hypercall(H_CPPR, h_cppr);
      spapr_register_hypercall(H_IPI, h_ipi);
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index 09476ae..4704c98 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -38,10 +38,6 @@
  typedef struct KVMXICSState {
      XICSState parent_obj;
- uint32_t set_xive_token;
-    uint32_t get_xive_token;
-    uint32_t int_off_token;
-    uint32_t int_on_token;
      int kernel_xics_fd;
  } KVMXICSState;
@@ -392,32 +388,30 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp)
          goto fail;
      }
- icpkvm->set_xive_token = spapr_rtas_register("ibm,set-xive", rtas_dummy);
-    icpkvm->get_xive_token = spapr_rtas_register("ibm,get-xive", rtas_dummy);
-    icpkvm->int_off_token = spapr_rtas_register("ibm,int-off", rtas_dummy);
-    icpkvm->int_on_token = spapr_rtas_register("ibm,int-on", rtas_dummy);
+    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(icpkvm->set_xive_token,
-                                         "ibm,set-xive");
+    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");
          goto fail;
      }
- rc = kvmppc_define_rtas_kernel_token(icpkvm->get_xive_token,
-                                         "ibm,get-xive");
+    rc = kvmppc_define_rtas_kernel_token(RTAS_IBM_GET_XIVE, "ibm,get-xive");
      if (rc < 0) {
          error_setg(errp, "kvmppc_define_rtas_kernel_token: ibm,get-xive");
          goto fail;
      }
- rc = kvmppc_define_rtas_kernel_token(icpkvm->int_on_token, "ibm,int-on");
+    rc = kvmppc_define_rtas_kernel_token(RTAS_IBM_INT_ON, "ibm,int-on");
      if (rc < 0) {
          error_setg(errp, "kvmppc_define_rtas_kernel_token: ibm,int-on");
          goto fail;
      }
- rc = kvmppc_define_rtas_kernel_token(icpkvm->int_off_token, "ibm,int-off");
+    rc = kvmppc_define_rtas_kernel_token(RTAS_IBM_INT_OFF, "ibm,int-off");
      if (rc < 0) {
          error_setg(errp, "kvmppc_define_rtas_kernel_token: ibm,int-off");
          goto fail;
diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
index af49c46..6a72ef4 100644
--- a/hw/nvram/spapr_nvram.c
+++ b/hw/nvram/spapr_nvram.c
@@ -153,8 +153,8 @@ static int spapr_nvram_init(VIOsPAPRDevice *dev)
          return -1;
      }
- spapr_rtas_register("nvram-fetch", rtas_nvram_fetch);
-    spapr_rtas_register("nvram-store", rtas_nvram_store);
+    spapr_rtas_register(RTAS_NVRAM_FETCH, "nvram-fetch", rtas_nvram_fetch);
+    spapr_rtas_register(RTAS_NVRAM_STORE, "nvram-store", rtas_nvram_store);
return 0;
  }
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 16fa49e..5ce96a7 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -317,5 +317,6 @@ void spapr_events_init(sPAPREnvironment *spapr)
      spapr->epow_irq = spapr_allocate_msi(0);
      spapr->epow_notifier.notify = spapr_powerdown_req;
      qemu_register_powerdown_notifier(&spapr->epow_notifier);
-    spapr_rtas_register("check-exception", check_exception);
+    spapr_rtas_register(RTAS_CHECK_EXCEPTION, "check-exception",
+                        check_exception);
  }
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 988f8cb..8607c88 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -909,14 +909,20 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
void spapr_pci_rtas_init(void)
  {
-    spapr_rtas_register("read-pci-config", rtas_read_pci_config);
-    spapr_rtas_register("write-pci-config", rtas_write_pci_config);
-    spapr_rtas_register("ibm,read-pci-config", rtas_ibm_read_pci_config);
-    spapr_rtas_register("ibm,write-pci-config", rtas_ibm_write_pci_config);
+    spapr_rtas_register(RTAS_READ_PCI_CONFIG, "read-pci-config",
+                        rtas_read_pci_config);
+    spapr_rtas_register(RTAS_WRITE_PCI_CONFIG, "write-pci-config",
+                        rtas_write_pci_config);
+    spapr_rtas_register(RTAS_IBM_READ_PCI_CONFIG, "ibm,read-pci-config",
+                        rtas_ibm_read_pci_config);
+    spapr_rtas_register(RTAS_IBM_WRITE_PCI_CONFIG, "ibm,write-pci-config",
+                        rtas_ibm_write_pci_config);
      if (msi_supported) {
-        spapr_rtas_register("ibm,query-interrupt-source-number",
+        spapr_rtas_register(RTAS_IBM_QUERY_INTERRUPT_SOURCE_NUMBER,
+                            "ibm,query-interrupt-source-number",
                              rtas_ibm_query_interrupt_source_number);
-        spapr_rtas_register("ibm,change-msi", rtas_ibm_change_msi);
+        spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi",
+                            rtas_ibm_change_msi);
      }
  }
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index ea4a2b2..6746f61 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -35,9 +35,6 @@
#include <libfdt.h> -#define TOKEN_BASE 0x2000
-#define TOKEN_MAX       0x100
-
  static void rtas_display_character(PowerPCCPU *cpu, sPAPREnvironment *spapr,
                                     uint32_t token, uint32_t nargs,
                                     target_ulong args,
@@ -270,17 +267,15 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
  static struct rtas_call {
      const char *name;
      spapr_rtas_fn fn;
-} rtas_table[TOKEN_MAX];
-
-static struct rtas_call *rtas_next = rtas_table;
+} rtas_table[RTAS_TOKEN_MAX];
target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPREnvironment *spapr,
                               uint32_t token, uint32_t nargs, target_ulong 
args,
                               uint32_t nret, target_ulong rets)
  {
-    if ((token >= TOKEN_BASE)
-        && ((token - TOKEN_BASE) < TOKEN_MAX)) {
-        struct rtas_call *call = rtas_table + (token - TOKEN_BASE);
+    if ((token >= RTAS_TOKEN_BASE)
+        && ((token - RTAS_TOKEN_BASE) < RTAS_TOKEN_MAX)) {
+        struct rtas_call *call = rtas_table + (token - RTAS_TOKEN_BASE);
if (call->fn) {
              call->fn(cpu, spapr, token, nargs, args, nret, rets);
@@ -302,23 +297,21 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, 
sPAPREnvironment *spapr,
      return H_PARAMETER;
  }
-int spapr_rtas_register(const char *name, spapr_rtas_fn fn)
+void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
  {
-    int i;
-
-    for (i = 0; i < (rtas_next - rtas_table); i++) {
-        if (strcmp(name, rtas_table[i].name) == 0) {
-            fprintf(stderr, "RTAS call \"%s\" registered twice\n", name);
-            exit(1);
-        }
+    if ((token < RTAS_TOKEN_BASE) ||
+        (token >= RTAS_TOKEN_BASE + RTAS_TOKEN_MAX)) {
+        fprintf(stderr, "RTAS invalid token 0x%x\n", token);
+        exit(1);
+    }
+    if (rtas_table[token].name) {
+        fprintf(stderr, "RTAS call \"%s\" is registered already as 0x%x\n",
+                rtas_table[token].name, token);
+        exit(1);
      }
- assert(rtas_next < (rtas_table + TOKEN_MAX));
-
-    rtas_next->name = name;
-    rtas_next->fn = fn;
-
-    return (rtas_next++ - rtas_table) + TOKEN_BASE;
+    rtas_table[token - RTAS_TOKEN_BASE].name = name;
+    rtas_table[token - RTAS_TOKEN_BASE].fn = fn;
  }
int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
@@ -358,7 +351,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
rtas_addr,
          return ret;
      }
- for (i = 0; i < TOKEN_MAX; i++) {
+    for (i = 0; i < RTAS_TOKEN_MAX; i++) {
          struct rtas_call *call = &rtas_table[i];
if (!call->name) {
@@ -366,7 +359,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
rtas_addr,
          }
ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
-                                    i + TOKEN_BASE);
+                                    i + RTAS_TOKEN_BASE);
          if (ret < 0) {
              fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
                      call->name, fdt_strerror(ret));
@@ -379,18 +372,24 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
rtas_addr,
static void core_rtas_register_types(void)
  {
-    spapr_rtas_register("display-character", rtas_display_character);
-    spapr_rtas_register("get-time-of-day", rtas_get_time_of_day);
-    spapr_rtas_register("set-time-of-day", rtas_set_time_of_day);
-    spapr_rtas_register("power-off", rtas_power_off);
-    spapr_rtas_register("system-reboot", rtas_system_reboot);
-    spapr_rtas_register("query-cpu-stopped-state",
+    spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
+                        rtas_display_character);
+    spapr_rtas_register(RTAS_GET_TIME_OF_DAY, "get-time-of-day",
+                        rtas_get_time_of_day);
+    spapr_rtas_register(RTAS_SET_TIME_OF_DAY, "set-time-of-day",
+                        rtas_set_time_of_day);
+    spapr_rtas_register(RTAS_POWER_OFF, "power-off", rtas_power_off);
+    spapr_rtas_register(RTAS_SYSTEM_REBOOT, "system-reboot",
+                        rtas_system_reboot);
+    spapr_rtas_register(RTAS_QUERY_CPU_STOPPED_STATE, 
"query-cpu-stopped-state",
                          rtas_query_cpu_stopped_state);
-    spapr_rtas_register("start-cpu", rtas_start_cpu);
-    spapr_rtas_register("stop-self", rtas_stop_self);
-    spapr_rtas_register("ibm,get-system-parameter",
+    spapr_rtas_register(RTAS_START_CPU, "start-cpu", rtas_start_cpu);
+    spapr_rtas_register(RTAS_STOP_SELF, "stop-self", rtas_stop_self);
+    spapr_rtas_register(RTAS_IBM_GET_SYSTEM_PARAMETER,
+                        "ibm,get-system-parameter",
                          rtas_ibm_get_system_parameter);
-    spapr_rtas_register("ibm,set-system-parameter",
+    spapr_rtas_register(RTAS_IBM_SET_SYSTEM_PARAMETER,
+                        "ibm,set-system-parameter",
                          rtas_ibm_set_system_parameter);
  }
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 04e16ae..a195fd1 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -517,8 +517,9 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
      spapr_register_hypercall(H_ENABLE_CRQ, h_enable_crq);
/* RTAS calls */
-    spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
-    spapr_rtas_register("quiesce", rtas_quiesce);
+    spapr_rtas_register(RTAS_IBM_SET_TCE_BYPASS, "ibm,set-tce-bypass",
+                        rtas_set_tce_bypass);
+    spapr_rtas_register(RTAS_QUIESCE, "quiesce", rtas_quiesce);
return bus;
  }
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 08c301f..86bcdb3 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -358,6 +358,44 @@ static inline int spapr_allocate_lsi(int hint)
  #define RTAS_OUT_NOT_SUPPORTED      -3
  #define RTAS_OUT_NOT_AUTHORIZED     -9002
+/* RTAS tokens */
+#define RTAS_TOKEN_BASE      0x2000
+#define RTAS_TOKEN_MAX       0x100

Now that you have a nice list of all RTAS token numbers...

+
+#define RTAS_DISPLAY_CHARACTER                  (RTAS_TOKEN_BASE + 0x00)
+#define RTAS_GET_TIME_OF_DAY                    (RTAS_TOKEN_BASE + 0x01)
+#define RTAS_SET_TIME_OF_DAY                    (RTAS_TOKEN_BASE + 0x02)
+#define RTAS_POWER_OFF                          (RTAS_TOKEN_BASE + 0x03)
+#define RTAS_SYSTEM_REBOOT                      (RTAS_TOKEN_BASE + 0x04)
+#define RTAS_QUERY_CPU_STOPPED_STATE            (RTAS_TOKEN_BASE + 0x05)
+#define RTAS_START_CPU                          (RTAS_TOKEN_BASE + 0x06)
+#define RTAS_STOP_SELF                          (RTAS_TOKEN_BASE + 0x07)
+#define RTAS_IBM_GET_SYSTEM_PARAMETER           (RTAS_TOKEN_BASE + 0x08)
+#define RTAS_IBM_SET_SYSTEM_PARAMETER           (RTAS_TOKEN_BASE + 0x09)
+#define RTAS_IBM_SET_XIVE                       (RTAS_TOKEN_BASE + 0x0A)
+#define RTAS_IBM_GET_XIVE                       (RTAS_TOKEN_BASE + 0x0B)
+#define RTAS_IBM_INT_OFF                        (RTAS_TOKEN_BASE + 0x0C)
+#define RTAS_IBM_INT_ON                         (RTAS_TOKEN_BASE + 0x0D)
+#define RTAS_CHECK_EXCEPTION                    (RTAS_TOKEN_BASE + 0x0E)
+#define RTAS_EVENT_SCAN                         (RTAS_TOKEN_BASE + 0x0F)
+#define RTAS_IBM_SET_TCE_BYPASS                 (RTAS_TOKEN_BASE + 0x10)
+#define RTAS_QUIESCE                            (RTAS_TOKEN_BASE + 0x11)
+#define RTAS_NVRAM_FETCH                        (RTAS_TOKEN_BASE + 0x12)
+#define RTAS_NVRAM_STORE                        (RTAS_TOKEN_BASE + 0x13)
+#define RTAS_READ_PCI_CONFIG                    (RTAS_TOKEN_BASE + 0x14)
+#define RTAS_WRITE_PCI_CONFIG                   (RTAS_TOKEN_BASE + 0x15)
+#define RTAS_IBM_READ_PCI_CONFIG                (RTAS_TOKEN_BASE + 0x16)
+#define RTAS_IBM_WRITE_PCI_CONFIG               (RTAS_TOKEN_BASE + 0x17)
+#define RTAS_IBM_QUERY_INTERRUPT_SOURCE_NUMBER  (RTAS_TOKEN_BASE + 0x18)
+#define RTAS_IBM_CHANGE_MSI                     (RTAS_TOKEN_BASE + 0x19)
+#define RTAS_SET_INDICATOR                      (RTAS_TOKEN_BASE + 0x1A)
+#define RTAS_SET_POWER_LEVEL                    (RTAS_TOKEN_BASE + 0x1B)
+#define RTAS_GET_POWER_LEVEL                    (RTAS_TOKEN_BASE + 0x1C)
+#define RTAS_GET_SENSOR_STATE                   (RTAS_TOKEN_BASE + 0x1D)
+#define RTAS_IBM_CONFIGURE_CONNECTOR            (RTAS_TOKEN_BASE + 0x1E)
+#define RTAS_IBM_OS_TERM                        (RTAS_TOKEN_BASE + 0x1F)
+#define RTAS_IBM_EXTENDED_OS_TERM               (RTAS_TOKEN_BASE + 0x20)

... you can just define RTAS_TOKEN_MAX as the last one.

#define RTAS_TOKEN_MAX        (RTAS_TOKEN_BASE + 0x21)

Then you can also do all the boundary checks on [RTAS_TOKEN_BASE..RTAS_TOKEN_MAX].

Apart from that tiny nit I like the patch.


Alex

+
  static inline uint64_t ppc64_phys_to_real(uint64_t addr)
  {
      return addr & ~0xF000000000000000ULL;
@@ -377,7 +415,7 @@ typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, 
sPAPREnvironment *spapr,
                                uint32_t token,
                                uint32_t nargs, target_ulong args,
                                uint32_t nret, target_ulong rets);
-int spapr_rtas_register(const char *name, spapr_rtas_fn fn);
+void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn);
  target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPREnvironment *spapr,
                               uint32_t token, uint32_t nargs, target_ulong 
args,
                               uint32_t nret, target_ulong rets);




reply via email to

[Prev in Thread] Current Thread [Next in Thread]