Re: [kvm-unit-tests PATCH v2] configure: arm/arm64: Add --earlycon option to set UART type and address

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <alexandru.elisei@xxxxxxx>
> ---
> 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/1569245722-23375-1-git-send-email-alexandru.elisei@xxxxxxx/
> 
>  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
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux