spapr: improve RTAS get_system_parameter

PATCH Applied Sam Bobroff 2 participants 8 messages Jun 25, 2014

Share & tools

Sam Bobroff

author

2014-06-25 03:55 UTC #0 lore raw

v2 cover spapr: improve RTAS get_system_parameter

This series of patches improves QEMU's emulation of the RTAS call "ibm,get-system-parameter" by adding support for the "UUID" and "SPLPAR Characteristics" parameters as well as fixing a problem in the the implementation of the "Platform Processor Diagnostics Run Mode" parameter.

It also improves the surrounding code a little to support the addition of additional system parameters.

Patchset changelog:

v2:
* Reordered patches to correct DIAGNOSTICS_RUN_MODE earlier so that new RTAS constants can be added at the same time and all of them moved out into spapr.h.
* Use g_strdup_printf()/g_free() rather than snprintf() to a stack array.
* Use cpu_physical_memory_write() instead of looping over each byte.

Sam Bobroff (4):
spapr: Add rtas_st_buffer utility function
spapr: Fix RTAS sysparm DIAGNOSTICS_RUN_MODE
spapr: Add RTAS sysparm UUID
spapr: Add RTAS sysparm SPLPAR Characteristics

 hw/ppc/spapr_rtas.c    | 30 +++++++++++++++++++++---------
 include/hw/ppc/spapr.h | 28 ++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 9 deletions(-)

Signature · 1 line

1.9.0

Sam Bobroff

author

2014-06-25 03:54 UTC #1 lore raw

PATCH v2 1/4 spapr: Add rtas_st_buffer utility function

Add a function to write lengh + data into a buffer as required for the emulation of the RTAS ibm,get-system-parameter call.

If the destination is smaller than the source, the write is truncated and success is returned. This matches the behaviour of pHyp.

This will be used in following patches.

Signed-off-by Sam Bobroff <[email protected]>

---
 include/hw/ppc/spapr.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

include/hw/ppc/spapr.h +13 -0

index 08c301f..088baba 100644
@@ -373,6 +373,19 @@ static inline void rtas_st(target_ulong phys, int n, uint32_t val)
     stl_be_phys(&address_space_memory, ppc64_phys_to_real(phys + 4*n), val);
 }
 
+
+static inline void rtas_st_buffer(target_ulong phys, target_ulong phys_len,
+                                  uint8_t *buffer, uint16_t buffer_len)
+{
+    if (phys_len < 2) {
+        return;
+    }
+    stw_be_phys(&address_space_memory,
+                ppc64_phys_to_real(phys), buffer_len);
+    cpu_physical_memory_write(ppc64_phys_to_real(phys + 2),
+                              buffer, MIN(buffer_len, phys_len - 2));
+}
+
 typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPREnvironment *spapr,
                               uint32_t token,
                               uint32_t nargs, target_ulong args,

Signature · 1 line

1.9.0

Sam Bobroff

author

2014-06-25 03:54 UTC #2 lore raw

PATCH v2 2/4 spapr: Fix RTAS sysparm DIAGNOSTICS_RUN_MODE

In reply to Sam Bobroff

This allows the ibm,get-system-parameter RTAS call to succeed for the DIAGNOSTICS_RUN_MODE system parameter.

The problem can be seen with "ppc64_cpu --run-mode" from the powerpc-utils package which fails before this patch with "Machine does not support diagnostic run mode".

This is corrected by using the rtas_st_buffer() function to write to the buffer.

The RTAS constants are also moved out into a header file, some new constants added and the surrounding code slightly simplified.

Signed-off-by Sam Bobroff <[email protected]>

---
 hw/ppc/spapr_rtas.c    | 18 +++++++++---------
 include/hw/ppc/spapr.h | 13 +++++++++++++
 2 files changed, 22 insertions(+), 9 deletions(-)

hw/ppc/spapr_rtas.c +9 -9

index ea4a2b2..d3b57a1 100644
@@ -224,8 +224,6 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     env->msr = 0;
 }
 
-#define DIAGNOSTICS_RUN_MODE        42
-
 static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                           sPAPREnvironment *spapr,
                                           uint32_t token, uint32_t nargs,

@@ -235,16 +233,18 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
     target_ulong parameter = rtas_ld(args, 0);
     target_ulong buffer = rtas_ld(args, 1);
     target_ulong length = rtas_ld(args, 2);
-    target_ulong ret = RTAS_OUT_NOT_SUPPORTED;
+    target_ulong ret = RTAS_OUT_SUCCESS;
 
     switch (parameter) {
-    case DIAGNOSTICS_RUN_MODE:
-        if (length == 1) {
-            rtas_st(buffer, 0, 0);
-            ret = RTAS_OUT_SUCCESS;
-        }
+    case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE: {
+        uint8_t param_val = DIAGNOSTICS_RUN_MODE_DISABLED;
+
+        rtas_st_buffer(buffer, length, &param_val, sizeof(param_val));
         break;
     }
+    default:
+        ret = RTAS_OUT_NOT_SUPPORTED;
+    }
 
     rtas_st(rets, 0, ret);
 }

@@ -259,7 +259,7 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
     target_ulong ret = RTAS_OUT_NOT_SUPPORTED;
 
     switch (parameter) {
-    case DIAGNOSTICS_RUN_MODE:
+    case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE:
         ret = RTAS_OUT_NOT_AUTHORIZED;
         break;
     }

include/hw/ppc/spapr.h +13 -0

index 088baba..9b28e96 100644
@@ -358,6 +358,19 @@ static inline int spapr_allocate_lsi(int hint)
 #define RTAS_OUT_NOT_SUPPORTED      -3
 #define RTAS_OUT_NOT_AUTHORIZED     -9002
 
+/* RTAS ibm,get-system-parameter token values */
+#define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE        42
+
+/* Possible values for the platform-processor-diagnostics-run-mode parameter
+ * of the RTAS ibm,get-system-parameter call.
+ * (Only _DISABLED is currently used. The rest are the possible values defined
+ * by SPAPR.)
+ */
+#define DIAGNOSTICS_RUN_MODE_DISABLED  0
+#define DIAGNOSTICS_RUN_MODE_STAGGERED 1
+#define DIAGNOSTICS_RUN_MODE_IMMEDIATE 2
+#define DIAGNOSTICS_RUN_MODE_PERIODIC  3
+
 static inline uint64_t ppc64_phys_to_real(uint64_t addr)
 {
     return addr & ~0xF000000000000000ULL;

Signature · 1 line

1.9.0

Alexander Graf

2014-06-25 11:10 UTC #3 lore raw

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 2/4] spapr: Fix RTAS sysparm DIAGNOSTICS_RUN_MODE

Quoted 71 lines · Sam Bobroff

This allows the ibm,get-system-parameter RTAS call to succeed for the DIAGNOSTICS_RUN_MODE system parameter.

The problem can be seen with "ppc64_cpu --run-mode" from the powerpc-utils package which fails before this patch with "Machine does not support diagnostic run mode".

This is corrected by using the rtas_st_buffer() function to write to the buffer.

The RTAS constants are also moved out into a header file, some new constants added and the surrounding code slightly simplified.

Signed-off-by Sam Bobroff <[email protected]>

---
  hw/ppc/spapr_rtas.c    | 18 +++++++++---------
  include/hw/ppc/spapr.h | 13 +++++++++++++
  2 files changed, 22 insertions(+), 9 deletions(-)

hw/ppc/spapr_rtas.c +9 -9

index ea4a2b2..d3b57a1 100644
@@ -224,8 +224,6 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPREnvironment *spapr,
      env->msr = 0;
  }
 
-#define DIAGNOSTICS_RUN_MODE        42
-
  static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                            sPAPREnvironment *spapr,
                                            uint32_t token, uint32_t nargs,

@@ -235,16 +233,18 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
      target_ulong parameter = rtas_ld(args, 0);
      target_ulong buffer = rtas_ld(args, 1);
      target_ulong length = rtas_ld(args, 2);
-    target_ulong ret = RTAS_OUT_NOT_SUPPORTED;
+    target_ulong ret = RTAS_OUT_SUCCESS;
 
      switch (parameter) {
-    case DIAGNOSTICS_RUN_MODE:
-        if (length == 1) {
-            rtas_st(buffer, 0, 0);
-            ret = RTAS_OUT_SUCCESS;
-        }
+    case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE: {
+        uint8_t param_val = DIAGNOSTICS_RUN_MODE_DISABLED;
+
+        rtas_st_buffer(buffer, length, &param_val, sizeof(param_val));
          break;
      }
+    default:
+        ret = RTAS_OUT_NOT_SUPPORTED;
+    }
 
      rtas_st(rets, 0, ret);
  }

@@ -259,7 +259,7 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
      target_ulong ret = RTAS_OUT_NOT_SUPPORTED;
 
      switch (parameter) {
-    case DIAGNOSTICS_RUN_MODE:
+    case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE:
          ret = RTAS_OUT_NOT_AUTHORIZED;
          break;
      }

include/hw/ppc/spapr.h +0 -0

index 088baba..9b28e96 100644

include/hw/ppc/spapr.h static inline int spapr_allocate_lsi(int hint) · line 367

Quoted 10 lines

include/hw/ppc/spapr.h +7 -0

@@ -358,6 +358,19 @@ static inline int spapr_allocate_lsi(int hint)
  #define RTAS_OUT_NOT_SUPPORTED      -3
  #define RTAS_OUT_NOT_AUTHORIZED     -9002
 
+/* RTAS ibm,get-system-parameter token values */
+#define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE        42
+
+/* Possible values for the platform-processor-diagnostics-run-mode parameter
+ * of the RTAS ibm,get-system-parameter call.
+ * (Only _DISABLED is currently used. The rest are the possible values defined
+ * by SPAPR.)

Comments like these are incredibly hard to keep in sync. When another patch adds support to return a different value, it's very likely nobody updates the comment above because it's out of scope.

So I'll remove the 2 lines above while applying the patch.

Alex

Quoted 9 lines

include/hw/ppc/spapr.h +6 -0

+ */
+#define DIAGNOSTICS_RUN_MODE_DISABLED  0
+#define DIAGNOSTICS_RUN_MODE_STAGGERED 1
+#define DIAGNOSTICS_RUN_MODE_IMMEDIATE 2
+#define DIAGNOSTICS_RUN_MODE_PERIODIC  3
+
  static inline uint64_t ppc64_phys_to_real(uint64_t addr)
  {
      return addr & ~0xF000000000000000ULL;

Sam Bobroff

author

2014-06-25 03:54 UTC #4 lore raw

PATCH v2 3/4 spapr: Add RTAS sysparm UUID

In reply to Sam Bobroff

Add support for the UUID parameter to the emulated RTAS call ibm,get-system-parameter.

Return the guest's UUID as the value for the RTAS UUID system parameter, or null (a zero length result) if it is not set.

Signed-off-by Sam Bobroff <[email protected]>

---
 hw/ppc/spapr_rtas.c    | 4 ++++
 include/hw/ppc/spapr.h | 1 +
 2 files changed, 5 insertions(+)

hw/ppc/spapr_rtas.c +4 -0

index d3b57a1..9804873 100644
@@ -242,6 +242,9 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
         rtas_st_buffer(buffer, length, &param_val, sizeof(param_val));
         break;
     }
+    case RTAS_SYSPARM_UUID:
+        rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
+        break;
     default:
         ret = RTAS_OUT_NOT_SUPPORTED;
     }

@@ -260,6 +263,7 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
 
     switch (parameter) {
     case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE:
+    case RTAS_SYSPARM_UUID:
         ret = RTAS_OUT_NOT_AUTHORIZED;
         break;
     }

include/hw/ppc/spapr.h +1 -0

index 9b28e96..fe02fb4 100644
@@ -360,6 +360,7 @@ static inline int spapr_allocate_lsi(int hint)
 
 /* RTAS ibm,get-system-parameter token values */
 #define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE        42
+#define RTAS_SYSPARM_UUID                        48
 
 /* Possible values for the platform-processor-diagnostics-run-mode parameter
  * of the RTAS ibm,get-system-parameter call.

Signature · 1 line

1.9.0

Sam Bobroff

author

2014-06-25 03:55 UTC #5 lore raw

PATCH v2 4/4 spapr: Add RTAS sysparm SPLPAR Characteristics

In reply to Sam Bobroff

Add support for the SPLPAR Characteristics parameter to the emulated RTAS call ibm,get-system-parameter.

The support provides just enough information to allow "cat /proc/powerpc/lparcfg" to succeed without generating a kernel error message.

Without this patch the above command will produce the following kernel
message: arch/powerpc/platforms/pseries/lparcfg.c \
parse_system_parameter_string Error calling get-system-parameter \
(0xfffffffd)

Signed-off-by Sam Bobroff <[email protected]>

---
 hw/ppc/spapr_rtas.c    | 8 ++++++++
 include/hw/ppc/spapr.h | 1 +
 2 files changed, 9 insertions(+)

hw/ppc/spapr_rtas.c +8 -0

index 9804873..a58abe6 100644
@@ -236,6 +236,13 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
     target_ulong ret = RTAS_OUT_SUCCESS;
 
     switch (parameter) {
+    case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
+        char *param_val = g_strdup_printf("MaxEntCap=%d,MaxPlatProcs=%d",
+                                          max_cpus, smp_cpus);
+        rtas_st_buffer(buffer, length, (uint8_t *)param_val, strlen(param_val));
+        g_free(param_val);
+        break;
+    }
     case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE: {
         uint8_t param_val = DIAGNOSTICS_RUN_MODE_DISABLED;

@@ -262,6 +269,7 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
     target_ulong ret = RTAS_OUT_NOT_SUPPORTED;
 
     switch (parameter) {
+    case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS:
     case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE:
     case RTAS_SYSPARM_UUID:
         ret = RTAS_OUT_NOT_AUTHORIZED;

include/hw/ppc/spapr.h +1 -0

index fe02fb4..c67dc22 100644
@@ -359,6 +359,7 @@ static inline int spapr_allocate_lsi(int hint)
 #define RTAS_OUT_NOT_AUTHORIZED     -9002
 
 /* RTAS ibm,get-system-parameter token values */
+#define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS      20
 #define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE        42
 #define RTAS_SYSPARM_UUID                        48

Signature · 1 line

1.9.0

Alexander Graf

Applied

2014-06-25 11:12 UTC #6 lore raw

In reply to Sam Bobroff

Quoted 7 lines · Sam Bobroff

This series of patches improves QEMU's emulation of the RTAS call "ibm,get-system-parameter" by adding support for the "UUID" and "SPLPAR Characteristics" parameters as well as fixing a problem in the the implementation of the "Platform Processor Diagnostics Run Mode" parameter.
It also improves the surrounding code a little to support the addition of additional system parameters.

Thanks, applied to ppc-next.

For the next patch set, please also mention changes from one version to the next in the individual patches :).

Alex

Sam Bobroff

author

2014-06-25 23:38 UTC #7 lore raw

Quoted 7 lines · Alexander Graf

Quoted 5 lines · Sam Bobroff
This series of patches improves QEMU's emulation of the RTAS call "ibm,get-system-parameter" by adding support for the "UUID" and "SPLPAR Characteristics" parameters as well as fixing a problem in the the implementation of the "Platform Processor Diagnostics Run Mode" parameter.

Quoted 6 lines

Quoted 2 lines
It also improves the surrounding code a little to support the addition of additional system parameters.
Thanks, applied to ppc-next.
For the next patch set, please also mention changes from one version to the next in the individual patches :).

I will, thanks for the feedback :-)

Quoted 1 line

Alex

Cheers,
Sam.