On 17.11.2013, at 22:09, Alexey Kardashevskiy <[email protected]> wrote:
> 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 <[email protected]> > --- > Changes: > v2: > * addressed comments from Alex Graf > --- > hw/ppc/spapr_rtas.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index eb542f2..8053a67 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -224,6 +224,50 @@ 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); > + target_ulong ret = -3; /* System parameter is not supported */ Is this an RTAS function defined return value or a global RTAS return value? > + > + switch (papameter) { > + case DIAGNOSTICS_RUN_MODE: > + if (length == 1) { > + rtas_st(buffer, 0, 0); > + ret = 0; /* Success */ I assume this one is RTAS global? > + } > + break; > + } > + > + rtas_st(rets, 0, ret); > +} > + > +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); */ Not addressed. Just remove this line until it gets used. > + target_ulong ret = -3; /* System parameter is not supported */ > + > + switch (papameter) { > + case DIAGNOSTICS_RUN_MODE: > + ret = -9002; /* Setting not allowed/authorized */ -9002 seems to always mean "Not Authorized". In fact, reading through the PAPR spec it seems as if we can easily give return values reasonable names: #define RTAS_OUT_SUCCESS 0 #define RTAS_OUT_ERROR -1 #define RTAS_OUT_BUSY -2 #define RTAS_OUT_NOT_SUPPORTED -3 #define RTAS_OUT_NOMEM -5 #define RTAS_OUT_NOT_AUTHORIZED -9002 #define RTAS_OUT_PARAM_ERROR -9999 We can't always have a 1:1 map between name and number, but at least name -> number should always be unique: (ibm,configure-connector) #define RTAS_OUT_DR_CONN_UNUSABLE -9003 #define RTAS_OUT_DR_CONN_NOT_SUPPORTED -9002 #define RTAS_OUT_DR_SYSTEM_NOT_SUPPORTED -9001 Do you see cases where this wouldn't work out? That way we don't have to have explicit comments in the code explaining what a return value really means, making the code more easy to read. Alex
