From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id CC8A5D8017E for ; Tue, 9 Apr 2024 04:29:43 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=1fLDXm73U11Sjs02VOJFC9x6AtA97ckZI05rKrVCPgw=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20240206; t=1712636982; v=1; b=rWNkw62QwhzA/qFsmZhj17v8Mb+Hn9iUEhEeTKh7xNm9RMQoD4PlsP0OqvmE7LUtZvw8HVZW NPFPzKYn6/dNaCQNXQJuyeN8shvCLs6yj4MKSvjyi2W+WCaFRF5uMG8TPaf0L/YezMPSNMYsRxt 340XZJYT04jYJRazVC2SAOCyEDb6+N08160icTDHErtD+eMB4vBALs0vA8PCtt9A8tmCKplTQmk kdH3ZDXsL1igZ/gnvhzdY6gBeA5yq4V/g2bFUoijWbObO9RqaoCJiIG2fOWXV4zZ953lektumQc ebVKKesr7gZ66giTke9JaNBsfCYWbiCd0HTly2jIEhGqg== X-Received: by 127.0.0.2 with SMTP id 3mVYYY7687511xImgApiXDaW; Mon, 08 Apr 2024 21:29:42 -0700 X-Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by mx.groups.io with SMTP id smtpd.web11.128904.1712636980431823183 for ; Mon, 08 Apr 2024 21:29:41 -0700 X-Received: from loongson.cn (unknown [10.40.24.149]) by gateway (Coremail) with SMTP id _____8Ax++gsxBRmObMkAA--.64578S3; Tue, 09 Apr 2024 12:29:32 +0800 (CST) X-Received: from [10.40.24.149] (unknown [10.40.24.149]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Bx1xEmxBRmYC12AA--.20072S3; Tue, 09 Apr 2024 12:29:26 +0800 (CST) Message-ID: <1ed0abe7-0fd7-400c-81b9-d0f4609901a8@loongson.cn> Date: Tue, 9 Apr 2024 12:29:26 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v2 00/13] Part 2 patch set to add LoongArch support into UefiCpuPkg To: devel@edk2.groups.io, ray.ni@intel.com, Gerd Hoffmann Cc: "Kumar, Rahul R" , Sami Mujawar , Sunil V L , Bibo Mao , Dongyan Qian References: <20240320084152.268323-1-lichao@loongson.cn> <17BFE34457098413.21233@groups.io> <17C02C7EE39BF604.20354@groups.io> <9972bb2c-0790-4879-89f3-946574f0d802@loongson.cn> From: "Chao Li" In-Reply-To: X-CM-TRANSID: AQAAf8Bx1xEmxBRmYC12AA--.20072S3 X-CM-SenderInfo: xolfxt3r6o00pqjv00gofq/1tbiAQACCGYTquoISgABs9 X-Coremail-Antispam: 1Uk129KBjDUn29KB7ZKAUJUUUUU529EdanIXcx71UUUUU7KY7 ZEXasCq-sGcSsGvfJ3UbIjqfuFe4nvWSU5nxnvy29KBjDU0xBIdaVrnUUvcSsGvfC2Kfnx nUUI43ZEXa7xR_UUUUUUUUU== Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Mon, 08 Apr 2024 21:29:41 -0700 Resent-From: lichao@loongson.cn Reply-To: devel@edk2.groups.io,lichao@loongson.cn List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: uR22o89HUkat2tVxL1RXhXIpx7686176AA= Content-Type: multipart/alternative; boundary="------------UUHlfB18rPDjPIQI0nz6Jtpj" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=rWNkw62Q; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=none --------------UUHlfB18rPDjPIQI0nz6Jtpj Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 > > For patch 13: Please make accordingly changes when you > address comments for patch 8. > > OK. > > > Thanks, > Ray > ------------------------------------------------------------------------ > *From:* Gerd Hoffmann > > *Sent:* Friday, March 22, 2024 20:39 > *To:* Chao Li > *Cc:* devel@edk2.groups.io > ; Ni, > Ray ; Kumar, > Rahul R > ; Sami Mujawar > ; > Sunil V L > ; Bibo Mao > ; > Dongyan Qian > > *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 > > > 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] -=-=-=-=-=-=-=-=-=-=-=- --------------UUHlfB18rPDjPIQI0nz6Jtpj Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

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>
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
>
>
> 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 (#117518) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------UUHlfB18rPDjPIQI0nz6Jtpj--