public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Brijesh Singh" <brijesh.singh@amd.com>
To: Laszlo Ersek <lersek@redhat.com>, devel@edk2.groups.io
Cc: brijesh.singh@amd.com, James Bottomley <jejb@linux.ibm.com>,
	Min Xu <min.m.xu@intel.com>, Jiewen Yao <jiewen.yao@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Erdem Aktas <erdemaktas@google.com>
Subject: Re: [edk2-devel] [PATCH 13/13] OvmfPkg/BaseMemEncryptSevLib: remove Flush parameter
Date: Tue, 11 May 2021 12:45:07 -0500	[thread overview]
Message-ID: <41c63c63-554d-1f7d-5d27-bd649261e897@amd.com> (raw)
In-Reply-To: <3c85698e-0b5e-38b0-f752-30204cb78cad@redhat.com>


On 5/11/21 6:55 AM, Laszlo Ersek wrote:
> I don't fully understand the updates in this patch:
>
> On 05/07/21 22:38, Brijesh Singh wrote:
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc383d8fdc1264644760508d91473b003%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563309382960811%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=M3xHM2yU0m3VtPn1xGe1k5Wq0d6Vbdf9gMqDX1NxpgA%3D&amp;reserved=0
>>
>> 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().
> This looks good; it matches my request (1) from:
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2021-May%2Fmsg00109.html&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc383d8fdc1264644760508d91473b003%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563309382960811%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=3TQrliMABDaa%2FtCN%2FvKewTmLfVRKIou2La5yRGGyzJY%3D&amp;reserved=0
>
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Erdem Aktas <erdemaktas@google.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>  OvmfPkg/Include/Library/MemEncryptSevLib.h    | 10 ++----
>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.h  | 10 ++----
>>  OvmfPkg/AmdSevDxe/AmdSevDxe.c                 |  3 +-
>>  OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                |  6 ++--
>>  .../Ia32/MemEncryptSevLib.c                   | 10 ++----
>>  .../X64/MemEncryptSevLib.c                    | 16 +++-------
>>  .../X64/PeiDxeVirtualMemory.c                 | 32 +++++++++++--------
>>  .../X64/SecVirtualMemory.c                    |  8 ++---
>>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     |  3 +-
>>  OvmfPkg/PlatformPei/AmdSev.c                  |  3 +-
>>  10 files changed, 35 insertions(+), 66 deletions(-)
>>
>> 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..41e4b291d070 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",
> (1) You missed my comment (2) in
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2021-May%2Fmsg00109.html&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc383d8fdc1264644760508d91473b003%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563309382960811%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=3TQrliMABDaa%2FtCN%2FvKewTmLfVRKIou2La5yRGGyzJY%3D&amp;reserved=0>.

Ah, I will fix in rev2.


>
>> 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..ad1021bd3e43 100644
>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
>> @@ -555,8 +555,7 @@ EnableReadOnlyPageWriteProtect (
>>                                        address of a memory region.
>>    @param[in]  Length                  The length of memory region
>>    @param[in]  Mode                    Set or Clear mode
>> -  @param[in]  CacheFlush              Flush the caches before applying the
>> -                                      encryption mask
>> +  @param[in]  Mmio                    The physical address range is Mmio.
>>  
>>    @retval RETURN_SUCCESS              The attributes were cleared for the
>>                                        memory region.
>> @@ -572,7 +571,7 @@ SetMemoryEncDec (
>>    IN    PHYSICAL_ADDRESS         PhysicalAddress,
>>    IN    UINTN                    Length,
>>    IN    MAP_RANGE_MODE           Mode,
>> -  IN    BOOLEAN                  CacheFlush
>> +  IN    BOOLEAN                  Mmio
>>    )
>>  {
>>    PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
>> @@ -585,12 +584,23 @@ SetMemoryEncDec (
>>    UINT64                         AddressEncMask;
>>    BOOLEAN                        IsWpEnabled;
>>    RETURN_STATUS                  Status;
>> +  BOOLEAN                        CacheFlush;
>>  
>>    //
>>    // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
>>    //
>>    PageMapLevel4Entry = NULL;
>>  
>> +  //
>> +  // The cache need to flushed for the non-Mmio address range.
>> +  //
>> +  if (Mmio == TRUE) {
>> +    CacheFlush = FALSE;
>> +  } else {
>> +    CacheFlush = TRUE;
>> +  }
>> +
>> +  //
>>    DEBUG ((
>>      DEBUG_VERBOSE,
>>      "%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx Mode=%a CacheFlush=%u\n",
> (2) The calculation of "CacheFlush" from "Mmio" is awkward. First, we
> don't compare BOOLEANs against TRUE or FALSE, BOOLEANs just stand alone
> in controlling expression (or otherwise "logical") context. Second, why
> not just write:
>
>   CacheFlush = !Mmio;
>
> But even so...
>
> (3) ... The introduction of the "Mmio" parameter is inexplicable to me.
> It apparently replaces CacheFlush (with inverse meaning), but neither
> the commit message, nor the (RFCv2 -> PATCH) changelog, explain why this
> replacement makes sense.

The  internal function is used for clearing the mask for both system RAM
as well as Mmio, so we need a way to tell the internal function that
call is for the Mmio range. I thought making all the changes in a single
file makes sense but I see it can get harder for the review. I guess I
could split the work in two patches

1) Drop the cache flush param from high level
MemEncryptSev{Set,Clear}PageEncMask and don't touch anything in the
SetMemoryEncDec()

2) Rename the Flush parameter to Mmio in the SetMemoryEncDec()

Does it makes sense to you ?


>
> The SetMemoryEncDec() function is an internal function (not a library
> class API), so this change doesn't necessarily conflict with the commit
> message -- but having this change in this particular patch (the last
> patch in the series) seems unjustified.
>
> In the previous version, we updated two SetMemoryEncDec() call sites: in
> InternalMemEncryptSevSetMemoryDecrypted() and
> InternalMemEncryptSevSetMemoryEncrypted(), we replaced the forwarding of
> "Flush" with TRUE constants.
>
> In this version, we update *three* SetMemoryEncDec() call sites:
>
> - in InternalMemEncryptSevSetMemoryDecrypted() and
> InternalMemEncryptSevSetMemoryEncrypted(), we replace the forwarding of
> "Flush" with FALSE constants,
>
> - and in InternalMemEncryptSevClearMmioPageEncMask(), we replace the
> *constant* FALSE with TRUE.
>
> I think this very last point -- regarding
> InternalMemEncryptSevClearMmioPageEncMask() -- shows that the
> replacement of CacheFlush with Mmio, at this point in the series, is
> unwarranted.
>
> Minimally, this replacement / negation should be a separate patch, but
> even then, I think we'd need a defensible purpose (which is not clear to
> me at this point); *plus*, the "re-calculation" of CacheFlush inside
> SetMemoryEncDec() from Mmio feels like a cop-out. It's only being done
> to save some additional replacements in the patch, but it leaves us with
> a stricly worse -- harder to understand -- function. If you really need
> this replacement / negation, then please do it in a separate patch, with
> a good commit message; furthermore, please replace CacheFlush
> *completely*, in SetMemoryEncDec() -- please don't reintroduce it.
>
> Thanks,
> Laszlo
>
>
>> @@ -828,8 +838,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 +850,7 @@ EFIAPI
>>  InternalMemEncryptSevSetMemoryDecrypted (
>>    IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
>>    IN  PHYSICAL_ADDRESS        PhysicalAddress,
>> -  IN  UINTN                   Length,
>> -  IN  BOOLEAN                 Flush
>> +  IN  UINTN                   Length
>>    )
>>  {
>>  
>> @@ -852,7 +859,7 @@ InternalMemEncryptSevSetMemoryDecrypted (
>>             PhysicalAddress,
>>             Length,
>>             ClearCBit,
>> -           Flush
>> +           FALSE
>>             );
>>  }
>>  
>> @@ -865,8 +872,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 +884,7 @@ EFIAPI
>>  InternalMemEncryptSevSetMemoryEncrypted (
>>    IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
>>    IN  PHYSICAL_ADDRESS        PhysicalAddress,
>> -  IN  UINTN                   Length,
>> -  IN  BOOLEAN                 Flush
>> +  IN  UINTN                   Length
>>    )
>>  {
>>    return SetMemoryEncDec (
>> @@ -888,7 +892,7 @@ InternalMemEncryptSevSetMemoryEncrypted (
>>             PhysicalAddress,
>>             Length,
>>             SetCBit,
>> -           Flush
>> +           FALSE
>>             );
>>  }
>>  
>> @@ -921,6 +925,6 @@ InternalMemEncryptSevClearMmioPageEncMask (
>>             PhysicalAddress,
>>             Length,
>>             ClearCBit,
>> -           FALSE
>> +           TRUE
>>             );
>>  }
>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c
>> index bca5e3febb1b..24d19d3ca161 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);
>>    }
>>

  reply	other threads:[~2021-05-11 17:45 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-07 20:38 [PATCH 00/13] Add GHCBv2 macro and helpers Brijesh Singh
2021-05-07 20:38 ` [PATCH 01/13] MdePkg/Register/Amd: expand the SEV MSR to include the SNP definition Brijesh Singh
2021-05-11  8:32   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 02/13] MdePkg/Amd: add white spaces to retain alignment for future expansion Brijesh Singh
2021-05-11  8:36   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 03/13] MdePkg/Register/Amd: define GHCB macros for hypervisor feature detection Brijesh Singh
2021-05-11  8:47   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 04/13] MdePkg/Register/Amd: define GHCB macro for Register GPA structure Brijesh Singh
2021-05-11  8:50   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 05/13] MdePkg/Register/Amd: define GHCB macro for the Page State Change Brijesh Singh
2021-05-11  8:59   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 06/13] MdePkg/Register/Amd: define GHCB macros for SNP AP creation Brijesh Singh
2021-05-11  9:59   ` [edk2-devel] " Laszlo Ersek
2021-05-11 15:43     ` Lendacky, Thomas
2021-05-13 11:29       ` Laszlo Ersek
2021-05-07 20:38 ` [PATCH 07/13] MdePkg/BaseLib: add support for PVALIDATE instruction Brijesh Singh
2021-05-11 10:29   ` [edk2-devel] " Laszlo Ersek
2021-05-11 17:18     ` Brijesh Singh
2021-05-07 20:38 ` [PATCH 08/13] MdePkg/BaseLib: add support for RMPADJUST instruction Brijesh Singh
2021-05-11 11:01   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 09/13] OvmfPkg/BaseMemEncryptSevLib: introduce MemEncryptSevClearMmioPageEncMask() Brijesh Singh
2021-05-11 11:16   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 10/13] OvmfPkg/AmdSevDxe: use MemEncryptSevClearMmioPageEncMask() to clear EncMask Brijesh Singh
2021-05-11 11:18   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 11/13] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: use Mmio helper to clear enc mask Brijesh Singh
2021-05-11 11:19   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 12/13] OvmfPkg/TpmMmioSevDecryptPei: use MemEncryptSevClearMmioPageEncMask() Brijesh Singh
2021-05-11 11:20   ` [edk2-devel] " Laszlo Ersek
2021-05-07 20:38 ` [PATCH 13/13] OvmfPkg/BaseMemEncryptSevLib: remove Flush parameter Brijesh Singh
2021-05-11 11:55   ` [edk2-devel] " Laszlo Ersek
2021-05-11 17:45     ` Brijesh Singh [this message]
2021-05-12 16:35       ` Brijesh Singh
2021-05-13 11:25         ` Laszlo Ersek
2021-05-13 11:24       ` Laszlo Ersek
2021-05-08  1:43 ` 回复: [edk2-devel] [PATCH 00/13] Add GHCBv2 macro and helpers gaoliming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=41c63c63-554d-1f7d-5d27-bd649261e897@amd.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox