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

[Qemu-devel] [PATCH 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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 08c301f..39a7764 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -373,6 +373,23 @@ 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)
+{
+    uint16_t i;
+
+    if (phys_len < 2) {
+        return;
+    }
+    stw_be_phys(&address_space_memory,
+                ppc64_phys_to_real(phys), buffer_len);
+    for (i = 0; i < MIN(buffer_len, phys_len - 2); i++) {
+        stb_phys(&address_space_memory,
+                 ppc64_phys_to_real(phys + 2 + i), buffer[i]);
+    }
+}
+
 typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPREnvironment *spapr,
                               uint32_t token,
                               uint32_t nargs, target_ulong args,
-- 
1.9.0

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

[Alexander Graf]

On 24.06.14 02:22, Sam Bobroff wrote:
> 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 | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 08c301f..39a7764 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -373,6 +373,23 @@ 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)
> +{
> +    uint16_t i;
> +
> +    if (phys_len < 2) {
> +        return;
> +    }
> +    stw_be_phys(&address_space_memory,
> +                ppc64_phys_to_real(phys), buffer_len);
> +    for (i = 0; i < MIN(buffer_len, phys_len - 2); i++) {
> +        stb_phys(&address_space_memory,
> +                 ppc64_phys_to_real(phys + 2 + i), buffer[i]);
> +    }

cpu_physical_memory_write()?


Alex

[Qemu-devel] [PATCH 2/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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index ea4a2b2..4f87673 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -225,6 +225,7 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPREnvironment *spapr,
 }
 
 #define DIAGNOSTICS_RUN_MODE        42
+#define UUID                        48
 
 static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                           sPAPREnvironment *spapr,
@@ -244,6 +245,10 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
             ret = RTAS_OUT_SUCCESS;
         }
         break;
+    case UUID:
+        rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
+        ret = RTAS_OUT_SUCCESS;
+        break;
     }
 
     rtas_st(rets, 0, ret);
@@ -260,6 +265,7 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
 
     switch (parameter) {
     case DIAGNOSTICS_RUN_MODE:
+    case UUID:
         ret = RTAS_OUT_NOT_AUTHORIZED;
         break;
     }
-- 
1.9.0

[Qemu-devel] [PATCH 3/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 function return value code is also slightly simplified.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 hw/ppc/spapr_rtas.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 4f87673..8d94845 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -236,19 +236,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;
+    uint8_t zero = 0;
 
     switch (parameter) {
     case DIAGNOSTICS_RUN_MODE:
-        if (length == 1) {
-            rtas_st(buffer, 0, 0);
-            ret = RTAS_OUT_SUCCESS;
-        }
+        rtas_st_buffer(buffer, length, &zero, sizeof zero);
         break;
     case UUID:
         rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
-        ret = RTAS_OUT_SUCCESS;
         break;
+    default:
+        ret = RTAS_OUT_NOT_SUPPORTED;
     }
 
     rtas_st(rets, 0, ret);
-- 
1.9.0

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

[Alexander Graf]

On 24.06.14 02:22, 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 function return value code is also slightly simplified.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
>   hw/ppc/spapr_rtas.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 4f87673..8d94845 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -236,19 +236,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;
> +    uint8_t zero = 0;
>   
>       switch (parameter) {
>       case DIAGNOSTICS_RUN_MODE:
> -        if (length == 1) {
> -            rtas_st(buffer, 0, 0);
> -            ret = RTAS_OUT_SUCCESS;
> -        }
> +        rtas_st_buffer(buffer, length, &zero, sizeof zero);

Please use sizeof(zero) here :).


Alex

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

[Alexander Graf]

On 24.06.14 13:58, Alexander Graf wrote:
>
> On 24.06.14 02:22, 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 function return value code is also slightly simplified.
>>
>> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
>> ---
>>   hw/ppc/spapr_rtas.c | 11 +++++------
>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 4f87673..8d94845 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -236,19 +236,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;
>> +    uint8_t zero = 0;
>>         switch (parameter) {
>>       case DIAGNOSTICS_RUN_MODE:
>> -        if (length == 1) {
>> -            rtas_st(buffer, 0, 0);
>> -            ret = RTAS_OUT_SUCCESS;
>> -        }
>> +        rtas_st_buffer(buffer, length, &zero, sizeof zero);
>
> Please use sizeof(zero) here :).

Also while at it, please make the code slightly more readable. We don't 
pass in "zero", we pass in DIAGNOSTICS_RUN_MODE_DISABLED, no? Please 
define the possible values in a header file and set it accordingly here.

Since it's limited to DIAGNOSTICS_RUN_MODE, please also make the 
variable limited to that scope then.


Alex

[Qemu-devel] [PATCH 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 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 8d94845..4270e7a 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -224,6 +224,7 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     env->msr = 0;
 }
 
+#define SPLPAR_CHARACTERISTICS      20
 #define DIAGNOSTICS_RUN_MODE        42
 #define UUID                        48
 
@@ -238,8 +239,20 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
     target_ulong length = rtas_ld(args, 2);
     target_ulong ret = RTAS_OUT_SUCCESS;
     uint8_t zero = 0;
+    uint8_t param_buf[64];
+    int param_len;
 
     switch (parameter) {
+    case SPLPAR_CHARACTERISTICS:
+        param_len = snprintf((char *)param_buf, sizeof param_buf,
+                             "MaxEntCap=%d,MaxPlatProcs=%d",
+                             max_cpus, smp_cpus);
+        if (param_len >= 0) {
+            rtas_st_buffer(buffer, length, param_buf, param_len);
+        } else {
+            ret = RTAS_OUT_HW_ERROR;
+        }
+        break;
     case DIAGNOSTICS_RUN_MODE:
         rtas_st_buffer(buffer, length, &zero, sizeof zero);
         break;
-- 
1.9.0

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

[Alexander Graf]

On 24.06.14 02:22, Sam Bobroff wrote:
> 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 | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 8d94845..4270e7a 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -224,6 +224,7 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>       env->msr = 0;
>   }
>   
> +#define SPLPAR_CHARACTERISTICS      20
>   #define DIAGNOSTICS_RUN_MODE        42
>   #define UUID                        48
>   
> @@ -238,8 +239,20 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>       target_ulong length = rtas_ld(args, 2);
>       target_ulong ret = RTAS_OUT_SUCCESS;
>       uint8_t zero = 0;
> +    uint8_t param_buf[64];
> +    int param_len;
>   
>       switch (parameter) {
> +    case SPLPAR_CHARACTERISTICS:
> +        param_len = snprintf((char *)param_buf, sizeof param_buf,
> +                             "MaxEntCap=%d,MaxPlatProcs=%d",
> +                             max_cpus, smp_cpus);

We have a nice g_strdup_printf() helper function that allocates memory 
for us automatically based on the printf size. Just use that one here ;).


Alex