2010-12-14 21:19:45

by Florian Fainelli

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

From: Florian Fainelli <[email protected]>

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 <[email protected]>
Cc: [email protected]
Cc: Ingo Molnar <[email protected]>
Cc: Yinghai Lu <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
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


2010-12-23 09:58:49

by Florian Fainelli

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

On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
> From: Florian Fainelli <[email protected]>
>
> 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 <[email protected]>
> Cc: [email protected]
> Cc: Ingo Molnar <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Signed-off-by: Florian Fainelli <[email protected]>

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");

2010-12-23 13:17:41

by Yinghai Lu

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



On Dec 23, 2010, at 1:57 AM, Florian Fainelli <[email protected]> wrote:

> On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
>> From: Florian Fainelli <[email protected]>
>>
>> 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 <[email protected]>
>> Cc: [email protected]
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Yinghai Lu <[email protected]>
>> Signed-off-by: Florian Fainelli <[email protected]>
>
> 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");

2010-12-23 13:26:23

by Florian Fainelli

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


----- "Yinghai" <[email protected]> a écrit :

> On Dec 23, 2010, at 1:57 AM, Florian Fainelli <[email protected]>
> wrote:
>
> > On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
> >> From: Florian Fainelli <[email protected]>
> >>
> >> 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 <[email protected]>
> >> Cc: [email protected]
> >> Cc: Ingo Molnar <[email protected]>
> >> Cc: Yinghai Lu <[email protected]>
> >> Signed-off-by: Florian Fainelli <[email protected]>
> >
> > 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");

2010-12-23 17:48:54

by Yinghai Lu

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

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 <[email protected]> wrote:

>
> ----- "Yinghai" <[email protected]> a écrit :
>
>> On Dec 23, 2010, at 1:57 AM, Florian Fainelli <[email protected]>
>> wrote:
>>
>>> On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
>>>> From: Florian Fainelli <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: Ingo Molnar <[email protected]>
>>>> Cc: Yinghai Lu <[email protected]>
>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>
>>> 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");

2010-12-23 18:51:58

by H. Peter Anvin

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

On 12/23/2010 05:16 AM, Yinghai wrote:
>
> On Dec 23, 2010, at 1:57 AM, Florian Fainelli <[email protected]> wrote:
>
>> On Tuesday 14 December 2010 22:19:39 Florian Fainelli wrote:
>>> From: Florian Fainelli <[email protected]>
>>>
>>> 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