Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg

["Chao Li" <lichao@loongson.cn>]

[-- Attachment #1: Type: text/plain, Size: 5781 bytes --]

Hi Ray,

I'm willing change it to a PEIM if it doesn't fit as a library. I think 
if it is a PEIM, we need a new GUID to sotre the memory map resouce, or 
use an already defined GUID.

I will put it under the UefiCpuPkg, called CpuMmuInitPei, folder: 
UefiCpuPkg/CpuMmuInitPei/LoongArch64/. May I?


Thanks,
Chao
On 2024/4/9 10:06, Ni, Ray wrote:
> 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.ni@intel.com>
>             <mailto:ray.ni@intel.com>
>             For patch 13: Please make accordingly changes when you
>             address comments for patch 8.
>
>         OK.
>
>
>             Thanks,
>             Ray
>             ------------------------------------------------------------------------
>             *From:* Gerd Hoffmann <kraxel@redhat.com>
>             <mailto:kraxel@redhat.com>
>             *Sent:* Friday, March 22, 2024 20:39
>             *To:* Chao Li <lichao@loongson.cn> <mailto:lichao@loongson.cn>
>             *Cc:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>             <devel@edk2.groups.io> <mailto:devel@edk2.groups.io>; Ni,
>             Ray <ray.ni@intel.com> <mailto:ray.ni@intel.com>; Kumar,
>             Rahul R <rahul.r.kumar@intel.com>
>             <mailto:rahul.r.kumar@intel.com>; Sami Mujawar
>             <sami.mujawar@arm.com> <mailto:sami.mujawar@arm.com>;
>             Sunil V L <sunilvl@ventanamicro.com>
>             <mailto:sunilvl@ventanamicro.com>; Bibo Mao
>             <maobibo@loongson.cn> <mailto:maobibo@loongson.cn>;
>             Dongyan Qian <qiandongyan@loongson.cn>
>             <mailto:qiandongyan@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
>             <https://edk2.groups.io/g/devel/message/114526>.
>             >
>             > And also separated from
>             https://edk2.groups.io/g/devel/message/116583
>             <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
>             <https://bugzilla.tianocore.org/show_bug.cgi?id=4726>
>             > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4734
>             <https://bugzilla.tianocore.org/show_bug.cgi?id=4734>
>             >
>             > PR: https://github.com/tianocore/edk2/pull/5483
>             <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 <kraxel@redhat.com>
>             <mailto:kraxel@redhat.com>
>
>             take care,
>               Gerd
>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117518): https://edk2.groups.io/g/devel/message/117518
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]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 15271 bytes --]