public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
@ 2024-03-20  8:41 Chao Li
  2024-03-22 12:39 ` Gerd Hoffmann
  0 siblings, 1 reply; 12+ messages in thread
From: Chao Li @ 2024-03-20  8:41 UTC (permalink / raw)
  To: devel
  Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Sami Mujawar, Sunil V L,
	Bibo Mao, Dongyan Qian

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.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Bibo Mao <maobibo@loongson.cn>
Cc: Dongyan Qian <qiandongyan@loongson.cn>

Chao Li (13):
  UefiCpuPkg/CpuTimerLib: Reorder the INF file alphabetically
  UefiCpuPkg/CpuExceptionHandlerLib: Reorder the INF files
    alphabetically
  UefiCpuPkg/MpInitLib: Reorder the INF files alphabetically
  UefiCpuPkg/CpuDxe: Reorder the INF file alphabetically
  UefiCpuPkg: Add LoongArch64 CPU Timer instance
  UefiCpuPkg: Add CPU exception library for LoongArch
  UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg
  UefiCpuPkg: Added a new PCD named PcdCpuExceptionVectorBaseAddress
  UefiCpuPkg: Add CpuMmuLib to UefiCpuPkg
  UefiCpuPkg: Add CpuMmuInitLib.h to UefiCpuPkg
  UefiCpuPkg: Add CpuMmuInitLib to UefiCpuPkg
  UefiCpuPkg: Add multiprocessor library for LoongArch64
  UefiCpuPkg: Add CpuDxe driver for LoongArch64

 UefiCpuPkg/CpuDxe/CpuDxe.inf                  |   37 +-
 UefiCpuPkg/CpuDxe/LoongArch64/CpuDxe.c        |  426 +++++
 UefiCpuPkg/CpuDxe/LoongArch64/CpuDxe.h        |  288 +++
 UefiCpuPkg/CpuDxe/LoongArch64/CpuMp.c         |  544 ++++++
 UefiCpuPkg/CpuDxe/LoongArch64/Exception.c     |  159 ++
 UefiCpuPkg/Include/Library/CpuMmuInitLib.h    |   34 +
 UefiCpuPkg/Include/Library/CpuMmuLib.h        |   55 +
 .../DxeCpuExceptionHandlerLib.inf             |   37 +-
 .../LoongArch/DxeExceptionLib.c               |  198 ++
 .../LoongArch/ExceptionCommon.c               |  171 ++
 .../LoongArch/ExceptionCommon.h               |  131 ++
 .../LoongArch64/ArchExceptionHandler.c        |  268 +++
 .../LoongArch64/ExceptionHandlerAsm.S         |  366 ++++
 .../LoongArch/SecPeiExceptionLib.c            |  102 ++
 .../PeiCpuExceptionHandlerLib.inf             |   16 +-
 .../SecPeiCpuExceptionHandlerLib.inf          |   31 +-
 .../SmmCpuExceptionHandlerLib.inf             |   16 +-
 .../Library/CpuMmuInitLib/CpuMmuInitLib.inf   |   41 +
 .../Library/CpuMmuInitLib/CpuMmuInitLib.uni   |   14 +
 .../CpuMmuInitLib/LoongArch64/CpuMmuInit.c    |  232 +++
 .../LoongArch64/TlbExceptionHandle.S          |   51 +
 .../LoongArch64/TlbExceptionHandle.h          |   36 +
 UefiCpuPkg/Library/CpuMmuLib/CpuMmuLib.inf    |   35 +
 UefiCpuPkg/Library/CpuMmuLib/CpuMmuLib.uni    |   14 +
 .../Library/CpuMmuLib/LoongArch64/CpuMmu.c    |  638 +++++++
 .../Library/CpuMmuLib/LoongArch64/Page.h      |   24 +
 .../CpuMmuLib/LoongArch64/TlbInvalid.S        |   24 +
 .../CpuMmuLib/LoongArch64/TlbInvalid.h        |   24 +
 .../Library/CpuTimerLib/BaseCpuTimerLib.inf   |   17 +-
 .../CpuTimerLib/LoongArch64/CpuTimerLib.c     |  250 +++
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   39 +-
 .../Library/MpInitLib/LoongArch64/DxeMpLib.c  |  480 +++++
 .../Library/MpInitLib/LoongArch64/MpLib.c     | 1621 +++++++++++++++++
 .../Library/MpInitLib/LoongArch64/MpLib.h     |  361 ++++
 .../Library/MpInitLib/LoongArch64/PeiMpLib.c  |  404 ++++
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   37 +-
 UefiCpuPkg/UefiCpuPkg.dec                     |   12 +
 UefiCpuPkg/UefiCpuPkg.dsc                     |    7 +
 38 files changed, 7161 insertions(+), 79 deletions(-)
 create mode 100644 UefiCpuPkg/CpuDxe/LoongArch64/CpuDxe.c
 create mode 100644 UefiCpuPkg/CpuDxe/LoongArch64/CpuDxe.h
 create mode 100644 UefiCpuPkg/CpuDxe/LoongArch64/CpuMp.c
 create mode 100644 UefiCpuPkg/CpuDxe/LoongArch64/Exception.c
 create mode 100644 UefiCpuPkg/Include/Library/CpuMmuInitLib.h
 create mode 100644 UefiCpuPkg/Include/Library/CpuMmuLib.h
 create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/DxeExceptionLib.c
 create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/ExceptionCommon.c
 create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/ExceptionCommon.h
 create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/LoongArch64/ArchExceptionHandler.c
 create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/LoongArch64/ExceptionHandlerAsm.S
 create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/SecPeiExceptionLib.c
 create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/CpuMmuInitLib.inf
 create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/CpuMmuInitLib.uni
 create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/LoongArch64/CpuMmuInit.c
 create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/LoongArch64/TlbExceptionHandle.S
 create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/LoongArch64/TlbExceptionHandle.h
 create mode 100644 UefiCpuPkg/Library/CpuMmuLib/CpuMmuLib.inf
 create mode 100644 UefiCpuPkg/Library/CpuMmuLib/CpuMmuLib.uni
 create mode 100644 UefiCpuPkg/Library/CpuMmuLib/LoongArch64/CpuMmu.c
 create mode 100644 UefiCpuPkg/Library/CpuMmuLib/LoongArch64/Page.h
 create mode 100644 UefiCpuPkg/Library/CpuMmuLib/LoongArch64/TlbInvalid.S
 create mode 100644 UefiCpuPkg/Library/CpuMmuLib/LoongArch64/TlbInvalid.h
 create mode 100644 UefiCpuPkg/Library/CpuTimerLib/LoongArch64/CpuTimerLib.c
 create mode 100644 UefiCpuPkg/Library/MpInitLib/LoongArch64/DxeMpLib.c
 create mode 100644 UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.c
 create mode 100644 UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.h
 create mode 100644 UefiCpuPkg/Library/MpInitLib/LoongArch64/PeiMpLib.c

-- 
2.27.0



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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
       [not found] <17BE6C7171CBC84C.24580@groups.io>
@ 2024-03-22  1:10 ` Chao Li
  0 siblings, 0 replies; 12+ messages in thread
From: Chao Li @ 2024-03-22  1:10 UTC (permalink / raw)
  To: devel
  Cc: Rahul Kumar, Gerd Hoffmann, Sami Mujawar, Sunil V L, Bibo Mao,
	Dongyan Qian

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

Hi Ray and other maintainers,

I submitted this series few days ago, could you review them and give me 
the R-B?


Thanks,
Chao
On 2024/3/20 16:41, 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 fromhttps://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.
>
> Cc: Ray Ni<ray.ni@intel.com>
> Cc: Rahul Kumar<rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann<kraxel@redhat.com>
> Cc: Sami Mujawar<sami.mujawar@arm.com>
> Cc: Sunil V L<sunilvl@ventanamicro.com>
> Cc: Bibo Mao<maobibo@loongson.cn>
> Cc: Dongyan Qian<qiandongyan@loongson.cn>
>
> Chao Li (13):
>    UefiCpuPkg/CpuTimerLib: Reorder the INF file alphabetically
>    UefiCpuPkg/CpuExceptionHandlerLib: Reorder the INF files
>      alphabetically
>    UefiCpuPkg/MpInitLib: Reorder the INF files alphabetically
>    UefiCpuPkg/CpuDxe: Reorder the INF file alphabetically
>    UefiCpuPkg: Add LoongArch64 CPU Timer instance
>    UefiCpuPkg: Add CPU exception library for LoongArch
>    UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg
>    UefiCpuPkg: Added a new PCD named PcdCpuExceptionVectorBaseAddress
>    UefiCpuPkg: Add CpuMmuLib to UefiCpuPkg
>    UefiCpuPkg: Add CpuMmuInitLib.h to UefiCpuPkg
>    UefiCpuPkg: Add CpuMmuInitLib to UefiCpuPkg
>    UefiCpuPkg: Add multiprocessor library for LoongArch64
>    UefiCpuPkg: Add CpuDxe driver for LoongArch64
>
>   UefiCpuPkg/CpuDxe/CpuDxe.inf                  |   37 +-
>   UefiCpuPkg/CpuDxe/LoongArch64/CpuDxe.c        |  426 +++++
>   UefiCpuPkg/CpuDxe/LoongArch64/CpuDxe.h        |  288 +++
>   UefiCpuPkg/CpuDxe/LoongArch64/CpuMp.c         |  544 ++++++
>   UefiCpuPkg/CpuDxe/LoongArch64/Exception.c     |  159 ++
>   UefiCpuPkg/Include/Library/CpuMmuInitLib.h    |   34 +
>   UefiCpuPkg/Include/Library/CpuMmuLib.h        |   55 +
>   .../DxeCpuExceptionHandlerLib.inf             |   37 +-
>   .../LoongArch/DxeExceptionLib.c               |  198 ++
>   .../LoongArch/ExceptionCommon.c               |  171 ++
>   .../LoongArch/ExceptionCommon.h               |  131 ++
>   .../LoongArch64/ArchExceptionHandler.c        |  268 +++
>   .../LoongArch64/ExceptionHandlerAsm.S         |  366 ++++
>   .../LoongArch/SecPeiExceptionLib.c            |  102 ++
>   .../PeiCpuExceptionHandlerLib.inf             |   16 +-
>   .../SecPeiCpuExceptionHandlerLib.inf          |   31 +-
>   .../SmmCpuExceptionHandlerLib.inf             |   16 +-
>   .../Library/CpuMmuInitLib/CpuMmuInitLib.inf   |   41 +
>   .../Library/CpuMmuInitLib/CpuMmuInitLib.uni   |   14 +
>   .../CpuMmuInitLib/LoongArch64/CpuMmuInit.c    |  232 +++
>   .../LoongArch64/TlbExceptionHandle.S          |   51 +
>   .../LoongArch64/TlbExceptionHandle.h          |   36 +
>   UefiCpuPkg/Library/CpuMmuLib/CpuMmuLib.inf    |   35 +
>   UefiCpuPkg/Library/CpuMmuLib/CpuMmuLib.uni    |   14 +
>   .../Library/CpuMmuLib/LoongArch64/CpuMmu.c    |  638 +++++++
>   .../Library/CpuMmuLib/LoongArch64/Page.h      |   24 +
>   .../CpuMmuLib/LoongArch64/TlbInvalid.S        |   24 +
>   .../CpuMmuLib/LoongArch64/TlbInvalid.h        |   24 +
>   .../Library/CpuTimerLib/BaseCpuTimerLib.inf   |   17 +-
>   .../CpuTimerLib/LoongArch64/CpuTimerLib.c     |  250 +++
>   UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   39 +-
>   .../Library/MpInitLib/LoongArch64/DxeMpLib.c  |  480 +++++
>   .../Library/MpInitLib/LoongArch64/MpLib.c     | 1621 +++++++++++++++++
>   .../Library/MpInitLib/LoongArch64/MpLib.h     |  361 ++++
>   .../Library/MpInitLib/LoongArch64/PeiMpLib.c  |  404 ++++
>   UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   37 +-
>   UefiCpuPkg/UefiCpuPkg.dec                     |   12 +
>   UefiCpuPkg/UefiCpuPkg.dsc                     |    7 +
>   38 files changed, 7161 insertions(+), 79 deletions(-)
>   create mode 100644 UefiCpuPkg/CpuDxe/LoongArch64/CpuDxe.c
>   create mode 100644 UefiCpuPkg/CpuDxe/LoongArch64/CpuDxe.h
>   create mode 100644 UefiCpuPkg/CpuDxe/LoongArch64/CpuMp.c
>   create mode 100644 UefiCpuPkg/CpuDxe/LoongArch64/Exception.c
>   create mode 100644 UefiCpuPkg/Include/Library/CpuMmuInitLib.h
>   create mode 100644 UefiCpuPkg/Include/Library/CpuMmuLib.h
>   create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/DxeExceptionLib.c
>   create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/ExceptionCommon.c
>   create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/ExceptionCommon.h
>   create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/LoongArch64/ArchExceptionHandler.c
>   create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/LoongArch64/ExceptionHandlerAsm.S
>   create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/LoongArch/SecPeiExceptionLib.c
>   create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/CpuMmuInitLib.inf
>   create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/CpuMmuInitLib.uni
>   create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/LoongArch64/CpuMmuInit.c
>   create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/LoongArch64/TlbExceptionHandle.S
>   create mode 100644 UefiCpuPkg/Library/CpuMmuInitLib/LoongArch64/TlbExceptionHandle.h
>   create mode 100644 UefiCpuPkg/Library/CpuMmuLib/CpuMmuLib.inf
>   create mode 100644 UefiCpuPkg/Library/CpuMmuLib/CpuMmuLib.uni
>   create mode 100644 UefiCpuPkg/Library/CpuMmuLib/LoongArch64/CpuMmu.c
>   create mode 100644 UefiCpuPkg/Library/CpuMmuLib/LoongArch64/Page.h
>   create mode 100644 UefiCpuPkg/Library/CpuMmuLib/LoongArch64/TlbInvalid.S
>   create mode 100644 UefiCpuPkg/Library/CpuMmuLib/LoongArch64/TlbInvalid.h
>   create mode 100644 UefiCpuPkg/Library/CpuTimerLib/LoongArch64/CpuTimerLib.c
>   create mode 100644 UefiCpuPkg/Library/MpInitLib/LoongArch64/DxeMpLib.c
>   create mode 100644 UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.c
>   create mode 100644 UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.h
>   create mode 100644 UefiCpuPkg/Library/MpInitLib/LoongArch64/PeiMpLib.c
>


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
  2024-03-20  8:41 Chao Li
@ 2024-03-22 12:39 ` Gerd Hoffmann
  2024-03-25  2:46   ` Ni, Ray
  0 siblings, 1 reply; 12+ messages in thread
From: Gerd Hoffmann @ 2024-03-22 12:39 UTC (permalink / raw)
  To: Chao Li
  Cc: devel, Ray Ni, Rahul Kumar, Sami Mujawar, Sunil V L, Bibo Mao,
	Dongyan Qian

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 <kraxel@redhat.com>

take care,
  Gerd



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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
  2024-03-22 12:39 ` Gerd Hoffmann
@ 2024-03-25  2:46   ` Ni, Ray
  2024-03-25  3:10     ` Chao Li
       [not found]     ` <17BFE34457098413.21233@groups.io>
  0 siblings, 2 replies; 12+ messages in thread
From: Ni, Ray @ 2024-03-25  2:46 UTC (permalink / raw)
  To: Gerd Hoffmann, Chao Li
  Cc: devel@edk2.groups.io, Kumar, Rahul R, Sami Mujawar, Sunil V L,
	Bibo Mao, Dongyan Qian

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

Chao,
Thank you for your patience for preparing the new version of patches.
However, I still have following minor comments:

For patches 1~6: Reviewed-by: Ray Ni <ray.ni@intel.com>
For patches 7: can you define meaning of bits in the Attributes/Mask? It seems you are reusing the definitions defined for 7.2.3 EFI_BOOT_SERVICES.GetMemoryMap(). It's fine. But please mention that in comments. Also please use UINT64 for Attributes instead of UINTN.
For patches 8: Can you rename PcdCpuExceptionVectorBaseAddress to PcdLoongArch64ExceptionVectorBaseAddress and put the PCD definition/reference in the DEC/INF LoongArch64 section?
For patches 9: Please make accordingly changes when you address comments for patch 7.
For patches 10, 11: Can the lib be avoided if the logic is implemented in CpuDxe driver?
For patch 12(UefiCpuPkg: Add multiprocessor library for LoongArch64): Reviewed-by: Ray Ni <ray.ni@intel.com>
For patch 13: Please make accordingly changes when you address comments for patch 8.

Thanks,
Ray
________________________________
From: Gerd Hoffmann <kraxel@redhat.com>
Sent: Friday, March 22, 2024 20:39
To: Chao Li <lichao@loongson.cn>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Sami Mujawar <sami.mujawar@arm.com>; Sunil V L <sunilvl@ventanamicro.com>; Bibo Mao <maobibo@loongson.cn>; Dongyan Qian <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.
>
> 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 <kraxel@redhat.com>

take care,
  Gerd



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
  2024-03-25  2:46   ` Ni, Ray
@ 2024-03-25  3:10     ` Chao Li
       [not found]     ` <17BFE34457098413.21233@groups.io>
  1 sibling, 0 replies; 12+ messages in thread
From: Chao Li @ 2024-03-25  3:10 UTC (permalink / raw)
  To: devel, ray.ni, Gerd Hoffmann
  Cc: Kumar, Rahul R, Sami Mujawar, Sunil V L, Bibo Mao, Dongyan Qian

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

Hi Ray,

Thank you for reviewing my patch series very carefully.

Here are my comments:


Thanks,
Chao
On 2024/3/25 10:46, Ni, Ray wrote:
> Chao,
> Thank you for your patience for preparing the new version of patches.
> However, I still have following minor comments:
>
> For patches 1~6: Reviewed-by: Ray Ni <ray.ni@intel.com>
> For patches 7: can you define meaning of bits in the Attributes/Mask? 
> It seems you are reusing the definitions defined for 7.2.3 
> EFI_BOOT_SERVICES.GetMemoryMap(). It's fine. But please mention that 
> in comments. Also please use UINT64 for Attributes instead of UINTN.
Yes, I'm reusing the definitions defined for 7.2.3, I will illustrate in 
the comments next time. About the UINTN, I will fix it next time.
> For patches 8: Can you rename PcdCpuExceptionVectorBaseAddress to 
> PcdLoongArch64ExceptionVectorBaseAddress and put the PCD 
> definition/reference in the DEC/INF LoongArch64 section?
OK, I will rename it next time.
> For patches 9: Please make accordingly changes when you address 
> comments for patch 7.
OK.
> 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>
> For patch 13: Please make accordingly changes when you address 
> comments for patch 8.
OK.
>
> Thanks,
> Ray
> ------------------------------------------------------------------------
> *From:* Gerd Hoffmann <kraxel@redhat.com>
> *Sent:* Friday, March 22, 2024 20:39
> *To:* Chao Li <lichao@loongson.cn>
> *Cc:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray 
> <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Sami 
> Mujawar <sami.mujawar@arm.com>; Sunil V L <sunilvl@ventanamicro.com>; 
> Bibo Mao <maobibo@loongson.cn>; Dongyan Qian <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.
> >
> > 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 <kraxel@redhat.com>
>
> take care,
>   Gerd
>
> 


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
       [not found]     ` <17BFE34457098413.21233@groups.io>
@ 2024-03-26  1:32       ` Chao Li
       [not found]       ` <17C02C7EE39BF604.20354@groups.io>
  1 sibling, 0 replies; 12+ messages in thread
From: Chao Li @ 2024-03-26  1:32 UTC (permalink / raw)
  To: devel, ray.ni, Gerd Hoffmann
  Cc: Kumar, Rahul R, Sami Mujawar, Sunil V L, Bibo Mao, Dongyan Qian

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

Hi Ray,

I responded your comments yesterday, it looks like we have different 
opinions on patches 10 and 11, the rest looks fine.

Could you please consider patches 10 and 11? I think this way is 
probably the best solution. I hope this series to be merged as soon as 
passable, because the next series is depedent on this series. :)

Hope to hear back from you!


Thanks,
Chao
On 2024/3/25 11:10, Chao Li wrote:
>
> Hi Ray,
>
> Thank you for reviewing my patch series very carefully.
>
> Here are my comments:
>
> On 2024/3/25 10:46, Ni, Ray wrote:
>> Chao,
>> Thank you for your patience for preparing the new version of patches.
>> However, I still have following minor comments:
>>
>> For patches 1~6: Reviewed-by: Ray Ni <ray.ni@intel.com>
>> For patches 7: can you define meaning of bits in the Attributes/Mask? 
>> It seems you are reusing the definitions defined for 7.2.3 
>> EFI_BOOT_SERVICES.GetMemoryMap(). It's fine. But please mention that 
>> in comments. Also please use UINT64 for Attributes instead of UINTN.
> Yes, I'm reusing the definitions defined for 7.2.3, I will illustrate 
> in the comments next time. About the UINTN, I will fix it next time.
>> For patches 8: Can you rename PcdCpuExceptionVectorBaseAddress to 
>> PcdLoongArch64ExceptionVectorBaseAddress and put the PCD 
>> definition/reference in the DEC/INF LoongArch64 section?
> OK, I will rename it next time.
>> For patches 9: Please make accordingly changes when you address 
>> comments for patch 7.
> OK.
>> 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>
>> For patch 13: Please make accordingly changes when you address 
>> comments for patch 8.
> OK.
>>
>> Thanks,
>> Ray
>> ------------------------------------------------------------------------
>> *From:* Gerd Hoffmann <kraxel@redhat.com>
>> *Sent:* Friday, March 22, 2024 20:39
>> *To:* Chao Li <lichao@loongson.cn>
>> *Cc:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray 
>> <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Sami 
>> Mujawar <sami.mujawar@arm.com>; Sunil V L <sunilvl@ventanamicro.com>; 
>> Bibo Mao <maobibo@loongson.cn>; Dongyan Qian <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.
>> >
>> > 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 <kraxel@redhat.com>
>>
>> take care,
>>   Gerd
>>
> 


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
       [not found]       ` <17C02C7EE39BF604.20354@groups.io>
@ 2024-03-29  1:28         ` Chao Li
  2024-04-09  2:06           ` Ni, Ray
       [not found]         ` <17C1180424B48FA9.19344@groups.io>
  1 sibling, 1 reply; 12+ messages in thread
From: Chao Li @ 2024-03-29  1:28 UTC (permalink / raw)
  To: devel, ray.ni, Gerd Hoffmann
  Cc: Kumar, Rahul R, Sami Mujawar, Sunil V L, Bibo Mao, Dongyan Qian

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

Hi Ray,

I guess you are very busy recently.

If you see this mail, please reply to me, can I can still use the 
low-high level libraries solution in next commit?


Thanks,
Chao
On 2024/3/26 09:32, Chao Li wrote:
>
> Hi Ray,
>
> I responded your comments yesterday, it looks like we have different 
> opinions on patches 10 and 11, the rest looks fine.
>
> Could you please consider patches 10 and 11? I think this way is 
> probably the best solution. I hope this series to be merged as soon as 
> passable, because the next series is depedent on this series. :)
>
> Hope to hear back from you!
>
> On 2024/3/25 11:10, Chao Li wrote:
>>
>> Hi Ray,
>>
>> Thank you for reviewing my patch series very carefully.
>>
>> Here are my comments:
>>
>> On 2024/3/25 10:46, Ni, Ray wrote:
>>> Chao,
>>> Thank you for your patience for preparing the new version of patches.
>>> However, I still have following minor comments:
>>>
>>> For patches 1~6: Reviewed-by: Ray Ni <ray.ni@intel.com>
>>> For patches 7: can you define meaning of bits in the 
>>> Attributes/Mask? It seems you are reusing the definitions defined 
>>> for 7.2.3 EFI_BOOT_SERVICES.GetMemoryMap(). It's fine. But please 
>>> mention that in comments. Also please use UINT64 for Attributes 
>>> instead of UINTN.
>> Yes, I'm reusing the definitions defined for 7.2.3, I will illustrate 
>> in the comments next time. About the UINTN, I will fix it next time.
>>> For patches 8: Can you rename PcdCpuExceptionVectorBaseAddress to 
>>> PcdLoongArch64ExceptionVectorBaseAddress and put the PCD 
>>> definition/reference in the DEC/INF LoongArch64 section?
>> OK, I will rename it next time.
>>> For patches 9: Please make accordingly changes when you address 
>>> comments for patch 7.
>> OK.
>>> 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>
>>> For patch 13: Please make accordingly changes when you address 
>>> comments for patch 8.
>> OK.
>>>
>>> Thanks,
>>> Ray
>>> ------------------------------------------------------------------------
>>> *From:* Gerd Hoffmann <kraxel@redhat.com>
>>> *Sent:* Friday, March 22, 2024 20:39
>>> *To:* Chao Li <lichao@loongson.cn>
>>> *Cc:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray 
>>> <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Sami 
>>> Mujawar <sami.mujawar@arm.com>; Sunil V L 
>>> <sunilvl@ventanamicro.com>; Bibo Mao <maobibo@loongson.cn>; Dongyan 
>>> Qian <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.
>>> >
>>> > 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 <kraxel@redhat.com>
>>>
>>> take care,
>>>   Gerd
>>>
> 


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
       [not found]         ` <17C1180424B48FA9.19344@groups.io>
@ 2024-04-03  1:21           ` Chao Li
  0 siblings, 0 replies; 12+ messages in thread
From: Chao Li @ 2024-04-03  1:21 UTC (permalink / raw)
  To: devel, ray.ni, Gerd Hoffmann
  Cc: Kumar, Rahul R, Sami Mujawar, Sunil V L, Bibo Mao, Dongyan Qian

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

Hi Ray,

I'd love to hear back from you, and ask for any more suggestions. Thank 
you !


Thanks,
Chao
On 2024/3/29 09:28, Chao Li wrote:
>
> Hi Ray,
>
> I guess you are very busy recently.
>
> If you see this mail, please reply to me, can I can still use the 
> low-high level libraries solution in next commit?
>
> On 2024/3/26 09:32, Chao Li wrote:
>>
>> Hi Ray,
>>
>> I responded your comments yesterday, it looks like we have different 
>> opinions on patches 10 and 11, the rest looks fine.
>>
>> Could you please consider patches 10 and 11? I think this way is 
>> probably the best solution. I hope this series to be merged as soon 
>> as passable, because the next series is depedent on this series. :)
>>
>> Hope to hear back from you!
>>
>> On 2024/3/25 11:10, Chao Li wrote:
>>>
>>> Hi Ray,
>>>
>>> Thank you for reviewing my patch series very carefully.
>>>
>>> Here are my comments:
>>>
>>> On 2024/3/25 10:46, Ni, Ray wrote:
>>>> Chao,
>>>> Thank you for your patience for preparing the new version of patches.
>>>> However, I still have following minor comments:
>>>>
>>>> For patches 1~6: Reviewed-by: Ray Ni <ray.ni@intel.com>
>>>> For patches 7: can you define meaning of bits in the 
>>>> Attributes/Mask? It seems you are reusing the definitions defined 
>>>> for 7.2.3 EFI_BOOT_SERVICES.GetMemoryMap(). It's fine. But please 
>>>> mention that in comments. Also please use UINT64 for Attributes 
>>>> instead of UINTN.
>>> Yes, I'm reusing the definitions defined for 7.2.3, I will 
>>> illustrate in the comments next time. About the UINTN, I will fix it 
>>> next time.
>>>> For patches 8: Can you rename PcdCpuExceptionVectorBaseAddress to 
>>>> PcdLoongArch64ExceptionVectorBaseAddress and put the PCD 
>>>> definition/reference in the DEC/INF LoongArch64 section?
>>> OK, I will rename it next time.
>>>> For patches 9: Please make accordingly changes when you address 
>>>> comments for patch 7.
>>> OK.
>>>> 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>
>>>> For patch 13: Please make accordingly changes when you address 
>>>> comments for patch 8.
>>> OK.
>>>>
>>>> Thanks,
>>>> Ray
>>>> ------------------------------------------------------------------------
>>>> *From:* Gerd Hoffmann <kraxel@redhat.com>
>>>> *Sent:* Friday, March 22, 2024 20:39
>>>> *To:* Chao Li <lichao@loongson.cn>
>>>> *Cc:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray 
>>>> <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Sami 
>>>> Mujawar <sami.mujawar@arm.com>; Sunil V L 
>>>> <sunilvl@ventanamicro.com>; Bibo Mao <maobibo@loongson.cn>; Dongyan 
>>>> Qian <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.
>>>> >
>>>> > 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 <kraxel@redhat.com>
>>>>
>>>> take care,
>>>>   Gerd
>>>>
> 


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
  2024-03-29  1:28         ` Chao Li
@ 2024-04-09  2:06           ` Ni, Ray
  2024-04-09  4:29             ` Chao Li
  0 siblings, 1 reply; 12+ messages in thread
From: Ni, Ray @ 2024-04-09  2:06 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gerd Hoffmann, lichao@loongson.cn
  Cc: Kumar, Rahul R, Sami Mujawar, Sunil V L, Bibo Mao, Dongyan Qian

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

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.
>
> 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 <kraxel@redhat.com><mailto:kraxel@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/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: 11091 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
  2024-04-09  2:06           ` Ni, Ray
@ 2024-04-09  4:29             ` Chao Li
  2024-04-09  5:27               ` Ni, Ray
  0 siblings, 1 reply; 12+ messages in thread
From: Chao Li @ 2024-04-09  4:29 UTC (permalink / raw)
  To: devel, ray.ni, Gerd Hoffmann
  Cc: Kumar, Rahul R, Sami Mujawar, Sunil V L, Bibo Mao, Dongyan Qian

[-- 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 --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
  2024-04-09  4:29             ` Chao Li
@ 2024-04-09  5:27               ` Ni, Ray
  2024-04-09  6:21                 ` Chao Li
  0 siblings, 1 reply; 12+ messages in thread
From: Ni, Ray @ 2024-04-09  5:27 UTC (permalink / raw)
  To: Chao Li, devel@edk2.groups.io, Gerd Hoffmann
  Cc: Kumar, Rahul R, Sami Mujawar, Sunil V L, Bibo Mao, Dongyan Qian

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

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 <lichao@loongson.cn>
Sent: Tuesday, April 9, 2024 12:29
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
Cc: Kumar, Rahul R <rahul.r.kumar@intel.com>; Sami Mujawar <sami.mujawar@arm.com>; Sunil V L <sunilvl@ventanamicro.com>; Bibo Mao <maobibo@loongson.cn>; Dongyan Qian <qiandongyan@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.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.
>
> 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 <kraxel@redhat.com><mailto:kraxel@redhat.com>

take care,
  Gerd




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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg
  2024-04-09  5:27               ` Ni, Ray
@ 2024-04-09  6:21                 ` Chao Li
  0 siblings, 0 replies; 12+ messages in thread
From: Chao Li @ 2024-04-09  6:21 UTC (permalink / raw)
  To: devel, ray.ni, Gerd Hoffmann
  Cc: Kumar, Rahul R, Sami Mujawar, Sunil V L, Bibo Mao, Dongyan Qian

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

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 <lichao@loongson.cn>
> *Sent:* Tuesday, April 9, 2024 12:29
> *To:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray 
> <ray.ni@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> *Cc:* Kumar, Rahul R <rahul.r.kumar@intel.com>; Sami Mujawar 
> <sami.mujawar@arm.com>; Sunil V L <sunilvl@ventanamicro.com>; Bibo Mao 
> <maobibo@loongson.cn>; Dongyan Qian <qiandongyan@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.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 (#117537): https://edk2.groups.io/g/devel/message/117537
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: 20989 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-04-09  6:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <17BE6C7171CBC84C.24580@groups.io>
2024-03-22  1:10 ` [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg Chao Li
2024-03-20  8:41 Chao Li
2024-03-22 12:39 ` Gerd Hoffmann
2024-03-25  2:46   ` Ni, Ray
2024-03-25  3:10     ` Chao Li
     [not found]     ` <17BFE34457098413.21233@groups.io>
2024-03-26  1:32       ` Chao Li
     [not found]       ` <17C02C7EE39BF604.20354@groups.io>
2024-03-29  1:28         ` Chao Li
2024-04-09  2:06           ` Ni, Ray
2024-04-09  4:29             ` Chao Li
2024-04-09  5:27               ` Ni, Ray
2024-04-09  6:21                 ` Chao Li
     [not found]         ` <17C1180424B48FA9.19344@groups.io>
2024-04-03  1:21           ` Chao Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox