Hi Laszlo,

Thanks,
Chao

On 2024/1/15 16:46, Laszlo Ersek wrote:

On 1/12/24 09:25, Chao Li wrote:

Moved the PlatformBootManagerLib to OvmfPkg and renamed to
PlatformBootManagerLibLight for easy use by other ARCH.

Build-tested only (with "ArmVirtQemu.dsc").

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4584

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Lazlo Ersek <lersek@redhat.com>
Signed-off-by: Chao Li <lichao@loongson.cn>
---
 ArmVirtPkg/ArmVirtPkg.ci.yaml                                  | 1 -
 ArmVirtPkg/ArmVirtPkg.dec                                      | 1 -
 ArmVirtPkg/ArmVirtQemu.dsc                                     | 2 +-
 ArmVirtPkg/ArmVirtQemuKernel.dsc                               | 2 +-
 .../Library/PlatformBootManagerLibLight}/PlatformBm.c          | 0
 .../Library/PlatformBootManagerLibLight}/PlatformBm.h          | 0
 .../PlatformBootManagerLibLight}/PlatformBootManagerLib.inf    | 3 +--
 .../Library/PlatformBootManagerLibLight}/QemuKernel.c          | 0
 8 files changed, 3 insertions(+), 6 deletions(-)
 rename {ArmVirtPkg/Library/PlatformBootManagerLib => OvmfPkg/Library/PlatformBootManagerLibLight}/PlatformBm.c (100%)
 rename {ArmVirtPkg/Library/PlatformBootManagerLib => OvmfPkg/Library/PlatformBootManagerLibLight}/PlatformBm.h (100%)
 rename {ArmVirtPkg/Library/PlatformBootManagerLib => OvmfPkg/Library/PlatformBootManagerLibLight}/PlatformBootManagerLib.inf (92%)
 rename {ArmVirtPkg/Library/PlatformBootManagerLib => OvmfPkg/Library/PlatformBootManagerLibLight}/QemuKernel.c (100%)

diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
index 506b0e72f0..b186d4eb42 100644
--- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
+++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
@@ -24,7 +24,6 @@
         ],
         ## Both file path and directory path are accepted.
         "IgnoreFiles": [
-            "Library/PlatformBootManagerLib/PlatformBm.c"
         ]
     },
     ## options defined .pytool/Plugin/CompilerPlugin

OK

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 315db4e8ea..6aa5ea05f4 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -27,7 +27,6 @@
 
 [LibraryClasses]
   ArmVirtMemInfoLib|Include/Library/ArmVirtMemInfoLib.h
-  FdtSerialPortAddressLib|Include/Library/FdtSerialPortAddressLib.h
 
 [Guids.common]
   gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 0x36, 0x5B, 0x80, 0x63, 0x66 } }

This hunk belongs to patch "ArmVirtPkg: Move the FdtSerialPortAddressLib
to OvmfPkg".

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 147180f645..e48c75b5e9 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -70,7 +70,7 @@
 
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
-  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBootManagerLib.inf
   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

OK

diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index c22a422353..668a65ba64 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -69,7 +69,7 @@
 
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
-  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBootManagerLib.inf
   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

OK

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBm.c
similarity index 100%
rename from ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
rename to OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBm.c

OK

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.h b/OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBm.h
similarity index 100%
rename from ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.h
rename to OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBm.h

OK

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBootManagerLib.inf
similarity index 92%
rename from ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
rename to OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBootManagerLib.inf
index 5bbd13aecb..f2fb69bd3c 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBootManagerLib.inf
@@ -20,7 +20,7 @@
 #
 # The following information is for reference only and not required by the build tools.
 #
-#  VALID_ARCHITECTURES           = ARM AARCH64
+#  VALID_ARCHITECTURES           = ARM AARCH64 LOONGARCH64
 #
 
 [Sources]

OK

@@ -29,7 +29,6 @@
   QemuKernel.c
 
 [Packages]
-  ArmVirtPkg/ArmVirtPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec

Hmmm. This makes me wonder.

If we can remove the ArmVirtPkg package dependency from the lib instance
in *this patch*, then we should be able to remove it *earlier* too
(i.e., independently), while the lib instance still exists under ArmVirtPkg.

I don't see why the "ArmVirtPkg.dec" dep becomes superfluous *right here*.

If I look at this INF file, as of commit 4a443f73fd67, I see at least
two "ArmVirtPkg.dec" dependencies:

[FixedPcd]
  gArmVirtTokenSpaceGuid.PcdUninstallMemAttrProtocol

[Pcd]
  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer

In patch 24 ("ArmVirtPkg: Move two PCD variables into OvmfPkg"), you
move these PCDs to OvmfPkg.

Ah, I understand now. In brief: this particular hunk belongs to patch 24
(where you correctly modify "PlatformBootManagerLib.inf" anyway). The
point is that, with the movement of both PCDs from the ArmVirt token
space to the OVMF token space, "PlatformBootManagerLib.inf"'s dependency
on "ArmVirtPkg.dec" disappears. Therefore the above hunk belongs to
patch 24.

... When you implement that, please build-test both patches 24 and 25.

More precisely, your patch set should build at every stage, considering
both ArmVirt and OVMF platforms.

The command "git rebase --exec" is useful for building a series at every
stage.

Do you means this change should belong in patch 24 is better?

BTW, I did build and tested after applying patches 24 and 25(building and testing with ArmVirtQemu.dec) and it works fine.

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c b/OvmfPkg/Library/PlatformBootManagerLibLight/QemuKernel.c
similarity index 100%
rename from ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
rename to OvmfPkg/Library/PlatformBootManagerLibLight/QemuKernel.c

OK

Laszlo

_._,_._,_

Groups.io Links:

You receive all messages sent to this group.

_._,_._,_