[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [Embedded-pv-devel] [PATCH v9] xen: add para-virtual sound interface header files



Hi Artem,

On 24/11/16 14:58, Artem Mygaiev wrote:
On 24.11.16 15:31, Jan Beulich wrote:
On 24.11.16 at 14:18, <artem_mygaiev@xxxxxxxx> wrote:
On 24.11.16 15:04, Jan Beulich wrote:
In my case I need it to at least define Linux specific __packed
attribute which is not supported by Win AFAIK.
... needs to avoid using platform specific constructs (or making
other assumptions on the platform). I can only re-iterate: Please
follow the model other PV protocols already present (without
repeating their mistakes, if at all possible).
But if we don't pack (or enforce some specific alignment) data
structures used between domains they may have different alignment on
different domains, no?
Let me repeat what I've said before: Please take a look at existing
interface headers. By properly placing fields and adding explicit
padding where needed, you can avoid such layout differences.

Don't get me wrong - I have checked the i/f headers before. My concern
is very generic: C standard defines alignment as an
*implementation-defined* integer, so current implementation is based
under two assumptions on the guest platforms: a) guest compiler does not
have "packing" enabled by default and b) alignment follows typical
values (1 byte for char, 2 bytes for short, ...). While this is true in
most cases, with some embedded RTOS this may become an issue.

I am not sure why you mentioned embedded RTOS, this is specific to the compiler and not an OS.

ARM provides a specification (see AAPCS [1] for ARM32 and AAPCS64 [2]) that explain the alignment of the structure and other stuff. Any compiler should implement for interoperability. If not, then you are already in trouble to issue hypercall to Xen when using standard headers.

x86 has fairly similar requirements as they are all sensible all not all specific to the ARM platform.

So it is possible to write header in the way which will accommodate all the architecture.


This is not critical/important ATM, and completely disconnected from the
purpose of the patch, so I agree it has to be rewritten according to
current practices.


[1] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf [2] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.