Hi
> > +EFI_STATUS
> > +EFIAPI
> > +BspAcceptMemoryResourceRange (
> > + IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> > + IN EFI_PHYSICAL_ADDRESS PhysicalEnd
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINT32 AcceptPageSize;
> > + UINT64 StartAddress1;
> > + UINT64 StartAddress2;
> > + UINT64 StartAddress3;
> > + UINT64 TotalLength;
> > + UINT64 Length1;
> > + UINT64 Length2;
> > + UINT64 Length3;
> > + UINT64 Pages;
> > +
> > + AcceptPageSize = FixedPcdGet32 (PcdTdxAcceptPageSize);
> > + TotalLength = PhysicalEnd - PhysicalAddress;
> > + StartAddress1 = 0;
> > + StartAddress2 = 0;
> > + StartAddress3 = 0;
> > + Length1 = 0;
> > + Length2 = 0;
> > + Length3 = 0;
> > +
> > + if (TotalLength == 0) {
> > + return EFI_SUCCESS;
> > + }
> > +
> > + if ((AcceptPageSize == SIZE_4KB) || (TotalLength <= SIZE_2MB)) {
> > + //
> > + // if total length is less than 2M, then we accept pages in 4k
> > + //
> > + StartAddress1 = 0;
> > + Length1 = 0;
> > + StartAddress2 = PhysicalAddress;
> > + Length2 = PhysicalEnd - PhysicalAddress;
> > + StartAddress3 = 0;
> > + Length3 = 0;
> > + AcceptPageSize = SIZE_4KB;
>
> You've zero-initialized everything above, no need to do that again.
Thanks for reminder. It will be updated in the next version.
> You also can just use StartAddress1 + Length1 which uses 4k pages anyway.
In the origin design, StartAddress1+Length1 and StartAddress3+Length3 are for
BSP, StartAddress2+Length2 is for BSP/APs. Now only BSP accepts memory. So
you're right. We can just use StartAddress1+Length1 which uses 4k pages anyway.
>
> > + } else if (AcceptPageSize == SIZE_2MB) {
> > + //
> > + // Total length is bigger than 2M and Page Accept size 2M is supported.
> > + //
> > + if ((PhysicalAddress & ALIGNED_2MB_MASK) == 0) {
>
> The logic is rather messy. It should become much simpler if you update
> PhysicalAddress and TotalLength. You can also use the alignment macros.
>
> if (ALIGN_VALUE(PhysicalAddress, SIZE_2MB) != PhysicalAddress) {
> StartAddress1 = PhysicalAddress;
> Length1 = ALIGN_VALUE(PhysicalAddress, SIZE_2MB) - PhysicalAddress;
> PhysicalAddress += Length1;
> TotalLength -= Length1;
> }
> if (TotalLength > SIZE_2MB) {
> StartAddress2 = PhysicalAddress;
> Length2 = TotalLength & ~(UINT64)ALIGNED_2MB_MASK
> PhysicalAddress += Length2;
> TotalLength -= Length2;
> }
> if (TotalLength) {
> StartAddress3 = PhysicalAddress;
> Length3 = TotalLength;
> }
>
Thanks for the suggestion. It's better. It will be updated in the next version.
Thanks
Min
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84969): https://edk2.groups.io/g/devel/message/84969
Mute This Topic: https://groups.io/mt/87696588/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
