Re: [edk2-devel] [PATCH v7 25/37] ArmVirtPkg: Move PlatformBootManagerLib to OvmfPkg

["Chao Li" <lichao@loongson.cn>]

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

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.
View/Reply Online (#113888): https://edk2.groups.io/g/devel/message/113888
Mute This Topic: https://groups.io/mt/103679477/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: 11433 bytes --]