From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org 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 E41A2211B508D for ; Wed, 9 Jan 2019 01:44:08 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6683D1B2855; Wed, 9 Jan 2019 09:44:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-11.rdu2.redhat.com [10.10.120.11]) by smtp.corp.redhat.com (Postfix) with ESMTP id 87A891057049; Wed, 9 Jan 2019 09:44:04 +0000 (UTC) To: Ard Biesheuvel , edk2-devel@lists.01.org Cc: Leif Lindholm , Michael D Kinney , Liming Gao , Jian J Wang , Hao Wu , Jagadeesh Ujja , Achin Gupta , Thomas Panakamattam Abraham , Sami Mujawar References: <20190103182825.32231-1-ard.biesheuvel@linaro.org> From: Laszlo Ersek Message-ID: Date: Wed, 9 Jan 2019 10:44:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190103182825.32231-1-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 09 Jan 2019 09:44:07 +0000 (UTC) Subject: Re: [PATCH 0/6] implement standalone MM versions of the variable runtime drivers X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Jan 2019 09:44:09 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/03/19 19:28, Ard Biesheuvel wrote: > This series proposed an alternative approach to the series sent out by > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > special PCD, as well as some other if() conditionals. > > The primary difference is that this series defines and implements > MmServicesTableLib in such a way that the traditional SMM drivers can > use it as well. This is appropriate, considering that the PI spec has > rebranded traditional SMM as one implementation of the generic MM > framework. > > Patch #1 is based on Jagadeesh's patch, and introduces the > MmServicesTableLib library class, but for all SMM flavours, not only > for standalone MM. > > Patch #2 implements MmServicesTableLib for traditional SMM > implementations. > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > driver that invoke boot services are separated from the core SMM > pieces. > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM > environment. > > Patches #5 and #6 do the same, respectively, for the variable runtime > driver. > > This approach minimizes the delta, and thus the maintenance burden, > between the traditional SMM and standalone MM drivers, while not > resorting to runtime checks or other conditionals in the code to > implement logic that should be decided at build time. > > Note that this series only covers part of the work contributed by > Jagadeesh. This series focuses on the MdePkg and MdeModulePkg changes > that affect shared code. > > Cc: Laszlo Ersek > Cc: Leif Lindholm > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Jian J Wang > Cc: Hao Wu > Cc: Jagadeesh Ujja > Cc: Achin Gupta > Cc: Thomas Panakamattam Abraham > Cc: Sami Mujawar I tried building this, on top of commit a53a888de8f5: build \ -a IA32 \ -p OvmfPkg/OvmfPkgIa32.dsc \ -t GCC48 \ -b NOOPT \ -D HTTP_BOOT_ENABLE \ -D NETWORK_IP6_ENABLE \ -D SECURE_BOOT_ENABLE \ -D SMM_REQUIRE \ -D TLS_ENABLE \ --cmd-len=65536 \ --hash \ --genfds-multi-thread but it failed with: > OvmfPkg/OvmfPkgIa32.dsc(...): error 4000: Instance of library class [MmServicesTableLib] is not found > in [MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf] [IA32] > consumed by module [MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf] You did mention earlier that adding new lib class resolutions to several x86 DSC files would be necessary, so this is not unexpected. Can you please insert such a patch for OvmfPkg between patches #2 and #3? I've looked at the current OVMF DSC files, and SmmServicesTableLib is resolved for two module types, > [LibraryClasses.common.DXE_SMM_DRIVER] > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > > [LibraryClasses.common.SMM_CORE] > SmmServicesTableLib|MdeModulePkg/Library/PiSmmCoreSmmServicesTableLib/PiSmmCoreSmmServicesTableLib.inf I assume it should be enough, for this series, to update the DXE_SMM_DRIVER resolution only, and to leave SMM_CORE alone. (Because, my understanding is that the current, x86 specific MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf module, of type SMM_CORE, will not be refactored; instead, it is entirely supplanted -- in the affected platforms -- by the StandaloneMmPkg/Core/StandaloneMmCore.inf module, which is of type MM_CORE_STANDALONE.) But, it's still not clear to me (without trying) whether I should resolve MmServicesTableLib for DXE_SMM_DRIVER in addition to SmmServicesTableLib, or in its place. I'd prefer not experimenting with this myself; I'd just like to apply the series, and build & test it. :) Thanks, Laszlo