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

["Laszlo Ersek" <lersek@redhat.com>]

On 1/16/24 12:54, Chao Li wrote:
> On 2024/1/15 16:46, Laszlo Ersek wrote:
>> On 1/12/24 09:25, Chao Li wrote:

>>> @@ -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?

Yes, please.

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

Thank you.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113906): https://edk2.groups.io/g/devel/message/113906
Mute This Topic: https://groups.io/mt/103679477/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-