From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by mx.groups.io with SMTP id smtpd.web09.14082.1583156052574987932 for ; Mon, 02 Mar 2020 05:34:13 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=hNa1Sreu; spf=pass (domain: linaro.org, ip: 209.85.128.68, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wm1-f68.google.com with SMTP id m3so10689888wmi.0 for ; Mon, 02 Mar 2020 05:34:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=p857dpSmRRGyf8iCoRKoIbRO9ZLijKpAnaoU68AOW0o=; b=hNa1Sreu/hajnzIofJpk7Vxqd504B3Xx1GcgogYf/vJ6cpcaArPdcNVJNBUvOQf1vB ohkRxQxby6uFCfzty99cppp3NHZ8AoCb5VKlgnZmv7E4MVHfjIJuEGFkCHCmbv90FpYP rgqu1rsNV/FlGQ0iRvdlmZi3yDzV49o48o1v0/DextEeg1HSkDqek34mv7JeyDHkdBdr FI3kmxas3HK+EeDGp+9tb1BpAo3xcScOeL2kkdOOfgRVMwiatsLrTY6RtN/vjAOq97h6 oeLDwxE6qWj96IpVlzwev2/6L5K++TWqgdd7E7hjzkBTafX70njMMITqh+UagvNsfw/F gm+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=p857dpSmRRGyf8iCoRKoIbRO9ZLijKpAnaoU68AOW0o=; b=C7XyJ2JPm/zjPfTra7rcU1YT3BWWrhfhZRamQwzanoM9blUYMBEcS/1hjQ3iNJGwWy xBn9+iT4e5Ft550qcxpoM1hGT092bLS8XvIIF9N9qPtTA/9x/6qlrU8YJGlCmWDcEeTo 0RQbmTAjUxVb41P5x+/elVnFuzUfVg8k0mUYksEGMarGg1+oWNW40skC7vUNkD1uq6Ta qJs/UBw2TkMGzf49HCFbWL7Rt4adinGbXDJYKJ4ourX5gGXyMyNP3culzkXYjy4gEuwi 9PblVpTpMxN/tZg1yo2Fnb6X/ZM/SwZC+WmMQaEy3+9YuUw8GM333fkKFqXCfelphR40 ARpw== X-Gm-Message-State: APjAAAUwhmrZ4ZgPNs8ShuA0Fa7981tntJxGzuTjNxksG0JVZhwRaiXA +Jl/r7HNc0QxFaGHGayO1DYWk8PHcXenNny2wmKRfA== X-Google-Smtp-Source: APXvYqwUULFbhPf6CDxiOr0int3sctRWlOymWX/Y+e+12x4p5jTGWlG/Wd+DnBeph9Ve17+/vjKzXaTtp/y5ahaug9o= X-Received: by 2002:a7b:cb93:: with SMTP id m19mr20845629wmi.133.1583156050157; Mon, 02 Mar 2020 05:34:10 -0800 (PST) MIME-Version: 1.0 References: <20200226221156.29589-1-lersek@redhat.com> <20200226221156.29589-6-lersek@redhat.com> In-Reply-To: <20200226221156.29589-6-lersek@redhat.com> From: "Ard Biesheuvel" Date: Mon, 2 Mar 2020 14:33:59 +0100 Message-ID: Subject: Re: [PATCH v2 05/16] OvmfPkg: enable CPU hotplug support in PiSmmCpuDxeSmm To: Laszlo Ersek Cc: edk2-devel-groups-io , Igor Mammedov , Jiewen Yao , Jordan Justen , Michael Kinney , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 26 Feb 2020 at 23:12, Laszlo Ersek wrote: > > Set "PcdCpuHotPlugSupport" to TRUE, when OVMF is built with SMM_REQUIRE. > Consequences: > > (1) In PiCpuSmmEntry() [UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c], > resources are allocated and populated in advance for all possible > (i.e., potentially hot-added) processors, rather than only the > processors present at boot. > > The possible count (called "mMaxNumberOfCpus") is set from > "PcdCpuMaxLogicalProcessorNumber"; we set the latter in > OvmfPkg/PlatformPei. (Refer to commit 83357313dd67, > "OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU > hotplug", 2020-01-29). > > (2) The AddProcessor() and RemoveProcessor() member functions of > EFI_SMM_CPU_SERVICE_PROTOCOL, implemented in > "UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c", are no longer > short-circuited to EFI_UNSUPPORTED. > > We'll rely on these functions in the CPU hotplug SMI handler, in a > subsequent patch. > > (3) In PiCpuSmmEntry(), the address of the CPU_HOT_PLUG_DATA structure (i= n > SMRAM) is exposed via the dynamic-only "PcdCpuHotPlugDataAddress". > > This structure is an information channel between the CPU hotplug SMI > handler, and EFI_SMM_CPU_SERVICE_PROTOCOL. Namely, at the first > "Index" where the following equality holds: > > CPU_HOT_PLUG_DATA.ApicId[Index] =3D=3D INVALID_APIC_ID > > a hot-plugged CPU can be accepted, with the steps below: > > (3.1) The hotplug SMI handler has to overwrite INVALID_APIC_ID with the > new CPU's APIC ID. > > (3.2) The new CPU's SMBASE has to be relocated to: > > CPU_HOT_PLUG_DATA.SmBase[Index] > > (which was precomputed in step (1) above). > > (3.3) The hotplug SMI handler is supposed to call > EFI_SMM_CPU_SERVICE_PROTOCOL.AddProcessor(). > > Note: we need not spell out "PcdCpuHotPlugDataAddress" in the > [PcdsDynamicDefault] sections of the OVMF DSC files, just so the PCD > become dynamically settable. That's because "UefiCpuPkg.dec" declares thi= s > PCD with [PcdsDynamic, PcdsDynamicEx] access methods *only*. > > 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: > > - 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/OvmfPkgIa32.dsc | 1 + > OvmfPkg/OvmfPkgIa32X64.dsc | 1 + > OvmfPkg/OvmfPkgX64.dsc | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 60d8af185b9c..8c065ca7cec9 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -414,44 +414,45 @@ [LibraryClasses.common.SMM_CORE] > !endif > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > > ########################################################################= ######## > # > # Pcd Section - list of all EDK II PCD Entries defined by this Platform. > # > ########################################################################= ######## > [PcdsFeatureFlag] > gEfiMdeModulePkgTokenSpaceGuid.PcdHiiOsRuntimeSupport|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE > !ifdef $(CSM_ENABLE) > gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE > !endif > !if $(SMM_REQUIRE) =3D=3D TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > + gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > > [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|F= ALSE > gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > !if ($(FD_SIZE_IN_KB) =3D=3D 1024) || ($(FD_SIZE_IN_KB) =3D=3D 2048) > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > !if $(NETWORK_TLS_ENABLE) =3D=3D FALSE > # match PcdFlashNvStorageVariableSize purely for convenience > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > !endif > !endif > !if $(FD_SIZE_IN_KB) =3D=3D 4096 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > !if $(NETWORK_TLS_ENABLE) =3D=3D FALSE > # match PcdFlashNvStorageVariableSize purely for convenience > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000 > !endif > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index be6bc7bd88a7..944b785e61a9 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -419,44 +419,45 @@ [LibraryClasses.common.SMM_CORE] > !endif > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > > ########################################################################= ######## > # > # Pcd Section - list of all EDK II PCD Entries defined by this Platform. > # > ########################################################################= ######## > [PcdsFeatureFlag] > gEfiMdeModulePkgTokenSpaceGuid.PcdHiiOsRuntimeSupport|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE > !ifdef $(CSM_ENABLE) > gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE > !endif > !if $(SMM_REQUIRE) =3D=3D TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > + gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > > [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|F= ALSE > gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > !if ($(FD_SIZE_IN_KB) =3D=3D 1024) || ($(FD_SIZE_IN_KB) =3D=3D 2048) > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > !if $(NETWORK_TLS_ENABLE) =3D=3D FALSE > # match PcdFlashNvStorageVariableSize purely for convenience > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > !endif > !endif > !if $(FD_SIZE_IN_KB) =3D=3D 4096 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > !if $(NETWORK_TLS_ENABLE) =3D=3D FALSE > # match PcdFlashNvStorageVariableSize purely for convenience > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000 > !endif > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index e258c474b60d..8de0f7179784 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -419,44 +419,45 @@ [LibraryClasses.common.SMM_CORE] > !endif > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > > ########################################################################= ######## > # > # Pcd Section - list of all EDK II PCD Entries defined by this Platform. > # > ########################################################################= ######## > [PcdsFeatureFlag] > gEfiMdeModulePkgTokenSpaceGuid.PcdHiiOsRuntimeSupport|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE > gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE > !ifdef $(CSM_ENABLE) > gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE > !endif > !if $(SMM_REQUIRE) =3D=3D TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > + gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > > [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|F= ALSE > gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > !if ($(FD_SIZE_IN_KB) =3D=3D 1024) || ($(FD_SIZE_IN_KB) =3D=3D 2048) > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > !if $(NETWORK_TLS_ENABLE) =3D=3D FALSE > # match PcdFlashNvStorageVariableSize purely for convenience > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > !endif > !endif > !if $(FD_SIZE_IN_KB) =3D=3D 4096 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > !if $(NETWORK_TLS_ENABLE) =3D=3D FALSE > # match PcdFlashNvStorageVariableSize purely for convenience > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000 > !endif > -- > 2.19.1.3.g30247aa5d201 > >