[Qemu-devel] [PATCH v2 0/4] spapr: improve RTAS get_system_parameter

[Qemu-devel] [PATCH v2 0/4] spapr: improve RTAS get_system_parameter

[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.

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(-)

-- 
1.9.0

[Qemu-devel] [PATCH v2 1/4] spapr: Add rtas_st_buffer utility function

[Sam Bobroff]

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 <sam.bobroff@au1.ibm.com>
---
 include/hw/ppc/spapr.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 08c301f..088baba 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -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,
-- 
1.9.0

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

[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 <sam.bobroff@au1.ibm.com>
---
 hw/ppc/spapr_rtas.c    | 18 +++++++++---------
 include/hw/ppc/spapr.h | 13 +++++++++++++
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index ea4a2b2..d3b57a1 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -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;
     }
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 088baba..9b28e96 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -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;
-- 
1.9.0

[Qemu-devel] [PATCH v2 3/4] spapr: Add RTAS sysparm UUID

[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 <sam.bobroff@au1.ibm.com>
---
 hw/ppc/spapr_rtas.c    | 4 ++++
 include/hw/ppc/spapr.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index d3b57a1..9804873 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -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;
     }
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 9b28e96..fe02fb4 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -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.
-- 
1.9.0

[Qemu-devel] [PATCH v2 4/4] spapr: Add RTAS sysparm SPLPAR Characteristics

[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 <sam.bobroff@au1.ibm.com>
---
 hw/ppc/spapr_rtas.c    | 8 ++++++++
 include/hw/ppc/spapr.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 9804873..a58abe6 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -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;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index fe02fb4..c67dc22 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -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
 
-- 
1.9.0

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

[Alexander Graf]

On 25.06.14 05:54, Sam Bobroff wrote:
> 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 <sam.bobroff@au1.ibm.com>
> ---
>   hw/ppc/spapr_rtas.c    | 18 +++++++++---------
>   include/hw/ppc/spapr.h | 13 +++++++++++++
>   2 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index ea4a2b2..d3b57a1 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -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;
>       }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 088baba..9b28e96 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -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

> + */
> +#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;

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/4] spapr: improve RTAS get_system_parameter

[Alexander Graf]

On 25.06.14 05:54, Sam Bobroff wrote:
> 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

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/4] spapr: improve RTAS get_system_parameter

[Sam Bobroff]

On 25/06/14 21:12, Alexander Graf wrote:
> 
> On 25.06.14 05:54, Sam Bobroff wrote:
>> 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 :).

I will, thanks for the feedback :-)
 
> Alex

Cheers,
Sam.