From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, oleksiyy@ami.com
Cc: liming.gao@intel.com, michael.d.kinney@intel.com,
dandan.bi@intel.com, ray.ni@intel.com, rahul1.kumar@intel.com,
Felixp@ami.com
Subject: Re: [edk2-devel] [PATCH V3 2/4] MdeModulePkg: Add New Memory Attributes
Date: Mon, 6 Jul 2020 11:13:55 +0200 [thread overview]
Message-ID: <a5ebc20f-e532-314f-c4e5-d376b0d59294@redhat.com> (raw)
In-Reply-To: <20200702205039.52400-3-oleksiyy@ami.com>
On 07/02/20 22:50, Oleksiy Yakovlev wrote:
> Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
> attributes introduced in UEFI 2.8
> (UEFI 2.8, mantis 1919 and 1872)
> Use attributes bitmasks, defined in MdePkg.
>
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 11 ++---------
> MdeModulePkg/Core/Dxe/Mem/Page.c | 9 +++------
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 7 ++-----
> MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 10 ++--------
> 4 files changed, 9 insertions(+), 28 deletions(-)
This is missing Dandan's R-b from
<https://edk2.groups.io/g/devel/message/61630>. (To be picked up by the
maintainer that merges the series, if no more versions of the set are
needed.)
Thanks
Laszlo
>
> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> index 74f3b1b..2d8c076 100644
> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> @@ -35,13 +35,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #define PRESENT_MEMORY_ATTRIBUTES (EFI_RESOURCE_ATTRIBUTE_PRESENT)
>
> -#define EXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_UC | EFI_MEMORY_WC | \
> - EFI_MEMORY_WT | EFI_MEMORY_WB | \
> - EFI_MEMORY_WP | EFI_MEMORY_UCE)
> -
> -#define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | \
> - EFI_MEMORY_RO)
> -
> //
> // Module Variables
> //
> @@ -665,7 +658,7 @@ ConverToCpuArchAttributes (
> {
> UINT64 CpuArchAttributes;
>
> - CpuArchAttributes = Attributes & NONEXCLUSIVE_MEMORY_ATTRIBUTES;
> + CpuArchAttributes = Attributes & EFI_MEMORY_ATTRIBUTE_MASK;
>
> if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
> CpuArchAttributes |= EFI_MEMORY_UC;
> @@ -951,7 +944,7 @@ CoreConvertSpace (
> // Keep original CPU arch attributes when caller just calls
> // SetMemorySpaceAttributes() with none CPU arch attributes (for example, RUNTIME).
> //
> - Attributes |= (Entry->Attributes & (EXCLUSIVE_MEMORY_ATTRIBUTES | NONEXCLUSIVE_MEMORY_ATTRIBUTES));
> + Attributes |= (Entry->Attributes & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
> }
> Entry->Attributes = Attributes;
> break;
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 1f0e3d9..2c2c9cd 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -1857,8 +1857,7 @@ CoreGetMemoryMap (
> MemoryMap->VirtualStart = 0;
> MemoryMap->NumberOfPages = RShiftU64 ((MergeGcdMapEntry.EndAddress - MergeGcdMapEntry.BaseAddress + 1), EFI_PAGE_SHIFT);
> MemoryMap->Attribute = (MergeGcdMapEntry.Attributes & ~EFI_MEMORY_PORT_IO) |
> - (MergeGcdMapEntry.Capabilities & (EFI_MEMORY_RP | EFI_MEMORY_WP | EFI_MEMORY_XP | EFI_MEMORY_RO |
> - EFI_MEMORY_UC | EFI_MEMORY_UCE | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB));
> + (MergeGcdMapEntry.Capabilities & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
>
> if (MergeGcdMapEntry.GcdMemoryType == EfiGcdMemoryTypeReserved) {
> MemoryMap->Type = EfiReservedMemoryType;
> @@ -1892,8 +1891,7 @@ CoreGetMemoryMap (
> MemoryMap->VirtualStart = 0;
> MemoryMap->NumberOfPages = RShiftU64 ((MergeGcdMapEntry.EndAddress - MergeGcdMapEntry.BaseAddress + 1), EFI_PAGE_SHIFT);
> MemoryMap->Attribute = MergeGcdMapEntry.Attributes | EFI_MEMORY_NV |
> - (MergeGcdMapEntry.Capabilities & (EFI_MEMORY_RP | EFI_MEMORY_WP | EFI_MEMORY_XP | EFI_MEMORY_RO |
> - EFI_MEMORY_UC | EFI_MEMORY_UCE | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB));
> + (MergeGcdMapEntry.Capabilities & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
> MemoryMap->Type = EfiPersistentMemory;
>
> //
> @@ -1935,8 +1933,7 @@ CoreGetMemoryMap (
> MemoryMapEnd = MemoryMap;
> MemoryMap = MemoryMapStart;
> while (MemoryMap < MemoryMapEnd) {
> - MemoryMap->Attribute &= ~(UINT64)(EFI_MEMORY_RP | EFI_MEMORY_RO |
> - EFI_MEMORY_XP);
> + MemoryMap->Attribute &= ~(UINT64)EFI_MEMORY_ATTRIBUTE_MASK;
> MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
> }
> MergeMemoryMap (MemoryMapStart, &BufferSize, Size);
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index 92a442f..7d1daf0 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -42,9 +42,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include "DxeMain.h"
> #include "Mem/HeapGuard.h"
>
> -#define CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
> -#define MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
> -
> //
> // Image type definitions
> //
> @@ -221,7 +218,7 @@ SetUefiImageMemoryAttributes (
> Status = CoreGetMemorySpaceDescriptor(BaseAddress, &Descriptor);
> ASSERT_EFI_ERROR(Status);
>
> - FinalAttributes = (Descriptor.Attributes & CACHE_ATTRIBUTE_MASK) | (Attributes & MEMORY_ATTRIBUTE_MASK);
> + FinalAttributes = (Descriptor.Attributes & EFI_CACHE_ATTRIBUTE_MASK) | (Attributes & EFI_MEMORY_ATTRIBUTE_MASK);
>
> DEBUG ((DEBUG_INFO, "SetUefiImageMemoryAttributes - 0x%016lx - 0x%016lx (0x%016lx)\n", BaseAddress, Length, FinalAttributes));
>
> @@ -924,7 +921,7 @@ InitializeDxeNxMemoryProtectionPolicy (
> (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)) {
>
> Attributes = GetPermissionAttributeForMemoryType (EfiConventionalMemory) |
> - (Entry->Attributes & CACHE_ATTRIBUTE_MASK);
> + (Entry->Attributes & EFI_CACHE_ATTRIBUTE_MASK);
>
> DEBUG ((DEBUG_INFO,
> "Untested GCD memory space region: - 0x%016lx - 0x%016lx (0x%016lx)\n",
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> index 0385f1d..599a0cd 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> @@ -39,12 +39,6 @@
>
> #define SMRAM_CAPABILITIES (EFI_MEMORY_WB | EFI_MEMORY_UC)
>
> -#define MEMORY_CACHE_ATTRIBUTES (EFI_MEMORY_UC | EFI_MEMORY_WC | \
> - EFI_MEMORY_WT | EFI_MEMORY_WB | \
> - EFI_MEMORY_WP | EFI_MEMORY_UCE)
> -
> -#define MEMORY_PAGE_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | EFI_MEMORY_RO)
> -
> //
> // Function prototypes from produced protocols
> //
> @@ -1710,7 +1704,7 @@ SmmIplEntry (
> CpuArch = NULL;
> Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&CpuArch);
> if (!EFI_ERROR (Status)) {
> - MemDesc.Attributes &= ~(MEMORY_CACHE_ATTRIBUTES | MEMORY_PAGE_ATTRIBUTES);
> + MemDesc.Attributes &= ~(EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK);
> MemDesc.Attributes |= EFI_MEMORY_WB;
> Status = gDS->SetMemorySpaceAttributes (
> mSmramCacheBase,
> @@ -1727,7 +1721,7 @@ SmmIplEntry (
> &MemDesc
> );
> DEBUG ((DEBUG_INFO, "SMRAM attributes: %016lx\n", MemDesc.Attributes));
> - ASSERT ((MemDesc.Attributes & MEMORY_PAGE_ATTRIBUTES) == 0);
> + ASSERT ((MemDesc.Attributes & EFI_MEMORY_ATTRIBUTE_MASK) == 0);
> );
> }
> //
>
next prev parent reply other threads:[~2020-07-06 9:14 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-02 20:50 [PATCH V3 0/4] Add New Memory Attributes Oleksiy Yakovlev
2020-07-02 20:50 ` [PATCH V3 1/4] MdePkg: " Oleksiy Yakovlev
2020-07-06 9:11 ` [edk2-devel] " Laszlo Ersek
2020-07-02 20:50 ` [PATCH V3 2/4] MdeModulePkg: " Oleksiy Yakovlev
2020-07-06 9:13 ` Laszlo Ersek [this message]
2020-07-13 16:12 ` [edk2-devel] " Laszlo Ersek
2020-07-02 20:50 ` [PATCH V3 3/4] UefiCpuPkg: Update EFI_MEMORY_CACHETYPE_MASK definition Oleksiy Yakovlev
2020-07-06 9:16 ` [edk2-devel] " Laszlo Ersek
2020-07-13 16:13 ` Laszlo Ersek
2020-07-14 0:13 ` Dong, Eric
2020-07-02 20:50 ` [PATCH V3 4/4] UefiCpuPkg: Add New Memory Attributes Oleksiy Yakovlev
2020-07-13 16:14 ` [edk2-devel] " Laszlo Ersek
2020-07-14 0:16 ` Dong, Eric
2020-07-06 9:57 ` [edk2-devel] [PATCH V3 0/4] " Laszlo Ersek
2020-07-15 15:07 ` Laszlo Ersek
2023-11-17 8:07 ` Dhaval Sharma
2023-11-17 8:25 ` Laszlo Ersek
2023-11-20 4:59 ` Dhaval Sharma
2023-11-21 15:03 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a5ebc20f-e532-314f-c4e5-d376b0d59294@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox