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 90ACC211AE8C3 for ; Wed, 9 Jan 2019 07:05:05 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EFBE42D7E4; Wed, 9 Jan 2019 15:05:04 +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 39D245D9C9; Wed, 9 Jan 2019 15:04:59 +0000 (UTC) To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , 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: <1ece7e1e-26de-bf6b-55ea-b1e12bce3788@redhat.com> Date: Wed, 9 Jan 2019 16:04:59 +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: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 09 Jan 2019 15:05:05 +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 15:05:05 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/09/19 11:28, Ard Biesheuvel wrote: > On Wed, 9 Jan 2019 at 10:44, Laszlo Ersek wrote: >> 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. :) >> > > At the moment, you will need both resolutions for DXE_SMM_DRIVERS > globally. Based on the outcome of the review of this series, this is > something we will need to clean up going forward, but currently, even > the drivers that are updated to use MmServicesTableLib pull in > libraries that depend on SmmServicesTableLib. > > This should be a rather straight-forward search/replace operation > [famous last words], and I can commit to dedicating some of my time to > getting this fixed throughout, at least to the point where modules > that consume MmServicesTableLib no longer have to supply a resolution > for SmmServicesTableLib as well. > > So, I will include a patch in the next revision of the series. Great, thank you. This is exactly the info I needed. > In the mean time, the hunk below should suffice to complete your regression > testing. > > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -391,6 +391,7 @@ > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > !ifdef $(DEBUG_ON_SERIAL_PORT) > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > !else > I'll replicate this to the other two DSC files [*], and then retry the tests. [*] SMM in OVMF has a non-intuitive restriction, in relation to X64 PEI. SMM "just works" in the IA32 and IA32X64 builds, however, in the X64 build, one has to disable S3 support on the QEMU command line, or else we hang the boot intentionally. See commit 5133d1f1d297 ("OvmfPkg: replace README fine print about X64 SMM S3 with PlatformPei check", 2015-11-30). For this reason, the IA32X64 build is considered the most-featureful, if -D SMM_REQUIRE is desired. For those that insist on the X64 build nevertheless, OvmfPkg/README documents the QEMU option that disables S3, on the Q35 machine type, which is required for SMM anyway: -global ICH9-LPC.disable_s3=1 When using libvirt, the matching knob is: https://libvirt.org/formatdomain.html#elementsPowerManagement Personally, I focus my SMM testing on IA32 and IA32X64; I almost never test SMM in the X64 build. This is because S3 has historically proved very effective at triggering multiprocessing bugs in core SMM code, and in general UefiCpuPkg infrastructure. Thus, my SMM regression tests: https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt always include S3 suspend/resume, and that precludes the X64 build of OVMF. ... Sorry about the wall of text, I just wanted to explain why precisely the short hunk you pasted is unhelpful in this case. Obviously, it does answer my question, I can copy the class resolution to the other two DSC files, and in the final OvmfPkg patch, we should update all three DSC files. I'll be back with test results later. Thanks! Laszlo