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 C8B6F82215 for ; Tue, 21 Feb 2017 00:47:41 -0800 (PST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (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 3727F804E5; Tue, 21 Feb 2017 08:47:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-47.phx2.redhat.com [10.3.116.47]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L8ldw4032469; Tue, 21 Feb 2017 03:47:40 -0500 To: "Yao, Jiewen" , "'edk2-devel@ml01.01.org'" References: <1486571434-20000-1-git-send-email-jiewen.yao@intel.com> <9a8b8dec-d2e6-22f0-f1d7-29167a4838bc@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C503A8EB18E@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8F2685@shsmsx102.ccr.corp.intel.com> Cc: "Kinney, Michael D" , "Justen, Jordan L" , "Tian, Feng" , "Zeng, Star" , "Gao, Liming" From: Laszlo Ersek Message-ID: Date: Tue, 21 Feb 2017 09:47:38 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A8F2685@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 21 Feb 2017 08:47:42 +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: Tue, 21 Feb 2017 08:47:42 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 02/21/17 08:06, Yao, Jiewen wrote: > HI Laszlo > > We have enabled OVMF KVM, and validated this patch. Fantastic!!! Thank you for making the effort! > There is no impact to OVMF. Fedora Guest boot and S3 passes. Hooray! :) Thanks! Laszlo > > > > Thank you > > Yao Jiewen > > > > > > *From:*Yao, Jiewen > *Sent:* Thursday, February 9, 2017 1:58 AM > *To:* Yao, Jiewen ; edk2-devel@ml01.01.org > *Cc:* Kinney, Michael D ; Justen, Jordan L > ; Tian, Feng ; Zeng, > Star ; Gao, Liming ; Laszlo > Ersek (lersek@redhat.com) > *Subject:* RE: [edk2] [PATCH 00/12] Add SmiHandlerProfile feature > > > > Hi Laszlo > > Thanks for the comment. > > > > To clarify something: > > > > 1) I did not enable SMI handler profile for OVMF because I notice > the OVMF just use very few SMI handlers, and it does not have > SmmChildDispatcher. Just like what you mentioned. > > > > 2) I forget to mention: I did regression test for OVMF IA32 and > IA32X64 with default SmiHandlerProfile is DISABLED. > > I boot to UEFI SHELL only, and I do not enter OS. > > I think the risk is very low, because if SMI handler profile is > disabled, there is no code logic change. > > > > VOID > > SmmCoreInitializeSmiHandlerProfile ( > > VOID > > ) > > { > > EFI_STATUS Status; > > VOID *Registration; > > EFI_HANDLE Handle; > > > > if((PcdGet8 (PcdSmiHandlerProfilePropertyMask) & 0x1) != 0) { > > InsertTailList (&mRootSmiEntryList, &mRootSmiEntry.AllEntries); > > > > Status = gSmst->SmmRegisterProtocolNotify ( > > &gEfiSmmReadyToLockProtocolGuid, > > SmmReadyToLockInSmiHandlerProfile, > > &Registration > > ); > > ASSERT_EFI_ERROR (Status); > > > > Handle = NULL; > > Status = gSmst->SmmInstallProtocolInterface ( > > &Handle, > > &gSmiHandlerProfileGuid, > > EFI_NATIVE_INTERFACE, > > &mSmiHandlerProfile > > ); > > ASSERT_EFI_ERROR (Status); > > } > > } > > > > I do understand your concern, because OVMF has some special feature > which might be different with real hardware. > > > > At same time, I have to mention that, it is very hard to setup KVM in > Linux. I did try to follow your description before on NX. > > I spent one week, but still fail to launch the final image on my side, I > have no idea on what is wrong. That was a horrible experience. > > > > I will have a try to follow your new document. > > If it can be setup successfully, easily and fortunately, I do want to > run it. > > > > Thank you > > Yao Jiewen > > > > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek > Sent: Wednesday, February 8, 2017 9:34 AM > To: Yao, Jiewen >; edk2-devel@ml01.01.org > > Cc: Kinney, Michael D >; Justen, Jordan L >; Tian, Feng >; Zeng, Star >; Gao, Liming > > Subject: Re: [edk2] [PATCH 00/12] Add SmiHandlerProfile feature > > 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.i >> nf create mode 100644 >> MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.u >> ni create mode 100644 >> MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfoEx >> tra.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.i >> nf create mode 100644 >> MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.u >> ni 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 >> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >