From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web10.36478.1613417514410424370 for ; Mon, 15 Feb 2021 11:31:54 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Crcp/Bob; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1613417513; 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=oSVGUgsE3ND5N9xjoRek6CH8i1j544ROI/7ZK/yX/4k=; b=Crcp/Bob7AN/zZ9NotURA3eHcUqxzexi11IvTUEQhPpyd+wZoViSk3y+k1ftQRfcLGOn6h elLrxEdVCcQI5DlyL/Eqek9lmSxrbXq0D7XiAiSmLXTKN0cX6SEYJUl0UruKr7lZFihLIQ jZM8Em6u3webVuCG4O09MKxR5QTUDmU= 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-140-6Om0IEzrOxWqaYFWSpvlpQ-1; Mon, 15 Feb 2021 14:31:52 -0500 X-MC-Unique: 6Om0IEzrOxWqaYFWSpvlpQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C47A4804038; Mon, 15 Feb 2021 19:31:50 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-65.ams2.redhat.com [10.36.112.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 214DD5C241; Mon, 15 Feb 2021 19:31:48 +0000 (UTC) Subject: Re: [PATCH v2 2/4] UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors To: mikuback@linux.microsoft.com, devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Rahul Kumar References: <20210213005806.2927-1-mikuback@linux.microsoft.com> <20210213005806.2927-3-mikuback@linux.microsoft.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 15 Feb 2021 20:31:48 +0100 MIME-Version: 1.0 In-Reply-To: <20210213005806.2927-3-mikuback@linux.microsoft.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 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/13/21 01:58, mikuback@linux.microsoft.com wrote: > From: Michael Kubacki > > There's currently two library instances: > 1. SmmCpuFeaturesLib > 2. SmmCpuFeaturesLibStm > > There's two constructor functions: > 1. SmmCpuFeaturesLibConstructor() > 2. SmmCpuFeaturesLibStmConstructor() > > SmmCpuFeaturesLibConstructor() is called by > SmmCpuFeaturesLibStmConstructor() since the functionality in that > function is required by both library instances. > > The declaration for SmmCpuFeaturesLibConstructor() is embedded in > "SmmStm.c" instead of being declared in a header file. Further, > that constructor function is called by the STM specific constructor. > > This change moves the common code to a function called > CpuFeaturesLibInitialization() which is declared in an internal > library header file "CpuFeaturesLib.h". Each constructor simply > calls this function to perform the common functionality. > > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > Signed-off-by: Michael Kubacki > --- > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 19 +++++++---------- > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c | 22 ++++++++++++++++++++ > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c | 21 ++----------------- > UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h | 12 +++++++++++ > 4 files changed, 43 insertions(+), 31 deletions(-) > > diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > index 75bde752785a..e74f87f3f266 100644 > --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > @@ -2,6 +2,7 @@ > The CPU specific programming for PiSmmCpuDxeSmm module. > > Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -63,19 +64,15 @@ BOOLEAN mNeedConfigureMtrrs = TRUE; > BOOLEAN *mSmrrEnabled; > > /** > - The constructor function > + Performs library initialization. > > - @param[in] ImageHandle The firmware allocated handle for the EFI image. > - @param[in] SystemTable A pointer to the EFI System Table. > - > - @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. > + This initialization function contains common functionality shared betwen all > + library instance constructors. > > **/ > -EFI_STATUS > -EFIAPI > -SmmCpuFeaturesLibConstructor ( > - IN EFI_HANDLE ImageHandle, > - IN EFI_SYSTEM_TABLE *SystemTable > +VOID > +CpuFeaturesLibInitialization ( > + VOID > ) > { > UINT32 RegEax; > @@ -162,8 +159,6 @@ SmmCpuFeaturesLibConstructor ( > // > mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); > ASSERT (mSmrrEnabled != NULL); > - > - return EFI_SUCCESS; > } > > /** > diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c > index c562582ccee0..eebbbfd00a83 100644 > --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c > +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c > @@ -3,6 +3,7 @@ The CPU specific programming for PiSmmCpuDxeSmm module when STM support > is not included. > > Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -82,3 +83,24 @@ SmmCpuFeaturesInstallSmiHandler ( > ) > { > } > + > +/** > + The constructor function for the library instance without STM. > + > + @param[in] ImageHandle The firmware allocated handle for the EFI image. > + @param[in] SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. > + > +**/ > +EFI_STATUS > +EFIAPI > +SmmCpuFeaturesLibConstructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + CpuFeaturesLibInitialization (); > + > + return EFI_SUCCESS; > +} > diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c > index b5aad41fdb64..4b6bf958cedf 100644 > --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c > +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c > @@ -30,22 +30,6 @@ > #define RDWR_ACCS 3 > #define FULL_ACCS 7 > > -/** > - The constructor function > - > - @param[in] ImageHandle The firmware allocated handle for the EFI image. > - @param[in] SystemTable A pointer to the EFI System Table. > - > - @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. > - > -**/ > -EFI_STATUS > -EFIAPI > -SmmCpuFeaturesLibConstructor ( > - IN EFI_HANDLE ImageHandle, > - IN EFI_SYSTEM_TABLE *SystemTable > - ); > - > EFI_HANDLE mStmSmmCpuHandle = NULL; > > BOOLEAN mLockLoadMonitor = FALSE; > @@ -138,10 +122,9 @@ SmmCpuFeaturesLibStmConstructor ( > SmmCpuFeaturesLibStmSmiEntryFixupAddress (); > > // > - // Call the common constructor function > + // Perform library initialization common across all instances > // > - Status = SmmCpuFeaturesLibConstructor (ImageHandle, SystemTable); > - ASSERT_EFI_ERROR (Status); > + CpuFeaturesLibInitialization (); > > // > // Lookup the MP Services Protocol > diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h > index 4645bbb066c9..f9a758e82558 100644 > --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h > +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h > @@ -9,6 +9,18 @@ > #ifndef _CPU_FEATURES_LIB_H_ > #define _CPU_FEATURES_LIB_H_ > > +/** > + Performs library initialization. > + > + This initialization function contains common functionality shared betwen all > + library instance constructors. > + > +**/ > +VOID > +CpuFeaturesLibInitialization ( > + VOID > + ); > + > /** > Internal worker function that is called to complete CPU initialization at the > end of SmmCpuFeaturesInitializeProcessor(). > Reviewed-by: Laszlo Ersek