Xen-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] automation: introduce a Xilinx hardware test
@ 2023-03-03 23:57 Stefano Stabellini
  2023-03-03 23:57 ` [PATCH v2 1/2] automation: add Ubuntu container for Xilinx hardware tests Stefano Stabellini
  2023-03-03 23:57 ` [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware Stefano Stabellini
  0 siblings, 2 replies; 10+ messages in thread
From: Stefano Stabellini @ 2023-03-03 23:57 UTC (permalink / raw
  To: xen-devel; +Cc: sstabellini, cardoe, michal.orzel

Hi all,

This short patch series introduces the first Xen gitlab-ci test run on
real hardware: a physical Xilinx ZCU102 board.

The gitlab container is run on a workstation physically connected to a
ZCU102 board. The test script looks very similar to a regular QEMU test
script, except that at the end it reboots the physical board and
connects to the serial instead of launching QEMU.

The gitlab runner is currently restricted to
https://gitlab.com/xen-project/xen. A CI/CD variable called
"XILINX_JOBS" is set to true for https://gitlab.com/xen-project/xen, so
that we can signal the pipeline to create a xilinx job, otherwise the
job is skipped as the runner is not available.

Cheers,

Stefano


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] automation: add Ubuntu container for Xilinx hardware tests
  2023-03-03 23:57 [PATCH v2 0/2] automation: introduce a Xilinx hardware test Stefano Stabellini
@ 2023-03-03 23:57 ` Stefano Stabellini
  2023-03-06  8:39   ` Michal Orzel
  2023-03-03 23:57 ` [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware Stefano Stabellini
  1 sibling, 1 reply; 10+ messages in thread
From: Stefano Stabellini @ 2023-03-03 23:57 UTC (permalink / raw
  To: xen-devel; +Cc: sstabellini, cardoe, michal.orzel, Stefano Stabellini

From: Stefano Stabellini <stefano.stabellini@amd.com>

This container is only run on the controller PC (x86) to trigger the
test on a connected Xilinx ZCU102 physical board.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 .../build/ubuntu/xenial-xilinx.dockerfile     | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 automation/build/ubuntu/xenial-xilinx.dockerfile

diff --git a/automation/build/ubuntu/xenial-xilinx.dockerfile b/automation/build/ubuntu/xenial-xilinx.dockerfile
new file mode 100644
index 0000000000..7e4f5d6605
--- /dev/null
+++ b/automation/build/ubuntu/xenial-xilinx.dockerfile
@@ -0,0 +1,25 @@
+FROM ubuntu:16.04
+LABEL maintainer.name="The Xen Project " \
+      maintainer.email="xen-devel@lists.xenproject.org"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+# build depends
+RUN apt-get update && \
+    apt-get --quiet --yes install \
+        snmp \
+        snmp-mibs-downloader \
+        u-boot-tools \
+        device-tree-compiler \
+        cpio \
+        git \
+        gzip \
+        file \
+        && \
+        apt-get autoremove -y && \
+        apt-get clean && \
+        rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware
  2023-03-03 23:57 [PATCH v2 0/2] automation: introduce a Xilinx hardware test Stefano Stabellini
  2023-03-03 23:57 ` [PATCH v2 1/2] automation: add Ubuntu container for Xilinx hardware tests Stefano Stabellini
@ 2023-03-03 23:57 ` Stefano Stabellini
  2023-03-06  9:21   ` Michal Orzel
  2023-03-06 10:25   ` Andrew Cooper
  1 sibling, 2 replies; 10+ messages in thread
From: Stefano Stabellini @ 2023-03-03 23:57 UTC (permalink / raw
  To: xen-devel; +Cc: sstabellini, cardoe, michal.orzel, Stefano Stabellini

From: Stefano Stabellini <stefano.stabellini@amd.com>

The test prepares dom0 and domU binaries and boot artifacts, similarly
to the existing QEMU test. (TBD: share preparation steps with the
regular QEMU tests.)

However, instead of running the test inside QEMU as usual, it copies
the binaries to the tftp server root, triggers a Xilinx ZCU102 board
reboot, and connects to the real serial of the board.

For now and due to its novelty, allow_failure on the Xilinx hardware
test, and only run the job on protected branches with XILINX_JOBS set to
true (the "master" and "staging" on gitlab.com/xen-project/xen qualify).

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v2:
- only execute the xilinx job on protected branches with XILINX_JOBS set
to true
---
 automation/gitlab-ci/test.yaml                |  22 ++++
 .../scripts/xilinx-smoke-dom0less-arm64.sh    | 115 ++++++++++++++++++
 2 files changed, 137 insertions(+)
 create mode 100755 automation/scripts/xilinx-smoke-dom0less-arm64.sh

diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 1c5f400b68..8f94cad32c 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -85,6 +85,28 @@ build-each-commit-gcc:
   tags:
     - x86_64
 
+xilinx-smoke-dom0less-arm64-gcc:
+  extends: .test-jobs-common
+  variables:
+    CONTAINER: ubuntu:xenial-xilinx
+    LOGFILE: qemu-smoke-xilinx.log
+  artifacts:
+    paths:
+      - smoke.serial
+      - '*.log'
+    when: always
+  tags:
+    - xilinx
+  script:
+    - ./automation/scripts/xilinx-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE}
+  needs:
+    - *arm64-test-needs
+    - alpine-3.12-gcc-arm64
+  allow_failure: true
+  only:
+    variables:
+      - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
+
 qemu-smoke-dom0-arm64-gcc:
   extends: .qemu-arm64
   script:
diff --git a/automation/scripts/xilinx-smoke-dom0less-arm64.sh b/automation/scripts/xilinx-smoke-dom0less-arm64.sh
new file mode 100755
index 0000000000..180c8b5f1c
--- /dev/null
+++ b/automation/scripts/xilinx-smoke-dom0less-arm64.sh
@@ -0,0 +1,115 @@
+#!/bin/bash
+
+set -ex
+
+test_variant=$1
+
+if [ -z "${test_variant}" ]; then
+    passed="ping test passed"
+    domU_check="
+until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do
+    sleep 30
+done
+echo \"${passed}\"
+"
+fi
+
+# DomU
+mkdir -p rootfs
+cd rootfs
+tar xzf ../binaries/initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+echo "#!/bin/sh
+
+${domU_check}
+/bin/sh" > etc/local.d/xen.start
+chmod +x etc/local.d/xen.start
+echo "rc_verbose=yes" >> etc/rc.conf
+find . | cpio -H newc -o | gzip > ../binaries/domU-rootfs.cpio.gz
+cd ..
+rm -rf rootfs
+
+# DOM0 rootfs
+mkdir -p rootfs
+cd rootfs
+tar xzf ../binaries/initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+cp -ar ../binaries/dist/install/* .
+
+echo "#!/bin/bash
+
+export LD_LIBRARY_PATH=/usr/local/lib
+bash /etc/init.d/xencommons start
+
+/usr/local/lib/xen/bin/init-dom0less
+
+brctl addbr xenbr0
+brctl addif xenbr0 eth0
+ifconfig eth0 up
+ifconfig xenbr0 up
+ifconfig xenbr0 192.168.0.1
+
+xl network-attach 1 type=vif
+${dom0_check}
+" > etc/local.d/xen.start
+chmod +x etc/local.d/xen.start
+echo "rc_verbose=yes" >> etc/rc.conf
+find . | cpio -H newc -o | gzip > ../binaries/dom0-rootfs.cpio.gz
+cd ..
+
+
+TFTP=/scratch/gitlab-runner/tftp
+START=`pwd`
+
+# ImageBuilder
+echo 'MEMORY_START="0"
+MEMORY_END="0xC0000000"
+
+DEVICE_TREE="mpsoc.dtb"
+XEN="xen"
+DOM0_KERNEL="Image"
+DOM0_RAMDISK="dom0-rootfs.cpio.gz"
+XEN_CMD="console=dtuart dom0_mem=1024M"
+
+NUM_DOMUS=1
+DOMU_KERNEL[0]="Image"
+DOMU_RAMDISK[0]="domU-rootfs.cpio.gz"
+DOMU_MEM[0]="1024"
+
+LOAD_CMD="tftpb"
+UBOOT_SOURCE="boot.source"
+UBOOT_SCRIPT="boot.scr"' > $TFTP/config
+
+cp -f binaries/xen $TFTP/
+cp -f binaries/Image $TFTP/
+cp -f binaries/dom0-rootfs.cpio.gz $TFTP/
+cp -f binaries/domU-rootfs.cpio.gz $TFTP/
+
+rm -rf imagebuilder
+git clone https://gitlab.com/ViryaOS/imagebuilder
+bash imagebuilder/scripts/uboot-script-gen -t tftp -d $TFTP/ -c $TFTP/config
+
+# restart the board
+cd /scratch/gitlab-runner
+bash zcu102.sh 2
+sleep 5
+bash zcu102.sh 1
+sleep 5
+cd $START
+
+# connect to serial
+set +e
+stty -F /dev/ttyUSB0 115200
+timeout -k 1 120 nohup sh -c "cat /dev/ttyUSB0 | tee smoke.serial"
+
+set -e
+(grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
+exit 0
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] automation: add Ubuntu container for Xilinx hardware tests
  2023-03-03 23:57 ` [PATCH v2 1/2] automation: add Ubuntu container for Xilinx hardware tests Stefano Stabellini
@ 2023-03-06  8:39   ` Michal Orzel
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Orzel @ 2023-03-06  8:39 UTC (permalink / raw
  To: Stefano Stabellini, xen-devel; +Cc: cardoe, Stefano Stabellini

Hi Stefano,

On 04/03/2023 00:57, Stefano Stabellini wrote:
> 
> 
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> This container is only run on the controller PC (x86) to trigger the
> test on a connected Xilinx ZCU102 physical board.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
>  .../build/ubuntu/xenial-xilinx.dockerfile     | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 automation/build/ubuntu/xenial-xilinx.dockerfile
> 
> diff --git a/automation/build/ubuntu/xenial-xilinx.dockerfile b/automation/build/ubuntu/xenial-xilinx.dockerfile
> new file mode 100644
> index 0000000000..7e4f5d6605
> --- /dev/null
> +++ b/automation/build/ubuntu/xenial-xilinx.dockerfile
> @@ -0,0 +1,25 @@
> +FROM ubuntu:16.04
> +LABEL maintainer.name="The Xen Project " \
> +      maintainer.email="xen-devel@lists.xenproject.org"
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV USER root
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +# build depends
This comment is a bit misleading given the usage of this container.
Something like "board bringup depends" would be better and it would make clear
why we have this container.

Apart from that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware
  2023-03-03 23:57 ` [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware Stefano Stabellini
@ 2023-03-06  9:21   ` Michal Orzel
  2023-03-06 23:15     ` Stefano Stabellini
  2023-03-06 10:25   ` Andrew Cooper
  1 sibling, 1 reply; 10+ messages in thread
From: Michal Orzel @ 2023-03-06  9:21 UTC (permalink / raw
  To: Stefano Stabellini, xen-devel; +Cc: cardoe, Stefano Stabellini

Hi Stefano,

On 04/03/2023 00:57, Stefano Stabellini wrote:
> 
> 
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> The test prepares dom0 and domU binaries and boot artifacts, similarly
> to the existing QEMU test. (TBD: share preparation steps with the
> regular QEMU tests.)
> 
> However, instead of running the test inside QEMU as usual, it copies
> the binaries to the tftp server root, triggers a Xilinx ZCU102 board
> reboot, and connects to the real serial of the board.
> 
> For now and due to its novelty, allow_failure on the Xilinx hardware
> test, and only run the job on protected branches with XILINX_JOBS set to
> true (the "master" and "staging" on gitlab.com/xen-project/xen qualify).
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> Changes in v2:
> - only execute the xilinx job on protected branches with XILINX_JOBS set
> to true
> ---
>  automation/gitlab-ci/test.yaml                |  22 ++++
>  .../scripts/xilinx-smoke-dom0less-arm64.sh    | 115 ++++++++++++++++++
>  2 files changed, 137 insertions(+)
>  create mode 100755 automation/scripts/xilinx-smoke-dom0less-arm64.sh
> 
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 1c5f400b68..8f94cad32c 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -85,6 +85,28 @@ build-each-commit-gcc:
>    tags:
>      - x86_64
> 
> +xilinx-smoke-dom0less-arm64-gcc:
> +  extends: .test-jobs-common
> +  variables:
> +    CONTAINER: ubuntu:xenial-xilinx
> +    LOGFILE: qemu-smoke-xilinx.log
> +  artifacts:
> +    paths:
> +      - smoke.serial
> +      - '*.log'
> +    when: always
> +  tags:
> +    - xilinx
> +  script:
> +    - ./automation/scripts/xilinx-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE}
> +  needs:
> +    - *arm64-test-needs
> +    - alpine-3.12-gcc-arm64
> +  allow_failure: true
> +  only:
> +    variables:
> +      - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
> +
Surely we will want to have more tests running on our HW board.
For that, let's introduce a template and a job separately right away:

.xilinx-arm64:
  extends: .test-jobs-common
  variables:
    CONTAINER: ubuntu:xenial-xilinx
    LOGFILE: qemu-smoke-xilinx.log
  artifacts:
    paths:
      - smoke.serial
      - '*.log'
    when: always
  allow_failure: true
  only:
    variables:
      - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
  tags:
    - xilinx


xilinx-smoke-dom0less-arm64-gcc:
  extends: .xilinx-arm64
  script:
    - ./automation/scripts/xilinx-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE}
  needs:
    - *arm64-test-needs
    - alpine-3.12-gcc-arm64


>  qemu-smoke-dom0-arm64-gcc:
>    extends: .qemu-arm64
>    script:
> diff --git a/automation/scripts/xilinx-smoke-dom0less-arm64.sh b/automation/scripts/xilinx-smoke-dom0less-arm64.sh
> new file mode 100755
> index 0000000000..180c8b5f1c
> --- /dev/null
> +++ b/automation/scripts/xilinx-smoke-dom0less-arm64.sh
> @@ -0,0 +1,115 @@
> +#!/bin/bash
> +
> +set -ex
> +
> +test_variant=$1
> +
> +if [ -z "${test_variant}" ]; then
> +    passed="ping test passed"
> +    domU_check="
> +until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do
> +    sleep 30
> +done
> +echo \"${passed}\"
> +"
> +fi
> +
> +# DomU
> +mkdir -p rootfs
> +cd rootfs
> +tar xzf ../binaries/initrd.tar.gz
> +mkdir proc
> +mkdir run
> +mkdir srv
> +mkdir sys
> +rm var/run
> +echo "#!/bin/sh
> +
> +${domU_check}
> +/bin/sh" > etc/local.d/xen.start
> +chmod +x etc/local.d/xen.start
> +echo "rc_verbose=yes" >> etc/rc.conf
> +find . | cpio -H newc -o | gzip > ../binaries/domU-rootfs.cpio.gz
> +cd ..
> +rm -rf rootfs
Why do we create a domU rootfs in a different manner compared to qemu one?
I think this could be done using busybox (it would have to be installed in container)
and it would make it easier to share qemu/xilinx script.

> +
> +# DOM0 rootfs
> +mkdir -p rootfs
> +cd rootfs
> +tar xzf ../binaries/initrd.tar.gz
> +mkdir proc
> +mkdir run
> +mkdir srv
> +mkdir sys
> +rm var/run
> +cp -ar ../binaries/dist/install/* .
> +
> +echo "#!/bin/bash
> +
> +export LD_LIBRARY_PATH=/usr/local/lib
> +bash /etc/init.d/xencommons start
> +
> +/usr/local/lib/xen/bin/init-dom0less
> +
> +brctl addbr xenbr0
> +brctl addif xenbr0 eth0
> +ifconfig eth0 up
> +ifconfig xenbr0 up
> +ifconfig xenbr0 192.168.0.1
> +
> +xl network-attach 1 type=vif
> +${dom0_check}
> +" > etc/local.d/xen.start
> +chmod +x etc/local.d/xen.start
> +echo "rc_verbose=yes" >> etc/rc.conf
> +find . | cpio -H newc -o | gzip > ../binaries/dom0-rootfs.cpio.gz
> +cd ..
> +
> +
> +TFTP=/scratch/gitlab-runner/tftp
> +START=`pwd`
> +
> +# ImageBuilder
> +echo 'MEMORY_START="0"
> +MEMORY_END="0xC0000000"
This is incorrect for zcu102. It should be the end address of a first memory bank
which is 0x7ff00000.

> +
> +DEVICE_TREE="mpsoc.dtb"
Can we do something to expose this dtb so that everyone with no access to a runner can see it?
It will become necessary when we start introducing some passthrough test jobs.
Also, how about naming this: mpsoc_smmu.dtb to make it clear that we are running with IOMMU enabled?
In the future we might want to test both configurations, especially for static partitioning use cases.

> +XEN="xen"
> +DOM0_KERNEL="Image"
> +DOM0_RAMDISK="dom0-rootfs.cpio.gz"
> +XEN_CMD="console=dtuart dom0_mem=1024M"
With just "console=dtuart" we are relying on a presence of "stdout-path" under /chosen which is optional.
I would suggest: "console=dtuart dtuart=serial0"

> +
> +NUM_DOMUS=1
> +DOMU_KERNEL[0]="Image"
> +DOMU_RAMDISK[0]="domU-rootfs.cpio.gz"
> +DOMU_MEM[0]="1024"
> +
> +LOAD_CMD="tftpb"
> +UBOOT_SOURCE="boot.source"
> +UBOOT_SCRIPT="boot.scr"' > $TFTP/config
> +
> +cp -f binaries/xen $TFTP/
> +cp -f binaries/Image $TFTP/
> +cp -f binaries/dom0-rootfs.cpio.gz $TFTP/
> +cp -f binaries/domU-rootfs.cpio.gz $TFTP/
> +
> +rm -rf imagebuilder
> +git clone https://gitlab.com/ViryaOS/imagebuilder
> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d $TFTP/ -c $TFTP/config
> +
> +# restart the board
> +cd /scratch/gitlab-runner
> +bash zcu102.sh 2
> +sleep 5
> +bash zcu102.sh 1
> +sleep 5
> +cd $START
> +
> +# connect to serial
> +set +e
> +stty -F /dev/ttyUSB0 115200
> +timeout -k 1 120 nohup sh -c "cat /dev/ttyUSB0 | tee smoke.serial"
> +
> +set -e
> +(grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
> +exit 0
> --
> 2.25.1
> 

Great job, this is awesome.

~Michal


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware
  2023-03-03 23:57 ` [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware Stefano Stabellini
  2023-03-06  9:21   ` Michal Orzel
@ 2023-03-06 10:25   ` Andrew Cooper
  2023-03-06 23:02     ` Stefano Stabellini
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2023-03-06 10:25 UTC (permalink / raw
  To: Stefano Stabellini, xen-devel; +Cc: cardoe, michal.orzel, Stefano Stabellini

On 03/03/2023 11:57 pm, Stefano Stabellini wrote:
> +  only:
> +    variables:
> +      - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"

We don't want to protect every branch of a tree that only a select
number of people can push to, nor (for this, or others configured with
the runner), want to impose branching conventions on them.

In all anticipated cases, those able to push would also be able to
reconfigure the protected-ness of branches, so this doesn't gain us any
security I don't think, but it certainly puts more hoops in the way to
be jumped through.

~Andrew


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware
  2023-03-06 10:25   ` Andrew Cooper
@ 2023-03-06 23:02     ` Stefano Stabellini
  2023-03-07 15:21       ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 10+ messages in thread
From: Stefano Stabellini @ 2023-03-06 23:02 UTC (permalink / raw
  To: Andrew Cooper
  Cc: Stefano Stabellini, xen-devel, cardoe, michal.orzel,
	Stefano Stabellini

On Mon, 6 Mar 2023, Andrew Cooper wrote:
> On 03/03/2023 11:57 pm, Stefano Stabellini wrote:
> > +  only:
> > +    variables:
> > +      - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
> 
> We don't want to protect every branch of a tree that only a select
> number of people can push to,

Actually this is useful, more on this below


> nor (for this, or others configured with
> the runner), want to impose branching conventions on them.
> 
> In all anticipated cases, those able to push would also be able to
> reconfigure the protected-ness of branches, so this doesn't gain us any
> security I don't think, but it certainly puts more hoops in the way to
> be jumped through.

It is true that it adds a small inconvenience to the user, but I think
the benefits outweigh the inconvenience at the moment (that could change
though.)

With this, I can register the gitlab runner with a specific gitlab
project (for instance
https://gitlab.com/xen-project/people/sstabellini/xen) then I can mark
all branches as "protected" and select very specific access permissions,
e.g. I can give individual access to Julien, Bertrand, Michal, anyone,
to specific branches, which is great to allow them to run individual
pre-commit tests permanently or temporarily.

I couldn't find another way to do it at the moment, as non-protected
branches don't come with detailed access permissions. But it is possible
that as we setup a new sub-group under https://gitlab.com/xen-project
for people with access to the runner, then we might be able to remove
this restriction because it becomes unnecessary. We can remove the
protected check at that point.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware
  2023-03-06  9:21   ` Michal Orzel
@ 2023-03-06 23:15     ` Stefano Stabellini
  0 siblings, 0 replies; 10+ messages in thread
From: Stefano Stabellini @ 2023-03-06 23:15 UTC (permalink / raw
  To: Michal Orzel; +Cc: Stefano Stabellini, xen-devel, cardoe, Stefano Stabellini

On Mon, 6 Mar 2023, Michal Orzel wrote:
> Hi Stefano,
> 
> On 04/03/2023 00:57, Stefano Stabellini wrote:
> > 
> > 
> > From: Stefano Stabellini <stefano.stabellini@amd.com>
> > 
> > The test prepares dom0 and domU binaries and boot artifacts, similarly
> > to the existing QEMU test. (TBD: share preparation steps with the
> > regular QEMU tests.)
> > 
> > However, instead of running the test inside QEMU as usual, it copies
> > the binaries to the tftp server root, triggers a Xilinx ZCU102 board
> > reboot, and connects to the real serial of the board.
> > 
> > For now and due to its novelty, allow_failure on the Xilinx hardware
> > test, and only run the job on protected branches with XILINX_JOBS set to
> > true (the "master" and "staging" on gitlab.com/xen-project/xen qualify).
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > ---
> > Changes in v2:
> > - only execute the xilinx job on protected branches with XILINX_JOBS set
> > to true
> > ---
> >  automation/gitlab-ci/test.yaml                |  22 ++++
> >  .../scripts/xilinx-smoke-dom0less-arm64.sh    | 115 ++++++++++++++++++
> >  2 files changed, 137 insertions(+)
> >  create mode 100755 automation/scripts/xilinx-smoke-dom0less-arm64.sh
> > 
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index 1c5f400b68..8f94cad32c 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -85,6 +85,28 @@ build-each-commit-gcc:
> >    tags:
> >      - x86_64
> > 
> > +xilinx-smoke-dom0less-arm64-gcc:
> > +  extends: .test-jobs-common
> > +  variables:
> > +    CONTAINER: ubuntu:xenial-xilinx
> > +    LOGFILE: qemu-smoke-xilinx.log
> > +  artifacts:
> > +    paths:
> > +      - smoke.serial
> > +      - '*.log'
> > +    when: always
> > +  tags:
> > +    - xilinx
> > +  script:
> > +    - ./automation/scripts/xilinx-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE}
> > +  needs:
> > +    - *arm64-test-needs
> > +    - alpine-3.12-gcc-arm64
> > +  allow_failure: true
> > +  only:
> > +    variables:
> > +      - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
> > +
> Surely we will want to have more tests running on our HW board.
> For that, let's introduce a template and a job separately right away:
> 
> .xilinx-arm64:
>   extends: .test-jobs-common
>   variables:
>     CONTAINER: ubuntu:xenial-xilinx
>     LOGFILE: qemu-smoke-xilinx.log
>   artifacts:
>     paths:
>       - smoke.serial
>       - '*.log'
>     when: always
>   allow_failure: true
>   only:
>     variables:
>       - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
>   tags:
>     - xilinx
> 
> 
> xilinx-smoke-dom0less-arm64-gcc:
>   extends: .xilinx-arm64
>   script:
>     - ./automation/scripts/xilinx-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE}
>   needs:
>     - *arm64-test-needs
>     - alpine-3.12-gcc-arm64

Makes sense


> >  qemu-smoke-dom0-arm64-gcc:
> >    extends: .qemu-arm64
> >    script:
> > diff --git a/automation/scripts/xilinx-smoke-dom0less-arm64.sh b/automation/scripts/xilinx-smoke-dom0less-arm64.sh
> > new file mode 100755
> > index 0000000000..180c8b5f1c
> > --- /dev/null
> > +++ b/automation/scripts/xilinx-smoke-dom0less-arm64.sh
> > @@ -0,0 +1,115 @@
> > +#!/bin/bash
> > +
> > +set -ex
> > +
> > +test_variant=$1
> > +
> > +if [ -z "${test_variant}" ]; then
> > +    passed="ping test passed"
> > +    domU_check="
> > +until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do
> > +    sleep 30
> > +done
> > +echo \"${passed}\"
> > +"
> > +fi
> > +
> > +# DomU
> > +mkdir -p rootfs
> > +cd rootfs
> > +tar xzf ../binaries/initrd.tar.gz
> > +mkdir proc
> > +mkdir run
> > +mkdir srv
> > +mkdir sys
> > +rm var/run
> > +echo "#!/bin/sh
> > +
> > +${domU_check}
> > +/bin/sh" > etc/local.d/xen.start
> > +chmod +x etc/local.d/xen.start
> > +echo "rc_verbose=yes" >> etc/rc.conf
> > +find . | cpio -H newc -o | gzip > ../binaries/domU-rootfs.cpio.gz
> > +cd ..
> > +rm -rf rootfs
> Why do we create a domU rootfs in a different manner compared to qemu one?
> I think this could be done using busybox (it would have to be installed in container)
> and it would make it easier to share qemu/xilinx script.

That's because we cannot reuse the busybox steps as-is: this container
is running on an x86 machine while busybox is an arm64 binary. I think
that problem could be solved somehow, but I didn't want to solve it
right now.


> > +
> > +# DOM0 rootfs
> > +mkdir -p rootfs
> > +cd rootfs
> > +tar xzf ../binaries/initrd.tar.gz
> > +mkdir proc
> > +mkdir run
> > +mkdir srv
> > +mkdir sys
> > +rm var/run
> > +cp -ar ../binaries/dist/install/* .
> > +
> > +echo "#!/bin/bash
> > +
> > +export LD_LIBRARY_PATH=/usr/local/lib
> > +bash /etc/init.d/xencommons start
> > +
> > +/usr/local/lib/xen/bin/init-dom0less
> > +
> > +brctl addbr xenbr0
> > +brctl addif xenbr0 eth0
> > +ifconfig eth0 up
> > +ifconfig xenbr0 up
> > +ifconfig xenbr0 192.168.0.1
> > +
> > +xl network-attach 1 type=vif
> > +${dom0_check}
> > +" > etc/local.d/xen.start
> > +chmod +x etc/local.d/xen.start
> > +echo "rc_verbose=yes" >> etc/rc.conf
> > +find . | cpio -H newc -o | gzip > ../binaries/dom0-rootfs.cpio.gz
> > +cd ..
> > +
> > +
> > +TFTP=/scratch/gitlab-runner/tftp
> > +START=`pwd`
> > +
> > +# ImageBuilder
> > +echo 'MEMORY_START="0"
> > +MEMORY_END="0xC0000000"
> This is incorrect for zcu102. It should be the end address of a first memory bank
> which is 0x7ff00000.

Thanks!


> > +
> > +DEVICE_TREE="mpsoc.dtb"
> Can we do something to expose this dtb so that everyone with no access to a runner can see it?
> It will become necessary when we start introducing some passthrough test jobs.
> Also, how about naming this: mpsoc_smmu.dtb to make it clear that we are running with IOMMU enabled?
> In the future we might want to test both configurations, especially for static partitioning use cases.

Yes, the dtb is public under
arch/arm64/boot/dts/xilinx/zynqmp-zcu102-rev1.1.dts in Linux.

We could rebuild it from the upstream version if we wanted but I thought
for now is more convenient to just use it as is. But I can do two simple
things to make things clearer and easier to maintain:
- export mpsoc.dtb among the artifacts, so that anyone can download
- rename it to mpsoc_smmu.dtb as you suggested

 
> > +XEN="xen"
> > +DOM0_KERNEL="Image"
> > +DOM0_RAMDISK="dom0-rootfs.cpio.gz"
> > +XEN_CMD="console=dtuart dom0_mem=1024M"
> With just "console=dtuart" we are relying on a presence of "stdout-path" under /chosen which is optional.
> I would suggest: "console=dtuart dtuart=serial0"

OK


> > +
> > +NUM_DOMUS=1
> > +DOMU_KERNEL[0]="Image"
> > +DOMU_RAMDISK[0]="domU-rootfs.cpio.gz"
> > +DOMU_MEM[0]="1024"
> > +
> > +LOAD_CMD="tftpb"
> > +UBOOT_SOURCE="boot.source"
> > +UBOOT_SCRIPT="boot.scr"' > $TFTP/config
> > +
> > +cp -f binaries/xen $TFTP/
> > +cp -f binaries/Image $TFTP/
> > +cp -f binaries/dom0-rootfs.cpio.gz $TFTP/
> > +cp -f binaries/domU-rootfs.cpio.gz $TFTP/
> > +
> > +rm -rf imagebuilder
> > +git clone https://gitlab.com/ViryaOS/imagebuilder
> > +bash imagebuilder/scripts/uboot-script-gen -t tftp -d $TFTP/ -c $TFTP/config
> > +
> > +# restart the board
> > +cd /scratch/gitlab-runner
> > +bash zcu102.sh 2
> > +sleep 5
> > +bash zcu102.sh 1
> > +sleep 5
> > +cd $START
> > +
> > +# connect to serial
> > +set +e
> > +stty -F /dev/ttyUSB0 115200
> > +timeout -k 1 120 nohup sh -c "cat /dev/ttyUSB0 | tee smoke.serial"
> > +
> > +set -e
> > +(grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
> > +exit 0
> > --
> > 2.25.1
> > 
> 
> Great job, this is awesome.

Thank you! :-)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware
  2023-03-06 23:02     ` Stefano Stabellini
@ 2023-03-07 15:21       ` Marek Marczykowski-Górecki
  2023-03-07 20:21         ` Stefano Stabellini
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Marczykowski-Górecki @ 2023-03-07 15:21 UTC (permalink / raw
  To: Stefano Stabellini
  Cc: Andrew Cooper, xen-devel, cardoe, michal.orzel,
	Stefano Stabellini

[-- Attachment #1: Type: text/plain, Size: 2243 bytes --]

On Mon, Mar 06, 2023 at 03:02:51PM -0800, Stefano Stabellini wrote:
> On Mon, 6 Mar 2023, Andrew Cooper wrote:
> > On 03/03/2023 11:57 pm, Stefano Stabellini wrote:
> > > +  only:
> > > +    variables:
> > > +      - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
> > 
> > We don't want to protect every branch of a tree that only a select
> > number of people can push to,
> 
> Actually this is useful, more on this below
> 
> 
> > nor (for this, or others configured with
> > the runner), want to impose branching conventions on them.
> > 
> > In all anticipated cases, those able to push would also be able to
> > reconfigure the protected-ness of branches, so this doesn't gain us any
> > security I don't think, but it certainly puts more hoops in the way to
> > be jumped through.
> 
> It is true that it adds a small inconvenience to the user, but I think
> the benefits outweigh the inconvenience at the moment (that could change
> though.)
> 
> With this, I can register the gitlab runner with a specific gitlab
> project (for instance
> https://gitlab.com/xen-project/people/sstabellini/xen) then I can mark
> all branches as "protected" and select very specific access permissions,
> e.g. I can give individual access to Julien, Bertrand, Michal, anyone,
> to specific branches, which is great to allow them to run individual
> pre-commit tests permanently or temporarily.
> 
> I couldn't find another way to do it at the moment, as non-protected
> branches don't come with detailed access permissions. But it is possible
> that as we setup a new sub-group under https://gitlab.com/xen-project
> for people with access to the runner, then we might be able to remove
> this restriction because it becomes unnecessary. We can remove the
> protected check at that point.

You can configure runner to run only jobs from protected branches. This
way it actually prevent running jobs from non-protected branches. Just a
condition in .gitlab-ci.yml can be simply removed by anybody who wants
to abuse your runner (and have push access to non-protected branch -
which may or may not include all of patchew).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware
  2023-03-07 15:21       ` Marek Marczykowski-Górecki
@ 2023-03-07 20:21         ` Stefano Stabellini
  0 siblings, 0 replies; 10+ messages in thread
From: Stefano Stabellini @ 2023-03-07 20:21 UTC (permalink / raw
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Andrew Cooper, xen-devel, cardoe,
	michal.orzel, Stefano Stabellini

[-- Attachment #1: Type: text/plain, Size: 2778 bytes --]

On Tue, 7 Mar 2023, Marek Marczykowski-Górecki wrote:
> On Mon, Mar 06, 2023 at 03:02:51PM -0800, Stefano Stabellini wrote:
> > On Mon, 6 Mar 2023, Andrew Cooper wrote:
> > > On 03/03/2023 11:57 pm, Stefano Stabellini wrote:
> > > > +  only:
> > > > +    variables:
> > > > +      - $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
> > > 
> > > We don't want to protect every branch of a tree that only a select
> > > number of people can push to,
> > 
> > Actually this is useful, more on this below
> > 
> > 
> > > nor (for this, or others configured with
> > > the runner), want to impose branching conventions on them.
> > > 
> > > In all anticipated cases, those able to push would also be able to
> > > reconfigure the protected-ness of branches, so this doesn't gain us any
> > > security I don't think, but it certainly puts more hoops in the way to
> > > be jumped through.
> > 
> > It is true that it adds a small inconvenience to the user, but I think
> > the benefits outweigh the inconvenience at the moment (that could change
> > though.)
> > 
> > With this, I can register the gitlab runner with a specific gitlab
> > project (for instance
> > https://gitlab.com/xen-project/people/sstabellini/xen) then I can mark
> > all branches as "protected" and select very specific access permissions,
> > e.g. I can give individual access to Julien, Bertrand, Michal, anyone,
> > to specific branches, which is great to allow them to run individual
> > pre-commit tests permanently or temporarily.
> > 
> > I couldn't find another way to do it at the moment, as non-protected
> > branches don't come with detailed access permissions. But it is possible
> > that as we setup a new sub-group under https://gitlab.com/xen-project
> > for people with access to the runner, then we might be able to remove
> > this restriction because it becomes unnecessary. We can remove the
> > protected check at that point.
> 
> You can configure runner to run only jobs from protected branches. This
> way it actually prevent running jobs from non-protected branches. Just a
> condition in .gitlab-ci.yml can be simply removed by anybody who wants
> to abuse your runner (and have push access to non-protected branch -
> which may or may not include all of patchew).

Yes, I did configure the runner only to execute protected jobs. The
$CI_COMMIT_REF_PROTECTED check in automation/gitlab-ci/test.yaml is
needed so that the xilinx job won't be created on pipelines for
non-protected branches where the runner won't run, hence the job has no
chance of completing successfully.

If I don't add the $CI_COMMIT_REF_PROTECTED check, the job will be
created in the pipeline but it will remain stuck as "paused" waiting for
the runner to become available (but the runner never will.)

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-03-07 20:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-03 23:57 [PATCH v2 0/2] automation: introduce a Xilinx hardware test Stefano Stabellini
2023-03-03 23:57 ` [PATCH v2 1/2] automation: add Ubuntu container for Xilinx hardware tests Stefano Stabellini
2023-03-06  8:39   ` Michal Orzel
2023-03-03 23:57 ` [PATCH v2 2/2] automation: introduce a dom0less test run on Xilinx hardware Stefano Stabellini
2023-03-06  9:21   ` Michal Orzel
2023-03-06 23:15     ` Stefano Stabellini
2023-03-06 10:25   ` Andrew Cooper
2023-03-06 23:02     ` Stefano Stabellini
2023-03-07 15:21       ` Marek Marczykowski-Górecki
2023-03-07 20:21         ` Stefano Stabellini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).