Re: Adding Bhyve support into upstream EDK2

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

On 03/06/20 17:09, Rebecca Cran wrote:
> I'm currently working on updating EDK2 support for Bhyve
> (https://bhyve.org/) from the edk2-stable201903 tag to
> edk2-stable202002. It's currently kept in a separate repo
> (https://github.com/freebsd/uefi-edk2), but I'd like to discuss pushing
> support upstream into the main edk2 repo (I guess into edk2-staging as a
> first step?).
> 
> 
> Would that be something people would be open to considering, or should
> it remain separate? Should it be a new top-level package (e.g. BhyvePkg)
> or could it be just a configuration option when building OVMF? It's
> currently maintained as a set of patches against OvmfPkg, which seems to
> work quite well.

It depends:

- on the amount of patches you have,

- on the complexity the patches introduce.

For example, ArmVirtPkg platforms use a large number (large proportion)
of OvmfPkg modules. I still very much like ArmVirtPkg to stay a separate
package.

For another example, IA32/X64 Xen support is presently "mixed into"
OvmfPkg code that otherwise targets QEMU. This mixture can be witnessed
at two levels:

- Xen-specific modules pulled into OvmfPkg{Ia32,Ia32X64,X64}.{dsc,fdf},

- dynamically enabled/disabled Xen-guest code that's part of the same
source files (or same modules) that otherwise target QEMU.

There is no general recipe for keeping things shared by default, and
coding up the exceptions one by one, versus keeping things duplicated by
default, and extracting the common parts one by one. Again, it depends
on code size and complexity.

As I mentioned, I strongly prefer ArmVirtPkg to stay separate from
OvmfPkg. Leif disagreed with that, IIRC. I don't remember how Ard feels
about it.

Regarding Xen, I seem to recall that both Ard and myself prefer to keep
Xen as a separate *platform* (DSC, FDF) in both ArmVirtPkg and OvmfPkg.

ArmVirtPkg has always been like that (Ard did the Xen enablement that
way, up-front -- see commit 22455087dc37).

In OvmfPkg, we had started from the opposite direction, and it's quite
recent that Anthony has proposed to isolate Xen support to a dedicated
platform -- see <https://bugzilla.tianocore.org/show_bug.cgi?id=2122>.
(Which is a very welcome proposal, as far as I'm concerned.)

So... if you have a small number of trivial patches (or at least
well-separable drivers, libraries etc) for supporting bhyve, those could
be added to OvmfPkg, perhaps.

If you needed to hook a bunch of complex / deep "if"s into existent
code, then I would very much *NOT* like that, on the other hand.

Because the latter kind of code:
- makes human reasoning difficult,
- is virtually a recipe for regressions.

Regressions because: imagine there is a patch (motivated by a QEMU
feature) that rearranges some code that's also used, in a *slightly*
customized manner, on bhyve. If we mess up the code for bhyve, we don't
notice, because we don't *test* on bhyve. So in such cases, code
duplication / separation is actually beneficial, because it prevents
users / developers of platform X from regressing platform Y. As you see
such separation is mostly driven by what contributors run on, and test on.

Quick request: please do not ask me to look at the current bhyve
patches, off-list. Feel free to post them as an RFC series, instead.

Another reason I'd likely prefer bhyve to be reasonably separate is
maintenance responsibility. We have a pathname pattern based
Maintainers.txt now -- I would want to be able to assign BZs, and patch
reviews, immediately to you, for anything bhyve-related.

(I mean... it's not like I am some special "arbitrator" or whatever;
instead, the Maintainers.txt patterns should tell *anyone* asking, that
you would be responsible for bhyve patch review.)

In that sense, BhyvePkg would surely be my preference. I don't use
bhyve, so I want none of that responsibility, and also no increased
chance of regressions (in either direction: I don't want bhyve patches
to break OVMF on QEMU, and I don't want to break bhyve support with
QEMU-oriented patches).

FWIW, I do agree that bhyve support would be nice to have up-stream, and
specifically in "edk2", not "edk2-platforms". Virtual platforms are very
useful for testing core code updates, and therefore they get a pass into
"edk2". (I specifically remember an on-list discussion about the edk2
vs. edk2-platforms split, and virtual platforms and emulators were
*generally* permitted in edk2.)

Thanks
Laszlo