Re: [PATCH 1/5] OvmfPkg: introduce Q35TsegSizeLib (class header and sole lib instance)

[Jordan Justen <jordan.l.justen@intel.com>]

On 2017-06-19 12:39:51, Laszlo Ersek wrote:
> 
> Even if the PCDs were technically doable, I think I'd slightly prefer
> the current approach. The PCI config space accesses at module startup
> are minimal, and they affect only three modules. OTOH, introducing two
> PCDs feels, in this particular case, a bit like "polluting" OvmfPkg.dec.
> (Well, "PcdPreferredEsmramcTsegSzMask" would *replace*
> "PcdQ35TsegMbytes", so it would mean the addition of one PCD.)
> 
> Anyway, this is just a slight preference. What I feel much more strongly
> about are the library APIs.

The library is the part that seems odd to me. A library just to deal
with Q35 tseg seems a bit specific.

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.

* 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

Now, as usual, I expect you'll point out some trivially obvious issue
that I missed. :)

-Jordan