From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by mx.groups.io with SMTP id smtpd.web09.14414.1583157499220172454 for ; Mon, 02 Mar 2020 05:58:19 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=TxJLQ5tv; spf=pass (domain: linaro.org, ip: 209.85.221.67, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f67.google.com with SMTP id z15so12763543wrl.1 for ; Mon, 02 Mar 2020 05:58:19 -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=agcGjtCyD0/itRc2jUicIYg8JEBeXw8W+7lw6FbyBc8=; b=TxJLQ5tvvOJ3Uo9PeC4L5fbSAuJ3c6a/0nU8hi7lIJQuaoLGKdFg4zqIzDRBZC2e1l GqJXYI2mjZBrfKxmLdZY1CXVLmrvY7v+SK1DG9/1sqrweCFpWfWw6yUySTuXBDhlfAWg o7vxJohD8Rc3LwyiPqDu2008UadAqjTKgy99N88Qtw+g4YNmG0zRdkl+Pr4sMUCrN5kV Gx9V8fez6iPWSKF55UFj/eCZwBJiMGdLdoFesABfVRC7CW1QuexWNoVjWQGvGnL/AnMt qOoCdNb3p4x8a0MjbuOO6okMiWZhaMLw+l7iID0Z3NpKZrh4kw6Jh6e/yfLSgPGOp35L thFg== 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=agcGjtCyD0/itRc2jUicIYg8JEBeXw8W+7lw6FbyBc8=; b=sM6FYFspb23y3MxyjiY6d15vSGx8lhhUM8WyfntgfSXt5MfHqwIStmuICgVsk34i75 6+bdLwBwP68gZw4o+YykgYFyhi+0euzr3/Q2TP64vIbd5yF6M/ErAAgBn2q/zR0Iw5f8 2zAK+Uaigzc7w9qPYdvAykUTZz9HxtPuM1BSW5R4y62lPZMuwZTvaQIRZKf9/0fTGSmX GgvFe9ldJxTv2iEOIFlWYVeh744At5B4Q0Hsf4/TwgN6wvrpsIJTF49gjIJ5S690vhtc DEbri/pLwgIGm5Q96hHHTKFtRFSl8CjSWn81Vjm1C21khKtR84QHhT593zqTgi0fkRkl 9zAQ== X-Gm-Message-State: ANhLgQ2PS3uOEjQ0pLG7JSlE4gE68rnCO3hFHip9WnOAeoV0prxw8J7r IqAZnMMPt49UGMKGJbGnwf/lwkAtrJ+HyaGs93ThXg== X-Google-Smtp-Source: ADFU+vv6knK/sFl4h5YAAQfuyf3BnqFdtEz2JYpPZb0meVoj6COex/DmTNWq+4nI1CPt7VkKH9ssP3PfSOJLlNy0GOY= X-Received: by 2002:a5d:6051:: with SMTP id j17mr2534154wrt.151.1583157497726; Mon, 02 Mar 2020 05:58:17 -0800 (PST) MIME-Version: 1.0 References: <20200226221156.29589-1-lersek@redhat.com> <20200226221156.29589-11-lersek@redhat.com> In-Reply-To: <20200226221156.29589-11-lersek@redhat.com> From: "Ard Biesheuvel" Date: Mon, 2 Mar 2020 14:58:06 +0100 Message-ID: Subject: Re: [PATCH v2 10/16] OvmfPkg/CpuHotplugSmm: collect CPUs with events 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: > > Call QemuCpuhpCollectApicIds() in the root MMI handler. The APIC IDs of > the hotplugged CPUs will be used for several purposes in subsequent > patches. > > For calling QemuCpuhpCollectApicIds(), pre-allocate both of its output > arrays "PluggedApicIds" and "ToUnplugApicIds" in the driver's entry point > function. The allocation size is dictated by the possible CPU count, whic= h > we fetch from "CPU_HOT_PLUG_DATA.ArrayLength". > > The CPU_HOT_PLUG_DATA structure in SMRAM is an out-of-band information > channel between this driver and PiSmmCpuDxeSmm, underlying > EFI_SMM_CPU_SERVICE_PROTOCOL. > > In order to consume "CPU_HOT_PLUG_DATA.ArrayLength", extend the driver's > DEPEX to EFI_SMM_CPU_SERVICE_PROTOCOL. PiSmmCpuDxeSmm stores the address > of CPU_HOT_PLUG_DATA to "PcdCpuHotPlugDataAddress", before it produces > EFI_SMM_CPU_SERVICE_PROTOCOL. > > Stash the protocol at once, as it will be needed later. > > 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/CpuHotplugSmm/CpuHotplugSmm.inf | 7 +- > OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 111 +++++++++++++++++++- > 2 files changed, 115 insertions(+), 3 deletions(-) > > diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf b/OvmfPkg/CpuHotplug= Smm/CpuHotplugSmm.inf > index ab690a9e5e20..31c1ee1c9f6d 100644 > --- a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf > +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf > @@ -11,41 +11,46 @@ [Defines] > PI_SPECIFICATION_VERSION =3D 0x00010046 #= PI-1.7.0 > BASE_NAME =3D CpuHotplugSmm > FILE_GUID =3D 84EEA114-C6BE-4445-8F90-51D97863E363 > MODULE_TYPE =3D DXE_SMM_DRIVER > ENTRY_POINT =3D CpuHotplugEntry > > # > # The following information is for reference only and not required by th= e build > # tools. > # > # VALID_ARCHITECTURES =3D IA32 X64 > # > > [Sources] > ApicId.h > CpuHotplug.c > QemuCpuhp.c > QemuCpuhp.h > > [Packages] > MdePkg/MdePkg.dec > OvmfPkg/OvmfPkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > > [LibraryClasses] > BaseLib > DebugLib > MmServicesTableLib > PcdLib > + SafeIntLib > UefiDriverEntryPoint > > [Protocols] > gEfiMmCpuIoProtocolGuid ## C= ONSUMES > + gEfiSmmCpuServiceProtocolGuid ## C= ONSUMES > > [Pcd] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress ## C= ONSUMES > gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase ## C= ONSUMES > > [FeaturePcd] > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire ## C= ONSUMES > > [Depex] > - gEfiMmCpuIoProtocolGuid > + gEfiMmCpuIoProtocolGuid AND > + gEfiSmmCpuServiceProtocolGuid > diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/C= puHotplug.c > index 5df8c689c63a..42e023cb85c0 100644 > --- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c > +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c > @@ -1,46 +1,76 @@ > /** @file > Root SMI handler for VCPU hotplug SMIs. > > Copyright (c) 2020, Red Hat, Inc. > > SPDX-License-Identifier: BSD-2-Clause-Patent > **/ > > +#include // CPU_HOT_PLUG_DATA > #include // ICH9_APM_CNT > #include // QEMU_CPUHP_CMD_GET_PENDI= NG > #include // CpuDeadLoop() > #include // ASSERT() > #include // gMmst > #include // PcdGetBool() > +#include // SafeUintnSub() > #include // EFI_MM_CPU_IO_PROTOCOL > +#include // EFI_SMM_CPU_SERVICE_PROT= OCOL > #include // EFI_STATUS > > +#include "ApicId.h" // APIC_ID > #include "QemuCpuhp.h" // QemuCpuhpWriteCpuSelecto= r() > > // > // We use this protocol for accessing IO Ports. > // > STATIC EFI_MM_CPU_IO_PROTOCOL *mMmCpuIo; > // > +// The following protocol is used to report the addition or removal of a= CPU to > +// the SMM CPU driver (PiSmmCpuDxeSmm). > +// > +STATIC EFI_SMM_CPU_SERVICE_PROTOCOL *mMmCpuService; > +// > +// This structure is a communication side-channel between the > +// EFI_SMM_CPU_SERVICE_PROTOCOL consumer (i.e., this driver) and provide= r > +// (i.e., PiSmmCpuDxeSmm). > +// > +STATIC CPU_HOT_PLUG_DATA *mCpuHotPlugData; > +// > +// SMRAM arrays for fetching the APIC IDs of processors with pending eve= nts (of > +// known event types), for the time of just one MMI. > +// > +// The lifetimes of these arrays match that of this driver only because = we > +// don't want to allocate SMRAM at OS runtime, and potentially fail (or > +// fragment the SMRAM map). > +// > +// These arrays provide room for ("possible CPU count" minus one) APIC I= Ds > +// each, as we don't expect every possible CPU to appear, or disappear, = in a > +// single MMI. The numbers of used (populated) elements in the arrays ar= e > +// determined on every MMI separately. > +// > +STATIC APIC_ID *mPluggedApicIds; > +STATIC APIC_ID *mToUnplugApicIds; > +// > // Represents the registration of the CPU Hotplug MMI handler. > // > STATIC EFI_HANDLE mDispatchHandle; > > > /** > CPU Hotplug MMI handler function. > > This is a root MMI handler. > > @param[in] DispatchHandle The unique handle assigned to this hand= ler by > EFI_MM_SYSTEM_TABLE.MmiHandlerRegister(= ). > > @param[in] Context Context passed in by > EFI_MM_SYSTEM_TABLE.MmiManage(). Due to > CpuHotplugMmi() being a root MMI handle= r, > Context is ASSERT()ed to be NULL. > > @param[in,out] CommBuffer Ignored, due to CpuHotplugMmi() being a= root > MMI handler. > > @param[in,out] CommBufferSize Ignored, due to CpuHotplugMmi() being a= root > @@ -65,162 +95,239 @@ STATIC EFI_HANDLE mDispatchHandle; > but other handlers should = still > be called. > > @retval EFI_WARN_INTERRUPT_SOURCE_PENDING The MMI source is still pe= nding, > and other handlers should = still > be called. > > @retval EFI_INTERRUPT_PENDING The MMI source could not b= e > quiesced. > **/ > STATIC > EFI_STATUS > EFIAPI > CpuHotplugMmi ( > IN EFI_HANDLE DispatchHandle, > IN CONST VOID *Context OPTIONAL, > IN OUT VOID *CommBuffer OPTIONAL, > IN OUT UINTN *CommBufferSize OPTIONAL > ) > { > EFI_STATUS Status; > UINT8 ApmControl; > + UINT32 PluggedCount; > + UINT32 ToUnplugCount; > > // > // Assert that we are entering this function due to our root MMI handl= er > // registration. > // > ASSERT (DispatchHandle =3D=3D mDispatchHandle); > // > // When MmiManage() is invoked to process root MMI handlers, the calle= r (the > // MM Core) is expected to pass in a NULL Context. MmiManage() then pa= sses > // the same NULL Context to individual handlers. > // > ASSERT (Context =3D=3D NULL); > // > // Read the MMI command value from the APM Control Port, to see if thi= s is an > // MMI we should care about. > // > Status =3D mMmCpuIo->Io.Read (mMmCpuIo, MM_IO_UINT8, ICH9_APM_CNT, 1, > &ApmControl); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "%a: failed to read ICH9_APM_CNT: %r\n", __FUNC= TION__, > Status)); > // > // We couldn't even determine if the MMI was for us or not. > // > goto Fatal; > } > > if (ApmControl !=3D ICH9_APM_CNT_CPU_HOTPLUG) { > // > // The MMI is not for us. > // > return EFI_WARN_INTERRUPT_SOURCE_QUIESCED; > } > > + // > + // Collect the CPUs with pending events. > + // > + Status =3D QemuCpuhpCollectApicIds ( > + mMmCpuIo, > + mCpuHotPlugData->ArrayLength, // PossibleCpuCount > + mCpuHotPlugData->ArrayLength - 1, // ApicIdCount > + mPluggedApicIds, > + &PluggedCount, > + mToUnplugApicIds, > + &ToUnplugCount > + ); > + if (EFI_ERROR (Status)) { > + goto Fatal; > + } > + if (ToUnplugCount > 0) { > + DEBUG ((DEBUG_ERROR, "%a: hot-unplug is not supported yet\n", > + __FUNCTION__)); > + goto Fatal; > + } > + > // > // We've handled this MMI. > // > return EFI_SUCCESS; > > Fatal: > ASSERT (FALSE); > CpuDeadLoop (); > // > // We couldn't handle this MMI. > // > return EFI_INTERRUPT_PENDING; > } > > > // > // Entry point function of this driver. > // > EFI_STATUS > EFIAPI > CpuHotplugEntry ( > IN EFI_HANDLE ImageHandle, > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > EFI_STATUS Status; > + UINTN Size; > > // > // This module should only be included when SMM support is required. > // > ASSERT (FeaturePcdGet (PcdSmmSmramRequire)); > // > // This driver depends on the dynamically detected "SMRAM at default S= MBASE" > // feature. > // > if (!PcdGetBool (PcdQ35SmramAtDefaultSmbase)) { > return EFI_UNSUPPORTED; > } > > // > // Errors from here on are fatal; we cannot allow the boot to proceed = if we > // can't set up this driver to handle CPU hotplug. > // > // First, collect the protocols needed later. All of these protocols a= re > // listed in our module DEPEX. > // > Status =3D gMmst->MmLocateProtocol (&gEfiMmCpuIoProtocolGuid, > NULL /* Registration */, (VOID **)&mMmCpuIo); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "%a: locate MmCpuIo: %r\n", __FUNCTION__, Statu= s)); > goto Fatal; > } > + Status =3D gMmst->MmLocateProtocol (&gEfiSmmCpuServiceProtocolGuid, > + NULL /* Registration */, (VOID **)&mMmCpuService); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: locate MmCpuService: %r\n", __FUNCTION__, > + Status)); > + goto Fatal; > + } > + > + // > + // Our DEPEX on EFI_SMM_CPU_SERVICE_PROTOCOL guarantees that PiSmmCpuD= xeSmm > + // has pointed PcdCpuHotPlugDataAddress to CPU_HOT_PLUG_DATA in SMRAM. > + // > + mCpuHotPlugData =3D (VOID *)(UINTN)PcdGet64 (PcdCpuHotPlugDataAddress)= ; > + if (mCpuHotPlugData =3D=3D NULL) { > + Status =3D EFI_NOT_FOUND; > + DEBUG ((DEBUG_ERROR, "%a: CPU_HOT_PLUG_DATA: %r\n", __FUNCTION__, St= atus)); > + goto Fatal; > + } > + // > + // If the possible CPU count is 1, there's nothing for this driver to = do. > + // > + if (mCpuHotPlugData->ArrayLength =3D=3D 1) { > + return EFI_UNSUPPORTED; > + } > + // > + // Allocate the data structures that depend on the possible CPU count. > + // > + if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, &Size= )) || > + RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Size, &Size))) { > + Status =3D EFI_ABORTED; > + DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_PLUG_DATA\n", __FUNCTION__= )); > + goto Fatal; > + } > + Status =3D gMmst->MmAllocatePool (EfiRuntimeServicesData, Size, > + (VOID **)&mPluggedApicIds); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Sta= tus)); > + goto Fatal; > + } > + Status =3D gMmst->MmAllocatePool (EfiRuntimeServicesData, Size, > + (VOID **)&mToUnplugApicIds); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Sta= tus)); > + goto ReleasePluggedApicIds; > + } > > // > // Sanity-check the CPU hotplug interface. > // > // Both of the following features are part of QEMU 5.0, introduced pri= marily > // in commit range 3e08b2b9cb64..3a61c8db9d25: > // > // (a) the QEMU_CPUHP_CMD_GET_ARCH_ID command of the modern CPU hotplu= g > // interface, > // > // (b) the "SMRAM at default SMBASE" feature. > // > // From these, (b) is restricted to 5.0+ machine type versions, while = (a) > // does not depend on machine type version. Because we ensured the str= icter > // condition (b) through PcdQ35SmramAtDefaultSmbase above, the (a) > // QEMU_CPUHP_CMD_GET_ARCH_ID command must now be available too. While= we > // can't verify the presence of precisely that command, we can still v= erify > // (sanity-check) that the modern interface is active, at least. > // > // Consult the "Typical usecases | Detecting and enabling modern CPU h= otplug > // interface" section in QEMU's "docs/specs/acpi_cpu_hotplug.txt", on = the > // following. > // > QemuCpuhpWriteCpuSelector (mMmCpuIo, 0); > QemuCpuhpWriteCpuSelector (mMmCpuIo, 0); > QemuCpuhpWriteCommand (mMmCpuIo, QEMU_CPUHP_CMD_GET_PENDING); > if (QemuCpuhpReadCommandData2 (mMmCpuIo) !=3D 0) { > Status =3D EFI_NOT_FOUND; > DEBUG ((DEBUG_ERROR, "%a: modern CPU hotplug interface: %r\n", > __FUNCTION__, Status)); > - goto Fatal; > + goto ReleaseToUnplugApicIds; > } > > // > // Register the handler for the CPU Hotplug MMI. > // > Status =3D gMmst->MmiHandlerRegister ( > CpuHotplugMmi, > NULL, // HandlerType: root MMI handler > &mDispatchHandle > ); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "%a: MmiHandlerRegister(): %r\n", __FUNCTION__, > Status)); > - goto Fatal; > + goto ReleaseToUnplugApicIds; > } > > return EFI_SUCCESS; > > +ReleaseToUnplugApicIds: > + gMmst->MmFreePool (mToUnplugApicIds); > + mToUnplugApicIds =3D NULL; > + > +ReleasePluggedApicIds: > + gMmst->MmFreePool (mPluggedApicIds); > + mPluggedApicIds =3D NULL; > + > Fatal: > ASSERT (FALSE); > CpuDeadLoop (); > return Status; > } > -- > 2.19.1.3.g30247aa5d201 > >