Re: [Qemu-devel] [PATCH v2] spapr: Add "memop" hypercall

[Alexander Graf]

On 25.05.2012, at 11:24, Benjamin Herrenschmidt <address@hidden> wrote:

> On Fri, 2012-05-25 at 10:54 +0200, Alexander Graf wrote:
> 
>>>> case x:
>>>>   foo();
>>>>   break();
>>>> 
>>>>> break;
>>>>> +        case 1: tmp = lduw_phys(src); break;
>>>>> +        case 2: tmp = ldl_phys(src);  break;
>>>>> +        case 3: tmp = ldq_phys(src);  break;
>>>>> +        default:
>>>>> +        return H_PARAMETER;
>>> 
>>> Checkpatch absolutely complained and I decided to ignore it, seriously,
>>> you really want to replace a nice & readable piece of code with
>>> something that takes 3 pages and is generally gross & ugly ?
>>> 
>>> Some times, you have to ignore check patch and let sanity prevail.
>> 
>> I'm not all that keen on coding style rules. But check out
>> arch/powerpc/kvm/emulate.c and tell me that it's a good idea to go
>> with this "clean" approach. If you want it really clean, put the whole
>> chunk above into a geberic helper that allows for everyone to say
>> "read n bytes of data with native endianness into a u64". In that
>> code, the more verbose coding style checkpatch suggests doesn't hurt
>> and your function becomes even easier to read :)
> 
> I find your lack of taste disturbing Luke :-)

Heh :). I merely try to not collide with laws I can't influence easily and 
don't bother to change enough ;).

> 
>> Yeah, it's a shame I didn't read through it more thoroughly earlier - at 
>> least it didn't take weeks in this round ;).
>> 
>> No worries though, if you can't make it until Monday, I'll fix it up myself 
>> afterwards :). There's no black magic involved here,
>> so I should be ok to respin myself.
> 
> Sure. No worries. To test it properly you really need a newer SLOF and
> the patch to add -vga tho, I'll sort that out when I'm back.

Hrm. Send them over in a private mail and push the WIP SLOF to a branch 
somewhere then please. There are some community folks interested in VGA with -M 
pseries, so I'd like to at least have it prototype style available for folks to 
play with :)


Alex