randconfig bug: ARM/KVM link error in hyp_idmap section

randconfig bug: ARM/KVM link error in hyp_idmap section

[Arnd Bergmann]

Since 0394e1f60520 ("ARM: KVM: enforce maximum size for identity
mapped code"), some randconfigs started failing because the hyp_idmap
section grows too large. I've tracked this down to the problem of
veneers getting erroneously added to this section, but I'm not
sure about the fix. The patch below is what I came up with.

Any other ideas?

	Arnd

8<----
Subject: [PATCH] ARM: KVM: avoid "HYP init code too big" error

When building large kernels, the linker will emit lots of veneers
into the .hyp.idmap.text section, which causes it to grow beyond
one page, and that triggers the build error.

This moves the section into .rodata instead, which avoids the
veneers and is safe because the code is not executed directly
but always copied into a separate page first.

I am unsure if I wrote this the correct way though, so
it needs to be reviewed carefully.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index ce01a2d3339f..f4de6e16d951 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -23,10 +23,14 @@
 	VMLINUX_SYMBOL(__idmap_text_start) = .;				\
 	*(.idmap.text)							\
 	VMLINUX_SYMBOL(__idmap_text_end) = .;				\
-	. = ALIGN(32);							\
+	. = ALIGN(32);							
+
+#define IDMAP_RODATA							\
+	.rodata : {							\
 	VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;			\
 	*(.hyp.idmap.text)						\
-	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
+	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;			\
+	}
 
 #ifdef CONFIG_HOTPLUG_CPU
 #define ARM_CPU_DISCARD(x)
@@ -123,6 +127,7 @@ SECTIONS
 	. = ALIGN(1<<SECTION_SHIFT);
 #endif
 	RO_DATA(PAGE_SIZE)
+	IDMAP_RODATA
 
 	. = ALIGN(4);
 	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {

randconfig bug: ARM/KVM link error in hyp_idmap section

[Arnd Bergmann]

Since 0394e1f60520 ("ARM: KVM: enforce maximum size for identity
mapped code"), some randconfigs started failing because the hyp_idmap
section grows too large. I've tracked this down to the problem of
veneers getting erroneously added to this section, but I'm not
sure about the fix. The patch below is what I came up with.

Any other ideas?

	Arnd

8<----
Subject: [PATCH] ARM: KVM: avoid "HYP init code too big" error

When building large kernels, the linker will emit lots of veneers
into the .hyp.idmap.text section, which causes it to grow beyond
one page, and that triggers the build error.

This moves the section into .rodata instead, which avoids the
veneers and is safe because the code is not executed directly
but always copied into a separate page first.

I am unsure if I wrote this the correct way though, so
it needs to be reviewed carefully.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index ce01a2d3339f..f4de6e16d951 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -23,10 +23,14 @@
 	VMLINUX_SYMBOL(__idmap_text_start) = .;				\
 	*(.idmap.text)							\
 	VMLINUX_SYMBOL(__idmap_text_end) = .;				\
-	. = ALIGN(32);							\
+	. = ALIGN(32);							
+
+#define IDMAP_RODATA							\
+	.rodata : {							\
 	VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;			\
 	*(.hyp.idmap.text)						\
-	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
+	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;			\
+	}
 
 #ifdef CONFIG_HOTPLUG_CPU
 #define ARM_CPU_DISCARD(x)
@@ -123,6 +127,7 @@ SECTIONS
 	. = ALIGN(1<<SECTION_SHIFT);
 #endif
 	RO_DATA(PAGE_SIZE)
+	IDMAP_RODATA
 
 	. = ALIGN(4);
 	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {

randconfig bug: ARM/KVM link error in hyp_idmap section

[Arnd Bergmann]

Since 0394e1f60520 ("ARM: KVM: enforce maximum size for identity
mapped code"), some randconfigs started failing because the hyp_idmap
section grows too large. I've tracked this down to the problem of
veneers getting erroneously added to this section, but I'm not
sure about the fix. The patch below is what I came up with.

Any other ideas?

	Arnd

8<----
Subject: [PATCH] ARM: KVM: avoid "HYP init code too big" error

When building large kernels, the linker will emit lots of veneers
into the .hyp.idmap.text section, which causes it to grow beyond
one page, and that triggers the build error.

This moves the section into .rodata instead, which avoids the
veneers and is safe because the code is not executed directly
but always copied into a separate page first.

I am unsure if I wrote this the correct way though, so
it needs to be reviewed carefully.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index ce01a2d3339f..f4de6e16d951 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -23,10 +23,14 @@
 	VMLINUX_SYMBOL(__idmap_text_start) = .;				\
 	*(.idmap.text)							\
 	VMLINUX_SYMBOL(__idmap_text_end) = .;				\
-	. = ALIGN(32);							\
+	. = ALIGN(32);							
+
+#define IDMAP_RODATA							\
+	.rodata : {							\
 	VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;			\
 	*(.hyp.idmap.text)						\
-	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
+	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;			\
+	}
 
 #ifdef CONFIG_HOTPLUG_CPU
 #define ARM_CPU_DISCARD(x)
@@ -123,6 +127,7 @@ SECTIONS
 	. = ALIGN(1<<SECTION_SHIFT);
 #endif
 	RO_DATA(PAGE_SIZE)
+	IDMAP_RODATA
 
 	. = ALIGN(4);
 	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {

Re: randconfig bug: ARM/KVM link error in hyp_idmap section

[Arnd Bergmann]

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

On Thursday 29 January 2015 16:23:42 Christoffer Dall wrote:
> On Wed, Jan 28, 2015 at 8:57 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> > 8<----
> > Subject: [PATCH] ARM: KVM: avoid "HYP init code too big" error
> >
> > When building large kernels, the linker will emit lots of veneers
> > into the .hyp.idmap.text section, which causes it to grow beyond
> > one page, and that triggers the build error.
> >
> 
> do you have a config handy that generates this error?

Attached to this mail. Use on linux-next

> > This moves the section into .rodata instead, which avoids the
> > veneers and is safe because the code is not executed directly
> > but always copied into a separate page first.
> >
> > I am unsure if I wrote this the correct way though, so
> > it needs to be reviewed carefully.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >
> > diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> > index ce01a2d3339f..f4de6e16d951 100644
> > --- a/arch/arm/kernel/vmlinux.lds.S
> > +++ b/arch/arm/kernel/vmlinux.lds.S
> > @@ -23,10 +23,14 @@
> >         VMLINUX_SYMBOL(__idmap_text_start) = .;                         \
> >         *(.idmap.text)                                                  \
> >         VMLINUX_SYMBOL(__idmap_text_end) = .;                           \
> > -       . = ALIGN(32);                                                  \
> > +       . = ALIGN(32);
> > +
> > +#define IDMAP_RODATA                                                   \
> > +       .rodata : {                                                     \
> >         VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;                     \
> >         *(.hyp.idmap.text)                                              \
> > -       VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
> > +       VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;                       \
> > +       }
> >
> >  #ifdef CONFIG_HOTPLUG_CPU
> >  #define ARM_CPU_DISCARD(x)
> > @@ -123,6 +127,7 @@ SECTIONS
> >         . = ALIGN(1<<SECTION_SHIFT);
> >  #endif
> >         RO_DATA(PAGE_SIZE)
> > +       IDMAP_RODATA
> >
> >         . = ALIGN(4);
> >         __ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
> >
> >
> the changes look ok, but I don't understand why putting stuff in rodata is
> a good solution, is it simply by chance that the linker then generates
> fewer veneers there?  I think we're only branching internally in the hyp
> idmap text page anyhow, so wondering why this appears in the first place...
> hmmm.

The linker will not generate any veneers for .rodata because it does not
expect executable code in there. As I said, above, this is also correct
because it matches how we access that section (read-only, never execute).

	Arnd

[-- Attachment #2: 0x203B430A-config.gz --]
[-- Type: application/gzip, Size: 26652 bytes --]

randconfig bug: ARM/KVM link error in hyp_idmap section

[Arnd Bergmann]

On Thursday 29 January 2015 16:23:42 Christoffer Dall wrote:
> On Wed, Jan 28, 2015 at 8:57 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> > 8<----
> > Subject: [PATCH] ARM: KVM: avoid "HYP init code too big" error
> >
> > When building large kernels, the linker will emit lots of veneers
> > into the .hyp.idmap.text section, which causes it to grow beyond
> > one page, and that triggers the build error.
> >
> 
> do you have a config handy that generates this error?

Attached to this mail. Use on linux-next

> > This moves the section into .rodata instead, which avoids the
> > veneers and is safe because the code is not executed directly
> > but always copied into a separate page first.
> >
> > I am unsure if I wrote this the correct way though, so
> > it needs to be reviewed carefully.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >
> > diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> > index ce01a2d3339f..f4de6e16d951 100644
> > --- a/arch/arm/kernel/vmlinux.lds.S
> > +++ b/arch/arm/kernel/vmlinux.lds.S
> > @@ -23,10 +23,14 @@
> >         VMLINUX_SYMBOL(__idmap_text_start) = .;                         \
> >         *(.idmap.text)                                                  \
> >         VMLINUX_SYMBOL(__idmap_text_end) = .;                           \
> > -       . = ALIGN(32);                                                  \
> > +       . = ALIGN(32);
> > +
> > +#define IDMAP_RODATA                                                   \
> > +       .rodata : {                                                     \
> >         VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;                     \
> >         *(.hyp.idmap.text)                                              \
> > -       VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
> > +       VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;                       \
> > +       }
> >
> >  #ifdef CONFIG_HOTPLUG_CPU
> >  #define ARM_CPU_DISCARD(x)
> > @@ -123,6 +127,7 @@ SECTIONS
> >         . = ALIGN(1<<SECTION_SHIFT);
> >  #endif
> >         RO_DATA(PAGE_SIZE)
> > +       IDMAP_RODATA
> >
> >         . = ALIGN(4);
> >         __ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
> >
> >
> the changes look ok, but I don't understand why putting stuff in rodata is
> a good solution, is it simply by chance that the linker then generates
> fewer veneers there?  I think we're only branching internally in the hyp
> idmap text page anyhow, so wondering why this appears in the first place...
> hmmm.

The linker will not generate any veneers for .rodata because it does not
expect executable code in there. As I said, above, this is also correct
because it matches how we access that section (read-only, never execute).

	Arnd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0x203B430A-config.gz
Type: application/gzip
Size: 26652 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150129/e2aa6fcd/attachment-0001.gz>

Re: randconfig bug: ARM/KVM link error in hyp_idmap section

[Marc Zyngier]

Hi Arnd,

On 29/01/15 15:53, Arnd Bergmann wrote:
> On Thursday 29 January 2015 16:23:42 Christoffer Dall wrote:
>> On Wed, Jan 28, 2015 at 8:57 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>>> 8<----
>>> Subject: [PATCH] ARM: KVM: avoid "HYP init code too big" error
>>>
>>> When building large kernels, the linker will emit lots of veneers
>>> into the .hyp.idmap.text section, which causes it to grow beyond
>>> one page, and that triggers the build error.
>>>
>>
>> do you have a config handy that generates this error?
> 
> Attached to this mail. Use on linux-next
> 
>>> This moves the section into .rodata instead, which avoids the
>>> veneers and is safe because the code is not executed directly
>>> but always copied into a separate page first.
>>>
>>> I am unsure if I wrote this the correct way though, so
>>> it needs to be reviewed carefully.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>
>>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>>> index ce01a2d3339f..f4de6e16d951 100644
>>> --- a/arch/arm/kernel/vmlinux.lds.S
>>> +++ b/arch/arm/kernel/vmlinux.lds.S
>>> @@ -23,10 +23,14 @@
>>>         VMLINUX_SYMBOL(__idmap_text_start) = .;                         \
>>>         *(.idmap.text)                                                  \
>>>         VMLINUX_SYMBOL(__idmap_text_end) = .;                           \
>>> -       . = ALIGN(32);                                                  \
>>> +       . = ALIGN(32);
>>> +
>>> +#define IDMAP_RODATA                                                   \
>>> +       .rodata : {                                                     \
>>>         VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;                     \
>>>         *(.hyp.idmap.text)                                              \
>>> -       VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
>>> +       VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;                       \
>>> +       }
>>>
>>>  #ifdef CONFIG_HOTPLUG_CPU
>>>  #define ARM_CPU_DISCARD(x)
>>> @@ -123,6 +127,7 @@ SECTIONS
>>>         . = ALIGN(1<<SECTION_SHIFT);
>>>  #endif
>>>         RO_DATA(PAGE_SIZE)
>>> +       IDMAP_RODATA
>>>
>>>         . = ALIGN(4);
>>>         __ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
>>>
>>>
>> the changes look ok, but I don't understand why putting stuff in rodata is
>> a good solution, is it simply by chance that the linker then generates
>> fewer veneers there?  I think we're only branching internally in the hyp
>> idmap text page anyhow, so wondering why this appears in the first place...
>> hmmm.
> 
> The linker will not generate any veneers for .rodata because it does not
> expect executable code in there. As I said, above, this is also correct
> because it matches how we access that section (read-only, never execute).

Not sure about the later point. We only copy the code if it is not page
aligned, and use it in place otherwise. I guess we could change that,
but we'd need the same change for arm64.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

randconfig bug: ARM/KVM link error in hyp_idmap section

[Marc Zyngier]

Hi Arnd,

On 29/01/15 15:53, Arnd Bergmann wrote:
> On Thursday 29 January 2015 16:23:42 Christoffer Dall wrote:
>> On Wed, Jan 28, 2015 at 8:57 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>>> 8<----
>>> Subject: [PATCH] ARM: KVM: avoid "HYP init code too big" error
>>>
>>> When building large kernels, the linker will emit lots of veneers
>>> into the .hyp.idmap.text section, which causes it to grow beyond
>>> one page, and that triggers the build error.
>>>
>>
>> do you have a config handy that generates this error?
> 
> Attached to this mail. Use on linux-next
> 
>>> This moves the section into .rodata instead, which avoids the
>>> veneers and is safe because the code is not executed directly
>>> but always copied into a separate page first.
>>>
>>> I am unsure if I wrote this the correct way though, so
>>> it needs to be reviewed carefully.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>
>>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>>> index ce01a2d3339f..f4de6e16d951 100644
>>> --- a/arch/arm/kernel/vmlinux.lds.S
>>> +++ b/arch/arm/kernel/vmlinux.lds.S
>>> @@ -23,10 +23,14 @@
>>>         VMLINUX_SYMBOL(__idmap_text_start) = .;                         \
>>>         *(.idmap.text)                                                  \
>>>         VMLINUX_SYMBOL(__idmap_text_end) = .;                           \
>>> -       . = ALIGN(32);                                                  \
>>> +       . = ALIGN(32);
>>> +
>>> +#define IDMAP_RODATA                                                   \
>>> +       .rodata : {                                                     \
>>>         VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;                     \
>>>         *(.hyp.idmap.text)                                              \
>>> -       VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
>>> +       VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;                       \
>>> +       }
>>>
>>>  #ifdef CONFIG_HOTPLUG_CPU
>>>  #define ARM_CPU_DISCARD(x)
>>> @@ -123,6 +127,7 @@ SECTIONS
>>>         . = ALIGN(1<<SECTION_SHIFT);
>>>  #endif
>>>         RO_DATA(PAGE_SIZE)
>>> +       IDMAP_RODATA
>>>
>>>         . = ALIGN(4);
>>>         __ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
>>>
>>>
>> the changes look ok, but I don't understand why putting stuff in rodata is
>> a good solution, is it simply by chance that the linker then generates
>> fewer veneers there?  I think we're only branching internally in the hyp
>> idmap text page anyhow, so wondering why this appears in the first place...
>> hmmm.
> 
> The linker will not generate any veneers for .rodata because it does not
> expect executable code in there. As I said, above, this is also correct
> because it matches how we access that section (read-only, never execute).

Not sure about the later point. We only copy the code if it is not page
aligned, and use it in place otherwise. I guess we could change that,
but we'd need the same change for arm64.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: randconfig bug: ARM/KVM link error in hyp_idmap section

[Marc Zyngier]

On 29/01/15 17:40, Christoffer Dall wrote:
> 
> 
> On Thu, Jan 29, 2015 at 5:01 PM, Marc Zyngier <marc.zyngier@arm.com
> <mailto:marc.zyngier@arm.com>> wrote:
> 
>     Hi Arnd,
> 
>     On 29/01/15 15:53, Arnd Bergmann wrote:
>     > On Thursday 29 January 2015 16:23:42 Christoffer Dall wrote:
>     >> the changes look ok, but I don't understand why putting stuff in
>     rodata is
>     >> a good solution, is it simply by chance that the linker then
>     generates
>     >> fewer veneers there?  I think we're only branching internally in
>     the hyp
>     >> idmap text page anyhow, so wondering why this appears in the
>     first place...
>     >> hmmm.
>     >
>     > The linker will not generate any veneers for .rodata because it
>     does not
>     > expect executable code in there. As I said, above, this is also
>     correct
>     > because it matches how we access that section (read-only, never
>     execute).
> 
>     Not sure about the later point. We only copy the code if it is not page
>     aligned, and use it in place otherwise. I guess we could change that,
>     but we'd need the same change for arm64.
> 
> 
> I'd be ok with changing that... 

In which case I have no further objection.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

randconfig bug: ARM/KVM link error in hyp_idmap section

[Marc Zyngier]

On 29/01/15 17:40, Christoffer Dall wrote:
> 
> 
> On Thu, Jan 29, 2015 at 5:01 PM, Marc Zyngier <marc.zyngier@arm.com
> <mailto:marc.zyngier@arm.com>> wrote:
> 
>     Hi Arnd,
> 
>     On 29/01/15 15:53, Arnd Bergmann wrote:
>     > On Thursday 29 January 2015 16:23:42 Christoffer Dall wrote:
>     >> the changes look ok, but I don't understand why putting stuff in
>     rodata is
>     >> a good solution, is it simply by chance that the linker then
>     generates
>     >> fewer veneers there?  I think we're only branching internally in
>     the hyp
>     >> idmap text page anyhow, so wondering why this appears in the
>     first place...
>     >> hmmm.
>     >
>     > The linker will not generate any veneers for .rodata because it
>     does not
>     > expect executable code in there. As I said, above, this is also
>     correct
>     > because it matches how we access that section (read-only, never
>     execute).
> 
>     Not sure about the later point. We only copy the code if it is not page
>     aligned, and use it in place otherwise. I guess we could change that,
>     but we'd need the same change for arm64.
> 
> 
> I'd be ok with changing that... 

In which case I have no further objection.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...