[edk2-devel] [Patch V4 18/21] UefiCpuPkg: Combine branch for non-present and leaf ParentEntry

[Ni, Ray]

> -    //
> -    // The parent entry is CR3 or PML5E/PML4E/PDPTE/PDE.
> +    // When ParentPagingEntry is non-present, parent entry is CR3 or
> PML5E/PML4E/PDPTE/PDE.
>      // It does NOT point to an existing page directory.
> +    // When ParentPagingEntry is present, parent entry is leaf PDPTE_1G or
> PDE_2M. Split to 2M or 4K pages.
> +    // Note: it's impossible the parent entry is a PTE_4K.
>      //
> -    ASSERT (Buffer == NULL || *BufferSize >= SIZE_4KB);
> -    CreateNew    = TRUE;
> -    *BufferSize -= SIZE_4KB;
> +    OneOfPagingEntry.Pnle.Uint64 = 0;
> +    PleBAttribute.Uint64         = PageTableLibGetPleBMapAttribute
> (&ParentPagingEntry->PleB, ParentAttribute);
> 
> -    if (Modify) {
> -      ParentPagingEntry->Uintn = (UINTN)Buffer + *BufferSize;
> -      ZeroMem ((VOID *)ParentPagingEntry->Uintn, SIZE_4KB);
> +    if (ParentPagingEntry->Pce.Present == 0) {
>        //
> -      // Set default attribute bits for PML5E/PML4E/PDPTE/PDE.
> +      // [LinearAddress, LinearAddress + Length] contains non-present range.
>        //
> -      PageTableLibSetPnle (&ParentPagingEntry->Pnle, &NopAttribute,
> &AllOneMask);
> +      Status = IsAttributesAndMaskValidForNonPresentEntry (Attribute,
> Mask);
> +      if (RETURN_ERROR (Status)) {
> +        return Status;
> +      }

1. do you think put " OneOfPagingEntry.Pnle.Uint64 = 0;" in the if-body is better?
So both if and else initializes OneOfPagingEntry.
Other logic looks good to me.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101689): https://edk2.groups.io/g/devel/message/101689
Mute This Topic: https://groups.io/mt/97796396/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3943202/1813853/130120423/xyzzy [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-