Re: [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init

["Marcin Juszkiewicz" <marcin.juszkiewicz@linaro.org>]

W dniu 14.03.2024 o 16:13, Ard Biesheuvel pisze:
>> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
>> @@ -1,7 +1,7 @@
>>   ## @file
>>   #  This driver effectuates SbsaQemu platform configuration settings
>>   #
>> -#  Copyright (c) 2019, Linaro Ltd. All rights reserved.
>> +#  Copyright (c) Linaro Ltd. All rights reserved.
>>   #
>>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>>   #
>> @@ -32,6 +32,7 @@ [LibraryClasses]
>>     PcdLib
>>     DebugLib
>>     NonDiscoverableDeviceRegistrationLib
>> +  SbsaQemuHardwareInfoLib
>>     UefiDriverEntryPoint
>>
>>   [Pcd]
>> @@ -46,6 +47,7 @@ [Pcd]
>>     gArmTokenSpaceGuid.PcdGicDistributorBase
>>     gArmTokenSpaceGuid.PcdGicRedistributorsBase
>>     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
>> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
>>
>>
>>   [Depex]

> How is it guaranteed that other components will only see the correct
> core count? DXE dispatch is ordered using a dependency graph, so all
> users of this PCD should never execute before this driver.

SbsaQemuPlatformDxe is a DXE, right? So it is called on platform init.

At the end of initialization it calls SbsaQemuGetCpuCount() from 
SbsaQemuHardwareInfoLib to SET this PCD. It does not use it during 
platform init cause it does not require this information. But calls 
function to make sure that amount of cpus is known to whatever will be 
called later.

Sure, maybe SbsaQemuHardwareInfoLib should be something else (DXE, 
Protocol or other EDK2 magic thing) but it is set of functions to be 
called from other places of EDK2.

> This is why PCDs suck for dynamic information, to be honest. Much
> better to use a protocol (DEPEXes declare dependencies on protocols,
> so a driver will never run before the protocols it depends on have
> been made available)

I am still learning. Will look at other platforms.

> Given that this is intended as reference code, I think it is very
> important to get this right.

Fully agree.


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