From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web12.6295.1615889532725133020 for ; Tue, 16 Mar 2021 03:12:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=R+GmHHw8; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1615889531; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gUlDXzUI3rCZ4CFRXq44QKcHitQFGjIUdJb+EqvUODo=; b=R+GmHHw8dwRvgcO4gIoyLr0h6PIRHc+OffjZ49slvbM4TERBpuHFGF7MI3D9ZBzdkxkCoe azivOQ3cqjX9Nnvr8rFKuuQsPqlNI9ZXLFwB+mOlLtqm3ZWpC5glSQFO3HM2gIzKf1tx4n cFvBEdmF9VB2C3C82Jg4tcPvdasTOI8= 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-109-uY5g7JQYP-6pNKYNuIiDEA-1; Tue, 16 Mar 2021 06:12:08 -0400 X-MC-Unique: uY5g7JQYP-6pNKYNuIiDEA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A3800107ACCD; Tue, 16 Mar 2021 10:12:06 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-138.ams2.redhat.com [10.36.114.138]) by smtp.corp.redhat.com (Postfix) with ESMTP id C09BD6C355; Tue, 16 Mar 2021 10:12:04 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v9 06/10] OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state To: devel@edk2.groups.io, ankur.a.arora@oracle.com Cc: imammedo@redhat.com, boris.ostrovsky@oracle.com, Jordan Justen , Ard Biesheuvel , Aaron Young References: <20210312062656.2477515-1-ankur.a.arora@oracle.com> <20210312062656.2477515-7-ankur.a.arora@oracle.com> From: "Laszlo Ersek" Message-ID: <11b6e119-ac13-d985-6874-8577e946f09d@redhat.com> Date: Tue, 16 Mar 2021 11:12:03 +0100 MIME-Version: 1.0 In-Reply-To: <20210312062656.2477515-7-ankur.a.arora@oracle.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 03/12/21 07:26, Ankur Arora wrote: > Init CPU_HOT_EJECT_DATA, which will be used to share CPU ejection > state between SmmCpuFeaturesLib (via PiSmmCpuDxeSmm) and CpuHotPlugSmm. > > The init happens via SmmCpuFeaturesSmmRelocationComplete(), and so it > will run as part of the PiSmmCpuDxeSmm entry point function, > PiCpuSmmEntry(). Once inited, CPU_HOT_EJECT_DATA is exposed via > PcdCpuHotEjectDataAddress. > > The CPU hot-eject handler (CPU_HOT_EJECT_DATA->Handler) is setup when > there is an ejection request via CpuHotplugSmm. > > Cc: Laszlo Ersek > Cc: Jordan Justen > Cc: Ard Biesheuvel > Cc: Igor Mammedov > Cc: Boris Ostrovsky > Cc: Aaron Young > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > Signed-off-by: Ankur Arora > --- > > Notes: > Addresses the following comments from v8: > > (1) Remove line before the "if (MaxNumberofCpus == 1)" check. > (3) Fixup the space around "||". > (2,6) Simplify the three SafeInt multiplication into the ones suggested > by Laszlo. > (4) Get rid of the mixed sizeof(mCpuHotEjectData->QemuSelectorMap[0]) and > sizeof(UINT64) in favour of UINT64 everywhere. I was planning to use > the first, but describing the alignment needed is easier in terms of the > second. > Also, as Laszlo's comments on v8-patch-9 mention, we don't really need > this alignment for correctness reasons. This patch retains it, so we > don't pay access penalty for unaligned access. > (5) Change alignment from UINT64 to UINT64-1. > (7) Use the more idiomatic ALIGN_POINTER instead of ALIGN_VALUE. > (8) RETURN_ERROR -> ASSERT_RETURN_ERROR. > > .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 ++ > .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 77 ++++++++++++++++++++++ > 2 files changed, 81 insertions(+) Reviewed-by: Laszlo Ersek Thanks! Laszlo > > diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > index 97a10afb6e27..8a426a4c10fb 100644 > --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > @@ -30,9 +30,13 @@ [LibraryClasses] > BaseMemoryLib > DebugLib > MemEncryptSevLib > + MemoryAllocationLib > PcdLib > + SafeIntLib > SmmServicesTableLib > UefiBootServicesTableLib > > [Pcd] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber > + gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress > gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase > diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > index 7ef7ed98342e..5c025bc717c3 100644 > --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > @@ -11,10 +11,13 @@ > #include > #include > #include > +#include > #include > +#include > #include > #include > #include > +#include > #include > #include > #include > @@ -171,6 +174,77 @@ SmmCpuFeaturesHookReturnFromSmm ( > return OriginalInstructionPointer; > } > > +STATIC CPU_HOT_EJECT_DATA *mCpuHotEjectData = NULL; > + > +/** > + Initialize mCpuHotEjectData if PcdCpuMaxLogicalProcessorNumber > 1. > + > + Also setup the corresponding PcdCpuHotEjectDataAddress. > +**/ > +STATIC > +VOID > +InitCpuHotEjectData ( > + VOID > + ) > +{ > + UINTN Size; > + UINT32 Idx; > + UINT32 MaxNumberOfCpus; > + RETURN_STATUS PcdStatus; > + > + MaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber); > + if (MaxNumberOfCpus == 1) { > + return; > + } > + > + // > + // We allocate CPU_HOT_EJECT_DATA and CPU_HOT_EJECT_DATA->QemuSelectorMap[] > + // in a single allocation, and explicitly align the QemuSelectorMap[] (which > + // is a UINT64 array) at its natural boundary. > + // Accordingly, allocate: > + // sizeof(*mCpuHotEjectData) + (MaxNumberOfCpus * sizeof(UINT64)) > + // and, add sizeof(UINT64) - 1 to use as padding if needed. > + // > + > + if (RETURN_ERROR (SafeUintnMult (MaxNumberOfCpus, sizeof (UINT64), &Size)) || > + RETURN_ERROR (SafeUintnAdd (Size, sizeof (*mCpuHotEjectData), &Size)) || > + RETURN_ERROR (SafeUintnAdd (Size, sizeof (UINT64) - 1, &Size))) { > + DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_EJECT_DATA\n", __FUNCTION__)); > + goto Fatal; > + } > + > + mCpuHotEjectData = AllocatePool (Size); > + if (mCpuHotEjectData == NULL) { > + ASSERT (mCpuHotEjectData != NULL); > + goto Fatal; > + } > + > + mCpuHotEjectData->Handler = NULL; > + mCpuHotEjectData->ArrayLength = MaxNumberOfCpus; > + > + mCpuHotEjectData->QemuSelectorMap = ALIGN_POINTER (mCpuHotEjectData + 1, > + sizeof (UINT64)); > + // > + // We use mCpuHotEjectData->QemuSelectorMap to map > + // ProcessorNum -> QemuSelector. Initialize to invalid values. > + // > + for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) { > + mCpuHotEjectData->QemuSelectorMap[Idx] = CPU_EJECT_QEMU_SELECTOR_INVALID; > + } > + > + // > + // Expose address of CPU Hot eject Data structure > + // > + PcdStatus = PcdSet64S (PcdCpuHotEjectDataAddress, > + (UINTN)(VOID *)mCpuHotEjectData); > + ASSERT_RETURN_ERROR (PcdStatus); > + > + return; > + > +Fatal: > + CpuDeadLoop (); > +} > + > /** > Hook point in normal execution mode that allows the one CPU that was elected > as monarch during System Management Mode initialization to perform additional > @@ -188,6 +262,9 @@ SmmCpuFeaturesSmmRelocationComplete ( > UINTN MapPagesBase; > UINTN MapPagesCount; > > + > + InitCpuHotEjectData (); > + > if (!MemEncryptSevIsEnabled ()) { > return; > } >