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

Authored by jhb on Jul 29 2024, 8:44 PM.

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

jhb created this revision.Jul 29 2024, 8:44 PM

Herald added a reviewer: bhyve. · View Herald TranscriptJul 29 2024, 8:44 PM

Herald added subscribers: bcran, rgrimes, imp. · View Herald Transcript

jhb requested review of this revision.Jul 29 2024, 8:44 PM

jhb added a parent revision: D46179: bhyve: Treat the COMMAND register for PCI passthru devices as emulated.

Harbormaster completed remote builds in B58879: Diff 141543.Jul 29 2024, 8:44 PM

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

jhb added inline comments.Jul 29 2024, 10:40 PM

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?

jrtc27 added inline comments.Jul 29 2024, 10:42 PM

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.

jhb added inline comments.Jul 29 2024, 10:46 PM

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.

Revision Contents
Changeset List

Path

Size

usr.sbin/

bhyve/

pci_emul.c

20 lines

Diff 141543

View Options

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