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.web08.14554.1621223217399123167 for ; Sun, 16 May 2021 20:46:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=LWoBcrxR; 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=1621223216; 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=t11mEf2/1F24bnyuGz3FMFbYIp2XRhEBPqOPwemMfhc=; b=LWoBcrxRFjMALre+hyx0Lgp6PTiiwo6N5/iStne+SWQqKAgYr1ry9rbwJ9UbBD3T7AuWBF ltcFoZPPSxkmkB7o9VhXvi7m2zn94v4E+OKdnxHoEPX4vow+iRmHwY0FEAyDDbo9NNU69s QOUImxni5B2FlEUEkKLRJUuAg5oQaMg= 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-145-LMOw4LY8P4CUdsZyCNFgQg-1; Sun, 16 May 2021 23:46:52 -0400 X-MC-Unique: LMOw4LY8P4CUdsZyCNFgQg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id F2202800FF0; Mon, 17 May 2021 03:46:50 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-22.ams2.redhat.com [10.36.112.22]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5243E5B683; Mon, 17 May 2021 03:46:47 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 13/13] OvmfPkg/BaseMemEncryptSevLib: remove Flush parameter To: devel@edk2.groups.io, brijesh.singh@amd.com Cc: James Bottomley , Min Xu , Jiewen Yao , Tom Lendacky , Jordan Justen , Ard Biesheuvel , Erdem Aktas References: <20210512234615.1726-1-brijesh.singh@amd.com> <20210512234615.1726-14-brijesh.singh@amd.com> From: "Laszlo Ersek" Message-ID: <78213012-8bcd-0a6a-fda8-44812c2df2f5@redhat.com> Date: Mon, 17 May 2021 05:46:45 +0200 MIME-Version: 1.0 In-Reply-To: <20210512234615.1726-14-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 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 05/13/21 01:46, Brijesh Singh wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275 > > The Flush parameter is used to provide a hint whether the specified range > is Mmio address. Now that we have a dedicated helper to clear the > memory encryption mask for the Mmio address range, its safe to remove the > Flush parameter from MemEncryptSev{Set,Clear}PageEncMask(). > > Since the address specified in the MemEncryptSev{Set,Clear}PageEncMask() > points to a system RAM, thus a cache flush is required during the > encryption mask update. > > Cc: James Bottomley > Cc: Min Xu > Cc: Jiewen Yao > Cc: Tom Lendacky > Cc: Jordan Justen > Cc: Ard Biesheuvel > Cc: Laszlo Ersek > Cc: Erdem Aktas > Signed-off-by: Brijesh Singh > --- > OvmfPkg/Include/Library/MemEncryptSevLib.h | 10 ++-------- > .../BaseMemEncryptSevLib/X64/VirtualMemory.h | 10 ++-------- > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 3 +-- > OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 6 ++---- > .../BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c | 10 ++-------- > .../BaseMemEncryptSevLib/X64/MemEncryptSevLib.c | 16 ++++------------ > .../X64/PeiDxeVirtualMemory.c | 14 ++++---------- > .../BaseMemEncryptSevLib/X64/SecVirtualMemory.c | 8 ++------ > .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 3 +-- > OvmfPkg/PlatformPei/AmdSev.c | 3 +-- > 10 files changed, 21 insertions(+), 62 deletions(-) Reviewed-by: Laszlo Ersek (v2 patches 10 through 12 are OK as well) Thanks Laszlo > > diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h > index b91490d5d44d..76d06c206c8b 100644 > --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h > +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h > @@ -100,8 +100,6 @@ MemEncryptSevIsEnabled ( > address of a memory region. > @param[in] NumPages The number of pages from start memory > region. > - @param[in] Flush Flush the caches before clearing the bit > - (mostly TRUE except MMIO addresses) > > @retval RETURN_SUCCESS The attributes were cleared for the > memory region. > @@ -114,8 +112,7 @@ EFIAPI > MemEncryptSevClearPageEncMask ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS BaseAddress, > - IN UINTN NumPages, > - IN BOOLEAN Flush > + IN UINTN NumPages > ); > > /** > @@ -128,8 +125,6 @@ MemEncryptSevClearPageEncMask ( > address of a memory region. > @param[in] NumPages The number of pages from start memory > region. > - @param[in] Flush Flush the caches before setting the bit > - (mostly TRUE except MMIO addresses) > > @retval RETURN_SUCCESS The attributes were set for the memory > region. > @@ -142,8 +137,7 @@ EFIAPI > MemEncryptSevSetPageEncMask ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS BaseAddress, > - IN UINTN NumPages, > - IN BOOLEAN Flush > + IN UINTN NumPages > ); > > > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h > index 8dc39e647b90..21bbbd1c4f9c 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h > @@ -58,8 +58,6 @@ InternalGetMemEncryptionAddressMask ( > @param[in] PhysicalAddress The physical address that is the start > address of a memory region. > @param[in] Length The length of memory region > - @param[in] Flush Flush the caches before applying the > - encryption mask > > @retval RETURN_SUCCESS The attributes were cleared for the > memory region. > @@ -72,8 +70,7 @@ EFIAPI > InternalMemEncryptSevSetMemoryDecrypted ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS PhysicalAddress, > - IN UINTN Length, > - IN BOOLEAN Flush > + IN UINTN Length > ); > > /** > @@ -85,8 +82,6 @@ InternalMemEncryptSevSetMemoryDecrypted ( > @param[in] PhysicalAddress The physical address that is the start > address of a memory region. > @param[in] Length The length of memory region > - @param[in] Flush Flush the caches before applying the > - encryption mask > > @retval RETURN_SUCCESS The attributes were set for the memory > region. > @@ -99,8 +94,7 @@ EFIAPI > InternalMemEncryptSevSetMemoryEncrypted ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS PhysicalAddress, > - IN UINTN Length, > - IN BOOLEAN Flush > + IN UINTN Length > ); > > /** > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > index 80831b81facf..c66c4e9b9272 100644 > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > @@ -120,8 +120,7 @@ AmdSevDxeEntryPoint ( > Status = MemEncryptSevClearPageEncMask ( > 0, // Cr3BaseAddress -- use current CR3 > MapPagesBase, // BaseAddress > - MapPagesCount, // NumPages > - TRUE // Flush > + MapPagesCount // NumPages > ); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "%a: MemEncryptSevClearPageEncMask(): %r\n", > diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c > index 49ffa2448811..b30628078f73 100644 > --- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c > +++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c > @@ -252,8 +252,7 @@ IoMmuMap ( > Status = MemEncryptSevClearPageEncMask ( > 0, > MapInfo->PlainTextAddress, > - MapInfo->NumberOfPages, > - TRUE > + MapInfo->NumberOfPages > ); > ASSERT_EFI_ERROR (Status); > if (EFI_ERROR (Status)) { > @@ -407,8 +406,7 @@ IoMmuUnmapWorker ( > Status = MemEncryptSevSetPageEncMask ( > 0, > MapInfo->PlainTextAddress, > - MapInfo->NumberOfPages, > - TRUE > + MapInfo->NumberOfPages > ); > ASSERT_EFI_ERROR (Status); > if (EFI_ERROR (Status)) { > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c > index 169d3118e44f..be260e0d1014 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c > @@ -25,8 +25,6 @@ > address of a memory region. > @param[in] NumPages The number of pages from start memory > region. > - @param[in] Flush Flush the caches before clearing the bit > - (mostly TRUE except MMIO addresses) > > @retval RETURN_SUCCESS The attributes were cleared for the > memory region. > @@ -39,8 +37,7 @@ EFIAPI > MemEncryptSevClearPageEncMask ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS BaseAddress, > - IN UINTN NumPages, > - IN BOOLEAN Flush > + IN UINTN NumPages > ) > { > // > @@ -59,8 +56,6 @@ MemEncryptSevClearPageEncMask ( > address of a memory region. > @param[in] NumPages The number of pages from start memory > region. > - @param[in] Flush Flush the caches before setting the bit > - (mostly TRUE except MMIO addresses) > > @retval RETURN_SUCCESS The attributes were set for the memory > region. > @@ -73,8 +68,7 @@ EFIAPI > MemEncryptSevSetPageEncMask ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS BaseAddress, > - IN UINTN NumPages, > - IN BOOLEAN Flush > + IN UINTN NumPages > ) > { > // > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c > index a2bf698bcde7..a57e8fd37fa7 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c > @@ -27,8 +27,6 @@ > address of a memory region. > @param[in] NumPages The number of pages from start memory > region. > - @param[in] Flush Flush the caches before clearing the bit > - (mostly TRUE except MMIO addresses) > > @retval RETURN_SUCCESS The attributes were cleared for the > memory region. > @@ -41,15 +39,13 @@ EFIAPI > MemEncryptSevClearPageEncMask ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS BaseAddress, > - IN UINTN NumPages, > - IN BOOLEAN Flush > + IN UINTN NumPages > ) > { > return InternalMemEncryptSevSetMemoryDecrypted ( > Cr3BaseAddress, > BaseAddress, > - EFI_PAGES_TO_SIZE (NumPages), > - Flush > + EFI_PAGES_TO_SIZE (NumPages) > ); > } > > @@ -63,8 +59,6 @@ MemEncryptSevClearPageEncMask ( > address of a memory region. > @param[in] NumPages The number of pages from start memory > region. > - @param[in] Flush Flush the caches before setting the bit > - (mostly TRUE except MMIO addresses) > > @retval RETURN_SUCCESS The attributes were set for the memory > region. > @@ -77,15 +71,13 @@ EFIAPI > MemEncryptSevSetPageEncMask ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS BaseAddress, > - IN UINTN NumPages, > - IN BOOLEAN Flush > + IN UINTN NumPages > ) > { > return InternalMemEncryptSevSetMemoryEncrypted ( > Cr3BaseAddress, > BaseAddress, > - EFI_PAGES_TO_SIZE (NumPages), > - Flush > + EFI_PAGES_TO_SIZE (NumPages) > ); > } > > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > index a18d336a8789..c696745f9d26 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > @@ -828,8 +828,6 @@ SetMemoryEncDec ( > @param[in] PhysicalAddress The physical address that is the start > address of a memory region. > @param[in] Length The length of memory region > - @param[in] Flush Flush the caches before applying the > - encryption mask > > @retval RETURN_SUCCESS The attributes were cleared for the > memory region. > @@ -842,8 +840,7 @@ EFIAPI > InternalMemEncryptSevSetMemoryDecrypted ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS PhysicalAddress, > - IN UINTN Length, > - IN BOOLEAN Flush > + IN UINTN Length > ) > { > > @@ -852,7 +849,7 @@ InternalMemEncryptSevSetMemoryDecrypted ( > PhysicalAddress, > Length, > ClearCBit, > - Flush > + TRUE > ); > } > > @@ -865,8 +862,6 @@ InternalMemEncryptSevSetMemoryDecrypted ( > @param[in] PhysicalAddress The physical address that is the start > address of a memory region. > @param[in] Length The length of memory region > - @param[in] Flush Flush the caches before applying the > - encryption mask > > @retval RETURN_SUCCESS The attributes were set for the memory > region. > @@ -879,8 +874,7 @@ EFIAPI > InternalMemEncryptSevSetMemoryEncrypted ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS PhysicalAddress, > - IN UINTN Length, > - IN BOOLEAN Flush > + IN UINTN Length > ) > { > return SetMemoryEncDec ( > @@ -888,7 +882,7 @@ InternalMemEncryptSevSetMemoryEncrypted ( > PhysicalAddress, > Length, > SetCBit, > - Flush > + TRUE > ); > } > > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c > index e0d3a15e8503..a155c8729bfb 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c > @@ -42,8 +42,6 @@ InternalGetMemEncryptionAddressMask ( > @param[in] PhysicalAddress The physical address that is the start > address of a memory region. > @param[in] Length The length of memory region > - @param[in] Flush Flush the caches before applying the > - encryption mask > > @retval RETURN_SUCCESS The attributes were cleared for the > memory region. > @@ -56,8 +54,7 @@ EFIAPI > InternalMemEncryptSevSetMemoryDecrypted ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS PhysicalAddress, > - IN UINTN Length, > - IN BOOLEAN Flush > + IN UINTN Length > ) > { > // > @@ -89,8 +86,7 @@ EFIAPI > InternalMemEncryptSevSetMemoryEncrypted ( > IN PHYSICAL_ADDRESS Cr3BaseAddress, > IN PHYSICAL_ADDRESS PhysicalAddress, > - IN UINTN Length, > - IN BOOLEAN Flush > + IN UINTN Length > ) > { > // > diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > index fdf2380974fa..c7cc5b0389c8 100644 > --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > @@ -283,8 +283,7 @@ SmmCpuFeaturesSmmRelocationComplete ( > Status = MemEncryptSevSetPageEncMask ( > 0, // Cr3BaseAddress -- use current CR3 > MapPagesBase, // BaseAddress > - MapPagesCount, // NumPages > - TRUE // Flush > + MapPagesCount // NumPages > ); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "%a: MemEncryptSevSetPageEncMask(): %r\n", > diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c > index dddffdebda4b..a8bf610022ba 100644 > --- a/OvmfPkg/PlatformPei/AmdSev.c > +++ b/OvmfPkg/PlatformPei/AmdSev.c > @@ -72,8 +72,7 @@ AmdSevEsInitialize ( > DecryptStatus = MemEncryptSevClearPageEncMask ( > 0, > GhcbBasePa + EFI_PAGES_TO_SIZE (PageCount), > - 1, > - TRUE > + 1 > ); > ASSERT_RETURN_ERROR (DecryptStatus); > } >