| Message ID | 20220707203803.798317-3-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Create a test job for testing static memory on qemu | expand |
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,
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
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, >
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,
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,
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.
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.
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,
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 >
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.
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,
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.
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
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 > > > >
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 > > > > > > > >
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
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