Note: groups.io will be down for maintenance this evening, starting at 11pm Pacific Time (6am Tuesday 8/3/2020 UTC), for up to two hours.
OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve
I've finished the work to move BhyvePkg into OvmfPkg/Bhyve, and the changes are available at https://github.com/bcran/edk2/commit/09cfc08b50992ed3f76459611e8a928b74ed8c8a .
Since it's such a large patch (over 400KB, but the vast majority of it is existing changes from previously-reviewed patches), I'm posting a link to the changes here in case there are any fundamental issues with it before I post it for review.
I've run PatchCheck.py and the only issue it flagged is that the 'license' on VbeShim.h is wrong, and that's because the header is non-standard:
// THIS FILE WAS GENERATED BY "VbeShim.sh". DO NOT EDIT.
On 06/26/20 17:38, Rebecca Cran wrote:
I've finished the work to move BhyvePkg into OvmfPkg/Bhyve, and theI'll look at the patches on the list. (I don't believe in reviewing
before reviewing. In case I need to make comments for what I find in
your repo, I could only make them without any context to quote.)
I've run PatchCheck.py and the only issue it flagged is that theThis likely comes from BaseTools commit a4cfb842fca9
("BaseTools/PatchCheck.py: Add LicenseCheck", 2020-06-12).
One approach would be to remove "VbeShim.h" from the tracked files under
OvmfPkg, replacing it with a PREBUILD command in the OVMF DSC files.
(Then Bhyve could do the same.)
However, the generator, namely "VbeShim.sh", is not written in Python,
but in (POSIX) shell, and so it can't be called from PREBUILD (I think
it would break OVMF builds on Windows).
I don't know what to tell you, other than the blanket license
enforcement from commit a4cfb842fca9 is likely wrong.
The generated include file *must* be a ".h" file, otherwise the INF file
reference won't be able to trigger an incremental build, if I understand
correctly. So replacing the ".h" suffix with something else, such as
".genh" (for "generated header") won't work, I believe.
Modifying the printf invocations in the generator script to also output
a license header would not be right either, IMO. A license tag makes no
sense (I think) without a copyright (C) statement. And what copyright
(C) notice should we put on a generated file?
Furthermore, although "ReadMe.rst" in the project root states
Contributions of code put into the public domain can also be accepted.
I don't see how the license check implemented in commit a4cfb842fca9
would accommodate a public domain contribution. (I think it would be
fine to place the the generated header file in the public domain, *if*
(a) we could express that somehow (is there an SPDX tag for that?),
*and* (b) if that would eliminate the need for a (C) notice / authorship
Note that this generator use case is not unique to QemuVideoDxe; see for
example commit 1e9d6b0f98b5 ("OvmfPkg/OvmfXen: Creating an ELF header",
I've now filed a bug for BaseTools:
Once that bug is solved -- that is, once we standardize a tag for
marking generated source files as such --, we can update "VbeShim.sh" to
produce the tag, in "VbeShim.h". Then OvmfPkg/Bhyve can do the same.
On 7/2/20 3:27 AM, Laszlo Ersek wrote:
I'll look at the patches on the list. (I don't believe in reviewingThanks, I'll wait for the changes to PatchCheck.py to land, then send the bhyve patch to the list. I think at this point I've updated all the licenses to be BSD+Patent with the exception of the generated file.
On 07/03/20 05:13, Rebecca Cran wrote:
On 7/2/20 3:27 AM, Laszlo Ersek wrote:Sounds great!I'll look at the patches on the list. (I don't believe in reviewingThanks, I'll wait for the changes to PatchCheck.py to land, then send