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 8BA7C8216D for ; Thu, 23 Feb 2017 09:22:54 -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 17AE97FB81; Thu, 23 Feb 2017 17:22:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-58.phx2.redhat.com [10.3.116.58]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1NHMrwU030114; Thu, 23 Feb 2017 12:22:54 -0500 To: Jordan Justen , edk2-devel-01 References: <20170223014814.10937-1-lersek@redhat.com> <148786919988.21275.4276476967482960800@jljusten-ivb> Cc: Ard Biesheuvel From: Laszlo Ersek Message-ID: <80d820c8-a378-01bf-e216-5cd51488cb22@redhat.com> Date: Thu, 23 Feb 2017 18:22:51 +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: <148786919988.21275.4276476967482960800@jljusten-ivb> 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.25]); Thu, 23 Feb 2017 17:22:55 +0000 (UTC) Subject: Re: [PATCH 00/12] ArmVirtPkg, OvmfPkg: factor out QemuFwCfgS3Lib 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: Thu, 23 Feb 2017 17:22:54 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 02/23/17 17:59, Jordan Justen wrote: > On 2017-02-22 17:48:02, Laszlo Ersek wrote: >> The new QemuFwCfgS3Lib class has two goals: >> >> (a) to query whether S3 support was enabled on the QEMU command line, >> >> (b) to save fw_cfg DMA operations that are to be replayed at S3 resume >> time, and more easily for the programmer than hacking Boot Script >> opcodes manually. >> >> Patches #1 through #5 introduce the new library class, with Base Null, >> PEI fw_cfg, and DXE fw_cfg instances, covering goal (a) in both >> ArmVirtPkg and OvmfPkg. >> >> Patch #6 retires QemuFwCfgS3Enabled() from QemuFwCfgLib (including >> library class and all instances), and switches all client modules to >> QemuFwCfgS3Lib. This separates S3 concerns from QemuFwCfgLib. > > 1-6 Reviewed-by: Jordan Justen Thank you! ... Because goal (b) is mentioned in some of the commit messages and source files of patches #1 through #6, I think it best if we only commit these patches all together, once they are fully reviewed. I'm in no rush with this :) (Unlike with WRITE_POINTER.) Cheers! Laszlo > >> Patches #7 through #10 cover goal (b) for all three library instances >> (at levels of support that are appropriate for each, of course). >> >> Patches #11 and #12 put the new library class to use in >> OvmfPkg/SmmControl2Dxe and OvmfPkg/AcpiPlatformDxe, eliminating such >> ACPI S3 Boot Script opcode hacking that is related to fw_cfg. (For >> OvmfPkg/SmmControl2Dxe, that's "most of it", for >> OvmfPkg/AcpiPlatformDxe, it's "all of it".) >> >> I tested: >> - ArmVirtQemu boot, >> - OVMF boot without SMM, with S3 enabled and disabled, using a Linux >> guest (and when S3 was enabled, I exercised it), >> - OVMF boot with SMM, with S3 enabled and disabled, using Linux and >> Windows guests, >> - and whenever S3 was enabled, I exercised it >> - and in the Windows guest, I tested VMGENID / WRITE_POINTER too. >> >> The diffstat looks scary, but it's due to comments, I promise. >> >> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=394 >> Repo: https://github.com/lersek/edk2.git >> Branch: fw_cfg_s3 >> >> NOTE: if you want to fetch & test the branch, you'll have to revert >> recent commit dc4c770763d0 ("BaseTools: add error check for Macro usage >> in the INF file", 2017-02-20) on top. It causes BaseTools to mis-build >> OVMF. I reported the regression on the list already, in that patch's >> thread. >> >> Cc: Ard Biesheuvel >> Cc: Jordan Justen >> >> Thanks >> Laszlo >> >> Laszlo Ersek (12): >> OvmfPkg: introduce QemuFwCfgS3Lib class >> OvmfPkg/QemuFwCfgS3Lib: add initial Base Null library instance >> OvmfPkg/QemuFwCfgS3Lib: add initial PEI and DXE fw_cfg library >> instances >> ArmVirtPkg: resolve QemuFwCfgS3Lib >> OvmfPkg: resolve QemuFwCfgS3Lib >> ArmVirtPkg, OvmfPkg: retire QemuFwCfgS3Enabled() from QemuFwCfgLib >> OvmfPkg/QemuFwCfgS3Lib: add boot script opcode generation APIs to >> libclass >> OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for Base Null instance >> OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for PEI fw_cfg instance >> OvmfPkg/QemuFwCfgS3Lib: implement opcode APIs for DXE fw_cfg instance >> OvmfPkg/SmmControl2Dxe: save fw_cfg boot script with QemuFwCfgS3Lib >> OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script with QemuFwCfgS3Lib >> >> ArmVirtPkg/ArmVirtQemu.dsc | 1 + >> ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 + >> ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 17 - >> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 2 +- >> OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 2 +- >> OvmfPkg/AcpiPlatformDxe/BootScript.c | 262 ++----- >> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 8 + >> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf | 2 +- >> OvmfPkg/Include/Library/QemuFwCfgLib.h | 14 - >> OvmfPkg/Include/Library/QemuFwCfgS3Lib.h | 357 +++++++++ >> OvmfPkg/Library/LockBoxLib/LockBoxDxe.c | 1 + >> OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf | 1 + >> OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h | 1 + >> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 + >> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 28 - >> OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf | 43 ++ >> OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf | 46 ++ >> OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf | 44 ++ >> OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c | 109 +++ >> OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3BasePei.c | 227 ++++++ >> OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Dxe.c | 791 ++++++++++++++++++++ >> OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Pei.c | 85 +++ >> OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c | 48 ++ >> OvmfPkg/OvmfPkg.dec | 4 + >> OvmfPkg/OvmfPkgIa32.dsc | 3 + >> OvmfPkg/OvmfPkgIa32X64.dsc | 3 + >> OvmfPkg/OvmfPkgX64.dsc | 3 + >> OvmfPkg/PlatformPei/Platform.c | 1 + >> OvmfPkg/PlatformPei/PlatformPei.inf | 1 + >> OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 224 ++---- >> OvmfPkg/SmmControl2Dxe/SmiFeatures.h | 5 +- >> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c | 6 +- >> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf | 1 + >> 33 files changed, 1917 insertions(+), 425 deletions(-) >> create mode 100644 OvmfPkg/Include/Library/QemuFwCfgS3Lib.h >> create mode 100644 OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf >> create mode 100644 OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf >> create mode 100644 OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf >> create mode 100644 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Base.c >> create mode 100644 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3BasePei.c >> create mode 100644 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Dxe.c >> create mode 100644 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3Pei.c >> create mode 100644 OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c >> >> -- >> 2.9.3 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel