Re: [PATCH v8 08/12] tests/vm: Added a new script for ubuntu.aarch64.

[Robert Foley]

On Sun, 31 May 2020 at 06:54, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
> > Hi Robert,
> >
> > On 5/29/20 10:34 PM, Robert Foley wrote:
> <snip>
> >> diff --git a/configure b/configure
> >> index d82de47fdd..8a286e75a5 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -411,6 +411,7 @@ prefix="/usr/local"
> >>  mandir="\${prefix}/share/man"
> >>  datadir="\${prefix}/share"
> >>  firmwarepath="\${prefix}/share/qemu-firmware"
> >> +efi_aarch64=""
> >>  qemu_docdir="\${prefix}/share/doc/qemu"
> >>  bindir="\${prefix}/bin"
> >>  libdir="\${prefix}/lib"
> >> @@ -1099,6 +1100,8 @@ for opt do
> >>    ;;
> >>    --firmwarepath=*) firmwarepath="$optarg"
> >>    ;;
> >> +  --efi-aarch64=*) efi_aarch64="$optarg"
> >> +  ;;
> >>    --host=*|--build=*|\
> >>    --disable-dependency-tracking|\
> >>    --sbindir=*|--sharedstatedir=*|\
> >> @@ -1753,6 +1756,7 @@ Advanced options (experts only):
> >>    --sysconfdir=PATH        install config in PATH$confsuffix
> >>    --localstatedir=PATH     install local state in PATH (set at runtime on 
> >> win32)
> >>    --firmwarepath=PATH      search PATH for firmware files
> >> +  --efi-aarch64=PATH       PATH of efi file to use for aarch64 VMs.
> >>    --with-confsuffix=SUFFIX suffix for QEMU data inside 
> >> datadir/libdir/sysconfdir [$confsuffix]
> >>    --with-pkgversion=VERS   use specified string as sub-version of the 
> >> package
> >>    --enable-debug           enable common debug build options
> >> @@ -3548,6 +3552,20 @@ EOF
> >>    fi
> >>  fi
> >>
> >> +############################################
> >> +# efi-aarch64 probe
> >> +# Check for efi files needed by aarch64 VMs.
> >> +# By default we will use the efi included with QEMU.
> >> +# Allow user to override the path for efi also.
> >> +if ! test -f "$efi_aarch64"; then
> >> +    if test -f $source_path/pc-bios/edk2-aaarch64-code.fd.bz2; then
> >> +        # valid after build
> >> +        efi_aarch64=$PWD/pc-bios/edk2-aarch64-code.fd
> >> +    else
> >> +        efi_aarch64=""
> >> +    fi
> >> +fi
> >> +
> <snip>
> >>
> >>  IMAGES_DIR := $(HOME)/.cache/qemu-vm/images
> >> @@ -23,6 +26,11 @@ vm-help vm-test:
> >>  ifneq ($(GENISOIMAGE),)
> >>      @echo "  vm-build-centos                 - Build QEMU in CentOS VM, 
> >> with Docker"
> >>      @echo "  vm-build-ubuntu.i386            - Build QEMU in ubuntu i386 
> >> VM"
> >> +ifneq ($(EFI_AARCH64),)
> >> +    @echo "  vm-build-ubuntu.aarch64         - Build QEMU in ubuntu 
> >> aarch64 VM"
> >> +else
> >> +    @echo "  (install qemu-efi-aarch64 to build centos/ubuntu aarch64 
> >> images.)"
> >
> > I'm not sure your test is working well, I have qemu-efi-aarch64
> > installed and it not automatically discovered.
> >
> > # apt install qemu-efi-aarch64
> > qemu-efi-aarch64 is already the newest version
> > (0~20191122.bd85bf54-2ubuntu3).
> >
> > Hint: I'm using out-of-tree builds.
>
> I think efi_aarch64="" needs to be the default path, which can then be
> overridden by the command line or finally fall back to the built in
> image.

I think what we are saying here is that if we want to use the qemu-efi-aarch64
package's image we should override it on the command line via --efi-aarch64.

Seems like the text above needs adjustment to something like:
> +ifneq ($(EFI_AARCH64),)
> +    @echo "  vm-build-ubuntu.aarch64         - Build QEMU in ubuntu aarch64 
> VM"
> +else
       @echo "  to build centos/ubuntu aarch64 images use configure
--efi-aarch64"

Thanks & Regards,
-Rob
>
> --
> Alex Bennée