Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 6 Sep 2024 02:10:30 +0000
From:      Shawn Webb <shawn.webb@hardenedbsd.org>
To:        Mark Johnston <markj@freebsd.org>
Cc:        src-committers@freebsd.org, dev-commits-src-all@freebsd.org,  dev-commits-src-main@freebsd.org
Subject:   Re: git: e962b37bf0ff - main - bhyve: Do not enable PCI BAR decoding if a boot ROM is present
Message-ID:  <qkp2zbmykgwsbrxekut35rexlktypzg5oj2bbfslig7eksprpi@2lw5x47mtytp>
In-Reply-To: <202408191359.47JDxAbK026029@gitrepo.freebsd.org>
References:  <202408191359.47JDxAbK026029@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--g5khyvupkahmttj3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hey Mark,

This commit seems to force me to now pass "-o pci.enable_bars=3Dtrue" 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,

--=20
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/03A=
4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc

On Mon, Aug 19, 2024 at 01:59:10PM UTC, Mark Johnston wrote:
> The branch main has been updated by markj:
>=20
> URL: https://cgit.FreeBSD.org/src/commit/?id=3De962b37bf0ffe7f30f5b025b46=
ea49ba01c71f2f
>=20
> 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
>=20
>     bhyve: Do not enable PCI BAR decoding if a boot ROM is present
>    =20
>     Let the boot ROM handle BAR initialization.  This fixes a problem whe=
re
>     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.
>    =20
>     The old behaviour can be restored by setting the pci.enable_bars
>     configuration variable.
>    =20
>     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(-)
>=20
> 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 otherwis=
e.
>  .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 @@
> =20
>  #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);
>  	}
> =20
> +	/*
> +	 * 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, c=
onst int idx,
>  		pci_set_cfgdata32(pdi, PCIR_BAR(idx + 1), bar >> 32);
>  	}
> =20
> -	if (type !=3D 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;
>  	}
> =20
>  	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);
> =20
> -	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);
> =20
>  	err =3D (*pde->pe_init)(pdi, fi->fi_config);
>  	if (err =3D=3D 0)
>=20

--g5khyvupkahmttj3
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAmbaZIgACgkQ/y5nonf4
4fqYDQ//eMyzfZ4TVp20Zm7/sUsJh6YKTROAvwI0Lzt7H70FavlXkkUF1+L1qqTQ
J9xpLPlN8ee+24b7DcQh4Aad+JGSB0rc46fhDSlBx2RMkceiL+fvbVtPSNdSRfF6
qrpoShG0PKqR9hwINK8lqrRTT/SyF6g4AKh1n+mt9jEtJ9thj8QP53mLEYqiKukH
/Kg+grYL0QsmYJYI2LCPJBXSXFF+5IyOtmt6QVn+W2v0WTQr0OBtHIeKZNKv0/02
CJGjRRx0I5yt1aAXNGZJXwzICLbqeucuxf3+jaCo900fsIqSQK4QbVRufTkWcFvp
t0r2Piwi9TjaoOZpo61mdbN1k+2YXyQkXiKfVh6Os7Y4afBLLJb8v10d3EobqYXp
4B5atNWnAmbpB9TRaPM2LKsJyTzcDKSRFk2DHxM4Z0j0Jhr96K1U0DffuEL+Dokt
v5nHXSNDltnXQo9+iP4FIThJJEIStxXf20LoCZcwPOvg+llthwSMhLIOhwn9e+75
A77V3cV7/bNbu/0a47kwLNoyLMkBepTXkbBIME71EFYhO6StP5t+zLHWXbV3T0eM
X/ZcQspiOs5KYcOSP/XV0BJkgebqhokIioNI4MFothxP4Pqxc9xZPtBu98nnkt7v
ZBeGucp42vo1eyp2AEUrJVimI3P4vk6JVkLUJIs9rDQr5OxL+4M=
=CpdA
-----END PGP SIGNATURE-----

--g5khyvupkahmttj3--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?qkp2zbmykgwsbrxekut35rexlktypzg5oj2bbfslig7eksprpi>