diff --git a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c index 0c8e91e793..09b87a511d 100644 --- a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c +++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c @@ -500,10 +500,12 @@ DmaAllocateAlignedBuffer ( { EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor; VOID *Allocation; - UINT64 MemType; + UINT64 Attributes; UNCACHED_ALLOCATION *Alloc; EFI_STATUS Status; + Attributes = EFI_MEMORY_XP; + if (Alignment == 0) { Alignment = EFI_PAGE_SIZE; } @@ -534,9 +536,9 @@ DmaAllocateAlignedBuffer ( // Choose a suitable uncached memory type that is supported by the region if (GcdDescriptor.Capabilities & EFI_MEMORY_WC) { - MemType = EFI_MEMORY_WC; + Attributes |= EFI_MEMORY_WC; } else if (GcdDescriptor.Capabilities & EFI_MEMORY_UC) { - MemType = EFI_MEMORY_UC; + Attributes |= EFI_MEMORY_UC; } else { Status = EFI_UNSUPPORTED; goto FreeBuffer; @@ -553,11 +555,37 @@ DmaAllocateAlignedBuffer ( InsertHeadList (&UncachedAllocationList, &Alloc->Link); - // Remap the region with the new attributes + // Ensure that EFI_MEMORY_XP is in the capability set + if ((GcdDescriptor.Capabilities & EFI_MEMORY_XP) != EFI_MEMORY_XP) { + Status = gDS->SetMemorySpaceCapabilities ( + (PHYSICAL_ADDRESS)(UINTN)Allocation, + EFI_PAGES_TO_SIZE (Pages), + GcdDescriptor.Capabilities | EFI_MEMORY_XP + ); + + // if we were to fail setting the capability, this would indicate an internal failure of the GCD code. We should + // assert here to let a platform know something went crazy, but for a release build we can let the allocation occur + // without the EFI_MEMORY_XP bit set, as that was the existing behavior + if (EFI_ERROR (Status)) { + DEBUG (( + DEBUG_ERROR, + "%a failed to set EFI_MEMORY_XP capability on 0x%llx for length 0x%llx. Attempting to allocate without XP set.\n", + __func__, + Allocation, + EFI_PAGES_TO_SIZE (Pages) + )); + + ASSERT_EFI_ERROR (Status); + + Attributes &= ~EFI_MEMORY_XP; + } + } + + // Remap the region with the new attributes and mark it non-executable Status = gDS->SetMemorySpaceAttributes ( (PHYSICAL_ADDRESS)(UINTN)Allocation, EFI_PAGES_TO_SIZE (Pages), - MemType | EFI_MEMORY_XP // MU_CHANGE: Allocate DMA memory XP by default + Attributes ); if (EFI_ERROR (Status)) { goto FreeAlloc; diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c index ee13c3436c..b04998ba04 100644 --- a/FmpDevicePkg/FmpDxe/FmpDxe.c +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c @@ -181,10 +181,12 @@ GetImageTypeIdGuid ( if (ImageTypeIdGuidSize == sizeof (EFI_GUID)) { FmpDeviceLibGuid = (EFI_GUID *)PcdGetPtr (PcdFmpDeviceImageTypeIdGuid); } else { - // MU_CHANGE start - this is a misconfiguration error, we should assert - DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Fall back to ImageTypeIdGuid of gEfiCallerIdGuid. FmpDxe error: misconfiguration\n", mImageIdName)); + DEBUG (( + DEBUG_ERROR, + "FmpDxe(%s): Fall back to ImageTypeIdGuid of gEfiCallerIdGuid. FmpDxe error: misconfiguration\n", + mImageIdName + )); ASSERT (FALSE); - // MU_CHANGE end FmpDeviceLibGuid = &gEfiCallerIdGuid; } } diff --git a/PrmPkg/PrmConfigDxe/PrmConfigDxe.c b/PrmPkg/PrmConfigDxe/PrmConfigDxe.c index df26a25d90..7a3913ee09 100644 --- a/PrmPkg/PrmConfigDxe/PrmConfigDxe.c +++ b/PrmPkg/PrmConfigDxe/PrmConfigDxe.c @@ -152,17 +152,15 @@ SetRuntimeMemoryRangeAttributes ( continue; } + // The memory space descriptor access attributes are not accurate. Don't pass + // in access attributes so SetMemorySpaceAttributes() doesn't update them. + // EFI_MEMORY_RUNTIME is not a CPU arch attribute, so calling + // SetMemorySpaceAttributes() with only it set will not clear existing page table + // attributes for this region, such as EFI_MEMORY_XP Status = gDS->SetMemorySpaceAttributes ( RuntimeMmioRanges->Range[Index].PhysicalBaseAddress, (UINT64)RuntimeMmioRanges->Range[Index].Length, - // MU_CHANGE START: The memory space descriptor access attributes are not accurate. Don't pass - // in access attributes so SetMemorySpaceAttributes() doesn't update them. - // EFI_MEMORY_RUNTIME is not a CPU arch attribute, so calling - // SetMemorySpaceAttributes() with only it set will not clear existing page table - // attributes for this region, such as EFI_MEMORY_XP - // Descriptor.Attributes | EFI_MEMORY_RUNTIME EFI_MEMORY_RUNTIME - // MU_CHANGE END ); ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) {