[edk2-devel] [Patch V2 05/14] UefiCpuPkg/CpuPageTebleLib: Check Mask and Attr in PageTableMap

[Ni, Ray]

> +**/
> +RETURN_STATUS
> +CheckMaskAndAttrForNotPresentEntry (
> +  IN     IA32_MAP_ATTRIBUTE  *Attribute,
> +  IN     IA32_MAP_ATTRIBUTE  *Mask
> +  )
> +{
> +  if ((Attribute->Bits.Present == 0) || (Mask->Bits.Present == 0) || (Mask-
> >Bits.ReadWrite == 0) ||

1. I think we can allow caller to set a not-present range as not-present.
Even though it's meaningless😊
So, I think we can remove the Attribute.Present check.


2. The function name can be more readable: how about IsAllAttributesSetForNonPresentEntry()?


> +      (Mask->Bits.UserSupervisor == 0) || (Mask->Bits.WriteThrough == 0) ||
> (Mask->Bits.CacheDisabled == 0) ||
> +      (Mask->Bits.Accessed == 0) || (Mask->Bits.Dirty == 0) || (Mask-
> >Bits.Pat == 0) || (Mask->Bits.Global == 0) ||
> +      (Mask->Bits.PageTableBaseAddress == 0) || (Mask->Bits.ProtectionKey
> == 0) || (Mask->Bits.Nx == 0))
> +  {
> +    return RETURN_INVALID_PARAMETER;
> +  }
> +
> +  return RETURN_SUCCESS;
> +}

> +    PagingEntry           = (IA32_PAGING_ENTRY
> *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&ParentPagingEntry-
> >Pnle);
> +    PagingEntryIndexLimit = (BitFieldRead64 (LinearAddress + Length - 1,
> BitStart + 9, 63) > BitFieldRead64 (LinearAddress + Offset, BitStart + 9, 63)) ?
> 511 :

3. Can you add more comments for the code here?





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101194): https://edk2.groups.io/g/devel/message/101194
Mute This Topic: https://groups.io/mt/97469476/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]
-=-=-=-=-=-=-=-=-=-=-=-