Hi Xenia > -----Original Message----- > From: Xenia Ragiadakou <burzalod...@gmail.com> > Sent: Monday, July 11, 2022 11:29 PM > To: Penny Zheng <penny.zh...@arm.com>; Stefano Stabellini > <sstabell...@kernel.org>; Julien Grall <jul...@xen.org> > Cc: xen-devel@lists.xenproject.org; Doug Goldstein <car...@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-boun...@lists.xenproject.org> On Behalf Of > >> Xenia Ragiadakou > >> Sent: Friday, July 8, 2022 5:54 PM > >> To: Stefano Stabellini <sstabell...@kernel.org>; Julien Grall > >> <jul...@xen.org> > >> Cc: xen-devel@lists.xenproject.org; Doug Goldstein > >> <car...@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 <burzalod...@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 > > > > > > > >