[GIT PULL] xen: branch for v6.3-rc3

2023-03-16 Thread Juergen Gross
Linus,

Please git pull the following tag:

 git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git for-linus-6.3-rc3-tag

xen: branch for v6.3-rc3

It contains:

- a small cleanup series for xen time handling
- a patch for enabling the VGA console in a Xen PVH dom0
- a small cleanup patch for the xenfs driver

Thanks.

Juergen

 arch/x86/include/asm/xen/cpuid.h | 22 ++
 arch/x86/xen/Makefile|  2 +-
 arch/x86/xen/enlighten_pv.c  |  3 ++-
 arch/x86/xen/enlighten_pvh.c | 13 +
 arch/x86/xen/time.c  |  7 ++-
 arch/x86/xen/vga.c   |  5 ++---
 arch/x86/xen/xen-ops.h   |  7 ---
 drivers/xen/xenfs/xensyms.c  | 10 +-
 include/xen/interface/platform.h |  3 +++
 9 files changed, 50 insertions(+), 22 deletions(-)

Jan Beulich (1):
  x86/PVH: obtain VGA console info in Dom0

Krister Johansen (2):
  xen: update arch/x86/include/asm/xen/cpuid.h
  x86/xen/time: cleanup xen_tsc_safe_clocksource

Yu Zhe (1):
  xen: remove unnecessary (void*) conversions



[ovmf test] 179705: all pass - PUSHED

2023-03-16 Thread osstest service owner
flight 179705 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179705/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 0e5717009779ec6c1e35f979426a2cd407b3e73a
baseline version:
 ovmf 16e0969ef775b898ac700f3261d76030b8ab9ef0

Last test of basis   179698  2023-03-16 21:45:32 Z0 days
Testing same since   179705  2023-03-17 04:10:46 Z0 days1 attempts


People who touched revisions under test:
  Matt DeVillier 

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
   16e0969ef7..0e57170097  0e5717009779ec6c1e35f979426a2cd407b3e73a -> 
xen-tested-master



[qemu-mainline test] 179682: regressions - trouble: fail/pass/starved

2023-03-16 Thread osstest service owner
flight 179682 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179682/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt  14 guest-start  fail REGR. vs. 179518
 test-amd64-i386-libvirt-xsm  14 guest-start  fail REGR. vs. 179518
 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 179518
 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 179518
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-installfail REGR. vs. 179518
 test-amd64-i386-libvirt-pair 25 guest-start/debian   fail REGR. vs. 179518
 test-amd64-amd64-xl-qcow212 debian-di-installfail REGR. vs. 179518
 test-amd64-amd64-libvirt-vhd 12 debian-di-installfail REGR. vs. 179518
 test-amd64-amd64-libvirt-xsm 14 guest-start  fail REGR. vs. 179518
 test-arm64-arm64-libvirt-xsm 14 guest-start  fail REGR. vs. 179518
 test-amd64-i386-libvirt-raw  12 debian-di-installfail REGR. vs. 179518
 test-amd64-amd64-libvirt 14 guest-start  fail REGR. vs. 179518
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install 
fail REGR. vs. 179518
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install 
fail REGR. vs. 179518
 test-arm64-arm64-libvirt-raw 12 debian-di-installfail REGR. vs. 179518
 test-amd64-i386-xl-vhd   12 debian-di-installfail REGR. vs. 179518
 test-arm64-arm64-xl-vhd  12 debian-di-installfail REGR. vs. 179518
 test-amd64-amd64-libvirt-pair 25 guest-start/debian  fail REGR. vs. 179518

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 179518
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 179518
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 179518
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 179518
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 179518
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-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-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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   starved  n/a
 build-armhf-libvirt   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   starved  n/a
 build-armhf   2 hosts-allocate   starved  n/a

version targeted for testing:
 qemuu9636e513255362c4a329e3e5fb2c97dab3c5ce47
baseline version:
 qemuu7b0f0aa55fd292fa3489755a3a896e496c51ea86

Last test of basis   179518  2023-03-09 10:37:19 Z7 days
Failing since179526  2023-03-10 01:53:40 Z7 days   12 attempts
Testing same since   179682  2023-03-16 13:21:44 Z0 days1 attempts


People who touched revisions under test:
  Akihiko Odaki 
  Albert Esteve 
  Alex Bennée 
  Alex Williamson 
  Alistair Francis 
  Andreas Schwab 
  Anton Johansson 
  Avihai Horon 
  BALATON Zoltan 
  Bernhard Beschow 
  Carlos López 
  Cédric Le Goater 
  Cédric Le Goater 
  Damien Hedde 
  Daniel P. Berrangé 
  David Hildenbrand 
  David Woodhouse 
  David Woodhouse 
  Dr. David Alan Gilbert 
  Eugenio Pérez 
  Fabiano Rosas 
  Fan Ni 
  f

[PATCH 0/2] automation: add another Gitlab-CI test, x86-64 S3 this time

2023-03-16 Thread Marek Marczykowski-Górecki
This series adds another Gitlab-CI test running on a real hardware.
The gitlab-runner I have currently connected to
https://gitlab.com/marmarek/xen. I'll need a registration token to connect it to
the official repo.
Example test run: https://gitlab.com/marmarek/xen/-/jobs/3952014197

Details about the setup:
https://www.qubes-os.org/news/2022/05/05/automated-os-testing-on-physical-laptops/

Marek Marczykowski-Górecki (2):
  automation: build 6.1.19 kernel for x86-64 dom0
  automation: add a suspend test on an Alder Lake system

 automation/gitlab-ci/build.yaml |  11 +-
 automation/gitlab-ci/test.yaml  |  31 +++-
 automation/scripts/qubes-x86-64-suspend.sh  | 155 +-
 automation/tests-artifacts/kernel/6.1.19.dockerfile |  40 +++-
 4 files changed, 237 insertions(+)
 create mode 100755 automation/scripts/qubes-x86-64-suspend.sh
 create mode 100644 automation/tests-artifacts/kernel/6.1.19.dockerfile

base-commit: be62b1fc2aa7375d553603fca07299da765a89fe
-- 
git-series 0.9.1



[PATCH 2/2] automation: add a suspend test on an Alder Lake system

2023-03-16 Thread Marek Marczykowski-Górecki
This is a first test using Qubes OS CI infra. The gitlab-runner has
access to ssh-based control interface (control@thor.testnet, ssh key
exposed to the test via ssh-agent) and pre-configured HTTP dir for boot
files (mapped under /scratch/gitlab-runner/tftp inside the container).
Details about the setup are described on
https://www.qubes-os.org/news/2022/05/05/automated-os-testing-on-physical-laptops/

This test boots Xen, and try if S3 works. It runs on a ADL-based desktop
system. The test script is based on the Xilinx one.

The machine needs newer kernel than other x86 tests run, so use 6.1.x
kernel added in previous commit.

When building rootfs, use fakeroot to preserve device files when
repacking rootfs archives in a non-privileged container.

Signed-off-by: Marek Marczykowski-Górecki 
---
I'm bad at naming things. Things that I could use naming suggestions:
 - test script (qubes-x86-64-suspend.sh) - this might be okay?
 - test template job name (.adl-x86-64)
 - test job name (adl-suspend-x86-64-gcc)
 - tag (qubes-hw2)

For context, our CI has several machines, named test-X or hwX, each
controlled with matching hal900X RPi (this is where gitlab-runner is).
This test uses only one specific hw, but I plan adding few others too.
---
 automation/gitlab-ci/test.yaml |  31 -
 automation/scripts/qubes-x86-64-suspend.sh | 155 ++-
 2 files changed, 186 insertions(+)
 create mode 100755 automation/scripts/qubes-x86-64-suspend.sh

diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 2e1a6886df7f..f5511dd6da70 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -15,6 +15,10 @@
 .arm32-test-needs: &arm32-test-needs
   - qemu-system-aarch64-6.0.0-arm32-export
 
+.x86-64-test-needs: &x86-64-test-needs
+  - alpine-3.12-rootfs-export
+  - kernel-6.1.19-export
+
 .qemu-arm64:
   extends: .test-jobs-common
   variables:
@@ -84,6 +88,25 @@
   tags:
 - xilinx
 
+.adl-x86-64:
+  extends: .test-jobs-common
+  variables:
+# the test controller runs on RPi4
+CONTAINER: alpine:3.12-arm64v8
+LOGFILE: smoke-test.log
+  artifacts:
+paths:
+  - smoke.serial
+  - '*.log'
+when: always
+  only:
+variables:
+  - $QUBES_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
+  before_script:
+- apk --no-cache add openssh-client fakeroot
+  tags:
+- qubes-hw2
+
 # Test jobs
 build-each-commit-gcc:
   extends: .test-jobs-common
@@ -110,6 +133,14 @@ xilinx-smoke-dom0less-arm64-gcc:
 - *arm64-test-needs
 - alpine-3.12-gcc-arm64
 
+adl-suspend-x86-64-gcc:
+  extends: .adl-x86-64
+  script:
+- ./automation/scripts/qubes-x86-64-suspend.sh s3 2>&1 | tee ${LOGFILE}
+  needs:
+- *x86-64-test-needs
+- alpine-3.12-gcc
+
 qemu-smoke-dom0-arm64-gcc:
   extends: .qemu-arm64
   script:
diff --git a/automation/scripts/qubes-x86-64-suspend.sh 
b/automation/scripts/qubes-x86-64-suspend.sh
new file mode 100755
index ..fc479c9faaec
--- /dev/null
+++ b/automation/scripts/qubes-x86-64-suspend.sh
@@ -0,0 +1,155 @@
+#!/bin/sh
+
+set -ex
+
+test_variant=$1
+
+wait_and_wakeup=
+if [ -z "${test_variant}" ]; then
+passed="ping test passed"
+domU_check="
+until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do
+sleep 30
+done
+echo \"${passed}\"
+"
+elif [ "${test_variant}" = "s3" ]; then
+passed="suspend test passed"
+wait_and_wakeup="started, suspending"
+domU_check="
+ifconfig eth0 192.168.0.2
+echo domU started
+"
+dom0_check="
+until grep 'domU started' /var/log/xen/console/guest-domU.log; do
+sleep 1
+done
+echo \"${wait_and_wakeup}\"
+set -x
+echo deep > /sys/power/mem_sleep
+echo mem > /sys/power/state
+# now wait for resume
+sleep 5
+# get domU console content into test log
+tail -n 100 /var/log/xen/console/guest-domU.log
+xl list
+xl dmesg | grep 'Finishing wakeup from ACPI S3 state' || exit 1
+# check if domU is still alive
+ping -c 10 192.168.0.2 || exit 1
+echo \"${passed}\"
+"
+fi
+
+# DomU
+mkdir -p rootfs
+cd rootfs
+fakeroot -s ../fakeroot-save tar xzf ../binaries/initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+echo "#!/bin/sh
+
+${domU_check}
+/bin/sh" > etc/local.d/xen.start
+chmod +x etc/local.d/xen.start
+echo "rc_verbose=yes" >> etc/rc.conf
+find . | fakeroot -i ../fakeroot-save cpio -H newc -o | gzip > 
../binaries/domU-rootfs.cpio.gz
+cd ..
+rm -rf rootfs
+
+# DOM0 rootfs
+mkdir -p rootfs
+cd rootfs
+fakeroot -s ../fakeroot-save tar xzf ../binaries/initrd.tar.gz
+mkdir boot
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+cp -ar ../binaries/dist/install/* .
+
+echo "#!/bin/bash
+
+export LD_LIBRARY_PATH=/usr/local/lib
+bash /etc/init.d/xencommons start
+
+brctl addbr xenbr0
+brctl addif xenbr0 eth0
+ifconfig eth0 up
+ifconfig xenbr0 up
+ifconfig xenbr0 192.168.0.1
+
+xl create /etc/xen/domU.cfg
+${dom0_check}
+" > etc/local.d/xen.start
+chmod +x etc/local.d/xen.start
+# just PVH for now

[PATCH 1/2] automation: build 6.1.19 kernel for x86-64 dom0

2023-03-16 Thread Marek Marczykowski-Górecki
It will be used in tests added in subsequent patches.
Enable config options needed for those tests.

Signed-off-by: Marek Marczykowski-Górecki 
---
 automation/gitlab-ci/build.yaml | 11 -
 automation/tests-artifacts/kernel/6.1.19.dockerfile | 40 ++-
 2 files changed, 51 insertions(+)
 create mode 100644 automation/tests-artifacts/kernel/6.1.19.dockerfile

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 38bb22d8609b..e1799d454c76 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -790,3 +790,14 @@ kernel-5.10.74-export:
   - binaries/bzImage
   tags:
 - x86_64
+
+kernel-6.1.19-export:
+  extends: .test-jobs-artifact-common
+  image: registry.gitlab.com/xen-project/xen/tests-artifacts/kernel:6.1.19
+  script:
+- mkdir binaries && cp /bzImage binaries/bzImage
+  artifacts:
+paths:
+  - binaries/bzImage
+  tags:
+- x86_64
diff --git a/automation/tests-artifacts/kernel/6.1.19.dockerfile 
b/automation/tests-artifacts/kernel/6.1.19.dockerfile
new file mode 100644
index ..c2171555a0a6
--- /dev/null
+++ b/automation/tests-artifacts/kernel/6.1.19.dockerfile
@@ -0,0 +1,40 @@
+FROM debian:unstable
+LABEL maintainer.name="The Xen Project" \
+  maintainer.email="xen-devel@lists.xenproject.org"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV LINUX_VERSION=6.1.19
+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 \
+libelf-dev \
+&& \
+apt-get autoremove -y && \
+apt-get clean && \
+rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
+
+# Build the kernel
+RUN curl -fsSLO 
https://cdn.kernel.org/pub/linux/kernel/v6.x/linux-"$LINUX_VERSION".tar.xz && \
+tar xvJf linux-"$LINUX_VERSION".tar.xz && \
+cd linux-"$LINUX_VERSION" && \
+make defconfig && \
+make xen.config && \
+scripts/config --enable BRIDGE && \
+scripts/config --enable IGC && \
+cp .config .config.orig && \
+cat .config.orig | grep XEN | grep =m |sed 's/=m/=y/g' >> .config && \
+make -j$(nproc) bzImage && \
+cp arch/x86/boot/bzImage / && \
+cd /build && \
+rm -rf linux-"$LINUX_VERSION"*
-- 
git-series 0.9.1



RE: [PATCH v2 0/4] P2M improvements for Arm

2023-03-16 Thread Henry Wang
Hi everyone,

> -Original Message-
> Subject: [PATCH v2 0/4] P2M improvements for Arm
> 
> There are some clean-up/improvement work that can be done in the
> Arm P2M code triggered by [1] and [2]. These were found at the 4.17
> code freeze period so the issues were not fixed at that time.
> Therefore do the follow-ups here.
> 
> Patch#1 addresses one comment in [1]. It was sent earlier and reviewed
> once. Pick the updated version, i.e. "[PATCH v2] xen/arm: Reduce
> redundant clear root pages when teardown p2m", to this series.
> 
> Patch#2 is a new patch based on v1 comments, this is a pre-requisite
> patch for patch#3 where the deferring of GICv2 CPU interface mapping
> should also be applied for new vGIC.
> 
> Patch#3 and #4 addresses the comment in [2] following the discussion
> between two possible options.
> 
> [1] https://lore.kernel.org/xen-devel/a947e0b4-8f76-cea6-893f-
> abf30ff95...@xen.org/
> [2] https://lore.kernel.org/xen-devel/e6643bfc-5bdf-f685-1b68-
> b28d34107...@xen.org/
> 
> v1 -> v2:
> 1. Move in-code comment for p2m_force_tlb_flush_sync() on top of
>p2m_clear_root_pages().
> 2. Add a new patch as patch #2.
> 3. Correct style in in-code comment in patch #3.
> 4. Avoid open-coding gfn_eq() and gaddr_to_gfn(d->arch.vgic.cbase).
> 5. Apply same changes for the new vGICv2 implementation, update the
>commit message accordingly.
> 6. Add in-code comment in old GICv2's vgic_v2_domain_init() and
>new GICv2's vgic_v2_map_resources() to mention the mapping of the
>virtual CPU interface is deferred until first access.
> 7. Add reviewed-by and acked-by tags accordingly.
> 
> Henry Wang (4):
>   xen/arm: Reduce redundant clear root pages when teardown p2m
>   xen/arm: Rename vgic_cpu_base and vgic_dist_base for new vGIC
>   xen/arm: Defer GICv2 CPU interface mapping until the first access
>   xen/arm: Clean-up in p2m_init() and p2m_final_teardown()

Gentle ping of last 3 patches of this series as it has been a while. I
understand since we lost all arm32 boards in OSSTest so it is not likely
for a GICv2 series to progress, but this series passed our internal CI on
all GICv2 boards and it would be good to have some feedbacks so that
I can make the series ready to be committed before the arm32 boards
are re-added to OSSTest.

Thanks very much!

Kind regards,
Henry



Re: [PATCH v2 2/2] automation: arm64: Create test jobs for testing static shared memory on qemu

2023-03-16 Thread Stefano Stabellini
On Thu, 16 Mar 2023, jiamei.xie wrote:
> Create 2 new test jobs, called qemu-smoke-dom0less-arm64-gcc-static-shared-mem
> and qemu-smoke-dom0less-arm64-gcc-debug-static-shared-mem.
> 
> Adjust qemu-smoke-dom0less-arm64.sh script to accomodate the static
> shared memory test as a new test variant. The test variant is determined
> based on the first argument passed to the script. For testing static
> shared memory, the argument is 'static-shared-mem'.
> 
> The test configures two dom0less DOMUs with a static shared memory
> region and adds a check in the init script.
> 
> The check consists in comparing the contents of the 
> /proc/device-tree/reserved-memory
> xen-shmem entry with the static shared memory range and id with which
> DOMUs were configured. If the memory layout is correct, a message gets
> printed by DOMU.
> 
> At the end of the qemu run, the script searches for the specific message
> in the logs and fails if not found.
> 
> Signed-off-by: jiamei.xie 

Reviewed-by: Stefano Stabellini 


> ---
> Changes from v1:
>  - Move the second domU creation to the general ImageBuilder script.
> ---
>  automation/gitlab-ci/build.yaml   | 18 
>  automation/gitlab-ci/test.yaml| 16 ++
>  .../scripts/qemu-smoke-dom0less-arm64.sh  | 29 +++
>  3 files changed, 63 insertions(+)
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 38bb22d860..820cc0af83 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -623,6 +623,24 @@ alpine-3.12-gcc-debug-arm64-staticmem:
>CONFIG_UNSUPPORTED=y
>CONFIG_STATIC_MEMORY=y
>  
> +alpine-3.12-gcc-arm64-static-shared-mem:
> +  extends: .gcc-arm64-build
> +  variables:
> +CONTAINER: alpine:3.12-arm64v8
> +EXTRA_XEN_CONFIG: |
> +  CONFIG_UNSUPPORTED=y
> +  CONFIG_STATIC_MEMORY=y
> +  CONFIG_STATIC_SHM=y
> +
> +alpine-3.12-gcc-debug-arm64-static-shared-mem:
> +  extends: .gcc-arm64-build-debug
> +  variables:
> +CONTAINER: alpine:3.12-arm64v8
> +EXTRA_XEN_CONFIG: |
> +  CONFIG_UNSUPPORTED=y
> +  CONFIG_STATIC_MEMORY=y
> +  CONFIG_STATIC_SHM=y
> +
>  alpine-3.12-gcc-arm64-boot-cpupools:
>extends: .gcc-arm64-build
>variables:
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 37465305ff..d75662358f 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -174,6 +174,22 @@ qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
> - *arm64-test-needs
> - alpine-3.12-gcc-debug-arm64
>  
> +qemu-smoke-dom0less-arm64-gcc-static-shared-mem:
> +  extends: .qemu-arm64
> +  script:
> +- ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-shared-mem 
> 2>&1 | tee ${LOGFILE}
> +  needs:
> +- *arm64-test-needs
> +- alpine-3.12-gcc-arm64-static-shared-mem
> +
> +qemu-smoke-dom0less-arm64-gcc-debug-static-shared-mem:
> +  extends: .qemu-arm64
> +  script:
> +- ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-shared-mem 
> 2>&1 | tee ${LOGFILE}
> +  needs:
> +- *arm64-test-needs
> +- alpine-3.12-gcc-debug-arm64-static-shared-mem
> +
>  qemu-smoke-dom0less-arm64-gcc-boot-cpupools:
>extends: .qemu-arm64
>script:
> diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh 
> b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> index 2d69d976ea..75f575424a 100755
> --- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> +++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> @@ -32,6 +32,25 @@ if [[ "${test_variant}" == "static-heap" ]]; then
>  domU_check="echo \"${passed}\""
>  fi
>  
> +
> +if [[ "${test_variant}" == "static-shared-mem" ]]; then
> +passed="${test_variant} test passed"
> +SHARED_MEM_HOST="5000"
> +SHARED_MEM_GUEST="400"
> +SHARED_MEM_SIZE="1000"
> +SHARED_MEM_ID="my-shared-mem-0"
> +
> +domU_check="
> +current_id=\$(cat /proc/device-tree/reserved-memory/xen-shmem@400/xen,id 
> 2>/dev/null)
> +expected_id=\"\$(echo ${SHARED_MEM_ID})\"
> +current_reg=\$(hexdump -e '16/1 \"%02x\"' 
> /proc/device-tree/reserved-memory/xen-shmem@400/reg 2>/dev/null)
> +expected_reg=$(printf \"%016x%016x\" 0x${SHARED_MEM_GUEST} 
> 0x${SHARED_MEM_SIZE})
> +if [[ \"\${expected_reg}\" == \"\${current_reg}\" && \"\${current_id}\" == 
> \"\${expected_id}\" ]]; then
> +echo \"${passed}\"
> +fi
> +"
> +fi
> +
>  if [[ "${test_variant}" == "boot-cpupools" ]]; then
>  # Check if domU0 (id=1) is assigned to Pool-1 with null scheduler
>  passed="${test_variant} test passed"
> @@ -124,6 +143,9 @@ NUM_DOMUS=1
>  DOMU_KERNEL[0]="Image"
>  DOMU_RAMDISK[0]="initrd"
>  DOMU_MEM[0]="256"
> +DOMU_KERNEL[1]="Image"
> +DOMU_RAMDISK[1]="initrd"
> +DOMU_MEM[1]="256"
>  
>  LOAD_CMD="tftpb"
>  UBOOT_SOURCE="boot.source"
> @@ -133,6 +155,13 @@ if [[ "${test_variant}" == "static-mem" ]]; then
>  echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_siz

[ovmf test] 179698: all pass - PUSHED

2023-03-16 Thread osstest service owner
flight 179698 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179698/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 16e0969ef775b898ac700f3261d76030b8ab9ef0
baseline version:
 ovmf 997c6967b00cdd797fe787567a28a7565aafd301

Last test of basis   179690  2023-03-16 16:12:11 Z0 days
Testing same since   179698  2023-03-16 21:45:32 Z0 days1 attempts


People who touched revisions under test:
  Ard Biesheuvel 

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
   997c6967b0..16e0969ef7  16e0969ef775b898ac700f3261d76030b8ab9ef0 -> 
xen-tested-master



[linux-linus test] 179675: regressions - trouble: fail/pass/starved

2023-03-16 Thread osstest service owner
flight 179675 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179675/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-credit1  20 guest-localmigrate/x10   fail REGR. vs. 178042
 test-amd64-amd64-xl-shadow   18 guest-localmigrate   fail REGR. vs. 178042
 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start   fail REGR. vs. 178042
 test-amd64-amd64-xl-multivcpu 19 guest-saverestore.2 fail REGR. vs. 178042
 test-amd64-amd64-xl-xsm  19 guest-saverestore.2  fail REGR. vs. 178042
 test-amd64-amd64-xl-pvhv2-amd 14 guest-start fail REGR. vs. 178042
 test-amd64-amd64-freebsd11-amd64 13 guest-start  fail REGR. vs. 178042
 test-amd64-amd64-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 178042
 test-amd64-amd64-libvirt-xsm 14 guest-start  fail REGR. vs. 178042
 test-amd64-coresched-amd64-xl 17 guest-saverestore   fail REGR. vs. 178042
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 17 guest-saverestore.2 fail REGR. 
vs. 178042
 test-arm64-arm64-xl-thunderx 14 guest-start  fail REGR. vs. 178042
 test-arm64-arm64-libvirt-xsm 14 guest-start  fail REGR. vs. 178042
 test-amd64-amd64-xl-credit2 22 guest-start/debian.repeat fail REGR. vs. 178042
 test-arm64-arm64-xl-credit2 18 guest-start/debian.repeat fail REGR. vs. 178042
 test-arm64-arm64-xl  14 guest-start  fail REGR. vs. 178042
 test-amd64-amd64-qemuu-nested-intel 12 debian-hvm-install fail REGR. vs. 178042
 test-amd64-amd64-libvirt 14 guest-start  fail REGR. vs. 178042
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install 
fail REGR. vs. 178042
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 12 debian-hvm-install fail 
REGR. vs. 178042
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 12 debian-hvm-install fail 
REGR. vs. 178042
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 12 debian-hvm-install fail REGR. vs. 
178042
 test-amd64-amd64-xl-qemuu-ovmf-amd64 12 debian-hvm-install fail REGR. vs. 
178042
 test-amd64-amd64-xl-qemut-debianhvm-amd64 12 debian-hvm-install fail REGR. vs. 
178042
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 12 debian-hvm-install 
fail REGR. vs. 178042
 test-amd64-amd64-pair25 guest-start/debian   fail REGR. vs. 178042
 test-amd64-amd64-qemuu-nested-amd 12 debian-hvm-install  fail REGR. vs. 178042
 test-amd64-amd64-xl-vhd  12 debian-di-installfail REGR. vs. 178042
 test-amd64-amd64-pygrub  12 debian-di-installfail REGR. vs. 178042
 test-amd64-amd64-libvirt-raw 12 debian-di-installfail REGR. vs. 178042
 test-amd64-amd64-libvirt-qcow2 12 debian-di-install  fail REGR. vs. 178042
 test-arm64-arm64-xl-vhd  12 debian-di-installfail REGR. vs. 178042
 test-arm64-arm64-libvirt-raw 12 debian-di-installfail REGR. vs. 178042
 test-amd64-amd64-freebsd12-amd64 17 guest-localmigrate fail in 179656 REGR. 
vs. 178042
 test-amd64-amd64-xl-pvshim 17 guest-saverestore fail in 179656 REGR. vs. 178042
 test-amd64-amd64-xl   17 guest-saverestore fail in 179656 REGR. vs. 178042
 test-amd64-amd64-xl-pvhv2-intel 18 guest-localmigrate fail in 179656 REGR. vs. 
178042
 test-amd64-amd64-libvirt-pair 27 guest-migrate/dst_host/src_host fail in 
179656 REGR. vs. 178042
 test-arm64-arm64-xl-xsm 18 guest-start/debian.repeat fail in 179656 REGR. vs. 
178042
 test-arm64-arm64-xl-credit1 18 guest-start/debian.repeat fail in 179656 REGR. 
vs. 178042
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 18 guest-localmigrate/x10 fail in 
179656 REGR. vs. 178042

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-credit1  14 guest-start  fail in 179656 pass in 179675
 test-amd64-amd64-xl-shadow  17 guest-saverestore fail in 179656 pass in 179675
 test-amd64-amd64-xl-xsm18 guest-localmigrate fail in 179656 pass in 179675
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 15 guest-saverestore fail in 
179656 pass in 179675
 test-amd64-amd64-xl-rtds   18 guest-localmigrate fail in 179656 pass in 179675
 test-amd64-amd64-xl-credit2  21 guest-stop   fail in 179656 pass in 179675
 test-amd64-coresched-amd64-xl 14 guest-start fail in 179656 pass in 179675
 test-amd64-amd64-freebsd12-amd64 13 guest-startfail pass in 179656
 test-amd64-amd64-xl  14 guest-startfail pass in 179656
 test-amd64-amd64-libvirt-pair 25 guest-start/debianfail pass in 179656
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 15 guest-saverestore fail pass in 
179656
 test-arm64-arm64-xl-xsm  14 guest-startfail pass in 179656
 test-arm64-arm64-xl-credit1  17 guest-stop fail pass in 179656
 test-amd64-amd64-xl-pvshim   14 guest-startfail pass in 179656
 test-amd64-amd64-xl-pvhv2-intel 14 guest-start fail pass in 179656

Regressions which are regarded as allowable (not bloc

Re: [PATCH v2 1/2] automation: arm64: Create test jobs for testing static heap on qemu

2023-03-16 Thread Stefano Stabellini
On Thu, 16 Mar 2023, jiamei.xie wrote:
> From: Jiamei Xie 
> 
> Create 2 new test jobs, called qemu-smoke-dom0less-arm64-gcc-staticheap
> and qemu-smoke-dom0less-arm64-gcc-debug-staticheap.
> 
> Add property "xen,static-heap" under /chosen node to enable static-heap.
> If the domU can start successfully with static-heap enabled, then this
> test pass.
> 
> ImageBuillder sets the kernel and ramdisk range based on the file size.
> It will use the memory range between 0x4560 to 0x47AED1E8. It uses
> MEMORY_START and MEMORY_END from the cfg file as a range in which it can
> instruct u-boot where to place the images.
> 
> Change MEMORY_END to 0x5000 for all test cases.
> 
> Signed-off-by: Jiamei Xie 

Reviewed-by: Stefano Stabellini 


> ---
> Changes from v1:
>  - Change MEMORY_END to 0x5000 for all test cases.
>  - Update commit message.
> ---
>  automation/gitlab-ci/test.yaml| 16 ++
>  .../scripts/qemu-smoke-dom0less-arm64.sh  | 21 ++-
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 2e1a6886df..37465305ff 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -158,6 +158,22 @@ qemu-smoke-dom0less-arm64-gcc-debug-staticmem:
>  - *arm64-test-needs
>  - alpine-3.12-gcc-debug-arm64-staticmem
>  
> +qemu-smoke-dom0less-arm64-gcc-staticheap:
> + extends: .qemu-arm64
> + script:
> +   - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap 2>&1 | 
> tee ${LOGFILE}
> + needs:
> +   - *arm64-test-needs
> +   - alpine-3.12-gcc-arm64
> +
> +qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
> + extends: .qemu-arm64
> + script:
> +   - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap 2>&1 | 
> tee ${LOGFILE}
> + needs:
> +   - *arm64-test-needs
> +   - alpine-3.12-gcc-debug-arm64
> +
>  qemu-smoke-dom0less-arm64-gcc-boot-cpupools:
>extends: .qemu-arm64
>script:
> diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh 
> b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> index 182a4b6c18..2d69d976ea 100755
> --- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> +++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> @@ -27,6 +27,11 @@ fi
>  "
>  fi
>  
> +if [[ "${test_variant}" == "static-heap" ]]; then
> +passed="${test_variant} test passed"
> +domU_check="echo \"${passed}\""
> +fi
> +
>  if [[ "${test_variant}" == "boot-cpupools" ]]; then
>  # Check if domU0 (id=1) is assigned to Pool-1 with null scheduler
>  passed="${test_variant} test passed"
> @@ -107,7 +112,7 @@ cd ..
>  
>  # ImageBuilder
>  echo 'MEMORY_START="0x4000"
> -MEMORY_END="0xC000"
> +MEMORY_END="0x5000"
>  
>  DEVICE_TREE="virt-gicv2.dtb"
>  XEN="xen"
> @@ -128,6 +133,20 @@ if [[ "${test_variant}" == "static-mem" ]]; then
>  echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_size}\"" >> 
> binaries/config
>  fi
>  
> +if [[ "${test_variant}" == "static-heap" ]]; then
> +# ImageBuilder uses the config file to create the uboot script. 
> Devicetree
> +# will be set via the generated uboot script.
> +# The valid memory range is 0x4000 to 0x8000 as defined before.
> +# ImageBuillder sets the kernel and ramdisk range based on the file size.
> +# It will use the memory range between 0x4560 to 0x47AED1E8, and
> +# MEMORY_END has been set to 0x5000 above, so set memory range 
> between
> +# 0x5000 and 0x8000 as static heap.
> +echo  '
> +XEN_STATIC_HEAP="0x5000 0x3000"
> +# The size of static heap should be greater than the guest memory
> +DOMU_MEM[0]="128"' >> binaries/config
> +fi
> +
>  if [[ "${test_variant}" == "boot-cpupools" ]]; then
>  echo '
>  CPUPOOL[0]="cpu@1 null"
> -- 
> 2.25.1
> 



Re: [ImageBuilder][PATCH v2 2/2] uboot-script-gen: add support for static shared memory

2023-03-16 Thread Stefano Stabellini
On Thu, 16 Mar 2023, jiamei.xie wrote:
> Introduce support for creating shared-mem node for dom0less domUs in
> the device tree. Add the following option:
> - DOMU_SHARED_MEM[number]="SHM-ID HPA GPA size"
>   if specified, indicate the unique identifier of the shared memory
>   region is SHM-ID, the host physical address HPA will get mapped at
>   guest address GPA in domU and the memory of size will be reserved to
>   be shared memory.
> 
> The static shared memory is used between two dom0less domUs.
> 
> Below is an example:
> NUM_DOMUS=2
> DOMU_SHARED_MEM[0]="my-shared-mem-0 0x5000 0x600 0x1000"
> DOMU_SHARED_MEM[1]="my-shared-mem-0 0x5000 0x600 0x1000"
> 
> This static shared memory region is identified as "my-shared-mem-0",
> host physical address starting at 0x5000 of 256MB will be reserved
> to be shared between two domUs. It will get mapped at 0x600 in both
> guest physical address space. Both DomUs are the borrower domain, the
> owner domain is the default owner domain DOMID_IO.
> 
> Signed-off-by: jiamei.xie 

The patch is good just a couple of minor comments


> ---
> Changes from v1:
>  - Rather than two separate properties and just use one like follows:
>Change
>  DOMU_SHARED_MEM[0]="0x5000 0x600 0x1000"
>  DOMU_SHARED_MEM_ID[0]="my-shared-mem-0"
>to
>  DOMU_SHARED_MEM[0]="my-shared-mem-0 0x5000 0x600 0x1000"
> - Use split_value function instead of opencoding it.
> ---
>  README.md| 17 +
>  scripts/uboot-script-gen | 27 +++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/README.md b/README.md
> index 78b83f1..fe5d205 100644
> --- a/README.md
> +++ b/README.md
> @@ -196,6 +196,23 @@ Where:
>if specified, indicates the host physical address regions
>[baseaddr, baseaddr + size) to be reserved to the VM for static allocation.
>  
> +- DOMU_SHARED_MEM[number]="SHM-ID HPA GPA size"
> +  if specified, indicate SHM-ID represents the unique identifier of the 
> shared
> +  memory region, the host physical address HPA will get mapped at guest
> +  address GPA in domU and the memory of size will be reserved to be shared
> +  memory. The shared memory is used between two dom0less domUs.
> +
> +  Below is an example:
> +  NUM_DOMUS=2
> +  DOMU_SHARED_MEM[0]="my-shared-mem-0 0x5000 0x600 0x1000"
> +  DOMU_SHARED_MEM[1]="my-shared-mem-0 0x5000 0x600 0x1000"
> +
> +  This static shared memory region is identified as "my-shared-mem-0", host
> +  physical address starting at 0x5000 of 256MB will be reserved to be
> +  shared between two domUs. It will get mapped at 0x600 in both guest
> +  physical address space. Both DomUs are the borrower domain, the owner
> +  domain is the default owner domain DOMID_IO.
> +
>  - DOMU_DIRECT_MAP[number] can be set to 1 or 0.
>If set to 1, the VM is direct mapped. The default is 1.
>This is only applicable when DOMU_STATIC_MEM is specified.
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index cca3e59..46ea7e5 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -204,6 +204,28 @@ function add_device_tree_xen_static_heap()
>  dt_set "$path" "xen,static-heap" "hex" "${cells[*]}"
>  }
>  
> +function add_device_tree_static_shared_mem()
> +{
> +local path=$1
> +local domid=$2

I don't think we need a "domid" parameter, do we? Given that the node
name has the address in it, it cannot conflict anyway with other similar
nodes. So, the following should work?

dt_set "${path}/shared-mem@${shared_mem_host}" "compatible" "str" 
"xen,domain-shared-memory-v1"


> +local shared_mem=$3
> +local SHARED_MEM_ID=${shared_mem%% *}
> +local regions="${shared_mem#* }"
> +local cells=()
> +local SHARED_MEM_HOST=${regions%% *}

please use lower capital letters for local variables, so shared_mem_host
instead of SHARED_MEM_HOST


> +dt_mknode "${path}" "domU${domid}-shared-mem@${SHARED_MEM_HOST}"
> +
> +for val in ${regions[@]}
> +do
> +cells+=("$(split_value $val)")
> +done
> +
> +dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "compatible" 
> "str" "xen,domain-shared-memory-v1"
> +dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "xen,shm-id" 
> "str" "${SHARED_MEM_ID}"
> +dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" 
> "xen,shared-mem" "hex" "${cells[*]}"
> +}
> +
>  function add_device_tree_cpupools()
>  {
>  local cpu
> @@ -329,6 +351,11 @@ function xen_device_tree_editing()
>  dt_set "/chosen/domU$i" "xen,enhanced" "str" "enabled"
>  fi
>  
> +if test -n "${DOMU_SHARED_MEM[i]}"
> +then
> +add_device_tree_static_shared_mem "/chosen/domU${i}" "${i}" 
> "${DOMU_SHARED_MEM[i]}"
> +fi

The reason why I suggested to remove the "domid" parameter above is
that ${i} here is not actually the domid, it is just the n

Re: [ImageBuilder][PATCH v2 1/2] uboot-script-gen: Add XEN_STATIC_HEAP

2023-03-16 Thread Stefano Stabellini
On Thu, 16 Mar 2023, Michal Orzel wrote:
> On 16/03/2023 10:09, jiamei.xie wrote:
> > From: jiamei Xie 
> > 
> > Add a new config parameter to configure Xen static heap.
> > XEN_STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
> > if specified, indicates the host physical address regions
> > [baseaddr, baseaddr + size) to be reserved as Xen static heap.
> > 
> > For instance, XEN_STATIC_HEAP="0x5000 0x3000", if specified,
> > indicates the host memory region starting from paddr 0x5000
> > with a size of 0x3000 to be reserved as static heap.
> > 
> > Signed-off-by: jiamei Xie 
> Reviewed-by: Michal Orzel 

Acked-by: Stefano Stabellini 



Re: [RFC XEN PATCH 6/6] tools/libs/light: pci: translate irq to gsi

2023-03-16 Thread Stefano Stabellini
On Thu, 16 Mar 2023, Jan Beulich wrote:
> On 16.03.2023 10:27, Roger Pau Monné wrote:
> > On Thu, Mar 16, 2023 at 09:55:03AM +0100, Jan Beulich wrote:
> >> On 16.03.2023 01:44, Stefano Stabellini wrote:
> >>> On Wed, 15 Mar 2023, Roger Pau Monné wrote:
>  On Sun, Mar 12, 2023 at 03:54:55PM +0800, Huang Rui wrote:
> > From: Chen Jiqian 
> >
> > Use new xc_physdev_gsi_from_irq to get the GSI number
> >
> > Signed-off-by: Chen Jiqian 
> > Signed-off-by: Huang Rui 
> > ---
> >  tools/libs/light/libxl_pci.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> > index f4c4f17545..47cf2799bf 100644
> > --- a/tools/libs/light/libxl_pci.c
> > +++ b/tools/libs/light/libxl_pci.c
> > @@ -1486,6 +1486,7 @@ static void pci_add_dm_done(libxl__egc *egc,
> >  goto out_no_irq;
> >  }
> >  if ((fscanf(f, "%u", &irq) == 1) && irq) {
> > +irq = xc_physdev_gsi_from_irq(ctx->xch, irq);
> 
>  This is just a shot in the dark, because I don't really have enough
>  context to understand what's going on here, but see below.
> 
>  I've taken a look at this on my box, and it seems like on
>  dom0 the value returned by /sys/bus/pci/devices/SBDF/irq is not
>  very consistent.
> 
>  If devices are in use by a driver the irq sysfs node reports either
>  the GSI irq or the MSI IRQ (in case a single MSI interrupt is
>  setup).
> 
>  It seems like pciback in Linux does something to report the correct
>  value:
> 
>  root@lcy2-dt107:~# cat /sys/bus/pci/devices/\:00\:14.0/irq
>  74
>  root@lcy2-dt107:~# xl pci-assignable-add 00:14.0
>  root@lcy2-dt107:~# cat /sys/bus/pci/devices/\:00\:14.0/irq
>  16
> 
>  As you can see, making the device assignable changed the value
>  reported by the irq node to be the GSI instead of the MSI IRQ, I would
>  think you are missing something similar in the PVH setup (some pciback
>  magic)?
> 
>  Albeit I have no idea why you would need to translate from IRQ to GSI
>  in the way you do in this and related patches, because I'm missing the
>  context.
> >>>
> >>> As I mention in another email, also keep in mind that we need QEMU to
> >>> work and QEMU calls:
> >>> 1) xc_physdev_map_pirq (this is also called from libxl)
> >>> 2) xc_domain_bind_pt_pci_irq
> >>>
> >>>
> >>> In this case IRQ != GSI (IRQ == 112, GSI == 28). Sysfs returns the IRQ
> >>> in Linux (112), but actually xc_physdev_map_pirq expects the GSI, not
> >>> the IRQ. If you look at the implementation of xc_physdev_map_pirq,
> >>> you'll the type is "MAP_PIRQ_TYPE_GSI" and also see the check in Xen
> >>> xen/arch/x86/irq.c:allocate_and_map_gsi_pirq:
> >>>
> >>> if ( index < 0 || index >= nr_irqs_gsi )
> >>> {
> >>> dprintk(XENLOG_G_ERR, "dom%d: map invalid irq %d\n", d->domain_id,
> >>> index);
> >>> return -EINVAL;
> >>> }
> >>>
> >>> nr_irqs_gsi < 112, and the check will fail.
> >>>
> >>> So we need to pass the GSI to xc_physdev_map_pirq. To do that, we need
> >>> to discover the GSI number corresponding to the IRQ number.
> >>
> >> That's one possible approach. Another could be (making a lot of 
> >> assumptions)
> >> that a PVH Dom0 would pass in the IRQ it knows for this interrupt and Xen
> >> then translates that to GSI, knowing that PVH doesn't have (host) GSIs
> >> exposed to it.
> > 
> > I don't think Xen can translate a Linux IRQ to a GSI, as that's a
> > Linux abstraction Xen has no part in.
> 
> Well, I was talking about whatever Dom0 and Xen use to communicate. I.e.
> if at all I might have meant pIRQ, but now that you mention ...
> 
> > The GSIs exposed to a PVH dom0 are the native (host) ones, as we
> > create an emulated IO-APIC topology that mimics the physical one.
> > 
> > Question here is why Linux ends up with a IRQ != GSI, as it's my
> > understanding on Linux GSIs will always be identity mapped to IRQs, and
> > the IRQ space up to the last possible GSI is explicitly reserved for
> > this purpose.
> 
> ... this I guess pIRQ was a PV-only concept, and it really ought to be
> GSI in the PVH case. So yes, it then all boils down to that Linux-
> internal question.

Excellent question but we'll have to wait for Ray as he is the one with
access to the hardware. But I have this data I can share in the
meantime:

[1.260378] IRQ to pin mappings:
[1.260387] IRQ1 -> 0:1
[1.260395] IRQ2 -> 0:2
[1.260403] IRQ3 -> 0:3
[1.260410] IRQ4 -> 0:4
[1.260418] IRQ5 -> 0:5
[1.260425] IRQ6 -> 0:6
[1.260432] IRQ7 -> 0:7
[1.260440] IRQ8 -> 0:8
[1.260447] IRQ9 -> 0:9
[1.260455] IRQ10 -> 0:10
[1.260462] IRQ11 -> 0:11
[1.260470] IRQ12 -> 0:12
[1.260478] IRQ13 -> 0:13
[1.260485] IRQ14 -> 0:14
[1.260493] IRQ15 -> 0:15
[1.260505] IRQ106 -> 1:8
[1.260513] 

Re: [RFC PATCH 1/5] x86/xen: disable swiotlb for xen pvh

2023-03-16 Thread Stefano Stabellini
On Thu, 16 Mar 2023, Juergen Gross wrote:
> On 16.03.23 14:53, Alex Deucher wrote:
> > On Thu, Mar 16, 2023 at 9:48 AM Juergen Gross  wrote:
> > > 
> > > On 16.03.23 14:45, Alex Deucher wrote:
> > > > On Thu, Mar 16, 2023 at 3:50 AM Jan Beulich  wrote:
> > > > > 
> > > > > On 16.03.2023 00:25, Stefano Stabellini wrote:
> > > > > > On Wed, 15 Mar 2023, Jan Beulich wrote:
> > > > > > > On 15.03.2023 01:52, Stefano Stabellini wrote:
> > > > > > > > On Mon, 13 Mar 2023, Jan Beulich wrote:
> > > > > > > > > On 12.03.2023 13:01, Huang Rui wrote:
> > > > > > > > > > Xen PVH is the paravirtualized mode and takes advantage of
> > > > > > > > > > hardware
> > > > > > > > > > virtualization support when possible. It will using the
> > > > > > > > > > hardware IOMMU
> > > > > > > > > > support instead of xen-swiotlb, so disable swiotlb if
> > > > > > > > > > current domain is
> > > > > > > > > > Xen PVH.
> > > > > > > > > 
> > > > > > > > > But the kernel has no way (yet) to drive the IOMMU, so how can
> > > > > > > > > it get
> > > > > > > > > away without resorting to swiotlb in certain cases (like I/O
> > > > > > > > > to an
> > > > > > > > > address-restricted device)?
> > > > > > > > 
> > > > > > > > I think Ray meant that, thanks to the IOMMU setup by Xen, there
> > > > > > > > is no
> > > > > > > > need for swiotlb-xen in Dom0. Address translations are done by
> > > > > > > > the IOMMU
> > > > > > > > so we can use guest physical addresses instead of machine
> > > > > > > > addresses for
> > > > > > > > DMA. This is a similar case to Dom0 on ARM when the IOMMU is
> > > > > > > > available
> > > > > > > > (see include/xen/arm/swiotlb-xen.h:xen_swiotlb_detect, the
> > > > > > > > corresponding
> > > > > > > > case is XENFEAT_not_direct_mapped).
> > > > > > > 
> > > > > > > But how does Xen using an IOMMU help with, as said,
> > > > > > > address-restricted
> > > > > > > devices? They may still need e.g. a 32-bit address to be
> > > > > > > programmed in,
> > > > > > > and if the kernel has memory beyond the 4G boundary not all I/O
> > > > > > > buffers
> > > > > > > may fulfill this requirement.
> > > > > > 
> > > > > > In short, it is going to work as long as Linux has guest physical
> > > > > > addresses (not machine addresses, those could be anything) lower
> > > > > > than
> > > > > > 4GB.
> > > > > > 
> > > > > > If the address-restricted device does DMA via an IOMMU, then the
> > > > > > device
> > > > > > gets programmed by Linux using its guest physical addresses (not
> > > > > > machine
> > > > > > addresses).
> > > > > > 
> > > > > > The 32-bit restriction would be applied by Linux to its choice of
> > > > > > guest
> > > > > > physical address to use to program the device, the same way it does
> > > > > > on
> > > > > > native. The device would be fine as it always uses Linux-provided
> > > > > > <4GB
> > > > > > addresses. After the IOMMU translation (pagetable setup by Xen), we
> > > > > > could get any address, including >4GB addresses, and that is
> > > > > > expected to
> > > > > > work.
> > > > > 
> > > > > I understand that's the "normal" way of working. But whatever the
> > > > > swiotlb
> > > > > is used for in baremetal Linux, that would similarly require its use
> > > > > in
> > > > > PVH (or HVM) aiui. So unconditionally disabling it in PVH would look
> > > > > to
> > > > > me like an incomplete attempt to disable its use altogether on x86.
> > > > > What
> > > > > difference of PVH vs baremetal am I missing here?
> > > > 
> > > > swiotlb is not usable for GPUs even on bare metal.  They often have
> > > > hundreds or megs or even gigs of memory mapped on the device at any
> > > > given time.  Also, AMD GPUs support 44-48 bit DMA masks (depending on
> > > > the chip family).
> > > 
> > > But the swiotlb isn't per device, but system global.
> > 
> > Sure, but if the swiotlb is in use, then you can't really use the GPU.
> > So you get to pick one.
> 
> The swiotlb is used only for buffers which are not within the DMA mask of a
> device (see dma_direct_map_page()). So an AMD GPU supporting a 44 bit DMA mask
> won't use the swiotlb unless you have a buffer above guest physical address of
> 16TB (so basically never).
> 
> Disabling swiotlb in such a guest would OTOH mean, that a device with only
> 32 bit DMA mask passed through to this guest couldn't work with buffers
> above 4GB.
> 
> I don't think this is acceptable.

>From the Xen subsystem in Linux point of view, the only thing we need to
do is to make sure *not* to enable swiotlb_xen (yes "swiotlb_xen", not
the global swiotlb) on PVH because it is not needed anyway.

I think we should leave the global "swiotlb" setting alone. The global
swiotlb is not relevant to Xen anyway, and surely baremetal Linux has to
have a way to deal with swiotlb/GPU incompatibilities.

We just have to avoid making things worse on Xen, and for that we just
need to avoid unconditionally enabling swiotlb-xen. If the Xen subsystem
doesn't enable swiotlb_xen/swiotlb, a

Re: [PATCH v2] xen/console: Skip switching serial input to non existing domains

2023-03-16 Thread Stefano Stabellini
On Thu, 16 Mar 2023, Jan Beulich wrote:
> On 16.03.2023 11:26, Michal Orzel wrote:
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -490,7 +490,24 @@ static void switch_serial_input(void)
> >  }
> >  else
> >  {
> > -console_rx++;
> > +unsigned int next_rx = console_rx + 1;
> > +
> > +/* Skip switching serial input to non existing domains */
> > +while ( next_rx < max_init_domid + 1 )
> > +{
> > +struct domain *d = rcu_lock_domain_by_id(next_rx - 1);
> > +
> > +if ( d )
> > +{
> > +rcu_unlock_domain(d);
> > +break;
> > +}
> > +
> > +next_rx++;
> > +}
> > +
> > +console_rx = next_rx;
> > +
> >  printk("*** Serial input to DOM%d", console_rx - 1);
> >  }
> 
> While at the first glance (when you sent it in reply to v1) it looked okay,
> I'm afraid it really isn't: Please consider what happens when the last of
> the DomU-s doesn't exist anymore. (You don't really check whether it still
> exists, because the range check comes ahead of the existence one.) In that
> case you want to move from second-to-last to Xen. I expect the entire
> if/else construct wants to be inside the loop.

I don't think we need another loop, just a check if we found a domain or
not. E.g.:


unsigned int next_rx = console_rx + 1;

/* Skip switching serial input to non existing domains */
while ( next_rx < max_init_domid + 1 )
{
struct domain *d = rcu_lock_domain_by_id(next_rx - 1);

if ( d )
{
rcu_unlock_domain(d);
console_rx = next_rx;
printk("*** Serial input to DOM%d", console_rx - 1);
break;
}

next_rx++;
}

/* no domain found */
console_rx = 0;
printk("*** Serial input to Xen");



Re: [BUG] x2apic broken with current AMD hardware

2023-03-16 Thread Elliott Mitchell
On Mon, Mar 13, 2023 at 08:01:02AM +0100, Jan Beulich wrote:
> On 11.03.2023 01:09, Elliott Mitchell wrote:
> > On Thu, Mar 09, 2023 at 10:03:23AM +0100, Jan Beulich wrote:
> >>
> >> In any event you will want to collect a serial log at maximum verbosity.
> >> It would also be of interest to know whether turning off the IOMMU avoids
> >> the issue as well (on the assumption that your system has less than 255
> >> CPUs).
> > 
> > I think I might have figured out the situation in a different fashion.
> > 
> > I was taking a look at the BIOS manual for this motherboard and noticed
> > a mention of a "Local APIC Mode" setting.  Four values are listed
> > "Compatibility", "xAPIC", "x2APIC", and "Auto".
> > 
> > That is the sort of setting I likely left at "Auto" and that may well
> > result in x2 functionality being disabled.  Perhaps the x2APIC
> > functionality on AMD is detecting whether the hardware is present, and
> > failing to test whether it has been enabled?  (could be useful to output
> > a message suggesting enabling the hardware feature)
> 
> Can we please move to a little more technical terms here? What is "present"
> and "enabled" in your view? I don't suppose you mean the CPUID bit (which
> we check) and the x2APIC-mode-enable one (which we drive as needed). It's
> also left unclear what the four modes of BIOS operation evaluate to. Even
> if we knew that, overriding e.g. "Compatibility" (which likely means some
> form of "disabled" / "hidden") isn't normally an appropriate thing to do.
> In "Auto" mode Xen likely should work - the only way I could interpret the
> the other modes are "xAPIC" meaning no x2APIC ACPI tables entries (and
> presumably the CPUID bit also masked), "x2APIC" meaning x2APIC mode pre-
> enabled by firmware, and "Auto" leaving it to the OS to select. Yet that's
> speculation on my part ...

I provided the information I had discovered.  There is a setting for this
motherboard (likely present on some similar motherboards) which /may/
effect the issue.  I doubt I've tried "compatibility", but none of the
values I've tried have gotten the system to boot without "x2apic=false"
on Xen's command-line.

When setting to "x2APIC" just after "(XEN) AMD-Vi: IOMMU Extended Features:"
I see the line "(XEN) - x2APIC".  Later is the line
"(XEN) x2APIC mode is already enabled by BIOS."  I'll guess "Auto"
leaves the x2APIC turned off since neither line is present.

Both cases the line "(XEN) Switched to APIC driver x2apic_cluster" is
present (so perhaps "Auto" merely doesn't activate it).

Appears error_interrupt() needs locking or some concurrency handling
mechanism since the last error is jumbled.  With the setting "x2APIC"
I get a bunch of:
"(XEN) APIC error on CPU#: 00(08)(XEN) APIC error on CPU#: 00(08)"
(apparently one for each core)
Followed by "Receive accept error, Receive accept error," (again,
apparently one for each core).  Then a bunch of newlines (same pattern).

With the setting "auto" the last message is a series of
"(XEN) CPU#: No irq handler for vector ## (IRQ -2147483648, LAPIC)" on
2 different cores.  Rather more of the lines were from one core, the
vector value varied some.

Notable both sets of final error messages appeared after the Domain 0
kernel thought it had been operating for >30 seconds.  Lack of
response to interrupt generating events (pressing keys on USB keyboard)
suggests interrupts weren't getting through.


With "x2apic=false" error messages similar to the "Local APIC Mode"
of "x2APIC" appear >45 seconds after Domain 0 kernel start.  Of note
first "(XEN) APIC error on CPU#: 00(08)(XEN) APIC error on CPU#: 00(08)"
appears for all cores with "Receive accept error,".

Yet later a variation on this message starts appearing:
"(XEN) APIC error on CPU#: 08(08)(XEN) APIC error on CPU#: 08(08)"
this one appears multiple times.


If one was to want full logs, the lack of secure communications channel
would be an issue (since filtering out identifying data is difficult).
DSA-3072 with SHA2-256 is now less than wonderful, but DSA-1024 and
ElGamal 2048 are right out.


-- 
(\___(\___(\__  --=> 8-) EHM <=--  __/)___/)___/)
 \BS (| ehem+sig...@m5p.com  PGP 87145445 |)   /
  \_CS\   |  _  -O #include  O-   _  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445





[xen-unstable test] 179668: tolerable trouble: fail/pass/starved - PUSHED

2023-03-16 Thread osstest service owner
flight 179668 xen-unstable real [real]
flight 179694 xen-unstable real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/179668/
http://logs.test-lab.xenproject.org/osstest/logs/179694/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-examine   6 xen-install fail pass in 179694-retest
 test-amd64-i386-pair11 xen-install/dst_host fail pass in 179694-retest
 test-amd64-amd64-xl-qcow2 21 guest-start/debian.repeat fail pass in 
179694-retest

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 179633
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 179633
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 179633
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 179633
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 179633
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 179633
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 179633
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 179633
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 179633
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-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  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  14 migrate-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-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 build-armhf-libvirt   1 build-check(1)   starved  n/a
 test-armhf-armhf-examine  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   starved  n/a
 build-armhf   2 hosts-allocate   starved  n/a

version targeted for testing:
 xen  b2ea81d2b935474cf27a76b4aa143ae897e82457
baseline version:
 xen  de819c96c863467b6e625cd7197d17682f6c6122

Last test of basis   179633  2023-03-14 21:38:53 Z1 days
Failing since179649  2023-03-15 13:29:34 Z1 days2 attempts
Testing same since   179668  2023-03-16 06:30:55 Z0 days1 attempts


People who touched revisions under test:
  Andrei Cherechesu 
  Anthony PERARD 
  George Dunlap 
  Jason Andryuk 
  Roger Pau Monne 
  Roger Pau Monné 

jobs:
 build-amd64-xsm  pass
 build-a

Re: [PATCH v2 5/7] tools: Use -s for python shebangs

2023-03-16 Thread Marek Marczykowski-Górecki
On Thu, Mar 16, 2023 at 07:37:44PM +, Andrew Cooper wrote:
> This is mandated by the Fedora packaging guidelines because it is a security
> vulnerability otherwise in suid scripts.  While Xen doesn't have suid scripts,
> it's a very good idea generally, because it prevents the users local python
> environment interfering from system packaged scripts.
> 
> pygrub is the odd-script-out, being installed by distutils rather than
> manually with INSTALL_PYTHON_PROG.  distutils has no nice way of editing the
> shebang, so arrange to use INSTALL_PYTHON_PROG on pygrub too.
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Wei Liu 
> CC: Anthony PERARD 
> CC: Marek Marczykowski-Górecki 
> CC: Bernhard Kaindl 
> 
> v2:
>  * Remove accidental setuputils dependency.

... and tools/python/setup.py doesn't install any scripts, so it isn't
relevant there.

Acked-by: Marek Marczykowski-Górecki 

> ---
>  tools/Rules.mk| 2 +-
>  tools/pygrub/Makefile | 4 +++-
>  tools/pygrub/setup.py | 1 -
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 6e135387bd7e..18cf83f5be83 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -179,7 +179,7 @@ CFLAGS += $(CFLAGS-y)
>  CFLAGS += $(EXTRA_CFLAGS_XEN_TOOLS)
>  
>  INSTALL_PYTHON_PROG = \
> - $(XEN_ROOT)/tools/python/install-wrap "$(PYTHON_PATH)" $(INSTALL_PROG)
> + $(XEN_ROOT)/tools/python/install-wrap "$(PYTHON_PATH) -s" 
> $(INSTALL_PROG)
>  
>  %.opic: %.c
>   $(CC) $(CPPFLAGS) -DPIC $(CFLAGS) $(CFLAGS_$*.opic) -fPIC -c -o $@ $< 
> $(APPEND_CFLAGS)
> diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
> index 29ad0513212f..4963bc89c6ed 100644
> --- a/tools/pygrub/Makefile
> +++ b/tools/pygrub/Makefile
> @@ -18,8 +18,10 @@ build:
>  .PHONY: install
>  install: all
>   $(INSTALL_DIR) $(DESTDIR)/$(bindir)
> + $(INSTALL_DIR) $(DESTDIR)/$(LIBEXEC_BIN)
>   $(setup.py) install --record $(INSTALL_LOG) $(PYTHON_PREFIX_ARG) \
> - --root="$(DESTDIR)" --install-scripts=$(LIBEXEC_BIN) --force
> + --root="$(DESTDIR)" --force
> + $(INSTALL_PYTHON_PROG) src/pygrub $(DESTDIR)/$(LIBEXEC_BIN)/pygrub
>   set -e; if [ $(bindir) != $(LIBEXEC_BIN) -a \
>"`readlink -f $(DESTDIR)/$(bindir)`" != \
>"`readlink -f $(LIBEXEC_BIN)`" ]; then \
> diff --git a/tools/pygrub/setup.py b/tools/pygrub/setup.py
> index 0e4e3d02d372..502aa4df2dae 100644
> --- a/tools/pygrub/setup.py
> +++ b/tools/pygrub/setup.py
> @@ -23,7 +23,6 @@ setup(name='pygrub',
>author_email='ka...@redhat.com',
>license='GPL',
>package_dir={'grub': 'src', 'fsimage': 'src'},
> -  scripts = ["src/pygrub"],
>packages=pkgs,
>ext_modules = [ xenfsimage ]
>)
> -- 
> 2.30.2
> 

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


signature.asc
Description: PGP signature


[PATCH v2 5/7] tools: Use -s for python shebangs

2023-03-16 Thread Andrew Cooper
This is mandated by the Fedora packaging guidelines because it is a security
vulnerability otherwise in suid scripts.  While Xen doesn't have suid scripts,
it's a very good idea generally, because it prevents the users local python
environment interfering from system packaged scripts.

pygrub is the odd-script-out, being installed by distutils rather than
manually with INSTALL_PYTHON_PROG.  distutils has no nice way of editing the
shebang, so arrange to use INSTALL_PYTHON_PROG on pygrub too.

Signed-off-by: Andrew Cooper 
---
CC: Wei Liu 
CC: Anthony PERARD 
CC: Marek Marczykowski-Górecki 
CC: Bernhard Kaindl 

v2:
 * Remove accidental setuputils dependency.
---
 tools/Rules.mk| 2 +-
 tools/pygrub/Makefile | 4 +++-
 tools/pygrub/setup.py | 1 -
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 6e135387bd7e..18cf83f5be83 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -179,7 +179,7 @@ CFLAGS += $(CFLAGS-y)
 CFLAGS += $(EXTRA_CFLAGS_XEN_TOOLS)
 
 INSTALL_PYTHON_PROG = \
-   $(XEN_ROOT)/tools/python/install-wrap "$(PYTHON_PATH)" $(INSTALL_PROG)
+   $(XEN_ROOT)/tools/python/install-wrap "$(PYTHON_PATH) -s" 
$(INSTALL_PROG)
 
 %.opic: %.c
$(CC) $(CPPFLAGS) -DPIC $(CFLAGS) $(CFLAGS_$*.opic) -fPIC -c -o $@ $< 
$(APPEND_CFLAGS)
diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
index 29ad0513212f..4963bc89c6ed 100644
--- a/tools/pygrub/Makefile
+++ b/tools/pygrub/Makefile
@@ -18,8 +18,10 @@ build:
 .PHONY: install
 install: all
$(INSTALL_DIR) $(DESTDIR)/$(bindir)
+   $(INSTALL_DIR) $(DESTDIR)/$(LIBEXEC_BIN)
$(setup.py) install --record $(INSTALL_LOG) $(PYTHON_PREFIX_ARG) \
-   --root="$(DESTDIR)" --install-scripts=$(LIBEXEC_BIN) --force
+   --root="$(DESTDIR)" --force
+   $(INSTALL_PYTHON_PROG) src/pygrub $(DESTDIR)/$(LIBEXEC_BIN)/pygrub
set -e; if [ $(bindir) != $(LIBEXEC_BIN) -a \
 "`readlink -f $(DESTDIR)/$(bindir)`" != \
 "`readlink -f $(LIBEXEC_BIN)`" ]; then \
diff --git a/tools/pygrub/setup.py b/tools/pygrub/setup.py
index 0e4e3d02d372..502aa4df2dae 100644
--- a/tools/pygrub/setup.py
+++ b/tools/pygrub/setup.py
@@ -23,7 +23,6 @@ setup(name='pygrub',
   author_email='ka...@redhat.com',
   license='GPL',
   package_dir={'grub': 'src', 'fsimage': 'src'},
-  scripts = ["src/pygrub"],
   packages=pkgs,
   ext_modules = [ xenfsimage ]
   )
-- 
2.30.2




Re: [PATCH v5 1/4] PCI: Introduce pci_dev_for_each_resource()

2023-03-16 Thread kernel test robot
Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus powerpc/next powerpc/fixes linus/master 
v6.3-rc2 next-20230316]
[cannot apply to soc/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/PCI-Introduce-pci_dev_for_each_resource/20230315-032821
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:
https://lore.kernel.org/r/20230314192634.63531-2-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v5 1/4] PCI: Introduce pci_dev_for_each_resource()
config: powerpc-randconfig-r032-20230312 
(https://download.01.org/0day-ci/archive/20230317/202303170223.v0xqhs1v-...@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 
67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# 
https://github.com/intel-lab-lkp/linux/commit/85cdf4746b716f7b6c14d7dc5cd907c3c2a1fb0c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Andy-Shevchenko/PCI-Introduce-pci_dev_for_each_resource/20230315-032821
git checkout 85cdf4746b716f7b6c14d7dc5cd907c3c2a1fb0c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 
O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 
O=build_dir ARCH=powerpc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot 
| Link: 
https://lore.kernel.org/oe-kbuild-all/202303170223.v0xqhs1v-...@intel.com/

All errors (new ones prefixed by >>):

>> arch/powerpc/platforms/52xx/mpc52xx_pci.c:331:6: error: unused variable 'i' 
>> [-Werror,-Wunused-variable]
   int i;
   ^
   1 error generated.


vim +/i +331 arch/powerpc/platforms/52xx/mpc52xx_pci.c

f42963f8646540 Grant Likely2006-12-12  326  
f42963f8646540 Grant Likely2006-12-12  327  static void
f42963f8646540 Grant Likely2006-12-12  328  
mpc52xx_pci_fixup_resources(struct pci_dev *dev)
f42963f8646540 Grant Likely2006-12-12  329  {
85cdf4746b716f Mika Westerberg 2023-03-14  330  struct resource *res;
f42963f8646540 Grant Likely2006-12-12 @331  int i;
f42963f8646540 Grant Likely2006-12-12  332  
59510820fff76f Randy Dunlap2021-04-28  333  pr_debug("%s() 
%.4x:%.4x\n", __func__, dev->vendor, dev->device);
f42963f8646540 Grant Likely2006-12-12  334  
f42963f8646540 Grant Likely2006-12-12  335  /* We don't rely on 
boot loader for PCI and resets all
f42963f8646540 Grant Likely2006-12-12  336 devices */
85cdf4746b716f Mika Westerberg 2023-03-14  337  
pci_dev_for_each_resource_p(dev, res) {
f42963f8646540 Grant Likely2006-12-12  338  if (res->end > 
res->start) {/* Only valid resources */
f42963f8646540 Grant Likely2006-12-12  339  
res->end -= res->start;
f42963f8646540 Grant Likely2006-12-12  340  
res->start = 0;
f42963f8646540 Grant Likely2006-12-12  341  
res->flags |= IORESOURCE_UNSET;
f42963f8646540 Grant Likely2006-12-12  342  }
f42963f8646540 Grant Likely2006-12-12  343  }
f42963f8646540 Grant Likely2006-12-12  344  
f42963f8646540 Grant Likely2006-12-12  345  /* The PCI Host bridge 
of MPC52xx has a prefetch memory resource
f42963f8646540 Grant Likely2006-12-12  346 fixed to 1Gb. 
Doesn't fit in the resource system so we remove it */
f42963f8646540 Grant Likely2006-12-12  347  if ( (dev->vendor == 
PCI_VENDOR_ID_MOTOROLA) &&
f42963f8646540 Grant Likely2006-12-12  348   (   dev->device == 
PCI_DEVICE_ID_MOTOROLA_MPC5200
f42963f8646540 Grant Likely2006-12-12  349|| dev->device == 
PCI_DEVICE_ID_MOTOROLA_MPC5200B) ) {
f42963f8646540 Grant Likely2006-12-12  350  struct resource 
*res = &dev->resource[1];
f42963f8646540 Grant Likely2006-12-12  351  res->start = 
res->end = res->flags = 0;
f42963f8646540 Grant Likely2006-12-12  352  }
f42963f8646540 Grant Likely2006-12-12  353  }
f42963f8646540 Grant Likely2006-12-12  354  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests



Re: [PATCH 7/7] tools/python: Drop shebangs from library files

2023-03-16 Thread Marek Marczykowski-Górecki
On Tue, Mar 14, 2023 at 02:15:20PM +, Andrew Cooper wrote:
> These aren't runable scripts, so shouldn't have shebangs.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Marek Marczykowski-Górecki 

> ---
> CC: Wei Liu 
> CC: Anthony PERARD 
> CC: Marek Marczykowski-Górecki 
> CC: Bernhard Kaindl 
> ---
>  tools/python/xen/migration/legacy.py | 1 -
>  tools/python/xen/migration/libxc.py  | 1 -
>  tools/python/xen/migration/libxl.py  | 1 -
>  tools/python/xen/migration/public.py | 1 -
>  tools/python/xen/migration/tests.py  | 1 -
>  tools/python/xen/migration/verify.py | 1 -
>  tools/python/xen/migration/xl.py | 1 -
>  tools/python/xen/util.py | 1 -
>  8 files changed, 8 deletions(-)
> 
> diff --git a/tools/python/xen/migration/legacy.py 
> b/tools/python/xen/migration/legacy.py
> index 6456d6157ce3..e196ca876095 100644
> --- a/tools/python/xen/migration/legacy.py
> +++ b/tools/python/xen/migration/legacy.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>  
>  """
> diff --git a/tools/python/xen/migration/libxc.py 
> b/tools/python/xen/migration/libxc.py
> index 9881f5ced4ea..e52e632cb106 100644
> --- a/tools/python/xen/migration/libxc.py
> +++ b/tools/python/xen/migration/libxc.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>  
>  """
> diff --git a/tools/python/xen/migration/libxl.py 
> b/tools/python/xen/migration/libxl.py
> index 5c4d4fe0631b..5dcb50fe0207 100644
> --- a/tools/python/xen/migration/libxl.py
> +++ b/tools/python/xen/migration/libxl.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>  
>  """
> diff --git a/tools/python/xen/migration/public.py 
> b/tools/python/xen/migration/public.py
> index fab2f84587b7..23183ef67db8 100644
> --- a/tools/python/xen/migration/public.py
> +++ b/tools/python/xen/migration/public.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>  
>  """
> diff --git a/tools/python/xen/migration/tests.py 
> b/tools/python/xen/migration/tests.py
> index f22e2c2b7cf0..fcf94b0bb264 100644
> --- a/tools/python/xen/migration/tests.py
> +++ b/tools/python/xen/migration/tests.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>  
>  """
> diff --git a/tools/python/xen/migration/verify.py 
> b/tools/python/xen/migration/verify.py
> index 1e38f4a3c01e..b847c4bd220f 100644
> --- a/tools/python/xen/migration/verify.py
> +++ b/tools/python/xen/migration/verify.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>  
>  """
> diff --git a/tools/python/xen/migration/xl.py 
> b/tools/python/xen/migration/xl.py
> index 978e744dfd95..139d496654df 100644
> --- a/tools/python/xen/migration/xl.py
> +++ b/tools/python/xen/migration/xl.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>  
>  """
> diff --git a/tools/python/xen/util.py b/tools/python/xen/util.py
> index a11358eefa13..47ceb5bd21fe 100644
> --- a/tools/python/xen/util.py
> +++ b/tools/python/xen/util.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>  
>  import os
> -- 
> 2.30.2
> 

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


signature.asc
Description: PGP signature


Re: [PATCH 6/7] tools/python: Improve unit test handling

2023-03-16 Thread Marek Marczykowski-Górecki
On Tue, Mar 14, 2023 at 02:15:19PM +, Andrew Cooper wrote:
>  * Add X86_{CPUID,MSR}_POLICY_FORMAT checks which were missed previously.
>  * Drop test_suite().  It hasn't been necessary since the Py2.3 era.
>  * Drop the __main__ logic.  This can't be used without manually adjusting the
>include path, and `make test` knows how to do the right thing.
>  * For `make test`, use `-v` to see which tests have been discovered and run.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Marek Marczykowski-Górecki 

> ---
> CC: Wei Liu 
> CC: Anthony PERARD 
> CC: Marek Marczykowski-Górecki 
> CC: Bernhard Kaindl 
> ---
>  tools/python/Makefile   |  2 +-
>  tools/python/xen/migration/tests.py | 14 ++
>  2 files changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/python/Makefile b/tools/python/Makefile
> index 511e7deae409..697299bf2802 100644
> --- a/tools/python/Makefile
> +++ b/tools/python/Makefile
> @@ -36,7 +36,7 @@ uninstall:
>  
>  .PHONY: test
>  test:
> - LD_LIBRARY_PATH=$$(readlink -f ../libs/ctrl):$$(readlink -f 
> ../xenstore) $(PYTHON) -m unittest discover
> + LD_LIBRARY_PATH=$$(readlink -f ../libs/ctrl):$$(readlink -f 
> ../xenstore) $(PYTHON) -m unittest discover -v
>  
>  .PHONY: clean
>  clean:
> diff --git a/tools/python/xen/migration/tests.py 
> b/tools/python/xen/migration/tests.py
> index ff2768946bb5..f22e2c2b7cf0 100644
> --- a/tools/python/xen/migration/tests.py
> +++ b/tools/python/xen/migration/tests.py
> @@ -26,6 +26,8 @@ class TestLibxc(unittest.TestCase):
>   (libxc.X86_TSC_INFO_FORMAT, 24),
>   (libxc.HVM_PARAMS_ENTRY_FORMAT, 16),
>   (libxc.HVM_PARAMS_FORMAT, 8),
> + (libxc.X86_CPUID_POLICY_FORMAT, 24),
> + (libxc.X86_MSR_POLICY_FORMAT, 16),
>   ):
>  self.assertEqual(calcsize(fmt), sz)
>  
> @@ -40,15 +42,3 @@ class TestLibxl(unittest.TestCase):
>   (libxl.EMULATOR_HEADER_FORMAT, 8),
>   ):
>  self.assertEqual(calcsize(fmt), sz)
> -
> -
> -def test_suite():
> -suite = unittest.TestSuite()
> -
> -suite.addTest(unittest.makeSuite(TestLibxc))
> -suite.addTest(unittest.makeSuite(TestLibxl))
> -
> -return suite
> -
> -if __name__ == "__main__":
> -unittest.main()
> -- 
> 2.30.2
> 

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


signature.asc
Description: PGP signature


[ovmf test] 179690: all pass - PUSHED

2023-03-16 Thread osstest service owner
flight 179690 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179690/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 997c6967b00cdd797fe787567a28a7565aafd301
baseline version:
 ovmf 7cfe9048e3ecad7988d66ce1f0ec20d9aef509ee

Last test of basis   179677  2023-03-16 11:12:17 Z0 days
Testing same since   179690  2023-03-16 16:12:11 Z0 days1 attempts


People who touched revisions under test:
  Tuan Phan 

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
   7cfe9048e3..997c6967b0  997c6967b00cdd797fe787567a28a7565aafd301 -> 
xen-tested-master



Re: [PATCH 3/7] tools: Delete trailing whitespace in python scripts

2023-03-16 Thread Marek Marczykowski-Górecki
On Tue, Mar 14, 2023 at 02:15:16PM +, Andrew Cooper wrote:
> No functional change.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Marek Marczykowski-Górecki 

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


signature.asc
Description: PGP signature


Re: [PATCH 2/7] tools/misc: Drop xencons

2023-03-16 Thread Marek Marczykowski-Górecki
On Tue, Mar 14, 2023 at 02:15:15PM +, Andrew Cooper wrote:
> This is a python script which has it's shebang modified by be python3, but
> was never converted to be python3 compatible.
> 
> The most recent reference I can find to this script (which isn't incidental
> adjustments in the makefile) is from the Xen book, fileish 561e30b80402 which
> says
> 
>   %%   Alternatively, if the
>   %% Xen machine is connected to a serial-port server then we supply a
>   %% dumb TCP terminal client, {\tt xencons}.
> 
> So this a not-invented-here version of telnet.  Delete it.
> 
> Signed-off-by: Andrew Cooper 

Not sure if necessary, but in any case:

Acked-by: Marek Marczykowski-Górecki 

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


signature.asc
Description: PGP signature


Re: [PATCH 1/7] tools/python: Drop pylintrc

2023-03-16 Thread Marek Marczykowski-Górecki
On Tue, Mar 14, 2023 at 02:15:14PM +, Andrew Cooper wrote:
> This was added in 2004 in c/s b7d4a69f0ccb5 and has never been referenced
> since.  Given the the commit message of simply "Added .", it was quite
> possibly a mistake in the first place.
> 
> Signed-off-by: Andrew Cooper 

Ideally, we would have a pylint integrated into CI, but given frequency
of changes to that code, I don't think its worth it, so:

Acked-by: Marek Marczykowski-Górecki 

> ---
> CC: Wei Liu 
> CC: Anthony PERARD 
> CC: Marek Marczykowski-Górecki 
> CC: Bernhard Kaindl 
> ---
>  tools/python/pylintrc | 307 --
>  1 file changed, 307 deletions(-)
>  delete mode 100644 tools/python/pylintrc
> 
> diff --git a/tools/python/pylintrc b/tools/python/pylintrc
> deleted file mode 100644
> index efc4b0b3b2dd..
> --- a/tools/python/pylintrc
> +++ /dev/null
> @@ -1,307 +0,0 @@
> -# lint Python modules using external checkers.
> -#
> 
> -# This is the main checker controling the other ones and the reports 
> 
> -# generation. It is itself both a raw checker and an astng checker in 
> order  
> -# to:
> 
> -# * handle message activation / deactivation at the module level 
> 
> -# * handle some basic but necessary stats'data (number of classes, 
> methods...)
> -#
>  
> -# This checker also defines the following reports:   
>  
> -#   * R0001: Total errors / warnings 
>  
> -#   * R0002: % errors / warnings by module   
>  
> -#   * R0003: Messages
>  
> -#   * R0004: Global evaluation   
>  
> -# 
> -[MASTER]
> -# Add  to the black list. It should be a base name, not a
> -# path. You may set this option multiple times.
> -ignore=CVS
> -
> -# Pickle collected data for later comparisons.
> -persistent=yes
> -
> -# Set the cache size for astng objects.
> -cache-size=500
> -
> -
> -
> -[REPORTS]
> -# Tells wether to display a full report or only the messages
> -reports=yes
> -
> -# Use HTML as output format instead of text
> -html=no
> -
> -# Use a parseable text output format, so your favorite text editor will be 
> able
> -# to jump to the line corresponding to a message.
> -parseable=no
> -
> -# Colorizes text output using ansi escape codes
> -color=no
> -
> -# Put messages in a separate file for each module / package specified on the
> -# command line instead of printing them on stdout. Reports (if any) will be
> -# written in a file name "pylint_global.[txt|html]".
> -files-output=no
> -
> -# Python expression which should return a note less than 10 (10 is the 
> highest
> -# note).You have access to the variables errors warning, statement which
> -# respectivly contain the number of errors / warnings messages and the total
> -# number of statements analyzed. This is used by the global evaluation report
> -# (R0004).
> -evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / 
> statement) * 10)
> -
> -# Add a comment according to your evaluation note. This is used by the global
> -# evaluation report (R0004).
> -comment=no
> -
> -# Include message's id in output
> -include-ids=yes
> -
> -
> -
> -# checks for  
> -# * unused variables / imports   
> 
> -# * undefined variables  
> 
> -# * redefinition of variable from builtins or from an outer scope
> 
> -# * use of variable before assigment 
> 
> -# 
> -[VARIABLES]
> -# Enable / disable this checker
> -enable-variables=yes
> -
> -# Tells wether we should check for unused import in __init__ files.
> -init-import=no
> -
> -# List of variable names used for dummy variables (i.e. not used).
> -dummy-variables=_,_1,_2,_3,_4,_5,dummy
> -
> -
> -
> -# checks for :
> -# * doc strings  
> 
> -# * modules / classes / functions / methods / arguments / variables name 
> 
> -# * number of arguments, local variables, branchs, returns and 
> statements in
> -# functions, methods   
> -# * required module attributes   
>   
> -# * dangerous default values as arguments
> 
> -# * redefinition of function / method / class  

[PATCH 1/4] tools: convert setup.py to use setuptools

2023-03-16 Thread Marek Marczykowski-Górecki
Python distutils is deprecated and is going to be removed in Python
3.12. Migrate to setuptools.
Setuptools in Python 3.11 complains:
SetuptoolsDeprecationWarning: setup.py install is deprecated. Use build and pip 
and other standards-based tools.
Keep using setup.py anyway to build C extension.

Signed-off-by: Marek Marczykowski-Górecki 
---
 tools/pygrub/setup.py | 3 +--
 tools/python/setup.py | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/pygrub/setup.py b/tools/pygrub/setup.py
index 0e4e3d02d372..5db743180713 100644
--- a/tools/pygrub/setup.py
+++ b/tools/pygrub/setup.py
@@ -1,5 +1,4 @@
-from distutils.core import setup, Extension
-from distutils.ccompiler import new_compiler
+from setuptools import setup, Extension
 import os
 import sys
 
diff --git a/tools/python/setup.py b/tools/python/setup.py
index 721a3141d7b7..7d57ccfbfffb 100644
--- a/tools/python/setup.py
+++ b/tools/python/setup.py
@@ -1,5 +1,4 @@
-
-from distutils.core import setup, Extension
+from setuptools import setup, Extension
 import os, sys
 
 XEN_ROOT = "../.."
-- 
2.39.2




[PATCH 4/4] Update README to state Python3 requirement

2023-03-16 Thread Marek Marczykowski-Górecki
Python2 is not supported anymore.

Signed-off-by: Marek Marczykowski-Górecki 
---
 README | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/README b/README
index 755b3d8eaf8f..b2d9d79d891b 100644
--- a/README
+++ b/README
@@ -50,7 +50,7 @@ provided by your OS distributor:
 - GNU Binutils 2.24 or later
 * POSIX compatible awk
 * Development install of zlib (e.g., zlib-dev)
-* Development install of Python 2.6 or later (e.g., python-dev)
+* Development install of Python 3.2 or later (e.g., python3-dev)
 * Development install of curses (e.g., libncurses-dev)
 * Development install of openssl (e.g., openssl-dev)
 * Development install of x11 (e.g. xorg-x11-dev)
@@ -180,9 +180,9 @@ Python Runtime Libraries
 
 Various tools, such as pygrub, have the following runtime dependencies:
 
-* Python 2.6 or later.
+* Python 3.2 or later.
   URL:http://www.python.org/
-  Debian: python
+  Debian: python3
 
 Note that the build system expects `python` to be available. If your system
 only has `python2` or `python3` but not `python` (as in Linux From Scratch),
-- 
2.39.2




[PATCH 2/4] tools: don't use distutils in configure nor Makefile

2023-03-16 Thread Marek Marczykowski-Górecki
Python distutils is deprecated and is going to be removed in Python
3.12. The distutils.sysconfig is available as sysconfig module in
stdlib since Python 3.2, so use that directly.

Signed-off-by: Marek Marczykowski-Górecki 
---
 m4/python_devel.m4   | 28 ++--
 tools/libs/stat/Makefile |  4 ++--
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/m4/python_devel.m4 b/m4/python_devel.m4
index bbf1e0354b2b..bb60857b030a 100644
--- a/m4/python_devel.m4
+++ b/m4/python_devel.m4
@@ -5,21 +5,21 @@ ac_previous_libs=$LIBS
 AC_PATH_PROG([pyconfig], [$PYTHON-config], [no])
 AS_IF([test x"$pyconfig" = x"no"], [
 dnl For those that don't have python-config
-CPPFLAGS="$CFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print("-I" + distutils.sysconfig.get_config_var("INCLUDEPY"))'`"
-CPPFLAGS="$CPPFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("CFLAGS"))'`"
-LDFLAGS="$LDFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print("-L" + distutils.sysconfig.get_python_lib(plat_specific=1,\
+CPPFLAGS="$CFLAGS `$PYTHON -c 'import sysconfig; \
+print("-I" + sysconfig.get_config_var("INCLUDEPY"))'`"
+CPPFLAGS="$CPPFLAGS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("CFLAGS"))'`"
+LDFLAGS="$LDFLAGS `$PYTHON -c 'import sysconfig; \
+print("-L" + sysconfig.get_python_lib(plat_specific=1,\
 standard_lib=1) + "/config")'`"
-LDFLAGS="$LDFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("LINKFORSHARED"))'`"
-LDFLAGS="$LDFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("LDFLAGS"))'`"
-LIBS="$LIBS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("LIBS"))'`"
-LIBS="$LIBS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("SYSLIBS"))'`"
+LDFLAGS="$LDFLAGS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("LINKFORSHARED"))'`"
+LDFLAGS="$LDFLAGS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("LDFLAGS"))'`"
+LIBS="$LIBS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("LIBS"))'`"
+LIBS="$LIBS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("SYSLIBS"))'`"
 ], [
 dnl If python-config is found use it
 CPPFLAGS="$CFLAGS `$PYTHON-config --cflags`"
diff --git a/tools/libs/stat/Makefile b/tools/libs/stat/Makefile
index ee5c42bf7b4d..a968eaff48cb 100644
--- a/tools/libs/stat/Makefile
+++ b/tools/libs/stat/Makefile
@@ -73,8 +73,8 @@ $(PYLIB): $(PYSRC)
 python-bindings: $(PYLIB) $(PYMOD)
 
 pythonlibdir = $(shell $(PYTHON) -c \
-  'import distutils.sysconfig as cfg; \
-   print(cfg.get_python_lib(False, False, prefix="$(prefix)"))')
+  'import sysconfig; \
+   print(sysconfig.get_python_lib("platlib", vars={"platbase": 
"$(prefix)"}))')
 
 .PHONY: install-python-bindings
 install-python-bindings: $(PYLIB) $(PYMOD)
-- 
2.39.2




[xen-unstable-smoke test] 179687: tolerable trouble: pass/starved - PUSHED

2023-03-16 Thread osstest service owner
flight 179687 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179687/

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   1 build-check(1)   starved  n/a
 build-armhf   2 hosts-allocate   starved  n/a

version targeted for testing:
 xen  36e49fc8cbad21a4308b4701caaa685ef04e120b
baseline version:
 xen  5d8f05e10646aeef1c4a49610c0c44a7cdaf62a3

Last test of basis   179681  2023-03-16 13:00:25 Z0 days
Testing same since   179687  2023-03-16 15:00:53 Z0 days1 attempts


People who touched revisions under test:
  Henry Wang 
  Jan Beulich 
  Jason Andryuk 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  starved 
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  starved 
 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
   5d8f05e106..36e49fc8cb  36e49fc8cbad21a4308b4701caaa685ef04e120b -> smoke



Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Jan Beulich
On 14.03.2023 21:56, Volodymyr Babchuk wrote:
> +static inline void refcnt_put(refcnt_t *refcnt, void (*destructor)(refcnt_t 
> *refcnt))

Hmm, this means all callers need to pass (and agree on) the supposedly
single destructor function that needs calling. Wouldn't the destructor
function better be stored elsewhere (and supplied to e.g. refcnt_init())?

> +{
> +int ret = atomic_dec_return(&refcnt->refcnt);
> +
> +if ( ret == 0 )
> +destructor(refcnt);
> +
> +if ( unlikely(ret < 0))
> +{
> +atomic_set(&refcnt->refcnt, REFCNT_SATURATED);

It's undefined whether *refcnt still exists once the destructor was
called (which would have happened before we can make it here). While
even the atomic_dec_return() above would already have acted in an
unknown way in this case I don't think it's a good idea to access the
object yet another time. (Same for the "negative" case in
refcnt_get() then.)

Jan



Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Jan Beulich
On 16.03.2023 17:48, Roger Pau Monné wrote:
> On Thu, Mar 16, 2023 at 05:43:18PM +0100, Jan Beulich wrote:
>> On 16.03.2023 17:39, Roger Pau Monné wrote:
>>> On Thu, Mar 16, 2023 at 05:32:38PM +0100, Jan Beulich wrote:
 On 16.03.2023 17:19, Roger Pau Monné wrote:
> On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
>> +static inline void refcnt_get(refcnt_t *refcnt)
>> +{
>> +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
>
> Occurred to me while looking at the next patch:
>
> Don't you also need to print a warning (and saturate the counter
> maybe?) if old == 0, as that would imply the caller is attempting
> to take a reference of an object that should be destroyed? IOW: it
> would point to some kind of memory leak.

 Hmm, I notice the function presently returns void. I think what to do
 when the counter is zero needs leaving to the caller. See e.g.
 get_page() which will simply indicate failure to the caller in case
 the refcnt is zero. (There overflow handling also is left to the
 caller ... All that matters is whether a ref can be acquired.)
>>>
>>> Hm, likely.  I guess pages never go away even when it's refcount
>>> reaches 0.
>>>
>>> For the pdev case attempting to take a refcount on an object that has
>>> 0 refcounts implies that the caller is using leaked memory, as the
>>> point an object reaches 0 it supposed to be destroyed.
>>
>> Hmm, my thinking was that a device would remain at refcnt 0 until it is
>> actually removed, i.e. refcnt == 0 being a prereq for pci_remove_device()
>> to be willing to do anything at all. But maybe that's not a viable model.
> 
> Right, I think the intention was for pci_remove_device() to drop the
> refcount to 0 and do the removal, so the refcount should be 1 when
> calling pci_remove_device().  But none of this is written down, so
> it's mostly my assumptions from looking at the code.

Could such work at all? The function can't safely drop a reference
and _then_ check whether it was the last one. The function either has
to take refcnt == 0 as prereq, or it needs to be the destructor
function that refcnt_put() calls.

Jan



Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Roger Pau Monné
On Thu, Mar 16, 2023 at 05:43:18PM +0100, Jan Beulich wrote:
> On 16.03.2023 17:39, Roger Pau Monné wrote:
> > On Thu, Mar 16, 2023 at 05:32:38PM +0100, Jan Beulich wrote:
> >> On 16.03.2023 17:19, Roger Pau Monné wrote:
> >>> On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
>  +static inline void refcnt_get(refcnt_t *refcnt)
>  +{
>  +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
> >>>
> >>> Occurred to me while looking at the next patch:
> >>>
> >>> Don't you also need to print a warning (and saturate the counter
> >>> maybe?) if old == 0, as that would imply the caller is attempting
> >>> to take a reference of an object that should be destroyed? IOW: it
> >>> would point to some kind of memory leak.
> >>
> >> Hmm, I notice the function presently returns void. I think what to do
> >> when the counter is zero needs leaving to the caller. See e.g.
> >> get_page() which will simply indicate failure to the caller in case
> >> the refcnt is zero. (There overflow handling also is left to the
> >> caller ... All that matters is whether a ref can be acquired.)
> > 
> > Hm, likely.  I guess pages never go away even when it's refcount
> > reaches 0.
> > 
> > For the pdev case attempting to take a refcount on an object that has
> > 0 refcounts implies that the caller is using leaked memory, as the
> > point an object reaches 0 it supposed to be destroyed.
> 
> Hmm, my thinking was that a device would remain at refcnt 0 until it is
> actually removed, i.e. refcnt == 0 being a prereq for pci_remove_device()
> to be willing to do anything at all. But maybe that's not a viable model.

Right, I think the intention was for pci_remove_device() to drop the
refcount to 0 and do the removal, so the refcount should be 1 when
calling pci_remove_device().  But none of this is written down, so
it's mostly my assumptions from looking at the code.

I have some comments about the model in patch 2, I think we need to
clarify the intended usage on the commit log about pdev and refcounts.

Thanks, Roger.



[PATCH v2 2/3] xen/riscv: setup initial pagetables

2023-03-16 Thread Oleksii Kurochko
Signed-off-by: Oleksii Kurochko 
---
Changes in V2:
 * Update the commit message
---
 xen/arch/riscv/setup.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index b56c69a3dc..a3481973ff 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* Xen stack for bringing up the first CPU. */
@@ -53,6 +54,10 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
 
 test_macros_from_bug_h();
 
+setup_initial_pagetables();
+
+enable_mmu();
+
 early_printk("All set up\n");
 
 for ( ;; )
-- 
2.39.2




[PATCH v2 3/3] xen/riscv: remove dummy_bss variable

2023-03-16 Thread Oleksii Kurochko
After introduction of initial pagetables there is no any sense
in dummy_bss variable as bss section will not be empty anymore.

Signed-off-by: Oleksii Kurochko 
---
Changes in V2:
 * patch was introduced in the current one patch series (v2).
---
 xen/arch/riscv/setup.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index a3481973ff..276efb8034 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -15,14 +15,6 @@ unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
 
 struct boot_info boot_info = { (unsigned long)&_start, (unsigned long)&_end, 
0UL, 0UL };
 
-/*  
- * To be sure that .bss isn't zero. It will simplify code of
- * .bss initialization.
- * TODO:
- *   To be deleted when the first real .bss user appears
- */
-int dummy_bss __attribute__((unused));
-
 static void fill_boot_info(void)
 {
 boot_info.load_start = (unsigned long)_start;
-- 
2.39.2




[PATCH v2 1/3] xen/riscv: introduce setup_initial_pages

2023-03-16 Thread Oleksii Kurochko
Mostly the code for setup_initial_pages was taken from Bobby's
repo except for the following changes:
* Use only a minimal part of the code enough to enable MMU
* rename {_}setup_initial_pagetables functions
* add an argument for setup_initial_mapping to have
  an opportunity to make set PTE flags.
* update setup_initial_pagetables function to map sections
  with correct PTE flags.
* introduce separate enable_mmu() to be able for proper
  handling the case when load start address isn't equal to
  linker start address.
* map linker addresses range to load addresses range without
  1:1 mapping.
* add safety checks such as:
  * Xen size is less than page size
  * linker addresses range doesn't overlap load addresses
range
* Rework macros {THIRD,SECOND,FIRST,ZEROETH}_{SHIFT,MASK}
* change PTE_LEAF_DEFAULT to RX instead of RWX.
* Remove phys_offset as it isn't used now.
* Remove alignment  of {map, pa}_start &= XEN_PT_LEVEL_MAP_MASK(0); in
  setup_inital_mapping() as they should be already aligned.
* Remove clear_pagetables() as initial pagetables will be
  zeroed during bss initialization
* Remove __attribute__((section(".entry")) for setup_initial_pagetables()
  as there is no such section in xen.lds.S
* Update the argument of pte_is_valid() to "const pte_t *p"

Origin: https://gitlab.com/xen-on-risc-v/xen/-/tree/riscv-rebase 4af165b468af
Signed-off-by: Oleksii Kurochko 
---
Changes in V2:
 * update the commit message:
 * Remove {ZEROETH,FIRST,...}_{SHIFT,MASK, SIZE,...} and
   introduce instead of them XEN_PT_LEVEL_*() and LEVEL_*
 * Rework pt_linear_offset() and pt_index based on  XEN_PT_LEVEL_*()
 * Remove clear_pagetables() functions as pagetables were zeroed during
   .bss initialization
 * Rename _setup_initial_pagetables() to setup_initial_mapping()
 * Make PTE_DEFAULT equal to RX.
 * Update prototype of setup_initial_mapping(..., bool writable) -> 
   setup_initial_mapping(..., UL flags)  
 * Update calls of setup_initial_mapping according to new prototype
 * Remove unnecessary call of:
   _setup_initial_pagetables(..., load_addr_start, load_addr_end, 
load_addr_start, ...)
 * Define index* in the loop of setup_initial_mapping
 * Remove attribute "__attribute__((section(".entry")))" for 
setup_initial_pagetables()
   as we don't have such section
 * make arguments of paddr_to_pte() and pte_is_valid() as const.
 * make xen_second_pagetable static.
 * use  instead of declaring extern unsigned long _stext, 0etext, 
_srodata, _erodata
 * update  'extern unsigned long __init_begin' to 'extern unsigned long 
__init_begin[]'
 * use aligned() instead of "__attribute__((__aligned__(PAGE_SIZE)))"
 * set __section(".bss.page_aligned") for page tables arrays
 * fix identatations
 * Change '__attribute__((section(".entry")))' to '__init'
 * Remove phys_offset as it isn't used now.
 * Remove alignment  of {map, pa}_start &= XEN_PT_LEVEL_MAP_MASK(0); in
   setup_inital_mapping() as they should be already aligned.
 * Remove clear_pagetables() as initial pagetables will be
   zeroed during bss initialization
 * Remove __attribute__((section(".entry")) for setup_initial_pagetables()
   as there is no such section in xen.lds.S
 * Update the argument of pte_is_valid() to "const pte_t *p"
---
 xen/arch/riscv/Makefile   |   1 +
 xen/arch/riscv/include/asm/mm.h   |   8 ++
 xen/arch/riscv/include/asm/page.h |  67 +
 xen/arch/riscv/mm.c   | 121 ++
 xen/arch/riscv/riscv64/head.S |  65 
 xen/arch/riscv/xen.lds.S  |   2 +
 6 files changed, 264 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/mm.h
 create mode 100644 xen/arch/riscv/include/asm/page.h
 create mode 100644 xen/arch/riscv/mm.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 443f6bf15f..956ceb02df 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += entry.o
+obj-y += mm.o
 obj-$(CONFIG_RISCV_64) += riscv64/
 obj-y += sbi.o
 obj-y += setup.o
diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
new file mode 100644
index 00..3cc98fe45b
--- /dev/null
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -0,0 +1,8 @@
+#ifndef _ASM_RISCV_MM_H
+#define _ASM_RISCV_MM_H
+
+void setup_initial_pagetables(void);
+
+extern void enable_mmu(void);
+
+#endif /* _ASM_RISCV_MM_H */
diff --git a/xen/arch/riscv/include/asm/page.h 
b/xen/arch/riscv/include/asm/page.h
new file mode 100644
index 00..fb8329a191
--- /dev/null
+++ b/xen/arch/riscv/include/asm/page.h
@@ -0,0 +1,67 @@
+#ifndef _ASM_RISCV_PAGE_H
+#define _ASM_RISCV_PAGE_H
+
+#include 
+#include 
+
+#define PAGE_ENTRIES(1 << PAGETABLE_ORDER)
+#define VPN_MASK((unsigned long)(PAGE_ENTRIES - 1))
+
+#define PAGE_ORDER  (12)
+
+#ifdef CONFIG_RISCV_64
+#define PAGETABLE_ORDER (9)
+#else /* CONFIG_RISCV_32 */
+#define PAGETABLE_ORDE

[PATCH v2 0/3] enable MMU for RISC-V

2023-03-16 Thread Oleksii Kurochko
The patch series is based on top of  'RISCV basic exception handling
implementation' patch series. [1]

The patch series introduces the following things:
1. Functionality to build the page tables for Xen that map
   link-time to physical-time location.
2. Check that Xen is less then page size.
3. Check that load addresses don't overlap with linker addresses.
4. Prepare things for proper switch to virtual memory world.
5. Load the built page table into the SATP
6. Enable MMU.

[1] 
https://lore.kernel.org/xen-devel/cover.1678976127.git.oleksii.kuroc...@gmail.com/T/#t

---
Changes in V2:
  * Remove {ZEROETH,FIRST,...}_{SHIFT,MASK, SIZE,...} and
introduce instead of them XEN_PT_LEVEL_*() and LEVEL_*
  * Rework pt_linear_offset() and pt_index based on  XEN_PT_LEVEL_*()
  * Remove clear_pagetables() functions as pagetables were zeroed during
.bss initialization
  * Rename _setup_initial_pagetables() to setup_initial_mapping()
  * Make PTE_DEFAULT equal to RX.
  * Update prototype of setup_initial_mapping(..., bool writable) -> 
setup_initial_mapping(..., UL flags)  
  * Update calls of setup_initial_mapping according to new prototype
  * Remove unnecessary call of:
_setup_initial_pagetables(..., load_addr_start, load_addr_end, 
load_addr_start, ...)
  * Define index* in the loop of setup_initial_mapping
  * Remove attribute "__attribute__((section(".entry")))" for 
setup_initial_pagetables()
as we don't have such section
  * make arguments of paddr_to_pte() and pte_is_valid() as const.
  * use  instead of declaring extern unsigned long _stext, 
0etext, _srodata, _erodata
  * update  'extern unsigned long __init_begin' to 'extern unsigned long 
__init_begin[]'
  * use aligned() instead of "__attribute__((__aligned__(PAGE_SIZE)))"
  * set __section(".bss.page_aligned") for page tables arrays
  * fix identatations
  * Change '__attribute__((section(".entry")))' to '__init'
  * Remove alignment  of {map, pa}_start &= XEN_PT_LEVEL_MAP_MASK(0); in
setup_inital_mapping() as they should be already aligned.
  * Remove clear_pagetables() as initial pagetables will be
zeroed during bss initialization
  * Remove __attribute__((section(".entry")) for setup_initial_pagetables()
as there is no such section in xen.lds.S
  * Update the argument of pte_is_valid() to "const pte_t *p"
  * Remove patch "[PATCH v1 3/3] automation: update RISC-V smoke test" from the 
patch series
as it was introduced simplified approach for RISC-V smoke test by Andrew 
Cooper
  * Add patch [[xen/riscv: remove dummy_bss variable] as there is no any sense 
in
dummy_bss variable after introduction of inittial page tables.
---
Oleksii Kurochko (3):
  xen/riscv: introduce setup_initial_pages
  xen/riscv: setup initial pagetables
  xen/riscv: remove dummy_bss variable

 xen/arch/riscv/Makefile   |   1 +
 xen/arch/riscv/include/asm/mm.h   |   8 ++
 xen/arch/riscv/include/asm/page.h |  67 +
 xen/arch/riscv/mm.c   | 121 ++
 xen/arch/riscv/riscv64/head.S |  65 
 xen/arch/riscv/setup.c|  13 ++--
 xen/arch/riscv/xen.lds.S  |   2 +
 7 files changed, 269 insertions(+), 8 deletions(-)
 create mode 100644 xen/arch/riscv/include/asm/mm.h
 create mode 100644 xen/arch/riscv/include/asm/page.h
 create mode 100644 xen/arch/riscv/mm.c

-- 
2.39.2




Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Jan Beulich
On 16.03.2023 17:39, Roger Pau Monné wrote:
> On Thu, Mar 16, 2023 at 05:32:38PM +0100, Jan Beulich wrote:
>> On 16.03.2023 17:19, Roger Pau Monné wrote:
>>> On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
 +static inline void refcnt_get(refcnt_t *refcnt)
 +{
 +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
>>>
>>> Occurred to me while looking at the next patch:
>>>
>>> Don't you also need to print a warning (and saturate the counter
>>> maybe?) if old == 0, as that would imply the caller is attempting
>>> to take a reference of an object that should be destroyed? IOW: it
>>> would point to some kind of memory leak.
>>
>> Hmm, I notice the function presently returns void. I think what to do
>> when the counter is zero needs leaving to the caller. See e.g.
>> get_page() which will simply indicate failure to the caller in case
>> the refcnt is zero. (There overflow handling also is left to the
>> caller ... All that matters is whether a ref can be acquired.)
> 
> Hm, likely.  I guess pages never go away even when it's refcount
> reaches 0.
> 
> For the pdev case attempting to take a refcount on an object that has
> 0 refcounts implies that the caller is using leaked memory, as the
> point an object reaches 0 it supposed to be destroyed.

Hmm, my thinking was that a device would remain at refcnt 0 until it is
actually removed, i.e. refcnt == 0 being a prereq for pci_remove_device()
to be willing to do anything at all. But maybe that's not a viable model.

Jan




[PATCH v4] acpi/processor: fix evaluating _PDC method when running as Xen dom0

2023-03-16 Thread Roger Pau Monne
In ACPI systems, the OS can direct power management, as opposed to the
firmware.  This OS-directed Power Management is called OSPM.  Part of
telling the firmware that the OS going to direct power management is
making ACPI "_PDC" (Processor Driver Capabilities) calls.  These _PDC
methods must be evaluated for every processor object.  If these _PDC
calls are not completed for every processor it can lead to
inconsistency and later failures in things like the CPU frequency
driver.

In a Xen system, the dom0 kernel is responsible for system-wide power
management.  The dom0 kernel is in charge of OSPM.  However, the
number of CPUs available to dom0 can be different than the number of
CPUs physically present on the system.

This leads to a problem: the dom0 kernel needs to evaluate _PDC for
all the processors, but it can't always see them.

In dom0 kernels, ignore the existing ACPI method for determining if a
processor is physically present because it might not be accurate.
Instead, ask the hypervisor for this information.

Fix this by introducing a custom function to use when running as Xen
dom0 in order to check whether a processor object matches a CPU that's
online.  Such checking is done using the existing information fetched
by the Xen pCPU subsystem, extending it to also store the ACPI ID.

This ensures that _PDC method gets evaluated for all physically online
CPUs, regardless of the number of CPUs made available to dom0.

Fixes: 5d554a7bb064 ('ACPI: processor: add internal 
processor_physically_present()')
Signed-off-by: Roger Pau Monné 
---
Changes since v3:
 - Protect xen_processor_present() definition with CONFIG_ACPI.

Changes since v2:
 - Extend and use the existing pcpu functionality.

Changes since v1:
 - Reword commit message.
---
 arch/x86/include/asm/xen/hypervisor.h | 10 ++
 drivers/acpi/processor_pdc.c  | 11 +++
 drivers/xen/pcpu.c| 21 +
 3 files changed, 42 insertions(+)

diff --git a/arch/x86/include/asm/xen/hypervisor.h 
b/arch/x86/include/asm/xen/hypervisor.h
index 5fc35f889cd1..990a1609677e 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -63,4 +63,14 @@ void __init xen_pvh_init(struct boot_params *boot_params);
 void __init mem_map_via_hcall(struct boot_params *boot_params_p);
 #endif
 
+#if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI)
+bool __init xen_processor_present(uint32_t acpi_id);
+#else
+static inline bool xen_processor_present(uint32_t acpi_id)
+{
+   BUG();
+   return false;
+}
+#endif
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index 8c3f82c9fff3..18fb04523f93 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -14,6 +14,8 @@
 #include 
 #include 
 
+#include 
+
 #include "internal.h"
 
 static bool __init processor_physically_present(acpi_handle handle)
@@ -47,6 +49,15 @@ static bool __init processor_physically_present(acpi_handle 
handle)
return false;
}
 
+   if (xen_initial_domain())
+   /*
+* When running as a Xen dom0 the number of processors Linux
+* sees can be different from the real number of processors on
+* the system, and we still need to execute _PDC for all of
+* them.
+*/
+   return xen_processor_present(acpi_id);
+
type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
cpuid = acpi_get_cpuid(handle, type, acpi_id);
 
diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c
index fd3a644b0855..034d05e56507 100644
--- a/drivers/xen/pcpu.c
+++ b/drivers/xen/pcpu.c
@@ -58,6 +58,7 @@ struct pcpu {
struct list_head list;
struct device dev;
uint32_t cpu_id;
+   uint32_t acpi_id;
uint32_t flags;
 };
 
@@ -249,6 +250,7 @@ static struct pcpu *create_and_register_pcpu(struct 
xenpf_pcpuinfo *info)
 
INIT_LIST_HEAD(&pcpu->list);
pcpu->cpu_id = info->xen_cpuid;
+   pcpu->acpi_id = info->acpi_id;
pcpu->flags = info->flags;
 
/* Need hold on xen_pcpu_lock before pcpu list manipulations */
@@ -381,3 +383,22 @@ static int __init xen_pcpu_init(void)
return ret;
 }
 arch_initcall(xen_pcpu_init);
+
+#ifdef CONFIG_ACPI
+bool __init xen_processor_present(uint32_t acpi_id)
+{
+   struct pcpu *pcpu;
+   bool online = false;
+
+   mutex_lock(&xen_pcpu_lock);
+   list_for_each_entry(pcpu, &xen_pcpus, list)
+   if (pcpu->acpi_id == acpi_id) {
+   online = pcpu->flags & XEN_PCPU_FLAGS_ONLINE;
+   break;
+   }
+
+   mutex_unlock(&xen_pcpu_lock);
+
+   return online;
+}
+#endif
-- 
2.39.0




Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Roger Pau Monné
On Thu, Mar 16, 2023 at 05:32:38PM +0100, Jan Beulich wrote:
> On 16.03.2023 17:19, Roger Pau Monné wrote:
> > On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
> >> +static inline void refcnt_get(refcnt_t *refcnt)
> >> +{
> >> +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
> > 
> > Occurred to me while looking at the next patch:
> > 
> > Don't you also need to print a warning (and saturate the counter
> > maybe?) if old == 0, as that would imply the caller is attempting
> > to take a reference of an object that should be destroyed? IOW: it
> > would point to some kind of memory leak.
> 
> Hmm, I notice the function presently returns void. I think what to do
> when the counter is zero needs leaving to the caller. See e.g.
> get_page() which will simply indicate failure to the caller in case
> the refcnt is zero. (There overflow handling also is left to the
> caller ... All that matters is whether a ref can be acquired.)

Hm, likely.  I guess pages never go away even when it's refcount
reaches 0.

For the pdev case attempting to take a refcount on an object that has
0 refcounts implies that the caller is using leaked memory, as the
point an object reaches 0 it supposed to be destroyed.

Returning success would be fine, as then for the pdev use-case we
could print a warning and likely take some action to prevent further
damage if possible.

Thanks, Roger.



Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Jan Beulich
On 16.03.2023 17:19, Roger Pau Monné wrote:
> On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
>> +static inline void refcnt_get(refcnt_t *refcnt)
>> +{
>> +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
> 
> Occurred to me while looking at the next patch:
> 
> Don't you also need to print a warning (and saturate the counter
> maybe?) if old == 0, as that would imply the caller is attempting
> to take a reference of an object that should be destroyed? IOW: it
> would point to some kind of memory leak.

Hmm, I notice the function presently returns void. I think what to do
when the counter is zero needs leaving to the caller. See e.g.
get_page() which will simply indicate failure to the caller in case
the refcnt is zero. (There overflow handling also is left to the
caller ... All that matters is whether a ref can be acquired.)

Jan



Re: [PATCH v3 3/6] vpci: crash domain if we wasn't able to (un) map vPCI regions

2023-03-16 Thread Roger Pau Monné
On Tue, Mar 14, 2023 at 08:56:30PM +, Volodymyr Babchuk wrote:
> In that unlikely case, when map_range() fails to do it's job,
> domain memory mapping will be left in inconsistent state. As there is
> no easy way to remove stale p2m mapping we need to crash domain, as
> FIXME suggests.
> 
> Signed-off-by: Volodymyr Babchuk 
> 
> ---
> 
> v3:
>  - new patch
> ---
>  xen/drivers/vpci/header.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index ec2e978a4e..8319fe4c1d 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -162,14 +162,11 @@ bool vpci_process_pending(struct vcpu *v)
>  rangeset_destroy(v->vpci.mem);
>  v->vpci.mem = NULL;
>  if ( rc )
> -/*
> - * FIXME: in case of failure remove the device from the domain.
> - * Note that there might still be leftover mappings. While this 
> is
> - * safe for Dom0, for DomUs the domain will likely need to be
> - * killed in order to avoid leaking stale p2m mappings on
> - * failure.
> - */
> +{
>  vpci_remove_device(v->vpci.pdev);
> +if ( !is_hardware_domain(v->domain) )
> +domain_crash(v->domain);

No need to remove the device if you are crashing the domain, so the
vpci_remove_device() call can be placed in the else branch of the
conditional.

Thanks, Roger.



Re: [RFC PATCH 1/5] x86/xen: disable swiotlb for xen pvh

2023-03-16 Thread Roger Pau Monné
On Sun, Mar 12, 2023 at 08:01:53PM +0800, Huang Rui wrote:
> Xen PVH is the paravirtualized mode and takes advantage of hardware
> virtualization support when possible. It will using the hardware IOMMU
> support instead of xen-swiotlb, so disable swiotlb if current domain is
> Xen PVH.
> 
> Signed-off-by: Huang Rui 
> ---
>  arch/x86/kernel/pci-dma.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index 30bbe4abb5d6..f5c73dd18f2a 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -74,6 +74,12 @@ static inline void __init pci_swiotlb_detect(void)
>  #ifdef CONFIG_SWIOTLB_XEN
>  static void __init pci_xen_swiotlb_init(void)
>  {
> + /* Xen PVH domain won't use swiotlb */
> + if (xen_pvh_domain()) {
> + x86_swiotlb_enable = false;
> + return;
> + }

I'm very confused by this: pci_xen_swiotlb_init() is only called for
PV domains, see the only caller in pci_iommu_alloc().  So this is just
dead code.

> +
>   if (!xen_initial_domain() && !x86_swiotlb_enable)
>   return;
>   x86_swiotlb_enable = true;
> @@ -86,7 +92,7 @@ static void __init pci_xen_swiotlb_init(void)
>  
>  int pci_xen_swiotlb_init_late(void)
>  {
> - if (dma_ops == &xen_swiotlb_dma_ops)
> + if (xen_pvh_domain() || dma_ops == &xen_swiotlb_dma_ops)

Same here, this function is only called by
pcifront_connect_and_init_dma() and pcifront should never attach on a
PVH domain, hence it's also dead code.

Thanks, Roger.



Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Roger Pau Monné
On Thu, Mar 16, 2023 at 03:03:42PM +0100, Jan Beulich wrote:
> On 16.03.2023 14:54, Roger Pau Monné wrote:
> > On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
> >> +{
> >> +return atomic_read(&refcnt->refcnt);
> >> +}
> >> +
> >> +static inline void refcnt_get(refcnt_t *refcnt)
> >> +{
> >> +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
> >> +
> >> +if ( unlikely(old < 0) || unlikely (old + 1 < 0) )
> >  ^ extra space
> > 
> > You want a single unlikely for both conditions.
> 
> Are you sure? My experience was generally the other way around: likely()
> and unlikely() become ineffectual as soon as the compiler needs more
> than one branch for the inner construct (ie generally for and && or ||).

Oh, OK, never mind then. We have examples of both in the code base.

Roger.



Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Roger Pau Monné
On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
> We can use reference counter to ease up object lifetime management.
> This patch adds very basic support for reference counters. refcnt
> should be used in the following way:
> 
> 1. Protected structure should have refcnt_t field
> 
> 2. This field should be initialized with refcnt_init() during object
> construction.
> 
> 3. If code holds a valid pointer to a structure/object it can increase
> refcount with refcnt_get(). No additional locking is required.
> 
> 4. Code should call refcnt_put() before dropping pointer to a
> protected structure. `destructor` is a call back function that should
> destruct object and free all resources, including structure protected
> itself. Destructor will be called if reference counter reaches zero.
> 
> 5. If code does not hold a valid pointer to a protected structure it
> should use other locking mechanism to obtain a pointer. For example,
> it should lock a list that hold protected objects.
> 
> Signed-off-by: Volodymyr Babchuk 
> 
> ---
> v3:
>   - moved in from another patch series
>   - used structure to encapsulate refcnt_t
>   - removed #ifndef NDEBUG in favor of just calling ASSERT
>   - added assertion to refcnt_put
>   - added saturation support: code catches overflow and underflow
>   - added EMACS magic at end of the file
>   - fixed formatting
> ---
>  xen/include/xen/refcnt.h | 59 
>  1 file changed, 59 insertions(+)
>  create mode 100644 xen/include/xen/refcnt.h
> 
> diff --git a/xen/include/xen/refcnt.h b/xen/include/xen/refcnt.h
> new file mode 100644
> index 00..1ac05d927c
> --- /dev/null
> +++ b/xen/include/xen/refcnt.h
> @@ -0,0 +1,59 @@
> +#ifndef __XEN_REFCNT_H__
> +#define __XEN_REFCNT_H__
> +
> +#include 
> +#include 
> +
> +#define REFCNT_SATURATED (INT_MIN / 2)
> +
> +typedef struct {
> +atomic_t refcnt;
> +} refcnt_t;
> +
> +static inline void refcnt_init(refcnt_t *refcnt)
> +{
> +atomic_set(&refcnt->refcnt, 1);
> +}
> +
> +static inline int refcnt_read(refcnt_t *refcnt)
> +{
> +return atomic_read(&refcnt->refcnt);
> +}
> +
> +static inline void refcnt_get(refcnt_t *refcnt)
> +{
> +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);

Occurred to me while looking at the next patch:

Don't you also need to print a warning (and saturate the counter
maybe?) if old == 0, as that would imply the caller is attempting
to take a reference of an object that should be destroyed? IOW: it
would point to some kind of memory leak.

Thanks, Roger.



Re: [PATCH v3 2/6] xen: pci: introduce reference counting for pdev

2023-03-16 Thread Roger Pau Monné
On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
> Prior to this change, lifetime of pci_dev objects was protected by global
> pcidevs_lock(). Long-term plan is to remove this log, so we need some
   ^ lock

I wouldn't say remove, as one way or another we need a lock to protect
concurrent accesses.

> other mechanism to ensure that those objects will not disappear under
> feet of code that access them. Reference counting is a good choice as
> it provides easy to comprehend way to control object lifetime.
> 
> This patch adds two new helper functions: pcidev_get() and
> pcidev_put(). pcidev_get() will increase reference counter, while
> pcidev_put() will decrease it, destroying object when counter reaches
> zero.
> 
> pcidev_get() should be used only when you already have a valid pointer
> to the object or you are holding lock that protects one of the
> lists (domain, pseg or ats) that store pci_dev structs.
> 
> pcidev_get() is rarely used directly, because there already are
> functions that will provide valid pointer to pci_dev struct:
> pci_get_pdev(), pci_get_real_pdev(). They will lock appropriate list,
> find needed object and increase its reference counter before returning
> to the caller.
> 
> Naturally, pci_put() should be called after finishing working with a
> received object. This is the reason why this patch have so many
> pcidev_put()s and so little pcidev_get()s: existing calls to
> pci_get_*() functions now will increase reference counter
> automatically, we just need to decrease it back when we finished.

After looking a bit into this, I would like to ask whether it's been
considered the need to increase the refcount for each use of a pdev.

For example I would consider the initial alloc_pdev() to take a
refcount, and then pci_remove_device() _must_ be the function that
removes the last refcount, so that it can return -EBUSY otherwise (see
my comment below).

I would also think that having the device assigned to a guest will take
another refcount, and then any usage from further callers (ie: like
vpci) will need some kind of protection from preventing the device
from being deassigned from a domain while vPCI handlers are running,
and the current refcount won't help with that.

That makes me wonder if for example callers of pci_get_pdev(d, sbdf)
do need to take an extra refcount, because such access is already
protected from the pdev going away by the fact that the device is
assigned to a guest.  But maybe it's too much work to separate users
of pci_get_pdev(d, ...); vs pci_get_pdev(NULL, ...);.

There's also a window when the refcount is dropped to 0, and the
destruction function is called, but at the same time a concurrent
thread could attempt to take a reference to the pdev still?

> 
> This patch removes "const" qualifier from some pdev pointers because
> pcidev_put() technically alters the contents of pci_dev structure.
> 
> Signed-off-by: Volodymyr Babchuk 
> Suggested-by: Jan Beulich 
> 
> ---
> 
> v3:
>  - Moved in from another patch series
>  - Fixed code formatting (tabs -> spaces)
>  - Removed erroneous pcidev_put in vga.c
>  - Added missing pcidev_put in couple of places
>  - removed mention of pci_get_pdev_by_domain()
> ---
>  xen/arch/x86/hvm/vmsi.c  |   2 +-
>  xen/arch/x86/irq.c   |   4 +
>  xen/arch/x86/msi.c   |  44 +++-
>  xen/arch/x86/pci.c   |   3 +
>  xen/arch/x86/physdev.c   |  17 ++-
>  xen/common/sysctl.c  |   7 +-
>  xen/drivers/passthrough/amd/iommu_init.c |  12 +-
>  xen/drivers/passthrough/amd/iommu_map.c  |   6 +-
>  xen/drivers/passthrough/pci.c| 138 +++
>  xen/drivers/passthrough/vtd/quirks.c |   2 +
>  xen/drivers/video/vga.c  |   7 +-
>  xen/drivers/vpci/vpci.c  |  16 ++-
>  xen/include/xen/pci.h|  18 +++
>  13 files changed, 215 insertions(+), 61 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
> index 3cd4923060..8c3d673872 100644
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -914,7 +914,7 @@ int vpci_msix_arch_print(const struct vpci_msix *msix)
>  
>  spin_unlock(&msix->pdev->vpci->lock);
>  process_pending_softirqs();
> -/* NB: we assume that pdev cannot go away for an alive domain. */
> +
>  if ( !pdev->vpci || !spin_trylock(&pdev->vpci->lock) )
>  return -EBUSY;
>  if ( pdev->vpci->msix != msix )
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index 20150b1c7f..87464d82c8 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -2175,6 +2175,7 @@ int map_domain_pirq(
>  msi->entry_nr = ret;
>  ret = -ENFILE;
>  }
> +pcidev_put(pdev);
>  goto done;
>  }
>  
> @@ -218

Re: [PATCH v2] xen/console: Skip switching serial input to non existing domains

2023-03-16 Thread Jan Beulich
On 16.03.2023 15:15, Michal Orzel wrote:
> 
> 
> On 16/03/2023 12:11, Jan Beulich wrote:
>> Caution: This message originated from an External Source. Use proper caution 
>> when opening attachments, clicking links, or responding.
>>
>>
>> On 16.03.2023 11:26, Michal Orzel wrote:
>>> --- a/xen/drivers/char/console.c
>>> +++ b/xen/drivers/char/console.c
>>> @@ -490,7 +490,24 @@ static void switch_serial_input(void)
>>>  }
>>>  else
>>>  {
>>> -console_rx++;
>>> +unsigned int next_rx = console_rx + 1;
>>> +
>>> +/* Skip switching serial input to non existing domains */
>>> +while ( next_rx < max_init_domid + 1 )
>>> +{
>>> +struct domain *d = rcu_lock_domain_by_id(next_rx - 1);
>>> +
>>> +if ( d )
>>> +{
>>> +rcu_unlock_domain(d);
>>> +break;
>>> +}
>>> +
>>> +next_rx++;
>>> +}
>>> +
>>> +console_rx = next_rx;
>>> +
>>>  printk("*** Serial input to DOM%d", console_rx - 1);
>>>  }
>>
>> While at the first glance (when you sent it in reply to v1) it looked okay,
>> I'm afraid it really isn't: Please consider what happens when the last of
>> the DomU-s doesn't exist anymore. (You don't really check whether it still
>> exists, because the range check comes ahead of the existence one.) In that
>> case you want to move from second-to-last to Xen. I expect the entire
>> if/else construct wants to be inside the loop.
> I did this deliberately because I do not think the situation you describe is 
> possible
> (i.e. no domains at all - Xen still usable). With hardware domain in place, 
> we can e.g. destroy the domain
> which would invoke domain_kill() -> domain_destroy() that would free domain 
> struct.
> Without hwdom, the domain cannot kill/destroy itself. It can do the shutdown 
> but it will not
> destroy it (at least this is what I tested). So I do not think there can be a 
> scenario where
> there is not a single domain while Xen running and be usable.

I didn't talk about "no domain left at all", but about the case where the
domain with the highest domain ID is gone.

Jan



Re: [PATCH v3 3/3] xen/riscv: initialize .bss section

2023-03-16 Thread Bobby Eshleman
On Fri, Mar 03, 2023 at 12:24:24PM +0200, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko 
> ---
>  xen/arch/riscv/riscv64/head.S | 9 +
>  xen/arch/riscv/setup.c| 8 
>  2 files changed, 17 insertions(+)
> 
> diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
> index adf5d6c74a..8887f0cbd4 100644
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -1,3 +1,4 @@
> +#include 
>  #include 
>  
>  .section .text.header, "ax", %progbits
> @@ -18,6 +19,14 @@ ENTRY(start)
>  li  t0, SSTATUS_FS
>  csrcCSR_SSTATUS, t0
>  
> +/* Clear the BSS */
> +la  t3, __bss_start
> +la  t4, __bss_end
> +.L_clear_bss:
> +REG_S   zero, (t3)
> +add t3, t3, __SIZEOF_POINTER__
> +bltut3, t4, .L_clear_bss
> +
>  la  sp, cpu0_boot_stack
>  li  t0, STACK_SIZE
>  add sp, sp, t0
> diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> index d9723fe1c0..929565720b 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -7,6 +7,14 @@
>  unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
>  __aligned(STACK_SIZE);
>  
> +/*  
> + * To be sure that .bss isn't zero. It will simplify code of
> + * .bss initialization.
> + * TODO:
> + *   To be deleted when the first real .bss user appears
> + */
> +int dummy_bss __attribute__((unused));
> +
>  void __init noreturn start_xen(unsigned long bootcpu_id,
> unsigned long dtb_base)
>  {
> -- 
> 2.39.0
> 
> 

Reviewed-by: Bobby Eshleman 



Re: Aw: Re: Re: [help] Xen 4.14.5 on Devuan 4.0 Chimaera, regression from Xen 4.0.1

2023-03-16 Thread Jan Beulich
On 14.03.2023 16:11, Andrew Cooper wrote:
> On 14/03/2023 2:53 pm, Denis wrote:
>> On 14.03.2023 07:37; Jan Beulich wrote:
>>> On 14.03.2023 02:15, Denis wrote:
 On 13.03.2023 10:36, Jan wrote
> On 10.03.2023 21:50, Denis wrote:
>> Should I test something else?
> ... there was no request for any further testing here, for the moment.
 ah...sorry, going by "Would be nice to have this confirmed forthe system
 in question, i.e. without Xen underneath Linux" I thought I could test
 something which might help shed some light on all of this.
>>> Well, yes, that Linux-without-Xen test would still be useful to have
>>> results from. I didn't account for this in my earlier reply because
>>> I had asked for it before already, and I did take "something else"
>>> for meaning anything that might have turned up as useful from the new
>>> data you had provided.
>> What tests could I do or what info should I provide to help?
> 
> Can you please rebuild Xen with this patch:
> 
> diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c
> b/xen/drivers/passthrough/amd/iommu_acpi.c
> index 2fdebd2d74c9..747eae25f56c 100644
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -1033,7 +1033,7 @@ static int __init parse_ivrs_table(struct
> acpi_table_header *table)
>  const struct acpi_ivrs_header *ivrs_block;
>  unsigned long length;
>  unsigned int apic;
> -    bool_t sb_ioapic = !iommu_intremap;
> +    bool_t sb_ioapic = 1;
>  int error = 0;
>  
>  BUG_ON(!table);
> 
> which should cause the behaviour to revert back to that of Xen 4.0.1 
> (i.e. it will fully ignore the checks relating to the southbridge ioapic).

Alternatively you may want to try the change below (I think I have now
convinced myself that the state change is still possible at this point
in time), with the intended effect of ...

> Confirm that with this, and booting Xen simply with `iommu=1` that full
> DMA remapping and interrupt remapping is considered active.

... DMA remapping active, but interrupt mapping off (i.e. matching
Linux behavior), without any overriding command line options.

Jan

AMD/IOMMU: allow DMA remapping to remain enabled when there's no southbridge 
IO-APIC

The original Linux commit that our respective code was derived from
isn't as heavyhanded as our cloned code: It only disables interrupt
remapping in such a case. Follow that model, noting that it is still
early enough to turn interrupt remapping off on its own.

Fixes: 06bbcaf48d09 ("AMD IOMMU: fail if there is no southbridge IO-APIC")
Reported-by: Denis 
Signed-off-by: Jan Beulich 
---
Note that the alternative of disabling per-device interrupt remapping
is undesirable as per XSA-36, yet then again it may still be better than
turning off interrupt remapping altogether. Thoughts?

--- unstable.orig/xen/drivers/passthrough/amd/iommu_acpi.c
+++ unstable/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -1183,7 +1183,7 @@ static int __init cf_check parse_ivrs_ta
 if ( !error && !sb_ioapic )
 {
 if ( amd_iommu_perdev_intremap )
-error = -ENXIO;
+iommu_intremap = iommu_intremap_off;
 printk("%sNo southbridge IO-APIC found in IVRS table\n",
amd_iommu_perdev_intremap ? XENLOG_ERR : XENLOG_WARNING);
 }






Re: [PATCH v3 2/3] xen/riscv: read/save hart_id and dtb_base passed by bootloader

2023-03-16 Thread Bobby Eshleman
On Fri, Mar 03, 2023 at 12:24:23PM +0200, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko 
> ---
> Changes since v2:
>  * Add the comment for start() function with the explanation what and
>how OpenSBI pass to start() function.
>  * Clean up start() code related to read&save hart_id & dtb_base.  
> ---
> Changes since v1:
>  * read/save/pass of hart_id and  dtb_base passed by a bootloader
>were moved to head.S. 
>  * Update start_xen() to recieve hard_id & dtb_base
> ---
>  xen/arch/riscv/riscv64/head.S | 5 +
>  xen/arch/riscv/setup.c| 3 ++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
> index 52fa41c778..adf5d6c74a 100644
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -2,6 +2,11 @@
>  
>  .section .text.header, "ax", %progbits
>  
> +/*
> + * OpenSBI pass to start():
> + *   a0 -> hart_id ( bootcpu_id )
> + *   a1 -> dtb_base 
> + */
>  ENTRY(start)
>  /* Mask all interrupts */
>  csrwCSR_SIE, zero
> diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> index 1c87899e8e..d9723fe1c0 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -7,7 +7,8 @@
>  unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
>  __aligned(STACK_SIZE);
>  
> -void __init noreturn start_xen(void)
> +void __init noreturn start_xen(unsigned long bootcpu_id,
> +   unsigned long dtb_base)
>  {
>  early_printk("Hello from C env\n");
>  
> -- 
> 2.39.0
> 
> 

Reviewed-by: Bobby Eshleman 



Re: [PATCH v3 1/3] xen/riscv: disable fpu

2023-03-16 Thread Bobby Eshleman
On Fri, Mar 03, 2023 at 12:24:22PM +0200, Oleksii Kurochko wrote:
> Disable FPU to detect illegal usage of floating point in kernel
> space.
> 
> Signed-off-by: Oleksii Kurochko 
> ---
>  xen/arch/riscv/riscv64/head.S | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
> index ffd95f9f89..52fa41c778 100644
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -6,6 +6,13 @@ ENTRY(start)
>  /* Mask all interrupts */
>  csrwCSR_SIE, zero
>  
> +/*
> + * Disable FPU to detect illegal usage of
> + * floating point in kernel space
> + */
> +li  t0, SSTATUS_FS
> +csrcCSR_SSTATUS, t0
> +
>  la  sp, cpu0_boot_stack
>  li  t0, STACK_SIZE
>  add sp, sp, t0
> -- 
> 2.39.0
> 
> 

My last email had the wrong trailer:

Reviewed-by: Bobby Eshleman 



Re: [PATCH v3 1/3] xen/riscv: disable fpu

2023-03-16 Thread Bobby Eshleman
On Fri, Mar 03, 2023 at 12:24:22PM +0200, Oleksii Kurochko wrote:
> Disable FPU to detect illegal usage of floating point in kernel
> space.
> 
> Signed-off-by: Oleksii Kurochko 
> ---
>  xen/arch/riscv/riscv64/head.S | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
> index ffd95f9f89..52fa41c778 100644
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -6,6 +6,13 @@ ENTRY(start)
>  /* Mask all interrupts */
>  csrwCSR_SIE, zero
>  
> +/*
> + * Disable FPU to detect illegal usage of
> + * floating point in kernel space
> + */
> +li  t0, SSTATUS_FS
> +csrcCSR_SSTATUS, t0
> +
>  la  sp, cpu0_boot_stack
>  li  t0, STACK_SIZE
>  add sp, sp, t0
> -- 
> 2.39.0
> 
> 

Acked-by: Bobby Eshleman 



[ovmf test] 179677: all pass - PUSHED

2023-03-16 Thread osstest service owner
flight 179677 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179677/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 7cfe9048e3ecad7988d66ce1f0ec20d9aef509ee
baseline version:
 ovmf 961792c9d61f7e03e0c1b6b9f93b8b306df94bf9

Last test of basis   179625  2023-03-14 10:15:04 Z2 days
Testing same since   179677  2023-03-16 11:12:17 Z0 days1 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Sunil V L 

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
   961792c9d6..7cfe9048e3  7cfe9048e3ecad7988d66ce1f0ec20d9aef509ee -> 
xen-tested-master



[xen-unstable-smoke test] 179681: tolerable trouble: pass/starved - PUSHED

2023-03-16 Thread osstest service owner
flight 179681 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179681/

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   1 build-check(1)   starved  n/a
 build-armhf   2 hosts-allocate   starved  n/a

version targeted for testing:
 xen  5d8f05e10646aeef1c4a49610c0c44a7cdaf62a3
baseline version:
 xen  b2ea81d2b935474cf27a76b4aa143ae897e82457

Last test of basis   179660  2023-03-16 00:02:00 Z0 days
Testing same since   179681  2023-03-16 13:00:25 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  starved 
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  starved 
 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
   b2ea81d2b9..5d8f05e106  5d8f05e10646aeef1c4a49610c0c44a7cdaf62a3 -> smoke



[PATCH v5 7/7] xen/riscv: test basic handling stuff

2023-03-16 Thread Oleksii Kurochko
Signed-off-by: Oleksii Kurochko 
Acked-by: Alistair Francis 
---
Changes in V5:
  - Nothing changed
---
Changes in V4:
  - Add Acked-by: Alistair Francis 
---
Changes in V3:
  - Nothing changed
---
Changes in V2:
  - Nothing changed
---
 xen/arch/riscv/setup.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index b44d105b5f..b56c69a3dc 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,3 +1,4 @@
+#include 
 #include 
 #include 
 #include 
@@ -27,6 +28,20 @@ static void fill_boot_info(void)
 boot_info.load_end   = (unsigned long)_end;
 }
 
+static void test_run_in_exception(struct cpu_user_regs *regs)
+{
+early_printk("If you see this message, ");
+early_printk("run_in_exception_handler is most likely working\n");
+}
+
+static void test_macros_from_bug_h(void)
+{
+run_in_exception_handler(test_run_in_exception);
+WARN();
+early_printk("If you see this message, ");
+early_printk("WARN is most likely working\n");
+}
+
 void __init noreturn start_xen(unsigned long bootcpu_id,
unsigned long dtb_paddr)
 {
@@ -36,6 +51,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
 
 trap_init();
 
+test_macros_from_bug_h();
+
 early_printk("All set up\n");
 
 for ( ;; )
-- 
2.39.2




[PATCH v5 6/7] xen/riscv: introduce an implementation of macros from

2023-03-16 Thread Oleksii Kurochko
The patch introduces macros: BUG(), WARN(), run_in_exception(),
assert_failed.
To be precise, the macros from generic bug implementation ()
will be used.

The implementation uses "ebreak" instruction in combination with
diffrent bug frame tables (for each type) which contains useful
information.

Signed-off-by: Oleksii Kurochko 
---
Changes in V5:
  - Remove "#include " from  as there is no any need in 
it anymore
  - Update macros GET_INSN_LENGTH: remove UL and 'unsigned int len;' from it
  - Remove " include " from risc/setup.c. it is not needed in the 
current version of
the patch
  - change an argument type from vaddr_t to uint32_t for is_valid_bugaddr and 
introduce read_instr() to
read instruction properly as the length of qinstruction can be either 32 or 
16 bits.
  - Code style fixes
  - update the comments before do_bug_frame() in riscv/trap.c
  - Refactor is_valid_bugaddr() function.
  - introduce macros cast_to_bug_frame(addr) to hide casts.
  - use LINK_TO_LOAD() for addresses which are linker time relative.
---
Changes in V4:
  - Updates in RISC-V's :
* Add explanatory comment about why there is only defined for 32-bits length
  instructions and 16/32-bits BUG_INSN_{16,32}.
* Change 'unsigned long' to 'unsigned int' inside GET_INSN_LENGTH().
* Update declaration of is_valid_bugaddr(): switch return type from int to 
bool
  and the argument from 'unsigned int' to 'vaddr'.
  - Updates in RISC-V's traps.c:
* replace /xen and /asm includes 
* update definition of is_valid_bugaddr():switch return type from int to 
bool
  and the argument from 'unsigned int' to 'vaddr'. Code style inside 
function
  was updated too.
* do_bug_frame() refactoring:
  * local variables start and bug became 'const struct bug_frame'
  * bug_frames[] array became 'static const struct bug_frame[] = ...'
  * remove all casts
  * remove unneeded comments and add an explanatory comment that the 
do_bug_frame()
will be switched to a generic one.
* do_trap() refactoring:
  * read 16-bits value instead of 32-bits as compressed instruction can
be used and it might happen than only 16-bits may be accessible.
  * code style updates
  * re-use instr variable instead of re-reading instruction.
  - Updates in setup.c:
* add blank line between xen/ and asm/ includes.
---
Changes in V3:
  - Rebase the patch "xen/riscv: introduce an implementation of macros
from " on top of patch series [introduce generic implementation
of macros from bug.h]
---
Changes in V2:
  - Remove __ in define namings
  - Update run_in_exception_handler() with
register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
  - Remove bug_instr_t type and change it's usage to uint32_t
---
 xen/arch/riscv/include/asm/bug.h   |  28 +
 xen/arch/riscv/include/asm/processor.h |   2 +
 xen/arch/riscv/traps.c | 139 +
 xen/arch/riscv/xen.lds.S   |  10 ++
 4 files changed, 179 insertions(+)

diff --git a/xen/arch/riscv/include/asm/bug.h b/xen/arch/riscv/include/asm/bug.h
index e8b1e40823..bf3194443f 100644
--- a/xen/arch/riscv/include/asm/bug.h
+++ b/xen/arch/riscv/include/asm/bug.h
@@ -7,4 +7,32 @@
 #ifndef _ASM_RISCV_BUG_H
 #define _ASM_RISCV_BUG_H
 
+#ifndef __ASSEMBLY__
+
+#define BUG_INSTR "ebreak"
+
+/*
+ * The base instruction set has a fixed length of 32-bit naturally aligned
+ * instructions.
+ *
+ * There are extensions of variable length ( where each instruction can be
+ * any number of 16-bit parcels in length ) but they aren't used in Xen
+ * and Linux kernel ( where these definitions were taken from ).
+ *
+ * Compressed ISA is used now where the instruction length is 16 bit  and
+ * 'ebreak' instruction, in this case, can be either 16 or 32 bit (
+ * depending on if compressed ISA is used or not )
+ */
+#define INSN_LENGTH_MASK_UL(0x3)
+#define INSN_LENGTH_32  _UL(0x3)
+
+#define BUG_INSN_32 _UL(0x00100073) /* ebreak */
+#define BUG_INSN_16 _UL(0x9002) /* c.ebreak */
+#define COMPRESSED_INSN_MASK_UL(0x)
+
+#define GET_INSN_LENGTH(insn)   \
+(((insn) & INSN_LENGTH_MASK) == INSN_LENGTH_32 ? 4 : 2) \
+
+#endif /* !__ASSEMBLY__ */
+
 #endif /* _ASM_RISCV_BUG_H */
diff --git a/xen/arch/riscv/include/asm/processor.h 
b/xen/arch/riscv/include/asm/processor.h
index a71448e02e..ef23d9589e 100644
--- a/xen/arch/riscv/include/asm/processor.h
+++ b/xen/arch/riscv/include/asm/processor.h
@@ -69,6 +69,8 @@ static inline void die(void)
 wfi();
 }
 
+void show_execution_state(const struct cpu_user_regs *regs);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_PROCESSOR_H */
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index 581f34efbc..2afcabb912 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -5,6 +5,8 @@
  * RISC-V Trap handlers
  */
 
+#include 
+#include 
 #include 
 
 #include 
@@ 

[PATCH v5 0/7] RISCV basic exception handling implementation

2023-03-16 Thread Oleksii Kurochko
The patch series is based on:
* [introduce generic implementation of macros from bug.h][1]
* [Do basic initialization things][2]
* [Deal with GOT stuff for RISC-V][3]
which haven't been commited yet.

The patch series provides a basic implementation of exception handling.
It can do only basic things such as decode a cause of an exception,
save/restore registers and execute "wfi" instruction if an exception
can not be handled.

To verify that exception handling works well it was implemented macros
from  such as BUG/WARN/run_in_exception/assert_failed.
The implementation of macros is used "ebreak" instruction and set up bug
frame tables for each type of macros.
Also it was implemented register save/restore to return and continue work
after WARN/run_in_exception.
Not all functionality of the macros was implemented as some of them
require hard-panic the system which is not available now. Instead of
hard-panic 'wfi' instruction is used but it should be definitely changed
in the neareset future.
It wasn't implemented show_execution_state() and stack trace discovering
as it's not necessary now.

[1] 
https://lore.kernel.org/xen-devel/cover.1678900513.git.oleksii.kuroc...@gmail.com/
[2] 
https://lore.kernel.org/xen-devel/cover.1677838213.git.oleksii.kuroc...@gmail.com/
[3] 
https://lore.kernel.org/xen-devel/69e031eb-6172-1ab0-5ffa-4650f69e8...@citrix.com/T/#t
---
Changes in V5:
 - Rebase on top of [1] and [2]
 - Add new patch which introduces stub for  to keep Xen compilable
   as in the patch [xen/riscv: introduce decode_cause() stuff] is used
   header  which requires .
 - Remove  from riscv/traps/c as nothing would require
   inclusion.
 - decode_reserved_interrupt_cause(), decode_interrupt_cause(),
   decode_cause, do_unexpected_trap() were made as static they are expected
   to be used only in traps.c
 - Remove "#include " from  as there is no any need in 
it anymore
 - Update macros GET_INSN_LENGTH: remove UL and 'unsigned int len;' from it
 - Remove " include " from risc/setup.c. it is not needed in the 
current version of
   the patch
 - change an argument type from vaddr_t to uint32_t for is_valid_bugaddr and 
introduce 
   read_instr() to read instruction properly as the length of qinstruction can 
be
   either 32 or 16 bits.
 - Code style fixes
 - update the comments before do_bug_frame() in riscv/trap.c
 - [[PATCH v4 5/5] automation: modify RISC-V smoke test ] was dropped as it was 
provided
   more simple solution by Andrew.  CI: Simplify RISCV smoke testing
 - Refactor is_valid_bugaddr() function.
 - 2 new patches ([PATCH v5 {1-2}/7]) were introduced, the goal of which is to 
recalculate
   addresses used in traps.c, which can be linker time relative. It is needed 
as we don't
   have enabled MMU yet.
---
Changes in V4:
  - Rebase the patch series on top of new version of [introduce generic
implementation of macros from bug.h] patch series.
  - Update the cover letter message as 'Early printk' was merged and
the current one patch series is based only on [introduce generic
implementation of macros from bug.h] which hasn't been commited yet.
  - The following patches of the patch series were merged to staging:
  [PATCH v3 01/14] xen/riscv: change ISA to r64G
  [PATCH v3 02/14] xen/riscv: add  header
  [PATCH v3 03/14] xen/riscv: add  header
  [PATCH v3 05/14] xen/riscv: introduce empty 
  [PATCH v3 06/14] xen/riscv: introduce empty 
  [PATCH v3 07/14] xen/riscv: introduce exception context
  [PATCH v3 08/14] xen/riscv: introduce exception handlers implementation
  [PATCH v3 10/14] xen/riscv: mask all interrupts
  - Fix addressed comments in xen-devel mailing list.

---
Changes in V3:
  - Change the name of config RISCV_ISA_RV64IMA to RISCV_ISA_RV64G
as instructions from Zicsr and Zifencei extensions aren't part of
I extension any more.
  - Rebase the patch "xen/riscv: introduce an implementation of macros
from " on top of patch series [introduce generic implementation
of macros from bug.h]
  - Update commit messages
---
Changes in V2:
  - take the latest riscv_encoding.h from OpenSBI, update it with Xen
related changes, and update the commit message with "Origin:"
tag and the commit message itself.
  - add "Origin:" tag to the commit messag of the patch
[xen/riscv: add  header].
  - Remove the patch [xen/riscv: add early_printk_hnum() function] as the
functionality provided by the patch isn't used now.
  - Refactor prcoess.h: move structure offset defines to asm-offsets.c,
change register_t to unsigned long.
  - Refactor entry.S to use offsets defined in asm-offsets.C
  - Rename {__,}handle_exception to handle_trap() and do_trap() to be more
consistent with RISC-V spec.
  - Merge the pathc which introduces do_unexpected_trap() with the patch
[xen/riscv: introduce exception handlers implementation].
  - Rename setup_trap_handler() to trap_init() and update correspondingly
the patches in the patch series.
  - Refa

[PATCH v5 5/7] xen/riscv: introduce trap_init()

2023-03-16 Thread Oleksii Kurochko
Signed-off-by: Oleksii Kurochko 
Reviewed-by: Alistair Francis 
---
Changes in V5:
  - Nothing changed
---
Changes in V4:
  - Nothing changed
---
Changes in V3:
  - Nothing changed
---
Changes in V2:
  - Rename setup_trap_handler() to trap_init().
  - Add Reviewed-by to the commit message.
---
 xen/arch/riscv/include/asm/traps.h | 1 +
 xen/arch/riscv/setup.c | 5 +
 xen/arch/riscv/traps.c | 7 +++
 3 files changed, 13 insertions(+)

diff --git a/xen/arch/riscv/include/asm/traps.h 
b/xen/arch/riscv/include/asm/traps.h
index f3fb6b25d1..f1879294ef 100644
--- a/xen/arch/riscv/include/asm/traps.h
+++ b/xen/arch/riscv/include/asm/traps.h
@@ -7,6 +7,7 @@
 
 void do_trap(struct cpu_user_regs *cpu_regs);
 void handle_trap(void);
+void trap_init(void);
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 36556eb779..b44d105b5f 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -3,7 +3,9 @@
 #include 
 
 #include 
+#include 
 #include 
+#include 
 
 /* Xen stack for bringing up the first CPU. */
 unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
@@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
 
 fill_boot_info();
 
+trap_init();
+
 early_printk("All set up\n");
+
 for ( ;; )
 asm volatile ("wfi");
 
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index 8a1529e0c5..581f34efbc 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -13,6 +13,13 @@
 #include 
 #include 
 
+void trap_init(void)
+{
+unsigned long addr = (unsigned long)&handle_trap;
+
+csr_write(CSR_STVEC, addr);
+}
+
 static const char *decode_trap_cause(unsigned long cause)
 {
 static const char *const trap_causes[] = {
-- 
2.39.2




[PATCH v5 4/7] xen/riscv: introduce decode_cause() stuff

2023-03-16 Thread Oleksii Kurochko
The patch introduces stuff needed to decode a reason of an
exception.

Signed-off-by: Oleksii Kurochko 
---
Changes in V5:
  - Remove  from riscv/traps/c as nothing would require
inclusion.
  - decode_reserved_interrupt_cause(), decode_interrupt_cause(), decode_cause, 
do_unexpected_trap()
were made as static they are expected to be used only in traps.c
  - use LINK_TO_LOAD() for addresses which can be linker time relative.
---
Changes in V4:
  - fix string in decode_reserved_interrupt_cause()
---
Changes in V3:
  - Nothing changed
---
Changes in V2:
  - Make decode_trap_cause() more optimization friendly.
  - Merge the pathc which introduces do_unexpected_trap() to the current one.
---
 xen/arch/riscv/traps.c | 87 +-
 1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index ccd3593f5a..8a1529e0c5 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -4,10 +4,95 @@
  *
  * RISC-V Trap handlers
  */
+
+#include 
+
+#include 
+#include 
+#include 
 #include 
 #include 
 
-void do_trap(struct cpu_user_regs *cpu_regs)
+static const char *decode_trap_cause(unsigned long cause)
+{
+static const char *const trap_causes[] = {
+[CAUSE_MISALIGNED_FETCH] = "Instruction Address Misaligned",
+[CAUSE_FETCH_ACCESS] = "Instruction Access Fault",
+[CAUSE_ILLEGAL_INSTRUCTION] = "Illegal Instruction",
+[CAUSE_BREAKPOINT] = "Breakpoint",
+[CAUSE_MISALIGNED_LOAD] = "Load Address Misaligned",
+[CAUSE_LOAD_ACCESS] = "Load Access Fault",
+[CAUSE_MISALIGNED_STORE] = "Store/AMO Address Misaligned",
+[CAUSE_STORE_ACCESS] = "Store/AMO Access Fault",
+[CAUSE_USER_ECALL] = "Environment Call from U-Mode",
+[CAUSE_SUPERVISOR_ECALL] = "Environment Call from S-Mode",
+[CAUSE_MACHINE_ECALL] = "Environment Call from M-Mode",
+[CAUSE_FETCH_PAGE_FAULT] = "Instruction Page Fault",
+[CAUSE_LOAD_PAGE_FAULT] = "Load Page Fault",
+[CAUSE_STORE_PAGE_FAULT] = "Store/AMO Page Fault",
+[CAUSE_FETCH_GUEST_PAGE_FAULT] = "Instruction Guest Page Fault",
+[CAUSE_LOAD_GUEST_PAGE_FAULT] = "Load Guest Page Fault",
+[CAUSE_VIRTUAL_INST_FAULT] = "Virtualized Instruction Fault",
+[CAUSE_STORE_GUEST_PAGE_FAULT] = "Guest Store/AMO Page Fault",
+};
+
+if ( cause < ARRAY_SIZE(trap_causes) && trap_causes[cause] )
+return trap_causes[cause];
+return "UNKNOWN";
+}
+
+static const char *decode_reserved_interrupt_cause(unsigned long irq_cause)
+{
+switch ( irq_cause )
+{
+case IRQ_M_SOFT:
+return "M-mode Software Interrupt";
+case IRQ_M_TIMER:
+return "M-mode TIMER Interrupt";
+case IRQ_M_EXT:
+return "M-mode External Interrupt";
+default:
+return "UNKNOWN IRQ type";
+}
+}
+
+static const char *decode_interrupt_cause(unsigned long cause)
+{
+unsigned long irq_cause = cause & ~CAUSE_IRQ_FLAG;
+
+switch ( irq_cause )
+{
+case IRQ_S_SOFT:
+return "Supervisor Software Interrupt";
+case IRQ_S_TIMER:
+return "Supervisor Timer Interrupt";
+case IRQ_S_EXT:
+return "Supervisor External Interrupt";
+default:
+return decode_reserved_interrupt_cause(irq_cause);
+}
+}
+
+static const char *decode_cause(unsigned long cause)
+{
+if ( cause & CAUSE_IRQ_FLAG )
+return decode_interrupt_cause(cause);
+
+return decode_trap_cause(cause);
+}
+
+static void do_unexpected_trap(const struct cpu_user_regs *regs)
 {
+unsigned long cause = csr_read(CSR_SCAUSE);
+
+early_printk("Unhandled exception: ");
+early_printk(LINK_TO_LOAD(decode_cause(cause)));
+early_printk("\n");
+
 die();
 }
+
+void do_trap(struct cpu_user_regs *cpu_regs)
+{
+do_unexpected_trap(cpu_regs);
+}
-- 
2.39.2




[PATCH v5 3/7] xen/riscv: introduce dummy

2023-03-16 Thread Oleksii Kurochko
 will be used in the patch "xen/riscv: introduce
decode_cause() stuff" and requires 

Signed-off-by: Oleksii Kurochko 
---
Changes in V5:
 * the patch was introduced in the current patch series (V5)
---
 xen/arch/riscv/include/asm/bug.h | 10 ++
 1 file changed, 10 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/bug.h

diff --git a/xen/arch/riscv/include/asm/bug.h b/xen/arch/riscv/include/asm/bug.h
new file mode 100644
index 00..e8b1e40823
--- /dev/null
+++ b/xen/arch/riscv/include/asm/bug.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2021-2023 Vates
+ *
+ */
+#ifndef _ASM_RISCV_BUG_H
+#define _ASM_RISCV_BUG_H
+
+#endif /* _ASM_RISCV_BUG_H */
-- 
2.39.2




[PATCH v5 2/7] xen/riscv: initialize boot_info structure

2023-03-16 Thread Oleksii Kurochko
Signed-off-by: Oleksii Kurochko 
---
Changes in V5:
 * the patch was introduced in the current patch series (V5)
---
 xen/arch/riscv/setup.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 0908bdb9f9..36556eb779 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,12 +1,16 @@
 #include 
 #include 
+#include 
 
+#include 
 #include 
 
 /* Xen stack for bringing up the first CPU. */
 unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
 __aligned(STACK_SIZE);
 
+struct boot_info boot_info = { (unsigned long)&_start, (unsigned long)&_end, 
0UL, 0UL };
+
 /*  
  * To be sure that .bss isn't zero. It will simplify code of
  * .bss initialization.
@@ -15,11 +19,19 @@ unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
  */
 int dummy_bss __attribute__((unused));
 
+static void fill_boot_info(void)
+{
+boot_info.load_start = (unsigned long)_start;
+boot_info.load_end   = (unsigned long)_end;
+}
+
 void __init noreturn start_xen(unsigned long bootcpu_id,
unsigned long dtb_paddr)
 {
 early_printk("Hello from C env\n");
 
+fill_boot_info();
+
 early_printk("All set up\n");
 for ( ;; )
 asm volatile ("wfi");
-- 
2.39.2




[PATCH v5 1/7] xen/riscv: introduce boot information structure

2023-03-16 Thread Oleksii Kurochko
The structure holds information about:
1. linker start/end address
2. load start/end address

Also the patch introduces offsets for boot information structure
members to access them in assembly code.

Signed-off-by: Oleksii Kurochko 
---
Changes in V5:
 * the patch was introduced in the current patch series (V5)
---
 xen/arch/riscv/include/asm/boot-info.h | 15 +++
 xen/arch/riscv/riscv64/asm-offsets.c   |  3 +++
 2 files changed, 18 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/boot-info.h

diff --git a/xen/arch/riscv/include/asm/boot-info.h 
b/xen/arch/riscv/include/asm/boot-info.h
new file mode 100644
index 00..cda3d278f5
--- /dev/null
+++ b/xen/arch/riscv/include/asm/boot-info.h
@@ -0,0 +1,15 @@
+#ifndef _ASM_BOOT_INFO_H
+#define _ASM_BOOT_INFO_H
+
+extern struct boot_info {
+unsigned long linker_start;
+unsigned long linker_end;
+unsigned long load_start;
+unsigned long load_end;
+} boot_info;
+
+/* LINK_TO_LOAD() and LOAD_TO_LINK() works only when MMU isn't enabled. */
+#define LINK_TO_LOAD(addr) ((addr) - boot_info.linker_start + 
boot_info.load_start)
+#define LOAD_TO_LINK(addr) ((addr) - boot_info.load_start + 
boot_info.linker_start)
+
+#endif
\ No newline at end of file
diff --git a/xen/arch/riscv/riscv64/asm-offsets.c 
b/xen/arch/riscv/riscv64/asm-offsets.c
index d632b75c2a..6b89e9a91d 100644
--- a/xen/arch/riscv/riscv64/asm-offsets.c
+++ b/xen/arch/riscv/riscv64/asm-offsets.c
@@ -1,5 +1,6 @@
 #define COMPILE_OFFSETS
 
+#include 
 #include 
 #include 
 
@@ -50,4 +51,6 @@ void asm_offsets(void)
 OFFSET(CPU_USER_REGS_SEPC, struct cpu_user_regs, sepc);
 OFFSET(CPU_USER_REGS_SSTATUS, struct cpu_user_regs, sstatus);
 OFFSET(CPU_USER_REGS_PREGS, struct cpu_user_regs, pregs);
+OFFSET(BI_LINKER_START, struct boot_info, linker_start);
+OFFSET(BI_LOAD_START, struct boot_info, load_start);
 }
-- 
2.39.2




Re: [PATCH v2] xen/console: Skip switching serial input to non existing domains

2023-03-16 Thread George Dunlap
On Thu, Mar 16, 2023 at 2:15 PM Michal Orzel  wrote:

>
>
> On 16/03/2023 12:11, Jan Beulich wrote:
> > Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> >
> >
> > On 16.03.2023 11:26, Michal Orzel wrote:
> >> --- a/xen/drivers/char/console.c
> >> +++ b/xen/drivers/char/console.c
> >> @@ -490,7 +490,24 @@ static void switch_serial_input(void)
> >>  }
> >>  else
> >>  {
> >> -console_rx++;
> >> +unsigned int next_rx = console_rx + 1;
> >> +
> >> +/* Skip switching serial input to non existing domains */
> >> +while ( next_rx < max_init_domid + 1 )
> >> +{
> >> +struct domain *d = rcu_lock_domain_by_id(next_rx - 1);
> >> +
> >> +if ( d )
> >> +{
> >> +rcu_unlock_domain(d);
> >> +break;
> >> +}
> >> +
> >> +next_rx++;
> >> +}
> >> +
> >> +console_rx = next_rx;
> >> +
> >>  printk("*** Serial input to DOM%d", console_rx - 1);
> >>  }
> >
> > While at the first glance (when you sent it in reply to v1) it looked
> okay,
> > I'm afraid it really isn't: Please consider what happens when the last of
> > the DomU-s doesn't exist anymore. (You don't really check whether it
> still
> > exists, because the range check comes ahead of the existence one.) In
> that
> > case you want to move from second-to-last to Xen. I expect the entire
> > if/else construct wants to be inside the loop.
> I did this deliberately because I do not think the situation you describe
> is possible
> (i.e. no domains at all - Xen still usable). With hardware domain in
> place, we can e.g. destroy the domain
> which would invoke domain_kill() -> domain_destroy() that would free
> domain struct.
> Without hwdom, the domain cannot kill/destroy itself. It can do the
> shutdown but it will not
> destroy it (at least this is what I tested). So I do not think there can
> be a scenario where
> there is not a single domain while Xen running and be usable.


We've actually been discussing something like this.  Consider if someone
wanted to use Xen as part of a system architected like Amazon's Nitro: You
could have a DPU that ran the "toolstack", and gave Xen commands to create
or destroy domains.  It could dynamically create SRIOV PCI devices to be
passed directly through to guests.  In this case, you might run into a
situation where no VMs existed, and yet the system was not dead.

 -George


Re: [RFC PATCH 1/5] x86/xen: disable swiotlb for xen pvh

2023-03-16 Thread Juergen Gross

On 16.03.23 14:53, Alex Deucher wrote:

On Thu, Mar 16, 2023 at 9:48 AM Juergen Gross  wrote:


On 16.03.23 14:45, Alex Deucher wrote:

On Thu, Mar 16, 2023 at 3:50 AM Jan Beulich  wrote:


On 16.03.2023 00:25, Stefano Stabellini wrote:

On Wed, 15 Mar 2023, Jan Beulich wrote:

On 15.03.2023 01:52, Stefano Stabellini wrote:

On Mon, 13 Mar 2023, Jan Beulich wrote:

On 12.03.2023 13:01, Huang Rui wrote:

Xen PVH is the paravirtualized mode and takes advantage of hardware
virtualization support when possible. It will using the hardware IOMMU
support instead of xen-swiotlb, so disable swiotlb if current domain is
Xen PVH.


But the kernel has no way (yet) to drive the IOMMU, so how can it get
away without resorting to swiotlb in certain cases (like I/O to an
address-restricted device)?


I think Ray meant that, thanks to the IOMMU setup by Xen, there is no
need for swiotlb-xen in Dom0. Address translations are done by the IOMMU
so we can use guest physical addresses instead of machine addresses for
DMA. This is a similar case to Dom0 on ARM when the IOMMU is available
(see include/xen/arm/swiotlb-xen.h:xen_swiotlb_detect, the corresponding
case is XENFEAT_not_direct_mapped).


But how does Xen using an IOMMU help with, as said, address-restricted
devices? They may still need e.g. a 32-bit address to be programmed in,
and if the kernel has memory beyond the 4G boundary not all I/O buffers
may fulfill this requirement.


In short, it is going to work as long as Linux has guest physical
addresses (not machine addresses, those could be anything) lower than
4GB.

If the address-restricted device does DMA via an IOMMU, then the device
gets programmed by Linux using its guest physical addresses (not machine
addresses).

The 32-bit restriction would be applied by Linux to its choice of guest
physical address to use to program the device, the same way it does on
native. The device would be fine as it always uses Linux-provided <4GB
addresses. After the IOMMU translation (pagetable setup by Xen), we
could get any address, including >4GB addresses, and that is expected to
work.


I understand that's the "normal" way of working. But whatever the swiotlb
is used for in baremetal Linux, that would similarly require its use in
PVH (or HVM) aiui. So unconditionally disabling it in PVH would look to
me like an incomplete attempt to disable its use altogether on x86. What
difference of PVH vs baremetal am I missing here?


swiotlb is not usable for GPUs even on bare metal.  They often have
hundreds or megs or even gigs of memory mapped on the device at any
given time.  Also, AMD GPUs support 44-48 bit DMA masks (depending on
the chip family).


But the swiotlb isn't per device, but system global.


Sure, but if the swiotlb is in use, then you can't really use the GPU.
So you get to pick one.


The swiotlb is used only for buffers which are not within the DMA mask of a
device (see dma_direct_map_page()). So an AMD GPU supporting a 44 bit DMA mask
won't use the swiotlb unless you have a buffer above guest physical address of
16TB (so basically never).

Disabling swiotlb in such a guest would OTOH mean, that a device with only
32 bit DMA mask passed through to this guest couldn't work with buffers
above 4GB.

I don't think this is acceptable.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH v2] xen/console: Skip switching serial input to non existing domains

2023-03-16 Thread Michal Orzel



On 16/03/2023 12:11, Jan Beulich wrote:
> Caution: This message originated from an External Source. Use proper caution 
> when opening attachments, clicking links, or responding.
> 
> 
> On 16.03.2023 11:26, Michal Orzel wrote:
>> --- a/xen/drivers/char/console.c
>> +++ b/xen/drivers/char/console.c
>> @@ -490,7 +490,24 @@ static void switch_serial_input(void)
>>  }
>>  else
>>  {
>> -console_rx++;
>> +unsigned int next_rx = console_rx + 1;
>> +
>> +/* Skip switching serial input to non existing domains */
>> +while ( next_rx < max_init_domid + 1 )
>> +{
>> +struct domain *d = rcu_lock_domain_by_id(next_rx - 1);
>> +
>> +if ( d )
>> +{
>> +rcu_unlock_domain(d);
>> +break;
>> +}
>> +
>> +next_rx++;
>> +}
>> +
>> +console_rx = next_rx;
>> +
>>  printk("*** Serial input to DOM%d", console_rx - 1);
>>  }
> 
> While at the first glance (when you sent it in reply to v1) it looked okay,
> I'm afraid it really isn't: Please consider what happens when the last of
> the DomU-s doesn't exist anymore. (You don't really check whether it still
> exists, because the range check comes ahead of the existence one.) In that
> case you want to move from second-to-last to Xen. I expect the entire
> if/else construct wants to be inside the loop.
I did this deliberately because I do not think the situation you describe is 
possible
(i.e. no domains at all - Xen still usable). With hardware domain in place, we 
can e.g. destroy the domain
which would invoke domain_kill() -> domain_destroy() that would free domain 
struct.
Without hwdom, the domain cannot kill/destroy itself. It can do the shutdown 
but it will not
destroy it (at least this is what I tested). So I do not think there can be a 
scenario where
there is not a single domain while Xen running and be usable.

~Michal



Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Jan Beulich
On 16.03.2023 14:54, Roger Pau Monné wrote:
> On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
>> --- /dev/null
>> +++ b/xen/include/xen/refcnt.h
>> @@ -0,0 +1,59 @@
> 
> This seems to be missing some kind of license, can we have an SPDX tag
> at least?

Not "at least", but strictly that way for any new code. Patches to convert
various existing code to SPDX are already pending iirc.

>> +#ifndef __XEN_REFCNT_H__
>> +#define __XEN_REFCNT_H__
>> +
>> +#include 
>> +#include 
>> +
>> +#define REFCNT_SATURATED (INT_MIN / 2)
>> +
>> +typedef struct {
>> +atomic_t refcnt;
>> +} refcnt_t;
>> +
>> +static inline void refcnt_init(refcnt_t *refcnt)
>> +{
>> +atomic_set(&refcnt->refcnt, 1);
>> +}
>> +
>> +static inline int refcnt_read(refcnt_t *refcnt)
> 
> const.
> 
>> +{
>> +return atomic_read(&refcnt->refcnt);
>> +}
>> +
>> +static inline void refcnt_get(refcnt_t *refcnt)
>> +{
>> +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
>> +
>> +if ( unlikely(old < 0) || unlikely (old + 1 < 0) )
>  ^ extra space
> 
> You want a single unlikely for both conditions.

Are you sure? My experience was generally the other way around: likely()
and unlikely() become ineffectual as soon as the compiler needs more
than one branch for the inner construct (ie generally for and && or ||).

Jan



Re: [PATCH v2 0/2] deal with GOT stuff for RISC-V

2023-03-16 Thread Andrew Cooper
On 16/03/2023 1:22 pm, Oleksii Kurochko wrote:
> Oleksii Kurochko (2):
>   xen/riscv: add EMBEDDED_EXTRA_CFLAGS to CFLAGS
>   xen/riscv: add explicit check that .got{.plt} is empty

Acked-by: Andrew Cooper 



Re: [RFC PATCH 1/5] x86/xen: disable swiotlb for xen pvh

2023-03-16 Thread Jan Beulich
On 16.03.2023 14:53, Alex Deucher wrote:
> On Thu, Mar 16, 2023 at 9:48 AM Juergen Gross  wrote:
>>
>> On 16.03.23 14:45, Alex Deucher wrote:
>>> On Thu, Mar 16, 2023 at 3:50 AM Jan Beulich  wrote:

 On 16.03.2023 00:25, Stefano Stabellini wrote:
> On Wed, 15 Mar 2023, Jan Beulich wrote:
>> On 15.03.2023 01:52, Stefano Stabellini wrote:
>>> On Mon, 13 Mar 2023, Jan Beulich wrote:
 On 12.03.2023 13:01, Huang Rui wrote:
> Xen PVH is the paravirtualized mode and takes advantage of hardware
> virtualization support when possible. It will using the hardware IOMMU
> support instead of xen-swiotlb, so disable swiotlb if current domain 
> is
> Xen PVH.

 But the kernel has no way (yet) to drive the IOMMU, so how can it get
 away without resorting to swiotlb in certain cases (like I/O to an
 address-restricted device)?
>>>
>>> I think Ray meant that, thanks to the IOMMU setup by Xen, there is no
>>> need for swiotlb-xen in Dom0. Address translations are done by the IOMMU
>>> so we can use guest physical addresses instead of machine addresses for
>>> DMA. This is a similar case to Dom0 on ARM when the IOMMU is available
>>> (see include/xen/arm/swiotlb-xen.h:xen_swiotlb_detect, the corresponding
>>> case is XENFEAT_not_direct_mapped).
>>
>> But how does Xen using an IOMMU help with, as said, address-restricted
>> devices? They may still need e.g. a 32-bit address to be programmed in,
>> and if the kernel has memory beyond the 4G boundary not all I/O buffers
>> may fulfill this requirement.
>
> In short, it is going to work as long as Linux has guest physical
> addresses (not machine addresses, those could be anything) lower than
> 4GB.
>
> If the address-restricted device does DMA via an IOMMU, then the device
> gets programmed by Linux using its guest physical addresses (not machine
> addresses).
>
> The 32-bit restriction would be applied by Linux to its choice of guest
> physical address to use to program the device, the same way it does on
> native. The device would be fine as it always uses Linux-provided <4GB
> addresses. After the IOMMU translation (pagetable setup by Xen), we
> could get any address, including >4GB addresses, and that is expected to
> work.

 I understand that's the "normal" way of working. But whatever the swiotlb
 is used for in baremetal Linux, that would similarly require its use in
 PVH (or HVM) aiui. So unconditionally disabling it in PVH would look to
 me like an incomplete attempt to disable its use altogether on x86. What
 difference of PVH vs baremetal am I missing here?
>>>
>>> swiotlb is not usable for GPUs even on bare metal.  They often have
>>> hundreds or megs or even gigs of memory mapped on the device at any
>>> given time.  Also, AMD GPUs support 44-48 bit DMA masks (depending on
>>> the chip family).
>>
>> But the swiotlb isn't per device, but system global.
> 
> Sure, but if the swiotlb is in use, then you can't really use the GPU.
> So you get to pick one.

Yet that "pick one" then can't be an unconditional disable in the source code.
If there's no way to avoid swiotlb on a per-device basis, then users will need
to be told to arrange for this via command line option when they want to use
the GPU is certain ways.

Jan



Re: [PATCH v2 2/2] xen/riscv: add explicit check that .got{.plt} is empty

2023-03-16 Thread Jan Beulich
On 16.03.2023 14:22, Oleksii Kurochko wrote:
> The GOT sections usage should be avoided in the hypervisor
> so to catch such use cases earlier when GOT things are
> produced the patch introduces .got and .got.plt sections
> and adds asserts that they're empty.
> 
> The sections won't be created until they remain
> empty otherwise the asserts would cause early failure.
> 
> Signed-off-by: Oleksii Kurochko 

Reviewed-by: Jan Beulich 

And again a Suggested-by to Andrew perhaps.

Jan



Re: [PATCH v3 1/6] xen: add reference counter support

2023-03-16 Thread Roger Pau Monné
On Tue, Mar 14, 2023 at 08:56:29PM +, Volodymyr Babchuk wrote:
> We can use reference counter to ease up object lifetime management.
> This patch adds very basic support for reference counters. refcnt
> should be used in the following way:
> 
> 1. Protected structure should have refcnt_t field
> 
> 2. This field should be initialized with refcnt_init() during object
> construction.
> 
> 3. If code holds a valid pointer to a structure/object it can increase
> refcount with refcnt_get(). No additional locking is required.
> 
> 4. Code should call refcnt_put() before dropping pointer to a
> protected structure. `destructor` is a call back function that should
> destruct object and free all resources, including structure protected
> itself. Destructor will be called if reference counter reaches zero.
> 
> 5. If code does not hold a valid pointer to a protected structure it
> should use other locking mechanism to obtain a pointer. For example,
> it should lock a list that hold protected objects.

Sorry, I didn't look at the previous versions, but did we consider
importing refcount_t and related logic from Linux?

> 
> Signed-off-by: Volodymyr Babchuk 
> 
> ---
> v3:
>   - moved in from another patch series
>   - used structure to encapsulate refcnt_t
>   - removed #ifndef NDEBUG in favor of just calling ASSERT
>   - added assertion to refcnt_put
>   - added saturation support: code catches overflow and underflow
>   - added EMACS magic at end of the file
>   - fixed formatting
> ---
>  xen/include/xen/refcnt.h | 59 
>  1 file changed, 59 insertions(+)
>  create mode 100644 xen/include/xen/refcnt.h
> 
> diff --git a/xen/include/xen/refcnt.h b/xen/include/xen/refcnt.h
> new file mode 100644
> index 00..1ac05d927c
> --- /dev/null
> +++ b/xen/include/xen/refcnt.h
> @@ -0,0 +1,59 @@

This seems to be missing some kind of license, can we have an SPDX tag
at least?

> +#ifndef __XEN_REFCNT_H__
> +#define __XEN_REFCNT_H__
> +
> +#include 
> +#include 
> +
> +#define REFCNT_SATURATED (INT_MIN / 2)
> +
> +typedef struct {
> +atomic_t refcnt;
> +} refcnt_t;
> +
> +static inline void refcnt_init(refcnt_t *refcnt)
> +{
> +atomic_set(&refcnt->refcnt, 1);
> +}
> +
> +static inline int refcnt_read(refcnt_t *refcnt)

const.

> +{
> +return atomic_read(&refcnt->refcnt);
> +}
> +
> +static inline void refcnt_get(refcnt_t *refcnt)
> +{
> +int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
> +
> +if ( unlikely(old < 0) || unlikely (old + 1 < 0) )
 ^ extra space

You want a single unlikely for both conditions.

> +{
> +atomic_set(&refcnt->refcnt, REFCNT_SATURATED);
> +printk(XENLOG_ERR"refcnt saturation: old = %d\n", old);

Should this be printed only once for refcount? I fear it might spam
the console once a refcnt hits it.

> +WARN();
> +}
> +}
> +
> +static inline void refcnt_put(refcnt_t *refcnt, void (*destructor)(refcnt_t 
> *refcnt))
> +{
> +int ret = atomic_dec_return(&refcnt->refcnt);
> +
> +if ( ret == 0 )
> +destructor(refcnt);
> +
> +if ( unlikely(ret < 0))
^ missing space
> +{
> +atomic_set(&refcnt->refcnt, REFCNT_SATURATED);
> +printk(XENLOG_ERR"refcnt already hit 0: val = %d\n", ret);

Same here regarding the spamming.

> +WARN();
> +}
> +}
> +

Extra newline.

I will look at further patches to see how this gets used.

Thanks, Roger.



Re: [PATCH v2 1/2] xen/riscv: add EMBEDDED_EXTRA_CFLAGS to CFLAGS

2023-03-16 Thread Jan Beulich
On 16.03.2023 14:22, Oleksii Kurochko wrote:
> The patch is needed to keep all address of cpu0_boot_stack
> PC-relative.
> 
> Pseudoinstruction 'la' can be transformed to 'auipc/addi' or
> 'auipc/l{w|d}'. It depends on the .option directive: nopic and pic
> or compiler flags.
> 
> Right now, 'la' transforms to 'auipc/l{w|d}', which in case of
> cpu0_boot_stack[] will lead to the usage of _GLOBAL_OFFSET_TABLE_
> where all addresses will be without counting that it might happen
> that linker address != load address ( so addresses inside got
> sections will be relative to linker time ).
> 
> It happens becuase the compiler from riscv64 docker compiled with
> --enable-default-pie:
>   [user@49295ae49cbe build]$ riscv64-linux-gnu-gcc -v
>   Using built-in specs.
>   COLLECT_GCC=riscv64-linux-gnu-gcc
>   COLLECT_LTO_WRAPPER=/usr/lib/gcc/riscv64-linux-gnu/12.2.0/lto-wrapper
>   Target: riscv64-linux-gnu
>   Configured with: /build/riscv64-linux-gnu-gcc/src/gcc-12.2.0/configure
>   --prefix=/usr --program-prefix=riscv64-linux-gnu- --with-local-
>   prefix=/usr/riscv64-linux-gnu --with-sysroot=/usr/riscv64-linux-gnu --
>   with-build-sysroot=/usr/riscv64-linux-gnu --libdir=/usr/lib --
>   libexecdir=/usr/lib --target=riscv64-linux-gnu --host=x86_64-pc-linux-
>   gnu --build=x86_64-pc-linux-gnu --with-system-zlib --with-isl --with-
>   linker-hash-style=gnu --disable-nls --disable-libunwind-exceptions --
>   disable-libstdcxx-pch --disable-libssp --disable-multilib --disable-
>   werror --enable-languages=c,c++ --enable-shared --enable-threads=posix
>   --enable-__cxa_atexit --enable-clocale=gnu --enable-gnu-unique-object -
>   -enable-linker-build-id --enable-lto --enable-plugin --enable-install-
>   libiberty --enable-gnu-indirect-function --enable-default-pie --enable-
>   checking=release
>   Thread model: posix
>   Supported LTO compression algorithms: zlib zstd
>   gcc version 12.2.0 (GCC)
> 
> Looking at gcc spec file for the RISC-V architecture:
>   [user@49295ae49cbe build]$ riscv64-linux-gnu-gcc -dumpspecs | grep -i
>   pic
>   --traditional-format %(subtarget_asm_debugging_spec) %{fno-pie|fno-
>   PIE|fno-pic|fno-PIC:;:-fpic} %{march=*} %{mabi=*} %{mno-relax} %{mbig-
>   endian} %{mlittle-endian} %(subtarget_asm_spec)%{misa-spec=*}
> which means that -fpic is enabled if none of the following options are
> present on the command line:
> -fno-pie
> -fno-PIE
> -fno-pic
> -fno-PIC
> 
> That's the reasons why 'la' is transformed to 'aupic/l{w|d} GOT' and
> not be dependent on the toolchain used.
> 
> Signed-off-by: Oleksii Kurochko 

Reviewed-by: Jan Beulich 

There may also want to be Suggested-by to both Andrew and me.

Jan



Re: [RFC PATCH 1/5] x86/xen: disable swiotlb for xen pvh

2023-03-16 Thread Alex Deucher
On Thu, Mar 16, 2023 at 9:48 AM Juergen Gross  wrote:
>
> On 16.03.23 14:45, Alex Deucher wrote:
> > On Thu, Mar 16, 2023 at 3:50 AM Jan Beulich  wrote:
> >>
> >> On 16.03.2023 00:25, Stefano Stabellini wrote:
> >>> On Wed, 15 Mar 2023, Jan Beulich wrote:
>  On 15.03.2023 01:52, Stefano Stabellini wrote:
> > On Mon, 13 Mar 2023, Jan Beulich wrote:
> >> On 12.03.2023 13:01, Huang Rui wrote:
> >>> Xen PVH is the paravirtualized mode and takes advantage of hardware
> >>> virtualization support when possible. It will using the hardware IOMMU
> >>> support instead of xen-swiotlb, so disable swiotlb if current domain 
> >>> is
> >>> Xen PVH.
> >>
> >> But the kernel has no way (yet) to drive the IOMMU, so how can it get
> >> away without resorting to swiotlb in certain cases (like I/O to an
> >> address-restricted device)?
> >
> > I think Ray meant that, thanks to the IOMMU setup by Xen, there is no
> > need for swiotlb-xen in Dom0. Address translations are done by the IOMMU
> > so we can use guest physical addresses instead of machine addresses for
> > DMA. This is a similar case to Dom0 on ARM when the IOMMU is available
> > (see include/xen/arm/swiotlb-xen.h:xen_swiotlb_detect, the corresponding
> > case is XENFEAT_not_direct_mapped).
> 
>  But how does Xen using an IOMMU help with, as said, address-restricted
>  devices? They may still need e.g. a 32-bit address to be programmed in,
>  and if the kernel has memory beyond the 4G boundary not all I/O buffers
>  may fulfill this requirement.
> >>>
> >>> In short, it is going to work as long as Linux has guest physical
> >>> addresses (not machine addresses, those could be anything) lower than
> >>> 4GB.
> >>>
> >>> If the address-restricted device does DMA via an IOMMU, then the device
> >>> gets programmed by Linux using its guest physical addresses (not machine
> >>> addresses).
> >>>
> >>> The 32-bit restriction would be applied by Linux to its choice of guest
> >>> physical address to use to program the device, the same way it does on
> >>> native. The device would be fine as it always uses Linux-provided <4GB
> >>> addresses. After the IOMMU translation (pagetable setup by Xen), we
> >>> could get any address, including >4GB addresses, and that is expected to
> >>> work.
> >>
> >> I understand that's the "normal" way of working. But whatever the swiotlb
> >> is used for in baremetal Linux, that would similarly require its use in
> >> PVH (or HVM) aiui. So unconditionally disabling it in PVH would look to
> >> me like an incomplete attempt to disable its use altogether on x86. What
> >> difference of PVH vs baremetal am I missing here?
> >
> > swiotlb is not usable for GPUs even on bare metal.  They often have
> > hundreds or megs or even gigs of memory mapped on the device at any
> > given time.  Also, AMD GPUs support 44-48 bit DMA masks (depending on
> > the chip family).
>
> But the swiotlb isn't per device, but system global.

Sure, but if the swiotlb is in use, then you can't really use the GPU.
So you get to pick one.

Alex



Re: [RFC PATCH 1/5] x86/xen: disable swiotlb for xen pvh

2023-03-16 Thread Juergen Gross

On 16.03.23 14:45, Alex Deucher wrote:

On Thu, Mar 16, 2023 at 3:50 AM Jan Beulich  wrote:


On 16.03.2023 00:25, Stefano Stabellini wrote:

On Wed, 15 Mar 2023, Jan Beulich wrote:

On 15.03.2023 01:52, Stefano Stabellini wrote:

On Mon, 13 Mar 2023, Jan Beulich wrote:

On 12.03.2023 13:01, Huang Rui wrote:

Xen PVH is the paravirtualized mode and takes advantage of hardware
virtualization support when possible. It will using the hardware IOMMU
support instead of xen-swiotlb, so disable swiotlb if current domain is
Xen PVH.


But the kernel has no way (yet) to drive the IOMMU, so how can it get
away without resorting to swiotlb in certain cases (like I/O to an
address-restricted device)?


I think Ray meant that, thanks to the IOMMU setup by Xen, there is no
need for swiotlb-xen in Dom0. Address translations are done by the IOMMU
so we can use guest physical addresses instead of machine addresses for
DMA. This is a similar case to Dom0 on ARM when the IOMMU is available
(see include/xen/arm/swiotlb-xen.h:xen_swiotlb_detect, the corresponding
case is XENFEAT_not_direct_mapped).


But how does Xen using an IOMMU help with, as said, address-restricted
devices? They may still need e.g. a 32-bit address to be programmed in,
and if the kernel has memory beyond the 4G boundary not all I/O buffers
may fulfill this requirement.


In short, it is going to work as long as Linux has guest physical
addresses (not machine addresses, those could be anything) lower than
4GB.

If the address-restricted device does DMA via an IOMMU, then the device
gets programmed by Linux using its guest physical addresses (not machine
addresses).

The 32-bit restriction would be applied by Linux to its choice of guest
physical address to use to program the device, the same way it does on
native. The device would be fine as it always uses Linux-provided <4GB
addresses. After the IOMMU translation (pagetable setup by Xen), we
could get any address, including >4GB addresses, and that is expected to
work.


I understand that's the "normal" way of working. But whatever the swiotlb
is used for in baremetal Linux, that would similarly require its use in
PVH (or HVM) aiui. So unconditionally disabling it in PVH would look to
me like an incomplete attempt to disable its use altogether on x86. What
difference of PVH vs baremetal am I missing here?


swiotlb is not usable for GPUs even on bare metal.  They often have
hundreds or megs or even gigs of memory mapped on the device at any
given time.  Also, AMD GPUs support 44-48 bit DMA masks (depending on
the chip family).


But the swiotlb isn't per device, but system global.


Juergen



OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


Re: [RFC PATCH 1/5] x86/xen: disable swiotlb for xen pvh

2023-03-16 Thread Alex Deucher
On Thu, Mar 16, 2023 at 3:50 AM Jan Beulich  wrote:
>
> On 16.03.2023 00:25, Stefano Stabellini wrote:
> > On Wed, 15 Mar 2023, Jan Beulich wrote:
> >> On 15.03.2023 01:52, Stefano Stabellini wrote:
> >>> On Mon, 13 Mar 2023, Jan Beulich wrote:
>  On 12.03.2023 13:01, Huang Rui wrote:
> > Xen PVH is the paravirtualized mode and takes advantage of hardware
> > virtualization support when possible. It will using the hardware IOMMU
> > support instead of xen-swiotlb, so disable swiotlb if current domain is
> > Xen PVH.
> 
>  But the kernel has no way (yet) to drive the IOMMU, so how can it get
>  away without resorting to swiotlb in certain cases (like I/O to an
>  address-restricted device)?
> >>>
> >>> I think Ray meant that, thanks to the IOMMU setup by Xen, there is no
> >>> need for swiotlb-xen in Dom0. Address translations are done by the IOMMU
> >>> so we can use guest physical addresses instead of machine addresses for
> >>> DMA. This is a similar case to Dom0 on ARM when the IOMMU is available
> >>> (see include/xen/arm/swiotlb-xen.h:xen_swiotlb_detect, the corresponding
> >>> case is XENFEAT_not_direct_mapped).
> >>
> >> But how does Xen using an IOMMU help with, as said, address-restricted
> >> devices? They may still need e.g. a 32-bit address to be programmed in,
> >> and if the kernel has memory beyond the 4G boundary not all I/O buffers
> >> may fulfill this requirement.
> >
> > In short, it is going to work as long as Linux has guest physical
> > addresses (not machine addresses, those could be anything) lower than
> > 4GB.
> >
> > If the address-restricted device does DMA via an IOMMU, then the device
> > gets programmed by Linux using its guest physical addresses (not machine
> > addresses).
> >
> > The 32-bit restriction would be applied by Linux to its choice of guest
> > physical address to use to program the device, the same way it does on
> > native. The device would be fine as it always uses Linux-provided <4GB
> > addresses. After the IOMMU translation (pagetable setup by Xen), we
> > could get any address, including >4GB addresses, and that is expected to
> > work.
>
> I understand that's the "normal" way of working. But whatever the swiotlb
> is used for in baremetal Linux, that would similarly require its use in
> PVH (or HVM) aiui. So unconditionally disabling it in PVH would look to
> me like an incomplete attempt to disable its use altogether on x86. What
> difference of PVH vs baremetal am I missing here?

swiotlb is not usable for GPUs even on bare metal.  They often have
hundreds or megs or even gigs of memory mapped on the device at any
given time.  Also, AMD GPUs support 44-48 bit DMA masks (depending on
the chip family).

Alex



Re: [PATCH v2 1/5] x86/paging: fold most HAP and shadow final teardown

2023-03-16 Thread Jan Beulich
On 16.03.2023 14:28, Roger Pau Monné wrote:
> On Thu, Mar 16, 2023 at 01:57:45PM +0100, Jan Beulich wrote:
>> On 16.03.2023 13:24, Roger Pau Monné wrote:
>>> Maybe we want to pull that hap_teardown() out of hap_final_teardown()
>>
>> That's what I'm doing here.
> 
> Oh, sorry, I've missed that chunk.  Then:
> 
> Reviewed-by: Roger Pau Monné 

Thanks. I'll take the liberty and swap '.' with 'r' ...

Jan




Re: [PATCH] build: omit "source" symlink when building hypervisor in-tree

2023-03-16 Thread Jan Beulich
On 15.03.2023 16:20, Anthony PERARD wrote:
> On Wed, Mar 15, 2023 at 03:56:21PM +0100, Jan Beulich wrote:
>> This symlink is getting in the way of using e.g. "find" on the xen/
>> subtree, and it isn't really needed when not building out-of-tree:
>> the one use that there was can easily be avoided.
>>
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/common/efi/efi-common.mk
>> +++ b/xen/common/efi/efi-common.mk
>> @@ -5,11 +5,16 @@ CFLAGS-y += -fshort-wchar
>>  CFLAGS-y += -iquote $(srctree)/common/efi
>>  CFLAGS-y += -iquote $(srcdir)
>>  
>> +source :=
>> +ifneq ($(abs_objtree),$(abs_srctree))
> 
> Could you use "ifdef building_out_of_srctree" instead, or at least use
> the variable $(building_out_of_srctree)? At least that mean there's a
> single way in the tree to differentiate between both kind of build.

I should have added a remark, I realize. I am actually aware of that
variable and also the fact that it is getting exported, but I was
seriously wondering why we do that: It's redundant information, and imo
a variable of this name shouldn't really be exported.

Furthermore I consider the conditional I'm presently using (matching
the one controlling the definition of building_out_of_srctree) more
descriptive: I had to go and convince myself that the variable really
is set based on comparing the paths; I had suspected it might be some
other conditional, not the least because of me not expecting that we'd
carry (and even export) redundant information.

So yes, if you insist I will switch. My preferred route would be to
ditch building_out_of_srctree, though.

>> +source := source/
>> +endif
>> +
>>  # Part of the command line transforms $(obj)
>>  # e.g.: It transforms "dir/foo/bar" into successively
>>  #   "dir foo bar", ".. .. ..", "../../.."
>>  $(obj)/%.c: $(srctree)/common/efi/%.c FORCE
>> -$(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, 
>> ,$(obj/source/common/efi/$(> +$(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, 
>> ,$(obj/$(source)common/efi/$( 
> Instead of $(source), I did proposed initially
> "$(if $(building_out_of_srctree),source/)" for here, or it that making
> the command line too long?
> https://lore.kernel.org/xen-devel/YebpHJk1JIArcdvW@perard/t/#u

Oh, I'm sorry for driving you into adding that symlink, which is now
getting in the way. But yes, putting it inline would imo make an already
too long line yet worse. Otoh I could of course wrap the line, albeit
some care may then be needed to not introduce whitespace in the wrong
place.

> Having "source := $(if $(building_out_of_srctree),source/)" might be an
> ok alternative in place of the use if "ifneq/endif" which take 4 lines?

As per above, if you're determined that building_out_of_srctree should
stay around, then I could do so. Alternatively how about

source := $(if $(patsubst $(abs_objtree),,$(abs_srctree)),,source/)

or the same with $(subst ...) or yet shorter

source := $(if $(abs_srctree:$(abs_objtree)=),,source/)

if you're after cutting the number of lines?

Jan



Re: [PATCH v2 1/5] x86/paging: fold most HAP and shadow final teardown

2023-03-16 Thread Roger Pau Monné
On Thu, Mar 16, 2023 at 01:57:45PM +0100, Jan Beulich wrote:
> On 16.03.2023 13:24, Roger Pau Monné wrote:
> > On Mon, Jan 09, 2023 at 02:39:19PM +0100, Jan Beulich wrote:
> >> HAP does a few things beyond what's common, which are left there at
> >> least for now. Common operations, however, are moved to
> >> paging_final_teardown(), allowing shadow_final_teardown() to go away.
> >>
> >> While moving (and hence generalizing) the respective SHADOW_PRINTK()
> >> drop the logging of total_pages from the 2nd instance - the value is
> >> necessarily zero after {hap,shadow}_set_allocation() - and shorten the
> >> messages, in part accounting for PAGING_PRINTK() logging __func__
> >> already.
> >>
> >> Signed-off-by: Jan Beulich 
> >> ---
> >> The remaining parts of hap_final_teardown() could be moved as well, at
> >> the price of a CONFIG_HVM conditional. I wasn't sure whether that was
> >> deemed reasonable.
> >> ---
> >> v2: Shorten PAGING_PRINTK() messages. Adjust comments while being
> >> moved.
> >>
> >> --- a/xen/arch/x86/include/asm/shadow.h
> >> +++ b/xen/arch/x86/include/asm/shadow.h
> >> @@ -78,9 +78,6 @@ int shadow_domctl(struct domain *d,
> >>  void shadow_vcpu_teardown(struct vcpu *v);
> >>  void shadow_teardown(struct domain *d, bool *preempted);
> >>  
> >> -/* Call once all of the references to the domain have gone away */
> >> -void shadow_final_teardown(struct domain *d);
> >> -
> >>  void sh_remove_shadows(struct domain *d, mfn_t gmfn, int fast, int all);
> >>  
> >>  /* Adjust shadows ready for a guest page to change its type. */
> >> --- a/xen/arch/x86/mm/hap/hap.c
> >> +++ b/xen/arch/x86/mm/hap/hap.c
> >> @@ -268,8 +268,8 @@ static void hap_free(struct domain *d, m
> >>  
> >>  /*
> >>   * For dying domains, actually free the memory here. This way less 
> >> work is
> >> - * left to hap_final_teardown(), which cannot easily have preemption 
> >> checks
> >> - * added.
> >> + * left to paging_final_teardown(), which cannot easily have 
> >> preemption
> >> + * checks added.
> >>   */
> >>  if ( unlikely(d->is_dying) )
> >>  {
> >> @@ -552,18 +552,6 @@ void hap_final_teardown(struct domain *d
> >>  for (i = 0; i < MAX_NESTEDP2M; i++) {
> >>  p2m_teardown(d->arch.nested_p2m[i], true, NULL);
> >>  }
> >> -
> >> -if ( d->arch.paging.total_pages != 0 )
> >> -hap_teardown(d, NULL);
> >> -
> >> -p2m_teardown(p2m_get_hostp2m(d), true, NULL);
> >> -/* Free any memory that the p2m teardown released */
> >> -paging_lock(d);
> >> -hap_set_allocation(d, 0, NULL);
> >> -ASSERT(d->arch.paging.p2m_pages == 0);
> >> -ASSERT(d->arch.paging.free_pages == 0);
> >> -ASSERT(d->arch.paging.total_pages == 0);
> >> -paging_unlock(d);
> >>  }
> >>  
> >>  void hap_vcpu_teardown(struct vcpu *v)
> >> --- a/xen/arch/x86/mm/paging.c
> >> +++ b/xen/arch/x86/mm/paging.c
> >> @@ -842,10 +842,45 @@ int paging_teardown(struct domain *d)
> >>  /* Call once all of the references to the domain have gone away */
> >>  void paging_final_teardown(struct domain *d)
> >>  {
> >> -if ( hap_enabled(d) )
> >> +bool hap = hap_enabled(d);
> >> +
> >> +PAGING_PRINTK("%pd start: total = %u, free = %u, p2m = %u\n",
> >> +  d, d->arch.paging.total_pages,
> >> +  d->arch.paging.free_pages, d->arch.paging.p2m_pages);
> >> +
> >> +if ( hap )
> >>  hap_final_teardown(d);
> >> +
> >> +/*
> >> + * Remove remaining paging memory.  This can be nonzero on certain 
> >> error
> >> + * paths.
> >> + */
> >> +if ( d->arch.paging.total_pages )
> >> +{
> >> +if ( hap )
> >> +hap_teardown(d, NULL);
> >> +else
> >> +shadow_teardown(d, NULL);
> > 
> > For a logical PoV, shouldn't hap_teardown() be called before
> > hap_final_teardown()?
> 
> Yes and no: The meaning of "final" has changed - previously it meant "the
> final parts of tearing down" while now it means "the parts of tearing
> down which must be done during final cleanup". I can't think of a better
> name, so I left "hap_final_teardown" as it was.
> 
> > Also hap_final_teardown() already contains a call to hap_teardown() if
> > total_pages != 0, so this is just redundant in the HAP case?
> 
> Well, like in shadow_final_teardown() there was such a call prior to this
> change, but there's none left now.
> 
> > Maybe we want to pull that hap_teardown() out of hap_final_teardown()
> 
> That's what I'm doing here.

Oh, sorry, I've missed that chunk.  Then:

Reviewed-by: Roger Pau Monné 

Thanks, Roger.



[PATCH v2 0/2] deal with GOT stuff for RISC-V

2023-03-16 Thread Oleksii Kurochko
The patch series introduces things to deal with GOT stuff whichwas faced
during the work on [1].

Initially, the issue was with 'la' pseudo instruction, which transformed to
'aupic/l{w|d} GOT' instead of 'auipc/addi'.
The transformation dependson .option {nopic, pic} directive or compiler flags.

Right now, 'la' transforms to 'auipc/l{w|d}', which in case of cpu0_boot_stack[]
will lead to the usage of _GLOBAL_OFFSET_TABLE_and addresses inside GOT
sections will be relative to linker time addresses.

At least there are two reasons for that:
1. GCC compiler used in RISCV64 container is compiled with
  --enable-default-pie flag.
2. GCC spec file for the RISC-V architecture by default enabled -fpic:
   [user@49295ae49cbe build]$ riscv64-linux-gnu-gcc -dumpspecs | grep -i pic
--traditional-format %(subtarget_asm_debugging_spec) %{fno-pie|fno- 
PIE|fno-pic|fno-PIC:;:-fpic} %{march=*} %{mabi=*} %{mno-relax} %{mbig- endian} 
%{mlittle-endian} %(subtarget_asm_spec)%{misa-spec=*}
  which means that -fpic is enabled if none of the following options are
  present on the command line: fno-pie, -fno-PIE, -fno-pic, -fno-PIC

To resolve that, it was added EMBEDDED_EXTRA_CFLAGS to RISCV's CFLAGS as it was
done for other architecture.

To catch use cases when GOT things will be produced by something was createthe
second patch of the patch series:
[xen/riscv: add explicit check that.got{.plt} is empty] which add .got&.got.plt
sections to xen.lds.S and alsoadds asserts to check that the mentioned sections
are empty otherwise, it will be produced a compilation error with the message
that the sections aren't empty.

[1]:
https://lore.kernel.org/xen-devel/22c46432-e940-914e-53c2-2913607be...@suse.com/T/#t

---
Changes in V2:
 * The patch [1] was refactored and lead to the patch [xen/riscv: add
   EMBEDDED_EXTRA_CFLAGS to CFLAGS].
 * In addition to the patch [1] was created another patch [xen/riscv: add 
explicit
   check that .got{.plt} is empty] to be sure that .got{.plt} sections
   weren't produced.
---
Oleksii Kurochko (2):
  xen/riscv: add EMBEDDED_EXTRA_CFLAGS to CFLAGS
  xen/riscv: add explicit check that .got{.plt} is empty

 xen/arch/riscv/arch.mk   |  2 ++
 xen/arch/riscv/xen.lds.S | 13 +
 2 files changed, 15 insertions(+)

-- 
2.39.2




[PATCH v2 2/2] xen/riscv: add explicit check that .got{.plt} is empty

2023-03-16 Thread Oleksii Kurochko
The GOT sections usage should be avoided in the hypervisor
so to catch such use cases earlier when GOT things are
produced the patch introduces .got and .got.plt sections
and adds asserts that they're empty.

The sections won't be created until they remain
empty otherwise the asserts would cause early failure.

Signed-off-by: Oleksii Kurochko 
---
Changes in V2:
 * the patch was introduced in patch series v2.
---
 xen/arch/riscv/xen.lds.S | 13 +
 1 file changed, 13 insertions(+)

diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index ca57cce75c..f299ea8422 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -1,3 +1,4 @@
+#include 
 #include 
 
 #undef ENTRY
@@ -123,6 +124,15 @@ SECTIONS
 *(SORT(.init_array.*))
 __ctors_end = .;
 } :text
+
+.got : {
+*(.got)
+} : text
+
+.got.plt : {
+*(.got.plt)
+} : text
+
 . = ALIGN(POINTER_ALIGN);
 __init_end = .;
 
@@ -156,3 +166,6 @@ SECTIONS
 
 ELF_DETAILS_SECTIONS
 }
+
+ASSERT(!SIZEOF(.got),  ".got non-empty")
+ASSERT(!SIZEOF(.got.plt),  ".got.plt non-empty")
-- 
2.39.2




[PATCH v2 1/2] xen/riscv: add EMBEDDED_EXTRA_CFLAGS to CFLAGS

2023-03-16 Thread Oleksii Kurochko
The patch is needed to keep all address of cpu0_boot_stack
PC-relative.

Pseudoinstruction 'la' can be transformed to 'auipc/addi' or
'auipc/l{w|d}'. It depends on the .option directive: nopic and pic
or compiler flags.

Right now, 'la' transforms to 'auipc/l{w|d}', which in case of
cpu0_boot_stack[] will lead to the usage of _GLOBAL_OFFSET_TABLE_
where all addresses will be without counting that it might happen
that linker address != load address ( so addresses inside got
sections will be relative to linker time ).

It happens becuase the compiler from riscv64 docker compiled with
--enable-default-pie:
  [user@49295ae49cbe build]$ riscv64-linux-gnu-gcc -v
  Using built-in specs.
  COLLECT_GCC=riscv64-linux-gnu-gcc
  COLLECT_LTO_WRAPPER=/usr/lib/gcc/riscv64-linux-gnu/12.2.0/lto-wrapper
  Target: riscv64-linux-gnu
  Configured with: /build/riscv64-linux-gnu-gcc/src/gcc-12.2.0/configure
  --prefix=/usr --program-prefix=riscv64-linux-gnu- --with-local-
  prefix=/usr/riscv64-linux-gnu --with-sysroot=/usr/riscv64-linux-gnu --
  with-build-sysroot=/usr/riscv64-linux-gnu --libdir=/usr/lib --
  libexecdir=/usr/lib --target=riscv64-linux-gnu --host=x86_64-pc-linux-
  gnu --build=x86_64-pc-linux-gnu --with-system-zlib --with-isl --with-
  linker-hash-style=gnu --disable-nls --disable-libunwind-exceptions --
  disable-libstdcxx-pch --disable-libssp --disable-multilib --disable-
  werror --enable-languages=c,c++ --enable-shared --enable-threads=posix
  --enable-__cxa_atexit --enable-clocale=gnu --enable-gnu-unique-object -
  -enable-linker-build-id --enable-lto --enable-plugin --enable-install-
  libiberty --enable-gnu-indirect-function --enable-default-pie --enable-
  checking=release
  Thread model: posix
  Supported LTO compression algorithms: zlib zstd
  gcc version 12.2.0 (GCC)

Looking at gcc spec file for the RISC-V architecture:
  [user@49295ae49cbe build]$ riscv64-linux-gnu-gcc -dumpspecs | grep -i
  pic
  --traditional-format %(subtarget_asm_debugging_spec) %{fno-pie|fno-
  PIE|fno-pic|fno-PIC:;:-fpic} %{march=*} %{mabi=*} %{mno-relax} %{mbig-
  endian} %{mlittle-endian} %(subtarget_asm_spec)%{misa-spec=*}
which means that -fpic is enabled if none of the following options are
present on the command line:
-fno-pie
-fno-PIE
-fno-pic
-fno-PIC

That's the reasons why 'la' is transformed to 'aupic/l{w|d} GOT' and
not be dependent on the toolchain used.

Signed-off-by: Oleksii Kurochko 
---
 Changes in V2:
 * instead of changing 'la' to 'lla' to keep cpu0_boot_stack PC-relative
   it was updated CFLAGS with EMBEDDED_EXTRA_CFLAGS which contains
   -fno-PIE thereby 'la' will be transformed to 'auipc/addi' without
   GOT usage.
 * update the commit message with additional details.
---
 xen/arch/riscv/arch.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 45fe858ee0..7448f759b4 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -1,6 +1,8 @@
 
 # RISCV-specific definitions
 
+$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+
 CFLAGS-$(CONFIG_RISCV_64) += -mabi=lp64
 
 riscv-march-$(CONFIG_RISCV_ISA_RV64G) := rv64g
-- 
2.39.2




[qemu-mainline test] 179657: regressions - trouble: fail/pass/starved

2023-03-16 Thread osstest service owner
flight 179657 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179657/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt  14 guest-start  fail REGR. vs. 179518
 test-amd64-i386-libvirt-xsm  14 guest-start  fail REGR. vs. 179518
 test-amd64-i386-libvirt-pair 25 guest-start/debian   fail REGR. vs. 179518
 test-amd64-amd64-xl-qcow212 debian-di-installfail REGR. vs. 179518
 test-amd64-amd64-libvirt-vhd 12 debian-di-installfail REGR. vs. 179518
 test-amd64-amd64-libvirt-xsm 14 guest-start  fail REGR. vs. 179518
 test-arm64-arm64-libvirt-xsm 14 guest-start  fail REGR. vs. 179518
 test-amd64-i386-libvirt-raw  12 debian-di-installfail REGR. vs. 179518
 test-amd64-amd64-libvirt 14 guest-start  fail REGR. vs. 179518
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install 
fail REGR. vs. 179518
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install 
fail REGR. vs. 179518
 test-arm64-arm64-libvirt-raw 12 debian-di-installfail REGR. vs. 179518
 test-amd64-i386-xl-vhd   12 debian-di-installfail REGR. vs. 179518
 test-arm64-arm64-xl-vhd  12 debian-di-installfail REGR. vs. 179518
 test-amd64-amd64-libvirt-pair 25 guest-start/debian  fail REGR. vs. 179518

Tests which are failing intermittently (not blocking):
 test-amd64-i386-pair 11 xen-install/dst_host fail in 179644 pass in 179657
 test-amd64-amd64-qemuu-freebsd12-amd64 22 guest-start.2 fail in 179644 pass in 
179657
 test-amd64-i386-pair 10 xen-install/src_host   fail pass in 179644

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 179518
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 179518
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 179518
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 179518
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 179518
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-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-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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   starved  n/a
 build-armhf-libvirt   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   starved  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   starved  n/a
 build-armhf   2 hosts-allocate   starved  n/a

version targeted for testing:
 qemuu652737c8090eb3792f8b4c4b22ab12d7cc32073f
baseline version:
 qemuu7b0f0aa55fd292fa3489755a3a896e496c51ea86

Last test of basis   179518  2023-03-09 10:37:19 Z7 days
Failing since179526  2023-03-10 01:53:40 Z6 days   11 attempts
Testing same since   179644  2023-03-15 07:53:18 Z1 days2 attempts


People who touched revisions under test:
  Akihiko Odaki 
  Albert Esteve 
  Alex Bennée 
  Alex Williamson 
  Alistair Francis 
  Andreas Schwab 
  Anton Johansson 
  Avihai Horon 
  BALATON Zoltan 
  Bernhard Beschow 
  Carlos López 
  Cédric Le Goater 
  Cédric Le Goater 
  Damien Hedde 
  David Hildenbrand 
  David Woodhouse 
  David Woodhouse 
  Dr. David Alan Gilbert 
  Eug

seabios 1.16.2 release tagged (was: Re: [SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at). 0x1000 to detect Xen

2023-03-16 Thread Gerd Hoffmann
  Hi,

> Ok, we have as of today two changes:
> 
>   kraxel@sirius ~/projects/seabios (master)# git log --oneline rel-1.16.1..
>   ea1b7a073390 xen: require Xen info structure at 0x1000 to detect Xen
>   645a64b4911d usb: fix wrong init of keyboard/mouse's if first interface is 
> not boot protocol
> 
> With no changes since January and no known issues.
> I think we can safely tag the current state as 1.16.2.
> 
> I'll do that next monday (plus qemu pull request) unless
> there are objections until then.

Oops, totally forgot it.  Three days after the deadline, still no
objections raised, so I tagged the release today and uploaded the
source tarball.  qemu pull request for the update is out of the
door too.

take care,
  Gerd




RE: [PATCH] CHANGELOG: Mention xl/libxl SMBIOS support

2023-03-16 Thread Henry Wang
Hi Jason,

> -Original Message-
> From: Jason Andryuk 
> Subject: [PATCH] CHANGELOG: Mention xl/libxl SMBIOS support
> 
> Add an entry for the new xl/libxl SMBIOS support.
> 
> Signed-off-by: Jason Andryuk 

Acked-by: Henry Wang 

Kind regards,
Henry



Re: [PATCH v2 1/5] x86/paging: fold most HAP and shadow final teardown

2023-03-16 Thread Jan Beulich
On 16.03.2023 13:24, Roger Pau Monné wrote:
> On Mon, Jan 09, 2023 at 02:39:19PM +0100, Jan Beulich wrote:
>> HAP does a few things beyond what's common, which are left there at
>> least for now. Common operations, however, are moved to
>> paging_final_teardown(), allowing shadow_final_teardown() to go away.
>>
>> While moving (and hence generalizing) the respective SHADOW_PRINTK()
>> drop the logging of total_pages from the 2nd instance - the value is
>> necessarily zero after {hap,shadow}_set_allocation() - and shorten the
>> messages, in part accounting for PAGING_PRINTK() logging __func__
>> already.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> The remaining parts of hap_final_teardown() could be moved as well, at
>> the price of a CONFIG_HVM conditional. I wasn't sure whether that was
>> deemed reasonable.
>> ---
>> v2: Shorten PAGING_PRINTK() messages. Adjust comments while being
>> moved.
>>
>> --- a/xen/arch/x86/include/asm/shadow.h
>> +++ b/xen/arch/x86/include/asm/shadow.h
>> @@ -78,9 +78,6 @@ int shadow_domctl(struct domain *d,
>>  void shadow_vcpu_teardown(struct vcpu *v);
>>  void shadow_teardown(struct domain *d, bool *preempted);
>>  
>> -/* Call once all of the references to the domain have gone away */
>> -void shadow_final_teardown(struct domain *d);
>> -
>>  void sh_remove_shadows(struct domain *d, mfn_t gmfn, int fast, int all);
>>  
>>  /* Adjust shadows ready for a guest page to change its type. */
>> --- a/xen/arch/x86/mm/hap/hap.c
>> +++ b/xen/arch/x86/mm/hap/hap.c
>> @@ -268,8 +268,8 @@ static void hap_free(struct domain *d, m
>>  
>>  /*
>>   * For dying domains, actually free the memory here. This way less work 
>> is
>> - * left to hap_final_teardown(), which cannot easily have preemption 
>> checks
>> - * added.
>> + * left to paging_final_teardown(), which cannot easily have preemption
>> + * checks added.
>>   */
>>  if ( unlikely(d->is_dying) )
>>  {
>> @@ -552,18 +552,6 @@ void hap_final_teardown(struct domain *d
>>  for (i = 0; i < MAX_NESTEDP2M; i++) {
>>  p2m_teardown(d->arch.nested_p2m[i], true, NULL);
>>  }
>> -
>> -if ( d->arch.paging.total_pages != 0 )
>> -hap_teardown(d, NULL);
>> -
>> -p2m_teardown(p2m_get_hostp2m(d), true, NULL);
>> -/* Free any memory that the p2m teardown released */
>> -paging_lock(d);
>> -hap_set_allocation(d, 0, NULL);
>> -ASSERT(d->arch.paging.p2m_pages == 0);
>> -ASSERT(d->arch.paging.free_pages == 0);
>> -ASSERT(d->arch.paging.total_pages == 0);
>> -paging_unlock(d);
>>  }
>>  
>>  void hap_vcpu_teardown(struct vcpu *v)
>> --- a/xen/arch/x86/mm/paging.c
>> +++ b/xen/arch/x86/mm/paging.c
>> @@ -842,10 +842,45 @@ int paging_teardown(struct domain *d)
>>  /* Call once all of the references to the domain have gone away */
>>  void paging_final_teardown(struct domain *d)
>>  {
>> -if ( hap_enabled(d) )
>> +bool hap = hap_enabled(d);
>> +
>> +PAGING_PRINTK("%pd start: total = %u, free = %u, p2m = %u\n",
>> +  d, d->arch.paging.total_pages,
>> +  d->arch.paging.free_pages, d->arch.paging.p2m_pages);
>> +
>> +if ( hap )
>>  hap_final_teardown(d);
>> +
>> +/*
>> + * Remove remaining paging memory.  This can be nonzero on certain error
>> + * paths.
>> + */
>> +if ( d->arch.paging.total_pages )
>> +{
>> +if ( hap )
>> +hap_teardown(d, NULL);
>> +else
>> +shadow_teardown(d, NULL);
> 
> For a logical PoV, shouldn't hap_teardown() be called before
> hap_final_teardown()?

Yes and no: The meaning of "final" has changed - previously it meant "the
final parts of tearing down" while now it means "the parts of tearing
down which must be done during final cleanup". I can't think of a better
name, so I left "hap_final_teardown" as it was.

> Also hap_final_teardown() already contains a call to hap_teardown() if
> total_pages != 0, so this is just redundant in the HAP case?

Well, like in shadow_final_teardown() there was such a call prior to this
change, but there's none left now.

> Maybe we want to pull that hap_teardown() out of hap_final_teardown()

That's what I'm doing here.

> and re-order the logic so hap_teardown() is called before
> hap_final_teardown()?

I'm not convinced re-ordering would be correct; even if it was I wouldn't
want to change order of operations here. Instead I want to limit the
changes to just the folding of duplicate (with shadow) code.

Jan



Re: [PATCH v4 0/3] libxl smbios support

2023-03-16 Thread Andrew Cooper
On 16/03/2023 7:53 am, Jan Beulich wrote:
> On 06.03.2023 21:40, Jason Andryuk wrote:
>> hvm_xs_strings.h specifies xenstore entries which can be used to set or
>> override smbios strings.  hvmloader has support for reading them, but
>> xl/libxl support is not wired up.  This patches adds a new xl.cfg option
>> and libxl support to write the xenstore strings.
>>
>> The xl syntax looks like:
>> smbios=["bios_vendor=Xen Project","system_version=1.0"]
>>
>> The Go binding generation needed extending to support Arrays inside a
>> KeyedUnion, which is what the first patch does.  The generated go code
>> builds, but it is otherwise untested.
>>
>> There are also oem strings, oem-1..oem-99, that HVM loader supports.
>> xl parse multiple oem strings like smbios=["oem=A,oem=B"], libxl then
>> iterates over them and assigned to the oem-%d entries.  Both xl and
>> libxl check that the 99 string limit isn't exceeded.
>>
>> The rendered man page and html don't have a newline at the end of the
>> new section after patch 2.
>> """
>>battery_device_name=STRING
>>ms_vm_genid="OPTION"
>> """
>>
>> however the txt format is correct:
>> """
>> battery_device_name=STRING
>>
>> ms_vm_genid="OPTION"
>> """
>>
>> It goes away after patch 3 is applied since it adds text about the "oem"
>> option in between the two lines above.  I'm at a loss as to why this is
>> happening.
>>
>> v4 is a rebase and resend of v3.
>>
>> Jason Andryuk (3):
>>   golang/xenlight: Extend KeyedUnion to support Arrays
>>   xl/libxl: Add ability to specify SMBIOS strings
>>   xl/libxl: Add OEM string support to smbios
>>
>>  docs/man/xl.cfg.5.pod.in | 49 +++
>>  tools/golang/xenlight/gengotypes.py  | 41 +---
>>  tools/golang/xenlight/helpers.gen.go | 51 
>>  tools/golang/xenlight/types.gen.go   | 28 +++
>>  tools/include/libxl.h|  5 ++
>>  tools/libs/light/libxl_dom.c | 33 +
>>  tools/libs/light/libxl_types.idl | 27 +++
>>  tools/xl/xl_parse.c  | 71 +++-
>>  8 files changed, 288 insertions(+), 17 deletions(-)
> Is this work something that's worth mentioning in CHANGELOG.md?

Yes.  Thanks for remembering - I'd forgotten.

~Andrew



Re: [PATCH v2 3/5] x86/paging: move update_paging_modes() hook

2023-03-16 Thread Roger Pau Monné
On Mon, Jan 09, 2023 at 02:40:50PM +0100, Jan Beulich wrote:
> The hook isn't mode dependent, hence it's misplaced in struct
> paging_mode. (Or alternatively I see no reason why the alloc_page() and
> free_page() hooks don't also live there.) Move it to struct
> paging_domain.
> 
> Signed-off-by: Jan Beulich 

Reviewed-by: Roger Pau Monné 

Thanks, Roger.



[PATCH] CHANGELOG: Mention xl/libxl SMBIOS support

2023-03-16 Thread Jason Andryuk
Add an entry for the new xl/libxl SMBIOS support.

Signed-off-by: Jason Andryuk 
---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b116163b62..c978cfd9b6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@ The format is based on [Keep a 
Changelog](https://keepachangelog.com/en/1.0.0/)
  livelocks, instead of crashing the entire server.
- Bus-lock detection, used by Xen to mitigate (by rate-limiting) the system
  wide impact of a guest misusing atomic instructions.
+ - xl/libxl can customize SMBIOS strings for HVM guests.
 
 ## 
[4.17.0](https://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.17.0) 
- 2022-12-12
 
-- 
2.39.2




Re: [PATCH v2 2/5] x86/paging: drop set-allocation from final-teardown

2023-03-16 Thread Roger Pau Monné
On Mon, Jan 09, 2023 at 02:39:52PM +0100, Jan Beulich wrote:
> The fixes for XSA-410 have arranged for P2M pages being freed by P2M
> code to be properly freed directly, rather than being put back on the
> paging pool list. Therefore whatever p2m_teardown() may return will no
> longer need taking care of here. Drop the code, leaving the assertions
> in place and adding "total" back to the PAGING_PRINTK() message.
> 
> With merely the (optional) log message and the assertions left, there's
> really no point anymore to hold the paging lock there, so drop that too.
> 
> Requested-by: Andrew Cooper 
> Signed-off-by: Jan Beulich 

Reviewed-by: Roger Pau Monné 

> ---
> The remaining parts of hap_final_teardown() could be moved as well, at
> the price of a CONFIG_HVM conditional. I wasn't sure whether that was
> deemed reasonable.

I think it's cleaner to leave them as-is.

Thanks, Roger.



Re: [PATCH v4 0/3] libxl smbios support

2023-03-16 Thread Jason Andryuk
On Thu, Mar 16, 2023 at 3:53 AM Jan Beulich  wrote:
> Is this work something that's worth mentioning in CHANGELOG.md?

Sure, I'll add an entry.

Thanks,
Jason



Re: [XEN PATCH v2] x86/monitor: Add new monitor event to catch I/O instructions

2023-03-16 Thread Tamas K Lengyel
On Wed, Mar 15, 2023 at 2:55 PM Dmitry Isaykin 
wrote:
>
> Adds monitor support for I/O instructions.
>
> Signed-off-by: Dmitry Isaykin 
> Signed-off-by: Anton Belousov 

Acked-by: Tamas K Lengyel 


Re: [PATCH v2 1/5] x86/paging: fold most HAP and shadow final teardown

2023-03-16 Thread Roger Pau Monné
On Mon, Jan 09, 2023 at 02:39:19PM +0100, Jan Beulich wrote:
> HAP does a few things beyond what's common, which are left there at
> least for now. Common operations, however, are moved to
> paging_final_teardown(), allowing shadow_final_teardown() to go away.
> 
> While moving (and hence generalizing) the respective SHADOW_PRINTK()
> drop the logging of total_pages from the 2nd instance - the value is
> necessarily zero after {hap,shadow}_set_allocation() - and shorten the
> messages, in part accounting for PAGING_PRINTK() logging __func__
> already.
> 
> Signed-off-by: Jan Beulich 
> ---
> The remaining parts of hap_final_teardown() could be moved as well, at
> the price of a CONFIG_HVM conditional. I wasn't sure whether that was
> deemed reasonable.
> ---
> v2: Shorten PAGING_PRINTK() messages. Adjust comments while being
> moved.
> 
> --- a/xen/arch/x86/include/asm/shadow.h
> +++ b/xen/arch/x86/include/asm/shadow.h
> @@ -78,9 +78,6 @@ int shadow_domctl(struct domain *d,
>  void shadow_vcpu_teardown(struct vcpu *v);
>  void shadow_teardown(struct domain *d, bool *preempted);
>  
> -/* Call once all of the references to the domain have gone away */
> -void shadow_final_teardown(struct domain *d);
> -
>  void sh_remove_shadows(struct domain *d, mfn_t gmfn, int fast, int all);
>  
>  /* Adjust shadows ready for a guest page to change its type. */
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -268,8 +268,8 @@ static void hap_free(struct domain *d, m
>  
>  /*
>   * For dying domains, actually free the memory here. This way less work 
> is
> - * left to hap_final_teardown(), which cannot easily have preemption 
> checks
> - * added.
> + * left to paging_final_teardown(), which cannot easily have preemption
> + * checks added.
>   */
>  if ( unlikely(d->is_dying) )
>  {
> @@ -552,18 +552,6 @@ void hap_final_teardown(struct domain *d
>  for (i = 0; i < MAX_NESTEDP2M; i++) {
>  p2m_teardown(d->arch.nested_p2m[i], true, NULL);
>  }
> -
> -if ( d->arch.paging.total_pages != 0 )
> -hap_teardown(d, NULL);
> -
> -p2m_teardown(p2m_get_hostp2m(d), true, NULL);
> -/* Free any memory that the p2m teardown released */
> -paging_lock(d);
> -hap_set_allocation(d, 0, NULL);
> -ASSERT(d->arch.paging.p2m_pages == 0);
> -ASSERT(d->arch.paging.free_pages == 0);
> -ASSERT(d->arch.paging.total_pages == 0);
> -paging_unlock(d);
>  }
>  
>  void hap_vcpu_teardown(struct vcpu *v)
> --- a/xen/arch/x86/mm/paging.c
> +++ b/xen/arch/x86/mm/paging.c
> @@ -842,10 +842,45 @@ int paging_teardown(struct domain *d)
>  /* Call once all of the references to the domain have gone away */
>  void paging_final_teardown(struct domain *d)
>  {
> -if ( hap_enabled(d) )
> +bool hap = hap_enabled(d);
> +
> +PAGING_PRINTK("%pd start: total = %u, free = %u, p2m = %u\n",
> +  d, d->arch.paging.total_pages,
> +  d->arch.paging.free_pages, d->arch.paging.p2m_pages);
> +
> +if ( hap )
>  hap_final_teardown(d);
> +
> +/*
> + * Remove remaining paging memory.  This can be nonzero on certain error
> + * paths.
> + */
> +if ( d->arch.paging.total_pages )
> +{
> +if ( hap )
> +hap_teardown(d, NULL);
> +else
> +shadow_teardown(d, NULL);

For a logical PoV, shouldn't hap_teardown() be called before
hap_final_teardown()?

Also hap_final_teardown() already contains a call to hap_teardown() if
total_pages != 0, so this is just redundant in the HAP case?

Maybe we want to pull that hap_teardown() out of hap_final_teardown()
and re-order the logic so hap_teardown() is called before
hap_final_teardown()?

Thanks, Roger.



[libvirt test] 179665: regressions - trouble: blocked/fail/pass/starved

2023-03-16 Thread osstest service owner
flight 179665 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/179665/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail REGR. vs. 179640
 build-arm64-pvops 6 kernel-build fail REGR. vs. 179640

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 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  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 build-armhf-libvirt   1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   starved  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   starved  n/a
 build-armhf   2 hosts-allocate   starved  n/a

version targeted for testing:
 libvirt  3916df52a4a9174acc7812d15d8726651768f207
baseline version:
 libvirt  8386242bd0f6c1cb242f9c711e2ef864bf114d0d

Last test of basis   179640  2023-03-15 04:19:43 Z1 days
Testing same since   179665  2023-03-16 04:18:51 Z0 days1 attempts


People who touched revisions under test:
  Jiri Denemark 
  Ján Tomko 
  Michal Privoznik 
  Tim Wiederhake 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  starved 
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  starved 
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopsfail
 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 blocked 
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt blocked 
 test-armhf-armhf-libvirt starved 
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   blocked 
 test-armhf-armhf-libvirt-qcow2   starved 
 test-arm64-arm64-libvirt-raw blocked 
 test-armhf-armhf-libvirt-raw starved 
 test-amd64-i386-libvirt-raw  pass
 test-amd64-amd64-libvirt-vhd fail



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

Tes

Re: [PATCH 5/7] tools: Use -s for python shebangs

2023-03-16 Thread Anthony PERARD
On Tue, Mar 14, 2023 at 02:50:48PM +, Andrew Cooper wrote:
> On 14/03/2023 2:15 pm, Andrew Cooper wrote:
> > diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
> > index 29ad0513212f..04b3995cc0f6 100644
> > --- a/tools/pygrub/Makefile
> > +++ b/tools/pygrub/Makefile
> > @@ -7,7 +7,7 @@ PY_LDFLAGS = $(SHLIB_LDFLAGS) $(APPEND_LDFLAGS)
> >  INSTALL_LOG = build/installed_files.txt
> >  
> >  setup.py = CC="$(CC)" CFLAGS="$(PY_CFLAGS)" LDSHARED="$(CC)" 
> > LDFLAGS="$(PY_LDFLAGS)" \
> > -   $(PYTHON) setup.py
> > +   $(PYTHON) setup.py --executable="$(PYTHON_PATH) -s"
> 
> /sigh
> 
> CI has gone a wall of red to this.  Apparently --executable is only
> known by setuputils, not distutils.
> 
> Which is a problem because it means the main pygrub executable won't get
> a corrected shebang, as it doesn't pass through install-wrap.
> 
> In the short term, I could fix that by moving the main pygrub binary out
> of setup.py and do it manually.  Thoughts?

Or do the fixing in "setup.py", https://stackoverflow.com/a/17099342 :-)
But that probably more work that necessary.
Doing the binary installation in the Makefile sounds fine.

Thanks,

-- 
Anthony PERARD



[PATCH v2] vpci/msix: handle accesses adjacent to the MSI-X table

2023-03-16 Thread Roger Pau Monne
The handling of the MSI-X table accesses by Xen requires that any
pages part of the MSI-X related tables are not mapped into the domain
physmap.  As a result, any device registers in the same pages as the
start or the end of the MSIX or PBA tables is not currently
accessible, as the accesses are just dropped.

Note the spec forbids such placing of registers, as the MSIX and PBA
tables must be 4K isolated from any other registers:

"If a Base Address register that maps address space for the MSI-X
Table or MSI-X PBA also maps other usable address space that is not
associated with MSI-X structures, locations (e.g., for CSRs) used in
the other address space must not share any naturally aligned 4-KB
address range with one where either MSI-X structure resides."

Yet the 'Intel Wi-Fi 6 AX201' device on one of my boxes has registers
in the same page as the MSIX tables, and thus won't work on a PVH dom0
without this fix.

In order to cope with the behavior passthrough any accesses that fall
on the same page as the MSIX tables (but don't fall in between) it to
the underlying hardware.  Such forwarding also takes care of the PBA
accesses, so it allows to remove the code doing this handling in
msix_{read,write}.  Note that as a result accesses to the PBA array
are no longer limited to 4 and 8 byte sizes, there's no access size
restriction for PBA accesses documented in the specification.

Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - Properly handle the PBA also.
 - Merge the handlers for adjacent writes into the existing MSIX table
   ones.
---
 xen/drivers/vpci/msix.c | 329 +---
 xen/drivers/vpci/vpci.c |   7 +-
 xen/include/xen/vpci.h  |   8 +-
 3 files changed, 255 insertions(+), 89 deletions(-)

diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index bea0cc7aed..060b74d62b 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -27,6 +27,11 @@
 ((addr) >= vmsix_table_addr(vpci, nr) &&  \
  (addr) < vmsix_table_addr(vpci, nr) + vmsix_table_size(vpci, nr))
 
+#define VMSIX_ADDR_SAME_PAGE(addr, vpci, nr)  \
+(PFN_DOWN(addr) >= PFN_DOWN(vmsix_table_addr(vpci, nr)) &&\
+ PFN_DOWN((addr)) < PFN_UP(vmsix_table_addr(vpci, nr) +   \
+   vmsix_table_size(vpci, nr) - 1))
+
 static uint32_t cf_check control_read(
 const struct pci_dev *pdev, unsigned int reg, void *data)
 {
@@ -149,7 +154,7 @@ static struct vpci_msix *msix_find(const struct domain *d, 
unsigned long addr)
 
 for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
 if ( bars[msix->tables[i] & PCI_MSIX_BIRMASK].enabled &&
- VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, i) )
+ VMSIX_ADDR_SAME_PAGE(addr, msix->pdev->vpci, i) )
 return msix;
 }
 
@@ -182,93 +187,201 @@ static struct vpci_msix_entry *get_entry(struct 
vpci_msix *msix,
 return &msix->entries[(addr - start) / PCI_MSIX_ENTRY_SIZE];
 }
 
-static void __iomem *get_pba(struct vpci *vpci)
+static void __iomem *get_table(struct vpci *vpci, unsigned int slot)
 {
 struct vpci_msix *msix = vpci->msix;
 /*
- * PBA will only be unmapped when the device is deassigned, so access it
- * without holding the vpci lock.
+ * Regions will only be unmapped when the device is deassigned, so access
+ * them without holding the vpci lock.
  */
-void __iomem *pba = read_atomic(&msix->pba);
+void __iomem *table = read_atomic(&msix->table[slot]);
+paddr_t addr = 0;
 
-if ( likely(pba) )
-return pba;
+if ( likely(table) )
+return table;
 
-pba = ioremap(vmsix_table_addr(vpci, VPCI_MSIX_PBA),
-  vmsix_table_size(vpci, VPCI_MSIX_PBA));
-if ( !pba )
-return read_atomic(&msix->pba);
+switch ( slot )
+{
+case VPCI_MSIX_TBL_TAIL:
+addr = vmsix_table_size(vpci, VPCI_MSIX_TABLE);
+fallthrough;
+case VPCI_MSIX_TBL_HEAD:
+addr += vmsix_table_addr(vpci, VPCI_MSIX_TABLE);
+break;
+
+case VPCI_MSIX_PBA_TAIL:
+addr = vmsix_table_size(vpci, VPCI_MSIX_PBA);
+fallthrough;
+case VPCI_MSIX_PBA_HEAD:
+addr += vmsix_table_addr(vpci, VPCI_MSIX_PBA);
+break;
+}
+
+table = ioremap(round_pgdown(addr), PAGE_SIZE);
+if ( !table )
+return read_atomic(&msix->table[slot]);
 
 spin_lock(&vpci->lock);
-if ( !msix->pba )
+if ( !msix->table[slot] )
 {
-write_atomic(&msix->pba, pba);
+write_atomic(&msix->table[slot], table);
 spin_unlock(&vpci->lock);
 }
 else
 {
 spin_unlock(&vpci->lock);
-iounmap(pba);
+iounmap(table);
 }
 
-return read_atomic(&msix->pba);
+return read_atomic(&msix->table[slot]);
 }
 
-static int cf_check msix_read(
-struct vcpu *v, unsigned long addr, unsigned int len, unsigned long *da

Re: [PATCH 2/7] tools/misc: Drop xencons

2023-03-16 Thread Andrew Cooper
On 16/03/2023 11:33 am, Anthony PERARD wrote:
> On Tue, Mar 14, 2023 at 02:15:15PM +, Andrew Cooper wrote:
>> This is a python script which has it's shebang modified by be python3, but
>> was never converted to be python3 compatible.
> Not quite, that's the original shebang ;-)

What I meant was that INSTALL_PYTHON_PROG turns it into a python3 shebang.

I'll see if I can think of some slightly clearer wording.

>
>> The most recent reference I can find to this script (which isn't incidental
>> adjustments in the makefile) is from the Xen book, fileish 561e30b80402 which
>> says
>>
>>   %%   Alternatively, if the
>>   %% Xen machine is connected to a serial-port server then we supply a
>>   %% dumb TCP terminal client, {\tt xencons}.
>>
>> So this a not-invented-here version of telnet.  Delete it.
>>
>> Signed-off-by: Andrew Cooper 
> Acked-by: Anthony PERARD 

Thanks.

~Andrew



Re: [PATCH] move {,vcpu_}show_execution_state() declarations to common header

2023-03-16 Thread Andrew Cooper
On 16/03/2023 11:55 am, Jan Beulich wrote:
> These are used from common code, so their signatures should be
> consistent across architectures. This is achieved / guaranteed easiest
> when their declarations are in a common header.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

> ---
> There's no really good header to put the decls, imo; I wanted to avoid
> the already overcrowded sched.h.

Yeah - lets please not make sched any worse than it already is.

I can't suggest a better location than kernel.h, but I certainly would
like something else if one were to be found.

>  show_execution_state_nonconst(), being
> there solely for dump_execution_state(), could of course live in the
> upcoming xen/bug.h ...
>
> Is there a reason that Arm (still) expands dump_execution_state() to
> WARN()? Without that moving x86's show_execution_state_nonconst()
> definition to common code would also make sense (it could be done
> anyway, but then at the expense of introducing dead code to Arm),
> perhaps (see also above) into the upcoming common/bug.c.

That sounds like something that should be fixed up...



[PATCH] move {,vcpu_}show_execution_state() declarations to common header

2023-03-16 Thread Jan Beulich
These are used from common code, so their signatures should be
consistent across architectures. This is achieved / guaranteed easiest
when their declarations are in a common header.

Signed-off-by: Jan Beulich 
---
There's no really good header to put the decls, imo; I wanted to avoid
the already overcrowded sched.h. show_execution_state_nonconst(), being
there solely for dump_execution_state(), could of course live in the
upcoming xen/bug.h ...

Is there a reason that Arm (still) expands dump_execution_state() to
WARN()? Without that moving x86's show_execution_state_nonconst()
definition to common code would also make sense (it could be done
anyway, but then at the expense of introducing dead code to Arm),
perhaps (see also above) into the upcoming common/bug.c.

--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -232,7 +232,6 @@ struct arch_vcpu
 
 }  __cacheline_aligned;
 
-void vcpu_show_execution_state(struct vcpu *);
 void vcpu_show_registers(const struct vcpu *);
 void vcpu_switch_to_aarch64_mode(struct vcpu *);
 
--- a/xen/arch/arm/include/asm/processor.h
+++ b/xen/arch/arm/include/asm/processor.h
@@ -557,7 +557,6 @@ extern register_t __cpu_logical_map[];
 #ifndef __ASSEMBLY__
 void panic_PAR(uint64_t par);
 
-void show_execution_state(const struct cpu_user_regs *regs);
 /* Debugging functions are declared with external linkage to aid development. 
*/
 void show_registers(const struct cpu_user_regs *regs);
 void show_stack(const struct cpu_user_regs *regs);
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -681,7 +681,6 @@ void domain_cpu_policy_changed(struct do
 bool update_secondary_system_time(struct vcpu *,
   struct vcpu_time_info *);
 
-void vcpu_show_execution_state(struct vcpu *);
 void vcpu_show_registers(const struct vcpu *);
 
 static inline struct vcpu_guest_context *alloc_vcpu_guest_context(void)
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -464,8 +464,6 @@ static always_inline void rep_nop(void)
 void show_code(const struct cpu_user_regs *regs);
 void show_stack_overflow(unsigned int cpu, const struct cpu_user_regs *regs);
 void show_registers(const struct cpu_user_regs *regs);
-void show_execution_state(const struct cpu_user_regs *regs);
-void cf_check show_execution_state_nonconst(struct cpu_user_regs *regs);
 #define dump_execution_state() \
 run_in_exception_handler(show_execution_state_nonconst)
 void show_page_walk(unsigned long addr);
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -107,5 +107,12 @@ bool_t is_active_kernel_text(unsigned lo
 extern const char xen_config_data[];
 extern const unsigned int xen_config_data_size;
 
+struct cpu_user_regs;
+struct vcpu;
+
+void show_execution_state(const struct cpu_user_regs *regs);
+void cf_check show_execution_state_nonconst(struct cpu_user_regs *regs);
+void vcpu_show_execution_state(struct vcpu *);
+
 #endif /* _LINUX_KERNEL_H */
 



Re: [PATCH 4/7] tools/pygrub: Factor out common setup.py parts

2023-03-16 Thread Anthony PERARD
On Tue, Mar 14, 2023 at 02:15:17PM +, Andrew Cooper wrote:
> ... to mirror the tools/python side in c/s 2b8314a3c354.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD



Re: [PATCH 3/7] tools: Delete trailing whitespace in python scripts

2023-03-16 Thread Anthony PERARD
On Tue, Mar 14, 2023 at 02:15:16PM +, Andrew Cooper wrote:
> No functional change.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD



  1   2   >