From nobody Sat Feb 7 06:49:10 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+54958+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+54958+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1582755163; cv=none; d=zohomail.com; s=zohoarc; b=QbDaJoDRkwGUpq8N3GB57A5UqWVV4H1NkAEu6o/mk07iMviz0vR9QuoM14ABTOxhHG9PV4UL396n3msFkMGJcwxXAleVx+823nQGOauNxqDnpphhFr/FL76hN0ytsznJp/rEu+0V8wryCJhyCQX6OfKauKJXVD70wm4//xNRMlw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1582755163; 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=eTlccmdRyt6GPSk0dcUxTYe936E12Zpv4Z6pomnS6hA=; b=WYGV1irI4JGSO0R8FkGr6bV40SI/QcJWcBNq/Xebqiv+QgNgMAqL1mNK4ItKExCITXB1KK2nYkqSQOvXCpWp/45LZDjOv/BBKVifvZ6Dtt7XQYMyh1bfmMPf8Jwk+9BW5H9hjwDKlJgGxliQ6S4pxCdn2jxtyTECIUCoPKqXgB8= 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+54958+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 1582755163386569.8267177643413; Wed, 26 Feb 2020 14:12:43 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id q8buYY1788612xC6lisp0Dos; Wed, 26 Feb 2020 14:12:42 -0800 X-Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web09.296.1582755161584477370 for ; Wed, 26 Feb 2020 14:12:41 -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-427-OI0NQ6XJPAm1Ddaqi1u-dQ-1; Wed, 26 Feb 2020 17:12:39 -0500 X-MC-Unique: OI0NQ6XJPAm1Ddaqi1u-dQ-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 D063818CA240; Wed, 26 Feb 2020 22:12:37 +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 23ED5909E9; Wed, 26 Feb 2020 22:12:35 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Ard Biesheuvel , Igor Mammedov , Jiewen Yao , Jordan Justen , Michael Kinney , =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Subject: [edk2-devel] [PATCH v2 16/16] OvmfPkg/CpuS3DataDxe: enable S3 resume after CPU hotplug Date: Wed, 26 Feb 2020 23:11:56 +0100 Message-Id: <20200226221156.29589-17-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: LnpKWkdSbMR1pbur3TdAoKONx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1582755162; bh=eTlccmdRyt6GPSk0dcUxTYe936E12Zpv4Z6pomnS6hA=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=WNq3GerQ+nwhbB9mSKuajRNb98XBeCVykfgwc+sz7wOZ/Sg++87xm2Jshq1YdbORN9M doN4hJB5BhHUfdsJPbax7EHZ/VgYnFWA07R3PUyJfrVAhtEUjAV4n/9IYWh9S3sHRbHTg +97PqbK1cqGu/hewBgO5YPGsCcMk/0v1c6s= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" During normal boot, CpuS3DataDxe allocates - an empty CPU_REGISTER_TABLE entry in the "ACPI_CPU_DATA.PreSmmInitRegisterTable" array, and - an empty CPU_REGISTER_TABLE entry in the "ACPI_CPU_DATA.RegisterTable" array, for every CPU whose APIC ID CpuS3DataDxe can learn. Currently EFI_MP_SERVICES_PROTOCOL is used for both determining the number of CPUs -- the protocol reports the present-at-boot CPU count --, and for retrieving the APIC IDs of those CPUs. Consequently, if a CPU is hot-plugged at OS runtime, then S3 resume breaks. That's because PiSmmCpuDxeSmm will not find the hot-added CPU's APIC ID associated with any CPU_REGISTER_TABLE object, in the SMRAM copies of either of the "RegisterTable" and "PreSmmInitRegisterTable" arrays. The failure to match the hot-added CPU's APIC ID trips the ASSERT() in SetRegister() [UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c]. If "PcdQ35SmramAtDefaultSmbase" is TRUE, then: - prepare CPU_REGISTER_TABLE objects for all possible CPUs, not just the present-at-boot CPUs (PlatformPei stored the possible CPU count to "PcdCpuMaxLogicalProcessorNumber"); - use QEMU_CPUHP_CMD_GET_ARCH_ID for filling in the "InitialApicId" fields of the CPU_REGISTER_TABLE objects. This provides full APIC ID coverage for PiSmmCpuDxeSmm during S3 resume, accommodating CPUs hot-added at OS runtime. This patch is best reviewed with $ git show -b Cc: Ard Biesheuvel Cc: Igor Mammedov Cc: Jiewen Yao Cc: Jordan Justen Cc: Michael Kinney Cc: Philippe Mathieu-Daud=C3=A9 Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1512 Signed-off-by: Laszlo Ersek Acked-by: Ard Biesheuvel Reviewed-by: Ard Biesheuvel --- 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.) OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 4 + OvmfPkg/CpuS3DataDxe/CpuS3Data.c | 91 ++++++++++++++------ 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf b/OvmfPkg/CpuS3DataDxe/C= puS3DataDxe.inf index f9679e0c33b3..ceae1d4078c7 100644 --- a/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf +++ b/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf @@ -16,46 +16,50 @@ # ## =20 [Defines] INF_VERSION =3D 1.29 BASE_NAME =3D CpuS3DataDxe FILE_GUID =3D 229B7EFD-DA02-46B9-93F4-E20C009F94E9 MODULE_TYPE =3D DXE_DRIVER VERSION_STRING =3D 1.0 ENTRY_POINT =3D CpuS3DataInitialize =20 # The following information is for reference only and not required by the = build # tools. # # VALID_ARCHITECTURES =3D IA32 X64 =20 [Sources] CpuS3Data.c =20 [Packages] MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec + OvmfPkg/OvmfPkg.dec UefiCpuPkg/UefiCpuPkg.dec =20 [LibraryClasses] BaseLib BaseMemoryLib DebugLib + IoLib MemoryAllocationLib MtrrLib UefiBootServicesTableLib UefiDriverEntryPoint =20 [Guids] gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event =20 [Protocols] gEfiMpServiceProtocolGuid ## CONSUMES =20 [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## CON= SUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## CON= SUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CON= SUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuS3DataAddress ## PRO= DUCES + gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase ## CON= SUMES =20 [Depex] gEfiMpServiceProtocolGuid diff --git a/OvmfPkg/CpuS3DataDxe/CpuS3Data.c b/OvmfPkg/CpuS3DataDxe/CpuS3D= ata.c index 8bb9807cd501..bac7285aa2f3 100644 --- a/OvmfPkg/CpuS3DataDxe/CpuS3Data.c +++ b/OvmfPkg/CpuS3DataDxe/CpuS3Data.c @@ -4,51 +4,55 @@ ACPI CPU Data initialization module This module initializes the ACPI_CPU_DATA structure and registers the addr= ess of this structure in the PcdCpuS3DataAddress PCD. This is a generic/simple version of this module. It does not provide a machine check handler or CPU register initialization tables for ACPI S3 resume. It also only supports = the number of CPUs reported by the MP Services Protocol, so this module does n= ot support hot plug CPUs. This module can be copied into a CPU specific pack= age and customized if these additional features are required. =20 Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.
Copyright (c) 2015 - 2020, Red Hat, Inc. =20 SPDX-License-Identifier: BSD-2-Clause-Patent =20 **/ =20 #include =20 #include =20 #include #include #include +#include #include #include #include =20 #include #include =20 +#include +#include + // // Data structure used to allocate ACPI_CPU_DATA and its supporting struct= ures // typedef struct { ACPI_CPU_DATA AcpiCpuData; MTRR_SETTINGS MtrrTable; IA32_DESCRIPTOR GdtrProfile; IA32_DESCRIPTOR IdtrProfile; } ACPI_CPU_DATA_EX; =20 /** Allocate EfiACPIMemoryNVS memory. =20 @param[in] Size Size of memory to allocate. =20 @return Allocated address for output. =20 **/ VOID * AllocateAcpiNvsMemory ( IN UINTN Size ) @@ -144,89 +148,101 @@ CpuS3DataOnEndOfDxe ( to the address that ACPI_CPU_DATA is allocated at. =20 @param[in] ImageHandle The firmware allocated handle for the EFI image. @param[in] SystemTable A pointer to the EFI System Table. =20 @retval EFI_SUCCESS The entry point is executed successfully. @retval EFI_UNSUPPORTED Do not support ACPI S3. @retval other Some error occurs when executing this entry poi= nt. =20 **/ EFI_STATUS EFIAPI CpuS3DataInitialize ( IN EFI_HANDLE ImageHandle, IN EFI_SYSTEM_TABLE *SystemTable ) { EFI_STATUS Status; ACPI_CPU_DATA_EX *AcpiCpuDataEx; ACPI_CPU_DATA *AcpiCpuData; EFI_MP_SERVICES_PROTOCOL *MpServices; UINTN NumberOfCpus; - UINTN NumberOfEnabledProcessors; VOID *Stack; UINTN TableSize; CPU_REGISTER_TABLE *RegisterTable; UINTN Index; EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer; UINTN GdtSize; UINTN IdtSize; VOID *Gdt; VOID *Idt; EFI_EVENT Event; ACPI_CPU_DATA *OldAcpiCpuData; + BOOLEAN FetchPossibleApicIds; =20 if (!PcdGetBool (PcdAcpiS3Enable)) { return EFI_UNSUPPORTED; } =20 // // Set PcdCpuS3DataAddress to the base address of the ACPI_CPU_DATA stru= cture // OldAcpiCpuData =3D (ACPI_CPU_DATA *) (UINTN) PcdGet64 (PcdCpuS3DataAddre= ss); =20 AcpiCpuDataEx =3D AllocateZeroPages (sizeof (ACPI_CPU_DATA_EX)); ASSERT (AcpiCpuDataEx !=3D NULL); AcpiCpuData =3D &AcpiCpuDataEx->AcpiCpuData; =20 // - // Get MP Services Protocol + // The "SMRAM at default SMBASE" feature guarantees that + // QEMU_CPUHP_CMD_GET_ARCH_ID too is available. // - Status =3D gBS->LocateProtocol ( - &gEfiMpServiceProtocolGuid, - NULL, - (VOID **)&MpServices - ); - ASSERT_EFI_ERROR (Status); + FetchPossibleApicIds =3D PcdGetBool (PcdQ35SmramAtDefaultSmbase); =20 - // - // Get the number of CPUs - // - Status =3D MpServices->GetNumberOfProcessors ( - MpServices, - &NumberOfCpus, - &NumberOfEnabledProcessors - ); - ASSERT_EFI_ERROR (Status); + if (FetchPossibleApicIds) { + NumberOfCpus =3D PcdGet32 (PcdCpuMaxLogicalProcessorNumber); + } else { + UINTN NumberOfEnabledProcessors; + + // + // Get MP Services Protocol + // + Status =3D gBS->LocateProtocol ( + &gEfiMpServiceProtocolGuid, + NULL, + (VOID **)&MpServices + ); + ASSERT_EFI_ERROR (Status); + + // + // Get the number of CPUs + // + Status =3D MpServices->GetNumberOfProcessors ( + MpServices, + &NumberOfCpus, + &NumberOfEnabledProcessors + ); + ASSERT_EFI_ERROR (Status); + } AcpiCpuData->NumberOfCpus =3D (UINT32)NumberOfCpus; =20 // // Initialize ACPI_CPU_DATA fields // AcpiCpuData->StackSize =3D PcdGet32 (PcdCpuApStackSize); AcpiCpuData->ApMachineCheckHandlerBase =3D 0; AcpiCpuData->ApMachineCheckHandlerSize =3D 0; AcpiCpuData->GdtrProfile =3D (EFI_PHYSICAL_ADDRESS)(UINTN)&AcpiCpuDataE= x->GdtrProfile; AcpiCpuData->IdtrProfile =3D (EFI_PHYSICAL_ADDRESS)(UINTN)&AcpiCpuDataE= x->IdtrProfile; AcpiCpuData->MtrrTable =3D (EFI_PHYSICAL_ADDRESS)(UINTN)&AcpiCpuDataE= x->MtrrTable; =20 // // Allocate stack space for all CPUs. // Use ACPI NVS memory type because this data will be directly used by A= Ps // in S3 resume phase in long mode. Also during S3 resume, the stack buf= fer // will only be used as scratch space. i.e. we won't read anything from = it // before we write to it, in PiSmmCpuDxeSmm. // Stack =3D AllocateAcpiNvsMemory (NumberOfCpus * AcpiCpuData->StackSize); ASSERT (Stack !=3D NULL); AcpiCpuData->StackAddress =3D (EFI_PHYSICAL_ADDRESS)(UINTN)Stack; @@ -244,58 +260,83 @@ CpuS3DataInitialize ( IdtSize =3D AcpiCpuDataEx->IdtrProfile.Limit + 1; Gdt =3D AllocateZeroPages (GdtSize + IdtSize); ASSERT (Gdt !=3D NULL); Idt =3D (VOID *)((UINTN)Gdt + GdtSize); CopyMem (Gdt, (VOID *)AcpiCpuDataEx->GdtrProfile.Base, GdtSize); CopyMem (Idt, (VOID *)AcpiCpuDataEx->IdtrProfile.Base, IdtSize); AcpiCpuDataEx->GdtrProfile.Base =3D (UINTN)Gdt; AcpiCpuDataEx->IdtrProfile.Base =3D (UINTN)Idt; =20 if (OldAcpiCpuData !=3D NULL) { AcpiCpuData->RegisterTable =3D OldAcpiCpuData->RegisterTable; AcpiCpuData->PreSmmInitRegisterTable =3D OldAcpiCpuData->PreSmmInitReg= isterTable; AcpiCpuData->ApLocation =3D OldAcpiCpuData->ApLocation; CopyMem (&AcpiCpuData->CpuStatus, &OldAcpiCpuData->CpuStatus, sizeof (= CPU_STATUS_INFORMATION)); } else { // // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable= for all CPUs // TableSize =3D 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE); RegisterTable =3D (CPU_REGISTER_TABLE *)AllocateZeroPages (TableSize); ASSERT (RegisterTable !=3D NULL); =20 + if (FetchPossibleApicIds) { + // + // Write a valid selector so that other hotplug registers can be + // accessed. + // + IoWrite32 (ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_W_CPU_SEL, 0); + // + // We'll be fetching the APIC IDs. + // + IoWrite8 (ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_W_CMD, + QEMU_CPUHP_CMD_GET_ARCH_ID); + } for (Index =3D 0; Index < NumberOfCpus; Index++) { - Status =3D MpServices->GetProcessorInfo ( - MpServices, - Index, - &ProcessorInfoBuffer - ); - ASSERT_EFI_ERROR (Status); + UINT32 InitialApicId; =20 - RegisterTable[Index].InitialApicId =3D (UINT32)ProcessorInfoBuf= fer.ProcessorId; + if (FetchPossibleApicIds) { + IoWrite32 (ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_W_CPU_SEL, + (UINT32)Index); + InitialApicId =3D IoRead32 ( + ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_RW_CMD_DATA); + } else { + Status =3D MpServices->GetProcessorInfo ( + MpServices, + Index, + &ProcessorInfoBuffer + ); + ASSERT_EFI_ERROR (Status); + InitialApicId =3D (UINT32)ProcessorInfoBuffer.ProcessorId; + } + + DEBUG ((DEBUG_VERBOSE, "%a: Index=3D%05Lu ApicId=3D0x%08x\n", __FUNC= TION__, + (UINT64)Index, InitialApicId)); + + RegisterTable[Index].InitialApicId =3D InitialApicId; RegisterTable[Index].TableLength =3D 0; RegisterTable[Index].AllocatedSize =3D 0; RegisterTable[Index].RegisterTableEntry =3D 0; =20 - RegisterTable[NumberOfCpus + Index].InitialApicId =3D (UINT32)P= rocessorInfoBuffer.ProcessorId; + RegisterTable[NumberOfCpus + Index].InitialApicId =3D InitialAp= icId; RegisterTable[NumberOfCpus + Index].TableLength =3D 0; RegisterTable[NumberOfCpus + Index].AllocatedSize =3D 0; RegisterTable[NumberOfCpus + Index].RegisterTableEntry =3D 0; } AcpiCpuData->RegisterTable =3D (EFI_PHYSICAL_ADDRESS)(UINTN)= RegisterTable; AcpiCpuData->PreSmmInitRegisterTable =3D (EFI_PHYSICAL_ADDRESS)(UINTN)= (RegisterTable + NumberOfCpus); } =20 // // Set PcdCpuS3DataAddress to the base address of the ACPI_CPU_DATA stru= cture // Status =3D PcdSet64S (PcdCpuS3DataAddress, (UINT64)(UINTN)AcpiCpuData); ASSERT_EFI_ERROR (Status); =20 // // Register EFI_END_OF_DXE_EVENT_GROUP_GUID event. // The notification function allocates StartupVector and saves MTRRs for= ACPI_CPU_DATA // Status =3D gBS->CreateEventEx ( EVT_NOTIFY_SIGNAL, TPL_CALLBACK, CpuS3DataOnEndOfDxe, --=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 (#54958): https://edk2.groups.io/g/devel/message/54958 Mute This Topic: https://groups.io/mt/71575192/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-