Re: [Qemu-devel] [PATCH 2/2] pseries: Implement hcall-bulk hypervisor interface

[Alexander Graf <agraf@suse.de>]

On 09/01/2011 03:50 AM, David Gibson wrote:
> On Wed, Aug 31, 2011 at 11:22:18AM +0200, Alexander Graf wrote:
>> On 11.08.2011, at 04:36, David Gibson wrote:
> [snip]
>>> +#define H_BULK_REMOVE_TYPE             0xc000000000000000ULL
>>> +#define   H_BULK_REMOVE_REQUEST        0x4000000000000000ULL
>>> +#define   H_BULK_REMOVE_RESPONSE       0x8000000000000000ULL
>>> +#define   H_BULK_REMOVE_END            0xc000000000000000ULL
>>> +#define H_BULK_REMOVE_CODE             0x3000000000000000ULL
>>> +#define   H_BULK_REMOVE_SUCCESS        0x0000000000000000ULL
>>> +#define   H_BULK_REMOVE_NOT_FOUND      0x1000000000000000ULL
>>> +#define   H_BULK_REMOVE_PARM           0x2000000000000000ULL
>>> +#define   H_BULK_REMOVE_HW             0x3000000000000000ULL
>>> +#define H_BULK_REMOVE_RC               0x0c00000000000000ULL
>>> +#define H_BULK_REMOVE_FLAGS            0x0300000000000000ULL
>>> +#define   H_BULK_REMOVE_ABSOLUTE       0x0000000000000000ULL
>>> +#define   H_BULK_REMOVE_ANDCOND        0x0100000000000000ULL
>>> +#define   H_BULK_REMOVE_AVPN           0x0200000000000000ULL
>>> +#define H_BULK_REMOVE_PTEX             0x00ffffffffffffffULL
>> indenting looks broken.
> Nope, that's intended.  The unindented ones are the field masks in the
> parameter, the intended ones are the defined values for the
> corresponding field.

Interesting :). Makes sense now that you explain it - looked awkward at 
first.

>>> +
>>> +static target_ulong h_bulk_remove(CPUState *env, sPAPREnvironment *spapr,
>>> +                                  target_ulong opcode, target_ulong *args)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i<  4; i++) {
>>> +        target_ulong *tsh =&args[i*2];
>>> +        target_ulong tsl = args[i*2 + 1];
>> Mind to replace all those magic numbers by something more verbose?
> So, "all those" == 2.  I can replace the 4 with something I guess, but
> changing the 2 would just be silly - that code is just taking the
> arguments a pair at a time.

Yup. Thanks! Applied.


Alex