Page MenuHomeFreeBSD

Update bhyve related ports
ClosedPublic

Authored by bcran on Nov 15 2020, 11:24 PM.
Tags
None
Referenced Files
F99312180: D27230.id79846.diff
Tue, Oct 8, 7:22 AM
F99275976: D27230.id83910.diff
Mon, Oct 7, 10:54 PM
F99243301: D27230.id79846.diff
Mon, Oct 7, 3:43 PM
Unknown Object (File)
Mon, Oct 7, 10:50 AM
Unknown Object (File)
Sun, Oct 6, 6:18 AM
Unknown Object (File)
Sat, Oct 5, 8:16 AM
Unknown Object (File)
Fri, Oct 4, 3:26 PM
Unknown Object (File)
Fri, Oct 4, 11:23 AM

Details

Summary

sysutils/uefi-edk2-bhyve*: Update and migrate to Python 3

  • remove dependency on the now obsolete Python 2.7.
  • sysutils/uefi-edk2-bhyve now uses the GH/tianocore/edk2 upstream repo.
  • sysutils/uefi-edk2-bhyve-csm is no longer a slave port, since it continues to use the GH/freebsd/uefi-edk2 repo.
  • sysutils/uefi-edk2-bhyve-devel is no longer required, so delete it.
  • Update the port Makefiles to follow the style in sysutils/edk2.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 37009
Build 33898: arc lint + arc unit

Event Timeline

bcran created this revision.
bcran set the repository for this revision to rP FreeBSD ports repository.

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

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

Yes! That's a good idea, since I know people do still want CSM functionality.

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.

Updated the maintainer of the ports to be myself (bcran@FreeBSD.org).

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

The DISTVERSION is for the upcoming edk2-stable202011 tag that's being created next week.
I restored PORTEPOCH to ensure that this new version will upgrade older 0.2 installations.

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

sysutils/uefi-edk2-bhyve/Makefile
93

Can the non-vars version be installed with the original name of BHYVE_UEFI.fd ? A lot of scripts rely on this naming.

(this is BHYVE.fd in your current binary build)

  • 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.

bcran added inline comments.
sysutils/uefi-edk2-bhyve/Makefile
93

Thanks, I've fixed that in the latest revision.

Only other thing I can think of is to install VARS.fd as read-only since it's use is as a template and not a location for vars. But, this can be done later (as can cutting over to an edk2 stable tag)

This revision is now accepted and ready to land.Nov 30 2020, 6:07 AM
woodsb02 requested changes to this revision.Dec 3 2020, 2:54 AM
woodsb02 added a subscriber: woodsb02.

Hi Rebecca, looking good.
I've had a go at clarifying the "summary" about to match what the commit text would be. Can you please have a look and improve it if you can?
I have added in a few comments inline below - can you please have a look?
Cheers,
Ben

sysutils/Makefile
1419

The /usr/ports/MOVED file will also need an entry added to the bottom explaining this removal.

sysutils/uefi-edk2-bhyve-csm/Makefile
11

The ? is not required if there is no master/slave ports relationship

45

This is no loner required, since the only things requiring it were the if defined(WITH_CSM) statements, which are now removed.

47

Given this is no longer wrapped in an if statement, this can just be moved to the BUILD_ARGS list 2 lines above.

sysutils/uefi-edk2-bhyve-csm/pkg-descr
1

Could we please add another couple of lines of description, explaining when you would use this?
Perhaps explain the heritage of this firmware, and what UEFI, EDK2 and CSM mean, and why you would use this over other options.

sysutils/uefi-edk2-bhyve/pkg-descr
1

As above, can we please add a bit more description here?

This revision now requires changes to proceed.Dec 3 2020, 2:54 AM
bcran marked an inline comment as done.

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

Updated pkg-desc in both uefi-edk2-bhyve and uefi-edk2-bhyve. Please let me
know if it's too much now!

Removed PLIST_SUFFIX, since it's no longer used.

Install BHYVE_CODE.fd as BHYVE_UEFI.fd, since bhyve quits with an error
if BHYVE.fd is used.

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

bcran marked 5 inline comments as done.Dec 5 2020, 6:04 AM

Thanks Ben!

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.

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

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.

On my system this EFI version will fail, if a 64-bit BAR above 4 GB is used. GCD-Memory doesn't covers the MMIO-Space above 4 GB:

InitRootBridge: populated root bus 0, with room for 0 subordinate bus(es)
RootBridge: PciRoot(0x0)
  Support/Attr: 7001F / 7001F
    DmaAbove4G: No
NoExtConfSpace: Yes
     AllocAttr: 0 ()
           Bus: 0 - 0 Translation=0
            Io: 2000 - 20BF Translation=0
           Mem: C0000000 - C1FFFFFF Translation=0
    MemAbove4G: 4000000000 - 400FFFFFFF Translation=0
          PMem: FFFFFFFFFFFFFFFF - 0 Translation=0
   PMemAbove4G: FFFFFFFFFFFFFFFF - 0 Translation=0
GCDMemType Range                             Capabilities     Attributes
========== ================================= ================ ================
SystemMem  0000000000000000-000000000009FFFF 800000000002600F 0000000000000000*
MMIO       00000000000A0000-00000000000FFFFF C700000000026001 0000000000000000
SystemMem  0000000000100000-000000003EFFFFFF 800000000002600F 0000000000000000*
SystemMem  000000003F000000-000000003F1FFFFF 800000000002600F 0000000000020000*
SystemMem  000000003F200000-000000003FBBEFFF 800000000002600F 0000000000000000*
SystemMem  000000003FBBF000-000000003FBBFFFF 800000000002600F 0000000000004000*
SystemMem  000000003FBC0000-000000003FBC2FFF 800000000002600F 0000000000020000*
SystemMem  000000003FBC3000-000000003FBC5FFF 800000000002600F 0000000000004000*
SystemMem  000000003FBC6000-000000003FBC8FFF 800000000002600F 0000000000020000*
SystemMem  000000003FBC9000-000000003FBCAFFF 800000000002600F 0000000000004000*
SystemMem  000000003FBCB000-000000003FBFFFFF 800000000002600F 0000000000000000*
SystemMem  000000003FC00000-000000003FDFFFFF 800000000002600F 0000000000020000*
SystemMem  000000003FE00000-000000003FFFFFFF 800000000002600F 0000000000000000*
NonExist   0000000040000000-000000007FFFFFFF 0000000000000000 0000000000000000
MMIO       0000000080000000-00000000BFFFFFFF C700000000026001 0000000000000000
MMIO       00000000C0000000-00000000C1FFFFFF C700000000026001 0000000000000000*
MMIO       00000000C2000000-00000000FBFFFFFF C700000000026001 0000000000000000
NonExist   00000000FC000000-00000000FEBFFFFF 0000000000000000 0000000000000000
MMIO       00000000FEC00000-00000000FEC00FFF C700000000026001 0000000000000000
NonExist   00000000FEC01000-00000000FECFFFFF 0000000000000000 0000000000000000
MMIO       00000000FED00000-00000000FED003FF C700000000000001 0000000000000000
NonExist   00000000FED00400-00000000FEDFFFFF 0000000000000000 0000000000000000
MMIO       00000000FEE00000-00000000FEE00FFF C700000000026001 0000000000000000*
MMIO       00000000FEE01000-00000000FEEFFFFF C700000000026001 0000000000000000
NonExist   00000000FEF00000-0000000FFFFFFFFF 0000000000000000 0000000000000000


ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [PciHostBridgeDxe] /usr/ports/sysutils/uefi-edk2-bhyve/work/edk2-d4633b36b94f7b4a1f41901657cbbff452173d35/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c(332): !EFI_ERROR (CheckStatus)

GCD ends at 0x10 0000 0000 which is smaller than MemAbove4G (0x40 0000 0000). A possible solution could be to patch GetFirstNonAddress located in OvmfPkg/Bhyve/PlatformPei/MemDetect.c:

EDIT: Enabling Bus Enumeration fixes this issue too.

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

EDIT: Enabling Bus Enumeration fixes this issue too.

Thanks, I'm testing enabling bus enumeration now, and will submit a patch upstream.

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

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?

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

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?

I'm just getting back to it now. I'm currently working on the MemAbove4G issue, but I did run Windows 10 and it appears to work, but ended up hanging. I need to check if it's also a problem with old firmware.

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

@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.

Unless someone else is working on it, I think the solution would be to update the bhyve hypervisor to put the BAR somewhere relatively close above guest RAM. For example on a 16GB guest, KVM on Linux puts it at 32GB.

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.
  
//
// The minimum address width is 36 (covers up to and excluding 64 GB, which
// is the maximum for Ia32 + PAE). The theoretical architecture maximum for
// X64 long mode is 52 bits, but the DXE IPL clamps that down to 48 bits. We
// can simply assert that here, since 48 bits are good enough for 256 TB.
//

GCD map depends on gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base and gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size which are set at OvmfPkg/Bhyve/PlatformPei/MemDetect.c (GetFirstNonAddress):

BhyveX64.dsc:

gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x800000000

MemDetect.c (GetFirstNonAddress):

FirstNonAddress = BASE_4GB + GetSystemMemorySizeAbove4gb ();
Pci64Size = PcdGet64 (PcdPciMmio64Size);
Pci64Base = ALIGN_VALUE (FirstNonAddress, (UINT64)SIZE_1GB);
Pci64Size = ALIGN_VALUE (Pci64Size, (UINT64)SIZE_1GB);
Pci64Base = ALIGN_VALUE (Pci64Base, GetPowerOfTwo64 (Pci64Size));

//
// The core PciHostBridgeDxe driver will automatically add this range to
// the GCD memory space map through our PciHostBridgeLib instance; here we
// only need to set the PCDs.
//
PcdStatus = PcdSet64S (PcdPciMmio64Base, Pci64Base);
PcdStatus = PcdSet64S (PcdPciMmio64Size, Pci64Size);

gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base is set to guest RAM size and it's aligned on a 32 GB boundary (gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size == 0x800000000 == 32 GB).

I could create a patch for bhyve hypervisor to put 64 bit BARs on next 32 GB boundary behind guest RAM.

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

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.

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?

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?

Thanks. Yes, Peter signed off on it via Slack.

The 64-bit issue is still present in the updated EDK2 (and being worked on), but moving off python2/gcc48 is a more pressing issue.

Hi @bcran,
I have accepted this revision - giving you my approval to commit this to ports.
Please note that I have requested 4 changes to be made before you do (not necessary to re-submit for review).
The MOVED entry in particular is mandatory.
Thanks for all your work!
-@woodsb02

sysutils/Makefile
1419

Just to confirm - this is MOVED entry is still required. Proposed line at the end of the file (with no newline afterwards):

sysutils/uefi-edk2-bhyve-devel|uefi-edk2-bhyve|2021-02-17|Development version no longer necessary
sysutils/uefi-edk2-bhyve-csm/Makefile
62

The second indent here should be using a tab rather than spaces

sysutils/uefi-edk2-bhyve/Makefile
15

Pet portlint - please use a tab after these variable names, rather than spaces

36

Pet portlint - please remove second blank line

This revision is now accepted and ready to land.Feb 16 2021, 11:17 PM