From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58168) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZN4Xn-0005E2-BB for qemu-devel@nongnu.org; Wed, 05 Aug 2015 15:40:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZN4Xi-0004DC-Fz for qemu-devel@nongnu.org; Wed, 05 Aug 2015 15:40:07 -0400 Received: from mail-wi0-f182.google.com ([209.85.212.182]:34085) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZN4Xi-0004B4-8H for qemu-devel@nongnu.org; Wed, 05 Aug 2015 15:40:02 -0400 Received: by wicne3 with SMTP id ne3so17861608wic.1 for ; Wed, 05 Aug 2015 12:40:01 -0700 (PDT) Message-ID: <55C2668D.8040008@linaro.org> Date: Wed, 05 Aug 2015 22:39:57 +0300 From: Ivan Khoronzhuk MIME-Version: 1.0 References: <1438063215-4117-1-git-send-email-wei@redhat.com> <55C244D5.3040004@redhat.com> <55C25764.9090302@redhat.com> In-Reply-To: <55C25764.9090302@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek , Peter Maydell , Jean Delvare Cc: Wei Huang , Shannon Zhao , Andrew Jones , Eduardo Habkost , "Michael S. Tsirkin" , "Gabriel L. Somlo" , Ard Biesheuvel , QEMU Developers , Roy Franz , Igor Mammedov , Paolo Bonzini , Richard Henderson Hi Laszlo On 05.08.15 21:35, Laszlo Ersek wrote: > On 08/05/15 19:35, Peter Maydell wrote: >> On 5 August 2015 at 18:16, Laszlo Ersek wrote: >>> On 07/28/15 08:00, Wei Huang wrote: >>>> SMBIOS tables present userful system hardware info to management >>>> applications, such as DMI tools. Even though SMBIOS was originally >>>> developed for Intel x86, it has been extended to both Itanium and >>>> ARM (32bit & 64bit). More and more ARM server releases, such as >>>> RHEL Server for ARM, start to integrate support for SMBIOS. >>>> >>>> This patchset is intendted to provid SMBIOS tables for ARM mach-virt >>>> machine. The SMBIOS tables are created and stored in fw_cfg, relying on >>>> OVMF (AAVMF) to parse/present SMBIOS entry. >>>> >>>> RFC version have been tested by Laszlo using his customized version of >>>> AAVMF. We were able to detect SMBIOS 2.8 tables using dmidecode inside >>>> an AArch64 guest VM. Moving forward, it is better to support SMBIOS 3.0 >>>> for ARM guest VM. This new version (V1) integrates SMBIOS 3.0 support >>>> for ARM mach-virt. I have tested this version by forcing SMBIOS 2.1 >>>> format (i.e. passing SMBIOS_21_ENTRY_POINT to smbios_set_defaults()). >>>> SMBIOS 3.0 hasn't been tested yet as it requires AAVMF to install 3.0 entry. >>>> >>>> RFC->V1: >>>> * Add SMBIOS 3.0 support for buidling SMBIOS >>>> * Switch from SMBIOS 2.1 to 3.0 for ARM mach-virt >>>> * RFC version Tested-by Laszlo Ersek and Acked-by Gabriel Somlo >>>> >>>> Thanks, >>>> -Wei >>>> >>>> Wei Huang (6): >>>> smbios: extract x86 smbios building code into a function >>>> smbios: remove dependency on x86 e820 tables >>>> smbios: pass ram size as a parameter to build smbios tables >>>> smbios: move smbios code into a common folder >>>> smbios: add smbios 3.0 support >>>> smbios: implement smbios support for mach-virt >>>> >>>> arch_init.c | 2 +- >>>> default-configs/arm-softmmu.mak | 1 + >>>> default-configs/i386-softmmu.mak | 1 + >>>> default-configs/x86_64-softmmu.mak | 1 + >>>> hw/Makefile.objs | 1 + >>>> hw/arm/virt.c | 24 +++++++++ >>>> hw/i386/Makefile.objs | 2 +- >>>> hw/i386/pc.c | 56 ++++++++++++++------- >>>> hw/i386/pc_piix.c | 5 +- >>>> hw/i386/pc_q35.c | 5 +- >>>> hw/smbios/Makefile.objs | 1 + >>>> hw/{i386 => smbios}/smbios.c | 96 +++++++++++++++++++++++------------- >>>> include/hw/arm/virt-acpi-build.h | 1 + >>>> include/hw/{i386 => smbios}/smbios.h | 42 ++++++++++++++-- >>>> tests/bios-tables-test.c | 2 +- >>>> vl.c | 2 +- >>>> 16 files changed, 179 insertions(+), 63 deletions(-) >>>> create mode 100644 hw/smbios/Makefile.objs >>>> rename hw/{i386 => smbios}/smbios.c (93%) >>>> rename include/hw/{i386 => smbios}/smbios.h (84%) >>>> >>> >>> I was hoping there would be a focused review from the subsystem >>> maintainers / feature owners for this patchset. Thus far only Shannon >>> commented on the series, plus I tested it and reported a small bug (with >>> a fix). >>> >>> Peter: if I review this series (and version 2 that Wei is already >>> planning to post, in order to address the notes above, plus anything >>> that further review might turn up), will my review suffice for you to >>> apply this series (after 2.4 is out)? >> >> Maybe. I haven't looked at the series at all, because it fell >> under "not for 2.4 and not something I know enough about to >> easily and quickly review" (and besides 5 out of 6 patches are >> not ARM-related but just about refactoring the x86 code). >> >> What is SMBIOS supposed to provide for ARM virt anyway? > > No clue. It is dictated by the most recent SMBIOS (3.0) spec. The actual > contents is dictated by asset management needs ("run dmidecode, look at > this and that in the output"). So this series should not be reviewed > primarily for SMBIOS payload, but for correctness of refactoring, and > well-formedness of the new tables. At least the latter point seems to be > confirmed by testing already. > >> I would have expected all the information a guest needs >> to be in the dtb or ACPI tables... > > SMBIOS is usually not needed by the guest OS or the firmware. The spec says, > > [...] the System Management BIOS Reference Specification addresses > how motherboard and system vendors present management information > about their products in a standard format by extending the BIOS > interface on Intel architecture systems. The information is intended > to allow generic instrumentation to deliver this data to management > applications that use CIM (the WBEM data model) or direct access and > eliminates the need for error prone operations such as probing system > hardware for presence detection. [...] > > (The Intel-specificity in the Introduction that I quoted is certainly a > bug in the spec, because under 1 Scope | 1.1 Supported processor > architectures, Aarch32 and Aarch64 too are listed.) > > As far as I know (although I have no hard evidence), SMBIOS 3.0 was > brought about by ARM platform needs. Aarch64 hardware that I have access > to seems to expose SMBIOS 3.0 indeed, therefore we should do the same in > qemu. > >> Is support for this all in the mainline kernel yet? > > Yes, it is. See (minimally) > > $ git log --reverse fc43026278^.. -- drivers/firmware/dmi* > > There are patches for arch/arm64/ and drivers/firmware/efi/ too (Ard > will know better, he wrote at least a few of them). > > The userspace tools are in a more messy state AFAICT. For example I'm > unable to locate a *git* repository for upstream dmidecode. But, Ivan > and Roy should know better (Cc'd). There is no git repo for dmidecode. Only CVS: http://savannah.nongnu.org/cvs/?group=dmidecode AFAIK, It supports SMBIOSv3 already. + Jean Delvare, who is creator of dmidecode, as I know. > > See also CARD-1779. > > Also, guest userspace is not QEMU's problem :) > > Thanks > Laszlo > -- Regards, Ivan Khoronzhuk