答复: [PATCH] ARM64:DMI: Add smbios/dmi support on arm64

Fri Jun 6 05:58:14 PDT 2014

On Fri, 6 Jun 2014 01:57:42 +0000, liyi 00215672 <phoenix.liyi at huawei.com> wrote:
> Please see below:
> 
> -----é‚®ä»¶åŽŸä»¶-----
> å‘ä»¶äºº: Mark Rutland [mailto:mark.rutland at arm.com] 
> å‘é€æ—¶é—´: 2014å¹´6æœˆ5æ—¥ 23:34
> æ”¶ä»¶äºº: Yi Li
> æŠ„é€: ard.biesheuvel at linaro.org; Catalin Marinas; Will Deacon; Sudeep Holla; steve.capper at linaro.org; Lorenzo Pieralisi; Liviu Dudau; grant.likely at linaro.org; liyi 00215672; linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> ä¸»é¢˜: Re: [PATCH] ARM64:DMI: Add smbios/dmi support on arm64
> 
> On Wed, Jun 04, 2014 at 04:01:36PM +0100, Yi Li wrote:
> > Hi Mark,
> > 
> >      Please see the comments below:
> > 
> > On Wednesday, June 04, 2014 09:32 PM, Mark Rutland wrote:
> > > On Tue, Jun 03, 2014 at 04:57:13PM +0100, Yi Li wrote:
> > >> Add smbios/dmi support on arm64 system, it depends on
> > >> EFI boot.
> > > And what exactly does this provide us with?
> > >
> > > What is exposed through SMBIOS/DMI, and why would I want to enable it?
> > Yi: SMBIOS/DMI is one basic spec/feature for server product(like x86 and 
> > IA64).
> >       Many OEMs/ODMs hope to use ARM64 as server's processor.
> >       So we need to support SMBIOS on ARM64.
> > 
> >      SMBIOS mainly describes some hardware and software information for 
> > the system, like BIOS information
> >      CPU information,  Memory information ,and so on. please refer to 
> > http://www.dmtf.org/standards/smbios
> 
> What information does it provide that is useful that we cannot get from
> elsewhere?
> Yi: As my previous mail, SMBIOS is one basic specification for all server production (x86 and IA64).
>   It can provide some SN or UUID of production/motherboard/memory, I think you can not get that from elsewhere.
> 
> What use case is enabled by having this support, other than being able
> to tick a box that we have a feature we might not even need?
> Yi: dmidecode/ lshw tools can be used as use case to test dmi driver is working well or not .
>   And SMBIOS is one MUST section in SBBR document, so it is needed. 

Hi Mark,

SMbios is important for the 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. It is a very important tool for vendors.

It is also a trivial thing to enable. smbios only provides management
data. It doesn't affect the driver model and it doesn't have side
effects into other subsystems. This patch should be accepted.

However...

Yi, can you write a better change log for the patch. It is important to
say why a patch is important, not just what the patch does.

g.

> > 
> > >
> > >> Signed-off-by: Yi Li <yi.li at linaro.org>
> > >> ---
> > >>
> > >> Changes since v1:
> > >>    -Followed Ard Biesheuvel's suggestion to rebase the patch on
> > >>     Matt Fleming's arm64-efi branch.
> > >>
> > >>   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 6c71f12..13ee261 100644
> > >> --- a/arch/arm64/Kconfig
> > >> +++ b/arch/arm64/Kconfig
> > >> @@ -294,6 +294,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 0000000..f2198bf
> > >> --- /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 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 1
> > >> +
> > >> +#include <linux/slab.h>
> > >> +#include <asm/io.h>
> > > Shouldn't that be linux/efi.h?
> > >
> > > Why do we need asm/io.h?
> >      Yi: porting it from IA64 , so the io.h is not needed exactly!
> >           but slab.h is must included ,not efi.h (tested by compiling)
> 
> Below you refer to efi_lookup_mapped_addr, which (as far as I can tell)
> is defined in efi.h. You might be getting lucky with users of
> dmi_early_remap and dmi_remap having already incldued efi.h, but you
> shouldn't rely on it.
> 
> If you make use of something, you should include the header that defines
> it.
> 
> Yi: yes, correct. It is luck that dmi_scan.c has included efi.h , I will add efi.h in dmi.h later.
> 
> Cheers,
> Mark.
> 
> > >> +
> > >> +/* 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
> > > None of these seem to use anything from io.h directly.
> >      Yi: You are right , io.h doesn't need.
> > > Cheers,
> > > Mark.
> > 
> > 