On Thu, Mar 18, 2021 at 04:20:22PM +0000, Alexandru Elisei wrote:
> Currently, the UART early address is set indirectly with the --vmm option
> and there are only two possible values: if the VMM is qemu (the default),
> then the UART address is set to 0x09000000; if the VMM is kvmtool, then the
> UART address is set to 0x3f8.
>
> The upstream kvmtool commit 45b4968e0de1 ("hw/serial: ARM/arm64: Use MMIO
> at higher addresses") changed the UART address to 0x1000000, and
> kvm-unit-tests so far hasn't had mechanism to let the user set a specific
> address, which means that for recent versions of kvmtool the early UART
> won't be available.
>
> This situation will only become worse as kvm-unit-tests gains support to
> run as an EFI app, as each platform will have their own UART type and
> address.
>
> To address both issues, a new configure option is added, --earlycon. The
> syntax and semantics are identical to the kernel parameter with the same
> name. For example, for kvmtool, --earlycon=uart,mmio,0x1000000 will set the
> correct UART address. Specifying this option will overwrite the UART
> address set by --vmm.
>
> At the moment, the UART type and register width parameters are ignored
> since both qemu's and kvmtool's UART emulation use the same offset for the
> TX register and no other registers are used by kvm-unit-tests, but the
> parameters will become relevant once EFI support is added.
>
> Signed-off-by: Alexandru Elisei <[email protected]>
> ---
> Besides working with current versions of kvmtool, this will also make early
> console work if the user specifies a custom memory layout [1] (patches are
> old, but I plan to pick them up at some point in the future).
>
> Changes in v2:
> * kvmtool patches were merged, so I reworked the commit message to point to
> the corresponding kvmtool commit.
> * Restricted pl011 register size to 32 bits, as per Arm Base System
> Architecture 1.0 (DEN0094A), and to match Linux.
> * Reworked the way the fields are extracted to make it more precise
> (without the -s argument, the entire string is echo'ed when no delimiter
> is found).
You can also drop 'cut' and just do something like
IFS=, read -r name type_addr addr <<<"$earlycon"
> * The changes are not trivial, so I dropped Drew's Reviewed-by.
>
> [1]
> https://lore.kernel.org/kvm/[email protected]/
>
> configure | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/configure b/configure
> index cdcd34e94030..137b165db18f 100755
> --- a/configure
> +++ b/configure
> @@ -26,6 +26,7 @@ errata_force=0
> erratatxt="$srcdir/errata.txt"
> host_key_document=
> page_size=
> +earlycon=
>
> usage() {
> cat <<-EOF
> @@ -54,6 +55,18 @@ usage() {
> --page-size=PAGE_SIZE
> Specify the page size (translation granule)
> (4k, 16k or
> 64k, default is 64k, arm64 only)
> + --earlycon=EARLYCON
> + Specify the UART name, type and address
> (optional, arm and
> + arm64 only). The specified address will
> overwrite the UART
> + address set by the --vmm option. EARLYCON
> can be on of (case
'on of' typo still here
> + sensitive):
> + uart[8250],mmio,ADDR
> + Specify an 8250 compatible UART at address
> ADDR. Supported
> + register stride is 8 bit only.
> + pl011,ADDR
> + pl011,mmio32,ADDR
> + Specify a PL011 compatible UART at address
> ADDR. Supported
> + register stride is 32 bit only.
> EOF
> exit 1
> }
> @@ -112,6 +125,9 @@ while [[ "$1" = -* ]]; do
> --page-size)
> page_size="$arg"
> ;;
> + --earlycon)
> + earlycon="$arg"
> + ;;
> --help)
> usage
> ;;
> @@ -170,6 +186,51 @@ elif [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
> echo '--vmm must be one of "qemu" or "kvmtool"!'
> usage
> fi
> +
> + if [ "$earlycon" ]; then
> + # Append delimiter and use cut -s to prevent cut from ignoring the
> field
> + # argument if no delimiter is specified by the user.
> + earlycon="$earlycon,"
> + name=$(echo $earlycon|cut -sd',' -f1)
> + if [ "$name" != "uart" ] && [ "$name" != "uart8250" ] &&
> + [ "$name" != "pl011" ]; then
> + echo "unknown earlycon name: $name"
> + usage
> + fi
> +
> + if [ "$name" = "pl011" ]; then
> + type_addr=$(echo $earlycon|cut -sd',' -f2)
> + if [ -z "$type_addr" ]; then
> + echo "missing earlycon address"
> + usage
> + fi
> + addr=$(echo $earlycon|cut -sd',' -f3)
> + if [ -z "$addr" ]; then
Don't you need
if [ "$type_addr" = "mmio32" ]; then
echo "missing earlycon address"
usage
fi
here to avoid accepting
pl011,mmio32
and then assigning mmio32 to the address?
And/or should we do a quick sanity check on the address?
Something like
[[ $addr =~ ^0?x?[0-9a-f]+$ ]]
> + addr=$type_addr
> + else
> + if [ "$type_addr" != "mmio32" ]; then
> + echo "unknown $name earlycon type: $type_addr"
> + usage
> + fi
> + fi
> + else
> + type=$(echo $earlycon|cut -sd',' -f2)
> + if [ -z "$type" ]; then
> + echo "missing $name earlycon type"
> + usage
> + fi
> + if [ "$type" != "mmio" ]; then
> + echo "unknown $name earlycon type: $type"
> + usage
> + fi
> + addr=$(echo $earlycon|cut -sd',' -f3)
> + if [ -z "$addr" ]; then
> + echo "missing earlycon address"
> + usage
> + fi
> + fi
> + arm_uart_early_addr=$addr
> + fi
> elif [ "$arch" = "ppc64" ]; then
> testdir=powerpc
> firmware="$testdir/boot_rom.bin"
> --
> 2.30.2
>
Thanks,
drew
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
