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 6D435D800C4 for ; Wed, 15 Nov 2023 03:21:38 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=p/nBLAgQhqEfTYnFwYNfsfpXwB3Hl3r5gNDx3UpWEAk=; 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:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1700018497; v=1; b=FOpiLCrgU9GZ5CdJEcmTwTqSmD3tgmpF+ZBNlpWsymBgs7UE19Nqbk0K8XFLreSCvAgM0hu1 t/mGGRASLGtUlAfzFqABtNxS4in7vJfsz//3W/INZEHUlOw+8UIbHp9rDIMSydYhbfaKFftiGtH lwoGk5rhznA1ZagYyqgIs8WE= X-Received: by 127.0.0.2 with SMTP id LBbnYY7687511x2r8O1q4OGb; Tue, 14 Nov 2023 19:21:37 -0800 X-Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by mx.groups.io with SMTP id smtpd.web11.5111.1700018493126721350 for ; Tue, 14 Nov 2023 19:21:36 -0800 X-Received: from loongson.cn (unknown [10.40.24.149]) by gateway (Coremail) with SMTP id _____8AxlPAzOVRlMys6AA--.49093S3; Wed, 15 Nov 2023 11:21:24 +0800 (CST) X-Received: from [10.40.24.149] (unknown [10.40.24.149]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Dxnd4vOVRlcqNCAA--.16551S3; Wed, 15 Nov 2023 11:21:19 +0800 (CST) Message-ID: Date: Wed, 15 Nov 2023 11:21:19 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v2 24/30] OvmfPkg/LoongArchVirt: Add platform boot manager library To: devel@edk2.groups.io, lersek@redhat.com, Gerd Hoffmann Cc: Ard Biesheuvel , Jiewen Yao , Jordan Justen , Xianglai Li , Bibo Mao References: <20231106032521.2251143-1-lichao@loongson.cn> <20231106033021.2294243-1-lichao@loongson.cn> <0e2c2f4d-c559-f9f9-881a-143eec02de4d@redhat.com> <3e97f949-1ad3-4685-901f-945d8101a7df@loongson.cn> <7cc254fe-ac77-ea93-2c8d-d9ac2549da69@redhat.com> From: "Chao Li" In-Reply-To: <7cc254fe-ac77-ea93-2c8d-d9ac2549da69@redhat.com> X-CM-TRANSID: AQAAf8Dxnd4vOVRlcqNCAA--.16551S3 X-CM-SenderInfo: xolfxt3r6o00pqjv00gofq/1tbiAQAQCGVUKqkA8QACsY X-Coremail-Antispam: 1Uk129KBj93XoWxZw43Cr4UJF1rGrWDCF4DAwc_yoW5AryDpr W8JrWfCr1kKr1FvF18J34jqayFvwsIyFs8Jr98AF9Yka1DJF4F9ryjgrya93sxArs5Aayq vr4Yvw1UuFsYgwcCm3ZEXasCq-sJn29KB7ZKAUJUUUU5529EdanIXcx71UUUUU7KY7ZEXa sCq-sGcSsGvfJ3UbIjqfuFe4nvWSU5nxnvy29KBjDU0xBIdaVrnRJUUUylb4IE77IF4wAF F20E14v26r1j6r4UM7CY07I20VC2zVCF04k26cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r 106r15M28lY4IEw2IIxxk0rwA2F7IY1VAKz4vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAF wI0_JFI_Gr1l84ACjcxK6xIIjxv20xvEc7CjxVAFwI0_Jr0_Gr1l84ACjcxK6I8E87Iv67 AKxVWxJVW8Jr1l84ACjcxK6I8E87Iv6xkF7I0E14v26r4j6r4UJwAS0I0E0xvYzxvE52x0 82IY62kv0487Mc804VCY07AIYIkI8VC2zVCFFI0UMcIj6xIIjxv20xvE14v26r1Y6r17Mc Ij6I8E87Iv67AKxVWUJVW8JwAm72CE4IkC6x0Yz7v_Jr0_Gr1lF7xvr2IY64vIr41l7480 Y4vEI4kI2Ix0rVAqx4xJMxAIw28IcxkI7VAKI48JMxC20s026xCaFVCjc4AY6r1j6r4UMI 8I3I0E5I8CrVAFwI0_JrI_JrWlx2IqxVCjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AK xVWUtVW8ZwCIc40Y0x0EwIxGrwCI42IY6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI 8IcVCY1x0267AKxVWUJVW8JwCI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280 aVAFwI0_Jr0_Gr1lIxAIcVC2z280aVCY1x0267AKxVWUJVW8JbIYCTnIWIevJa73UjIFyT uYvjxUw9a9DUUUU 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 Reply-To: devel@edk2.groups.io,lichao@loongson.cn List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: XDH3ta73NWz6qGbOVo08DYpPx7686176AA= Content-Type: multipart/alternative; boundary="------------QeNqVdfONerxXoQXTGZQru4Y" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=FOpiLCrg; 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 --------------QeNqVdfONerxXoQXTGZQru4Y Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Well, let's discuss the ARM version name once moved. I have two plans: *Plan A:* Merge the ARM version into PlatformBootmanagerLib and modify the inf file to separate X64 and other ARCH, like be: [Sources.Common]     QemuKernel.c     BdsPlatform.h [Sources.X64, Sources.IA32]     BdsPlatform.c     PaltformData.c [Sources.ARM, Sources.AARCH64, Sources.RISCV64, Source.LOONGARCH64]     PlatformBm.c [LibraryClasses.Common]     BaseLib     BaseMemoryLib     BootLogoLib     DebugLib     DevicePathLib     MemoryAllocationLib     PcdLib     ... [LibraryClasses.X64, LibraryClasses.IA32]     QemuFwCfgLib     QemuFwCfgS3Lib     ... [LibraryClasses.ARM, LibraryClasses.AARCH64, LibraryClasses.RISCV, LibraryClasses.LOONGARCH64]     TpmPlatformHierarchyLib     ... The above pseudocode are unverified and will definitely be subject to modification. *Plan B:* Moved the ARM version into OvmfPkg and got a new name. In my opinion, the X86 version takes into account the STR, Tcg2 and Xen platform, so it look like more complete(only for X86, just my opinion). In this case, I think what about  the X86 version still being named PlatformBootManagerLib and the ARM version being named PlatformBootManagerLibLight? Both of the above plans can achieve the goal. I prefer Plan A. I want to know your opinions, so hope hear back from you! Thanks, Chao On 2023/11/13 19:08, Laszlo Ersek wrote: > On 11/10/23 10:46, Gerd Hoffmann wrote: >> On Fri, Nov 10, 2023 at 03:09:47PM +0800, Chao Li wrote: >>> Hi Laszlo, >>> >>> Sorry, I'm not check carefully, it is really **copied**, and we not think >>> the ARM version is not good enough. >>> >>> So, can I move this library to OvmfPkg so other ARCH use it easily? >> Moving code from ArmVirtPkg to OvmfPkg is fine. >> >> OvmfPkg is the home for both x86 virtual machine bits and shared code. >> The later used to be mostly virtio drivers, but with the arrival of >> riscv some fdt support code has already moved from ArmVirtPkg to OvmfPkg >> so arm and riscv can share it. Doing the same for loongarch is >> perfectly fine. > Agreed! > > The naming of course remains tricky. :) It's not easy to come up with > good names for distinguishing various instances of the same library class. > > I suggest renaming "OvmfPkg/PlatformBootManagerLib" to > "PlatformBootManagerLibX86", and calling ArmVirtPkg's instance (once > moved) PlatformBootManagerLibGeneric or just PlatformBootManagerLib. > > Laszlo > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111232): https://edk2.groups.io/g/devel/message/111232 Mute This Topic: https://groups.io/mt/102413902/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- --------------QeNqVdfONerxXoQXTGZQru4Y Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Well, let's discuss the ARM version name once moved.

I have two plans:

Plan A:

Merge the ARM version into PlatformBootmanagerLib and modify the inf file to separate X64 and other ARCH, like be:

[Sources.Common]

    QemuKernel.c

    BdsPlatform.h

[Sources.X64, Sources.IA32]

    BdsPlatform.c

    PaltformData.c

[Sources.ARM, Sources.AARCH64, Sources.RISCV64, Source.LOONGARCH64]

    PlatformBm.c

[LibraryClasses.Common]

    BaseLib

    BaseMemoryLib

    BootLogoLib

    DebugLib

    DevicePathLib

    MemoryAllocationLib

    PcdLib

    ...

[LibraryClasses.X64, LibraryClasses.IA32]

    QemuFwCfgLib

    QemuFwCfgS3Lib

    ...

[LibraryClasses.ARM, LibraryClasses.AARCH64, LibraryClasses.RISCV, LibraryClasses.LOONGARCH64]

    TpmPlatformHierarchyLib

    ...


The above pseudocode are unverified and will definitely be subject to modification.


Plan B:

Moved the ARM version into OvmfPkg and got a new name. In my opinion, the X86 version takes into account the STR, Tcg2 and Xen platform, so it look like more complete(only for X86, just my opinion). In this case, I think what about  the X86 version still being named PlatformBootManagerLib and the ARM version being named PlatformBootManagerLibLight?


Both of the above plans can achieve the goal. I prefer Plan A. I want to know your opinions, so hope hear back from you!


Thanks,
Chao
On 2023/11/13 19:08, Laszlo Ersek wrote:
On 11/10/23 10:46, Gerd Hoffmann wrote:

On Fri, Nov 10, 2023 at 03:09:47PM +0800, Chao Li wrote:

Hi Laszlo,

Sorry, I'm not check carefully, it is really **copied**, and we not think
the ARM version is not good enough.

So, can I move this library to OvmfPkg so other ARCH use it easily?

Moving code from ArmVirtPkg to OvmfPkg is fine.

OvmfPkg is the home for both x86 virtual machine bits and shared code.
The later used to be mostly virtio drivers, but with the arrival of
riscv some fdt support code has already moved from ArmVirtPkg to OvmfPkg
so arm and riscv can share it.  Doing the same for loongarch is
perfectly fine.

Agreed!

The naming of course remains tricky. :) It's not easy to come up with
good names for distinguishing various instances of the same library class.

I suggest renaming "OvmfPkg/PlatformBootManagerLib" to
"PlatformBootManagerLibX86", and calling ArmVirtPkg's instance (once
moved) PlatformBootManagerLibGeneric or just PlatformBootManagerLib.

Laszlo
_._,_._,_
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#111232) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------QeNqVdfONerxXoQXTGZQru4Y--