[Qemu-devel] [PATCHv2 0/3] Reduce abuse of rtas_st / rtas_ld

[Qemu-devel] [PATCHv2 0/3] Reduce abuse of rtas_st / rtas_ld

[David Gibson]

The rtas_ld() and rtas_st() helpers were designed for loading RTAS
arguments and storing RTAS returns which are in a simple, common array
format.

However, a number of RTAS routines - and even non-RTAS routines - have
started using these for accessing other memory buffers, where the
normal qemu memory access routines would be more appropriate.

This series removes some of these abuses of the RTAS accessors.

Changes in v2:
 * Reworked 1/3 to use a local helper instead of open-coding
 * Assorted small cleanups suggesed by Alexey Kardashevskiy

David Gibson (3):
  spapr: Small fixes to rtas_ibm_get_system_parameter, remove
    rtas_st_buffer
  spapr: Remove rtas_st_buffer_direct()
  spapr: Remove abuse of rtas_ld() in h_client_architecture_support

 hw/ppc/spapr_hcall.c   | 14 +++++++-------
 hw/ppc/spapr_rtas.c    | 39 ++++++++++++++++++++++++++++-----------
 include/hw/ppc/spapr.h | 36 +++++++++---------------------------
 3 files changed, 44 insertions(+), 45 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCHv2 1/3] spapr: Small fixes to rtas_ibm_get_system_parameter, remove rtas_st_buffer

[David Gibson]

rtas_st_buffer() appears in spapr.h as though it were a widely used helper,
but in fact it is only used for saving data in a format used by
rtas_ibm_get_system_parameter().  This changes it to a local helper more
specifically for that function.

While we're there fix a couple of small defects in
rtas_ibm_get_system_parameter:
  - For the string value SPLPAR_CHARACTERISTICS, it wasn't including the
    terminating \0 in the length which it should according to LoPAPR
    7.3.16.1
  - It now checks that the supplied buffer has at least enough space for
    the length of the returned data, and returns an error if it does not.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_rtas.c    | 22 ++++++++++++++++++----
 include/hw/ppc/spapr.h | 28 +++++++++-------------------
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 34b12a3..32cdd66 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -228,6 +228,20 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     env->msr = 0;
 }
 
+
+static inline int sysparm_st(target_ulong addr, target_ulong len,
+                             const void *val, uint16_t vallen)
+{
+    hwaddr phys = ppc64_phys_to_real(addr);
+
+    if (len < 2) {
+        return RTAS_OUT_SYSPARM_PARAM_ERROR;
+    }
+    stw_be_phys(&address_space_memory, phys, vallen);
+    cpu_physical_memory_write(phys + 2, val, MIN(len - 2, vallen));
+    return RTAS_OUT_SUCCESS;
+}
+
 static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                           sPAPRMachineState *spapr,
                                           uint32_t token, uint32_t nargs,
@@ -237,7 +251,7 @@ 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_SUCCESS;
+    target_ulong ret;
 
     switch (parameter) {
     case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
@@ -249,18 +263,18 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                           current_machine->ram_size / M_BYTE,
                                           smp_cpus,
                                           max_cpus);
-        rtas_st_buffer(buffer, length, (uint8_t *)param_val, strlen(param_val));
+        ret = sysparm_st(buffer, length, param_val, strlen(param_val) + 1);
         g_free(param_val);
         break;
     }
     case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE: {
         uint8_t param_val = DIAGNOSTICS_RUN_MODE_DISABLED;
 
-        rtas_st_buffer(buffer, length, &param_val, sizeof(param_val));
+        ret = sysparm_st(buffer, length, &param_val, sizeof(param_val));
         break;
     }
     case RTAS_SYSPARM_UUID:
-        rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
+        ret = sysparm_st(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
         break;
     default:
         ret = RTAS_OUT_NOT_SUPPORTED;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 53af76a..1e10fc9 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -408,14 +408,15 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
 #define RTAS_SLOT_PERM_ERR_LOG           2
 
 /* RTAS return codes */
-#define RTAS_OUT_SUCCESS            0
-#define RTAS_OUT_NO_ERRORS_FOUND    1
-#define RTAS_OUT_HW_ERROR           -1
-#define RTAS_OUT_BUSY               -2
-#define RTAS_OUT_PARAM_ERROR        -3
-#define RTAS_OUT_NOT_SUPPORTED      -3
-#define RTAS_OUT_NO_SUCH_INDICATOR  -3
-#define RTAS_OUT_NOT_AUTHORIZED     -9002
+#define RTAS_OUT_SUCCESS                        0
+#define RTAS_OUT_NO_ERRORS_FOUND                1
+#define RTAS_OUT_HW_ERROR                       -1
+#define RTAS_OUT_BUSY                           -2
+#define RTAS_OUT_PARAM_ERROR                    -3
+#define RTAS_OUT_NOT_SUPPORTED                  -3
+#define RTAS_OUT_NO_SUCH_INDICATOR              -3
+#define RTAS_OUT_NOT_AUTHORIZED                 -9002
+#define RTAS_OUT_SYSPARM_PARAM_ERROR            -9999
 
 /* RTAS tokens */
 #define RTAS_TOKEN_BASE      0x2000
@@ -513,17 +514,6 @@ static inline void rtas_st_buffer_direct(target_ulong phys,
                               MIN(buffer_len, phys_len));
 }
 
-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);
-    rtas_st_buffer_direct(phys + 2, phys_len - 2, buffer, buffer_len);
-}
-
 typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
                               uint32_t token,
                               uint32_t nargs, target_ulong args,
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 1/3] spapr: Small fixes to rtas_ibm_get_system_parameter, remove rtas_st_buffer

[Alexey Kardashevskiy]

On 01/19/2016 03:30 PM, David Gibson wrote:
> rtas_st_buffer() appears in spapr.h as though it were a widely used helper,
> but in fact it is only used for saving data in a format used by
> rtas_ibm_get_system_parameter().  This changes it to a local helper more
> specifically for that function.
>
> While we're there fix a couple of small defects in
> rtas_ibm_get_system_parameter:
>    - For the string value SPLPAR_CHARACTERISTICS, it wasn't including the
>      terminating \0 in the length which it should according to LoPAPR
>      7.3.16.1
>    - It now checks that the supplied buffer has at least enough space for
>      the length of the returned data, and returns an error if it does not.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr_rtas.c    | 22 ++++++++++++++++++----
>   include/hw/ppc/spapr.h | 28 +++++++++-------------------
>   2 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 34b12a3..32cdd66 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -228,6 +228,20 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>       env->msr = 0;
>   }
>
> +


Nit: unneeded empty line. Besides that,

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>



> +static inline int sysparm_st(target_ulong addr, target_ulong len,
> +                             const void *val, uint16_t vallen)
> +{
> +    hwaddr phys = ppc64_phys_to_real(addr);
> +
> +    if (len < 2) {
> +        return RTAS_OUT_SYSPARM_PARAM_ERROR;
> +    }
> +    stw_be_phys(&address_space_memory, phys, vallen);
> +    cpu_physical_memory_write(phys + 2, val, MIN(len - 2, vallen));
> +    return RTAS_OUT_SUCCESS;
> +}
> +
>   static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>                                             sPAPRMachineState *spapr,
>                                             uint32_t token, uint32_t nargs,
> @@ -237,7 +251,7 @@ 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_SUCCESS;
> +    target_ulong ret;
>
>       switch (parameter) {
>       case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
> @@ -249,18 +263,18 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>                                             current_machine->ram_size / M_BYTE,
>                                             smp_cpus,
>                                             max_cpus);
> -        rtas_st_buffer(buffer, length, (uint8_t *)param_val, strlen(param_val));
> +        ret = sysparm_st(buffer, length, param_val, strlen(param_val) + 1);
>           g_free(param_val);
>           break;
>       }
>       case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE: {
>           uint8_t param_val = DIAGNOSTICS_RUN_MODE_DISABLED;
>
> -        rtas_st_buffer(buffer, length, &param_val, sizeof(param_val));
> +        ret = sysparm_st(buffer, length, &param_val, sizeof(param_val));
>           break;
>       }
>       case RTAS_SYSPARM_UUID:
> -        rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
> +        ret = sysparm_st(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
>           break;
>       default:
>           ret = RTAS_OUT_NOT_SUPPORTED;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 53af76a..1e10fc9 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -408,14 +408,15 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
>   #define RTAS_SLOT_PERM_ERR_LOG           2
>
>   /* RTAS return codes */
> -#define RTAS_OUT_SUCCESS            0
> -#define RTAS_OUT_NO_ERRORS_FOUND    1
> -#define RTAS_OUT_HW_ERROR           -1
> -#define RTAS_OUT_BUSY               -2
> -#define RTAS_OUT_PARAM_ERROR        -3
> -#define RTAS_OUT_NOT_SUPPORTED      -3
> -#define RTAS_OUT_NO_SUCH_INDICATOR  -3
> -#define RTAS_OUT_NOT_AUTHORIZED     -9002
> +#define RTAS_OUT_SUCCESS                        0
> +#define RTAS_OUT_NO_ERRORS_FOUND                1
> +#define RTAS_OUT_HW_ERROR                       -1
> +#define RTAS_OUT_BUSY                           -2
> +#define RTAS_OUT_PARAM_ERROR                    -3
> +#define RTAS_OUT_NOT_SUPPORTED                  -3
> +#define RTAS_OUT_NO_SUCH_INDICATOR              -3
> +#define RTAS_OUT_NOT_AUTHORIZED                 -9002
> +#define RTAS_OUT_SYSPARM_PARAM_ERROR            -9999
>
>   /* RTAS tokens */
>   #define RTAS_TOKEN_BASE      0x2000
> @@ -513,17 +514,6 @@ static inline void rtas_st_buffer_direct(target_ulong phys,
>                                 MIN(buffer_len, phys_len));
>   }
>
> -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);
> -    rtas_st_buffer_direct(phys + 2, phys_len - 2, buffer, buffer_len);
> -}
> -
>   typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
>                                 uint32_t token,
>                                 uint32_t nargs, target_ulong args,
>


-- 
Alexey

Re: [Qemu-devel] [PATCHv2 1/3] spapr: Small fixes to rtas_ibm_get_system_parameter, remove rtas_st_buffer

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

On Tue, Jan 19, 2016 at 03:38:09PM +1100, Alexey Kardashevskiy wrote:
> On 01/19/2016 03:30 PM, David Gibson wrote:
> >rtas_st_buffer() appears in spapr.h as though it were a widely used helper,
> >but in fact it is only used for saving data in a format used by
> >rtas_ibm_get_system_parameter().  This changes it to a local helper more
> >specifically for that function.
> >
> >While we're there fix a couple of small defects in
> >rtas_ibm_get_system_parameter:
> >   - For the string value SPLPAR_CHARACTERISTICS, it wasn't including the
> >     terminating \0 in the length which it should according to LoPAPR
> >     7.3.16.1
> >   - It now checks that the supplied buffer has at least enough space for
> >     the length of the returned data, and returns an error if it does not.
> >
> >Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >---
> >  hw/ppc/spapr_rtas.c    | 22 ++++++++++++++++++----
> >  include/hw/ppc/spapr.h | 28 +++++++++-------------------
> >  2 files changed, 27 insertions(+), 23 deletions(-)
> >
> >diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >index 34b12a3..32cdd66 100644
> >--- a/hw/ppc/spapr_rtas.c
> >+++ b/hw/ppc/spapr_rtas.c
> >@@ -228,6 +228,20 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >      env->msr = 0;
> >  }
> >
> >+
> 
> 
> Nit: unneeded empty line. Besides that,

Ah, yes, fixed.

> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Thanks, I've merged this series into ppc-for-2.6.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Qemu-devel] [PATCHv2 2/3] spapr: Remove rtas_st_buffer_direct()

[David Gibson]

rtas_st_buffer_direct() is a not particularly useful wrapper around
cpu_physical_memory_write().  All the callers are in
rtas_ibm_configure_connector, where it's better handled by local helper.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_rtas.c    | 17 ++++++++++-------
 include/hw/ppc/spapr.h |  8 --------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 32cdd66..96759be 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -506,6 +506,13 @@ out:
 #define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
 #define CC_WA_LEN 4096
 
+static void configure_connector_st(target_ulong addr, target_ulong offset,
+                                   const void *buf, size_t len)
+{
+    cpu_physical_memory_write(ppc64_phys_to_real(addr + offset),
+                              buf, MIN(len, CC_WA_LEN - offset));
+}
+
 static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
                                          sPAPRMachineState *spapr,
                                          uint32_t token, uint32_t nargs,
@@ -571,8 +578,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
             /* provide the name of the next OF node */
             wa_offset = CC_VAL_DATA_OFFSET;
             rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
-            rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-                                  (uint8_t *)name, strlen(name) + 1);
+            configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
             resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
             break;
         case FDT_END_NODE:
@@ -597,8 +603,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
             /* provide the name of the next OF property */
             wa_offset = CC_VAL_DATA_OFFSET;
             rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
-            rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-                                  (uint8_t *)name, strlen(name) + 1);
+            configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
 
             /* provide the length and value of the OF property. data gets
              * placed immediately after NULL terminator of the OF property's
@@ -607,9 +612,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
             wa_offset += strlen(name) + 1,
             rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
             rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
-            rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-                                  (uint8_t *)((struct fdt_property *)prop)->data,
-                                  prop_len);
+            configure_connector_st(wa_addr, wa_offset, prop->data, prop_len);
             resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
             break;
         case FDT_END:
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 1e10fc9..1f9e722 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -506,14 +506,6 @@ 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_direct(target_ulong phys,
-                                         target_ulong phys_len,
-                                         uint8_t *buffer, uint16_t buffer_len)
-{
-    cpu_physical_memory_write(ppc64_phys_to_real(phys), buffer,
-                              MIN(buffer_len, phys_len));
-}
-
 typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
                               uint32_t token,
                               uint32_t nargs, target_ulong args,
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 2/3] spapr: Remove rtas_st_buffer_direct()

[Alexey Kardashevskiy]

On 01/19/2016 03:30 PM, David Gibson wrote:
> rtas_st_buffer_direct() is a not particularly useful wrapper around
> cpu_physical_memory_write().  All the callers are in
> rtas_ibm_configure_connector, where it's better handled by local helper.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>

> ---
>   hw/ppc/spapr_rtas.c    | 17 ++++++++++-------
>   include/hw/ppc/spapr.h |  8 --------
>   2 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 32cdd66..96759be 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -506,6 +506,13 @@ out:
>   #define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
>   #define CC_WA_LEN 4096
>
> +static void configure_connector_st(target_ulong addr, target_ulong offset,
> +                                   const void *buf, size_t len)
> +{
> +    cpu_physical_memory_write(ppc64_phys_to_real(addr + offset),
> +                              buf, MIN(len, CC_WA_LEN - offset));
> +}
> +
>   static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>                                            sPAPRMachineState *spapr,
>                                            uint32_t token, uint32_t nargs,
> @@ -571,8 +578,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>               /* provide the name of the next OF node */
>               wa_offset = CC_VAL_DATA_OFFSET;
>               rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
> -            rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
> -                                  (uint8_t *)name, strlen(name) + 1);
> +            configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
>               resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
>               break;
>           case FDT_END_NODE:
> @@ -597,8 +603,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>               /* provide the name of the next OF property */
>               wa_offset = CC_VAL_DATA_OFFSET;
>               rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
> -            rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
> -                                  (uint8_t *)name, strlen(name) + 1);
> +            configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
>
>               /* provide the length and value of the OF property. data gets
>                * placed immediately after NULL terminator of the OF property's
> @@ -607,9 +612,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>               wa_offset += strlen(name) + 1,
>               rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
>               rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
> -            rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
> -                                  (uint8_t *)((struct fdt_property *)prop)->data,
> -                                  prop_len);
> +            configure_connector_st(wa_addr, wa_offset, prop->data, prop_len);
>               resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
>               break;
>           case FDT_END:
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 1e10fc9..1f9e722 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -506,14 +506,6 @@ 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_direct(target_ulong phys,
> -                                         target_ulong phys_len,
> -                                         uint8_t *buffer, uint16_t buffer_len)
> -{
> -    cpu_physical_memory_write(ppc64_phys_to_real(phys), buffer,
> -                              MIN(buffer_len, phys_len));
> -}
> -
>   typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
>                                 uint32_t token,
>                                 uint32_t nargs, target_ulong args,
>


-- 
Alexey

[Qemu-devel] [PATCHv2 3/3] spapr: Remove abuse of rtas_ld() in h_client_architecture_support

[David Gibson]

h_client_architecture_support() uses rtas_ld() for general purpose memory
access, despite the fact that it's not an RTAS routine at all and rtas_ld
makes things more awkward.

Clean this up by replacing rtas_ld() calls with appropriate ldXX_phys()
calls.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr_hcall.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index cebceea..9dbdba9 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -861,7 +861,8 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
                                                   target_ulong opcode,
                                                   target_ulong *args)
 {
-    target_ulong list = args[0], ov_table;
+    target_ulong list = ppc64_phys_to_real(args[0]);
+    target_ulong ov_table, ov5;
     PowerPCCPUClass *pcc_ = POWERPC_CPU_GET_CLASS(cpu_);
     CPUState *cs;
     bool cpu_match = false, cpu_update = true, memory_update = false;
@@ -875,9 +876,9 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
     for (counter = 0; counter < 512; ++counter) {
         uint32_t pvr, pvr_mask;
 
-        pvr_mask = rtas_ld(list, 0);
+        pvr_mask = ldl_be_phys(&address_space_memory, list);
         list += 4;
-        pvr = rtas_ld(list, 0);
+        pvr = ldl_be_phys(&address_space_memory, list);
         list += 4;
 
         trace_spapr_cas_pvr_try(pvr);
@@ -948,14 +949,13 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
     /* For the future use: here @ov_table points to the first option vector */
     ov_table = list;
 
-    list = cas_get_option_vector(5, ov_table);
-    if (!list) {
+    ov5 = cas_get_option_vector(5, ov_table);
+    if (!ov5) {
         return H_SUCCESS;
     }
 
     /* @list now points to OV 5 */
-    list += 2;
-    ov5_byte2 = rtas_ld(list, 0) >> 24;
+    ov5_byte2 = ldub_phys(&address_space_memory, ov5 + 2);
     if (ov5_byte2 & OV5_DRCONF_MEMORY) {
         memory_update = true;
     }
-- 
2.5.0