From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 9ABD521A00AE5 for ; Mon, 26 Jun 2017 15:34:39 -0700 (PDT) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP; 26 Jun 2017 15:36:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,397,1493708400"; d="scan'208";a="119518371" Received: from slexing-mobl.amr.corp.intel.com (HELO localhost) ([10.252.128.225]) by fmsmga006.fm.intel.com with ESMTP; 26 Jun 2017 15:36:08 -0700 MIME-Version: 1.0 To: Laszlo Ersek Message-ID: <149851656823.26353.11207512663550762369@jljusten-skl> From: Jordan Justen In-Reply-To: <04147371-25a3-12d2-0cbd-6e07c816a880@redhat.com> Cc: Jeff Fan , Jiewen Yao , Michael Kinney , edk2-devel-01 References: <20170608171333.17937-1-lersek@redhat.com> <20170608171333.17937-2-lersek@redhat.com> <149789342556.32751.17592475673245441129@jljusten-skl> <384c7b21-dc46-25ba-c90e-75ae07ab5921@redhat.com> <149849999779.24605.15084992650352143991@jljusten-skl> <04147371-25a3-12d2-0cbd-6e07c816a880@redhat.com> User-Agent: alot/0.5.1 Date: Mon, 26 Jun 2017 15:36:08 -0700 Subject: Re: [PATCH 1/5] OvmfPkg: introduce Q35TsegSizeLib (class header and sole lib instance) 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: Mon, 26 Jun 2017 22:34:39 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-06-26 12:12:45, Laszlo Ersek wrote: > On 06/26/17 19:59, Jordan Justen wrote: > > I looked at this series a number of times. My thought is that: > > = > > * PcdQ35TsegMbytes should become dynamic > > = > > * No PCD for tseg mask > > = > > * PlatformPei adds knowledge of qemu ext tseg to set PcdQ35TsegMbytes > > = > > * SmmAccessPei adds dep on gEfiPeiMemoryDiscoveredPpiGuid to make sure > > PcdQ35TsegMbytes is set. > = > Arbitrary and quite ugly, but I guess I can live with it. I'm assuming you mean that the gEfiPeiMemoryDiscoveredPpiGuid dependency is arbitrary and quite ugly. I guess it doesn't seem arbitrary and ugly to me, since on most firmware you'd need to detect memory before considering anything SMM related. We should probably still minimize memory usage until Platform PEI 'installs' it, even though no configuration is required. > > * SmmAccessPei adds some duplicated code to know about qemu ext tseg > > for mask. (But, I think a reasonably small amount of dup code.) > > = > > * SmramInternal.c just reads the size from the PCD > = > This last bullet is a functional change for SmramAccessGetCapabilities() > that I wanted to avoid -- I sought to preserve the logic of working off > of the currently set (and possibly locked) contents of ESMRAMC.TSEG_SZ. > = > I guess the idea is still doable if SmramInternal.c introduces a global > variable for caching the PCD at module startup -- setting the variable > from the PCD would be the responsibility of each separate module, both > PEIM and DXE driver --, and then SmramAccessGetCapabilities() could use > the global variable if it read "11" binary from the TSEG_SZ bit-field > register. Why can't SmramInternal.c read the size from the PCD directly? -Jordan