Re: [Xen-devel] [RFC PATCH v3 05/18] xen/arm: ITS: Port ITS driver to xen

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Mon, Jun 29, 2015 at 9:13 PM, Vijay Kilari <vijay.kilari@xxxxxxxxx> wrote:
> On Mon, Jun 29, 2015 at 5:09 PM, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
>> On Mon, 2015-06-22 at 17:31 +0530, vijay.kilari@xxxxxxxxx wrote:
>> [...]
>>> +/*
>>> + * ITS command descriptors - parameters to be encoded in a command
>>> + * block.
>>> + */
>>> +struct its_cmd_desc {
>>> +    union {
>>> +        struct {
>>> +            struct its_collection *col;
>>> +            u32 event_id;
>>> +            u32 dev_id;
>>> +        } its_inv_cmd;
>> [...]
>>> +static struct its_collection *its_build_inv_cmd(its_cmd_block *cmd,
>>> +                                                struct its_cmd_desc *desc)
>>> +{
>>> +    memset(cmd, 0x0, sizeof(its_cmd_block));
>>> +    cmd->inv.cmd = GITS_CMD_INV;
>>> +    cmd->inv.devid = desc->its_inv_cmd.dev_id;
>>> +    cmd->inv.event = desc->its_inv_cmd.event_id;
>>> +
>>> +#ifdef DEBUG_GIC_ITS
>>> +    dump_cmd(cmd);
>>> +#endif
>>> +
>>> +    return desc->its_inv_cmd.col;
>>> +}
>> [...]
>>> +void its_send_inv(struct its_device *dev, struct its_collection *col,
>>> +                  u32 event_id)
>>> +{
>>> +    struct its_cmd_desc desc;
>>> +
>>> +    desc.its_inv_cmd.dev_id = dev->device_id;
>>> +    desc.its_inv_cmd.event_id = event_id;
>>> +    desc.its_inv_cmd.col = col;
>>> +
>>> +    its_send_single_command(dev->its, its_build_inv_cmd, &desc);
>>> +}
>> [...]
>>> +typedef struct __packed {
>>> +    u64 cmd:8;
>>> +    u64 res1:24;
>>> +    u64 devid:32;
>>> +    u64 event:32;
>>> +    u64 res2:32;
>>> +    u64 res3:64;
>>> +    u64 res4:64;
>>> +}inv_cmd_t;
>>
>> (I've trimmed this to just the INV command, but it's the same for all of
>> them)
>>
>> I suppose this is a mix of the way the Linux code was structured and my
>> request to use a struct/union to encode these things, but I'm afraid
>> this is not how I intended to suggest things be done.
>>
>> What I expected was something analogous to the hsr or lpae_t types, e.g.
>> a single:
>> union its_cmd {
>>     uint64_t bits[N];
>>
>>     struct {
>>         uint8_t cmd;
>>         uint8_t pad[...];
>>     } hdr;
>>
>>     struct {
>>         uint8_t cmd;
>>         uint8_t res1[3];
>>         uint32_t devid;
>>         uint32_t event;
>>         uint64_t res2[2];
>>     } inv;
>> };
>

 Commands like MAPD has 40 bit ITT, 5 bit Size and 1 bit valid bit. So we have
to still manage with bit fields to manage with ease. So we have to use
mix of bit fields
and normal types (uint8_t & uint32_t etc.,)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel