[PATCH] x86/trampoline: fallback to fixed trampoline address if no e820 map is provided

[PATCH] x86/trampoline: fallback to fixed trampoline address if no e820 map is provided

[Florian Fainelli]

From: Florian Fainelli <ffainelli@freebox.fr>

Since 893f38d1 (x86: Use find_e820() instead of hard coded trampoline address),
the trampoline base address should be provided by the BIOS as an e820 area.
Some minimalistic BIOSes (like CEFDK on CE4100) may not provide any e820 area,
still we should fallback to a fixed trampoline base address for SMP to work
correctly.

Cc: Dirk Brandewie <dirk.brandewie@gmail.com>
Cc: sodaville@linutronix.de
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
---
diff --git a/arch/x86/include/asm/trampoline.h b/arch/x86/include/asm/trampoline.h
index f4500fb..908b395 100644
--- a/arch/x86/include/asm/trampoline.h
+++ b/arch/x86/include/asm/trampoline.h
@@ -16,6 +16,7 @@ extern unsigned long initial_code;
 extern unsigned long initial_gs;
 
 #define TRAMPOLINE_SIZE roundup(trampoline_end - trampoline_data, PAGE_SIZE)
+#define TRAMPOLINE_BASE 0x6000
 
 extern unsigned long setup_trampoline(void);
 extern void __init reserve_trampoline_memory(void);
diff --git a/arch/x86/kernel/trampoline.c b/arch/x86/kernel/trampoline.c
index f1488a3..d025aae 100644
--- a/arch/x86/kernel/trampoline.c
+++ b/arch/x86/kernel/trampoline.c
@@ -21,8 +21,11 @@ void __init reserve_trampoline_memory(void)
 
 	/* Has to be in very low memory so we can execute real-mode AP code. */
 	mem = find_e820_area(0, 1<<20, TRAMPOLINE_SIZE, PAGE_SIZE);
-	if (mem == -1L)
-		panic("Cannot allocate trampoline\n");
+	if (mem == -1L) {
+		printk(KERN_INFO "BIOS did not provide e820 area for trampoline"
+				" ,using static trampoline address\n");
+		mem = TRAMPOLINE_BASE;
+	}
 
 	trampoline_base = __va(mem);
 	reserve_early(mem, mem + TRAMPOLINE_SIZE, "TRAMPOLINE");
-- 
1.7.1

Re: [PATCH] x86/trampoline: fallback to fixed trampoline address if no e820 map is provided

[Florian Fainelli]

On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
> From: Florian Fainelli <ffainelli@freebox.fr>
> 
> Since 893f38d1 (x86: Use find_e820() instead of hard coded trampoline
> address), the trampoline base address should be provided by the BIOS as an
> e820 area. Some minimalistic BIOSes (like CEFDK on CE4100) may not provide
> any e820 area, still we should fallback to a fixed trampoline base address
> for SMP to work correctly.
> 
> Cc: Dirk Brandewie <dirk.brandewie@gmail.com>
> Cc: sodaville@linutronix.de
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Yinghai Lu <yinghai@kernel.org>
> Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>

Ingo, Yinghai, anything wrong with that patch? Thank you.

> ---
> diff --git a/arch/x86/include/asm/trampoline.h
> b/arch/x86/include/asm/trampoline.h index f4500fb..908b395 100644
> --- a/arch/x86/include/asm/trampoline.h
> +++ b/arch/x86/include/asm/trampoline.h
> @@ -16,6 +16,7 @@ extern unsigned long initial_code;
>  extern unsigned long initial_gs;
> 
>  #define TRAMPOLINE_SIZE roundup(trampoline_end - trampoline_data,
> PAGE_SIZE) +#define TRAMPOLINE_BASE 0x6000
> 
>  extern unsigned long setup_trampoline(void);
>  extern void __init reserve_trampoline_memory(void);
> diff --git a/arch/x86/kernel/trampoline.c b/arch/x86/kernel/trampoline.c
> index f1488a3..d025aae 100644
> --- a/arch/x86/kernel/trampoline.c
> +++ b/arch/x86/kernel/trampoline.c
> @@ -21,8 +21,11 @@ void __init reserve_trampoline_memory(void)
> 
>  	/* Has to be in very low memory so we can execute real-mode AP code. */
>  	mem = find_e820_area(0, 1<<20, TRAMPOLINE_SIZE, PAGE_SIZE);
> -	if (mem == -1L)
> -		panic("Cannot allocate trampoline\n");
> +	if (mem == -1L) {
> +		printk(KERN_INFO "BIOS did not provide e820 area for trampoline"
> +				" ,using static trampoline address\n");
> +		mem = TRAMPOLINE_BASE;
> +	}
> 
>  	trampoline_base = __va(mem);
>  	reserve_early(mem, mem + TRAMPOLINE_SIZE, "TRAMPOLINE");

Re: [PATCH] x86/trampoline: fallback to fixed trampoline address if no e820 map is provided

[Yinghai]

On Dec 23, 2010, at 1:57 AM, Florian Fainelli <ffainelli@freebox.fr> wrote:

> On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
>> From: Florian Fainelli <ffainelli@freebox.fr>
>> 
>> Since 893f38d1 (x86: Use find_e820() instead of hard coded trampoline
>> address), the trampoline base address should be provided by the BIOS as an
>> e820 area. Some minimalistic BIOSes (like CEFDK on CE4100) may not provide
>> any e820 area, still we should fallback to a fixed trampoline base address
>> for SMP to work correctly.

No e820 memmap ?

Not sure how does it work

Do you have boot log with debug ?

Also recent kernel is using memblock already for finding free area.

Thanks



>> 
>> Cc: Dirk Brandewie <dirk.brandewie@gmail.com>
>> Cc: sodaville@linutronix.de
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Yinghai Lu <yinghai@kernel.org>
>> Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
> 
> Ingo, Yinghai, anything wrong with that patch? Thank you.
> 
>> ---
>> diff --git a/arch/x86/include/asm/trampoline.h
>> b/arch/x86/include/asm/trampoline.h index f4500fb..908b395 100644
>> --- a/arch/x86/include/asm/trampoline.h
>> +++ b/arch/x86/include/asm/trampoline.h
>> @@ -16,6 +16,7 @@ extern unsigned long initial_code;
>> extern unsigned long initial_gs;
>> 
>> #define TRAMPOLINE_SIZE roundup(trampoline_end - trampoline_data,
>> PAGE_SIZE) +#define TRAMPOLINE_BASE 0x6000
>> 
>> extern unsigned long setup_trampoline(void);
>> extern void __init reserve_trampoline_memory(void);
>> diff --git a/arch/x86/kernel/trampoline.c b/arch/x86/kernel/trampoline.c
>> index f1488a3..d025aae 100644
>> --- a/arch/x86/kernel/trampoline.c
>> +++ b/arch/x86/kernel/trampoline.c
>> @@ -21,8 +21,11 @@ void __init reserve_trampoline_memory(void)
>> 
>>    /* Has to be in very low memory so we can execute real-mode AP code. */
>>    mem = find_e820_area(0, 1<<20, TRAMPOLINE_SIZE, PAGE_SIZE);
>> -    if (mem == -1L)
>> -        panic("Cannot allocate trampoline\n");
>> +    if (mem == -1L) {
>> +        printk(KERN_INFO "BIOS did not provide e820 area for trampoline"
>> +                " ,using static trampoline address\n");
>> +        mem = TRAMPOLINE_BASE;
>> +    }
>> 
>>    trampoline_base = __va(mem);
>>    reserve_early(mem, mem + TRAMPOLINE_SIZE, "TRAMPOLINE");

Re: [PATCH] x86/trampoline: fallback to fixed trampoline address if no e820 map is provided

[Florian Fainelli]

----- "Yinghai" <yinghai@kernel.org> a écrit :

> On Dec 23, 2010, at 1:57 AM, Florian Fainelli <ffainelli@freebox.fr>
> wrote:
> 
> > On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
> >> From: Florian Fainelli <ffainelli@freebox.fr>
> >> 
> >> Since 893f38d1 (x86: Use find_e820() instead of hard coded
> trampoline
> >> address), the trampoline base address should be provided by the
> BIOS as an
> >> e820 area. Some minimalistic BIOSes (like CEFDK on CE4100) may not
> provide
> >> any e820 area, still we should fallback to a fixed trampoline base
> address
> >> for SMP to work correctly.
> 
> No e820 memmap ?

Yes, the BIOS is only doing the very minimal setup to get a kernel in real-mode booting.

> 
> Not sure how does it work
> 
> Do you have boot log with debug ?

I would need to produce one for you, give me a couple days to be in sync.

> 
> Also recent kernel is using memblock already for finding free area.
> 
> Thanks
> 
> 
> 
> >> 
> >> Cc: Dirk Brandewie <dirk.brandewie@gmail.com>
> >> Cc: sodaville@linutronix.de
> >> Cc: Ingo Molnar <mingo@elte.hu>
> >> Cc: Yinghai Lu <yinghai@kernel.org>
> >> Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
> > 
> > Ingo, Yinghai, anything wrong with that patch? Thank you.
> > 
> >> ---
> >> diff --git a/arch/x86/include/asm/trampoline.h
> >> b/arch/x86/include/asm/trampoline.h index f4500fb..908b395 100644
> >> --- a/arch/x86/include/asm/trampoline.h
> >> +++ b/arch/x86/include/asm/trampoline.h
> >> @@ -16,6 +16,7 @@ extern unsigned long initial_code;
> >> extern unsigned long initial_gs;
> >> 
> >> #define TRAMPOLINE_SIZE roundup(trampoline_end - trampoline_data,
> >> PAGE_SIZE) +#define TRAMPOLINE_BASE 0x6000
> >> 
> >> extern unsigned long setup_trampoline(void);
> >> extern void __init reserve_trampoline_memory(void);
> >> diff --git a/arch/x86/kernel/trampoline.c
> b/arch/x86/kernel/trampoline.c
> >> index f1488a3..d025aae 100644
> >> --- a/arch/x86/kernel/trampoline.c
> >> +++ b/arch/x86/kernel/trampoline.c
> >> @@ -21,8 +21,11 @@ void __init reserve_trampoline_memory(void)
> >> 
> >>    /* Has to be in very low memory so we can execute real-mode AP
> code. */
> >>    mem = find_e820_area(0, 1<<20, TRAMPOLINE_SIZE, PAGE_SIZE);
> >> -    if (mem == -1L)
> >> -        panic("Cannot allocate trampoline\n");
> >> +    if (mem == -1L) {
> >> +        printk(KERN_INFO "BIOS did not provide e820 area for
> trampoline"
> >> +                " ,using static trampoline address\n");
> >> +        mem = TRAMPOLINE_BASE;
> >> +    }
> >> 
> >>    trampoline_base = __va(mem);
> >>    reserve_early(mem, mem + TRAMPOLINE_SIZE, "TRAMPOLINE");

Re: [PATCH] x86/trampoline: fallback to fixed trampoline address if no e820 map is provided

[Yinghai]

Can you check if you can add ce4100 own memory_setup() and install it into x86_init.resources.memory_setup in x86_ce4100_early_setup?

Thanks

On Dec 23, 2010, at 5:20 AM, Florian Fainelli <ffainelli@freebox.fr> wrote:

> 
> ----- "Yinghai" <yinghai@kernel.org> a écrit :
> 
>> On Dec 23, 2010, at 1:57 AM, Florian Fainelli <ffainelli@freebox.fr>
>> wrote:
>> 
>>> On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
>>>> From: Florian Fainelli <ffainelli@freebox.fr>
>>>> 
>>>> Since 893f38d1 (x86: Use find_e820() instead of hard coded
>> trampoline
>>>> address), the trampoline base address should be provided by the
>> BIOS as an
>>>> e820 area. Some minimalistic BIOSes (like CEFDK on CE4100) may not
>> provide
>>>> any e820 area, still we should fallback to a fixed trampoline base
>> address
>>>> for SMP to work correctly.
>> 
>> No e820 memmap ?
> 
> Yes, the BIOS is only doing the very minimal setup to get a kernel in real-mode booting.
> 
>> 
>> Not sure how does it work
>> 
>> Do you have boot log with debug ?
> 
> I would need to produce one for you, give me a couple days to be in sync.
> 
>> 
>> Also recent kernel is using memblock already for finding free area.
>> 
>> Thanks
>> 
>> 
>> 
>>>> 
>>>> Cc: Dirk Brandewie <dirk.brandewie@gmail.com>
>>>> Cc: sodaville@linutronix.de
>>>> Cc: Ingo Molnar <mingo@elte.hu>
>>>> Cc: Yinghai Lu <yinghai@kernel.org>
>>>> Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
>>> 
>>> Ingo, Yinghai, anything wrong with that patch? Thank you.
>>> 
>>>> ---
>>>> diff --git a/arch/x86/include/asm/trampoline.h
>>>> b/arch/x86/include/asm/trampoline.h index f4500fb..908b395 100644
>>>> --- a/arch/x86/include/asm/trampoline.h
>>>> +++ b/arch/x86/include/asm/trampoline.h
>>>> @@ -16,6 +16,7 @@ extern unsigned long initial_code;
>>>> extern unsigned long initial_gs;
>>>> 
>>>> #define TRAMPOLINE_SIZE roundup(trampoline_end - trampoline_data,
>>>> PAGE_SIZE) +#define TRAMPOLINE_BASE 0x6000
>>>> 
>>>> extern unsigned long setup_trampoline(void);
>>>> extern void __init reserve_trampoline_memory(void);
>>>> diff --git a/arch/x86/kernel/trampoline.c
>> b/arch/x86/kernel/trampoline.c
>>>> index f1488a3..d025aae 100644
>>>> --- a/arch/x86/kernel/trampoline.c
>>>> +++ b/arch/x86/kernel/trampoline.c
>>>> @@ -21,8 +21,11 @@ void __init reserve_trampoline_memory(void)
>>>> 
>>>>   /* Has to be in very low memory so we can execute real-mode AP
>> code. */
>>>>   mem = find_e820_area(0, 1<<20, TRAMPOLINE_SIZE, PAGE_SIZE);
>>>> -    if (mem == -1L)
>>>> -        panic("Cannot allocate trampoline\n");
>>>> +    if (mem == -1L) {
>>>> +        printk(KERN_INFO "BIOS did not provide e820 area for
>> trampoline"
>>>> +                " ,using static trampoline address\n");
>>>> +        mem = TRAMPOLINE_BASE;
>>>> +    }
>>>> 
>>>>   trampoline_base = __va(mem);
>>>>   reserve_early(mem, mem + TRAMPOLINE_SIZE, "TRAMPOLINE");

Re: [sodaville] [PATCH] x86/trampoline: fallback to fixed trampoline address if no e820 map is provided

[H. Peter Anvin]

On 12/23/2010 05:16 AM, Yinghai wrote:
> 
> On Dec 23, 2010, at 1:57 AM, Florian Fainelli <ffainelli@freebox.fr> wrote:
> 
>> On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
>>> From: Florian Fainelli <ffainelli@freebox.fr>
>>>
>>> Since 893f38d1 (x86: Use find_e820() instead of hard coded trampoline
>>> address), the trampoline base address should be provided by the BIOS as an
>>> e820 area. Some minimalistic BIOSes (like CEFDK on CE4100) may not provide
>>> any e820 area, still we should fallback to a fixed trampoline base address
>>> for SMP to work correctly.
> 
> No e820 memmap ?
> 
> Not sure how does it work
> 
> Do you have boot log with debug ?
> 
> Also recent kernel is using memblock already for finding free area.
> 

OK, this deeply troubles me.  This is clearly not the right approach
here... the memory map should be fed to the kernel, this has been a
requirement all along, and Linus has very deliberately not agreed to
exceptions (and I agree with that stance.)

So this patch is a non-starter... I presume the memory map is being
provided in through the device (which means it should be fed into
memblock in current kernels, although the ordering may need to be tweaked?)

	-hpa