From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id B61EF74003A for ; Fri, 19 Jan 2024 19:18:08 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Ku2WjoUyfFPL69mPkCcB1N1yHZNenO6RZT4eHAHtMRY=; c=relaxed/simple; d=groups.io; h=Date:From:To:CC:Subject:Message-ID:References:MIME-Version:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition; s=20140610; t=1705691887; v=1; b=tUUVQjmwEsYOZMmBnFPDwG5WYgDKyMPEFS+Raim/5K9Dfa9jCP6VzhnfKGcRiRECb/O67hZz R+vbnZ/kDRgutq1bdrO+7uSoOlfGQeCKwORo1HzJcUPaL5xAaeri8EZFoNmTjnoj7mX+AaC2E8O BatbQTkJuc+LvRpFw5fJAoLc= X-Received: by 127.0.0.2 with SMTP id kudeYY7687511xxZ2pU81mXw; Fri, 19 Jan 2024 11:18:07 -0800 X-Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by mx.groups.io with SMTP id smtpd.web11.2628.1705691886694570981 for ; Fri, 19 Jan 2024 11:18:06 -0800 X-Received: from pps.filterd (m0279865.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.24/8.17.1.24) with ESMTP id 40JGAnwY013149; Fri, 19 Jan 2024 19:18:06 GMT X-Received: from nasanppmta01.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3vqh2k9sh9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 19 Jan 2024 19:18:06 +0000 (GMT) X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 40JJI5pS023149 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 19 Jan 2024 19:18:05 GMT X-Received: from qc-i7.hemma.eciton.net (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.40; Fri, 19 Jan 2024 11:18:04 -0800 Date: Fri, 19 Jan 2024 19:18:00 +0000 From: "Leif Lindholm" To: Marcin Juszkiewicz CC: , Ard Biesheuvel , Graeme Gregory Subject: Re: [edk2-devel] [PATCH edk2-platforms v2 1/4] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib Message-ID: References: <20240116-no-dt-for-cpu-v2-0-6cf078d9ab76@linaro.org> <20240116-no-dt-for-cpu-v2-1-6cf078d9ab76@linaro.org> MIME-Version: 1.0 In-Reply-To: <20240116-no-dt-for-cpu-v2-1-6cf078d9ab76@linaro.org> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-GUID: AZtZTqbU2LnIrIPbpg7wJlQHRm-dzGrH X-Proofpoint-ORIG-GUID: AZtZTqbU2LnIrIPbpg7wJlQHRm-dzGrH Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,quic_llindhol@quicinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: HsKOuFwjelkfri4Gj6woBF70x7686176AA= Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=tUUVQjmw; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=quicinc.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Tue, Jan 16, 2024 at 08:48:32 +0100, Marcin Juszkiewicz wrote: > This library provides functions to check for hardware information. > For now it covers CPU ones: > > - amount of cpu cores > - MPIDR value for cpu core > - NUMA node id for cpu core > > Values are read from TF-A using platform specific SMC calls. > > Signed-off-by: Marcin Juszkiewicz > --- > Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 3 +- > .../SbsaQemuHardwareInfoLib.inf | 32 +++++++ > .../SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h | 2 + > .../Include/Library/SbsaQemuHardwareInfoLib.h | 45 +++++++++ > .../SbsaQemuHardwareInfoLib.c | 100 ++++++++++++++++++++ > 5 files changed, 181 insertions(+), 1 deletion(-) > > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > index 378600050df9..07cb3490f4cf 100644 > --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > @@ -1,6 +1,6 @@ > # > # Copyright (c) 2021, NUVIA Inc. All rights reserved. > -# Copyright (c) 2019, Linaro Limited. All rights reserved. > +# Copyright (c) 2019-2024, Linaro Ltd. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -128,6 +128,7 @@ [LibraryClasses.common] > > FdtHelperLib|Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf > OemMiscLib|Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf > + SbsaQemuHardwareInfoLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf > > # Debug Support > PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf > diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf > new file mode 100644 > index 000000000000..8c2def1878e6 > --- /dev/null > +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf > @@ -0,0 +1,32 @@ > +#/* @file > +# > +# Copyright (c) Linaro Ltd. All rights reserved. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +#*/ > + > +[Defines] > + INF_VERSION = 0x0001001c > + BASE_NAME = SbsaQemuHardwareInfoLib > + FILE_GUID = 6454006f-6502-46e2-9be4-4bba8d4b29fb > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = ArmPlatformLib > + > +[Sources] > + SbsaQemuHardwareInfoLib.c > + > +[Packages] > + ArmPkg/ArmPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + MdePkg/MdePkg.dec > + Silicon/Qemu/SbsaQemu/SbsaQemu.dec > + > +[LibraryClasses] > + ArmSmcLib > + BaseMemoryLib > + DebugLib > + > + [Pcd] > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount > diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h > index 7934875e4aba..e33648ee1462 100644 > --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h > +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h > @@ -14,5 +14,7 @@ > #define SIP_SVC_VERSION SMC_SIP_FUNCTION_ID(1) > #define SIP_SVC_GET_GIC SMC_SIP_FUNCTION_ID(100) > #define SIP_SVC_GET_GIC_ITS SMC_SIP_FUNCTION_ID(101) > +#define SIP_SVC_GET_CPU_COUNT SMC_SIP_FUNCTION_ID(200) > +#define SIP_SVC_GET_CPU_NODE SMC_SIP_FUNCTION_ID(201) > > #endif /* SBSA_QEMU_SMC_H_ */ > diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h > new file mode 100644 > index 000000000000..45262baf3511 > --- /dev/null > +++ b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h > @@ -0,0 +1,45 @@ > +/** @file > +* > +* Copyright (c) Linaro Ltd. All rights reserved. > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > + > +#ifndef SBSA_QEMU_HARDWARE_INFO_ > +#define SBSA_QEMU_HARDWARE_INFO_ > + > +/** > + Get CPU count from information passed by Qemu. > + > +**/ > +VOID > +SbsaQemuGetCpuCount ( > + VOID > + ); > + > +/** > + Get MPIDR for a given cpu from device tree passed by Qemu. > + > + @param [in] CpuId Index of cpu to retrieve MPIDR value for. > + > + @retval MPIDR value of CPU at index > +**/ > +UINT64 > +SbsaQemuGetMpidr ( > + IN UINTN CpuId > + ); > + > +/** > + Get NUMA node id for a given cpu from device tree passed by Qemu. > + > + @param [in] CpuId Index of cpu to retrieve NUMA node id for. > + > + @retval NUMA node id for CPU at index > +**/ > +UINT64 > +SbsaQemuGetCpuNumaNode ( > + IN UINTN CpuId > + ); > + > +#endif /* SBSA_QEMU_HARDWARE_INFO_ */ > diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c > new file mode 100644 > index 000000000000..4df973fda75e > --- /dev/null > +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c > @@ -0,0 +1,100 @@ > +/** @file > +* > +* Copyright (c) 2021, NUVIA Inc. All rights reserved. > +* Copyright (c) Linaro Ltd. All rights reserved. > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > + > +#include > +#include > +#include > +#include > +#include > + > +/** > + Get CPU count from information passed by Qemu. > + > +**/ > +VOID > +SbsaQemuGetCpuCount ( > + VOID > + ) > +{ > + UINTN Arg0; > + UINTN SmcResult; > + RETURN_STATUS Result; > + > + SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_COUNT, &Arg0, NULL, NULL); > + if (SmcResult != SMC_ARCH_CALL_SUCCESS) { So, this may sound a little bit bonkers, but SMCCC doesn't define return codes for SiP functions. And the SMC_ARCH_CALL_SUCCESS macro denotes an Arm Architectural function. Could you #define an SMC_SIP_CALL_SUCCESS *as* SMC_ARCH_CALL_SUCCESS in some platform-specific header and then use that? > + DEBUG ((DEBUG_INFO, "Too old TF-A. We have to get cpu info from DT.\n")); We don't *know* the TF-A is too old. Rather print which call failed (and that we need to attempt to get the cpu count from DT). > + Arg0 = FdtHelperCountCpus(); We should probably assert this as != 0? Or is that done in the function? > + } > + > + Result = PcdSet32S (PcdCoreCount, Arg0); > + ASSERT_RETURN_ERROR (Result); > + > + Arg0 = PcdGet32 (PcdCoreCount); Seems redundant to re-set it to the value we know it already holds. > + > + DEBUG ((DEBUG_INFO, "We have %d cpus.\n", Arg0)); > +} > + > +/** > + Get MPIDR for a given cpu from device tree passed by Qemu. > + > + @param [in] CpuId Index of cpu to retrieve MPIDR value for. > + > + @retval MPIDR value of CPU at index > +**/ > +UINT64 > +SbsaQemuGetMpidr ( > + IN UINTN CpuId > + ) > +{ > + UINTN SmcResult; > + UINTN Arg0; > + UINTN Arg1; > + > + Arg0 = CpuId; > + > + SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL); > + if (SmcResult != SMC_ARCH_CALL_SUCCESS) { > + DEBUG ((DEBUG_INFO, "Too old TF-A. We have to get cpu info from DT.\n")); We don't *know* the TF-A is too old. Rather print which call failed (and that we need to attempt to get the cpu date from DT). > + Arg1 = FdtHelperGetMpidr(CpuId); > +} > + > + DEBUG ((DEBUG_ERROR, "MPIDR for CPU:%d = %d\n", CpuId, Arg1)); > + > + return Arg1; > +} > + > +/** > + Get NUMA node id for a given cpu from device tree passed by Qemu. > + > + @param [in] CpuId Index of cpu to retrieve NUMA node id for. > + > + @retval NUMA node id for CPU at index > +**/ > +UINT64 > +SbsaQemuGetCpuNumaNode ( > + IN UINTN CpuId > + ) > +{ > + UINTN SmcResult; > + UINTN Arg0; > + UINTN Arg1; > + > + Arg0 = CpuId; > + > + SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL); It does seem a bit wasteful we're making the same call twice per core, discarding one of the results. Could we have an init function that allocates an array and prepopulates it, with the Get-functions just returning values from the array? > + if (SmcResult != SMC_ARCH_CALL_SUCCESS) { > + /* No fallback to DeviceTree as we did not had that info earlier. */ We don't *know* the TF-A is too old. Rather print which call failed (and that we need to attempt to get the cpu count from DT). / Leif > + DEBUG ((DEBUG_ERROR, "Couldn't find information for CPU:%d\n", CpuId)); > + return 0; > + } > + > + DEBUG ((DEBUG_ERROR, "NUMA node for CPU:%d = %d\n", CpuId, Arg0)); > + > + return Arg0; > +} > > -- > 2.43.0 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114095): https://edk2.groups.io/g/devel/message/114095 Mute This Topic: https://groups.io/mt/103758014/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-