1. Patchew
  2. EDK2
  3. [edk2] [RFC v4 0/6] Stack trace support in X64 exception handling
  4. View patch

[edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses

Paulo Alcantara posted 6 patches 6 years, 11 months ago
There is a newer version of this series
[edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Paulo Alcantara 6 years, 11 months ago 
Introduce IsLinearAddressValid() function that will be used for
validating memory addresses that would get dereferenced during stack
traces in IA32 and X64 CPU exceptions.

Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Requested-by: Brian Johnson <brian.johnson@hpe.com>
Requested-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c | 382 ++++++++++++++++++++
 UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |  16 +
 2 files changed, 398 insertions(+)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
index 867c5c01d6..52b3eb1463 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
@@ -14,6 +14,9 @@
 
 #include "CpuExceptionCommon.h"
 
+#include <Register/Msr.h>
+#include <Library/DebugLib.h>
+
 //
 // Error code flag indicating whether or not an error code will be
 // pushed on the stack if an exception occurs.
@@ -194,3 +197,382 @@ GetPdbFileName (
     }
   }
 }
+
+/**
+  Check if a linear address is valid by walking the page tables in 4-level
+  paging mode.
+
+  @param[in]  Cr3             CR3 control register.
+  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
+  @param[in]  LinearAddress   Linear address to be checked.
+**/
+STATIC
+BOOLEAN
+Do4LevelPagingModeCheck (
+  IN UINTN            Cr3,
+  IN UINT8            MaxPhyAddrBits,
+  IN UINTN            LinearAddress
+  )
+{
+  UINT64 PhysicalAddress;
+  UINT64 *Pml4TableEntry;
+  UINT64 *PageDirPtrTableEntry;
+  UINT64 *PageDirEntry;
+  UINT64 *PageTableEntry;
+
+  //
+  // In 4-level paging mode, linear addresses are 48 bits wide
+  //
+  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
+    return FALSE;
+  }
+
+  //
+  // Calculate physical address of PML4E
+  //
+  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
+
+  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
+
+  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if a PDPTE is present
+  //
+  if ((*Pml4TableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Calculate physical address of PDPTE
+  //
+  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
+
+  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check whether a PDPTE or 1GiB page entry is present
+  //
+  if ((*PageDirPtrTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if PDPTE maps an 1GiB page
+  //
+  if ((*PageDirPtrTableEntry & BIT7) != 0) {
+    return TRUE;
+  }
+
+  //
+  // Calculate physical address of PDE
+  //
+  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
+                                             12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
+
+  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check whether a PDE or a 2MiB page entry is present
+  //
+  if ((*PageDirEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if PDE maps a 2MiB page
+  //
+  if ((*PageDirEntry & BIT7) != 0) {
+    return TRUE;
+  }
+
+  //
+  // Calculate physical address of PTE
+  //
+  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
+
+  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if PTE maps a 4KiB page
+  //
+  if ((*PageTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**
+  Check if a linear address is valid by walking the page tables in 32-bit paging
+  mode.
+
+  @param[in]  Cr3             CR3 control register.
+  @param[in]  Cr4             CR4 control register.
+  @param[in]  LinearAddress   Linear address to be checked.
+**/
+STATIC
+BOOLEAN
+Do32BitPagingModeCheck (
+  IN UINTN            Cr3,
+  IN UINTN            Cr4,
+  IN UINTN            LinearAddress
+  )
+{
+  UINT64 PhysicalAddress;
+  UINT32 *PageDirEntry;
+  UINT32 *PageTableEntry;
+
+  if (LinearAddress > MAX_UINT32) {
+    return FALSE;
+  }
+
+  //
+  // Calculate physical address of PDE
+  //
+  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
+  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
+
+  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check whether a PTE or a 4MiB page is present
+  //
+  if ((*PageDirEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if PDE maps a 4MiB page
+  //
+  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
+    return TRUE;
+  }
+
+  //
+  // Calculate physical address of PTE
+  //
+  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
+  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
+
+  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if PTE maps a 4KiB page
+  //
+  if ((*PageTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**
+  Check if a linear address is valid by walking the page tables in PAE paging
+  mode.
+
+  @param[in]  Cr3             CR3 control register.
+  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
+  @param[in]  LinearAddress   Linear address to be checked.
+**/
+STATIC
+BOOLEAN
+DoPAEPagingModeCheck (
+  IN UINTN            Cr3,
+  IN UINT8            MaxPhyAddrBits,
+  IN UINTN            LinearAddress
+  )
+{
+  UINT64 PhysicalAddress;
+  UINT64 *PageDirPtrTableEntry;
+  UINT64 *PageDirEntry;
+  UINT64 *PageTableEntry;
+
+  if (LinearAddress > MAX_UINT32) {
+    return FALSE;
+  }
+
+  //
+  // Calculate physical address of PDPTE
+  //
+  PhysicalAddress = (UINT32)Cr3 >> 5;
+
+  //
+  // Select PDPTE register
+  //
+  PhysicalAddress +=
+    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
+
+  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if PDE is present
+  //
+  if ((*PageDirPtrTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
+                                             12);
+  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
+  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
+
+  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check whether a PTE or a 2MiB page is present
+  //
+  if ((*PageDirEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if PDE maps a 2MiB page
+  //
+  if ((*PageDirEntry & BIT7) != 0) {
+    return TRUE;
+  }
+
+  //
+  // Calculate physical address of PTE
+  //
+  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
+  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
+
+  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if PTE maps a 4KiB page
+  //
+  if ((*PageTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**
+  Check if a linear address is valid.
+
+  @param[in]  Cr0            CR0 control register.
+  @param[in]  Cr3            CR3 control register.
+  @param[in]  Cr4            CR4 control register.
+  @param[in]  LinearAddress  Linear address to be checked.
+**/
+BOOLEAN
+IsLinearAddressValid (
+  IN  UINTN              Cr0,
+  IN  UINTN              Cr3,
+  IN  UINTN              Cr4,
+  IN  UINTN              LinearAddress
+  )
+{
+  UINT32                  Eax;
+  UINT32                  Edx;
+  UINT8                   MaxPhyAddrBits;
+  MSR_IA32_EFER_REGISTER  Msr;
+  BOOLEAN                 AddressValid;
+
+  //
+  // Check for valid input parameters
+  //
+  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if paging is disabled
+  //
+  if ((Cr0 & BIT31) == 0) {
+    //
+    // If CR4.PAE bit is set, then the linear (or physical) address supports
+    // only up to 36 bits.
+    //
+    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
+        LinearAddress > 0xFFFFFFFF) {
+      return FALSE;
+    }
+
+    return TRUE;
+  }
+
+  //
+  // Paging can be enabled only if CR0.PE bit is set
+  //
+  if ((Cr0 & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // CR3 register cannot be zero if paging is enabled
+  //
+  if (Cr3 == 0) {
+    return FALSE;
+  }
+
+  //
+  // Get MAXPHYADDR bits
+  //
+  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
+  if (Eax >= 0x80000008) {
+    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
+    MaxPhyAddrBits = (UINT8)Eax;
+  } else {
+    AsmCpuid (1, NULL, NULL, NULL, &Edx);
+    if ((Edx & BIT6) != 0) {
+      MaxPhyAddrBits = 36;
+    } else {
+      MaxPhyAddrBits = 32;
+    }
+  }
+
+  ASSERT (MaxPhyAddrBits > 0);
+
+  AddressValid = FALSE;
+
+  //
+  // check if CR4.PAE bit is not set
+  //
+  if ((Cr4 & BIT5) == 0) {
+    //
+    // Check if linear address is valid in 32-bit paging mode
+    //
+    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
+  } else {
+    if (MaxPhyAddrBits > 52) {
+      return FALSE;
+    }
+
+    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
+
+    if (Msr.Bits.LME == 0) {
+      //
+      // Check if linear address is valid in PAE paging mode
+      //
+      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits, LinearAddress);
+    } else {
+      //
+      // Check if linear address is valid in 4-level paging mode
+      //
+      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
+                                              LinearAddress);
+    }
+  }
+
+  return AddressValid;
+}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
index ec46c2d9d3..1b51034c25 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
@@ -330,5 +330,21 @@ GetPdbFileName (
   OUT CHAR8    **PdbFileName
   );
 
+/**
+  Check if a linear address is valid.
+
+  @param[in]  Cr0            CR0 control register.
+  @param[in]  Cr3            CR3 control register.
+  @param[in]  Cr4            CR4 control register.
+  @param[in]  LinearAddress  Linear address to be checked.
+**/
+BOOLEAN
+IsLinearAddressValid (
+  IN  UINTN              Cr0,
+  IN  UINTN              Cr3,
+  IN  UINTN              Cr4,
+  IN  UINTN              LinearAddress
+  );
+
 #endif
 
-- 
2.14.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Brian J. Johnson 6 years, 11 months ago 
On 12/28/2017 10:39 PM, Paulo Alcantara wrote:
> +  //
> +  // Check if paging is disabled
> +  //
> +  if ((Cr0 & BIT31) == 0) {
> +    //
> +    // If CR4.PAE bit is set, then the linear (or physical) address supports
> +    // only up to 36 bits.
> +    //
> +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
> +        LinearAddress > 0xFFFFFFFF) {
> +      return FALSE;
> +    }
> +
> +    return TRUE;
> +  }

Paulo,

The logic there doesn't look quite right:  if LinearAddress is between 
2^32 and 2^36-1, this code will always return FALSE, even if CR4.PAE is 
set.  Shouldn't it be:

    if ((UINT64)LinearAddress > 0xFFFFFFFFFULL ||
        ((Cr4 & BIT5) == 0 && LinearAddress > 0xFFFFFFFF)) {
      return FALSE;
    }

(I haven't examined all the code in detail, I just happened to notice 
this issue.)

This bug should get fixed before pushing this series.  I also have some 
more general design questions, which shouldn't hold up pushing the 
series, but I think merit some discussion:

This is great code for validating addresses in general, especially when 
guard pages are in use for NULL pointers, stack overflow, etc.  Thanks 
for adding it!  But for [er]sp and [er]bp validation, don't you really 
just want to know if the address is in the expected stack range?  Maybe 
the code which sets up the stack could communicate the valid range to 
CpuExceptionHandlerLib somehow.  It could use special debug register 
values like 
SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLib.c 
does.  Or perhaps it could use dynamic PCDs (although I don't know that 
it's a good idea to go looking up PCDs in an exception handler.)  Or 
maybe there's a more straightforward way....  It would have to take AP 
stacks into account, and probably SetJump/LongJump as well.  That may or 
may not be simpler than the current code....

More generally, I'd like to see some sort of platform-specific callout 
to further validate addresses.  Not all mapped addresses, or addresses 
up to the architectural limit, are safe to access.  For instance, reads 
to SMRAM outside of SMM will cause exceptions.  Also, we wouldn't want 
to go backtracing through MMIO or MMCFG space:  reads there could 
potentially have side effects on the hardware.

The rules can also vary at different points in boot.  For example, 
before memory is initialized, Intel Xeon processors generally execute 
32-bit code in cache-as-RAM mode, where the caches are jury-rigged to 
operate as temporary storage while the memory setup code is running.  In 
CAR mode, only a few address ranges can be accessed without causing 
machine checks:  the cache-as-RAM range containing the stack, heap, and 
HOB list, the architectural firmware range below 4G, and a few specific 
MMCFG and MMIO ranges.

So I'd like to suggest that you define an AddressValidationLib library 
class, which provides a routine which takes an address (or an address 
range?) and an indication of the intended use (memory read, memory 
write, execute/disassemble code, stack dump, IO, ...), and returns a 
value specifying if the access is:
- safe (IsLinearAddressValid() should return TRUE)
- unsafe (IsLinearAddressValid() should return FALSE)
- unknown (IsLinearAddressValid() should perform its other tests)

You can supply a NULL instance which always returns "unknown" for 
platforms which don't want to perform their own validation.

Thanks,
-- 
Brian J. Johnson
Enterprise X86 Lab

Hewlett Packard Enterprise
brian.johnson@hpe.com

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Paulo Alcantara 6 years, 11 months ago 
"Brian J. Johnson" <brian.johnson@hpe.com> writes:

Hi Brian,

> On 12/28/2017 10:39 PM, Paulo Alcantara wrote:
>> +  //
>> +  // Check if paging is disabled
>> +  //
>> +  if ((Cr0 & BIT31) == 0) {
>> +    //
>> +    // If CR4.PAE bit is set, then the linear (or physical) address supports
>> +    // only up to 36 bits.
>> +    //
>> +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
>> +        LinearAddress > 0xFFFFFFFF) {
>> +      return FALSE;
>> +    }
>> +
>> +    return TRUE;
>> +  }
>
> Paulo,
>
> The logic there doesn't look quite right:  if LinearAddress is between 
> 2^32 and 2^36-1, this code will always return FALSE, even if CR4.PAE is 
> set.  Shouldn't it be:
>
>     if ((UINT64)LinearAddress > 0xFFFFFFFFFULL ||
>         ((Cr4 & BIT5) == 0 && LinearAddress > 0xFFFFFFFF)) {
>       return FALSE;
>     }

You're right. The check is bogus and I'll fix it up in the next version.

>
> (I haven't examined all the code in detail, I just happened to notice 
> this issue.)

No problem. Your comments are very appreciable.

> This bug should get fixed before pushing this series.  I also have some 
> more general design questions, which shouldn't hold up pushing the 
> series, but I think merit some discussion:
>
> This is great code for validating addresses in general, especially when 
> guard pages are in use for NULL pointers, stack overflow, etc.  Thanks 
> for adding it!  But for [er]sp and [er]bp validation, don't you really 
> just want to know if the address is in the expected stack range?  Maybe 
> the code which sets up the stack could communicate the valid range to 
> CpuExceptionHandlerLib somehow.  It could use special debug register 
> values like 
> SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLib.c 
> does.  Or perhaps it could use dynamic PCDs (although I don't know that 
> it's a good idea to go looking up PCDs in an exception handler.)  Or 
> maybe there's a more straightforward way....  It would have to take AP 
> stacks into account, and probably SetJump/LongJump as well.  That may or 
> may not be simpler than the current code....

I'm not quite sure if I understood you correctly when you say that I
should be checking whether the address is in the expected range, but
using the debug registers to save call stack information seems like a
good idea, although it doesn't seem to be that simple as you mentioned.

>
> More generally, I'd like to see some sort of platform-specific callout 
> to further validate addresses.  Not all mapped addresses, or addresses 
> up to the architectural limit, are safe to access.  For instance, reads 
> to SMRAM outside of SMM will cause exceptions.  Also, we wouldn't want 
> to go backtracing through MMIO or MMCFG space:  reads there could 
> potentially have side effects on the hardware.

Yes - we should ensure that those regions are not accessed during the
stacktrace, as well as test this implementation a lot more.

>
> The rules can also vary at different points in boot.  For example, 
> before memory is initialized, Intel Xeon processors generally execute 
> 32-bit code in cache-as-RAM mode, where the caches are jury-rigged to 
> operate as temporary storage while the memory setup code is running.  In 
> CAR mode, only a few address ranges can be accessed without causing 
> machine checks:  the cache-as-RAM range containing the stack, heap, and 
> HOB list, the architectural firmware range below 4G, and a few specific 
> MMCFG and MMIO ranges.

Really great info. Thanks. We should take that into account as well.

>
> So I'd like to suggest that you define an AddressValidationLib library 
> class, which provides a routine which takes an address (or an address 
> range?) and an indication of the intended use (memory read, memory 
> write, execute/disassemble code, stack dump, IO, ...), and returns a 
> value specifying if the access is:
> - safe (IsLinearAddressValid() should return TRUE)
> - unsafe (IsLinearAddressValid() should return FALSE)
> - unknown (IsLinearAddressValid() should perform its other tests)
>
> You can supply a NULL instance which always returns "unknown" for 
> platforms which don't want to perform their own validation.

Great idea! I can do that for sure, but first of all, I'd like to make
sure the memory validation code is working correctly for IA32 and X64
platforms before introducing the AddressValidationLib library.

Thank you very much for the review and comments!

Paulo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Yao, Jiewen 6 years, 11 months ago 
Some suggestion:

1) I am not sure if it is proper to use ASSERT in an exception handler, because we know something is wrong.

  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);

I suggest we just do the check, and return FALSE, if the prerequisite is not satisfied.

2) Can we use meaningful definition for BIT0, BIT7?

  if ((*Pml4TableEntry & BIT0) == 0) {
  if ((*PageDirPtrTableEntry & BIT7) != 0) {

3) I am not sure if I understand below code.

  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);

If MaxPhyAddrBits is 48, you will get "Cr3 & 0x0FFFFFFFFFFFF000". Is that what you want? I think we need "Cr3 & 0x0000FFFFFFFFF000"
Should it be: PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) & (~0xFFF);

4) Can we use a more readable way to below? Personally, I do not suggest "<< 3", which is just the index calculation.

  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);

For example:
  PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) & (~0xFFF);
  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
  Index= (UINTN)(((UINT64)LinearAddress >> 39) & 0x1FF);
  PhysicalAddress = Pml4TableEntry[Index] & ((1ULL << MaxPhyAddrBits) - 1) & (~0xFFF);



Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Paulo
> Alcantara
> Sent: Friday, December 29, 2017 12:40 PM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to
> valid memory addresses
> 
> Introduce IsLinearAddressValid() function that will be used for
> validating memory addresses that would get dereferenced during stack
> traces in IA32 and X64 CPU exceptions.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Requested-by: Brian Johnson <brian.johnson@hpe.com>
> Requested-by: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
> ---
>  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c | 382
> ++++++++++++++++++++
>  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |  16 +
>  2 files changed, 398 insertions(+)
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> index 867c5c01d6..52b3eb1463 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> @@ -14,6 +14,9 @@
> 
>  #include "CpuExceptionCommon.h"
> 
> +#include <Register/Msr.h>
> +#include <Library/DebugLib.h>
> +
>  //
>  // Error code flag indicating whether or not an error code will be
>  // pushed on the stack if an exception occurs.
> @@ -194,3 +197,382 @@ GetPdbFileName (
>      }
>    }
>  }
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in 4-level
> +  paging mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +Do4LevelPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINT8            MaxPhyAddrBits,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT64 *Pml4TableEntry;
> +  UINT64 *PageDirPtrTableEntry;
> +  UINT64 *PageDirEntry;
> +  UINT64 *PageTableEntry;
> +
> +  //
> +  // In 4-level paging mode, linear addresses are 48 bits wide
> +  //
> +  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PML4E
> +  //
> +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
> +
> +  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if a PDPTE is present
> +  //
> +  if ((*Pml4TableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDPTE
> +  //
> +  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
> +
> +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PDPTE or 1GiB page entry is present
> +  //
> +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDPTE maps an 1GiB page
> +  //
> +  if ((*PageDirPtrTableEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDE
> +  //
> +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1)
> <<
> +                                             12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PDE or a 2MiB page entry is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 2MiB page
> +  //
> +  if ((*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in 32-bit paging
> +  mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  Cr4             CR4 control register.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +Do32BitPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINTN            Cr4,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT32 *PageDirEntry;
> +  UINT32 *PageTableEntry;
> +
> +  if (LinearAddress > MAX_UINT32) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDE
> +  //
> +  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
> +  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PTE or a 4MiB page is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 4MiB page
> +  //
> +  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
> +  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in PAE paging
> +  mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +DoPAEPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINT8            MaxPhyAddrBits,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT64 *PageDirPtrTableEntry;
> +  UINT64 *PageDirEntry;
> +  UINT64 *PageTableEntry;
> +
> +  if (LinearAddress > MAX_UINT32) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDPTE
> +  //
> +  PhysicalAddress = (UINT32)Cr3 >> 5;
> +
> +  //
> +  // Select PDPTE register
> +  //
> +  PhysicalAddress +=
> +    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
> +
> +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PDE is present
> +  //
> +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1)
> <<
> +                                             12);
> +  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PTE or a 2MiB page is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 2MiB page
> +  //
> +  if ((*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid.
> +
> +  @param[in]  Cr0            CR0 control register.
> +  @param[in]  Cr3            CR3 control register.
> +  @param[in]  Cr4            CR4 control register.
> +  @param[in]  LinearAddress  Linear address to be checked.
> +**/
> +BOOLEAN
> +IsLinearAddressValid (
> +  IN  UINTN              Cr0,
> +  IN  UINTN              Cr3,
> +  IN  UINTN              Cr4,
> +  IN  UINTN              LinearAddress
> +  )
> +{
> +  UINT32                  Eax;
> +  UINT32                  Edx;
> +  UINT8                   MaxPhyAddrBits;
> +  MSR_IA32_EFER_REGISTER  Msr;
> +  BOOLEAN                 AddressValid;
> +
> +  //
> +  // Check for valid input parameters
> +  //
> +  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if paging is disabled
> +  //
> +  if ((Cr0 & BIT31) == 0) {
> +    //
> +    // If CR4.PAE bit is set, then the linear (or physical) address supports
> +    // only up to 36 bits.
> +    //
> +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
> +        LinearAddress > 0xFFFFFFFF) {
> +      return FALSE;
> +    }
> +
> +    return TRUE;
> +  }
> +
> +  //
> +  // Paging can be enabled only if CR0.PE bit is set
> +  //
> +  if ((Cr0 & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // CR3 register cannot be zero if paging is enabled
> +  //
> +  if (Cr3 == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Get MAXPHYADDR bits
> +  //
> +  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
> +  if (Eax >= 0x80000008) {
> +    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
> +    MaxPhyAddrBits = (UINT8)Eax;
> +  } else {
> +    AsmCpuid (1, NULL, NULL, NULL, &Edx);
> +    if ((Edx & BIT6) != 0) {
> +      MaxPhyAddrBits = 36;
> +    } else {
> +      MaxPhyAddrBits = 32;
> +    }
> +  }
> +
> +  ASSERT (MaxPhyAddrBits > 0);
> +
> +  AddressValid = FALSE;
> +
> +  //
> +  // check if CR4.PAE bit is not set
> +  //
> +  if ((Cr4 & BIT5) == 0) {
> +    //
> +    // Check if linear address is valid in 32-bit paging mode
> +    //
> +    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
> +  } else {
> +    if (MaxPhyAddrBits > 52) {
> +      return FALSE;
> +    }
> +
> +    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
> +
> +    if (Msr.Bits.LME == 0) {
> +      //
> +      // Check if linear address is valid in PAE paging mode
> +      //
> +      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits,
> LinearAddress);
> +    } else {
> +      //
> +      // Check if linear address is valid in 4-level paging mode
> +      //
> +      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
> +                                              LinearAddress);
> +    }
> +  }
> +
> +  return AddressValid;
> +}
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> index ec46c2d9d3..1b51034c25 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> @@ -330,5 +330,21 @@ GetPdbFileName (
>    OUT CHAR8    **PdbFileName
>    );
> 
> +/**
> +  Check if a linear address is valid.
> +
> +  @param[in]  Cr0            CR0 control register.
> +  @param[in]  Cr3            CR3 control register.
> +  @param[in]  Cr4            CR4 control register.
> +  @param[in]  LinearAddress  Linear address to be checked.
> +**/
> +BOOLEAN
> +IsLinearAddressValid (
> +  IN  UINTN              Cr0,
> +  IN  UINTN              Cr3,
> +  IN  UINTN              Cr4,
> +  IN  UINTN              LinearAddress
> +  );
> +
>  #endif
> 
> --
> 2.14.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Yao, Jiewen 6 years, 11 months ago 
5) For CR4, please use meaning definition for BIT4/BIT5.
  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||

6) For IA32 PAE/PSE calculation, same comment for 3 and 4.

7) Last but not least important, would you please share the information on how do you validate the 32bit PAE/PSE/normal 4K page table?

Thank you
Yao Jiewen

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
> Jiewen
> Sent: Thursday, January 4, 2018 9:36 AM
> To: Paulo Alcantara <paulo@paulo.ac>; edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add
> helper to valid memory addresses
> 
> Some suggestion:
> 
> 1) I am not sure if it is proper to use ASSERT in an exception handler, because we
> know something is wrong.
> 
>   ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
> 
> I suggest we just do the check, and return FALSE, if the prerequisite is not
> satisfied.
> 
> 2) Can we use meaningful definition for BIT0, BIT7?
> 
>   if ((*Pml4TableEntry & BIT0) == 0) {
>   if ((*PageDirPtrTableEntry & BIT7) != 0) {
> 
> 3) I am not sure if I understand below code.
> 
>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>   PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1)
> << 12);
>   PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> 
> If MaxPhyAddrBits is 48, you will get "Cr3 & 0x0FFFFFFFFFFFF000". Is that what
> you want? I think we need "Cr3 & 0x0000FFFFFFFFF000"
> Should it be: PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) &
> (~0xFFF);
> 
> 4) Can we use a more readable way to below? Personally, I do not suggest "<< 3",
> which is just the index calculation.
> 
>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>   PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> 
> For example:
>   PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) & (~0xFFF);
>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>   Index= (UINTN)(((UINT64)LinearAddress >> 39) & 0x1FF);
>   PhysicalAddress = Pml4TableEntry[Index] & ((1ULL << MaxPhyAddrBits) - 1) &
> (~0xFFF);
> 
> 
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Paulo
> > Alcantara
> > Sent: Friday, December 29, 2017 12:40 PM
> > To: edk2-devel@lists.01.org
> > Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> > Subject: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper
> to
> > valid memory addresses
> >
> > Introduce IsLinearAddressValid() function that will be used for
> > validating memory addresses that would get dereferenced during stack
> > traces in IA32 and X64 CPU exceptions.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Requested-by: Brian Johnson <brian.johnson@hpe.com>
> > Requested-by: Jiewen Yao <jiewen.yao@intel.com>
> > Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
> > ---
> >  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c | 382
> > ++++++++++++++++++++
> >  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |
> 16 +
> >  2 files changed, 398 insertions(+)
> >
> > diff --git
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> > index 867c5c01d6..52b3eb1463 100644
> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> > @@ -14,6 +14,9 @@
> >
> >  #include "CpuExceptionCommon.h"
> >
> > +#include <Register/Msr.h>
> > +#include <Library/DebugLib.h>
> > +
> >  //
> >  // Error code flag indicating whether or not an error code will be
> >  // pushed on the stack if an exception occurs.
> > @@ -194,3 +197,382 @@ GetPdbFileName (
> >      }
> >    }
> >  }
> > +
> > +/**
> > +  Check if a linear address is valid by walking the page tables in 4-level
> > +  paging mode.
> > +
> > +  @param[in]  Cr3             CR3 control register.
> > +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> > +  @param[in]  LinearAddress   Linear address to be checked.
> > +**/
> > +STATIC
> > +BOOLEAN
> > +Do4LevelPagingModeCheck (
> > +  IN UINTN            Cr3,
> > +  IN UINT8            MaxPhyAddrBits,
> > +  IN UINTN            LinearAddress
> > +  )
> > +{
> > +  UINT64 PhysicalAddress;
> > +  UINT64 *Pml4TableEntry;
> > +  UINT64 *PageDirPtrTableEntry;
> > +  UINT64 *PageDirEntry;
> > +  UINT64 *PageTableEntry;
> > +
> > +  //
> > +  // In 4-level paging mode, linear addresses are 48 bits wide
> > +  //
> > +  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Calculate physical address of PML4E
> > +  //
> > +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
> > +
> > +  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
> > +
> > +  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check if a PDPTE is present
> > +  //
> > +  if ((*Pml4TableEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Calculate physical address of PDPTE
> > +  //
> > +  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> > 12);
> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
> > +
> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
> > +
> > +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check whether a PDPTE or 1GiB page entry is present
> > +  //
> > +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Check if PDPTE maps an 1GiB page
> > +  //
> > +  if ((*PageDirPtrTableEntry & BIT7) != 0) {
> > +    return TRUE;
> > +  }
> > +
> > +  //
> > +  // Calculate physical address of PDE
> > +  //
> > +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) -
> 1)
> > <<
> > +                                             12);
> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
> > +
> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> > +
> > +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check whether a PDE or a 2MiB page entry is present
> > +  //
> > +  if ((*PageDirEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Check if PDE maps a 2MiB page
> > +  //
> > +  if ((*PageDirEntry & BIT7) != 0) {
> > +    return TRUE;
> > +  }
> > +
> > +  //
> > +  // Calculate physical address of PTE
> > +  //
> > +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> 12);
> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
> > +
> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> > +
> > +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check if PTE maps a 4KiB page
> > +  //
> > +  if ((*PageTableEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  return TRUE;
> > +}
> > +
> > +/**
> > +  Check if a linear address is valid by walking the page tables in 32-bit paging
> > +  mode.
> > +
> > +  @param[in]  Cr3             CR3 control register.
> > +  @param[in]  Cr4             CR4 control register.
> > +  @param[in]  LinearAddress   Linear address to be checked.
> > +**/
> > +STATIC
> > +BOOLEAN
> > +Do32BitPagingModeCheck (
> > +  IN UINTN            Cr3,
> > +  IN UINTN            Cr4,
> > +  IN UINTN            LinearAddress
> > +  )
> > +{
> > +  UINT64 PhysicalAddress;
> > +  UINT32 *PageDirEntry;
> > +  UINT32 *PageTableEntry;
> > +
> > +  if (LinearAddress > MAX_UINT32) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Calculate physical address of PDE
> > +  //
> > +  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
> > +  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
> > +
> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> > +
> > +  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check whether a PTE or a 4MiB page is present
> > +  //
> > +  if ((*PageDirEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Check if PDE maps a 4MiB page
> > +  //
> > +  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
> > +    return TRUE;
> > +  }
> > +
> > +  //
> > +  // Calculate physical address of PTE
> > +  //
> > +  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
> > +  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
> > +
> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> > +
> > +  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check if PTE maps a 4KiB page
> > +  //
> > +  if ((*PageTableEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  return TRUE;
> > +}
> > +
> > +/**
> > +  Check if a linear address is valid by walking the page tables in PAE paging
> > +  mode.
> > +
> > +  @param[in]  Cr3             CR3 control register.
> > +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> > +  @param[in]  LinearAddress   Linear address to be checked.
> > +**/
> > +STATIC
> > +BOOLEAN
> > +DoPAEPagingModeCheck (
> > +  IN UINTN            Cr3,
> > +  IN UINT8            MaxPhyAddrBits,
> > +  IN UINTN            LinearAddress
> > +  )
> > +{
> > +  UINT64 PhysicalAddress;
> > +  UINT64 *PageDirPtrTableEntry;
> > +  UINT64 *PageDirEntry;
> > +  UINT64 *PageTableEntry;
> > +
> > +  if (LinearAddress > MAX_UINT32) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Calculate physical address of PDPTE
> > +  //
> > +  PhysicalAddress = (UINT32)Cr3 >> 5;
> > +
> > +  //
> > +  // Select PDPTE register
> > +  //
> > +  PhysicalAddress +=
> > +    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
> > +
> > +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check if PDE is present
> > +  //
> > +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) -
> 1)
> > <<
> > +                                             12);
> > +  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> > +
> > +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check whether a PTE or a 2MiB page is present
> > +  //
> > +  if ((*PageDirEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Check if PDE maps a 2MiB page
> > +  //
> > +  if ((*PageDirEntry & BIT7) != 0) {
> > +    return TRUE;
> > +  }
> > +
> > +  //
> > +  // Calculate physical address of PTE
> > +  //
> > +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> 12);
> > +  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> > +
> > +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> > +
> > +  //
> > +  // Check if PTE maps a 4KiB page
> > +  //
> > +  if ((*PageTableEntry & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  return TRUE;
> > +}
> > +
> > +/**
> > +  Check if a linear address is valid.
> > +
> > +  @param[in]  Cr0            CR0 control register.
> > +  @param[in]  Cr3            CR3 control register.
> > +  @param[in]  Cr4            CR4 control register.
> > +  @param[in]  LinearAddress  Linear address to be checked.
> > +**/
> > +BOOLEAN
> > +IsLinearAddressValid (
> > +  IN  UINTN              Cr0,
> > +  IN  UINTN              Cr3,
> > +  IN  UINTN              Cr4,
> > +  IN  UINTN              LinearAddress
> > +  )
> > +{
> > +  UINT32                  Eax;
> > +  UINT32                  Edx;
> > +  UINT8                   MaxPhyAddrBits;
> > +  MSR_IA32_EFER_REGISTER  Msr;
> > +  BOOLEAN                 AddressValid;
> > +
> > +  //
> > +  // Check for valid input parameters
> > +  //
> > +  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Check if paging is disabled
> > +  //
> > +  if ((Cr0 & BIT31) == 0) {
> > +    //
> > +    // If CR4.PAE bit is set, then the linear (or physical) address supports
> > +    // only up to 36 bits.
> > +    //
> > +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
> > +        LinearAddress > 0xFFFFFFFF) {
> > +      return FALSE;
> > +    }
> > +
> > +    return TRUE;
> > +  }
> > +
> > +  //
> > +  // Paging can be enabled only if CR0.PE bit is set
> > +  //
> > +  if ((Cr0 & BIT0) == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // CR3 register cannot be zero if paging is enabled
> > +  //
> > +  if (Cr3 == 0) {
> > +    return FALSE;
> > +  }
> > +
> > +  //
> > +  // Get MAXPHYADDR bits
> > +  //
> > +  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
> > +  if (Eax >= 0x80000008) {
> > +    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
> > +    MaxPhyAddrBits = (UINT8)Eax;
> > +  } else {
> > +    AsmCpuid (1, NULL, NULL, NULL, &Edx);
> > +    if ((Edx & BIT6) != 0) {
> > +      MaxPhyAddrBits = 36;
> > +    } else {
> > +      MaxPhyAddrBits = 32;
> > +    }
> > +  }
> > +
> > +  ASSERT (MaxPhyAddrBits > 0);
> > +
> > +  AddressValid = FALSE;
> > +
> > +  //
> > +  // check if CR4.PAE bit is not set
> > +  //
> > +  if ((Cr4 & BIT5) == 0) {
> > +    //
> > +    // Check if linear address is valid in 32-bit paging mode
> > +    //
> > +    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
> > +  } else {
> > +    if (MaxPhyAddrBits > 52) {
> > +      return FALSE;
> > +    }
> > +
> > +    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
> > +
> > +    if (Msr.Bits.LME == 0) {
> > +      //
> > +      // Check if linear address is valid in PAE paging mode
> > +      //
> > +      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits,
> > LinearAddress);
> > +    } else {
> > +      //
> > +      // Check if linear address is valid in 4-level paging mode
> > +      //
> > +      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
> > +                                              LinearAddress);
> > +    }
> > +  }
> > +
> > +  return AddressValid;
> > +}
> > diff --git
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> > index ec46c2d9d3..1b51034c25 100644
> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> > @@ -330,5 +330,21 @@ GetPdbFileName (
> >    OUT CHAR8    **PdbFileName
> >    );
> >
> > +/**
> > +  Check if a linear address is valid.
> > +
> > +  @param[in]  Cr0            CR0 control register.
> > +  @param[in]  Cr3            CR3 control register.
> > +  @param[in]  Cr4            CR4 control register.
> > +  @param[in]  LinearAddress  Linear address to be checked.
> > +**/
> > +BOOLEAN
> > +IsLinearAddressValid (
> > +  IN  UINTN              Cr0,
> > +  IN  UINTN              Cr3,
> > +  IN  UINTN              Cr4,
> > +  IN  UINTN              LinearAddress
> > +  );
> > +
> >  #endif
> >
> > --
> > 2.14.3
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Paulo Alcantara 6 years, 11 months ago 
"Yao, Jiewen" <jiewen.yao@intel.com> writes:

> 5) For CR4, please use meaning definition for BIT4/BIT5.
>   if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
>     if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||

OK.

>
> 6) For IA32 PAE/PSE calculation, same comment for 3 and 4.

OK.

>
> 7) Last but not least important, would you please share the information on how do you validate the 32bit PAE/PSE/normal 4K page table?

Since on IA32 we use 32-bit protected flat model and paging disabled
(OK?), I wasn't able to validate the paging modes other than 4-level
paging mode in X64. The memory validation code I wrote is heavily based
upon what I read from Intel SDM Vol 3A manual.

If you do have any idea on how to validate it -- whether it's a PoC or
test code -- please let me know, and then I validate it.

Thanks again for your review!

Paulo

>
> Thank you
> Yao Jiewen
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
>> Jiewen
>> Sent: Thursday, January 4, 2018 9:36 AM
>> To: Paulo Alcantara <paulo@paulo.ac>; edk2-devel@lists.01.org
>> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
>> Subject: Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add
>> helper to valid memory addresses
>> 
>> Some suggestion:
>> 
>> 1) I am not sure if it is proper to use ASSERT in an exception handler, because we
>> know something is wrong.
>> 
>>   ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
>> 
>> I suggest we just do the check, and return FALSE, if the prerequisite is not
>> satisfied.
>> 
>> 2) Can we use meaningful definition for BIT0, BIT7?
>> 
>>   if ((*Pml4TableEntry & BIT0) == 0) {
>>   if ((*PageDirPtrTableEntry & BIT7) != 0) {
>> 
>> 3) I am not sure if I understand below code.
>> 
>>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>>   PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1)
>> << 12);
>>   PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>> 
>> If MaxPhyAddrBits is 48, you will get "Cr3 & 0x0FFFFFFFFFFFF000". Is that what
>> you want? I think we need "Cr3 & 0x0000FFFFFFFFF000"
>> Should it be: PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) &
>> (~0xFFF);
>> 
>> 4) Can we use a more readable way to below? Personally, I do not suggest "<< 3",
>> which is just the index calculation.
>> 
>>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>>   PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
>>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>> 
>> For example:
>>   PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) & (~0xFFF);
>>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>>   Index= (UINTN)(((UINT64)LinearAddress >> 39) & 0x1FF);
>>   PhysicalAddress = Pml4TableEntry[Index] & ((1ULL << MaxPhyAddrBits) - 1) &
>> (~0xFFF);
>> 
>> 
>> 
>> Thank you
>> Yao Jiewen
>> 
>> 
>> > -----Original Message-----
>> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Paulo
>> > Alcantara
>> > Sent: Friday, December 29, 2017 12:40 PM
>> > To: edk2-devel@lists.01.org
>> > Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
>> > Subject: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper
>> to
>> > valid memory addresses
>> >
>> > Introduce IsLinearAddressValid() function that will be used for
>> > validating memory addresses that would get dereferenced during stack
>> > traces in IA32 and X64 CPU exceptions.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Cc: Eric Dong <eric.dong@intel.com>
>> > Cc: Laszlo Ersek <lersek@redhat.com>
>> > Requested-by: Brian Johnson <brian.johnson@hpe.com>
>> > Requested-by: Jiewen Yao <jiewen.yao@intel.com>
>> > Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
>> > ---
>> >  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c | 382
>> > ++++++++++++++++++++
>> >  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |
>> 16 +
>> >  2 files changed, 398 insertions(+)
>> >
>> > diff --git
>> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> > index 867c5c01d6..52b3eb1463 100644
>> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> > @@ -14,6 +14,9 @@
>> >
>> >  #include "CpuExceptionCommon.h"
>> >
>> > +#include <Register/Msr.h>
>> > +#include <Library/DebugLib.h>
>> > +
>> >  //
>> >  // Error code flag indicating whether or not an error code will be
>> >  // pushed on the stack if an exception occurs.
>> > @@ -194,3 +197,382 @@ GetPdbFileName (
>> >      }
>> >    }
>> >  }
>> > +
>> > +/**
>> > +  Check if a linear address is valid by walking the page tables in 4-level
>> > +  paging mode.
>> > +
>> > +  @param[in]  Cr3             CR3 control register.
>> > +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
>> > +  @param[in]  LinearAddress   Linear address to be checked.
>> > +**/
>> > +STATIC
>> > +BOOLEAN
>> > +Do4LevelPagingModeCheck (
>> > +  IN UINTN            Cr3,
>> > +  IN UINT8            MaxPhyAddrBits,
>> > +  IN UINTN            LinearAddress
>> > +  )
>> > +{
>> > +  UINT64 PhysicalAddress;
>> > +  UINT64 *Pml4TableEntry;
>> > +  UINT64 *PageDirPtrTableEntry;
>> > +  UINT64 *PageDirEntry;
>> > +  UINT64 *PageTableEntry;
>> > +
>> > +  //
>> > +  // In 4-level paging mode, linear addresses are 48 bits wide
>> > +  //
>> > +  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Calculate physical address of PML4E
>> > +  //
>> > +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
>> > +
>> > +  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
>> > +
>> > +  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check if a PDPTE is present
>> > +  //
>> > +  if ((*Pml4TableEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Calculate physical address of PDPTE
>> > +  //
>> > +  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> > 12);
>> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
>> > +
>> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
>> > +
>> > +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check whether a PDPTE or 1GiB page entry is present
>> > +  //
>> > +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Check if PDPTE maps an 1GiB page
>> > +  //
>> > +  if ((*PageDirPtrTableEntry & BIT7) != 0) {
>> > +    return TRUE;
>> > +  }
>> > +
>> > +  //
>> > +  // Calculate physical address of PDE
>> > +  //
>> > +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) -
>> 1)
>> > <<
>> > +                                             12);
>> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
>> > +
>> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> > +
>> > +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check whether a PDE or a 2MiB page entry is present
>> > +  //
>> > +  if ((*PageDirEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Check if PDE maps a 2MiB page
>> > +  //
>> > +  if ((*PageDirEntry & BIT7) != 0) {
>> > +    return TRUE;
>> > +  }
>> > +
>> > +  //
>> > +  // Calculate physical address of PTE
>> > +  //
>> > +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> 12);
>> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
>> > +
>> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> > +
>> > +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check if PTE maps a 4KiB page
>> > +  //
>> > +  if ((*PageTableEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  return TRUE;
>> > +}
>> > +
>> > +/**
>> > +  Check if a linear address is valid by walking the page tables in 32-bit paging
>> > +  mode.
>> > +
>> > +  @param[in]  Cr3             CR3 control register.
>> > +  @param[in]  Cr4             CR4 control register.
>> > +  @param[in]  LinearAddress   Linear address to be checked.
>> > +**/
>> > +STATIC
>> > +BOOLEAN
>> > +Do32BitPagingModeCheck (
>> > +  IN UINTN            Cr3,
>> > +  IN UINTN            Cr4,
>> > +  IN UINTN            LinearAddress
>> > +  )
>> > +{
>> > +  UINT64 PhysicalAddress;
>> > +  UINT32 *PageDirEntry;
>> > +  UINT32 *PageTableEntry;
>> > +
>> > +  if (LinearAddress > MAX_UINT32) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Calculate physical address of PDE
>> > +  //
>> > +  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
>> > +  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
>> > +
>> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> > +
>> > +  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check whether a PTE or a 4MiB page is present
>> > +  //
>> > +  if ((*PageDirEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Check if PDE maps a 4MiB page
>> > +  //
>> > +  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
>> > +    return TRUE;
>> > +  }
>> > +
>> > +  //
>> > +  // Calculate physical address of PTE
>> > +  //
>> > +  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
>> > +  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
>> > +
>> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> > +
>> > +  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check if PTE maps a 4KiB page
>> > +  //
>> > +  if ((*PageTableEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  return TRUE;
>> > +}
>> > +
>> > +/**
>> > +  Check if a linear address is valid by walking the page tables in PAE paging
>> > +  mode.
>> > +
>> > +  @param[in]  Cr3             CR3 control register.
>> > +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
>> > +  @param[in]  LinearAddress   Linear address to be checked.
>> > +**/
>> > +STATIC
>> > +BOOLEAN
>> > +DoPAEPagingModeCheck (
>> > +  IN UINTN            Cr3,
>> > +  IN UINT8            MaxPhyAddrBits,
>> > +  IN UINTN            LinearAddress
>> > +  )
>> > +{
>> > +  UINT64 PhysicalAddress;
>> > +  UINT64 *PageDirPtrTableEntry;
>> > +  UINT64 *PageDirEntry;
>> > +  UINT64 *PageTableEntry;
>> > +
>> > +  if (LinearAddress > MAX_UINT32) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Calculate physical address of PDPTE
>> > +  //
>> > +  PhysicalAddress = (UINT32)Cr3 >> 5;
>> > +
>> > +  //
>> > +  // Select PDPTE register
>> > +  //
>> > +  PhysicalAddress +=
>> > +    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
>> > +
>> > +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check if PDE is present
>> > +  //
>> > +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) -
>> 1)
>> > <<
>> > +                                             12);
>> > +  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
>> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> > +
>> > +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check whether a PTE or a 2MiB page is present
>> > +  //
>> > +  if ((*PageDirEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Check if PDE maps a 2MiB page
>> > +  //
>> > +  if ((*PageDirEntry & BIT7) != 0) {
>> > +    return TRUE;
>> > +  }
>> > +
>> > +  //
>> > +  // Calculate physical address of PTE
>> > +  //
>> > +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> 12);
>> > +  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
>> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> > +
>> > +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> > +
>> > +  //
>> > +  // Check if PTE maps a 4KiB page
>> > +  //
>> > +  if ((*PageTableEntry & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  return TRUE;
>> > +}
>> > +
>> > +/**
>> > +  Check if a linear address is valid.
>> > +
>> > +  @param[in]  Cr0            CR0 control register.
>> > +  @param[in]  Cr3            CR3 control register.
>> > +  @param[in]  Cr4            CR4 control register.
>> > +  @param[in]  LinearAddress  Linear address to be checked.
>> > +**/
>> > +BOOLEAN
>> > +IsLinearAddressValid (
>> > +  IN  UINTN              Cr0,
>> > +  IN  UINTN              Cr3,
>> > +  IN  UINTN              Cr4,
>> > +  IN  UINTN              LinearAddress
>> > +  )
>> > +{
>> > +  UINT32                  Eax;
>> > +  UINT32                  Edx;
>> > +  UINT8                   MaxPhyAddrBits;
>> > +  MSR_IA32_EFER_REGISTER  Msr;
>> > +  BOOLEAN                 AddressValid;
>> > +
>> > +  //
>> > +  // Check for valid input parameters
>> > +  //
>> > +  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Check if paging is disabled
>> > +  //
>> > +  if ((Cr0 & BIT31) == 0) {
>> > +    //
>> > +    // If CR4.PAE bit is set, then the linear (or physical) address supports
>> > +    // only up to 36 bits.
>> > +    //
>> > +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
>> > +        LinearAddress > 0xFFFFFFFF) {
>> > +      return FALSE;
>> > +    }
>> > +
>> > +    return TRUE;
>> > +  }
>> > +
>> > +  //
>> > +  // Paging can be enabled only if CR0.PE bit is set
>> > +  //
>> > +  if ((Cr0 & BIT0) == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // CR3 register cannot be zero if paging is enabled
>> > +  //
>> > +  if (Cr3 == 0) {
>> > +    return FALSE;
>> > +  }
>> > +
>> > +  //
>> > +  // Get MAXPHYADDR bits
>> > +  //
>> > +  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
>> > +  if (Eax >= 0x80000008) {
>> > +    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
>> > +    MaxPhyAddrBits = (UINT8)Eax;
>> > +  } else {
>> > +    AsmCpuid (1, NULL, NULL, NULL, &Edx);
>> > +    if ((Edx & BIT6) != 0) {
>> > +      MaxPhyAddrBits = 36;
>> > +    } else {
>> > +      MaxPhyAddrBits = 32;
>> > +    }
>> > +  }
>> > +
>> > +  ASSERT (MaxPhyAddrBits > 0);
>> > +
>> > +  AddressValid = FALSE;
>> > +
>> > +  //
>> > +  // check if CR4.PAE bit is not set
>> > +  //
>> > +  if ((Cr4 & BIT5) == 0) {
>> > +    //
>> > +    // Check if linear address is valid in 32-bit paging mode
>> > +    //
>> > +    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
>> > +  } else {
>> > +    if (MaxPhyAddrBits > 52) {
>> > +      return FALSE;
>> > +    }
>> > +
>> > +    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
>> > +
>> > +    if (Msr.Bits.LME == 0) {
>> > +      //
>> > +      // Check if linear address is valid in PAE paging mode
>> > +      //
>> > +      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits,
>> > LinearAddress);
>> > +    } else {
>> > +      //
>> > +      // Check if linear address is valid in 4-level paging mode
>> > +      //
>> > +      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
>> > +                                              LinearAddress);
>> > +    }
>> > +  }
>> > +
>> > +  return AddressValid;
>> > +}
>> > diff --git
>> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> > index ec46c2d9d3..1b51034c25 100644
>> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> > @@ -330,5 +330,21 @@ GetPdbFileName (
>> >    OUT CHAR8    **PdbFileName
>> >    );
>> >
>> > +/**
>> > +  Check if a linear address is valid.
>> > +
>> > +  @param[in]  Cr0            CR0 control register.
>> > +  @param[in]  Cr3            CR3 control register.
>> > +  @param[in]  Cr4            CR4 control register.
>> > +  @param[in]  LinearAddress  Linear address to be checked.
>> > +**/
>> > +BOOLEAN
>> > +IsLinearAddressValid (
>> > +  IN  UINTN              Cr0,
>> > +  IN  UINTN              Cr3,
>> > +  IN  UINTN              Cr4,
>> > +  IN  UINTN              LinearAddress
>> > +  );
>> > +
>> >  #endif
>> >
>> > --
>> > 2.14.3
>> >
>> > _______________________________________________
>> > edk2-devel mailing list
>> > edk2-devel@lists.01.org
>> > https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Yao, Jiewen 6 years, 11 months ago 
Thank you!

For 7), you are right that we disable IA32 paging by default.
However, we do support IA32 PAE in DxeIpl. Please refer to MdeModulePkg\Core\DxeIplPeim\Ia32\DxeLoadFunc.c
======================
    PageTables = 0;
    BuildPageTablesIa32Pae = (BOOLEAN) (PcdGetBool (PcdSetNxForStack) && IsIa32PaeSupport () && IsExecuteDisableBitAvailable ());
    if (BuildPageTablesIa32Pae) {
      PageTables = Create4GPageTablesIa32Pae (BaseOfStack, STACK_SIZE);
      EnableExecuteDisableBit ();
    }
======================

Please notice that we only support IA32 PAE, we do not support IA32 non-PAE mode so far. (no matter PSE is ON/OFF)

So, I suggest:
7.1) Please validate IA32 PAE mode. (You can enable PcdSetNxForStack)
7.2) If we cannot validate the IA32 non-PAE code, please remove them. You can just print "IA32 non-PAE - UNSUPPORTED" and return invalid address. Then it can save our development time, review time, and validation time. The key is that we only want to check in the validated code.

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Paulo
> Alcantara
> Sent: Thursday, January 4, 2018 9:30 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add
> helper to valid memory addresses
> 
> "Yao, Jiewen" <jiewen.yao@intel.com> writes:
> 
> > 5) For CR4, please use meaning definition for BIT4/BIT5.
> >   if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
> >     if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
> 
> OK.
> 
> >
> > 6) For IA32 PAE/PSE calculation, same comment for 3 and 4.
> 
> OK.
> 
> >
> > 7) Last but not least important, would you please share the information on
> how do you validate the 32bit PAE/PSE/normal 4K page table?
> 
> Since on IA32 we use 32-bit protected flat model and paging disabled
> (OK?), I wasn't able to validate the paging modes other than 4-level
> paging mode in X64. The memory validation code I wrote is heavily based
> upon what I read from Intel SDM Vol 3A manual.
> 
> If you do have any idea on how to validate it -- whether it's a PoC or
> test code -- please let me know, and then I validate it.
> 
> Thanks again for your review!
> 
> Paulo
> 
> >
> > Thank you
> > Yao Jiewen
> >
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Yao,
> >> Jiewen
> >> Sent: Thursday, January 4, 2018 9:36 AM
> >> To: Paulo Alcantara <paulo@paulo.ac>; edk2-devel@lists.01.org
> >> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> >> Subject: Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add
> >> helper to valid memory addresses
> >>
> >> Some suggestion:
> >>
> >> 1) I am not sure if it is proper to use ASSERT in an exception handler, because
> we
> >> know something is wrong.
> >>
> >>   ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
> >>
> >> I suggest we just do the check, and return FALSE, if the prerequisite is not
> >> satisfied.
> >>
> >> 2) Can we use meaningful definition for BIT0, BIT7?
> >>
> >>   if ((*Pml4TableEntry & BIT0) == 0) {
> >>   if ((*PageDirPtrTableEntry & BIT7) != 0) {
> >>
> >> 3) I am not sure if I understand below code.
> >>
> >>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> >>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> 12);
> >>   PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) -
> 1)
> >> << 12);
> >>   PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> 12);
> >>
> >> If MaxPhyAddrBits is 48, you will get "Cr3 & 0x0FFFFFFFFFFFF000". Is that
> what
> >> you want? I think we need "Cr3 & 0x0000FFFFFFFFF000"
> >> Should it be: PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1)
> &
> >> (~0xFFF);
> >>
> >> 4) Can we use a more readable way to below? Personally, I do not suggest
> "<< 3",
> >> which is just the index calculation.
> >>
> >>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> >>   PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
> >>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> 12);
> >>
> >> For example:
> >>   PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) &
> (~0xFFF);
> >>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >>   Index= (UINTN)(((UINT64)LinearAddress >> 39) & 0x1FF);
> >>   PhysicalAddress = Pml4TableEntry[Index] & ((1ULL << MaxPhyAddrBits) - 1)
> &
> >> (~0xFFF);
> >>
> >>
> >>
> >> Thank you
> >> Yao Jiewen
> >>
> >>
> >> > -----Original Message-----
> >> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> >> Paulo
> >> > Alcantara
> >> > Sent: Friday, December 29, 2017 12:40 PM
> >> > To: edk2-devel@lists.01.org
> >> > Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> >> > Subject: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add
> helper
> >> to
> >> > valid memory addresses
> >> >
> >> > Introduce IsLinearAddressValid() function that will be used for
> >> > validating memory addresses that would get dereferenced during stack
> >> > traces in IA32 and X64 CPU exceptions.
> >> >
> >> > Contributed-under: TianoCore Contribution Agreement 1.1
> >> > Cc: Eric Dong <eric.dong@intel.com>
> >> > Cc: Laszlo Ersek <lersek@redhat.com>
> >> > Requested-by: Brian Johnson <brian.johnson@hpe.com>
> >> > Requested-by: Jiewen Yao <jiewen.yao@intel.com>
> >> > Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
> >> > ---
> >> >  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c |
> 382
> >> > ++++++++++++++++++++
> >> >  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |
> >> 16 +
> >> >  2 files changed, 398 insertions(+)
> >> >
> >> > diff --git
> >> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> >> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> >> > index 867c5c01d6..52b3eb1463 100644
> >> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> >> > +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> >> > @@ -14,6 +14,9 @@
> >> >
> >> >  #include "CpuExceptionCommon.h"
> >> >
> >> > +#include <Register/Msr.h>
> >> > +#include <Library/DebugLib.h>
> >> > +
> >> >  //
> >> >  // Error code flag indicating whether or not an error code will be
> >> >  // pushed on the stack if an exception occurs.
> >> > @@ -194,3 +197,382 @@ GetPdbFileName (
> >> >      }
> >> >    }
> >> >  }
> >> > +
> >> > +/**
> >> > +  Check if a linear address is valid by walking the page tables in 4-level
> >> > +  paging mode.
> >> > +
> >> > +  @param[in]  Cr3             CR3 control register.
> >> > +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> >> > +  @param[in]  LinearAddress   Linear address to be checked.
> >> > +**/
> >> > +STATIC
> >> > +BOOLEAN
> >> > +Do4LevelPagingModeCheck (
> >> > +  IN UINTN            Cr3,
> >> > +  IN UINT8            MaxPhyAddrBits,
> >> > +  IN UINTN            LinearAddress
> >> > +  )
> >> > +{
> >> > +  UINT64 PhysicalAddress;
> >> > +  UINT64 *Pml4TableEntry;
> >> > +  UINT64 *PageDirPtrTableEntry;
> >> > +  UINT64 *PageDirEntry;
> >> > +  UINT64 *PageTableEntry;
> >> > +
> >> > +  //
> >> > +  // In 4-level paging mode, linear addresses are 48 bits wide
> >> > +  //
> >> > +  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Calculate physical address of PML4E
> >> > +  //
> >> > +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) <<
> 12);
> >> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
> >> > +
> >> > +  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
> >> > +
> >> > +  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check if a PDPTE is present
> >> > +  //
> >> > +  if ((*Pml4TableEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Calculate physical address of PDPTE
> >> > +  //
> >> > +  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1)
> <<
> >> > 12);
> >> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
> >> > +
> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
> >> > +
> >> > +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check whether a PDPTE or 1GiB page entry is present
> >> > +  //
> >> > +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Check if PDPTE maps an 1GiB page
> >> > +  //
> >> > +  if ((*PageDirPtrTableEntry & BIT7) != 0) {
> >> > +    return TRUE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Calculate physical address of PDE
> >> > +  //
> >> > +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits)
> -
> >> 1)
> >> > <<
> >> > +                                             12);
> >> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
> >> > +
> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> >> > +
> >> > +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check whether a PDE or a 2MiB page entry is present
> >> > +  //
> >> > +  if ((*PageDirEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Check if PDE maps a 2MiB page
> >> > +  //
> >> > +  if ((*PageDirEntry & BIT7) != 0) {
> >> > +    return TRUE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Calculate physical address of PTE
> >> > +  //
> >> > +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> >> 12);
> >> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
> >> > +
> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> >> > +
> >> > +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check if PTE maps a 4KiB page
> >> > +  //
> >> > +  if ((*PageTableEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  return TRUE;
> >> > +}
> >> > +
> >> > +/**
> >> > +  Check if a linear address is valid by walking the page tables in 32-bit
> paging
> >> > +  mode.
> >> > +
> >> > +  @param[in]  Cr3             CR3 control register.
> >> > +  @param[in]  Cr4             CR4 control register.
> >> > +  @param[in]  LinearAddress   Linear address to be checked.
> >> > +**/
> >> > +STATIC
> >> > +BOOLEAN
> >> > +Do32BitPagingModeCheck (
> >> > +  IN UINTN            Cr3,
> >> > +  IN UINTN            Cr4,
> >> > +  IN UINTN            LinearAddress
> >> > +  )
> >> > +{
> >> > +  UINT64 PhysicalAddress;
> >> > +  UINT32 *PageDirEntry;
> >> > +  UINT32 *PageTableEntry;
> >> > +
> >> > +  if (LinearAddress > MAX_UINT32) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Calculate physical address of PDE
> >> > +  //
> >> > +  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
> >> > +  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
> >> > +
> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> >> > +
> >> > +  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check whether a PTE or a 4MiB page is present
> >> > +  //
> >> > +  if ((*PageDirEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Check if PDE maps a 4MiB page
> >> > +  //
> >> > +  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
> >> > +    return TRUE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Calculate physical address of PTE
> >> > +  //
> >> > +  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
> >> > +  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
> >> > +
> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> >> > +
> >> > +  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check if PTE maps a 4KiB page
> >> > +  //
> >> > +  if ((*PageTableEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  return TRUE;
> >> > +}
> >> > +
> >> > +/**
> >> > +  Check if a linear address is valid by walking the page tables in PAE paging
> >> > +  mode.
> >> > +
> >> > +  @param[in]  Cr3             CR3 control register.
> >> > +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> >> > +  @param[in]  LinearAddress   Linear address to be checked.
> >> > +**/
> >> > +STATIC
> >> > +BOOLEAN
> >> > +DoPAEPagingModeCheck (
> >> > +  IN UINTN            Cr3,
> >> > +  IN UINT8            MaxPhyAddrBits,
> >> > +  IN UINTN            LinearAddress
> >> > +  )
> >> > +{
> >> > +  UINT64 PhysicalAddress;
> >> > +  UINT64 *PageDirPtrTableEntry;
> >> > +  UINT64 *PageDirEntry;
> >> > +  UINT64 *PageTableEntry;
> >> > +
> >> > +  if (LinearAddress > MAX_UINT32) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Calculate physical address of PDPTE
> >> > +  //
> >> > +  PhysicalAddress = (UINT32)Cr3 >> 5;
> >> > +
> >> > +  //
> >> > +  // Select PDPTE register
> >> > +  //
> >> > +  PhysicalAddress +=
> >> > +    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
> >> > +
> >> > +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check if PDE is present
> >> > +  //
> >> > +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits)
> -
> >> 1)
> >> > <<
> >> > +                                             12);
> >> > +  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> >> > +
> >> > +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check whether a PTE or a 2MiB page is present
> >> > +  //
> >> > +  if ((*PageDirEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Check if PDE maps a 2MiB page
> >> > +  //
> >> > +  if ((*PageDirEntry & BIT7) != 0) {
> >> > +    return TRUE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Calculate physical address of PTE
> >> > +  //
> >> > +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
> >> 12);
> >> > +  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> >> > +
> >> > +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> >> > +
> >> > +  //
> >> > +  // Check if PTE maps a 4KiB page
> >> > +  //
> >> > +  if ((*PageTableEntry & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  return TRUE;
> >> > +}
> >> > +
> >> > +/**
> >> > +  Check if a linear address is valid.
> >> > +
> >> > +  @param[in]  Cr0            CR0 control register.
> >> > +  @param[in]  Cr3            CR3 control register.
> >> > +  @param[in]  Cr4            CR4 control register.
> >> > +  @param[in]  LinearAddress  Linear address to be checked.
> >> > +**/
> >> > +BOOLEAN
> >> > +IsLinearAddressValid (
> >> > +  IN  UINTN              Cr0,
> >> > +  IN  UINTN              Cr3,
> >> > +  IN  UINTN              Cr4,
> >> > +  IN  UINTN              LinearAddress
> >> > +  )
> >> > +{
> >> > +  UINT32                  Eax;
> >> > +  UINT32                  Edx;
> >> > +  UINT8                   MaxPhyAddrBits;
> >> > +  MSR_IA32_EFER_REGISTER  Msr;
> >> > +  BOOLEAN                 AddressValid;
> >> > +
> >> > +  //
> >> > +  // Check for valid input parameters
> >> > +  //
> >> > +  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Check if paging is disabled
> >> > +  //
> >> > +  if ((Cr0 & BIT31) == 0) {
> >> > +    //
> >> > +    // If CR4.PAE bit is set, then the linear (or physical) address supports
> >> > +    // only up to 36 bits.
> >> > +    //
> >> > +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL)
> ||
> >> > +        LinearAddress > 0xFFFFFFFF) {
> >> > +      return FALSE;
> >> > +    }
> >> > +
> >> > +    return TRUE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Paging can be enabled only if CR0.PE bit is set
> >> > +  //
> >> > +  if ((Cr0 & BIT0) == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // CR3 register cannot be zero if paging is enabled
> >> > +  //
> >> > +  if (Cr3 == 0) {
> >> > +    return FALSE;
> >> > +  }
> >> > +
> >> > +  //
> >> > +  // Get MAXPHYADDR bits
> >> > +  //
> >> > +  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
> >> > +  if (Eax >= 0x80000008) {
> >> > +    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
> >> > +    MaxPhyAddrBits = (UINT8)Eax;
> >> > +  } else {
> >> > +    AsmCpuid (1, NULL, NULL, NULL, &Edx);
> >> > +    if ((Edx & BIT6) != 0) {
> >> > +      MaxPhyAddrBits = 36;
> >> > +    } else {
> >> > +      MaxPhyAddrBits = 32;
> >> > +    }
> >> > +  }
> >> > +
> >> > +  ASSERT (MaxPhyAddrBits > 0);
> >> > +
> >> > +  AddressValid = FALSE;
> >> > +
> >> > +  //
> >> > +  // check if CR4.PAE bit is not set
> >> > +  //
> >> > +  if ((Cr4 & BIT5) == 0) {
> >> > +    //
> >> > +    // Check if linear address is valid in 32-bit paging mode
> >> > +    //
> >> > +    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
> >> > +  } else {
> >> > +    if (MaxPhyAddrBits > 52) {
> >> > +      return FALSE;
> >> > +    }
> >> > +
> >> > +    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
> >> > +
> >> > +    if (Msr.Bits.LME == 0) {
> >> > +      //
> >> > +      // Check if linear address is valid in PAE paging mode
> >> > +      //
> >> > +      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits,
> >> > LinearAddress);
> >> > +    } else {
> >> > +      //
> >> > +      // Check if linear address is valid in 4-level paging mode
> >> > +      //
> >> > +      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
> >> > +                                              LinearAddress);
> >> > +    }
> >> > +  }
> >> > +
> >> > +  return AddressValid;
> >> > +}
> >> > diff --git
> >> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> >> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> >> > index ec46c2d9d3..1b51034c25 100644
> >> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> >> > +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> >> > @@ -330,5 +330,21 @@ GetPdbFileName (
> >> >    OUT CHAR8    **PdbFileName
> >> >    );
> >> >
> >> > +/**
> >> > +  Check if a linear address is valid.
> >> > +
> >> > +  @param[in]  Cr0            CR0 control register.
> >> > +  @param[in]  Cr3            CR3 control register.
> >> > +  @param[in]  Cr4            CR4 control register.
> >> > +  @param[in]  LinearAddress  Linear address to be checked.
> >> > +**/
> >> > +BOOLEAN
> >> > +IsLinearAddressValid (
> >> > +  IN  UINTN              Cr0,
> >> > +  IN  UINTN              Cr3,
> >> > +  IN  UINTN              Cr4,
> >> > +  IN  UINTN              LinearAddress
> >> > +  );
> >> > +
> >> >  #endif
> >> >
> >> > --
> >> > 2.14.3
> >> >
> >> > _______________________________________________
> >> > edk2-devel mailing list
> >> > edk2-devel@lists.01.org
> >> > https://lists.01.org/mailman/listinfo/edk2-devel
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Paulo Alcantara 6 years, 11 months ago 
"Yao, Jiewen" <jiewen.yao@intel.com> writes:

Hi Jiewen,

> For 7), you are right that we disable IA32 paging by default.
> However, we do support IA32 PAE in DxeIpl. Please refer to MdeModulePkg\Core\DxeIplPeim\Ia32\DxeLoadFunc.c
> ======================
>     PageTables = 0;
>     BuildPageTablesIa32Pae = (BOOLEAN) (PcdGetBool (PcdSetNxForStack) && IsIa32PaeSupport () && IsExecuteDisableBitAvailable ());
>     if (BuildPageTablesIa32Pae) {
>       PageTables = Create4GPageTablesIa32Pae (BaseOfStack, STACK_SIZE);
>       EnableExecuteDisableBit ();
>     }
> ======================
>
> Please notice that we only support IA32 PAE, we do not support IA32
> non-PAE mode so far. (no matter PSE is ON/OFF)

Cool! I didn't know about it.

>
> So, I suggest:
> 7.1) Please validate IA32 PAE mode. (You can enable PcdSetNxForStack)

I will. Thanks!

> 7.2) If we cannot validate the IA32 non-PAE code, please remove
> them. You can just print "IA32 non-PAE - UNSUPPORTED" and return
> invalid address. Then it can save our development time, review time,
> and validation time. The key is that we only want to check in the
> validated code.

You're right. I'll keep only the code that we were able to validate its
implementation (e.g. 4-level & 32-bit PAE).

Thanks!
Paulo

>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Paulo
>> Alcantara
>> Sent: Thursday, January 4, 2018 9:30 PM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
>> edk2-devel@lists.01.org
>> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
>> Subject: Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add
>> helper to valid memory addresses
>> 
>> "Yao, Jiewen" <jiewen.yao@intel.com> writes:
>> 
>> > 5) For CR4, please use meaning definition for BIT4/BIT5.
>> >   if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
>> >     if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
>> 
>> OK.
>> 
>> >
>> > 6) For IA32 PAE/PSE calculation, same comment for 3 and 4.
>> 
>> OK.
>> 
>> >
>> > 7) Last but not least important, would you please share the information on
>> how do you validate the 32bit PAE/PSE/normal 4K page table?
>> 
>> Since on IA32 we use 32-bit protected flat model and paging disabled
>> (OK?), I wasn't able to validate the paging modes other than 4-level
>> paging mode in X64. The memory validation code I wrote is heavily based
>> upon what I read from Intel SDM Vol 3A manual.
>> 
>> If you do have any idea on how to validate it -- whether it's a PoC or
>> test code -- please let me know, and then I validate it.
>> 
>> Thanks again for your review!
>> 
>> Paulo
>> 
>> >
>> > Thank you
>> > Yao Jiewen
>> >
>> >> -----Original Message-----
>> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Yao,
>> >> Jiewen
>> >> Sent: Thursday, January 4, 2018 9:36 AM
>> >> To: Paulo Alcantara <paulo@paulo.ac>; edk2-devel@lists.01.org
>> >> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
>> >> Subject: Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add
>> >> helper to valid memory addresses
>> >>
>> >> Some suggestion:
>> >>
>> >> 1) I am not sure if it is proper to use ASSERT in an exception handler, because
>> we
>> >> know something is wrong.
>> >>
>> >>   ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
>> >>
>> >> I suggest we just do the check, and return FALSE, if the prerequisite is not
>> >> satisfied.
>> >>
>> >> 2) Can we use meaningful definition for BIT0, BIT7?
>> >>
>> >>   if ((*Pml4TableEntry & BIT0) == 0) {
>> >>   if ((*PageDirPtrTableEntry & BIT7) != 0) {
>> >>
>> >> 3) I am not sure if I understand below code.
>> >>
>> >>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>> >>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> 12);
>> >>   PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) -
>> 1)
>> >> << 12);
>> >>   PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> 12);
>> >>
>> >> If MaxPhyAddrBits is 48, you will get "Cr3 & 0x0FFFFFFFFFFFF000". Is that
>> what
>> >> you want? I think we need "Cr3 & 0x0000FFFFFFFFF000"
>> >> Should it be: PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1)
>> &
>> >> (~0xFFF);
>> >>
>> >> 4) Can we use a more readable way to below? Personally, I do not suggest
>> "<< 3",
>> >> which is just the index calculation.
>> >>
>> >>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>> >>   PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
>> >>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> 12);
>> >>
>> >> For example:
>> >>   PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) &
>> (~0xFFF);
>> >>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >>   Index= (UINTN)(((UINT64)LinearAddress >> 39) & 0x1FF);
>> >>   PhysicalAddress = Pml4TableEntry[Index] & ((1ULL << MaxPhyAddrBits) - 1)
>> &
>> >> (~0xFFF);
>> >>
>> >>
>> >>
>> >> Thank you
>> >> Yao Jiewen
>> >>
>> >>
>> >> > -----Original Message-----
>> >> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> >> Paulo
>> >> > Alcantara
>> >> > Sent: Friday, December 29, 2017 12:40 PM
>> >> > To: edk2-devel@lists.01.org
>> >> > Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
>> >> > Subject: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add
>> helper
>> >> to
>> >> > valid memory addresses
>> >> >
>> >> > Introduce IsLinearAddressValid() function that will be used for
>> >> > validating memory addresses that would get dereferenced during stack
>> >> > traces in IA32 and X64 CPU exceptions.
>> >> >
>> >> > Contributed-under: TianoCore Contribution Agreement 1.1
>> >> > Cc: Eric Dong <eric.dong@intel.com>
>> >> > Cc: Laszlo Ersek <lersek@redhat.com>
>> >> > Requested-by: Brian Johnson <brian.johnson@hpe.com>
>> >> > Requested-by: Jiewen Yao <jiewen.yao@intel.com>
>> >> > Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
>> >> > ---
>> >> >  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c |
>> 382
>> >> > ++++++++++++++++++++
>> >> >  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |
>> >> 16 +
>> >> >  2 files changed, 398 insertions(+)
>> >> >
>> >> > diff --git
>> >> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> >> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> >> > index 867c5c01d6..52b3eb1463 100644
>> >> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> >> > +++
>> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> >> > @@ -14,6 +14,9 @@
>> >> >
>> >> >  #include "CpuExceptionCommon.h"
>> >> >
>> >> > +#include <Register/Msr.h>
>> >> > +#include <Library/DebugLib.h>
>> >> > +
>> >> >  //
>> >> >  // Error code flag indicating whether or not an error code will be
>> >> >  // pushed on the stack if an exception occurs.
>> >> > @@ -194,3 +197,382 @@ GetPdbFileName (
>> >> >      }
>> >> >    }
>> >> >  }
>> >> > +
>> >> > +/**
>> >> > +  Check if a linear address is valid by walking the page tables in 4-level
>> >> > +  paging mode.
>> >> > +
>> >> > +  @param[in]  Cr3             CR3 control register.
>> >> > +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
>> >> > +  @param[in]  LinearAddress   Linear address to be checked.
>> >> > +**/
>> >> > +STATIC
>> >> > +BOOLEAN
>> >> > +Do4LevelPagingModeCheck (
>> >> > +  IN UINTN            Cr3,
>> >> > +  IN UINT8            MaxPhyAddrBits,
>> >> > +  IN UINTN            LinearAddress
>> >> > +  )
>> >> > +{
>> >> > +  UINT64 PhysicalAddress;
>> >> > +  UINT64 *Pml4TableEntry;
>> >> > +  UINT64 *PageDirPtrTableEntry;
>> >> > +  UINT64 *PageDirEntry;
>> >> > +  UINT64 *PageTableEntry;
>> >> > +
>> >> > +  //
>> >> > +  // In 4-level paging mode, linear addresses are 48 bits wide
>> >> > +  //
>> >> > +  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Calculate physical address of PML4E
>> >> > +  //
>> >> > +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) <<
>> 12);
>> >> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
>> >> > +
>> >> > +  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
>> >> > +
>> >> > +  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check if a PDPTE is present
>> >> > +  //
>> >> > +  if ((*Pml4TableEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Calculate physical address of PDPTE
>> >> > +  //
>> >> > +  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1)
>> <<
>> >> > 12);
>> >> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
>> >> > +
>> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
>> >> > +
>> >> > +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check whether a PDPTE or 1GiB page entry is present
>> >> > +  //
>> >> > +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Check if PDPTE maps an 1GiB page
>> >> > +  //
>> >> > +  if ((*PageDirPtrTableEntry & BIT7) != 0) {
>> >> > +    return TRUE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Calculate physical address of PDE
>> >> > +  //
>> >> > +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits)
>> -
>> >> 1)
>> >> > <<
>> >> > +                                             12);
>> >> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
>> >> > +
>> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> >> > +
>> >> > +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check whether a PDE or a 2MiB page entry is present
>> >> > +  //
>> >> > +  if ((*PageDirEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Check if PDE maps a 2MiB page
>> >> > +  //
>> >> > +  if ((*PageDirEntry & BIT7) != 0) {
>> >> > +    return TRUE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Calculate physical address of PTE
>> >> > +  //
>> >> > +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> >> 12);
>> >> > +  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
>> >> > +
>> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> >> > +
>> >> > +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check if PTE maps a 4KiB page
>> >> > +  //
>> >> > +  if ((*PageTableEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  return TRUE;
>> >> > +}
>> >> > +
>> >> > +/**
>> >> > +  Check if a linear address is valid by walking the page tables in 32-bit
>> paging
>> >> > +  mode.
>> >> > +
>> >> > +  @param[in]  Cr3             CR3 control register.
>> >> > +  @param[in]  Cr4             CR4 control register.
>> >> > +  @param[in]  LinearAddress   Linear address to be checked.
>> >> > +**/
>> >> > +STATIC
>> >> > +BOOLEAN
>> >> > +Do32BitPagingModeCheck (
>> >> > +  IN UINTN            Cr3,
>> >> > +  IN UINTN            Cr4,
>> >> > +  IN UINTN            LinearAddress
>> >> > +  )
>> >> > +{
>> >> > +  UINT64 PhysicalAddress;
>> >> > +  UINT32 *PageDirEntry;
>> >> > +  UINT32 *PageTableEntry;
>> >> > +
>> >> > +  if (LinearAddress > MAX_UINT32) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Calculate physical address of PDE
>> >> > +  //
>> >> > +  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
>> >> > +  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
>> >> > +
>> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> >> > +
>> >> > +  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check whether a PTE or a 4MiB page is present
>> >> > +  //
>> >> > +  if ((*PageDirEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Check if PDE maps a 4MiB page
>> >> > +  //
>> >> > +  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
>> >> > +    return TRUE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Calculate physical address of PTE
>> >> > +  //
>> >> > +  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
>> >> > +  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
>> >> > +
>> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> >> > +
>> >> > +  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check if PTE maps a 4KiB page
>> >> > +  //
>> >> > +  if ((*PageTableEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  return TRUE;
>> >> > +}
>> >> > +
>> >> > +/**
>> >> > +  Check if a linear address is valid by walking the page tables in PAE paging
>> >> > +  mode.
>> >> > +
>> >> > +  @param[in]  Cr3             CR3 control register.
>> >> > +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
>> >> > +  @param[in]  LinearAddress   Linear address to be checked.
>> >> > +**/
>> >> > +STATIC
>> >> > +BOOLEAN
>> >> > +DoPAEPagingModeCheck (
>> >> > +  IN UINTN            Cr3,
>> >> > +  IN UINT8            MaxPhyAddrBits,
>> >> > +  IN UINTN            LinearAddress
>> >> > +  )
>> >> > +{
>> >> > +  UINT64 PhysicalAddress;
>> >> > +  UINT64 *PageDirPtrTableEntry;
>> >> > +  UINT64 *PageDirEntry;
>> >> > +  UINT64 *PageTableEntry;
>> >> > +
>> >> > +  if (LinearAddress > MAX_UINT32) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Calculate physical address of PDPTE
>> >> > +  //
>> >> > +  PhysicalAddress = (UINT32)Cr3 >> 5;
>> >> > +
>> >> > +  //
>> >> > +  // Select PDPTE register
>> >> > +  //
>> >> > +  PhysicalAddress +=
>> >> > +    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
>> >> > +
>> >> > +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check if PDE is present
>> >> > +  //
>> >> > +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits)
>> -
>> >> 1)
>> >> > <<
>> >> > +                                             12);
>> >> > +  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
>> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> >> > +
>> >> > +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check whether a PTE or a 2MiB page is present
>> >> > +  //
>> >> > +  if ((*PageDirEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Check if PDE maps a 2MiB page
>> >> > +  //
>> >> > +  if ((*PageDirEntry & BIT7) != 0) {
>> >> > +    return TRUE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Calculate physical address of PTE
>> >> > +  //
>> >> > +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> >> 12);
>> >> > +  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
>> >> > +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> >> > +
>> >> > +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> >> > +
>> >> > +  //
>> >> > +  // Check if PTE maps a 4KiB page
>> >> > +  //
>> >> > +  if ((*PageTableEntry & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  return TRUE;
>> >> > +}
>> >> > +
>> >> > +/**
>> >> > +  Check if a linear address is valid.
>> >> > +
>> >> > +  @param[in]  Cr0            CR0 control register.
>> >> > +  @param[in]  Cr3            CR3 control register.
>> >> > +  @param[in]  Cr4            CR4 control register.
>> >> > +  @param[in]  LinearAddress  Linear address to be checked.
>> >> > +**/
>> >> > +BOOLEAN
>> >> > +IsLinearAddressValid (
>> >> > +  IN  UINTN              Cr0,
>> >> > +  IN  UINTN              Cr3,
>> >> > +  IN  UINTN              Cr4,
>> >> > +  IN  UINTN              LinearAddress
>> >> > +  )
>> >> > +{
>> >> > +  UINT32                  Eax;
>> >> > +  UINT32                  Edx;
>> >> > +  UINT8                   MaxPhyAddrBits;
>> >> > +  MSR_IA32_EFER_REGISTER  Msr;
>> >> > +  BOOLEAN                 AddressValid;
>> >> > +
>> >> > +  //
>> >> > +  // Check for valid input parameters
>> >> > +  //
>> >> > +  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Check if paging is disabled
>> >> > +  //
>> >> > +  if ((Cr0 & BIT31) == 0) {
>> >> > +    //
>> >> > +    // If CR4.PAE bit is set, then the linear (or physical) address supports
>> >> > +    // only up to 36 bits.
>> >> > +    //
>> >> > +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL)
>> ||
>> >> > +        LinearAddress > 0xFFFFFFFF) {
>> >> > +      return FALSE;
>> >> > +    }
>> >> > +
>> >> > +    return TRUE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Paging can be enabled only if CR0.PE bit is set
>> >> > +  //
>> >> > +  if ((Cr0 & BIT0) == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // CR3 register cannot be zero if paging is enabled
>> >> > +  //
>> >> > +  if (Cr3 == 0) {
>> >> > +    return FALSE;
>> >> > +  }
>> >> > +
>> >> > +  //
>> >> > +  // Get MAXPHYADDR bits
>> >> > +  //
>> >> > +  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
>> >> > +  if (Eax >= 0x80000008) {
>> >> > +    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
>> >> > +    MaxPhyAddrBits = (UINT8)Eax;
>> >> > +  } else {
>> >> > +    AsmCpuid (1, NULL, NULL, NULL, &Edx);
>> >> > +    if ((Edx & BIT6) != 0) {
>> >> > +      MaxPhyAddrBits = 36;
>> >> > +    } else {
>> >> > +      MaxPhyAddrBits = 32;
>> >> > +    }
>> >> > +  }
>> >> > +
>> >> > +  ASSERT (MaxPhyAddrBits > 0);
>> >> > +
>> >> > +  AddressValid = FALSE;
>> >> > +
>> >> > +  //
>> >> > +  // check if CR4.PAE bit is not set
>> >> > +  //
>> >> > +  if ((Cr4 & BIT5) == 0) {
>> >> > +    //
>> >> > +    // Check if linear address is valid in 32-bit paging mode
>> >> > +    //
>> >> > +    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
>> >> > +  } else {
>> >> > +    if (MaxPhyAddrBits > 52) {
>> >> > +      return FALSE;
>> >> > +    }
>> >> > +
>> >> > +    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
>> >> > +
>> >> > +    if (Msr.Bits.LME == 0) {
>> >> > +      //
>> >> > +      // Check if linear address is valid in PAE paging mode
>> >> > +      //
>> >> > +      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits,
>> >> > LinearAddress);
>> >> > +    } else {
>> >> > +      //
>> >> > +      // Check if linear address is valid in 4-level paging mode
>> >> > +      //
>> >> > +      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
>> >> > +                                              LinearAddress);
>> >> > +    }
>> >> > +  }
>> >> > +
>> >> > +  return AddressValid;
>> >> > +}
>> >> > diff --git
>> >> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> >> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> >> > index ec46c2d9d3..1b51034c25 100644
>> >> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> >> > +++
>> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> >> > @@ -330,5 +330,21 @@ GetPdbFileName (
>> >> >    OUT CHAR8    **PdbFileName
>> >> >    );
>> >> >
>> >> > +/**
>> >> > +  Check if a linear address is valid.
>> >> > +
>> >> > +  @param[in]  Cr0            CR0 control register.
>> >> > +  @param[in]  Cr3            CR3 control register.
>> >> > +  @param[in]  Cr4            CR4 control register.
>> >> > +  @param[in]  LinearAddress  Linear address to be checked.
>> >> > +**/
>> >> > +BOOLEAN
>> >> > +IsLinearAddressValid (
>> >> > +  IN  UINTN              Cr0,
>> >> > +  IN  UINTN              Cr3,
>> >> > +  IN  UINTN              Cr4,
>> >> > +  IN  UINTN              LinearAddress
>> >> > +  );
>> >> > +
>> >> >  #endif
>> >> >
>> >> > --
>> >> > 2.14.3
>> >> >
>> >> > _______________________________________________
>> >> > edk2-devel mailing list
>> >> > edk2-devel@lists.01.org
>> >> > https://lists.01.org/mailman/listinfo/edk2-devel
>> >> _______________________________________________
>> >> edk2-devel mailing list
>> >> edk2-devel@lists.01.org
>> >> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Paulo Alcantara 6 years, 11 months ago 
"Yao, Jiewen" <jiewen.yao@intel.com> writes:

Hi Jiewen,

> Some suggestion:
>
> 1) I am not sure if it is proper to use ASSERT in an exception handler, because we know something is wrong.
>
>   ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
>
> I suggest we just do the check, and return FALSE, if the prerequisite is not satisfied.

OK.

>
> 2) Can we use meaningful definition for BIT0, BIT7?
>
>   if ((*Pml4TableEntry & BIT0) == 0) {
>   if ((*PageDirPtrTableEntry & BIT7) != 0) {

Sure. Sorry for missing that.

>
> 3) I am not sure if I understand below code.
>
>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>   PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>   PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>
> If MaxPhyAddrBits is 48, you will get "Cr3 & 0x0FFFFFFFFFFFF000". Is that what you want? I think we need "Cr3 & 0x0000FFFFFFFFF000"
> Should it be: PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) & (~0xFFF);

Yes, it should. I've just re-checked the Intel SDM Vol 3A and your
calculation is correct. Thanks! I'll fix that up in next series.

>
> 4) Can we use a more readable way to below? Personally, I do not suggest "<< 3", which is just the index calculation.
>
>   PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>   PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>   PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>
> For example:
>   PhysicalAddress = (UINT64)Cr3 & ((1ULL << MaxPhyAddrBits) - 1) & (~0xFFF);
>   Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>   Index= (UINTN)(((UINT64)LinearAddress >> 39) & 0x1FF);
>   PhysicalAddress = Pml4TableEntry[Index] & ((1ULL << MaxPhyAddrBits) - 1) & (~0xFFF);

Sure. This way is much clearer, indeed.

Thank you very much for the effort on reviewing this!

Paulo

>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Paulo
>> Alcantara
>> Sent: Friday, December 29, 2017 12:40 PM
>> To: edk2-devel@lists.01.org
>> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
>> Subject: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to
>> valid memory addresses
>> 
>> Introduce IsLinearAddressValid() function that will be used for
>> validating memory addresses that would get dereferenced during stack
>> traces in IA32 and X64 CPU exceptions.
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Requested-by: Brian Johnson <brian.johnson@hpe.com>
>> Requested-by: Jiewen Yao <jiewen.yao@intel.com>
>> Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
>> ---
>>  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c | 382
>> ++++++++++++++++++++
>>  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |  16 +
>>  2 files changed, 398 insertions(+)
>> 
>> diff --git
>> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> index 867c5c01d6..52b3eb1463 100644
>> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>> @@ -14,6 +14,9 @@
>> 
>>  #include "CpuExceptionCommon.h"
>> 
>> +#include <Register/Msr.h>
>> +#include <Library/DebugLib.h>
>> +
>>  //
>>  // Error code flag indicating whether or not an error code will be
>>  // pushed on the stack if an exception occurs.
>> @@ -194,3 +197,382 @@ GetPdbFileName (
>>      }
>>    }
>>  }
>> +
>> +/**
>> +  Check if a linear address is valid by walking the page tables in 4-level
>> +  paging mode.
>> +
>> +  @param[in]  Cr3             CR3 control register.
>> +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
>> +  @param[in]  LinearAddress   Linear address to be checked.
>> +**/
>> +STATIC
>> +BOOLEAN
>> +Do4LevelPagingModeCheck (
>> +  IN UINTN            Cr3,
>> +  IN UINT8            MaxPhyAddrBits,
>> +  IN UINTN            LinearAddress
>> +  )
>> +{
>> +  UINT64 PhysicalAddress;
>> +  UINT64 *Pml4TableEntry;
>> +  UINT64 *PageDirPtrTableEntry;
>> +  UINT64 *PageDirEntry;
>> +  UINT64 *PageTableEntry;
>> +
>> +  //
>> +  // In 4-level paging mode, linear addresses are 48 bits wide
>> +  //
>> +  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Calculate physical address of PML4E
>> +  //
>> +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>> +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
>> +
>> +  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
>> +
>> +  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check if a PDPTE is present
>> +  //
>> +  if ((*Pml4TableEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Calculate physical address of PDPTE
>> +  //
>> +  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
>> 12);
>> +  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
>> +
>> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
>> +
>> +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check whether a PDPTE or 1GiB page entry is present
>> +  //
>> +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Check if PDPTE maps an 1GiB page
>> +  //
>> +  if ((*PageDirPtrTableEntry & BIT7) != 0) {
>> +    return TRUE;
>> +  }
>> +
>> +  //
>> +  // Calculate physical address of PDE
>> +  //
>> +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1)
>> <<
>> +                                             12);
>> +  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
>> +
>> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> +
>> +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check whether a PDE or a 2MiB page entry is present
>> +  //
>> +  if ((*PageDirEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Check if PDE maps a 2MiB page
>> +  //
>> +  if ((*PageDirEntry & BIT7) != 0) {
>> +    return TRUE;
>> +  }
>> +
>> +  //
>> +  // Calculate physical address of PTE
>> +  //
>> +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>> +  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
>> +
>> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> +
>> +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check if PTE maps a 4KiB page
>> +  //
>> +  if ((*PageTableEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  return TRUE;
>> +}
>> +
>> +/**
>> +  Check if a linear address is valid by walking the page tables in 32-bit paging
>> +  mode.
>> +
>> +  @param[in]  Cr3             CR3 control register.
>> +  @param[in]  Cr4             CR4 control register.
>> +  @param[in]  LinearAddress   Linear address to be checked.
>> +**/
>> +STATIC
>> +BOOLEAN
>> +Do32BitPagingModeCheck (
>> +  IN UINTN            Cr3,
>> +  IN UINTN            Cr4,
>> +  IN UINTN            LinearAddress
>> +  )
>> +{
>> +  UINT64 PhysicalAddress;
>> +  UINT32 *PageDirEntry;
>> +  UINT32 *PageTableEntry;
>> +
>> +  if (LinearAddress > MAX_UINT32) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Calculate physical address of PDE
>> +  //
>> +  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
>> +  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
>> +
>> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> +
>> +  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check whether a PTE or a 4MiB page is present
>> +  //
>> +  if ((*PageDirEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Check if PDE maps a 4MiB page
>> +  //
>> +  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
>> +    return TRUE;
>> +  }
>> +
>> +  //
>> +  // Calculate physical address of PTE
>> +  //
>> +  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
>> +  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
>> +
>> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> +
>> +  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check if PTE maps a 4KiB page
>> +  //
>> +  if ((*PageTableEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  return TRUE;
>> +}
>> +
>> +/**
>> +  Check if a linear address is valid by walking the page tables in PAE paging
>> +  mode.
>> +
>> +  @param[in]  Cr3             CR3 control register.
>> +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
>> +  @param[in]  LinearAddress   Linear address to be checked.
>> +**/
>> +STATIC
>> +BOOLEAN
>> +DoPAEPagingModeCheck (
>> +  IN UINTN            Cr3,
>> +  IN UINT8            MaxPhyAddrBits,
>> +  IN UINTN            LinearAddress
>> +  )
>> +{
>> +  UINT64 PhysicalAddress;
>> +  UINT64 *PageDirPtrTableEntry;
>> +  UINT64 *PageDirEntry;
>> +  UINT64 *PageTableEntry;
>> +
>> +  if (LinearAddress > MAX_UINT32) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Calculate physical address of PDPTE
>> +  //
>> +  PhysicalAddress = (UINT32)Cr3 >> 5;
>> +
>> +  //
>> +  // Select PDPTE register
>> +  //
>> +  PhysicalAddress +=
>> +    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
>> +
>> +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check if PDE is present
>> +  //
>> +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1)
>> <<
>> +                                             12);
>> +  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
>> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
>> +
>> +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check whether a PTE or a 2MiB page is present
>> +  //
>> +  if ((*PageDirEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Check if PDE maps a 2MiB page
>> +  //
>> +  if ((*PageDirEntry & BIT7) != 0) {
>> +    return TRUE;
>> +  }
>> +
>> +  //
>> +  // Calculate physical address of PTE
>> +  //
>> +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
>> +  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
>> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
>> +
>> +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
>> +
>> +  //
>> +  // Check if PTE maps a 4KiB page
>> +  //
>> +  if ((*PageTableEntry & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  return TRUE;
>> +}
>> +
>> +/**
>> +  Check if a linear address is valid.
>> +
>> +  @param[in]  Cr0            CR0 control register.
>> +  @param[in]  Cr3            CR3 control register.
>> +  @param[in]  Cr4            CR4 control register.
>> +  @param[in]  LinearAddress  Linear address to be checked.
>> +**/
>> +BOOLEAN
>> +IsLinearAddressValid (
>> +  IN  UINTN              Cr0,
>> +  IN  UINTN              Cr3,
>> +  IN  UINTN              Cr4,
>> +  IN  UINTN              LinearAddress
>> +  )
>> +{
>> +  UINT32                  Eax;
>> +  UINT32                  Edx;
>> +  UINT8                   MaxPhyAddrBits;
>> +  MSR_IA32_EFER_REGISTER  Msr;
>> +  BOOLEAN                 AddressValid;
>> +
>> +  //
>> +  // Check for valid input parameters
>> +  //
>> +  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Check if paging is disabled
>> +  //
>> +  if ((Cr0 & BIT31) == 0) {
>> +    //
>> +    // If CR4.PAE bit is set, then the linear (or physical) address supports
>> +    // only up to 36 bits.
>> +    //
>> +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
>> +        LinearAddress > 0xFFFFFFFF) {
>> +      return FALSE;
>> +    }
>> +
>> +    return TRUE;
>> +  }
>> +
>> +  //
>> +  // Paging can be enabled only if CR0.PE bit is set
>> +  //
>> +  if ((Cr0 & BIT0) == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // CR3 register cannot be zero if paging is enabled
>> +  //
>> +  if (Cr3 == 0) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Get MAXPHYADDR bits
>> +  //
>> +  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
>> +  if (Eax >= 0x80000008) {
>> +    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
>> +    MaxPhyAddrBits = (UINT8)Eax;
>> +  } else {
>> +    AsmCpuid (1, NULL, NULL, NULL, &Edx);
>> +    if ((Edx & BIT6) != 0) {
>> +      MaxPhyAddrBits = 36;
>> +    } else {
>> +      MaxPhyAddrBits = 32;
>> +    }
>> +  }
>> +
>> +  ASSERT (MaxPhyAddrBits > 0);
>> +
>> +  AddressValid = FALSE;
>> +
>> +  //
>> +  // check if CR4.PAE bit is not set
>> +  //
>> +  if ((Cr4 & BIT5) == 0) {
>> +    //
>> +    // Check if linear address is valid in 32-bit paging mode
>> +    //
>> +    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
>> +  } else {
>> +    if (MaxPhyAddrBits > 52) {
>> +      return FALSE;
>> +    }
>> +
>> +    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
>> +
>> +    if (Msr.Bits.LME == 0) {
>> +      //
>> +      // Check if linear address is valid in PAE paging mode
>> +      //
>> +      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits,
>> LinearAddress);
>> +    } else {
>> +      //
>> +      // Check if linear address is valid in 4-level paging mode
>> +      //
>> +      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
>> +                                              LinearAddress);
>> +    }
>> +  }
>> +
>> +  return AddressValid;
>> +}
>> diff --git
>> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> index ec46c2d9d3..1b51034c25 100644
>> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
>> @@ -330,5 +330,21 @@ GetPdbFileName (
>>    OUT CHAR8    **PdbFileName
>>    );
>> 
>> +/**
>> +  Check if a linear address is valid.
>> +
>> +  @param[in]  Cr0            CR0 control register.
>> +  @param[in]  Cr3            CR3 control register.
>> +  @param[in]  Cr4            CR4 control register.
>> +  @param[in]  LinearAddress  Linear address to be checked.
>> +**/
>> +BOOLEAN
>> +IsLinearAddressValid (
>> +  IN  UINTN              Cr0,
>> +  IN  UINTN              Cr3,
>> +  IN  UINTN              Cr4,
>> +  IN  UINTN              LinearAddress
>> +  );
>> +
>>  #endif
>> 
>> --
>> 2.14.3
>> 
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] 答复: [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Fan Jeff 6 years, 11 months ago 
Paul,

+  //
+  // Calculate physical address of PML4E
+  //
+  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
+

Should not pass VS build, instead you could use LShiftU64/RShiftU64 to do 64bit shift operation as below:

  PhysicalAddress = (UINT64)Cr3 & LShiftU64 (LShiftU64 (1, MaxPhyAddrBits) - 1, 12);
  PhysicalAddress |= LShiftU64 (RShiftU64((UINT64)LinearAddress, 39) & 0x1FF), 3);

Jeff

发件人: Paulo Alcantara<mailto:paulo@paulo.ac>
发送时间: 2017年12月29日 12:41
收件人: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
抄送: Laszlo Ersek<mailto:lersek@redhat.com>; Eric Dong<mailto:eric.dong@intel.com>
主题: [edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses

Introduce IsLinearAddressValid() function that will be used for
validating memory addresses that would get dereferenced during stack
traces in IA32 and X64 CPU exceptions.

Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Requested-by: Brian Johnson <brian.johnson@hpe.com>
Requested-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c | 382 ++++++++++++++++++++
 UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |  16 +
 2 files changed, 398 insertions(+)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
index 867c5c01d6..52b3eb1463 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
@@ -14,6 +14,9 @@

 #include "CpuExceptionCommon.h"

+#include <Register/Msr.h>
+#include <Library/DebugLib.h>
+
 //
 // Error code flag indicating whether or not an error code will be
 // pushed on the stack if an exception occurs.
@@ -194,3 +197,382 @@ GetPdbFileName (
     }
   }
 }
+
+/**
+  Check if a linear address is valid by walking the page tables in 4-level
+  paging mode.
+
+  @param[in]  Cr3             CR3 control register.
+  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
+  @param[in]  LinearAddress   Linear address to be checked.
+**/
+STATIC
+BOOLEAN
+Do4LevelPagingModeCheck (
+  IN UINTN            Cr3,
+  IN UINT8            MaxPhyAddrBits,
+  IN UINTN            LinearAddress
+  )
+{
+  UINT64 PhysicalAddress;
+  UINT64 *Pml4TableEntry;
+  UINT64 *PageDirPtrTableEntry;
+  UINT64 *PageDirEntry;
+  UINT64 *PageTableEntry;
+
+  //
+  // In 4-level paging mode, linear addresses are 48 bits wide
+  //
+  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
+    return FALSE;
+  }
+
+  //
+  // Calculate physical address of PML4E
+  //
+  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
+
+  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
+
+  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if a PDPTE is present
+  //
+  if ((*Pml4TableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Calculate physical address of PDPTE
+  //
+  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
+
+  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check whether a PDPTE or 1GiB page entry is present
+  //
+  if ((*PageDirPtrTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if PDPTE maps an 1GiB page
+  //
+  if ((*PageDirPtrTableEntry & BIT7) != 0) {
+    return TRUE;
+  }
+
+  //
+  // Calculate physical address of PDE
+  //
+  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
+                                             12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
+
+  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check whether a PDE or a 2MiB page entry is present
+  //
+  if ((*PageDirEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if PDE maps a 2MiB page
+  //
+  if ((*PageDirEntry & BIT7) != 0) {
+    return TRUE;
+  }
+
+  //
+  // Calculate physical address of PTE
+  //
+  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
+
+  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if PTE maps a 4KiB page
+  //
+  if ((*PageTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**
+  Check if a linear address is valid by walking the page tables in 32-bit paging
+  mode.
+
+  @param[in]  Cr3             CR3 control register.
+  @param[in]  Cr4             CR4 control register.
+  @param[in]  LinearAddress   Linear address to be checked.
+**/
+STATIC
+BOOLEAN
+Do32BitPagingModeCheck (
+  IN UINTN            Cr3,
+  IN UINTN            Cr4,
+  IN UINTN            LinearAddress
+  )
+{
+  UINT64 PhysicalAddress;
+  UINT32 *PageDirEntry;
+  UINT32 *PageTableEntry;
+
+  if (LinearAddress > MAX_UINT32) {
+    return FALSE;
+  }
+
+  //
+  // Calculate physical address of PDE
+  //
+  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
+  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
+
+  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check whether a PTE or a 4MiB page is present
+  //
+  if ((*PageDirEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if PDE maps a 4MiB page
+  //
+  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
+    return TRUE;
+  }
+
+  //
+  // Calculate physical address of PTE
+  //
+  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
+  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
+
+  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
+
+  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if PTE maps a 4KiB page
+  //
+  if ((*PageTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**
+  Check if a linear address is valid by walking the page tables in PAE paging
+  mode.
+
+  @param[in]  Cr3             CR3 control register.
+  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
+  @param[in]  LinearAddress   Linear address to be checked.
+**/
+STATIC
+BOOLEAN
+DoPAEPagingModeCheck (
+  IN UINTN            Cr3,
+  IN UINT8            MaxPhyAddrBits,
+  IN UINTN            LinearAddress
+  )
+{
+  UINT64 PhysicalAddress;
+  UINT64 *PageDirPtrTableEntry;
+  UINT64 *PageDirEntry;
+  UINT64 *PageTableEntry;
+
+  if (LinearAddress > MAX_UINT32) {
+    return FALSE;
+  }
+
+  //
+  // Calculate physical address of PDPTE
+  //
+  PhysicalAddress = (UINT32)Cr3 >> 5;
+
+  //
+  // Select PDPTE register
+  //
+  PhysicalAddress +=
+    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
+
+  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if PDE is present
+  //
+  if ((*PageDirPtrTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) - 1) <<
+                                             12);
+  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
+  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
+
+  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check whether a PTE or a 2MiB page is present
+  //
+  if ((*PageDirEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if PDE maps a 2MiB page
+  //
+  if ((*PageDirEntry & BIT7) != 0) {
+    return TRUE;
+  }
+
+  //
+  // Calculate physical address of PTE
+  //
+  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
+  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
+  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
+
+  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
+
+  //
+  // Check if PTE maps a 4KiB page
+  //
+  if ((*PageTableEntry & BIT0) == 0) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**
+  Check if a linear address is valid.
+
+  @param[in]  Cr0            CR0 control register.
+  @param[in]  Cr3            CR3 control register.
+  @param[in]  Cr4            CR4 control register.
+  @param[in]  LinearAddress  Linear address to be checked.
+**/
+BOOLEAN
+IsLinearAddressValid (
+  IN  UINTN              Cr0,
+  IN  UINTN              Cr3,
+  IN  UINTN              Cr4,
+  IN  UINTN              LinearAddress
+  )
+{
+  UINT32                  Eax;
+  UINT32                  Edx;
+  UINT8                   MaxPhyAddrBits;
+  MSR_IA32_EFER_REGISTER  Msr;
+  BOOLEAN                 AddressValid;
+
+  //
+  // Check for valid input parameters
+  //
+  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
+    return FALSE;
+  }
+
+  //
+  // Check if paging is disabled
+  //
+  if ((Cr0 & BIT31) == 0) {
+    //
+    // If CR4.PAE bit is set, then the linear (or physical) address supports
+    // only up to 36 bits.
+    //
+    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
+        LinearAddress > 0xFFFFFFFF) {
+      return FALSE;
+    }
+
+    return TRUE;
+  }
+
+  //
+  // Paging can be enabled only if CR0.PE bit is set
+  //
+  if ((Cr0 & BIT0) == 0) {
+    return FALSE;
+  }
+
+  //
+  // CR3 register cannot be zero if paging is enabled
+  //
+  if (Cr3 == 0) {
+    return FALSE;
+  }
+
+  //
+  // Get MAXPHYADDR bits
+  //
+  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
+  if (Eax >= 0x80000008) {
+    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
+    MaxPhyAddrBits = (UINT8)Eax;
+  } else {
+    AsmCpuid (1, NULL, NULL, NULL, &Edx);
+    if ((Edx & BIT6) != 0) {
+      MaxPhyAddrBits = 36;
+    } else {
+      MaxPhyAddrBits = 32;
+    }
+  }
+
+  ASSERT (MaxPhyAddrBits > 0);
+
+  AddressValid = FALSE;
+
+  //
+  // check if CR4.PAE bit is not set
+  //
+  if ((Cr4 & BIT5) == 0) {
+    //
+    // Check if linear address is valid in 32-bit paging mode
+    //
+    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
+  } else {
+    if (MaxPhyAddrBits > 52) {
+      return FALSE;
+    }
+
+    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
+
+    if (Msr.Bits.LME == 0) {
+      //
+      // Check if linear address is valid in PAE paging mode
+      //
+      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits, LinearAddress);
+    } else {
+      //
+      // Check if linear address is valid in 4-level paging mode
+      //
+      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
+                                              LinearAddress);
+    }
+  }
+
+  return AddressValid;
+}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
index ec46c2d9d3..1b51034c25 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
@@ -330,5 +330,21 @@ GetPdbFileName (
   OUT CHAR8    **PdbFileName
   );

+/**
+  Check if a linear address is valid.
+
+  @param[in]  Cr0            CR0 control register.
+  @param[in]  Cr3            CR3 control register.
+  @param[in]  Cr4            CR4 control register.
+  @param[in]  LinearAddress  Linear address to be checked.
+**/
+BOOLEAN
+IsLinearAddressValid (
+  IN  UINTN              Cr0,
+  IN  UINTN              Cr3,
+  IN  UINTN              Cr4,
+  IN  UINTN              LinearAddress
+  );
+
 #endif

--
2.14.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] 答复: [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses
Posted by Paulo Alcantara 6 years, 11 months ago 
Hi Jeff,

On 1/3/2018 6:42 AM, Fan Jeff wrote:
> Paul,
> 
> +  //
> +  // Calculate physical address of PML4E
> +  //
> +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
> +
> 
> Should not pass VS build, instead you could use LShiftU64/RShiftU64 to 
> do 64bit shift operation as below:
> 
>    PhysicalAddress = (UINT64)Cr3 & LShiftU64 (LShiftU64 (1, 
> MaxPhyAddrBits) - 1, 12);
> 
>    PhysicalAddress |= LShiftU64 (RShiftU64((UINT64)LinearAddress, 39) & 
> 0x1FF), 3);

OK - I'll fix them up and then build-test it with IA32/X64 MSFT toolchain.

Thanks
Paulo

> 
> Jeff
> 
> *发件人: *Paulo Alcantara <mailto:paulo@paulo.ac>
> *发送时间: *2017年12月29日12:41
> *收件人: *edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> *抄送: *Laszlo Ersek <mailto:lersek@redhat.com>; Eric Dong 
> <mailto:eric.dong@intel.com>
> *主题: *[edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add 
> helper to valid memory addresses
> 
> Introduce IsLinearAddressValid() function that will be used for
> validating memory addresses that would get dereferenced during stack
> traces in IA32 and X64 CPU exceptions.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Requested-by: Brian Johnson <brian.johnson@hpe.com>
> Requested-by: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
> ---
>   UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c | 382 
> ++++++++++++++++++++
>   UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |  16 +
>   2 files changed, 398 insertions(+)
> 
> diff --git 
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> index 867c5c01d6..52b3eb1463 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> @@ -14,6 +14,9 @@
> 
>   #include "CpuExceptionCommon.h"
> 
> +#include <Register/Msr.h>
> +#include <Library/DebugLib.h>
> +
>   //
>   // Error code flag indicating whether or not an error code will be
>   // pushed on the stack if an exception occurs.
> @@ -194,3 +197,382 @@ GetPdbFileName (
>       }
>     }
>   }
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in 4-level
> +  paging mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +Do4LevelPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINT8            MaxPhyAddrBits,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT64 *Pml4TableEntry;
> +  UINT64 *PageDirPtrTableEntry;
> +  UINT64 *PageDirEntry;
> +  UINT64 *PageTableEntry;
> +
> +  //
> +  // In 4-level paging mode, linear addresses are 48 bits wide
> +  //
> +  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PML4E
> +  //
> +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
> +
> +  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if a PDPTE is present
> +  //
> +  if ((*Pml4TableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDPTE
> +  //
> +  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) 
> << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
> +
> +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PDPTE or 1GiB page entry is present
> +  //
> +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDPTE maps an 1GiB page
> +  //
> +  if ((*PageDirPtrTableEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDE
> +  //
> +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) 
> - 1) <<
> +                                             12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PDE or a 2MiB page entry is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 2MiB page
> +  //
> +  if ((*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in 
> 32-bit paging
> +  mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  Cr4             CR4 control register.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +Do32BitPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINTN            Cr4,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT32 *PageDirEntry;
> +  UINT32 *PageTableEntry;
> +
> +  if (LinearAddress > MAX_UINT32) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDE
> +  //
> +  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
> +  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PTE or a 4MiB page is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 4MiB page
> +  //
> +  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
> +  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in PAE 
> paging
> +  mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +DoPAEPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINT8            MaxPhyAddrBits,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT64 *PageDirPtrTableEntry;
> +  UINT64 *PageDirEntry;
> +  UINT64 *PageTableEntry;
> +
> +  if (LinearAddress > MAX_UINT32) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDPTE
> +  //
> +  PhysicalAddress = (UINT32)Cr3 >> 5;
> +
> +  //
> +  // Select PDPTE register
> +  //
> +  PhysicalAddress +=
> +    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
> +
> +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PDE is present
> +  //
> +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) 
> - 1) <<
> +                                             12);
> +  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PTE or a 2MiB page is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 2MiB page
> +  //
> +  if ((*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid.
> +
> +  @param[in]  Cr0            CR0 control register.
> +  @param[in]  Cr3            CR3 control register.
> +  @param[in]  Cr4            CR4 control register.
> +  @param[in]  LinearAddress  Linear address to be checked.
> +**/
> +BOOLEAN
> +IsLinearAddressValid (
> +  IN  UINTN              Cr0,
> +  IN  UINTN              Cr3,
> +  IN  UINTN              Cr4,
> +  IN  UINTN              LinearAddress
> +  )
> +{
> +  UINT32                  Eax;
> +  UINT32                  Edx;
> +  UINT8                   MaxPhyAddrBits;
> +  MSR_IA32_EFER_REGISTER  Msr;
> +  BOOLEAN                 AddressValid;
> +
> +  //
> +  // Check for valid input parameters
> +  //
> +  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if paging is disabled
> +  //
> +  if ((Cr0 & BIT31) == 0) {
> +    //
> +    // If CR4.PAE bit is set, then the linear (or physical) address 
> supports
> +    // only up to 36 bits.
> +    //
> +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
> +        LinearAddress > 0xFFFFFFFF) {
> +      return FALSE;
> +    }
> +
> +    return TRUE;
> +  }
> +
> +  //
> +  // Paging can be enabled only if CR0.PE bit is set
> +  //
> +  if ((Cr0 & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // CR3 register cannot be zero if paging is enabled
> +  //
> +  if (Cr3 == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Get MAXPHYADDR bits
> +  //
> +  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
> +  if (Eax >= 0x80000008) {
> +    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
> +    MaxPhyAddrBits = (UINT8)Eax;
> +  } else {
> +    AsmCpuid (1, NULL, NULL, NULL, &Edx);
> +    if ((Edx & BIT6) != 0) {
> +      MaxPhyAddrBits = 36;
> +    } else {
> +      MaxPhyAddrBits = 32;
> +    }
> +  }
> +
> +  ASSERT (MaxPhyAddrBits > 0);
> +
> +  AddressValid = FALSE;
> +
> +  //
> +  // check if CR4.PAE bit is not set
> +  //
> +  if ((Cr4 & BIT5) == 0) {
> +    //
> +    // Check if linear address is valid in 32-bit paging mode
> +    //
> +    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
> +  } else {
> +    if (MaxPhyAddrBits > 52) {
> +      return FALSE;
> +    }
> +
> +    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
> +
> +    if (Msr.Bits.LME == 0) {
> +      //
> +      // Check if linear address is valid in PAE paging mode
> +      //
> +      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits, 
> LinearAddress);
> +    } else {
> +      //
> +      // Check if linear address is valid in 4-level paging mode
> +      //
> +      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
> +                                              LinearAddress);
> +    }
> +  }
> +
> +  return AddressValid;
> +}
> diff --git 
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> index ec46c2d9d3..1b51034c25 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> @@ -330,5 +330,21 @@ GetPdbFileName (
>     OUT CHAR8    **PdbFileName
>     );
> 
> +/**
> +  Check if a linear address is valid.
> +
> +  @param[in]  Cr0            CR0 control register.
> +  @param[in]  Cr3            CR3 control register.
> +  @param[in]  Cr4            CR4 control register.
> +  @param[in]  LinearAddress  Linear address to be checked.
> +**/
> +BOOLEAN
> +IsLinearAddressValid (
> +  IN  UINTN              Cr0,
> +  IN  UINTN              Cr3,
> +  IN  UINTN              Cr4,
> +  IN  UINTN              LinearAddress
> +  );
> +
>   #endif
> 
> -- 
> 2.14.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.