Thanks for the comments. Will do the code change in next version patch set.
Thanks,
Dun
-----Original Message-----
From: Ni, Ray <ray...@intel.com>
Sent: Wednesday, March 15, 2023 1:34 PM
To: Tan, Dun <dun....@intel.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.d...@intel.com>; Kumar, Rahul R <rahul.r.ku...@intel.com>;
Gerd Hoffmann <kra...@redhat.com>
Subject: RE: [Patch V2 05/14] UefiCpuPkg/CpuPageTebleLib: Check Mask and Attr
in PageTableMap
> +**/
> +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 (#101222): https://edk2.groups.io/g/devel/message/101222
Mute This Topic: https://groups.io/mt/97469476/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
