Hi Ray,
Ha, you mean to move the ConfigureMemoryManagementUint instance into
some LoongArch PEIM, right? I just understood it wrong.
What I want to say is that this API can called in both virtual and
really mechine, if it be moved into the private code, then if other
platform want to call it, they will have to copy the same code under
their private code. So I think it is better if this API or function is
made public.
Thanks,
Chao
On 2024/4/9 13:27, Ni, Ray wrote:
Chao,
Current patch introduces the CpuMmuInitLib which contains
ConfigureMemoryManagementUnit () API. You told me the API will be
called by a PEIM.
Then, the new proposal is to move the library code into that PEIM.
I don't quite understand your needs of the new GUID to store the
memory map resource. How is the GUID used to store the memory map
resource?
Can the PEIM be placed in edk2-platforms repo?
Thanks,
Ray
------------------------------------------------------------------------
*From:* Chao Li <lic...@loongson.cn>
*Sent:* Tuesday, April 9, 2024 12:29
*To:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray
<ray...@intel.com>; Gerd Hoffmann <kra...@redhat.com>
*Cc:* Kumar, Rahul R <rahul.r.ku...@intel.com>; Sami Mujawar
<sami.muja...@arm.com>; Sunil V L <suni...@ventanamicro.com>; Bibo Mao
<maob...@loongson.cn>; Dongyan Qian <qiandong...@loongson.cn>
*Subject:* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add
LoongArch support into UefiCpuPkg
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 (#117537): https://edk2.groups.io/g/devel/message/117537
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]
-=-=-=-=-=-=-=-=-=-=-=-
