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 <ray...@intel.com><mailto:ray...@intel.com> For patch 13: Please make accordingly changes when you address comments for patch 8. OK. Thanks, Ray ________________________________ From: Gerd Hoffmann <kra...@redhat.com><mailto:kra...@redhat.com> Sent: Friday, March 22, 2024 20:39 To: Chao Li <lic...@loongson.cn><mailto:lic...@loongson.cn> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io><mailto:devel@edk2.groups.io>; Ni, Ray <ray...@intel.com><mailto:ray...@intel.com>; Kumar, Rahul R <rahul.r.ku...@intel.com><mailto:rahul.r.ku...@intel.com>; Sami Mujawar <sami.muja...@arm.com><mailto:sami.muja...@arm.com>; Sunil V L <suni...@ventanamicro.com><mailto:suni...@ventanamicro.com>; Bibo Mao <maob...@loongson.cn><mailto:maob...@loongson.cn>; Dongyan Qian <qiandong...@loongson.cn><mailto:qiandong...@loongson.cn> 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 <kra...@redhat.com><mailto:kra...@redhat.com> 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: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
