* [PATCH] xen/xsm: Annotate xsm_initcall() data as const
2016-06-09 9:58 [PATCH] xen/init: Annotate all command line parameter infrastructure as const Andrew Cooper
@ 2016-06-09 9:58 ` Andrew Cooper
2016-06-09 12:40 ` Jan Beulich
2016-06-09 14:26 ` Daniel De Graaf
2016-06-09 9:58 ` [PATCH] x86/boot: copy/clear sections more efficiently Andrew Cooper
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Andrew Cooper @ 2016-06-09 9:58 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Daniel De Graaf, Stefano Stabellini, Julien Grall,
Jan Beulich
Additionally, link it adjacently to the other constant init data.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/xen.lds.S | 9 ++++-----
xen/arch/x86/xen.lds.S | 9 ++++-----
xen/include/xsm/xsm.h | 2 +-
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 0ad2ad9..ff621b6 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -141,6 +141,10 @@ SECTIONS
*(.init.setup)
__setup_end = .;
+ __xsm_initcall_start = .;
+ *(.xsm_initcall.init)
+ __xsm_initcall_end = .;
+
*(.init.data)
*(.init.data.rel)
*(.init.data.rel.*)
@@ -163,11 +167,6 @@ SECTIONS
*(.initcall1.init)
__initcall_end = .;
} :text
- .xsm_initcall.init : {
- __xsm_initcall_start = .;
- *(.xsm_initcall.init)
- __xsm_initcall_end = .;
- } :text
__init_end_efi = .;
. = ALIGN(STACK_SIZE);
__init_end = .;
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e506714..301fd8c 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -158,6 +158,10 @@ SECTIONS
*(.init.setup)
__setup_end = .;
+ __xsm_initcall_start = .;
+ *(.xsm_initcall.init)
+ __xsm_initcall_end = .;
+
*(.init.data)
*(.init.data.rel)
*(.init.data.rel.*)
@@ -191,11 +195,6 @@ SECTIONS
*(.initcall1.init)
__initcall_end = .;
} :text
- .xsm_initcall.init : {
- __xsm_initcall_start = .;
- *(.xsm_initcall.init)
- __xsm_initcall_end = .;
- } :text
. = ALIGN(PAGE_SIZE);
__init_end = .;
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 8ed8ee5..c1aa69c 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -51,7 +51,7 @@ typedef void (*xsm_initcall_t)(void);
extern xsm_initcall_t __xsm_initcall_start[], __xsm_initcall_end[];
#define xsm_initcall(fn) \
- static xsm_initcall_t __initcall_##fn \
+ static const xsm_initcall_t __initcall_##fn \
__used_section(".xsm_initcall.init") = fn
struct xsm_operations {
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] xen/xsm: Annotate xsm_initcall() data as const
2016-06-09 9:58 ` [PATCH] xen/xsm: Annotate xsm_initcall() data " Andrew Cooper
@ 2016-06-09 12:40 ` Jan Beulich
2016-06-09 14:26 ` Daniel De Graaf
1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2016-06-09 12:40 UTC (permalink / raw)
To: Andrew Cooper
Cc: Julien Grall, Stefano Stabellini, Daniel De Graaf, Xen-devel
>>> On 09.06.16 at 11:58, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -51,7 +51,7 @@ typedef void (*xsm_initcall_t)(void);
> extern xsm_initcall_t __xsm_initcall_start[], __xsm_initcall_end[];
>
> #define xsm_initcall(fn) \
> - static xsm_initcall_t __initcall_##fn \
> + static const xsm_initcall_t __initcall_##fn \
> __used_section(".xsm_initcall.init") = fn
Same here - please make the bounding symbol declarations then
const too.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] xen/xsm: Annotate xsm_initcall() data as const
2016-06-09 9:58 ` [PATCH] xen/xsm: Annotate xsm_initcall() data " Andrew Cooper
2016-06-09 12:40 ` Jan Beulich
@ 2016-06-09 14:26 ` Daniel De Graaf
2016-06-09 14:28 ` Andrew Cooper
1 sibling, 1 reply; 13+ messages in thread
From: Daniel De Graaf @ 2016-06-09 14:26 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich
On 06/09/2016 05:58 AM, Andrew Cooper wrote:
> Additionally, link it adjacently to the other constant init data.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
I think this section can actually be removed instead: it only has
one user, additional users will need special switching code added
anyway, and it should make the setup code easier to follow.
I already have a patch for the removal; I'll send that series soon.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xen/xsm: Annotate xsm_initcall() data as const
2016-06-09 14:26 ` Daniel De Graaf
@ 2016-06-09 14:28 ` Andrew Cooper
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2016-06-09 14:28 UTC (permalink / raw)
To: Daniel De Graaf, Xen-devel; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich
On 09/06/16 15:26, Daniel De Graaf wrote:
> On 06/09/2016 05:58 AM, Andrew Cooper wrote:
>> Additionally, link it adjacently to the other constant init data.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> I think this section can actually be removed instead: it only has
> one user, additional users will need special switching code added
> anyway, and it should make the setup code easier to follow.
>
> I already have a patch for the removal; I'll send that series soon.
Ok - I will drop this patch from my series.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] x86/boot: copy/clear sections more efficiently
2016-06-09 9:58 [PATCH] xen/init: Annotate all command line parameter infrastructure as const Andrew Cooper
2016-06-09 9:58 ` [PATCH] xen/xsm: Annotate xsm_initcall() data " Andrew Cooper
@ 2016-06-09 9:58 ` Andrew Cooper
2016-06-09 12:42 ` Jan Beulich
2016-06-09 10:03 ` [PATCH] xen/init: Annotate all command line parameter infrastructure as const Andrew Cooper
2016-06-09 12:39 ` Jan Beulich
3 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2016-06-09 9:58 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Both the trampoline copy and BSS initialise can be performed more
efficiently by using 4-byte variants of the string operations.
On Intel systems with ERMSB (efficient rep movsb), this is no practical
difference. On all other systems, this is 4 times more efficient.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/arch/x86/boot/head.S | 9 +++++----
xen/arch/x86/xen.lds.S | 6 ++++++
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 0999997..6060ec2 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -128,7 +128,8 @@ __start:
mov $sym_phys(__bss_end),%ecx
sub %edi,%ecx
xor %eax,%eax
- rep stosb
+ shr $2,%ecx
+ rep stosl
/* Interrogate CPU extended features via CPUID. */
mov $0x80000000,%eax
@@ -192,8 +193,8 @@ __start:
/* Copy bootstrap trampoline to low memory, below 1MB. */
mov $sym_phys(trampoline_start),%esi
- mov $trampoline_end - trampoline_start,%ecx
- rep movsb
+ mov $((trampoline_end - trampoline_start) / 4),%ecx
+ rep movsl
/* Jump into the relocated trampoline. */
lret
@@ -205,6 +206,6 @@ reloc:
ENTRY(trampoline_start)
#include "trampoline.S"
-GLOBAL(trampoline_end)
+ENTRY(trampoline_end)
#include "x86_64.S"
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 301fd8c..8731b39 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -240,6 +240,7 @@ SECTIONS
*(.bss.percpu.read_mostly)
. = ALIGN(SMP_CACHE_BYTES);
__per_cpu_data_end = .;
+ . = ALIGN(4);
__bss_end = .;
} :text
_end = . ;
@@ -320,3 +321,8 @@ ASSERT(IS_ALIGNED(cpu0_stack, STACK_SIZE), "cpu0_stack misaligned")
ASSERT(IS_ALIGNED(__init_begin, PAGE_SIZE), "__init_begin misaligned")
ASSERT(IS_ALIGNED(__init_end, PAGE_SIZE), "__init_end misaligned")
+
+ASSERT(IS_ALIGNED(trampoline_start, 4), "trampoline_start misaligned")
+ASSERT(IS_ALIGNED(trampoline_end, 4), "trampoline_end misaligned")
+ASSERT(IS_ALIGNED(__bss_start, 4), "__bss_start misaligned")
+ASSERT(IS_ALIGNED(__bss_end, 4), "__bss_end misaligned")
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] x86/boot: copy/clear sections more efficiently
2016-06-09 9:58 ` [PATCH] x86/boot: copy/clear sections more efficiently Andrew Cooper
@ 2016-06-09 12:42 ` Jan Beulich
0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2016-06-09 12:42 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 09.06.16 at 11:58, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -128,7 +128,8 @@ __start:
> mov $sym_phys(__bss_end),%ecx
> sub %edi,%ecx
> xor %eax,%eax
> - rep stosb
> + shr $2,%ecx
> + rep stosl
Please take the opportunity and reduce the whitespace between
prefix and instruction to just one (to make clear "rep" is not the
instruction and "stosl" is not it operand).
> @@ -192,8 +193,8 @@ __start:
>
> /* Copy bootstrap trampoline to low memory, below 1MB. */
> mov $sym_phys(trampoline_start),%esi
> - mov $trampoline_end - trampoline_start,%ecx
> - rep movsb
> + mov $((trampoline_end - trampoline_start) / 4),%ecx
> + rep movsl
Same here.
With that
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xen/init: Annotate all command line parameter infrastructure as const
2016-06-09 9:58 [PATCH] xen/init: Annotate all command line parameter infrastructure as const Andrew Cooper
2016-06-09 9:58 ` [PATCH] xen/xsm: Annotate xsm_initcall() data " Andrew Cooper
2016-06-09 9:58 ` [PATCH] x86/boot: copy/clear sections more efficiently Andrew Cooper
@ 2016-06-09 10:03 ` Andrew Cooper
2016-06-09 12:39 ` Jan Beulich
3 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2016-06-09 10:03 UTC (permalink / raw)
To: Xen-devel; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich
CC'ing Stefano's correct address. (This patch pre-dates quite a lot of
recent changes).
On 09/06/16 10:58, Andrew Cooper wrote:
> There is no reason for any of it to be modified. Additionally, link
> .init.setup beside the other constant .init data.
>
> While editing this area, correct the types used in the extern
> declarations for __setup_start and __setup_end to match the types the
> linker actually produces.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
> ---
> xen/arch/arm/xen.lds.S | 11 ++++++-----
> xen/arch/x86/xen.lds.S | 11 ++++++-----
> xen/common/kernel.c | 2 +-
> xen/include/xen/init.h | 7 ++++---
> 4 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 76982b2..0ad2ad9 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -135,6 +135,12 @@ SECTIONS
> *(.init.rodata)
> *(.init.rodata.rel)
> *(.init.rodata.str*)
> +
> + . = ALIGN(POINTER_ALIGN);
> + __setup_start = .;
> + *(.init.setup)
> + __setup_end = .;
> +
> *(.init.data)
> *(.init.data.rel)
> *(.init.data.rel.*)
> @@ -145,11 +151,6 @@ SECTIONS
> __ctors_end = .;
> } :text
> . = ALIGN(32);
> - .init.setup : {
> - __setup_start = .;
> - *(.init.setup)
> - __setup_end = .;
> - } :text
> .init.proc.info : {
> __proc_info_start = .;
> *(.init.proc.info)
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index a43b29d..e506714 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -152,6 +152,12 @@ SECTIONS
> *(.init.rodata)
> *(.init.rodata.rel)
> *(.init.rodata.str*)
> +
> + . = ALIGN(POINTER_ALIGN);
> + __setup_start = .;
> + *(.init.setup)
> + __setup_end = .;
> +
> *(.init.data)
> *(.init.data.rel)
> *(.init.data.rel.*)
> @@ -178,11 +184,6 @@ SECTIONS
> __ctors_end = .;
> } :text
> . = ALIGN(32);
> - .init.setup : {
> - __setup_start = .;
> - *(.init.setup)
> - __setup_end = .;
> - } :text
> .initcall.init : {
> __initcall_start = .;
> *(.initcallpresmp.init)
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index 1a6823a..6f785bb 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -96,7 +96,7 @@ void __init cmdline_parse(const char *cmdline)
> if ( !bool_assert )
> optkey += 3;
>
> - for ( param = &__setup_start; param < &__setup_end; param++ )
> + for ( param = __setup_start; param < __setup_end; param++ )
> {
> if ( strcmp(param->name, optkey) )
> {
> diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h
> index 671ac81..9d7a080 100644
> --- a/xen/include/xen/init.h
> +++ b/xen/include/xen/init.h
> @@ -86,10 +86,11 @@ struct kernel_param {
> void *var;
> };
>
> -extern struct kernel_param __setup_start, __setup_end;
> +extern struct kernel_param __setup_start[], __setup_end[];
>
> -#define __setup_str static __initdata __attribute__((__aligned__(1))) char
> -#define __kparam static __initsetup \
> +#define __setup_str static const __initconstrel \
> + __attribute__((__aligned__(1))) char
> +#define __kparam static const __initsetup \
> __attribute__((__aligned__(sizeof(void *)))) struct kernel_param
>
> #define custom_param(_name, _var) \
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] xen/init: Annotate all command line parameter infrastructure as const
2016-06-09 9:58 [PATCH] xen/init: Annotate all command line parameter infrastructure as const Andrew Cooper
` (2 preceding siblings ...)
2016-06-09 10:03 ` [PATCH] xen/init: Annotate all command line parameter infrastructure as const Andrew Cooper
@ 2016-06-09 12:39 ` Jan Beulich
2016-06-09 13:45 ` Andrew Cooper
3 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2016-06-09 12:39 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Julien Grall, Stefano Stabellini, Xen-devel
>>> On 09.06.16 at 11:58, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -135,6 +135,12 @@ SECTIONS
> *(.init.rodata)
> *(.init.rodata.rel)
> *(.init.rodata.str*)
> +
> + . = ALIGN(POINTER_ALIGN);
> + __setup_start = .;
> + *(.init.setup)
> + __setup_end = .;
> +
> *(.init.data)
> *(.init.data.rel)
> *(.init.data.rel.*)
> @@ -145,11 +151,6 @@ SECTIONS
> __ctors_end = .;
> } :text
> . = ALIGN(32);
> - .init.setup : {
> - __setup_start = .;
> - *(.init.setup)
> - __setup_end = .;
> - } :text
> .init.proc.info : {
Surely that ALIGN() then has no reason to retain the 32 (similar
for x86)?
> --- a/xen/include/xen/init.h
> +++ b/xen/include/xen/init.h
> @@ -86,10 +86,11 @@ struct kernel_param {
> void *var;
> };
>
> -extern struct kernel_param __setup_start, __setup_end;
> +extern struct kernel_param __setup_start[], __setup_end[];
I thought your respective remark in the commit message would
also implied making these const now that they are.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] xen/init: Annotate all command line parameter infrastructure as const
2016-06-09 12:39 ` Jan Beulich
@ 2016-06-09 13:45 ` Andrew Cooper
2016-06-09 14:19 ` Julien Grall
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2016-06-09 13:45 UTC (permalink / raw)
To: Jan Beulich; +Cc: Julien Grall, Stefano Stabellini, Xen-devel
On 09/06/16 13:39, Jan Beulich wrote:
>>>> On 09.06.16 at 11:58, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/arm/xen.lds.S
>> +++ b/xen/arch/arm/xen.lds.S
>> @@ -135,6 +135,12 @@ SECTIONS
>> *(.init.rodata)
>> *(.init.rodata.rel)
>> *(.init.rodata.str*)
>> +
>> + . = ALIGN(POINTER_ALIGN);
>> + __setup_start = .;
>> + *(.init.setup)
>> + __setup_end = .;
>> +
>> *(.init.data)
>> *(.init.data.rel)
>> *(.init.data.rel.*)
>> @@ -145,11 +151,6 @@ SECTIONS
>> __ctors_end = .;
>> } :text
>> . = ALIGN(32);
>> - .init.setup : {
>> - __setup_start = .;
>> - *(.init.setup)
>> - __setup_end = .;
>> - } :text
>> .init.proc.info : {
> Surely that ALIGN() then has no reason to retain the 32 (similar
> for x86)?
I don't know where this ALIGN() came from, but I am hesitant to remove
it until I am sure it is safe to do so.
For both x86 and arm, .initcall will take a little more work to
disentangle, although I do intend to make it happen.
On arm, .init.proc.info is an array of 32byte elements. Looking at its
contents, it should be constant, and probably wants 4 byte alignment as
opposed to 32.
>
>> --- a/xen/include/xen/init.h
>> +++ b/xen/include/xen/init.h
>> @@ -86,10 +86,11 @@ struct kernel_param {
>> void *var;
>> };
>>
>> -extern struct kernel_param __setup_start, __setup_end;
>> +extern struct kernel_param __setup_start[], __setup_end[];
> I thought your respective remark in the commit message would
> also implied making these const now that they are.
Will do.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] xen/init: Annotate all command line parameter infrastructure as const
2016-06-09 13:45 ` Andrew Cooper
@ 2016-06-09 14:19 ` Julien Grall
2016-06-09 14:20 ` Julien Grall
2016-06-09 14:28 ` Andrew Cooper
0 siblings, 2 replies; 13+ messages in thread
From: Julien Grall @ 2016-06-09 14:19 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich; +Cc: Stefano Stabellini, Xen-devel
Hi Andrew,
On 09/06/16 14:45, Andrew Cooper wrote:
> On 09/06/16 13:39, Jan Beulich wrote:
>>>>> On 09.06.16 at 11:58, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/arch/arm/xen.lds.S
>>> +++ b/xen/arch/arm/xen.lds.S
>>> @@ -135,6 +135,12 @@ SECTIONS
>>> *(.init.rodata)
>>> *(.init.rodata.rel)
>>> *(.init.rodata.str*)
>>> +
>>> + . = ALIGN(POINTER_ALIGN);
>>> + __setup_start = .;
>>> + *(.init.setup)
>>> + __setup_end = .;
>>> +
>>> *(.init.data)
>>> *(.init.data.rel)
>>> *(.init.data.rel.*)
>>> @@ -145,11 +151,6 @@ SECTIONS
>>> __ctors_end = .;
>>> } :text
>>> . = ALIGN(32);
>>> - .init.setup : {
>>> - __setup_start = .;
>>> - *(.init.setup)
>>> - __setup_end = .;
>>> - } :text
>>> .init.proc.info : {
>> Surely that ALIGN() then has no reason to retain the 32 (similar
>> for x86)?
>
> I don't know where this ALIGN() came from, but I am hesitant to remove
> it until I am sure it is safe to do so.
>
> For both x86 and arm, .initcall will take a little more work to
> disentangle, although I do intend to make it happen.
>
> On arm, .init.proc.info is an array of 32byte elements. Looking at its
> contents, it should be constant, and probably wants 4 byte alignment as
> opposed to 32.
proc_info_list contains some pointers, so we want 4-byte for ARM32 and
8-byte for ARM64. I would use POINTER_ALIGN here.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] xen/init: Annotate all command line parameter infrastructure as const
2016-06-09 14:19 ` Julien Grall
@ 2016-06-09 14:20 ` Julien Grall
2016-06-09 14:28 ` Andrew Cooper
1 sibling, 0 replies; 13+ messages in thread
From: Julien Grall @ 2016-06-09 14:20 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich; +Cc: Stefano Stabellini, Xen-devel
(CC the correct email for Stefano)
On 09/06/16 15:19, Julien Grall wrote:
> Hi Andrew,
>
> On 09/06/16 14:45, Andrew Cooper wrote:
>> On 09/06/16 13:39, Jan Beulich wrote:
>>>>>> On 09.06.16 at 11:58, <andrew.cooper3@citrix.com> wrote:
>>>> --- a/xen/arch/arm/xen.lds.S
>>>> +++ b/xen/arch/arm/xen.lds.S
>>>> @@ -135,6 +135,12 @@ SECTIONS
>>>> *(.init.rodata)
>>>> *(.init.rodata.rel)
>>>> *(.init.rodata.str*)
>>>> +
>>>> + . = ALIGN(POINTER_ALIGN);
>>>> + __setup_start = .;
>>>> + *(.init.setup)
>>>> + __setup_end = .;
>>>> +
>>>> *(.init.data)
>>>> *(.init.data.rel)
>>>> *(.init.data.rel.*)
>>>> @@ -145,11 +151,6 @@ SECTIONS
>>>> __ctors_end = .;
>>>> } :text
>>>> . = ALIGN(32);
>>>> - .init.setup : {
>>>> - __setup_start = .;
>>>> - *(.init.setup)
>>>> - __setup_end = .;
>>>> - } :text
>>>> .init.proc.info : {
>>> Surely that ALIGN() then has no reason to retain the 32 (similar
>>> for x86)?
>>
>> I don't know where this ALIGN() came from, but I am hesitant to remove
>> it until I am sure it is safe to do so.
>>
>> For both x86 and arm, .initcall will take a little more work to
>> disentangle, although I do intend to make it happen.
>>
>> On arm, .init.proc.info is an array of 32byte elements. Looking at its
>> contents, it should be constant, and probably wants 4 byte alignment as
>> opposed to 32.
>
> proc_info_list contains some pointers, so we want 4-byte for ARM32 and
> 8-byte for ARM64. I would use POINTER_ALIGN here.
>
> Regards,
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] xen/init: Annotate all command line parameter infrastructure as const
2016-06-09 14:19 ` Julien Grall
2016-06-09 14:20 ` Julien Grall
@ 2016-06-09 14:28 ` Andrew Cooper
1 sibling, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2016-06-09 14:28 UTC (permalink / raw)
To: Julien Grall, Jan Beulich; +Cc: Stefano Stabellini, Xen-devel
On 09/06/16 15:19, Julien Grall wrote:
> Hi Andrew,
>
> On 09/06/16 14:45, Andrew Cooper wrote:
>> On 09/06/16 13:39, Jan Beulich wrote:
>>>>>> On 09.06.16 at 11:58, <andrew.cooper3@citrix.com> wrote:
>>>> --- a/xen/arch/arm/xen.lds.S
>>>> +++ b/xen/arch/arm/xen.lds.S
>>>> @@ -135,6 +135,12 @@ SECTIONS
>>>> *(.init.rodata)
>>>> *(.init.rodata.rel)
>>>> *(.init.rodata.str*)
>>>> +
>>>> + . = ALIGN(POINTER_ALIGN);
>>>> + __setup_start = .;
>>>> + *(.init.setup)
>>>> + __setup_end = .;
>>>> +
>>>> *(.init.data)
>>>> *(.init.data.rel)
>>>> *(.init.data.rel.*)
>>>> @@ -145,11 +151,6 @@ SECTIONS
>>>> __ctors_end = .;
>>>> } :text
>>>> . = ALIGN(32);
>>>> - .init.setup : {
>>>> - __setup_start = .;
>>>> - *(.init.setup)
>>>> - __setup_end = .;
>>>> - } :text
>>>> .init.proc.info : {
>>> Surely that ALIGN() then has no reason to retain the 32 (similar
>>> for x86)?
>>
>> I don't know where this ALIGN() came from, but I am hesitant to remove
>> it until I am sure it is safe to do so.
>>
>> For both x86 and arm, .initcall will take a little more work to
>> disentangle, although I do intend to make it happen.
>>
>> On arm, .init.proc.info is an array of 32byte elements. Looking at its
>> contents, it should be constant, and probably wants 4 byte alignment as
>> opposed to 32.
>
> proc_info_list contains some pointers, so we want 4-byte for ARM32 and
> 8-byte for ARM64. I would use POINTER_ALIGN here.
I will add a patch to my series making this change.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread