From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <lersek@redhat.com> 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 D40268034A for <edk2-devel@ml01.01.org>; Wed, 8 Mar 2017 00:40:50 -0800 (PST) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (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 1B015C04BD28; Wed, 8 Mar 2017 08:40:51 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-70.phx2.redhat.com [10.3.116.70]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v288emfh005808; Wed, 8 Mar 2017 03:40:49 -0500 To: Brijesh Singh <brijesh.ksingh@gmail.com> References: <148884284887.29188.7643544710695103939.stgit@brijesh-build-machine> <148884286215.29188.1084675072356724555.stgit@brijesh-build-machine> <da4ba777-b317-8932-b7a3-197fa5d8ad88@redhat.com> <CA+HCGMYYr-Esu3WhincUFijJ0iCLpz_CO2YXvpvEoc_CjC990w@mail.gmail.com> <dd9436dc-415c-9fab-081c-39dd2cd71fd5@redhat.com> <CA+HCGMaFREiwVM=6GRNpHEzr9a+527rUVQPi1BcqbsquHcsEdQ@mail.gmail.com> Cc: jordan.l.justen@intel.com, edk2-devel@ml01.01.org, Leo Duran <leo.duran@amd.com>, brijesh.singh@amd.com, Tom Lendacky <Thomas.Lendacky@amd.com> From: Laszlo Ersek <lersek@redhat.com> Message-ID: <d6ec6683-8c20-f874-dda0-500426510dbb@redhat.com> Date: Wed, 8 Mar 2017 09:40:46 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <CA+HCGMaFREiwVM=6GRNpHEzr9a+527rUVQPi1BcqbsquHcsEdQ@mail.gmail.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 08 Mar 2017 08:40:51 +0000 (UTC) Subject: Re: [RFC PATCH v1 2/5] OvmfPkg/MemcryptSevLib: Add SEV helper library X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development <edk2-devel.lists.01.org> List-Unsubscribe: <https://lists.01.org/mailman/options/edk2-devel>, <mailto:edk2-devel-request@lists.01.org?subject=unsubscribe> List-Archive: <http://lists.01.org/pipermail/edk2-devel/> List-Post: <mailto:edk2-devel@lists.01.org> List-Help: <mailto:edk2-devel-request@lists.01.org?subject=help> List-Subscribe: <https://lists.01.org/mailman/listinfo/edk2-devel>, <mailto:edk2-devel-request@lists.01.org?subject=subscribe> X-List-Received-Date: Wed, 08 Mar 2017 08:40:51 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/07/17 23:36, Brijesh Singh wrote: > > > On Tue, Mar 7, 2017 at 4:08 PM, Laszlo Ersek <lersek@redhat.com > <mailto:lersek@redhat.com>> wrote: > > On 03/07/17 20:14, Brijesh Singh wrote: > > On Tue, Mar 7, 2017 at 11:06 AM, Laszlo Ersek <lersek@redhat.com > <mailto:lersek@redhat.com>> wrote: > I think SevActive() and SevInitialize() should become part of > PlatformPei only (if that's possible). > > The upcoming PTE massaging functions could become part of the DMA lib > stuff that you mention (as functions with external linkage), and then > you could pull the DMA lib into QemuVideoDxe just to make these > functions available. > > Presently the suggested functions don't seem to justify two (or even > one) new libclass. > > > > I think I should be able to accomate SevInitialize() and SevActive() > function inside the PlatformPei. I will drop MemcryptSevLib library in > next rev. > > I will go with your idea for adding PTE massaging function directly > inside the DMA library and will link that into QemuVideoDxe. > > Only part which I have not yet figured out, how to deal with Qemu > FW_CFG DMA support, I believe some of FW_CFG DMA read and write > happens fairly early (PEI stage). That's right, off the top of my head, minimally PlatformPei uses fw_cfg heavily during PEI. > The PTE massaging code may need to > allocate memory and not sure how to allocate dynamic memory in early stages. > Any pointers ? You can use MemoryAllocationLib functions for that (such as AllocatePool() and AllocatePages()). The OVMF DSC files resolve the lib class for the PEI phase like this: MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf and the PeiMemoryAllocationLib instance maps those functions to the PEI services. A few important things about this: - AllocatePool() works up to only ~64KB in size, and the allocation is backed by a new HOB. Generally speaking, the HOB may be moved to a different spot in memory before entering the DXE phase, so pointers returned by such AllocatePool() calls (in PEI) are not safe to dereference in the DXE phase. - FreePool() does nothing, the allocated memory (the HOB, see above) is only released when the guest OS starts (and it drops all boot services data type memory). - AllocatePages() works as it says on the tin, and the pointer returned by it is safe to dereference in DXE. - FreePages() however is again a no-op, it practically leaks the memory, and only the guest OS will be able to release it (see FreePool() above). One workaround for this could be to stash the address of the PEI-phase allocation in a GUID HOB or a PCD, and then let some DXE driver in the DXE phase release the memory with gBS->FreePages(). I'm not sure though if this complexity is worth it. - Note that it is PlatformPei itself that installs the permanent PEI RAM. Before that happens, PEIMs (including PlatformPei itself) can only allocate memory from the temporary SEC/PEI heap, which is very very small, and only AllocatePool() would work at that point (AllocatePages() wouldn't). However, if you place the AllocatePages() function call after PublishPeiMemory(), then things should work. As far as I can see, you added MemcryptSevInitialize() to the end of InitializePlatform(); allocating pages at that point should be fine. - During S3 resume, a different (pre-reserved) memory area is used as permanent PEI RAM, which is quite smaller than the one used during normal boot. It means that, if you need a lot of memory for setting up SEV during S3 resume, AllocatePages() might run out of memory, and we might have to resize the pre-reservation mentioned above. - If you could reasonably bound the allocation size with a constant, it might be simplest to use static arrays / variables. Those would be dropped as soon as the PEI phase was exited. As one quirk however, you should not rely on such variables being zero-initialized during S3 resume. Thanks Laszlo