public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] xen-block: correctly define structures in public headers
@ 2013-12-03 10:57 Roger Pau Monne
  2013-12-03 11:01 ` David Vrabel
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Roger Pau Monne @ 2013-12-03 10:57 UTC (permalink / raw)
  To: xen-devel, linux-kernel
  Cc: Roger Pau Monne, Julien Grall, Konrad Rzeszutek Wilk,
	David Vrabel, Boris Ostrovsky, Stefano Stabellini

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.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reported-by: Julien Grall <julien.grall@linaro.org>
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 include/xen/interface/io/blkif.h |   28 +++++++---------------------
 1 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/include/xen/interface/io/blkif.h b/include/xen/interface/io/blkif.h
index 65e1209..002ea22 100644
--- a/include/xen/interface/io/blkif.h
+++ b/include/xen/interface/io/blkif.h
@@ -141,14 +141,11 @@ struct blkif_request_segment_aligned {
 	/* @last_sect: last sector in frame to transfer (inclusive).     */
 	uint8_t     first_sect, last_sect;
 	uint16_t    _pad; /* padding to make it 8 bytes, so it's cache-aligned */
-} __attribute__((__packed__));
+};
 
 struct blkif_request_rw {
 	uint8_t        nr_segments;  /* number of segments                   */
 	blkif_vdev_t   handle;       /* only for read/write requests         */
-#ifdef CONFIG_X86_64
-	uint32_t       _pad1;	     /* offsetof(blkif_request,u.rw.id) == 8 */
-#endif
 	uint64_t       id;           /* private guest value, echoed in resp  */
 	blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
 	struct blkif_request_segment {
@@ -157,47 +154,36 @@ struct blkif_request_rw {
 		/* @last_sect: last sector in frame to transfer (inclusive).     */
 		uint8_t     first_sect, last_sect;
 	} seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-} __attribute__((__packed__));
+};
 
 struct blkif_request_discard {
 	uint8_t        flag;         /* BLKIF_DISCARD_SECURE or zero.        */
 #define BLKIF_DISCARD_SECURE (1<<0)  /* ignored if discard-secure=0          */
 	blkif_vdev_t   _pad1;        /* only for read/write requests         */
-#ifdef CONFIG_X86_64
-	uint32_t       _pad2;        /* offsetof(blkif_req..,u.discard.id)==8*/
-#endif
 	uint64_t       id;           /* private guest value, echoed in resp  */
 	blkif_sector_t sector_number;
 	uint64_t       nr_sectors;
 	uint8_t        _pad3;
-} __attribute__((__packed__));
+};
 
 struct blkif_request_other {
 	uint8_t      _pad1;
 	blkif_vdev_t _pad2;        /* only for read/write requests         */
-#ifdef CONFIG_X86_64
-	uint32_t     _pad3;        /* offsetof(blkif_req..,u.other.id)==8*/
-#endif
 	uint64_t     id;           /* private guest value, echoed in resp  */
-} __attribute__((__packed__));
+};
 
 struct blkif_request_indirect {
 	uint8_t        indirect_op;
 	uint16_t       nr_segments;
-#ifdef CONFIG_X86_64
-	uint32_t       _pad1;        /* offsetof(blkif_...,u.indirect.id) == 8 */
-#endif
 	uint64_t       id;
 	blkif_sector_t sector_number;
 	blkif_vdev_t   handle;
 	uint16_t       _pad2;
 	grant_ref_t    indirect_grefs[BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST];
-#ifdef CONFIG_X86_64
-	uint32_t      _pad3;         /* make it 64 byte aligned */
-#else
+#ifdef CONFIG_X86_32
 	uint64_t      _pad3;         /* make it 64 byte aligned */
 #endif
-} __attribute__((__packed__));
+};
 
 struct blkif_request {
 	uint8_t        operation;    /* BLKIF_OP_???                         */
@@ -207,7 +193,7 @@ struct blkif_request {
 		struct blkif_request_other other;
 		struct blkif_request_indirect indirect;
 	} u;
-} __attribute__((__packed__));
+};
 
 struct blkif_response {
 	uint64_t        id;              /* copied from request */
-- 
1.7.7.5 (Apple Git-26)


^ permalink raw reply related	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2013-12-11 16:30 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-03 10:57 [PATCH RFC] xen-block: correctly define structures in public headers Roger Pau Monne
2013-12-03 11:01 ` David Vrabel
2013-12-03 11:07   ` [Xen-devel] " Paul Durrant
2013-12-03 11:08   ` Ian Campbell
2013-12-03 11:59     ` David Vrabel
2013-12-03 13:41       ` Ian Campbell
2013-12-03 15:11         ` David Vrabel
2013-12-03 15:17           ` Ian Campbell
2013-12-03 15:51             ` One Thousand Gnomes
2013-12-03 16:06               ` Ian Campbell
2013-12-03 20:11             ` Konrad Rzeszutek Wilk
2013-12-04  9:28               ` Ian Campbell
2013-12-04  9:43                 ` Roger Pau Monné
2013-12-04 12:10                 ` Ian Campbell
2013-12-04 11:03               ` Stefano Stabellini
2013-12-04 11:33                 ` Stefano Stabellini
2013-12-11 16:18               ` Stefano Stabellini
2013-12-11 16:23                 ` Roger Pau Monné
2013-12-11 16:29                   ` Stefano Stabellini
2013-12-03 11:09   ` Roger Pau Monné
2013-12-03 11:05 ` [Xen-devel] " Ian Campbell
2013-12-03 11:11   ` Jan Beulich
2013-12-03 11:15     ` Ian Campbell
2013-12-03 11:14   ` Roger Pau Monné
2013-12-03 11:14 ` Jan Beulich
2013-12-03 11:18   ` Roger Pau Monné

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox