Re: [PATCH v2] tools/libs/evtchn: replace assert()s in stubdom with proper locking

[Juergen Gross]

On 08.12.23 08:34, Jan Beulich wrote:

On 07.12.2023 07:25, Juergen Gross wrote:

In tools/libs/evtchn/minios.c there are assert()s for the current
thread being the main thread when binding an event channel.

As Mini-OS is supporting multiple threads, there is no real reason
why the binding shouldn't be allowed to happen in any other thread.

Drop the assert()s and replace them with proper locking of the
port_list.

Signed-off-by: Juergen Gross 


Is this a change I should pick up for backport?


This patch isn't really fixing a bug, but more enhancing functionality.

I don't think a backport is needed.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2] tools/libs/evtchn: replace assert()s in stubdom with proper locking

[Jan Beulich]

On 07.12.2023 07:25, Juergen Gross wrote:
> In tools/libs/evtchn/minios.c there are assert()s for the current
> thread being the main thread when binding an event channel.
> 
> As Mini-OS is supporting multiple threads, there is no real reason
> why the binding shouldn't be allowed to happen in any other thread.
> 
> Drop the assert()s and replace them with proper locking of the
> port_list.
> 
> Signed-off-by: Juergen Gross 

Is this a change I should pick up for backport?

Jan

Re: [PATCH v2] docs/misra/rules.rst: add more rules

[Jan Beulich]

On 08.12.2023 01:09, Stefano Stabellini wrote:
> Add the rules accepted in the last three MISRA C working group meetings.
> 
> Signed-off-by: Stefano Stabellini 

Acked-by: Jan Beulich 

> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -462,6 +462,13 @@ maintainers if you want to suggest a change.
>  
> while(0) and while(1) and alike are allowed.
>  
> +   * - `Rule 16.3 
> `_
> + - Required
> + - An unconditional break statement shall terminate every
> +   switch-clause
> + - In addition to break, also other flow control statements such as
> +   continue, return, goto are allowed.

To eliminate any room for doubt, maybe add "unconditional" also again here?

Jan

Re: [PATCH] docs/misra/rules.rst: add more rules

[Jan Beulich]

On 08.12.2023 01:08, Stefano Stabellini wrote:
> On Thu, 7 Dec 2023, Jan Beulich wrote:
>> On 07.12.2023 03:42, Stefano Stabellini wrote:
>>> On Wed, 6 Dec 2023, Jan Beulich wrote:
 On 06.12.2023 04:02, Stefano Stabellini wrote:
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -462,11 +462,23 @@ maintainers if you want to suggest a change.
>  
> while(0) and while(1) and alike are allowed.
>  
> +   * - `Rule 16.3 
> `_
> + - Required
> + - An unconditional break statement shall terminate every
> +   switch-clause
> + - In addition to break, also other flow control statements such as
> +   continue, return, goto are allowed.
> +
> * - `Rule 16.7 
> `_
>   - Required
>   - A switch-expression shall not have essentially Boolean type
>   -
>  
> +   * - `Rule 17.1 
> `_
> + - Required
> + - The features of  shall not be used
> + -

 Did we really accept this without any constraint (warranting mentioning
 here)?
>>>
>>> We agreed that in certain situations stdarg.h is OK to use and in those
>>> cases we would add a deviation. Would you like me to add something to
>>> that effect here? I could do that but it would sound a bit vague.  Also
>>> if we want to specify a project-wide deviation it would be better
>>> documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
>>> a note.
>>
>> I can see your point, and I don't have a good suggestion on possible text.
>> Still I wouldn't feel well ack-ing this in its present shape.
> 
> What about:
> 
>  - It is understood that in some limited circumstances  is
>appropriate to use, such as the implementation of printk. Those
>cases will be dealt with using deviations as usual, see
>docs/misra/deviations.rst and
>docs/misra/documenting-violations.rst.

Looks okay. Would also look okay if it was just the first sentence.

Jan

Re: [RFC KERNEL PATCH v2 2/3] xen/pvh: Unmask irq for passthrough device in PVH dom0

[Chen, Jiqian]

Thank Stefano and Juergen, I will use this approach in next version.

On 2023/12/7 14:43, Juergen Gross wrote:
> On 07.12.23 03:18, Stefano Stabellini wrote:
>> On Tue, 5 Dec 2023, Chen, Jiqian wrote:
>>> When PVH dom0 enable a device, it will get trigger and polarity from ACPI 
>>> (see acpi_pci_irq_enable)
>>> I have a version of patch which tried that way, see below:
>>
>> This approach looks much better. I think this patch is OKish. Juergen,
>> what do you think?
> 
> The approach seems to be fine.
> 
> 
> Juergen
> 
>>
>>
>>> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
>>> index ada3868c02c2..43e1bda9f946 100644
>>> --- a/arch/x86/xen/enlighten_pvh.c
>>> +++ b/arch/x86/xen/enlighten_pvh.c
>>> @@ -1,6 +1,7 @@
>>>   // SPDX-License-Identifier: GPL-2.0
>>>   #include 
>>>   #include 
>>> +#include 
>>>
>>>   #include 
>>>
>>> @@ -25,6 +26,127 @@
>>>   bool __ro_after_init xen_pvh;
>>>   EXPORT_SYMBOL_GPL(xen_pvh);
>>>
>>> +typedef struct gsi_info {
>>> +   int gsi;
>>> +   int trigger;
>>> +   int polarity;
>>> +   int pirq;
>>> +} gsi_info_t;
>>> +
>>> +struct acpi_prt_entry {
>>> +   struct acpi_pci_id  id;
>>> +   u8  pin;
>>> +   acpi_handle link;
>>> +   u32 index;  /* GSI, or link _CRS index 
>>> */
>>> +};
>>> +
>>> +static int xen_pvh_get_gsi_info(struct pci_dev *dev,
>>> +   gsi_info_t 
>>> *gsi_info)
>>> +{
>>> +   int gsi;
>>> +   u8 pin = 0;
>>> +   struct acpi_prt_entry *entry;
>>> +   int trigger = ACPI_LEVEL_SENSITIVE;
>>> +   int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC ?
>>> + ACPI_ACTIVE_HIGH : ACPI_ACTIVE_LOW;
>>> +
>>> +   if (dev)
>>> +   pin = dev->pin;
>>> +   if (!pin) {
>>> +   xen_raw_printk("No interrupt pin configured\n");
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   entry = acpi_pci_irq_lookup(dev, pin);
>>> +   if (entry) {
>>> +   if (entry->link)
>>> +   gsi = acpi_pci_link_allocate_irq(entry->link,
>>> +    entry->index,
>>> +    , 
>>> ,
>>> +    NULL);
>>> +   else
>>> +   gsi = entry->index;
>>> +   } else
>>> +   return -EINVAL;
>>> +
>>> +   gsi_info->gsi = gsi;
>>> +   gsi_info->trigger = trigger;
>>> +   gsi_info->polarity = polarity;
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +static int xen_pvh_map_pirq(gsi_info_t *gsi_info)
>>> +{
>>> +   struct physdev_map_pirq map_irq;
>>> +   int ret;
>>> +
>>> +   map_irq.domid = DOMID_SELF;
>>> +   map_irq.type = MAP_PIRQ_TYPE_GSI;
>>> +   map_irq.index = gsi_info->gsi;
>>> +   map_irq.pirq = gsi_info->gsi;
>>> +
>>> +   ret = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, _irq);
>>> +   gsi_info->pirq = map_irq.pirq;
>>> +
>>> +   return ret;
>>> +}
>>> +
>>> +static int xen_pvh_unmap_pirq(gsi_info_t *gsi_info)
>>> +{
>>> +   struct physdev_unmap_pirq unmap_irq;
>>> +
>>> +   unmap_irq.domid = DOMID_SELF;
>>> +   unmap_irq.pirq = gsi_info->pirq;
>>> +
>>> +   return HYPERVISOR_physdev_op(PHYSDEVOP_unmap_pirq, _irq);
>>> +}
>>> +
>>> +static int xen_pvh_setup_gsi(gsi_info_t *gsi_info)
>>> +{
>>> +   struct physdev_setup_gsi setup_gsi;
>>> +
>>> +   setup_gsi.gsi = gsi_info->gsi;
>>> +   setup_gsi.triggering = (gsi_info->trigger == ACPI_EDGE_SENSITIVE ? 
>>> 0 : 1);
>>> +   setup_gsi.polarity = (gsi_info->polarity == ACPI_ACTIVE_HIGH ? 0 : 
>>> 1);
>>> +
>>> +   return HYPERVISOR_physdev_op(PHYSDEVOP_setup_gsi, _gsi);
>>> +}
>>> +
>>> +int xen_pvh_passthrough_gsi(struct pci_dev *dev)
>>> +{
>>> +   int ret;
>>> +   gsi_info_t gsi_info;
>>> +
>>> +   if (!dev) {
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   ret = xen_pvh_get_gsi_info(dev, _info);
>>> +   if (ret) {
>>> +   xen_raw_printk("Fail to get gsi info!\n");
>>> +   return ret;
>>> +   }
>>> +
>>> +   ret = xen_pvh_map_pirq(_info);
>>> +   if (ret) {
>>> +   xen_raw_printk("Fail to map pirq for gsi (%d)!\n", 
>>> gsi_info.gsi);
>>> +   return ret;
>>> +   }
>>> +
>>> +   ret = xen_pvh_setup_gsi(_info);
>>> +   if (ret == -EEXIST) {
>>> +   ret = 0;
>>> +   xen_raw_printk("Already setup the GSI :%u\n", gsi_info.gsi);
>>> +   } else if (ret) {
>>> +   xen_raw_printk("Fail to setup gsi (%d)!\n", gsi_info.gsi);
>>> +   xen_pvh_unmap_pirq(_info);
>>> +   }
>>> +
>>> +   return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(xen_pvh_passthrough_gsi);
>>> +
>>>   void __init xen_pvh_init(struct boot_params 

[PATCH v2 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs

[Henry Wang]

Add a job in the build stage to export the TF-A, U-Boot and the
device tree for the FVP platform from the test artifact container.

Add a FVP smoke test job in the test stage to do the same test as
the `qemu-smoke-dom0-arm64-gcc` job.

Signed-off-by: Henry Wang 
Reviewed-by: Stefano Stabellini 
---
v2:
- Add Stefano's Reviewed-by tag.

Although it does not affect the functionality, I am still quite
confused about why the logs displayed by GitLab UI, for
example [1], is much less than the actual output (saved in the
artifacts, see [2]). Had a discussion with Michal on Matrix
and we agree that the log in gitlab UI is usually capped.

[1] https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5690876361
[2] 
https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5690876361/artifacts/file/smoke.serial
---
 automation/gitlab-ci/build.yaml | 17 +
 automation/gitlab-ci/test.yaml  | 22 ++
 2 files changed, 39 insertions(+)

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 32af30cced..89d2f01302 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -344,6 +344,23 @@ kernel-6.1.19-export:
   tags:
 - x86_64
 
+armfvp-uboot-tfa-2023.10-2.9.0-arm64-export:
+  extends: .test-jobs-artifact-common
+  image: 
registry.gitlab.com/xen-project/xen/tests-artifacts/armfvp-uboot-tfa:2023.10-2.9.0-arm64v8
+  script:
+- |
+   mkdir binaries && \
+   cp /bl1.bin binaries/bl1.bin && \
+   cp /fip.bin binaries/fip.bin && \
+   cp /fvp-base-gicv3-psci-1t.dtb binaries/fvp-base-gicv3-psci-1t.dtb
+  artifacts:
+paths:
+  - binaries/bl1.bin
+  - binaries/fip.bin
+  - binaries/fvp-base-gicv3-psci-1t.dtb
+  tags:
+- arm64
+
 # Jobs below this line
 
 # Build jobs needed for tests
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 6aabdb9d15..47e00d0a0b 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -96,6 +96,19 @@
   tags:
 - xilinx
 
+.fvp-arm64:
+  extends: .test-jobs-common
+  variables:
+CONTAINER: debian:bookworm-arm64v8-fvp
+LOGFILE: fvp-smoke-arm64.log
+  artifacts:
+paths:
+  - smoke.serial
+  - '*.log'
+when: always
+  tags:
+- arm64
+
 .adl-x86-64:
   extends: .test-jobs-common
   variables:
@@ -459,3 +472,12 @@ qemu-smoke-ppc64le-powernv9-gcc:
   needs:
 - qemu-system-ppc64-8.1.0-ppc64-export
 - debian-bullseye-gcc-ppc64le-debug
+
+fvp-smoke-dom0-arm64-gcc-debug:
+  extends: .fvp-arm64
+  script:
+- ./automation/scripts/fvp-base-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE}
+  needs:
+- *arm64-test-needs
+- armfvp-uboot-tfa-2023.10-2.9.0-arm64-export
+- alpine-3.18-gcc-debug-arm64
-- 
2.25.1

[PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs

[Henry Wang]

Fixed Virtual Platforms (FVPs) are complete simulations of an Arm
system, including processor, memory and peripherals. These are set
out in a "programmer's view", which gives programmers a comprehensive
model on which to build and test software. FVP can be configured to
different setups by its cmdline parameters, and hence having the FVP
in CI will provide us with the flexibility to test Arm features and
setups that we find difficult to use real hardware or emulators.

This commit adds a Dockerfile for the new arm64v8 container with
FVP installed, based on the debian bookworm-arm64v8 image. This
container will be used to run the FVP test jobs. Compared to the
debian bookworm-arm64v8 image, the packages in the newly added FVP
container does not contain the `u-boot-qemu`, and adds the `expect`
to run expect scripts introduced by following commits, `telnet` to
connect to FVP, and `tftpd-hpa` to provide the TFTP service for
the FVP.

Signed-off-by: Henry Wang 
Reviewed-by: Stefano Stabellini 
---
v2:
- Add Stefano's Reviewed-by tag.
---
 .../debian/bookworm-arm64v8-fvp.dockerfile| 64 +++
 1 file changed, 64 insertions(+)
 create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile

diff --git a/automation/build/debian/bookworm-arm64v8-fvp.dockerfile 
b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
new file mode 100644
index 00..3b87dc5a5b
--- /dev/null
+++ b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
@@ -0,0 +1,64 @@
+FROM --platform=linux/arm64/v8 debian:bookworm
+LABEL maintainer.name="The Xen Project" \
+  maintainer.email="xen-devel@lists.xenproject.org"
+
+ARG FVP_BASE_VERSION="11.23_9_Linux64_armv8l"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+# build depends
+RUN apt-get update && \
+apt-get --quiet --yes install \
+build-essential \
+zlib1g-dev \
+libncurses5-dev \
+libssl-dev \
+python3-dev \
+python3-setuptools \
+xorg-dev \
+uuid-dev \
+libyajl-dev \
+libaio-dev \
+libglib2.0-dev \
+clang \
+libpixman-1-dev \
+pkg-config \
+flex \
+bison \
+acpica-tools \
+libfdt-dev \
+bin86 \
+bcc \
+liblzma-dev \
+libnl-3-dev \
+ocaml-nox \
+libfindlib-ocaml-dev \
+markdown \
+transfig \
+pandoc \
+checkpolicy \
+wget \
+git \
+nasm \
+# for test phase, fvp-smoke-* jobs
+u-boot-tools \
+expect \
+device-tree-compiler \
+curl \
+cpio \
+busybox-static \
+telnet \
+tftpd-hpa \
+&& \
+apt-get autoremove -y && \
+apt-get clean && \
+rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
+
+RUN wget 
https://developer.arm.com/-/media/Files/downloads/ecosystem-models/FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz
 && \
+mkdir -p /FVP/FVP_Base_RevC-2xAEMvA && \
+tar -xvzf FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz -C 
/FVP/FVP_Base_RevC-2xAEMvA && \
+rm FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz
-- 
2.25.1

[PATCH v2 3/5] automation: Add the expect script with test case for FVP

[Henry Wang]

To interact with the FVP (for example entering the U-Boot shell
and transferring the files by TFTP), we need to connect the
corresponding port by the telnet first. Use an expect script to
simplify the automation of the whole "interacting with FVP" stuff.

The expect script will firstly detect the IP of the host, then
connect to the telnet port of the FVP, set the `serverip` and `ipaddr`
for the TFTP service in the U-Boot shell, and finally boot Xen from
U-Boot and wait for the expected results by Xen, Dom0 and DomU.

Signed-off-by: Henry Wang 
---
v2:
- No change.
---
 .../expect/fvp-base-smoke-dom0-arm64.exp  | 73 +++
 1 file changed, 73 insertions(+)
 create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp

diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp 
b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
new file mode 100755
index 00..25d9a5f81c
--- /dev/null
+++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
@@ -0,0 +1,73 @@
+#!/usr/bin/expect
+
+set timeout 2000
+
+# Command to use to run must be given as first argument
+# if options are required, quotes must be used:
+# xxx.exp "cmd opt1 opt2"
+set runcmd [lindex $argv 0]
+
+# Maximum number of line to be printed, this can be used to prevent runs to
+# go forever on errors when Xen is rebooting
+set maxline 1000
+
+# Configure slow parameters used with send -s
+# This allows us to slow down console writes to prevent issues with slow
+# emulators or targets.
+# Format here is: {NUM TIME} which reads as wait TIME seconds each NUM of
+# characters, here we send 1 char each 100ms
+set send_slow {1 .1}
+
+proc test_boot {{maxline} {host_ip}} {
+expect_after {
+-re "(.*)\r" {
+if {$maxline != 0} {
+set maxline [expr {$maxline - 1}]
+if {$maxline <= 0} {
+send_user "ERROR-Toomuch!\n"
+exit 2
+}
+}
+exp_continue
+}
+timeout {send_user "ERROR-Timeout!\n"; exit 3}
+eof {send_user "ERROR-EOF!\n"; exit 4}
+}
+
+# Extract the telnet port numbers from FVP output, because the telnet ports
+# are not guaranteed to be fixed numbers.
+expect -re {terminal_0: Listening for serial connection on port [0-9]+}
+set terminal_0 $expect_out(0,string)
+if {[regexp {port (\d+)} $terminal_0 match port_0]} {
+puts "terminal_0 port is $port_0"
+} else {
+puts "terminal_0 port not found"
+exit 5
+}
+
+spawn bash -c "telnet localhost $port_0"
+expect -re "Hit any key to stop autoboot.*"
+send -s "  \r"
+send -s "setenv serverip $host_ip; setenv ipaddr $host_ip; tftpb 
0x8020 boot.scr; source 0x8020\r"
+
+# Initial Xen boot
+expect -re "\(XEN\).*Freed .* init memory."
+
+# Dom0 and DomU
+expect -re "Domain-0.*"
+expect -re "BusyBox.*"
+expect -re "/ #.*"
+}
+
+# Get host IP
+spawn bash -c "hostname -I | awk '{print \$1}'"
+expect -re {(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})}
+set host_ip $expect_out(0,string)
+
+# Start the FVP and run the test
+spawn bash -c "$runcmd"
+
+test_boot 2000 "$host_ip"
+
+send_user "\nExecution with SUCCESS\n"
+exit 0
-- 
2.25.1

[PATCH v2 4/5] automation: Add the script for the FVP smoke test

[Henry Wang]

This commit adds the shell script for the FVP smoke test. Similarly
as the QEMU jobs, the shell script will firstly prepare the DomU
BusyBox image, use the ImageBuilder to arrange the binaries in memory
and generate the U-Boot script, then start the test. To provide the
TFTP service for the FVP, the shell script will also start the TFTP
service, and copy the binaries needed by test to the TFTP directory
used by the TFTP server.

Signed-off-by: Henry Wang 
---
v2:
- Set pipefail before running the expect script, so that the error
  won't be hid by pipe and the tee to the logfile.
---
 .../scripts/fvp-base-smoke-dom0-arm64.sh  | 120 ++
 1 file changed, 120 insertions(+)
 create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh

diff --git a/automation/scripts/fvp-base-smoke-dom0-arm64.sh 
b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
new file mode 100755
index 00..99097dad51
--- /dev/null
+++ b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
@@ -0,0 +1,120 @@
+#!/bin/bash
+
+set -ex
+
+# 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 ..
+
+mkdir -p rootfs
+cd rootfs
+tar xvzf ../initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+cp -ar ../dist/install/* .
+mv ../initrd.cpio.gz ./root
+cp ../Image ./root
+echo "name=\"test\"
+memory=512
+vcpus=1
+kernel=\"/root/Image\"
+ramdisk=\"/root/initrd.cpio.gz\"
+extra=\"console=hvc0 root=/dev/ram0 rdinit=/bin/sh\"
+" > root/test.cfg
+echo "#!/bin/bash
+
+export LD_LIBRARY_PATH=/usr/local/lib
+bash /etc/init.d/xencommons start
+
+xl list
+
+xl create -c /root/test.cfg
+
+" > 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 > ../xen-rootfs.cpio.gz
+cd ../..
+
+# Start a TFTP server to provide TFTP service to FVP
+service tftpd-hpa start
+
+# ImageBuilder
+echo 'MEMORY_START="0x8000"
+MEMORY_END="0xFF00"
+
+DEVICE_TREE="fvp-base-gicv3-psci-1t.dtb"
+XEN="xen"
+DOM0_KERNEL="Image"
+DOM0_RAMDISK="xen-rootfs.cpio.gz"
+XEN_CMD="console=dtuart dom0_mem=1024M console_timestamps=boot"
+
+NUM_DOMUS=0
+
+LOAD_CMD="tftpb"
+UBOOT_SOURCE="boot.source"
+UBOOT_SCRIPT="boot.scr"' > binaries/config
+rm -rf imagebuilder
+git clone https://gitlab.com/ViryaOS/imagebuilder
+bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c 
binaries/config
+
+# Copy files to the TFTP directory to use
+cp ./binaries/boot.scr /srv/tftp/
+cp ./binaries/Image /srv/tftp/
+cp ./binaries/xen-rootfs.cpio.gz /srv/tftp/
+cp ./binaries/xen /srv/tftp/
+cp ./binaries/fvp-base-gicv3-psci-1t.dtb /srv/tftp/
+
+# Start FVP
+TERM0_CFG="-C bp.terminal_0.mode=telnet -C bp.terminal_0.start_telnet=0"
+TERM1_CFG="-C bp.terminal_1.mode=telnet -C bp.terminal_1.start_telnet=0"
+TERM2_CFG="-C bp.terminal_2.mode=telnet -C bp.terminal_2.start_telnet=0"
+TERM3_CFG="-C bp.terminal_3.mode=telnet -C bp.terminal_3.start_telnet=0"
+
+VIRTIO_USER_NETWORK_CFG="-C bp.virtio_net.enabled=1 \
+-C bp.virtio_net.hostbridge.userNetworking=1 \
+-C bp.virtio_net.hostbridge.userNetPorts=8022=22 \
+-C bp.virtio_net.transport=legacy"
+
+# Set the pipefail so that the error code from the expect script won't
+# be hid by pipe and the tee command.
+set -o pipefail
+./automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp \
+
"/FVP/FVP_Base_RevC-2xAEMvA/Base_RevC_AEMvA_pkg/models/Linux64_armv8l_GCC-9.3/FVP_Base_RevC-2xAEMvA
 \
+-C bp.vis.disable_visualisation=1 \
+-C bp.ve_sysregs.exit_on_shutdown=1 \
+-C bp.secure_memory=0 \
+-C cache_state_modelled=0 \
+-C cluster0.has_arm_v8-4=1 \
+-C cluster1.has_arm_v8-4=1 \
+${TERM0_CFG} ${TERM1_CFG} ${TERM2_CFG} ${TERM3_CFG} \
+${VIRTIO_USER_NETWORK_CFG} \
+-C bp.secureflashloader.fname=$(pwd)/binaries/bl1.bin \
+-C bp.flashloader0.fname=$(pwd)/binaries/fip.bin" |& \
+tee smoke.serial
+
+exit 0
-- 
2.25.1

[PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP

[Henry Wang]

Unlike the emulators that currently being used in the CI pipelines,
the FVP must start at EL3. Therefore we need the firmware, i.e. the
TrustedFirmware-A (TF-A), for corresponding functionality.

There is a dedicated board (vexpress_fvp) in U-Boot (serve as the
BL33 of the TF-A) for the FVP platform, so the U-Boot should also be
compiled for the FVP platform instead of reusing the U-Boot for the
existing emulators used in the CI pipelines.

To avoid compiling TF-A and U-Boot everytime in the job, adding a
Dockerfile to the test artifacts to build TF-A v2.9.0 and U-Boot
v2023.10 for FVP. The binaries for the TF-A and U-Boot, as well as
the device tree for the FVP platform, will be saved (and exported by
the CI job introduced by following commits). Note that, a patch for
the TF-A will be applied before building to enable the virtio-net
and the virtio-rng device on the FVP. The virtio-net device will
provide the networking service for FVP, and the virtio-rng device
will improve the speed of the FVP.

Signed-off-by: Henry Wang 
Reviewed-by: Stefano Stabellini 
---
v2:
- Add Stefano's Reviewed-by tag.
---
 .../2023.10-2.9.0-arm64v8.dockerfile  | 48 +++
 1 file changed, 48 insertions(+)
 create mode 100644 
automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile

diff --git 
a/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile 
b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
new file mode 100644
index 00..6566b60545
--- /dev/null
+++ 
b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
@@ -0,0 +1,48 @@
+FROM --platform=linux/arm64/v8 debian:bookworm
+LABEL maintainer.name="The Xen Project" \
+  maintainer.email="xen-devel@lists.xenproject.org"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV UBOOT_VERSION="2023.10"
+ENV TFA_VERSION="v2.9.0"
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+# build depends
+RUN apt-get update && \
+apt-get --quiet --yes install \
+build-essential \
+libssl-dev \
+bc \
+curl \
+flex \
+bison \
+git \
+device-tree-compiler && \
+apt-get autoremove -y && \
+apt-get clean && \
+rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
+
+# Build U-Boot and TF-A
+RUN curl -fsSLO https://ftp.denx.de/pub/u-boot/u-boot-"$UBOOT_VERSION".tar.bz2 
&& \
+tar xvf u-boot-"$UBOOT_VERSION".tar.bz2 && \
+cd u-boot-"$UBOOT_VERSION" && \
+make -j$(nproc) V=1 vexpress_fvp_defconfig && \
+make -j$(nproc) V=1 all && \
+cd .. && \
+git clone --branch "$TFA_VERSION" --depth 1 
https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git && \
+cd trusted-firmware-a && \
+curl -fsSLO 
https://git.yoctoproject.org/meta-arm/plain/meta-arm-bsp/recipes-bsp/trusted-firmware-a/files/fvp-base/0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch
 \
+ --output 
0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
+git config --global user.email "y...@example.com" && \
+git config --global user.name "Your Name" && \
+git am 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
+make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 
FVP_DT_PREFIX=fvp-base-gicv3-psci-1t all && \
+make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 
FVP_DT_PREFIX=fvp-base-gicv3-psci-1t fip 
BL33=../u-boot-"$UBOOT_VERSION"/u-boot.bin && \
+cp build/fvp/debug/bl1.bin / && \
+cp build/fvp/debug/fip.bin / && \
+cp build/fvp/debug/fdts/fvp-base-gicv3-psci-1t.dtb / && \
+cd /build && \
+rm -rf u-boot-"$UBOOT_VERSION" trusted-firmware-a
-- 
2.25.1

[PATCH v2 0/5] automation: Support running FVP Dom0 smoke test for Arm

[Henry Wang]

This series adds the support for running FVP Dom0 smoke test for Arm on
the Arm64 GitLab CI runner. Detailed changes please refer to the commit
message of each commit.

An example test pipeline with these patches applied (with the docker
registry changed to my own registry and unrelated job removed) can be
found at:
https://gitlab.com/xen-project/people/henryw/xen/-/pipelines/1099757245

The second example of a negative test with breaking the expect script
by a "never met" condition can be found at:
https://gitlab.com/xen-project/people/henryw/xen/-/pipelines/1099757601
The job will fail as expected after the timeout set by the expect
script.

Henry Wang (5):
  automation: Add a Dockerfile for running FVP_Base jobs
  automation: Add the Dockerfile to build TF-A and U-Boot for FVP
  automation: Add the expect script with test case for FVP
  automation: Add the script for the FVP smoke test
  automation: Add the arm64 FVP build and Dom0 smoke test jobs

 .../debian/bookworm-arm64v8-fvp.dockerfile|  64 ++
 automation/gitlab-ci/build.yaml   |  17 +++
 automation/gitlab-ci/test.yaml|  22 
 .../expect/fvp-base-smoke-dom0-arm64.exp  |  73 +++
 .../scripts/fvp-base-smoke-dom0-arm64.sh  | 120 ++
 .../2023.10-2.9.0-arm64v8.dockerfile  |  48 +++
 6 files changed, 344 insertions(+)
 create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile
 create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
 create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh
 create mode 100644 
automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile

-- 
2.25.1

Re: [PATCH 3/5] automation: Add the expect script with test case for FVP

[Henry Wang]

Hi Stefano,

> On Dec 8, 2023, at 10:03, Henry Wang  wrote:
> 
> Hi Stefano,
> 
>> On Dec 8, 2023, at 09:38, Stefano Stabellini  wrote:
>>> +set host_ip $expect_out(0,string)
>>> +
>>> +# Start the FVP and run the test
>>> +spawn bash -c "$runcmd"
>>> +
>>> +test_boot 2000 "$host_ip"
>>> +
>>> +send_user "\nExecution with SUCCESS\n"
>> 
>> Won't this always return SUCCESS even in case of failure?
> 
> IMHO, if things fails, we have various exit code (1-5) for each failure case. 
> For example,
> if the FVP port somehow cannot be found, we exit with error code 5. This will 
> basically lead
> us to the only case where the failure is caused by the script fails to wait 
> for the expected
> string/regexp, and this case we have the timeout failure triggered by my 
> above-mentioned
> expect_after block.

I did a test to see if I break the expect script by adding below hunk:
```
--- a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
+++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
@@ -51,6 +51,7 @@ proc test_boot {{maxline} {host_ip}} {
 send -s "setenv serverip $host_ip; setenv ipaddr $host_ip; tftpb 
0x8020 boot.scr; source 0x8020\r"

 # Initial Xen boot
+expect -re "this is a hack to break the build"
 expect -re "\(XEN\).*Freed .* init memory."

 # Dom0 and DomU
```
The timeout did happen in the expect script after the set timeout, see [1]

However the job still passes, and I believe this is caused by the shell script:
```
./automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp \

"/FVP/FVP_Base_RevC-2xAEMvA/Base_RevC_AEMvA_pkg/models/Linux64_armv8l_GCC-9.3/FVP_Base_RevC-2xAEMvA
 \
-C bp.vis.disable_visualisation=1 \
-C bp.ve_sysregs.exit_on_shutdown=1 \
-C bp.secure_memory=0 \
-C cache_state_modelled=0 \
-C cluster0.has_arm_v8-4=1 \
-C cluster1.has_arm_v8-4=1 \
${TERM0_CFG} ${TERM1_CFG} ${TERM2_CFG} ${TERM3_CFG} \
${VIRTIO_USER_NETWORK_CFG} \
-C bp.secureflashloader.fname=$(pwd)/binaries/bl1.bin \
-C bp.flashloader0.fname=$(pwd)/binaries/fip.bin" |& \
tee smoke.serial

exit 0
```

The “|& tee smoke.serial” hides the error propagated by the expect script. I 
will send a v2 to fix it.

[1] 
https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5708263782/artifacts/file/smoke.serial

Kind regards,
Henry

> 
> Kind regards,
> Henry
> 
>>> +exit 0
>>> -- 
>>> 2.25.1
>>> 
> 

Re: [PATCH 3/5] automation: Add the expect script with test case for FVP

[Henry Wang]

Hi Stefano,

> On Dec 8, 2023, at 09:38, Stefano Stabellini  wrote:
> 
> On Thu, 7 Dec 2023, Henry Wang wrote:
>> To interact with the FVP (for example entering the U-Boot shell
>> and transferring the files by TFTP), we need to connect the
>> corresponding port by the telnet first. Use an expect script to
>> simplify the automation of the whole "interacting with FVP" stuff.
>> 
>> The expect script will firstly detect the IP of the host, then
>> connect to the telnet port of the FVP, set the `serverip` and `ipaddr`
>> for the TFTP service in the U-Boot shell, and finally boot Xen from
>> U-Boot and wait for the expected results by Xen, Dom0 and DomU.
> 
> I am not an expert in "expect" but this script looks great

:)

> 
> 
>> Signed-off-by: Henry Wang 
>> ---
>> .../expect/fvp-base-smoke-dom0-arm64.exp  | 73 +++
>> 1 file changed, 73 insertions(+)
>> create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>> 
>> +
>> +proc test_boot {{maxline} {host_ip}} {
>> +expect_after {
>> +-re "(.*)\r" {
>> +if {$maxline != 0} {
>> +set maxline [expr {$maxline - 1}]
>> +if {$maxline <= 0} {
>> +send_user "ERROR-Toomuch!\n"
>> +exit 2
>> +}
>> +}
>> +exp_continue
>> +}
>> +timeout {send_user "ERROR-Timeout!\n"; exit 3}
>> +eof {send_user "ERROR-EOF!\n"; exit 4}
>> +}
> 
> Why do we need this "expect_after" ?

It is basically for the error handling. Either there is too many characters 
triggered
by some misconfiguration of Xen/Kernel leading to Xen/Kernel constantly reboots,
or the code stuck somehow, we have a way to stop the script instead of waiting 
in
the infinity loop.

>> +# Extract the telnet port numbers from FVP output, because the telnet 
>> ports
>> +# are not guaranteed to be fixed numbers.
>> +expect -re {terminal_0: Listening for serial connection on port [0-9]+}
>> +set terminal_0 $expect_out(0,string)
>> +if {[regexp {port (\d+)} $terminal_0 match port_0]} {
>> +puts "terminal_0 port is $port_0"
>> +} else {
>> +puts "terminal_0 port not found"
>> +exit 5
>> +}
>> +
>> +spawn bash -c "telnet localhost $port_0"
>> +expect -re "Hit any key to stop autoboot.*"
>> +send -s "  \r"
>> +send -s "setenv serverip $host_ip; setenv ipaddr $host_ip; tftpb 
>> 0x8020 boot.scr; source 0x8020\r"
>> +
>> +# Initial Xen boot
>> +expect -re "\(XEN\).*Freed .* init memory."
>> +
>> +# Dom0 and DomU
>> +expect -re "Domain-0.*"
>> +expect -re "BusyBox.*"
>> +expect -re "/ #.*"
> 
> This is clear, excellent
> 
>> +}
>> +
>> +# Get host IP
>> +spawn bash -c "hostname -I | awk '{print \$1}'"
>> +expect -re {(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})}
> 
> Why d{1,3}?

I think that is because the IP pattern, we at least have one digit and at most 
3.

>> +set host_ip $expect_out(0,string)
>> +
>> +# Start the FVP and run the test
>> +spawn bash -c "$runcmd"
>> +
>> +test_boot 2000 "$host_ip"
>> +
>> +send_user "\nExecution with SUCCESS\n"
> 
> Won't this always return SUCCESS even in case of failure?

IMHO, if things fails, we have various exit code (1-5) for each failure case. 
For example,
if the FVP port somehow cannot be found, we exit with error code 5. This will 
basically lead
us to the only case where the failure is caused by the script fails to wait for 
the expected
string/regexp, and this case we have the timeout failure triggered by my 
above-mentioned
expect_after block.

Kind regards,
Henry

>> +exit 0
>> -- 
>> 2.25.1
>> 

Re: [PATCH 4/5] automation: Add the script for the FVP smoke test

[Henry Wang]

Hi Stefano,

> On Dec 8, 2023, at 09:41, Stefano Stabellini  wrote:
> 
> On Thu, 7 Dec 2023, Henry Wang wrote:
>> This commit adds the shell script for the FVP smoke test. Similarly
>> as the QEMU jobs, the shell script will firstly prepare the DomU
>> BusyBox image, use the ImageBuilder to arrange the binaries in memory
>> and generate the U-Boot script, then start the test. To provide the
>> TFTP service for the FVP, the shell script will also start the TFTP
>> service, and copy the binaries needed by test to the TFTP directory
>> used by the TFTP server.
>> 
>> Signed-off-by: Henry Wang 
>> ---
>> .../scripts/fvp-base-smoke-dom0-arm64.sh  | 117 ++
>> 1 file changed, 117 insertions(+)
>> create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh
> 
> This script is clear and it is fine:
> 
> Reviewed-by: Stefano Stabellini 

Thanks!

> 
> My only concern is that the expect checks on what booted (Xen, Dom0,
> DomU) are inside the script fvp-base-smoke-dom0-arm64.exp rather than in
> this script. So if in the future we are going to have multiple tests
> with different configurations (for instance see
> qemu-smoke-dom0less-arm64.sh) we'll have to find a way to reuse
> fvp-base-smoke-dom0-arm64.exp somehow.

We do have ways to reuse expect script, for example, we can extract the common
code (variables and test logic) to the common.tcl file and source the 
common.tcl file
in the expect script to reuse the code. The reason why I didn’t do that in this 
series
is that currently there is only one script so I feel that there is not much 
benefit to do
this instantly :) Let’s wait to see if there is more comments from others, and 
I am
definitely open to refactor if there is the need to extract the common logic 
(for example
when we add dom0less tests in the future).

Kind regards,
Henry

Re: [PATCH 1/5] automation: Add a Dockerfile for running FVP_Base jobs

[Stefano Stabellini]

On Thu, 7 Dec 2023, Stefano Stabellini wrote:
> On Thu, 7 Dec 2023, Henry Wang wrote:
> > Fixed Virtual Platforms (FVPs) are complete simulations of an Arm
> > system, including processor, memory and peripherals. These are set
> > out in a "programmer's view", which gives programmers a comprehensive
> > model on which to build and test software. FVP can be configured to
> > different setups by its cmdline parameters, and hence having the FVP
> > in CI will provide us with the flexibility to test Arm features and
> > setups that we find difficult to use real hardware or emulators.
> > 
> > This commit adds a Dockerfile for the new arm64v8 container with
> > FVP installed, based on the debian bookworm-arm64v8 image. This
> > container will be used to run the FVP test jobs. Compared to the
> > debian bookworm-arm64v8 image, the packages in the newly added FVP
> > container does not contain the `u-boot-qemu`, and adds the `expect`
> > to run expect scripts introduced by following commits, `telnet` to
> > connect to FVP, and `tftpd-hpa` to provide the TFTP service for
> > the FVP.
> > 
> > Signed-off-by: Henry Wang 
> > ---
> >  .../debian/bookworm-arm64v8-fvp.dockerfile| 64 +++
> >  1 file changed, 64 insertions(+)
> >  create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile
> > 
> > diff --git a/automation/build/debian/bookworm-arm64v8-fvp.dockerfile 
> > b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
> > new file mode 100644
> > index 00..3b87dc5a5b
> > --- /dev/null
> > +++ b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
> 
> Please move this container under automation/tests-artifacts/ because the
> container is only meant to be used for tests as opposed as to build Xen.
> I know that in other cases we have reused the build container but that
> just because it was already there an working for the purpose.
> 
> Also please name it based on the fvp rather than debian:
> 
> automation/tests-artifacts/armfvp/11.23_9-arm64v8.dockerfile
> 
> Everything else looks fine.

I take it back. We even have
automation/build/ubuntu/xenial-xilinx.dockerfile and
automation/build/debian/bookworm-cppcheck.dockerfile

At one point I think we should separate the build containers from the
ones we use for testing but I won't ask it here.

Reviewed-by: Stefano Stabellini 

Re: [PATCH 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs

[Stefano Stabellini]

On Thu, 7 Dec 2023, Henry Wang wrote:
> Add a job in the build stage to export the TF-A, U-Boot and the
> device tree for the FVP platform from the test artifact container.
> 
> Add a FVP smoke test job in the test stage to do the same test as
> the `qemu-smoke-dom0-arm64-gcc` job.
> 
> Signed-off-by: Henry Wang 

Reviewed-by: Stefano Stabellini 


> ---
> Although it does not affect the functionality, I am still quite
> confused about why the logs displayed by GitLab UI, for
> example [1], is much less than the actual output (saved in the
> artifacts, see [2]). Had a discussion with Michal on Matrix
> and we agree that the log in gitlab UI is usually capped.
> 
> [1] https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5700569676
> [2] 
> https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5700569676/artifacts/file/smoke.serial
> ---
>  automation/gitlab-ci/build.yaml | 17 +
>  automation/gitlab-ci/test.yaml  | 22 ++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 32af30cced..89d2f01302 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -344,6 +344,23 @@ kernel-6.1.19-export:
>tags:
>  - x86_64
>  
> +armfvp-uboot-tfa-2023.10-2.9.0-arm64-export:
> +  extends: .test-jobs-artifact-common
> +  image: 
> registry.gitlab.com/xen-project/xen/tests-artifacts/armfvp-uboot-tfa:2023.10-2.9.0-arm64v8
> +  script:
> +- |
> +   mkdir binaries && \
> +   cp /bl1.bin binaries/bl1.bin && \
> +   cp /fip.bin binaries/fip.bin && \
> +   cp /fvp-base-gicv3-psci-1t.dtb binaries/fvp-base-gicv3-psci-1t.dtb
> +  artifacts:
> +paths:
> +  - binaries/bl1.bin
> +  - binaries/fip.bin
> +  - binaries/fvp-base-gicv3-psci-1t.dtb
> +  tags:
> +- arm64
> +
>  # Jobs below this line
>  
>  # Build jobs needed for tests
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 6aabdb9d15..47e00d0a0b 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -96,6 +96,19 @@
>tags:
>  - xilinx
>  
> +.fvp-arm64:
> +  extends: .test-jobs-common
> +  variables:
> +CONTAINER: debian:bookworm-arm64v8-fvp
> +LOGFILE: fvp-smoke-arm64.log
> +  artifacts:
> +paths:
> +  - smoke.serial
> +  - '*.log'
> +when: always
> +  tags:
> +- arm64
> +
>  .adl-x86-64:
>extends: .test-jobs-common
>variables:
> @@ -459,3 +472,12 @@ qemu-smoke-ppc64le-powernv9-gcc:
>needs:
>  - qemu-system-ppc64-8.1.0-ppc64-export
>  - debian-bullseye-gcc-ppc64le-debug
> +
> +fvp-smoke-dom0-arm64-gcc-debug:
> +  extends: .fvp-arm64
> +  script:
> +- ./automation/scripts/fvp-base-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE}
> +  needs:
> +- *arm64-test-needs
> +- armfvp-uboot-tfa-2023.10-2.9.0-arm64-export
> +- alpine-3.18-gcc-debug-arm64
> -- 
> 2.25.1
> 

Re: [PATCH 4/5] automation: Add the script for the FVP smoke test

[Stefano Stabellini]

On Thu, 7 Dec 2023, Henry Wang wrote:
> This commit adds the shell script for the FVP smoke test. Similarly
> as the QEMU jobs, the shell script will firstly prepare the DomU
> BusyBox image, use the ImageBuilder to arrange the binaries in memory
> and generate the U-Boot script, then start the test. To provide the
> TFTP service for the FVP, the shell script will also start the TFTP
> service, and copy the binaries needed by test to the TFTP directory
> used by the TFTP server.
> 
> Signed-off-by: Henry Wang 
> ---
>  .../scripts/fvp-base-smoke-dom0-arm64.sh  | 117 ++
>  1 file changed, 117 insertions(+)
>  create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh
> 
> diff --git a/automation/scripts/fvp-base-smoke-dom0-arm64.sh 
> b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
> new file mode 100755
> index 00..716a63b0a8
> --- /dev/null
> +++ b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
> @@ -0,0 +1,117 @@
> +#!/bin/bash
> +
> +set -ex
> +
> +# 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 ..
> +
> +mkdir -p rootfs
> +cd rootfs
> +tar xvzf ../initrd.tar.gz
> +mkdir proc
> +mkdir run
> +mkdir srv
> +mkdir sys
> +rm var/run
> +cp -ar ../dist/install/* .
> +mv ../initrd.cpio.gz ./root
> +cp ../Image ./root
> +echo "name=\"test\"
> +memory=512
> +vcpus=1
> +kernel=\"/root/Image\"
> +ramdisk=\"/root/initrd.cpio.gz\"
> +extra=\"console=hvc0 root=/dev/ram0 rdinit=/bin/sh\"
> +" > root/test.cfg
> +echo "#!/bin/bash
> +
> +export LD_LIBRARY_PATH=/usr/local/lib
> +bash /etc/init.d/xencommons start
> +
> +xl list
> +
> +xl create -c /root/test.cfg
> +
> +" > 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 > ../xen-rootfs.cpio.gz
> +cd ../..
> +
> +# Start a TFTP server to provide TFTP service to FVP
> +service tftpd-hpa start
> +
> +# ImageBuilder
> +echo 'MEMORY_START="0x8000"
> +MEMORY_END="0xFF00"
> +
> +DEVICE_TREE="fvp-base-gicv3-psci-1t.dtb"
> +XEN="xen"
> +DOM0_KERNEL="Image"
> +DOM0_RAMDISK="xen-rootfs.cpio.gz"
> +XEN_CMD="console=dtuart dom0_mem=1024M console_timestamps=boot"
> +
> +NUM_DOMUS=0
> +
> +LOAD_CMD="tftpb"
> +UBOOT_SOURCE="boot.source"
> +UBOOT_SCRIPT="boot.scr"' > binaries/config
> +rm -rf imagebuilder
> +git clone https://gitlab.com/ViryaOS/imagebuilder
> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c 
> binaries/config
> +
> +# Copy files to the TFTP directory to use
> +cp ./binaries/boot.scr /srv/tftp/
> +cp ./binaries/Image /srv/tftp/
> +cp ./binaries/xen-rootfs.cpio.gz /srv/tftp/
> +cp ./binaries/xen /srv/tftp/
> +cp ./binaries/fvp-base-gicv3-psci-1t.dtb /srv/tftp/
> +
> +# Start FVP
> +TERM0_CFG="-C bp.terminal_0.mode=telnet -C bp.terminal_0.start_telnet=0"
> +TERM1_CFG="-C bp.terminal_1.mode=telnet -C bp.terminal_1.start_telnet=0"
> +TERM2_CFG="-C bp.terminal_2.mode=telnet -C bp.terminal_2.start_telnet=0"
> +TERM3_CFG="-C bp.terminal_3.mode=telnet -C bp.terminal_3.start_telnet=0"
> +
> +VIRTIO_USER_NETWORK_CFG="-C bp.virtio_net.enabled=1 \
> +-C bp.virtio_net.hostbridge.userNetworking=1 \
> +-C bp.virtio_net.hostbridge.userNetPorts=8022=22 \
> +-C bp.virtio_net.transport=legacy"
> +
> +./automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp \
> +
> "/FVP/FVP_Base_RevC-2xAEMvA/Base_RevC_AEMvA_pkg/models/Linux64_armv8l_GCC-9.3/FVP_Base_RevC-2xAEMvA
>  \
> +-C bp.vis.disable_visualisation=1 \
> +-C bp.ve_sysregs.exit_on_shutdown=1 \
> +-C bp.secure_memory=0 \
> +-C cache_state_modelled=0 \
> +-C cluster0.has_arm_v8-4=1 \
> +-C cluster1.has_arm_v8-4=1 \
> +${TERM0_CFG} ${TERM1_CFG} ${TERM2_CFG} ${TERM3_CFG} \
> +${VIRTIO_USER_NETWORK_CFG} \
> +-C bp.secureflashloader.fname=$(pwd)/binaries/bl1.bin \
> +-C bp.flashloader0.fname=$(pwd)/binaries/fip.bin" |& \
> +tee smoke.serial

This script is clear and it is fine:

Reviewed-by: Stefano Stabellini 

My only concern is that the expect checks on what booted (Xen, Dom0,
DomU) are inside the script fvp-base-smoke-dom0-arm64.exp rather than in
this script. So if in the future we are going to have multiple tests
with different configurations (for instance see
qemu-smoke-dom0less-arm64.sh) we'll have to find a way to reuse
fvp-base-smoke-dom0-arm64.exp somehow.

Re: [PATCH 3/5] automation: Add the expect script with test case for FVP

[Stefano Stabellini]

On Thu, 7 Dec 2023, Henry Wang wrote:
> To interact with the FVP (for example entering the U-Boot shell
> and transferring the files by TFTP), we need to connect the
> corresponding port by the telnet first. Use an expect script to
> simplify the automation of the whole "interacting with FVP" stuff.
> 
> The expect script will firstly detect the IP of the host, then
> connect to the telnet port of the FVP, set the `serverip` and `ipaddr`
> for the TFTP service in the U-Boot shell, and finally boot Xen from
> U-Boot and wait for the expected results by Xen, Dom0 and DomU.

I am not an expert in "expect" but this script looks great


> Signed-off-by: Henry Wang 
> ---
>  .../expect/fvp-base-smoke-dom0-arm64.exp  | 73 +++
>  1 file changed, 73 insertions(+)
>  create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> 
> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp 
> b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> new file mode 100755
> index 00..25d9a5f81c
> --- /dev/null
> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> @@ -0,0 +1,73 @@
> +#!/usr/bin/expect
> +
> +set timeout 2000
> +
> +# Command to use to run must be given as first argument
> +# if options are required, quotes must be used:
> +# xxx.exp "cmd opt1 opt2"
> +set runcmd [lindex $argv 0]
> +
> +# Maximum number of line to be printed, this can be used to prevent runs to
> +# go forever on errors when Xen is rebooting
> +set maxline 1000
> +
> +# Configure slow parameters used with send -s
> +# This allows us to slow down console writes to prevent issues with slow
> +# emulators or targets.
> +# Format here is: {NUM TIME} which reads as wait TIME seconds each NUM of
> +# characters, here we send 1 char each 100ms
> +set send_slow {1 .1}
> +
> +proc test_boot {{maxline} {host_ip}} {
> +expect_after {
> +-re "(.*)\r" {
> +if {$maxline != 0} {
> +set maxline [expr {$maxline - 1}]
> +if {$maxline <= 0} {
> +send_user "ERROR-Toomuch!\n"
> +exit 2
> +}
> +}
> +exp_continue
> +}
> +timeout {send_user "ERROR-Timeout!\n"; exit 3}
> +eof {send_user "ERROR-EOF!\n"; exit 4}
> +}

Why do we need this "expect_after" ?


> +# Extract the telnet port numbers from FVP output, because the telnet 
> ports
> +# are not guaranteed to be fixed numbers.
> +expect -re {terminal_0: Listening for serial connection on port [0-9]+}
> +set terminal_0 $expect_out(0,string)
> +if {[regexp {port (\d+)} $terminal_0 match port_0]} {
> +puts "terminal_0 port is $port_0"
> +} else {
> +puts "terminal_0 port not found"
> +exit 5
> +}
> +
> +spawn bash -c "telnet localhost $port_0"
> +expect -re "Hit any key to stop autoboot.*"
> +send -s "  \r"
> +send -s "setenv serverip $host_ip; setenv ipaddr $host_ip; tftpb 
> 0x8020 boot.scr; source 0x8020\r"
> +
> +# Initial Xen boot
> +expect -re "\(XEN\).*Freed .* init memory."
> +
> +# Dom0 and DomU
> +expect -re "Domain-0.*"
> +expect -re "BusyBox.*"
> +expect -re "/ #.*"

This is clear, excellent


> +}
> +
> +# Get host IP
> +spawn bash -c "hostname -I | awk '{print \$1}'"
> +expect -re {(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})}

Why d{1,3}?


> +set host_ip $expect_out(0,string)
> +
> +# Start the FVP and run the test
> +spawn bash -c "$runcmd"
> +
> +test_boot 2000 "$host_ip"
> +
> +send_user "\nExecution with SUCCESS\n"

Won't this always return SUCCESS even in case of failure?


> +exit 0
> -- 
> 2.25.1
> 

Re: [PATCH 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP

[Stefano Stabellini]

On Thu, 7 Dec 2023, Henry Wang wrote:
> Unlike the emulators that currently being used in the CI pipelines,
> the FVP must start at EL3. Therefore we need the firmware, i.e. the
> TrustedFirmware-A (TF-A), for corresponding functionality.
> 
> There is a dedicated board (vexpress_fvp) in U-Boot (serve as the
> BL33 of the TF-A) for the FVP platform, so the U-Boot should also be
> compiled for the FVP platform instead of reusing the U-Boot for the
> existing emulators used in the CI pipelines.
> 
> To avoid compiling TF-A and U-Boot everytime in the job, adding a
> Dockerfile to the test artifacts to build TF-A v2.9.0 and U-Boot
> v2023.10 for FVP. The binaries for the TF-A and U-Boot, as well as
> the device tree for the FVP platform, will be saved (and exported by
> the CI job introduced by following commits). Note that, a patch for
> the TF-A will be applied before building to enable the virtio-net
> and the virtio-rng device on the FVP. The virtio-net device will
> provide the networking service for FVP, and the virtio-rng device
> will improve the speed of the FVP.
> 
> Signed-off-by: Henry Wang 

Reviewed-by: Stefano Stabellini 


> ---
>  .../2023.10-2.9.0-arm64v8.dockerfile  | 48 +++
>  1 file changed, 48 insertions(+)
>  create mode 100644 
> automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
> 
> diff --git 
> a/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
>  
> b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
> new file mode 100644
> index 00..6566b60545
> --- /dev/null
> +++ 
> b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
> @@ -0,0 +1,48 @@
> +FROM --platform=linux/arm64/v8 debian:bookworm
> +LABEL maintainer.name="The Xen Project" \
> +  maintainer.email="xen-devel@lists.xenproject.org"
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV UBOOT_VERSION="2023.10"
> +ENV TFA_VERSION="v2.9.0"
> +ENV USER root
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +# build depends
> +RUN apt-get update && \
> +apt-get --quiet --yes install \
> +build-essential \
> +libssl-dev \
> +bc \
> +curl \
> +flex \
> +bison \
> +git \
> +device-tree-compiler && \
> +apt-get autoremove -y && \
> +apt-get clean && \
> +rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
> +
> +# Build U-Boot and TF-A
> +RUN curl -fsSLO 
> https://ftp.denx.de/pub/u-boot/u-boot-"$UBOOT_VERSION".tar.bz2 && \
> +tar xvf u-boot-"$UBOOT_VERSION".tar.bz2 && \
> +cd u-boot-"$UBOOT_VERSION" && \
> +make -j$(nproc) V=1 vexpress_fvp_defconfig && \
> +make -j$(nproc) V=1 all && \
> +cd .. && \
> +git clone --branch "$TFA_VERSION" --depth 1 
> https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git && \
> +cd trusted-firmware-a && \
> +curl -fsSLO 
> https://git.yoctoproject.org/meta-arm/plain/meta-arm-bsp/recipes-bsp/trusted-firmware-a/files/fvp-base/0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch
>  \
> + --output 
> 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
> +git config --global user.email "y...@example.com" && \
> +git config --global user.name "Your Name" && \
> +git am 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch 
> && \
> +make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 
> FVP_DT_PREFIX=fvp-base-gicv3-psci-1t all && \
> +make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 
> FVP_DT_PREFIX=fvp-base-gicv3-psci-1t fip 
> BL33=../u-boot-"$UBOOT_VERSION"/u-boot.bin && \
> +cp build/fvp/debug/bl1.bin / && \
> +cp build/fvp/debug/fip.bin / && \
> +cp build/fvp/debug/fdts/fvp-base-gicv3-psci-1t.dtb / && \
> +cd /build && \
> +rm -rf u-boot-"$UBOOT_VERSION" trusted-firmware-a
> -- 
> 2.25.1
> 

Re: [PATCH 1/5] automation: Add a Dockerfile for running FVP_Base jobs

[Stefano Stabellini]

On Thu, 7 Dec 2023, Henry Wang wrote:
> Fixed Virtual Platforms (FVPs) are complete simulations of an Arm
> system, including processor, memory and peripherals. These are set
> out in a "programmer's view", which gives programmers a comprehensive
> model on which to build and test software. FVP can be configured to
> different setups by its cmdline parameters, and hence having the FVP
> in CI will provide us with the flexibility to test Arm features and
> setups that we find difficult to use real hardware or emulators.
> 
> This commit adds a Dockerfile for the new arm64v8 container with
> FVP installed, based on the debian bookworm-arm64v8 image. This
> container will be used to run the FVP test jobs. Compared to the
> debian bookworm-arm64v8 image, the packages in the newly added FVP
> container does not contain the `u-boot-qemu`, and adds the `expect`
> to run expect scripts introduced by following commits, `telnet` to
> connect to FVP, and `tftpd-hpa` to provide the TFTP service for
> the FVP.
> 
> Signed-off-by: Henry Wang 
> ---
>  .../debian/bookworm-arm64v8-fvp.dockerfile| 64 +++
>  1 file changed, 64 insertions(+)
>  create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile
> 
> diff --git a/automation/build/debian/bookworm-arm64v8-fvp.dockerfile 
> b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
> new file mode 100644
> index 00..3b87dc5a5b
> --- /dev/null
> +++ b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile

Please move this container under automation/tests-artifacts/ because the
container is only meant to be used for tests as opposed as to build Xen.
I know that in other cases we have reused the build container but that
just because it was already there an working for the purpose.

Also please name it based on the fvp rather than debian:

automation/tests-artifacts/armfvp/11.23_9-arm64v8.dockerfile

Everything else looks fine.



> @@ -0,0 +1,64 @@
> +FROM --platform=linux/arm64/v8 debian:bookworm
> +LABEL maintainer.name="The Xen Project" \
> +  maintainer.email="xen-devel@lists.xenproject.org"
> +
> +ARG FVP_BASE_VERSION="11.23_9_Linux64_armv8l"
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV USER root
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +# build depends
> +RUN apt-get update && \
> +apt-get --quiet --yes install \
> +build-essential \
> +zlib1g-dev \
> +libncurses5-dev \
> +libssl-dev \
> +python3-dev \
> +python3-setuptools \
> +xorg-dev \
> +uuid-dev \
> +libyajl-dev \
> +libaio-dev \
> +libglib2.0-dev \
> +clang \
> +libpixman-1-dev \
> +pkg-config \
> +flex \
> +bison \
> +acpica-tools \
> +libfdt-dev \
> +bin86 \
> +bcc \
> +liblzma-dev \
> +libnl-3-dev \
> +ocaml-nox \
> +libfindlib-ocaml-dev \
> +markdown \
> +transfig \
> +pandoc \
> +checkpolicy \
> +wget \
> +git \
> +nasm \
> +# for test phase, fvp-smoke-* jobs
> +u-boot-tools \
> +expect \
> +device-tree-compiler \
> +curl \
> +cpio \
> +busybox-static \
> +telnet \
> +tftpd-hpa \
> +&& \
> +apt-get autoremove -y && \
> +apt-get clean && \
> +rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
> +
> +RUN wget 
> https://developer.arm.com/-/media/Files/downloads/ecosystem-models/FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz
>  && \
> +mkdir -p /FVP/FVP_Base_RevC-2xAEMvA && \
> +tar -xvzf FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz -C 
> /FVP/FVP_Base_RevC-2xAEMvA && \
> +rm FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz
> -- 
> 2.25.1
> 

[linux-linus test] 184021: regressions - FAIL

[osstest service owner]

flight 184021 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/184021/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-pvops  6 kernel-build fail REGR. vs. 183973

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 183973
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 183973
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 183973
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 183973
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 183973
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 183973
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 183973
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 183973
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass

version targeted for testing:
 linuxbee0e7762ad2c6025b9f5245c040fcc36ef2bde8
baseline version:
 linux815fb87b753055df2d9e50f6cd80eb10235fe3e9

Last test of basis   183973  2023-12-02 07:48:26 Z5 days
Failing since183977  2023-12-03 00:12:06 Z5 days   10 attempts
Testing same since   183993  2023-12-05 09:00:54 Z2 days4 attempts


People who touched revisions under test:
  Alex Williamson 
  Brett Creeley 
  Dan Carpenter 
  David Howells 
  Dmitry Antipov 
  Eugenio Pérez 
  Jason Gunthorpe 
  Jason Wang 
  Jens Axboe 
  Joao Martins 
  JP Kobryn 
  Juergen Gross 
  Linus Torvalds 
  Masami Hiramatsu (Google) 
  Michael Ellerman 
  Michael S. Tsirkin 
  Namjae Jeon 
  Nicholas Piggin 
  Paulo Alcantara (SUSE) 
  Paulo Alcantara 
  Robin Murphy 
  Sean Christopherson 
  Shannon Nelson 
  Steve French 
  Steve Sistare 
  Takashi Sakamoto 
  

Re: [RFC PATCH v1 1/1] xen/Makefile: introduce ARCH_FIXED_CONFIG for randconfig

[Stefano Stabellini]

On Thu, 7 Dec 2023, Oleksii wrote:
> On Thu, 2023-12-07 at 20:17 +, Andrew Cooper wrote:
> > On 07/12/2023 5:03 pm, Oleksii Kurochko wrote:
> > > ARCH_FIXED_CONFIG is required in the case of randconfig
> > > and CI for configs that aren't ready or are not
> > > supposed to be implemented for specific architecture.
> > > These configs should always be disabled to prevent randconfig
> > > related tests from failing.
> > > 
> > > Signed-off-by: Oleksii Kurochko 
> > > ---
> > >  xen/Makefile | 5 -
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/xen/Makefile b/xen/Makefile
> > > index ca571103c8..8ae8fe1480 100644
> > > --- a/xen/Makefile
> > > +++ b/xen/Makefile
> > > @@ -336,11 +336,14 @@ ifeq ($(config-build),y)
> > >  # *config targets only - make sure prerequisites are updated, and
> > > descend
> > >  # in tools/kconfig to make the *config target
> > >  
> > > +ARCH_FORCED_CONFIG :=
> > > $(srctree)/arch/$(SRCARCH)/configs/randomforced.config
> > > +
> > >  # Create a file for KCONFIG_ALLCONFIG which depends on the
> > > environment.
> > >  # This will be use by kconfig targets
> > > allyesconfig/allmodconfig/allnoconfig/randconfig
> > >  filechk_kconfig_allconfig = \
> > >  $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo
> > > 'CONFIG_XSM_FLASK_POLICY=n';) \
> > > -    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
> > > +    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
> > > +    $(if $(wildcard $(ARCH_FORCED_CONFIG)), cat
> > > $(ARCH_FORCED_CONFIG);) ) \
> > >  :
> > >  
> > >  .allconfig.tmp: FORCE
> > 
> > We already have infrastructure for this.  What's wrong with
> > EXTRA_FIXED_RANDCONFIG?
> Everything is fine; I don't know why there was only an issue with
> CONFIG_GRANT_TABLE on PPC. On the RISC-V side, there were more configs
> issues, prompting me to include all the configurations not implemented
> for RISC-V in EXTRA_FIXED_RANDCONFIG. You can find the added
> configurations in this commit:
> https://lore.kernel.org/xen-devel/b4e85f8f58787b4d179022973ce25673d6b56e36.1700761381.git.oleksii.kuroc...@gmail.com/#Z31automation:gitlab-ci:build.yaml
> 
> One challenge is that the same configurations need to be added multiple
> times for each build test using randconfig.

That's a lot of extra configs to add. Could you use a yaml anchor or a
.something to include? So that you define the full list only once at the
top of the file and then reuse it everywhere as needed.


> Another reason for this approach is a suggestion from Jan (probably I
> misunderstood it), who proposed using a template to instruct randconfig
> not to modify currently unnecessary configurations. You can find the
> suggestion and discussion here:
> https://lore.kernel.org/xen-devel/008d0c66-6816-4d12-9e1f-1878e982f...@suse.com/
> 
> Perhaps we could enhance the build script to fetch "fixed" configs from
> the architecture-specific fixed-defconfig instead of modifying the
> Makefile directly.

Sorry I missed the original thread somehow. Please use "automation" as
subject line tag for automation patches.

Re: [PATCH 3/3] xen: address violations of MISRA C:2012 Rule 14.4

[Stefano Stabellini]

On Thu, 7 Dec 2023, Simone Ballarin wrote:
> From: Maria Celeste Cesario 
> 
> The xen sources contain violations of MISRA C:2012 Rule 14.4 whose
> headline states:
> "The controlling expression of an if statement and the controlling
> expression of an iteration-statement shall have essentially Boolean type".
> 
> Struct domain member is_dying is an anonymous enum designed to act as boolean.
> Add deviation to mark its uses in controlling expressions as deliberate.
> 
> Signed-off-by: Maria Celeste Cesario 
> Signed-off-by: Simone Ballarin 

Reviewed-by: Stefano Stabellini 

Re: [PATCH 2/3] xen/x86: address violations of MISRA C:2012 Rule 14.4

[Stefano Stabellini]

On Thu, 7 Dec 2023, Simone Ballarin wrote:
> From: Maria Celeste Cesario 
> 
> The xen sources contain violations of MISRA C:2012 Rule 14.4 whose
> headline states:
> "The controlling expression of an if statement and the controlling
> expression of an iteration-statement shall have essentially Boolean type".
> 
> Add comparisons to avoid using enum constants as controlling expressions
> to comply with Rule 14.4.
> No functional change.
> 
> Signed-off-by: Maria Celeste Cesario  
> Signed-off-by: Simone Ballarin  

I think it would have been better to put all the iommu_intremap changed
in the same patch (patch #1). But anyway:

Reviewed-by: Stefano Stabellini 

Re: [PATCH 1/3] AMD/IOMMU: address violations of MISRA C:2012 Rule 14.4

[Stefano Stabellini]

On Thu, 7 Dec 2023, Simone Ballarin wrote:
> From: Maria Celeste Cesario 
> 
> The xen sources contain violations of MISRA C:2012 Rule 14.4 whose
> headline states:
> "The controlling expression of an if statement and the controlling
> expression of an iteration-statement shall have essentially Boolean type".
> 
> Add comparisons to avoid using enum constants as controlling expressions
> to comply with Rule 14.4.
> No functional change.
> 
> Signed-off-by: Maria Celeste Cesario  
> Signed-off-by: Simone Ballarin  

Reviewed-by: Stefano Stabellini 

Re: [XEN PATCH v2 2/5] xen/acpi: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Thu, 7 Dec 2023, Federico Serafini wrote:
> Add missing parameter names. No functional change.
> 
> Signed-off-by: Federico Serafini 

Reviewed-by: Stefano Stabellini 

Re: [XEN PATCH v2 4/5] x86/mm: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Thu, 7 Dec 2023, Federico Serafini wrote:
> Add missing parameter names. No functional change.
> 
> Signed-off-by: Federico Serafini 

Reviewed-by: Stefano Stabellini 

Re: [XEN PATCH v2 5/5] AMD/IOMMU: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Thu, 7 Dec 2023, Federico Serafini wrote:
> Add missing parameter names to address violations of MISRA C:2012
> Rule 8.2. Remove trailing spaces and use C standard types to comply
> with XEN coding style. No functional change.
> 
> Signed-off-by: Federico Serafini 

Reviewed-by: Stefano Stabellini 

Re: [XEN PATCH v2 3/5] x86/mm: remove compat_subarch_memory_op()

[Stefano Stabellini]

On Thu, 7 Dec 2023, Federico Serafini wrote:
> Remove remove compat_subarch_memory_op() declaration: there is no
> definition and there are no calls to such function in the XEN project.
> 
> Signed-off-by: Federico Serafini 

Reviewed-by: Stefano Stabellini 

Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3

[Stefano Stabellini]

On Thu, 7 Dec 2023, Julien Grall wrote:
> Hi Federico,
> 
> On 07/12/2023 09:08, Federico Serafini wrote:
> > MISRA C:2012 Rule 16.3 states that an unconditional break statement
> > shall terminate every switch-clause.
> > 
> > Update ECLAIR configuration to take into account:
> > - continue, goto, return statements;
> > - functions and macros that do not give the control back;
> > - fallthrough comments and pseudo-keywords.
> > 
> > Update docs/misra/deviations.rst accordingly.
> > 
> > Signed-off-by: Federico Serafini 
> > ---
> >   .../eclair_analysis/ECLAIR/deviations.ecl | 18 ++
> >   docs/misra/deviations.rst | 24 +++
> >   2 files changed, 42 insertions(+)
> 
> It would be good that this is depending on to be accepted:
> 
> https://lore.kernel.org/alpine.DEB.2.22.394.2312051859440.110490@ubuntu-linux-20-04-desktop.
> 
> > 
> > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > index b0c79741b5..df0b58a010 100644
> > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > @@ -321,6 +321,24 @@ statements are deliberate"
> >   -config=MC3R1.R14.3,statements={deliberate ,
> > "wrapped(any(),node(if_stmt))" }
> >   -doc_end
> >   +#
> > +# Series 16.
> > +#
> > +
> > +-doc_begin="Switch clauses ending with continue, goto, return statements
> > are safe."
> > +-config=MC3R1.R16.3,terminals+={safe,
> > "node(continue_stmt||goto_stmt||return_stmt)"}
> > +-doc_end
> > +
> > +-doc_begin="Switch clauses not ending with the break statement are safe if
> > a function/macro that does not give the control back is present."
> > +-config=MC3R1.R16.3,terminals+={safe,
> > "call(decl(name(__builtin_unreachable||do_unexpected_trap||fatal_trap||machine_halt||machine_restart||maybe_reboot||panic)))"}
> > +-config=MC3R1.R16.3,terminals+={safe,"macro(name(BUG||BUG_ON))"}
> > +-doc_end
> > +
> > +-doc_begin="Switch clauses not ending with the break statement are safe if
> > an explicit comment or pseudo-keyword indicating the fallthrough intention
> > is present."
> > +-config=MC3R1.R16.3,reports+={safe,
> > "any_area(any_loc(any_exp(text(^(?s).*([fF]all[- ]?[tT]hrough|FALL[-
> > ]?THROUGH).*$,0..1"}
> > +-config=MC3R1.R16.3,reports+={safe, "any_area(text(^(?s).*([fF]all[-
> > ]?[tT]hrough|FALL[- ]?THROUGH).*$,0..1))"}
> 
> This is not trivial to read. Can you document the exhaustive list of keywords
> you are actually trying to deviate on? Also, did you consider to harmonize to
> only a few?
> 
> > +-doc_end
> > +
> >   #
> >   # Series 20.
> >   #
> > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > index 6e7c4f25b8..fecd2bae8e 100644
> > --- a/docs/misra/deviations.rst
> > +++ b/docs/misra/deviations.rst
> > @@ -270,6 +270,30 @@ Deviations related to MISRA C:2012 Rules:
> >  statements are deliberate.
> >- Project-wide deviation; tagged as `disapplied` for ECLAIR.
> >   +   * - R16.3
> > + - Switch clauses ending with continue, goto, return statements are
> > safe.
> > + - Tagged as `safe` for ECLAIR.
> > +
> > +   * - R16.3
> > + - Switch clauses not ending with the break statement are safe if a
> > +   function/macro that does not give the control back is present.
> > + - Tagged as `safe` for ECLAIR, such functions/macros are:
> > +- __builtin_unreachable
> > +- do_unexpected_trap
> > +- fatal_trap
> > +- machine_halt
> > +- machine_restart
> > +- maybe_reboot
> > +- panic
> > +- BUG
> 
> To me, it seems to be odd to deviate R16.3 by function. Some of those have an
> attribute noreboot. Can this be used?

Just to clarify, I think Julien meant "noreturn" which is defined as
__attribute__((__noreturn__))

I think we need to deviate by function, rather than using SAF, because
we would have to use SAF in every switch rather than at the declaration
of panic and friends. But if we could use noreturn, that would be
awesome.


> > +- BUG_ON
> 
> BUG_ON() can return if the condition is false. So it doesn't look correct to
> deviate with the argument that the function doesn't give the control back...

+1


> > +
> > +   * - R16.3
> > + - Switch clauses not ending with the break statement are safe if an
> > +   explicit comment or pseudo-keyword indicating the fallthrough
> > intention
> > +   is present.
> > + - Tagged as `safe` for ECLAIR.
> > +
> >  * - R20.7
> >- Code violating Rule 20.7 is safe when macro parameters are used:
> >  (1) as function arguments;

[PATCH] docs/misra/rules.rst: add Rule 16.2

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini 

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 2b570af0e0..7cb9544a96 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -469,6 +469,15 @@ maintainers if you want to suggest a change.
  - In addition to break, also other flow control statements such as
continue, return, goto are allowed.
 
+   * - `Rule 16.2 
`_
+ - Required
+ - A switch label shall only be used when the most closely-enclosing
+   compound statement is the body of a switch statement
+ - The x86 emulator (xen/arch/x86/x86_emulate*) is exempt from
+   compliance with this rule. Efforts to make the x86 emulator
+   adhere to Rule 16.2 would result in increased complexity and
+   maintenance difficulty, and could potentially introduce bugs. 
+
* - `Rule 16.7 
`_
  - Required
  - A switch-expression shall not have essentially Boolean type

[PATCH v2] docs/misra/rules.rst: add more rules

[Stefano Stabellini]

Add the rules accepted in the last three MISRA C working group meetings.

Signed-off-by: Stefano Stabellini 
---
Changes in v2:
- remove 17.1 for now, to be a separate patch
- add a clarification comment for 17.7
---
 docs/misra/rules.rst | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 75921b9a34..2b570af0e0 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -462,6 +462,13 @@ maintainers if you want to suggest a change.
 
while(0) and while(1) and alike are allowed.
 
+   * - `Rule 16.3 
`_
+ - Required
+ - An unconditional break statement shall terminate every
+   switch-clause
+ - In addition to break, also other flow control statements such as
+   continue, return, goto are allowed.
+
* - `Rule 16.7 
`_
  - Required
  - A switch-expression shall not have essentially Boolean type
@@ -478,12 +485,27 @@ maintainers if you want to suggest a change.
have an explicit return statement with an expression
  -
 
+   * - `Rule 17.5 
`_
+ - Advisory
+ - The function argument corresponding to a parameter declared to
+   have an array type shall have an appropriate number of elements
+ -
+
* - `Rule 17.6 
`_
  - Mandatory
  - The declaration of an array parameter shall not contain the
static keyword between the [ ]
  -
 
+   * - `Rule 17.7 
`_
+ - Required
+ - The value returned by a function having non-void return type
+   shall be used
+ - Please beware that this rule has many violations in the Xen
+   codebase today, and its adoption is aspirational. However, when
+   submitting new patches please try to decrease the number of
+   violations when possible.
+
* - `Rule 18.3 
`_
  - Required
  - The relational operators > >= < and <= shall not be applied to objects 
of pointer type except where they point into the same object
@@ -498,6 +520,11 @@ maintainers if you want to suggest a change.
instances where Eclair is unable to verify that the code is valid
in regard to Rule 19.1. Caution reports are not violations.
 
+   * - `Rule 20.4 
`_
+ - Required
+ - A macro shall not be defined with the same name as a keyword
+ -
+
* - `Rule 20.7 
`_
  - Required
  - Expressions resulting from the expansion of macro parameters
@@ -506,6 +533,13 @@ maintainers if you want to suggest a change.
as function arguments, as macro arguments, array indices, lhs in
assignments
 
+   * - `Rule 20.9 
`_
+ - Required
+ - All identifiers used in the controlling expression of #if or
+   #elif preprocessing directives shall be #define'd before
+   evaluation
+ -
+
* - `Rule 20.13 
`_
  - Required
  - A line whose first token is # shall be a valid preprocessing
-- 
2.25.1

Re: [PATCH] docs/misra/rules.rst: add more rules

[Stefano Stabellini]

On Thu, 7 Dec 2023, Jan Beulich wrote:
> On 07.12.2023 03:42, Stefano Stabellini wrote:
> > On Wed, 6 Dec 2023, Jan Beulich wrote:
> >> On 06.12.2023 04:02, Stefano Stabellini wrote:
> >>> --- a/docs/misra/rules.rst
> >>> +++ b/docs/misra/rules.rst
> >>> @@ -462,11 +462,23 @@ maintainers if you want to suggest a change.
> >>>  
> >>> while(0) and while(1) and alike are allowed.
> >>>  
> >>> +   * - `Rule 16.3 
> >>> `_
> >>> + - Required
> >>> + - An unconditional break statement shall terminate every
> >>> +   switch-clause
> >>> + - In addition to break, also other flow control statements such as
> >>> +   continue, return, goto are allowed.
> >>> +
> >>> * - `Rule 16.7 
> >>> `_
> >>>   - Required
> >>>   - A switch-expression shall not have essentially Boolean type
> >>>   -
> >>>  
> >>> +   * - `Rule 17.1 
> >>> `_
> >>> + - Required
> >>> + - The features of  shall not be used
> >>> + -
> >>
> >> Did we really accept this without any constraint (warranting mentioning
> >> here)?
> > 
> > We agreed that in certain situations stdarg.h is OK to use and in those
> > cases we would add a deviation. Would you like me to add something to
> > that effect here? I could do that but it would sound a bit vague.  Also
> > if we want to specify a project-wide deviation it would be better
> > documented in docs/misra/deviations.rst. I would leave Rule 17.1 without
> > a note.
> 
> I can see your point, and I don't have a good suggestion on possible text.
> Still I wouldn't feel well ack-ing this in its present shape.

What about:

 - It is understood that in some limited circumstances  is
   appropriate to use, such as the implementation of printk. Those
   cases will be dealt with using deviations as usual, see
   docs/misra/deviations.rst and
   docs/misra/documenting-violations.rst.



> >>> @@ -478,12 +490,24 @@ maintainers if you want to suggest a change.
> >>> have an explicit return statement with an expression
> >>>   -
> >>>  
> >>> +   * - `Rule 17.5 
> >>> `_
> >>> + - Advisory
> >>> + - The function argument corresponding to a parameter declared to
> >>> +   have an array type shall have an appropriate number of elements
> >>> + -
> >>> +
> >>> * - `Rule 17.6 
> >>> `_
> >>>   - Mandatory
> >>>   - The declaration of an array parameter shall not contain the
> >>> static keyword between the [ ]
> >>>   -
> >>>  
> >>> +   * - `Rule 17.7 
> >>> `_
> >>> + - Required
> >>> + - The value returned by a function having non-void return type
> >>> +   shall be used
> >>> + -
> >>
> >> Same question here.
> > 
> > Here I was also thinking it might be good to add a comment. Maybe we could
> > add:
> > 
> >  - Please beware that this rule has many violations in the Xen
> >codebase today, and its adoption is aspirational. However, when
> >submitting new patches please try to decrease the number of
> >violations when possible.
> 
> Yea, I think this would be good to add.

I sent out a patch with this addition, and removing Rule 17.1 as that
one is a bit more complicated.

Re: [PATCH] MAINTAINERS: Hand over the release manager role to Oleksii Kurochko

[Henry Wang]

Hi Julien, Oleksii,

> On Dec 8, 2023, at 05:09, Oleksii  wrote:
> 
> Hi Julien and Henry,
> 
> On Thu, 2023-12-07 at 18:46 +, Julien Grall wrote:
>> Hi,
>> 
>> On 07/12/2023 16:20, Henry Wang wrote:
>>> I've finished the opportunity to do two releases (4.17 and 4.18)
>>> and Oleksii Kurochko has volunteered to be the next release
>>> manager.
>> 
>> Henry, thanks for your time as release manager.

Thank you Julien for your kind support as the release technician!

>> Oleksii, thanks for stepping up and good luck for the role!
> Thank you very much.
> 
> Just one question: Is it necessary to provide my ACK?

Oleksii: Yes please, your ack means you are happy with taking the role.

Also, any thing that you need, please don’t hesitate to reach out. I will stick
to the community (more as the code contributor though) so I will be more
than happy to help.

>>> Hand over the role to him by changing the maintainership of the
>>> CHANGELOG.md.
>>> 
>>> Signed-off-by: Henry Wang 
>> 
>> Acked-by: Julien Grall 
>> 
>> I didn't hear any objection during the community call. But I will
>> give 
>> until Tuesday morning (UK time) just in case. If there are none, then
>> I 
>> will commit it.

Thanks Julien.

Kind regards,
Henry

>> 
>> Cheers,
>> 
> 
> ~ Oleksii

xen | Failed pipeline for staging | bc4fe94a

[GitLab]

Pipeline #1099444749 has failed!

Project: xen ( https://gitlab.com/xen-project/xen )
Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging )

Commit: bc4fe94a ( 
https://gitlab.com/xen-project/xen/-/commit/bc4fe94a69d4dab103c37045d97e589ef75f8647
 )
Commit Message: tools/libs/evtchn: replace assert()s in stubdom...
Commit Author: Juergen Gross ( https://gitlab.com/jgross1 )
Committed by: Andrew Cooper ( https://gitlab.com/andyhhp )


Pipeline #1099444749 ( 
https://gitlab.com/xen-project/xen/-/pipelines/1099444749 ) triggered by Ganis 
( https://gitlab.com/ganis )
had 1 failed job.

Job #5706590895 ( https://gitlab.com/xen-project/xen/-/jobs/5706590895/raw )

Stage: test
Name: build-each-commit-gcc

-- 
You're receiving this email because of your account on gitlab.com.

[xen-unstable-smoke test] 184030: tolerable all pass - PUSHED

[osstest service owner]

flight 184030 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/184030/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  bc4fe94a69d4dab103c37045d97e589ef75f8647
baseline version:
 xen  d2b7c442b4a066bb670ee83e24800cabc415241d

Last test of basis   184026  2023-12-07 16:02:06 Z0 days
Testing same since   184030  2023-12-07 21:02:00 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Juergen Gross 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   d2b7c442b4..bc4fe94a69  bc4fe94a69d4dab103c37045d97e589ef75f8647 -> smoke

[ovmf test] 184028: all pass - PUSHED

[osstest service owner]

flight 184028 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/184028/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf eccdab611c01aa40b6cefcfbcb4d23e54b4c0ec6
baseline version:
 ovmf 238690a30d02d3f95f0355c88c35dc0e4232342a

Last test of basis   184027  2023-12-07 17:11:00 Z0 days
Testing same since   184028  2023-12-07 20:11:01 Z0 days1 attempts


People who touched revisions under test:
  Corvin Köhne 
  Gerd Hoffmann 
  Laszlo Ersek 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   238690a30d..eccdab611c  eccdab611c01aa40b6cefcfbcb4d23e54b4c0ec6 -> 
xen-tested-master

Re: [RFC PATCH] xen/arm: Add emulation of Debug Data Transfer Registers

[Stefano Stabellini]

On Thu, 7 Dec 2023, Julien Grall wrote:
> Hi Stefano,
> 
> On 05/12/2023 23:21, Stefano Stabellini wrote:
> > On Tue, 5 Dec 2023, Julien Grall wrote:
> > > I agree that crashing a guest is bad, but is lying to the domain really
> > > better? The consequence here is not that bad and hopefully it would be
> > > fairly
> > > easy to find. But this is not always the case. So I definitely would place
> > > a
> > > half-backed emulation more severe than a guest crash.
> > 
> > 
> > I see where Julien is coming from, but I would go with option two:
> > "emulate DCC the same way as KVM". That's because I don't think we can
> > get away with crashing the guest in all cases. Although the issue came
> > up with a Linux guest, it could have been triggered by a proprietary
> > operating system that we cannot change, and I think Xen should support
> > running unmodified operating systems.
> > 
> > If we go with a "half-backed emulation" solution, as Julien wrote, then
> > it is better to be more similar to other hypervisors, that's why I chose
> > option two instead of option three.
> > 
> > But at the same time I recognize the validity of Julien's words and it
> > makes me wonder if we should have a KCONFIG option or command line
> > option to switch the Xen behavior. We could use it to gate all the
> > "half-backed emulation" we do for compatibility.  Something like:
> > 
> > config PARTIAL_EMULATION
> >  bool "Partial Emulation"
> >  ---help---
> >Enables partial, not spec compliant, emulation of certain
> > register
> >  interfaces (e.g DCC UART) for guest compatibility. If you disable
> >  this option, Xen will crash the guest if the guest tries to access
> >  interfaces not fully emulated or virtualized.
> > 
> >  If you enable this option, the guest might misbehave due to non-spec
> >  compliant emulation done by Xen.
> 
> As I wrote to Ayan on Matrix today, I am not in favor of the emulation. Yet, I
> am not going to oppose (as in Nack it) if the other maintainers agree with it.

Thanks for being flexible


> The KConfig would be nice, the question is whether we want to (security)
> support such configuration? E.g. could this potentially introduce a security
> issue in the guest?

The important question is whether it could introduce a security issue in
Xen. If we think it wouldn't increase the attack surface significantly
then I would security support it otherwise not.


> Regarding the  emulation itself, I actually prefer 3 because at least the
> Linux drivers will be able to bail out rather than trying to use them.

I don't have a strong opinion between 2 and 3

[xen-unstable test] 184020: tolerable FAIL - PUSHED

[osstest service owner]

flight 184020 xen-unstable real [real]
flight 184029 xen-unstable real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/184020/
http://logs.test-lab.xenproject.org/osstest/logs/184029/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-credit1   8 xen-bootfail pass in 184029-retest

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-credit1 15 migrate-support-check fail in 184029 never pass
 test-armhf-armhf-xl-credit1 16 saverestore-support-check fail in 184029 never 
pass
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184005
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 184005
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184005
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 184005
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 184005
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 184005
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 184005
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 184005
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184005
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184005
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 184005
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184005
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  d4bfd3899886d0fbe259c20660dadb1e00170f2d
baseline version:
 xen  01da0aeecd41435cea8bd2fe0f547e0a474f6e45

Last test of basis   184005  2023-12-06 06:14:42 

Re: [PATCH] MAINTAINERS: Hand over the release manager role to Oleksii Kurochko

[Oleksii]

Hi Julien and Henry,

On Thu, 2023-12-07 at 18:46 +, Julien Grall wrote:
> Hi,
> 
> On 07/12/2023 16:20, Henry Wang wrote:
> > I've finished the opportunity to do two releases (4.17 and 4.18)
> > and Oleksii Kurochko has volunteered to be the next release
> > manager.
> 
> Henry, thanks for your time as release manager.
> Oleksii, thanks for stepping up and good luck for the role!
Thank you very much.

Just one question: Is it necessary to provide my ACK?

> 
> > Hand over the role to him by changing the maintainership of the
> > CHANGELOG.md.
> > 
> > Signed-off-by: Henry Wang 
> 
> Acked-by: Julien Grall 
> 
> I didn't hear any objection during the community call. But I will
> give 
> until Tuesday morning (UK time) just in case. If there are none, then
> I 
> will commit it.
> 
> Cheers,
> 

~ Oleksii

Re: [RFC PATCH v1 1/1] xen/Makefile: introduce ARCH_FIXED_CONFIG for randconfig

[Oleksii]

On Thu, 2023-12-07 at 20:17 +, Andrew Cooper wrote:
> On 07/12/2023 5:03 pm, Oleksii Kurochko wrote:
> > ARCH_FIXED_CONFIG is required in the case of randconfig
> > and CI for configs that aren't ready or are not
> > supposed to be implemented for specific architecture.
> > These configs should always be disabled to prevent randconfig
> > related tests from failing.
> > 
> > Signed-off-by: Oleksii Kurochko 
> > ---
> >  xen/Makefile | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/xen/Makefile b/xen/Makefile
> > index ca571103c8..8ae8fe1480 100644
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -336,11 +336,14 @@ ifeq ($(config-build),y)
> >  # *config targets only - make sure prerequisites are updated, and
> > descend
> >  # in tools/kconfig to make the *config target
> >  
> > +ARCH_FORCED_CONFIG :=
> > $(srctree)/arch/$(SRCARCH)/configs/randomforced.config
> > +
> >  # Create a file for KCONFIG_ALLCONFIG which depends on the
> > environment.
> >  # This will be use by kconfig targets
> > allyesconfig/allmodconfig/allnoconfig/randconfig
> >  filechk_kconfig_allconfig = \
> >  $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo
> > 'CONFIG_XSM_FLASK_POLICY=n';) \
> > -    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
> > +    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
> > +    $(if $(wildcard $(ARCH_FORCED_CONFIG)), cat
> > $(ARCH_FORCED_CONFIG);) ) \
> >  :
> >  
> >  .allconfig.tmp: FORCE
> 
> We already have infrastructure for this.  What's wrong with
> EXTRA_FIXED_RANDCONFIG?
Everything is fine; I don't know why there was only an issue with
CONFIG_GRANT_TABLE on PPC. On the RISC-V side, there were more configs
issues, prompting me to include all the configurations not implemented
for RISC-V in EXTRA_FIXED_RANDCONFIG. You can find the added
configurations in this commit:
https://lore.kernel.org/xen-devel/b4e85f8f58787b4d179022973ce25673d6b56e36.1700761381.git.oleksii.kuroc...@gmail.com/#Z31automation:gitlab-ci:build.yaml

One challenge is that the same configurations need to be added multiple
times for each build test using randconfig.

Another reason for this approach is a suggestion from Jan (probably I
misunderstood it), who proposed using a template to instruct randconfig
not to modify currently unnecessary configurations. You can find the
suggestion and discussion here:
https://lore.kernel.org/xen-devel/008d0c66-6816-4d12-9e1f-1878e982f...@suse.com/

Perhaps we could enhance the build script to fetch "fixed" configs from
the architecture-specific fixed-defconfig instead of modifying the
Makefile directly.

> 
> ---8<---
> 
> CI: Revert "automation: Drop ppc64le-*randconfig jobs", fix
> Randconfig
> with existing infrastructure
>     
> This reverts commit cbb71b95dd708b1e26899bbe1e7bf9a85081fd60.
> 
> Signed-off-by: Andrew Cooper 
> 
> diff --git a/automation/gitlab-ci/build.yaml
> b/automation/gitlab-ci/build.yaml
> index 32af30ccedc9..346d0400ed09 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -538,6 +538,7 @@ archlinux-current-gcc-riscv64-randconfig:
>  RANDCONFIG: y
>  EXTRA_FIXED_RANDCONFIG:
>    CONFIG_COVERAGE=n
> +  CONFIG_GRANT_TABLE=n
>  
>  archlinux-current-gcc-riscv64-debug-randconfig:
>    extends: .gcc-riscv64-cross-build-debug
> @@ -547,6 +548,7 @@ archlinux-current-gcc-riscv64-debug-randconfig:
>  RANDCONFIG: y
>  EXTRA_FIXED_RANDCONFIG:
>    CONFIG_COVERAGE=n
> +  CONFIG_GRANT_TABLE=n
>  
>  # Power cross-build
>  debian-bullseye-gcc-ppc64le:
> @@ -563,6 +565,26 @@ debian-bullseye-gcc-ppc64le-debug:
>  KBUILD_DEFCONFIG: ppc64_defconfig
>  HYPERVISOR_ONLY: y
>  
> +debian-bullseye-gcc-ppc64le-randconfig:
> +  extends: .gcc-ppc64le-cross-build
> +  variables:
> +    CONTAINER: debian:bullseye-ppc64le
> +    KBUILD_DEFCONFIG: ppc64_defconfig
> +    RANDCONFIG: y
> +    EXTRA_FIXED_RANDCONFIG:
> +  CONFIG_COVERAGE=n
> +  CONFIG_GRANT_TABLE=n
> +
> +debian-bullseye-gcc-ppc64le-debug-randconfig:
> +  extends: .gcc-ppc64le-cross-build-debug
> +  variables:
> +    CONTAINER: debian:bullseye-ppc64le
> +    KBUILD_DEFCONFIG: ppc64_defconfig
> +    RANDCONFIG: y
> +    EXTRA_FIXED_RANDCONFIG:
> +  CONFIG_COVERAGE=n
> +  CONFIG_GRANT_TABLE=n
> +
>  # Yocto test jobs
>  yocto-qemuarm64:
>    extends: .yocto-test-arm64
> 

~ Oleksii

Re: preparations for 4.17.3

[Andrew Cooper]

On 07/12/2023 7:06 am, Jan Beulich wrote:
> All,
>
> the release is about due. Please point out backports you find missing
> from the respective staging branch, but which you consider relevant.

e3c409d59ac8 "x86/x2apic: introduce a mixed physical/cluster mode"

There's now clear evidence on the "[PATCH] x86/x2apic: introduce a mixed
physical/cluster mode" that it fixes something on real systems.  At a
guess, a platform erratum concerning the use of external cluster mode
interrupts, but nevertheless its a genuine improvement too.

I've got backports to 4.17 and 4.13 easily to hand if you want.

~Andrew

Re: [RFC PATCH v1 1/1] xen/Makefile: introduce ARCH_FIXED_CONFIG for randconfig

[Andrew Cooper]

On 07/12/2023 5:03 pm, Oleksii Kurochko wrote:
> ARCH_FIXED_CONFIG is required in the case of randconfig
> and CI for configs that aren't ready or are not
> supposed to be implemented for specific architecture.
> These configs should always be disabled to prevent randconfig
> related tests from failing.
>
> Signed-off-by: Oleksii Kurochko 
> ---
>  xen/Makefile | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/xen/Makefile b/xen/Makefile
> index ca571103c8..8ae8fe1480 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -336,11 +336,14 @@ ifeq ($(config-build),y)
>  # *config targets only - make sure prerequisites are updated, and descend
>  # in tools/kconfig to make the *config target
>  
> +ARCH_FORCED_CONFIG := $(srctree)/arch/$(SRCARCH)/configs/randomforced.config
> +
>  # Create a file for KCONFIG_ALLCONFIG which depends on the environment.
>  # This will be use by kconfig targets 
> allyesconfig/allmodconfig/allnoconfig/randconfig
>  filechk_kconfig_allconfig = \
>  $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo 
> 'CONFIG_XSM_FLASK_POLICY=n';) \
> -$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
> +$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
> +$(if $(wildcard $(ARCH_FORCED_CONFIG)), cat $(ARCH_FORCED_CONFIG);) ) \
>  :
>  
>  .allconfig.tmp: FORCE

We already have infrastructure for this.  What's wrong with
EXTRA_FIXED_RANDCONFIG?

---8<---

CI: Revert "automation: Drop ppc64le-*randconfig jobs", fix Randconfig
with existing infrastructure
    
This reverts commit cbb71b95dd708b1e26899bbe1e7bf9a85081fd60.

Signed-off-by: Andrew Cooper 

diff --git a/automation/gitlab-ci/build.yaml
b/automation/gitlab-ci/build.yaml
index 32af30ccedc9..346d0400ed09 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -538,6 +538,7 @@ archlinux-current-gcc-riscv64-randconfig:
 RANDCONFIG: y
 EXTRA_FIXED_RANDCONFIG:
   CONFIG_COVERAGE=n
+  CONFIG_GRANT_TABLE=n
 
 archlinux-current-gcc-riscv64-debug-randconfig:
   extends: .gcc-riscv64-cross-build-debug
@@ -547,6 +548,7 @@ archlinux-current-gcc-riscv64-debug-randconfig:
 RANDCONFIG: y
 EXTRA_FIXED_RANDCONFIG:
   CONFIG_COVERAGE=n
+  CONFIG_GRANT_TABLE=n
 
 # Power cross-build
 debian-bullseye-gcc-ppc64le:
@@ -563,6 +565,26 @@ debian-bullseye-gcc-ppc64le-debug:
 KBUILD_DEFCONFIG: ppc64_defconfig
 HYPERVISOR_ONLY: y
 
+debian-bullseye-gcc-ppc64le-randconfig:
+  extends: .gcc-ppc64le-cross-build
+  variables:
+    CONTAINER: debian:bullseye-ppc64le
+    KBUILD_DEFCONFIG: ppc64_defconfig
+    RANDCONFIG: y
+    EXTRA_FIXED_RANDCONFIG:
+  CONFIG_COVERAGE=n
+  CONFIG_GRANT_TABLE=n
+
+debian-bullseye-gcc-ppc64le-debug-randconfig:
+  extends: .gcc-ppc64le-cross-build-debug
+  variables:
+    CONTAINER: debian:bullseye-ppc64le
+    KBUILD_DEFCONFIG: ppc64_defconfig
+    RANDCONFIG: y
+    EXTRA_FIXED_RANDCONFIG:
+  CONFIG_COVERAGE=n
+  CONFIG_GRANT_TABLE=n
+
 # Yocto test jobs
 yocto-qemuarm64:
   extends: .yocto-test-arm64

[ovmf test] 184027: all pass - PUSHED

[osstest service owner]

flight 184027 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/184027/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 238690a30d02d3f95f0355c88c35dc0e4232342a
baseline version:
 ovmf 553dfb0f57ae8a666938873cf836a33549568c87

Last test of basis   184023  2023-12-07 10:14:25 Z0 days
Testing same since   184027  2023-12-07 17:11:00 Z0 days1 attempts


People who touched revisions under test:
  Corvin Köhne 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   553dfb0f57..238690a30d  238690a30d02d3f95f0355c88c35dc0e4232342a -> 
xen-tested-master

xen | Failed pipeline for staging | d2b7c442

[GitLab]

Pipeline #1099081266 has failed!

Project: xen ( https://gitlab.com/xen-project/xen )
Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging )

Commit: d2b7c442 ( 
https://gitlab.com/xen-project/xen/-/commit/d2b7c442b4a066bb670ee83e24800cabc415241d
 )
Commit Message: CODING_STYLE: Add a section of the naming conve...
Commit Author: Julien Grall


Pipeline #1099081266 ( 
https://gitlab.com/xen-project/xen/-/pipelines/1099081266 ) triggered by Ganis 
( https://gitlab.com/ganis )
had 12 failed jobs.

Job #5704236009 ( https://gitlab.com/xen-project/xen/-/jobs/5704236009/raw )

Stage: build
Name: alpine-3.18-gcc-debug
Job #5704236006 ( https://gitlab.com/xen-project/xen/-/jobs/5704236006/raw )

Stage: build
Name: alpine-3.18-gcc
Job #5704236300 ( https://gitlab.com/xen-project/xen/-/jobs/5704236300/raw )

Stage: build
Name: archlinux-gcc
Job #5704236325 ( https://gitlab.com/xen-project/xen/-/jobs/5704236325/raw )

Stage: build
Name: debian-bookworm-gcc
Job #5704236327 ( https://gitlab.com/xen-project/xen/-/jobs/5704236327/raw )

Stage: build
Name: debian-bookworm-gcc-debug
Job #5704236339 ( https://gitlab.com/xen-project/xen/-/jobs/5704236339/raw )

Stage: build
Name: ubuntu-trusty-gcc
Job #5704236388 ( https://gitlab.com/xen-project/xen/-/jobs/5704236388/raw )

Stage: build
Name: opensuse-tumbleweed-gcc-debug
Job #5704236386 ( https://gitlab.com/xen-project/xen/-/jobs/5704236386/raw )

Stage: build
Name: opensuse-tumbleweed-gcc
Job #5704236391 ( https://gitlab.com/xen-project/xen/-/jobs/5704236391/raw )

Stage: test
Name: build-each-commit-gcc
Job #5704236303 ( https://gitlab.com/xen-project/xen/-/jobs/5704236303/raw )

Stage: build
Name: archlinux-gcc-debug
Job #5704236333 ( https://gitlab.com/xen-project/xen/-/jobs/5704236333/raw )

Stage: build
Name: debian-bookworm-32-gcc-debug
Job #5704236342 ( https://gitlab.com/xen-project/xen/-/jobs/5704236342/raw )

Stage: build
Name: ubuntu-trusty-gcc-debug

-- 
You're receiving this email because of your account on gitlab.com.

Re: [PATCH] Mini-OS: don't use objcopy --dump-section

[Andrew Cooper]

On 06/12/2023 4:17 pm, Juergen Gross wrote:
> The objcopy option "--dump-section" isn't supported in binutils before
> version 2.25, which are still supported by the Xen build system.
>
> Avoid that by using the "-O binary" format together with
> "--only-section". This requires to set the "alloc" section flag.
>
> Fixes: 33411a11f848 ("Mini-OS: hide all symbols not exported via 
> EXPORT_SYMBOLS()")
> Signed-off-by: Juergen Gross 

Reviewed-by: Andrew Cooper 

And FAOD, I'm taking this, plus a bump to the minios rev right now to
unbreak Gitlab CI, as it's been broken for 2 now.

~Andrew

Re: [PATCH] MAINTAINERS: Hand over the release manager role to Oleksii Kurochko

[Julien Grall]

Hi,

On 07/12/2023 16:20, Henry Wang wrote:

I've finished the opportunity to do two releases (4.17 and 4.18)
and Oleksii Kurochko has volunteered to be the next release manager.


Henry, thanks for your time as release manager.
Oleksii, thanks for stepping up and good luck for the role!


Hand over the role to him by changing the maintainership of the
CHANGELOG.md.

Signed-off-by: Henry Wang 


Acked-by: Julien Grall 

I didn't hear any objection during the community call. But I will give 
until Tuesday morning (UK time) just in case. If there are none, then I 
will commit it.


Cheers,

--
Julien Grall

[xen-unstable-smoke test] 184026: tolerable all pass - PUSHED

[osstest service owner]

flight 184026 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/184026/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  d2b7c442b4a066bb670ee83e24800cabc415241d
baseline version:
 xen  02d0a615b32d03702f79807fa5e88f0cf78dde84

Last test of basis   184025  2023-12-07 13:02:05 Z0 days
Testing same since   184026  2023-12-07 16:02:06 Z0 days1 attempts


People who touched revisions under test:
  George Dunlap 
  Juergen Gross 
  Julien Grall 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   02d0a615b3..d2b7c442b4  d2b7c442b4a066bb670ee83e24800cabc415241d -> smoke

Re: [PATCH v2 07/39] xen/riscv: introduce arch-riscv/hvm/save.h

[Shawn Anastasio]

On 12/5/23 9:59 AM, Jan Beulich wrote:
> On 24.11.2023 11:30, Oleksii Kurochko wrote:
>> --- a/xen/include/public/hvm/save.h
>> +++ b/xen/include/public/hvm/save.h
>> @@ -91,6 +91,8 @@ DECLARE_HVM_SAVE_TYPE(END, 0, struct hvm_save_end);
>>  #include "../arch-arm/hvm/save.h"
>>  #elif defined(__powerpc64__)
>>  #include "../arch-ppc.h"
>> +#elif defined(__riscv)
>> +#include "../arch-riscv/hvm/save.h"
>>  #else
>>  #error "unsupported architecture"
>>  #endif
> 
> The PPC part here looks bogus altogether. Shawn?
>

I think my original intention here was to avoid creating yet another
empty header while still having a place to put PPC-specific definitions
that might be required.

See as how the ARM file is entirely empty though, I doubt we'll be any
different, so this could definitely be dropped.

> Jan

Thanks,
Shawn

Re: [XEN PATCH] automation/eclair_analysis: file exclusion automation

[Julien Grall]

Hi Nicola,

On 07/12/2023 17:53, Nicola Vetrini wrote:

On 2023-12-07 18:08, Julien Grall wrote:

On 07/12/2023 11:39, Nicola Vetrini wrote:

+-doc_begin="libfdt is out of scope."
+-file_tag+={out_of_scope,"^xen/include/xen/libfdt/.*$"}


AFAICT, before this was marked as "adopted". But this is now moved to 
"out_of_scope". Can you explain why?


It also feels somewhat unrelated to the rest of the patch.

Cheers,


I mistakenly changed the tag. It is not unrelated, as it't not part of 
exclude-list.json (perhaps unintentionally). The manual exclusions that 
remain in out_of_scope.ecl are there for this reason, since I wanted to 
keep the set of excluded files as it was before.


Given that common/libfdt/* is part of the exclude-list.json, I can't see 
why include/xen/libfdt/* are not. So can you add it?


Cheers,

--
Julien Grall

Re: [XEN PATCH] automation/eclair_analysis: file exclusion automation

[Nicola Vetrini]

On 2023-12-07 18:08, Julien Grall wrote:

On 07/12/2023 11:39, Nicola Vetrini wrote:

+-doc_begin="libfdt is out of scope."
+-file_tag+={out_of_scope,"^xen/include/xen/libfdt/.*$"}


AFAICT, before this was marked as "adopted". But this is now moved to 
"out_of_scope". Can you explain why?


It also feels somewhat unrelated to the rest of the patch.

Cheers,


I mistakenly changed the tag. It is not unrelated, as it't not part of 
exclude-list.json (perhaps unintentionally). The manual exclusions that 
remain in out_of_scope.ecl are there for this reason, since I wanted to 
keep the set of excluded files as it was before.


--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

xen | Failed pipeline for staging | 25147005

[GitLab]

Pipeline #1098955890 has failed!

Project: xen ( https://gitlab.com/xen-project/xen )
Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging )

Commit: 25147005 ( 
https://gitlab.com/xen-project/xen/-/commit/25147005daf5a4e121b96496d6d208fac05fca35
 )
Commit Message: xen/sched: do some minor cleanup of sched_move_...
Commit Author: Juergen Gross ( https://gitlab.com/jgross1 )
Committed by: George Dunlap


Pipeline #1098955890 ( 
https://gitlab.com/xen-project/xen/-/pipelines/1098955890 ) triggered by Ganis 
( https://gitlab.com/ganis )
had 12 failed jobs.

Job #5703366092 ( https://gitlab.com/xen-project/xen/-/jobs/5703366092/raw )

Stage: build
Name: archlinux-gcc
Job #5703366143 ( https://gitlab.com/xen-project/xen/-/jobs/5703366143/raw )

Stage: build
Name: debian-bookworm-32-gcc-debug
Job #5703366193 ( https://gitlab.com/xen-project/xen/-/jobs/5703366193/raw )

Stage: build
Name: opensuse-tumbleweed-gcc
Job #5703366199 ( https://gitlab.com/xen-project/xen/-/jobs/5703366199/raw )

Stage: test
Name: build-each-commit-gcc
Job #5703366195 ( https://gitlab.com/xen-project/xen/-/jobs/5703366195/raw )

Stage: build
Name: opensuse-tumbleweed-gcc-debug
Job #5703366130 ( https://gitlab.com/xen-project/xen/-/jobs/5703366130/raw )

Stage: build
Name: debian-bookworm-gcc
Job #5703366094 ( https://gitlab.com/xen-project/xen/-/jobs/5703366094/raw )

Stage: build
Name: archlinux-gcc-debug
Job #5703366149 ( https://gitlab.com/xen-project/xen/-/jobs/5703366149/raw )

Stage: build
Name: ubuntu-trusty-gcc
Job #5703365823 ( https://gitlab.com/xen-project/xen/-/jobs/5703365823/raw )

Stage: build
Name: alpine-3.18-gcc
Job #5703365831 ( https://gitlab.com/xen-project/xen/-/jobs/5703365831/raw )

Stage: build
Name: alpine-3.18-gcc-debug
Job #5703366151 ( https://gitlab.com/xen-project/xen/-/jobs/5703366151/raw )

Stage: build
Name: ubuntu-trusty-gcc-debug
Job #5703366134 ( https://gitlab.com/xen-project/xen/-/jobs/5703366134/raw )

Stage: build
Name: debian-bookworm-gcc-debug

-- 
You're receiving this email because of your account on gitlab.com.

Re: [XEN PATCH] automation/eclair_analysis: file exclusion automation

[Julien Grall]

On 07/12/2023 11:39, Nicola Vetrini wrote:

+-doc_begin="libfdt is out of scope."
+-file_tag+={out_of_scope,"^xen/include/xen/libfdt/.*$"}


AFAICT, before this was marked as "adopted". But this is now moved to 
"out_of_scope". Can you explain why?


It also feels somewhat unrelated to the rest of the patch.

Cheers,

--
Julien Grall

[RFC PATCH v1 0/1] ARCH_FIXED_CONFIG introduction for randconfig

[Oleksii Kurochko]

Brief Overview:
In the earlier patch series [1], it was introduced a comprehensive set of 
changes
enabling a full Xen build for RISC-V.
This early support primarily provides the minimum stubs required for the RISC-V
Xen build. At this stage of development, many configs are deemed unnecessary and
are expected to be disabled.

Without ARCH_FIXED_CONFIG (or an alternative mechanism), the alternative is
updating CI's build.yaml in multiple instances with the same configs,
mirroring the approach taken in [1] to prevent the inadvertent activation of
unsupported configs.

For example, in scenarios like dom0less, we can exclude grant tables by setting
"CONFIG_GRANT_TABLE=n" in ARCH_FIXED_CONFIG. This eliminates the need for 
intricate
modifications to Kconfig configurations with conditions like "depends on X86 || 
ARM"
or the introduction of HAS_* conditions followed by Kconfig updates with
"depends on HAS_*," as illustrated in examples [2] or [3].

It might be useful for other architectures as well, especially for PPC, which is
currently under development.

There are several open questions:
- Does introduction of ARCH_FIXED_CONFIG make sense?
- Should ARCH_FIXED_CONFIG be re-used for *defconfig?

[1] 
https://lore.kernel.org/xen-devel/b4e85f8f58787b4d179022973ce25673d6b56e36.1700761381.git.oleksii.kuroc...@gmail.com/
[2] 
https://lore.kernel.org/xen-devel/cdc20255540a66ba0b6946ac6d48c11029cd3385.1701453087.git.oleksii.kuroc...@gmail.com/
[3] 
https://lore.kernel.org/xen-devel/d42a34866edc70a12736b5c6976aa1b44b4ebd8a.1701453087.git.oleksii.kuroc...@gmail.com/

Oleksii Kurochko (1):
  xen/Makefile: introduce ARCH_FIXED_CONFIG for randconfig

 xen/Makefile | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.43.0

[RFC PATCH v1 1/1] xen/Makefile: introduce ARCH_FIXED_CONFIG for randconfig

[Oleksii Kurochko]

ARCH_FIXED_CONFIG is required in the case of randconfig
and CI for configs that aren't ready or are not
supposed to be implemented for specific architecture.
These configs should always be disabled to prevent randconfig
related tests from failing.

Signed-off-by: Oleksii Kurochko 
---
 xen/Makefile | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index ca571103c8..8ae8fe1480 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -336,11 +336,14 @@ ifeq ($(config-build),y)
 # *config targets only - make sure prerequisites are updated, and descend
 # in tools/kconfig to make the *config target
 
+ARCH_FORCED_CONFIG := $(srctree)/arch/$(SRCARCH)/configs/randomforced.config
+
 # Create a file for KCONFIG_ALLCONFIG which depends on the environment.
 # This will be use by kconfig targets 
allyesconfig/allmodconfig/allnoconfig/randconfig
 filechk_kconfig_allconfig = \
 $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo 
'CONFIG_XSM_FLASK_POLICY=n';) \
-$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
+$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
+$(if $(wildcard $(ARCH_FORCED_CONFIG)), cat $(ARCH_FORCED_CONFIG);) ) \
 :
 
 .allconfig.tmp: FORCE
-- 
2.43.0

Re: [RFC PATCH] xen/arm: Add emulation of Debug Data Transfer Registers

[Julien Grall]

Hi Stefano,

On 05/12/2023 23:21, Stefano Stabellini wrote:

On Tue, 5 Dec 2023, Julien Grall wrote:

I agree that crashing a guest is bad, but is lying to the domain really
better? The consequence here is not that bad and hopefully it would be fairly
easy to find. But this is not always the case. So I definitely would place a
half-backed emulation more severe than a guest crash.



I see where Julien is coming from, but I would go with option two:
"emulate DCC the same way as KVM". That's because I don't think we can
get away with crashing the guest in all cases. Although the issue came
up with a Linux guest, it could have been triggered by a proprietary
operating system that we cannot change, and I think Xen should support
running unmodified operating systems.

If we go with a "half-backed emulation" solution, as Julien wrote, then
it is better to be more similar to other hypervisors, that's why I chose
option two instead of option three.

But at the same time I recognize the validity of Julien's words and it
makes me wonder if we should have a KCONFIG option or command line
option to switch the Xen behavior. We could use it to gate all the
"half-backed emulation" we do for compatibility.  Something like:

config PARTIAL_EMULATION
 bool "Partial Emulation"
 ---help---
  
 Enables partial, not spec compliant, emulation of certain register

 interfaces (e.g DCC UART) for guest compatibility. If you disable
 this option, Xen will crash the guest if the guest tries to access
 interfaces not fully emulated or virtualized.

 If you enable this option, the guest might misbehave due to non-spec
 compliant emulation done by Xen.


As I wrote to Ayan on Matrix today, I am not in favor of the emulation. 
Yet, I am not going to oppose (as in Nack it) if the other maintainers 
agree with it.


The KConfig would be nice, the question is whether we want to (security) 
support such configuration? E.g. could this potentially introduce a 
security issue in the guest?


Regarding the  emulation itself, I actually prefer 3 because at least 
the Linux drivers will be able to bail out rather than trying to use them.


Cheers,

--
Julien Grall

Re: [PATCH] xen/xenbus: client: fix kernel-doc comments

[Randy Dunlap]

On 12/7/23 01:27, Juergen Gross wrote:
> On 06.12.23 19:17, Randy Dunlap wrote:
>> Correct function kernel-doc notation to prevent warnings from
>> scripts/kernel-doc.
>>

>>
>> Signed-off-by: Randy Dunlap 
>> Cc: Juergen Gross 
>> Cc: Stefano Stabellini 
>> Cc: Oleksandr Tyshchenko 
>> Cc: xen-devel@lists.xenproject.org
> 
> Reviewed-by: Juergen Gross 
> 
> with one nit below (can be fixed while committing) ...

Ah yes, thanks.

-- 
~Randy

Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3

[Julien Grall]

Hi again,

On 07/12/2023 09:08, Federico Serafini wrote:

MISRA C:2012 Rule 16.3 states that an unconditional break statement
shall terminate every switch-clause.

Update ECLAIR configuration to take into account:
- continue, goto, return statements;
- functions and macros that do not give the control back;
- fallthrough comments and pseudo-keywords.

Update docs/misra/deviations.rst accordingly.

Signed-off-by: Federico Serafini 
---
  .../eclair_analysis/ECLAIR/deviations.ecl | 18 ++
  docs/misra/deviations.rst | 24 +++
  2 files changed, 42 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index b0c79741b5..df0b58a010 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -321,6 +321,24 @@ statements are deliberate"
  -config=MC3R1.R14.3,statements={deliberate , "wrapped(any(),node(if_stmt))" }
  -doc_end
  
+#

+# Series 16.
+#
+
+-doc_begin="Switch clauses ending with continue, goto, return statements are 
safe."
+-config=MC3R1.R16.3,terminals+={safe, 
"node(continue_stmt||goto_stmt||return_stmt)"}
+-doc_end
+
+-doc_begin="Switch clauses not ending with the break statement are safe if a 
function/macro that does not give the control back is present."
+-config=MC3R1.R16.3,terminals+={safe, 
"call(decl(name(__builtin_unreachable||do_unexpected_trap||fatal_trap||machine_halt||machine_restart||maybe_reboot||panic)))"}
+-config=MC3R1.R16.3,terminals+={safe,"macro(name(BUG||BUG_ON))"}
+-doc_end
+
+-doc_begin="Switch clauses not ending with the break statement are safe if an 
explicit comment or pseudo-keyword indicating the fallthrough intention is present."
+-config=MC3R1.R16.3,reports+={safe, 
"any_area(any_loc(any_exp(text(^(?s).*([fF]all[- ]?[tT]hrough|FALL[- 
]?THROUGH).*$,0..1"}
+-config=MC3R1.R16.3,reports+={safe, "any_area(text(^(?s).*([fF]all[- 
]?[tT]hrough|FALL[- ]?THROUGH).*$,0..1))"}
+-doc_end
+
  #
  # Series 20.
  #
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 6e7c4f25b8..fecd2bae8e 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -270,6 +270,30 @@ Deviations related to MISRA C:2012 Rules:
 statements are deliberate.
   - Project-wide deviation; tagged as `disapplied` for ECLAIR.
  
+   * - R16.3

+ - Switch clauses ending with continue, goto, return statements are safe.
+ - Tagged as `safe` for ECLAIR.
+
+   * - R16.3
+ - Switch clauses not ending with the break statement are safe if a
+   function/macro that does not give the control back is present.
+ - Tagged as `safe` for ECLAIR, such functions/macros are:
+- __builtin_unreachable
+- do_unexpected_trap
+- fatal_trap
+- machine_halt
+- machine_restart
+- maybe_reboot
+- panic
+- BUG
+- BUG_ON
+
+   * - R16.3
+ - Switch clauses not ending with the break statement are safe if an
+   explicit comment or pseudo-keyword indicating the fallthrough intention
+   is present.


One more thing. This is not explicit which comment should be added. But 
would should deprecate the comment in favor of "fallthrough".


The deviation should have it written down (similar to SAF-1 for rule 8.4).

Cheers,

--
Julien Grall

Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3

[Julien Grall]

Hi Federico,

On 07/12/2023 09:08, Federico Serafini wrote:

MISRA C:2012 Rule 16.3 states that an unconditional break statement
shall terminate every switch-clause.

Update ECLAIR configuration to take into account:
- continue, goto, return statements;
- functions and macros that do not give the control back;
- fallthrough comments and pseudo-keywords.

Update docs/misra/deviations.rst accordingly.

Signed-off-by: Federico Serafini 
---
  .../eclair_analysis/ECLAIR/deviations.ecl | 18 ++
  docs/misra/deviations.rst | 24 +++
  2 files changed, 42 insertions(+)


It would be good that this is depending on to be accepted:

https://lore.kernel.org/alpine.DEB.2.22.394.2312051859440.110490@ubuntu-linux-20-04-desktop.



diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index b0c79741b5..df0b58a010 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -321,6 +321,24 @@ statements are deliberate"
  -config=MC3R1.R14.3,statements={deliberate , "wrapped(any(),node(if_stmt))" }
  -doc_end
  
+#

+# Series 16.
+#
+
+-doc_begin="Switch clauses ending with continue, goto, return statements are 
safe."
+-config=MC3R1.R16.3,terminals+={safe, 
"node(continue_stmt||goto_stmt||return_stmt)"}
+-doc_end
+
+-doc_begin="Switch clauses not ending with the break statement are safe if a 
function/macro that does not give the control back is present."
+-config=MC3R1.R16.3,terminals+={safe, 
"call(decl(name(__builtin_unreachable||do_unexpected_trap||fatal_trap||machine_halt||machine_restart||maybe_reboot||panic)))"}
+-config=MC3R1.R16.3,terminals+={safe,"macro(name(BUG||BUG_ON))"}
+-doc_end
+
+-doc_begin="Switch clauses not ending with the break statement are safe if an 
explicit comment or pseudo-keyword indicating the fallthrough intention is present."
+-config=MC3R1.R16.3,reports+={safe, 
"any_area(any_loc(any_exp(text(^(?s).*([fF]all[- ]?[tT]hrough|FALL[- 
]?THROUGH).*$,0..1"}
+-config=MC3R1.R16.3,reports+={safe, "any_area(text(^(?s).*([fF]all[- 
]?[tT]hrough|FALL[- ]?THROUGH).*$,0..1))"}


This is not trivial to read. Can you document the exhaustive list of 
keywords you are actually trying to deviate on? Also, did you consider 
to harmonize to only a few?



+-doc_end
+
  #
  # Series 20.
  #
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 6e7c4f25b8..fecd2bae8e 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -270,6 +270,30 @@ Deviations related to MISRA C:2012 Rules:
 statements are deliberate.
   - Project-wide deviation; tagged as `disapplied` for ECLAIR.
  
+   * - R16.3

+ - Switch clauses ending with continue, goto, return statements are safe.
+ - Tagged as `safe` for ECLAIR.
+
+   * - R16.3
+ - Switch clauses not ending with the break statement are safe if a
+   function/macro that does not give the control back is present.
+ - Tagged as `safe` for ECLAIR, such functions/macros are:
+- __builtin_unreachable
+- do_unexpected_trap
+- fatal_trap
+- machine_halt
+- machine_restart
+- maybe_reboot
+- panic
+- BUG


To me, it seems to be odd to deviate R16.3 by function. Some of those 
have an attribute noreboot. Can this be used?



+- BUG_ON


BUG_ON() can return if the condition is false. So it doesn't look 
correct to deviate with the argument that the function doesn't give the 
control back...



+
+   * - R16.3
+ - Switch clauses not ending with the break statement are safe if an
+   explicit comment or pseudo-keyword indicating the fallthrough intention
+   is present.
+ - Tagged as `safe` for ECLAIR.
+
 * - R20.7
   - Code violating Rule 20.7 is safe when macro parameters are used:
 (1) as function arguments;


Cheers,

--
Julien Grall

[PATCH] MAINTAINERS: Hand over the release manager role to Oleksii Kurochko

[Henry Wang]

I've finished the opportunity to do two releases (4.17 and 4.18)
and Oleksii Kurochko has volunteered to be the next release manager.
Hand over the role to him by changing the maintainership of the
CHANGELOG.md.

Signed-off-by: Henry Wang 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0fcf5a6f36..702032cc12 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -278,7 +278,7 @@ S:  Supported
 F: xen/drivers/passthrough/arm/smmu-v3.c
 
 Change Log
-M: Henry Wang 
+M: Oleksii Kurochko 
 R: Community Manager 
 S: Maintained
 F: CHANGELOG.md
-- 
2.25.1

Re: [PATCH v2 17/39] xen/riscv: introduce asm/atomic.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> From: Bobby Eshleman 
> 
> Signed-off-by: Oleksii Kurochko 
> ---
> Changes in V2:
>  - Change an author of commit. I got this header from Bobby's old repo.

Not sure how to deal with that when there's not also an S-o-b.

> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/atomic.h
> @@ -0,0 +1,375 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Taken and modified from Linux.
> + * 
> + * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved.
> + * Copyright (C) 2012 Regents of the University of California
> + * Copyright (C) 2017 SiFive
> + * Copyright (C) 2021 Vates SAS
> + */
> +
> +#ifndef _ASM_RISCV_ATOMIC_H
> +#define _ASM_RISCV_ATOMIC_H
> +
> +#include 
> +#include 

This and ...

> +#include 
> +#include 

.. this header are only introduced later. Bad ordering of the series?

> +#include 
> +
> +void __bad_atomic_size(void);
> +
> +static always_inline void read_atomic_size(const volatile void *p,
> +   void *res,
> +   unsigned int size)
> +{
> +switch ( size ) {

Nit (style): Brace on its own line (again further down).

> +case 1: *(uint8_t *)res = readb((uint8_t *)p); break;
> +case 2: *(uint16_t *)res = readw((uint16_t *)p); break;
> +case 4: *(uint32_t *)res = readl((uint32_t *)p); break;
> +case 8: *(uint32_t *)res  = readq((uint64_t *)p); break;

Please don't cast away const-ness.

> +default: __bad_atomic_size(); break;
> +}
> +}
> +
> +#define read_atomic(p) ({   \
> +union { typeof(*p) val; char c[0]; } x_;\

Hmm, you avoid leading underscores here, but then ...

> +read_atomic_size(p, x_.c, sizeof(*p));  \
> +x_.val; \
> +})
> +
> +
> +#define write_atomic(p, x) ({   \
> +typeof(*p) __x = (x);   \

... they're still there here.

> +switch ( sizeof(*p) ) { \
> +case 1: writeb((uint8_t)__x,  (uint8_t *)  p); break;  \
> +case 2: writew((uint16_t)__x, (uint16_t *) p); break;  \
> +case 4: writel((uint32_t)__x, (uint32_t *) p); break;  \
> +case 8: writeq((uint64_t)__x, (uint64_t *) p); break;  \
> +default: __bad_atomic_size(); break;\
> +}   \
> +__x;\
> +})
> +
> +/* TODO: Fix this */
> +#define add_sized(p, x) ({  \
> +typeof(*(p)) __x = (x); \
> +switch ( sizeof(*(p)) ) \
> +{   \
> +case 1: writeb(read_atomic(p) + __x, (uint8_t *)(p)); break;\
> +case 2: writew(read_atomic(p) + __x, (uint16_t *)(p)); break;   \
> +case 4: writel(read_atomic(p) + __x, (uint32_t *)(p)); break;   \

Instead of this, considering the comment perhaps better just BUG()?

> +default: __bad_atomic_size(); break;\
> +}   \
> +})
> +
> +/*
> + *  __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving
> + *   non-scalar types unchanged.
> + *
> + * Prefer C11 _Generic for better compile-times and simpler code. Note: 
> 'char'
> + * is not type-compatible with 'signed char', and we define a separate case.
> + */
> +#define __scalar_type_to_expr_cases(type)   \
> +unsigned type:  (unsigned type)0,   \
> +signed type:(signed type)0
> +
> +#define __unqual_scalar_typeof(x) typeof(   \
> +_Generic((x),   \

I think you still owe us an update to ./README, clarifying what compiler 
versions
may be used for building RISC-V. Unless of course all that exist support 
_Generic
(which then would be nice to say in the description).

> +char:  (char)0, \
> +__scalar_type_to_expr_cases(char),  \
> +__scalar_type_to_expr_cases(short), \
> +__scalar_type_to_expr_cases(int),   \
> +__scalar_type_to_expr_cases(long),  \
> +__scalar_type_to_expr_cases(long long), \
> +default: (x)))
> +
> +#define READ_ONCE(x)  (*(const volatile __unqual_scalar_typeof(x) *)&(x))
> +#define WRITE_ONCE(x, val)  \
> +do {\
> +*(volatile typeof(x) *)&(x) 

[xen-unstable-smoke test] 184025: tolerable all pass - PUSHED

[osstest service owner]

flight 184025 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/184025/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  02d0a615b32d03702f79807fa5e88f0cf78dde84
baseline version:
 xen  dbe69e1c8555b40a43cde482615501eb8515ab80

Last test of basis   184022  2023-12-07 08:00:34 Z0 days
Testing same since   184025  2023-12-07 13:02:05 Z0 days1 attempts


People who touched revisions under test:
  Juergen Gross 
  Julien Grall 
  Michal Orzel 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   dbe69e1c85..02d0a615b3  02d0a615b32d03702f79807fa5e88f0cf78dde84 -> smoke

Re: [PATCH v2 16/39] xen/riscv: introduce asm/smp.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/smp.h
> @@ -0,0 +1,23 @@
> +#ifndef __ASM_RISCV_SMP_H
> +#define __ASM_RISCV_SMP_H
> +
> +#ifndef __ASSEMBLY__
> +#include 
> +#include 
> +#endif

If you want this to be possible to include from assembly files (I don't
know why you would want that), ...

> +DECLARE_PER_CPU(cpumask_var_t, cpu_sibling_mask);
> +DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask);

... these two would also need to live inside the #ifdef. Otherwise the
#ifdef wants dropping.

> +#define cpu_is_offline(cpu) unlikely(!cpu_online(cpu))

Seeing this is now the 4th instance, I guess we want to move it to
xen/smp.h. I'll try to remember making a patch.

> +/*
> + * Do we, for platform reasons, need to actually keep CPUs online when we
> + * would otherwise prefer them to be off?
> + */
> +#define park_offline_cpus false
> +
> +/* TODO: need to be implemeted */
> +#define smp_processor_id() (0)
> +
> +#endif
> \ No newline at end of file

You want to take care of this.

Jan

Re: [PATCH v2 15/39] xen/riscv: introduce flushtlb.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko 

Again, with an SPDX header
Acked-by: Jan Beulich 

Re: [PATCH v2 14/39] xen/riscv: introduce bitops.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko 

So this looks to have been taken from Linux, which could do with saying
(including which version or most recent commit). It may e.g. justify you
using tab indentation here, albeit ...

> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/bitops.h
> @@ -0,0 +1,288 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2012 Regents of the University of California */
> +
> +#ifndef _ASM_RISCV_BITOPS_H
> +#define _ASM_RISCV_BITOPS_H
> +
> +#include 
> +
> +#define BITOP_BITS_PER_WORD 32
> +#define BITOP_MASK(nr)   (1UL << ((nr) % BITOP_BITS_PER_WORD))
> +#define BITOP_WORD(nr)   ((nr) / BITOP_BITS_PER_WORD)
> +#define BITS_PER_BYTE8
> +
> +#define __set_bit(n,p)  set_bit(n,p)
> +#define __clear_bit(n,p)clear_bit(n,p)

... then please consistently. Other style related remarks made on the
system.h patch apply here as well (unless again there's a goal of
keeping the diff to the Linux original small; yet then I guess the
delta to the Linux file is already pretty large).

> +/* Based on linux/include/asm-generic/bitops/find.h */
> +
> +#ifndef find_next_bit
> +/**
> + * find_next_bit - find the next set bit in a memory region
> + * @addr: The address to base the search on
> + * @offset: The bitnumber to start searching at
> + * @size: The bitmap size in bits
> + */
> +extern unsigned long find_next_bit(const unsigned long *addr, unsigned long
> + size, unsigned long offset);
> +#endif
> +
> +#ifndef find_next_zero_bit
> +/**
> + * find_next_zero_bit - find the next cleared bit in a memory region
> + * @addr: The address to base the search on
> + * @offset: The bitnumber to start searching at
> + * @size: The bitmap size in bits
> + */
> +extern unsigned long find_next_zero_bit(const unsigned long *addr, unsigned
> + long size, unsigned long offset);
> +#endif
> +
> +/**
> + * find_first_bit - find the first set bit in a memory region
> + * @addr: The address to start the search at
> + * @size: The maximum size to search
> + *
> + * Returns the bit number of the first set bit.
> + */
> +extern unsigned long find_first_bit(const unsigned long *addr,
> + unsigned long size);
> +
> +/**
> + * find_first_zero_bit - find the first cleared bit in a memory region
> + * @addr: The address to start the search at
> + * @size: The maximum size to search
> + *
> + * Returns the bit number of the first cleared bit.
> + */
> +extern unsigned long find_first_zero_bit(const unsigned long *addr,
> +  unsigned long size);

Looking over the titles of the rest of the series, I can't spot where
these are going to be implemented. The again maybe you indeed can get
away without those, at least initially.

> +#define ffs(x) ({ unsigned int __t = (x); fls(__t & -__t); })

This wants to use ISOLATE_LSB() now.

Jan

Re: [PATCH v5 11/11] xen/arm: create another /memory node for static shm

[Michal Orzel]

Hi Penny,

On 06/12/2023 10:06, Penny Zheng wrote:
> 
> 
> Static shared memory region shall be described both under /memory and
> /reserved-memory.
> 
> We introduce export_shm_memory_node() to create another /memory node to
> contain the static shared memory ranges.
> 
> Signed-off-by: Penny Zheng 
> 
> ---
> v3 -> v4:
> new commit
> ---
> v4 -> v5:
> rebase and no changes
> ---
>  xen/arch/arm/dom0less-build.c   |  8 
>  xen/arch/arm/domain_build.c |  8 
>  xen/arch/arm/include/asm/static-shmem.h | 10 ++
>  xen/arch/arm/static-shmem.c | 19 +++
>  4 files changed, 45 insertions(+)
> 
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index ac096fa3fa..870b8a553f 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -645,6 +645,14 @@ static int __init prepare_dtb_domU(struct domain *d, 
> struct kernel_info *kinfo)
>  if ( ret )
>  goto err;
> 
> +/* Create a memory node to store the static shared memory regions */
> +if ( kinfo->shminfo.nr_banks != 0 )
There is no need for this check to be repeated every time 
export_shm_memory_node is used.
When the feature is disabled, export_shm_memory_node will return 0 anyway.
Furthermore, there is no need for kinfo->shminfo exposure. Please move the 
check to the function itself.

Also, I think both this and previous patch could be moved towards the beginning 
of the series.
They are not related to other things you do in the series.


> +{
> +ret = export_shm_memory_node(d, kinfo, addrcells, sizecells);
> +if ( ret )
> +goto err;
> +}
> +
>  ret = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
>  if ( ret )
>  goto err;
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index f098678ea3..4e450cb4c7 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1873,6 +1873,14 @@ static int __init handle_node(struct domain *d, struct 
> kernel_info *kinfo,
>  return res;
>  }
> 
> +/* Create a memory node to store the static shared memory regions */
> +if ( kinfo->shminfo.nr_banks != 0 )
> +{
> +res = export_shm_memory_node(d, kinfo, addrcells, sizecells);
> +if ( res )
> +return res;
> +}
> +
>  /* Avoid duplicate /reserved-memory nodes in Device Tree */
>  if ( !kinfo->resv_mem )
>  {
> diff --git a/xen/arch/arm/include/asm/static-shmem.h 
> b/xen/arch/arm/include/asm/static-shmem.h
> index 6cb4ef9646..385fd24c17 100644
> --- a/xen/arch/arm/include/asm/static-shmem.h
> +++ b/xen/arch/arm/include/asm/static-shmem.h
> @@ -38,6 +38,10 @@ int make_shm_memory_node(const struct domain *d,
>   void *fdt,
>   int addrcells, int sizecells,
>   const struct kernel_info *kinfo);
> +
> +int export_shm_memory_node(const struct domain *d,
> +   const struct kernel_info *kinfo,
> +   int addrcells, int sizecells);
>  #else /* !CONFIG_STATIC_SHM */
> 
>  static inline int make_resv_memory_node(const struct domain *d, void *fdt,
> @@ -86,6 +90,12 @@ static inline int make_shm_memory_node(const struct domain 
> *d,
>  return 0;
>  }
> 
> +static inline int export_shm_memory_node(const struct domain *d,
> + const struct kernel_info *kinfo,
> + int addrcells, int sizecells)
> +{
> +return 0;
> +}
>  #endif /* CONFIG_STATIC_SHM */
> 
>  #endif /* __ASM_STATIC_SHMEM_H_ */
> diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
> index bfce5bbad0..e583aae685 100644
> --- a/xen/arch/arm/static-shmem.c
> +++ b/xen/arch/arm/static-shmem.c
> @@ -505,6 +505,25 @@ int __init process_shm(struct domain *d, struct 
> kernel_info *kinfo,
>  return 0;
>  }
> 
> +int __init export_shm_memory_node(const struct domain *d,
> +  const struct kernel_info *kinfo,
> +  int addrcells, int sizecells)
> +{
> +unsigned int i = 0;
> +struct meminfo shm_meminfo;
> +
> +/* Extract meminfo from kinfo.shminfo */
> +for ( ; i < kinfo->shminfo.nr_banks; i++ )
> +{
> +shm_meminfo.bank[i].start = kinfo->shminfo.bank[i].membank.start;
> +shm_meminfo.bank[i].size = kinfo->shminfo.bank[i].membank.size;
> +shm_meminfo.bank[i].type = MEMBANK_DEFAULT;
Is all of this really needed? This series introduces so many structures to 
avoid using
meminfo but at the end we still need to use it. The amount of meminfo like 
structures copying
done in this series worries me a bit.

~Michal

xen | Failed pipeline for staging | 02d0a615

[GitLab]

Pipeline #1098841648 has failed!

Project: xen ( https://gitlab.com/xen-project/xen )
Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging )

Commit: 02d0a615 ( 
https://gitlab.com/xen-project/xen/-/commit/02d0a615b32d03702f79807fa5e88f0cf78dde84
 )
Commit Message: xen/arm: bootfdt: Check return code of device_t...
Commit Author: Michal Orzel ( https://gitlab.com/orzelmichal )
Committed by: Julien Grall


Pipeline #1098841648 ( 
https://gitlab.com/xen-project/xen/-/pipelines/1098841648 ) triggered by Ganis 
( https://gitlab.com/ganis )
had 12 failed jobs.

Job #5702575679 ( https://gitlab.com/xen-project/xen/-/jobs/5702575679/raw )

Stage: build
Name: archlinux-gcc-debug
Job #5702575693 ( https://gitlab.com/xen-project/xen/-/jobs/5702575693/raw )

Stage: build
Name: debian-bookworm-gcc
Job #5702575762 ( https://gitlab.com/xen-project/xen/-/jobs/5702575762/raw )

Stage: build
Name: opensuse-tumbleweed-gcc
Job #5702575709 ( https://gitlab.com/xen-project/xen/-/jobs/5702575709/raw )

Stage: build
Name: ubuntu-trusty-gcc
Job #5702575676 ( https://gitlab.com/xen-project/xen/-/jobs/5702575676/raw )

Stage: build
Name: archlinux-gcc
Job #5702575695 ( https://gitlab.com/xen-project/xen/-/jobs/5702575695/raw )

Stage: build
Name: debian-bookworm-gcc-debug
Job #5702575703 ( https://gitlab.com/xen-project/xen/-/jobs/5702575703/raw )

Stage: build
Name: debian-bookworm-32-gcc-debug
Job #5702575763 ( https://gitlab.com/xen-project/xen/-/jobs/5702575763/raw )

Stage: build
Name: opensuse-tumbleweed-gcc-debug
Job #5702575769 ( https://gitlab.com/xen-project/xen/-/jobs/5702575769/raw )

Stage: test
Name: build-each-commit-gcc
Job #5702575586 ( https://gitlab.com/xen-project/xen/-/jobs/5702575586/raw )

Stage: build
Name: alpine-3.18-gcc
Job #5702575587 ( https://gitlab.com/xen-project/xen/-/jobs/5702575587/raw )

Stage: build
Name: alpine-3.18-gcc-debug
Job #5702575712 ( https://gitlab.com/xen-project/xen/-/jobs/5702575712/raw )

Stage: build
Name: ubuntu-trusty-gcc-debug

-- 
You're receiving this email because of your account on gitlab.com.

Re: [PATCH v2 01/39] xen/riscv: disable unnecessary configs

[Jan Beulich]

On 07.12.2023 15:51, Oleksii wrote:
> On Thu, 2023-12-07 at 15:11 +0100, Jan Beulich wrote:
>> On 07.12.2023 14:44, Oleksii wrote:
>>> On Thu, 2023-12-07 at 11:00 +0100, Jan Beulich wrote:
 On 07.12.2023 10:22, Oleksii wrote:
> On Tue, 2023-12-05 at 16:38 +0100, Jan Beulich wrote:
>>> On 24.11.2023 11:30, Oleksii Kurochko wrote:
> The patch also fixes the build script as conf util
> expects
> to have each config on separate line.
>>>
>>> The approach doesn't really scale (it's already odd that
>>> you
>>> add
>>> the
>>> (apparently) same set four times. There's also zero
>>> justification
>>> for
>>> this kind of an adjustment (as per discussion elsewhere I
>>> don't
>>> think
>>> we should go this route, and hence arguments towards
>>> convincing
>>> me
>>> [and
>>> perhaps others] would be needed here).
> I agree that this may not be the best approach, but it seems
> like
> we
> don't have too many options to turn off a config for
> randconfig.
>
> To be honest, in my opinion (IMO), randconfig should either be
> removed
> or allowed to fail until most of the functionality is ready.
> Otherwise,
> there should be a need to introduce HAS_* or depend on
> 'SUPPORTED_ARCHS' for each config, or introduce a lot of stubs.
>
> Could you please suggest a better option?

 As to dropping randconfig tests, I'd like to refer you to Andrew,
 who
 is of the opinion that it was wrong to drop them for ppc. (I'm
 agreeing
 with him when taking a theoretical perspective, but I'm not happy
 with
 the practical consequences.)

 As to a better approach: Instead of listing the same set of
 options
 several times, can't there be a template config which is used to
 force
 randconfig to not touch certain settings? In fact at least for
 non-
 randconfig purposes I thought tiny64_defconfig /
 riscv64_defconfig
 already serve kind of a similar purpose. Imo the EXTRA_*CONFIG
 overrides
 are there for at most very few special case settings, not for
 purposes
 like you use them here.
>>> The template will be the really a good option.
>>>
>>> What do you think about the following patch which introduces arch-
>>> specific allrandom.config?
>>>
>>> diff --git a/xen/Makefile b/xen/Makefile
>>> index ca571103c8..cb1eca76c2 100644
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -336,11 +336,14 @@ ifeq ($(config-build),y)
>>>  # *config targets only - make sure prerequisites are updated, and
>>> descend
>>>  # in tools/kconfig to make the *config target
>>>  
>>> +ARCH_ALLRANDOM_CONFIG :=
>>> $(srctree)/arch/$(SRCARCH)/configs/allrandom.config
>>> +
>>>  # Create a file for KCONFIG_ALLCONFIG which depends on the
>>> environment.
>>>  # This will be use by kconfig targets
>>> allyesconfig/allmodconfig/allnoconfig/randconfig
>>>  filechk_kconfig_allconfig = \
>>>  $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo
>>> 'CONFIG_XSM_FLASK_POLICY=n';) \
>>> -    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
>>> +    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
>>> +    $(if $(wildcard $(ARCH_ALLRANDOM_CONFIG)), cat
>>> $(ARCH_ALLRANDOM_CONFIG);) ) \
>>>  :
>>
>> Something along these lines may be okay, but why would the name be
>> "allrandom" when the config is used elsewhere as well?
> The naming is not optimal. "unused.config" or "ignored.config" would be
> a better choice.

I don't think "unused" or "ignored" are properly describing what such
a file would hold. I was vaguely playing with "fixed" or "forced", fwiw.

>>  Further, besides
>> keeping randconfig and all*config from creating unusable configs, it
>> will at least want considering whether in other cases that set of
>> fixed
>> values shouldn't be used as well then.
> If I understood you correctly, the other case is *defconfig targets.
> Therefore, the following targets might also need to be updated by
> merging "unused.config" with {defconfig,%_defconfig}:
> 
> 
> defconfig: $(obj)/conf
> ifneq ($(wildcard
> $(srctree)/arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG)),)
>   @$(kecho) "*** Default configuration is based on
> '$(KBUILD_DEFCONFIG)'"
>   $(Q)$< $(silent) --
> defconfig=arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) $(Kconfig)
> else
>   @$(kecho) "*** Default configuration is based on target
> '$(KBUILD_DEFCONFIG)'"
>   $(Q)$(MAKE) -f $(srctree)/Makefile $(KBUILD_DEFCONFIG)
> endif
> 
> %_defconfig: $(obj)/conf
>   $(Q)$< $(silent) --defconfig=arch/$(SRCARCH)/configs/$@
> $(Kconfig)
> 
> However, I believe it's possible that for *defconfig, a configuration
> should be set to N, but in randconfig, it is still acceptable to be set
> to Y.

I'm not sure *defconfig should be affected by what's in the "locked
down" config, assuming they're consistent with what may not be changed.
But of course 

[libvirt test] 184019: tolerable all pass - PUSHED

[osstest service owner]

flight 184019 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/184019/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 184003
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 184003
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 184003
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  a949a53e1390462db101238c548b94d8600f9d11
baseline version:
 libvirt  cab49d394f4dee9a780b2702590b03e837e06892

Last test of basis   184003  2023-12-06 04:20:32 Z1 days
Testing same since   184019  2023-12-07 04:18:50 Z0 days1 attempts


People who touched revisions under test:
  Daniel P. Berrangé 
  Göran Uddeborg 
  Jonathon Jongsma 
  Michal Privoznik 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-qcow2   pass
 test-arm64-arm64-libvirt-raw pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-i386-libvirt-raw  pass
 test-amd64-amd64-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Re: [PATCH v2 13/39] xen/riscv: introduce asm/system.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/system.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_RISCV_BARRIER_H
> +#define _ASM_RISCV_BARRIER_H
> +
> +#include 
> +
> +#ifndef __ASSEMBLY__
> +
> +#define RISCV_FENCE(p, s) \
> +__asm__ __volatile__ ("fence " #p "," #s : : : "memory")

Nit (style): Missing blanks immediately inside the parentheses.

> +/* These barriers need to enforce ordering on both devices or memory. */
> +#define mb()RISCV_FENCE(iorw,iorw)
> +#define rmb()   RISCV_FENCE(ir,ir)
> +#define wmb()   RISCV_FENCE(ow,ow)

Nit (style): Missing blanks after the commas (also again below).

> +/* These barriers do not need to enforce ordering on devices, just memory. */
> +#define smp_mb()RISCV_FENCE(rw,rw)
> +#define smp_rmb()   RISCV_FENCE(r,r)
> +#define smp_wmb()   RISCV_FENCE(w,w)
> +#define smp_mb__before_atomic() smp_mb()
> +#define smp_mb__after_atomic()  smp_mb()
> +
> +/*
> +#define __smp_store_release(p, v)   \

Is there a need for the double underscores here? We try to not
introduce new instances of undue leading underscores, but there might
be e.g. a strong desire to stay in sync with, say, Linux.

> +do {\
> + compiletime_assert_atomic_type(*p); \
> + RISCV_FENCE(rw,w);  \
> + WRITE_ONCE(*p, v);  \

Nit: Can the trailing backslashes be aligned, please?

> +} while (0)
> +
> +#define __smp_load_acquire(p)   \
> +({  \
> +typeof(*p) ___p1 = READ_ONCE(*p);   \

Hmm, yet more leading underscores, and here surely not needed.

> +compiletime_assert_atomic_type(*p); \
> +RISCV_FENCE(r,rw);  \
> +___p1;  \
> +})
> +*/
> +
> +static inline unsigned long local_save_flags(void)
> +{
> +return csr_read(sstatus);
> +}
> +
> +static inline void local_irq_enable(void)
> +{
> +csr_set(sstatus, SSTATUS_SIE);
> +}
> +
> +static inline void local_irq_disable(void)
> +{
> +csr_clear(sstatus, SSTATUS_SIE);
> +}
> +
> +#define local_irq_save(x)   \
> +({  \
> +x = csr_read_clear(CSR_SSTATUS, SSTATUS_SIE);   \
> +local_irq_disable();\
> +})
> +
> +static inline void local_irq_restore(unsigned long flags)
> +{
> + csr_set(CSR_SSTATUS, flags & SSTATUS_SIE);
> +}
> +
> +static inline int local_irq_is_enabled(void)
> +{
> +unsigned long flags = local_save_flags();
> +
> +return flags & SSTATUS_SIE;

SSTATUS_SIE doesn't even happen to be 1, so I think you're better off
adding != 0, unless you would do as I think I had suggested before and
have the function return bool right away.

Jan

Re: [PATCH v2 01/39] xen/riscv: disable unnecessary configs

[Oleksii]

On Thu, 2023-12-07 at 15:11 +0100, Jan Beulich wrote:
> On 07.12.2023 14:44, Oleksii wrote:
> > On Thu, 2023-12-07 at 11:00 +0100, Jan Beulich wrote:
> > > On 07.12.2023 10:22, Oleksii wrote:
> > > > On Tue, 2023-12-05 at 16:38 +0100, Jan Beulich wrote:
> > > > > > On 24.11.2023 11:30, Oleksii Kurochko wrote:
> > > > > > > > The patch also fixes the build script as conf util
> > > > > > > > expects
> > > > > > > > to have each config on separate line.
> > > > > > 
> > > > > > The approach doesn't really scale (it's already odd that
> > > > > > you
> > > > > > add
> > > > > > the
> > > > > > (apparently) same set four times. There's also zero
> > > > > > justification
> > > > > > for
> > > > > > this kind of an adjustment (as per discussion elsewhere I
> > > > > > don't
> > > > > > think
> > > > > > we should go this route, and hence arguments towards
> > > > > > convincing
> > > > > > me
> > > > > > [and
> > > > > > perhaps others] would be needed here).
> > > > I agree that this may not be the best approach, but it seems
> > > > like
> > > > we
> > > > don't have too many options to turn off a config for
> > > > randconfig.
> > > > 
> > > > To be honest, in my opinion (IMO), randconfig should either be
> > > > removed
> > > > or allowed to fail until most of the functionality is ready.
> > > > Otherwise,
> > > > there should be a need to introduce HAS_* or depend on
> > > > 'SUPPORTED_ARCHS' for each config, or introduce a lot of stubs.
> > > > 
> > > > Could you please suggest a better option?
> > > 
> > > As to dropping randconfig tests, I'd like to refer you to Andrew,
> > > who
> > > is of the opinion that it was wrong to drop them for ppc. (I'm
> > > agreeing
> > > with him when taking a theoretical perspective, but I'm not happy
> > > with
> > > the practical consequences.)
> > > 
> > > As to a better approach: Instead of listing the same set of
> > > options
> > > several times, can't there be a template config which is used to
> > > force
> > > randconfig to not touch certain settings? In fact at least for
> > > non-
> > > randconfig purposes I thought tiny64_defconfig /
> > > riscv64_defconfig
> > > already serve kind of a similar purpose. Imo the EXTRA_*CONFIG
> > > overrides
> > > are there for at most very few special case settings, not for
> > > purposes
> > > like you use them here.
> > The template will be the really a good option.
> > 
> > What do you think about the following patch which introduces arch-
> > specific allrandom.config?
> > 
> > diff --git a/xen/Makefile b/xen/Makefile
> > index ca571103c8..cb1eca76c2 100644
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -336,11 +336,14 @@ ifeq ($(config-build),y)
> >  # *config targets only - make sure prerequisites are updated, and
> > descend
> >  # in tools/kconfig to make the *config target
> >  
> > +ARCH_ALLRANDOM_CONFIG :=
> > $(srctree)/arch/$(SRCARCH)/configs/allrandom.config
> > +
> >  # Create a file for KCONFIG_ALLCONFIG which depends on the
> > environment.
> >  # This will be use by kconfig targets
> > allyesconfig/allmodconfig/allnoconfig/randconfig
> >  filechk_kconfig_allconfig = \
> >  $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo
> > 'CONFIG_XSM_FLASK_POLICY=n';) \
> > -    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
> > +    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
> > +    $(if $(wildcard $(ARCH_ALLRANDOM_CONFIG)), cat
> > $(ARCH_ALLRANDOM_CONFIG);) ) \
> >  :
> 
> Something along these lines may be okay, but why would the name be
> "allrandom" when the config is used elsewhere as well?
The naming is not optimal. "unused.config" or "ignored.config" would be
a better choice.

>  Further, besides
> keeping randconfig and all*config from creating unusable configs, it
> will at least want considering whether in other cases that set of
> fixed
> values shouldn't be used as well then.
If I understood you correctly, the other case is *defconfig targets.
Therefore, the following targets might also need to be updated by
merging "unused.config" with {defconfig,%_defconfig}:


defconfig: $(obj)/conf
ifneq ($(wildcard
$(srctree)/arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG)),)
@$(kecho) "*** Default configuration is based on
'$(KBUILD_DEFCONFIG)'"
$(Q)$< $(silent) --
defconfig=arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) $(Kconfig)
else
@$(kecho) "*** Default configuration is based on target
'$(KBUILD_DEFCONFIG)'"
$(Q)$(MAKE) -f $(srctree)/Makefile $(KBUILD_DEFCONFIG)
endif

%_defconfig: $(obj)/conf
$(Q)$< $(silent) --defconfig=arch/$(SRCARCH)/configs/$@
$(Kconfig)

However, I believe it's possible that for *defconfig, a configuration
should be set to N, but in randconfig, it is still acceptable to be set
to Y.

~ Oleksii

Re: [PATCH] CODING_STYLE: Add a section of the naming convention

[Julien Grall]

Hi George,

On 06/12/2023 16:22, George Dunlap wrote:

On Wed, Dec 6, 2023 at 11:22 AM Julien Grall  wrote:


Hi,

On 06/12/2023 11:19, Andrew Cooper wrote:

On 06/12/2023 8:41 am, Jan Beulich wrote:

On 06.12.2023 03:21, George Dunlap wrote:

On Tue, Dec 5, 2023 at 6:12 PM Julien Grall  wrote:

From: Julien Grall 

Several maintainers have expressed a stronger preference
to use '-' when in filename and option that contains multiple
words.

So document it in CODING_STYLE.

Signed-off-by: Julien Grall 

---
  Changes in v2:
  - New wording
  - Update the section title
  - Add Jan's acked-by
---
   CODING_STYLE | 9 +
   1 file changed, 9 insertions(+)

diff --git a/CODING_STYLE b/CODING_STYLE
index ced3ade5a6fb..ed13ee2b664b 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -144,6 +144,15 @@ separate lines and each line should begin with a leading 
'*'.
* Note beginning and end markers on separate lines and leading '*'.
*/

+Naming convention for files and command line options
+
+
+'-' should be used to separate words in commandline options and filenames.
+E.g. timer-works.
+
+Note that some of the options and filenames are using '_'. This is now
+deprecated.

Sorry for not catching this last time; "are using X" isn't really
idiomatic English; more idiomatic would be something like the
following:

"Note that some existing options and file names use '_'.  This is now
deprecated."

Since we're changing things, I *think* most style guides would advise
against starting the sentence with a punctuation; so perhaps:

"Command-line options and file names should use '-' to separate words;
e.g., timer-works."

And what about adding to the last paragraph:

"When touching code around command-line parameters still using '_', it
is recommended to modify the documentation to say only '-', but modify
the code to accept both '-' and '_' (for backwards compatibility)."

In this context see
https://lists.xen.org/archives/html/xen-devel/2020-01/msg01945.html
and Andrew's response
https://lists.xen.org/archives/html/xen-devel/2020-01/msg02006.html
I'm still in favor of addressing the issue centrally (making unnecessary
adjustments like you suggest in the new paragraph). Yet I think Andrew's
objection would cover such adjustments as much as my generic solution.


Aliasing - and _ in the cmdline parsing breaks basic usability.

Its fine for new options to use -, and it's even fine-ish (but only if
you're going to be the one doing security backports) to rename internal
files.

But there is real and detrimental effect for altering the command line.

You will get people failing to express the option they intended when
working with an older form of Xen.  You will need an absurd number of
notes in the command line docs saying "newer versions of Xen accept an
alias but you need to use the underscore form for backwards compatibility".

Not to mention that there are years of notes scattered all around the
internet using the underscore forms, so it's likely that everyone will
continue to use the underscore form, meaning that you don't even have a
way to phase them out.

And for what?  An attempt to pretend that we don't have 2 decades of
history where underscores where the norm?

It's tinkering, for no useful benefit and a clear cost.


+1 with what Andrew said.


Haven't given it full thought, because I absolutely did not want to
make this change take longer to get in. :-). The existence of
disagreement is enough for me to withdraw my suggestion.


Thanks. I will commit the patch soon then. I am happy to continue the 
discussion though. I would love to get the file CODING_STYLE reflecting 
more our style policies :).


Cheers,

--
Julien Grall

Re: [PATCH] xen/arm: bootfdt: Check return code of device_tree_for_each_node()

[Michal Orzel]

On 07/12/2023 13:54, Julien Grall wrote:
> 
> 
> Hi Michal,
> 
> On 07/12/2023 12:39, Michal Orzel wrote:
>> On 07/12/2023 13:20, Julien Grall wrote:
>>>
>>>
>>> Hi Michal,
>>>
>>> On 07/12/2023 10:14, Michal Orzel wrote:
 As a result of not checking the return code of device_tree_for_each_node()
 in boot_fdt_info(), any error occured during early FDT parsing does not
 stop Xen from booting. This can result in an unwanted behavior in later
 boot stages. Fix it by checking the return code and panicing on an error.

 Fixes: 9cf4a9a46717 ("device tree: add device_tree_for_each_node()")
 Signed-off-by: Michal Orzel 
>>>
>>> With one remark below:
>>>
>>> Acked-by: Julien Grall 
>>>
 ---
 I've lost count how many times I had to fix missing rc check. However, I 
 have
 not yet found any checker for this (-Wunused-result is pretty useless).
 ---
xen/arch/arm/bootfdt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
 index b1f03eb2fcdd..f496a8cf9494 100644
 --- a/xen/arch/arm/bootfdt.c
 +++ b/xen/arch/arm/bootfdt.c
 @@ -541,7 +541,9 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t 
 paddr)

add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);

 -device_tree_for_each_node((void *)fdt, 0, early_scan_node, NULL);
 +ret = device_tree_for_each_node((void *)fdt, 0, early_scan_node, 
 NULL);
 +if ( ret )
 +panic("Early FDT parsing failed (%d)\n", ret);
>>>
>>> AFAIU, the parsing is done before the console is setup. This means a
>>> user would not be able to get some logs unless they are re-compiling Xen
>>> with earlyprintk.
>>>
>>> I understand this is not a new issue, but I am getting concerned of the
>>> amount of check we add before the console is up and running.
>>>
>>> What is the impact if we don't check the return here? Is it missing regions?
>> There are many things that can go wrong.
>> Quite recently, I faced an issue where I specified 2 dom0less domUs in 
>> configuration
>> and due to the error while parsing the last node of domU1, domU2 node was 
>> skipped and
>> Xen booted only domU1 without giving any errors.
>>
>> Issues with shared memory led to either Xen continue to run with improper 
>> configuration,
>> silencing overlap conditions, errors at later boot stages that were 
>> impossible to deduct
>> from the logs.
>>
>> All in all, early boot code parsing assume the error to result in a failure 
>> and the parsing
>> for domain creation makes this assumption as well (checks are more relaxed 
>> to avoid duplication).
>>
>> For now, we can't do anything better than panicing early if we want to avoid 
>> unwanted behavior.
>>
>>>
>>> I wonder whether we can either enable the console earlier, or make
>>> earlyprintk more dynamic (similar to what Linux is doing with
>>> earlyprintk=...).
>> The most imporatant part is early fdt parsing. The main console init cannot 
>> be moved that early.
> 
> I think we need to understand a bit more why because on x86
> consoel_init_preirq() is called very early. So we ought to be able to do
> the same on Arm.
But this won't cover early fdt parsing. I don't think we can get away without 
adding earlycon
and earlycon helpers in all the serial drivers. arm_uart_init depends on 
unflattening fdt which
depends on relocating fdt which is done after parsing FDT. We want to be able 
to print messages
after early_fdt_map. Once FDT is mapped, the only thing we need is to retrieve 
the bootargs to parse
for earlycon= , add the region to fixmap and register the handlers.

~Michal

Re: [PATCH v2 00/39] Enable build of full Xen for RISC-V

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> Bobby Eshleman (1):
>   xen/riscv: introduce asm/atomic.h
> 
> Oleksii Kurochko (38):
>   xen/riscv: disable unnecessary configs
>   xen/riscv: use some asm-generic headers
>   xen/riscv:introduce asm/byteorder.h
>   xen/riscv: add public arch-riscv.h
>   xen/riscv: introduce spinlock.h
>   xen/riscv: introduce fence.h
>   xen/riscv: introduce arch-riscv/hvm/save.h
>   xen/riscv: introduce asm/cpufeature.h
>   xen/riscv: introduce asm/guest_atomics.h
>   xen/riscv: introduce asm/iommu.h
>   xen/riscv: introduce asm/nospec.h
>   xen/riscv: introduce asm/setup.h
>   xen/riscv: introduce asm/system.h
>   xen/riscv: introduce bitops.h
>   xen/riscv: introduce flushtlb.h
>   xen/riscv: introduce asm/smp.h
>   xen/riscv: introduce cmpxchg.h
>   xen/riscv: introduce asm/io.h
>   xen/riscv: define bug frame tables in xen.lds.S
>   xen/riscv: introduce bit operations
>   xen/riscv: introduce asm/domain.h
>   xen/riscv: introduce asm/guest_access.h
>   xen/riscv: introduce asm/irq.h
>   xen/riscv: introduce asm/p2m.h
>   xen/riscv: introduce asm/regs.h
>   xen/riscv: introduce asm/time.h
>   xen/riscv: introduce asm/event.h

Throughout here, would you please try to be consistent about (not) using asm/
prefixes?

Jan

Re: [PATCH v2 12/39] xen/riscv: introduce asm/setup.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko 

With at least SPDX header
Acked-by: Jan Beulich 

Re: [PATCH v2 11/39] xen/riscv: introduce asm/nospec.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/nospec.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. */
> +
> +#ifndef _ASM_RISCV_NOSPEC_H
> +#define _ASM_RISCV_NOSPEC_H
> +
> +static inline bool evaluate_nospec(bool condition)
> +{
> +return condition;
> +}
> +
> +static inline void block_speculation(void)
> +{
> +}
> +
> +#endif /* _ASM_RISCV_NOSPEC_H */

This being identical between Arm, PPC, and now RISC-V, wouldn't this be another
asm-generic/ candidate? (Whether such trivial stubs are copyrightable is, as
per earlier remarks, at least questionable to me.)

Jan

Re: [PATCH v2 08/39] xen/riscv: introduce asm/cpufeature.h

[Jan Beulich]

On 07.12.2023 15:19, Jan Beulich wrote:
> On 24.11.2023 11:30, Oleksii Kurochko wrote:
>> Signed-off-by: Oleksii Kurochko 
> 
> Acked-by: Jan Beulich 

Actually - with an SPDX header added. I only now realize that I
committed the earlier two patches without paying attention to this
aspect. I'd appreciate if in the next version you could include an
incremental change. And obviously in all other new headers such a
comment (and perhaps also a formatting footer) wants introducing
as well.

Jan

Re: [PATCH 2/3] xen/x86: address violations of MISRA C:2012 Rule 14.4

[Simone Ballarin]

On 07/12/23 15:15, Jan Beulich wrote:

On 07.12.2023 14:53, Simone Ballarin wrote:

On 07/12/23 11:54, Jan Beulich wrote:

On 07.12.2023 10:48, Simone Ballarin wrote:

--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -279,7 +279,7 @@ static int hpet_msi_write(struct hpet_event_channel *ch, 
struct msi_msg *msg)
   {
   ch->msi.msg = *msg;
   
-if ( iommu_intremap )

+if ( iommu_intremap != iommu_intremap_off )
   {
   int rc = iommu_update_ire_from_msi(>msi, msg);
   
@@ -353,7 +353,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)

   u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
   irq_desc_t *desc = irq_to_desc(ch->msi.irq);
   
-if ( iommu_intremap )

+if ( iommu_intremap != iommu_intremap_off )
   {
   ch->msi.hpet_id = hpet_blockid;
   ret = iommu_setup_hpet_msi(>msi);
@@ -372,7 +372,7 @@ static int __init hpet_setup_msi_irq(struct 
hpet_event_channel *ch)
   ret = __hpet_setup_msi_irq(desc);
   if ( ret < 0 )
   {
-if ( iommu_intremap )
+if ( iommu_intremap != iommu_intremap_off )
   iommu_update_ire_from_msi(>msi, NULL);
   return ret;
   }
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 7f8e794254..72dce2e4ab 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -189,7 +189,7 @@ static int write_msi_msg(struct msi_desc *entry, struct 
msi_msg *msg)
   {
   entry->msg = *msg;
   
-if ( iommu_intremap )

+if ( iommu_intremap != iommu_intremap_off )
   {
   int rc;
   
@@ -555,7 +555,7 @@ int msi_free_irq(struct msi_desc *entry)

   destroy_irq(entry[nr].irq);
   
   /* Free the unused IRTE if intr remap enabled */

-if ( iommu_intremap )
+if ( iommu_intremap != iommu_intremap_off )
   iommu_update_ire_from_msi(entry + nr, NULL);
   }
   


All of this would logically be part of patch 1. Is there a particular reason
why it wasn't done right there?


These changes and the ones in patch 1 are related, but still remain
independent. Patch 1 can be accepted without patch 2 and vice versa.
So we've decided to split the commits because patch 1 is in common
code, while patch 2 is in x86-specific code.


Just to clarify: While not located under arch/x86/, what patch 1 touches
is still x86-specific code. It's subject prefix also wrongly says
AMD/IOMMU: when it also touches VT-d code. Especially with the changes
here folded in, x86/IOMMU: might be more appropriate.



OK, then I'll move the changes and use x86/IOMMU.
Thanks.


Jan



--
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)

Re: [PATCH v2 10/39] xen/riscv: introduce asm/iommu.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/iommu.h
> @@ -0,0 +1,7 @@
> +#ifndef __ASM_RISCV_IOMMU_H__
> +#define __ASM_RISCV_IOMMU_H__
> +
> +struct arch_iommu {
> +};
> +
> +#endif /* __ASM_IOMMU_H__ */

Instead of adding this header, didn't we discuss to make the #include in
xen/iommu.h depend on CONFIG_HAS_PASSTHROUGH? Also - no SPDX or footer
here?

Jan

Re: [PATCH v2 09/39] xen/riscv: introduce asm/guest_atomics.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/guest_atomics.h
> @@ -0,0 +1,48 @@
> +#ifndef __ASM_RISCV_GUEST_ATOMICS_H
> +#define __ASM_RISCV_GUEST_ATOMICS_H
> +
> +/*
> + * TODO: implement guest atomics
> + */

Along with this, wouldn't it be better to have e.g. ASSERT_UNREACHABLE()
in the unimplemented functions?

Jan

> +#define guest_testop(name)  \
> +static inline int guest_##name(struct domain *d, int nr, volatile void *p)  \
> +{   \
> +(void) d;   \
> +(void) nr;  \
> +(void) p;   \
> +\
> +return 0;   \
> +}
> +
> +#define guest_bitop(name)   \
> +static inline void guest_##name(struct domain *d, int nr, volatile void *p) \
> +{   \
> +(void) d;   \
> +(void) nr;  \
> +(void) p;   \
> +}
> +
> +guest_bitop(set_bit)
> +guest_bitop(clear_bit)
> +guest_bitop(change_bit)
> +
> +#undef guest_bitop
> +
> +guest_testop(test_and_set_bit)
> +guest_testop(test_and_clear_bit)
> +guest_testop(test_and_change_bit)
> +
> +#undef guest_testop
> +
> +#define guest_test_bit(d, nr, p) ((void)(d), test_bit(nr, p))
> +
> +#endif /* __ASM_RISCV_GUEST_ATOMICS_H */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */

Re: [PATCH v2 08/39] xen/riscv: introduce asm/cpufeature.h

[Jan Beulich]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko 

Acked-by: Jan Beulich 

Would have been nice ...

> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/cpufeature.h
> @@ -0,0 +1,22 @@
> +#ifndef __ASM_RISCV_CPUFEATURE_H
> +#define __ASM_RISCV_CPUFEATURE_H
> +
> +#ifndef __ASSEMBLY__
> +
> +static inline int cpu_nr_siblings(unsigned int cpu)

... for this to have return type unsigned int, but I see you're staying in
line with oddities elsewhere. Just one more place to touch down the road.

Jan

Re: [PATCH 2/3] xen/x86: address violations of MISRA C:2012 Rule 14.4

[Jan Beulich]

On 07.12.2023 14:53, Simone Ballarin wrote:
> On 07/12/23 11:54, Jan Beulich wrote:
>> On 07.12.2023 10:48, Simone Ballarin wrote:
>>> --- a/xen/arch/x86/hpet.c
>>> +++ b/xen/arch/x86/hpet.c
>>> @@ -279,7 +279,7 @@ static int hpet_msi_write(struct hpet_event_channel 
>>> *ch, struct msi_msg *msg)
>>>   {
>>>   ch->msi.msg = *msg;
>>>   
>>> -if ( iommu_intremap )
>>> +if ( iommu_intremap != iommu_intremap_off )
>>>   {
>>>   int rc = iommu_update_ire_from_msi(>msi, msg);
>>>   
>>> @@ -353,7 +353,7 @@ static int __init hpet_setup_msi_irq(struct 
>>> hpet_event_channel *ch)
>>>   u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
>>>   irq_desc_t *desc = irq_to_desc(ch->msi.irq);
>>>   
>>> -if ( iommu_intremap )
>>> +if ( iommu_intremap != iommu_intremap_off )
>>>   {
>>>   ch->msi.hpet_id = hpet_blockid;
>>>   ret = iommu_setup_hpet_msi(>msi);
>>> @@ -372,7 +372,7 @@ static int __init hpet_setup_msi_irq(struct 
>>> hpet_event_channel *ch)
>>>   ret = __hpet_setup_msi_irq(desc);
>>>   if ( ret < 0 )
>>>   {
>>> -if ( iommu_intremap )
>>> +if ( iommu_intremap != iommu_intremap_off )
>>>   iommu_update_ire_from_msi(>msi, NULL);
>>>   return ret;
>>>   }
>>> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
>>> index 7f8e794254..72dce2e4ab 100644
>>> --- a/xen/arch/x86/msi.c
>>> +++ b/xen/arch/x86/msi.c
>>> @@ -189,7 +189,7 @@ static int write_msi_msg(struct msi_desc *entry, struct 
>>> msi_msg *msg)
>>>   {
>>>   entry->msg = *msg;
>>>   
>>> -if ( iommu_intremap )
>>> +if ( iommu_intremap != iommu_intremap_off )
>>>   {
>>>   int rc;
>>>   
>>> @@ -555,7 +555,7 @@ int msi_free_irq(struct msi_desc *entry)
>>>   destroy_irq(entry[nr].irq);
>>>   
>>>   /* Free the unused IRTE if intr remap enabled */
>>> -if ( iommu_intremap )
>>> +if ( iommu_intremap != iommu_intremap_off )
>>>   iommu_update_ire_from_msi(entry + nr, NULL);
>>>   }
>>>   
>>
>> All of this would logically be part of patch 1. Is there a particular reason
>> why it wasn't done right there?
> 
> These changes and the ones in patch 1 are related, but still remain
> independent. Patch 1 can be accepted without patch 2 and vice versa.
> So we've decided to split the commits because patch 1 is in common
> code, while patch 2 is in x86-specific code.

Just to clarify: While not located under arch/x86/, what patch 1 touches
is still x86-specific code. It's subject prefix also wrongly says
AMD/IOMMU: when it also touches VT-d code. Especially with the changes
here folded in, x86/IOMMU: might be more appropriate.

Jan

Re: [XEN PATCH v2 3/3] xen/mm: add declaration for first_valid_mfn

[Nicola Vetrini]

On 2023-12-07 14:52, Julien Grall wrote:

Hi,

On 07/12/2023 11:11, Nicola Vetrini wrote:

Such declaration is needed to comply with MISRA C Rule 8.4, because a
compatible declaration is not visible in xen/common/page_alloc.c, 
where the
variable is defined. That variable can't yet be static because of the 
lack of

support from ARM and PPC for NUMA.

No functional change.

Signed-off-by: Nicola Vetrini 
---
Having this declaration essentially sidesteps the current 
impossibility

of having a static variable, as described in the comments in
ARM and PCC's asm/numa.h.
With this change, is there any reason to keep the various declaration 
of first_valid_mfn in numa.h?


Cheers,


Good point: no reason comes to mind. I didn't think of it while revising 
the patch.


--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [PATCH v2 01/39] xen/riscv: disable unnecessary configs

[Jan Beulich]

On 07.12.2023 14:44, Oleksii wrote:
> On Thu, 2023-12-07 at 11:00 +0100, Jan Beulich wrote:
>> On 07.12.2023 10:22, Oleksii wrote:
>>> On Tue, 2023-12-05 at 16:38 +0100, Jan Beulich wrote:
> On 24.11.2023 11:30, Oleksii Kurochko wrote:
>>> The patch also fixes the build script as conf util expects
>>> to have each config on separate line.
>
> The approach doesn't really scale (it's already odd that you
> add
> the
> (apparently) same set four times. There's also zero
> justification
> for
> this kind of an adjustment (as per discussion elsewhere I don't
> think
> we should go this route, and hence arguments towards convincing
> me
> [and
> perhaps others] would be needed here).
>>> I agree that this may not be the best approach, but it seems like
>>> we
>>> don't have too many options to turn off a config for randconfig.
>>>
>>> To be honest, in my opinion (IMO), randconfig should either be
>>> removed
>>> or allowed to fail until most of the functionality is ready.
>>> Otherwise,
>>> there should be a need to introduce HAS_* or depend on
>>> 'SUPPORTED_ARCHS' for each config, or introduce a lot of stubs.
>>>
>>> Could you please suggest a better option?
>>
>> As to dropping randconfig tests, I'd like to refer you to Andrew, who
>> is of the opinion that it was wrong to drop them for ppc. (I'm
>> agreeing
>> with him when taking a theoretical perspective, but I'm not happy
>> with
>> the practical consequences.)
>>
>> As to a better approach: Instead of listing the same set of options
>> several times, can't there be a template config which is used to
>> force
>> randconfig to not touch certain settings? In fact at least for non-
>> randconfig purposes I thought tiny64_defconfig / riscv64_defconfig
>> already serve kind of a similar purpose. Imo the EXTRA_*CONFIG
>> overrides
>> are there for at most very few special case settings, not for
>> purposes
>> like you use them here.
> The template will be the really a good option.
> 
> What do you think about the following patch which introduces arch-
> specific allrandom.config?
> 
> diff --git a/xen/Makefile b/xen/Makefile
> index ca571103c8..cb1eca76c2 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -336,11 +336,14 @@ ifeq ($(config-build),y)
>  # *config targets only - make sure prerequisites are updated, and
> descend
>  # in tools/kconfig to make the *config target
>  
> +ARCH_ALLRANDOM_CONFIG :=
> $(srctree)/arch/$(SRCARCH)/configs/allrandom.config
> +
>  # Create a file for KCONFIG_ALLCONFIG which depends on the
> environment.
>  # This will be use by kconfig targets
> allyesconfig/allmodconfig/allnoconfig/randconfig
>  filechk_kconfig_allconfig = \
>  $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo
> 'CONFIG_XSM_FLASK_POLICY=n';) \
> -$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
> +$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
> +$(if $(wildcard $(ARCH_ALLRANDOM_CONFIG)), cat
> $(ARCH_ALLRANDOM_CONFIG);) ) \
>  :

Something along these lines may be okay, but why would the name be
"allrandom" when the config is used elsewhere as well? Further, besides
keeping randconfig and all*config from creating unusable configs, it
will at least want considering whether in other cases that set of fixed
values shouldn't be used as well then.

Jan

[ovmf test] 184023: all pass - PUSHED

[osstest service owner]

flight 184023 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/184023/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 553dfb0f57ae8a666938873cf836a33549568c87
baseline version:
 ovmf ff4c49a5ee38d613384fb2e318d891a800d32999

Last test of basis   184017  2023-12-07 01:45:05 Z0 days
Testing same since   184023  2023-12-07 10:14:25 Z0 days1 attempts


People who touched revisions under test:
  Sheng Wei 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   ff4c49a5ee..553dfb0f57  553dfb0f57ae8a666938873cf836a33549568c87 -> 
xen-tested-master

[PATCH 4/5] automation: Add the script for the FVP smoke test

[Henry Wang]

This commit adds the shell script for the FVP smoke test. Similarly
as the QEMU jobs, the shell script will firstly prepare the DomU
BusyBox image, use the ImageBuilder to arrange the binaries in memory
and generate the U-Boot script, then start the test. To provide the
TFTP service for the FVP, the shell script will also start the TFTP
service, and copy the binaries needed by test to the TFTP directory
used by the TFTP server.

Signed-off-by: Henry Wang 
---
 .../scripts/fvp-base-smoke-dom0-arm64.sh  | 117 ++
 1 file changed, 117 insertions(+)
 create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh

diff --git a/automation/scripts/fvp-base-smoke-dom0-arm64.sh 
b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
new file mode 100755
index 00..716a63b0a8
--- /dev/null
+++ b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
@@ -0,0 +1,117 @@
+#!/bin/bash
+
+set -ex
+
+# 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 ..
+
+mkdir -p rootfs
+cd rootfs
+tar xvzf ../initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+cp -ar ../dist/install/* .
+mv ../initrd.cpio.gz ./root
+cp ../Image ./root
+echo "name=\"test\"
+memory=512
+vcpus=1
+kernel=\"/root/Image\"
+ramdisk=\"/root/initrd.cpio.gz\"
+extra=\"console=hvc0 root=/dev/ram0 rdinit=/bin/sh\"
+" > root/test.cfg
+echo "#!/bin/bash
+
+export LD_LIBRARY_PATH=/usr/local/lib
+bash /etc/init.d/xencommons start
+
+xl list
+
+xl create -c /root/test.cfg
+
+" > 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 > ../xen-rootfs.cpio.gz
+cd ../..
+
+# Start a TFTP server to provide TFTP service to FVP
+service tftpd-hpa start
+
+# ImageBuilder
+echo 'MEMORY_START="0x8000"
+MEMORY_END="0xFF00"
+
+DEVICE_TREE="fvp-base-gicv3-psci-1t.dtb"
+XEN="xen"
+DOM0_KERNEL="Image"
+DOM0_RAMDISK="xen-rootfs.cpio.gz"
+XEN_CMD="console=dtuart dom0_mem=1024M console_timestamps=boot"
+
+NUM_DOMUS=0
+
+LOAD_CMD="tftpb"
+UBOOT_SOURCE="boot.source"
+UBOOT_SCRIPT="boot.scr"' > binaries/config
+rm -rf imagebuilder
+git clone https://gitlab.com/ViryaOS/imagebuilder
+bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c 
binaries/config
+
+# Copy files to the TFTP directory to use
+cp ./binaries/boot.scr /srv/tftp/
+cp ./binaries/Image /srv/tftp/
+cp ./binaries/xen-rootfs.cpio.gz /srv/tftp/
+cp ./binaries/xen /srv/tftp/
+cp ./binaries/fvp-base-gicv3-psci-1t.dtb /srv/tftp/
+
+# Start FVP
+TERM0_CFG="-C bp.terminal_0.mode=telnet -C bp.terminal_0.start_telnet=0"
+TERM1_CFG="-C bp.terminal_1.mode=telnet -C bp.terminal_1.start_telnet=0"
+TERM2_CFG="-C bp.terminal_2.mode=telnet -C bp.terminal_2.start_telnet=0"
+TERM3_CFG="-C bp.terminal_3.mode=telnet -C bp.terminal_3.start_telnet=0"
+
+VIRTIO_USER_NETWORK_CFG="-C bp.virtio_net.enabled=1 \
+-C bp.virtio_net.hostbridge.userNetworking=1 \
+-C bp.virtio_net.hostbridge.userNetPorts=8022=22 \
+-C bp.virtio_net.transport=legacy"
+
+./automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp \
+
"/FVP/FVP_Base_RevC-2xAEMvA/Base_RevC_AEMvA_pkg/models/Linux64_armv8l_GCC-9.3/FVP_Base_RevC-2xAEMvA
 \
+-C bp.vis.disable_visualisation=1 \
+-C bp.ve_sysregs.exit_on_shutdown=1 \
+-C bp.secure_memory=0 \
+-C cache_state_modelled=0 \
+-C cluster0.has_arm_v8-4=1 \
+-C cluster1.has_arm_v8-4=1 \
+${TERM0_CFG} ${TERM1_CFG} ${TERM2_CFG} ${TERM3_CFG} \
+${VIRTIO_USER_NETWORK_CFG} \
+-C bp.secureflashloader.fname=$(pwd)/binaries/bl1.bin \
+-C bp.flashloader0.fname=$(pwd)/binaries/fip.bin" |& \
+tee smoke.serial
+
+exit 0
-- 
2.25.1

[PATCH 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs

[Henry Wang]

Add a job in the build stage to export the TF-A, U-Boot and the
device tree for the FVP platform from the test artifact container.

Add a FVP smoke test job in the test stage to do the same test as
the `qemu-smoke-dom0-arm64-gcc` job.

Signed-off-by: Henry Wang 
---
Although it does not affect the functionality, I am still quite
confused about why the logs displayed by GitLab UI, for
example [1], is much less than the actual output (saved in the
artifacts, see [2]). Had a discussion with Michal on Matrix
and we agree that the log in gitlab UI is usually capped.

[1] https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5700569676
[2] 
https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5700569676/artifacts/file/smoke.serial
---
 automation/gitlab-ci/build.yaml | 17 +
 automation/gitlab-ci/test.yaml  | 22 ++
 2 files changed, 39 insertions(+)

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 32af30cced..89d2f01302 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -344,6 +344,23 @@ kernel-6.1.19-export:
   tags:
 - x86_64
 
+armfvp-uboot-tfa-2023.10-2.9.0-arm64-export:
+  extends: .test-jobs-artifact-common
+  image: 
registry.gitlab.com/xen-project/xen/tests-artifacts/armfvp-uboot-tfa:2023.10-2.9.0-arm64v8
+  script:
+- |
+   mkdir binaries && \
+   cp /bl1.bin binaries/bl1.bin && \
+   cp /fip.bin binaries/fip.bin && \
+   cp /fvp-base-gicv3-psci-1t.dtb binaries/fvp-base-gicv3-psci-1t.dtb
+  artifacts:
+paths:
+  - binaries/bl1.bin
+  - binaries/fip.bin
+  - binaries/fvp-base-gicv3-psci-1t.dtb
+  tags:
+- arm64
+
 # Jobs below this line
 
 # Build jobs needed for tests
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 6aabdb9d15..47e00d0a0b 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -96,6 +96,19 @@
   tags:
 - xilinx
 
+.fvp-arm64:
+  extends: .test-jobs-common
+  variables:
+CONTAINER: debian:bookworm-arm64v8-fvp
+LOGFILE: fvp-smoke-arm64.log
+  artifacts:
+paths:
+  - smoke.serial
+  - '*.log'
+when: always
+  tags:
+- arm64
+
 .adl-x86-64:
   extends: .test-jobs-common
   variables:
@@ -459,3 +472,12 @@ qemu-smoke-ppc64le-powernv9-gcc:
   needs:
 - qemu-system-ppc64-8.1.0-ppc64-export
 - debian-bullseye-gcc-ppc64le-debug
+
+fvp-smoke-dom0-arm64-gcc-debug:
+  extends: .fvp-arm64
+  script:
+- ./automation/scripts/fvp-base-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE}
+  needs:
+- *arm64-test-needs
+- armfvp-uboot-tfa-2023.10-2.9.0-arm64-export
+- alpine-3.18-gcc-debug-arm64
-- 
2.25.1

[PATCH 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP

[Henry Wang]

Unlike the emulators that currently being used in the CI pipelines,
the FVP must start at EL3. Therefore we need the firmware, i.e. the
TrustedFirmware-A (TF-A), for corresponding functionality.

There is a dedicated board (vexpress_fvp) in U-Boot (serve as the
BL33 of the TF-A) for the FVP platform, so the U-Boot should also be
compiled for the FVP platform instead of reusing the U-Boot for the
existing emulators used in the CI pipelines.

To avoid compiling TF-A and U-Boot everytime in the job, adding a
Dockerfile to the test artifacts to build TF-A v2.9.0 and U-Boot
v2023.10 for FVP. The binaries for the TF-A and U-Boot, as well as
the device tree for the FVP platform, will be saved (and exported by
the CI job introduced by following commits). Note that, a patch for
the TF-A will be applied before building to enable the virtio-net
and the virtio-rng device on the FVP. The virtio-net device will
provide the networking service for FVP, and the virtio-rng device
will improve the speed of the FVP.

Signed-off-by: Henry Wang 
---
 .../2023.10-2.9.0-arm64v8.dockerfile  | 48 +++
 1 file changed, 48 insertions(+)
 create mode 100644 
automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile

diff --git 
a/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile 
b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
new file mode 100644
index 00..6566b60545
--- /dev/null
+++ 
b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
@@ -0,0 +1,48 @@
+FROM --platform=linux/arm64/v8 debian:bookworm
+LABEL maintainer.name="The Xen Project" \
+  maintainer.email="xen-devel@lists.xenproject.org"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV UBOOT_VERSION="2023.10"
+ENV TFA_VERSION="v2.9.0"
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+# build depends
+RUN apt-get update && \
+apt-get --quiet --yes install \
+build-essential \
+libssl-dev \
+bc \
+curl \
+flex \
+bison \
+git \
+device-tree-compiler && \
+apt-get autoremove -y && \
+apt-get clean && \
+rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
+
+# Build U-Boot and TF-A
+RUN curl -fsSLO https://ftp.denx.de/pub/u-boot/u-boot-"$UBOOT_VERSION".tar.bz2 
&& \
+tar xvf u-boot-"$UBOOT_VERSION".tar.bz2 && \
+cd u-boot-"$UBOOT_VERSION" && \
+make -j$(nproc) V=1 vexpress_fvp_defconfig && \
+make -j$(nproc) V=1 all && \
+cd .. && \
+git clone --branch "$TFA_VERSION" --depth 1 
https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git && \
+cd trusted-firmware-a && \
+curl -fsSLO 
https://git.yoctoproject.org/meta-arm/plain/meta-arm-bsp/recipes-bsp/trusted-firmware-a/files/fvp-base/0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch
 \
+ --output 
0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
+git config --global user.email "y...@example.com" && \
+git config --global user.name "Your Name" && \
+git am 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
+make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 
FVP_DT_PREFIX=fvp-base-gicv3-psci-1t all && \
+make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 
FVP_DT_PREFIX=fvp-base-gicv3-psci-1t fip 
BL33=../u-boot-"$UBOOT_VERSION"/u-boot.bin && \
+cp build/fvp/debug/bl1.bin / && \
+cp build/fvp/debug/fip.bin / && \
+cp build/fvp/debug/fdts/fvp-base-gicv3-psci-1t.dtb / && \
+cd /build && \
+rm -rf u-boot-"$UBOOT_VERSION" trusted-firmware-a
-- 
2.25.1

[PATCH 0/5] automation: Support running FVP Dom0 smoke test for Arm

[Henry Wang]

This series adds the support for running FVP Dom0 smoke test for Arm on
the Arm64 GitLab CI runner. Detailed changes please refer to the commit
message of each commit.

An example test pipeline with these patches applied (with the docker
registry changed to my own registry and unrelated job removed) can be
found at:
https://gitlab.com/xen-project/people/henryw/xen/-/pipelines/1098531057

Henry Wang (5):
  automation: Add a Dockerfile for running FVP_Base jobs
  automation: Add the Dockerfile to build TF-A and U-Boot for FVP
  automation: Add the expect script with test case for FVP
  automation: Add the script for the FVP smoke test
  automation: Add the arm64 FVP build and Dom0 smoke test jobs

 .../debian/bookworm-arm64v8-fvp.dockerfile|  64 ++
 automation/gitlab-ci/build.yaml   |  17 +++
 automation/gitlab-ci/test.yaml|  22 
 .../expect/fvp-base-smoke-dom0-arm64.exp  |  73 +++
 .../scripts/fvp-base-smoke-dom0-arm64.sh  | 117 ++
 .../2023.10-2.9.0-arm64v8.dockerfile  |  48 +++
 6 files changed, 341 insertions(+)
 create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile
 create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
 create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh
 create mode 100644 
automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile

-- 
2.25.1

[PATCH 3/5] automation: Add the expect script with test case for FVP

[Henry Wang]

To interact with the FVP (for example entering the U-Boot shell
and transferring the files by TFTP), we need to connect the
corresponding port by the telnet first. Use an expect script to
simplify the automation of the whole "interacting with FVP" stuff.

The expect script will firstly detect the IP of the host, then
connect to the telnet port of the FVP, set the `serverip` and `ipaddr`
for the TFTP service in the U-Boot shell, and finally boot Xen from
U-Boot and wait for the expected results by Xen, Dom0 and DomU.

Signed-off-by: Henry Wang 
---
 .../expect/fvp-base-smoke-dom0-arm64.exp  | 73 +++
 1 file changed, 73 insertions(+)
 create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp

diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp 
b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
new file mode 100755
index 00..25d9a5f81c
--- /dev/null
+++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
@@ -0,0 +1,73 @@
+#!/usr/bin/expect
+
+set timeout 2000
+
+# Command to use to run must be given as first argument
+# if options are required, quotes must be used:
+# xxx.exp "cmd opt1 opt2"
+set runcmd [lindex $argv 0]
+
+# Maximum number of line to be printed, this can be used to prevent runs to
+# go forever on errors when Xen is rebooting
+set maxline 1000
+
+# Configure slow parameters used with send -s
+# This allows us to slow down console writes to prevent issues with slow
+# emulators or targets.
+# Format here is: {NUM TIME} which reads as wait TIME seconds each NUM of
+# characters, here we send 1 char each 100ms
+set send_slow {1 .1}
+
+proc test_boot {{maxline} {host_ip}} {
+expect_after {
+-re "(.*)\r" {
+if {$maxline != 0} {
+set maxline [expr {$maxline - 1}]
+if {$maxline <= 0} {
+send_user "ERROR-Toomuch!\n"
+exit 2
+}
+}
+exp_continue
+}
+timeout {send_user "ERROR-Timeout!\n"; exit 3}
+eof {send_user "ERROR-EOF!\n"; exit 4}
+}
+
+# Extract the telnet port numbers from FVP output, because the telnet ports
+# are not guaranteed to be fixed numbers.
+expect -re {terminal_0: Listening for serial connection on port [0-9]+}
+set terminal_0 $expect_out(0,string)
+if {[regexp {port (\d+)} $terminal_0 match port_0]} {
+puts "terminal_0 port is $port_0"
+} else {
+puts "terminal_0 port not found"
+exit 5
+}
+
+spawn bash -c "telnet localhost $port_0"
+expect -re "Hit any key to stop autoboot.*"
+send -s "  \r"
+send -s "setenv serverip $host_ip; setenv ipaddr $host_ip; tftpb 
0x8020 boot.scr; source 0x8020\r"
+
+# Initial Xen boot
+expect -re "\(XEN\).*Freed .* init memory."
+
+# Dom0 and DomU
+expect -re "Domain-0.*"
+expect -re "BusyBox.*"
+expect -re "/ #.*"
+}
+
+# Get host IP
+spawn bash -c "hostname -I | awk '{print \$1}'"
+expect -re {(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})}
+set host_ip $expect_out(0,string)
+
+# Start the FVP and run the test
+spawn bash -c "$runcmd"
+
+test_boot 2000 "$host_ip"
+
+send_user "\nExecution with SUCCESS\n"
+exit 0
-- 
2.25.1

[PATCH 1/5] automation: Add a Dockerfile for running FVP_Base jobs

[Henry Wang]

Fixed Virtual Platforms (FVPs) are complete simulations of an Arm
system, including processor, memory and peripherals. These are set
out in a "programmer's view", which gives programmers a comprehensive
model on which to build and test software. FVP can be configured to
different setups by its cmdline parameters, and hence having the FVP
in CI will provide us with the flexibility to test Arm features and
setups that we find difficult to use real hardware or emulators.

This commit adds a Dockerfile for the new arm64v8 container with
FVP installed, based on the debian bookworm-arm64v8 image. This
container will be used to run the FVP test jobs. Compared to the
debian bookworm-arm64v8 image, the packages in the newly added FVP
container does not contain the `u-boot-qemu`, and adds the `expect`
to run expect scripts introduced by following commits, `telnet` to
connect to FVP, and `tftpd-hpa` to provide the TFTP service for
the FVP.

Signed-off-by: Henry Wang 
---
 .../debian/bookworm-arm64v8-fvp.dockerfile| 64 +++
 1 file changed, 64 insertions(+)
 create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile

diff --git a/automation/build/debian/bookworm-arm64v8-fvp.dockerfile 
b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
new file mode 100644
index 00..3b87dc5a5b
--- /dev/null
+++ b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
@@ -0,0 +1,64 @@
+FROM --platform=linux/arm64/v8 debian:bookworm
+LABEL maintainer.name="The Xen Project" \
+  maintainer.email="xen-devel@lists.xenproject.org"
+
+ARG FVP_BASE_VERSION="11.23_9_Linux64_armv8l"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+# build depends
+RUN apt-get update && \
+apt-get --quiet --yes install \
+build-essential \
+zlib1g-dev \
+libncurses5-dev \
+libssl-dev \
+python3-dev \
+python3-setuptools \
+xorg-dev \
+uuid-dev \
+libyajl-dev \
+libaio-dev \
+libglib2.0-dev \
+clang \
+libpixman-1-dev \
+pkg-config \
+flex \
+bison \
+acpica-tools \
+libfdt-dev \
+bin86 \
+bcc \
+liblzma-dev \
+libnl-3-dev \
+ocaml-nox \
+libfindlib-ocaml-dev \
+markdown \
+transfig \
+pandoc \
+checkpolicy \
+wget \
+git \
+nasm \
+# for test phase, fvp-smoke-* jobs
+u-boot-tools \
+expect \
+device-tree-compiler \
+curl \
+cpio \
+busybox-static \
+telnet \
+tftpd-hpa \
+&& \
+apt-get autoremove -y && \
+apt-get clean && \
+rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
+
+RUN wget 
https://developer.arm.com/-/media/Files/downloads/ecosystem-models/FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz
 && \
+mkdir -p /FVP/FVP_Base_RevC-2xAEMvA && \
+tar -xvzf FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz -C 
/FVP/FVP_Base_RevC-2xAEMvA && \
+rm FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz
-- 
2.25.1

Re: [PATCH 2/3] xen/x86: address violations of MISRA C:2012 Rule 14.4

[Simone Ballarin]

On 07/12/23 11:54, Jan Beulich wrote:

On 07.12.2023 10:48, Simone Ballarin wrote:

--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -279,7 +279,7 @@ static int hpet_msi_write(struct hpet_event_channel *ch, 
struct msi_msg *msg)
  {
  ch->msi.msg = *msg;
  
-if ( iommu_intremap )

+if ( iommu_intremap != iommu_intremap_off )
  {
  int rc = iommu_update_ire_from_msi(>msi, msg);
  
@@ -353,7 +353,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)

  u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
  irq_desc_t *desc = irq_to_desc(ch->msi.irq);
  
-if ( iommu_intremap )

+if ( iommu_intremap != iommu_intremap_off )
  {
  ch->msi.hpet_id = hpet_blockid;
  ret = iommu_setup_hpet_msi(>msi);
@@ -372,7 +372,7 @@ static int __init hpet_setup_msi_irq(struct 
hpet_event_channel *ch)
  ret = __hpet_setup_msi_irq(desc);
  if ( ret < 0 )
  {
-if ( iommu_intremap )
+if ( iommu_intremap != iommu_intremap_off )
  iommu_update_ire_from_msi(>msi, NULL);
  return ret;
  }
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 7f8e794254..72dce2e4ab 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -189,7 +189,7 @@ static int write_msi_msg(struct msi_desc *entry, struct 
msi_msg *msg)
  {
  entry->msg = *msg;
  
-if ( iommu_intremap )

+if ( iommu_intremap != iommu_intremap_off )
  {
  int rc;
  
@@ -555,7 +555,7 @@ int msi_free_irq(struct msi_desc *entry)

  destroy_irq(entry[nr].irq);
  
  /* Free the unused IRTE if intr remap enabled */

-if ( iommu_intremap )
+if ( iommu_intremap != iommu_intremap_off )
  iommu_update_ire_from_msi(entry + nr, NULL);
  }
  


All of this would logically be part of patch 1. Is there a particular reason
why it wasn't done right there?


These changes and the ones in patch 1 are related, but still remain
independent. Patch 1 can be accepted without patch 2 and vice versa.
So we've decided to split the commits because patch 1 is in common
code, while patch 2 is in x86-specific code.

No other real reasons, but in any case we can move these changes to
patch 1.




--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1320,7 +1320,7 @@ x86_emulate(
  ea.bytes = 2;
  goto srcmem_common;
  case SrcMem:
-if ( state->simd_size )
+if ( state->simd_size != simd_none )
  break;
  ea.bytes = (d & ByteOp) ? 1 : op_bytes;
  srcmem_common:
@@ -1460,7 +1460,7 @@ x86_emulate(
  /* Becomes a normal DstMem operation from here on. */
  case DstMem:
  generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD);
-if ( state->simd_size )
+if ( state->simd_size != simd_none )
  {
  generate_exception_if(lock_prefix, X86_EXC_UD);
  break;
@@ -8176,7 +8176,7 @@ x86_emulate(
  goto done;
  }
  
-if ( state->rmw )

+if ( state->rmw != rmw_NONE )
  {
  ea.val = src.val;
  op_bytes = dst.bytes;
@@ -8205,7 +8205,7 @@ x86_emulate(
  
  dst.type = OP_NONE;

  }
-else if ( state->simd_size )
+else if ( state->simd_size != simd_none )
  {
  generate_exception_if(!op_bytes, X86_EXC_UD);
  generate_exception_if((vex.opcx && (d & TwoOp) &&


I'd be (somewhat reluctantly) okay with ack-ing this part of the patch.

Jan


--
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)

Re: [XEN PATCH v2 3/3] xen/mm: add declaration for first_valid_mfn

[Julien Grall]

Hi,

On 07/12/2023 11:11, Nicola Vetrini wrote:

Such declaration is needed to comply with MISRA C Rule 8.4, because a
compatible declaration is not visible in xen/common/page_alloc.c, where the
variable is defined. That variable can't yet be static because of the lack of
support from ARM and PPC for NUMA.

No functional change.

Signed-off-by: Nicola Vetrini 
---
Having this declaration essentially sidesteps the current impossibility
of having a static variable, as described in the comments in
ARM and PCC's asm/numa.h.
With this change, is there any reason to keep the various declaration of 
first_valid_mfn in numa.h?


Cheers,

--
Julien Grall

Re: [PATCH v2 01/39] xen/riscv: disable unnecessary configs

[Oleksii]

On Thu, 2023-12-07 at 11:00 +0100, Jan Beulich wrote:
> On 07.12.2023 10:22, Oleksii wrote:
> > On Tue, 2023-12-05 at 16:38 +0100, Jan Beulich wrote:
> > > > On 24.11.2023 11:30, Oleksii Kurochko wrote:
> > > > > > The patch also fixes the build script as conf util expects
> > > > > > to have each config on separate line.
> > > > 
> > > > The approach doesn't really scale (it's already odd that you
> > > > add
> > > > the
> > > > (apparently) same set four times. There's also zero
> > > > justification
> > > > for
> > > > this kind of an adjustment (as per discussion elsewhere I don't
> > > > think
> > > > we should go this route, and hence arguments towards convincing
> > > > me
> > > > [and
> > > > perhaps others] would be needed here).
> > I agree that this may not be the best approach, but it seems like
> > we
> > don't have too many options to turn off a config for randconfig.
> > 
> > To be honest, in my opinion (IMO), randconfig should either be
> > removed
> > or allowed to fail until most of the functionality is ready.
> > Otherwise,
> > there should be a need to introduce HAS_* or depend on
> > 'SUPPORTED_ARCHS' for each config, or introduce a lot of stubs.
> > 
> > Could you please suggest a better option?
> 
> As to dropping randconfig tests, I'd like to refer you to Andrew, who
> is of the opinion that it was wrong to drop them for ppc. (I'm
> agreeing
> with him when taking a theoretical perspective, but I'm not happy
> with
> the practical consequences.)
> 
> As to a better approach: Instead of listing the same set of options
> several times, can't there be a template config which is used to
> force
> randconfig to not touch certain settings? In fact at least for non-
> randconfig purposes I thought tiny64_defconfig / riscv64_defconfig
> already serve kind of a similar purpose. Imo the EXTRA_*CONFIG
> overrides
> are there for at most very few special case settings, not for
> purposes
> like you use them here.
The template will be the really a good option.

What do you think about the following patch which introduces arch-
specific allrandom.config?

diff --git a/xen/Makefile b/xen/Makefile
index ca571103c8..cb1eca76c2 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -336,11 +336,14 @@ ifeq ($(config-build),y)
 # *config targets only - make sure prerequisites are updated, and
descend
 # in tools/kconfig to make the *config target
 
+ARCH_ALLRANDOM_CONFIG :=
$(srctree)/arch/$(SRCARCH)/configs/allrandom.config
+
 # Create a file for KCONFIG_ALLCONFIG which depends on the
environment.
 # This will be use by kconfig targets
allyesconfig/allmodconfig/allnoconfig/randconfig
 filechk_kconfig_allconfig = \
 $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo
'CONFIG_XSM_FLASK_POLICY=n';) \
-$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
+$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
+$(if $(wildcard $(ARCH_ALLRANDOM_CONFIG)), cat
$(ARCH_ALLRANDOM_CONFIG);) ) \
 :

If this patch is OK then it can be reused for patches:
https://lore.kernel.org/xen-devel/cdc20255540a66ba0b6946ac6d48c11029cd3385.1701453087.git.oleksii.kuroc...@gmail.com/
https://lore.kernel.org/xen-devel/d42a34866edc70a12736b5c6976aa1b44b4ebd8a.1701453087.git.oleksii.kuroc...@gmail.com/

> 
> > > > > > --- a/automation/gitlab-ci/build.yaml
> > > > > > +++ b/automation/gitlab-ci/build.yaml
> > > > > > @@ -522,6 +522,38 @@ archlinux-current-gcc-riscv64:
> > > > > >  CONTAINER: archlinux:current-riscv64
> > > > > >  KBUILD_DEFCONFIG: tiny64_defconfig
> > > > > >  HYPERVISOR_ONLY: y
> > > > > > +    EXTRA_XEN_CONFIG:
> > > > > > +  CONFIG_COVERAGE=n
> > > > > > +  CONFIG_GRANT_TABLE=n
> > > > > > +  CONFIG_SCHED_CREDIT=n
> > > > > > +  CONFIG_SCHED_CREDIT2=n
> > > > > > +  CONFIG_SCHED_RTDS=n
> > > > > > +  CONFIG_SCHED_NULL=n
> > > > > > +  CONFIG_SCHED_ARINC653=n
> > > > > > +  CONFIG_TRACEBUFFER=n
> > > > > > +  CONFIG_HYPFS=n
> > > > > > +  CONFIG_GRANT_TABLE=n
> > > > > > +  CONFIG_SPECULATIVE_HARDEN_ARRAY=n
> > > > > > +  CONFIG_ARGO=n
> > > > > > +  CONFIG_HYPFS_CONFIG=n
> > > > > > +  CONFIG_CORE_PARKING=n
> > > > > > +  CONFIG_DEBUG_TRACE=n
> > > > > > +  CONFIG_IOREQ_SERVER=n
> > > > > > +  CONFIG_CRASH_DEBUG=n
> > > > > > +  CONFIG_KEXEC=n
> > > > > > +  CONFIG_LIVEPATCH=n
> > > > > > +  CONFIG_MEM_ACCESS=n
> > > > > > +  CONFIG_NUMA=n
> > > > > > +  CONFIG_PERF_COUNTERS=n
> > > > > > +  CONFIG_HAS_PMAP=n
> > > > > > +  CONFIG_TRACEBUFFER=n
> > > > > > +  CONFIG_XENOPROF=n
> > > > > > +  CONFIG_COMPAT=n
> > > > > > +  CONFIG_COVERAGE=n
> > > > > > +  CONFIG_UBSAN=n
> > > > > > +  CONFIG_NEEDS_LIBELF=n
> > > > > > +  CONFIG_XSM=n
> > > > > > +
> > > > > >  
> > > > > >  archlinux-current-gcc-riscv64-debug:
> > > > > >    extends: .gcc-riscv64-cross-build-debug
> > > > 
> > > > I think I've said so elsewhere before: Please avoid introducing
> > > > double
> > > > 

Re: [PATCH v2] tools/libs/evtchn: replace assert()s in stubdom with proper locking

[Jason Andryuk]

On Thu, Dec 7, 2023 at 1:26 AM Juergen Gross  wrote:
>
> In tools/libs/evtchn/minios.c there are assert()s for the current
> thread being the main thread when binding an event channel.
>
> As Mini-OS is supporting multiple threads, there is no real reason
> why the binding shouldn't be allowed to happen in any other thread.
>
> Drop the assert()s and replace them with proper locking of the
> port_list.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Jason Andryuk 

Thank you for doing this.

-Jason

Re: [PATCH v2 0/5] tools/xenstored: remove some command line options

[Julien Grall]

On 07/12/2023 08:26, Juergen Gross wrote:

On 21.11.23 12:40, Juergen Gross wrote:

Remove some command line options which have no real use case.

Changes in V2:
- moved removal of "-N" into last patch of the series, as this is the
   only option which seems to have a use case (OTOH using it has some
   downsides as well).

Juergen Gross (5):
   tools/xenstored: remove "-D" command line parameter
   tools/xenstored: remove "-V" command line option
   tools/xenstored: remove the "-P" command line option
   tools/xenstored: remove the "-R" command line option
   tools/xenstored: remove "-N" command line option

  tools/xenstored/core.c | 81 +++---
  1 file changed, 12 insertions(+), 69 deletions(-)



I think at least patches 1-4 can go in as they all have the required Acks.


Thanks for the reminder. I have committed patches #1-#4.

Cheers,

--
Julien Grall

Re: [PATCH] xen/arm: bootfdt: Check return code of device_tree_for_each_node()

[Julien Grall]

On 07/12/2023 12:20, Julien Grall wrote:

Hi Michal,

On 07/12/2023 10:14, Michal Orzel wrote:
As a result of not checking the return code of 
device_tree_for_each_node()

in boot_fdt_info(), any error occured during early FDT parsing does not
stop Xen from booting. This can result in an unwanted behavior in later
boot stages. Fix it by checking the return code and panicing on an error.

Fixes: 9cf4a9a46717 ("device tree: add device_tree_for_each_node()")
Signed-off-by: Michal Orzel 


With one remark below:

Acked-by: Julien Grall 


It is now committed.

Cheers,

--
Julien Grall

Re: [PATCH v2 3/3] xen/sched: do some minor cleanup of sched_move_domain()

[George Dunlap]

On Mon, Dec 4, 2023 at 3:23 PM Juergen Gross  wrote:
>
> Do some minor cleanups:
>
> - Move setting of old_domdata and old_units next to each other
> - Drop incrementing unit_idx in the final loop of sched_move_domain()
>   as it isn't used afterwards
> - Rename new_p to new_cpu and unit_p to unit_cpu
>
> Signed-off-by: Juergen Gross 

Reviewed-by: George Dunlap 

Re: ARM: MISRA C:2012 Rule 5.6

[Julien Grall]

On 07/12/2023 12:44, Jan Beulich wrote:

On 07.12.2023 12:48, Federico Serafini wrote:

On 07/12/23 12:43, Federico Serafini wrote:

Hello everyone,

Rule 5.6 states that a typedef name shall be a unique identifier.
This is to avoid developer confusion.

For ARM, the violations left [1] are generated by two definitions
of the type phys_addr_t within two different files.
I would like to ask if this is intentional or not:
if it is intentional and it is not causing any confusion between XEN
developers, then I think violations involving phys_addr_t can be
deviated.

[1]
https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/staging/ARM64-2023/429/PROJECT.ecd;/by_service/MC3R1.R5.6.html


Adding XEN mailing list in CC.


Thanks.

These are files ported from Linux, where I assume the typedef-s were added
to limit the changes which would need making elsewhere. Still I think that's
exactly what we (now) have xen/linux-compat.h for. IOW - just move the
typedef-s there?


+1. I was about to suggest the same.

Cheers,

--
Julien Grall

Re: [PATCH] xen/arm: bootfdt: Check return code of device_tree_for_each_node()

[Julien Grall]

Hi Michal,

On 07/12/2023 12:39, Michal Orzel wrote:

On 07/12/2023 13:20, Julien Grall wrote:



Hi Michal,

On 07/12/2023 10:14, Michal Orzel wrote:

As a result of not checking the return code of device_tree_for_each_node()
in boot_fdt_info(), any error occured during early FDT parsing does not
stop Xen from booting. This can result in an unwanted behavior in later
boot stages. Fix it by checking the return code and panicing on an error.

Fixes: 9cf4a9a46717 ("device tree: add device_tree_for_each_node()")
Signed-off-by: Michal Orzel 


With one remark below:

Acked-by: Julien Grall 


---
I've lost count how many times I had to fix missing rc check. However, I have
not yet found any checker for this (-Wunused-result is pretty useless).
---
   xen/arch/arm/bootfdt.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index b1f03eb2fcdd..f496a8cf9494 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -541,7 +541,9 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)

   add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);

-device_tree_for_each_node((void *)fdt, 0, early_scan_node, NULL);
+ret = device_tree_for_each_node((void *)fdt, 0, early_scan_node, NULL);
+if ( ret )
+panic("Early FDT parsing failed (%d)\n", ret);


AFAIU, the parsing is done before the console is setup. This means a
user would not be able to get some logs unless they are re-compiling Xen
with earlyprintk.

I understand this is not a new issue, but I am getting concerned of the
amount of check we add before the console is up and running.

What is the impact if we don't check the return here? Is it missing regions?

There are many things that can go wrong.
Quite recently, I faced an issue where I specified 2 dom0less domUs in 
configuration
and due to the error while parsing the last node of domU1, domU2 node was 
skipped and
Xen booted only domU1 without giving any errors.

Issues with shared memory led to either Xen continue to run with improper 
configuration,
silencing overlap conditions, errors at later boot stages that were impossible 
to deduct
from the logs.

All in all, early boot code parsing assume the error to result in a failure and 
the parsing
for domain creation makes this assumption as well (checks are more relaxed to 
avoid duplication).

For now, we can't do anything better than panicing early if we want to avoid 
unwanted behavior.



I wonder whether we can either enable the console earlier, or make
earlyprintk more dynamic (similar to what Linux is doing with
earlyprintk=...).

The most imporatant part is early fdt parsing. The main console init cannot be 
moved that early.


I think we need to understand a bit more why because on x86 
consoel_init_preirq() is called very early. So we ought to be able to do 
the same on Arm.


I will add this in my list after I get the earlyprintk working while 
switching the MMU.


Cheers,

--
Julien Grall