From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 4238CD80248 for ; Tue, 9 Jul 2024 13:00:44 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=iM9+PB1NL3hVTjFpQaKGXNJYFMq6KeIvudutAOSd/YA=; 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:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition; s=20240206; t=1720530043; v=1; b=IgjRGKHFjInRf7YLGjg8s890cRUT9WXgbOplQt2mfj5q/6NrVSM3tOFPuTBHwXo3AT/EGjeN RMb2mBXnp80pjJVG1NAYndSPsil95D10o76BYjm36+PJom/MsJajI/u4ivfdTwQsnRkY8/HcUmY tU2jj8/ZWDURzi/b73luLQM+XaVqsib+TELYu0UdFynxOexdtiQEAHK9gHqtZoZpHEssC2mdD9h IvgbzRbSto4nxPZUOeCZ0YXNEr1d5eZtrkOy1KoVMgaVUM8o+updHswCE6il6PeTn3Q91smMDmz hKCGoRVE4QpbN/10TQzABRJy1i6U9Z8yIh4f+lJpr+eqw== X-Received: by 127.0.0.2 with SMTP id qF1JYY7687511xQvYa2vov68; Tue, 09 Jul 2024 06:00:42 -0700 X-Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by mx.groups.io with SMTP id smtpd.web11.12323.1720530042110466852 for ; Tue, 09 Jul 2024 06:00:42 -0700 X-Received: from pps.filterd (m0279871.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 469Bb96l001911; Tue, 9 Jul 2024 13:00:41 GMT X-Received: from nasanppmta02.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 406wgwpek4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 09 Jul 2024 13:00:41 +0000 (GMT) X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA02.qualcomm.com (8.17.1.19/8.17.1.19) with ESMTPS id 469D0HiV020607 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 9 Jul 2024 13:00:17 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.1544.9; Tue, 9 Jul 2024 06:00:15 -0700 Date: Tue, 9 Jul 2024 14:00:12 +0100 From: "Leif Lindholm" To: Marcin Juszkiewicz CC: , Xiong Yining , Ard Biesheuvel , Graeme Gregory , Chen Baozi Subject: Re: [edk2-devel] [PATCH edk2-platforms v3 5/5] SbsaQemu: introduce helper in PPTT generation Message-ID: References: <20240709-acpi65-v3-0-ee93ba536fcf@linaro.org> <20240709-acpi65-v3-5-ee93ba536fcf@linaro.org> MIME-Version: 1.0 In-Reply-To: <20240709-acpi65-v3-5-ee93ba536fcf@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: e_9qqJ5sLP1LU9UiQ52x0aax5g3gE8P9 X-Proofpoint-ORIG-GUID: e_9qqJ5sLP1LU9UiQ52x0aax5g3gE8P9 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 Resent-Date: Tue, 09 Jul 2024 06:00:42 -0700 Resent-From: quic_llindhol@quicinc.com Reply-To: devel@edk2.groups.io,quic_llindhol@quicinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: SuQRjYNBjKN0kFioqbi8J0Bcx7686176AA= 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=20240206 header.b=IgjRGKHF; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=quicinc.com (policy=none) On Tue, Jul 09, 2024 at 12:47:10 +0200, Marcin Juszkiewicz wrote: > Function AddPpttTable() adding PPTT got too long. This change moves part > of it into helper function AddCoresToPpttTable() which takes care of > generating entries for Core and below (Cache, Thread). > > Signed-off-by: Marcin Juszkiewicz > --- > .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 237 +++++++++++--------- > 1 file changed, 133 insertions(+), 104 deletions(-) > > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > index a7a9664abdcb..a4b2ee2fdcb0 100644 > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > @@ -29,6 +29,9 @@ > > static UINTN GicItsBase; > > +static UINTN CpuId; > +static UINTN CacheId; > + static variables are supposed to have g (global) or m (module) prefix. This is local, so m. (Yes, that means I missed that when reviewing the GitIts bits.) Also, why are these in a #pragma pack(1) block? > #pragma pack () > > /* > @@ -491,6 +494,126 @@ AddSsdtTable ( > return Status; > } > STATIC > +UINT32 > +AddCoresToPpttTable ( > + UINT8 *New, > + UINT32 ClusterIndex, > + CpuTopology CpuTopo > + ) > +{ > + UINT32 L1DCacheIndex; > + UINT32 L1ICacheIndex; > + UINT32 L2CacheIndex; > + UINT32 CoreIndex; > + UINT32 Index; > + UINT32 CoreCpuId; > + UINT32 CoreNum; > + UINT32 ThreadNum; > + UINT32 *PrivateResourcePtr; > + > + EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS CoreFlags = { > + EFI_ACPI_6_5_PPTT_PACKAGE_NOT_PHYSICAL, > + EFI_ACPI_6_5_PPTT_PROCESSOR_ID_VALID, > + EFI_ACPI_6_5_PPTT_PROCESSOR_IS_NOT_THREAD, > + EFI_ACPI_6_5_PPTT_NODE_IS_LEAF, > + EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL > + }; > + > + if (CpuTopo.Threads > 1) { > + // The Thread structure is the leaf structure, adjust the value of CoreFlags. > + CoreFlags.AcpiProcessorIdValid = EFI_ACPI_6_5_PPTT_PROCESSOR_ID_INVALID; > + CoreFlags.NodeIsALeaf = EFI_ACPI_6_5_PPTT_NODE_IS_NOT_LEAF; > + } > + > + EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS ThreadFlags = { > + EFI_ACPI_6_5_PPTT_PACKAGE_NOT_PHYSICAL, > + EFI_ACPI_6_5_PPTT_PROCESSOR_ID_VALID, > + EFI_ACPI_6_5_PPTT_PROCESSOR_IS_THREAD, > + EFI_ACPI_6_5_PPTT_NODE_IS_LEAF, > + EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL > + }; > + > + EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE L1DCache = SBSAQEMU_ACPI_PPTT_L1_D_CACHE_STRUCT; > + EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE L1ICache = SBSAQEMU_ACPI_PPTT_L1_I_CACHE_STRUCT; > + EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE L2Cache = SBSAQEMU_ACPI_PPTT_L2_CACHE_STRUCT; > + > + CoreIndex = ClusterIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > + Index = CoreIndex; > + > + for (CoreNum = 0; CoreNum < CpuTopo.Cores; CoreNum++) { > + if (CpuTopo.Threads == 1) { > + CoreCpuId = CpuId; > + } else { > + CoreCpuId = 0; > + } > + > + // space for Core + PrivateResourcePtr > + Index += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > + Index += sizeof (UINT32) * 2; > + > + L1DCacheIndex = Index; > + L1ICacheIndex = L1DCacheIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > + L2CacheIndex = L1ICacheIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > + > + EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR Core = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT ( > + CoreFlags, > + ClusterIndex, > + CoreCpuId, > + 2 > + ); > + > + CopyMem (New, &Core, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR)); > + New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > + > + PrivateResourcePtr = (UINT32 *)New; > + PrivateResourcePtr[0] = L1DCacheIndex; > + PrivateResourcePtr[1] = L1ICacheIndex; > + New += (2 * sizeof (UINT32)); > + > + // Add L1 D Cache structure > + L1DCache.CacheId = CacheId++; > + CopyMem (New, &L1DCache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE)); > + ((EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = L2CacheIndex; > + New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > + > + // Add L1 I Cache structure > + L1ICache.CacheId = CacheId++; > + CopyMem (New, &L1ICache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE)); > + ((EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = L2CacheIndex; > + New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > + > + // Add L2 Cache structure > + L2Cache.CacheId = CacheId++; > + CopyMem (New, &L2Cache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE)); > + New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > + > + Index += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE) * 3; > + > + if (CpuTopo.Threads == 1) { > + CpuId++; > + } else { > + // Add the Thread PPTT structure > + for (ThreadNum = 0; ThreadNum < CpuTopo.Threads; ThreadNum++) { > + EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR Thread = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT ( > + ThreadFlags, > + CoreIndex, > + CpuId, > + 0 > + ); > + CopyMem (New, &Thread, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR)); > + New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > + CpuId++; > + } > + > + Index += CpuTopo.Threads * sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > + } > + > + CoreIndex = Index; > + } > + > + return CoreIndex - ClusterIndex - sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > +} > + > /* > * A function that adds the PPTT ACPI table. > */ > @@ -502,18 +625,17 @@ AddPpttTable ( > EFI_STATUS Status; > UINTN TableHandle; > UINT32 TableSize; > + UINT32 CoresPartSize; > + UINT32 SocketNum; > + UINT32 ClusterNum; > + UINT32 SocketIndex; > + UINT32 ClusterIndex; > EFI_PHYSICAL_ADDRESS PageAddress; > UINT8 *New; > - UINT32 CpuId; > CpuTopology CpuTopo; > - UINT32 CacheId; > > GetCpuTopology (&CpuTopo); > > - EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE L1DCache = SBSAQEMU_ACPI_PPTT_L1_D_CACHE_STRUCT; > - EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE L1ICache = SBSAQEMU_ACPI_PPTT_L1_I_CACHE_STRUCT; > - EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE L2Cache = SBSAQEMU_ACPI_PPTT_L2_CACHE_STRUCT; > - > EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS SocketFlags = { > EFI_ACPI_6_5_PPTT_PACKAGE_PHYSICAL, > EFI_ACPI_6_5_PPTT_PROCESSOR_ID_INVALID, > @@ -530,28 +652,6 @@ AddPpttTable ( > EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL > }; > > - EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS CoreFlags = { > - EFI_ACPI_6_5_PPTT_PACKAGE_NOT_PHYSICAL, > - EFI_ACPI_6_5_PPTT_PROCESSOR_ID_VALID, > - EFI_ACPI_6_5_PPTT_PROCESSOR_IS_NOT_THREAD, > - EFI_ACPI_6_5_PPTT_NODE_IS_LEAF, > - EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL > - }; > - > - if (CpuTopo.Threads > 1) { > - // The Thread structure is the leaf structure, adjust the value of CoreFlags. > - CoreFlags.AcpiProcessorIdValid = EFI_ACPI_6_5_PPTT_PROCESSOR_ID_INVALID; > - CoreFlags.NodeIsALeaf = EFI_ACPI_6_5_PPTT_NODE_IS_NOT_LEAF; > - } > - > - EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS ThreadFlags = { > - EFI_ACPI_6_5_PPTT_PACKAGE_NOT_PHYSICAL, > - EFI_ACPI_6_5_PPTT_PROCESSOR_ID_VALID, > - EFI_ACPI_6_5_PPTT_PROCESSOR_IS_THREAD, > - EFI_ACPI_6_5_PPTT_NODE_IS_LEAF, > - EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL > - }; > - > EFI_ACPI_DESCRIPTION_HEADER Header = > SBSAQEMU_ACPI_HEADER ( > EFI_ACPI_6_5_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE, > @@ -590,11 +690,9 @@ AddPpttTable ( > ((EFI_ACPI_DESCRIPTION_HEADER *)New)->Length = TableSize; > New += sizeof (EFI_ACPI_DESCRIPTION_HEADER); > > - UINT32 SocketNum, ClusterNum, CoreNum, ThreadNum; > - UINT32 SocketIndex, ClusterIndex, CoreIndex, L1DCacheIndex, L1ICacheIndex, L2CacheIndex; > + CpuId = 0; > + CacheId = 1; // 0 is not a valid Cache ID. > > - CpuId = 0; > - CacheId = 1; // 0 is not a valid Cache ID. > SocketIndex = sizeof (EFI_ACPI_DESCRIPTION_HEADER); > for (SocketNum = 0; SocketNum < CpuTopo.Sockets; SocketNum++) { > // Add the Socket PPTT structure > @@ -609,8 +707,6 @@ AddPpttTable ( > > ClusterIndex = SocketIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > for (ClusterNum = 0; ClusterNum < CpuTopo.Clusters; ClusterNum++) { > - CoreIndex = ClusterIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > - > // Add the Cluster PPTT structure > EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR Cluster = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT ( > ClusterFlags, > @@ -621,76 +717,9 @@ AddPpttTable ( > CopyMem (New, &Cluster, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR)); > New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > > - for (CoreNum = 0; CoreNum < CpuTopo.Cores; CoreNum++) { > - UINT32 *PrivateResourcePtr; > - UINT32 CoreCpuId; > - > - // two UINT32s for PrivateResourcePtr data > - L1DCacheIndex = CoreIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR) + sizeof (UINT32) * 2; > - L1ICacheIndex = L1DCacheIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > - L2CacheIndex = L1ICacheIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > - > - if (CpuTopo.Threads == 1) { > - CoreCpuId = CpuId; > - } else { > - CoreCpuId = 0; > - } > - > - EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR Core = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT ( > - CoreFlags, > - ClusterIndex, > - CoreCpuId, > - 2 > - ); > - CopyMem (New, &Core, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR)); > - New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > - > - PrivateResourcePtr = (UINT32 *)New; > - PrivateResourcePtr[0] = L1DCacheIndex; > - PrivateResourcePtr[1] = L1ICacheIndex; > - New += (2 * sizeof (UINT32)); > - > - // Add L1 D Cache structure > - L1DCache.CacheId = CacheId++; > - CopyMem (New, &L1DCache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE)); > - ((EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = L2CacheIndex; > - New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > - > - // Add L1 I Cache structure > - L1ICache.CacheId = CacheId++; > - CopyMem (New, &L1ICache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE)); > - ((EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = L2CacheIndex; > - New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > - > - // Add L2 Cache structure > - L2Cache.CacheId = CacheId++; > - CopyMem (New, &L2Cache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE)); > - New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE); > - > - if (CpuTopo.Threads == 1) { > - CpuId++; > - } else { > - // Add the Thread PPTT structure > - for (ThreadNum = 0; ThreadNum < CpuTopo.Threads; ThreadNum++) { > - EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR Thread = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT ( > - ThreadFlags, > - CoreIndex, > - CpuId, > - 0 > - ); > - CopyMem (New, &Thread, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR)); > - New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > - CpuId++; > - } > - > - CoreIndex += CpuTopo.Threads * sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR); > - } > - > - CoreIndex += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR) + sizeof (UINT32) * 2; > - CoreIndex += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE) * 3; > - } > - > - ClusterIndex = CoreIndex; > + CoresPartSize = AddCoresToPpttTable (New, ClusterIndex, CpuTopo); > + ClusterIndex += CoresPartSize; This sounds like ClisterIndex is no longer an Index after this patch. Should it be renamed? / Leif > + New += CoresPartSize; > } > > SocketIndex = ClusterIndex; > > -- > 2.45.2 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119841): https://edk2.groups.io/g/devel/message/119841 Mute This Topic: https://groups.io/mt/107120147/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-