[PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 1565 bytes --]


Hi,

Attached patch makes the PV block driver for 32bit PV guests working on
64bit PV Dom0.

In the io/blkif.h,  the 'id' field of
struct blkif_request is 64bit, but on a LP32 system it's aligned on 
32bits, while it'll be aligned to 64bits on a LP64 system. So on LP64 there'll 
be 4 bytes of padding between handle and id. The same is true for
struct blkif_response: it's 11 bytes long, so will be aligned differently
in the ring between a 32 and 64bit systems.

In order to fix this, attached patch introduces in the public headers,
blkif_request/blkif_response, blkif_x86_32_request/blkif_x86_32_response
and blkif_x86_64_request/blkif_x86_64_response.
NetBSD is doing so io/blkif.h in the public headers, while Linux
does so in include/xen/blkif.h in the linux sources. But the result is
the same ...

Successfully tested with the combinations:

64bit Dom0                32bit PAE PV guest
NetBSD                       Linux
Linux                            NetBSD

The combinations 64bit Dom0, 64bit PV DomU works with and without this patch.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

[-- Attachment #2: public_blkif.diff --]
[-- Type: text/x-diff, Size: 3119 bytes --]

diff -r 37fae02cc335 xen/include/public/io/blkif.h
--- a/xen/include/public/io/blkif.h	Fri Jul 25 15:03:03 2008 +0100
+++ b/xen/include/public/io/blkif.h	Mon Jul 28 11:02:12 2008 +0200
@@ -91,6 +91,8 @@ struct blkif_request_segment {
     uint8_t     first_sect, last_sect;
 };
 
+/* native-type requests/responses (always used in frontends ) */
+
 struct blkif_request {
     uint8_t        operation;    /* BLKIF_OP_???                         */
     uint8_t        nr_segments;  /* number of segments                   */
@@ -108,6 +110,44 @@ struct blkif_response {
 };
 typedef struct blkif_response blkif_response_t;
 
+/* i386 requests/responses */
+struct blkif_x86_32_request {
+    uint8_t        operation;    /* BLKIF_OP_???                         */
+    uint8_t        nr_segments;  /* number of segments                   */
+    blkif_vdev_t   handle;       /* only for read/write requests         */
+    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 seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+} /* __packed */;
+typedef struct blkif_x86_32_request blkif_x86_32_request_t;
+
+struct blkif_x86_32_response {
+    uint64_t        id;              /* copied from request */
+    uint8_t         operation;       /* copied from request */
+    uint8_t         _pad; 
+    int16_t         status;          /* BLKIF_RSP_???       */
+} /* __packed */;
+typedef struct blkif_x86_32_response blkif_x86_32_response_t;
+
+/* amd64-type requests/responses (always used in frontends ) */
+
+struct blkif_x86_64_request {
+    uint8_t          operation;    /* BLKIF_OP_???                         */
+    uint8_t          nr_segments;  /* number of segments                   */
+    blkif_vdev_t     handle;       /* only for read/write requests         */
+    uint64_aligned_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 seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+};
+typedef struct blkif_x86_64_request blkif_x86_64_request_t;
+
+struct blkif_x86_64_response {
+    uint64_aligned_t id; /* copied from request */
+    uint8_t          operation;       /* copied from request */
+    int16_t          status;          /* BLKIF_RSP_???       */
+};
+typedef struct blkif_x86_64_response blkif_x86_64_response_t;
+
 /*
  * STATUS RETURN CODES.
  */
@@ -123,6 +163,15 @@ typedef struct blkif_response blkif_resp
  */
 
 DEFINE_RING_TYPES(blkif, struct blkif_request, struct blkif_response);
+DEFINE_RING_TYPES(blkif_x86_32, struct blkif_x86_32_request, struct blkif_x86_32_response);
+DEFINE_RING_TYPES(blkif_x86_64, struct blkif_x86_64_request, struct blkif_x86_64_response);
+
+union blkif_back_ring_proto {
+	blkif_back_ring_t ring_n; /* native/common members */
+	blkif_x86_32_back_ring_t ring_32;
+	blkif_x86_64_back_ring_t ring_64;
+};
+typedef union blkif_back_ring_proto blkif_back_ring_proto_t;
 
 #define VDISK_CDROM        0x1
 #define VDISK_REMOVABLE    0x2

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[Keir Fraser]

Just do the same as we do in Linux and put these defs in your NetBSD private
header space.

 -- Keir

On 28/7/08 12:46, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> 
> Hi,
> 
> Attached patch makes the PV block driver for 32bit PV guests working on
> 64bit PV Dom0.
> 
> In the io/blkif.h,  the 'id' field of
> struct blkif_request is 64bit, but on a LP32 system it's aligned on
> 32bits, while it'll be aligned to 64bits on a LP64 system. So on LP64 there'll
> be 4 bytes of padding between handle and id. The same is true for
> struct blkif_response: it's 11 bytes long, so will be aligned differently
> in the ring between a 32 and 64bit systems.
> 
> In order to fix this, attached patch introduces in the public headers,
> blkif_request/blkif_response, blkif_x86_32_request/blkif_x86_32_response
> and blkif_x86_64_request/blkif_x86_64_response.
> NetBSD is doing so io/blkif.h in the public headers, while Linux
> does so in include/xen/blkif.h in the linux sources. But the result is
> the same ...
> 
> Successfully tested with the combinations:
> 
> 64bit Dom0                32bit PAE PV guest
> NetBSD                       Linux
> Linux                            NetBSD
> 
> The combinations 64bit Dom0, 64bit PV DomU works with and without this patch.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 

Re: [PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[Christoph Egger]

On Monday 28 July 2008 14:23:00 Keir Fraser wrote:
> Just do the same as we do in Linux and put these defs in your NetBSD
> private header space.

That sounds like "All OS's have to go the Linux way" (which also includes
Xen's Minios (stubdom))  instead of "fix it once and for all".

Christoph

>
>  -- Keir
>
> On 28/7/08 12:46, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > Hi,
> >
> > Attached patch makes the PV block driver for 32bit PV guests working on
> > 64bit PV Dom0.
> >
> > In the io/blkif.h,  the 'id' field of
> > struct blkif_request is 64bit, but on a LP32 system it's aligned on
> > 32bits, while it'll be aligned to 64bits on a LP64 system. So on LP64
> > there'll be 4 bytes of padding between handle and id. The same is true
> > for struct blkif_response: it's 11 bytes long, so will be aligned
> > differently in the ring between a 32 and 64bit systems.
> >
> > In order to fix this, attached patch introduces in the public headers,
> > blkif_request/blkif_response, blkif_x86_32_request/blkif_x86_32_response
> > and blkif_x86_64_request/blkif_x86_64_response.
> > NetBSD is doing so io/blkif.h in the public headers, while Linux
> > does so in include/xen/blkif.h in the linux sources. But the result is
> > the same ...
> >
> > Successfully tested with the combinations:
> >
> > 64bit Dom0                32bit PAE PV guest
> > NetBSD                       Linux
> > Linux                            NetBSD
> >
> > The combinations 64bit Dom0, 64bit PV DomU works with and without this
> > patch.
> >
> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>



-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[Keir Fraser]

On 28/7/08 13:31, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> On Monday 28 July 2008 14:23:00 Keir Fraser wrote:
>> Just do the same as we do in Linux and put these defs in your NetBSD
>> private header space.
> 
> That sounds like "All OS's have to go the Linux way" (which also includes
> Xen's Minios (stubdom))  instead of "fix it once and for all".

Quite the opposite. By doing it in your own header files you can specify the
structure names and so on exactly how you would like them to be for the
particular OS concerned.

 -- Keir

Re: [PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[Christoph Egger]

On Monday 28 July 2008 14:36:49 Keir Fraser wrote:
> On 28/7/08 13:31, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > On Monday 28 July 2008 14:23:00 Keir Fraser wrote:
> >> Just do the same as we do in Linux and put these defs in your NetBSD
> >> private header space.
> >
> > That sounds like "All OS's have to go the Linux way" (which also includes
> > Xen's Minios (stubdom))  instead of "fix it once and for all".
>
> Quite the opposite. By doing it in your own header files you can specify
> the structure names and so on exactly how you would like them to be for the
> particular OS concerned.

I'm concerned about having/keeping the API/ABI the same between all OS's and
not about their implementation.

Christoph


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[Keir Fraser]

On 28/7/08 13:52, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> Quite the opposite. By doing it in your own header files you can specify
>> the structure names and so on exactly how you would like them to be for the
>> particular OS concerned.
> 
> I'm concerned about having/keeping the API/ABI the same between all OS's and
> not about their implementation.

The interfaces (both the API exposed by public/ headers; and the ABI they
describe) won't change. At least not in a way that isn't backward
compatible. Or we'd be breaking old OSes all the time!

 -- Keir

Re: [PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[Keir Fraser]

On 28/7/08 14:00, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:

>>> Quite the opposite. By doing it in your own header files you can specify
>>> the structure names and so on exactly how you would like them to be for the
>>> particular OS concerned.
>> 
>> I'm concerned about having/keeping the API/ABI the same between all OS's and
>> not about their implementation.
> 
> The interfaces (both the API exposed by public/ headers; and the ABI they
> describe) won't change. At least not in a way that isn't backward
> compatible. Or we'd be breaking old OSes all the time!

I might add: why would placing those extra struct definitions in the public
headers make it any more or less likely that we'll break compatibility in
future? That makes no sense to me.

 -- Keir

Re: [PATCH][XEN] PV blk backend: Make 32bit PV guests work on 64bit PV Dom0

[Christoph Egger]

On Monday 28 July 2008 15:00:35 Keir Fraser wrote:
> On 28/7/08 13:52, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> >> Quite the opposite. By doing it in your own header files you can specify
> >> the structure names and so on exactly how you would like them to be for
> >> the particular OS concerned.
> >
> > I'm concerned about having/keeping the API/ABI the same between all OS's
> > and not about their implementation.
>
> The interfaces (both the API exposed by public/ headers; and the ABI they
> describe) won't change. At least not in a way that isn't backward
> compatible. Or we'd be breaking old OSes all the time!

Haven't those got broken with the removal of non-PAE support? :)

Christoph

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy