From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (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 31AA8211F8875 for ; Wed, 27 Jun 2018 09:59:23 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EE232BF578; Wed, 27 Jun 2018 16:59:22 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-54.rdu2.redhat.com [10.10.121.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id D72F22026D5B; Wed, 27 Jun 2018 16:59:21 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org Cc: Tom Lendacky , Star Zeng , Eric Dong , "Jordan Justen (Intel address)" References: <1530042365-9979-1-git-send-email-brijesh.singh@amd.com> From: Laszlo Ersek Message-ID: <272c4a0f-fcc1-2899-e31d-a3207feb51ed@redhat.com> Date: Wed, 27 Jun 2018 18:59:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 27 Jun 2018 16:59:23 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 27 Jun 2018 16:59:23 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [RFC PATCH 1/1] OvmfPkg/QemuFlash: Fix Runtime variable access when SEV is enabled X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jun 2018 16:59:24 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 06/27/18 18:34, Brijesh Singh wrote: > On 06/27/2018 07:54 AM, Laszlo Ersek wrote: >> On 06/26/18 21:46, Brijesh Singh wrote: >>> After that, any access >>> to the flash will end up going through the encryption engine. I did try >>> hacking EDK2 to restore the C-bit >> >> (I continue to be annoyed that the memory encryption bit is not exposed >> in the GCD memory space attributes explicitly.) >> >>> but that was not sufficient because UEFI >>> runtime services are mapped as "encrypted" in OS page table >> >> What do you mean here? Runtime services *code* or runtime services >> *data*? Code must obviously be remain encrypted (otherwise we cannot >> execute it in SEV). Runtime Services Data should also be mapped as >> encrypted (it is normal RAM that is not used for guest<->hypervisor >> exchange). > > Sorry, I was meaning to say both the "code" and "data" are mapped as > encrypted by the OS. > >>> hence we end up accessing the flash as encrypted when OS requests to >>> update the variables. >> >> I don't understand the "hence" here; I don't see how the implication >> follows. runtime services code and data should be encrypted. Runtime >> MMIO should be un-encrypted. >> >> Ohh, wait, in MarkMemoryRangeForRuntimeAccess(), we use >> "EfiGcdMemoryTypeSystemMemory". I don't have a clue why that is a good >> idea. That should have been EfiGcdMemoryTypeMemoryMappedIo. > > Right, the memory is marked as 'system ram' and not 'mmio'. > Just to experiment, I did try changing it to 'mmio' to see if OS will > map this  region as "unencrypted" but ovmf fails with below error > message after changing it from systemRAM->mmio > > ConvertPages: failed to find range FFC00000 - FFFFFFFF > ASSERT_EFI_ERROR (Status = Not Found) > ASSERT [FvbServicesRuntimeDxe] > /home/amd/workdir/upstream/edk2/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServie.c(864): > !EFI_ERROR (Status) This error occurs because (I think) you modified only the AddMemorySpace call. If you change the GCD type on that, then please update the subsequent AllocatePages as well, from EfiRuntimeServicesData to EfiMemoryMappedIO. The spec says about the latter enum constant, "Used by system firmware to request that a memory-mapped IO region be mapped by the OS to a virtual address so it can be accessed by EFI runtime services." It seems appropriate (and I'm a bit confused why we haven't used the MMIO GCD and UEFI enum values for the memory type, all this time.) > Since this efi runtime data is mapped as C=1 by the OS, hence when OS > asks efi to update the runtime variable we end up accessing the memory > region with C=1 (runtime services are executed using OS pagetable). Indeed. (And, this is only a problem when SMM is not used, i.e. when the full variable driver stack is non-SMM, just DXE. In the SMM case, the SMM page tables are used, and the OS cannot interfere with that.) Anyway, in the pure DXE / runtime driver case, do you think a guest kernel patch will be necessary too? Perhaps if you change the UEFI memmap entry type (see AllocatePages above) to MMIO, then the guest kernel could technically honor that. Thanks Laszlo