| Message ID | 1411167781-18571-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 668ebd106860f09f43993517f786a2ddfd0f9ebe |
| Headers | show |
On Sat, Sep 20, 2014 at 12:03:01AM +0100, Ard Biesheuvel wrote: > From: Yi Li <yi.li@linaro.org> > > SMBIOS is important for server hardware vendors. It implements a spec for > providing descriptive information about the platform. Things like serial > numbers, physical layout of the ports, build configuration data, and the like. > > This has been tested by dmidecode and lshw tools. > > This patch adds the call to dmi_scan_machine() to arm64_enter_virtual_mode(), > as that is the point where the EFI Configuration Tables are registered as > being available. It needs to be in an early_initcall anyway as dmi_id_init(), > which is an arch_initcall itself, depends on dmi_scan_machine() having been > called already. > > Signed-off-by: Yi Li <yi.li@linaro.org> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Merged (until the next revert ;)). Thanks.
On Mon, Sep 22, 2014 at 11:09:12AM +0100, Catalin Marinas wrote: > On Sat, Sep 20, 2014 at 12:03:01AM +0100, Ard Biesheuvel wrote: > > From: Yi Li <yi.li@linaro.org> > > > > SMBIOS is important for server hardware vendors. It implements a spec for > > providing descriptive information about the platform. Things like serial > > numbers, physical layout of the ports, build configuration data, and the like. > > > > This has been tested by dmidecode and lshw tools. > > > > This patch adds the call to dmi_scan_machine() to arm64_enter_virtual_mode(), > > as that is the point where the EFI Configuration Tables are registered as > > being available. It needs to be in an early_initcall anyway as dmi_id_init(), > > which is an arch_initcall itself, depends on dmi_scan_machine() having been > > called already. > > > > Signed-off-by: Yi Li <yi.li@linaro.org> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Merged (until the next revert ;)). Thanks. And I'm about to revert it now. If Linux doesn't boot as an EFI application, with this patch applied I get lots of: WARNING: CPU: 4 PID: 1 at /work/Linux/linux-2.6-aarch64/drivers/firmware/dmi_scan.c:591 dmi_matches+0x10c/0x110() dmi check: not initialized yet. Modules linked in: CPU: 4 PID: 1 Comm: swapper/0 Not tainted 3.17.0-rc4+ #606 Call trace: [<ffffffc000087fb0>] dump_backtrace+0x0/0x124 [<ffffffc0000880e4>] show_stack+0x10/0x1c [<ffffffc0004d58f8>] dump_stack+0x74/0xb8 [<ffffffc0000ab640>] warn_slowpath_common+0x8c/0xb4 [<ffffffc0000ab6b4>] warn_slowpath_fmt+0x4c/0x58 [<ffffffc0003f2d7c>] dmi_matches+0x108/0x110 [<ffffffc0003f2da8>] dmi_check_system+0x24/0x68 [<ffffffc0006974c4>] atkbd_init+0x10/0x34 [<ffffffc0000814ac>] do_one_initcall+0x88/0x1a0 [<ffffffc00067aab4>] kernel_init_freeable+0x148/0x1e8 [<ffffffc0004d2c64>] kernel_init+0x10/0xd4
On 22 September 2014 19:02, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Mon, Sep 22, 2014 at 11:09:12AM +0100, Catalin Marinas wrote: >> On Sat, Sep 20, 2014 at 12:03:01AM +0100, Ard Biesheuvel wrote: >> > From: Yi Li <yi.li@linaro.org> >> > >> > SMBIOS is important for server hardware vendors. It implements a spec for >> > providing descriptive information about the platform. Things like serial >> > numbers, physical layout of the ports, build configuration data, and the like. >> > >> > This has been tested by dmidecode and lshw tools. >> > >> > This patch adds the call to dmi_scan_machine() to arm64_enter_virtual_mode(), >> > as that is the point where the EFI Configuration Tables are registered as >> > being available. It needs to be in an early_initcall anyway as dmi_id_init(), >> > which is an arch_initcall itself, depends on dmi_scan_machine() having been >> > called already. >> > >> > Signed-off-by: Yi Li <yi.li@linaro.org> >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> Merged (until the next revert ;)). Thanks. > > And I'm about to revert it now. If Linux doesn't boot as an EFI > application, with this patch applied I get lots of: > And I guess each warning is for a different driver? That is another x86-ism we hadn't spotted that we need to get rid of: DMI is used for quirks handling on x86, and apparently it gets nervous if CONFIG_DMI is enabled but DMI never gets initialized. Can I assume that quirks handling based on the DMI system name is something you would prefer to steer clear of on arm64? This is non-trivial to fix in any case, so reverting it is the only option. If nobody else picks this up in the mean time, I will revisit this after my vacation (in the hope your patience hasn't run out yet)
On 22 September 2014 21:44, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 22 September 2014 19:02, Catalin Marinas <catalin.marinas@arm.com> wrote: >> On Mon, Sep 22, 2014 at 11:09:12AM +0100, Catalin Marinas wrote: >>> On Sat, Sep 20, 2014 at 12:03:01AM +0100, Ard Biesheuvel wrote: >>> > From: Yi Li <yi.li@linaro.org> >>> > >>> > SMBIOS is important for server hardware vendors. It implements a spec for >>> > providing descriptive information about the platform. Things like serial >>> > numbers, physical layout of the ports, build configuration data, and the like. >>> > >>> > This has been tested by dmidecode and lshw tools. >>> > >>> > This patch adds the call to dmi_scan_machine() to arm64_enter_virtual_mode(), >>> > as that is the point where the EFI Configuration Tables are registered as >>> > being available. It needs to be in an early_initcall anyway as dmi_id_init(), >>> > which is an arch_initcall itself, depends on dmi_scan_machine() having been >>> > called already. >>> > >>> > Signed-off-by: Yi Li <yi.li@linaro.org> >>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> >>> Merged (until the next revert ;)). Thanks. >> >> And I'm about to revert it now. If Linux doesn't boot as an EFI >> application, with this patch applied I get lots of: >> > > And I guess each warning is for a different driver? > That is another x86-ism we hadn't spotted that we need to get rid of: > DMI is used for quirks handling on x86, and apparently it gets nervous > if CONFIG_DMI is enabled but DMI never gets initialized. Can I assume > that quirks handling based on the DMI system name is something you > would prefer to steer clear of on arm64? > > This is non-trivial to fix in any case, so reverting it is the only > option. If nobody else picks this up in the mean time, I will revisit > this after my vacation (in the hope your patience hasn't run out yet) > As it turns out, this particular issue is caused by how I moved the call to dmi_scan_machine() into arm64_enter_virtual_mode(), so it is not present in Yi's original patch. When CONFIG_DMI is enabled, dmi_scan_machine() needs to be called unconditionally, even if we are running without EFI config tables (or any EFI for that matter) @Yi: could you rework the patch so dmi_scan_machine() is called from a separate function again? But this time, please add it to arch/arm64/kernel/efi.c and invoke it using core_initcall not arch_initcall. core_initcalls are dispatched before arch_initcall but after early_initcall so that should be the right moment to get DMI initialized. Could you also test it with and without EFI? Thanks, Ard. >> WARNING: CPU: 4 PID: 1 at >> /work/Linux/linux-2.6-aarch64/drivers/firmware/dmi_scan.c:591 >> dmi_matches+0x10c/0x110() >> dmi check: not initialized yet. >> Modules linked in: >> CPU: 4 PID: 1 Comm: swapper/0 Not tainted 3.17.0-rc4+ #606 >> Call trace: >> [<ffffffc000087fb0>] dump_backtrace+0x0/0x124 >> [<ffffffc0000880e4>] show_stack+0x10/0x1c >> [<ffffffc0004d58f8>] dump_stack+0x74/0xb8 >> [<ffffffc0000ab640>] warn_slowpath_common+0x8c/0xb4 >> [<ffffffc0000ab6b4>] warn_slowpath_fmt+0x4c/0x58 >> [<ffffffc0003f2d7c>] dmi_matches+0x108/0x110 >> [<ffffffc0003f2da8>] dmi_check_system+0x24/0x68 >> [<ffffffc0006974c4>] atkbd_init+0x10/0x34 >> [<ffffffc0000814ac>] do_one_initcall+0x88/0x1a0 >> [<ffffffc00067aab4>] kernel_init_freeable+0x148/0x1e8 >> [<ffffffc0004d2c64>] kernel_init+0x10/0xd4 >> >> -- >> Catalin
On Tue, 2014-09-23 at 10:23 +0200, Ard Biesheuvel wrote: > On 22 September 2014 21:44, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 22 September 2014 19:02, Catalin Marinas <catalin.marinas@arm.com> wrote: > >> On Mon, Sep 22, 2014 at 11:09:12AM +0100, Catalin Marinas wrote: > >>> On Sat, Sep 20, 2014 at 12:03:01AM +0100, Ard Biesheuvel wrote: > >>> > From: Yi Li <yi.li@linaro.org> > >>> > > >>> > SMBIOS is important for server hardware vendors. It implements a spec for > >>> > providing descriptive information about the platform. Things like serial > >>> > numbers, physical layout of the ports, build configuration data, and the like. > >>> > > >>> > This has been tested by dmidecode and lshw tools. > >>> > > >>> > This patch adds the call to dmi_scan_machine() to arm64_enter_virtual_mode(), > >>> > as that is the point where the EFI Configuration Tables are registered as > >>> > being available. It needs to be in an early_initcall anyway as dmi_id_init(), > >>> > which is an arch_initcall itself, depends on dmi_scan_machine() having been > >>> > called already. > >>> > > >>> > Signed-off-by: Yi Li <yi.li@linaro.org> > >>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >>> > >>> Merged (until the next revert ;)). Thanks. > >> > >> And I'm about to revert it now. If Linux doesn't boot as an EFI > >> application, with this patch applied I get lots of: > >> > > > > And I guess each warning is for a different driver? > > That is another x86-ism we hadn't spotted that we need to get rid of: > > DMI is used for quirks handling on x86, and apparently it gets nervous > > if CONFIG_DMI is enabled but DMI never gets initialized. Can I assume > > that quirks handling based on the DMI system name is something you > > would prefer to steer clear of on arm64? > > > > This is non-trivial to fix in any case, so reverting it is the only > > option. If nobody else picks this up in the mean time, I will revisit > > this after my vacation (in the hope your patience hasn't run out yet) > > > > As it turns out, this particular issue is caused by how I moved the > call to dmi_scan_machine() into arm64_enter_virtual_mode(), so it is > not present in Yi's original patch. When CONFIG_DMI is enabled, > dmi_scan_machine() needs to be called unconditionally, even if we are > running without EFI config tables (or any EFI for that matter) > > @Yi: could you rework the patch so dmi_scan_machine() is called from a > separate function again? But this time, please add it to > arch/arm64/kernel/efi.c and invoke it using core_initcall not > arch_initcall. core_initcalls are dispatched before arch_initcall but > after early_initcall so that should be the right moment to get DMI > initialized. > > Could you also test it with and without EFI? efi.c is only compiled when CONFIG_EFI is defined, so you'd have to put it somewhere else. Does dmi make any sense for arm64 in the absence of EFI? Why not make CONFIG_DMI depend on EFI. You still want to use a core_initcall in case runtime is disabled, but you could put it in efi.c in that case. > > Thanks, > Ard. > > > > >> WARNING: CPU: 4 PID: 1 at > >> /work/Linux/linux-2.6-aarch64/drivers/firmware/dmi_scan.c:591 > >> dmi_matches+0x10c/0x110() > >> dmi check: not initialized yet. > >> Modules linked in: > >> CPU: 4 PID: 1 Comm: swapper/0 Not tainted 3.17.0-rc4+ #606 > >> Call trace: > >> [<ffffffc000087fb0>] dump_backtrace+0x0/0x124 > >> [<ffffffc0000880e4>] show_stack+0x10/0x1c > >> [<ffffffc0004d58f8>] dump_stack+0x74/0xb8 > >> [<ffffffc0000ab640>] warn_slowpath_common+0x8c/0xb4 > >> [<ffffffc0000ab6b4>] warn_slowpath_fmt+0x4c/0x58 > >> [<ffffffc0003f2d7c>] dmi_matches+0x108/0x110 > >> [<ffffffc0003f2da8>] dmi_check_system+0x24/0x68 > >> [<ffffffc0006974c4>] atkbd_init+0x10/0x34 > >> [<ffffffc0000814ac>] do_one_initcall+0x88/0x1a0 > >> [<ffffffc00067aab4>] kernel_init_freeable+0x148/0x1e8 > >> [<ffffffc0004d2c64>] kernel_init+0x10/0xd4 > >> > >> -- > >> Catalin > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, 2014-09-23 at 17:21 -0400, Mark Salter wrote: > On Tue, 2014-09-23 at 10:23 +0200, Ard Biesheuvel wrote: > > On 22 September 2014 21:44, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > > On 22 September 2014 19:02, Catalin Marinas <catalin.marinas@arm.com> wrote: > > >> On Mon, Sep 22, 2014 at 11:09:12AM +0100, Catalin Marinas wrote: > > >>> On Sat, Sep 20, 2014 at 12:03:01AM +0100, Ard Biesheuvel wrote: > > >>> > From: Yi Li <yi.li@linaro.org> > > >>> > > > >>> > SMBIOS is important for server hardware vendors. It implements a spec for > > >>> > providing descriptive information about the platform. Things like serial > > >>> > numbers, physical layout of the ports, build configuration data, and the like. > > >>> > > > >>> > This has been tested by dmidecode and lshw tools. > > >>> > > > >>> > This patch adds the call to dmi_scan_machine() to arm64_enter_virtual_mode(), > > >>> > as that is the point where the EFI Configuration Tables are registered as > > >>> > being available. It needs to be in an early_initcall anyway as dmi_id_init(), > > >>> > which is an arch_initcall itself, depends on dmi_scan_machine() having been > > >>> > called already. > > >>> > > > >>> > Signed-off-by: Yi Li <yi.li@linaro.org> > > >>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > >>> > > >>> Merged (until the next revert ;)). Thanks. > > >> > > >> And I'm about to revert it now. If Linux doesn't boot as an EFI > > >> application, with this patch applied I get lots of: > > >> > > > > > > And I guess each warning is for a different driver? > > > That is another x86-ism we hadn't spotted that we need to get rid of: > > > DMI is used for quirks handling on x86, and apparently it gets nervous > > > if CONFIG_DMI is enabled but DMI never gets initialized. Can I assume > > > that quirks handling based on the DMI system name is something you > > > would prefer to steer clear of on arm64? > > > > > > This is non-trivial to fix in any case, so reverting it is the only > > > option. If nobody else picks this up in the mean time, I will revisit > > > this after my vacation (in the hope your patience hasn't run out yet) > > > > > > > As it turns out, this particular issue is caused by how I moved the > > call to dmi_scan_machine() into arm64_enter_virtual_mode(), so it is > > not present in Yi's original patch. When CONFIG_DMI is enabled, > > dmi_scan_machine() needs to be called unconditionally, even if we are > > running without EFI config tables (or any EFI for that matter) > > > > @Yi: could you rework the patch so dmi_scan_machine() is called from a > > separate function again? But this time, please add it to > > arch/arm64/kernel/efi.c and invoke it using core_initcall not > > arch_initcall. core_initcalls are dispatched before arch_initcall but > > after early_initcall so that should be the right moment to get DMI > > initialized. > > > > Could you also test it with and without EFI? > > efi.c is only compiled when CONFIG_EFI is defined, so you'd have to > put it somewhere else. > > Does dmi make any sense for arm64 in the absence of EFI? Why not make > CONFIG_DMI depend on EFI. You still want to use a core_initcall in case > runtime is disabled, but you could put it in efi.c in that case. > Hmm, well maybe I should have read the patch more closely. It already depends on EFI...
On 23 September 2014 23:21, Mark Salter <msalter@redhat.com> wrote: > On Tue, 2014-09-23 at 10:23 +0200, Ard Biesheuvel wrote: >> On 22 September 2014 21:44, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > On 22 September 2014 19:02, Catalin Marinas <catalin.marinas@arm.com> wrote: >> >> On Mon, Sep 22, 2014 at 11:09:12AM +0100, Catalin Marinas wrote: >> >>> On Sat, Sep 20, 2014 at 12:03:01AM +0100, Ard Biesheuvel wrote: >> >>> > From: Yi Li <yi.li@linaro.org> >> >>> > >> >>> > SMBIOS is important for server hardware vendors. It implements a spec for >> >>> > providing descriptive information about the platform. Things like serial >> >>> > numbers, physical layout of the ports, build configuration data, and the like. >> >>> > >> >>> > This has been tested by dmidecode and lshw tools. >> >>> > >> >>> > This patch adds the call to dmi_scan_machine() to arm64_enter_virtual_mode(), >> >>> > as that is the point where the EFI Configuration Tables are registered as >> >>> > being available. It needs to be in an early_initcall anyway as dmi_id_init(), >> >>> > which is an arch_initcall itself, depends on dmi_scan_machine() having been >> >>> > called already. >> >>> > >> >>> > Signed-off-by: Yi Li <yi.li@linaro.org> >> >>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >>> >> >>> Merged (until the next revert ;)). Thanks. >> >> >> >> And I'm about to revert it now. If Linux doesn't boot as an EFI >> >> application, with this patch applied I get lots of: >> >> >> > >> > And I guess each warning is for a different driver? >> > That is another x86-ism we hadn't spotted that we need to get rid of: >> > DMI is used for quirks handling on x86, and apparently it gets nervous >> > if CONFIG_DMI is enabled but DMI never gets initialized. Can I assume >> > that quirks handling based on the DMI system name is something you >> > would prefer to steer clear of on arm64? >> > >> > This is non-trivial to fix in any case, so reverting it is the only >> > option. If nobody else picks this up in the mean time, I will revisit >> > this after my vacation (in the hope your patience hasn't run out yet) >> > >> >> As it turns out, this particular issue is caused by how I moved the >> call to dmi_scan_machine() into arm64_enter_virtual_mode(), so it is >> not present in Yi's original patch. When CONFIG_DMI is enabled, >> dmi_scan_machine() needs to be called unconditionally, even if we are >> running without EFI config tables (or any EFI for that matter) >> >> @Yi: could you rework the patch so dmi_scan_machine() is called from a >> separate function again? But this time, please add it to >> arch/arm64/kernel/efi.c and invoke it using core_initcall not >> arch_initcall. core_initcalls are dispatched before arch_initcall but >> after early_initcall so that should be the right moment to get DMI >> initialized. >> >> Could you also test it with and without EFI? > > efi.c is only compiled when CONFIG_EFI is defined, so you'd have to > put it somewhere else. > > Does dmi make any sense for arm64 in the absence of EFI? Why not make > CONFIG_DMI depend on EFI. You still want to use a core_initcall in case > runtime is disabled, but you could put it in efi.c in that case. > CONFIG_DMI already depends on CONFIG_EFI, so putting the core_initcall in efi.c makes perfect sense. The problem Catalin hit was a CONFIG_EFI/CONFIG_DMI capable kernel booted through another bootloader.
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index fd4e81a4e1ce..c69ab5a3a321 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -368,6 +368,17 @@ config EFI allow the kernel to be booted as an EFI application. This is only useful on systems that have UEFI firmware. +config DMI + bool "Enable support for SMBIOS (DMI) tables" + depends on EFI + default y + help + This enables SMBIOS/DMI feature for systems. + + This option is only useful on systems that have UEFI firmware. + However, even with this option, the resultant kernel should + continue to boot on existing non-UEFI platforms. + endmenu menu "Userspace binary formats" diff --git a/arch/arm64/include/asm/dmi.h b/arch/arm64/include/asm/dmi.h new file mode 100644 index 000000000000..69d37d87b159 --- /dev/null +++ b/arch/arm64/include/asm/dmi.h @@ -0,0 +1,31 @@ +/* + * arch/arm64/include/asm/dmi.h + * + * Copyright (C) 2013 Linaro Limited. + * Written by: Yi Li (yi.li@linaro.org) + * + * based on arch/ia64/include/asm/dmi.h + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + */ + +#ifndef __ASM_DMI_H +#define __ASM_DMI_H + +#include <linux/io.h> +#include <linux/slab.h> + +/* + * According to section 2.3.6 of the UEFI spec, the firmware should not + * request a virtual mapping for configuration tables such as SMBIOS. + * This means we have to map them before use. + */ +#define dmi_early_remap(x, l) ioremap_cache(x, l) +#define dmi_early_unmap(x, l) iounmap(x) +#define dmi_remap(x, l) ioremap_cache(x, l) +#define dmi_unmap(x) iounmap(x) +#define dmi_alloc(l) kzalloc(l, GFP_KERNEL) + +#endif diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c index 03aaa99e1ea0..b71ab0e5780c 100644 --- a/arch/arm64/kernel/efi.c +++ b/arch/arm64/kernel/efi.c @@ -11,6 +11,7 @@ * */ +#include <linux/dmi.h> #include <linux/efi.h> #include <linux/export.h> #include <linux/memblock.h> @@ -435,6 +436,13 @@ static int __init arm64_enter_virtual_mode(void) } set_bit(EFI_SYSTEM_TABLES, &efi.flags); + /* + * DMI depends on EFI on arm64, and dmi_scan_machine() needs to be + * called early because dmi_id_init(), which is an arch_initcall itself, + * depends on dmi_scan_machine() having been called already. + */ + dmi_scan_machine(); + local_irq_save(flags); cpu_switch_mm(idmap_pg_dir, &init_mm);