Re: [PATCH v3 01/12] automation: add a QEMU aarch64 smoke test

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 4 Dec 2020, Wei Liu wrote:
> On Tue, Nov 24, 2020 at 08:27:34PM -0800, Stefano Stabellini wrote:
> > Use QEMU to start Xen (just the hypervisor) up until it stops because
> > there is no dom0 kernel to boot.
> > 
> > It is based on the existing build job unstable-arm64v8.
> > 
> > Also use make -j$(nproc) to build Xen.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > ---
> > Changes in v2:
> > - fix x86_32 build
> > ---
> >  automation/gitlab-ci/test.yaml         | 22 ++++++++++++++++++
> >  automation/scripts/build               |  6 ++---
> >  automation/scripts/qemu-smoke-arm64.sh | 32 ++++++++++++++++++++++++++
> >  3 files changed, 57 insertions(+), 3 deletions(-)
> >  create mode 100755 automation/scripts/qemu-smoke-arm64.sh
> > 
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index 793feafe8b..35346e3f6e 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -22,6 +22,28 @@ build-each-commit-gcc:
> >      - /^coverity-tested\/.*/
> >      - /^stable-.*/
> >  
> > +qemu-smoke-arm64-gcc:
> > +  stage: test
> > +  image: registry.gitlab.com/xen-project/xen/${CONTAINER}
> > +  variables:
> > +    CONTAINER: debian:unstable-arm64v8
> > +  script:
> > +    - ./automation/scripts/qemu-smoke-arm64.sh 2>&1 | tee 
> > qemu-smoke-arm64.log
> > +  dependencies:
> > +    - debian-unstable-gcc-arm64
> > +  artifacts:
> > +    paths:
> > +      - smoke.serial
> > +      - '*.log'
> > +    when: always
> > +  tags:
> > +    - arm64
> > +  except:
> > +    - master
> > +    - smoke
> > +    - /^coverity-tested\/.*/
> > +    - /^stable-.*/
> > +
> >  qemu-smoke-x86-64-gcc:
> >    stage: test
> >    image: registry.gitlab.com/xen-project/xen/${CONTAINER}
> > diff --git a/automation/scripts/build b/automation/scripts/build
> > index 0cd0f3971d..7038e5eb50 100755
> > --- a/automation/scripts/build
> > +++ b/automation/scripts/build
> > @@ -10,9 +10,9 @@ cc-ver()
> >  
> >  # random config or default config
> >  if [[ "${RANDCONFIG}" == "y" ]]; then
> > -    make -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig
> > +    make -j$(nproc) -C xen 
> > KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig
> >  else
> > -    make -C xen defconfig
> > +    make -j$(nproc) -C xen defconfig
> >  fi
> >  
> >  # build up our configure options
> > @@ -45,7 +45,7 @@ make -j$(nproc) dist
> >  # Extract artifacts to avoid getting rewritten by customised builds
> >  cp xen/.config xen-config
> >  mkdir binaries
> > -if [[ "${XEN_TARGET_ARCH}" == "x86_64" ]]; then
> > +if [[ "${XEN_TARGET_ARCH}" != "x86_32" ]]; then
> >      cp xen/xen binaries/xen
> >  fi
> >  
> > diff --git a/automation/scripts/qemu-smoke-arm64.sh 
> > b/automation/scripts/qemu-smoke-arm64.sh
> > new file mode 100755
> > index 0000000000..a7efbf8b6f
> > --- /dev/null
> > +++ b/automation/scripts/qemu-smoke-arm64.sh
> > @@ -0,0 +1,32 @@
> > +#!/bin/bash
> > +
> > +set -ex
> > +
> > +# Install QEMU
> > +export DEBIAN_FRONTENT=noninteractive
> > +apt-get -qy update
> > +apt-get -qy install --no-install-recommends qemu-system-aarch64 \
> > +                                            u-boot-qemu
> > +
> > +# XXX Silly workaround to get the following QEMU command to work
> > +cp /usr/share/qemu/pvh.bin /usr/share/qemu/efi-virtio.rom
> 
> Can you explain a bit more why this workaround works at all?
> 
> Not a blocking comment, but this will help other people who try to
> modify this script.

Yeah: the following QEMU command just after the copy is:

  qemu-system-aarch64 \
     -machine virtualization=true \
     -cpu cortex-a57 -machine type=virt \
     -m 512 -display none \
     -machine dumpdtb=binaries/virt-gicv3.dtb

The purpose for this command is just to generate the dtb for the
platform, see the "dumpdtb" option.

This version of QEMU refuses to do that unless it can load
"efi-virtio.rom"; although it doesn't make any sense because:
- we are not running anything here, only generating a DTB, no ROMs
  should be needed
- below when we actualy start QEMU to do emulation with the same
  options, "efi-virtio.rom" is not actually needed


I can expand a bit more on the comment, maybe:

# XXX Silly workaround to get the following QEMU command to work
# QEMU looks for "efi-virtio.rom" even if it is unneeded


Thank you for the ack on the series by the way. If you are OK with it, I
am going to wait for a couple of days in case of further comments, and
if there aren't any I'll commit the series making this change on commit.