From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3112F21E78215 for ; Tue, 3 Oct 2017 18:19:22 -0700 (PDT) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP; 03 Oct 2017 18:22:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,475,1500966000"; d="scan'208";a="906443728" Received: from orsmsx106.amr.corp.intel.com ([10.22.225.133]) by FMSMGA003.fm.intel.com with ESMTP; 03 Oct 2017 18:22:42 -0700 Received: from orsmsx154.amr.corp.intel.com (10.22.226.12) by ORSMSX106.amr.corp.intel.com (10.22.225.133) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 3 Oct 2017 18:22:42 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.135]) by ORSMSX154.amr.corp.intel.com ([169.254.11.132]) with mapi id 14.03.0319.002; Tue, 3 Oct 2017 18:22:42 -0700 From: "Kinney, Michael D" To: "Yao, Jiewen" , Leo Duran , "edk2-devel@lists.01.org" , "Kinney, Michael D" Thread-Topic: [edk2] [PATCH v2 0/9] Enhanced SMM support to AMD-based x86 systems. Thread-Index: AQHTPJhkI1Cj+QfLzUmyEDv+p/fisaLTUZYA//+QbeA= Date: Wed, 4 Oct 2017 01:22:41 +0000 Message-ID: References: <1507070305-6727-1-git-send-email-leo.duran@amd.com> <74D8A39837DF1E4DA445A8C0B3885C503A9CE1F2@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A9CE1F2@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Subject: Re: [PATCH v2 0/9] Enhanced SMM support to AMD-based x86 systems. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Oct 2017 01:19:23 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Leo, The general design issue here is that a PCD and PCD macro were added to a UefiCpuPkg include file Include/Register/SmramSaveStateMap.h. This means any library or module that includes this package include file will break the build until the PCD is added to the INF file for that library or module. The INF file for a module is supposed to express the PCDs that the library or module sources access. We do not expect #include statements from package include files to require adding additional PCDs. Updating the code for these values to be detected or configurable is a good idea. One option is to update the C code that currently uses the #define names to use the PCD access function directly, instead of adding the PcdGetxxx() to the Include/Register/SmramSaveStateMap.h file. Jiewen's idea to remove the PCD and detect the offset from CPUID also looks like a reasonable approach. Thanks, Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > Behalf Of Yao, Jiewen > Sent: Tuesday, October 3, 2017 5:50 PM > To: Leo Duran ; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH v2 0/9] Enhanced SMM support to AMD- > based x86 systems. >=20 > Thanks Leo. > I agree we should support AMD-based X86, since the most code is > sharable. >=20 > [1] Comment for PcdCpuSmmSmramSaveStateMapOffset. >=20 > I am a little concerned about the patch 6/9, because it is an > incompatible change. > This change will cause all existing intel platform code change, > to add PCD in INF, such as OvmfPkg (patch 4/9) and QuarkPkg > (patch 5/9). >=20 > I am thinking an compatible way, to prevent existing intel > platform code change. >=20 > 1) We can define below according to Intel SDM and AMD SDM. >=20 > #define INTEL_SMRAM_SAVE_STATE_MAP_OFFSET 0xfc00 > #define AMD_SMRAM_SAVE_STATE_MAP_OFFSET 0xfe00 > // This is to provide compatibility. > #define SMRAM_SAVE_STATE_MAP_OFFSET > INTEL_SMRAM_SAVE_STATE_MAP_OFFSET >=20 > 2) Because the system has capability to *detect* the CPU, there > is no need to let user to *configure*. > I do not suggest we use PCD. IMHO, it is more a system > attribute, instead of a user configurable data. For example, a > user cannot configure it to 0xFE00 for Intel CPU, or 0xFC00 for > AMD CPU. >=20 > 3) I think we can have a CPUID check in the entrypoint, and > patch the OFFSET at anywhere. >=20 >=20 > [2] Comment for PcdCpuSmmPSDOffset. > I do not mind, since it is driver internal configuration and it > won't break the compatibility. >=20 > Thank you > Yao Jiewen >=20 >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > Behalf Of Leo > > Duran > > Sent: Wednesday, October 4, 2017 6:38 AM > > To: edk2-devel@lists.01.org > > Subject: [edk2] [PATCH v2 0/9] Enhanced SMM support to AMD- > based x86 > > systems. > > > > UefiCpuPkg: > > This patch-set introduces a couple of FixedPCDs to replace > > Intel-specific macros, for SMM support on AMD-based x86 > systems. > > > > 1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map > Offset. > > 2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in > SMRAM. > > > > OvmfPkg and QuarkSocPkg: > > The PcdCpuSmmSmramSaveStateMapOffset PCD is declared just to > resolve > > the macro replaced by the shared Library/SmmCpuFeaturesLib.h > file. > > > > Changes since v1: > > Revision to Cc list for UefiCpuPkg. > > > > Leo Duran (9): > > UefiCpuPkg: UefiCpuPkg.dec > > UefiCpuPkg: PiSmmCpuDxeSmm driver. > > UefiCpuPkg: SmmCpuFeaturesLib library. > > OvmfPkg: SmmCpuFeaturesLib library. > > QuarkSocPkg: SmmCpuFeaturesLib library. > > UefiCpuPkg: Register/SmramSaveStateMap.h > > UefiCpuPkg: PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > > UefiCpuPkg: PiSmmCpuDxeSmm driver. > > UefiCpuPkg: SmmCpuFeaturesLib library. > > > > OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > | 5 > > +++++ > > .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > | 4 > > ++++ > > UefiCpuPkg/Include/Register/SmramSaveStateMap.h > | 4 > > +++- > > UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S > | 4 > > +++- > > UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm > | 4 > > +++- > > UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm > | 4 > > +++- > > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf > | 5 > > +++++ > > UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf > | 6 > > ++++++ > > UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S > | 4 > > +++- > > UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm > | 4 > > +++- > > UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm > | 4 > > +++- > > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S > | 4 > > +++- > > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm > | > > 4 +++- > > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm > | > > 4 +++- > > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > > | 2 +- > > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > > | 4 ++++ > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S > | > > 4 +++- > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm > | > > 4 +++- > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > | > > 4 +++- > > UefiCpuPkg/UefiCpuPkg.dec > | 9 > > +++++++++ > > 20 files changed, 73 insertions(+), 14 deletions(-) > > > > -- > > 2.7.4 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel