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

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@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
Xenia Ragiadakou
Sent: Friday, July 8, 2022 5:54 PM
To: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>
Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Doug Goldstein <cardoe@xxxxxxxxxx>
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@xxxxxxxxx>
---
    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








 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.