Re: [PATCH v3 0/6] Add BhyvePkg, to support the bhyve hypervisor

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

Hi Rebecca,

On 04/21/20 05:09, Rebecca Cran wrote:
> I noticed a few things wrong with the v2 series:
> 
> o BhyveFwCtlLibNull is no longer used, and could be deleted.
> o Some changes from the v1 AcpiPlatformDxe patch were left in OvmfPkg.
> o BhyvePkg/License.txt still referred to OvmfPkg. I've updated it so
>   everything under BhyvePkg should be BSD-2-Clause.
> 
> This patch series can also be found at
> https://git.bsdio.com/bcran/edk2-bhyve/commits/branch/master
> 
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> 
> Rebecca Cran (6):
>   OvmfPkg: Add bhyve support into AcpiTimerLib
>   OvmfPkg: Add QemuFwCfgLibNull
>   OvmfPkg: Add VBE2 mode info structure to LegacyVgaBios.h
>   Add BhyvePkg, to support the bhyve hypervisor
>   BhyvePkg: Add PlatformPei
>   BhyvePkg: Add AcpiPlatformDxe

Sorry, I'm confused by the structure of this series / updates in this
series.

(1) For example, I can't find the patch that adds
"BhyvePkg/BhyvePkgX64.dsc", in spite of the file being listed in the
cumulative diffstat below.

(2) I also don't really understand why v2 / v3 have been posted, given
that the bhyve-specific ResetSystemLib instance that I suggested under
v1, based on your proposed code, still depends on the ResetSystemLib
cleanup series that I posted. The idea is that you would base the new
bhyve ResetSystemLib instance on my ResetSystemLib refactoring.

Because my series has not been merged yet, for such a bhyve rebase you'd
have to pick up my patches from the list temporarily. That's a 100%
usable approach, but then, this v3 series of yours does not seem to
introduce *any* ResetSystemLib instance. Have you decided to postpone
that work for later?

(Unfortunately, I can't check the ResetSystemLib resolution that you use
in "BhyvePkgX64.dsc", because, again, the v3 series doesn't actually
*contain* "BhyvePkgX64.dsc".)

(3) Regarding the v2 set -- there you mention:

On 04/21/20 04:04, Rebecca Cran wrote:
> changing FILE_GUIDs to be unique
> (except AcpiTables, which needs to be the same between OvmfPkg and
> BhyvePkg),

and I don't understand that... Oh wait, I do: the GUID

  7E374E25-8E01-4FEE-87F2-390C23C606CD

is a well-known GUID (inside ek2, that is). It's not specific to
OvmfPkg. BaseTools calls the GUID "ACPI table storage", and it is
declared in "MdeModulePkg/MdeModulePkg.dec", as
"PcdAcpiTableStorageFile" ("FFS filename to find the ACPI tables").


In the end, please wait until I get around merging the ResetSystemLib
refactoring <https://bugzilla.tianocore.org/show_bug.cgi?id=2675>. Then,
please post a new, comprehensive bhyve set. Patch sets pending review on
a mailing list are not incremental; new versions entirely supersede
earlier versions. Patches are considered incremental only when (a)
earlier patches have been merged, or (b) there's an agreement that in
the particular situation a new patch (or a few patches) can be appended
to a pending series.

Thanks
Laszlo