* [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 related [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 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 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 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 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
* [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 related [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 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 related [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 related [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 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 related [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
* [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 related [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 related [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 related [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