Page MenuHomeFreeBSD

bhyve pci_emul: Do not enable BAR decoding when booting from a bootrom
AbandonedPublic

Authored by jhb on Jul 29 2024, 8:44 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jan 11, 4:39 PM
Unknown Object (File)
Dec 2 2024, 10:57 PM
Unknown Object (File)
Nov 25 2024, 2:04 PM
Unknown Object (File)
Nov 19 2024, 2:52 AM
Unknown Object (File)
Nov 19 2024, 2:52 AM
Unknown Object (File)
Nov 19 2024, 2:32 AM
Unknown Object (File)
Nov 13 2024, 11:20 AM
Unknown Object (File)
Oct 15 2024, 7:45 AM
Subscribers

Details

Reviewers
jrtc27
markj
Group Reviewers
bhyve
Summary

Boot ROMs such as u-boot and UEFI expect PCI devices to have decoding
disabled on startup and size BARs without explicitly disabling
decoding first. This can cause confusion with register/unregistering
memory regions during BAR sizing resulting in assertion failures in
bhyve itself.

PR: 265869
Sponsored by: Chelsio Communications

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58879
Build 55766: arc lint + arc unit

Event Timeline

Jess reported this previously with u-boot, but I ran into this today with UEFI.

s/UEFI/EDK2/ in the description

usr.sbin/bhyve/pci_emul.c
987

This changes PCIBAR_NONE's behaviour; is that intended?

This is similar to D45049 but also changes the behaviour of amd64. In that revision I mentioned hitting a problem with EDK2, but I can't reproduce it anymore...

usr.sbin/bhyve/pci_emul.c
798

This is the second place we use this pattern now (another instance in bhyverun.c). If we had a ${ARCH}/bhyverun_machdep.h I would maybe define a romboot() macro there instead.

987

It shouldn't? decoding is set to false before the giant switch and the PCIBAR_NONE case doesn't set it, so it should remain false?

usr.sbin/bhyve/pci_emul.c
987

PCIBAR_NONE != PCIBAR_ROM, so it previously called register_bar, but now doesn't since decoding is false.

usr.sbin/bhyve/pci_emul.c
987

That was probably a bug before. Note that modify_bar_registration returns an EINVAL error for PCI_BAR_NONE and then assert()'s that error == 0, so it didn't work before and after this change would no longer be an assertion failure. I'm not sure anything passes PCI_BAR_NONE here.

Hmm, so I think we probably should do the version with a tunable so we can override it if needed. Today I hit the same issue I think you hit originally with EDK2 where for some reason bhyve assigned resources to pass through devices in higher numbered slots first, but EDK2 started at slot 0 so it started assigning conflicting resources. I do think we should make the tunable default to false when using a bootrom, but it can be forced on for old EDK2 ROMs if really needed. I would do an early return as I do in this version instead of indenting all the code and doing a dummy read/write of the cmd register.

D45049 will fix this once it is merged.