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 59FC8AC10A5 for ; Wed, 24 Jan 2024 12:56:00 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=mCv87Nilz5/S5YAErZ45hb0tdojXehOjpNeUP3e4SG4=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:Organization:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1706100959; v=1; b=Q6kGVfy59xL3SV7L6soEa5dGJPAGuU0wMrNifv9SzEiZchy1AY7P35+0DNT+ISXZJ5UeKcZw zPFWzOoABTYe9cW0tpQvGyUII/P5Ghs/qY/uUNkg1iNHLZecJ3hUbwu2xlxlPTHKGSwfkOXNj4E rU3g/UWJaka5NGBv70RAe7+g= X-Received: by 127.0.0.2 with SMTP id hBpIYY7687511xNWrGwmnPDz; Wed, 24 Jan 2024 04:55:59 -0800 X-Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com [209.85.208.49]) by mx.groups.io with SMTP id smtpd.web11.21789.1706100958054262348 for ; Wed, 24 Jan 2024 04:55:58 -0800 X-Received: by mail-ed1-f49.google.com with SMTP id 4fb4d7f45d1cf-55a8fb31fc2so4894807a12.0 for ; Wed, 24 Jan 2024 04:55:57 -0800 (PST) X-Gm-Message-State: JcHlziqH3XWyHo6wxdjTcP1gx7686176AA= X-Google-Smtp-Source: AGHT+IGr9zePEiHf0ev7wS2R/OiLShtFn/fPBQ1fluaii6SxFlOQZHpyhP4r7LARmKcJJB2qD4AvrA== X-Received: by 2002:a17:907:c243:b0:a2d:a6b7:cce with SMTP id tj3-20020a170907c24300b00a2da6b70ccemr1098267ejc.122.1706100956053; Wed, 24 Jan 2024 04:55:56 -0800 (PST) X-Received: from [192.168.200.206] (83.11.3.72.ipv4.supernova.orange.pl. [83.11.3.72]) by smtp.gmail.com with ESMTPSA id vh3-20020a170907d38300b00a2e7d1b6042sm9476336ejc.196.2024.01.24.04.55.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 24 Jan 2024 04:55:55 -0800 (PST) Message-ID: <7eebb4a3-b431-4d7f-8027-1ec96c9bb68a@linaro.org> Date: Wed, 24 Jan 2024 13:55:54 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH edk2-platforms v2 1/4] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib To: devel@edk2.groups.io, quic_llindhol@quicinc.com Cc: Ard Biesheuvel , Graeme Gregory References: <20240116-no-dt-for-cpu-v2-0-6cf078d9ab76@linaro.org> <20240116-no-dt-for-cpu-v2-1-6cf078d9ab76@linaro.org> From: "Marcin Juszkiewicz" Organization: Linaro In-Reply-To: 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,marcin.juszkiewicz@linaro.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Language: pl-PL, en-GB, en-HK Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=Q6kGVfy5; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linaro.org (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 W dniu 19.01.2024 o 20:18, Leif Lindholm pisze: > 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/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? Added it in Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h and used. >> + 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). Changed: DEBUG ((DEBUG_INFO, "SIP_SVC_GET_CPU_COUNT call failed. We have to get cpu info from DT.\n")); >> + Arg0 = FdtHelperCountCpus(); > > We should probably assert this as != 0? Or is that done in the function? Added code to stop going if we do not have cpu information. In such case we will not be back here with wrong info. > >> + } >> + >> + 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. Done >> + >> + 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). done >> +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? good idea, will look into >> + 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). done -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114287): https://edk2.groups.io/g/devel/message/114287 Mute This Topic: https://groups.io/mt/103758014/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-