Assorted bhyve bug fixes

usr.sbin/bhyve/aarch64/vmexit.c

@@ -43,6 +43,7 @@
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <unistd.h>
 
 #include <vmmapi.h>
 
@@ -132,6 +133,11 @@ vmexit_debug(struct vmctx *ctx __unused, struct vcpu *vcpu,
     struct vm_run *vmrun __unused)
 {
 	gdb_cpu_suspend(vcpu);
+	/*
+	 * XXX-MJ sleep for a short period to avoid chewing up the CPU in the
+	 * window between activation of the vCPU thread and the STARTUP IPI.
+	 */
+	usleep(1000);
 	return (VMEXIT_CONTINUE);
 }
 
@@ -243,7 +249,8 @@ vmexit_smccc(struct vmctx *ctx, struct vcpu *vcpu, struct vm_run *vmrun)
 			how = VM_SUSPEND_POWEROFF;
 		else
 			how = VM_SUSPEND_RESET;
-		vm_suspend(ctx, how);
+		error = vm_suspend(ctx, how);
+		assert(error == 0 || errno == EALREADY);
 		break;
 	default:
 		break;

usr.sbin/bhyve/amd64/bhyverun_machdep.c

@@ -37,6 +37,7 @@
 #include "acpi.h"
 #include "atkbdc.h"
 #include "bhyverun.h"
+#include "bootrom.h"
 #include "config.h"
 #include "debug.h"
 #include "e820.h"
@@ -237,6 +238,18 @@ bhyve_optparse(int argc, char **argv)
 			bhyve_usage(1);
 		}
 	}
+
+	/* Handle backwards compatibility aliases in config options. */
+	if (get_config_value("lpc.bootrom") != NULL &&
+	    get_config_value("bootrom") == NULL) {
+		warnx("lpc.bootrom is deprecated, use '-o bootrom' instead");
+		set_config_value("bootrom", get_config_value("lpc.bootrom"));
+	}
+	if (get_config_value("lpc.bootvars") != NULL &&
+	    get_config_value("bootvars") == NULL) {
+		warnx("lpc.bootvars is deprecated, use '-o bootvars' instead");
+		set_config_value("bootvars", get_config_value("lpc.bootvars"));
+	}
 }
 
 void
@@ -287,7 +300,7 @@ bhyve_start_vcpu(struct vcpu *vcpu, bool bsp)
 	int error;
 
 	if (bsp) {
-		if (lpc_bootrom()) {
+		if (bootrom_boot()) {
 			error = vm_set_capability(vcpu,
 			    VM_CAP_UNRESTRICTED_GUEST, 1);
 			if (error != 0) {
@@ -328,6 +341,9 @@ bhyve_init_platform(struct vmctx *ctx, struct vcpu *bsp __unused)
 	rtc_init(ctx);
 	sci_init(ctx);
 	error = e820_init(ctx);
+	if (error != 0)
+		return (error);
+	error = bootrom_loadrom(ctx);
 	if (error != 0)
 		return (error);
 
@@ -351,7 +367,7 @@ bhyve_init_platform_late(struct vmctx *ctx, struct vcpu *bsp __unused)
 	if (error != 0)
 		return (error);
 
-	if (lpc_bootrom() && strcmp(lpc_fwcfg(), "bhyve") == 0)
+	if (bootrom_boot() && strcmp(lpc_fwcfg(), "bhyve") == 0)
 		fwctl_init();
 
 	if (get_config_bool("acpi_tables")) {

usr.sbin/bhyve/amd64/ioapic.c

@@ -33,6 +33,7 @@
 #include <machine/vmm.h>
 #include <vmmapi.h>
 
+#include "bootrom.h"
 #include "ioapic.h"
 #include "pci_emul.h"
 #include "pci_lpc.h"
@@ -72,7 +73,7 @@ ioapic_pci_alloc_irq(struct pci_devinst *pi)
 
 	if (pci_pins == 0)
 		return (-1);
-	if (lpc_bootrom()) {
+	if (bootrom_boot()) {
 		/* For external bootrom use fixed mapping. */
 		return (16 + (4 + pi->pi_slot + pi->pi_lintr.pin) % 8);
 	}

usr.sbin/bhyve/amd64/pci_irq.c

@@ -38,6 +38,7 @@
 #include <vmmapi.h>
 
 #include "acpi.h"
+#include "bootrom.h"
 #include "inout.h"
 #include "ioapic.h"
 #include "pci_emul.h"
@@ -205,7 +206,7 @@ pirq_alloc_pin(struct pci_devinst *pi)
 
 	pirq_cold = 0;
 
-	if (lpc_bootrom()) {
+	if (bootrom_boot()) {
 		/* For external bootrom use fixed mapping. */
 		best_pin = (4 + pi->pi_slot + pi->pi_lintr.pin) % 8;
 	} else {

usr.sbin/bhyve/amd64/pci_lpc.c

@@ -104,15 +104,15 @@ lpc_device_parse(const char *opts)
 			if (romfile == NULL) {
 				errx(4, "invalid bootrom option \"%s\"", opts);
 			}
-			set_config_value("lpc.bootrom", romfile);
+			set_config_value("bootrom", romfile);
 
 			varfile = strsep(&str, ",");
 			if (varfile == NULL) {
 				error = 0;
 				goto done;
 			}
 			if (strchr(varfile, '=') == NULL) {
-				set_config_value("lpc.bootvars", varfile);
+				set_config_value("bootvars", varfile);
 			} else {
 				/* varfile doesn't exist, it's another config
 				 * option */
@@ -182,13 +182,6 @@ lpc_print_supported_devices(void)
 	printf("%s\n", pctestdev_getname());
 }
 
-const char *
-lpc_bootrom(void)
-{
-
-	return (get_config_value("lpc.bootrom"));
-}
-
 const char *
 lpc_fwcfg(void)
 {
@@ -256,14 +249,6 @@ lpc_init(struct vmctx *ctx)
 	const char *backend, *name;
 	char *node_name;
 	int unit, error;
-	const nvlist_t *nvl;
-
-	nvl = find_config_node("lpc");
-	if (nvl != NULL && nvlist_exists(nvl, "bootrom")) {
-		error = bootrom_loadrom(ctx, nvl);
-		if (error)
-			return (error);
-	}
 
 	/* COM1 and COM2 */
 	for (unit = 0; unit < LPC_UART_NUM; unit++) {

usr.sbin/bhyve/amd64/pci_lpc.h

@@ -69,7 +69,6 @@ int	lpc_device_parse(const char *opt);
 void    lpc_print_supported_devices(void);
 char	*lpc_pirq_name(int pin);
 void	lpc_pirq_routed(void);
-const char *lpc_bootrom(void);
 const char *lpc_fwcfg(void);
 
 #endif

usr.sbin/bhyve/bhyve_config.5

@@ -23,7 +23,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd November 20, 2023
+.Dd August 13, 2024
 .Dt BHYVE_CONFIG 5
 .Os
 .Sh NAME
@@ -120,6 +120,17 @@ The value must be formatted as described in
 .Xr expand_number 3 .
 .It Va memory.wired Ta bool Ta false Ta
 Wire guest memory.
+.It Va bootrom Ta path Ta Ta
+Path to a boot ROM.
+During initialization of the guest, the contents of this file are copied into
+the guest's memory.
+If a boot ROM is present, a firmware interface device is
+also enabled for use by the boot ROM.
+.It Va bootvars Ta path Ta Ta
+Path to boot VARS.
+The contents of this file are copied beneath the boot ROM.
+Firmware can write to it to save variables.
+All variables will be persistent even on reboots of the guest.
 .It Va acpi_tables Ta bool Ta true Ta
 Generate ACPI tables.
 .It Va acpi_tables_in_memory Ta bool Ta true Ta
@@ -146,6 +157,9 @@ Specify the keyboard layout name with the file name in
 This value only works when loaded with UEFI mode for VNC, and
 used a VNC client that don't support QEMU Extended Key Event
 Message (e.g. TightVNC).
+.It Va pci.enable_bars Ta bool Ta Ta
+Enable and map PCI BARs before executing any guest code.
+This setting is false by default when using a boot ROM and true otherwise.
 .It Va tpm.path Ta string Ta Ta
 Path to the host TPM device.
 This is typically /dev/tpm0.
@@ -550,17 +564,6 @@ The following nodes are available under
 .Va lpc :
 .Bl -column "pc-testdev" "Format" "Default"
 .It Sy Name Ta Sy Format Ta Sy Default Ta Sy Description
-.It Va bootrom Ta path Ta Ta
-Path to a boot ROM.
-The contents of this file are copied into the guest's
-memory ending just before the 4GB physical address.
-If a boot ROM is present, a firmware interface device is
-also enabled for use by the boot ROM.
-.It Va bootvars Ta path Ta Ta
-Path to boot VARS.
-The contents of this file are copied beneath the boot ROM.
-Firmware can write to it to save variables.
-All variables will be persistent even on reboots of the guest.
 .It Va com1 Ta node Ta Ta
 Settings for the COM1 serial port device.
 .It Va com2 Ta node Ta Ta

usr.sbin/bhyve/bhyverun.c

@@ -413,7 +413,8 @@ fbsdrun_addcpu(int vcpuid)
 
 	CPU_SET_ATOMIC(vcpuid, &cpumask);
 
-	vm_suspend_cpu(vi->vcpu);
+	error = vm_suspend_cpu(vi->vcpu);
+	assert(error == 0);
 
 	error = pthread_create(&thr, NULL, fbsdrun_start_thread, vi);
 	assert(error == 0);
@@ -524,12 +525,7 @@ do_open(const char *vmname)
 
 	reinit = false;
 
-#ifdef __amd64__
-	romboot = lpc_bootrom() != NULL;
-#else
-	romboot = true;
-#endif
-
+	romboot = bootrom_boot();
 	error = vm_create(vmname);
 	if (error) {
 		if (errno == EEXIST) {

usr.sbin/bhyve/bootrom.c

@@ -192,7 +192,7 @@ bootrom_alloc(struct vmctx *ctx, size_t len, int prot, int flags,
 }
 
 int
-bootrom_loadrom(struct vmctx *ctx, const nvlist_t *nvl)
+bootrom_loadrom(struct vmctx *ctx)
 {
 	struct stat sbuf;
 	ssize_t rlen;
@@ -204,9 +204,9 @@ bootrom_loadrom(struct vmctx *ctx, const nvlist_t *nvl)
 	rv = -1;
 	varfd = -1;
 
-	bootrom = get_config_value_node(nvl, "bootrom");
+	bootrom = get_config_value("bootrom");
 	if (bootrom == NULL) {
-		return (-1);
+		return (0);
 	}
 
 	/*
@@ -235,7 +235,7 @@ bootrom_loadrom(struct vmctx *ctx, const nvlist_t *nvl)
 
 	rom_size = sbuf.st_size;
 
-	varfile = get_config_value_node(nvl, "bootvars");
+	varfile = get_config_value("bootvars");
 	var_size = 0;
 	if (varfile != NULL) {
 		varfd = open(varfile, O_RDWR);
@@ -314,3 +314,12 @@ bootrom_loadrom(struct vmctx *ctx, const nvlist_t *nvl)
 	free(romfile);
 	return (rv);
 }
+
+/*
+ * Are we relying on a bootrom to initialize the guest's CPU context?
+ */
+bool
+bootrom_boot(void)
+{
+	return (get_config_value("bootrom") != NULL);
+}

usr.sbin/bhyve/bootrom.h

@@ -45,6 +45,7 @@ enum {
 };
 int bootrom_alloc(struct vmctx *ctx, size_t len, int prot, int flags,
     char **region_out, uint64_t *gpa_out);
-int bootrom_loadrom(struct vmctx *ctx, const nvlist_t *nvl);
+bool bootrom_boot(void);
+int bootrom_loadrom(struct vmctx *ctx);
 
 #endif

usr.sbin/bhyve/gdb.c

@@ -696,6 +696,28 @@ append_packet_data(const uint8_t *data, size_t len)
 	}
 }
 
+static void
+append_binary_data(const uint8_t *data, size_t len)
+{
+	uint8_t buf[2];
+
+	for (; len > 0; data++, len--) {
+		switch (*data) {
+		case '}':
+		case '#':
+		case '$':
+		case '*':
+			buf[0] = 0x7d;
+			buf[1] = *data ^ 0x20;
+			append_packet_data(buf, 2);
+			break;
+		default:
+			append_packet_data(data, 1);
+			break;
+		}
+	}
+}
+
 static void
 append_string(const char *str)
 {
@@ -975,8 +997,28 @@ gdb_cpu_add(struct vcpu *vcpu)
 	 * executing the first instruction.
 	 */
 	if (!CPU_EMPTY(&vcpus_suspended)) {
+		cpuset_t suspended;
+		int error;
+
+		error = vm_debug_cpus(ctx, &suspended);
+		assert(error == 0);
+
 		CPU_SET(vcpuid, &vcpus_suspended);
 		_gdb_cpu_suspend(vcpu, false);
+
+		/*
+		 * In general, APs are started in a suspended mode such that
+		 * they exit with VM_EXITCODE_DEBUG until the BSP starts them.
+		 * In particular, this refers to the kernel's view of the vCPU
+		 * state rather than our own.  If the debugger resumes guest
+		 * execution, vCPUs will be unsuspended from the kernel's point
+		 * of view, so we should restore the previous state before
+		 * continuing.
+		 */
+		if (CPU_ISSET(vcpuid, &suspended)) {
+			error = vm_suspend_cpu(vcpu);
+			assert(error == 0);
+		}
 	}
 	pthread_mutex_unlock(&gdb_lock);
 }
@@ -1867,10 +1909,10 @@ gdb_query(const uint8_t *data, size_t len)
 			append_char('l');
 		} else if (doff + dlen >= xmllen) {
 			append_char('l');
-			append_packet_data(xml + doff, xmllen - doff);
+			append_binary_data(xml + doff, xmllen - doff);
 		} else {
 			append_char('m');
-			append_packet_data(xml + doff, dlen);
+			append_binary_data(xml + doff, dlen);
 		}
 		finish_packet();
 		(void)munmap(__DECONST(void *, xml), xmllen);
@@ -1934,11 +1976,11 @@ gdb_query(const uint8_t *data, size_t len)
 			append_char('l');
 		} else if (doff + dlen >= sizeof(capbuf)) {
 			append_char('l');
-			append_packet_data(capbuf + doff,
+			append_binary_data(capbuf + doff,
 			    sizeof(capbuf) - doff);
 		} else {
 			append_char('m');
-			append_packet_data(capbuf + doff, dlen);
+			append_binary_data(capbuf + doff, dlen);
 		}
 		finish_packet();
 #else