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.web08.1243.1610698672854182891 for ; Fri, 15 Jan 2021 00:17:53 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=epzPJeaQ; 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=1610698672; 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=kiCyHgXs+cszVOdFNBPDdXQPKxuV7am7NJuA7V/I0As=; b=epzPJeaQl7ElVljXv8HZuNeeHLqxUlIeWwAPETvmeIOfZzxw3eAG+PUUuatfvMNsJJX/2u zgcnnHUFeVaIzDagI6aS+xbvryyNRl7BN2NeZ9oDgMbupXXNedsl2ihaZdHQ427v/k0Cie ArYDW7HJOdf0bbbXNGDt11DbKzOtJJ0= 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-63-RHAD1rQ3MDC4GUFtvavW2w-1; Fri, 15 Jan 2021 03:17:48 -0500 X-MC-Unique: RHAD1rQ3MDC4GUFtvavW2w-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 52F818066E0; Fri, 15 Jan 2021 08:17:46 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-223.ams2.redhat.com [10.36.112.223]) by smtp.corp.redhat.com (Postfix) with ESMTP id 504205D752; Fri, 15 Jan 2021 08:17:44 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 04/10] UefiCpuPkg: add CPU ejection support To: devel@edk2.groups.io, ankur.a.arora@oracle.com Cc: imammedo@redhat.com, Eric Dong , Ray Ni , Rahul Kumar , Boris Ostrovsky , Aaron Young References: <20210115074533.277448-1-ankur.a.arora@oracle.com> <20210115074533.277448-5-ankur.a.arora@oracle.com> From: "Laszlo Ersek" Message-ID: <6d95cb8b-1ae9-91c8-4cae-f34ee2bade37@redhat.com> Date: Fri, 15 Jan 2021 09:17:43 +0100 MIME-Version: 1.0 In-Reply-To: <20210115074533.277448-5-ankur.a.arora@oracle.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 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 Hi Ankur, On 01/15/21 08:45, Ankur Arora wrote: > Define CPU_HOT_EJECT_DATA and add PCD PcdCpuHotEjectDataAddress, > which would be used to share CPU ejection state between > PiSmmCpuDxeSmm and OvmfPkg/CpuHotPlugSmm. > > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > Cc: Boris Ostrovsky > Cc: Aaron Young > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 > Signed-off-by: Ankur Arora > --- > UefiCpuPkg/Include/CpuHotPlugData.h | 21 +++++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + > UefiCpuPkg/UefiCpuPkg.dec | 5 +++++ > UefiCpuPkg/UefiCpuPkg.uni | 4 ++++ > 4 files changed, 31 insertions(+) > > diff --git a/UefiCpuPkg/Include/CpuHotPlugData.h b/UefiCpuPkg/Include/CpuHotPlugData.h > index 6321a149fa52..86f964550655 100644 > --- a/UefiCpuPkg/Include/CpuHotPlugData.h > +++ b/UefiCpuPkg/Include/CpuHotPlugData.h > @@ -2,6 +2,7 @@ > Definition for a structure sharing information for CPU hot plug. > > Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2021, Oracle Corporation.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -24,4 +25,24 @@ typedef struct { > UINT32 SmrrSize; > } CPU_HOT_PLUG_DATA; > > +typedef > +VOID > +(EFIAPI *CPU_HOT_EJECT_FN)( > + IN UINTN ProcessorNum > + ); > + > +#define CPU_EJECT_INVALID (MAX_UINT64) > +#define CPU_EJECT_WORKER (MAX_UINT64-1) > + > +#define CPU_HOT_EJECT_DATA_REVISION_1 0x00000001 > + > +typedef struct { > + UINT32 Revision; // Used for version identification for this structure > + UINT32 ArrayLength; // The entries number of the following ApicId array and SmBase array > + > + UINT64 *ApicIdMap; // Pointer to CpuIndex->ApicId map. Holds APIC IDs for pending ejects > + CPU_HOT_EJECT_FN Handler; // Handler for CPU ejection > + UINT64 Reserved; > +} CPU_HOT_EJECT_DATA; > + > #endif I'm sorry, I still don't understand -- why is it necessary to modify UefiCpuPkg *at all*, for this feature? (1) The structure type for the data exchange should be in an OvmfPkg header file. (2) The dynamic PCD for transferring the address of the structure should be declared in the "OvmfPkg.dec" file. (3) The "handler" call is made - from SmmCpuFeaturesRendezvousExit() in OvmfPkg/Library/SmmCpuFeaturesLib, - to CpuEject() in OvmfPkg/CpuHotplugSmm. First, this call should not need a function pointer at all -- the CpuEject() logic should be flattened into OvmfPkg/Library/SmmCpuFeaturesLib). Second, if that's not possible -- please explain why --, then a function pointer might be justified after all, but the information channel still seems to have zero relevance for UefiCpuPkg. It's possible I'm not seeing something; please explain. Thanks Laszlo > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > index 76b146299679..f79c874d74f1 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > @@ -131,6 +131,7 @@ > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout ## CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuS3DataAddress ## SOMETIMES_CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress ## SOMETIMES_PRODUCES > + gUefiCpuPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress ## SOMETIMES_PRODUCES > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable ## CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode ## CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmShadowStackSize ## SOMETIMES_CONSUMES > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec > index d83c084467b3..704ccc05f662 100644 > --- a/UefiCpuPkg/UefiCpuPkg.dec > +++ b/UefiCpuPkg/UefiCpuPkg.dec > @@ -339,6 +339,11 @@ > # @ValidList 0x80000001 | 0 > gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress|0x0|UINT64|0x60000011 > > + ## Contains the pointer to a CPU Hot Eject Data structure if CPU hot-plug is supported. > + # @Prompt The pointer to CPU Hot Eject Data. > + # @ValidList 0x80000001 | 0 > + gUefiCpuPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress|0x0|UINT64|0x60000017 > + > ## Indicates processor feature capabilities, each bit corresponding to a specific feature. > # @Prompt Processor feature capabilities. > # @ValidList 0x80000001 | 0 > diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni > index 219c1963bf08..b1721f256632 100644 > --- a/UefiCpuPkg/UefiCpuPkg.uni > +++ b/UefiCpuPkg/UefiCpuPkg.uni > @@ -140,6 +140,10 @@ > > #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuHotPlugDataAddress_HELP #language en-US "Contains the pointer to a CPU Hot Plug Data structure if CPU hot-plug is supported." > > +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuHotEjectDataAddress_PROMPT #language en-US "The pointer to CPU Hot Eject Data" > + > +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuHotEjectDataAddress_HELP #language en-US "Contains the pointer to a CPU Hot Eject Data structure if CPU hot-plug is supported." > + > #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_PROMPT #language en-US "Number of reserved variable MTRRs" > > #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_HELP #language en-US "Specifies the number of variable MTRRs reserved for OS use." >