Re: [PATCH v3 5/5] OvmfPkg/SmmCpuFeaturesLib: Skip SMBASE configuration

["Gerd Hoffmann" <kraxel@redhat.com>]

  Hi,

> > > Ok.  So new Intel processors apparently got new MSR(s) to set SMBASE
> > > directly.  Any specific reason why you don't add support for that to
> > > PiSmmCpuDxeSmm?  That would avoid needing the new HOB (and the related
> > > problems with the 8190 cpu limit) in the first place.
> 
> The reason is the hardware interface to set SMBASE is in thread scope meaning
> such firmware-hardware communication needs to be done for each CPU thread.
> 
> It's doable to program the hardware interface using DXE MP service protocol in
> CpuSmm driver's entry point.
> But, considering the standalone MM environment where the CpuMm driver runs
> in a isolated environment and it cannot invoke any DXE or PEI MP service, you could
> understand that why we choose to put the hardware interface programming in a separate
> PEI module. This is the major reason.

Ok, *that* finally makes sense to me.  Can you please add a source code
comment explaining this to the patch series?  Patch #1 (which adds the
interface) is the best place I think.

> I admit that a minor benefit of this design is we can isolate the
> private hardware interface programming in a close-source module.
> Otherwise, the SmmCpuFeaturesLib might need to expose a new API for
> the hardware interface programming.

"benefit" and "closed-source" in one sentence while discussion patches
for an open source project.

And you are wondering (see parallel mail by Jiaxin) why outsiders get
the impression you are trying to hide information.

No further questions.

> Though this new HOB is not in PI spec, you remind me that we might
> need to add more fields to the HOB so a way to distinguish between
> different versions of the HOB should be considered.  The way could be
> to introduce a new GUID for new version of HOB, or add a field
> (version?) in the HOB.  I prefer the second.

Established practice is to use a new GUID.  We should stick to that.

take care,
  Gerd