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

["Laszlo Ersek" <lersek@redhat.com>]

I'm going to comment on this one email up-stream, because it showcases
the community problem, as far as I'm concerned, and because Jiaxin made
a reference to my initial request.

On 2/2/23 10:00, Gerd Hoffmann wrote:
>   Hi,
> 
>>> But the serialized SMBASE programming still happens, now in the PEI
>>> module, and I don't see a good reason why the runtime the new PEI module
>>> and the runtime of PiSmmCpuDxeSmm combined is faster than before.
>>
>> As I said, PEI module can also programs SMBASE in parallel, for
>> example program the some register directly instead of depending the
>> existing RSM instruction to reload the SMBASE register with the new
>> allocated SMBASE each time when it exits SMM.
> 
> 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.

See this is *exactly* my problem. The *whole work* on this should have
started like this, with a new Feature Request Bugzilla:

"Intel are introducing a new processor register (MSR or other method)
with their XY product line where firmware can program the SMBASE
independently of the RSM instruction. The PEI code performing this logic
will not be open sourced, similarly to other things that are kept
binary-only in the FSP (firmware support packages), and perhaps
similarly to how memory chips are initialized in the PEI phase too, by
"MRC" (memory reference code). Because there is no intent to open source
the initialization logic, possibly due to the new MSR not even being
slated for documentation in the Intel SDM, we need a new *binary-only*
interface."

*That* would have been honest, straight talk. Not this smoke-screen with
"another vendor might have a different method". That's entirely
speculative generality. Speculative generality has been an anti-pattern
in software development for decades, even merely for technical means,
but here the justification for it is not even technical: the language
around the generality is just to hide the one actual purpose of the
feature. Don't do that please. Describe your *specific* use case, list
your arguments, and then explain your approach for making it
regression-free for the existent cases.

PI and UEFI are all about binary interfaces between proprietary vendors.
As much as I disagree with the entire concept [*], I accept it as a fact
of life. I just ask that, whenever that pattern (= introducing ABIs,
rather than APIs) is exercised, at least the *actual goal* be documented.

([*] I disagree with the concept for two reasons. One, ideological (no
further explanation needed). Two, practical. If you "standardize" an
interface when you have *one* application of it, that's always trouble.
The second application, if there's ever going to be a second one, will
almost surely not fit within the framework of the "standard". So it's
best to either open source all the implementations, or at least openly
document their *actual* interface needs. Clearly state that the initial
interface implementation is "work in progress". If there are multiple
(divergent) applications, *then* try to extract something common, either
from the open source code bases, or the clearly documented data
dependencies, and then codify that. UEFI is *hugely* harmed by the
proliferation of protocols, where every new feature needs to be
standardized, as soon as one implementation exists. These protocols then
get ossified and linger around for absolutely forever. I feel that it's
totally self-inflicted damage; it is the consequence of the proprietary
software development model -- effectively the unwillingness to share.)

>>> Do you intent submitting code for OVMF producing such a HOB?
>>> There isn't any in this series.
>>
>> No, we won't do that. 
> 
> Then there is no point in changing the OVMF code,
> other than maybe adding an ASSERT that there is no such HOB.

I agree.

My initial request was for *considering* OVMF's library instance.
"Considering" means evaluating, and modifying *if* necessary, and *as*
necessary. I could not perform this evaluation myself: initially, the
purpose of the new interface was obscure, so I couldn't tell if OVMF was
affected or not.

Now that we *know* that OVMF is unaffected, we should just use library
instances for what library instances exist for: customization (platform
or otherwise). Because OVMF will not contain a PEI module initializing
the SMBASE regs as described, there is no need for adding (dead) code to
OVMF's lib instance. Adding the ASSERT() *definitely* makes sense
however: it's a statement that we have *considered* the applicability of
the feature. That is *precisely* what I missed from the initial
announcement.

Look, when you see a summary diffstat for a patch set that modifies N
instances of a library class, but not the one in OVMF -- or in any one
particular subsystem --, you ask yourself: "is this an oversight, or is
this intentional? did they just miss or ignore that platform or
subsystem, or did they evaluate it as unaffected"?

*That* is exactly what should have been in the original BZ or cover
letter. "I've grepped the edk2 and edk2-platforms tree for all instances
of this library class; I need to modify instances X, Y, Z; I *know* I
*need not* modify P, Q; and I'm *unsure* about U, V and W. Please advise".

The ASSERT() will give us a good opportunity to write a comment and/or
commit message around this.

This whole ordeal is just another piece of evidence for why I cannot
remain a member of this community. Hiding information, as a *basic modus
operandi*, is incompatible with open source development.

It's *fine* if a project or a contributor adopts a "my way or the high
way" attitude. Throwing open source code over the wall is still open
source. It's not open development any more, but still open source -- it
is still valuable (though less so). There may be many *valid* reasons
for doing open source, but not open development. What pains me is the
dishonest or at least mixed / sloppy messaging about *what edk2 is*. Is
it open source, or is it open development?

https://github.com/tianocore/tianocore.github.io/wiki/TianoCore-Who-we-are

Laszlo