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.480.1615227387777223250 for ; Mon, 08 Mar 2021 10:16:28 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=dQLdilzY; 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=1615227386; 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=TqSrOmslPF8R+J9Myh9dhzlWlD9GZn3n7NiD+2sXoto=; b=dQLdilzY5SmxEi3MjEkD0HNqSxCAVkw/64jY2YGPn4mSeMr5tk89vDGHf9l+89MsU+Letv 4sMJcPVwGAId7JkLUk1oDI3s0ClKwaNGKskq4vv+2sEosNSefiyB27R137cD0HQyxjvHe6 wW2UfRLGFTZ2W4f43WC86rG3g9MsfrU= 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-405-zhvZiHgUOzW065Lsklo84g-1; Mon, 08 Mar 2021 13:16:19 -0500 X-MC-Unique: zhvZiHgUOzW065Lsklo84g-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 428821934101; Mon, 8 Mar 2021 18:16:18 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-175.ams2.redhat.com [10.36.112.175]) by smtp.corp.redhat.com (Postfix) with ESMTP id DE15660C17; Mon, 8 Mar 2021 18:16:16 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support To: devel@edk2.groups.io, mikuback@linux.microsoft.com Cc: Eric Dong , Ray Ni , Rahul Kumar References: <20210217213227.1277-1-mikuback@linux.microsoft.com> From: "Laszlo Ersek" Message-ID: <79effef5-317d-2002-f9be-54c86d5880cf@redhat.com> Date: Mon, 8 Mar 2021 19:16:15 +0100 MIME-Version: 1.0 In-Reply-To: <20210217213227.1277-1-mikuback@linux.microsoft.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 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 02/17/21 22:32, Michael Kubacki wrote: > From: Michael Kubacki > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3218 > > The present SmmCpuFeaturesLib implementation in UefiCpuPkg can be > useful for IA32/X64 platforms that need a library instance for > a Standalone MM environment. Much of the logic can be reused and a > new INF can isolate the differences unique to Standalone MM. > > This patch series contains an initial set of changes for cleaning > up pre-existing design issues in the library. The final two patches > contain changes needed for Standalone MM support. > > Here's an overview of how the three library instances are organized > that may be a useful reference (provided by Laszlo): > > Traditional, Traditional, Standalone, > no STM STM no STM > > Entry point type DXE DXE MM > > Lib inst. init. basic STM basic > > Processor init. basic STM basic > > PCD access any any fixed > > * Traditional no STM = SmmCpuFeaturesLib.inf > * Traditional STM = SmmCpuFeaturesLibStm.inf > * Standalone no STM = StandaloneMmCpuFeaturesLib.inf > > V3 changes: > > PATCH v3 2/5 is a new patch in the series that renames the file > SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c to more clearly > identify implementation in the file as shared between all library > instances. > > PATCH v3 3/5 adds a new source file SmmCpuFeaturesLib.c that > contains the constructor specific to the Traditional MM no > STM library instance. This was previously implemented in a > file built by the Standalone MM instance and while not > harmful, it was not clean. > > PATCH v3 4/5 updates "@retval" to "@return" in the documentation > for GetCpuMaxLogicalProcessorNumber() since it is not a constant > return value. > > PATCH v3 5/5 contains a commit message update to note that all > instances of "PiSmm.h" in the library source files have been > updated to "PiMm.h" for consistency throughout the library. > > V2 changes: > > Due to some pre-existing design issues in the library that > affected a single v1 patch that add Standalone MM support, > it was suggested to first address those issues and then add the > new INF StandaloneMmCpuFeaturesLib.inf. > > To address these concerns, the following v1 patch was converted > into a v2 patch series: > https://edk2.groups.io/g/devel/message/71626 > > The first two patches in v2 primarily addressed those concerns. > > PATCH v2 1/4 and PATCH v2 2/4 focused on fixing pre-existing > design issues. > > PATCH v2 3/4 and PATCH v2 4/4 focused on the changes needed to add > Standalone MM support. > > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > Signed-off-by: Michael Kubacki > > Michael Kubacki (5): > UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to > header > UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c > UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors > UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber > UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support > > UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c | 2 +- > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 608 +------------------- > UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c} | 36 +- > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c | 3 +- > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c | 26 +- > UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c | 50 ++ > UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c | 28 + > UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c | 2 +- > UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h | 48 ++ > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 3 + > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf | 4 +- > UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} | 22 +- > UefiCpuPkg/UefiCpuPkg.dsc | 1 + > 13 files changed, 172 insertions(+), 661 deletions(-) > copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c} (93%) > create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c > create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c > create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h > copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} (53%) > Merged as commit range 94fa95c8746c..edd46cd407ea, via . I had to replace the guard macro "_CPU_FEATURES_LIB_H_" with "CPU_FEATURES_LIB_H_" (removing the underscore prefix) in the first patch. That's because commit 6ffbb3581ab7 ("BaseTools: Align include guards policy", 2021-02-26) was merged after this series had been posted, and now "_CPU_FEATURES_LIB_H_" triggered ECC error 8003. Thanks, Laszlo