xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] Requisite paches for xSplice v3 (not yet posted).
@ 2016-02-12  3:08 Konrad Rzeszutek Wilk
  2016-02-12  3:08 ` [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs; Konrad Rzeszutek Wilk
                   ` (4 more replies)
  0 siblings, 5 replies; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12  3:08 UTC (permalink / raw)
  To: ian.campbell, ian.jackson, jbeulich, xen-devel

This mismatch of patches came about as I was redoing the
xSplice patches based on reviews and splitting and/or doing some
extra things.

The first patch:

 [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct

Allows one to include the keyhandler.h (ARM one) file without having 
extra header file dependencies.

 [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.

This allows us to use the Elf_ macros - instead of having to do
Elf32_ or Elf64_.

 [PATCH v3 3/5] build: remove .config from /boot when uninstalling.

A little fix.
 [PATCH v3 4/5] mkelf32: Remove the 32-bit hypervisor support.
 [PATCH v3 5/5] mkelf32: Close those file descriptors in the error

And those were found when I was making the xen.gz able to
contain the ELF_NOTE.

Please review at your convience.

 xen/Makefile                 |   1 +
 xen/arch/x86/boot/mkelf32.c  | 110 ++++++++++++++++---------------------------
 xen/include/asm-arm/config.h |   2 +
 xen/include/xen/keyhandler.h |   1 +
 4 files changed, 45 insertions(+), 69 deletions(-)

Konrad Rzeszutek Wilk (5):
      hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
      arm/config: Declare ELFSIZE_[32|64] respectively.
      build: remove .config from /boot when uninstalling.
      mkelf32: Remove the 32-bit hypervisor support.
      mkelf32: Close those file descriptors in the error paths.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
  2016-02-12  3:08 [PATCH v3] Requisite paches for xSplice v3 (not yet posted) Konrad Rzeszutek Wilk
@ 2016-02-12  3:08 ` Konrad Rzeszutek Wilk
  2016-02-12  8:51   ` Jan Beulich
  2016-02-12 11:37   ` Stefano Stabellini
  2016-02-12  3:08 ` [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively Konrad Rzeszutek Wilk
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12  3:08 UTC (permalink / raw)
  To: ian.campbell, ian.jackson, jbeulich, xen-devel
  Cc: stefano.stabellini, wei.liu2, Konrad Rzeszutek Wilk

in the keyhandler.h file. Otherwise on ARM builds if we
just use the keyhandler file - the compile will fail.

CC: ian.campbell@citrix.com
CC: wei.liu2@citrix.com
CC: stefano.stabellini@citrix.com
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/include/xen/keyhandler.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/include/xen/keyhandler.h b/xen/include/xen/keyhandler.h
index 06c05c8..e361558 100644
--- a/xen/include/xen/keyhandler.h
+++ b/xen/include/xen/keyhandler.h
@@ -19,6 +19,7 @@
  */
 typedef void (keyhandler_fn_t)(unsigned char key);
 
+struct cpu_user_regs;
 /*
  * Callback type for irq_keyhandler.
  *
-- 
2.4.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12  3:08 [PATCH v3] Requisite paches for xSplice v3 (not yet posted) Konrad Rzeszutek Wilk
  2016-02-12  3:08 ` [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs; Konrad Rzeszutek Wilk
@ 2016-02-12  3:08 ` Konrad Rzeszutek Wilk
  2016-02-12 11:26   ` Stefano Stabellini
  2016-02-12  3:08 ` [PATCH v3 3/5] build: remove .config from /boot when uninstalling Konrad Rzeszutek Wilk
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12  3:08 UTC (permalink / raw)
  To: ian.campbell, ian.jackson, jbeulich, xen-devel
  Cc: stefano.stabellini, wei.liu2, Konrad Rzeszutek Wilk

Otherwise any code that tries to use Elf_* macros instead of
Elf32_ or Elf_64 fails to compile.

CC: ian.campbell@citrix.com
CC: wei.liu2@citrix.com
CC: stefano.stabellini@citrix.com
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/include/asm-arm/config.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index bd832df..4ea66bf 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -15,8 +15,10 @@
 
 #if defined(CONFIG_ARM_64)
 # define LONG_BYTEORDER 3
+# define ELFSIZE 64
 #else
 # define LONG_BYTEORDER 2
+# define ELFSIZE 32
 #endif
 
 #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
-- 
2.4.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/5] build: remove .config from /boot when uninstalling.
  2016-02-12  3:08 [PATCH v3] Requisite paches for xSplice v3 (not yet posted) Konrad Rzeszutek Wilk
  2016-02-12  3:08 ` [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs; Konrad Rzeszutek Wilk
  2016-02-12  3:08 ` [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively Konrad Rzeszutek Wilk
@ 2016-02-12  3:08 ` Konrad Rzeszutek Wilk
  2016-02-12 14:11   ` Doug Goldstein
  2016-02-12  3:08 ` [PATCH v3 4/5] mkelf32: Remove the 32-bit hypervisor support Konrad Rzeszutek Wilk
  2016-02-12  3:08 ` [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths Konrad Rzeszutek Wilk
  4 siblings, 1 reply; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12  3:08 UTC (permalink / raw)
  To: ian.campbell, ian.jackson, jbeulich, xen-devel
  Cc: cardoe, Konrad Rzeszutek Wilk

c/s 361b4f9f0f0d4adc19df428e224a7b8fa62cd392
"build: save generated xen .config" forgot to remove
the config file when uninstalling.

CC: ian.campbell@citrix.com
CC: jbeulich@suse.com
CC: cardoe@cardoe.com
CC: ian.jackson@eu.citrix.com
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/Makefile b/xen/Makefile
index 8b530c2..5d98bcb 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -81,6 +81,7 @@ _uninstall: D=$(DESTDIR)
 _uninstall: T=$(notdir $(TARGET))
 _uninstall: Z=$(CONFIG_XEN_INSTALL_SUFFIX)
 _uninstall:
+	rm -f $(D)$(BOOT_DIR)/$(T)-$(XEN_FULLVERSION).config
 	rm -f $(D)$(BOOT_DIR)/$(T)-$(XEN_FULLVERSION)$(Z)
 	rm -f $(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION)$(Z)
 	rm -f $(D)$(BOOT_DIR)/$(T)-$(XEN_VERSION)$(Z)
-- 
2.4.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 4/5] mkelf32: Remove the 32-bit hypervisor support.
  2016-02-12  3:08 [PATCH v3] Requisite paches for xSplice v3 (not yet posted) Konrad Rzeszutek Wilk
                   ` (2 preceding siblings ...)
  2016-02-12  3:08 ` [PATCH v3 3/5] build: remove .config from /boot when uninstalling Konrad Rzeszutek Wilk
@ 2016-02-12  3:08 ` Konrad Rzeszutek Wilk
  2016-02-12  3:08 ` [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths Konrad Rzeszutek Wilk
  4 siblings, 0 replies; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12  3:08 UTC (permalink / raw)
  To: ian.campbell, ian.jackson, jbeulich, xen-devel; +Cc: Konrad Rzeszutek Wilk

We do not compile 32-bit hypervisor anymore so the code for
the ELFCLASS32 is effectively dead.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/arch/x86/boot/mkelf32.c | 88 +++++++++++++++------------------------------
 1 file changed, 28 insertions(+), 60 deletions(-)

diff --git a/xen/arch/x86/boot/mkelf32.c b/xen/arch/x86/boot/mkelf32.c
index b369222..993a7ee 100644
--- a/xen/arch/x86/boot/mkelf32.c
+++ b/xen/arch/x86/boot/mkelf32.c
@@ -3,7 +3,7 @@
  * 
  * Usage: elf-prefix <in-image> <out-image> <load-base>
  * 
- * Converts an Elf32 or Elf64 executable binary <in-image> into a simple Elf32
+ * Converts an Elf64 executable binary <in-image> into a simple Elf32
  * image <out-image> comprising a single chunk to be loaded at <load-base>. 
  */
 
@@ -235,7 +235,6 @@ int main(int argc, char **argv)
     int        bytes, todo, i;
 
     Elf32_Ehdr in32_ehdr;
-    Elf32_Phdr in32_phdr;
 
     Elf64_Ehdr in64_ehdr;
     Elf64_Phdr in64_phdr;
@@ -271,70 +270,39 @@ int main(int argc, char **argv)
     big_endian = (*(u16 *)in32_ehdr.e_ident == ((ELFMAG0 << 8) | ELFMAG1));
 
     endianadjust_ehdr32(&in32_ehdr);
-    switch ( in32_ehdr.e_ident[EI_CLASS] )
+    if ( in32_ehdr.e_ident[EI_CLASS] != ELFCLASS64 )
     {
-    case ELFCLASS32:
-        if ( in32_ehdr.e_phentsize != sizeof(in32_phdr) )
-        {
-            fprintf(stderr, "Bad program header size (%d != %d).\n",
-                    (int)in32_ehdr.e_phentsize, (int)sizeof(in32_phdr));
-            return 1;
-        }
-
-        if ( in32_ehdr.e_phnum != 1 )
-        {
-            fprintf(stderr, "Expect precisely 1 program header; found %d.\n",
-                    (int)in32_ehdr.e_phnum);
-            return 1;
-        }
-
-        (void)lseek(infd, in32_ehdr.e_phoff, SEEK_SET);
-        do_read(infd, &in32_phdr, sizeof(in32_phdr));
-        endianadjust_phdr32(&in32_phdr);
-
-        (void)lseek(infd, in32_phdr.p_offset, SEEK_SET);
-        dat_siz = (u32)in32_phdr.p_filesz;
-
-        /* Do not use p_memsz: it does not include BSS alignment padding. */
-        /*mem_siz = (u32)in32_phdr.p_memsz;*/
-        mem_siz = (u32)(final_exec_addr - in32_phdr.p_vaddr);
-        break;
-
-    case ELFCLASS64:
-        (void)lseek(infd, 0, SEEK_SET);
-        do_read(infd, &in64_ehdr, sizeof(in64_ehdr));
-        endianadjust_ehdr64(&in64_ehdr);
-
-        if ( in64_ehdr.e_phentsize != sizeof(in64_phdr) )
-        {
-            fprintf(stderr, "Bad program header size (%d != %d).\n",
-                    (int)in64_ehdr.e_phentsize, (int)sizeof(in64_phdr));
-            return 1;
-        }
+        fprintf(stderr, "Bad program header class - we only do 64-bit!.\n");
+        return 1;
+    }
+    (void)lseek(infd, 0, SEEK_SET);
+    do_read(infd, &in64_ehdr, sizeof(in64_ehdr));
+    endianadjust_ehdr64(&in64_ehdr);
 
-        if ( in64_ehdr.e_phnum != 1 )
-        {
-            fprintf(stderr, "Expect precisly 1 program header; found %d.\n",
-                    (int)in64_ehdr.e_phnum);
-            return 1;
-        }
+    if ( in64_ehdr.e_phentsize != sizeof(in64_phdr) )
+    {
+        fprintf(stderr, "Bad program header size (%d != %d).\n",
+                (int)in64_ehdr.e_phentsize, (int)sizeof(in64_phdr));
+        return 1;
+    }
 
-        (void)lseek(infd, in64_ehdr.e_phoff, SEEK_SET);
-        do_read(infd, &in64_phdr, sizeof(in64_phdr));
-        endianadjust_phdr64(&in64_phdr);
+    if ( in64_ehdr.e_phnum != 1 )
+    {
+        fprintf(stderr, "Expect precisly 1 program header; found %d.\n",
+                (int)in64_ehdr.e_phnum);
+        return 1;
+    }
 
-        (void)lseek(infd, in64_phdr.p_offset, SEEK_SET);
-        dat_siz = (u32)in64_phdr.p_filesz;
+    (void)lseek(infd, in64_ehdr.e_phoff, SEEK_SET);
+    do_read(infd, &in64_phdr, sizeof(in64_phdr));
+    endianadjust_phdr64(&in64_phdr);
 
-        /* Do not use p_memsz: it does not include BSS alignment padding. */
-        /*mem_siz = (u32)in64_phdr.p_memsz;*/
-        mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
-        break;
+    (void)lseek(infd, in64_phdr.p_offset, SEEK_SET);
+    dat_siz = (u32)in64_phdr.p_filesz;
 
-    default:
-        fprintf(stderr, "Input image must be a 32- or 64-bit Elf image.\n");
-        return 1;
-    }
+    /* Do not use p_memsz: it does not include BSS alignment padding. */
+    /*mem_siz = (u32)in64_phdr.p_memsz;*/
+    mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
 
     /*
      * End the image on a page boundary. This gets round alignment bugs
-- 
2.4.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths.
  2016-02-12  3:08 [PATCH v3] Requisite paches for xSplice v3 (not yet posted) Konrad Rzeszutek Wilk
                   ` (3 preceding siblings ...)
  2016-02-12  3:08 ` [PATCH v3 4/5] mkelf32: Remove the 32-bit hypervisor support Konrad Rzeszutek Wilk
@ 2016-02-12  3:08 ` Konrad Rzeszutek Wilk
  2016-02-18 16:29   ` Jan Beulich
  4 siblings, 1 reply; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12  3:08 UTC (permalink / raw)
  To: ian.campbell, ian.jackson, jbeulich, xen-devel; +Cc: Konrad Rzeszutek Wilk

While we are operating here we may as well fix some of the
file descriptor leaks.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/arch/x86/boot/mkelf32.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/boot/mkelf32.c b/xen/arch/x86/boot/mkelf32.c
index 993a7ee..890ae6d 100644
--- a/xen/arch/x86/boot/mkelf32.c
+++ b/xen/arch/x86/boot/mkelf32.c
@@ -230,9 +230,9 @@ int main(int argc, char **argv)
     u64        final_exec_addr;
     u32        loadbase, dat_siz, mem_siz;
     char      *inimage, *outimage;
-    int        infd, outfd;
+    int        infd, outfd = -1;
     char       buffer[1024];
-    int        bytes, todo, i;
+    int        bytes, todo, i, rc = 1;
 
     Elf32_Ehdr in32_ehdr;
 
@@ -256,7 +256,7 @@ int main(int argc, char **argv)
     {
         fprintf(stderr, "Failed to open input image '%s': %d (%s).\n",
                 inimage, errno, strerror(errno));
-        return 1;
+        goto out;
     }
 
     do_read(infd, &in32_ehdr, sizeof(in32_ehdr));
@@ -264,7 +264,7 @@ int main(int argc, char **argv)
          (in32_ehdr.e_ident[EI_DATA] != ELFDATA2LSB) )
     {
         fprintf(stderr, "Input image must be a little-endian Elf image.\n");
-        return 1;
+        goto out;
     }
 
     big_endian = (*(u16 *)in32_ehdr.e_ident == ((ELFMAG0 << 8) | ELFMAG1));
@@ -273,7 +273,7 @@ int main(int argc, char **argv)
     if ( in32_ehdr.e_ident[EI_CLASS] != ELFCLASS64 )
     {
         fprintf(stderr, "Bad program header class - we only do 64-bit!.\n");
-        return 1;
+        goto out;
     }
     (void)lseek(infd, 0, SEEK_SET);
     do_read(infd, &in64_ehdr, sizeof(in64_ehdr));
@@ -283,14 +283,14 @@ int main(int argc, char **argv)
     {
         fprintf(stderr, "Bad program header size (%d != %d).\n",
                 (int)in64_ehdr.e_phentsize, (int)sizeof(in64_phdr));
-        return 1;
+        goto out;
     }
 
     if ( in64_ehdr.e_phnum != 1 )
     {
         fprintf(stderr, "Expect precisly 1 program header; found %d.\n",
                 (int)in64_ehdr.e_phnum);
-        return 1;
+        goto out;
     }
 
     (void)lseek(infd, in64_ehdr.e_phoff, SEEK_SET);
@@ -327,7 +327,7 @@ int main(int argc, char **argv)
     {
         fprintf(stderr, "Failed to open output image '%s': %d (%s).\n",
                 outimage, errno, strerror(errno));
-        return 1;
+        goto out;
     }
 
     endianadjust_ehdr32(&out_ehdr);
@@ -339,7 +339,7 @@ int main(int argc, char **argv)
     if ( (bytes = RAW_OFFSET - sizeof(out_ehdr) - sizeof(out_phdr)) < 0 )
     {
         fprintf(stderr, "Header overflow.\n");
-        return 1;
+        goto out;
     }
     do_write(outfd, buffer, bytes);
 
@@ -358,10 +358,14 @@ int main(int argc, char **argv)
     do_write(outfd, out_shstrtab, sizeof(out_shstrtab));
     do_write(outfd, buffer, 4-((sizeof(out_shstrtab)+dat_siz)&3));
 
-    close(infd);
-    close(outfd);
+    rc = 0;
+out:
+    if ( infd != -1 )
+        close(infd);
+    if ( outfd != -1 )
+        close(outfd);
 
-    return 0;
+    return rc;
 }
 
 /*
-- 
2.4.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
  2016-02-12  3:08 ` [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs; Konrad Rzeszutek Wilk
@ 2016-02-12  8:51   ` Jan Beulich
  2016-02-12 14:06     ` Konrad Rzeszutek Wilk
  2016-02-12 11:37   ` Stefano Stabellini
  1 sibling, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2016-02-12  8:51 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: ian.jackson, wei.liu2, stefano.stabellini, ian.campbell, xen-devel

>>> On 12.02.16 at 04:08, <konrad.wilk@oracle.com> wrote:
> in the keyhandler.h file. Otherwise on ARM builds if we
> just use the keyhandler file - the compile will fail.
> 
> CC: ian.campbell@citrix.com 
> CC: wei.liu2@citrix.com 
> CC: stefano.stabellini@citrix.com 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

See commit d1d181328d.

Jan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12  3:08 ` [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively Konrad Rzeszutek Wilk
@ 2016-02-12 11:26   ` Stefano Stabellini
  2016-02-12 14:17     ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 34+ messages in thread
From: Stefano Stabellini @ 2016-02-12 11:26 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: wei.liu2, ian.campbell, ian.jackson, stefano.stabellini,
	jbeulich, xen-devel

On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote:
> Otherwise any code that tries to use Elf_* macros instead of
> Elf32_ or Elf_64 fails to compile.
> 
> CC: ian.campbell@citrix.com
> CC: wei.liu2@citrix.com
> CC: stefano.stabellini@citrix.com
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  xen/include/asm-arm/config.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index bd832df..4ea66bf 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -15,8 +15,10 @@
>  
>  #if defined(CONFIG_ARM_64)
>  # define LONG_BYTEORDER 3
> +# define ELFSIZE 64
>  #else
>  # define LONG_BYTEORDER 2
> +# define ELFSIZE 32
>  #endif

I wonder if we should use ELF64 on ARM32 too, for simplicity (x86 only
uses ELF64) and because ARM32 is LPAE.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
  2016-02-12  3:08 ` [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs; Konrad Rzeszutek Wilk
  2016-02-12  8:51   ` Jan Beulich
@ 2016-02-12 11:37   ` Stefano Stabellini
  2016-02-12 12:20     ` Jan Beulich
  1 sibling, 1 reply; 34+ messages in thread
From: Stefano Stabellini @ 2016-02-12 11:37 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: wei.liu2, ian.campbell, ian.jackson, stefano.stabellini,
	jbeulich, xen-devel

On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote:
> in the keyhandler.h file. Otherwise on ARM builds if we
> just use the keyhandler file - the compile will fail.
> 
> CC: ian.campbell@citrix.com
> CC: wei.liu2@citrix.com
> CC: stefano.stabellini@citrix.com
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  xen/include/xen/keyhandler.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/include/xen/keyhandler.h b/xen/include/xen/keyhandler.h
> index 06c05c8..e361558 100644
> --- a/xen/include/xen/keyhandler.h
> +++ b/xen/include/xen/keyhandler.h
> @@ -19,6 +19,7 @@
>   */
>  typedef void (keyhandler_fn_t)(unsigned char key);
>  
> +struct cpu_user_regs;
>  /*
>   * Callback type for irq_keyhandler.
>   *

I think that the right fix would be to #include <asm/processor.h>.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
  2016-02-12 11:37   ` Stefano Stabellini
@ 2016-02-12 12:20     ` Jan Beulich
  2016-02-12 12:46       ` Stefano Stabellini
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2016-02-12 12:20 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: wei.liu2, ian.campbell, ian.jackson, stefano.stabellini, xen-devel

>>> On 12.02.16 at 12:37, <stefano.stabellini@eu.citrix.com> wrote:
> On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote:
>> --- a/xen/include/xen/keyhandler.h
>> +++ b/xen/include/xen/keyhandler.h
>> @@ -19,6 +19,7 @@
>>   */
>>  typedef void (keyhandler_fn_t)(unsigned char key);
>>  
>> +struct cpu_user_regs;
>>  /*
>>   * Callback type for irq_keyhandler.
>>   *
> 
> I think that the right fix would be to #include <asm/processor.h>.

I disagree - for one this isn't where the structure gets defined,
and then this is the "include everything everywhere" attitude
which tends to needlessly slow down builds (avoiding the need
to include everything everywhere is actually one of the
purposes of such forward declarations, which allows going as
far as having the full definition in just a single source file).

Jan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
  2016-02-12 12:20     ` Jan Beulich
@ 2016-02-12 12:46       ` Stefano Stabellini
  2016-02-12 14:53         ` Jan Beulich
  0 siblings, 1 reply; 34+ messages in thread
From: Stefano Stabellini @ 2016-02-12 12:46 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	stefano.stabellini, xen-devel

On Fri, 12 Feb 2016, Jan Beulich wrote:
> >>> On 12.02.16 at 12:37, <stefano.stabellini@eu.citrix.com> wrote:
> > On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote:
> >> --- a/xen/include/xen/keyhandler.h
> >> +++ b/xen/include/xen/keyhandler.h
> >> @@ -19,6 +19,7 @@
> >>   */
> >>  typedef void (keyhandler_fn_t)(unsigned char key);
> >>  
> >> +struct cpu_user_regs;
> >>  /*
> >>   * Callback type for irq_keyhandler.
> >>   *
> > 
> > I think that the right fix would be to #include <asm/processor.h>.
> 
> I disagree - for one this isn't where the structure gets defined,

Ah, sorry, it is on ARM (actually to be precise, the header files are
asm-arm/arm64/processor.h and asm-arm/arm32/processor.h, included by
asm-arm/processor.h depending on the architecture). I see now that the
corresponding x86 header is actually arch-x86/xen.h.


> and then this is the "include everything everywhere" attitude
> which tends to needlessly slow down builds (avoiding the need
> to include everything everywhere is actually one of the
> purposes of such forward declarations, which allows going as
> far as having the full definition in just a single source file).

Fair enough, but keyhandler.h directly uses struct cpu_user_regs as
parameter in two functions, so I think it would be right to include an
header file with the definition. It's too bad that the names for the ARM
headers and the x86 headers don't match. Given that, I understand why
Konrad went with this solution instead, and I am OK with it.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
  2016-02-12  8:51   ` Jan Beulich
@ 2016-02-12 14:06     ` Konrad Rzeszutek Wilk
  2016-02-12 14:54       ` Jan Beulich
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12 14:06 UTC (permalink / raw)
  To: Jan Beulich
  Cc: ian.jackson, wei.liu2, stefano.stabellini, ian.campbell, xen-devel

On Fri, Feb 12, 2016 at 01:51:28AM -0700, Jan Beulich wrote:
> >>> On 12.02.16 at 04:08, <konrad.wilk@oracle.com> wrote:
> > in the keyhandler.h file. Otherwise on ARM builds if we
> > just use the keyhandler file - the compile will fail.
> > 
> > CC: ian.campbell@citrix.com 
> > CC: wei.liu2@citrix.com 
> > CC: stefano.stabellini@citrix.com 
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> See commit d1d181328d.

Weird. When I did git rebase origin/staging it didn't
notice that commit. And it just applied it on top - so I had
two 'struct cpu_regs;' in the header file.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/5] build: remove .config from /boot when uninstalling.
  2016-02-12  3:08 ` [PATCH v3 3/5] build: remove .config from /boot when uninstalling Konrad Rzeszutek Wilk
@ 2016-02-12 14:11   ` Doug Goldstein
  0 siblings, 0 replies; 34+ messages in thread
From: Doug Goldstein @ 2016-02-12 14:11 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, ian.campbell, ian.jackson, jbeulich, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 453 bytes --]

On 2/11/16 9:08 PM, Konrad Rzeszutek Wilk wrote:
> c/s 361b4f9f0f0d4adc19df428e224a7b8fa62cd392
> "build: save generated xen .config" forgot to remove
> the config file when uninstalling.
> 
> CC: ian.campbell@citrix.com
> CC: jbeulich@suse.com
> CC: cardoe@cardoe.com
> CC: ian.jackson@eu.citrix.com
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12 11:26   ` Stefano Stabellini
@ 2016-02-12 14:17     ` Konrad Rzeszutek Wilk
  2016-02-12 15:04       ` Jan Beulich
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12 14:17 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: wei.liu2, ian.campbell, ian.jackson, stefano.stabellini,
	jbeulich, xen-devel

On Fri, Feb 12, 2016 at 11:26:10AM +0000, Stefano Stabellini wrote:
> On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote:
> > Otherwise any code that tries to use Elf_* macros instead of
> > Elf32_ or Elf_64 fails to compile.
> > 
> > CC: ian.campbell@citrix.com
> > CC: wei.liu2@citrix.com
> > CC: stefano.stabellini@citrix.com
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  xen/include/asm-arm/config.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> > index bd832df..4ea66bf 100644
> > --- a/xen/include/asm-arm/config.h
> > +++ b/xen/include/asm-arm/config.h
> > @@ -15,8 +15,10 @@
> >  
> >  #if defined(CONFIG_ARM_64)
> >  # define LONG_BYTEORDER 3
> > +# define ELFSIZE 64
> >  #else
> >  # define LONG_BYTEORDER 2
> > +# define ELFSIZE 32
> >  #endif
> 
> I wonder if we should use ELF64 on ARM32 too, for simplicity (x86 only
> uses ELF64) and because ARM32 is LPAE.

Done. And this resolves also the question Jan raised in the design
document about ARM32 and the ELF payload declaring the ELF only in
64-bit syntax.

Thanks! Updated the patch to be:
 
P.S.
It compiles without trouble.
>From 7756ddc1e2aa0be487df05ce76577c6fa15a75ce Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 5 Feb 2016 10:44:45 -0500
Subject: [PATCH] arm/config: Declare ELFSIZE_64.

Otherwise any code that tries to use Elf_* macros would
require us to use Elf64_* types instead of the more
friendly Elf_ one.

This is OK to do since 32-bit ARM uses LPAE mode.

CC: ian.campbell@citrix.com
CC: wei.liu2@citrix.com
CC: stefano.stabellini@citrix.com
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/include/asm-arm/config.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index bd832df..d5321b4 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -15,8 +15,10 @@
 
 #if defined(CONFIG_ARM_64)
 # define LONG_BYTEORDER 3
+# define ELFSIZE 64
 #else
 # define LONG_BYTEORDER 2
+# define ELFSIZE 64
 #endif
 
 #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
-- 
2.4.3

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
  2016-02-12 12:46       ` Stefano Stabellini
@ 2016-02-12 14:53         ` Jan Beulich
  0 siblings, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2016-02-12 14:53 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: wei.liu2, ian.campbell, ian.jackson, stefano.stabellini, xen-devel

>>> On 12.02.16 at 13:46, <stefano.stabellini@eu.citrix.com> wrote:
> On Fri, 12 Feb 2016, Jan Beulich wrote:
>> and then this is the "include everything everywhere" attitude
>> which tends to needlessly slow down builds (avoiding the need
>> to include everything everywhere is actually one of the
>> purposes of such forward declarations, which allows going as
>> far as having the full definition in just a single source file).
> 
> Fair enough, but keyhandler.h directly uses struct cpu_user_regs as
> parameter in two functions, so I think it would be right to include an
> header file with the definition.

No, full definitions need only be visible when structure instances
are used, not when just pointers to them get declared. The sole
reason why the forward declaration is needed is because
otherwise declaration and definition of the function wouldn't
match, as without the forward declaration the scope of the
structure is that of the function (instead of the global scope).

Jan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
  2016-02-12 14:06     ` Konrad Rzeszutek Wilk
@ 2016-02-12 14:54       ` Jan Beulich
  0 siblings, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2016-02-12 14:54 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: ian.jackson, wei.liu2, stefano.stabellini, ian.campbell, xen-devel

>>> On 12.02.16 at 15:06, <konrad.wilk@oracle.com> wrote:
> On Fri, Feb 12, 2016 at 01:51:28AM -0700, Jan Beulich wrote:
>> >>> On 12.02.16 at 04:08, <konrad.wilk@oracle.com> wrote:
>> > in the keyhandler.h file. Otherwise on ARM builds if we
>> > just use the keyhandler file - the compile will fail.
>> > 
>> > CC: ian.campbell@citrix.com 
>> > CC: wei.liu2@citrix.com 
>> > CC: stefano.stabellini@citrix.com 
>> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> 
>> See commit d1d181328d.
> 
> Weird. When I did git rebase origin/staging it didn't
> notice that commit. And it just applied it on top - so I had
> two 'struct cpu_regs;' in the header file.

That's likely because I moved the declaration down a few lines
while applying.

Jan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12 14:17     ` Konrad Rzeszutek Wilk
@ 2016-02-12 15:04       ` Jan Beulich
  2016-02-12 15:26         ` Stefano Stabellini
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2016-02-12 15:04 UTC (permalink / raw)
  To: Stefano Stabellini, Konrad Rzeszutek Wilk
  Cc: ian.jackson, wei.liu2, stefano.stabellini, ian.campbell, xen-devel

>>> On 12.02.16 at 15:17, <konrad.wilk@oracle.com> wrote:
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -15,8 +15,10 @@
>  
>  #if defined(CONFIG_ARM_64)
>  # define LONG_BYTEORDER 3
> +# define ELFSIZE 64
>  #else
>  # define LONG_BYTEORDER 2
> +# define ELFSIZE 64
>  #endif

Leaving the question - why twice instead of outside the #ifdef?

Jan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12 15:04       ` Jan Beulich
@ 2016-02-12 15:26         ` Stefano Stabellini
  2016-02-12 15:56           ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 34+ messages in thread
From: Stefano Stabellini @ 2016-02-12 15:26 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	stefano.stabellini, xen-devel

On Fri, 12 Feb 2016, Jan Beulich wrote:
> >>> On 12.02.16 at 15:17, <konrad.wilk@oracle.com> wrote:
> > --- a/xen/include/asm-arm/config.h
> > +++ b/xen/include/asm-arm/config.h
> > @@ -15,8 +15,10 @@
> >  
> >  #if defined(CONFIG_ARM_64)
> >  # define LONG_BYTEORDER 3
> > +# define ELFSIZE 64
> >  #else
> >  # define LONG_BYTEORDER 2
> > +# define ELFSIZE 64
> >  #endif
> 
> Leaving the question - why twice instead of outside the #ifdef?

Right, please move it out of the #ifdef.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12 15:26         ` Stefano Stabellini
@ 2016-02-12 15:56           ` Konrad Rzeszutek Wilk
  2016-02-12 15:57             ` Stefano Stabellini
  2016-03-16 17:32             ` Julien Grall
  0 siblings, 2 replies; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12 15:56 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: wei.liu2, ian.campbell, ian.jackson, stefano.stabellini,
	Jan Beulich, xen-devel

On Fri, Feb 12, 2016 at 03:26:14PM +0000, Stefano Stabellini wrote:
> On Fri, 12 Feb 2016, Jan Beulich wrote:
> > >>> On 12.02.16 at 15:17, <konrad.wilk@oracle.com> wrote:
> > > --- a/xen/include/asm-arm/config.h
> > > +++ b/xen/include/asm-arm/config.h
> > > @@ -15,8 +15,10 @@
> > >  
> > >  #if defined(CONFIG_ARM_64)
> > >  # define LONG_BYTEORDER 3
> > > +# define ELFSIZE 64
> > >  #else
> > >  # define LONG_BYTEORDER 2
> > > +# define ELFSIZE 64
> > >  #endif
> > 
> > Leaving the question - why twice instead of outside the #ifdef?
> 
> Right, please move it out of the #ifdef.

Done!

>From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 5 Feb 2016 10:44:45 -0500
Subject: [PATCH] arm/config: Declare ELFSIZE_64.

Otherwise any code that tries to use Elf_* macros would
require us to use Elf64_* types instead of the more
friendly Elf_ one.

This is OK to do since 32-bit ARM uses LPAE mode.

CC: ian.campbell@citrix.com
CC: wei.liu2@citrix.com
CC: stefano.stabellini@citrix.com
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/include/asm-arm/config.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index bd832df..a1b968d 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -25,6 +25,9 @@
 /* xen_ulong_t is always 64 bits */
 #define BITS_PER_XEN_ULONG 64
 
+/* And ELF files are also 64-bit. */
+#define ELFSIZE 64
+
 #define CONFIG_PAGING_ASSISTANCE 1
 
 #define CONFIG_PAGING_LEVELS 3
-- 
2.1.0

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12 15:56           ` Konrad Rzeszutek Wilk
@ 2016-02-12 15:57             ` Stefano Stabellini
  2016-02-12 17:50               ` Konrad Rzeszutek Wilk
  2016-03-16 17:32             ` Julien Grall
  1 sibling, 1 reply; 34+ messages in thread
From: Stefano Stabellini @ 2016-02-12 15:57 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	stefano.stabellini, Jan Beulich, xen-devel

On Fri, 12 Feb 2016, Konrad Rzeszutek Wilk wrote:
> On Fri, Feb 12, 2016 at 03:26:14PM +0000, Stefano Stabellini wrote:
> > On Fri, 12 Feb 2016, Jan Beulich wrote:
> > > >>> On 12.02.16 at 15:17, <konrad.wilk@oracle.com> wrote:
> > > > --- a/xen/include/asm-arm/config.h
> > > > +++ b/xen/include/asm-arm/config.h
> > > > @@ -15,8 +15,10 @@
> > > >  
> > > >  #if defined(CONFIG_ARM_64)
> > > >  # define LONG_BYTEORDER 3
> > > > +# define ELFSIZE 64
> > > >  #else
> > > >  # define LONG_BYTEORDER 2
> > > > +# define ELFSIZE 64
> > > >  #endif
> > > 
> > > Leaving the question - why twice instead of outside the #ifdef?
> > 
> > Right, please move it out of the #ifdef.
> 
> Done!
> 
> >From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Fri, 5 Feb 2016 10:44:45 -0500
> Subject: [PATCH] arm/config: Declare ELFSIZE_64.
> 
> Otherwise any code that tries to use Elf_* macros would
> require us to use Elf64_* types instead of the more
> friendly Elf_ one.
> 
> This is OK to do since 32-bit ARM uses LPAE mode.
> 
> CC: ian.campbell@citrix.com
> CC: wei.liu2@citrix.com
> CC: stefano.stabellini@citrix.com
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  xen/include/asm-arm/config.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index bd832df..a1b968d 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -25,6 +25,9 @@
>  /* xen_ulong_t is always 64 bits */
>  #define BITS_PER_XEN_ULONG 64
>  
> +/* And ELF files are also 64-bit. */
> +#define ELFSIZE 64
> +
>  #define CONFIG_PAGING_ASSISTANCE 1
>  
>  #define CONFIG_PAGING_LEVELS 3
> -- 
> 2.1.0
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12 15:57             ` Stefano Stabellini
@ 2016-02-12 17:50               ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-12 17:50 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: wei.liu2, ian.campbell, ian.jackson, stefano.stabellini,
	Jan Beulich, xen-devel

On Fri, Feb 12, 2016 at 03:57:25PM +0000, Stefano Stabellini wrote:
> On Fri, 12 Feb 2016, Konrad Rzeszutek Wilk wrote:
> > On Fri, Feb 12, 2016 at 03:26:14PM +0000, Stefano Stabellini wrote:
> > > On Fri, 12 Feb 2016, Jan Beulich wrote:
> > > > >>> On 12.02.16 at 15:17, <konrad.wilk@oracle.com> wrote:
> > > > > --- a/xen/include/asm-arm/config.h
> > > > > +++ b/xen/include/asm-arm/config.h
> > > > > @@ -15,8 +15,10 @@
> > > > >  
> > > > >  #if defined(CONFIG_ARM_64)
> > > > >  # define LONG_BYTEORDER 3
> > > > > +# define ELFSIZE 64
> > > > >  #else
> > > > >  # define LONG_BYTEORDER 2
> > > > > +# define ELFSIZE 64
> > > > >  #endif
> > > > 
> > > > Leaving the question - why twice instead of outside the #ifdef?
> > > 
> > > Right, please move it out of the #ifdef.
> > 
> > Done!
> > 
> > >From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001
> > From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Date: Fri, 5 Feb 2016 10:44:45 -0500
> > Subject: [PATCH] arm/config: Declare ELFSIZE_64.
> > 
> > Otherwise any code that tries to use Elf_* macros would
> > require us to use Elf64_* types instead of the more
> > friendly Elf_ one.
> > 
> > This is OK to do since 32-bit ARM uses LPAE mode.
> > 
> > CC: ian.campbell@citrix.com
> > CC: wei.liu2@citrix.com
> > CC: stefano.stabellini@citrix.com
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

applied. Thanks!
> 
> 
> >  xen/include/asm-arm/config.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> > index bd832df..a1b968d 100644
> > --- a/xen/include/asm-arm/config.h
> > +++ b/xen/include/asm-arm/config.h
> > @@ -25,6 +25,9 @@
> >  /* xen_ulong_t is always 64 bits */
> >  #define BITS_PER_XEN_ULONG 64
> >  
> > +/* And ELF files are also 64-bit. */
> > +#define ELFSIZE 64
> > +
> >  #define CONFIG_PAGING_ASSISTANCE 1
> >  
> >  #define CONFIG_PAGING_LEVELS 3
> > -- 
> > 2.1.0
> > 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths.
  2016-02-12  3:08 ` [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths Konrad Rzeszutek Wilk
@ 2016-02-18 16:29   ` Jan Beulich
  2016-02-18 16:39     ` Ian Jackson
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2016-02-18 16:29 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, ian.jackson, ian.campbell

>>> On 12.02.16 at 04:08, <konrad.wilk@oracle.com> wrote:
> While we are operating here we may as well fix some of the
> file descriptor leaks.

I'm not convinced. The added goto-s make the code uglier to read,
and this being a standalone utility there's not really any leak here.

Jan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths.
  2016-02-18 16:29   ` Jan Beulich
@ 2016-02-18 16:39     ` Ian Jackson
  2016-02-18 16:45       ` Jan Beulich
  0 siblings, 1 reply; 34+ messages in thread
From: Ian Jackson @ 2016-02-18 16:39 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, ian.campbell

Jan Beulich writes ("Re: [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths."):
> On 12.02.16 at 04:08, <konrad.wilk@oracle.com> wrote:
> > While we are operating here we may as well fix some of the
> > file descriptor leaks.
> 
> I'm not convinced. The added goto-s make the code uglier to read,
> and this being a standalone utility there's not really any leak here.

I don't buy this `uglier to read'.  What `return 1' does is make me
think `is some resource being leaked' and `do I need to audit this
thing'.

Ian.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths.
  2016-02-18 16:39     ` Ian Jackson
@ 2016-02-18 16:45       ` Jan Beulich
  2016-02-18 21:12         ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2016-02-18 16:45 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, ian.campbell

>>> On 18.02.16 at 17:39, <Ian.Jackson@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [PATCH v3 5/5] mkelf32: Close those file descriptors 
> in the error paths."):
>> On 12.02.16 at 04:08, <konrad.wilk@oracle.com> wrote:
>> > While we are operating here we may as well fix some of the
>> > file descriptor leaks.
>> 
>> I'm not convinced. The added goto-s make the code uglier to read,
>> and this being a standalone utility there's not really any leak here.
> 
> I don't buy this `uglier to read'.  What `return 1' does is make me
> think `is some resource being leaked' and `do I need to audit this
> thing'.

Certainly a matter of taste to some degree - goto-s are always
ugly to read to my eyes. Irrespective of this I don't buy the leak
aspect for a non-library like, short running build utility. The close()
calls are just more code, with absolutely no added benefit to the
system the code runs on.

Jan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths.
  2016-02-18 16:45       ` Jan Beulich
@ 2016-02-18 21:12         ` Konrad Rzeszutek Wilk
  2016-02-19 11:39           ` Jan Beulich
  2016-02-19 11:42           ` Ian Jackson
  0 siblings, 2 replies; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-18 21:12 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Ian Jackson, ian.campbell

On Thu, Feb 18, 2016 at 09:45:30AM -0700, Jan Beulich wrote:
> >>> On 18.02.16 at 17:39, <Ian.Jackson@eu.citrix.com> wrote:
> > Jan Beulich writes ("Re: [PATCH v3 5/5] mkelf32: Close those file descriptors 
> > in the error paths."):
> >> On 12.02.16 at 04:08, <konrad.wilk@oracle.com> wrote:
> >> > While we are operating here we may as well fix some of the
> >> > file descriptor leaks.
> >> 
> >> I'm not convinced. The added goto-s make the code uglier to read,
> >> and this being a standalone utility there's not really any leak here.
> > 
> > I don't buy this `uglier to read'.  What `return 1' does is make me
> > think `is some resource being leaked' and `do I need to audit this
> > thing'.
> 
> Certainly a matter of taste to some degree - goto-s are always
> ugly to read to my eyes. Irrespective of this I don't buy the leak
> aspect for a non-library like, short running build utility. The close()
> calls are just more code, with absolutely no added benefit to the
> system the code runs on.

<chuckles>

If I turned them in:

	if (..blah..)
	{
		close(infd);
		return -1;
	}

would that satisfy you?

(Irrespective of the 'no added benefit to the system the code runs
on.').

> 
> Jan
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths.
  2016-02-18 21:12         ` Konrad Rzeszutek Wilk
@ 2016-02-19 11:39           ` Jan Beulich
  2016-02-19 11:42           ` Ian Jackson
  1 sibling, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2016-02-19 11:39 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Ian Jackson, ian.campbell

>>> On 18.02.16 at 22:12, <konrad.wilk@oracle.com> wrote:
> On Thu, Feb 18, 2016 at 09:45:30AM -0700, Jan Beulich wrote:
>> >>> On 18.02.16 at 17:39, <Ian.Jackson@eu.citrix.com> wrote:
>> > Jan Beulich writes ("Re: [PATCH v3 5/5] mkelf32: Close those file 
> descriptors 
>> > in the error paths."):
>> >> On 12.02.16 at 04:08, <konrad.wilk@oracle.com> wrote:
>> >> > While we are operating here we may as well fix some of the
>> >> > file descriptor leaks.
>> >> 
>> >> I'm not convinced. The added goto-s make the code uglier to read,
>> >> and this being a standalone utility there's not really any leak here.
>> > 
>> > I don't buy this `uglier to read'.  What `return 1' does is make me
>> > think `is some resource being leaked' and `do I need to audit this
>> > thing'.
>> 
>> Certainly a matter of taste to some degree - goto-s are always
>> ugly to read to my eyes. Irrespective of this I don't buy the leak
>> aspect for a non-library like, short running build utility. The close()
>> calls are just more code, with absolutely no added benefit to the
>> system the code runs on.
> 
> <chuckles>
> 
> If I turned them in:
> 
> 	if (..blah..)
> 	{
> 		close(infd);
> 		return -1;
> 	}
> 
> would that satisfy you?

Since presumably this would be on a number of error paths, I'm
afraid ...

> (Irrespective of the 'no added benefit to the system the code runs
> on.').

... this aspect would still be an relevant one.

Jan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths.
  2016-02-18 21:12         ` Konrad Rzeszutek Wilk
  2016-02-19 11:39           ` Jan Beulich
@ 2016-02-19 11:42           ` Ian Jackson
  1 sibling, 0 replies; 34+ messages in thread
From: Ian Jackson @ 2016-02-19 11:42 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, ian.campbell, Jan Beulich

Konrad Rzeszutek Wilk writes ("Re: [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths."):
> <chuckles>
> 
> If I turned them in:
> 
> 	if (..blah..)
> 	{
> 		close(infd);
> 		return -1;
> 	}
> 
> would that satisfy you?

I would strongly resist that.  That coding style is an error handling
antipattern.

Ian.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-02-12 15:56           ` Konrad Rzeszutek Wilk
  2016-02-12 15:57             ` Stefano Stabellini
@ 2016-03-16 17:32             ` Julien Grall
  2016-03-16 17:52               ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 34+ messages in thread
From: Julien Grall @ 2016-03-16 17:32 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Stefano Stabellini
  Cc: wei.liu2, ian.campbell, ian.jackson, stefano.stabellini,
	Jan Beulich, xen-devel

Hi Konrad,

Sorry for the late answer on this patch. I noticed the problem while I 
was reviewing your xSplice patch series.

On 12/02/2016 15:56, Konrad Rzeszutek Wilk wrote:
>  From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Fri, 5 Feb 2016 10:44:45 -0500
> Subject: [PATCH] arm/config: Declare ELFSIZE_64.
>
> Otherwise any code that tries to use Elf_* macros would
> require us to use Elf64_* types instead of the more
> friendly Elf_ one.
>
> This is OK to do since 32-bit ARM uses LPAE mode.

That's not true. Some of structures have a different layout based on the 
file class (i.e ELFSIZE in Xen).

For 32-bit ARM, ELFCLASS32 (i.e 32-bit data types) will always be used. 
So we need to set ELFSIZE to 32.

Otherwise Xen won't be able to interpret correctly the ELF note holding 
the build-id (see [1]).

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2015-11/msg00632.html

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-03-16 17:32             ` Julien Grall
@ 2016-03-16 17:52               ` Konrad Rzeszutek Wilk
  2016-03-16 18:08                 ` Julien Grall
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-16 17:52 UTC (permalink / raw)
  To: Julien Grall
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	stefano.stabellini, Jan Beulich, xen-devel

On Wed, Mar 16, 2016 at 05:32:09PM +0000, Julien Grall wrote:
> Hi Konrad,
> 
> Sorry for the late answer on this patch. I noticed the problem while I was
> reviewing your xSplice patch series.
> 
> On 12/02/2016 15:56, Konrad Rzeszutek Wilk wrote:
> > From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001
> >From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >Date: Fri, 5 Feb 2016 10:44:45 -0500
> >Subject: [PATCH] arm/config: Declare ELFSIZE_64.
> >
> >Otherwise any code that tries to use Elf_* macros would
> >require us to use Elf64_* types instead of the more
> >friendly Elf_ one.
> >
> >This is OK to do since 32-bit ARM uses LPAE mode.
> 
> That's not true. Some of structures have a different layout based on the
> file class (i.e ELFSIZE in Xen).
> 
> For 32-bit ARM, ELFCLASS32 (i.e 32-bit data types) will always be used. So
> we need to set ELFSIZE to 32.

OK. Let me send out a patch to fix that. Thanks for the heads up!
> 
> Otherwise Xen won't be able to interpret correctly the ELF note holding the
> build-id (see [1]).
> 
> Regards,
> 
> [1] http://lists.xen.org/archives/html/xen-devel/2015-11/msg00632.html
> 
> -- 
> Julien Grall

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-03-16 17:52               ` Konrad Rzeszutek Wilk
@ 2016-03-16 18:08                 ` Julien Grall
  2016-03-16 19:21                   ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 34+ messages in thread
From: Julien Grall @ 2016-03-16 18:08 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	stefano.stabellini, Jan Beulich, xen-devel

Hi Konrad

On 16/03/2016 17:52, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 16, 2016 at 05:32:09PM +0000, Julien Grall wrote:
>> Sorry for the late answer on this patch. I noticed the problem while I was
>> reviewing your xSplice patch series.
>>
>> On 12/02/2016 15:56, Konrad Rzeszutek Wilk wrote:
>>>  From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001
>>> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Date: Fri, 5 Feb 2016 10:44:45 -0500
>>> Subject: [PATCH] arm/config: Declare ELFSIZE_64.
>>>
>>> Otherwise any code that tries to use Elf_* macros would
>>> require us to use Elf64_* types instead of the more
>>> friendly Elf_ one.
>>>
>>> This is OK to do since 32-bit ARM uses LPAE mode.
>>
>> That's not true. Some of structures have a different layout based on the
>> file class (i.e ELFSIZE in Xen).
>>
>> For 32-bit ARM, ELFCLASS32 (i.e 32-bit data types) will always be used. So
>> we need to set ELFSIZE to 32.
>
> OK. Let me send out a patch to fix that. Thanks for the heads up!

With this change, do you need to revise the answer to the question Jan 
raised in the design document about ARM32 and the ELF payload declaring 
the ELF only 64-bit syntax?

Regards,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-03-16 18:08                 ` Julien Grall
@ 2016-03-16 19:21                   ` Konrad Rzeszutek Wilk
  2016-03-17  1:16                     ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-16 19:21 UTC (permalink / raw)
  To: Julien Grall
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	stefano.stabellini, Jan Beulich, xen-devel

On Wed, Mar 16, 2016 at 06:08:06PM +0000, Julien Grall wrote:
> Hi Konrad
> 
> On 16/03/2016 17:52, Konrad Rzeszutek Wilk wrote:
> >On Wed, Mar 16, 2016 at 05:32:09PM +0000, Julien Grall wrote:
> >>Sorry for the late answer on this patch. I noticed the problem while I was
> >>reviewing your xSplice patch series.
> >>
> >>On 12/02/2016 15:56, Konrad Rzeszutek Wilk wrote:
> >>> From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001
> >>>From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >>>Date: Fri, 5 Feb 2016 10:44:45 -0500
> >>>Subject: [PATCH] arm/config: Declare ELFSIZE_64.
> >>>
> >>>Otherwise any code that tries to use Elf_* macros would
> >>>require us to use Elf64_* types instead of the more
> >>>friendly Elf_ one.
> >>>
> >>>This is OK to do since 32-bit ARM uses LPAE mode.
> >>
> >>That's not true. Some of structures have a different layout based on the
> >>file class (i.e ELFSIZE in Xen).
> >>
> >>For 32-bit ARM, ELFCLASS32 (i.e 32-bit data types) will always be used. So
> >>we need to set ELFSIZE to 32.
> >
> >OK. Let me send out a patch to fix that. Thanks for the heads up!
> 
> With this change, do you need to revise the answer to the question Jan
> raised in the design document about ARM32 and the ELF payload declaring the
> ELF only 64-bit syntax?

Yes I will. Thank you for raising that as I completly forgot it!

I think we can still keep the same structure and size. But instead of
using ELF types in the design (and in the code) I will swap them to
proper uintXX_t types.

> 
> Regards,
> 
> -- 
> Julien Grall

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-03-16 19:21                   ` Konrad Rzeszutek Wilk
@ 2016-03-17  1:16                     ` Konrad Rzeszutek Wilk
  2016-03-17 11:04                       ` Julien Grall
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-17  1:16 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	Julien Grall, stefano.stabellini, Jan Beulich, xen-devel

On Wed, Mar 16, 2016 at 03:21:51PM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 16, 2016 at 06:08:06PM +0000, Julien Grall wrote:
> > Hi Konrad
> > 
> > On 16/03/2016 17:52, Konrad Rzeszutek Wilk wrote:
> > >On Wed, Mar 16, 2016 at 05:32:09PM +0000, Julien Grall wrote:
> > >>Sorry for the late answer on this patch. I noticed the problem while I was
> > >>reviewing your xSplice patch series.
> > >>
> > >>On 12/02/2016 15:56, Konrad Rzeszutek Wilk wrote:
> > >>> From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001
> > >>>From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > >>>Date: Fri, 5 Feb 2016 10:44:45 -0500
> > >>>Subject: [PATCH] arm/config: Declare ELFSIZE_64.
> > >>>
> > >>>Otherwise any code that tries to use Elf_* macros would
> > >>>require us to use Elf64_* types instead of the more
> > >>>friendly Elf_ one.
> > >>>
> > >>>This is OK to do since 32-bit ARM uses LPAE mode.
> > >>
> > >>That's not true. Some of structures have a different layout based on the
> > >>file class (i.e ELFSIZE in Xen).
> > >>
> > >>For 32-bit ARM, ELFCLASS32 (i.e 32-bit data types) will always be used. So
> > >>we need to set ELFSIZE to 32.
> > >
> > >OK. Let me send out a patch to fix that. Thanks for the heads up!
> > 
> > With this change, do you need to revise the answer to the question Jan
> > raised in the design document about ARM32 and the ELF payload declaring the
> > ELF only 64-bit syntax?
> 
> Yes I will. Thank you for raising that as I completly forgot it!
> 
> I think we can still keep the same structure and size. But instead of
> using ELF types in the design (and in the code) I will swap them to
> proper uintXX_t types.

And here is the patch. The change for uintXX_t type worked out - same
size and offset as on 64-bit. Thought I am tempted to add some
more BUILD_BUG checks.

From 7007f1a3fa3a77a725f529420c7aea0e8ebdc9fa Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 16 Mar 2016 16:08:25 -0400
Subject: [PATCH v4 01/35] arm/config: Declare ELFSIZE_[32|64]

The commit bcfaea685d38c08e5eb90797512ab80f0bc69d0c
"arm/config: Declare ELFSIZE_64" was not correct.

For 32-bit ARM, ELFCLASS32 (i.e. 32-bit data types) will always
be used so we need to set ELFSIZE to 32.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
--
---
 xen/include/asm-arm/config.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 7ceb5c5..2d11b62 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -9,8 +9,10 @@
 
 #if defined(CONFIG_ARM_64)
 # define LONG_BYTEORDER 3
+# define ELFSIZE 64
 #else
 # define LONG_BYTEORDER 2
+# define ELFSIZE 32
 #endif
 
 #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
@@ -20,9 +22,6 @@
 /* xen_ulong_t is always 64 bits */
 #define BITS_PER_XEN_ULONG 64
 
-/* And ELF files are also 64-bit. */
-#define ELFSIZE 64
-
 #define CONFIG_PAGING_ASSISTANCE 1
 
 #define CONFIG_PAGING_LEVELS 3
-- 
2.5.0

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-03-17  1:16                     ` Konrad Rzeszutek Wilk
@ 2016-03-17 11:04                       ` Julien Grall
  2016-03-17 18:52                         ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 34+ messages in thread
From: Julien Grall @ 2016-03-17 11:04 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Konrad Rzeszutek Wilk
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	stefano.stabellini, Jan Beulich, xen-devel

Hi Konrad,

On 17/03/16 01:16, Konrad Rzeszutek Wilk wrote:
> And here is the patch. The change for uintXX_t type worked out - same
> size and offset as on 64-bit. Thought I am tempted to add some
> more BUILD_BUG checks.
>
>  From 7007f1a3fa3a77a725f529420c7aea0e8ebdc9fa Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Wed, 16 Mar 2016 16:08:25 -0400
> Subject: [PATCH v4 01/35] arm/config: Declare ELFSIZE_[32|64]
>
> The commit bcfaea685d38c08e5eb90797512ab80f0bc69d0c
> "arm/config: Declare ELFSIZE_64" was not correct.
>
> For 32-bit ARM, ELFCLASS32 (i.e. 32-bit data types) will always
> be used so we need to set ELFSIZE to 32.
>
> Reported-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Regards,

>
> ---
> Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>
> --
> ---
>   xen/include/asm-arm/config.h | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 7ceb5c5..2d11b62 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -9,8 +9,10 @@
>
>   #if defined(CONFIG_ARM_64)
>   # define LONG_BYTEORDER 3
> +# define ELFSIZE 64
>   #else
>   # define LONG_BYTEORDER 2
> +# define ELFSIZE 32
>   #endif
>
>   #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
> @@ -20,9 +22,6 @@
>   /* xen_ulong_t is always 64 bits */
>   #define BITS_PER_XEN_ULONG 64
>
> -/* And ELF files are also 64-bit. */
> -#define ELFSIZE 64
> -
>   #define CONFIG_PAGING_ASSISTANCE 1
>
>   #define CONFIG_PAGING_LEVELS 3
>

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
  2016-03-17 11:04                       ` Julien Grall
@ 2016-03-17 18:52                         ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-17 18:52 UTC (permalink / raw)
  To: Julien Grall
  Cc: wei.liu2, ian.campbell, Stefano Stabellini, ian.jackson,
	stefano.stabellini, Jan Beulich, xen-devel

On Thu, Mar 17, 2016 at 11:04:29AM +0000, Julien Grall wrote:
> Hi Konrad,
> 
> On 17/03/16 01:16, Konrad Rzeszutek Wilk wrote:
> >And here is the patch. The change for uintXX_t type worked out - same
> >size and offset as on 64-bit. Thought I am tempted to add some
> >more BUILD_BUG checks.
> >
> > From 7007f1a3fa3a77a725f529420c7aea0e8ebdc9fa Mon Sep 17 00:00:00 2001
> >From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >Date: Wed, 16 Mar 2016 16:08:25 -0400
> >Subject: [PATCH v4 01/35] arm/config: Declare ELFSIZE_[32|64]
> >
> >The commit bcfaea685d38c08e5eb90797512ab80f0bc69d0c
> >"arm/config: Declare ELFSIZE_64" was not correct.
> >
> >For 32-bit ARM, ELFCLASS32 (i.e. 32-bit data types) will always
> >be used so we need to set ELFSIZE to 32.
> >
> >Reported-by: Julien Grall <julien.grall@arm.com>
> >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Acked-by: Julien Grall <julien.grall@arm.com>

Thank you. Patch applied.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2016-03-17 18:52 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-12  3:08 [PATCH v3] Requisite paches for xSplice v3 (not yet posted) Konrad Rzeszutek Wilk
2016-02-12  3:08 ` [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs; Konrad Rzeszutek Wilk
2016-02-12  8:51   ` Jan Beulich
2016-02-12 14:06     ` Konrad Rzeszutek Wilk
2016-02-12 14:54       ` Jan Beulich
2016-02-12 11:37   ` Stefano Stabellini
2016-02-12 12:20     ` Jan Beulich
2016-02-12 12:46       ` Stefano Stabellini
2016-02-12 14:53         ` Jan Beulich
2016-02-12  3:08 ` [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively Konrad Rzeszutek Wilk
2016-02-12 11:26   ` Stefano Stabellini
2016-02-12 14:17     ` Konrad Rzeszutek Wilk
2016-02-12 15:04       ` Jan Beulich
2016-02-12 15:26         ` Stefano Stabellini
2016-02-12 15:56           ` Konrad Rzeszutek Wilk
2016-02-12 15:57             ` Stefano Stabellini
2016-02-12 17:50               ` Konrad Rzeszutek Wilk
2016-03-16 17:32             ` Julien Grall
2016-03-16 17:52               ` Konrad Rzeszutek Wilk
2016-03-16 18:08                 ` Julien Grall
2016-03-16 19:21                   ` Konrad Rzeszutek Wilk
2016-03-17  1:16                     ` Konrad Rzeszutek Wilk
2016-03-17 11:04                       ` Julien Grall
2016-03-17 18:52                         ` Konrad Rzeszutek Wilk
2016-02-12  3:08 ` [PATCH v3 3/5] build: remove .config from /boot when uninstalling Konrad Rzeszutek Wilk
2016-02-12 14:11   ` Doug Goldstein
2016-02-12  3:08 ` [PATCH v3 4/5] mkelf32: Remove the 32-bit hypervisor support Konrad Rzeszutek Wilk
2016-02-12  3:08 ` [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths Konrad Rzeszutek Wilk
2016-02-18 16:29   ` Jan Beulich
2016-02-18 16:39     ` Ian Jackson
2016-02-18 16:45       ` Jan Beulich
2016-02-18 21:12         ` Konrad Rzeszutek Wilk
2016-02-19 11:39           ` Jan Beulich
2016-02-19 11:42           ` Ian Jackson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).