Re: git: e962b37bf0ff - main - bhyve: Do not enable PCI BAR decoding if a boot ROM is present

Hey Mark,

This commit seems to force me to now pass "-o pci.enable_bars=true" to
all my VMs on amd64. I wonder if that might be a POLA violation. I
didn't realize that I needed to set that until I bisected the src
tree, looking for the commit that broke bhyve for me.

Is changing the default here really worth it for amd64? If so, I'm
thinking this should be in both RELNOTES and UPDATING. I now have to
propigate re-enabling this across my entire infrastructure.

Thanks,

-- 
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

Tor-ified Signal: +1 303-901-1600 / shawn_webb_opsec.50
https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc

On Mon, Aug 19, 2024 at 01:59:10PM UTC, Mark Johnston wrote:
> The branch main has been updated by markj:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=e962b37bf0ffe7f30f5b025b46ea49ba01c71f2f
> 
> commit e962b37bf0ffe7f30f5b025b46ea49ba01c71f2f
> Author:     Mark Johnston <markj@FreeBSD.org>
> AuthorDate: 2024-08-19 13:56:06 +0000
> Commit:     Mark Johnston <markj@FreeBSD.org>
> CommitDate: 2024-08-19 13:56:06 +0000
> 
>     bhyve: Do not enable PCI BAR decoding if a boot ROM is present
>     
>     Let the boot ROM handle BAR initialization.  This fixes a problem where
>     u-boot's BAR remapping conflicts with some limitations in bhyve.  See
>     https://lists.freebsd.org/archives/freebsd-virtualization/2024-April/002103.html
>     for a description of what goes wrong.
>     
>     The old behaviour can be restored by setting the pci.enable_bars
>     configuration variable.
>     
>     Reviewed by:    corvink, jhb
>     Sponsored by:   Innovate UK
>     Differential Revision:  https://reviews.freebsd.org/D45049
> ---
>  usr.sbin/bhyve/bhyve_config.5 |  3 +++
>  usr.sbin/bhyve/pci_emul.c     | 27 ++++++++++++++++++++++++---
>  2 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/usr.sbin/bhyve/bhyve_config.5 b/usr.sbin/bhyve/bhyve_config.5
> index ebbb206cca9f..25185e2ef1b4 100644
> --- a/usr.sbin/bhyve/bhyve_config.5
> +++ b/usr.sbin/bhyve/bhyve_config.5
> @@ -157,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.
> diff --git a/usr.sbin/bhyve/pci_emul.c b/usr.sbin/bhyve/pci_emul.c
> index 00e9138d3910..e066d6766f3c 100644
> --- a/usr.sbin/bhyve/pci_emul.c
> +++ b/usr.sbin/bhyve/pci_emul.c
> @@ -48,6 +48,7 @@
>  
>  #include "acpi.h"
>  #include "bhyverun.h"
> +#include "bootrom.h"
>  #include "config.h"
>  #include "debug.h"
>  #ifdef __amd64__
> @@ -853,6 +854,14 @@ pci_emul_alloc_bar(struct pci_devinst *pdi, int idx, enum pcibar_type type,
>  		TAILQ_INSERT_BEFORE(bar, new_bar, chain);
>  	}
>  
> +	/*
> +	 * Enable PCI BARs only if we don't have a boot ROM, i.e., bhyveload was
> +	 * used to load the initial guest image.  Otherwise, we rely on the boot
> +	 * ROM to handle this.
> +	 */
> +	if (!get_config_bool_default("pci.enable_bars", !bootrom_boot()))
> +		return (0);
> +
>  	/*
>  	 * pci_passthru devices synchronize their physical and virtual command
>  	 * register on init. For that reason, the virtual cmd reg should be
> @@ -966,8 +975,19 @@ pci_emul_assign_bar(struct pci_devinst *const pdi, const int idx,
>  		pci_set_cfgdata32(pdi, PCIR_BAR(idx + 1), bar >> 32);
>  	}
>  
> -	if (type != PCIBAR_ROM) {
> -		register_bar(pdi, idx);
> +	switch (type) {
> +	case PCIBAR_IO:
> +		if (porten(pdi))
> +			register_bar(pdi, idx);
> +		break;
> +	case PCIBAR_MEM32:
> +	case PCIBAR_MEM64:
> +	case PCIBAR_MEMHI64:
> +		if (memen(pdi))
> +			register_bar(pdi, idx);
> +		break;
> +	default:
> +		break;
>  	}
>  
>  	return (0);
> @@ -1140,7 +1160,8 @@ pci_emul_init(struct vmctx *ctx, struct pci_devemu *pde, int bus, int slot,
>  	pci_set_cfgdata8(pdi, PCIR_INTLINE, 255);
>  	pci_set_cfgdata8(pdi, PCIR_INTPIN, 0);
>  
> -	pci_set_cfgdata8(pdi, PCIR_COMMAND, PCIM_CMD_BUSMASTEREN);
> +	if (!get_config_bool_default("pci.enable_bars", !bootrom_boot()))
> +		pci_set_cfgdata8(pdi, PCIR_COMMAND, PCIM_CMD_BUSMASTEREN);
>  
>  	err = (*pde->pe_init)(pdi, fi->fi_config);
>  	if (err == 0)
>