Chao, Thank you for your patience for preparing the new version of patches. However, I still have following minor comments: For patches 1~6: Reviewed-by: Ray Ni For patches 7: can you define meaning of bits in the Attributes/Mask? It seems you are reusing the definitions defined for 7.2.3 EFI_BOOT_SERVICES.GetMemoryMap(). It's fine. But please mention that in comments. Also please use UINT64 for Attributes instead of UINTN. For patches 8: Can you rename PcdCpuExceptionVectorBaseAddress to PcdLoongArch64ExceptionVectorBaseAddress and put the PCD definition/reference in the DEC/INF LoongArch64 section? For patches 9: Please make accordingly changes when you address comments for patch 7. For patches 10, 11: Can the lib be avoided if the logic is implemented in CpuDxe driver? For patch 12(UefiCpuPkg: Add multiprocessor library for LoongArch64): Reviewed-by: Ray Ni For patch 13: Please make accordingly changes when you address comments for patch 8. Thanks, Ray ________________________________ From: Gerd Hoffmann Sent: Friday, March 22, 2024 20:39 To: Chao Li Cc: devel@edk2.groups.io ; Ni, Ray ; Kumar, Rahul R ; Sami Mujawar ; Sunil V L ; Bibo Mao ; Dongyan Qian Subject: Re: [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg On Wed, Mar 20, 2024 at 04:41:52PM +0800, Chao Li wrote: > This patch set adjusted some order in UefiCpuPig alphabetically, added > LoongArch libraries and drivers into UefiCpuPkg, it is a continuation of > the first patch series v8 submitted at > https://edk2.groups.io/g/devel/message/114526. > > And also separated from https://edk2.groups.io/g/devel/message/116583. > > This series only contents the changes for UefiCpuPkg. > > Patch1-Patch4: Reorder some INF files located in UefiCpuPkg > alphabetically. > > Patch5-Patch13: Added Timer, CpuMmuLib, CpuMmuInitLib, MpInitLib, CpuDxe > for LoongArch, and added some PCD and header files requested by the > above libraries and drivers. > > Modfied modules: UefiCpuPkg > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4726 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4734 > > PR: https://github.com/tianocore/edk2/pull/5483 > > V1 -> V2: > 1. Removed PcdCpuMmuIsEnabled. > 2. Removed API GetMemoryRegionAttributes API as it is no longer needed. > 3. Patch3, added two empty line in DXE and PEI INF files. > 4. Patch5, added the Status check in GetTimeInnanoSecond function. > 5. Separated into two series, this is series one, and the second one is > OvmfPkg. While I can't comment on the loongarch architecture details the code and the integration into build system looks overall sane to me. Series: Acked-by: Gerd Hoffmann take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117077): https://edk2.groups.io/g/devel/message/117077 Mute This Topic: https://groups.io/mt/105041080/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-