| Message ID | 1405079210-30044-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 11, 2014 at 12:46:50PM +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. > > Signed-off-by: Yi Li <yi.li@linaro.org> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > Changes since previous version: > - changed double inclusion guard to arm64 flavour > - use kzalloc(GFP_KERNEL) as GFP_ATOMIC is not needed > - do a sanity check on the size of the requested mapping Technically, this patch looks fine to me: Reviewed-by: Will Deacon <will.deacon@arm.com> It would be good if somebody with a relevant platform could confirm that it works/is useful though. Will
On 14 July 2014 15:36, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Jul 11, 2014 at 12:46:50PM +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. >> >> Signed-off-by: Yi Li <yi.li@linaro.org> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> Changes since previous version: >> - changed double inclusion guard to arm64 flavour >> - use kzalloc(GFP_KERNEL) as GFP_ATOMIC is not needed >> - do a sanity check on the size of the requested mapping > > Technically, this patch looks fine to me: > > Reviewed-by: Will Deacon <will.deacon@arm.com> > Thanks. > It would be good if somebody with a relevant platform could confirm that > it works/is useful though. > The usefulness has been debated here a couple of times before: Grant's take on it is here http://marc.info/?l=linux-arm-kernel&m=140206407411522&w=2 As to whether it works: I have asked Yi to retest this version of the patch with lshw and dmidecode @Yi: could you please share your findings here?
On 14 July 2014 15:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 14 July 2014 15:36, Will Deacon <will.deacon@arm.com> wrote: >> On Fri, Jul 11, 2014 at 12:46:50PM +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. >>> >>> Signed-off-by: Yi Li <yi.li@linaro.org> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> --- >>> Changes since previous version: >>> - changed double inclusion guard to arm64 flavour >>> - use kzalloc(GFP_KERNEL) as GFP_ATOMIC is not needed >>> - do a sanity check on the size of the requested mapping >> >> Technically, this patch looks fine to me: >> >> Reviewed-by: Will Deacon <will.deacon@arm.com> >> Hello Catalin, This has been tested by Yi on FVP and Juno. Are you ok to take this?
On Sun, Jul 20, 2014 at 11:25:34AM +0100, Ard Biesheuvel wrote: > On 14 July 2014 15:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 14 July 2014 15:36, Will Deacon <will.deacon@arm.com> wrote: > >> On Fri, Jul 11, 2014 at 12:46:50PM +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. > >>> > >>> Signed-off-by: Yi Li <yi.li@linaro.org> > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >>> --- > >>> Changes since previous version: > >>> - changed double inclusion guard to arm64 flavour > >>> - use kzalloc(GFP_KERNEL) as GFP_ATOMIC is not needed > >>> - do a sanity check on the size of the requested mapping > >> > >> Technically, this patch looks fine to me: > >> > >> Reviewed-by: Will Deacon <will.deacon@arm.com> > >> > > Hello Catalin, > > This has been tested by Yi on FVP and Juno. > Are you ok to take this? Out of interest, how did you test it on Juno? I tried but got some messages about missing/invalid DMI data or something similar. Leif reckons I need some tricked out firmware for the FVP to get something working, but even then the information is apparently bogus. Will
Ard, I have some technical questions below before merging this patch. On Fri, Jul 11, 2014 at 12:46:50PM +0100, Ard Biesheuvel wrote: > --- /dev/null > +++ b/arch/arm64/include/asm/dmi.h > @@ -0,0 +1,41 @@ [...] > +static inline void __iomem *dmi_remap(u64 phys, u64 size) > +{ > + void __iomem *p = efi_lookup_mapped_addr(phys); When are dmi_remap/dmi_early_remap() called? A quick grep through the kernel shows that it is at least called once from dmi_scan_machine(). The latter is a device_initcall() in this patch. However, the comments for efi_lookup_mapped_addr() state that it should only be called between efi_enter_virtual_mode and efi_free_boot_services. The latter is invoked from an early_initcall(). Could you please clarify which part is wrong here? > + > + /* > + * If the mapping spans multiple pages, do a minimal check to ensure > + * that the mapping returned by efi_lookup_mapped_addr() covers the > + * whole requested range (but ignore potential holes) > + */ > + if ((phys & ~PAGE_MASK) + size > PAGE_SIZE > + && (p + size - 1) != efi_lookup_mapped_addr(phys + size - 1)) > + return NULL; > + return p; > +} > + > +/* Reuse existing UEFI mappings for DMI */ > +#define dmi_alloc(l) kzalloc(l, GFP_KERNEL) > +#define dmi_early_remap(x, l) dmi_remap(x, l) > +#define dmi_early_unmap(x, l) > +#define dmi_unmap(x) Same questions as above, when are these functions called? Can we not just use early_ioremap/ioremap like x86?
On 21 July 2014 12:03, Catalin Marinas <catalin.marinas@arm.com> wrote: > Ard, > > I have some technical questions below before merging this patch. > > On Fri, Jul 11, 2014 at 12:46:50PM +0100, Ard Biesheuvel wrote: >> --- /dev/null >> +++ b/arch/arm64/include/asm/dmi.h >> @@ -0,0 +1,41 @@ > [...] >> +static inline void __iomem *dmi_remap(u64 phys, u64 size) >> +{ >> + void __iomem *p = efi_lookup_mapped_addr(phys); > > When are dmi_remap/dmi_early_remap() called? A quick grep through the > kernel shows that it is at least called once from dmi_scan_machine(). > The latter is a device_initcall() in this patch. However, the comments > for efi_lookup_mapped_addr() state that it should only be called between > efi_enter_virtual_mode and efi_free_boot_services. The latter is invoked > from an early_initcall(). Could you please clarify which part is wrong > here? > The comment about efi_lookup_mapped_addr() is wrong. Those mappings are always available. As the comment is in shared code, I will propose a patch to Matt Fleming to clarify it. >> + >> + /* >> + * If the mapping spans multiple pages, do a minimal check to ensure >> + * that the mapping returned by efi_lookup_mapped_addr() covers the >> + * whole requested range (but ignore potential holes) >> + */ >> + if ((phys & ~PAGE_MASK) + size > PAGE_SIZE >> + && (p + size - 1) != efi_lookup_mapped_addr(phys + size - 1)) >> + return NULL; >> + return p; >> +} >> + >> +/* Reuse existing UEFI mappings for DMI */ >> +#define dmi_alloc(l) kzalloc(l, GFP_KERNEL) >> +#define dmi_early_remap(x, l) dmi_remap(x, l) >> +#define dmi_early_unmap(x, l) >> +#define dmi_unmap(x) > > Same questions as above, when are these functions called? Can we not > just use early_ioremap/ioremap like x86? > x86 uses DMI for quirks handling, so there the call to dmi_scan_machine() needs to occur very early, hence the early versions of map/unmap. In our case, DMI/SMBIOS is UEFI only and not executed early, so we can just reuse the mappings that have already been set up for UEFI by arm64_enter_virtual_mode().
On Mon, Jul 21, 2014 at 11:18:37AM +0100, Ard Biesheuvel wrote: > On 21 July 2014 12:03, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Fri, Jul 11, 2014 at 12:46:50PM +0100, Ard Biesheuvel wrote: > >> --- /dev/null > >> +++ b/arch/arm64/include/asm/dmi.h > >> @@ -0,0 +1,41 @@ > > [...] > >> +static inline void __iomem *dmi_remap(u64 phys, u64 size) > >> +{ > >> + void __iomem *p = efi_lookup_mapped_addr(phys); > > > > When are dmi_remap/dmi_early_remap() called? A quick grep through the > > kernel shows that it is at least called once from dmi_scan_machine(). > > The latter is a device_initcall() in this patch. However, the comments > > for efi_lookup_mapped_addr() state that it should only be called between > > efi_enter_virtual_mode and efi_free_boot_services. The latter is invoked > > from an early_initcall(). Could you please clarify which part is wrong > > here? > > The comment about efi_lookup_mapped_addr() is wrong. Those mappings > are always available. > As the comment is in shared code, I will propose a patch to Matt > Fleming to clarify it. Thanks for the clarification. I'll merge this patch as is.
Hi Will, You need to add the SMBIOS support in UEFI on JUNO ,then you will get some proper information from DMI driver. Thanks! Yi -----邮件原件----- 发件人: Will Deacon [mailto:will.deacon@arm.com] 发送时间: 2014年7月21日 16:48 收件人: Ard Biesheuvel 抄送: Catalin Marinas; yi.li@linaro.org; liyi 00215672; grant.likely@linaro.org; leif.lindholm@linaro.org; linux-arm-kernel@lists.infradead.org; Mark Rutland 主题: Re: [PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support On Sun, Jul 20, 2014 at 11:25:34AM +0100, Ard Biesheuvel wrote: > On 14 July 2014 15:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 14 July 2014 15:36, Will Deacon <will.deacon@arm.com> wrote: > >> On Fri, Jul 11, 2014 at 12:46:50PM +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. > >>> > >>> Signed-off-by: Yi Li <yi.li@linaro.org> > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >>> --- > >>> Changes since previous version: > >>> - changed double inclusion guard to arm64 flavour > >>> - use kzalloc(GFP_KERNEL) as GFP_ATOMIC is not needed > >>> - do a sanity check on the size of the requested mapping > >> > >> Technically, this patch looks fine to me: > >> > >> Reviewed-by: Will Deacon <will.deacon@arm.com> > >> > > Hello Catalin, > > This has been tested by Yi on FVP and Juno. > Are you ok to take this? Out of interest, how did you test it on Juno? I tried but got some messages about missing/invalid DMI data or something similar. Leif reckons I need some tricked out firmware for the FVP to get something working, but even then the information is apparently bogus. Will
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..b0882a8620e1 --- /dev/null +++ b/arch/arm64/include/asm/dmi.h @@ -0,0 +1,41 @@ +/* + * 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/slab.h> +#include <linux/efi.h> + +static inline void __iomem *dmi_remap(u64 phys, u64 size) +{ + void __iomem *p = efi_lookup_mapped_addr(phys); + + /* + * If the mapping spans multiple pages, do a minimal check to ensure + * that the mapping returned by efi_lookup_mapped_addr() covers the + * whole requested range (but ignore potential holes) + */ + if ((phys & ~PAGE_MASK) + size > PAGE_SIZE + && (p + size - 1) != efi_lookup_mapped_addr(phys + size - 1)) + return NULL; + return p; +} + +/* Reuse existing UEFI mappings for DMI */ +#define dmi_alloc(l) kzalloc(l, GFP_KERNEL) +#define dmi_early_remap(x, l) dmi_remap(x, l) +#define dmi_early_unmap(x, l) +#define dmi_unmap(x) + +#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);