LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* randconfig bug: ARM/KVM link error in hyp_idmap section
@ 2015-01-28 19:57 Arnd Bergmann
       [not found] ` <CAEDV+gJ3YO8oeka_yaJGg+LXA0Od0TGkZb8vD_q74xNRx9g0Rg@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2015-01-28 19:57 UTC (permalink / raw)
  To: Marc Zyngier, Christoffer Dall
  Cc: Russell King, linux-kernel, kvm, linux-arm-kernel

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) {


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

* Re: randconfig bug: ARM/KVM link error in hyp_idmap section
       [not found] ` <CAEDV+gJ3YO8oeka_yaJGg+LXA0Od0TGkZb8vD_q74xNRx9g0Rg@mail.gmail.com>
@ 2015-01-29 15:53   ` Arnd Bergmann
  2015-01-29 16:01     ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2015-01-29 15:53 UTC (permalink / raw)
  To: Christoffer Dall
  Cc: Marc Zyngier, Russell King, linux-kernel, KVM General, linux-arm-kernel

[-- 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 --]

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

* Re: randconfig bug: ARM/KVM link error in hyp_idmap section
  2015-01-29 15:53   ` Arnd Bergmann
@ 2015-01-29 16:01     ` Marc Zyngier
       [not found]       ` <CAEDV+gKzkER4n=oNiM1wNmNnbMEosYKo=Awu8j8R5Fr7kzE_Ew@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2015-01-29 16:01 UTC (permalink / raw)
  To: Arnd Bergmann, Christoffer Dall
  Cc: Russell King, linux-kernel, KVM General, linux-arm-kernel

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...

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

* Re: randconfig bug: ARM/KVM link error in hyp_idmap section
       [not found]       ` <CAEDV+gKzkER4n=oNiM1wNmNnbMEosYKo=Awu8j8R5Fr7kzE_Ew@mail.gmail.com>
@ 2015-01-29 17:51         ` Marc Zyngier
  0 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2015-01-29 17:51 UTC (permalink / raw)
  To: Christoffer Dall
  Cc: Arnd Bergmann, Russell King, linux-kernel, KVM General, linux-arm-kernel

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...

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

end of thread, other threads:[~2015-01-29 17:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-28 19:57 randconfig bug: ARM/KVM link error in hyp_idmap section Arnd Bergmann
     [not found] ` <CAEDV+gJ3YO8oeka_yaJGg+LXA0Od0TGkZb8vD_q74xNRx9g0Rg@mail.gmail.com>
2015-01-29 15:53   ` Arnd Bergmann
2015-01-29 16:01     ` Marc Zyngier
     [not found]       ` <CAEDV+gKzkER4n=oNiM1wNmNnbMEosYKo=Awu8j8R5Fr7kzE_Ew@mail.gmail.com>
2015-01-29 17:51         ` Marc Zyngier

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