[PATCH 0/2] Create a test job for testing static memory on qemu

[PATCH 0/2] Create a test job for testing static memory on qemu

[Xenia Ragiadakou]

This patch series
- removes all the references to the XEN_CONFIG_EXPERT environmental variable
which is not used anymore
- creates a trivial arm64 test job that boots xen on qemu with a direct mapped
dom0less domu with static memory and verifies, based on its logs, that the
domu's memory node ranges are the same as the static memory ranges with which
it was configured

Xenia Ragiadakou (2):
  automation: Remove XEN_CONFIG_EXPERT leftovers
  automation: arm64: Create a test job for testing static allocation on
    qemu

 automation/build/README.md                 |   3 -
 automation/configs/arm/static_mem          |   3 +
 automation/gitlab-ci/test.yaml             |  24 +++++
 automation/scripts/build                   |   8 +-
 automation/scripts/containerize            |   1 -
 automation/scripts/qemu-staticmem-arm64.sh | 114 +++++++++++++++++++++
 6 files changed, 147 insertions(+), 6 deletions(-)
 create mode 100644 automation/configs/arm/static_mem
 create mode 100755 automation/scripts/qemu-staticmem-arm64.sh

-- 
2.34.1

[PATCH 1/2] automation: Remove XEN_CONFIG_EXPERT leftovers

[Xenia Ragiadakou]

The EXPERT config option cannot anymore be selected via the environmental
variable XEN_CONFIG_EXPERT. Remove stale references to XEN_CONFIG_EXPERT
from the automation code.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 automation/build/README.md      | 3 ---
 automation/scripts/build        | 4 ++--
 automation/scripts/containerize | 1 -
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/automation/build/README.md b/automation/build/README.md
index 2137957408..00305eed03 100644
--- a/automation/build/README.md
+++ b/automation/build/README.md
@@ -65,9 +65,6 @@ understands.
 - CONTAINER_NO_PULL: If set to 1, the script will not pull from docker hub.
   This is useful when testing container locally.
 
-- XEN_CONFIG_EXPERT: If this is defined in your shell it will be
-  automatically passed through to the container.
-
 If your docker host has Linux kernel > 4.11, and you want to use containers
 that run old glibc (for example, CentOS 6 or SLES11SP4), you may need to add
 
diff --git a/automation/scripts/build b/automation/scripts/build
index 281f8b1fcc..21b3bc57c8 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -91,6 +91,6 @@ for cfg in `ls ${cfg_dir}`; do
     echo "Building $cfg"
     make -j$(nproc) -C xen clean
     rm -f xen/.config
-    make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} XEN_CONFIG_EXPERT=y defconfig
-    make -j$(nproc) -C xen XEN_CONFIG_EXPERT=y
+    make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
+    make -j$(nproc) -C xen
 done
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index 8992c67278..9d4beca4fa 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -101,7 +101,6 @@ exec ${docker_cmd} run \
     -v "${CONTAINER_PATH}":/build:rw${selinux} \
     -v "${HOME}/.ssh":/root/.ssh:ro \
     ${SSH_AUTH_DIR:+-v "${SSH_AUTH_DIR}":/tmp/ssh-agent${selinux}} \
-    ${XEN_CONFIG_EXPERT:+-e XEN_CONFIG_EXPERT=${XEN_CONFIG_EXPERT}} \
     ${CONTAINER_ARGS} \
     -${termint}i --rm -- \
     ${CONTAINER} \
-- 
2.34.1

[PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Xenia Ragiadakou]

Add an arm subdirectory under automation/configs for the arm specific configs
and add a config that enables static allocation.

Modify the build script to search for configs also in this subdirectory and to
keep the generated xen binary, suffixed with the config file name, as artifact.

Create a test job that
- boots xen on qemu with a single direct mapped dom0less guest configured with
statically allocated memory
- verifies that the memory ranges reported in the guest's logs are the same
with the provided static memory regions

For guest kernel, use the 5.9.9 kernel from the tests-artifacts containers.
Use busybox-static package, to create the guest ramdisk.
To generate the u-boot script, use ImageBuilder.
Use the qemu from the tests-artifacts containers.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 automation/configs/arm/static_mem          |   3 +
 automation/gitlab-ci/test.yaml             |  24 +++++
 automation/scripts/build                   |   4 +
 automation/scripts/qemu-staticmem-arm64.sh | 114 +++++++++++++++++++++
 4 files changed, 145 insertions(+)
 create mode 100644 automation/configs/arm/static_mem
 create mode 100755 automation/scripts/qemu-staticmem-arm64.sh

diff --git a/automation/configs/arm/static_mem b/automation/configs/arm/static_mem
new file mode 100644
index 0000000000..84675ddf4e
--- /dev/null
+++ b/automation/configs/arm/static_mem
@@ -0,0 +1,3 @@
+CONFIG_EXPERT=y
+CONFIG_UNSUPPORTED=y
+CONFIG_STATIC_MEMORY=y
\ No newline at end of file
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 42cd725a12..dc181f3777 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -205,3 +205,27 @@ qemu-smoke-x86-64-clang-pvh:
     - smoke
     - /^coverity-tested\/.*/
     - /^stable-.*/
+
+qemu-staticmem-arm64-gcc:
+  stage: test
+  image: registry.gitlab.com/xen-project/xen/${CONTAINER}
+  variables:
+    CONTAINER: debian:unstable-arm64v8
+  script:
+    - ./automation/scripts/qemu-staticmem-arm64.sh 2>&1 | tee qemu-staticmem-arm64.log
+  dependencies:
+    - debian-unstable-gcc-arm64
+    - kernel-5.9.9-arm64-export
+    - qemu-system-aarch64-6.0.0-arm64-export
+  artifacts:
+    paths:
+      - qemu-staticmem.serial
+      - '*.log'
+    when: always
+  tags:
+    - arm64
+  except:
+    - master
+    - smoke
+    - /^coverity-tested\/.*/
+    - /^stable-.*/
diff --git a/automation/scripts/build b/automation/scripts/build
index 21b3bc57c8..9c6196d9bd 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -83,6 +83,7 @@ fi
 # Build all the configs we care about
 case ${XEN_TARGET_ARCH} in
     x86_64) arch=x86 ;;
+    arm64) arch=arm ;;
     *) exit 0 ;;
 esac
 
@@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
     rm -f xen/.config
     make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
     make -j$(nproc) -C xen
+    if [[ ${arch} == "arm" ]]; then
+        cp xen/xen binaries/xen-${cfg}
+    fi
 done
diff --git a/automation/scripts/qemu-staticmem-arm64.sh b/automation/scripts/qemu-staticmem-arm64.sh
new file mode 100755
index 0000000000..5b89a151aa
--- /dev/null
+++ b/automation/scripts/qemu-staticmem-arm64.sh
@@ -0,0 +1,114 @@
+#!/bin/bash
+
+base=(0x50000000 0x100000000)
+size=(0x10000000 0x10000000)
+
+set -ex
+
+apt-get -qy update
+apt-get -qy install --no-install-recommends u-boot-qemu \
+                                            u-boot-tools \
+                                            device-tree-compiler \
+                                            cpio \
+                                            curl \
+                                            busybox-static
+
+# DomU Busybox
+cd binaries
+mkdir -p initrd
+mkdir -p initrd/bin
+mkdir -p initrd/sbin
+mkdir -p initrd/etc
+mkdir -p initrd/dev
+mkdir -p initrd/proc
+mkdir -p initrd/sys
+mkdir -p initrd/lib
+mkdir -p initrd/var
+mkdir -p initrd/mnt
+cp /bin/busybox initrd/bin/busybox
+initrd/bin/busybox --install initrd/bin
+echo "#!/bin/sh
+
+mount -t proc proc /proc
+mount -t sysfs sysfs /sys
+mount -t devtmpfs devtmpfs /dev
+/bin/sh" > initrd/init
+chmod +x initrd/init
+cd initrd
+find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
+cd ../..
+
+# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
+curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
+
+./binaries/qemu-system-aarch64 -nographic \
+    -M virtualization=true \
+    -M virt \
+    -M virt,gic-version=2 \
+    -cpu cortex-a57 \
+    -smp 2 \
+    -m 8G \
+    -M dumpdtb=binaries/virt-gicv2.dtb
+
+#dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts
+
+# ImageBuilder
+rm -rf imagebuilder
+git clone https://gitlab.com/ViryaOS/imagebuilder
+
+echo "MEMORY_START=\"0x40000000\"
+MEMORY_END=\"0x0200000000\"
+
+DEVICE_TREE=\"virt-gicv2.dtb\"
+
+XEN=\"xen-static_mem\"
+XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
+
+NUM_DOMUS=1
+DOMU_MEM[0]=512
+DOMU_VCPUS[0]=1
+DOMU_KERNEL[0]=\"Image\"
+DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
+DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
+DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
+
+UBOOT_SOURCE=\"boot.source\"
+UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
+
+bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/imagebuilder_config
+
+# Run the test
+rm -f qemu-staticmem.serial
+set +e
+echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
+timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
+    -M virtualization=true \
+    -M virt \
+    -M virt,gic-version=2 \
+    -cpu cortex-a57 \
+    -smp 2 \
+    -m 8G \
+    -no-reboot \
+    -device virtio-net-pci,netdev=vnet0 -netdev user,id=vnet0,tftp=binaries \
+    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
+    -dtb ./binaries/virt-gicv2.dtb \
+    |& tee qemu-staticmem.serial
+
+set -e
+
+(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
+
+for ((i=0; i<${#base[@]}; i++))
+do
+    start="$(printf "0x%016x" ${base[$i]})"
+    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
+    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
+    if test $? -eq 1
+    then
+        exit 1
+    fi
+done
+
+(grep -q "BusyBox" qemu-staticmem.serial) || exit 1
+
+exit 0
-- 
2.34.1

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Julien Grall]

Hi Xenia,

On 07/07/2022 21:38, Xenia Ragiadakou wrote:
> Add an arm subdirectory under automation/configs for the arm specific configs
> and add a config that enables static allocation.
> 
> Modify the build script to search for configs also in this subdirectory and to
> keep the generated xen binary, suffixed with the config file name, as artifact.
> 
> Create a test job that
> - boots xen on qemu with a single direct mapped dom0less guest configured with
> statically allocated memory
> - verifies that the memory ranges reported in the guest's logs are the same
> with the provided static memory regions
> 
> For guest kernel, use the 5.9.9 kernel from the tests-artifacts containers.
> Use busybox-static package, to create the guest ramdisk.
> To generate the u-boot script, use ImageBuilder.
> Use the qemu from the tests-artifacts containers.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> ---
>   automation/configs/arm/static_mem          |   3 +
>   automation/gitlab-ci/test.yaml             |  24 +++++
>   automation/scripts/build                   |   4 +
>   automation/scripts/qemu-staticmem-arm64.sh | 114 +++++++++++++++++++++
>   4 files changed, 145 insertions(+)
>   create mode 100644 automation/configs/arm/static_mem
>   create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
> 
> diff --git a/automation/configs/arm/static_mem b/automation/configs/arm/static_mem
> new file mode 100644
> index 0000000000..84675ddf4e
> --- /dev/null
> +++ b/automation/configs/arm/static_mem
> @@ -0,0 +1,3 @@
> +CONFIG_EXPERT=y
> +CONFIG_UNSUPPORTED=y
> +CONFIG_STATIC_MEMORY=y
> \ No newline at end of file

Any particular reason to build a new Xen rather enable 
CONFIG_STATIC_MEMORY in the existing build?

> diff --git a/automation/scripts/build b/automation/scripts/build
> index 21b3bc57c8..9c6196d9bd 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -83,6 +83,7 @@ fi
>   # Build all the configs we care about
>   case ${XEN_TARGET_ARCH} in
>       x86_64) arch=x86 ;;
> +    arm64) arch=arm ;;
>       *) exit 0 ;;
>   esac
>   
> @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
>       rm -f xen/.config
>       make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
>       make -j$(nproc) -C xen
> +    if [[ ${arch} == "arm" ]]; then
> +        cp xen/xen binaries/xen-${cfg}
> +    fi

This feels a bit of a hack to be arm only. Can you explain why this is 
not enabled for x86 (other than this is not yet used)?

>   done
> diff --git a/automation/scripts/qemu-staticmem-arm64.sh b/automation/scripts/qemu-staticmem-arm64.sh
> new file mode 100755
> index 0000000000..5b89a151aa
> --- /dev/null
> +++ b/automation/scripts/qemu-staticmem-arm64.sh
> @@ -0,0 +1,114 @@
> +#!/bin/bash
> +
> +base=(0x50000000 0x100000000)
> +size=(0x10000000 0x10000000)

 From the name, it is not clear what the base and size refers too. 
Looking a bit below, it seems to be referring to the domain memory. If 
so, I would suggest to comment and rename to "domu_{base, size}".

> +
> +set -ex
> +
> +apt-get -qy update
> +apt-get -qy install --no-install-recommends u-boot-qemu \
> +                                            u-boot-tools \
> +                                            device-tree-compiler \
> +                                            cpio \
> +                                            curl \
> +                                            busybox-static
> +
> +# DomU Busybox
> +cd binaries
> +mkdir -p initrd
> +mkdir -p initrd/bin
> +mkdir -p initrd/sbin
> +mkdir -p initrd/etc
> +mkdir -p initrd/dev
> +mkdir -p initrd/proc
> +mkdir -p initrd/sys
> +mkdir -p initrd/lib
> +mkdir -p initrd/var
> +mkdir -p initrd/mnt
> +cp /bin/busybox initrd/bin/busybox
> +initrd/bin/busybox --install initrd/bin
> +echo "#!/bin/sh
> +
> +mount -t proc proc /proc
> +mount -t sysfs sysfs /sys
> +mount -t devtmpfs devtmpfs /dev
> +/bin/sh" > initrd/init
> +chmod +x initrd/init
> +cd initrd
> +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
> +cd ../..
> +
> +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
> +curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
> +
> +./binaries/qemu-system-aarch64 -nographic \
> +    -M virtualization=true \
> +    -M virt \
> +    -M virt,gic-version=2 \
> +    -cpu cortex-a57 \
> +    -smp 2 \
> +    -m 8G \
> +    -M dumpdtb=binaries/virt-gicv2.dtb
> +
> +#dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts
> +
> +# ImageBuilder
> +rm -rf imagebuilder
> +git clone https://gitlab.com/ViryaOS/imagebuilder
> +
> +echo "MEMORY_START=\"0x40000000\"
> +MEMORY_END=\"0x0200000000\"
> +
> +DEVICE_TREE=\"virt-gicv2.dtb\"
> +
> +XEN=\"xen-static_mem\"
> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"

AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is 
also not clear why you need to pass xsm=dummy.

> +
> +NUM_DOMUS=1
> +DOMU_MEM[0]=512
> +DOMU_VCPUS[0]=1
> +DOMU_KERNEL[0]=\"Image\"
> +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
> +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
> +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
> +
> +UBOOT_SOURCE=\"boot.source\"
> +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
> +
> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/imagebuilder_config
> +
> +# Run the test
> +rm -f qemu-staticmem.serial
> +set +e
> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
> +timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
> +    -M virtualization=true \
> +    -M virt \
> +    -M virt,gic-version=2 \
> +    -cpu cortex-a57 \
> +    -smp 2 \
> +    -m 8G \
> +    -no-reboot \
> +    -device virtio-net-pci,netdev=vnet0 -netdev user,id=vnet0,tftp=binaries \
> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
> +    -dtb ./binaries/virt-gicv2.dtb \
> +    |& tee qemu-staticmem.serial
> +
> +set -e

A lot of the code above is duplicated from qemu-smoke-arm64.sh. I think 
it would be better to consolidate in a single script. Looking briefly 
throught the existing scripts, it looks like it is possible to pass 
arguments (see qemu-smoke-x86-64.sh).

> +
> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
> +
> +for ((i=0; i<${#base[@]}; i++))
> +do
> +    start="$(printf "0x%016x" ${base[$i]})"
> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
> +    if test $? -eq 1
> +    then
> +        exit 1
> +    fi
> +done

Please add a comment on top to explain what this is meant to do. 
However, I think we should avoid relying on output that we have written 
ourself. IOW, relying on Xen/Linux to always write the same message is 
risky because they can change at any time.

> +
> +(grep -q "BusyBox" qemu-staticmem.serial) || exit 1

Cheers,

-- 
Julien Grall

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Stefano Stabellini]

On Thu, 7 Jul 2022, Julien Grall wrote:
> Hi Xenia,
> 
> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
> > Add an arm subdirectory under automation/configs for the arm specific
> > configs
> > and add a config that enables static allocation.
> > 
> > Modify the build script to search for configs also in this subdirectory and
> > to
> > keep the generated xen binary, suffixed with the config file name, as
> > artifact.
> > 
> > Create a test job that
> > - boots xen on qemu with a single direct mapped dom0less guest configured
> > with
> > statically allocated memory
> > - verifies that the memory ranges reported in the guest's logs are the same
> > with the provided static memory regions
> > 
> > For guest kernel, use the 5.9.9 kernel from the tests-artifacts containers.
> > Use busybox-static package, to create the guest ramdisk.
> > To generate the u-boot script, use ImageBuilder.
> > Use the qemu from the tests-artifacts containers.
> > 
> > Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> > ---
> >   automation/configs/arm/static_mem          |   3 +
> >   automation/gitlab-ci/test.yaml             |  24 +++++
> >   automation/scripts/build                   |   4 +
> >   automation/scripts/qemu-staticmem-arm64.sh | 114 +++++++++++++++++++++
> >   4 files changed, 145 insertions(+)
> >   create mode 100644 automation/configs/arm/static_mem
> >   create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
> > 
> > diff --git a/automation/configs/arm/static_mem
> > b/automation/configs/arm/static_mem
> > new file mode 100644
> > index 0000000000..84675ddf4e
> > --- /dev/null
> > +++ b/automation/configs/arm/static_mem
> > @@ -0,0 +1,3 @@
> > +CONFIG_EXPERT=y
> > +CONFIG_UNSUPPORTED=y
> > +CONFIG_STATIC_MEMORY=y
> > \ No newline at end of file
> 
> Any particular reason to build a new Xen rather enable CONFIG_STATIC_MEMORY in
> the existing build?
> 
> > diff --git a/automation/scripts/build b/automation/scripts/build
> > index 21b3bc57c8..9c6196d9bd 100755
> > --- a/automation/scripts/build
> > +++ b/automation/scripts/build
> > @@ -83,6 +83,7 @@ fi
> >   # Build all the configs we care about
> >   case ${XEN_TARGET_ARCH} in
> >       x86_64) arch=x86 ;;
> > +    arm64) arch=arm ;;
> >       *) exit 0 ;;
> >   esac
> >   @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
> >       rm -f xen/.config
> >       make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
> >       make -j$(nproc) -C xen
> > +    if [[ ${arch} == "arm" ]]; then
> > +        cp xen/xen binaries/xen-${cfg}
> > +    fi
> 
> This feels a bit of a hack to be arm only. Can you explain why this is not
> enabled for x86 (other than this is not yet used)?
> 
> >   done
> > diff --git a/automation/scripts/qemu-staticmem-arm64.sh
> > b/automation/scripts/qemu-staticmem-arm64.sh
> > new file mode 100755
> > index 0000000000..5b89a151aa
> > --- /dev/null
> > +++ b/automation/scripts/qemu-staticmem-arm64.sh
> > @@ -0,0 +1,114 @@
> > +#!/bin/bash
> > +
> > +base=(0x50000000 0x100000000)
> > +size=(0x10000000 0x10000000)
> 
> From the name, it is not clear what the base and size refers too. Looking a
> bit below, it seems to be referring to the domain memory. If so, I would
> suggest to comment and rename to "domu_{base, size}".
> 
> > +
> > +set -ex
> > +
> > +apt-get -qy update
> > +apt-get -qy install --no-install-recommends u-boot-qemu \
> > +                                            u-boot-tools \
> > +                                            device-tree-compiler \
> > +                                            cpio \
> > +                                            curl \
> > +                                            busybox-static
> > +
> > +# DomU Busybox
> > +cd binaries
> > +mkdir -p initrd
> > +mkdir -p initrd/bin
> > +mkdir -p initrd/sbin
> > +mkdir -p initrd/etc
> > +mkdir -p initrd/dev
> > +mkdir -p initrd/proc
> > +mkdir -p initrd/sys
> > +mkdir -p initrd/lib
> > +mkdir -p initrd/var
> > +mkdir -p initrd/mnt
> > +cp /bin/busybox initrd/bin/busybox
> > +initrd/bin/busybox --install initrd/bin
> > +echo "#!/bin/sh
> > +
> > +mount -t proc proc /proc
> > +mount -t sysfs sysfs /sys
> > +mount -t devtmpfs devtmpfs /dev
> > +/bin/sh" > initrd/init
> > +chmod +x initrd/init
> > +cd initrd
> > +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
> > +cd ../..
> > +
> > +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
> > +curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
> > +
> > +./binaries/qemu-system-aarch64 -nographic \
> > +    -M virtualization=true \
> > +    -M virt \
> > +    -M virt,gic-version=2 \
> > +    -cpu cortex-a57 \
> > +    -smp 2 \
> > +    -m 8G \
> > +    -M dumpdtb=binaries/virt-gicv2.dtb
> > +
> > +#dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts
> > +
> > +# ImageBuilder
> > +rm -rf imagebuilder
> > +git clone https://gitlab.com/ViryaOS/imagebuilder
> > +
> > +echo "MEMORY_START=\"0x40000000\"
> > +MEMORY_END=\"0x0200000000\"
> > +
> > +DEVICE_TREE=\"virt-gicv2.dtb\"
> > +
> > +XEN=\"xen-static_mem\"
> > +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
> 
> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is also not
> clear why you need to pass xsm=dummy.
> 
> > +
> > +NUM_DOMUS=1
> > +DOMU_MEM[0]=512
> > +DOMU_VCPUS[0]=1
> > +DOMU_KERNEL[0]=\"Image\"
> > +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
> > +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
> > +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
> > +
> > +UBOOT_SOURCE=\"boot.source\"
> > +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
> > +
> > +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c
> > binaries/imagebuilder_config
> > +
> > +# Run the test
> > +rm -f qemu-staticmem.serial
> > +set +e
> > +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
> > +timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
> > +    -M virtualization=true \
> > +    -M virt \
> > +    -M virt,gic-version=2 \
> > +    -cpu cortex-a57 \
> > +    -smp 2 \
> > +    -m 8G \
> > +    -no-reboot \
> > +    -device virtio-net-pci,netdev=vnet0 -netdev user,id=vnet0,tftp=binaries
> > \
> > +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
> > +    -dtb ./binaries/virt-gicv2.dtb \
> > +    |& tee qemu-staticmem.serial
> > +
> > +set -e
> 
> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I think it
> would be better to consolidate in a single script. Looking briefly throught
> the existing scripts, it looks like it is possible to pass arguments (see
> qemu-smoke-x86-64.sh).
 
One idea would be to make the script common and "source" a second
script or config file with just the ImageBuilder configuration because
it looks like it is the only thing different.


> > +
> > +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
> > +
> > +for ((i=0; i<${#base[@]}; i++))
> > +do
> > +    start="$(printf "0x%016x" ${base[$i]})"
> > +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
> > +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
> > +    if test $? -eq 1
> > +    then
> > +        exit 1
> > +    fi
> > +done
> 
> Please add a comment on top to explain what this is meant to do. However, I
> think we should avoid relying on output that we have written ourself. IOW,
> relying on Xen/Linux to always write the same message is risky because they
> can change at any time.

Especially if we make the script common, then we could just rely on the
existing check to see if the guest started correctly (no special check
for static memory).


> > +
> > +(grep -q "BusyBox" qemu-staticmem.serial) || exit 1
 

Re: [PATCH 1/2] automation: Remove XEN_CONFIG_EXPERT leftovers

[Stefano Stabellini]

On Thu, 7 Jul 2022, Xenia Ragiadakou wrote:
> The EXPERT config option cannot anymore be selected via the environmental
> variable XEN_CONFIG_EXPERT. Remove stale references to XEN_CONFIG_EXPERT
> from the automation code.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  automation/build/README.md      | 3 ---
>  automation/scripts/build        | 4 ++--
>  automation/scripts/containerize | 1 -
>  3 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/automation/build/README.md b/automation/build/README.md
> index 2137957408..00305eed03 100644
> --- a/automation/build/README.md
> +++ b/automation/build/README.md
> @@ -65,9 +65,6 @@ understands.
>  - CONTAINER_NO_PULL: If set to 1, the script will not pull from docker hub.
>    This is useful when testing container locally.
>  
> -- XEN_CONFIG_EXPERT: If this is defined in your shell it will be
> -  automatically passed through to the container.
> -
>  If your docker host has Linux kernel > 4.11, and you want to use containers
>  that run old glibc (for example, CentOS 6 or SLES11SP4), you may need to add
>  
> diff --git a/automation/scripts/build b/automation/scripts/build
> index 281f8b1fcc..21b3bc57c8 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -91,6 +91,6 @@ for cfg in `ls ${cfg_dir}`; do
>      echo "Building $cfg"
>      make -j$(nproc) -C xen clean
>      rm -f xen/.config
> -    make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} XEN_CONFIG_EXPERT=y defconfig
> -    make -j$(nproc) -C xen XEN_CONFIG_EXPERT=y
> +    make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
> +    make -j$(nproc) -C xen
>  done
> diff --git a/automation/scripts/containerize b/automation/scripts/containerize
> index 8992c67278..9d4beca4fa 100755
> --- a/automation/scripts/containerize
> +++ b/automation/scripts/containerize
> @@ -101,7 +101,6 @@ exec ${docker_cmd} run \
>      -v "${CONTAINER_PATH}":/build:rw${selinux} \
>      -v "${HOME}/.ssh":/root/.ssh:ro \
>      ${SSH_AUTH_DIR:+-v "${SSH_AUTH_DIR}":/tmp/ssh-agent${selinux}} \
> -    ${XEN_CONFIG_EXPERT:+-e XEN_CONFIG_EXPERT=${XEN_CONFIG_EXPERT}} \
>      ${CONTAINER_ARGS} \
>      -${termint}i --rm -- \
>      ${CONTAINER} \
> -- 
> 2.34.1
> 

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Xenia Ragiadakou]

Hi Julien,

On 7/8/22 01:26, Julien Grall wrote:
> Hi Xenia,
> 
> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
>> Add an arm subdirectory under automation/configs for the arm specific 
>> configs
>> and add a config that enables static allocation.
>>
>> Modify the build script to search for configs also in this 
>> subdirectory and to
>> keep the generated xen binary, suffixed with the config file name, as 
>> artifact.
>>
>> Create a test job that
>> - boots xen on qemu with a single direct mapped dom0less guest 
>> configured with
>> statically allocated memory
>> - verifies that the memory ranges reported in the guest's logs are the 
>> same
>> with the provided static memory regions
>>
>> For guest kernel, use the 5.9.9 kernel from the tests-artifacts 
>> containers.
>> Use busybox-static package, to create the guest ramdisk.
>> To generate the u-boot script, use ImageBuilder.
>> Use the qemu from the tests-artifacts containers.
>>
>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>> ---
>>   automation/configs/arm/static_mem          |   3 +
>>   automation/gitlab-ci/test.yaml             |  24 +++++
>>   automation/scripts/build                   |   4 +
>>   automation/scripts/qemu-staticmem-arm64.sh | 114 +++++++++++++++++++++
>>   4 files changed, 145 insertions(+)
>>   create mode 100644 automation/configs/arm/static_mem
>>   create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
>>
>> diff --git a/automation/configs/arm/static_mem 
>> b/automation/configs/arm/static_mem
>> new file mode 100644
>> index 0000000000..84675ddf4e
>> --- /dev/null
>> +++ b/automation/configs/arm/static_mem
>> @@ -0,0 +1,3 @@
>> +CONFIG_EXPERT=y
>> +CONFIG_UNSUPPORTED=y
>> +CONFIG_STATIC_MEMORY=y
>> \ No newline at end of file
> 
> Any particular reason to build a new Xen rather enable 
> CONFIG_STATIC_MEMORY in the existing build

IIUC, the xen binary (built with the arm64_defconfig) is used by the two 
other arm64 test jobs qemu-smoke-arm64-gcc and qemu-alpine-arm64-gcc. I 
did not want to change its configuration.

If there is no issue with changing its configuration, I can enable 
static memory and use this one. But to be honest, I would like to be 
able also to test with custom configs.

>> diff --git a/automation/scripts/build b/automation/scripts/build
>> index 21b3bc57c8..9c6196d9bd 100755
>> --- a/automation/scripts/build
>> +++ b/automation/scripts/build
>> @@ -83,6 +83,7 @@ fi
>>   # Build all the configs we care about
>>   case ${XEN_TARGET_ARCH} in
>>       x86_64) arch=x86 ;;
>> +    arm64) arch=arm ;;
>>       *) exit 0 ;;
>>   esac
>> @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
>>       rm -f xen/.config
>>       make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} 
>> defconfig
>>       make -j$(nproc) -C xen
>> +    if [[ ${arch} == "arm" ]]; then
>> +        cp xen/xen binaries/xen-${cfg}
>> +    fi
> 
> This feels a bit of a hack to be arm only. Can you explain why this is 
> not enabled for x86 (other than this is not yet used)?

I did not want to change the existing behavior for x86.

>>   done
>> diff --git a/automation/scripts/qemu-staticmem-arm64.sh 
>> b/automation/scripts/qemu-staticmem-arm64.sh
>> new file mode 100755
>> index 0000000000..5b89a151aa
>> --- /dev/null
>> +++ b/automation/scripts/qemu-staticmem-arm64.sh
>> @@ -0,0 +1,114 @@
>> +#!/bin/bash
>> +
>> +base=(0x50000000 0x100000000)
>> +size=(0x10000000 0x10000000)
> 
>  From the name, it is not clear what the base and size refers too. 
> Looking a bit below, it seems to be referring to the domain memory. If 
> so, I would suggest to comment and rename to "domu_{base, size}".

They are the static memory regions allocated to the domu.

>> +
>> +set -ex
>> +
>> +apt-get -qy update
>> +apt-get -qy install --no-install-recommends u-boot-qemu \
>> +                                            u-boot-tools \
>> +                                            device-tree-compiler \
>> +                                            cpio \
>> +                                            curl \
>> +                                            busybox-static
>> +
>> +# DomU Busybox
>> +cd binaries
>> +mkdir -p initrd
>> +mkdir -p initrd/bin
>> +mkdir -p initrd/sbin
>> +mkdir -p initrd/etc
>> +mkdir -p initrd/dev
>> +mkdir -p initrd/proc
>> +mkdir -p initrd/sys
>> +mkdir -p initrd/lib
>> +mkdir -p initrd/var
>> +mkdir -p initrd/mnt
>> +cp /bin/busybox initrd/bin/busybox
>> +initrd/bin/busybox --install initrd/bin
>> +echo "#!/bin/sh
>> +
>> +mount -t proc proc /proc
>> +mount -t sysfs sysfs /sys
>> +mount -t devtmpfs devtmpfs /dev
>> +/bin/sh" > initrd/init
>> +chmod +x initrd/init
>> +cd initrd
>> +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
>> +cd ../..
>> +
>> +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
>> +curl -fsSLO 
>> https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
>> +
>> +./binaries/qemu-system-aarch64 -nographic \
>> +    -M virtualization=true \
>> +    -M virt \
>> +    -M virt,gic-version=2 \
>> +    -cpu cortex-a57 \
>> +    -smp 2 \
>> +    -m 8G \
>> +    -M dumpdtb=binaries/virt-gicv2.dtb
>> +
>> +#dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts
>> +
>> +# ImageBuilder
>> +rm -rf imagebuilder
>> +git clone https://gitlab.com/ViryaOS/imagebuilder
>> +
>> +echo "MEMORY_START=\"0x40000000\"
>> +MEMORY_END=\"0x0200000000\"
>> +
>> +DEVICE_TREE=\"virt-gicv2.dtb\"
>> +
>> +XEN=\"xen-static_mem\"
>> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
> 
> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is 
> also not clear why you need to pass xsm=dummy.

It is not clear to me either :). I will remove them.

>> +
>> +NUM_DOMUS=1
>> +DOMU_MEM[0]=512
>> +DOMU_VCPUS[0]=1
>> +DOMU_KERNEL[0]=\"Image\"
>> +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
>> +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
>> +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
>> +
>> +UBOOT_SOURCE=\"boot.source\"
>> +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
>> +
>> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c 
>> binaries/imagebuilder_config
>> +
>> +# Run the test
>> +rm -f qemu-staticmem.serial
>> +set +e
>> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 
>> 0x40000000"| \
>> +timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
>> +    -M virtualization=true \
>> +    -M virt \
>> +    -M virt,gic-version=2 \
>> +    -cpu cortex-a57 \
>> +    -smp 2 \
>> +    -m 8G \
>> +    -no-reboot \
>> +    -device virtio-net-pci,netdev=vnet0 -netdev 
>> user,id=vnet0,tftp=binaries \
>> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
>> +    -dtb ./binaries/virt-gicv2.dtb \
>> +    |& tee qemu-staticmem.serial
>> +
>> +set -e
> 
> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I think 
> it would be better to consolidate in a single script. Looking briefly 
> throught the existing scripts, it looks like it is possible to pass 
> arguments (see qemu-smoke-x86-64.sh).

Yes, if there is no issue with changing qemu-smoke-arm64.sh, this is a 
very good idea.

>> +
>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
>> +
>> +for ((i=0; i<${#base[@]}; i++))
>> +do
>> +    start="$(printf "0x%016x" ${base[$i]})"
>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
>> +    if test $? -eq 1
>> +    then
>> +        exit 1
>> +    fi
>> +done
> 
> Please add a comment on top to explain what this is meant to do. 
> However, I think we should avoid relying on output that we have written 
> ourself. IOW, relying on Xen/Linux to always write the same message is 
> risky because they can change at any time.

The kernel is taken from a test-artifact container, so, IIUC, it won't 
change.

>> +
>> +(grep -q "BusyBox" qemu-staticmem.serial) || exit 1
> 
> Cheers,
> 

-- 
Xenia

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Julien Grall]

Hi,

On 08/07/2022 00:05, Stefano Stabellini wrote:
> On Thu, 7 Jul 2022, Julien Grall wrote:
>>> +# Run the test
>>> +rm -f qemu-staticmem.serial
>>> +set +e
>>> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
>>> +timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
>>> +    -M virtualization=true \
>>> +    -M virt \
>>> +    -M virt,gic-version=2 \
>>> +    -cpu cortex-a57 \
>>> +    -smp 2 \
>>> +    -m 8G \
>>> +    -no-reboot \
>>> +    -device virtio-net-pci,netdev=vnet0 -netdev user,id=vnet0,tftp=binaries
>>> \
>>> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
>>> +    -dtb ./binaries/virt-gicv2.dtb \
>>> +    |& tee qemu-staticmem.serial
>>> +
>>> +set -e
>>
>> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I think it
>> would be better to consolidate in a single script. Looking briefly throught
>> the existing scripts, it looks like it is possible to pass arguments (see
>> qemu-smoke-x86-64.sh).
>   
> One idea would be to make the script common and "source" a second
> script or config file with just the ImageBuilder configuration because
> it looks like it is the only thing different.

This would mean creating a new bash script for every new test. This 
sounds a bit pointless if the only difference is the ImageBuilder 
configuration. Instead, it would be better to pass an argument to the 
script (like qemu-smoke-x86-64.sh) indicating which test we are going to 
perform.

Cheers,

-- 
Julien Grall

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Julien Grall]

On 08/07/2022 08:17, Xenia Ragiadakou wrote:
> Hi Julien,

Hi Xenia,

> On 7/8/22 01:26, Julien Grall wrote:
>> Hi Xenia,
>>
>> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
>>> Add an arm subdirectory under automation/configs for the arm specific 
>>> configs
>>> and add a config that enables static allocation.
>>>
>>> Modify the build script to search for configs also in this 
>>> subdirectory and to
>>> keep the generated xen binary, suffixed with the config file name, as 
>>> artifact.
>>>
>>> Create a test job that
>>> - boots xen on qemu with a single direct mapped dom0less guest 
>>> configured with
>>> statically allocated memory
>>> - verifies that the memory ranges reported in the guest's logs are 
>>> the same
>>> with the provided static memory regions
>>>
>>> For guest kernel, use the 5.9.9 kernel from the tests-artifacts 
>>> containers.
>>> Use busybox-static package, to create the guest ramdisk.
>>> To generate the u-boot script, use ImageBuilder.
>>> Use the qemu from the tests-artifacts containers.
>>>
>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>>> ---
>>>   automation/configs/arm/static_mem          |   3 +
>>>   automation/gitlab-ci/test.yaml             |  24 +++++
>>>   automation/scripts/build                   |   4 +
>>>   automation/scripts/qemu-staticmem-arm64.sh | 114 +++++++++++++++++++++
>>>   4 files changed, 145 insertions(+)
>>>   create mode 100644 automation/configs/arm/static_mem
>>>   create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
>>>
>>> diff --git a/automation/configs/arm/static_mem 
>>> b/automation/configs/arm/static_mem
>>> new file mode 100644
>>> index 0000000000..84675ddf4e
>>> --- /dev/null
>>> +++ b/automation/configs/arm/static_mem
>>> @@ -0,0 +1,3 @@
>>> +CONFIG_EXPERT=y
>>> +CONFIG_UNSUPPORTED=y
>>> +CONFIG_STATIC_MEMORY=y
>>> \ No newline at end of file
>>
>> Any particular reason to build a new Xen rather enable 
>> CONFIG_STATIC_MEMORY in the existing build
> 
> IIUC, the xen binary (built with the arm64_defconfig) is used by the two 
> other arm64 test jobs qemu-smoke-arm64-gcc and qemu-alpine-arm64-gcc. I 
> did not want to change its configuration.
> 
> If there is no issue with changing its configuration, I can enable 
> static memory and use this one. 

I would expect a Xen built to CONFIG_STATIC_MEMORY to continue to work 
in normal case. So it should be fine to enable by default.

> But to be honest, I would like to be 
> able also to test with custom configs.

That's fine. But in this case...

> 
>>> diff --git a/automation/scripts/build b/automation/scripts/build
>>> index 21b3bc57c8..9c6196d9bd 100755
>>> --- a/automation/scripts/build
>>> +++ b/automation/scripts/build
>>> @@ -83,6 +83,7 @@ fi
>>>   # Build all the configs we care about
>>>   case ${XEN_TARGET_ARCH} in
>>>       x86_64) arch=x86 ;;
>>> +    arm64) arch=arm ;;
>>>       *) exit 0 ;;
>>>   esac
>>> @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
>>>       rm -f xen/.config
>>>       make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} 
>>> defconfig
>>>       make -j$(nproc) -C xen
>>> +    if [[ ${arch} == "arm" ]]; then
>>> +        cp xen/xen binaries/xen-${cfg}
>>> +    fi
>>
>> This feels a bit of a hack to be arm only. Can you explain why this is 
>> not enabled for x86 (other than this is not yet used)?
> 
> I did not want to change the existing behavior for x86.


... I don't think this should be restricted to arm. I would also 
consider to do this change separately to avoid mixing infrastructure 
change and new test.

[...]

>>> +# ImageBuilder
>>> +rm -rf imagebuilder
>>> +git clone https://gitlab.com/ViryaOS/imagebuilder
>>> +
>>> +echo "MEMORY_START=\"0x40000000\"
>>> +MEMORY_END=\"0x0200000000\"
>>> +
>>> +DEVICE_TREE=\"virt-gicv2.dtb\"
>>> +
>>> +XEN=\"xen-static_mem\"
>>> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
>>
>> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is 
>> also not clear why you need to pass xsm=dummy.
> 
> It is not clear to me either :). I will remove them.

Where was this command line copied from? If it is an Arm documentation 
(or script), then they should be corrected.

>>> +
>>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
>>> +
>>> +for ((i=0; i<${#base[@]}; i++))
>>> +do
>>> +    start="$(printf "0x%016x" ${base[$i]})"
>>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
>>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
>>> +    if test $? -eq 1
>>> +    then
>>> +        exit 1
>>> +    fi
>>> +done
>>
>> Please add a comment on top to explain what this is meant to do. 
>> However, I think we should avoid relying on output that we have 
>> written ourself. IOW, relying on Xen/Linux to always write the same 
>> message is risky because they can change at any time.
> 
> The kernel is taken from a test-artifact container, so, IIUC, it won't 
> change.

This statement is correct today. However, we may decide to update the 
kernel or test multiple kernels (with different ouput).

In the first case, it would be a matter of updating the script. This is 
annoying but not too bad. In the second case, we would need to have "if 
version X ... else if version Y ... ".

Cheers,

-- 
Julien Grall

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Xenia Ragiadakou]

On 7/8/22 10:35, Julien Grall wrote:
> Hi,
> 
> On 08/07/2022 00:05, Stefano Stabellini wrote:
>> On Thu, 7 Jul 2022, Julien Grall wrote:
>>>> +# Run the test
>>>> +rm -f qemu-staticmem.serial
>>>> +set +e
>>>> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 
>>>> 0x40000000"| \
>>>> +timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
>>>> +    -M virtualization=true \
>>>> +    -M virt \
>>>> +    -M virt,gic-version=2 \
>>>> +    -cpu cortex-a57 \
>>>> +    -smp 2 \
>>>> +    -m 8G \
>>>> +    -no-reboot \
>>>> +    -device virtio-net-pci,netdev=vnet0 -netdev 
>>>> user,id=vnet0,tftp=binaries
>>>> \
>>>> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
>>>> +    -dtb ./binaries/virt-gicv2.dtb \
>>>> +    |& tee qemu-staticmem.serial
>>>> +
>>>> +set -e
>>>
>>> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I 
>>> think it
>>> would be better to consolidate in a single script. Looking briefly 
>>> throught
>>> the existing scripts, it looks like it is possible to pass arguments 
>>> (see
>>> qemu-smoke-x86-64.sh).
>> One idea would be to make the script common and "source" a second
>> script or config file with just the ImageBuilder configuration because
>> it looks like it is the only thing different.
> 
> This would mean creating a new bash script for every new test. This 
> sounds a bit pointless if the only difference is the ImageBuilder 
> configuration. Instead, it would be better to pass an argument to the 
> script (like qemu-smoke-x86-64.sh) indicating which test we are going to 
> perform.

I agree with Julien, also because the ImageBuilder script depends on how 
qemu is configured. It is not completely independent.

-- 
Xenia

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Xenia Ragiadakou]

On 7/8/22 10:46, Julien Grall wrote:
> 
> 
> On 08/07/2022 08:17, Xenia Ragiadakou wrote:
>> Hi Julien,
> 
> Hi Xenia,
> 
>> On 7/8/22 01:26, Julien Grall wrote:
>>> Hi Xenia,
>>>
>>> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
>>>> Add an arm subdirectory under automation/configs for the arm 
>>>> specific configs
>>>> and add a config that enables static allocation.
>>>>
>>>> Modify the build script to search for configs also in this 
>>>> subdirectory and to
>>>> keep the generated xen binary, suffixed with the config file name, 
>>>> as artifact.
>>>>
>>>> Create a test job that
>>>> - boots xen on qemu with a single direct mapped dom0less guest 
>>>> configured with
>>>> statically allocated memory
>>>> - verifies that the memory ranges reported in the guest's logs are 
>>>> the same
>>>> with the provided static memory regions
>>>>
>>>> For guest kernel, use the 5.9.9 kernel from the tests-artifacts 
>>>> containers.
>>>> Use busybox-static package, to create the guest ramdisk.
>>>> To generate the u-boot script, use ImageBuilder.
>>>> Use the qemu from the tests-artifacts containers.
>>>>
>>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>>>> ---
>>>>   automation/configs/arm/static_mem          |   3 +
>>>>   automation/gitlab-ci/test.yaml             |  24 +++++
>>>>   automation/scripts/build                   |   4 +
>>>>   automation/scripts/qemu-staticmem-arm64.sh | 114 
>>>> +++++++++++++++++++++
>>>>   4 files changed, 145 insertions(+)
>>>>   create mode 100644 automation/configs/arm/static_mem
>>>>   create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
>>>>
>>>> diff --git a/automation/configs/arm/static_mem 
>>>> b/automation/configs/arm/static_mem
>>>> new file mode 100644
>>>> index 0000000000..84675ddf4e
>>>> --- /dev/null
>>>> +++ b/automation/configs/arm/static_mem
>>>> @@ -0,0 +1,3 @@
>>>> +CONFIG_EXPERT=y
>>>> +CONFIG_UNSUPPORTED=y
>>>> +CONFIG_STATIC_MEMORY=y
>>>> \ No newline at end of file
>>>
>>> Any particular reason to build a new Xen rather enable 
>>> CONFIG_STATIC_MEMORY in the existing build
>>
>> IIUC, the xen binary (built with the arm64_defconfig) is used by the 
>> two other arm64 test jobs qemu-smoke-arm64-gcc and 
>> qemu-alpine-arm64-gcc. I did not want to change its configuration.
>>
>> If there is no issue with changing its configuration, I can enable 
>> static memory and use this one. 
> 
> I would expect a Xen built to CONFIG_STATIC_MEMORY to continue to work 
> in normal case. So it should be fine to enable by default.
> 
>> But to be honest, I would like to be able also to test with custom 
>> configs.
> 
> That's fine. But in this case...
> 
>>
>>>> diff --git a/automation/scripts/build b/automation/scripts/build
>>>> index 21b3bc57c8..9c6196d9bd 100755
>>>> --- a/automation/scripts/build
>>>> +++ b/automation/scripts/build
>>>> @@ -83,6 +83,7 @@ fi
>>>>   # Build all the configs we care about
>>>>   case ${XEN_TARGET_ARCH} in
>>>>       x86_64) arch=x86 ;;
>>>> +    arm64) arch=arm ;;
>>>>       *) exit 0 ;;
>>>>   esac
>>>> @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
>>>>       rm -f xen/.config
>>>>       make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} 
>>>> defconfig
>>>>       make -j$(nproc) -C xen
>>>> +    if [[ ${arch} == "arm" ]]; then
>>>> +        cp xen/xen binaries/xen-${cfg}
>>>> +    fi
>>>
>>> This feels a bit of a hack to be arm only. Can you explain why this 
>>> is not enabled for x86 (other than this is not yet used)?
>>
>> I did not want to change the existing behavior for x86.
> 
> 
> ... I don't think this should be restricted to arm. I would also 
> consider to do this change separately to avoid mixing infrastructure 
> change and new test.

Sure.

> 
> [...]
> 
>>>> +# ImageBuilder
>>>> +rm -rf imagebuilder
>>>> +git clone https://gitlab.com/ViryaOS/imagebuilder
>>>> +
>>>> +echo "MEMORY_START=\"0x40000000\"
>>>> +MEMORY_END=\"0x0200000000\"
>>>> +
>>>> +DEVICE_TREE=\"virt-gicv2.dtb\"
>>>> +
>>>> +XEN=\"xen-static_mem\"
>>>> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
>>>
>>> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is 
>>> also not clear why you need to pass xsm=dummy.
>>
>> It is not clear to me either :). I will remove them.
> 
> Where was this command line copied from? If it is an Arm documentation 
> (or script), then they should be corrected.

Don't worry :) I was using them when debugging a script for parsing the 
xen cmdline and I dragged them around. Sorry about that.
>>>> +
>>>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
>>>> +
>>>> +for ((i=0; i<${#base[@]}; i++))
>>>> +do
>>>> +    start="$(printf "0x%016x" ${base[$i]})"
>>>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
>>>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
>>>> +    if test $? -eq 1
>>>> +    then
>>>> +        exit 1
>>>> +    fi
>>>> +done
>>>
>>> Please add a comment on top to explain what this is meant to do. 
>>> However, I think we should avoid relying on output that we have 
>>> written ourself. IOW, relying on Xen/Linux to always write the same 
>>> message is risky because they can change at any time.
>>
>> The kernel is taken from a test-artifact container, so, IIUC, it won't 
>> change.
> 
> This statement is correct today. However, we may decide to update the 
> kernel or test multiple kernels (with different ouput).
> 
> In the first case, it would be a matter of updating the script. This is 
> annoying but not too bad. In the second case, we would need to have "if 
> version X ... else if version Y ... ".

The particular test was relying and had a dependency on this kernel.
If the test is merged into the qemu-smoke-arm64.sh, the check above will 
leave and it will be tested whether the guest makes it to the busybox, 
based on the busybox logs, which also may change at any time.

-- 
Xenia

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Julien Grall]

On 08/07/2022 10:00, Xenia Ragiadakou wrote:
>>>>> +
>>>>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || 
>>>>> exit 1
>>>>> +
>>>>> +for ((i=0; i<${#base[@]}; i++))
>>>>> +do
>>>>> +    start="$(printf "0x%016x" ${base[$i]})"
>>>>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
>>>>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
>>>>> +    if test $? -eq 1
>>>>> +    then
>>>>> +        exit 1
>>>>> +    fi
>>>>> +done
>>>>
>>>> Please add a comment on top to explain what this is meant to do. 
>>>> However, I think we should avoid relying on output that we have 
>>>> written ourself. IOW, relying on Xen/Linux to always write the same 
>>>> message is risky because they can change at any time.
>>>
>>> The kernel is taken from a test-artifact container, so, IIUC, it 
>>> won't change.
>>
>> This statement is correct today. However, we may decide to update the 
>> kernel or test multiple kernels (with different ouput).
>>
>> In the first case, it would be a matter of updating the script. This 
>> is annoying but not too bad. In the second case, we would need to have 
>> "if version X ... else if version Y ... ".
> 
> The particular test was relying and had a dependency on this kernel.
I think you missed my point. I don't disagree that the test today 
expects a specific version. However, there are nothing preventing us to 
change that so we long we have a matching initramfs/kernel.

> If the test is merged into the qemu-smoke-arm64.sh, the check above will 
> leave and it will be tested whether the guest makes it to the busybox, 
> based on the busybox logs, which also may change at any time.

I don't think I suggested that relying on busybox is better. Ideally we 
should run a script that prints a message.

But this is not something I am going to argue for in this patch. Relying 
on one piece of line is already far better than trying to check logs for 
each components (e.g. xen, kernel, busysbox).

Cheers,

-- 
Julien Grall

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Xenia Ragiadakou]

Hi Stefano,

On 7/8/22 02:05, Stefano Stabellini wrote:
> On Thu, 7 Jul 2022, Julien Grall wrote:
>> Hi Xenia,
>>
>> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
>>> Add an arm subdirectory under automation/configs for the arm specific
>>> configs
>>> and add a config that enables static allocation.
>>>
>>> Modify the build script to search for configs also in this subdirectory and
>>> to
>>> keep the generated xen binary, suffixed with the config file name, as
>>> artifact.
>>>
>>> Create a test job that
>>> - boots xen on qemu with a single direct mapped dom0less guest configured
>>> with
>>> statically allocated memory
>>> - verifies that the memory ranges reported in the guest's logs are the same
>>> with the provided static memory regions
>>>
>>> For guest kernel, use the 5.9.9 kernel from the tests-artifacts containers.
>>> Use busybox-static package, to create the guest ramdisk.
>>> To generate the u-boot script, use ImageBuilder.
>>> Use the qemu from the tests-artifacts containers.
>>>
>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>>> ---
>>>    automation/configs/arm/static_mem          |   3 +
>>>    automation/gitlab-ci/test.yaml             |  24 +++++
>>>    automation/scripts/build                   |   4 +
>>>    automation/scripts/qemu-staticmem-arm64.sh | 114 +++++++++++++++++++++
>>>    4 files changed, 145 insertions(+)
>>>    create mode 100644 automation/configs/arm/static_mem
>>>    create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
>>>
>>> diff --git a/automation/configs/arm/static_mem
>>> b/automation/configs/arm/static_mem
>>> new file mode 100644
>>> index 0000000000..84675ddf4e
>>> --- /dev/null
>>> +++ b/automation/configs/arm/static_mem
>>> @@ -0,0 +1,3 @@
>>> +CONFIG_EXPERT=y
>>> +CONFIG_UNSUPPORTED=y
>>> +CONFIG_STATIC_MEMORY=y
>>> \ No newline at end of file
>>
>> Any particular reason to build a new Xen rather enable CONFIG_STATIC_MEMORY in
>> the existing build?
>>
>>> diff --git a/automation/scripts/build b/automation/scripts/build
>>> index 21b3bc57c8..9c6196d9bd 100755
>>> --- a/automation/scripts/build
>>> +++ b/automation/scripts/build
>>> @@ -83,6 +83,7 @@ fi
>>>    # Build all the configs we care about
>>>    case ${XEN_TARGET_ARCH} in
>>>        x86_64) arch=x86 ;;
>>> +    arm64) arch=arm ;;
>>>        *) exit 0 ;;
>>>    esac
>>>    @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
>>>        rm -f xen/.config
>>>        make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
>>>        make -j$(nproc) -C xen
>>> +    if [[ ${arch} == "arm" ]]; then
>>> +        cp xen/xen binaries/xen-${cfg}
>>> +    fi
>>
>> This feels a bit of a hack to be arm only. Can you explain why this is not
>> enabled for x86 (other than this is not yet used)?
>>
>>>    done
>>> diff --git a/automation/scripts/qemu-staticmem-arm64.sh
>>> b/automation/scripts/qemu-staticmem-arm64.sh
>>> new file mode 100755
>>> index 0000000000..5b89a151aa
>>> --- /dev/null
>>> +++ b/automation/scripts/qemu-staticmem-arm64.sh
>>> @@ -0,0 +1,114 @@
>>> +#!/bin/bash
>>> +
>>> +base=(0x50000000 0x100000000)
>>> +size=(0x10000000 0x10000000)
>>
>>  From the name, it is not clear what the base and size refers too. Looking a
>> bit below, it seems to be referring to the domain memory. If so, I would
>> suggest to comment and rename to "domu_{base, size}".
>>
>>> +
>>> +set -ex
>>> +
>>> +apt-get -qy update
>>> +apt-get -qy install --no-install-recommends u-boot-qemu \
>>> +                                            u-boot-tools \
>>> +                                            device-tree-compiler \
>>> +                                            cpio \
>>> +                                            curl \
>>> +                                            busybox-static
>>> +
>>> +# DomU Busybox
>>> +cd binaries
>>> +mkdir -p initrd
>>> +mkdir -p initrd/bin
>>> +mkdir -p initrd/sbin
>>> +mkdir -p initrd/etc
>>> +mkdir -p initrd/dev
>>> +mkdir -p initrd/proc
>>> +mkdir -p initrd/sys
>>> +mkdir -p initrd/lib
>>> +mkdir -p initrd/var
>>> +mkdir -p initrd/mnt
>>> +cp /bin/busybox initrd/bin/busybox
>>> +initrd/bin/busybox --install initrd/bin
>>> +echo "#!/bin/sh
>>> +
>>> +mount -t proc proc /proc
>>> +mount -t sysfs sysfs /sys
>>> +mount -t devtmpfs devtmpfs /dev
>>> +/bin/sh" > initrd/init
>>> +chmod +x initrd/init
>>> +cd initrd
>>> +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
>>> +cd ../..
>>> +
>>> +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
>>> +curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
>>> +
>>> +./binaries/qemu-system-aarch64 -nographic \
>>> +    -M virtualization=true \
>>> +    -M virt \
>>> +    -M virt,gic-version=2 \
>>> +    -cpu cortex-a57 \
>>> +    -smp 2 \
>>> +    -m 8G \
>>> +    -M dumpdtb=binaries/virt-gicv2.dtb
>>> +
>>> +#dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts
>>> +
>>> +# ImageBuilder
>>> +rm -rf imagebuilder
>>> +git clone https://gitlab.com/ViryaOS/imagebuilder
>>> +
>>> +echo "MEMORY_START=\"0x40000000\"
>>> +MEMORY_END=\"0x0200000000\"
>>> +
>>> +DEVICE_TREE=\"virt-gicv2.dtb\"
>>> +
>>> +XEN=\"xen-static_mem\"
>>> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
>>
>> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is also not
>> clear why you need to pass xsm=dummy.
>>
>>> +
>>> +NUM_DOMUS=1
>>> +DOMU_MEM[0]=512
>>> +DOMU_VCPUS[0]=1
>>> +DOMU_KERNEL[0]=\"Image\"
>>> +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
>>> +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
>>> +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
>>> +
>>> +UBOOT_SOURCE=\"boot.source\"
>>> +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
>>> +
>>> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c
>>> binaries/imagebuilder_config
>>> +
>>> +# Run the test
>>> +rm -f qemu-staticmem.serial
>>> +set +e
>>> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
>>> +timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
>>> +    -M virtualization=true \
>>> +    -M virt \
>>> +    -M virt,gic-version=2 \
>>> +    -cpu cortex-a57 \
>>> +    -smp 2 \
>>> +    -m 8G \
>>> +    -no-reboot \
>>> +    -device virtio-net-pci,netdev=vnet0 -netdev user,id=vnet0,tftp=binaries
>>> \
>>> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
>>> +    -dtb ./binaries/virt-gicv2.dtb \
>>> +    |& tee qemu-staticmem.serial
>>> +
>>> +set -e
>>
>> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I think it
>> would be better to consolidate in a single script. Looking briefly throught
>> the existing scripts, it looks like it is possible to pass arguments (see
>> qemu-smoke-x86-64.sh).
>   
> One idea would be to make the script common and "source" a second
> script or config file with just the ImageBuilder configuration because
> it looks like it is the only thing different.
> 
> 
>>> +
>>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
>>> +
>>> +for ((i=0; i<${#base[@]}; i++))
>>> +do
>>> +    start="$(printf "0x%016x" ${base[$i]})"
>>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
>>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
>>> +    if test $? -eq 1
>>> +    then
>>> +        exit 1
>>> +    fi
>>> +done
>>
>> Please add a comment on top to explain what this is meant to do. However, I
>> think we should avoid relying on output that we have written ourself. IOW,
>> relying on Xen/Linux to always write the same message is risky because they
>> can change at any time.
> 
> Especially if we make the script common, then we could just rely on the
> existing check to see if the guest started correctly (no special check
> for static memory).

In this case, how the test will verify that the static memory 
configuration has been taken into account and has not just been ignored?

>>> +
>>> +(grep -q "BusyBox" qemu-staticmem.serial) || exit 1
>   

-- 
Xenia

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Stefano Stabellini]

On Fri, 8 Jul 2022, Xenia Ragiadakou wrote:
> On 7/8/22 02:05, Stefano Stabellini wrote:
> > On Thu, 7 Jul 2022, Julien Grall wrote:
> > > Hi Xenia,
> > > 
> > > On 07/07/2022 21:38, Xenia Ragiadakou wrote:
> > > > Add an arm subdirectory under automation/configs for the arm specific
> > > > configs
> > > > and add a config that enables static allocation.
> > > > 
> > > > Modify the build script to search for configs also in this subdirectory
> > > > and
> > > > to
> > > > keep the generated xen binary, suffixed with the config file name, as
> > > > artifact.
> > > > 
> > > > Create a test job that
> > > > - boots xen on qemu with a single direct mapped dom0less guest
> > > > configured
> > > > with
> > > > statically allocated memory
> > > > - verifies that the memory ranges reported in the guest's logs are the
> > > > same
> > > > with the provided static memory regions
> > > > 
> > > > For guest kernel, use the 5.9.9 kernel from the tests-artifacts
> > > > containers.
> > > > Use busybox-static package, to create the guest ramdisk.
> > > > To generate the u-boot script, use ImageBuilder.
> > > > Use the qemu from the tests-artifacts containers.
> > > > 
> > > > Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> > > > ---
> > > >    automation/configs/arm/static_mem          |   3 +
> > > >    automation/gitlab-ci/test.yaml             |  24 +++++
> > > >    automation/scripts/build                   |   4 +
> > > >    automation/scripts/qemu-staticmem-arm64.sh | 114
> > > > +++++++++++++++++++++
> > > >    4 files changed, 145 insertions(+)
> > > >    create mode 100644 automation/configs/arm/static_mem
> > > >    create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
> > > > 
> > > > diff --git a/automation/configs/arm/static_mem
> > > > b/automation/configs/arm/static_mem
> > > > new file mode 100644
> > > > index 0000000000..84675ddf4e
> > > > --- /dev/null
> > > > +++ b/automation/configs/arm/static_mem
> > > > @@ -0,0 +1,3 @@
> > > > +CONFIG_EXPERT=y
> > > > +CONFIG_UNSUPPORTED=y
> > > > +CONFIG_STATIC_MEMORY=y
> > > > \ No newline at end of file
> > > 
> > > Any particular reason to build a new Xen rather enable
> > > CONFIG_STATIC_MEMORY in
> > > the existing build?
> > > 
> > > > diff --git a/automation/scripts/build b/automation/scripts/build
> > > > index 21b3bc57c8..9c6196d9bd 100755
> > > > --- a/automation/scripts/build
> > > > +++ b/automation/scripts/build
> > > > @@ -83,6 +83,7 @@ fi
> > > >    # Build all the configs we care about
> > > >    case ${XEN_TARGET_ARCH} in
> > > >        x86_64) arch=x86 ;;
> > > > +    arm64) arch=arm ;;
> > > >        *) exit 0 ;;
> > > >    esac
> > > >    @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
> > > >        rm -f xen/.config
> > > >        make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg}
> > > > defconfig
> > > >        make -j$(nproc) -C xen
> > > > +    if [[ ${arch} == "arm" ]]; then
> > > > +        cp xen/xen binaries/xen-${cfg}
> > > > +    fi
> > > 
> > > This feels a bit of a hack to be arm only. Can you explain why this is not
> > > enabled for x86 (other than this is not yet used)?
> > > 
> > > >    done
> > > > diff --git a/automation/scripts/qemu-staticmem-arm64.sh
> > > > b/automation/scripts/qemu-staticmem-arm64.sh
> > > > new file mode 100755
> > > > index 0000000000..5b89a151aa
> > > > --- /dev/null
> > > > +++ b/automation/scripts/qemu-staticmem-arm64.sh
> > > > @@ -0,0 +1,114 @@
> > > > +#!/bin/bash
> > > > +
> > > > +base=(0x50000000 0x100000000)
> > > > +size=(0x10000000 0x10000000)
> > > 
> > >  From the name, it is not clear what the base and size refers too. Looking
> > > a
> > > bit below, it seems to be referring to the domain memory. If so, I would
> > > suggest to comment and rename to "domu_{base, size}".
> > > 
> > > > +
> > > > +set -ex
> > > > +
> > > > +apt-get -qy update
> > > > +apt-get -qy install --no-install-recommends u-boot-qemu \
> > > > +                                            u-boot-tools \
> > > > +                                            device-tree-compiler \
> > > > +                                            cpio \
> > > > +                                            curl \
> > > > +                                            busybox-static
> > > > +
> > > > +# DomU Busybox
> > > > +cd binaries
> > > > +mkdir -p initrd
> > > > +mkdir -p initrd/bin
> > > > +mkdir -p initrd/sbin
> > > > +mkdir -p initrd/etc
> > > > +mkdir -p initrd/dev
> > > > +mkdir -p initrd/proc
> > > > +mkdir -p initrd/sys
> > > > +mkdir -p initrd/lib
> > > > +mkdir -p initrd/var
> > > > +mkdir -p initrd/mnt
> > > > +cp /bin/busybox initrd/bin/busybox
> > > > +initrd/bin/busybox --install initrd/bin
> > > > +echo "#!/bin/sh
> > > > +
> > > > +mount -t proc proc /proc
> > > > +mount -t sysfs sysfs /sys
> > > > +mount -t devtmpfs devtmpfs /dev
> > > > +/bin/sh" > initrd/init
> > > > +chmod +x initrd/init
> > > > +cd initrd
> > > > +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
> > > > +cd ../..
> > > > +
> > > > +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
> > > > +curl -fsSLO
> > > > https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
> > > > +
> > > > +./binaries/qemu-system-aarch64 -nographic \
> > > > +    -M virtualization=true \
> > > > +    -M virt \
> > > > +    -M virt,gic-version=2 \
> > > > +    -cpu cortex-a57 \
> > > > +    -smp 2 \
> > > > +    -m 8G \
> > > > +    -M dumpdtb=binaries/virt-gicv2.dtb
> > > > +
> > > > +#dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts
> > > > +
> > > > +# ImageBuilder
> > > > +rm -rf imagebuilder
> > > > +git clone https://gitlab.com/ViryaOS/imagebuilder
> > > > +
> > > > +echo "MEMORY_START=\"0x40000000\"
> > > > +MEMORY_END=\"0x0200000000\"
> > > > +
> > > > +DEVICE_TREE=\"virt-gicv2.dtb\"
> > > > +
> > > > +XEN=\"xen-static_mem\"
> > > > +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
> > > 
> > > AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is also
> > > not
> > > clear why you need to pass xsm=dummy.
> > > 
> > > > +
> > > > +NUM_DOMUS=1
> > > > +DOMU_MEM[0]=512
> > > > +DOMU_VCPUS[0]=1
> > > > +DOMU_KERNEL[0]=\"Image\"
> > > > +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
> > > > +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
> > > > +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
> > > > +
> > > > +UBOOT_SOURCE=\"boot.source\"
> > > > +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
> > > > +
> > > > +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c
> > > > binaries/imagebuilder_config
> > > > +
> > > > +# Run the test
> > > > +rm -f qemu-staticmem.serial
> > > > +set +e
> > > > +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source
> > > > 0x40000000"| \
> > > > +timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
> > > > +    -M virtualization=true \
> > > > +    -M virt \
> > > > +    -M virt,gic-version=2 \
> > > > +    -cpu cortex-a57 \
> > > > +    -smp 2 \
> > > > +    -m 8G \
> > > > +    -no-reboot \
> > > > +    -device virtio-net-pci,netdev=vnet0 -netdev
> > > > user,id=vnet0,tftp=binaries
> > > > \
> > > > +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
> > > > +    -dtb ./binaries/virt-gicv2.dtb \
> > > > +    |& tee qemu-staticmem.serial
> > > > +
> > > > +set -e
> > > 
> > > A lot of the code above is duplicated from qemu-smoke-arm64.sh. I think it
> > > would be better to consolidate in a single script. Looking briefly
> > > throught
> > > the existing scripts, it looks like it is possible to pass arguments (see
> > > qemu-smoke-x86-64.sh).
> >   One idea would be to make the script common and "source" a second
> > script or config file with just the ImageBuilder configuration because
> > it looks like it is the only thing different.
> > 
> > 
> > > > +
> > > > +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
> > > > +
> > > > +for ((i=0; i<${#base[@]}; i++))
> > > > +do
> > > > +    start="$(printf "0x%016x" ${base[$i]})"
> > > > +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
> > > > +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
> > > > +    if test $? -eq 1
> > > > +    then
> > > > +        exit 1
> > > > +    fi
> > > > +done
> > > 
> > > Please add a comment on top to explain what this is meant to do. However,
> > > I
> > > think we should avoid relying on output that we have written ourself. IOW,
> > > relying on Xen/Linux to always write the same message is risky because
> > > they
> > > can change at any time.
> > 
> > Especially if we make the script common, then we could just rely on the
> > existing check to see if the guest started correctly (no special check
> > for static memory).
> 
> In this case, how the test will verify that the static memory configuration
> has been taken into account and has not just been ignored?

There is no simple way that I can think of.

We can trust that it worked, without checking that the ranges were
actually static as requested.

We can parse the Xen output like you did, although if it changes then
the test will break.

Or we could add a script to detect and print specific output but I
don't know if there is something under /proc or /sys that we could
simply "cat" from bash to check it.

If there is a simple way to do this by cat'ing /proc or /sys then I
think that would be great. Otherwise I think we could do as you did in
this patch, which is not perfect but it works and if something changes
in the Xen output we'll detect it right away given that gitlab-ci is run
pre-commit.

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Julien Grall]

Hi,

On 08/07/2022 16:56, Stefano Stabellini wrote:
> Or we could add a script to detect and print specific output but I
> don't know if there is something under /proc or /sys that we could
> simply "cat" from bash to check it.

The domain device-tree should be /proc/device-tree. So you could check 
the properties from there.

Cheers,

-- 
Julien Grall

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Xenia Ragiadakou]

On 7/8/22 18:56, Stefano Stabellini wrote:
> On Fri, 8 Jul 2022, Xenia Ragiadakou wrote:
>> On 7/8/22 02:05, Stefano Stabellini wrote:
>>> On Thu, 7 Jul 2022, Julien Grall wrote:
>>>> Hi Xenia,
>>>>
>>>> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
>>>>> Add an arm subdirectory under automation/configs for the arm specific
>>>>> configs
>>>>> and add a config that enables static allocation.
>>>>>
>>>>> Modify the build script to search for configs also in this subdirectory
>>>>> and
>>>>> to
>>>>> keep the generated xen binary, suffixed with the config file name, as
>>>>> artifact.
>>>>>
>>>>> Create a test job that
>>>>> - boots xen on qemu with a single direct mapped dom0less guest
>>>>> configured
>>>>> with
>>>>> statically allocated memory
>>>>> - verifies that the memory ranges reported in the guest's logs are the
>>>>> same
>>>>> with the provided static memory regions
>>>>>
>>>>> For guest kernel, use the 5.9.9 kernel from the tests-artifacts
>>>>> containers.
>>>>> Use busybox-static package, to create the guest ramdisk.
>>>>> To generate the u-boot script, use ImageBuilder.
>>>>> Use the qemu from the tests-artifacts containers.
>>>>>
>>>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>>>>> ---
>>>>>     automation/configs/arm/static_mem          |   3 +
>>>>>     automation/gitlab-ci/test.yaml             |  24 +++++
>>>>>     automation/scripts/build                   |   4 +
>>>>>     automation/scripts/qemu-staticmem-arm64.sh | 114
>>>>> +++++++++++++++++++++
>>>>>     4 files changed, 145 insertions(+)
>>>>>     create mode 100644 automation/configs/arm/static_mem
>>>>>     create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
>>>>>
>>>>> diff --git a/automation/configs/arm/static_mem
>>>>> b/automation/configs/arm/static_mem
>>>>> new file mode 100644
>>>>> index 0000000000..84675ddf4e
>>>>> --- /dev/null
>>>>> +++ b/automation/configs/arm/static_mem
>>>>> @@ -0,0 +1,3 @@
>>>>> +CONFIG_EXPERT=y
>>>>> +CONFIG_UNSUPPORTED=y
>>>>> +CONFIG_STATIC_MEMORY=y
>>>>> \ No newline at end of file
>>>>
>>>> Any particular reason to build a new Xen rather enable
>>>> CONFIG_STATIC_MEMORY in
>>>> the existing build?
>>>>
>>>>> diff --git a/automation/scripts/build b/automation/scripts/build
>>>>> index 21b3bc57c8..9c6196d9bd 100755
>>>>> --- a/automation/scripts/build
>>>>> +++ b/automation/scripts/build
>>>>> @@ -83,6 +83,7 @@ fi
>>>>>     # Build all the configs we care about
>>>>>     case ${XEN_TARGET_ARCH} in
>>>>>         x86_64) arch=x86 ;;
>>>>> +    arm64) arch=arm ;;
>>>>>         *) exit 0 ;;
>>>>>     esac
>>>>>     @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
>>>>>         rm -f xen/.config
>>>>>         make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg}
>>>>> defconfig
>>>>>         make -j$(nproc) -C xen
>>>>> +    if [[ ${arch} == "arm" ]]; then
>>>>> +        cp xen/xen binaries/xen-${cfg}
>>>>> +    fi
>>>>
>>>> This feels a bit of a hack to be arm only. Can you explain why this is not
>>>> enabled for x86 (other than this is not yet used)?
>>>>
>>>>>     done
>>>>> diff --git a/automation/scripts/qemu-staticmem-arm64.sh
>>>>> b/automation/scripts/qemu-staticmem-arm64.sh
>>>>> new file mode 100755
>>>>> index 0000000000..5b89a151aa
>>>>> --- /dev/null
>>>>> +++ b/automation/scripts/qemu-staticmem-arm64.sh
>>>>> @@ -0,0 +1,114 @@
>>>>> +#!/bin/bash
>>>>> +
>>>>> +base=(0x50000000 0x100000000)
>>>>> +size=(0x10000000 0x10000000)
>>>>
>>>>   From the name, it is not clear what the base and size refers too. Looking
>>>> a
>>>> bit below, it seems to be referring to the domain memory. If so, I would
>>>> suggest to comment and rename to "domu_{base, size}".
>>>>
>>>>> +
>>>>> +set -ex
>>>>> +
>>>>> +apt-get -qy update
>>>>> +apt-get -qy install --no-install-recommends u-boot-qemu \
>>>>> +                                            u-boot-tools \
>>>>> +                                            device-tree-compiler \
>>>>> +                                            cpio \
>>>>> +                                            curl \
>>>>> +                                            busybox-static
>>>>> +
>>>>> +# DomU Busybox
>>>>> +cd binaries
>>>>> +mkdir -p initrd
>>>>> +mkdir -p initrd/bin
>>>>> +mkdir -p initrd/sbin
>>>>> +mkdir -p initrd/etc
>>>>> +mkdir -p initrd/dev
>>>>> +mkdir -p initrd/proc
>>>>> +mkdir -p initrd/sys
>>>>> +mkdir -p initrd/lib
>>>>> +mkdir -p initrd/var
>>>>> +mkdir -p initrd/mnt
>>>>> +cp /bin/busybox initrd/bin/busybox
>>>>> +initrd/bin/busybox --install initrd/bin
>>>>> +echo "#!/bin/sh
>>>>> +
>>>>> +mount -t proc proc /proc
>>>>> +mount -t sysfs sysfs /sys
>>>>> +mount -t devtmpfs devtmpfs /dev
>>>>> +/bin/sh" > initrd/init
>>>>> +chmod +x initrd/init
>>>>> +cd initrd
>>>>> +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
>>>>> +cd ../..
>>>>> +
>>>>> +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
>>>>> +curl -fsSLO
>>>>> https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
>>>>> +
>>>>> +./binaries/qemu-system-aarch64 -nographic \
>>>>> +    -M virtualization=true \
>>>>> +    -M virt \
>>>>> +    -M virt,gic-version=2 \
>>>>> +    -cpu cortex-a57 \
>>>>> +    -smp 2 \
>>>>> +    -m 8G \
>>>>> +    -M dumpdtb=binaries/virt-gicv2.dtb
>>>>> +
>>>>> +#dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts
>>>>> +
>>>>> +# ImageBuilder
>>>>> +rm -rf imagebuilder
>>>>> +git clone https://gitlab.com/ViryaOS/imagebuilder
>>>>> +
>>>>> +echo "MEMORY_START=\"0x40000000\"
>>>>> +MEMORY_END=\"0x0200000000\"
>>>>> +
>>>>> +DEVICE_TREE=\"virt-gicv2.dtb\"
>>>>> +
>>>>> +XEN=\"xen-static_mem\"
>>>>> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
>>>>
>>>> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is also
>>>> not
>>>> clear why you need to pass xsm=dummy.
>>>>
>>>>> +
>>>>> +NUM_DOMUS=1
>>>>> +DOMU_MEM[0]=512
>>>>> +DOMU_VCPUS[0]=1
>>>>> +DOMU_KERNEL[0]=\"Image\"
>>>>> +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
>>>>> +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
>>>>> +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
>>>>> +
>>>>> +UBOOT_SOURCE=\"boot.source\"
>>>>> +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
>>>>> +
>>>>> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c
>>>>> binaries/imagebuilder_config
>>>>> +
>>>>> +# Run the test
>>>>> +rm -f qemu-staticmem.serial
>>>>> +set +e
>>>>> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source
>>>>> 0x40000000"| \
>>>>> +timeout -k 1 60 ./binaries/qemu-system-aarch64 -nographic \
>>>>> +    -M virtualization=true \
>>>>> +    -M virt \
>>>>> +    -M virt,gic-version=2 \
>>>>> +    -cpu cortex-a57 \
>>>>> +    -smp 2 \
>>>>> +    -m 8G \
>>>>> +    -no-reboot \
>>>>> +    -device virtio-net-pci,netdev=vnet0 -netdev
>>>>> user,id=vnet0,tftp=binaries
>>>>> \
>>>>> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
>>>>> +    -dtb ./binaries/virt-gicv2.dtb \
>>>>> +    |& tee qemu-staticmem.serial
>>>>> +
>>>>> +set -e
>>>>
>>>> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I think it
>>>> would be better to consolidate in a single script. Looking briefly
>>>> throught
>>>> the existing scripts, it looks like it is possible to pass arguments (see
>>>> qemu-smoke-x86-64.sh).
>>>    One idea would be to make the script common and "source" a second
>>> script or config file with just the ImageBuilder configuration because
>>> it looks like it is the only thing different.
>>>
>>>
>>>>> +
>>>>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) || exit 1
>>>>> +
>>>>> +for ((i=0; i<${#base[@]}; i++))
>>>>> +do
>>>>> +    start="$(printf "0x%016x" ${base[$i]})"
>>>>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
>>>>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
>>>>> +    if test $? -eq 1
>>>>> +    then
>>>>> +        exit 1
>>>>> +    fi
>>>>> +done
>>>>
>>>> Please add a comment on top to explain what this is meant to do. However,
>>>> I
>>>> think we should avoid relying on output that we have written ourself. IOW,
>>>> relying on Xen/Linux to always write the same message is risky because
>>>> they
>>>> can change at any time.
>>>
>>> Especially if we make the script common, then we could just rely on the
>>> existing check to see if the guest started correctly (no special check
>>> for static memory).
>>
>> In this case, how the test will verify that the static memory configuration
>> has been taken into account and has not just been ignored?
> 
> There is no simple way that I can think of.
> 
> We can trust that it worked, without checking that the ranges were
> actually static as requested.
> 
> We can parse the Xen output like you did, although if it changes then
> the test will break.
> 
> Or we could add a script to detect and print specific output but I
> don't know if there is something under /proc or /sys that we could
> simply "cat" from bash to check it.
> 
> If there is a simple way to do this by cat'ing /proc or /sys then I
> think that would be great. Otherwise I think we could do as you did in
> this patch, which is not perfect but it works and if something changes
> in the Xen output we'll detect it right away given that gitlab-ci is run
> pre-commit.

Ok, I 'll send another patch that will enable static memory for all arm 
xen builds and will use a single script.
For the static memory case, I will try to verify the values of the 
guest's dt memory node via /proc/device-tree.

Also, I will do some minor cleanups in qemu-smoke-arm64.sh because there 
are some inconsistencies for instance the dtb is named virt-gicv3 while 
runs with gicv2, a comment states that qemu gets installed while it is 
taken from test-artifacts container.

Thanks to both of you for the review.

-- 
Xenia

RE: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Penny Zheng]

Hi Xenia

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Xenia Ragiadakou
> Sent: Friday, July 8, 2022 5:54 PM
> To: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>
> Cc: xen-devel@lists.xenproject.org; Doug Goldstein <cardoe@cardoe.com>
> Subject: Re: [PATCH 2/2] automation: arm64: Create a test job for testing
> static allocation on qemu
> 
> Hi Stefano,
> 
> On 7/8/22 02:05, Stefano Stabellini wrote:
> > On Thu, 7 Jul 2022, Julien Grall wrote:
> >> Hi Xenia,
> >>
> >> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
> >>> Add an arm subdirectory under automation/configs for the arm
> >>> specific configs and add a config that enables static allocation.
> >>>
> >>> Modify the build script to search for configs also in this
> >>> subdirectory and to keep the generated xen binary, suffixed with the
> >>> config file name, as artifact.
> >>>
> >>> Create a test job that
> >>> - boots xen on qemu with a single direct mapped dom0less guest
> >>> configured with statically allocated memory

Although you said booting a single direct mapped dom0less guest
configured with statically allocated memory here, later in code, you are
only enabling statically allocated memory in the ImageBuilder script,
missing the direct-map property.
 
> >>> - verifies that the memory ranges reported in the guest's logs are
> >>> the same with the provided static memory regions
> >>>
> >>> For guest kernel, use the 5.9.9 kernel from the tests-artifacts containers.
> >>> Use busybox-static package, to create the guest ramdisk.
> >>> To generate the u-boot script, use ImageBuilder.
> >>> Use the qemu from the tests-artifacts containers.
> >>>
> >>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> >>> ---
> >>>    automation/configs/arm/static_mem          |   3 +
> >>>    automation/gitlab-ci/test.yaml             |  24 +++++
> >>>    automation/scripts/build                   |   4 +
> >>>    automation/scripts/qemu-staticmem-arm64.sh | 114
> +++++++++++++++++++++
> >>>    4 files changed, 145 insertions(+)
> >>>    create mode 100644 automation/configs/arm/static_mem
> >>>    create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
> >>>
> >>> diff --git a/automation/configs/arm/static_mem
> >>> b/automation/configs/arm/static_mem
> >>> new file mode 100644
> >>> index 0000000000..84675ddf4e
> >>> --- /dev/null
> >>> +++ b/automation/configs/arm/static_mem
> >>> @@ -0,0 +1,3 @@
> >>> +CONFIG_EXPERT=y
> >>> +CONFIG_UNSUPPORTED=y
> >>> +CONFIG_STATIC_MEMORY=y
> >>> \ No newline at end of file
> >>
> >> Any particular reason to build a new Xen rather enable
> >> CONFIG_STATIC_MEMORY in the existing build?
> >>
> >>> diff --git a/automation/scripts/build b/automation/scripts/build
> >>> index 21b3bc57c8..9c6196d9bd 100755
> >>> --- a/automation/scripts/build
> >>> +++ b/automation/scripts/build
> >>> @@ -83,6 +83,7 @@ fi
> >>>    # Build all the configs we care about
> >>>    case ${XEN_TARGET_ARCH} in
> >>>        x86_64) arch=x86 ;;
> >>> +    arm64) arch=arm ;;
> >>>        *) exit 0 ;;
> >>>    esac
> >>>    @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
> >>>        rm -f xen/.config
> >>>        make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
> >>>        make -j$(nproc) -C xen
> >>> +    if [[ ${arch} == "arm" ]]; then
> >>> +        cp xen/xen binaries/xen-${cfg}
> >>> +    fi
> >>
> >> This feels a bit of a hack to be arm only. Can you explain why this
> >> is not enabled for x86 (other than this is not yet used)?
> >>
> >>>    done
> >>> diff --git a/automation/scripts/qemu-staticmem-arm64.sh
> >>> b/automation/scripts/qemu-staticmem-arm64.sh
> >>> new file mode 100755
> >>> index 0000000000..5b89a151aa
> >>> --- /dev/null
> >>> +++ b/automation/scripts/qemu-staticmem-arm64.sh
> >>> @@ -0,0 +1,114 @@
> >>> +#!/bin/bash
> >>> +
> >>> +base=(0x50000000 0x100000000)
> >>> +size=(0x10000000 0x10000000)
> >>
> >>  From the name, it is not clear what the base and size refers too.
> >> Looking a bit below, it seems to be referring to the domain memory.
> >> If so, I would suggest to comment and rename to "domu_{base, size}".
> >>
> >>> +
> >>> +set -ex
> >>> +
> >>> +apt-get -qy update
> >>> +apt-get -qy install --no-install-recommends u-boot-qemu \
> >>> +                                            u-boot-tools \
> >>> +                                            device-tree-compiler \
> >>> +                                            cpio \
> >>> +                                            curl \
> >>> +                                            busybox-static
> >>> +
> >>> +# DomU Busybox
> >>> +cd binaries
> >>> +mkdir -p initrd
> >>> +mkdir -p initrd/bin
> >>> +mkdir -p initrd/sbin
> >>> +mkdir -p initrd/etc
> >>> +mkdir -p initrd/dev
> >>> +mkdir -p initrd/proc
> >>> +mkdir -p initrd/sys
> >>> +mkdir -p initrd/lib
> >>> +mkdir -p initrd/var
> >>> +mkdir -p initrd/mnt
> >>> +cp /bin/busybox initrd/bin/busybox
> >>> +initrd/bin/busybox --install initrd/bin echo "#!/bin/sh
> >>> +
> >>> +mount -t proc proc /proc
> >>> +mount -t sysfs sysfs /sys
> >>> +mount -t devtmpfs devtmpfs /dev
> >>> +/bin/sh" > initrd/init
> >>> +chmod +x initrd/init
> >>> +cd initrd
> >>> +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
> >>> +cd ../..
> >>> +
> >>> +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded curl
> >>> +-fsSLO
> >>> +https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
> >>> +
> >>> +./binaries/qemu-system-aarch64 -nographic \
> >>> +    -M virtualization=true \
> >>> +    -M virt \
> >>> +    -M virt,gic-version=2 \
> >>> +    -cpu cortex-a57 \
> >>> +    -smp 2 \
> >>> +    -m 8G \
> >>> +    -M dumpdtb=binaries/virt-gicv2.dtb
> >>> +
> >>> +#dtc -I dtb -O dts binaries/virt-gicv2.dtb >
> >>> +binaries/virt-gicv2.dts
> >>> +
> >>> +# ImageBuilder
> >>> +rm -rf imagebuilder
> >>> +git clone https://gitlab.com/ViryaOS/imagebuilder
> >>> +
> >>> +echo "MEMORY_START=\"0x40000000\"
> >>> +MEMORY_END=\"0x0200000000\"
> >>> +
> >>> +DEVICE_TREE=\"virt-gicv2.dtb\"
> >>> +
> >>> +XEN=\"xen-static_mem\"
> >>> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
> >>
> >> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is
> >> also not clear why you need to pass xsm=dummy.
> >>
> >>> +
> >>> +NUM_DOMUS=1
> >>> +DOMU_MEM[0]=512
> >>> +DOMU_VCPUS[0]=1
> >>> +DOMU_KERNEL[0]=\"Image\"
> >>> +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
> >>> +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
> >>> +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
> >>> +

You would like to add  DOMU_DIRECT_MAP[0] = 1 to enable direct-map.

> >>> +UBOOT_SOURCE=\"boot.source\"
> >>> +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
> >>> +
> >>> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c
> >>> binaries/imagebuilder_config
> >>> +
> >>> +# Run the test
> >>> +rm -f qemu-staticmem.serial
> >>> +set +e
> >>> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source
> >>> +0x40000000"| \ timeout -k 1 60 ./binaries/qemu-system-aarch64 -
> nographic \
> >>> +    -M virtualization=true \
> >>> +    -M virt \
> >>> +    -M virt,gic-version=2 \
> >>> +    -cpu cortex-a57 \
> >>> +    -smp 2 \
> >>> +    -m 8G \
> >>> +    -no-reboot \
> >>> +    -device virtio-net-pci,netdev=vnet0 -netdev
> >>> +user,id=vnet0,tftp=binaries
> >>> \
> >>> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
> >>> +    -dtb ./binaries/virt-gicv2.dtb \
> >>> +    |& tee qemu-staticmem.serial
> >>> +
> >>> +set -e
> >>
> >> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I
> >> think it would be better to consolidate in a single script. Looking
> >> briefly throught the existing scripts, it looks like it is possible
> >> to pass arguments (see qemu-smoke-x86-64.sh).
> >
> > One idea would be to make the script common and "source" a second
> > script or config file with just the ImageBuilder configuration because
> > it looks like it is the only thing different.
> >
> >
> >>> +
> >>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) ||
> >>> +exit 1
> >>> +
> >>> +for ((i=0; i<${#base[@]}; i++))
> >>> +do
> >>> +    start="$(printf "0x%016x" ${base[$i]})"
> >>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
> >>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
> >>> +    if test $? -eq 1
> >>> +    then
> >>> +        exit 1
> >>> +    fi
> >>> +done
> >>
> >> Please add a comment on top to explain what this is meant to do.
> >> However, I think we should avoid relying on output that we have
> >> written ourself. IOW, relying on Xen/Linux to always write the same
> >> message is risky because they can change at any time.
> >
> > Especially if we make the script common, then we could just rely on
> > the existing check to see if the guest started correctly (no special
> > check for static memory).
> 
> In this case, how the test will verify that the static memory configuration has
> been taken into account and has not just been ignored?
> 

If only statically allocated memory is enabled, guest memory address will still be mapped
to GUEST_RAM_BASE(0x40000000)

> >>> +
> >>> +(grep -q "BusyBox" qemu-staticmem.serial) || exit 1
> >
> 
> --
> Xenia

---
Cheers,
Penny Zheng

Re: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Xenia Ragiadakou]

Hi Penny,

On 11/7/22 12:02, Penny Zheng wrote:
> Hi Xenia
> 
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
>> Xenia Ragiadakou
>> Sent: Friday, July 8, 2022 5:54 PM
>> To: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>
>> Cc: xen-devel@lists.xenproject.org; Doug Goldstein <cardoe@cardoe.com>
>> Subject: Re: [PATCH 2/2] automation: arm64: Create a test job for testing
>> static allocation on qemu
>>
>> Hi Stefano,
>>
>> On 7/8/22 02:05, Stefano Stabellini wrote:
>>> On Thu, 7 Jul 2022, Julien Grall wrote:
>>>> Hi Xenia,
>>>>
>>>> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
>>>>> Add an arm subdirectory under automation/configs for the arm
>>>>> specific configs and add a config that enables static allocation.
>>>>>
>>>>> Modify the build script to search for configs also in this
>>>>> subdirectory and to keep the generated xen binary, suffixed with the
>>>>> config file name, as artifact.
>>>>>
>>>>> Create a test job that
>>>>> - boots xen on qemu with a single direct mapped dom0less guest
>>>>> configured with statically allocated memory
> 
> Although you said booting a single direct mapped dom0less guest
> configured with statically allocated memory here, later in code, you are
> only enabling statically allocated memory in the ImageBuilder script,
> missing the direct-map property.
>   
>>>>> - verifies that the memory ranges reported in the guest's logs are
>>>>> the same with the provided static memory regions
>>>>>
>>>>> For guest kernel, use the 5.9.9 kernel from the tests-artifacts containers.
>>>>> Use busybox-static package, to create the guest ramdisk.
>>>>> To generate the u-boot script, use ImageBuilder.
>>>>> Use the qemu from the tests-artifacts containers.
>>>>>
>>>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>>>>> ---
>>>>>     automation/configs/arm/static_mem          |   3 +
>>>>>     automation/gitlab-ci/test.yaml             |  24 +++++
>>>>>     automation/scripts/build                   |   4 +
>>>>>     automation/scripts/qemu-staticmem-arm64.sh | 114
>> +++++++++++++++++++++
>>>>>     4 files changed, 145 insertions(+)
>>>>>     create mode 100644 automation/configs/arm/static_mem
>>>>>     create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
>>>>>
>>>>> diff --git a/automation/configs/arm/static_mem
>>>>> b/automation/configs/arm/static_mem
>>>>> new file mode 100644
>>>>> index 0000000000..84675ddf4e
>>>>> --- /dev/null
>>>>> +++ b/automation/configs/arm/static_mem
>>>>> @@ -0,0 +1,3 @@
>>>>> +CONFIG_EXPERT=y
>>>>> +CONFIG_UNSUPPORTED=y
>>>>> +CONFIG_STATIC_MEMORY=y
>>>>> \ No newline at end of file
>>>>
>>>> Any particular reason to build a new Xen rather enable
>>>> CONFIG_STATIC_MEMORY in the existing build?
>>>>
>>>>> diff --git a/automation/scripts/build b/automation/scripts/build
>>>>> index 21b3bc57c8..9c6196d9bd 100755
>>>>> --- a/automation/scripts/build
>>>>> +++ b/automation/scripts/build
>>>>> @@ -83,6 +83,7 @@ fi
>>>>>     # Build all the configs we care about
>>>>>     case ${XEN_TARGET_ARCH} in
>>>>>         x86_64) arch=x86 ;;
>>>>> +    arm64) arch=arm ;;
>>>>>         *) exit 0 ;;
>>>>>     esac
>>>>>     @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
>>>>>         rm -f xen/.config
>>>>>         make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
>>>>>         make -j$(nproc) -C xen
>>>>> +    if [[ ${arch} == "arm" ]]; then
>>>>> +        cp xen/xen binaries/xen-${cfg}
>>>>> +    fi
>>>>
>>>> This feels a bit of a hack to be arm only. Can you explain why this
>>>> is not enabled for x86 (other than this is not yet used)?
>>>>
>>>>>     done
>>>>> diff --git a/automation/scripts/qemu-staticmem-arm64.sh
>>>>> b/automation/scripts/qemu-staticmem-arm64.sh
>>>>> new file mode 100755
>>>>> index 0000000000..5b89a151aa
>>>>> --- /dev/null
>>>>> +++ b/automation/scripts/qemu-staticmem-arm64.sh
>>>>> @@ -0,0 +1,114 @@
>>>>> +#!/bin/bash
>>>>> +
>>>>> +base=(0x50000000 0x100000000)
>>>>> +size=(0x10000000 0x10000000)
>>>>
>>>>   From the name, it is not clear what the base and size refers too.
>>>> Looking a bit below, it seems to be referring to the domain memory.
>>>> If so, I would suggest to comment and rename to "domu_{base, size}".
>>>>
>>>>> +
>>>>> +set -ex
>>>>> +
>>>>> +apt-get -qy update
>>>>> +apt-get -qy install --no-install-recommends u-boot-qemu \
>>>>> +                                            u-boot-tools \
>>>>> +                                            device-tree-compiler \
>>>>> +                                            cpio \
>>>>> +                                            curl \
>>>>> +                                            busybox-static
>>>>> +
>>>>> +# DomU Busybox
>>>>> +cd binaries
>>>>> +mkdir -p initrd
>>>>> +mkdir -p initrd/bin
>>>>> +mkdir -p initrd/sbin
>>>>> +mkdir -p initrd/etc
>>>>> +mkdir -p initrd/dev
>>>>> +mkdir -p initrd/proc
>>>>> +mkdir -p initrd/sys
>>>>> +mkdir -p initrd/lib
>>>>> +mkdir -p initrd/var
>>>>> +mkdir -p initrd/mnt
>>>>> +cp /bin/busybox initrd/bin/busybox
>>>>> +initrd/bin/busybox --install initrd/bin echo "#!/bin/sh
>>>>> +
>>>>> +mount -t proc proc /proc
>>>>> +mount -t sysfs sysfs /sys
>>>>> +mount -t devtmpfs devtmpfs /dev
>>>>> +/bin/sh" > initrd/init
>>>>> +chmod +x initrd/init
>>>>> +cd initrd
>>>>> +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
>>>>> +cd ../..
>>>>> +
>>>>> +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded curl
>>>>> +-fsSLO
>>>>> +https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
>>>>> +
>>>>> +./binaries/qemu-system-aarch64 -nographic \
>>>>> +    -M virtualization=true \
>>>>> +    -M virt \
>>>>> +    -M virt,gic-version=2 \
>>>>> +    -cpu cortex-a57 \
>>>>> +    -smp 2 \
>>>>> +    -m 8G \
>>>>> +    -M dumpdtb=binaries/virt-gicv2.dtb
>>>>> +
>>>>> +#dtc -I dtb -O dts binaries/virt-gicv2.dtb >
>>>>> +binaries/virt-gicv2.dts
>>>>> +
>>>>> +# ImageBuilder
>>>>> +rm -rf imagebuilder
>>>>> +git clone https://gitlab.com/ViryaOS/imagebuilder
>>>>> +
>>>>> +echo "MEMORY_START=\"0x40000000\"
>>>>> +MEMORY_END=\"0x0200000000\"
>>>>> +
>>>>> +DEVICE_TREE=\"virt-gicv2.dtb\"
>>>>> +
>>>>> +XEN=\"xen-static_mem\"
>>>>> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
>>>>
>>>> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It is
>>>> also not clear why you need to pass xsm=dummy.
>>>>
>>>>> +
>>>>> +NUM_DOMUS=1
>>>>> +DOMU_MEM[0]=512
>>>>> +DOMU_VCPUS[0]=1
>>>>> +DOMU_KERNEL[0]=\"Image\"
>>>>> +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
>>>>> +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
>>>>> +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]} ${size[1]}\"
>>>>> +
> 
> You would like to add  DOMU_DIRECT_MAP[0] = 1 to enable direct-map.

The imagebuilder configuration option DOMU_DIRECT_MAP defaults to 1.

But I could add DOMU_DIRECT_MAP[0]=1 in the script to make it clearer.

>>>>> +UBOOT_SOURCE=\"boot.source\"
>>>>> +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
>>>>> +
>>>>> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c
>>>>> binaries/imagebuilder_config
>>>>> +
>>>>> +# Run the test
>>>>> +rm -f qemu-staticmem.serial
>>>>> +set +e
>>>>> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source
>>>>> +0x40000000"| \ timeout -k 1 60 ./binaries/qemu-system-aarch64 -
>> nographic \
>>>>> +    -M virtualization=true \
>>>>> +    -M virt \
>>>>> +    -M virt,gic-version=2 \
>>>>> +    -cpu cortex-a57 \
>>>>> +    -smp 2 \
>>>>> +    -m 8G \
>>>>> +    -no-reboot \
>>>>> +    -device virtio-net-pci,netdev=vnet0 -netdev
>>>>> +user,id=vnet0,tftp=binaries
>>>>> \
>>>>> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
>>>>> +    -dtb ./binaries/virt-gicv2.dtb \
>>>>> +    |& tee qemu-staticmem.serial
>>>>> +
>>>>> +set -e
>>>>
>>>> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I
>>>> think it would be better to consolidate in a single script. Looking
>>>> briefly throught the existing scripts, it looks like it is possible
>>>> to pass arguments (see qemu-smoke-x86-64.sh).
>>>
>>> One idea would be to make the script common and "source" a second
>>> script or config file with just the ImageBuilder configuration because
>>> it looks like it is the only thing different.
>>>
>>>
>>>>> +
>>>>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) ||
>>>>> +exit 1
>>>>> +
>>>>> +for ((i=0; i<${#base[@]}; i++))
>>>>> +do
>>>>> +    start="$(printf "0x%016x" ${base[$i]})"
>>>>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
>>>>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
>>>>> +    if test $? -eq 1
>>>>> +    then
>>>>> +        exit 1
>>>>> +    fi
>>>>> +done
>>>>
>>>> Please add a comment on top to explain what this is meant to do.
>>>> However, I think we should avoid relying on output that we have
>>>> written ourself. IOW, relying on Xen/Linux to always write the same
>>>> message is risky because they can change at any time.
>>>
>>> Especially if we make the script common, then we could just rely on
>>> the existing check to see if the guest started correctly (no special
>>> check for static memory).
>>
>> In this case, how the test will verify that the static memory configuration has
>> been taken into account and has not just been ignored?
>>
> 
> If only statically allocated memory is enabled, guest memory address will still be mapped
> to GUEST_RAM_BASE(0x40000000)
> 
>>>>> +
>>>>> +(grep -q "BusyBox" qemu-staticmem.serial) || exit 1
>>>
>>
>> --
>> Xenia
> 
> ---
> Cheers,
> Penny Zheng
> 
> 
> 
> 

RE: [PATCH 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Penny Zheng]

Hi Xenia

> -----Original Message-----
> From: Xenia Ragiadakou <burzalodowa@gmail.com>
> Sent: Monday, July 11, 2022 11:29 PM
> To: Penny Zheng <Penny.Zheng@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Julien Grall <julien@xen.org>
> Cc: xen-devel@lists.xenproject.org; Doug Goldstein <cardoe@cardoe.com>
> Subject: Re: [PATCH 2/2] automation: arm64: Create a test job for testing
> static allocation on qemu
> 
> Hi Penny,
> 
> On 11/7/22 12:02, Penny Zheng wrote:
> > Hi Xenia
> >
> >> -----Original Message-----
> >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> >> Xenia Ragiadakou
> >> Sent: Friday, July 8, 2022 5:54 PM
> >> To: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> >> <julien@xen.org>
> >> Cc: xen-devel@lists.xenproject.org; Doug Goldstein
> >> <cardoe@cardoe.com>
> >> Subject: Re: [PATCH 2/2] automation: arm64: Create a test job for
> >> testing static allocation on qemu
> >>
> >> Hi Stefano,
> >>
> >> On 7/8/22 02:05, Stefano Stabellini wrote:
> >>> On Thu, 7 Jul 2022, Julien Grall wrote:
> >>>> Hi Xenia,
> >>>>
> >>>> On 07/07/2022 21:38, Xenia Ragiadakou wrote:
> >>>>> Add an arm subdirectory under automation/configs for the arm
> >>>>> specific configs and add a config that enables static allocation.
> >>>>>
> >>>>> Modify the build script to search for configs also in this
> >>>>> subdirectory and to keep the generated xen binary, suffixed with
> >>>>> the config file name, as artifact.
> >>>>>
> >>>>> Create a test job that
> >>>>> - boots xen on qemu with a single direct mapped dom0less guest
> >>>>> configured with statically allocated memory
> >
> > Although you said booting a single direct mapped dom0less guest
> > configured with statically allocated memory here, later in code, you
> > are only enabling statically allocated memory in the ImageBuilder
> > script, missing the direct-map property.
> >
> >>>>> - verifies that the memory ranges reported in the guest's logs are
> >>>>> the same with the provided static memory regions
> >>>>>
> >>>>> For guest kernel, use the 5.9.9 kernel from the tests-artifacts
> containers.
> >>>>> Use busybox-static package, to create the guest ramdisk.
> >>>>> To generate the u-boot script, use ImageBuilder.
> >>>>> Use the qemu from the tests-artifacts containers.
> >>>>>
> >>>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> >>>>> ---
> >>>>>     automation/configs/arm/static_mem          |   3 +
> >>>>>     automation/gitlab-ci/test.yaml             |  24 +++++
> >>>>>     automation/scripts/build                   |   4 +
> >>>>>     automation/scripts/qemu-staticmem-arm64.sh | 114
> >> +++++++++++++++++++++
> >>>>>     4 files changed, 145 insertions(+)
> >>>>>     create mode 100644 automation/configs/arm/static_mem
> >>>>>     create mode 100755 automation/scripts/qemu-staticmem-arm64.sh
> >>>>>
> >>>>> diff --git a/automation/configs/arm/static_mem
> >>>>> b/automation/configs/arm/static_mem
> >>>>> new file mode 100644
> >>>>> index 0000000000..84675ddf4e
> >>>>> --- /dev/null
> >>>>> +++ b/automation/configs/arm/static_mem
> >>>>> @@ -0,0 +1,3 @@
> >>>>> +CONFIG_EXPERT=y
> >>>>> +CONFIG_UNSUPPORTED=y
> >>>>> +CONFIG_STATIC_MEMORY=y
> >>>>> \ No newline at end of file
> >>>>
> >>>> Any particular reason to build a new Xen rather enable
> >>>> CONFIG_STATIC_MEMORY in the existing build?
> >>>>
> >>>>> diff --git a/automation/scripts/build b/automation/scripts/build
> >>>>> index 21b3bc57c8..9c6196d9bd 100755
> >>>>> --- a/automation/scripts/build
> >>>>> +++ b/automation/scripts/build
> >>>>> @@ -83,6 +83,7 @@ fi
> >>>>>     # Build all the configs we care about
> >>>>>     case ${XEN_TARGET_ARCH} in
> >>>>>         x86_64) arch=x86 ;;
> >>>>> +    arm64) arch=arm ;;
> >>>>>         *) exit 0 ;;
> >>>>>     esac
> >>>>>     @@ -93,4 +94,7 @@ for cfg in `ls ${cfg_dir}`; do
> >>>>>         rm -f xen/.config
> >>>>>         make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg}
> defconfig
> >>>>>         make -j$(nproc) -C xen
> >>>>> +    if [[ ${arch} == "arm" ]]; then
> >>>>> +        cp xen/xen binaries/xen-${cfg}
> >>>>> +    fi
> >>>>
> >>>> This feels a bit of a hack to be arm only. Can you explain why this
> >>>> is not enabled for x86 (other than this is not yet used)?
> >>>>
> >>>>>     done
> >>>>> diff --git a/automation/scripts/qemu-staticmem-arm64.sh
> >>>>> b/automation/scripts/qemu-staticmem-arm64.sh
> >>>>> new file mode 100755
> >>>>> index 0000000000..5b89a151aa
> >>>>> --- /dev/null
> >>>>> +++ b/automation/scripts/qemu-staticmem-arm64.sh
> >>>>> @@ -0,0 +1,114 @@
> >>>>> +#!/bin/bash
> >>>>> +
> >>>>> +base=(0x50000000 0x100000000)
> >>>>> +size=(0x10000000 0x10000000)
> >>>>
> >>>>   From the name, it is not clear what the base and size refers too.
> >>>> Looking a bit below, it seems to be referring to the domain memory.
> >>>> If so, I would suggest to comment and rename to "domu_{base, size}".
> >>>>
> >>>>> +
> >>>>> +set -ex
> >>>>> +
> >>>>> +apt-get -qy update
> >>>>> +apt-get -qy install --no-install-recommends u-boot-qemu \
> >>>>> +                                            u-boot-tools \
> >>>>> +                                            device-tree-compiler \
> >>>>> +                                            cpio \
> >>>>> +                                            curl \
> >>>>> +                                            busybox-static
> >>>>> +
> >>>>> +# DomU Busybox
> >>>>> +cd binaries
> >>>>> +mkdir -p initrd
> >>>>> +mkdir -p initrd/bin
> >>>>> +mkdir -p initrd/sbin
> >>>>> +mkdir -p initrd/etc
> >>>>> +mkdir -p initrd/dev
> >>>>> +mkdir -p initrd/proc
> >>>>> +mkdir -p initrd/sys
> >>>>> +mkdir -p initrd/lib
> >>>>> +mkdir -p initrd/var
> >>>>> +mkdir -p initrd/mnt
> >>>>> +cp /bin/busybox initrd/bin/busybox initrd/bin/busybox --install
> >>>>> +initrd/bin echo "#!/bin/sh
> >>>>> +
> >>>>> +mount -t proc proc /proc
> >>>>> +mount -t sysfs sysfs /sys
> >>>>> +mount -t devtmpfs devtmpfs /dev
> >>>>> +/bin/sh" > initrd/init
> >>>>> +chmod +x initrd/init
> >>>>> +cd initrd
> >>>>> +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
> >>>>> +cd ../..
> >>>>> +
> >>>>> +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded curl
> >>>>> +-fsSLO
> >>>>> +https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
> >>>>> +
> >>>>> +./binaries/qemu-system-aarch64 -nographic \
> >>>>> +    -M virtualization=true \
> >>>>> +    -M virt \
> >>>>> +    -M virt,gic-version=2 \
> >>>>> +    -cpu cortex-a57 \
> >>>>> +    -smp 2 \
> >>>>> +    -m 8G \
> >>>>> +    -M dumpdtb=binaries/virt-gicv2.dtb
> >>>>> +
> >>>>> +#dtc -I dtb -O dts binaries/virt-gicv2.dtb >
> >>>>> +binaries/virt-gicv2.dts
> >>>>> +
> >>>>> +# ImageBuilder
> >>>>> +rm -rf imagebuilder
> >>>>> +git clone https://gitlab.com/ViryaOS/imagebuilder
> >>>>> +
> >>>>> +echo "MEMORY_START=\"0x40000000\"
> >>>>> +MEMORY_END=\"0x0200000000\"
> >>>>> +
> >>>>> +DEVICE_TREE=\"virt-gicv2.dtb\"
> >>>>> +
> >>>>> +XEN=\"xen-static_mem\"
> >>>>> +XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"
> >>>>
> >>>> AFAIK, earlyprintk is not an option for Xen on Arm (at least). It
> >>>> is also not clear why you need to pass xsm=dummy.
> >>>>
> >>>>> +
> >>>>> +NUM_DOMUS=1
> >>>>> +DOMU_MEM[0]=512
> >>>>> +DOMU_VCPUS[0]=1
> >>>>> +DOMU_KERNEL[0]=\"Image\"
> >>>>> +DOMU_RAMDISK[0]=\"initrd.cpio.gz\"
> >>>>> +DOMU_CMD[0]=\"earlyprintk console=ttyAMA0\"
> >>>>> +DOMU_STATIC_MEM[0]=\"${base[0]} ${size[0]} ${base[1]}
> ${size[1]}\"
> >>>>> +
> >
> > You would like to add  DOMU_DIRECT_MAP[0] = 1 to enable direct-map.
> 
> The imagebuilder configuration option DOMU_DIRECT_MAP defaults to 1.
> 
> But I could add DOMU_DIRECT_MAP[0]=1 in the script to make it clearer.
> 

Oh, sorry about that. Then everything is set clearly for me~~~

> >>>>> +UBOOT_SOURCE=\"boot.source\"
> >>>>> +UBOOT_SCRIPT=\"boot.scr\"" > binaries/imagebuilder_config
> >>>>> +
> >>>>> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/
> >>>>> +-c
> >>>>> binaries/imagebuilder_config
> >>>>> +
> >>>>> +# Run the test
> >>>>> +rm -f qemu-staticmem.serial
> >>>>> +set +e
> >>>>> +echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source
> >>>>> +0x40000000"| \ timeout -k 1 60 ./binaries/qemu-system-aarch64 -
> >> nographic \
> >>>>> +    -M virtualization=true \
> >>>>> +    -M virt \
> >>>>> +    -M virt,gic-version=2 \
> >>>>> +    -cpu cortex-a57 \
> >>>>> +    -smp 2 \
> >>>>> +    -m 8G \
> >>>>> +    -no-reboot \
> >>>>> +    -device virtio-net-pci,netdev=vnet0 -netdev
> >>>>> +user,id=vnet0,tftp=binaries
> >>>>> \
> >>>>> +    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin \
> >>>>> +    -dtb ./binaries/virt-gicv2.dtb \
> >>>>> +    |& tee qemu-staticmem.serial
> >>>>> +
> >>>>> +set -e
> >>>>
> >>>> A lot of the code above is duplicated from qemu-smoke-arm64.sh. I
> >>>> think it would be better to consolidate in a single script. Looking
> >>>> briefly throught the existing scripts, it looks like it is possible
> >>>> to pass arguments (see qemu-smoke-x86-64.sh).
> >>>
> >>> One idea would be to make the script common and "source" a second
> >>> script or config file with just the ImageBuilder configuration
> >>> because it looks like it is the only thing different.
> >>>
> >>>
> >>>>> +
> >>>>> +(grep -q "Xen dom0less mode detected" qemu-staticmem.serial) ||
> >>>>> +exit 1
> >>>>> +
> >>>>> +for ((i=0; i<${#base[@]}; i++))
> >>>>> +do
> >>>>> +    start="$(printf "0x%016x" ${base[$i]})"
> >>>>> +    end="$(printf "0x%016x" $((${base[$i]} + ${size[$i]} - 1)))"
> >>>>> +    grep -q "node   0: \[mem ${start}-${end}\]" qemu-staticmem.serial
> >>>>> +    if test $? -eq 1
> >>>>> +    then
> >>>>> +        exit 1
> >>>>> +    fi
> >>>>> +done
> >>>>
> >>>> Please add a comment on top to explain what this is meant to do.
> >>>> However, I think we should avoid relying on output that we have
> >>>> written ourself. IOW, relying on Xen/Linux to always write the same
> >>>> message is risky because they can change at any time.
> >>>
> >>> Especially if we make the script common, then we could just rely on
> >>> the existing check to see if the guest started correctly (no special
> >>> check for static memory).
> >>
> >> In this case, how the test will verify that the static memory
> >> configuration has been taken into account and has not just been ignored?
> >>
> >
> > If only statically allocated memory is enabled, guest memory address
> > will still be mapped to GUEST_RAM_BASE(0x40000000)
> >
> >>>>> +
> >>>>> +(grep -q "BusyBox" qemu-staticmem.serial) || exit 1
> >>>
> >>
> >> --
> >> Xenia
> >
> > ---
> > Cheers,
> > Penny Zheng
> >
> >
> >
> >