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

[Alexander Graf <agraf@suse.de>]

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