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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Oleksandr,

On 24/11/16 17:33, Oleksandr Andrushchenko wrote:
> > > > > +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
To confirm, you don't plan to reduce the size of the command packet to 
32 octet?
> > > 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
But your 32 value seems to be as random as 40. I guess they define a 
size that would fit all the command and would potentially allow the 
addition of commands.
--
Julien Grall

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