[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[Ard Biesheuvel]

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

 arch/arm64/Kconfig           | 10 ++++++++++
 arch/arm64/include/asm/dmi.h | 41 +++++++++++++++++++++++++++++++++++++++++
 arch/arm64/kernel/setup.c    |  2 ++
 3 files changed, 53 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..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 at 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);
-- 
1.8.3.2

[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[Will Deacon]

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

[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[Ard Biesheuvel]

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?

-- 
Ard.

[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[Ard Biesheuvel]

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?

-- 
Ard.


>
> 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?
>

[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[Will Deacon]

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

答复: [PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[liyi 00215672]

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 at arm.com] 
????: 2014?7?21? 16:48
???: Ard Biesheuvel
??: Catalin Marinas; yi.li at linaro.org; liyi 00215672; grant.likely at linaro.org; leif.lindholm at linaro.org; linux-arm-kernel at 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

[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[Catalin Marinas]

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?

-- 
Catalin

[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[Ard Biesheuvel]

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().

-- 
Ard.

[PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support

[Catalin Marinas]

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.

-- 
Catalin