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

Commit Message

Konrad Rzeszutek Wilk March 17, 2016, 1:16 a.m. UTC

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 <[e-mail]>
> > >>>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 <[e-mail]>
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 <[e-mail]>
Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>

---
Cc: Stefano Stabellini <[e-mail]>
Cc: Julien Grall <[e-mail]>
--
---
 xen/include/asm-arm/config.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Julien Grall March 17, 2016, 11:04 a.m. UTC | #1

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 <[e-mail]>
> 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 <[e-mail]>
> Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>

Acked-by: Julien Grall <[e-mail]>

Regards,

>
> ---
> Cc: Stefano Stabellini <[e-mail]>
> Cc: Julien Grall <[e-mail]>
> --
> ---
>   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
>

Konrad Rzeszutek Wilk March 17, 2016, 6:52 p.m. UTC | #2

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 <[e-mail]>
> >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 <[e-mail]>
> >Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
> 
> Acked-by: Julien Grall <[e-mail]>

Thank you. Patch applied.

Patch

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