x86, trampoline_32: Put trampoline_data into readonly section.
Put trampoline_data into readonly section when cpu
hotplug is supported. We cannot free up trampoline_data
because its been referenced from tboot_setup_sleep() which
is in non-cpuinit section.
We were warned by the following warning:
WARNING: arch/x86/kernel/built-in.o(.text+0x13c77): Section mismatch
in reference from the function tboot_setup_sleep() to the variable
.cpuinit.rodata:trampoline_end
The function tboot_setup_sleep() references
the variable __cpuinitconst trampoline_end.
This is often because tboot_setup_sleep lacks a __cpuinitconst
annotation or the annotation of trampoline_end is wrong.
WARNING: arch/x86/kernel/built-in.o(.text+0x13c84): Section mismatch
in reference from the function tboot_setup_sleep() to the variable
.cpuinit.rodata:trampoline_data
The function tboot_setup_sleep() references
the variable __cpuinitconst trampoline_data.
This is often because tboot_setup_sleep lacks a __cpuinitconst
annotation or the annotation of trampoline_data is wrong.
Signed-off-by: Rakib Mullick <[email protected]>
---
--- linus/arch/x86/kernel/trampoline_32.S 2009-12-04 17:17:51.000000000 +0600
+++ rakib/arch/x86/kernel/trampoline_32.S 2009-12-04 17:24:44.000000000 +0600
@@ -32,8 +32,13 @@
#include <asm/segment.h>
#include <asm/page_types.h>
+#ifdef CONFIG_ACPI_SLEEP
+.section .rodata, "a", @progbits
+#else
/* We can free up trampoline after bootup if cpu hotplug is not supported. */
__CPUINITRODATA
+#endif
+
.code16
ENTRY(trampoline_data)
On 12/04/2009 08:24 AM, Rakib Mullick wrote:
> WARNING: arch/x86/kernel/built-in.o(.text+0x13c77): Section mismatch
> in reference from the function tboot_setup_sleep() to the variable
> .cpuinit.rodata:trampoline_end
> The function tboot_setup_sleep() references
> the variable __cpuinitconst trampoline_end.
> This is often because tboot_setup_sleep lacks a __cpuinitconst
> annotation or the annotation of trampoline_end is wrong.
>
> WARNING: arch/x86/kernel/built-in.o(.text+0x13c84): Section mismatch
> in reference from the function tboot_setup_sleep() to the variable
> .cpuinit.rodata:trampoline_data
> The function tboot_setup_sleep() references
> the variable __cpuinitconst trampoline_data.
> This is often because tboot_setup_sleep lacks a __cpuinitconst
> annotation or the annotation of trampoline_data is wrong.
>
> --- linus/arch/x86/kernel/trampoline_32.S 2009-12-04 17:17:51.000000000 +0600
> +++ rakib/arch/x86/kernel/trampoline_32.S 2009-12-04 17:24:44.000000000 +0600
> @@ -32,8 +32,13 @@
> #include <asm/segment.h>
> #include <asm/page_types.h>
>
> +#ifdef CONFIG_ACPI_SLEEP
> +.section .rodata, "a", @progbits
> +#else
> /* We can free up trampoline after bootup if cpu hotplug is not supported. */
> __CPUINITRODATA
> +#endif
> +
This is a false positive.
The reference in question is actually:
add_mac_region(virt_to_phys(trampoline_base), TRAMPOLINE_SIZE);
... which in turn is defined as ...
#define TRAMPOLINE_SIZE roundup(trampoline_end - trampoline_data, PAGE_SIZE)
This causes "references" of two symbols which aren't actually referenced
at all.
Perhaps the better thing would be to define trampoline_size as an
absolute symbol in arch/x86/kernel/trampoline_*.S, especially since it
probably generates bad code to subtract two global symbols like that.
-hpa
On 12/5/09, H. Peter Anvin <[email protected]> wrote:
> On 12/04/2009 08:24 AM, Rakib Mullick wrote:
>
> Perhaps the better thing would be to define trampoline_size as an
> absolute symbol in arch/x86/kernel/trampoline_*.S, especially since it
> probably generates bad code to subtract two global symbols like that.
>
But - trampoline_data is placed into - cpu init readonly data section.
And we are
referencing it from non-cpuinit function. ....... so I think the
problem remains.
Isn't it ? Or am I missing anything?
Rakib
>
> -hpa
>
On 12/04/2009 08:47 PM, Rakib Mullick wrote:
> On 12/5/09, H. Peter Anvin<[email protected]> wrote:
>> On 12/04/2009 08:24 AM, Rakib Mullick wrote:
>>
>> Perhaps the better thing would be to define trampoline_size as an
>> absolute symbol in arch/x86/kernel/trampoline_*.S, especially since it
>> probably generates bad code to subtract two global symbols like that.
>>
> But - trampoline_data is placed into - cpu init readonly data section.
> And we are
> referencing it from non-cpuinit function. ....... so I think the
> problem remains.
> Isn't it ? Or am I missing anything?
>
No, we're not actually referencing it at all. We're just using its
length, whereas in reality we actually have copied the code elsewhere
already.
-hpa