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 <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

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

If there is no issue with changing its configuration, I can enable static memory and use this one.

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

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

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


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

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

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


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

[...]

+# ImageBuilder
+rm -rf imagebuilder
+git clone https://gitlab.com/ViryaOS/imagebuilder
+
+echo "MEMORY_START=\"0x40000000\"
+MEMORY_END=\"0x0200000000\"
+
+DEVICE_TREE=\"virt-gicv2.dtb\"
+
+XEN=\"xen-static_mem\"
+XEN_CMD=\"console=dtuart earlyprintk xsm=dummy\"

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

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

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

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

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

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

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

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

Cheers,

--
Julien Grall

Reply via email to