Re: Should we revert "mm: New XENMEM space, XENMAPSPACE_gmfn_range"?

[Keir Fraser <keir@xen.org>]

On 01/08/2012 18:55, "Stefano Stabellini" <Stefano.Stabellini@eu.citrix.com>
wrote:

> On Wed, 16 Nov 2011, Jean Guyader wrote:
>> 
>> XENMAPSPACE_gmfn_range is like XENMAPSPACE_gmfn but it runs on
>> a range of pages. The size of the range is defined in a new field.
>> 
>> This new field .size is located in the 16 bits padding between .domid
>> and .space in struct xen_add_to_physmap to stay compatible with older
>> versions.
>> 
>> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> 
> Hi all,
> I was reading more about this commit because this patch breaks the ABI
> on ARM, when I realized that on x86 there is no standard that specifies
> the alignment of fields in a struct.
> As a consequence I don't think we can really be sure that between .domid
> and .space we always have 16 bits of padding.

Well, on x86 there *is* 16 bits of padding between the .domid and .space
fields. That's our ABI. Regardless of whether we rely on not-really-existent
padding rules in the compiler. If someone compiles in an environment that
aligns stuff differently, we have to rewrite our headers. :)

We don't have a supported ABI on ARM until 4.2.0 is released at the
earliest.

Should we be changing our rules on public headers, allowing
compiler-specific extensions to precisely lay out our structures? Quite
possibly. We used to do that, but it got shot down by the ppc and ia64
arches who wanted an easier life and just rely on compiler default layout
for a particular platform. Of course those maintainers aren't actually
voting any more. ;)

 -- Keir

> I am afraid that if a user compiles Linux or another guest kernel with a
> compiler other than gcc, this hypercall might break. In fact it already
> happened just switching from x86 to ARM.
> Also, considering that the memory.h interface is supposed to be ANSI C,
> isn't it wrong to assume compiler specific artifacts anyway?
> Considering that we haven't made any releases yet with the change in
> memory.h, shouldn't we revert the commit before it is too late?
> 
> - Stefano