Re: [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case
> when clear RP
> 
> In ConvertMemoryPageAttributes() function, when clear RP for a
> specific range [BaseAddress, BaseAddress + Length], it means to
> set the present bit to 1 and assign default value for other
> attributes in page table. The default attributes for the input
> specific range are NX disabled and ReadOnly. If there is existing
> present range in [BaseAddress, BaseAddress + Length] and the
> attributes are not NX disabled or not ReadOnly, then output the
> DEBUG message to indicate that the NX and ReadOnly attributes of
> the existing present range are modified in the function.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 48
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 12723e5750..862b3e9720 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -349,6 +349,8 @@ ConvertMemoryPageAttributes (
>    IA32_MAP_ENTRY        *Map;
>    UINTN                 Count;
>    UINTN                 Index;
> +  UINT64                OverlappedRangeBase;
> +  UINT64                OverlappedRangeLimit;
> 
>    ASSERT (Attributes != 0);
>    ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
> @@ -430,6 +432,52 @@ ConvertMemoryPageAttributes (
>        // By default memory is Ring 3 accessble.
>        //
>        PagingAttribute.Bits.UserSupervisor = 1;
> +
> +      DEBUG_CODE_BEGIN ();
> +      if (((Attributes & EFI_MEMORY_RO) == 0) || (((Attributes &
> EFI_MEMORY_XP) == 0) && (mXdSupported))) {
> +        //
> +        // When mapping a range to present and EFI_MEMORY_RO or
> EFI_MEMORY_XP is not specificed,
> +        // check if [BaseAddress, BaseAddress + Length] contains present range.
> +        // Existing Present range in [BaseAddress, BaseAddress + Length] is set to
> NX disable or ReadOnly.
> +        //
> +        Count  = 0;
> +        Map    = NULL;
> +        Status = PageTableParse (PageTableBase, mPagingMode, NULL, &Count);
> +
> +        while (Status == RETURN_BUFFER_TOO_SMALL) {
> +          if (Map != NULL) {
> +            FreePool (Map);
> +          }
> +
> +          Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
> +          ASSERT (Map != NULL);
> +          Status = PageTableParse (PageTableBase, mPagingMode, Map, &Count);
> +        }
> +
> +        ASSERT_RETURN_ERROR (Status);
> +        for (Index = 0; Index < Count; Index++) {
> +          if (Map[Index].LinearAddress >= BaseAddress + Length) {
> +            break;
> +          }
> +
> +          if ((BaseAddress < Map[Index].LinearAddress + Map[Index].Length) &&
> (BaseAddress + Length > Map[Index].LinearAddress)) {
> +            OverlappedRangeBase  = MAX (BaseAddress,
> Map[Index].LinearAddress);
> +            OverlappedRangeLimit = MIN (BaseAddress + Length,
> Map[Index].LinearAddress + Map[Index].Length);
> +
> +            if (((Attributes & EFI_MEMORY_RO) == 0) &&
> (Map[Index].Attribute.Bits.ReadWrite == 1)) {
> +              DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes:
> [0x%lx, 0x%lx] is set from ReadWrite to ReadOnly\n", OverlappedRangeBase,
> OverlappedRangeLimit));
> +            }
> +
> +            if (((Attributes & EFI_MEMORY_XP) == 0) && (mXdSupported) &&
> (Map[Index].Attribute.Bits.Nx == 1)) {
> +              DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes:
> [0x%lx, 0x%lx] is set from NX enabled to NX disabled\n",
> OverlappedRangeBase, OverlappedRangeLimit));
> +            }
> +          }
> +        }
> +
> +        FreePool (Map);
> +      }
> +
> +      DEBUG_CODE_END ();
>      }
>    }
> 
> --
> 2.31.1.windows.1