From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 21C93820D5 for ; Wed, 8 Feb 2017 09:34:04 -0800 (PST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AD61F804F0; Wed, 8 Feb 2017 17:34:04 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-31.phx2.redhat.com [10.3.116.31]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v18HY2H5001226; Wed, 8 Feb 2017 12:34:02 -0500 To: Jiewen Yao , edk2-devel@ml01.01.org References: <1486571434-20000-1-git-send-email-jiewen.yao@intel.com> Cc: Michael D Kinney , Liming Gao , Feng Tian , Star Zeng , "Jordan Justen (Intel address)" From: Laszlo Ersek Message-ID: <9a8b8dec-d2e6-22f0-f1d7-29167a4838bc@redhat.com> Date: Wed, 8 Feb 2017 18:34:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <1486571434-20000-1-git-send-email-jiewen.yao@intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Wed, 08 Feb 2017 17:34:04 +0000 (UTC) Subject: Re: [PATCH 00/12] Add SmiHandlerProfile feature X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Feb 2017 17:34:04 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit CC Jordan On 02/08/17 17:30, Jiewen Yao wrote: > This series patch add SMI handler profile. > > The purpose of SMI handler profile is to add the capability to > dump all SMI handlers produced by the firmware in a given boot. > The SMI handlers here include > 1) Root SMI handlers registered with SMST->SmiHandlerRegister by SmmCore. > 2) GUID SMI handlers registered with SMST->SmiHandlerRegister by SmmCore. > 3) Hardware SMI handlers registered with SMM_XXX_DISPATCH_PROTOCOL->Register > by SmmChildDispatcher module. > > The final log is an XML format log, including all SMM image name, all SMI > handlers name, type, location (source file and line number), and the caller > who registeres the handler. > > We enabled Quark platform to show how to add support in SmmChildDispatcher. Thanks for the CC. This module seems quite large, and OVMF (to my knowledge) doesn't include any special handlers. (Nothing from category (3), and I couldn't say if anything from category (2)). Do you think this module would be useful to enable in OVMF, conditionally, with -D SMI_HANDLER_PROFILE_ENABLE? Beyond the question of its utility to OVMF developers and users, requiring a separate UEFI app to dump the profile, presumably from the UEFI shell, is not really flexible for OVMF. We can't / don't build applications into the firmware image (beyond the shell and UiApp), so users would have to build a virtual disk image with this app, or we'd have to provide scripts for the same. For now, I believe we don't wish to enable this in OVMF. If you CC'd me for regression-testing PiSmmCore, as it is built into OVMF, with the default PcdSmiHandlerProfilePropertyMask=0 value, then I respectfully point you to the following article: https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt Please set up a KVM environment as documented there, and adopt it for regular SMM testing :) My strong hope is that, by adopting KVM as a regular test platform, Intel developers won't only catch OVMF regressions before they post patches, but they should also see their own productivity increase. I assume it must be much easier to click "Play" in the Virt-Manager GUI and to boot to an OS, on top of the new firmware, in seconds, than to flash firmware to a physical board. It does not replace physical board testing, but should catch a number of errors more gracefully. The test cases I wrote up include UEFI variable testing and ACPI S3. The guest OSes covered are Fedora 25 and Windows 10. Please give it a look. Thanks! Laszlo > > NOTE: This SMI handler profile is a *DEBUG* feature only, to help platform > developer or test engineer to audit the SMI handlers. Please do not include > it in the final *RELEASE* image. > > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Feng Tian > Cc: Star Zeng > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao > > Jiewen Yao (12): > MdePkg/Include: Add SmiHandlerProfileLib.h > MdePkg/SmiHandlerProfileLibNull: Add NULL instance. > MdePkg/dsc: add SmiHandlerProfileLib to dsc. > MdeModulePkg/include: Add SmiHandlerProfile header file. > MdeModulePkg/dec: Add PcdSmiHandlerProfilePropertyMask. > MdeModulePkg/SmmSmiHandlerProfileLib: Add SMM instance. > MdeModulePkg/PiSmmCore: Add SmiHandlerProfile support. > MdeModulePkg/App: Add SmiHandlerProfile dump app. > MdeModulePkg/dsc: add SmiHandlerProfile to dsc. > BaseTool/Script: Add SmiHandleProfile OS tool to get symbol. > QuarkSocPkg/SmmChildDispatch: Add SmiHandlerProfile support. > QuarkPlatformPkg: enable SmiHandlerProfile. > > BaseTools/Scripts/SmiHandlerProfileSymbolGen.py | 351 ++++++ > MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c | 685 +++++++++++ > MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf | 65 + > MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.uni | 22 + > MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfoExtra.uni | 19 + > MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 4 +- > MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 89 +- > MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 17 +- > MdeModulePkg/Core/PiSmmCore/Smi.c | 46 +- > MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c | 1261 ++++++++++++++++++++ > MdeModulePkg/Include/Guid/SmiHandlerProfile.h | 177 +++ > MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c | 106 ++ > MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.inf | 46 + > MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.uni | 21 + > MdeModulePkg/MdeModulePkg.dec | 11 +- > MdeModulePkg/MdeModulePkg.dsc | 2 + > MdePkg/Include/Library/SmiHandlerProfileLib.h | 81 ++ > MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c | 72 ++ > MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.inf | 36 + > MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.uni | 21 + > MdePkg/MdePkg.dec | 5 +- > MdePkg/MdePkg.dsc | 3 +- > QuarkPlatformPkg/Quark.dsc | 16 +- > QuarkPlatformPkg/Quark.fdf | 3 +- > QuarkPlatformPkg/QuarkMin.dsc | 5 +- > QuarkPlatformPkg/Readme.md | 29 +- > QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c | 19 +- > QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmDispatcher.inf | 3 +- > 28 files changed, 3159 insertions(+), 56 deletions(-) > create mode 100644 BaseTools/Scripts/SmiHandlerProfileSymbolGen.py > create mode 100644 MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c > create mode 100644 MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf > create mode 100644 MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.uni > create mode 100644 MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfoExtra.uni > create mode 100644 MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c > create mode 100644 MdeModulePkg/Include/Guid/SmiHandlerProfile.h > create mode 100644 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c > create mode 100644 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.inf > create mode 100644 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.uni > create mode 100644 MdePkg/Include/Library/SmiHandlerProfileLib.h > create mode 100644 MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c > create mode 100644 MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.inf > create mode 100644 MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.uni >