Re: [Xen-devel] [PATCH RFC] xen-block: correctly define structures in public headers

[Ian Campbell <Ian.Campbell@citrix.com>]

On Tue, 2013-12-03 at 11:59 +0000, David Vrabel wrote:
> On 03/12/13 11:08, Ian Campbell wrote:
> > On Tue, 2013-12-03 at 11:01 +0000, David Vrabel wrote:
> >> On 03/12/13 10:57, Roger Pau Monne wrote:
> >>> Using __packed__ on the public interface is not correct, this
> >>> structures should be compiled using the native ABI, and __packed__
> >>> should only be used in the backend counterpart of those structures
> >>> (which needs to handle different ABIs).
> >>>
> >>> This was even worse in the ARM case, where the Linux kernel was
> >>> incorrectly using the X86_32 protocol ABI. This patch fixes it, but
> >>> also breaks compatibility, so an ARM DomU kernel compiled with
> >>> this patch will fail to communicate with PV disk devices unless the
> >>> Dom0 also has this patch.
> >>
> >> This ABI change needs to be justified.  Why do you think it is
> >> acceptable to break existing Linux guests?  Because I don't think it is.
> > 
> > As I explained in my reply those guests are buggy.
> 
> The kernel has a strong policy on not changing ABIs, even to fix bugs.
> I don't think a bug fix alone is sufficient justification for ABI breakage.

This is not the kernels ABI to fix in stone. The ABI is defined by the
upstream Xen project and Linux has failed to follow the specified ABI
correctly.

> 1. The ARM ABI for blkif was specified as uniform between 32-bit and
> 64-bit and is equivalent to the x86_64 ABI.

The ARM ABI is specified here:
http://xenbits.xen.org/docs/unstable/hypercall/arm/include,public,arch-arm.h.html#incontents_arm_abi

> 2. ARM 32-bit back and frontend implementation did /not/ use this
> defined ABI, but instead used the x86_32 ABI.  What did 32-bit ARM
> frontends report as their ABI? x86_32? or native?

They reported themselves as ARM (XEN_IO_PROTO_ABI_ARM == "arm") but they
unintentionally implemented something else which was akin to the x86_32
API. They didn't actually "implement the x86_32 API", it's perhaps
confusing to say that they did.

> 3. ARM 64-bit back and frontend implementation did use the specified
> ABI, but the backend is not compatible with 32-bit ARM guests.  What did
> 64-bit ARM frontends report as their ABI?  x86_64? or native?

They report themselves as ARM (XEN_IO_PROTO_ABI_ARM == "arm").

> 4. Support for 64-bit ARM guests is not upstream in Linux yet (so I
> don't mind if 64-bit guests are broken).

Yes, it is upstream.

> I think this should be resolved in a backward compatible way.

No. Linux is in error here and should be fixed. We have done this before
(e.g. with event channels being the wrong bit size).

There is no deployed legacy of guests to support here. At this stage we
have not even formally committed to keeping the hypercall ABI stable. We
will likely do so with the Xen 4.4 release.

Ian.