From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.3558.1611613065293302251 for ; Mon, 25 Jan 2021 14:17:46 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=iesXsztT; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1611613064; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aRW4r5QzRRvgsAkSaYgQbqLzg+B8MgkqhZnwtkOF8eY=; b=iesXsztTHZe5BhbSWrnJopiLqbVWkL+UtfO8GTnmcklztO6FkgSL7U8Q+YOFNdh149Y6lE 5l0uOWo9+m81qDkMlvFyjyDC1JV7MwFyTR+pnF837ce/5a4kbsV7wwRA4CW/zbmvreggK5 a9r7ascbkaXgLOjYokNDXSwXskL6vcU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-593-I7hCYtaeNl638wOVi04FpA-1; Mon, 25 Jan 2021 17:17:40 -0500 X-MC-Unique: I7hCYtaeNl638wOVi04FpA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7360E18C8C02; Mon, 25 Jan 2021 22:17:39 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-67.ams2.redhat.com [10.36.112.67]) by smtp.corp.redhat.com (Postfix) with ESMTP id 012F619C47; Mon, 25 Jan 2021 22:17:37 +0000 (UTC) Subject: Re: [PATCH v2] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Use physical address with SEV-ES To: Tom Lendacky , devel@edk2.groups.io Cc: Brijesh Singh , Jordan Justen , Ard Biesheuvel References: <84a5f9161541db5aa3b57c96b737afbcb4b6189d.1611410263.git.thomas.lendacky@amd.com> <02e1643a-397f-b24d-fb86-f80e868fbb77@amd.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 25 Jan 2021 23:17:37 +0100 MIME-Version: 1.0 In-Reply-To: <02e1643a-397f-b24d-fb86-f80e868fbb77@amd.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/25/21 14:55, Tom Lendacky wrote: > On 1/23/21 7:57 AM, Tom Lendacky wrote: >> From: Tom Lendacky >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3183 >> >> Under SEV-ES, a write to the flash device is done using a direct VMGEXIT >> to perform an MMIO write. The address provided to the MMIO write must be >> the physical address of the MMIO write destitnation. During boot, OVMF >> runs with an identity mapped pagetable structure so that VA == PA and the >> VMGEXIT MMIO write destination is just the virtual address of the flash >> area address being written. >> >> However, when the UEFI SetVitualAddressMap() API is invoked, an identity > > s/SetVitualAddressMap/SetVirtualAddressMap/ > > I can fix that if another version is required, otherwise, can it be fixed > on commit? Yes, I'll fix it on merge. I made the exact same typo in my v1 comment -- and that was because I copied the mistyped string from the BZ :) (See c#0, c#1.) Thanks! Laszlo > > Thanks, > Tom > >> mapped pagetable structure may not be in place and using the virtual >> address for the flash area address is no longer valid. This results in >> writes to the flash not being performed successfully. This can be seen >> by attempting to change the boot order under Linux. The update will >> appear to be performed, based on the output of the command. But rebooting >> the guest will show that the new boot order has not been set. >> >> To remedy this, save the value of the flash base physical address before >> converting the address as part of SetVirtualAddressMap(). The physical >> address can then be calculated by obtaining the offset of the MMIO target >> virtual address relative to the flash base virtual address and adding that >> to the original flash base physical address. The resulting value produces >> a successful MMIO write during runtime services. >> >> Fixes: 437eb3f7a8db7681afe0e6064d3a8edb12abb766 >> Cc: Jordan Justen >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Signed-off-by: Tom Lendacky >> --- >> .../QemuFlashDxe.c | 20 ++++++++++++++++++- >> 1 file changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c >> index 1b0742967f71..d303b0078b08 100644 >> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c >> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c >> @@ -16,11 +16,17 @@ >> >> #include "QemuFlash.h" >> >> +STATIC EFI_PHYSICAL_ADDRESS mSevEsFlashPhysBase; >> + >> VOID >> QemuFlashConvertPointers ( >> VOID >> ) >> { >> + if (MemEncryptSevEsIsEnabled ()) { >> + mSevEsFlashPhysBase = (UINTN) mFlashBase; >> + } >> + >> EfiConvertPointer (0x0, (VOID **) &mFlashBase); >> } >> >> @@ -52,11 +58,23 @@ QemuFlashPtrWrite ( >> if (MemEncryptSevEsIsEnabled ()) { >> MSR_SEV_ES_GHCB_REGISTER Msr; >> GHCB *Ghcb; >> + EFI_PHYSICAL_ADDRESS PhysAddr; >> BOOLEAN InterruptState; >> >> Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB); >> Ghcb = Msr.Ghcb; >> >> + // >> + // The MMIO write needs to be to the physical address of the flash pointer. >> + // Since this service is available as part of the EFI runtime services, >> + // account for a non-identity mapped VA after SetVitualAddressMap(). >> + // >> + if (mSevEsFlashPhysBase == 0) { >> + PhysAddr = (UINTN) Ptr; >> + } else { >> + PhysAddr = mSevEsFlashPhysBase + (Ptr - mFlashBase); >> + } >> + >> // >> // Writing to flash is emulated by the hypervisor through the use of write >> // protection. This won't work for an SEV-ES guest because the write won't >> @@ -68,7 +86,7 @@ QemuFlashPtrWrite ( >> Ghcb->SharedBuffer[0] = Value; >> Ghcb->SaveArea.SwScratch = (UINT64) (UINTN) Ghcb->SharedBuffer; >> VmgSetOffsetValid (Ghcb, GhcbSwScratch); >> - VmgExit (Ghcb, SVM_EXIT_MMIO_WRITE, (UINT64) (UINTN) Ptr, 1); >> + VmgExit (Ghcb, SVM_EXIT_MMIO_WRITE, PhysAddr, 1); >> VmgDone (Ghcb, InterruptState); >> } else { >> *Ptr = Value; >> >