| Message ID | 1404297277-28295-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 2 July 2014 12:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> 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. > > Signed-off-by: Yi Li <yi.li@linaro.org> > [ardb: whitespace, commit log tweaks] > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > Resending on behalf of Yi. > Please consider for 3.17. > Ping? > arch/arm64/Kconfig | 10 ++++++++++ > arch/arm64/include/asm/dmi.h | 28 ++++++++++++++++++++++++++++ > arch/arm64/kernel/setup.c | 2 ++ > 3 files changed, 40 insertions(+) > create mode 100644 arch/arm64/include/asm/dmi.h > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a474de346be6..560996c4a172 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -307,6 +307,16 @@ 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..b8758f46fb42 > --- /dev/null > +++ b/arch/arm64/include/asm/dmi.h > @@ -0,0 +1,28 @@ > +/* > + * 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 1 > + > +#include <linux/slab.h> > +#include <linux/efi.h> > + > +/* Use efi mappings for DMI */ > +#define dmi_early_remap(x, l) efi_lookup_mapped_addr(x) > +#define dmi_early_unmap(x, l) > +#define dmi_remap(x, l) efi_lookup_mapped_addr(x) > +#define dmi_unmap(x) > +#define dmi_alloc(l) kzalloc(l, GFP_ATOMIC) > + > +#endif > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > index 46d1125571f6..4075e46282b1 100644 > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -43,6 +43,7 @@ > #include <linux/of_fdt.h> > #include <linux/of_platform.h> > #include <linux/efi.h> > +#include <linux/dmi.h> > > #include <asm/fixmap.h> > #include <asm/cputype.h> > @@ -413,6 +414,7 @@ void __init setup_arch(char **cmdline_p) > static int __init arm64_device_init(void) > { > of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); > + dmi_scan_machine(); > return 0; > } > arch_initcall_sync(arm64_device_init); > -- > 1.8.3.2 >
On Tue, Jul 08, 2014 at 09:55:53AM +0100, Ard Biesheuvel wrote: > On 2 July 2014 12:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> 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. > > > > Signed-off-by: Yi Li <yi.li@linaro.org> > > [ardb: whitespace, commit log tweaks] > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > > > Resending on behalf of Yi. > > Please consider for 3.17. > > > > Ping? Not sure what this does, but I can review it on a superficial level :) > > diff --git a/arch/arm64/include/asm/dmi.h b/arch/arm64/include/asm/dmi.h > > new file mode 100644 > > index 000000000000..b8758f46fb42 > > --- /dev/null > > +++ b/arch/arm64/include/asm/dmi.h > > @@ -0,0 +1,28 @@ > > +/* > > + * 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 1 You don't need the 1 here (copied from ia64?). Also, please try to follow the existing style for arm64 and use __ASM_DMI_H. > > +#include <linux/slab.h> > > +#include <linux/efi.h> > > + > > +/* Use efi mappings for DMI */ > > +#define dmi_early_remap(x, l) efi_lookup_mapped_addr(x) Throwing away the length doesn't feel right, especially since the efi map *does* have a size field. > > +#define dmi_early_unmap(x, l) > > +#define dmi_remap(x, l) efi_lookup_mapped_addr(x) How do we guarantee that we don't call these after efi_free_boot_services? (it would be good to enforce that somehow). > > +#define dmi_unmap(x) > > +#define dmi_alloc(l) kzalloc(l, GFP_ATOMIC) Why does this have to be atomic? Will
On 8 July 2014 11:15, Will Deacon <will.deacon@arm.com> wrote: > On Tue, Jul 08, 2014 at 09:55:53AM +0100, Ard Biesheuvel wrote: >> On 2 July 2014 12:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> 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. >> > >> > Signed-off-by: Yi Li <yi.li@linaro.org> >> > [ardb: whitespace, commit log tweaks] >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > --- >> > >> > Resending on behalf of Yi. >> > Please consider for 3.17. >> > >> >> Ping? > > Not sure what this does, but I can review it on a superficial level :) > Thanks. >> > diff --git a/arch/arm64/include/asm/dmi.h b/arch/arm64/include/asm/dmi.h >> > new file mode 100644 >> > index 000000000000..b8758f46fb42 >> > --- /dev/null >> > +++ b/arch/arm64/include/asm/dmi.h >> > @@ -0,0 +1,28 @@ >> > +/* >> > + * 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 1 > > You don't need the 1 here (copied from ia64?). Also, please try to follow > the existing style for arm64 and use __ASM_DMI_H. > Yes, copy/paste from ia64. Will fix it up. >> > +#include <linux/slab.h> >> > +#include <linux/efi.h> >> > + >> > +/* Use efi mappings for DMI */ >> > +#define dmi_early_remap(x, l) efi_lookup_mapped_addr(x) > > Throwing away the length doesn't feel right, especially since the efi map > *does* have a size field. > The thing to realize here is that, instead of doing actual ioremap() or early_ioremap() calls, we just reuse an existing EFI mapping here. (On x86, DMI/SMBIOS and EFI are not as tightly coupled, whereas on arm64, the former implies the latter). So would you prefer some kind of test against the size of the mapping before doing that? >> > +#define dmi_early_unmap(x, l) >> > +#define dmi_remap(x, l) efi_lookup_mapped_addr(x) > > How do we guarantee that we don't call these after efi_free_boot_services? > (it would be good to enforce that somehow). > That is also an x86-ism, unfortunately. In our case, efi_lookup_mapped_addr() can be called at any time. The comment next to the definition of efi_lookup_mapped_addr() is in shared code, and should probably be updated to reflect this. >> > +#define dmi_unmap(x) >> > +#define dmi_alloc(l) kzalloc(l, GFP_ATOMIC) > > Why does this have to be atomic? > Also copy/paste from ia64. GFP_KERNEL should be fine here
On Tue, Jul 08, 2014 at 10:27:06AM +0100, Ard Biesheuvel wrote: > On 8 July 2014 11:15, Will Deacon <will.deacon@arm.com> wrote: > > On Tue, Jul 08, 2014 at 09:55:53AM +0100, Ard Biesheuvel wrote: > >> > +#include <linux/slab.h> > >> > +#include <linux/efi.h> > >> > + > >> > +/* Use efi mappings for DMI */ > >> > +#define dmi_early_remap(x, l) efi_lookup_mapped_addr(x) > > > > Throwing away the length doesn't feel right, especially since the efi map > > *does* have a size field. > > > > The thing to realize here is that, instead of doing actual ioremap() > or early_ioremap() calls, we just reuse an existing EFI mapping here. > (On x86, DMI/SMBIOS and EFI are not as tightly coupled, whereas on > arm64, the former implies the latter). So would you prefer some kind > of test against the size of the mapping before doing that? Yeah, might not be a bad idea. Even if the thing happens to work by construction, we still have an API that takes a size, so checking it's what we have mapped sounds sensible. Will
On 8 July 2014 11:30, Will Deacon <will.deacon@arm.com> wrote: > On Tue, Jul 08, 2014 at 10:27:06AM +0100, Ard Biesheuvel wrote: >> On 8 July 2014 11:15, Will Deacon <will.deacon@arm.com> wrote: >> > On Tue, Jul 08, 2014 at 09:55:53AM +0100, Ard Biesheuvel wrote: >> >> > +#include <linux/slab.h> >> >> > +#include <linux/efi.h> >> >> > + >> >> > +/* Use efi mappings for DMI */ >> >> > +#define dmi_early_remap(x, l) efi_lookup_mapped_addr(x) >> > >> > Throwing away the length doesn't feel right, especially since the efi map >> > *does* have a size field. >> > >> >> The thing to realize here is that, instead of doing actual ioremap() >> or early_ioremap() calls, we just reuse an existing EFI mapping here. >> (On x86, DMI/SMBIOS and EFI are not as tightly coupled, whereas on >> arm64, the former implies the latter). So would you prefer some kind >> of test against the size of the mapping before doing that? > > Yeah, might not be a bad idea. Even if the thing happens to work by > construction, we still have an API that takes a size, so checking it's what > we have mapped sounds sensible. > So unless anyone objects, I am going to take a shortcut here and just check whether efi_lookup_mapped_addr(phys) + len -1 == efi_lookup_mapped_addr(phys + len -1) This doesn't check whether there are holes in the mapping, but anything more would imply creating a new function like efi_lookup_mapped_addr() that does take a size and iterates over all mappings in between.
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index a474de346be6..560996c4a172 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -307,6 +307,16 @@ 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..b8758f46fb42 --- /dev/null +++ b/arch/arm64/include/asm/dmi.h @@ -0,0 +1,28 @@ +/* + * 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 1 + +#include <linux/slab.h> +#include <linux/efi.h> + +/* Use efi mappings for DMI */ +#define dmi_early_remap(x, l) efi_lookup_mapped_addr(x) +#define dmi_early_unmap(x, l) +#define dmi_remap(x, l) efi_lookup_mapped_addr(x) +#define dmi_unmap(x) +#define dmi_alloc(l) kzalloc(l, GFP_ATOMIC) + +#endif diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 46d1125571f6..4075e46282b1 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -43,6 +43,7 @@ #include <linux/of_fdt.h> #include <linux/of_platform.h> #include <linux/efi.h> +#include <linux/dmi.h> #include <asm/fixmap.h> #include <asm/cputype.h> @@ -413,6 +414,7 @@ void __init setup_arch(char **cmdline_p) static int __init arm64_device_init(void) { of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); + dmi_scan_machine(); return 0; } arch_initcall_sync(arm64_device_init);