Page MenuHomeFreeBSD

bhyve: support relocating fbuf and passthru data BARs
ClosedPublic

Authored by khng on Mar 14 2020, 3:17 AM.
Tags
Referenced Files
Unknown Object (File)
Sat, Oct 5, 2:32 AM
Unknown Object (File)
Sat, Oct 5, 2:14 AM
Unknown Object (File)
Thu, Oct 3, 12:56 PM
Unknown Object (File)
Tue, Oct 1, 12:27 PM
Unknown Object (File)
Thu, Sep 26, 11:44 AM
Unknown Object (File)
Thu, Sep 26, 4:25 AM
Unknown Object (File)
Wed, Sep 25, 8:15 PM
Unknown Object (File)
Wed, Sep 25, 3:32 PM

Details

Summary

We want to allow the UEFI firmware to enumerate and assign
addresses to PCI devices so we can boot from NVMe[1]. Address
assignment of PCI BARs is properly handled by the PCI emulation
code in general, but a few specific cases need additional support.
fbuf and passthru map additional objects into the guest physical
address space and so need to handle address updates. Here we add a
callback to emulated PCI devices to inform them of a BAR
configuration change. fbuf and passthru then watch for these BAR
changes and relocate the frame buffer memory segment and passthru
device mmio area respectively.

We also add new VM_MUNMAP_MEMSEG and VM_UNMAP_PPTDEV_MMIO ioctls
to vmm(4) to facilitate the unmapping needed for addres updates.

[1]: https://github.com/freebsd/uefi-edk2/pull/9/

Originally by: scottph
MFC After: 1 week
Sponsored by: Intel Corporation

Test Plan

Run a vm with fbuf and a firmware including the change from https://github.com/freebsd/uefi-edk2/pull/9/. See that the frame buffer is visible in the bootloader.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

When this was applied on top of illumos, I observered the follwing

I'm assuming this is an illumos specific issue, but it might be worth to test this on FreeBSD too.

I'm assuming this is an illumos specific issue, but it might be worth to test this on FreeBSD too.

It will happen on FreeBSD as well. passthru devices don't allow remapping, and that's why the original UEFI firmware had enumeration disabled.

It could well be worth investigating if the changes to UEFI to properly support PcdPciDisableBusEnumeration = TRUE are less invasive than allowing runtime BAR changes
(Intel ACRN has this as TRUE)

It will happen on FreeBSD as well. passthru devices don't allow remapping, and that's why the original UEFI firmware had enumeration disabled.

Interesting, without the change form https://github.com/freebsd/uefi-edk2/pull/9/ these NICs are working fine with passthru. (Not tried NVMe as this is not yet supported on illumos).

So it will be a choice of a UEFI firmware that does VNC and passthru but no NVMe or one that does VNC and NVMe, that will be confusing.

So it will be a choice of a UEFI firmware that does VNC and passthru but no NVMe or one
that does VNC and NVMe, that will be confusing.

The older UDK2014.1-based firmware works fine with all combinations; the regressions in the sync just need to be fixed.

scottph retitled this revision from bhyve: support relocating the fbuf data BAR to bhyve: support relocating fbuf and passthru data BARs.
scottph edited the summary of this revision. (Show Details)

It could well be worth investigating if the changes to UEFI to properly support PcdPciDisableBusEnumeration = TRUE are less invasive than allowing runtime BAR changes
(Intel ACRN has this as TRUE)

I haven't looked into the edk2 code to see what relaxing its expectations would look like, but one much simpler way to work around the heartburn it gets from the nvme model is to just change its bar from MEM_64 to MEM_32: https://gist.github.com/d-scott-phillips/d71d8d0c856301e5738ee73b68e56ad4 (I'm not sure if the nvme spec requires the bar to be 64-bit capable.)

(I'm not sure if the nvme spec requires the bar to be 64-bit capable.)

Looks like it's only recommended ("2.1.10 Offset 10h: MLBAR (BAR0) – Memory Register Base Address, lower 32-bits") so forcing 32-bit could be a quick fix.

I haven't looked into the edk2 code to see what relaxing its expectations would look like, but one much simpler way to work around the heartburn it gets from the nvme model is to just change its bar from MEM_64 to MEM_32: https://gist.github.com/d-scott-phillips/d71d8d0c856301e5738ee73b68e56ad4 (I'm not sure if the nvme spec requires the bar to be 64-bit capable.)

I've not been following this review, but I did run into a problem with bhyve's use of the MEM64 BAR recently with my work on updating UEFI firmware to edk2-stable202002. It turns out edk2 used to assume that MEM64 BARs were located above 4GB. That problem has since been fixed, and I've found NVMe to work well now. I've not however tested PCI passthrough yet.

That problem has since been fixed, and I've found NVMe to work well now.

Is this with PcdPciDisableBusEnumeration = TRUE in UEFI ?

Is this with PcdPciDisableBusEnumeration = TRUE in UEFI ?

Yes.

EPT invalidation has been tidied up with rS367593 so this is fine to go in now (and is gating D26209)

This revision is now accepted and ready to land.Nov 13 2020, 8:41 AM

Is this waiting on anything?

I've been meaning to spend some spare time on re-testing various
cases to make sure it hasn't broken something in the meantime. If
somebody has time to test out fbuf and passthru cases both with
and without https://github.com/freebsd/uefi-edk2/pull/9/ in the
uefi, that could help speed things along.

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
sys/amd64/include/vmm_dev.h
310

45 is now IOCNUM_PPTDEV_DISABLE_MSIX, please switch to 46 instead.

khng added a reviewer: scottph.
khng added a reviewer: scottph.
khng planned changes to this revision.Mar 17 2021, 1:14 PM
khng edited the summary of this revision. (Show Details)

Catch up with the latest changes in bhyve and vmm(4).

Add Originally by: scottph

This revision is now accepted and ready to land.Mar 18 2021, 12:51 AM

The "originally by" should probably be spelled "Submitted by".

Approved by: philip (mentor)

This revision was automatically updated to reflect the committed changes.

Sorry for the late reply. I am running through one of the reports which triggers the assert at the end of this function in 13.0-R and wondered why there is no error != 0 checks for the changes in pci_emul.c

usr.sbin/bhyve/pci_emul.c
528

Can someone explain to me what happens here in case register/unregister_inout have returned an error?
(same below for _mem).