[PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Roger Pau Monne]

At the moment the only consumer of this structure is x86, but other arches
might also use it, so make all the fields 64bits. On x86 Xen will still try
to place everything below the 4GiB boundary, but that might not be feasible
in other arches.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Requested-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/include/xc_dom.h | 20 ++++++++++----------
 xen/include/public/xen.h     | 24 +++++++++++++-----------
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index 6ebe946..746cce2 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -227,16 +227,16 @@ struct xc_dom_image {
  * be required in the future if there are alignment changes.
  */
 struct hvm_start_info {
-    uint32_t magic;             /* Contains the magic value 0x336ec578       */
-                                /* ("xEn3" with the 0x80 bit of the "E" set).*/
-    uint32_t version;           /* Version of this structure.                */
-    uint32_t flags;             /* SIF_xxx flags.                            */
-    uint32_t cmdline_paddr;     /* Physical address of the command line.     */
-    uint32_t nr_modules;        /* Number of modules passed to the kernel.   */
-    uint32_t modlist_paddr;     /* Physical address of an array of           */
-                                /* hvm_modlist_entry.                        */
-    uint32_t rsdp_paddr;        /* Physical address of the RSDP ACPI data    */
-                                /* structure.                                */
+    uint64_t magic;         /* Contains the magic value 0x746f6f62336ec578   */
+                            /* ("xEn3boot" with the 0x80 bit of the "E" set).*/
+    uint64_t version;       /* Version of this structure.                    */
+    uint64_t flags;         /* SIF_xxx flags.                                */
+    uint64_t cmdline_paddr; /* Physical address of the command line.         */
+    uint64_t nr_modules;    /* Number of modules passed to the kernel.       */
+    uint64_t modlist_paddr; /* Physical address of an array of               */
+                            /* hvm_modlist_entry.                            */
+    uint64_t rsdp_paddr;    /* Physical address of the RSDP ACPI data        */
+                            /* structure.                                    */
 } __attribute__((packed));
 
 struct hvm_modlist_entry {
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 435d789..59f3c2f 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -814,23 +814,23 @@ typedef struct start_info start_info_t;
  *
  *  0 +----------------+
  *    | magic          | Contains the magic value XEN_HVM_START_MAGIC_VALUE
- *    |                | ("xEn3" with the 0x80 bit of the "E" set).
- *  4 +----------------+
+ *    |                | ("xEn3boot" with the 0x80 bit of the "E" set).
+ *  8 +----------------+
  *    | version        | Version of this structure. Current version is 0. New
  *    |                | versions are guaranteed to be backwards-compatible.
- *  8 +----------------+
+ * 16 +----------------+
  *    | flags          | SIF_xxx flags.
- * 12 +----------------+
+ * 24 +----------------+
  *    | cmdline_paddr  | Physical address of the command line,
  *    |                | a zero-terminated ASCII string.
- * 16 +----------------+
+ * 32 +----------------+
  *    | nr_modules     | Number of modules passed to the kernel.
- * 20 +----------------+
+ * 40 +----------------+
  *    | modlist_paddr  | Physical address of an array of modules
  *    |                | (layout of the structure below).
- * 24 +----------------+
+ * 48 +----------------+
  *    | rsdp_paddr     | Physical address of the RSDP ACPI data structure.
- * 28 +----------------+
+ * 56 +----------------+
  *
  * The layout of each entry in the module structure is the following:
  *
@@ -845,10 +845,12 @@ typedef struct start_info start_info_t;
  *    | reserved       |
  * 32 +----------------+
  *
- * The address and size of the modules is a 64bit unsigned integer. However
- * Xen will always try to place all modules below the 4GiB boundary.
+ * The address and sizes are always a 64bit little endian unsigned integer.
+ *
+ * NB: Xen on x86 will always try to place all the data below the 4GiB
+ * boundary.
  */
-#define XEN_HVM_START_MAGIC_VALUE 0x336ec578
+#define XEN_HVM_START_MAGIC_VALUE 0x746f6f62336ec578
 
 /* New console union for dom0 introduced in 0x00030203. */
 #if __XEN_INTERFACE_VERSION__ < 0x00030203
-- 
2.6.4 (Apple Git-63)


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

Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Wei Liu]

On Fri, Apr 08, 2016 at 05:47:36PM +0200, Roger Pau Monne wrote:
> At the moment the only consumer of this structure is x86, but other arches
> might also use it, so make all the fields 64bits. On x86 Xen will still try
> to place everything below the 4GiB boundary, but that might not be feasible
> in other arches.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Requested-by: Jan Beulich <jbeulich@suse.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxc/include/xc_dom.h | 20 ++++++++++----------

Subject to an ack from HV maintainers:

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Andrew Cooper]

On 08/04/16 16:47, Roger Pau Monne wrote:
> At the moment the only consumer of this structure is x86, but other arches
> might also use it, so make all the fields 64bits. On x86 Xen will still try
> to place everything below the 4GiB boundary, but that might not be feasible
> in other arches.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Requested-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

I hope never to need to use this for x86, but leaving some wiggleroom is
probably best.  If the very worst happens, the guest can just enable PAE
paging to get access to the data.

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

Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits"):
> On 08/04/16 16:47, Roger Pau Monne wrote:
> > At the moment the only consumer of this structure is x86, but other arches
> > might also use it, so make all the fields 64bits. On x86 Xen will still try
> > to place everything below the 4GiB boundary, but that might not be feasible
> > in other arches.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Requested-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> I hope never to need to use this for x86, but leaving some wiggleroom is
> probably best.  If the very worst happens, the guest can just enable PAE
> paging to get access to the data.

Thanks.

Queued.

Ian.

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

Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Jan Beulich]

>>> On 08.04.16 at 17:47, <roger.pau@citrix.com> wrote:
> At the moment the only consumer of this structure is x86, but other arches
> might also use it, so make all the fields 64bits. On x86 Xen will still try
> to place everything below the 4GiB boundary, but that might not be feasible
> in other arches.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Requested-by: Jan Beulich <jbeulich@suse.com>

I don't think I've asked for _all_ fields to be made 64-bit. In
particular...

> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -227,16 +227,16 @@ struct xc_dom_image {
>   * be required in the future if there are alignment changes.
>   */
>  struct hvm_start_info {
> -    uint32_t magic;             /* Contains the magic value 0x336ec578       */
> -                                /* ("xEn3" with the 0x80 bit of the "E" set).*/
> -    uint32_t version;           /* Version of this structure.                */
> -    uint32_t flags;             /* SIF_xxx flags.                            */
> -    uint32_t cmdline_paddr;     /* Physical address of the command line.     */
> -    uint32_t nr_modules;        /* Number of modules passed to the kernel.   */
> -    uint32_t modlist_paddr;     /* Physical address of an array of           */
> -                                /* hvm_modlist_entry.                        */
> -    uint32_t rsdp_paddr;        /* Physical address of the RSDP ACPI data    */
> -                                /* structure.                                */
> +    uint64_t magic;         /* Contains the magic value 0x746f6f62336ec578   */
> +                            /* ("xEn3boot" with the 0x80 bit of the "E" set).*/
> +    uint64_t version;       /* Version of this structure.                    */
> +    uint64_t flags;         /* SIF_xxx flags.                                */

... none of these 3 need to be 64-bit, nor ...

> +    uint64_t cmdline_paddr; /* Physical address of the command line.         */
> +    uint64_t nr_modules;    /* Number of modules passed to the kernel.       */

...  this one.

Nacked-by: Jan Beulich <jbeulich@suse.com>

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

Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Jan Beulich]

>>> On 08.04.16 at 18:01, <Ian.Jackson@eu.citrix.com> wrote:
> Andrew Cooper writes ("Re: [PATCH] xen: change the sizes of fields in the HVM 
> start info layout to be 64bits"):
>> On 08/04/16 16:47, Roger Pau Monne wrote:
>> > At the moment the only consumer of this structure is x86, but other arches
>> > might also use it, so make all the fields 64bits. On x86 Xen will still try
>> > to place everything below the 4GiB boundary, but that might not be feasible
>> > in other arches.
>> >
>> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> > Requested-by: Jan Beulich <jbeulich@suse.com>
>> 
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> 
>> I hope never to need to use this for x86, but leaving some wiggleroom is
>> probably best.  If the very worst happens, the guest can just enable PAE
>> paging to get access to the data.
> 
> Thanks.
> 
> Queued.

And already applied as I see. I think rushing things in like this is
not a solution, no matter that we want to freeze the tree today.
Changes like this should be free to go in as de-facto bug fixes
after the freeze date.

Jan

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

Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Wei Liu]

On Fri, Apr 08, 2016 at 10:27:42AM -0600, Jan Beulich wrote:
> >>> On 08.04.16 at 17:47, <roger.pau@citrix.com> wrote:
> > At the moment the only consumer of this structure is x86, but other arches
> > might also use it, so make all the fields 64bits. On x86 Xen will still try
> > to place everything below the 4GiB boundary, but that might not be feasible
> > in other arches.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Requested-by: Jan Beulich <jbeulich@suse.com>
> 
> I don't think I've asked for _all_ fields to be made 64-bit. In
> particular...
> 
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -227,16 +227,16 @@ struct xc_dom_image {
> >   * be required in the future if there are alignment changes.
> >   */
> >  struct hvm_start_info {
> > -    uint32_t magic;             /* Contains the magic value 0x336ec578       */
> > -                                /* ("xEn3" with the 0x80 bit of the "E" set).*/
> > -    uint32_t version;           /* Version of this structure.                */
> > -    uint32_t flags;             /* SIF_xxx flags.                            */
> > -    uint32_t cmdline_paddr;     /* Physical address of the command line.     */
> > -    uint32_t nr_modules;        /* Number of modules passed to the kernel.   */
> > -    uint32_t modlist_paddr;     /* Physical address of an array of           */
> > -                                /* hvm_modlist_entry.                        */
> > -    uint32_t rsdp_paddr;        /* Physical address of the RSDP ACPI data    */
> > -                                /* structure.                                */
> > +    uint64_t magic;         /* Contains the magic value 0x746f6f62336ec578   */
> > +                            /* ("xEn3boot" with the 0x80 bit of the "E" set).*/
> > +    uint64_t version;       /* Version of this structure.                    */
> > +    uint64_t flags;         /* SIF_xxx flags.                                */
> 
> ... none of these 3 need to be 64-bit, nor ...
> 
> > +    uint64_t cmdline_paddr; /* Physical address of the command line.         */
> > +    uint64_t nr_modules;    /* Number of modules passed to the kernel.       */
> 
> ...  this one.
> 
> Nacked-by: Jan Beulich <jbeulich@suse.com>

Ian committed this already. You might want to revert it then.

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

Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Roger Pau Monné]

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

On Fri, 8 Apr 2016, Jan Beulich wrote:
> >>> On 08.04.16 at 17:47, <roger.pau@citrix.com> wrote:
> > At the moment the only consumer of this structure is x86, but other arches
> > might also use it, so make all the fields 64bits. On x86 Xen will still try
> > to place everything below the 4GiB boundary, but that might not be feasible
> > in other arches.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Requested-by: Jan Beulich <jbeulich@suse.com>
> 
> I don't think I've asked for _all_ fields to be made 64-bit. In
> particular...
> 
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -227,16 +227,16 @@ struct xc_dom_image {
> >   * be required in the future if there are alignment changes.
> >   */
> >  struct hvm_start_info {
> > -    uint32_t magic;             /* Contains the magic value 0x336ec578       */
> > -                                /* ("xEn3" with the 0x80 bit of the "E" set).*/
> > -    uint32_t version;           /* Version of this structure.                */
> > -    uint32_t flags;             /* SIF_xxx flags.                            */
> > -    uint32_t cmdline_paddr;     /* Physical address of the command line.     */
> > -    uint32_t nr_modules;        /* Number of modules passed to the kernel.   */
> > -    uint32_t modlist_paddr;     /* Physical address of an array of           */
> > -                                /* hvm_modlist_entry.                        */
> > -    uint32_t rsdp_paddr;        /* Physical address of the RSDP ACPI data    */
> > -                                /* structure.                                */
> > +    uint64_t magic;         /* Contains the magic value 0x746f6f62336ec578   */
> > +                            /* ("xEn3boot" with the 0x80 bit of the "E" set).*/
> > +    uint64_t version;       /* Version of this structure.                    */
> > +    uint64_t flags;         /* SIF_xxx flags.                                */
> 
> ... none of these 3 need to be 64-bit, nor ...
> 
> > +    uint64_t cmdline_paddr; /* Physical address of the command line.         */
> > +    uint64_t nr_modules;    /* Number of modules passed to the kernel.       */
> 
> ...  this one.
> 
> Nacked-by: Jan Beulich <jbeulich@suse.com>

I know, but I don't see any harm in them being 64bits, and then the layout 
is clearer IMHO.

Roger. 

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH] xen: change the sizes of fields in the HVM start info layout to be 64bits

[Wei Liu]

On Fri, Apr 08, 2016 at 10:30:25AM -0600, Jan Beulich wrote:
> >>> On 08.04.16 at 18:01, <Ian.Jackson@eu.citrix.com> wrote:
> > Andrew Cooper writes ("Re: [PATCH] xen: change the sizes of fields in the HVM 
> > start info layout to be 64bits"):
> >> On 08/04/16 16:47, Roger Pau Monne wrote:
> >> > At the moment the only consumer of this structure is x86, but other arches
> >> > might also use it, so make all the fields 64bits. On x86 Xen will still try
> >> > to place everything below the 4GiB boundary, but that might not be feasible
> >> > in other arches.
> >> >
> >> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> > Requested-by: Jan Beulich <jbeulich@suse.com>
> >> 
> >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> 
> >> I hope never to need to use this for x86, but leaving some wiggleroom is
> >> probably best.  If the very worst happens, the guest can just enable PAE
> >> paging to get access to the data.
> > 
> > Thanks.
> > 
> > Queued.
> 
> And already applied as I see. I think rushing things in like this is
> not a solution, no matter that we want to freeze the tree today.
> Changes like this should be free to go in as de-facto bug fixes
> after the freeze date.
> 

Yes, I'm fine with this changing after the freeze. I certainly don't
want to release a wrong interface.

Wei.

> Jan
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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