Chao, Sorry I missed your mail. If ConfigureMemoryManagementUnit() is called in PEI, can you move the logic to a LoongArch specific PEIM? My concern is we may need more review on the lib API ConfigureMemoryManagementUnit() if we position it as a library.
If we move the logic in a PEIM and the implementation becomes a PEIM internal logic, we can lower the quality expectation of the function prototype as no other module is able to call it. Thanks, Ray For patches 10, 11: Can the lib be avoided if the logic is implemented in CpuDxe driver? This library is will be called in the PEI stage, so I can't move it under the CpuDxe. This library is the low-level libary of CpuMmuLib, which will consume CpuMmuLIb to configure the MMU. This way is suggested by Laszlo, who saied if CpuMmuLib can not content the configure API(high-level libary is the basecal libaray, it should not include the configure API), we can split it into two, where the hight-livel is CpuMmuLib, and the low-level is CpuMmuInitLib. For patch 12(UefiCpuPkg: Add multiprocessor library for LoongArch64): Reviewed-by: Ray Ni <[email protected]><mailto:[email protected]> For patch 13: Please make accordingly changes when you address comments for patch 8. OK. Thanks, Ray ________________________________ From: Gerd Hoffmann <[email protected]><mailto:[email protected]> Sent: Friday, March 22, 2024 20:39 To: Chao Li <[email protected]><mailto:[email protected]> Cc: [email protected]<mailto:[email protected]> <[email protected]><mailto:[email protected]>; Ni, Ray <[email protected]><mailto:[email protected]>; Kumar, Rahul R <[email protected]><mailto:[email protected]>; Sami Mujawar <[email protected]><mailto:[email protected]>; Sunil V L <[email protected]><mailto:[email protected]>; Bibo Mao <[email protected]><mailto:[email protected]>; Dongyan Qian <[email protected]><mailto:[email protected]> 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 <[email protected]><mailto:[email protected]> take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117516): https://edk2.groups.io/g/devel/message/117516 Mute This Topic: https://groups.io/mt/105041080/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
