Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: add ibm, (get|set)-system-parameter

[Alexander Graf]

Am 15.11.2013 um 00:47 schrieb Alexey Kardashevskiy <address@hidden>:

> This adds very basic handlers for ibm,get-system-parameter and
> ibm,set-system-parameter RTAS calls.
> 
> The only parameter handled at the moment is
> "platform-processor-diagnostics-run-mode" which is always disabled and
> does not support changing. This is expected to make
> "ppc64_cpu --run-mode=1" happy.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> hw/ppc/spapr_rtas.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index eb542f2..c9328dc 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -224,6 +224,49 @@ 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,
> +                                          target_ulong args,
> +                                          uint32_t nret, target_ulong rets)
> +{
> +    target_ulong papameter = rtas_ld(args, 0);
> +    target_ulong buffer = rtas_ld(args, 1);
> +    target_ulong length = rtas_ld(args, 2);
> +
> +    switch (papameter) {
> +    case DIAGNOSTICS_RUN_MODE:
> +        if (length == 1) {
> +            stb_phys(buffer, 0);

Didn't we add magic in rtas_st to deal with 0xc00... addresses? Calling 
stb_phys would break that again.

> +            rtas_st(rets, 0, 0);
> +            return;
> +        }
> +        break;
> +    }
> +
> +    rtas_st(rets, 0, -3);

Please put the return value into a variable and only have this exit point from 
the function.

What does -3 mean?

> +}
> +
> +static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
> +                                          sPAPREnvironment *spapr,
> +                                          uint32_t token, uint32_t nargs,
> +                                          target_ulong args,
> +                                          uint32_t nret, target_ulong rets)
> +{
> +    target_ulong papameter = rtas_ld(args, 0);
> +    /* target_ulong buffer = rtas_ld(args, 1); */

?

> +
> +    switch (papameter) {
> +    case DIAGNOSTICS_RUN_MODE:
> +        rtas_st(rets, 0, -9002);

Magic number?

> +        return;
> +    }
> +
> +    rtas_st(rets, 0, -3);

Same comments as above.

Alex

> +}
> +
> static struct rtas_call {
>     const char *name;
>     spapr_rtas_fn fn;
> @@ -345,6 +388,10 @@ static void core_rtas_register_types(void)
>                         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",
> +                        rtas_ibm_get_system_parameter);
> +    spapr_rtas_register("ibm,set-system-parameter",
> +                        rtas_ibm_set_system_parameter);
> }
> 
> type_init(core_rtas_register_types)
> -- 
> 1.8.4.rc4
> 
>