From nobody Sat Feb 7 05:14:49 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+54946+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+54946+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1582755139; cv=none; d=zohomail.com; s=zohoarc; b=oCOWkvGtAkBaaVBqMJRDJkcsTUU21F8fEGH8l7p2HMPL/RjCe/mj4aJ5HZn9xFHjAj4AWxnQMBVUzZLVkCIdtfw0GzZgrlmA1YInmcKzIEhhMDr2dQ8iIegbSrOHp4klSIpMEITZf5O0C9YsaoaYhAJBqlohWV7ADfJo7ILxC0w= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1582755139; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=rY3pyrsahPfbkw9RHthJISaIvDstjOcqRb3uJvuvl+I=; b=JfeD4I24VVQHfEudNhyNq8FDz+xQbprhGH9BT4KO3qdO0rnh3Tr5QtZkGW1ox5mWVNz0PssXkTAA7+mSIvlUnZs/ydA4tEsB0PRpunYFcH0TtgbWTqYkuYErCpyONBBKV/5vL9QSRhm2/KKj8mxB7mzCjFdQuyVPaS09jq+H058= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+54946+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1582755139310897.8751393422112; Wed, 26 Feb 2020 14:12:19 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id AKEbYY1788612xQPsWiNV2al; Wed, 26 Feb 2020 14:12:18 -0800 X-Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.234.1582755137875141529 for ; Wed, 26 Feb 2020 14:12:18 -0800 X-Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-468-Fn4QO4VoPEavl3aH2A6g3Q-1; Wed, 26 Feb 2020 17:12:10 -0500 X-MC-Unique: Fn4QO4VoPEavl3aH2A6g3Q-1 X-Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A0A228017DF; Wed, 26 Feb 2020 22:12:08 +0000 (UTC) X-Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-185.ams2.redhat.com [10.36.116.185]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8CEE060BEF; Wed, 26 Feb 2020 22:12:06 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Ard Biesheuvel , Eric Dong , Igor Mammedov , Jiewen Yao , Jordan Justen , Michael Kinney , =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= , Ray Ni Subject: [edk2-devel] [PATCH v2 02/16] UefiCpuPkg/PiSmmCpuDxeSmm: fix S3 Resume for CPU hotplug Date: Wed, 26 Feb 2020 23:11:42 +0100 Message-Id: <20200226221156.29589-3-lersek@redhat.com> In-Reply-To: <20200226221156.29589-1-lersek@redhat.com> References: <20200226221156.29589-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Unsubscribe: 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,lersek@redhat.com X-Gm-Message-State: Hgw8kVgYZFhvDtVeADN8hhUEx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1582755138; bh=rY3pyrsahPfbkw9RHthJISaIvDstjOcqRb3uJvuvl+I=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=axumoddSKU3DFsWkYC7BvVC+/dI7sIYDVc6kijh4czjKdrGSIa7U7BprZ6PUOARSo0H tbxiWdECGQmXE0GoeZakrrs+eoJFwFgEyYUcDFFIaMvvwZyjTTGTVpaHx1+o5knGLUos5 8mt0M8Zrdzx8YirGYSj4lKvYctbPKKQjVy4= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" The "ACPI_CPU_DATA.NumberOfCpus" field is specified as follows, in "UefiCpuPkg/Include/AcpiCpuData.h" (rewrapped for this commit message): // // The number of CPUs. If a platform does not support hot plug CPUs, // then this is the number of CPUs detected when the platform is booted, // regardless of being enabled or disabled. If a platform does support // hot plug CPUs, then this is the maximum number of CPUs that the // platform supports. // The InitializeCpuBeforeRebase() and InitializeCpuAfterRebase() functions in "UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c" try to restore CPU configuration on the S3 Resume path for *all* CPUs accounted for in "ACPI_CPU_DATA.NumberOfCpus". This is wrong, as with CPU hotplug, not all of the possible CPUs may be present at the time of S3 Suspend / Resume. The symptom is an infinite wait. Instead, the "mNumberOfCpus" variable should be used, which is properly maintained through the EFI_SMM_CPU_SERVICE_PROTOCOL implementation (see SmmAddProcessor(), SmmRemoveProcessor(), SmmCpuUpdate() in "UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c"). When CPU hotplug is disabled, "mNumberOfCpus" is constant, and equals "ACPI_CPU_DATA.NumberOfCpus" at all times. Cc: Ard Biesheuvel Cc: Eric Dong Cc: Igor Mammedov Cc: Jiewen Yao Cc: Jordan Justen Cc: Michael Kinney Cc: Philippe Mathieu-Daud=C3=A9 Cc: Ray Ni Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1512 Signed-off-by: Laszlo Ersek Acked-by: Ard Biesheuvel Reviewed-by: Eric Dong --- Notes: v2: =20 - Pick up Ard's Acked-by, which is conditional on approval from Intel reviewers on Cc. (I'd like to save Ard the churn of re-acking unmodified patches.) UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/= CpuS3.c index ba5cc0194c2d..1e0840119724 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -597,75 +597,85 @@ PrepareApStartupVector ( } =20 /** The function is invoked before SMBASE relocation in S3 path to restores = CPU status. =20 The function is invoked before SMBASE relocation in S3 path. It does fir= st time microcode load and restores MTRRs for both BSP and APs. =20 **/ VOID InitializeCpuBeforeRebase ( VOID ) { LoadMtrrData (mAcpiCpuData.MtrrTable); =20 SetRegister (TRUE); =20 ProgramVirtualWireMode (); =20 PrepareApStartupVector (mAcpiCpuData.StartupVector); =20 - mNumberToFinish =3D mAcpiCpuData.NumberOfCpus - 1; + if (FeaturePcdGet (PcdCpuHotPlugSupport)) { + ASSERT (mNumberOfCpus <=3D mAcpiCpuData.NumberOfCpus); + } else { + ASSERT (mNumberOfCpus =3D=3D mAcpiCpuData.NumberOfCpus); + } + mNumberToFinish =3D mNumberOfCpus - 1; mExchangeInfo->ApFunction =3D (VOID *) (UINTN) InitializeAp; =20 // // Execute code for before SmmBaseReloc. Note: This flag is maintained a= cross S3 boots. // mInitApsAfterSmmBaseReloc =3D FALSE; =20 // // Send INIT IPI - SIPI to all APs // SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); =20 while (mNumberToFinish > 0) { CpuPause (); } } =20 /** The function is invoked after SMBASE relocation in S3 path to restores C= PU status. =20 The function is invoked after SMBASE relocation in S3 path. It restores = configuration according to data saved by normal boot path for both BSP and APs. =20 **/ VOID InitializeCpuAfterRebase ( VOID ) { - mNumberToFinish =3D mAcpiCpuData.NumberOfCpus - 1; + if (FeaturePcdGet (PcdCpuHotPlugSupport)) { + ASSERT (mNumberOfCpus <=3D mAcpiCpuData.NumberOfCpus); + } else { + ASSERT (mNumberOfCpus =3D=3D mAcpiCpuData.NumberOfCpus); + } + mNumberToFinish =3D mNumberOfCpus - 1; =20 // // Signal that SMM base relocation is complete and to continue initializ= ation for all APs. // mInitApsAfterSmmBaseReloc =3D TRUE; =20 // // Must begin set register after all APs have continue their initializat= ion. // This is a requirement to support semaphore mechanism in register tabl= e. // Because if semaphore's dependence type is package type, semaphore wil= l wait // for all Aps in one package finishing their tasks before set next regi= ster // for all APs. If the Aps not begin its task during BSP doing its task,= the // BSP thread will hang because it is waiting for other Aps in the same // package finishing their task. // SetRegister (FALSE); =20 while (mNumberToFinish > 0) { CpuPause (); } } =20 --=20 2.19.1.3.g30247aa5d201 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54946): https://edk2.groups.io/g/devel/message/54946 Mute This Topic: https://groups.io/mt/71575170/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-