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.web11.1517.1613767169782662592 for ; Fri, 19 Feb 2021 12:39:30 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=V++cGwJq; 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=1613767168; 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=hIrSkyCw5gZZ9KMi9a8FNr28OVCGAXR2qnhIBRrqDbk=; b=V++cGwJq6dZbsBTW3YHckbJ5nvCMLC49774WhGLdgj2c9c8G714bmK6YFehTvb7vceff2N nZNytHbA1ZFRm5txnz8bLwczEeLfufaL7zRklTOvS0mon3WIG1zorks3KmdAKx1VGQJEuI Yo+LG3/0sIclN5GfhKo8+EK64RNOeKI= 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-366-sGNN2cHPOSORms3rx2vCNQ-1; Fri, 19 Feb 2021 15:39:25 -0500 X-MC-Unique: sGNN2cHPOSORms3rx2vCNQ-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 0C892100A61D; Fri, 19 Feb 2021 20:39:24 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-87.ams2.redhat.com [10.36.113.87]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9F4185D720; Fri, 19 Feb 2021 20:39:22 +0000 (UTC) Subject: Re: [PATCH v3 3/5] UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors To: mikuback@linux.microsoft.com, devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Rahul Kumar References: <20210217213227.1277-1-mikuback@linux.microsoft.com> <20210217213227.1277-4-mikuback@linux.microsoft.com> From: "Laszlo Ersek" Message-ID: <7c653d91-5669-71f9-4899-598a19424567@redhat.com> Date: Fri, 19 Feb 2021 21:39:21 +0100 MIME-Version: 1.0 In-Reply-To: <20210217213227.1277-4-mikuback@linux.microsoft.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 On 02/17/21 22:32, 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. > > Additionally, SmmCpuFeaturesLibConstructor() is moved from > SmmCpuFeaturesLibNoStm.c into a instance-specific file allowing > SmmCpuFeaturesLibNoStm.c to contain no STM implementation agnostic > to a particular library instance. > > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > Signed-off-by: Michael Kubacki > Reviewed-by: Laszlo Ersek > --- > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 31 ++++++++++++++++++++ > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c | 19 +++++------- > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c | 23 ++------------- > UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h | 12 ++++++++ > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 1 + > 5 files changed, 54 insertions(+), 32 deletions(-) The update looks OK, thanks. Laszlo > > diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > new file mode 100644 > index 000000000000..00948a191fad > --- /dev/null > +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > @@ -0,0 +1,31 @@ > +/** @file > +Implementation specific to the SmmCpuFeatureLib library instance. > + > +Copyright (c) Microsoft Corporation.
> +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include "CpuFeaturesLib.h" > + > +/** > + The constructor function for the Traditional MM 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/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c > index 36c48310c31e..7a919c5ee70f 100644 > --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c > +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c > @@ -2,6 +2,7 @@ > Implementation shared across all library instances. > > 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/SmmStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c > index b5aad41fdb64..dcc2e9f9a09a 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; > @@ -112,7 +96,7 @@ UINTN mMsegSize = 0; > BOOLEAN mStmConfigurationTableInitialized = FALSE; > > /** > - The constructor function > + The constructor function for the Traditional MM library instance with STM. > > @param[in] ImageHandle The firmware allocated handle for the EFI image. > @param[in] SystemTable A pointer to the EFI System Table. > @@ -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(). > diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > index 7ebb0b0ef011..ddd00eeceb84 100644 > --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > @@ -18,6 +18,7 @@ [Defines] > > [Sources] > CpuFeaturesLib.h > + SmmCpuFeaturesLib.c > SmmCpuFeaturesLibCommon.c > SmmCpuFeaturesLibNoStm.c > >