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

["Ard Biesheuvel" <ardb@kernel.org>]

On Wed, 6 Mar 2024 at 12:42, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> We read it once and store in Pcd for future use.
>
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>  .../SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf | 4 +++-
>  .../SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c   | 9 +++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> index 19534b7a274a..9752694a432b 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> +++ 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.

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)

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


> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> index 4ebbe7c93a19..14e1ec7eab29 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> @@ -1,7 +1,7 @@
>  /** @file
> -*  FDT client protocol driver for qemu,mach-virt-ahci DT node
> +*  SbsaQemu Platform Initialization
>  *
> -*  Copyright (c) 2019, Linaro Ltd. All rights reserved.
> +*  Copyright (c) Linaro Ltd. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -12,13 +12,12 @@
>  #include <Library/DebugLib.h>
>  #include <Library/NonDiscoverableDeviceRegistrationLib.h>
>  #include <Library/PcdLib.h>
> +#include <Library/SbsaQemuHardwareInfoLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiDriverEntryPoint.h>
>  #include <IndustryStandard/SbsaQemuSmc.h>
>  #include <IndustryStandard/SbsaQemuPlatformVersion.h>
>
> -#include <Protocol/FdtClient.h>
> -
>  EFI_STATUS
>  EFIAPI
>  InitializeSbsaQemuPlatformDxe (
> @@ -123,5 +122,7 @@ InitializeSbsaQemuPlatformDxe (
>      }
>    }
>
> +  SbsaQemuGetCpuCount ();
> +
>    return EFI_SUCCESS;
>  }
>
> --
> 2.44.0
>


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