Re: [Embedded-pv-devel] [PATCH v10] xen: add para-virtual sound interface header file

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

>>>> +struct xensnd_close_req {
>>>> +};
>>>
>>>
>>> I'm afraid structures without any members aren't permitted by C89.
>>>
>> There are 2 options I see:
>> 1. Remove empty structures
>> 2. Leave them in, but add a comment and add a dummy place holder
>> I would prefer option 1 as this way the interface seems to be clearer,
>> like all requests are defined etc.
Typo: I would prefer option 2 with placeholders and comments
>>>>
>>>> +struct xensnd_request {
>>>> +     uint8_t raw[64];
>>>> +};
>>>> +
>>>> +struct xensnd_response {
>>>> +     uint8_t raw[64];
>>>> +};
>>>
>>>
>>> What are these two needed for now? If you want to enforce a
>>> minimum size, that would better go ...
>>>
>> That is because we want to be cache line aligned.
>
>
> But this is only accurate to your platform. There is HW available with 128
> bytes.
>
> For instance on Xen ARM, all the structures are aligned to 128 bytes in
> order to fit in most of cache line. Although technically it would be
> possible have bigger one (see the definition of CSSIDR).
>
So, then I will align to 32
>> What it means in practice
>> is not clear as we have so many HW around, so it is not possible to
>> fit all. So the options could be:
>> 1. 32 or 64
>> 2. 48 I think is not an option here
>
>
> Why 48?
It is not an option, saw something like this in fbif.h, but it is 40
actually there:
#define XENFB_OUT_EVENT_SIZE 40

>
> Regards,
>
> --
> Julien Grall
Best regards,
Oleksandr Andrushchenko

_______________________________________________
Embedded-pv-devel mailing list
Embedded-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/embedded-pv-devel