Reviewed-by: Ray Ni <ray...@intel.com>
> -----Original Message-----
> From: Taylor Beebe <taylor.d.be...@gmail.com>
> Sent: Thursday, June 16, 2022 5:23 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Kumar,
> Rahul1 <rahul1.ku...@intel.com>
> Subject: [PATCH v1 1/1] UefiCpuPkg: CpuDxe: Set RW and P Attributes on Split
> Pages
>
> From: Taylor Beebe <t...@taylorbeebe.com>
>
> A memory range can be submitted for attribute changes which is large
> enough to not require a page split during the attribute update. Consider
> the following scenario:
>
> 1. An attribute update removed the RW attribute on a range large enough
> to not require a page split.
> 2. Later, an attributes update is called to re-add the RW attribute for
> a subsection of that larger page which requires a split
> 3. The attribute update logic performs a page split, so now the parent
> and child pages have matching attributes
> 4. Then, the attribute update logic changes the child page to have the
> RW attribute.
> 5. The child page would then correctly have the RW attribute added but
> the parent page would still have the RW attribute removed which will
> cause an improper access violation.
>
> The page being split should have loose attributes to accommodate the
> above case. The split page should always have the attributes set so
> the lowest level page frame determines the access rights as detailed
> in 4.10.2.2 of the Intel 64 and IA-32 Architectures Software
> Developer Manual. Setting the User/Supervisor attribute shouldn't
> be necessary.
>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Signed-off-by: Taylor Beebe <t...@taylorbeebe.com>
> ---
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index f7a4d92e921a..288d9996f6c3 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -38,6 +38,8 @@
> #define IA32_PG_NX BIT63
>
> #define PAGE_ATTRIBUTE_BITS (IA32_PG_D | IA32_PG_A | IA32_PG_U | IA32_PG_RW
> | IA32_PG_P)
> +#define PAGE_ATTRIBUTE_BITS_POST_SPLIT (IA32_PG_RW | IA32_PG_P)
> +
> //
> // Bits 1, 2, 5, 6 are reserved in the IA32 PAE PDPTE
> // X64 PAE PDPTE does not have such restriction
> @@ -583,7 +585,7 @@ SplitPage (
> NewPageEntry[Index] = (BaseAddress + SIZE_4KB * Index) |
> AddressEncMask | ((*PageEntry) &
> PAGE_PROGATE_BITS);
> }
>
> - (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> ((*PageEntry) & PAGE_ATTRIBUTE_BITS);
> + (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> PAGE_ATTRIBUTE_BITS_POST_SPLIT;
> return RETURN_SUCCESS;
> } else {
> return RETURN_UNSUPPORTED;
> @@ -606,7 +608,7 @@ SplitPage (
> NewPageEntry[Index] = (BaseAddress + SIZE_2MB * Index) |
> AddressEncMask | IA32_PG_PS | ((*PageEntry) &
> PAGE_PROGATE_BITS);
> }
>
> - (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> ((*PageEntry) & PAGE_ATTRIBUTE_BITS);
> + (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> PAGE_ATTRIBUTE_BITS_POST_SPLIT;
> return RETURN_SUCCESS;
> } else {
> return RETURN_UNSUPPORTED;
> --
> 2.32.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90705): https://edk2.groups.io/g/devel/message/90705
Mute This Topic: https://groups.io/mt/91786433/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
