Re: [Xen-devel] [PATCH v12] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.11.16 at 14:59, <andr2000@xxxxxxxxx> wrote:
> On 11/25/2016 02:54 PM, Jan Beulich wrote:
>>>>> On 25.11.16 at 12:57, <andr2000@xxxxxxxxx> wrote:
>>> +struct xensnd_page_directory {
>>> +    grant_ref_t gref_dir_next_page;
>>> +    uint32_t num_grefs;
>> You can't fit that many requests on one page anyway, so I think it
>> would be better to limit this to 16 bits, and leave 16 bits reserved
>> for future use (specified as must-be-zero).
> these are not for requests, but for buffers. you can imagine the size of 
> this
> buffer if we play something like 8 channels 192kHz...

I don't understand: You have one such entry per page, so how
can it be larger than (or even equal to) PAGE_SIZE / sizeof(grant_ref_t)?

>>> +union xensnd_req {
>>> +    struct {
>>> +        uint16_t id;
>>> +        uint8_t operation;
>>> +        uint8_t stream_idx;
>>> +        uint32_t padding;
>>> +        union {
>>> +            struct xensnd_open_req open;
>>> +            struct xensnd_close_req close;
>>> +            struct xensnd_write_req write;
>>> +            struct xensnd_read_req read;
>>> +            struct xensnd_get_vol_req get_vol;
>>> +            struct xensnd_set_vol_req set_vol;
>>> +            struct xensnd_mute_req mute;
>>> +            struct xensnd_unmute_req unmute;
>>> +        } op;
>>> +    } data;
>>> +    uint8_t raw[64];
>> This is still misplaced imo, and belongs into the inner union, with the
>> array size suitably reduced. After all the initial part is mean to be
>> common for all verbs aiui, i.e. including future ones.
> if we put this in then you have to find the biggest structure and 
> calculate its size so
> you know how many bytes need to be in that padding array
> if you change something you have to redo the same again and beware
> of the fact that this is needed

No, that's why I've asked for it to be a member of the inner union.

Jan


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