I tested the code from https://github.com/FreeBSD-UPB/freebsd-src/tree/projects/bhyvearm64/ (as of 2021-05-29) on my SolidRun Honeycomb system and it fails to allocate resources in the gicv3 code when I run kldload vmm.

With examples like Apple's "goto fail" bug (https://www.imperialviolet.org/2014/02/22/applebug.html) I'm surprised style(9) only allows, and doesn't recommend or mandate braces around single-line statements.

I noticed this is against the svn repo:

Repository rS FreeBSD src repository - subversion"

You'll probably want to switch over to Git and rebase this patch.

Just a comment typo: "number of contained objecct handles" - should be "object".

@manu Sorry, could you re-approve the change please?

In D29254#656248, @manu wrote:

OK I see. Well for now just have a build time dep is better then.
Go ahead and commit this.

We'll also need to update the port revision.

In D29254#655711, @manu wrote:

Is the 4.8 version required for something ? Otherwise depending on lang/gcc would be better (not really sure if we have some way to do that).

@woodsb02 Could you review and approve this for me, please?

Now the firmware work has been committed, I'll switch back to working on this.

Committed as r565866.

In D27230#642824, @jhb wrote:

I'm not a ports committer so can't really review the ports part, but I think moving the non-CSM ROM to the newer one seems fine. I think grehan@ has also already signed off on this?

At this point, I'd like to get this committed as soon as possible so we can move away from gcc 4.8 and python 2.

Updated to code from today's master, 2021-02-14.

The patch no longer applies against 13-CURRENT: I needed to apply several lines manually.

This works for me. I was seeing an assert failure in pci_emul.c, but it disappeared for some reason.
One person on Slack said they'd like the BAR in a fixed (high) location, so I wouldn't be confident committing this without other people (e.g. @jhb and @grehan ) reviewing it too.

OVMF puts it at the next 16GB boundary, which might be better.

Also, /boot/boot1.efifat is no longer included in -CURRENT.

/boot/boot1.efifat is the 800 KB filesystem image. /boot/boot1.efi is the EFI bootloader application.

UEFI is configuring an address size less than that which the host CPU supports, because (cc @grehan) :

//
// As guest-physical memory size grows, the permanent PEI RAM requirements
// are dominated by the identity-mapping page tables built by the DXE IPL.
// The DXL IPL keys off of the physical address bits advertized in the CPU
// HOB. To conserve memory, we calculate the minimum address width here.

@c.koehne_beckhoff.com
I see what's going wrong, and have reproduced the issue on my Threadripper system.
EDK2 firmware configures the maximum address based on the size of guest memory, with a minimum of 64GB (36 bits).
But the PCI code in bhyve puts the BAR at (highest host CPU address / 4) - meaning that on my system it puts it at 64TB.

I successfully installed and ran Windows 10 with the new firmware today.
So I think the last remaining issue will be the MemAbove4G problem.

The patch no longer applies cleanly: I had to make some changes.
But after that, I was able to run Ubuntu 20.10 with gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration set to TRUE and FALSE with the following command line, which uses both rfb and passthru:

bhyve -AH -s 0:0,hostbridge -s 31:0,lpc -c 1 -m 16G -l bootrom,BHYVE_CODE.fd -l com1,stdio -s 3:0,ahci-hd,bhyve-ubuntu-20_10.img  -s 7,ahci-cd, -s 8,virtio-rnd -s 6,virtio-net,tap0 -s 9,fbuf,rfb=10.0.10.117:5900,w=1920,h=1200 -s 10,xhci,tablet -s 11,passthru,24/0/0 -S guest

In D27230#619596, @woodsb02 wrote:

Apologies for being absent recently - life took over... but I’ll have some time to look at this again between Xmas and New Year.

In D27230#616878, @bcran wrote:

I've had a report that Windows 10 crashes (bugchecks) with this firmware, so I need to do more testing (on FreeBSD 11, 12 and 13-CURRENT) before committing it.

Any progress fixing this? Would you like to commit the update using one revision older from upstream that didn’t have this issue?

This was fixed in rS359719, so I think this review should be closed.

In D27230#617382, @c.koehne_beckhoff.com wrote:

I've uploaded new binaries to https://people.freebsd.org/~bcran/bhyve/BhyveX64-20201213/ .

I've had a report that Windows 10 crashes (bugchecks) with this firmware, so I need to do more testing (on FreeBSD 11, 12 and 13-CURRENT) before committing it.

Update the version to 20201213 and move the GH tag today's sources

The CSM port shouldn't really default to a debug build, but I can fix that in a later change since it's existing behavior.

Thanks Ben!

Oh I also bumped the PORTEPOCH to 2 to make sure the new version of g20201129 is considered greater than the previous 0.2.

Fixed the license lines in uefi-edk2-bhyve, since it uses BSD-2-Clause-Patent.

• Install BHYVE.fd as BHYVE_UEFI.fd.
• Switched version to g20201129 and updated github tag to use the

latest code on master, because it includes several important fixes.

This fixes the amount of memory displayed in the UiApp to be the same as passed on the bhyve command line. Otherwise, 8GB gets displayed as 4, 32GB as 28 etc.

Fixed the CSM port so it actually builds with CSM support.

I've also updated the name/version fields of the uefi-edk2-bhyve port to be:

PORTNAME=	uefi-edk2-bhyve
DISTVERSIONPREFIX=	edk2-stable
DISTVERSION=	202011
PORTEPOCH=	1

Restored the uefi-edk2-bhyve-csm port, with contents from the existing
uefi-edk2-bhyve directory. Changed the installed filename to be
BHYVE_UEFI_CSM.fd.

In D27230#607935, @grehan wrote:

Can we keep the CSM one until the bitter end, and build it from the existing repo ?

Use #defines for the version and date.
Only bump the firmware fields.

In D26428#588166, @1983-01-06_gmx.net wrote:

I don't consider UEFI to be basic at all, both are completely different. If you would refer to it as firmware, I would agree, but not to UEFI as BIOS.

Thanks for committing this, @scottph! I've been busy with a new job so haven't had time to work on FreeBSD recently.

@scottph has indicated he no longer has the time/interest in working on this, so I'll take over and work to get it committed.

In D24066#534662, @grehan wrote:

Is this with PcdPciDisableBusEnumeration = TRUE in UEFI ?

In D19976#515732, @mat wrote:

Has this been committed yet?

@scottph Are you still interested in working on this?

In D24066#533961, @scottph wrote:

Works great here.

• Change comments to C style.

@rgrimes Could you approve the latest patch, or provide comments on what should be changed please?

Upload the full patch.

• Change overflow error to a warning and truncate type 17 memory size.

In D24107#532656, @grehan wrote:

There is no need to error out on overflow here. The type17 isn't used by most o/s's for determining how much memory is available: there is e820, the EFI memory map, etc etc. Perhaps print an error in bhyve, buit there doesn't seem any point in erroring out.

• Rework type17 generation and error out on overflow.

• Revert bumping the SMBIOS version.

• Fixes based on review feedback.

In D24107#532393, @jhb wrote:

That is what the 0x7fff value is for when using extended btw, is to report the max possible size for legacy systems. I agree that we should use the non-extended size when it fits.

In D24107#532401, @rgrimes wrote:

In D24107#532391, @jhb wrote:

Rebecca is it ok with you if I go create a review to do the bcdrev fix?

In D24107#530787, @rgrimes wrote:

This calculation seems odd, I see later it is overwritten for large memory, cant we just continue to use extended all the time and leave this set at 0x7fff? Never mind, spec says not. Ok how about doing: