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...@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
            <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 <kra...@redhat.com>
            <mailto:kra...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to