bhyve: Treat BAR registration failures as non-fatal

Some guest operating systems (at least 9front) will leave device I/O
space decoding enabled while probing BAR sizes.  In this case, a
sufficiently large 32-bit BAR can overlap with the HPET MMIO region
fixed at 0xfed00000.  This results in an assertion failure in
modify_bar_registration(), even though the collision is transient, i.e.,
the guest will promptly map the BAR elsewhere.

Tolerate such overlaps.  Keep track of a BAR's registration state and
make sure that modify_bar_registration() doesn't try to unregister an
I/O space if registration failed.

This fixes a problem where a 32MB framebuffer ends up overlapping with
the HPET MMIO region, causing some guests to crash with an assertion
failure when probing BARs during boot.

PR:		290098
Reviewed by:	bnovkov
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D60748

usr.sbin/bhyve/pci_emul.c

@@ -639,12 +639,16 @@ static void
 modify_bar_registration(struct pci_devinst *pi, int idx, int registration)
 {
 	struct pci_devemu *pe;
+	struct pcibar *bar;
 	int error;
-	enum pcibar_type type;
 
 	pe = pi->pi_d;
-	type = pi->pi_bar[idx].type;
-	switch (type) {
+	bar = &pi->pi_bar[idx];
+
+	if (!registration && !bar->registered)
+		return;
+
+	switch (bar->type) {
 	case PCIBAR_IO:
 	{
 #ifdef __amd64__
@@ -652,30 +656,34 @@ modify_bar_registration(struct pci_devinst *pi, int idx, int registration)
 
 		bzero(&iop, sizeof(struct inout_port));
 		iop.name = pi->pi_name;
-		iop.port = pi->pi_bar[idx].addr;
-		iop.size = pi->pi_bar[idx].size;
+		iop.port = bar->addr;
+		iop.size = bar->size;
 		if (registration) {
 			iop.flags = IOPORT_F_INOUT;
 			iop.handler = pci_emul_io_handler;
 			iop.arg = pi;
 			error = register_inout(&iop);
-		} else
+		} else {
 			error = unregister_inout(&iop);
+			assert(error == 0);
+		}
 #else
 		struct mem_range mr;
 
 		bzero(&mr, sizeof(struct mem_range));
 		mr.name = pi->pi_name;
-		mr.base = pi->pi_bar[idx].addr;
-		mr.size = pi->pi_bar[idx].size;
+		mr.base = bar->addr;
+		mr.size = bar->size;
 		if (registration) {
 			mr.flags = MEM_F_RW;
 			mr.handler = pci_emul_iomem_handler;
 			mr.arg1 = pi;
 			mr.arg2 = idx;
 			error = register_mem(&mr);
-		} else
+		} else {
 			error = unregister_mem(&mr);
+			assert(error == 0);
+		}
 #endif
 		break;
 	}
@@ -686,16 +694,18 @@ modify_bar_registration(struct pci_devinst *pi, int idx, int registration)
 
 		bzero(&mr, sizeof(struct mem_range));
 		mr.name = pi->pi_name;
-		mr.base = pi->pi_bar[idx].addr;
-		mr.size = pi->pi_bar[idx].size;
+		mr.base = bar->addr;
+		mr.size = bar->size;
 		if (registration) {
 			mr.flags = MEM_F_RW;
 			mr.handler = pci_emul_mem_handler;
 			mr.arg1 = pi;
 			mr.arg2 = idx;
 			error = register_mem(&mr);
-		} else
+		} else {
 			error = unregister_mem(&mr);
+			assert(error == 0);
+		}
 		break;
 	}
 	case PCIBAR_ROM:
@@ -705,10 +715,16 @@ modify_bar_registration(struct pci_devinst *pi, int idx, int registration)
 		error = EINVAL;
 		break;
 	}
-	assert(error == 0);
 
-	if (pe->pe_baraddr != NULL)
-		(*pe->pe_baraddr)(pi, idx, registration, pi->pi_bar[idx].addr);
+	/*
+	 * An error may legitimately occur if the guest is probing BARs and
+	 * leaves decoding enabled.  Keep track of this if it happens so that we
+	 * can skip unregistration.
+	 */
+	bar->registered = (registration && error == 0);
+
+	if (error == 0 && pe->pe_baraddr != NULL)
+		(*pe->pe_baraddr)(pi, idx, registration, bar->addr);
 }
 
 static void

usr.sbin/bhyve/pci_emul.h

@@ -39,6 +39,7 @@
 #include <dev/pci/pcireg.h>
 
 #include <assert.h>
+#include <stdbool.h>
 
 #include "pci_irq.h"
 
@@ -99,6 +100,7 @@ struct pcibar {
 	uint64_t		size;
 	uint64_t		addr;
 	uint8_t			lobits;
+	bool			registered;	/* io region is registered */
 };
 
 #define PI_NAMESZ	40