Skip to content

Commit

Permalink
Update the ARM MemoryAttributeProtocol to Not Check if Region is Syst…
Browse files Browse the repository at this point in the history
…em Memory

Description

Project Mu allows the use of the memory attribute protocol to update regions
which are not of GCD type System Memory. Also, this check can cause a lock
issue after the CPU architecture protocol is installed due to the GCD lock
being held during GCD synchronization and a call being made to get memory
attributes using the memory attribute protocol.

- [x] Impacts functionality?
  - **Functionality** - Does the change ultimately impact how firmware functions?
  - Examples: Add a new library, publish a new PPI, update an algorithm, ...
- [ ] Impacts security?
  - **Security** - Does the change have a direct security impact on an application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
  - **Breaking change** - Will anyone consuming this change experience a break
    in build or boot behavior?
  - Examples: Add a new library class, move a module to a different repo, call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
  - **Documentation** - Does the change contain explicit documentation additions
    outside direct code modifications (and comments)?
  - Examples: Update readme file, add feature readme file, link to documentation
    on an a separate Web page, ...

How This Was Tested

Running QemuSbsaPkg

Integration Instructions

N/A
  • Loading branch information
TaylorBeebe authored and kenlautner committed Dec 19, 2023
1 parent d23da0c commit e773d72
Showing 1 changed file with 52 additions and 44 deletions.
96 changes: 52 additions & 44 deletions ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,43 @@

#include "CpuDxe.h"

/**
Check whether the provided memory range is covered by a single entry of type
EfiGcdSystemMemory in the GCD memory map.
@param BaseAddress The physical address that is the start address of
a memory region.
@param Length The size in bytes of the memory region.
@return Whether the region is system memory or not.
**/
STATIC
BOOLEAN
RegionIsSystemMemory (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
)
{
EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
EFI_PHYSICAL_ADDRESS GcdEndAddress;
EFI_STATUS Status;

Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &GcdDescriptor);
if (EFI_ERROR (Status) ||
(GcdDescriptor.GcdMemoryType != EfiGcdMemoryTypeSystemMemory))
{
return FALSE;
}

GcdEndAddress = GcdDescriptor.BaseAddress + GcdDescriptor.Length;

//
// Return TRUE if the GCD descriptor covers the range entirely
//
return GcdEndAddress >= (BaseAddress + Length);
}
// MU_CHANGE START: Don't check for GCD system memory when using EFI Attributes Protocol
// /**
// Check whether the provided memory range is covered by a single entry of type
// EfiGcdSystemMemory in the GCD memory map.

// @param BaseAddress The physical address that is the start address of
// a memory region.
// @param Length The size in bytes of the memory region.

// @return Whether the region is system memory or not.
// **/
// STATIC
// BOOLEAN
// RegionIsSystemMemory (
// IN EFI_PHYSICAL_ADDRESS BaseAddress,
// IN UINT64 Length
// )
// {
// EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
// EFI_PHYSICAL_ADDRESS GcdEndAddress;
// EFI_STATUS Status;

// Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &GcdDescriptor);
// if (EFI_ERROR (Status) ||
// (GcdDescriptor.GcdMemoryType != EfiGcdMemoryTypeSystemMemory))
// {
// return FALSE;
// }

// GcdEndAddress = GcdDescriptor.BaseAddress + GcdDescriptor.Length;

// //
// // Return TRUE if the GCD descriptor covers the range entirely
// //
// return GcdEndAddress >= (BaseAddress + Length);
// }
// MU_CHANGE END

/**
This function retrieves the attributes of the memory region specified by
Expand Down Expand Up @@ -85,9 +87,11 @@ GetMemoryAttributes (
return EFI_INVALID_PARAMETER;
}

if (!RegionIsSystemMemory (BaseAddress, Length)) {
return EFI_UNSUPPORTED;
}
// MU_CHANGE START: Don't check for GCD system memory when using EFI Attributes Protocol
// if (!RegionIsSystemMemory (BaseAddress, Length)) {
// return EFI_UNSUPPORTED;
// }
// MU_CHANGE END

DEBUG ((
DEBUG_VERBOSE,
Expand Down Expand Up @@ -198,9 +202,11 @@ SetMemoryAttributes (
return EFI_INVALID_PARAMETER;
}

if (!RegionIsSystemMemory (BaseAddress, Length)) {
return EFI_UNSUPPORTED;
}
// MU_CHANGE START: Don't check for GCD system memory when using EFI Attributes Protocol
// if (!RegionIsSystemMemory (BaseAddress, Length)) {
// return EFI_UNSUPPORTED;
// }
// MU_CHANGE END

return ArmSetMemoryAttributes (BaseAddress, Length, Attributes, Attributes);
}
Expand Down Expand Up @@ -259,9 +265,11 @@ ClearMemoryAttributes (
return EFI_INVALID_PARAMETER;
}

if (!RegionIsSystemMemory (BaseAddress, Length)) {
return EFI_UNSUPPORTED;
}
// MU_CHANGE START: Don't check for GCD system memory when using EFI Attributes Protocol
// if (!RegionIsSystemMemory (BaseAddress, Length)) {
// return EFI_UNSUPPORTED;
// }
// MU_CHANGE END

return ArmSetMemoryAttributes (BaseAddress, Length, 0, Attributes);
}
Expand Down

0 comments on commit e773d72

Please sign in to comment.