git: 43caa2e805c2 - main - bhyve: Make boot ROM handling more consistent

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Mon, 19 Aug 2024 13:59:09 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=43caa2e805c28a236e6624aedd91591d7018fce5

commit 43caa2e805c28a236e6624aedd91591d7018fce5
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-08-19 13:55:47 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-08-19 13:55:47 +0000

    bhyve: Make boot ROM handling more consistent
    
    - On amd64, deprecate lpc.bootrom and lpc.bootvars.  Use top-level
      config variables instead.
    - Introduce a generic predicate which can be used to determine whether
      the guest has a boot ROM.
    
    Reviewed by:    corvink, jhb
    MFC after:      2 weeks
    Sponsored by:   Innovate UK
    Differential Revision:  https://reviews.freebsd.org/D46282
---
 usr.sbin/bhyve/amd64/bhyverun_machdep.c | 20 ++++++++++++++++++--
 usr.sbin/bhyve/amd64/ioapic.c           |  3 ++-
 usr.sbin/bhyve/amd64/pci_irq.c          |  3 ++-
 usr.sbin/bhyve/amd64/pci_lpc.c          | 19 ++-----------------
 usr.sbin/bhyve/amd64/pci_lpc.h          |  1 -
 usr.sbin/bhyve/bhyve_config.5           | 24 ++++++++++++------------
 usr.sbin/bhyve/bhyverun.c               |  7 +------
 usr.sbin/bhyve/bootrom.c                | 17 +++++++++++++----
 usr.sbin/bhyve/bootrom.h                |  3 ++-
 9 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/usr.sbin/bhyve/amd64/bhyverun_machdep.c b/usr.sbin/bhyve/amd64/bhyverun_machdep.c
index c453092107d5..d51ad3a5fc05 100644
--- a/usr.sbin/bhyve/amd64/bhyverun_machdep.c
+++ b/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"
@@ -241,6 +242,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
@@ -291,7 +304,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) {
@@ -332,6 +345,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);
 
@@ -355,7 +371,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")) {
diff --git a/usr.sbin/bhyve/amd64/ioapic.c b/usr.sbin/bhyve/amd64/ioapic.c
index 9ad1c501fbae..494fb0c7ae82 100644
--- a/usr.sbin/bhyve/amd64/ioapic.c
+++ b/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);
 	}
diff --git a/usr.sbin/bhyve/amd64/pci_irq.c b/usr.sbin/bhyve/amd64/pci_irq.c
index 7e1aee7fbb1d..fea6d9a2591c 100644
--- a/usr.sbin/bhyve/amd64/pci_irq.c
+++ b/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 {
diff --git a/usr.sbin/bhyve/amd64/pci_lpc.c b/usr.sbin/bhyve/amd64/pci_lpc.c
index 57d2333edcc6..ed41a800a2ea 100644
--- a/usr.sbin/bhyve/amd64/pci_lpc.c
+++ b/usr.sbin/bhyve/amd64/pci_lpc.c
@@ -104,7 +104,7 @@ 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) {
@@ -112,7 +112,7 @@ lpc_device_parse(const char *opts)
 				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++) {
diff --git a/usr.sbin/bhyve/amd64/pci_lpc.h b/usr.sbin/bhyve/amd64/pci_lpc.h
index 2dca8f7bec24..402eae082545 100644
--- a/usr.sbin/bhyve/amd64/pci_lpc.h
+++ b/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
diff --git a/usr.sbin/bhyve/bhyve_config.5 b/usr.sbin/bhyve/bhyve_config.5
index d0e5c8ae47d3..ebbb206cca9f 100644
--- a/usr.sbin/bhyve/bhyve_config.5
+++ b/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
@@ -550,17 +561,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
diff --git a/usr.sbin/bhyve/bhyverun.c b/usr.sbin/bhyve/bhyverun.c
index f844da90e76c..41655a188bf9 100644
--- a/usr.sbin/bhyve/bhyverun.c
+++ b/usr.sbin/bhyve/bhyverun.c
@@ -525,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) {
diff --git a/usr.sbin/bhyve/bootrom.c b/usr.sbin/bhyve/bootrom.c
index 1d461ba76597..e4adaca55947 100644
--- a/usr.sbin/bhyve/bootrom.c
+++ b/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 @@ done:
 	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);
+}
diff --git a/usr.sbin/bhyve/bootrom.h b/usr.sbin/bhyve/bootrom.h
index d22ac3718fa2..0477b0f35218 100644
--- a/usr.sbin/bhyve/bootrom.h
+++ b/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