| Message ID | 1406810830-6099-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | a28e3f4b90543f7c249a956e3ca518e243a04618 |
| Headers | show |
On 31 July 2014 14:47, 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> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > It turns out that efi_lookup_mapped_addr() is not appropriate after all for > remapping the address of the SMBIOS tables. The reason is that the UEFI spec > forbids virtual mappings being requested for configuration tables (which is not > currently honored by Tianocore/EDK2) > @Yi: could you retest yet again, please? @Will; this superseded the patch that is already queued up in for-next/core, so perhaps you could drop that while we get this tested? > > arch/arm64/Kconfig | 11 +++++++++++ > arch/arm64/include/asm/dmi.h | 33 +++++++++++++++++++++++++++++++++ > arch/arm64/kernel/setup.c | 2 ++ > 3 files changed, 46 insertions(+) > create mode 100644 arch/arm64/include/asm/dmi.h > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 839f48c26ef0..0b2350470ac9 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -308,6 +308,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..0a07357bb776 > --- /dev/null > +++ b/arch/arm64/include/asm/dmi.h > @@ -0,0 +1,33 @@ > +/* > + * 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 must not request > + * a virtual mapping for configuration tables such as SMBIOS. This means we have > + * to map them before use. As SMBIOS tables are typed as EfiRuntimeServicesData, > + * they should always reside in DRAM, so we can use ioremap_cache() here, which > + * will give us the existing linear mapping if the address is covered by it. > + */ > +#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/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 Thu, Jul 31, 2014 at 01:48:36PM +0100, Ard Biesheuvel wrote: > On 31 July 2014 14:47, 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> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > > > It turns out that efi_lookup_mapped_addr() is not appropriate after all for > > remapping the address of the SMBIOS tables. The reason is that the UEFI spec > > forbids virtual mappings being requested for configuration tables (which is not > > currently honored by Tianocore/EDK2) > > > > @Yi: could you retest yet again, please? > > @Will; this superseded the patch that is already queued up in > for-next/core, so perhaps you could drop that while we get this > tested? Reverted. I'd really appreciate help testing this stuff in future -- it was a pig getting it going on the foundation model and the whole setup felt rather contrived. Anyway, thanks for letting me know. Will
On 31 July 2014 16:20, Yi Li <yi.li@linaro.org> wrote: > Hi Ard, > > The patch works well on FVP model, and here are the boot log and > dmidecode+lshw result: > Thanks Yi! Can we help Will out if he wants to test this himself?
On Thursday, July 31, 2014 10:26 PM, Ard Biesheuvel wrote: > On 31 July 2014 16:20, Yi Li <yi.li@linaro.org> wrote: >> Hi Ard, >> >> The patch works well on FVP model, and here are the boot log and >> dmidecode+lshw result: >> > Thanks Yi! > > Can we help Will out if he wants to test this himself? @Will: which platform do you have on hand? I am using FVP model for testing. First, you need to merge SMBIOS support into UEFI. Then, compile and install the dmidecode(open source) on the linux ,and run it.
On Thu, Jul 31, 2014 at 04:51:41PM +0100, Yi Li wrote: > On Thursday, July 31, 2014 10:26 PM, Ard Biesheuvel wrote: > > On 31 July 2014 16:20, Yi Li <yi.li@linaro.org> wrote: > >> Hi Ard, > >> > >> The patch works well on FVP model, and here are the boot log and > >> dmidecode+lshw result: > >> > > Thanks Yi! > > > > Can we help Will out if he wants to test this himself? > @Will: which platform do you have on hand? I am using FVP model > for testing. > First, you need to merge SMBIOS support into UEFI. > Then, compile and install the dmidecode(open source) on the linux > ,and run it. I've got some stuff from Leif for testing this, but what I'm really after is something that would have spotted this problem. Are there any compliance tests, for example? Will
On Thu, Jul 31, 2014 at 04:53:38PM +0100, Will Deacon wrote: > > >> The patch works well on FVP model, and here are the boot log and > > >> dmidecode+lshw result: > > >> > > > Thanks Yi! > > > > > > Can we help Will out if he wants to test this himself? > > @Will: which platform do you have on hand? I am using FVP model > > for testing. > > First, you need to merge SMBIOS support into UEFI. > > Then, compile and install the dmidecode(open source) on the linux > > ,and run it. > > I've got some stuff from Leif for testing this, but what I'm really after is > something that would have spotted this problem. Are there any compliance > tests, for example? The patch as previously submitted (and accepted) was functional on all platforms it has been tested on. It is technically not compliant with the UEFI specification, but it works with the images we had. The worst-case failure would have been the lookup function returning NULL and the SMBIOS tables being unavailable even though they existed. What you are asking for would be a test for UEFI, not for the kernel. (And this one might be useful to add to the fwts if it's not already part of it.) / Leif
On 31 July 2014 18:15, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Thu, Jul 31, 2014 at 04:53:38PM +0100, Will Deacon wrote: >> > >> The patch works well on FVP model, and here are the boot log and >> > >> dmidecode+lshw result: >> > >> >> > > Thanks Yi! >> > > >> > > Can we help Will out if he wants to test this himself? >> > @Will: which platform do you have on hand? I am using FVP model >> > for testing. >> > First, you need to merge SMBIOS support into UEFI. >> > Then, compile and install the dmidecode(open source) on the linux >> > ,and run it. >> >> I've got some stuff from Leif for testing this, but what I'm really after is >> something that would have spotted this problem. Are there any compliance >> tests, for example? > > The patch as previously submitted (and accepted) was functional on all > platforms it has been tested on. It is technically not compliant with > the UEFI specification, but it works with the images we had. > The worst-case failure would have been the lookup function returning > NULL and the SMBIOS tables being unavailable even though they existed. > Indeed. It would be bad for us to rely on non-compliant but wide spread behavior, so I think it is a good thing to shake out these bugs now, even if they haven't caused any problems. > What you are asking for would be a test for UEFI, not for the kernel. > (And this one might be useful to add to the fwts if it's not already > part of it.) > Yes, it would be good if we could test for patterns like this, (i.e., config tables with EFI_MEMORY_RUNTIME set).
On 31 July 2014 06:01, Will Deacon <will.deacon@arm.com> wrote: > On Thu, Jul 31, 2014 at 01:48:36PM +0100, Ard Biesheuvel wrote: >> On 31 July 2014 14:47, 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> >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > --- >> > >> > It turns out that efi_lookup_mapped_addr() is not appropriate after all for >> > remapping the address of the SMBIOS tables. The reason is that the UEFI spec >> > forbids virtual mappings being requested for configuration tables (which is not >> > currently honored by Tianocore/EDK2) >> > >> >> @Yi: could you retest yet again, please? >> >> @Will; this superseded the patch that is already queued up in >> for-next/core, so perhaps you could drop that while we get this >> tested? > > Reverted. I'd really appreciate help testing this stuff in future -- it was > a pig getting it going on the foundation model and the whole setup felt > rather contrived. > > Anyway, thanks for letting me know. > Hi Will, Are you ok to take this patch now, for 3.18? Surely, we could get it tested today or tomorrow if you would still like do it yourself? Yi has tested it on a recent RC so we could ask him to demonstrate if for us. Cheers, Ard.
On Wed, Sep 17, 2014 at 11:14:24PM +0100, Ard Biesheuvel wrote: > On 31 July 2014 06:01, Will Deacon <will.deacon@arm.com> wrote: > > On Thu, Jul 31, 2014 at 01:48:36PM +0100, Ard Biesheuvel wrote: > >> On 31 July 2014 14:47, 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> > >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> > --- > >> > > >> > It turns out that efi_lookup_mapped_addr() is not appropriate after all for > >> > remapping the address of the SMBIOS tables. The reason is that the UEFI spec > >> > forbids virtual mappings being requested for configuration tables (which is not > >> > currently honored by Tianocore/EDK2) > >> > > >> > >> @Yi: could you retest yet again, please? > >> > >> @Will; this superseded the patch that is already queued up in > >> for-next/core, so perhaps you could drop that while we get this > >> tested? > > > > Reverted. I'd really appreciate help testing this stuff in future -- it was > > a pig getting it going on the foundation model and the whole setup felt > > rather contrived. > > > > Anyway, thanks for letting me know. > > > > Hi Will, > > Are you ok to take this patch now, for 3.18? Surely, we could get it > tested today or tomorrow if you would still like do it yourself? Yi > has tested it on a recent RC so we could ask him to demonstrate if for > us. Any chance you could resend the fixed patch, please? Catalin's dealing with the 3.18 tree, so he should be able to apply it then. Will
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 839f48c26ef0..0b2350470ac9 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -308,6 +308,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..0a07357bb776 --- /dev/null +++ b/arch/arm64/include/asm/dmi.h @@ -0,0 +1,33 @@ +/* + * 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 must not request + * a virtual mapping for configuration tables such as SMBIOS. This means we have + * to map them before use. As SMBIOS tables are typed as EfiRuntimeServicesData, + * they should always reside in DRAM, so we can use ioremap_cache() here, which + * will give us the existing linear mapping if the address is covered by it. + */ +#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/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);