Re: [PATCH] fix DMI for EFI

Re: [PATCH] fix DMI for EFI

[Brian Maly]

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

Updated patch. Might be good for someone who runs VMware to verify things still work though.
Looks right now I think.


Brian


>This init_hypervisor comment looks important, no? Possibly it needs
>to be moved after the new placement? Doesn't look like the probe_roms
>will be an issue.

>regards, Kyle


[-- Attachment #2: fix-DMI-for-EFI-2.6.29.patch --]
[-- Type: text/x-patch, Size: 941 bytes --]

--- a/arch/x86/kernel/setup.c	2009-02-08 15:37:27.000000000 -0500
+++ b/arch/x86/kernel/setup.c	2009-03-01 02:27:45.000000000 -0500
@@ -770,16 +770,6 @@ void __init setup_arch(char **cmdline_p)
 
 	finish_e820_parsing();
 
-	dmi_scan_machine();
-
-	dmi_check_system(bad_bios_dmi_table);
-
-	/*
-	 * VMware detection requires dmi to be available, so this
-	 * needs to be done after dmi_scan_machine, for the BP.
-	 */
-	init_hypervisor(&boot_cpu_data);
-
 #ifdef CONFIG_X86_32
 	probe_roms();
 #endif
@@ -792,6 +782,16 @@ void __init setup_arch(char **cmdline_p)
 	if (efi_enabled)
 		efi_init();
 
+	dmi_scan_machine();
+	
+	dmi_check_system(bad_bios_dmi_table);
+
+	/*
+	 * VMware detection requires dmi to be available, so this
+	 * needs to be done after dmi_scan_machine, for the BP.
+	 */
+	init_hypervisor(&boot_cpu_data);
+
 #ifdef CONFIG_X86_32
 	if (ppro_with_ram_bug()) {
 		e820_update_range(0x70000000ULL, 0x40000ULL, E820_RAM,

Re: [PATCH] fix DMI for EFI

[Yinghai Lu]

it will break dmi check for 64k ram corruption...


YH

Re: [PATCH] fix DMI for EFI

[Brian Maly]

probe_roms() needs to be after dmi?

Brian


Yinghai Lu wrote:
> it will break dmi check for 64k ram corruption...
>
>
> YH
>   

Re: [PATCH] fix DMI for EFI

[Yinghai Lu]

please use git log -p arch/x86/kernel/setup.c to check the history...

YH


commit 2216d199b1430d1c0affb1498a9ebdbd9c0de439
Author: Yinghai Lu <yhlu.kernel@gmail.com>
Date:   Mon Sep 22 02:52:26 2008 -0700

    x86: fix CONFIG_X86_RESERVE_LOW_64K=y

    The bad_bios_dmi_table() quirk never triggered because we do DMI setup
    too late. Move it a bit earlier.

    Also change the CONFIG_X86_RESERVE_LOW_64K quirk to operate on the e820
    table directly instead of messing with early reservations - this handles
    overlaps (which do occur in this low range of RAM) more gracefully.

    Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 161f1b3..d29951c 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -735,7 +735,8 @@ static int __init dmi_low_memory_corruption(const
struct dmi_system_id *d)
                "%s detected: BIOS may corrupt low RAM, working it around.\n",
                d->ident);

-       reserve_early_overlap_ok(0x0, 0x10000, "BIOS quirk");
+       e820_update_range(0, 0x10000, E820_RAM, E820_RESERVED);
+       sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);

        return 0;
 }
@@ -784,8 +785,6 @@ void __init setup_arch(char **cmdline_p)
        printk(KERN_INFO "Command line: %s\n", boot_command_line);
 #endif

-       dmi_check_system(bad_bios_dmi_table);
-
        early_cpu_init();
        early_ioremap_init();

@@ -880,6 +879,10 @@ void __init setup_arch(char **cmdline_p)

        finish_e820_parsing();

+       dmi_scan_machine();
+
+       dmi_check_system(bad_bios_dmi_table);
+
 #ifdef CONFIG_X86_32
        probe_roms();
 #endif
@@ -967,8 +970,6 @@ void __init setup_arch(char **cmdline_p)
        vsmp_init();
 #endif

-       dmi_scan_machine();
-
        io_delay_init();

Re: [PATCH] fix DMI for EFI

[Brian Maly]

Thanks. At this point maybe its easier just to move efi_init instead. 
Ill re-work, test and post a followup...

Brian


Yinghai Lu wrote:
> please use git log -p arch/x86/kernel/setup.c to check the history...
>
> YH
>
>
> commit 2216d199b1430d1c0affb1498a9ebdbd9c0de439
> Author: Yinghai Lu <yhlu.kernel@gmail.com>
> Date:   Mon Sep 22 02:52:26 2008 -0700
>
>     x86: fix CONFIG_X86_RESERVE_LOW_64K=y
>
>     The bad_bios_dmi_table() quirk never triggered because we do DMI setup
>     too late. Move it a bit earlier.
>
>     Also change the CONFIG_X86_RESERVE_LOW_64K quirk to operate on the e820
>     table directly instead of messing with early reservations - this handles
>     overlaps (which do occur in this low range of RAM) more gracefully.
>
>     Signed-off-by: Ingo Molnar <mingo@elte.hu>
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 161f1b3..d29951c 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -735,7 +735,8 @@ static int __init dmi_low_memory_corruption(const
> struct dmi_system_id *d)
>                 "%s detected: BIOS may corrupt low RAM, working it around.\n",
>                 d->ident);
>
> -       reserve_early_overlap_ok(0x0, 0x10000, "BIOS quirk");
> +       e820_update_range(0, 0x10000, E820_RAM, E820_RESERVED);
> +       sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
>
>         return 0;
>  }
> @@ -784,8 +785,6 @@ void __init setup_arch(char **cmdline_p)
>         printk(KERN_INFO "Command line: %s\n", boot_command_line);
>  #endif
>
> -       dmi_check_system(bad_bios_dmi_table);
> -
>         early_cpu_init();
>         early_ioremap_init();
>
> @@ -880,6 +879,10 @@ void __init setup_arch(char **cmdline_p)
>
>         finish_e820_parsing();
>
> +       dmi_scan_machine();
> +
> +       dmi_check_system(bad_bios_dmi_table);
> +
>  #ifdef CONFIG_X86_32
>         probe_roms();
>  #endif
> @@ -967,8 +970,6 @@ void __init setup_arch(char **cmdline_p)
>         vsmp_init();
>  #endif
>
> -       dmi_scan_machine();
> -
>         io_delay_init();
>   

Re: [PATCH] fix DMI for EFI

[Brian Maly]

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

New patch. Its easier I think to just move efi_init() earlier. Seems 
less risky and less drawbacks.

Signed-off-by: Brian Maly <bmaly@redhat>


 setup.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)



[-- Attachment #2: fix-DMI-for-EFI-2.6.29.patch --]
[-- Type: text/x-patch, Size: 575 bytes --]

--- a/arch/x86/kernel/setup.c	2009-03-03 21:24:53.000000000 -0500
+++ b/arch/x86/kernel/setup.c	2009-03-03 21:26:00.000000000 -0500
@@ -770,6 +770,9 @@ void __init setup_arch(char **cmdline_p)
 
 	finish_e820_parsing();
 
+	if (efi_enabled)
+		efi_init();
+
 	dmi_scan_machine();
 
 	dmi_check_system(bad_bios_dmi_table);
@@ -789,8 +792,6 @@ void __init setup_arch(char **cmdline_p)
 	insert_resource(&iomem_resource, &data_resource);
 	insert_resource(&iomem_resource, &bss_resource);
 
-	if (efi_enabled)
-		efi_init();
 
 #ifdef CONFIG_X86_32
 	if (ppro_with_ram_bug()) {

Re: [PATCH] fix DMI for EFI

[Ingo Molnar]

* Brian Maly <bmaly@redhat.com> wrote:

> New patch. Its easier I think to just move efi_init() earlier. Seems  
> less risky and less drawbacks.
>
> Signed-off-by: Brian Maly <bmaly@redhat>
>
>
> setup.c |    5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>

> --- a/arch/x86/kernel/setup.c	2009-03-03 21:24:53.000000000 -0500
> +++ b/arch/x86/kernel/setup.c	2009-03-03 21:26:00.000000000 -0500
> @@ -770,6 +770,9 @@ void __init setup_arch(char **cmdline_p)
>  
>  	finish_e820_parsing();
>  
> +	if (efi_enabled)
> +		efi_init();
> +
>  	dmi_scan_machine();

Hm, could we ever find ourselves in the positition of having to 
access the DMI strings table in the EFI init code, to implement 
a quirk? I think that's quite plausible.

OTOH, with some DMI quirks in EFI tables it's a bit of a chicken 
and egg problem. Can DMI strings ever be outside of EFI tables 
on EFI systems?

	Ingo

Re: [PATCH] fix DMI for EFI

[Brian Maly]

>Hm, could we ever find ourselves in the positition of having to 
>access the DMI strings table in the EFI init code, to implement 
>a quirk? I think that's quite plausible.

>OTOH, with some DMI quirks in EFI tables it's a bit of a chicken 
>and egg problem. Can DMI strings ever be outside of EFI tables 
>on EFI systems?

>	Ingo

Right now it is EFI that loads the dmi tables (and all other tables such 
as ACPI), so it is a chicken and the egg. I think that if dmi is ever 
needed by EFI, the dmi scan will have to be moved inside efi_init. We 
could move the dmi scan into efi_init and do the dmi scan the moment we 
have the dmi table. Thats really the soonest point we could scan on EFI 
systems. Im OK with moving dmi to efi_init if you prefer.

Brian

Re: [PATCH] fix DMI for EFI

[Ingo Molnar]

* Brian Maly <bmaly@redhat.com> wrote:

>
>
>> Hm, could we ever find ourselves in the positition of having to access 
>> the DMI strings table in the EFI init code, to implement a quirk? I 
>> think that's quite plausible.
>
>> OTOH, with some DMI quirks in EFI tables it's a bit of a chicken and 
>> egg problem. Can DMI strings ever be outside of EFI tables on EFI 
>> systems?
>
>> 	Ingo
>
> Right now it is EFI that loads the dmi tables (and all other 
> tables such as ACPI), so it is a chicken and the egg. I think 
> that if dmi is ever needed by EFI, the dmi scan will have to 
> be moved inside efi_init. We could move the dmi scan into 
> efi_init and do the dmi scan the moment we have the dmi table. 
> Thats really the soonest point we could scan on EFI systems. 
> Im OK with moving dmi to efi_init if you prefer.

Your patch looks fine, but i'd like to hear the opinion of Ying 
Huang as well.

	Ingo

Re: [PATCH] fix DMI for EFI

[Brian Maly]

>Your patch looks fine, but i'd like to hear the opinion of Ying 
>Huang as well.

	
I had asked him to look at this before I posted and he had no objections. He may have an opinion on moving dmi scan into efi_init, though this can certainly be done later if and when needed.

Brian
 

Re: [PATCH] fix DMI for EFI

[Ingo Molnar]

* Brian Maly <bmaly@redhat.com> wrote:

>> Your patch looks fine, but i'd like to hear the opinion of 
>> Ying Huang as well.
>
> 	
> I had asked him to look at this before I posted and he had no 
> objections. He may have an opinion on moving dmi scan into 
> efi_init, though this can certainly be done later if and when 
> needed.

No, i think your patch is fine as-is - moving the dmi scan to 
EFI init just makes EFI bootstrap a bit less standard - hence a 
bit more fragile - and that's never good.

Since your patch fixes real issues on Apple hardware i also 
queued it up in tip:x86/urgent for .29-rc8.

	Ingo

Re: [PATCH] fix DMI for EFI

[Huang Ying]

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

On Thu, 2009-03-05 at 01:54 +0800, Ingo Molnar wrote:
> * Brian Maly <bmaly@redhat.com> wrote:
> 
> >
> >
> >> Hm, could we ever find ourselves in the positition of having to access 
> >> the DMI strings table in the EFI init code, to implement a quirk? I 
> >> think that's quite plausible.
> >
> >> OTOH, with some DMI quirks in EFI tables it's a bit of a chicken and 
> >> egg problem. Can DMI strings ever be outside of EFI tables on EFI 
> >> systems?
> >
> >> 	Ingo
> >
> > Right now it is EFI that loads the dmi tables (and all other 
> > tables such as ACPI), so it is a chicken and the egg. I think 
> > that if dmi is ever needed by EFI, the dmi scan will have to 
> > be moved inside efi_init. We could move the dmi scan into 
> > efi_init and do the dmi scan the moment we have the dmi table. 
> > Thats really the soonest point we could scan on EFI systems. 
> > Im OK with moving dmi to efi_init if you prefer.
> 
> Your patch looks fine, but i'd like to hear the opinion of Ying 
> Huang as well.

This patch is OK for me.

Thanks,
Huang Ying


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] fix DMI for EFI

[Ingo Molnar]

* Huang Ying <ying.huang@intel.com> wrote:

> On Thu, 2009-03-05 at 01:54 +0800, Ingo Molnar wrote:
> > * Brian Maly <bmaly@redhat.com> wrote:
> > 
> > >
> > >
> > >> Hm, could we ever find ourselves in the positition of having to access 
> > >> the DMI strings table in the EFI init code, to implement a quirk? I 
> > >> think that's quite plausible.
> > >
> > >> OTOH, with some DMI quirks in EFI tables it's a bit of a chicken and 
> > >> egg problem. Can DMI strings ever be outside of EFI tables on EFI 
> > >> systems?
> > >
> > >> 	Ingo
> > >
> > > Right now it is EFI that loads the dmi tables (and all other 
> > > tables such as ACPI), so it is a chicken and the egg. I think 
> > > that if dmi is ever needed by EFI, the dmi scan will have to 
> > > be moved inside efi_init. We could move the dmi scan into 
> > > efi_init and do the dmi scan the moment we have the dmi table. 
> > > Thats really the soonest point we could scan on EFI systems. 
> > > Im OK with moving dmi to efi_init if you prefer.
> > 
> > Your patch looks fine, but i'd like to hear the opinion of Ying 
> > Huang as well.
> 
> This patch is OK for me.

thanks. There are two EFI patches queued up for .29:

dd39ecf: x86: EFI: Back efi_ioremap with init_memory_mapping instead of FIX_MAP
ff0c087: x86: fix DMI on EFI

	Ingo

[tip:x86/urgent] x86: fix DMI on EFI

[Brian Maly]

Commit-ID:  ff0c0874905fb312ca1491bbdac2653b0b48c20b
Gitweb:     http://git.kernel.org/tip/ff0c0874905fb312ca1491bbdac2653b0b48c20b
Author:     "Brian Maly" <bmaly@redhat.com>
AuthorDate: Tue, 3 Mar 2009 21:55:31 -0500
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 4 Mar 2009 18:55:56 +0100

x86: fix DMI on EFI

Impact: reactivate DMI quirks on EFI hardware

DMI tables are loaded by EFI, so the dmi calls must happen after
efi_init() and not before.

Currently Apple hardware uses DMI to determine the framebuffer mappings
for efifb. Without DMI working you also have no video on MacBook Pro.

This patch resolves the DMI issue for EFI hardware (DMI is now properly
detected at boot), and additionally efifb now loads on Apple hardware
(i.e. video works).

Signed-off-by: Brian Maly <bmaly@redhat>
Acked-by: Yinghai Lu <yinghai@kernel.org>
Cc: ying.huang@intel.com
LKML-Reference: <49ADEDA3.1030406@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

 arch/x86/kernel/setup.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


---
 arch/x86/kernel/setup.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c461f6d..6a8811a 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -770,6 +770,9 @@ void __init setup_arch(char **cmdline_p)
 
 	finish_e820_parsing();
 
+	if (efi_enabled)
+		efi_init();
+
 	dmi_scan_machine();
 
 	dmi_check_system(bad_bios_dmi_table);
@@ -789,8 +792,6 @@ void __init setup_arch(char **cmdline_p)
 	insert_resource(&iomem_resource, &data_resource);
 	insert_resource(&iomem_resource, &bss_resource);
 
-	if (efi_enabled)
-		efi_init();
 
 #ifdef CONFIG_X86_32
 	if (ppro_with_ram_bug()) {

[PATCH] fix DMI for EFI

[Brian Maly]

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


DMI tables are loaded by EFI, so the dmi calls must happen after 
efi_init() and not before. Currently Apple hardware uses DMI to 
determine the framebuffer mappings for efifb. Without DMI working you 
also have no video on MacBook Pro. This patch resolves the DMI issue for 
EFI hardware (DMI is now properly detected at boot), and additionally 
efifb now loads on Apple hardware (i.e. video works).


Signed-off-by: Brian Maly <bmaly@redhat>

setup.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


[-- Attachment #2: fix-DMI-for-EFI-2.6.29.patch --]
[-- Type: text/x-patch, Size: 686 bytes --]

--- a/arch/x86/kernel/setup.c	2009-02-08 15:37:27.000000000 -0500
+++ b/arch/x86/kernel/setup.c	2009-03-01 01:09:41.000000000 -0500
@@ -770,10 +770,6 @@ void __init setup_arch(char **cmdline_p)
 
 	finish_e820_parsing();
 
-	dmi_scan_machine();
-
-	dmi_check_system(bad_bios_dmi_table);
-
 	/*
 	 * VMware detection requires dmi to be available, so this
 	 * needs to be done after dmi_scan_machine, for the BP.
@@ -792,6 +788,10 @@ void __init setup_arch(char **cmdline_p)
 	if (efi_enabled)
 		efi_init();
 
+	dmi_scan_machine();
+	
+	dmi_check_system(bad_bios_dmi_table);
+
 #ifdef CONFIG_X86_32
 	if (ppro_with_ram_bug()) {
 		e820_update_range(0x70000000ULL, 0x40000ULL, E820_RAM,

Re: [PATCH] fix DMI for EFI

[Kyle McMartin]

On Sat, Feb 28, 2009 at 08:33:15PM -0500, Brian Maly wrote:
> -	dmi_scan_machine();
> -
> -	dmi_check_system(bad_bios_dmi_table);
> -
>  	/*
>  	 * VMware detection requires dmi to be available, so this
>  	 * needs to be done after dmi_scan_machine, for the BP.

This init_hypervisor comment looks important, no? Possibly it needs
to be moved after the new placement? Doesn't look like the probe_roms
will be an issue.

regards, Kyle

> @@ -792,6 +788,10 @@ void __init setup_arch(char **cmdline_p)
>  	if (efi_enabled)
>  		efi_init();
>  
> +	dmi_scan_machine();
> +	
> +	dmi_check_system(bad_bios_dmi_table);
> +
>  #ifdef CONFIG_X86_32
>  	if (ppro_with_ram_bug()) {
>  		e820_update_range(0x70000000ULL, 0x40000ULL, E820_RAM,