Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 13 Apr 2017 03:29:28 -0700 (PDT)
From:      "Rodney W. Grimes" <freebsd@pdx.rh.CN85.dnsmgr.net>
To:        Allan Jude <allanjude@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r316746 - head/share/examples/bhyve
Message-ID:  <201704131029.v3DATSUQ000973@pdx.rh.CN85.dnsmgr.net>
In-Reply-To: <201704130007.v3D07eQ9014409@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
> Author: allanjude
> Date: Thu Apr 13 00:07:39 2017
> New Revision: 316746
> URL: https://svnweb.freebsd.org/changeset/base/316746
> 
> Log:
>   Add UEFI support to vmrun.sh
>   
>   Adds:
>    -E: Use UEFI mode
>    -f: path to UEFI firmware image (default: path to uefi-edk2-bhyve package)
>    -F: UEFI framebuffer size (default: w=1024,h=768)
>    -L: IP to listen for VNC connections on (default: 127.0.0.1)
>    -P: Port to listen for VNC connections on (default: 5900)
>    -T: Enable tablnet device (for VNC)
>    -v: Wait for VNC client before booting VM
>   
>   Submitted by:	Shawn Webb <shawn.webb@hardenedbsd.org>
>   MFC after:	2 weeks
>   Relnotes:	yes
>   Differential Revision:	https://reviews.freebsd.org/D10378

This spent 15 minutes in differtial?  Something is wrong if that is how
we use the diffential process.  That isnt even enough time for someone
to have reacted to a notice this was there.
We do mailing list CC: for bugzilla for when bugs are related to areas,
do we do anything like that in reviews.freebsd.org?  Or just the projects
lists?

As a general not I dont think we want vmrun.sh to become a too relied upon
way to start bhyve vm's, a million options to this and you still wont have
it doing what needs done, and the more we add to this the more we are going
to have to keep it around and support it.  I would like to see vmrun.sh
go away sooner rather than later with the use of config files.

> Modified:
>   head/share/examples/bhyve/vmrun.sh
> 
> Modified: head/share/examples/bhyve/vmrun.sh
> ==============================================================================
> --- head/share/examples/bhyve/vmrun.sh	Wed Apr 12 21:37:12 2017	(r316745)
> +++ head/share/examples/bhyve/vmrun.sh	Thu Apr 13 00:07:39 2017	(r316746)
> @@ -46,10 +46,12 @@ errmsg() {
>  usage() {
>  	local msg=$1
>  
> -	echo "Usage: vmrun.sh [-ahi] [-c <CPUs>] [-C <console>] [-d <disk file>]"
> -	echo "                [-e <name=value>] [-g <gdbport> ] [-H <directory>]"
> +	echo "Usage: vmrun.sh [-aEhiTv] [-c <CPUs>] [-C <console>] [-d <disk file>]"
> +	echo "                [-e <name=value>] [-f <path of firmware>] [-F <size>]"
> +	echo "                [-g <gdbport> ] [-H <directory>]"
>  	echo "                [-I <location of installation iso>] [-l <loader>]"
> -	echo "                [-m <memsize>] [-t <tapdev>] <vmname>"
> +	echo "                [-L <VNC IP for UEFI framebuffer>]"
> +	echo "                [-m <memsize>] [-P <port>] [-t <tapdev>] <vmname>"
>  	echo ""
>  	echo "       -h: display this help message"
>  	echo "       -a: force memory mapped local APIC access"
> @@ -57,15 +59,22 @@ usage() {
>  	echo "       -C: console device (default is ${DEFAULT_CONSOLE})"
>  	echo "       -d: virtio diskdev file (default is ${DEFAULT_VIRTIO_DISK})"
>  	echo "       -e: set FreeBSD loader environment variable"
> +	echo "       -E: Use UEFI mode"
> +	echo "       -f: Use a specific UEFI firmware"
> +	echo "       -F: Use a custom UEFI GOP framebuffer size (default: w=1024,h=768)"
>  	echo "       -g: listen for connection from kgdb at <gdbport>"
>  	echo "       -H: host filesystem to export to the loader"
>  	echo "       -i: force boot of the Installation CDROM image"
>  	echo "       -I: Installation CDROM image location (default is ${DEFAULT_ISOFILE})"
>  	echo "       -l: the OS loader to use (default is /boot/userboot.so)"
> +	echo "       -L: IP address for UEFI GOP VNC server (default: 127.0.0.1)"
>  	echo "       -m: memory size (default is ${DEFAULT_MEMSIZE})"
>  	echo "       -p: pass-through a host PCI device at bus/slot/func (e.g. 10/0/0)"
> +	echo "       -P: UEFI GOP VNC port (default: 5900)"
>  	echo "       -t: tap device for virtio-net (default is $DEFAULT_TAPDEV)"
> +	echo "       -T: Enable tablet device (for UEFI GOP)"
>  	echo "       -u: RTC keeps UTC time"
> +	echo "       -v: Wait for VNC client connection before booting VM"
>  	echo "       -w: ignore unimplemented MSRs"
>  	echo ""
>  	[ -n "$msg" ] && errmsg "$msg"
> @@ -95,7 +104,16 @@ loader_opt=""
>  bhyverun_opt="-H -A -P"
>  pass_total=0
>  
> -while getopts ac:C:d:e:g:hH:iI:l:m:p:t:uw c ; do
> +# EFI-specific options
> +efi_mode=0
> +efi_firmware="/usr/local/share/uefi-firmware/BHYVE_UEFI.fd"

Hard coding /usr/local in the base system is not a good idea, as far
as I know this has never been acceptable as the user is allowed to
load ports to any PREFIX they prefer.

> +vncwait=""
> +vnchost="127.0.0.1"
> +vncport=5900
> +fbsize="w=1024,h=768"

These already have defaults inside the binaries, duplicating
them here only leads to maintanance issues.

> +tablet=""
> +
> +while getopts ac:C:d:e:Ef:F:g:hH:iI:l:m:p:P:t:Tuvw c ; do
>  	case $c in
>  	a)
>  		bhyverun_opt="${bhyverun_opt} -a"
> @@ -116,6 +134,15 @@ while getopts ac:C:d:e:g:hH:iI:l:m:p:t:u
>  	e)
>  		loader_opt="${loader_opt} -e ${OPTARG}"
>  		;;
> +	E)
> +		efi_mode=1
> +		;;
> +	f)
> +		efi_firmware="${OPTARG}"
> +		;;
> +	F)
> +		fbsize="${OPTARG}"
> +		;;
>  	g)	
>  		gdbport=${OPTARG}
>  		;;
> @@ -131,6 +158,9 @@ while getopts ac:C:d:e:g:hH:iI:l:m:p:t:u
>  	l)
>  		loader_opt="${loader_opt} -l ${OPTARG}"
>  		;;
> +	L)
> +		vnchost="${OPTARG}"
> +		;;
>  	m)
>  		memsize=${OPTARG}
>  		;;
> @@ -138,13 +168,22 @@ while getopts ac:C:d:e:g:hH:iI:l:m:p:t:u
>  		eval "pass_dev${pass_total}=\"${OPTARG}\""
>  		pass_total=$(($pass_total + 1))
>  		;;
> +	P)
> +		vncport="${OPTARG}"
> +		;;
>  	t)
>  		eval "tap_dev${tap_total}=\"${OPTARG}\""
>  		tap_total=$(($tap_total + 1))
>  		;;
> +	T)
> +		tablet="-s 30,xhci,tablet"
> +		;;
>  	u)	
>  		bhyverun_opt="${bhyverun_opt} -u"
>  		;;
> +	v)
> +		vncwait=",wait"
> +		;;
>  	w)
>  		bhyverun_opt="${bhyverun_opt} -w"
>  		;;
> @@ -181,6 +220,13 @@ if [ ${pass_total} -gt 0 ]; then
>  	bhyverun_opt="${bhyverun_opt} -S"
>  fi
>  
> +if [ ${efi_mode} -gt 0 ]; then
> +	if [ ! -f ${efi_firmware} ]; then
> +		echo "Error: EFI Firmware ${efi_firmware} doesn't exist. Try: pkg install uefi-edk2-bhyve"

Not sure if it is a good idea to make recomendations that well probably
outdate faster than releases, and uefi-edk2 is not the only optional
firmware we can load, iirc.

> +		exit 1
> +	fi
> +fi
> +
>  make_and_check_diskdev()
>  {
>      local virtio_diskdev="$1"
> @@ -243,11 +289,13 @@ while [ 1 ]; do
>  		installer_opt=""
>  	fi
>  
> -	${LOADER} -c ${console} -m ${memsize} ${BOOTDISKS} ${loader_opt} \
> -		${vmname}
> -	bhyve_exit=$?
> -	if [ $bhyve_exit -ne 0 ]; then
> -		break
> +	if [ ${efi_mode} -eq 0 ]; then
> +		${LOADER} -c ${console} -m ${memsize} ${BOOTDISKS} ${loader_opt} \
> +			${vmname}
> +		bhyve_exit=$?
> +		if [ $bhyve_exit -ne 0 ]; then
> +			break
> +		fi
>  	fi
>  
>  	#
> @@ -281,10 +329,18 @@ while [ 1 ]; do
>  	    i=$(($i + 1))
>          done
>  
> +	efiargs=""
> +	if [ ${efi_mode} -gt 0 ]; then
> +		efiargs="-s 29,fbuf,tcp=${vnchost}:${vncport},${fbsize}${vncwait}"

This can be recoded so that if the user does not set vnchost, vncport, fbsize, vncwait these
are nulled and the defaults that already exist in the binary are used.

> +		efiargs="${efiargs} -l bootrom,${efi_firmware}"
> +		efiargs="${efiargs} ${tablet}"
> +	fi
> +
>  	${FBSDRUN} -c ${cpus} -m ${memsize} ${bhyverun_opt}		\
>  		-g ${gdbport}						\
>  		-s 0:0,hostbridge					\
>  		-s 1:0,lpc						\
> +		${efiargs}						\
>  		${devargs}						\
>  		-l com1,${console}					\
>  		${installer_opt}					\
> 
> 

-- 
Rod Grimes                                                 rgrimes@freebsd.org



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