Re: [edk2-devel] [PATCH v2 10/15] OvmfPkg/MemEncryptSevLib: Coding style fixes in prep for SEC library

["Laszlo Ersek" <lersek@redhat.com>]

On 01/06/21 22:21, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3108
> 
> Creating an SEC version of the library requires renaming an existing file
> which will result in the existing code failing ECC. Prior to renaming the
> existing file, fix the coding style to avoid the ECC failure.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> index 6422bc53bd5d..3a5bab657bd7 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> @@ -192,7 +192,8 @@ Split2MPageTo4K (
>  {
>    PHYSICAL_ADDRESS                  PhysicalAddress4K;
>    UINTN                             IndexOfPageTableEntries;
> -  PAGE_TABLE_4K_ENTRY               *PageTableEntry, *PageTableEntry1;
> +  PAGE_TABLE_4K_ENTRY               *PageTableEntry;
> +  PAGE_TABLE_4K_ENTRY               *PageTableEntry1;
>    UINT64                            AddressEncMask;
>  
>    PageTableEntry = AllocatePageTableMemory(1);
> @@ -472,7 +473,7 @@ Split1GPageTo2M (
>  /**
>    Set or Clear the memory encryption bit
>  
> -  @param[in]      PagetablePoint        Page table entry pointer (PTE).
> +  @param[in, out] PageTablePointer      Page table entry pointer (PTE).
>    @param[in]      Mode                  Set or Clear encryption bit
>  
>  **/
> @@ -562,7 +563,6 @@ EnableReadOnlyPageWriteProtect (
>    @retval RETURN_UNSUPPORTED          Setting the memory encyrption attribute
>                                        is not supported
>  **/
> -
>  STATIC
>  RETURN_STATUS
>  EFIAPI
> @@ -635,7 +635,7 @@ SetMemoryEncDec (
>  
>    Status = EFI_SUCCESS;
>  
> -  while (Length)
> +  while (Length != 0)
>    {
>      //
>      // If Cr3BaseAddress is not specified then read the current CR3
> @@ -683,7 +683,7 @@ SetMemoryEncDec (
>        // Valid 1GB page
>        // If we have at least 1GB to go, we can just update this entry
>        //
> -      if (!(PhysicalAddress & (BIT30 - 1)) && Length >= BIT30) {
> +      if ((PhysicalAddress & (BIT30 - 1)) == 0 && Length >= BIT30) {
>          SetOrClearCBit(&PageDirectory1GEntry->Uint64, Mode);
>          DEBUG ((
>            DEBUG_VERBOSE,
> @@ -744,7 +744,7 @@ SetMemoryEncDec (
>          // Valid 2MB page
>          // If we have at least 2MB left to go, we can just update this entry
>          //
> -        if (!(PhysicalAddress & (BIT21-1)) && Length >= BIT21) {
> +        if ((PhysicalAddress & (BIT21-1)) == 0 && Length >= BIT21) {
>            SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode);
>            PhysicalAddress += BIT21;
>            Length -= BIT21;
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>