Re: [edk2-devel] [PATCH v6 24/36] ArmVirtPkg: Move PlatformBootManagerLib to OvmfPkg

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

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

Hi Laszlo,


Thanks,
Chao
On 2024/1/8 22:02, Laszlo Ersek wrote:
> On 1/5/24 10:45, 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
>>   .../PlatformBootManagerLib.inf                             | 7 +++----
>>   .../Library/PlatformBootManagerLibLight}/QemuKernel.c      | 0
>>   OvmfPkg/OvmfPkg.dec                                        | 4 ++++
>>   9 files changed, 9 insertions(+), 8 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
> You don't seem to be reinstating this under OvmfPkg, so I think the same source file under OvmfPkg will cause a CI failure.

There was no CI failure, there was a PR for edk2 in the cover letter, 
all of the CI are passed.

https://github.com/tianocore/edk2/pull/5208

>
>> 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 } }
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index a03c30995b..2ed7863a98 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -144,6 +144,10 @@
>>     #
>>     HardwareInfoLib|Include/Library/HardwareInfoLib.h
>>   
>> +  ##  @libraryclass  FdtSerialPortAddressLib
>> +  #
>> +  FdtSerialPortAddressLib|Include/Library/FdtSerialPortAddressLib.h
>> +
>>   [Guids]
>>     gUefiOvmfPkgTokenSpaceGuid            = {0x93bb96af, 0xb9f2, 0x4eb8, {0x94, 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}}
>>     gEfiXenInfoGuid                       = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
> These two hunks don't seem to belong in this patch -- I think they might belong to patch 22, "ArmVirtPkg: Move the FdtSerialPortAddressLib to OvmfPkg"; is that right?
>
> Also, the lib classes in the [LibraryClasses] section of the DEC file was originally meant to be lexicographically sorted. Over time, soring errors got introduced; it would be nice to restore the sort order in a separate patch. (Although it's not a pre-requisite for this patch set to be accepted, I guess.)
You are right, this change should belong to patch 22, in V7 I would sort 
it lexicographically.
>
>
>> 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
>> 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
>> 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
>> 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
>> 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 1ddcd107d2..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]
>> @@ -29,7 +29,6 @@
>>     QemuKernel.c
>>   
>>   [Packages]
>> -  ArmVirtPkg/ArmVirtPkg.dec
>>     MdeModulePkg/MdeModulePkg.dec
>>     MdePkg/MdePkg.dec
>>     OvmfPkg/OvmfPkg.dec
>> @@ -56,15 +55,15 @@
>>     UefiRuntimeServicesTableLib
>>   
>>   [FixedPcd]
>> -  gUefiOvmfPkgTokenSpaceGuid.PcdUninstallMemAttrProtocol
>>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
>>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
>>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdUninstallMemAttrProtocol
>>   
>>   [Pcd]
>> -  gUefiOvmfPkgTokenSpaceGuid.PcdTerminalTypeGuidBuffer
>>     gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdTerminalTypeGuidBuffer
>>   
>>   [Guids]
>>     gEfiEndOfDxeEventGroupGuid
>> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c b/OvmfPkg/Library/PlatformBootManagerLibLight/QemuKernel.c
> These last few hunks seem to restore sorting order (is that right?), which is welcome, but probably belong in a separate patch.
Yes, the sort orders should appear in the patch 23, I will adjust them 
in V7.
>
>> similarity index 100%
>> rename from ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
>> rename to OvmfPkg/Library/PlatformBootManagerLibLight/QemuKernel.c
> Looks OK to me otherwise.
>
> Thanks
> Laszlo
>
>
>
> 
>


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