[linux-linus test] 186030: tolerable FAIL - PUSHED
flight 186030 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/186030/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-credit2 8 xen-boot fail in 186023 pass in 186030 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail in 186023 pass in 186030 test-armhf-armhf-xl-qcow2 8 xen-boot fail pass in 186023 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-arndale 8 xen-bootfail in 186023 like 186010 test-armhf-armhf-xl-multivcpu 8 xen-boot fail in 186023 like 186010 test-armhf-armhf-xl 8 xen-bootfail in 186023 like 186010 test-armhf-armhf-xl-raw 8 xen-bootfail in 186023 like 186010 test-armhf-armhf-examine 8 reboot fail in 186023 like 186018 test-armhf-armhf-xl-rtds15 migrate-support-check fail in 186023 never pass test-armhf-armhf-xl-rtds 16 saverestore-support-check fail in 186023 never pass test-armhf-armhf-xl-qcow2 14 migrate-support-check fail in 186023 never pass test-armhf-armhf-xl-qcow2 15 saverestore-support-check fail in 186023 never pass test-armhf-armhf-xl-rtds 8 xen-boot fail like 186010 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 186018 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 186018 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 186018 test-armhf-armhf-xl-credit1 8 xen-boot fail like 186018 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 186018 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 186018 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 186018 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 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-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-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-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-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-armhf-armhf-xl-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-raw 15 saverestore-support-checkfail never pass version targeted for testing: linuxea5f6ad9ad9645733b72ab53a98e719b460d36a6 baseline version: linux3c999d1ae3c75991902a1a7dad0cb62c2a3008b4 Last test of basis 186018 2024-05-16 15:33:02 Z1 days Testing same since 186023 2024-05-17 01:13:59 Z1 days2 attempts
Re: [PATCH 06/12] RFC: automation: Add linux stubdom build and smoke test
On Fri, May 17, 2024 at 05:40:52PM -0700, Stefano Stabellini wrote: > On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote: > > Add minimal linux-stubdom smoke test. It starts a simple HVM with > > linux-stubdom. The actual stubdom implementation is taken from Qubes OS > > and then stripped off Qubes-specific code. In particular, the remaining > > code does _not_ support: > > - direct kernel boot (implemented by relaying on specific guest disk > >laying in Qubes OS) > > - graphical console (used Qubes GUI agent injected into > >stubdomain's qemu) > > - audio input/output (used Qubes audio agent inside stubdomain) > > - USB passthrough (used qrexec <-> usbip proxy inside stubdomain) > > - setting up DHCP server (assumes guest addressing used in Qubes OS) > > > > For this smoke test, the relevant part is missing direct kernel boot, as > > that's used in other smoke tests. Solve this by preparing disk image > > with proper bootloader (grub) installed. Since the test script is > > running on arm64 to control x86_64 box, it cannot (easily) install grub > > directly. For this reason, prepare bootsector as part of the Xen build > > (which runs on x86_64) and then prepend do the disk image during the > > test (and adjust partitions table afterwards). > > I am not an expert on this, but do you think it would be possible to use > network boot and tftp instead of grub on emulated disk? That would not > require us to build neither /grub-core.img nor build_domU_disk(). Honestly, I don't know. I guess I'd need at least dnsmasq in dom0, and also iPXE for the domU (if not built already?). I can try for this test. But a later test (the PCI one) connects a network card and dom0 can't really setup own DHCP on that network. Additionally combining this with vif network for PXE might be confusing down the road. > I am trying to avoid grub-core.img and disk.img because I think direct > kernel boot or network boot are easier to maintain and more similar to > the other tests. If you see the ARM tests, they all use tftp boot. The ARM ones boot as dom0less, where there is only one boot mode for the system to start in. Here, we have two: xen+dom0 (which already does network boot), and then domU started by dom0. The latter would need either a separate DHCP server on a separate network (vif interface in dom0 should be fine), or some other way to separate dom0/domU boot mode. That said, the stubdom used in Qubes does support direct kernel boot. It is removed from this version, because it relies on specific disk layout (it reserves xvdd for this purpose). But I do want to bring this capability to the upstream version too at some point. > > Signed-off-by: Marek Marczykowski-Górecki > > --- > > The test is implemented using hardware runner, because some of the > > further tests will require it (for example PCI passthrough with > > stubdomain). But if there is strong desire to have stubdomain tested > > inside qemu tests (to be included in patchew runs), it is probably an > > option for this basic smoke test. > > Thanks for this amazing work. This is a great start, we can see how to > create more tests after merging this one. > > > > For now I'm keeping stubdomain code (build and glue scripts) in separate > > repository on my github account. This is far from ideal. What would be > > preferred option? New repository on xenbits? Or add directly into > > xen.git (stubdom directory)? Honestly, I'd rather avoid the latter, as > > from packager point of view those are mostly separate beings (similar to > > qemu, where many use distribution-provide one instead of the one bundled > > with Xen) and it's convenient to not need to rebuild stubdomain on every > > hypervisor change (like a security patch). > > My suggestion is to create repositories under gitlab.com/xen-project gitlab.com/xen-project/stubdom-dm-linux ? Initially I can create the repository under people/marmarek/. Is there any preference regarding git history? I see two options: 1. Preserve the current history, where there is a lot of qubes-specific work and on top a bunch of commits making it not qubes-specific (this is what is there now). 2. Start with fresh history and reference original repository (and the commit id) in the initial commit message. > > Another topic is QEMU version inside stubdomain. It needs to be a > > separate build due to vastly different configure options, so I cannot > > reuse the qemu binary built for dom0 (or distribution-provided one if > > Xen is configured to use it). But also, at this moment qemu for > > stubdomain needs few extra patches that are not upstream yet. > > What should be the proper solution here (after upstreaming all the > > patches)? > > It is fine to use a different QEMU version. For now, I would suggest to > keep the QEMU patches as patches to be applied, in the new repositories > under gitlab.com/xen-project Ok, makes sense. > > Generally, I try to add tests early, even though there is still some > > work
Re: [PATCH for-4.19? v3 4/6] x86: Make the maximum number of altp2m views configurable
> -ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx); > +ap2m = d->arch.altp2m_p2m[altp2m_idx]; Why is it no longer necessary to use array_access_nospec? Tamas
Re: [PATCH v10 11/14] xen/riscv: introduce vm_event_*() functions
On Fri, May 17, 2024 at 9:55 AM Oleksii Kurochko wrote: > > Signed-off-by: Oleksii Kurochko Acked-by: Tamas K Lengyel
Re: [PATCH v10 07/14] xen/riscv: introduce monitor.h
On Fri, May 17, 2024 at 9:55 AM Oleksii Kurochko wrote: > > Signed-off-by: Oleksii Kurochko Acked-by: Tamas K Lengyel
Re: [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option
> Currently altp2m support provided for VT-d only, so option is dependant on > VMX. No clue what is meant by "support provided for VT-d only". Altp2m has nothing to do with VT-d. It would be more accurate to say it's only implemented for Intel EPT. Tamas
Re: [PATCH 10/12] automation: stubdom test with PCI passthrough
On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote:
> Based on the initial stubdomain test and existing PCI passthrough tests,
> add one that combines both.
> Schedule it on the AMD runner, as it has less tests right now.
>
> Signed-off-by: Marek Marczykowski-Górecki
With the caveat that if we do tftp boot this might have to change:
Acked-by: Stefano Stabellini
> ---
> automation/gitlab-ci/test.yaml | 8
> automation/scripts/qubes-x86-64.sh | 30 +-
> 2 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index e3910f4c1a9f..76cc430ae00f 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -231,6 +231,14 @@ zen3p-pci-hvm-x86-64-gcc-debug:
> - *x86-64-test-needs
> - alpine-3.19-gcc-debug
>
> +zen3p-pci-stubdom-x86-64-gcc-debug:
> + extends: .zen3p-x86-64
> + script:
> +- ./automation/scripts/qubes-x86-64.sh pci-stubdom 2>&1 | tee ${LOGFILE}
> + needs:
> +- *x86-64-test-needs
> +- alpine-3.19-gcc-debug
> +
> qemu-smoke-dom0-arm64-gcc:
>extends: .qemu-arm64
>script:
> diff --git a/automation/scripts/qubes-x86-64.sh
> b/automation/scripts/qubes-x86-64.sh
> index fc73403dbadf..816c16fbab3e 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -98,8 +98,8 @@ ping -c 10 192.168.0.2 || exit 1
> echo \"${passed}\"
> "
>
> -### test: pci-pv, pci-hvm
> -elif [ "${test_variant}" = "pci-pv" ] || [ "${test_variant}" = "pci-hvm" ];
> then
> +### test: pci-pv, pci-hvm, pci-stubdom
> +elif [ "${test_variant}" = "pci-pv" ] || [ "${test_variant}" = "pci-hvm" ]
> || [ "${test_variant}" = "pci-stubdom" ]; then
>
> if [ -z "$PCIDEV" ]; then
> echo "Please set 'PCIDEV' variable with BDF of test network adapter"
> >&2
> @@ -109,15 +109,35 @@ elif [ "${test_variant}" = "pci-pv" ] || [
> "${test_variant}" = "pci-hvm" ]; then
>
> passed="pci test passed"
>
> -domU_config='
> +domain_type="${test_variant#pci-}"
> +if [ "$test_variant" = "pci-stubdom" ]; then
> +domain_type="hvm"
> +domU_config='
> +type = "hvm"
> +disk = [ "/srv/disk.img,format=raw,vdev=xvda" ]
> +device_model_version = "qemu-xen"
> +device_model_stubdomain_override = 1
> +on_reboot = "destroy"
> +# libxl configures vkb backend to be dom0 instead of the stubdomain, defer
> +# changing that until there is consensus what to do about VGA output (VNC)
> +vkb_device = 0
> +'
> +domU_disk_path=/srv/disk.img
> +else
> +domU_config='
> type = "'${test_variant#pci-}'"
> -name = "domU"
> kernel = "/boot/vmlinuz"
> ramdisk = "/boot/initrd-domU"
> extra = "root=/dev/ram0 console=hvc0 earlyprintk=xen"
> +disk = [ ]
> +'
> +fi
> +
> +# common part
> +domU_config="$domU_config"'
> +name = "domU"
> memory = 512
> vif = [ ]
> -disk = [ ]
> pci = [ "'$PCIDEV',seize=1" ]
> on_reboot = "destroy"
> '
> --
> git-series 0.9.1
>
Re: [PATCH 06/12] RFC: automation: Add linux stubdom build and smoke test
On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote: > Add minimal linux-stubdom smoke test. It starts a simple HVM with > linux-stubdom. The actual stubdom implementation is taken from Qubes OS > and then stripped off Qubes-specific code. In particular, the remaining > code does _not_ support: > - direct kernel boot (implemented by relaying on specific guest disk >laying in Qubes OS) > - graphical console (used Qubes GUI agent injected into >stubdomain's qemu) > - audio input/output (used Qubes audio agent inside stubdomain) > - USB passthrough (used qrexec <-> usbip proxy inside stubdomain) > - setting up DHCP server (assumes guest addressing used in Qubes OS) > > For this smoke test, the relevant part is missing direct kernel boot, as > that's used in other smoke tests. Solve this by preparing disk image > with proper bootloader (grub) installed. Since the test script is > running on arm64 to control x86_64 box, it cannot (easily) install grub > directly. For this reason, prepare bootsector as part of the Xen build > (which runs on x86_64) and then prepend do the disk image during the > test (and adjust partitions table afterwards). I am not an expert on this, but do you think it would be possible to use network boot and tftp instead of grub on emulated disk? That would not require us to build neither /grub-core.img nor build_domU_disk(). I am trying to avoid grub-core.img and disk.img because I think direct kernel boot or network boot are easier to maintain and more similar to the other tests. If you see the ARM tests, they all use tftp boot. > Signed-off-by: Marek Marczykowski-Górecki > --- > The test is implemented using hardware runner, because some of the > further tests will require it (for example PCI passthrough with > stubdomain). But if there is strong desire to have stubdomain tested > inside qemu tests (to be included in patchew runs), it is probably an > option for this basic smoke test. Thanks for this amazing work. This is a great start, we can see how to create more tests after merging this one. > For now I'm keeping stubdomain code (build and glue scripts) in separate > repository on my github account. This is far from ideal. What would be > preferred option? New repository on xenbits? Or add directly into > xen.git (stubdom directory)? Honestly, I'd rather avoid the latter, as > from packager point of view those are mostly separate beings (similar to > qemu, where many use distribution-provide one instead of the one bundled > with Xen) and it's convenient to not need to rebuild stubdomain on every > hypervisor change (like a security patch). My suggestion is to create repositories under gitlab.com/xen-project > Another topic is QEMU version inside stubdomain. It needs to be a > separate build due to vastly different configure options, so I cannot > reuse the qemu binary built for dom0 (or distribution-provided one if > Xen is configured to use it). But also, at this moment qemu for > stubdomain needs few extra patches that are not upstream yet. > What should be the proper solution here (after upstreaming all the > patches)? It is fine to use a different QEMU version. For now, I would suggest to keep the QEMU patches as patches to be applied, in the new repositories under gitlab.com/xen-project > Generally, I try to add tests early, even though there is still some > work to do for proper stubdomain integration into upstream Xen, so any > cleanups and future changes (like the CDROM libxl patches by Jason > Andryuk) can be made with more confidence and reduce risk of > regressions. > > The patch is RFC only because of the stubdom repository location. > --- > automation/build/alpine/3.19-arm64v8.dockerfile | 2 +- > automation/build/alpine/3.19.dockerfile | 9 ++- > automation/gitlab-ci/build.yaml | 3 +- > automation/gitlab-ci/test.yaml| 8 +- > automation/scripts/build | 12 ++- > automation/scripts/qubes-x86-64.sh| 87 +++- > automation/tests-artifacts/alpine/3.19.dockerfile | 6 +- > 7 files changed, 123 insertions(+), 4 deletions(-) > > diff --git a/automation/build/alpine/3.19-arm64v8.dockerfile > b/automation/build/alpine/3.19-arm64v8.dockerfile > index 158cf465a9ff..12810f87ecc6 100644 > --- a/automation/build/alpine/3.19-arm64v8.dockerfile > +++ b/automation/build/alpine/3.19-arm64v8.dockerfile > @@ -47,3 +47,5 @@ RUN apk --no-cache add \ ># qubes test deps >openssh-client \ >fakeroot \ > + sfdisk \ > + e2fsprogs \ > diff --git a/automation/build/alpine/3.19.dockerfile > b/automation/build/alpine/3.19.dockerfile > index 0be6d7c85fe7..108284613987 100644 > --- a/automation/build/alpine/3.19.dockerfile > +++ b/automation/build/alpine/3.19.dockerfile > @@ -49,3 +49,12 @@ RUN apk --no-cache add \ >pixman-dev \ ># livepatch-tools deps >elfutils-dev \ > + # stubdom deps > + dracut-core \ > + quilt \ > + gnupg \ > +
[linux-6.1 test] 186028: tolerable FAIL - PUSHED
flight 186028 linux-6.1 real [real] http://logs.test-lab.xenproject.org/osstest/logs/186028/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 185901 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 185901 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 185901 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 185901 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 185901 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 185901 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-qcow214 migrate-support-checkfail never pass test-armhf-armhf-xl-qcow215 saverestore-support-checkfail never pass version targeted for testing: linux4078fa637fcd80c8487680ec2e4ef7c58308e9aa baseline version: linux909ba1f1b4146de529469910c1bd0b1248964536 Last test of basis 185901 2024-05-02 15:12:21 Z 15 days Testing same since 186028 2024-05-17 10:16:47 Z0 days1 attempts People who touched revisions under test: Adam Goldman Al Viro Alan Stern Aleksa Savic Alex Deucher Alexander Gordeev Alexander Potapenko Alexander Stein Alexander Usyskin Alexandra Winter Alexei Starovoitov Allen Pais Aman Dhoot Amit Sunil Dhamne Anand Jain Andrea Righi Andrea Righi # non-hostprogs Andreas Gruenbacher Andrei Matei Andrew Donnellan Andrew Morton Andrew Price Andrey Ryabinin Andrii Nakryiko Andy Shevchenko AngeloGioacchino Del Regno Anton Protopopov Arjan van de Ven Arnaldo Carvalho de Melo Arnd Bergmann Asahi Lina Asbjørn Sloth Tønnesen Aurabindo
Re: [PATCH 03/12] automation: switch to alpine:3.19
On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote: > Alpine 3.19 is needed for upcoming stubdomain tests, as linux stubdomain > build requires dracut-core package (dracut-install tool specifically) > which isn't available in 3.18. While technically it will be needed only > in the x86_64 builds, switch Alpine version everywhere for uniformity. > Note this bumps kernel version requirement on docker runners - > dracut-install uses faccessat2() syscall which was introduced in Linux > 5.8. > > Signed-off-by: Marek Marczykowski-Górecki Reviewed-by: Stefano Stabellini
Re: [PATCH 08/12] automation: update kernel for x86 tests
On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote:
> Update 6.1.x kernel to the latest version in this branch. This is
> especially needed to include MSI-X related fixes for stubdomain
> ("xen-pciback: Consider INTx disabled when MSI/MSI-X is enabled").
>
> Signed-off-by: Marek Marczykowski-Górecki
Reviewed-by: Stefano Stabellini
> ---
> automation/gitlab-ci/build.yaml | 4 +-
> automation/gitlab-ci/test.yaml | 2 +-
> automation/tests-artifacts/kernel/6.1.19.dockerfile | 40 +--
> automation/tests-artifacts/kernel/6.1.90.dockerfile | 40 ++-
> 4 files changed, 43 insertions(+), 43 deletions(-)
> delete mode 100644 automation/tests-artifacts/kernel/6.1.19.dockerfile
> create mode 100644 automation/tests-artifacts/kernel/6.1.90.dockerfile
>
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 783a0687ba34..9b9e5464f179 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -331,9 +331,9 @@ alpine-3.19-rootfs-export:
>tags:
> - x86_64
>
> -kernel-6.1.19-export:
> +kernel-6.1.90-export:
>extends: .test-jobs-artifact-common
> - image: registry.gitlab.com/xen-project/xen/tests-artifacts/kernel:6.1.19
> + image: registry.gitlab.com/xen-project/xen/tests-artifacts/kernel:6.1.90
>script:
> - mkdir binaries && cp /bzImage binaries/bzImage
>artifacts:
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 80d10eb7f476..e3910f4c1a9f 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -12,7 +12,7 @@
>
> .x86-64-test-needs:
>- alpine-3.19-rootfs-export
> - - kernel-6.1.19-export
> + - kernel-6.1.90-export
>
> .qemu-arm64:
>extends: .test-jobs-common
> diff --git a/automation/tests-artifacts/kernel/6.1.19.dockerfile
> b/automation/tests-artifacts/kernel/6.1.19.dockerfile
> deleted file mode 100644
> index 3a4096780d20..
> --- a/automation/tests-artifacts/kernel/6.1.19.dockerfile
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -FROM --platform=linux/amd64 debian:bookworm
> -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"*
> diff --git a/automation/tests-artifacts/kernel/6.1.90.dockerfile
> b/automation/tests-artifacts/kernel/6.1.90.dockerfile
> new file mode 100644
> index ..46cadf02ca78
> --- /dev/null
> +++ b/automation/tests-artifacts/kernel/6.1.90.dockerfile
> @@ -0,0 +1,40 @@
> +FROM --platform=linux/amd64 debian:bookworm
> +LABEL maintainer.name="The Xen Project" \
> + maintainer.email="xen-devel@lists.xenproject.org"
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV LINUX_VERSION=6.1.90
> +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 04/12] automation: increase verbosity of starting a domain
On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote:
> And start collecting qemu log earlier, so it isn't lost in case of a
> timeout during domain startup.
>
> Signed-off-by: Marek Marczykowski-Górecki
Acked-by: Stefano Stabellini
> ---
> automation/scripts/qemu-alpine-x86_64.sh| 2 +-
> automation/scripts/qemu-smoke-dom0-arm32.sh | 2 +-
> automation/scripts/qemu-smoke-dom0-arm64.sh | 2 +-
> automation/scripts/qubes-x86-64.sh | 4 ++--
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/automation/scripts/qemu-alpine-x86_64.sh
> b/automation/scripts/qemu-alpine-x86_64.sh
> index 8e398dcea34b..a188d60ea6f3 100755
> --- a/automation/scripts/qemu-alpine-x86_64.sh
> +++ b/automation/scripts/qemu-alpine-x86_64.sh
> @@ -56,7 +56,7 @@ bash /etc/init.d/xencommons start
>
> xl list
>
> -xl create -c /root/test.cfg
> +xl -vvv create -c /root/test.cfg
>
> " > etc/local.d/xen.start
> chmod +x etc/local.d/xen.start
> diff --git a/automation/scripts/qemu-smoke-dom0-arm32.sh
> b/automation/scripts/qemu-smoke-dom0-arm32.sh
> index d91648905669..3d208cd55bfa 100755
> --- a/automation/scripts/qemu-smoke-dom0-arm32.sh
> +++ b/automation/scripts/qemu-smoke-dom0-arm32.sh
> @@ -21,7 +21,7 @@ echo "#!/bin/bash
>
> xl list
>
> -xl create -c /root/test.cfg
> +xl -vvv create -c /root/test.cfg
>
> " > ./root/xen.start
> echo "bash /root/xen.start" >> ./etc/init.d/xen-watchdog
> diff --git a/automation/scripts/qemu-smoke-dom0-arm64.sh
> b/automation/scripts/qemu-smoke-dom0-arm64.sh
> index e0bb37af3610..afc24074eef8 100755
> --- a/automation/scripts/qemu-smoke-dom0-arm64.sh
> +++ b/automation/scripts/qemu-smoke-dom0-arm64.sh
> @@ -52,7 +52,7 @@ bash /etc/init.d/xencommons start
>
> xl list
>
> -xl create -c /root/test.cfg
> +xl -vvv create -c /root/test.cfg
>
> " > etc/local.d/xen.start
> chmod +x etc/local.d/xen.start
> diff --git a/automation/scripts/qubes-x86-64.sh
> b/automation/scripts/qubes-x86-64.sh
> index 4beeff17d31b..bd620b0d9273 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -112,7 +112,6 @@ echo \"${passed}\"
> "
>
> dom0_check="
> -tail -F /var/log/xen/qemu-dm-domU.log &
> until grep -q \"^domU Welcome to Alpine Linux\"
> /var/log/xen/console/guest-domU.log; do
> sleep 1
> done
> @@ -167,7 +166,8 @@ ifconfig xenbr0 192.168.0.1
>
> # get domU console content into test log
> tail -F /var/log/xen/console/guest-domU.log 2>/dev/null | sed -e
> \"s/^/(domU) /\" &
> -xl create /etc/xen/domU.cfg
> +tail -F /var/log/xen/qemu-dm-domU.log 2>/dev/null | sed -e \"s/^/(qemu-dm)
> /\" &
> +xl -vvv create /etc/xen/domU.cfg
> ${dom0_check}
> " > etc/local.d/xen.start
> chmod +x etc/local.d/xen.start
> --
> git-series 0.9.1
>
Re: [PATCH 05/12] automation: prevent grub unpacking initramfs
On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote: > It fails on larger initramfs (~250MB one), let Linux do it. > > Signed-off-by: Marek Marczykowski-Górecki Acked-by: Stefano Stabellini > --- > automation/scripts/qubes-x86-64.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/automation/scripts/qubes-x86-64.sh > b/automation/scripts/qubes-x86-64.sh > index bd620b0d9273..77cb0d45815d 100755 > --- a/automation/scripts/qubes-x86-64.sh > +++ b/automation/scripts/qubes-x86-64.sh > @@ -189,7 +189,7 @@ CONTROLLER=control@thor.testnet > echo " > multiboot2 (http)/gitlab-ci/xen $CONSOLE_OPTS loglvl=all guest_loglvl=all > dom0_mem=4G console_timestamps=boot $extra_xen_opts > module2 (http)/gitlab-ci/vmlinuz console=hvc0 root=/dev/ram0 earlyprintk=xen > -module2 (http)/gitlab-ci/initrd-dom0 > +module2 --nounzip (http)/gitlab-ci/initrd-dom0 > " > $TFTP/grub.cfg > > cp -f binaries/xen $TFTP/xen > -- > git-series 0.9.1 >
Re: [PATCH 02/12] automation: update fedora build to F39
On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote: > Fedora 29 is long EOL > > Signed-off-by: Marek Marczykowski-Górecki Acked-by: Stefano Stabellini > --- > automation/build/fedora/29.dockerfile | 46 + > automation/build/fedora/39.dockerfile | 46 - > automation/gitlab-ci/build.yaml | 4 +- > 3 files changed, 48 insertions(+), 48 deletions(-) > delete mode 100644 automation/build/fedora/29.dockerfile > create mode 100644 automation/build/fedora/39.dockerfile > > diff --git a/automation/build/fedora/29.dockerfile > b/automation/build/fedora/29.dockerfile > deleted file mode 100644 > index f473ae13e7c1.. > --- a/automation/build/fedora/29.dockerfile > +++ /dev/null > @@ -1,46 +0,0 @@ > -FROM --platform=linux/amd64 fedora:29 > -LABEL maintainer.name="The Xen Project" \ > - maintainer.email="xen-devel@lists.xenproject.org" > - > -# install Xen depends > -RUN dnf -y install \ > -clang \ > -gcc \ > -gcc-c++ \ > -ncurses-devel \ > -zlib-devel \ > -openssl-devel \ > -python-devel \ > -python3-devel \ > -libuuid-devel \ > -pkgconfig \ > -flex \ > -bison \ > -libaio-devel \ > -glib2-devel \ > -yajl-devel \ > -pixman-devel \ > -glibc-devel \ > -make \ > -binutils \ > -git \ > -wget \ > -acpica-tools \ > -python-markdown \ > -patch \ > -checkpolicy \ > -dev86 \ > -xz-devel \ > -bzip2 \ > -nasm \ > -ocaml \ > -ocaml-findlib \ > -golang \ > -# QEMU > -ninja-build \ > -&& dnf clean all && \ > -rm -rf /var/cache/dnf > - > -RUN useradd --create-home user > -USER user > -WORKDIR /build > diff --git a/automation/build/fedora/39.dockerfile > b/automation/build/fedora/39.dockerfile > new file mode 100644 > index ..054f73444060 > --- /dev/null > +++ b/automation/build/fedora/39.dockerfile > @@ -0,0 +1,46 @@ > +FROM --platform=linux/amd64 fedora:39 > +LABEL maintainer.name="The Xen Project" \ > + maintainer.email="xen-devel@lists.xenproject.org" > + > +# install Xen depends > +RUN dnf -y install \ > +clang \ > +gcc \ > +gcc-c++ \ > +ncurses-devel \ > +zlib-devel \ > +openssl-devel \ > +python-devel \ > +python3-devel \ > +libuuid-devel \ > +pkgconfig \ > +flex \ > +bison \ > +libaio-devel \ > +glib2-devel \ > +yajl-devel \ > +pixman-devel \ > +glibc-devel \ > +make \ > +binutils \ > +git \ > +wget \ > +acpica-tools \ > +python-markdown \ > +patch \ > +checkpolicy \ > +dev86 \ > +xz-devel \ > +bzip2 \ > +nasm \ > +ocaml \ > +ocaml-findlib \ > +golang \ > +# QEMU > +ninja-build \ > +&& dnf clean all && \ > +rm -rf /var/cache/dnf > + > +RUN useradd --create-home user > +USER user > +WORKDIR /build > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml > index 49d6265ad5b4..69665ec5b11f 100644 > --- a/automation/gitlab-ci/build.yaml > +++ b/automation/gitlab-ci/build.yaml > @@ -691,12 +691,12 @@ debian-bookworm-32-gcc-debug: > fedora-gcc: >extends: .gcc-x86-64-build >variables: > -CONTAINER: fedora:29 > +CONTAINER: fedora:39 > > fedora-gcc-debug: >extends: .gcc-x86-64-build-debug >variables: > -CONTAINER: fedora:29 > +CONTAINER: fedora:39 > > # Ubuntu Trusty's Clang is 3.4 while Xen requires 3.5 > > -- > git-series 0.9.1 >
Re: [PATCH 01/12] automation: include domU kernel messages in the console output log
On Thu, 16 May 2024, Marek Marczykowski-Górecki wrote:
> Signed-off-by: Marek Marczykowski-Górecki
Acked-by: Stefano Stabellini
> ---
> automation/scripts/qubes-x86-64.sh | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/automation/scripts/qubes-x86-64.sh
> b/automation/scripts/qubes-x86-64.sh
> index d81ed7b931cf..4beeff17d31b 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -131,6 +131,8 @@ mkdir sys
> rm var/run
> echo "#!/bin/sh
>
> +echo 8 > /proc/sys/kernel/printk
> +
> ${domU_check}
> " > etc/local.d/xen.start
> chmod +x etc/local.d/xen.start
> --
> git-series 0.9.1
>
Re: [XEN PATCH] automation/eclair_analysis: set MISRA C Rule 10.2 as clean
On Fri, 17 May 2024, Nicola Vetrini wrote:
> This rule has no more violations in the codebase, so it can be
> set as clean.
>
> No functional change.
>
> Signed-off-by: Nicola Vetrini
Reviewed-by: Stefano Stabellini
> ---
> automation/eclair_analysis/ECLAIR/tagging.ecl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl
> b/automation/eclair_analysis/ECLAIR/tagging.ecl
> index acea15f486a1..b7a9f75b275b 100644
> --- a/automation/eclair_analysis/ECLAIR/tagging.ecl
> +++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
> @@ -19,7 +19,7 @@
>
> -doc_begin="Clean guidelines: new violations for these guidelines are not
> accepted."
>
> --service_selector={clean_guidelines_common,
> "MC3R1.D1.1||MC3R1.D2.1||MC3R1.D4.1||MC3R1.D4.11||MC3R1.D4.14||MC3R1.R11.7||MC3R1.R11.9||MC3R1.R12.5||MC3R1.R1.1||MC3R1.R1.3||MC3R1.R1.4||MC3R1.R14.1||MC3R1.R16.7||MC3R1.R17.1||MC3R1.R17.3||MC3R1.R17.4||MC3R1.R17.5||MC3R1.R17.6||MC3R1.R20.13||MC3R1.R20.14||MC3R1.R20.4||MC3R1.R20.9||MC3R1.R21.10||MC3R1.R21.13||MC3R1.R21.19||MC3R1.R21.21||MC3R1.R21.9||MC3R1.R2.2||MC3R1.R22.2||MC3R1.R22.4||MC3R1.R22.5||MC3R1.R22.6||MC3R1.R2.6||MC3R1.R3.1||MC3R1.R3.2||MC3R1.R4.1||MC3R1.R4.2||MC3R1.R5.1||MC3R1.R5.2||MC3R1.R5.4||MC3R1.R5.6||MC3R1.R6.1||MC3R1.R6.2||MC3R1.R7.1||MC3R1.R7.2||MC3R1.R7.4||MC3R1.R8.1||MC3R1.R8.10||MC3R1.R8.12||MC3R1.R8.14||MC3R1.R8.2||MC3R1.R8.5||MC3R1.R8.6||MC3R1.R8.8||MC3R1.R9.2||MC3R1.R9.3||MC3R1.R9.4||MC3R1.R9.5"
> +-service_selector={clean_guidelines_common,
> "MC3R1.D1.1||MC3R1.D2.1||MC3R1.D4.1||MC3R1.D4.11||MC3R1.D4.14||MC3R1.R10.2||MC3R1.R11.7||MC3R1.R11.9||MC3R1.R12.5||MC3R1.R1.1||MC3R1.R1.3||MC3R1.R1.4||MC3R1.R14.1||MC3R1.R16.7||MC3R1.R17.1||MC3R1.R17.3||MC3R1.R17.4||MC3R1.R17.5||MC3R1.R17.6||MC3R1.R20.13||MC3R1.R20.14||MC3R1.R20.4||MC3R1.R20.9||MC3R1.R21.10||MC3R1.R21.13||MC3R1.R21.19||MC3R1.R21.21||MC3R1.R21.9||MC3R1.R2.2||MC3R1.R22.2||MC3R1.R22.4||MC3R1.R22.5||MC3R1.R22.6||MC3R1.R2.6||MC3R1.R3.1||MC3R1.R3.2||MC3R1.R4.1||MC3R1.R4.2||MC3R1.R5.1||MC3R1.R5.2||MC3R1.R5.4||MC3R1.R5.6||MC3R1.R6.1||MC3R1.R6.2||MC3R1.R7.1||MC3R1.R7.2||MC3R1.R7.4||MC3R1.R8.1||MC3R1.R8.10||MC3R1.R8.12||MC3R1.R8.14||MC3R1.R8.2||MC3R1.R8.5||MC3R1.R8.6||MC3R1.R8.8||MC3R1.R9.2||MC3R1.R9.3||MC3R1.R9.4||MC3R1.R9.5"
> }
>
> -setq=target,getenv("XEN_TARGET_ARCH")
> --
> 2.34.1
>
Re: [PATCH] Revert "evtchn: refuse EVTCHNOP_status for Xen-bound event channels"
On Fri, 17 May 2024, Jan Beulich wrote: > On 17.05.2024 03:21, Stefano Stabellini wrote: > > On Thu, 16 May 2024, Jan Beulich wrote: > >> 1) In the discussion George claimed that exposing status information in > >> an uncontrolled manner is okay. I'm afraid I have to disagree, seeing > >> how a similar assumption by CPU designers has led to a flood of > >> vulnerabilities over the last 6+ years. Information exposure imo is never > >> okay, unless it can be _proven_ that absolutely nothing "useful" can be > >> inferred from it. (I'm having difficulty seeing how such a proof might > >> look like.) > > > > Many would agree that it is better not to expose status information in > > an uncontrolled manner. Anyway, let's focus on the actionable. > > > > > >> 2) Me pointing out that the XSM hook might similarly get in the way of > >> debugging, Andrew suggested that this is not an issue because any sensible > >> XSM policy used in such an environment would grant sufficient privilege to > >> Dom0. Yet that then still doesn't cover why DomU-s also can obtain status > >> for Xen-internal event channels. The debugging argument then becomes weak, > >> as in that case the XSM hook is possibly going to get in the way. > >> > >> 3) In the discussion Andrew further gave the impression that evtchn_send() > >> had no XSM check. Yet it has; the difference to evtchn_status() is that > >> the latter uses XSM_TARGET while the former uses XSM_HOOK. (Much like > >> evtchn_status() may indeed be useful for debugging, evtchn_send() may be > >> similarly useful to allow getting a stuck channel unstuck.) > >> > >> In summary I continue to think that an outright revert was inappropriate. > >> DomU-s should continue to be denied status information on Xen-internal > >> event channels, unconditionally and independent of whether dummy, silo, or > >> Flask is in use. > > > > I think DomU-s should continue to be denied status information on > > Xen-internal event channels *based on the default dummy, silo, or Flask > > policy*. It is not up to us to decide the security policy, only to > > enforce it and provide sensible defaults. > > > > In any case, the XSM_TARGET check in evtchn_status seems to do what we > > want? > > No. XSM_TARGET permits the "owning" (not really, but it's its table) domain > access. See xsm_default_action() in xsm/dummy.h. Sorry I still don't understand. Why is that a problem? It looks like the wanted default behavior?
[xen-unstable test] 186027: tolerable FAIL - PUSHED
flight 186027 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/186027/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 186021 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 186021 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 186021 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 186021 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 186021 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 186021 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail 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-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-qcow214 migrate-support-checkfail never pass test-armhf-armhf-xl-qcow215 saverestore-support-checkfail never pass test-armhf-armhf-xl-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-vhd 15 saverestore-support-checkfail never pass version targeted for testing: xen 53dc37829c31edf02e8fc56aeccca8d60f6e2f5e baseline version: xen ae7584f63678cd9adc1c2f3a1e813b67a6b24544 Last test of basis 186021 2024-05-16 21:08:45 Z0 days Testing same since 186027 2024-05-17 07:07:37 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Edgar E. Iglesias Julien Grall Luca Fancellu Roger Pau Monné jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-arm64 pass build-armhf pass build-i386
[PATCH v15 5/5] xen/arm: account IO handlers for emulated PCI MSI-X
From: Oleksandr Andrushchenko
At the moment, we always allocate an extra 16 slots for IO handlers
(see MAX_IO_HANDLER). So while adding IO trap handlers for the emulated
MSI-X registers we need to explicitly tell that we have additional IO
handlers, so those are accounted.
Signed-off-by: Oleksandr Andrushchenko
Acked-by: Julien Grall
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
---
This depends on a constant defined in ("vpci: add initial support for
virtual PCI bus topology"), so cannot be committed without the
dependency.
Since v5:
- optimize with IS_ENABLED(CONFIG_HAS_PCI_MSI) since VPCI_MAX_VIRT_DEV is
defined unconditionally
New in v5
---
xen/arch/arm/vpci.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 516933bebfb3..4779bbfa9be3 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -132,6 +132,8 @@ static int vpci_get_num_handlers_cb(struct domain *d,
unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
{
+unsigned int count;
+
if ( !has_vpci(d) )
return 0;
@@ -152,7 +154,17 @@ unsigned int domain_vpci_get_num_mmio_handlers(struct
domain *d)
* For guests each host bridge requires one region to cover the
* configuration space. At the moment, we only expose a single host bridge.
*/
-return 1;
+count = 1;
+
+/*
+ * There's a single MSI-X MMIO handler that deals with both PBA
+ * and MSI-X tables per each PCI device being passed through.
+ * Maximum number of emulated virtual devices is VPCI_MAX_VIRT_DEV.
+ */
+if ( IS_ENABLED(CONFIG_HAS_PCI_MSI) )
+count += VPCI_MAX_VIRT_DEV;
+
+return count;
}
/*
--
2.45.1
[PATCH v15 3/5] vpci: add initial support for virtual PCI bus topology
From: Oleksandr Andrushchenko
Assign SBDF to the PCI devices being passed through with bus 0.
The resulting topology is where PCIe devices reside on the bus 0 of the
root complex itself (embedded endpoints).
This implementation is limited to 32 devices which are allowed on
a single PCI bus.
Please note, that at the moment only function 0 of a multifunction
device can be passed through.
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
Acked-by: Jan Beulich
---
In v15:
- add Jan's A-b
In v13:
- s/depends on/select/ in Kconfig
- check pdev->sbdf.fn instead of two booleans in add_virtual_device()
- comment #endifs in sched.h
- clarify comment about limits in vpci.h with seg/bus limit
In v11:
- Fixed code formatting
- Removed bogus write_unlock() call
- Fixed type for new_dev_number
In v10:
- Removed ASSERT(pcidevs_locked())
- Removed redundant code (local sbdf variable, clearing sbdf during
device removal, etc)
- Added __maybe_unused attribute to "out:" label
- Introduced HAS_VPCI_GUEST_SUPPORT Kconfig option, as this is the
first patch where it is used (previously was in "vpci: add hooks for
PCI device assign/de-assign")
In v9:
- Lock in add_virtual_device() replaced with ASSERT (thanks, Stewart)
In v8:
- Added write lock in add_virtual_device
Since v6:
- re-work wrt new locking scheme
- OT: add ASSERT(pcidevs_write_locked()); to add_virtual_device()
Since v5:
- s/vpci_add_virtual_device/add_virtual_device and make it static
- call add_virtual_device from vpci_assign_device and do not use
REGISTER_VPCI_INIT machinery
- add pcidevs_locked ASSERT
- use DECLARE_BITMAP for vpci_dev_assigned_map
Since v4:
- moved and re-worked guest sbdf initializers
- s/set_bit/__set_bit
- s/clear_bit/__clear_bit
- minor comment fix s/Virtual/Guest/
- added VPCI_MAX_VIRT_DEV constant (PCI_SLOT(~0) + 1) which will be used
later for counting the number of MMIO handlers required for a guest
(Julien)
Since v3:
- make use of VPCI_INIT
- moved all new code to vpci.c which belongs to it
- changed open-coded 31 to PCI_SLOT(~0)
- added comments and code to reject multifunction devices with
functions other than 0
- updated comment about vpci_dev_next and made it unsigned int
- implement roll back in case of error while assigning/deassigning devices
- s/dom%pd/%pd
Since v2:
- remove casts that are (a) malformed and (b) unnecessary
- add new line for better readability
- remove CONFIG_HAS_VPCI_GUEST_SUPPORT ifdef's as the relevant vPCI
functions are now completely gated with this config
- gate common code with CONFIG_HAS_VPCI_GUEST_SUPPORT
New in v2
---
xen/drivers/Kconfig | 4 +++
xen/drivers/vpci/vpci.c | 57 +
xen/include/xen/sched.h | 10 +++-
xen/include/xen/vpci.h | 12 +
4 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index db94393f47a6..20050e9bb8b3 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -15,4 +15,8 @@ source "drivers/video/Kconfig"
config HAS_VPCI
bool
+config HAS_VPCI_GUEST_SUPPORT
+ bool
+ select HAS_VPCI
+
endmenu
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 97e115dc5798..23722634d50b 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -40,6 +40,49 @@ extern vpci_register_init_t *const __start_vpci_array[];
extern vpci_register_init_t *const __end_vpci_array[];
#define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
+#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
+static int add_virtual_device(struct pci_dev *pdev)
+{
+struct domain *d = pdev->domain;
+unsigned int new_dev_number;
+
+if ( is_hardware_domain(d) )
+return 0;
+
+ASSERT(rw_is_write_locked(>domain->pci_lock));
+
+/*
+ * Each PCI bus supports 32 devices/slots at max or up to 256 when
+ * there are multi-function ones which are not yet supported.
+ */
+if ( pdev->sbdf.fn )
+{
+gdprintk(XENLOG_ERR, "%pp: only function 0 passthrough supported\n",
+ >sbdf);
+return -EOPNOTSUPP;
+}
+new_dev_number = find_first_zero_bit(d->vpci_dev_assigned_map,
+ VPCI_MAX_VIRT_DEV);
+if ( new_dev_number == VPCI_MAX_VIRT_DEV )
+return -ENOSPC;
+
+__set_bit(new_dev_number, >vpci_dev_assigned_map);
+
+/*
+ * Both segment and bus number are 0:
+ * - we emulate a single host bridge for the guest, e.g. segment 0
+ * - with bus 0 the virtual devices are seen as embedded
+ *endpoints behind the root complex
+ *
+ * TODO: add support for multi-function devices.
+ */
+pdev->vpci->guest_sbdf = PCI_SBDF(0, 0, new_dev_number, 0);
+
+return 0;
+}
+
+#endif /* CONFIG_HAS_VPCI_GUEST_SUPPORT */
+
void vpci_deassign_device(struct pci_dev *pdev)
{
unsigned int i;
@@ -49,6 +92,12 @@ void vpci_deassign_device(struct pci_dev
[PATCH v15 4/5] xen/arm: translate virtual PCI bus topology for guests
From: Oleksandr Andrushchenko
There are three originators for the PCI configuration space access:
1. The domain that owns physical host bridge: MMIO handlers are
there so we can update vPCI register handlers with the values
written by the hardware domain, e.g. physical view of the registers
vs guest's view on the configuration space.
2. Guest access to the passed through PCI devices: we need to properly
map virtual bus topology to the physical one, e.g. pass the configuration
space access to the corresponding physical devices.
3. Emulated host PCI bridge access. It doesn't exist in the physical
topology, e.g. it can't be mapped to some physical host bridge.
So, all access to the host bridge itself needs to be trapped and
emulated.
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
---
In v15:
- base on top of ("arm/vpci: honor access size when returning an error")
In v11:
- Fixed format issues
- Added ASSERT_UNREACHABLE() to the dummy implementation of
vpci_translate_virtual_device()
- Moved variable in vpci_sbdf_from_gpa(), now it is easier to follow
the logic in the function
Since v9:
- Commend about required lock replaced with ASSERT()
- Style fixes
- call to vpci_translate_virtual_device folded into vpci_sbdf_from_gpa
Since v8:
- locks moved out of vpci_translate_virtual_device()
Since v6:
- add pcidevs locking to vpci_translate_virtual_device
- update wrt to the new locking scheme
Since v5:
- add vpci_translate_virtual_device for #ifndef CONFIG_HAS_VPCI_GUEST_SUPPORT
case to simplify ifdefery
- add ASSERT(!is_hardware_domain(d)); to vpci_translate_virtual_device
- reset output register on failed virtual SBDF translation
Since v4:
- indentation fixes
- constify struct domain
- updated commit message
- updates to the new locking scheme (pdev->vpci_lock)
Since v3:
- revisit locking
- move code to vpci.c
Since v2:
- pass struct domain instead of struct vcpu
- constify arguments where possible
- gate relevant code with CONFIG_HAS_VPCI_GUEST_SUPPORT
New in v2
---
xen/arch/arm/vpci.c | 45 -
xen/drivers/vpci/vpci.c | 24 ++
xen/include/xen/vpci.h | 12 +++
3 files changed, 71 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index b63a356bb4a8..516933bebfb3 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -7,33 +7,53 @@
#include
-static pci_sbdf_t vpci_sbdf_from_gpa(const struct pci_host_bridge *bridge,
- paddr_t gpa)
+static bool vpci_sbdf_from_gpa(struct domain *d,
+ const struct pci_host_bridge *bridge,
+ paddr_t gpa, pci_sbdf_t *sbdf)
{
-pci_sbdf_t sbdf;
+bool translated = true;
+
+ASSERT(sbdf);
if ( bridge )
{
-sbdf.sbdf = VPCI_ECAM_BDF(gpa - bridge->cfg->phys_addr);
-sbdf.seg = bridge->segment;
-sbdf.bus += bridge->cfg->busn_start;
+sbdf->sbdf = VPCI_ECAM_BDF(gpa - bridge->cfg->phys_addr);
+sbdf->seg = bridge->segment;
+sbdf->bus += bridge->cfg->busn_start;
}
else
-sbdf.sbdf = VPCI_ECAM_BDF(gpa - GUEST_VPCI_ECAM_BASE);
+{
+/*
+ * For the passed through devices we need to map their virtual SBDF
+ * to the physical PCI device being passed through.
+ */
+sbdf->sbdf = VPCI_ECAM_BDF(gpa - GUEST_VPCI_ECAM_BASE);
+read_lock(>pci_lock);
+translated = vpci_translate_virtual_device(d, sbdf);
+read_unlock(>pci_lock);
+}
-return sbdf;
+return translated;
}
static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
register_t *r, void *p)
{
struct pci_host_bridge *bridge = p;
-pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
+pci_sbdf_t sbdf;
const unsigned int access_size = (1U << info->dabt.size) * 8;
const register_t invalid = GENMASK_ULL(access_size - 1, 0);
/* data is needed to prevent a pointer cast on 32bit */
unsigned long data;
+ASSERT(!bridge == !is_hardware_domain(v->domain));
+
+if ( !vpci_sbdf_from_gpa(v->domain, bridge, info->gpa, ) )
+{
+*r = invalid;
+return 1;
+}
+
if ( vpci_ecam_read(sbdf, ECAM_REG_OFFSET(info->gpa),
1U << info->dabt.size, ) )
{
@@ -50,7 +70,12 @@ static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
register_t r, void *p)
{
struct pci_host_bridge *bridge = p;
-pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
+pci_sbdf_t sbdf;
+
+ASSERT(!bridge == !is_hardware_domain(v->domain));
+
+if ( !vpci_sbdf_from_gpa(v->domain, bridge, info->gpa, ) )
+return 1;
return vpci_ecam_write(sbdf, ECAM_REG_OFFSET(info->gpa),
1U << info->dabt.size, r);
diff --git a/xen/drivers/vpci/vpci.c
[PATCH v15 2/5] vpci/header: emulate PCI_COMMAND register for guests
From: Oleksandr Andrushchenko Xen and/or Dom0 may have put values in PCI_COMMAND which they expect to remain unaltered. PCI_COMMAND_SERR bit is a good example: while the guest's (domU) view of this will want to be zero (for now), the host having set it to 1 should be preserved, or else we'd effectively be giving the domU control of the bit. Thus, PCI_COMMAND register needs proper emulation in order to honor host's settings. According to "PCI LOCAL BUS SPECIFICATION, REV. 3.0", section "6.2.2 Device Control" the reset state of the command register is typically 0, so when assigning a PCI device use 0 as the initial state for the guest's (domU) view of the command register. Here is the full list of command register bits with notes about PCI/PCIe specification, and how Xen handles the bit. QEMU's behavior is also documented here since that is our current reference implementation for PCI passthrough. PCI_COMMAND_IO (bit 0) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. QEMU sets this bit to 1 in hardware if an I/O BAR is exposed to the guest. Xen domU: (rsvdp_mask) We treat this bit as RsvdP for now since we don't yet support I/O BARs for domUs. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_MEMORY (bit 1) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. QEMU sets this bit to 1 in hardware if a Memory BAR is exposed to the guest. Xen domU/dom0: We handle writes to this bit by mapping/unmapping BAR regions. Xen domU: For devices assigned to DomUs, memory decoding will be disabled at the time of initialization. PCI_COMMAND_MASTER (bit 2) PCIe 6.1: RW PCI LB 3.0: RW QEMU: Pass through writes to hardware. Xen domU/dom0: Pass through writes to hardware. PCI_COMMAND_SPECIAL (bit 3) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: RW QEMU: Pass through writes to hardware. Xen domU/dom0: Pass through writes to hardware. PCI_COMMAND_INVALIDATE (bit 4) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: RW QEMU: Pass through writes to hardware. Xen domU/dom0: Pass through writes to hardware. PCI_COMMAND_VGA_PALETTE (bit 5) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: RW QEMU: Pass through writes to hardware. Xen domU/dom0: Pass through writes to hardware. PCI_COMMAND_PARITY (bit 6) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. Xen domU: (rsvdp_mask) We treat this bit as RsvdP. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_WAIT (bit 7) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: hardwire to 0 QEMU: res_mask Xen domU: (rsvdp_mask) We treat this bit as RsvdP. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_SERR (bit 8) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. Xen domU: (rsvdp_mask) We treat this bit as RsvdP. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_FAST_BACK (bit 9) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. Xen domU: (rsvdp_mask) We treat this bit as RsvdP. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_INTX_DISABLE (bit 10) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. QEMU checks if INTx was mapped for a device. If it is not, then guest can't control PCI_COMMAND_INTX_DISABLE bit. Xen domU: We prohibit a guest from enabling INTx if MSI(X) is enabled. Xen dom0: We allow dom0 to control this bit freely. Bits 11-15 PCIe 6.1: RsvdP PCI LB 3.0: Reserved QEMU: res_mask Xen domU/dom0: rsvdp_mask Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Volodymyr Babchuk Signed-off-by: Stewart Hildebrand Reviewed-by: Jan Beulich --- RFC: There is an unaddressed question for Roger: should we update the guest view of the PCI_COMMAND_INTX_DISABLE bit in msi.c/msix.c:control_write()? See prior discussion at [1]. In my opinion, I think we should make sure that hardware state and the guest view are consistent (i.e. don't lie to the guest). [1] https://lore.kernel.org/xen-devel/86b25777-788c-4b9a-8166-a6f8174be...@suse.com/ In v15: - add Jan's R-b - add blank line after declaration in msi.c:control_write() In v14: - check for 0->1 transition in INTX_DISABLE-setting logic in msi.c:control_write() to match msix.c:control_write() - clear domU-controllable bits in header.c:init_header() In v13: - Update right away (don't defer) PCI_COMMAND_MEMORY bit in guest_cmd variable in cmd_write() - Make comment single line in
[PATCH v15 1/5] arm/vpci: honor access size when returning an error
From: Volodymyr Babchuk
Guest can try to read config space using different access sizes: 8,
16, 32, 64 bits. We need to take this into account when we are
returning an error back to MMIO handler, otherwise it is possible to
provide more data than requested: i.e. guest issues LDRB instruction
to read one byte, but we are writing 0x in the target
register.
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
---
v14->v15:
* re-order so this patch comes before ("xen/arm: translate virtual PCI
bus topology for guests")
* s/access_mask/invalid/
* add U suffix to 1
* s/uint8_t/unsigned int/
* s/uint64_t/register_t/
* although Julien gave an Acked-by on v14, I omitted it due to the
changes made in v15
v9->10:
* New patch in v10.
---
xen/arch/arm/vpci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 3bc4bb55082a..b63a356bb4a8 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -29,6 +29,8 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
{
struct pci_host_bridge *bridge = p;
pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
+const unsigned int access_size = (1U << info->dabt.size) * 8;
+const register_t invalid = GENMASK_ULL(access_size - 1, 0);
/* data is needed to prevent a pointer cast on 32bit */
unsigned long data;
@@ -39,7 +41,7 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
return 1;
}
-*r = ~0ul;
+*r = invalid;
return 0;
}
--
2.45.1
[PATCH v15 0/5] PCI devices passthrough on Arm, part 3
This is next version of vPCI rework. Aim of this series is to prepare
ground for introducing PCI support on ARM platform.
in v15:
- reorder so ("arm/vpci: honor access size when returning an error")
comes first
in v14:
- drop first 9 patches as they were committed
- updated ("vpci/header: emulate PCI_COMMAND register for guests")
in v13:
- drop ("xen/arm: vpci: permit access to guest vpci space") as it was
unnecessary
in v12:
- I (Stewart) coordinated with Volodomyr to send this whole series. So,
add my (Stewart) Signed-off-by to all patches.
- The biggest change is to re-work the PCI_COMMAND register patch.
Additional feedback has also been addressed - see individual patches.
- Drop ("pci: msi: pass pdev to pci_enable_msi() function") and
("pci: introduce per-domain PCI rwlock") as they were committed
- Rename ("rangeset: add rangeset_empty() function")
to ("rangeset: add rangeset_purge() function")
- Rename ("vpci/header: rework exit path in init_bars")
to ("vpci/header: rework exit path in init_header()")
in v11:
- Added my (Volodymyr) Signed-off-by tag to all patches
- Patch "vpci/header: emulate PCI_COMMAND register for guests" is in
intermediate state, because it was agreed to rework it once Stewart's
series on register handling are in.
- Addressed comments, please see patch descriptions for details.
in v10:
- Removed patch ("xen/arm: vpci: check guest range"), proper fix
for the issue is part of ("vpci/header: emulate PCI_COMMAND
register for guests")
- Removed patch ("pci/header: reset the command register when adding
devices")
- Added patch ("rangeset: add rangeset_empty() function") because
this function is needed in ("vpci/header: handle p2m range sets
per BAR")
- Added ("vpci/header: handle p2m range sets per BAR") which addressed
an issue discovered by Andrii Chepurnyi during virtio integration
- Added ("pci: msi: pass pdev to pci_enable_msi() function"), which is
prereq for ("pci: introduce per-domain PCI rwlock")
- Fixed "Since v9/v8/... " comments in changelogs to reduce confusion.
I left "Since" entries for older versions, because they were added
by original author of the patches.
in v9:
v9 includes addressed commentes from a previous one. Also it
introduces a couple patches from Stewart. This patches are related to
vPCI use on ARM. Patch "vpci/header: rework exit path in init_bars"
was factored-out from "vpci/header: handle p2m range sets per BAR".
in v8:
The biggest change from previous, mistakenly named, v7 series is how
locking is implemented. Instead of d->vpci_rwlock we introduce
d->pci_lock which has broader scope, as it protects not only domain's
vpci state, but domain's list of PCI devices as well.
As we discussed in IRC with Roger, it is not feasible to rework all
the existing code to use the new lock right away. It was agreed that
any write access to d->pdev_list will be protected by **both**
d->pci_lock in write mode and pcidevs_lock(). Read access on other
hand should be protected by either d->pci_lock in read mode or
pcidevs_lock(). It is expected that existing code will use
pcidevs_lock() and new users will use new rw lock. Of course, this
does not mean that new users shall not use pcidevs_lock() when it is
appropriate.
Changes from previous versions are described in each separate patch.
Oleksandr Andrushchenko (4):
vpci/header: emulate PCI_COMMAND register for guests
vpci: add initial support for virtual PCI bus topology
xen/arm: translate virtual PCI bus topology for guests
xen/arm: account IO handlers for emulated PCI MSI-X
Volodymyr Babchuk (1):
arm/vpci: honor access size when returning an error
xen/arch/arm/vpci.c| 63 +++--
xen/drivers/Kconfig| 4 ++
xen/drivers/vpci/header.c | 60 +---
xen/drivers/vpci/msi.c | 9 +
xen/drivers/vpci/msix.c| 7
xen/drivers/vpci/vpci.c| 81 ++
xen/include/xen/pci_regs.h | 1 +
xen/include/xen/sched.h| 10 -
xen/include/xen/vpci.h | 27 +
9 files changed, 243 insertions(+), 19 deletions(-)
base-commit: 21611c68702dae2e18cb519a6e166cdceeaff4ca
--
2.45.1
[PATCH v2 2/2] tools/xg: Clean up xend-style overrides for CPU policies
Factor out policy getters/setters from both (CPUID and MSR) policy override
functions. Additionally, use host policy rather than featureset when
preparing the cur policy, saving one hypercall and several lines of
boilerplate.
No functional change intended.
Signed-off-by: Alejandro Vallejo
---
v2:
* Cosmetic change to comment (// into /**/).
* Added missing null pointer check to MSR override function.
---
tools/libs/guest/xg_cpuid_x86.c | 445 ++--
1 file changed, 130 insertions(+), 315 deletions(-)
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 4f4b86b59470..74bca0e65b69 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -36,6 +36,34 @@ enum {
#define bitmaskof(idx) (1u << ((idx) & 31))
#define featureword_of(idx) ((idx) >> 5)
+static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t *policy)
+{
+uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
+int rc;
+
+rc = x86_cpuid_copy_from_buffer(>policy, policy->leaves,
+policy->nr_leaves, _leaf,
_subleaf);
+if ( rc )
+{
+if ( err_leaf != -1 )
+ERROR("Failed to deserialise CPUID (err leaf %#x, subleaf %#x) (%d
= %s)",
+ err_leaf, err_subleaf, -rc, strerror(-rc));
+return rc;
+}
+
+rc = x86_msr_copy_from_buffer(>policy, policy->msrs,
+ policy->nr_msrs, _msr);
+if ( rc )
+{
+if ( err_msr != -1 )
+ERROR("Failed to deserialise MSR (err MSR %#x) (%d = %s)",
+ err_msr, -rc, strerror(-rc));
+return rc;
+}
+
+return 0;
+}
+
int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps)
{
struct xen_sysctl sysctl = {};
@@ -260,102 +288,34 @@ static int compare_leaves(const void *l, const void *r)
return 0;
}
-static xen_cpuid_leaf_t *find_leaf(
-xen_cpuid_leaf_t *leaves, unsigned int nr_leaves,
-const struct xc_xend_cpuid *xend)
+static xen_cpuid_leaf_t *find_leaf(xc_cpu_policy_t *p,
+ const struct xc_xend_cpuid *xend)
{
const xen_cpuid_leaf_t key = { xend->leaf, xend->subleaf };
-return bsearch(, leaves, nr_leaves, sizeof(*leaves), compare_leaves);
+return bsearch(, p->leaves, ARRAY_SIZE(p->leaves),
+ sizeof(*p->leaves), compare_leaves);
}
-static int xc_cpuid_xend_policy(
-xc_interface *xch, uint32_t domid, const struct xc_xend_cpuid *xend)
+static int xc_cpuid_xend_policy(xc_interface *xch, uint32_t domid,
+const struct xc_xend_cpuid *xend,
+xc_cpu_policy_t *host,
+xc_cpu_policy_t *def,
+xc_cpu_policy_t *cur)
{
-int rc;
-bool hvm;
-xc_domaininfo_t di;
-unsigned int nr_leaves, nr_msrs;
-uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
-/*
- * Three full policies. The host, default for the domain type,
- * and domain current.
- */
-xen_cpuid_leaf_t *host = NULL, *def = NULL, *cur = NULL;
-unsigned int nr_host, nr_def, nr_cur;
-
-if ( (rc = xc_domain_getinfo_single(xch, domid, )) < 0 )
-{
-PERROR("Failed to obtain d%d info", domid);
-rc = -errno;
-goto fail;
-}
-hvm = di.flags & XEN_DOMINF_hvm_guest;
-
-rc = xc_cpu_policy_get_size(xch, _leaves, _msrs);
-if ( rc )
-{
-PERROR("Failed to obtain policy info size");
-rc = -errno;
-goto fail;
-}
-
-rc = -ENOMEM;
-if ( (host = calloc(nr_leaves, sizeof(*host))) == NULL ||
- (def = calloc(nr_leaves, sizeof(*def))) == NULL ||
- (cur = calloc(nr_leaves, sizeof(*cur))) == NULL )
-{
-ERROR("Unable to allocate memory for %u CPUID leaves", nr_leaves);
-goto fail;
-}
-
-/* Get the domain's current policy. */
-nr_msrs = 0;
-nr_cur = nr_leaves;
-rc = get_domain_cpu_policy(xch, domid, _cur, cur, _msrs, NULL);
-if ( rc )
-{
-PERROR("Failed to obtain d%d current policy", domid);
-rc = -errno;
-goto fail;
-}
-
-/* Get the domain type's default policy. */
-nr_msrs = 0;
-nr_def = nr_leaves;
-rc = get_system_cpu_policy(xch, hvm ? XEN_SYSCTL_cpu_policy_hvm_default
-: XEN_SYSCTL_cpu_policy_pv_default,
- _def, def, _msrs, NULL);
-if ( rc )
-{
-PERROR("Failed to obtain %s def policy", hvm ? "hvm" : "pv");
-rc = -errno;
-goto fail;
-}
-
-/* Get the host policy. */
-nr_msrs = 0;
-nr_host = nr_leaves;
-rc = get_system_cpu_policy(xch, XEN_SYSCTL_cpu_policy_host,
- _host, host, _msrs, NULL);
-if ( rc )
-{
-PERROR("Failed to obtain host policy");
-rc = -errno;
-goto fail;
[PATCH v2 0/3] Clean the policy manipulation path in domain creation
v2: * Removed xc_cpu_policy from xenguest.h * Added accessors for xc_cpu_policy so the serialised form can be extracted. * Modified xen-cpuid to use accessors. Original cover letter In the context of creating a domain, we currently issue a lot of hypercalls redundantly while populating its CPU policy; likely a side effect of organic growth more than anything else. However, the worst part is not the overhead (this is a glacially cold path), but the insane amounts of boilerplate that make it really hard to pick apart what's going on. One major contributor to this situation is the fact that what's effectively "setup" and "teardown" phases in policy manipulation are not factored out from the functions that perform said manipulations, leading to the same getters and setter being invoked many times, when once each would do. Another big contributor is the code being unaware of when a policy is serialised and when it's not. This patch attempts to alleviate this situation, yielding over 200 LoC reduction. Patch 1: Mechanical change. Makes xc_cpu_policy_t public so it's usable from clients of libxc/libxg. Patch 2: Changes the (de)serialization wrappers in xenguest so they always serialise to/from the internal buffers of xc_cpu_policy_t. The struct is suitably expanded to hold extra information required. Patch 3: Performs the refactor of the policy manipulation code so that it follows a strict: PULL_POLICIES, MUTATE_POLICY (n times), PUSH_POLICY. Alejandro Vallejo (2): tools/xg: Streamline cpu policy serialise/deserialise calls tools/xg: Clean up xend-style overrides for CPU policies tools/include/xenguest.h| 8 +- tools/libs/guest/xg_cpuid_x86.c | 537 ++-- tools/libs/guest/xg_private.h | 2 + tools/libs/guest/xg_sr_common_x86.c | 54 +-- tools/misc/xen-cpuid.c | 43 +-- 5 files changed, 231 insertions(+), 413 deletions(-) -- 2.34.1
[PATCH v2 1/2] tools/xg: Streamline cpu policy serialise/deserialise calls
The idea is to use xc_cpu_policy_t as a single object containing both the
serialised and deserialised forms of the policy. Note that we need lengths
for the arrays, as the serialised policies may be shorter than the array
capacities.
* Add the serialised lengths to the struct so we can distinguish
between length and capacity of the serialisation buffers.
* Remove explicit buffer+lengths in serialise/deserialise calls
and use the internal buffer inside xc_cpu_policy_t instead.
* Refactor everything to use the new serialisation functions.
* Remove redundant serialization calls and avoid allocating dynamic
memory aside from the policy objects in xen-cpuid. Also minor cleanup
in the policy print call sites.
No functional change intended.
Signed-off-by: Alejandro Vallejo
---
v2:
* Removed v1/patch1.
* Added the accessors suggested in feedback.
---
tools/include/xenguest.h| 8 ++-
tools/libs/guest/xg_cpuid_x86.c | 98 -
tools/libs/guest/xg_private.h | 2 +
tools/libs/guest/xg_sr_common_x86.c | 54 ++--
tools/misc/xen-cpuid.c | 43 -
5 files changed, 104 insertions(+), 101 deletions(-)
diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h
index e01f494b772a..563811cd8dde 100644
--- a/tools/include/xenguest.h
+++ b/tools/include/xenguest.h
@@ -799,14 +799,16 @@ int xc_cpu_policy_set_domain(xc_interface *xch, uint32_t
domid,
xc_cpu_policy_t *policy);
/* Manipulate a policy via architectural representations. */
-int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t *policy,
-xen_cpuid_leaf_t *leaves, uint32_t *nr_leaves,
-xen_msr_entry_t *msrs, uint32_t *nr_msrs);
+int xc_cpu_policy_serialise(xc_interface *xch, xc_cpu_policy_t *policy);
int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t *policy,
const xen_cpuid_leaf_t *leaves,
uint32_t nr);
int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t *policy,
const xen_msr_entry_t *msrs, uint32_t nr);
+int xc_cpu_policy_get_leaves(xc_interface *xch, const xc_cpu_policy_t *policy,
+ const xen_cpuid_leaf_t **leaves, uint32_t *nr);
+int xc_cpu_policy_get_msrs(xc_interface *xch, const xc_cpu_policy_t *policy,
+ const xen_msr_entry_t **msrs, uint32_t *nr);
/* Compatibility calculations. */
bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 4453178100ad..4f4b86b59470 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -834,14 +834,13 @@ void xc_cpu_policy_destroy(xc_cpu_policy_t *policy)
}
}
-static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t *policy,
- unsigned int nr_leaves, unsigned int nr_entries)
+static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t *policy)
{
uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
int rc;
rc = x86_cpuid_copy_from_buffer(>policy, policy->leaves,
-nr_leaves, _leaf, _subleaf);
+policy->nr_leaves, _leaf,
_subleaf);
if ( rc )
{
if ( err_leaf != -1 )
@@ -851,7 +850,7 @@ static int deserialize_policy(xc_interface *xch,
xc_cpu_policy_t *policy,
}
rc = x86_msr_copy_from_buffer(>policy, policy->msrs,
- nr_entries, _msr);
+ policy->nr_msrs, _msr);
if ( rc )
{
if ( err_msr != -1 )
@@ -878,7 +877,10 @@ int xc_cpu_policy_get_system(xc_interface *xch, unsigned
int policy_idx,
return rc;
}
-rc = deserialize_policy(xch, policy, nr_leaves, nr_msrs);
+policy->nr_leaves = nr_leaves;
+policy->nr_msrs = nr_msrs;
+
+rc = deserialize_policy(xch, policy);
if ( rc )
{
errno = -rc;
@@ -903,7 +905,10 @@ int xc_cpu_policy_get_domain(xc_interface *xch, uint32_t
domid,
return rc;
}
-rc = deserialize_policy(xch, policy, nr_leaves, nr_msrs);
+policy->nr_leaves = nr_leaves;
+policy->nr_msrs = nr_msrs;
+
+rc = deserialize_policy(xch, policy);
if ( rc )
{
errno = -rc;
@@ -917,17 +922,14 @@ int xc_cpu_policy_set_domain(xc_interface *xch, uint32_t
domid,
xc_cpu_policy_t *policy)
{
uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
-unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
-unsigned int nr_msrs = ARRAY_SIZE(policy->msrs);
int rc;
-rc = xc_cpu_policy_serialise(xch, policy, policy->leaves, _leaves,
- policy->msrs, _msrs);
+rc = xc_cpu_policy_serialise(xch, policy);
[PATCH v4 0/2] Finalise bridge VLAN support
Hello, The first two patches have been merged so this version has the remaining two. Only the first of these two patches has changed since v3. Summary of changes from v3 to v4: - Drop merged patches. - _vif_vlan_setup: Add comment that we delete vid 1 due to it being automatically added. - _vif_vlan_setup: Restructure to avoid using eval and simplify. - _vif_vlan_membership: Add comment that we remove trailing spaces due to readarray behaviour and change the code to do it only on the last element of the array, prior to entering the loop. - _vif_vlan_membership: Simplify the loop iterating through terms. Regards, Leigh. --- Leigh Brown (2): tools/hotplug/Linux: Add bridge VLAN support tools/examples: Example Linux bridge VLAN config docs/misc/linux-bridge-vlan/README | 68 + docs/misc/linux-bridge-vlan/br0.netdev | 7 ++ docs/misc/linux-bridge-vlan/br0.network| 8 ++ docs/misc/linux-bridge-vlan/enp0s0.network | 16 +++ tools/hotplug/Linux/xen-network-common.sh | 109 + 5 files changed, 208 insertions(+) create mode 100644 docs/misc/linux-bridge-vlan/README create mode 100644 docs/misc/linux-bridge-vlan/br0.netdev create mode 100644 docs/misc/linux-bridge-vlan/br0.network create mode 100644 docs/misc/linux-bridge-vlan/enp0s0.network -- 2.39.2
[PATCH v4 2/2] tools/examples: Example Linux bridge VLAN config
Add a new directory linux-bridge-vlan with example files showing how to configure systemd-networkd to support a bridge VLAN configuration. Signed-off-by: Leigh Brown --- docs/misc/linux-bridge-vlan/README | 68 ++ docs/misc/linux-bridge-vlan/br0.netdev | 7 +++ docs/misc/linux-bridge-vlan/br0.network| 8 +++ docs/misc/linux-bridge-vlan/enp0s0.network | 16 + 4 files changed, 99 insertions(+) create mode 100644 docs/misc/linux-bridge-vlan/README create mode 100644 docs/misc/linux-bridge-vlan/br0.netdev create mode 100644 docs/misc/linux-bridge-vlan/br0.network create mode 100644 docs/misc/linux-bridge-vlan/enp0s0.network diff --git a/docs/misc/linux-bridge-vlan/README b/docs/misc/linux-bridge-vlan/README new file mode 100644 index 00..9a048bca39 --- /dev/null +++ b/docs/misc/linux-bridge-vlan/README @@ -0,0 +1,68 @@ +Linux Xen Dom0 single bridge multiple VLAN configuration with systemd += + +Introduction + + +This directory contains example files to be placed in /etc/systemd/network +to enable a single bridge with multiple VLAN support. + +The example is to support the scenario where the Xen host network interface +is connected to an Ethernet switch configured as a trunk port. Each domain +VIF can then be configured with one or more VLAN IDs, one of which will be +the PVID. + +The example files create a bridge device called br0, with a physical interface +called enp0s0. You will need to update this with your system's device name. + +Key points of the configuration are: + +1. In br0.netdev, VLANFiltering=on is set. This is required to ensure the + VLAN tags are handled correctly. If it is not set then the packets + from the VIF interfaces will not have the correct VLAN tags set. + +2. In br0.network, a system IPv4 address is configured that can be updated + according to your local network settings. + +3. In enp0s0.network, Bridge=br0 sets the bridge device to connect to. There + is also a [BridgeVLAN] section for each VLAN allowed on the external + interface. Note, if you want to create an internal VLAN private to the + host, do not include its VLAN ID in this file. + + +Domain configuration + + +Add the vlan= keyword to the vif definition in the domain. The simplest +and most common example is a domain that wishes to connect to a single VLAN: + +vif = [ 'mac=xx:xx:xx:xx:xx:xx, bridge=br0, vlan=10' ] + +If you wish to configure a domain to route between two VLANs, you have two +options. Option 1 is to create multiple interfaces on different VLANs: + +vif = [ 'mac=xx:xx:xx:xx:xx:xx, bridge=br0, vlan=10', +'max=xx:xx:xx:xx:xx:xx, bridge=br0, vlan=20' ] + +Alternatively, you can create single interface: + +vif = [ 'mac=xx:xx:xx:xx:xx:xx, bridge=br0, vlan=10p/20' ] + +In the domain, you would, for example, use enX0 for VLAN 10 and enX0.20 for +VLAN 20. + + +Hints and tips +-- + +You can run the following commands on dom0 or a driver domain: + +1. To check if vlan_filtering is enabled: + # cat /sys/devices/virtual/net//bridge/vlan_filtering + +2. To check the bridge port VLAN assignments: + # bridge vlan + +3. To check the vlan setting in the xenstore (dom0 only): + # xenstore-ls -f | grep 'vlan =' + diff --git a/docs/misc/linux-bridge-vlan/br0.netdev b/docs/misc/linux-bridge-vlan/br0.netdev new file mode 100644 index 00..ae1fe487c3 --- /dev/null +++ b/docs/misc/linux-bridge-vlan/br0.netdev @@ -0,0 +1,7 @@ +[NetDev] +Name=br0 +Kind=bridge +MACAddress=xx:xx:xx:xx:xx:xx + +[Bridge] +VLANFiltering=on diff --git a/docs/misc/linux-bridge-vlan/br0.network b/docs/misc/linux-bridge-vlan/br0.network new file mode 100644 index 00..b56203b66a --- /dev/null +++ b/docs/misc/linux-bridge-vlan/br0.network @@ -0,0 +1,8 @@ +[Match] +Name=br0 + +[Network] +DNS=8.8.8.8 +#Domains=example.com +Address=10.1.1.10/24 +Gateway=10.1.1.1 diff --git a/docs/misc/linux-bridge-vlan/enp0s0.network b/docs/misc/linux-bridge-vlan/enp0s0.network new file mode 100644 index 00..6ee3154dfc --- /dev/null +++ b/docs/misc/linux-bridge-vlan/enp0s0.network @@ -0,0 +1,16 @@ +[Match] +Name=enp0s0 + +[Network] +Bridge=br0 + +# If Jumbo frames are required +#[Link] +#MTUBytes=9000 + +[BridgeVLAN] +VLAN=10 + +[BridgeVLAN] +VLAN=20 + -- 2.39.2
[PATCH v4 1/2] tools/hotplug/Linux: Add bridge VLAN support
Update add_to_bridge shell function to read the vlan parameter from
xenstore and set the bridge VLAN configuration for the VID.
Add additional helper functions to parse the vlan specification,
which consists of one or more of the following:
a) single VLAN (e.g. 10).
b) contiguous range of VLANs (e.g. 10-15).
c) discontiguous range with base, increment and count
(e.g. 100+10x9 which gives VLAN IDs 100, 110, ... 190).
A single VLAN can be suffixed with "p" to indicate the PVID, or
"u" to indicate untagged. A range of VLANs can be suffixed with
"u" to indicate untagged. A complex example would be:
vlan=1p/10-15/20-25u
This capability requires the iproute2 bridge command to be
installed. An error will be generated if the vlan parameter is
set and the bridge command is not available.
Signed-off-by: Leigh Brown
---
tools/hotplug/Linux/xen-network-common.sh | 109 ++
1 file changed, 109 insertions(+)
diff --git a/tools/hotplug/Linux/xen-network-common.sh
b/tools/hotplug/Linux/xen-network-common.sh
index 42fa704e8d..31d359b83c 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -121,10 +121,111 @@ create_bridge () {
fi
}
+_vif_vlan_add() {
+# References vlans and pvid variables from the calling function
+local -i vid=$1
+local flag=${2:-}
+
+if (( vid < 1 || vid > 4094 )) ;then
+fatal "vlan id $vid not between 1 and 4094"
+fi
+if [[ -n "${vlans[$vid]}" ]] ;then
+fatal "vlan id $vid specified more than once"
+fi
+case $flag in
+ p) if (( pvid != 0 )) ;then
+fatal "more than one pvid specified ($vid and $pvid)"
+fi
+pvid=$vid
+vlans[$vid]=p ;;
+ u) vlans[$vid]=u ;;
+ *) vlans[$vid]=t ;;
+esac
+}
+
+_vif_vlan_parse_term() {
+local vid incr last term=${1:-}
+
+if [[ $term =~ ^([0-9]+)([pu])?$ ]] ;then
+_vif_vlan_add ${BASH_REMATCH[1]} ${BASH_REMATCH[2]}
+elif [[ $term =~ ^([0-9]+)-([0-9]+)(u)?$ ]] ;then
+vid=${BASH_REMATCH[1]}
+last=${BASH_REMATCH[2]}
+if (( last >= vid )) ;then
+for (( ; vid<=last; vid++ )) ;do
+_vif_vlan_add $vid ${BASH_REMATCH[3]}
+done
+else
+fatal "invalid vlan id range: $term"
+fi
+elif [[ $term =~ ^([0-9]+)\+([0-9]+)x([0-9]+)(u)?$ ]] ;then
+vid=${BASH_REMATCH[1]}
+incr=${BASH_REMATCH[2]}
+for (( j=${BASH_REMATCH[3]}; j>0; --j, vid+=incr ))
+do
+_vif_vlan_add $vid ${BASH_REMATCH[4]}
+done
+else
+fatal "invalid vlan specification: $term"
+fi
+}
+
+_vif_vlan_validate_pvid() {
+# References vlans and pvid variables from the calling function
+if (( pvid == 0 )) ;then
+if (( ${#vlans[@]} == 1 )) ;then
+vlans[${!vlans[*]}]=p
+else
+fatal "pvid required when using multiple vlan ids"
+fi
+fi
+}
+
+_vif_vlan_setup() {
+# References vlans and dev variable from the calling function
+local -i vid
+local -a args
+
+# Remove the default vlan id automatically added to the vif
+bridge vlan del dev $dev vid 1
+
+# Add the required vlans
+for vid in ${!vlans[@]} ;do
+case ${vlans[$vid]} in
+ p) args=(pvid untagged) ;;
+ u) args=(untagged) ;;
+ t) args=() ;;
+esac
+bridge vlan add dev $dev vid $vid ${args[@]}
+done
+}
+
+_vif_vlan_membership() {
+# The vlans, pvid and dev variables are used by sub-functions
+local -A vlans=()
+local -a terms=()
+local -i i pvid=0
+local dev=$1 term
+
+# Split the vlan specification string into its terms, removing the newline
+# that readarray adds to the last element
+readarray -d / -t terms <<<$2
+terms[-1]=${terms[-1]%%[[:space:]]}
+
+for term in ${terms[@]} ;do
+_vif_vlan_parse_term $term
+done
+
+_vif_vlan_validate_pvid
+_vif_vlan_setup
+return 0
+}
+
# Usage: add_to_bridge bridge dev
add_to_bridge () {
local bridge=$1
local dev=$2
+local vlan=$(xenstore_read_default "$XENBUS_PATH/vlan" "")
# Don't add $dev to $bridge if it's already on the bridge.
if [ ! -e "/sys/class/net/${bridge}/brif/${dev}" ]; then
@@ -134,6 +235,14 @@ add_to_bridge () {
else
ip link set ${dev} master ${bridge}
fi
+if [ -n "${vlan}" ] ;then
+log debug "configuring vlans for ${dev} on ${bridge}"
+if which bridge >&/dev/null; then
+_vif_vlan_membership "${dev}" "${vlan}"
+else
+fatal "vlan configuration failed: bridge command not found"
+fi
+fi
else
log debug "$dev already on bridge $bridge"
fi
--
2.39.2
[PATCH for-4.19] x86/msi: prevent watchdog triggering when dumping MSI state
Use the same check that's used in dump_irqs(). Signed-off-by: Roger Pau Monné --- xen/arch/x86/msi.c | 4 1 file changed, 4 insertions(+) diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c index 19830528b65a..0c97fbb3fc03 100644 --- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -1451,6 +1452,9 @@ static void cf_check dump_msi(unsigned char key) unsigned long flags; const char *type = "???"; +if ( !(irq & 0x1f) ) +process_pending_softirqs(); + if ( !irq_desc_initialized(desc) ) continue; -- 2.44.0
[libvirt test] 186025: tolerable all pass - PUSHED
flight 186025 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/186025/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 186011 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-vhd 15 saverestore-support-checkfail never pass version targeted for testing: libvirt 34f52aec286af47066484e8a96ecba0ef8e79451 baseline version: libvirt 8b133e82fc62188b04dcdb2ddfc5589a48222a2c Last test of basis 186011 2024-05-16 04:20:35 Z1 days Testing same since 186025 2024-05-17 04:20:41 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Peter Krempa Rayhan Faizel jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-amd64-libvirt-qcow2 pass test-arm64-arm64-libvirt-qcow2 pass test-amd64-amd64-libvirt-raw pass test-arm64-arm64-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass test-armhf-armhf-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs 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/libvirt.git 8b133e82fc..34f52aec28 34f52aec286af47066484e8a96ecba0ef8e79451 -> xen-tested-master
[PATCH v10 13/14] xen/riscv: enable full Xen build
Signed-off-by: Oleksii Kurochko
Reviewed-by: Jan Beulich
---
Changes in V5-V10:
- Nothing changed. Only rebase.
---
Changes in V4:
- drop stubs for irq_actor_none() and irq_actor_none() as common/irq.c is
compiled now.
- drop defintion of max_page in stubs.c as common/page_alloc.c is compiled now.
- drop printk() related changes in riscv/early_printk.c as common version will
be used.
---
Changes in V3:
- Reviewed-by: Jan Beulich
- unrealted change dropped in tiny64_defconfig
---
Changes in V2:
- Nothing changed. Only rebase.
---
xen/arch/riscv/Makefile | 16 +++-
xen/arch/riscv/arch.mk| 4 -
xen/arch/riscv/early_printk.c | 168 --
xen/arch/riscv/stubs.c| 24 -
4 files changed, 15 insertions(+), 197 deletions(-)
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 60afbc0ad9..81b77b13d6 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -12,10 +12,24 @@ $(TARGET): $(TARGET)-syms
$(OBJCOPY) -O binary -S $< $@
$(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
- $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@
+ $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< \
+ $(objtree)/common/symbols-dummy.o -o $(dot-target).0
+ $(NM) -pa --format=sysv $(dot-target).0 \
+ | $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
+ > $(dot-target).0.S
+ $(MAKE) $(build)=$(@D) $(dot-target).0.o
+ $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< \
+ $(dot-target).0.o -o $(dot-target).1
+ $(NM) -pa --format=sysv $(dot-target).1 \
+ | $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
+ > $(dot-target).1.S
+ $(MAKE) $(build)=$(@D) $(dot-target).1.o
+ $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
+ $(dot-target).1.o -o $@
$(NM) -pa --format=sysv $@ \
| $(objtree)/tools/symbols --all-symbols --xensyms --sysv
--sort \
> $@.map
+ rm -f $(@D)/.$(@F).[0-9]*
$(obj)/xen.lds: $(src)/xen.lds.S FORCE
$(call if_changed_dep,cpp_lds_S)
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 8c071aff65..17827c302c 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -38,7 +38,3 @@ extensions := $(subst $(space),,$(extensions))
# -mcmodel=medlow would force Xen into the lower half.
CFLAGS += $(riscv-generic-flags)$(extensions) -mstrict-align -mcmodel=medany
-
-# TODO: Drop override when more of the build is working
-override ALL_OBJS-y = arch/$(SRCARCH)/built_in.o
-override ALL_LIBS-y =
diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c
index 60742a042d..610c814f54 100644
--- a/xen/arch/riscv/early_printk.c
+++ b/xen/arch/riscv/early_printk.c
@@ -40,171 +40,3 @@ void early_printk(const char *str)
str++;
}
}
-
-/*
- * The following #if 1 ... #endif should be removed after printk
- * and related stuff are ready.
- */
-#if 1
-
-#include
-#include
-
-/**
- * strlen - Find the length of a string
- * @s: The string to be sized
- */
-size_t (strlen)(const char * s)
-{
-const char *sc;
-
-for (sc = s; *sc != '\0'; ++sc)
-/* nothing */;
-return sc - s;
-}
-
-/**
- * memcpy - Copy one area of memory to another
- * @dest: Where to copy to
- * @src: Where to copy from
- * @count: The size of the area.
- *
- * You should not use this function to access IO space, use memcpy_toio()
- * or memcpy_fromio() instead.
- */
-void *(memcpy)(void *dest, const void *src, size_t count)
-{
-char *tmp = (char *) dest, *s = (char *) src;
-
-while (count--)
-*tmp++ = *s++;
-
-return dest;
-}
-
-int vsnprintf(char* str, size_t size, const char* format, va_list args)
-{
-size_t i = 0; /* Current position in the output string */
-size_t written = 0; /* Total number of characters written */
-char* dest = str;
-
-while ( format[i] != '\0' && written < size - 1 )
-{
-if ( format[i] == '%' )
-{
-i++;
-
-if ( format[i] == '\0' )
-break;
-
-if ( format[i] == '%' )
-{
-if ( written < size - 1 )
-{
-dest[written] = '%';
-written++;
-}
-i++;
-continue;
-}
-
-/*
- * Handle format specifiers.
- * For simplicity, only %s and %d are implemented here.
- */
-
-if ( format[i] == 's' )
-{
-char* arg = va_arg(args, char*);
-size_t arglen = strlen(arg);
-
-size_t remaining = size - written - 1;
-
-if ( arglen > remaining )
-arglen = remaining;
-
-memcpy(dest + written, arg, arglen);
-
-written += arglen;
-
[PATCH v10 10/14] xen/riscv: add minimal stuff to mm.h to build full Xen
Signed-off-by: Oleksii Kurochko
Acked-by: Jan Beulich
---
Changes in V8-V10:
- Nothing changed only rebase.
---
Changes in V7:
- update argument type of maddr_to_virt() function: unsigned long -> paddr_t
- rename argument of PFN_ORDER(): pfn -> pg.
- add Acked-by: Jan Beulich
---
Changes in V6:
- drop __virt_to_maddr() ( transform to macro ) and __maddr_to_virt ( rename
to maddr_to_virt ).
- parenthesize va in definition of vmap_to_mfn().
- Code style fixes.
---
Changes in V5:
- update the comment around "struct domain *domain;" : zero -> NULL
- fix ident. for unsigned long val;
- put page_to_virt() and virt_to_page() close to each other.
- drop unnessary leading underscore
- drop a space before the comment: /* Count of uses of this frame as its
current type. */
- drop comment about a page 'not as a shadow'. it is not necessary for RISC-V
---
Changes in V4:
- update an argument name of PFN_ORDERN macros.
- drop pad at the end of 'struct page_info'.
- Change message -> subject in "Changes in V3"
- delete duplicated macros from riscv/mm.h
- fix identation in struct page_info
- align comment for PGC_ macros
- update definitions of domain_set_alloc_bitsize() and
domain_clamp_alloc_bitsize()
- drop unnessary comments.
- s/BUG/BUG_ON("...")
- define __virt_to_maddr, __maddr_to_virt as stubs
- add inclusion of xen/mm-frame.h for mfn_x and others
- include "xen/mm.h" instead of "asm/mm.h" to fix compilation issues:
In file included from arch/riscv/setup.c:7:
./arch/riscv/include/asm/mm.h:60:28: error: field 'list' has incomplete
type
60 | struct page_list_entry list;
|^~~~
./arch/riscv/include/asm/mm.h:81:43: error: 'MAX_ORDER' undeclared here
(not in a function)
81 | unsigned long first_dirty:MAX_ORDER + 1;
| ^
./arch/riscv/include/asm/mm.h:81:31: error: bit-field 'first_dirty'
width not an integer constant
81 | unsigned long first_dirty:MAX_ORDER + 1;
- Define __virt_to_mfn() and __mfn_to_virt() using maddr_to_mfn() and
mfn_to_maddr().
---
Changes in V3:
- update the commit title
- introduce DIRECTMAP_VIRT_START.
- drop changes related pfn_to_paddr() and paddr_to_pfn as they were remvoe in
[PATCH v2 32/39] xen/riscv: add minimal stuff to asm/page.h to build full Xen
- code style fixes.
- drop get_page_nr and put_page_nr as they don't need for time being
- drop CONFIG_STATIC_MEMORY related things
- code style fixes
---
Changes in V2:
- define stub for arch_get_dma_bitsize(void)
---
xen/arch/riscv/include/asm/mm.h | 240
xen/arch/riscv/mm.c | 2 +-
xen/arch/riscv/setup.c | 2 +-
3 files changed, 242 insertions(+), 2 deletions(-)
diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 07c7a0abba..cc4a07a71c 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -3,11 +3,246 @@
#ifndef _ASM_RISCV_MM_H
#define _ASM_RISCV_MM_H
+#include
+#include
+#include
+#include
+#include
+
#include
#define pfn_to_paddr(pfn) ((paddr_t)(pfn) << PAGE_SHIFT)
#define paddr_to_pfn(pa) ((unsigned long)((pa) >> PAGE_SHIFT))
+#define paddr_to_pdx(pa)mfn_to_pdx(maddr_to_mfn(pa))
+#define gfn_to_gaddr(gfn) pfn_to_paddr(gfn_x(gfn))
+#define gaddr_to_gfn(ga)_gfn(paddr_to_pfn(ga))
+#define mfn_to_maddr(mfn) pfn_to_paddr(mfn_x(mfn))
+#define maddr_to_mfn(ma)_mfn(paddr_to_pfn(ma))
+#define vmap_to_mfn(va) maddr_to_mfn(virt_to_maddr((vaddr_t)(va)))
+#define vmap_to_page(va)mfn_to_page(vmap_to_mfn(va))
+
+static inline void *maddr_to_virt(paddr_t ma)
+{
+BUG_ON("unimplemented");
+return NULL;
+}
+
+#define virt_to_maddr(va) ({ BUG_ON("unimplemented"); 0; })
+
+/* Convert between Xen-heap virtual addresses and machine frame numbers. */
+#define __virt_to_mfn(va) mfn_x(maddr_to_mfn(virt_to_maddr(va)))
+#define __mfn_to_virt(mfn) maddr_to_virt(mfn_to_maddr(_mfn(mfn)))
+
+/*
+ * We define non-underscored wrappers for above conversion functions.
+ * These are overriden in various source files while underscored version
+ * remain intact.
+ */
+#define virt_to_mfn(va) __virt_to_mfn(va)
+#define mfn_to_virt(mfn)__mfn_to_virt(mfn)
+
+struct page_info
+{
+/* Each frame can be threaded onto a doubly-linked list. */
+struct page_list_entry list;
+
+/* Reference count and various PGC_xxx flags and fields. */
+unsigned long count_info;
+
+/* Context-dependent fields follow... */
+union {
+/* Page is in use: ((count_info & PGC_count_mask) != 0). */
+struct {
+/* Type reference count and various PGT_xxx flags and fields. */
+unsigned long type_info;
+} inuse;
+
+/* Page is on a free list: ((count_info & PGC_count_mask) == 0). */
+union {
+
[PATCH v10 11/14] xen/riscv: introduce vm_event_*() functions
Signed-off-by: Oleksii Kurochko
---
Changes in V5-V10:
- Only rebase was done.
---
Changes in V4:
- New patch.
---
xen/arch/riscv/Makefile | 1 +
xen/arch/riscv/vm_event.c | 19 +++
2 files changed, 20 insertions(+)
create mode 100644 xen/arch/riscv/vm_event.c
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 2fefe14e7c..1ed1a8369b 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_RISCV_64) += riscv64/
obj-y += sbi.o
obj-y += setup.o
obj-y += traps.o
+obj-y += vm_event.o
$(TARGET): $(TARGET)-syms
$(OBJCOPY) -O binary -S $< $@
diff --git a/xen/arch/riscv/vm_event.c b/xen/arch/riscv/vm_event.c
new file mode 100644
index 00..bb1fc73bc1
--- /dev/null
+++ b/xen/arch/riscv/vm_event.c
@@ -0,0 +1,19 @@
+#include
+
+struct vm_event_st;
+struct vcpu;
+
+void vm_event_fill_regs(struct vm_event_st *req)
+{
+BUG_ON("unimplemented");
+}
+
+void vm_event_set_registers(struct vcpu *v, struct vm_event_st *rsp)
+{
+BUG_ON("unimplemented");
+}
+
+void vm_event_monitor_next_interrupt(struct vcpu *v)
+{
+/* Not supported on RISCV. */
+}
--
2.45.0
[PATCH v10 06/14] xen/riscv: introduce atomic.h
Initially the patch was introduced by Bobby, who takes the header from
Linux kernel.
The following changes were done on top of Bobby's changes:
- atomic##prefix##_*xchg_*(atomic##prefix##_t *v, c_t n) were updated
to use__*xchg_generic()
- drop casts in write_atomic() as they are unnecessary
- drop introduction of WRITE_ONCE() and READ_ONCE().
Xen provides ACCESS_ONCE()
- remove zero-length array access in read_atomic()
- drop defines similar to pattern:
#define atomic_add_return_relaxed atomic_add_return_relaxed
- move not RISC-V specific functions to asm-generic/atomics-ops.h
- drop atomic##prefix##_{cmp}xchg_{release, aquire, release}() as they
are not used in Xen.
- update the defintion of atomic##prefix##_{cmp}xchg according to
{cmp}xchg() implementation in Xen.
- some ATOMIC_OP() macros were updated:
- drop size argument for ATOMIC_OP which defines atomic##prefix##_xchg()
and atomic##prefix##_cmpxchg().
- drop c_op argument for ATOMIC_OPS which defines ATOMIC_OPS(and, and),
ATOMIC_OPS( or, or), ATOMIC_OPS(xor, xor), ATOMIC_OPS(add, add, +),
ATOMIC_OPS(sub, add, -) as c_op is always "+" for them.
- drop "" from definition of __atomic_{acquire/release"}_fence.
The current implementation is the same with 8e86f0b409a4
("arm64: atomics: fix use of acquire + release for full barrier
semantics") [1].
RISC-V could combine acquire and release into the SC
instructions and it could reduce a fence instruction to gain better
performance. Here is related description from RISC-V ISA 10.2
Load-Reserved/Store-Conditional Instructions:
- .aq: The LR/SC sequence can be given acquire semantics by
setting the aq bit on the LR instruction.
- .rl: The LR/SC sequence can be given release semantics by
setting the rl bit on the SC instruction.
- .aqrl: Setting the aq bit on the LR instruction, and setting
both the aq and the rl bit on the SC instruction makes
the LR/SC sequence sequentially consistent, meaning that
it cannot be reordered with earlier or later memory
operations from the same hart.
Software should not set the rl bit on an LR instruction unless
the aq bit is also set, nor should software set the aq bit on an
SC instruction unless the rl bit is also set. LR.rl and SC.aq
instructions are not guaranteed to provide any stronger ordering
than those with both bits clear, but may result in lower
performance.
Also, I way of transforming ".rl + full barrier" to ".aqrl" was approved
by (the author of the RVWMO spec) [2]
[1]
https://patchwork.kernel.org/project/linux-arm-kernel/patch/1391516953-14541-1-git-send-email-will.dea...@arm.com/
[2]
https://lore.kernel.org/linux-riscv/41e01514-74ca-84f2-f5cc-2645c444f...@nvidia.com/
Signed-off-by: Bobby Eshleman
Signed-off-by: Oleksii Kurochko
Acked-by: Jan Beulich
---
Changes in V10:
- drop unnessary parentheses around p in add_sized()
- add Acked-by: Jan Beulich
---
Changes in V9:
- update the defintion of write_atomic macros:
drop the return value as this macros isn't expeceted to return something
drop unnessary parentheses around p.
- drop casts inside _add_sized() for ptr variable as they aren't necessary.
---
Changes in V8:
- add the explanatory comment to _add_sized().
- drop "" in __atomic_{acquire, release}_fence().
- code style fixes in atomic##prefix##_##op##_return(): indentation.
- drop an unary_op argument ("+") for ATOMIC_OPS(and, and), ATOMIC_OPS( or,
or), ATOMIC_OPS(xor, xor)
and use "+" directly inside definition of ATOMIC_OPS().
- drop c_op for ATOMIC_OPS(add, add, +) and ATOMIC_OPS(sub, add, -) as it is
always "+" for now.
Just use "+" inside definition of ATOMIC_OPS().
- drop size argument for ATOMIC_OP() defintions of
atomic##prefix##_{xchg,cmpxchg}()
- update the commit message.
---
Changes in V7:
- drop relaxed version of atomic ops as they are not used.
- update the commit message
- code style fixes
- refactor functions write_atomic(), add_sized() to be able to use #ifdef
CONFIG_RISCV_32 ... #endif
for {write,read}q().
- update ATOMIC_OPS to receive unary operator.
- update the header on top of atomic-ops.h.
- some minor movements of function inside atomic-ops.h header.
---
Changes in V6:
- drop atomic##prefix##_{cmp}xchg_{release, aquire, relaxed} as they aren't
used
by Xen
- code style fixes.
- %s/__asm__ __volatile__/asm volatile
- add explanational comments.
- move inclusion of "#include " further down in
atomic.h
header.
---
Changes in V5:
- fence.h changes were moved to separate patch as patches related to io.h and
cmpxchg.h,
which are dependecies for this patch, also needed changes in fence.h
- remove accessing of zero-length array
- drops cast in write_atomic()
- drop introduction of WRITE_ONCE() and READ_ONCE().
- drop defines similar to pattern #define atomic_add_return_relaxed
atomic_add_return_relaxed
- Xen code style fixes
- move not
[PATCH v10 12/14] xen/riscv: add minimal amount of stubs to build full Xen
Signed-off-by: Oleksii Kurochko
Acked-by: Jan Beulich
---
Changes in V7-V10:
- Only rebase was done.
---
Changes in V6:
- update the commit in stubs.c around /* ... common/irq.c ... */
- add Acked-by: Jan Beulich
---
Changes in V5:
- drop unrelated changes
- assert_failed("unimplmented...") change to BUG_ON()
---
Changes in V4:
- added new stubs which are necessary for compilation after rebase:
__cpu_up(), __cpu_disable(), __cpu_die()
from smpboot.c
- back changes related to printk() in early_printk() as they should be
removed in the next patch to avoid
compilation error.
- update definition of cpu_khz: __read_mostly -> __ro_after_init.
- drop vm_event_reset_vmtrace(). It is defibed in asm-generic/vm_event.h.
- move vm_event_*() functions from stubs.c to riscv/vm_event.c.
- s/BUG/BUG_ON("unimplemented") in stubs.c
- back irq_actor_none() and irq_actor_none() as common/irq.c isn't compiled
at this moment,
so this function are needed to avoid compilation error.
- defined max_page to avoid compilation error, it will be removed as soon as
common/page_alloc.c will
be compiled.
---
Changes in V3:
- code style fixes.
- update attribute for frametable_base_pdx and frametable_virt_end to
__ro_after_init.
insteaf of read_mostly.
- use BUG() instead of assert_failed/WARN for newly introduced stubs.
- drop "#include " in stubs.c and use forward declaration
instead.
- drop ack_node() and end_node() as they aren't used now.
---
Changes in V2:
- define udelay stub
- remove 'select HAS_PDX' from RISC-V Kconfig because of
https://lore.kernel.org/xen-devel/20231006144405.1078260-1-andrew.coop...@citrix.com/
---
xen/arch/riscv/Makefile | 1 +
xen/arch/riscv/mm.c | 50 +
xen/arch/riscv/setup.c | 8 +
xen/arch/riscv/stubs.c | 439
xen/arch/riscv/traps.c | 25 +++
5 files changed, 523 insertions(+)
create mode 100644 xen/arch/riscv/stubs.c
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 1ed1a8369b..60afbc0ad9 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -4,6 +4,7 @@ obj-y += mm.o
obj-$(CONFIG_RISCV_64) += riscv64/
obj-y += sbi.o
obj-y += setup.o
+obj-y += stubs.o
obj-y += traps.o
obj-y += vm_event.o
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index fe3a43be20..2c3fb7d72e 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
+#include
#include
#include
#include
@@ -14,6 +15,9 @@
#include
#include
+unsigned long __ro_after_init frametable_base_pdx;
+unsigned long __ro_after_init frametable_virt_end;
+
struct mmu_desc {
unsigned int num_levels;
unsigned int pgtbl_count;
@@ -294,3 +298,49 @@ unsigned long __init calc_phys_offset(void)
phys_offset = load_start - XEN_VIRT_START;
return phys_offset;
}
+
+void put_page(struct page_info *page)
+{
+BUG_ON("unimplemented");
+}
+
+unsigned long get_upper_mfn_bound(void)
+{
+/* No memory hotplug yet, so current memory limit is the final one. */
+return max_page - 1;
+}
+
+void arch_dump_shared_mem_info(void)
+{
+BUG_ON("unimplemented");
+}
+
+int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
+{
+BUG_ON("unimplemented");
+return -1;
+}
+
+int xenmem_add_to_physmap_one(struct domain *d, unsigned int space,
+ union add_to_physmap_extra extra,
+ unsigned long idx, gfn_t gfn)
+{
+BUG_ON("unimplemented");
+
+return 0;
+}
+
+int destroy_xen_mappings(unsigned long s, unsigned long e)
+{
+BUG_ON("unimplemented");
+return -1;
+}
+
+int map_pages_to_xen(unsigned long virt,
+ mfn_t mfn,
+ unsigned long nr_mfns,
+ unsigned int flags)
+{
+BUG_ON("unimplemented");
+return -1;
+}
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 98a94c4c48..8bb5bdb2ae 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,11 +1,19 @@
/* SPDX-License-Identifier: GPL-2.0-only */
+#include
#include
#include
#include
+#include
+
#include
+void arch_get_xen_caps(xen_capabilities_info_t *info)
+{
+BUG_ON("unimplemented");
+}
+
/* Xen stack for bringing up the first CPU. */
unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
__aligned(STACK_SIZE);
diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
new file mode 100644
index 00..8285bcffef
--- /dev/null
+++ b/xen/arch/riscv/stubs.c
@@ -0,0 +1,439 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+/* smpboot.c */
+
+cpumask_t cpu_online_map;
+cpumask_t cpu_present_map;
+cpumask_t cpu_possible_map;
+
+/* ID of the PCPU we're running on */
+DEFINE_PER_CPU(unsigned int, cpu_id);
+/* XXX these seem awfully x86ish... */
+/* representing HT siblings of each logical CPU
[PATCH v10 05/14] xen/riscv: introduce cmpxchg.h
The header was taken from Linux kernl 6.4.0-rc1.
Addionally, were updated:
* add emulation of {cmp}xchg for 1/2 byte types using 32-bit atomic
access.
* replace tabs with spaces
* replace __* variale with *__
* introduce generic version of xchg_* and cmpxchg_*.
* drop {cmp}xchg{release,relaxed,acquire} as Xen doesn't use them
* drop barries and use instruction suffixices instead ( .aq, .rl, .aqrl )
Implementation of 4- and 8-byte cases were updated according to the spec:
```
Linux Construct RVWMO AMO Mapping
...
atomic amo.{w|d}.aqrl
Linux Construct RVWMO LR/SC Mapping
...
atomic loop: lr.{w|d}.aq; ; sc.{w|d}.aqrl; bnez loop
Table A.5: Mappings from Linux memory primitives to RISC-V primitives
```
The current implementation is the same with 8e86f0b409a4
("arm64: atomics: fix use of acquire + release for full barrier
semantics") [1].
RISC-V could combine acquire and release into the SC
instructions and it could reduce a fence instruction to gain better
performance. Here is related description from RISC-V ISA 10.2
Load-Reserved/Store-Conditional Instructions:
- .aq: The LR/SC sequence can be given acquire semantics by
setting the aq bit on the LR instruction.
- .rl: The LR/SC sequence can be given release semantics by
setting the rl bit on the SC instruction.
- .aqrl: Setting the aq bit on the LR instruction, and setting
both the aq and the rl bit on the SC instruction makes
the LR/SC sequence sequentially consistent, meaning that
it cannot be reordered with earlier or later memory
operations from the same hart.
Software should not set the rl bit on an LR instruction unless
the aq bit is also set, nor should software set the aq bit on an
SC instruction unless the rl bit is also set. LR.rl and SC.aq
instructions are not guaranteed to provide any stronger ordering
than those with both bits clear, but may result in lower
performance.
Also, I way of transforming ".rl + full barrier" to ".aqrl" was approved
by (the author of the RVWMO spec) [2]
[1]
https://patchwork.kernel.org/project/linux-arm-kernel/patch/1391516953-14541-1-git-send-email-will.dea...@arm.com/
[2]
https://lore.kernel.org/linux-riscv/41e01514-74ca-84f2-f5cc-2645c444f...@nvidia.com/
Signed-off-by: Oleksii Kurochko
Acked-by: Jan Beulich
---
Changes in V10:
- Code style fixes for __xchg, __cmpxchg ( the line was too long )
- change type of size argument from int to unsigned int for
__bad_{cmp}xchg(), __{cmp}xchg().
- drop parentheses around ptr in {cmp}xchg() when ptr is an argument of
__{cmp}xchg().
- add Acked-by: Jan Beulich
---
Changes in V9:
- update return type of __bad_xchg();
- update the comment above __bad_cmpxchg();
- update the default case inside __xchg() to be aligned with similar default
case in __cmpxchg().
---
Changes in V8:
- use __bad_{xchg,cmpxch}(ptr,size) insetead of STATIC_ASSERT_UNREACHABLE() to
make this patch be independent from the macros that haven't been committed
yet
and may never be.
---
Changes in V7:
- replace __*() -> _*() in cmpxchg.h
- add () around ptr in _amoswap_generic(), emulate_xchg_1_2()
- fix typos
- code style fixes.
- refactor emulate_xcgh_1_2():
- add parentheses for new argument.
- use instead of constant 0x4 -> sizeof(*aligned_ptr).
- add alignment_mask to save sizeof(*aligned_ptr) - sizeof(*(ptr));
- s/CONFIG_32BIT/CONFIG_RISCV_32
- drop unnecessary parentheses in xchg()
- drop register in _generic_cmpxchg()
- refactor and update prototype of _generic_cmpxchg():
add named operands, return value instead of passing ret as an argument, drop
%z and J
constraints for mask operand as it can't be zero
- refactor and code style fixes in emulate_cmpxchg_1_2():
- add explanatory comment for emulate_cmpxchg_1_2().
- add parentheses for old and new arguments.
- use instead of constant 0x4 -> sizeof(*aligned_ptr).
- add alignment_mask to save sizeof(*aligned_ptr) - sizeof(*(ptr));
- drop unnessary parenthesses in cmpxchg().
- update the commit message.
- s/__asm__ __volatile__/asm volatile
---
Changes in V6:
- update the commit message? ( As before I don't understand this point. Can
you give an example of what sort of opcode / instruction is missing?)
- Code style fixes
- change sizeof(*ptr) -> sizeof(*(ptr))
- update operands names and some local variables for macros emulate_xchg_1_2()
and emulate_cmpxchg_1_2()
- drop {cmp}xchg_{relaxed,acquire,release) versions as they aren't needed for
Xen
- update __amoswap_generic() prototype and defintion: drop pre and post
barries.
- update emulate_xchg_1_2() prototype and definion: add lr_sfx, drop pre and
post barries.
- rename __xchg_generic to __xchg(), make __xchg as static inline function to
be able to "#ifndef CONFIG_32BIT case 8:... "
---
Changes in V5:
- update the commit message.
- drop ALIGN_DOWN().
- update the definition
[PATCH v10 09/14] xen/riscv: add required things to current.h
Add minimal requied things to be able to build full Xen.
Signed-off-by: Oleksii Kurochko
Acked-by: Jan Beulich
---
Changes in V5-V10:
- Nothing changed. Only rebase.
---
Changes in V4:
- BUG() was changed to BUG_ON("unimplemented");
- Change "xen/bug.h" to "xen/lib.h" as BUG_ON is defined in xen/lib.h.
- Add Acked-by: Jan Beulich
---
Changes in V3:
- add SPDX
- drop a forward declaration of struct vcpu;
- update guest_cpu_user_regs() macros
- replace get_processor_id with smp_processor_id
- update the commit message
- code style fixes
---
Changes in V2:
- Nothing changed. Only rebase.
---
xen/arch/riscv/include/asm/current.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/xen/arch/riscv/include/asm/current.h
b/xen/arch/riscv/include/asm/current.h
index d84f15dc50..aedb6dc732 100644
--- a/xen/arch/riscv/include/asm/current.h
+++ b/xen/arch/riscv/include/asm/current.h
@@ -3,6 +3,21 @@
#ifndef __ASM_CURRENT_H
#define __ASM_CURRENT_H
+#include
+#include
+#include
+
+#ifndef __ASSEMBLY__
+
+/* Which VCPU is "current" on this PCPU. */
+DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
+
+#define currentthis_cpu(curr_vcpu)
+#define set_current(vcpu) do { current = (vcpu); } while (0)
+#define get_cpu_current(cpu) per_cpu(curr_vcpu, cpu)
+
+#define guest_cpu_user_regs() ({ BUG_ON("unimplemented"); NULL; })
+
#define switch_stack_and_jump(stack, fn) do { \
asm volatile ( \
"mv sp, %0\n" \
@@ -10,4 +25,8 @@
unreachable(); \
} while ( false )
+#define get_per_cpu_offset() __per_cpu_offset[smp_processor_id()]
+
+#endif /* __ASSEMBLY__ */
+
#endif /* __ASM_CURRENT_H */
--
2.45.0
[PATCH v10 03/14] xen/bitops: implement fls{l}() in common logic
To avoid the compilation error below, it is needed to update to places
in common/page_alloc.c where flsl() is used as now flsl() returns unsigned int:
./include/xen/kernel.h:18:21: error: comparison of distinct pointer types lacks
a cast [-Werror]
18 | (void) (&_x == &_y);\
| ^~
common/page_alloc.c:1843:34: note: in expansion of macro 'min'
1843 | unsigned int inc_order = min(MAX_ORDER, flsl(e - s) - 1);
generic_fls{l} was used instead of __builtin_clz{l}(x) as if x is 0,
the result in undefined.
The prototype of the per-architecture fls{l}() functions was changed to
return 'unsigned int' to align with the generic implementation of these
functions and avoid introducing signed/unsigned mismatches.
Signed-off-by: Oleksii Kurochko
---
The patch is almost independent from Andrew's patch series
(
https://lore.kernel.org/xen-devel/20240313172716.2325427-1-andrew.coop...@citrix.com/T/#t)
except test_fls() function which IMO can be merged as a separate patch after
Andrew's patch
will be fully ready.
---
Changes in V10:
- update return type of arch_flsl() across arcitectures to 'unsigned int' to
be aligned
with return type of generic flsl() in xen/bitops.h.
- switch inline to always_inline for arch_flsl() across architectures to be in
sync
with other similar changes.
- define arch_flsl as arch_fls not just only fls.
- update the commit message ( add information that per-arch fls{l)() protypes
were
changed ).
---
Changes in V9:
- update return type of fls and flsl() to unsigned int to be aligned with other
bit ops.
- update places where return value of fls() and flsl() is compared with int.
- update the commit message.
---
Changes in V8:
- do proper rebase: back definition of fls{l} to the current patch.
- drop the changes which removed ffz() in PPC. it should be done not
in this patch.
- add a message after Signed-off.
---
Changes in V7:
- Code style fixes
---
Changes in V6:
- new patch for the patch series.
---
xen/arch/arm/include/asm/arm32/bitops.h | 2 +-
xen/arch/arm/include/asm/arm64/bitops.h | 6 ++
xen/arch/arm/include/asm/bitops.h | 9 +++--
xen/arch/ppc/include/asm/bitops.h | 3 ---
xen/arch/x86/include/asm/bitops.h | 12 +++-
xen/common/bitops.c | 22 ++
xen/common/page_alloc.c | 4 ++--
xen/include/xen/bitops.h| 24
8 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/include/asm/arm32/bitops.h
b/xen/arch/arm/include/asm/arm32/bitops.h
index d0309d47c1..9ee96f568b 100644
--- a/xen/arch/arm/include/asm/arm32/bitops.h
+++ b/xen/arch/arm/include/asm/arm32/bitops.h
@@ -1,7 +1,7 @@
#ifndef _ARM_ARM32_BITOPS_H
#define _ARM_ARM32_BITOPS_H
-#define flsl fls
+#define arch_flsl arch_fls
/*
* Little endian assembly bitops. nr = 0 -> byte 0 bit 0.
diff --git a/xen/arch/arm/include/asm/arm64/bitops.h
b/xen/arch/arm/include/asm/arm64/bitops.h
index 906d84e5f2..d942077392 100644
--- a/xen/arch/arm/include/asm/arm64/bitops.h
+++ b/xen/arch/arm/include/asm/arm64/bitops.h
@@ -1,17 +1,15 @@
#ifndef _ARM_ARM64_BITOPS_H
#define _ARM_ARM64_BITOPS_H
-static inline int flsl(unsigned long x)
+static always_inline unsigned int arch_flsl(unsigned long x)
{
uint64_t ret;
-if (__builtin_constant_p(x))
- return generic_flsl(x);
-
asm("clz\t%0, %1" : "=r" (ret) : "r" (x));
return BITS_PER_LONG - ret;
}
+#define arch_flsl arch_flsl
/* Based on linux/include/asm-generic/bitops/find.h */
diff --git a/xen/arch/arm/include/asm/bitops.h
b/xen/arch/arm/include/asm/bitops.h
index 8e16335e76..f428cf8338 100644
--- a/xen/arch/arm/include/asm/bitops.h
+++ b/xen/arch/arm/include/asm/bitops.h
@@ -78,17 +78,14 @@ bool clear_mask16_timeout(uint16_t mask, volatile void *p,
* the clz instruction for much better code efficiency.
*/
-static inline int fls(unsigned int x)
+static always_inline unsigned int arch_fls(unsigned int x)
{
-int ret;
-
-if (__builtin_constant_p(x))
- return generic_fls(x);
+unsigned int ret;
asm("clz\t%"__OP32"0, %"__OP32"1" : "=r" (ret) : "r" (x));
return 32 - ret;
}
-
+#define arch_fls arch_fls
#define arch_ffs(x) ({ unsigned int __t = (x); fls(ISOLATE_LSB(__t)); })
#define arch_ffsl(x) ({ unsigned long __t = (x); flsl(ISOLATE_LSB(__t)); })
diff --git a/xen/arch/ppc/include/asm/bitops.h
b/xen/arch/ppc/include/asm/bitops.h
index 049aa62b89..46154fc957 100644
--- a/xen/arch/ppc/include/asm/bitops.h
+++ b/xen/arch/ppc/include/asm/bitops.h
@@ -119,9 +119,6 @@ static inline int test_and_set_bit(unsigned int nr,
volatile void *addr)
(volatile unsigned int *)addr + BITOP_WORD(nr)) != 0;
}
-#define flsl(x) generic_flsl(x)
-#define fls(x) generic_fls(x)
-
/**
* hweightN - returns the hamming weight
[PATCH v10 14/14] xen/README: add compiler and binutils versions for RISC-V64
This patch doesn't represent a strict lower bound for GCC and GNU Binutils; rather, these versions are specifically employed by the Xen RISC-V container and are anticipated to undergo continuous testing. Older GCC and GNU Binutils would work, but this is not a guarantee. While it is feasible to utilize Clang, it's important to note that, currently, there is no Xen RISC-V CI job in place to verify the seamless functioning of the build with Clang. Signed-off-by: Oleksii Kurochko --- Changes in V5-V10: - Nothing changed. Only rebase. --- Changes in V6: - update the message in README. --- Changes in V5: - update the commit message and README file with additional explanation about GCC and GNU Binutils version. Additionally, it was added information about Clang. --- Changes in V4: - Update version of GCC (12.2) and GNU Binutils (2.39) to the version which are in Xen's contrainter for RISC-V --- Changes in V3: - new patch --- README | 4 1 file changed, 4 insertions(+) diff --git a/README b/README index c8a108449e..30da5ff9c0 100644 --- a/README +++ b/README @@ -48,6 +48,10 @@ provided by your OS distributor: - For ARM 64-bit: - GCC 5.1 or later - GNU Binutils 2.24 or later + - For RISC-V 64-bit: +- GCC 12.2 or later +- GNU Binutils 2.39 or later + Older GCC and GNU Binutils would work, but this is not a guarantee. * POSIX compatible awk * Development install of zlib (e.g., zlib-dev) * Development install of Python 2.7 or later (e.g., python-dev) -- 2.45.0
[PATCH v10 01/14] xen/riscv: disable unnecessary configs
Disables unnecessary configs for two cases:
1. By utilizing EXTRA_FIXED_RANDCONFIG for randconfig builds (GitLab CI jobs).
2. By using tiny64_defconfig for non-randconfig builds.
Only configs which lead to compilation issues were disabled.
Remove lines related to disablement of configs which aren't affected
compilation:
-# CONFIG_SCHED_CREDIT is not set
-# CONFIG_SCHED_RTDS is not set
-# CONFIG_SCHED_NULL is not set
-# CONFIG_SCHED_ARINC653 is not set
-# CONFIG_TRACEBUFFER is not set
-# CONFIG_HYPFS is not set
-# CONFIG_SPECULATIVE_HARDEN_ARRAY is not set
To allow CONFIG_ARGO build happy it was included to
as ARGO requires p2m_type_t ( p2m_ram_rw ) and declaration of
check_get_page_from_gfn() from xen/p2m-common.h.
Also, it was included to asm/p2m.h as after the latter was
included to the compilation error that EINVAL, EOPNOTSUPP
aren't declared started to occur.
CONFIG_XSM=n as it requires an introduction of:
* boot_module_find_by_kind()
* BOOTMOD_XSM
* struct bootmodule
* copy_from_paddr()
The mentioned things aren't introduced now.
CPU_BOOT_TIME_CPUPOOLS requires an introduction of cpu_physical_id() and
acpi_disabled, so it is disabled for now.
PERF_COUNTERS requires asm/perf.h and asm/perfc-defn.h, so it is
also disabled for now, as RISC-V hasn't introduced this headers yet.
LIVEPATCH isn't ready for RISC-V too and it can be overriden by randconfig,
so to avoid compilation errors for randconfig it is disabled for now.
Signed-off-by: Oleksii Kurochko
---
Changes in V10:
- Nothing changed. Only rebase.
---
Changes in V9:
- update the commit message: add info about LIVEPATCH and PERF_COUNTERS.
---
Changes in V8:
- disabled CPU_BOOT_TIME_CPUPOOLS as it requires an introduction of
cpu_physical_id() and acpi_disabled.
- leave XSM disabled, add explanation in the commit message.
- drop HYPFS as the patch was provided to resolve compilation issue when this
condif is enabled for RISC-V.
- include asm/p2m.h to asm/domain.h, and xen/errno.h to asm/p2m.h to drop ARGO
config from
tiny64_defconfing and build.yaml.
- update the commit message.
---
Changes in V7:
- Disable only configs which cause compilation issues.
- Update the commit message.
---
Changes in V6:
- Nothing changed. Only rebase.
---
Changes in V5:
- Rebase and drop duplicated configs in EXTRA_FIXED_RANDCONFIG list
- Update the commit message
---
Changes in V4:
- Nothing changed. Only rebase
---
Changes in V3:
- Remove EXTRA_FIXED_RANDCONFIG for non-randconfig jobs.
For non-randconfig jobs, it is sufficient to disable configs by using the
defconfig.
- Remove double blank lines in build.yaml file before
archlinux-current-gcc-riscv64-debug
---
Changes in V2:
- update the commit message.
- remove xen/arch/riscv/Kconfig changes.
---
automation/gitlab-ci/build.yaml | 4
xen/arch/riscv/configs/tiny64_defconfig | 12 +---
xen/arch/riscv/include/asm/domain.h | 2 ++
xen/arch/riscv/include/asm/p2m.h| 2 ++
4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 49d6265ad5..ff5c9055d1 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -494,10 +494,14 @@ alpine-3.18-gcc-debug-arm64-earlyprintk:
.riscv-fixed-randconfig:
variables:
EXTRA_FIXED_RANDCONFIG: |
+ CONFIG_BOOT_TIME_CPUPOOLS=n
CONFIG_COVERAGE=n
CONFIG_EXPERT=y
CONFIG_GRANT_TABLE=n
CONFIG_MEM_ACCESS=n
+ CONFIG_PERF_COUNTERS=n
+ CONFIG_LIVEPATCH=n
+ CONFIG_XSM=n
archlinux-current-gcc-riscv64:
extends: .gcc-riscv64-cross-build
diff --git a/xen/arch/riscv/configs/tiny64_defconfig
b/xen/arch/riscv/configs/tiny64_defconfig
index 09defe236b..fc7a04872f 100644
--- a/xen/arch/riscv/configs/tiny64_defconfig
+++ b/xen/arch/riscv/configs/tiny64_defconfig
@@ -1,12 +1,10 @@
-# CONFIG_SCHED_CREDIT is not set
-# CONFIG_SCHED_RTDS is not set
-# CONFIG_SCHED_NULL is not set
-# CONFIG_SCHED_ARINC653 is not set
-# CONFIG_TRACEBUFFER is not set
-# CONFIG_HYPFS is not set
+# CONFIG_BOOT_TIME_CPUPOOLS is not set
# CONFIG_GRANT_TABLE is not set
-# CONFIG_SPECULATIVE_HARDEN_ARRAY is not set
# CONFIG_MEM_ACCESS is not set
+# CONFIG_PERF_COUNTERS is not set
+# CONFIG_COVERAGE is not set
+# CONFIG_LIVEPATCH is not set
+# CONFIG_XSM is not set
CONFIG_RISCV_64=y
CONFIG_DEBUG=y
diff --git a/xen/arch/riscv/include/asm/domain.h
b/xen/arch/riscv/include/asm/domain.h
index 027bfa8a93..16a9dd57aa 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -5,6 +5,8 @@
#include
#include
+#include
+
struct hvm_domain
{
uint64_t params[HVM_NR_PARAMS];
diff --git a/xen/arch/riscv/include/asm/p2m.h b/xen/arch/riscv/include/asm/p2m.h
index 387f372b5d..26860c0ae7 100644
--- a/xen/arch/riscv/include/asm/p2m.h
+++ b/xen/arch/riscv/include/asm/p2m.h
@@ -2,6 +2,8 @@
#ifndef __ASM_RISCV_P2M_H__
#define __ASM_RISCV_P2M_H__
[PATCH v10 07/14] xen/riscv: introduce monitor.h
Signed-off-by: Oleksii Kurochko
---
Changes in V4-V10:
- Nothing changed. Only rebase.
---
Changes in V3:
- new patch.
---
xen/arch/riscv/include/asm/monitor.h | 26 ++
1 file changed, 26 insertions(+)
create mode 100644 xen/arch/riscv/include/asm/monitor.h
diff --git a/xen/arch/riscv/include/asm/monitor.h
b/xen/arch/riscv/include/asm/monitor.h
new file mode 100644
index 00..f4fe2c0690
--- /dev/null
+++ b/xen/arch/riscv/include/asm/monitor.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_RISCV_MONITOR_H__
+#define __ASM_RISCV_MONITOR_H__
+
+#include
+
+#include
+
+struct domain;
+
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+BUG_ON("unimplemented");
+return 0;
+}
+
+#endif /* __ASM_RISCV_MONITOR_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.45.0
[PATCH v10 04/14] xen/riscv: introduce bitops.h
Taken from Linux-6.4.0-rc1
Xen's bitops.h consists of several Linux's headers:
* linux/arch/include/asm/bitops.h:
* The following function were removed as they aren't used in Xen:
* test_and_set_bit_lock
* clear_bit_unlock
* __clear_bit_unlock
* The following functions were renamed in the way how they are
used by common code:
* __test_and_set_bit
* __test_and_clear_bit
* The declaration and implementation of the following functios
were updated to make Xen build happy:
* clear_bit
* set_bit
* __test_and_clear_bit
* __test_and_set_bit
Signed-off-by: Oleksii Kurochko
Acked-by: Jan Beulich
---
Changes in V10:
- update the error message BITS_PER_LONG -> BITOP_BITS_PER_WORD
---
Changes in V9:
- add Acked-by: Jan Beulich
- drop redefinition of bitop_uint_t in asm/types.h as some operation in Xen
common code expects
to work with 32-bit quantities.
- s/BITS_PER_LONG/BITOP_BITS_PER_WORD in asm/bitops.h around __AMO() macros.
---
Changes in V8:
- define bitop_uint_t in after the changes in patch related to
introduction of
"introduce generic non-atomic test_*bit()".
- drop duplicated __set_bit() and __clear_bit().
- drop duplicated comment: /* Based on linux/arch/include/asm/bitops.h */.
- update type of res and mask in test_and_op_bit_ord(): unsigned long ->
bitop_uint_t.
- drop 1 padding blank in test_and_op_bit_ord().
- update definition of
test_and_set_bit(),test_and_clear_bit(),test_and_change_bit:
change return type to bool.
- change addr argument type of test_and_change_bit(): unsigned long * -> void
*.
- move test_and_change_bit() closer to other test_and-s function.
- Code style fixes: tabs -> space.
- s/#undef __op_bit/#undef op_bit.
- update the commit message: delete information about generic-non-atomic.h
changes as now
it is a separate patch.
---
Changes in V7:
- Update the commit message.
- Drop "__" for __op_bit and __op_bit_ord as they are atomic.
- add comment above __set_bit and __clear_bit about why they are defined as
atomic.
- align bitops_uint_t with __AMO().
- make changes after generic non-atomic test_*bit() were changed.
- s/__asm__ __volatile__/asm volatile
---
Changes in V6:
- rebase clean ups were done: drop unused asm-generic includes
---
Changes in V5:
- new patch
---
xen/arch/riscv/include/asm/bitops.h | 137
1 file changed, 137 insertions(+)
create mode 100644 xen/arch/riscv/include/asm/bitops.h
diff --git a/xen/arch/riscv/include/asm/bitops.h
b/xen/arch/riscv/include/asm/bitops.h
new file mode 100644
index 00..7f7af3fda1
--- /dev/null
+++ b/xen/arch/riscv/include/asm/bitops.h
@@ -0,0 +1,137 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2012 Regents of the University of California */
+
+#ifndef _ASM_RISCV_BITOPS_H
+#define _ASM_RISCV_BITOPS_H
+
+#include
+
+#if BITOP_BITS_PER_WORD == 64
+#define __AMO(op) "amo" #op ".d"
+#elif BITOP_BITS_PER_WORD == 32
+#define __AMO(op) "amo" #op ".w"
+#else
+#error "Unexpected BITOP_BITS_PER_WORD"
+#endif
+
+/* Based on linux/arch/include/asm/bitops.h */
+
+/*
+ * Non-atomic bit manipulation.
+ *
+ * Implemented using atomics to be interrupt safe. Could alternatively
+ * implement with local interrupt masking.
+ */
+#define __set_bit(n, p) set_bit(n, p)
+#define __clear_bit(n, p)clear_bit(n, p)
+
+#define test_and_op_bit_ord(op, mod, nr, addr, ord) \
+({ \
+bitop_uint_t res, mask; \
+mask = BITOP_MASK(nr); \
+asm volatile ( \
+__AMO(op) #ord " %0, %2, %1"\
+: "=r" (res), "+A" (addr[BITOP_WORD(nr)]) \
+: "r" (mod(mask)) \
+: "memory");\
+((res & mask) != 0);\
+})
+
+#define op_bit_ord(op, mod, nr, addr, ord) \
+asm volatile ( \
+__AMO(op) #ord " zero, %1, %0" \
+: "+A" (addr[BITOP_WORD(nr)]) \
+: "r" (mod(BITOP_MASK(nr))) \
+: "memory");
+
+#define test_and_op_bit(op, mod, nr, addr)\
+test_and_op_bit_ord(op, mod, nr, addr, .aqrl)
+#define op_bit(op, mod, nr, addr) \
+op_bit_ord(op, mod, nr, addr, )
+
+/* Bitmask modifiers */
+#define NOP(x)(x)
+#define NOT(x)(~(x))
+
+/**
+ * test_and_set_bit - Set a bit and return its old value
+ * @nr: Bit to set
+ * @addr: Address to count from
+ */
+static inline bool test_and_set_bit(int nr, volatile void *p)
+{
+volatile bitop_uint_t *addr = p;
+
+return test_and_op_bit(or, NOP, nr, addr);
+}
+
+/**
+ * test_and_clear_bit - Clear a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ */
+static inline bool test_and_clear_bit(int nr,
[PATCH v10 08/14] xen/riscv: add definition of __read_mostly
The definition of __read_mostly should be removed in:
https://lore.kernel.org/xen-devel/f25eb5c9-7c14-6e23-8535-2c66772b3...@suse.com/
The patch introduces it in arch-specific header to not
block enabling of full Xen build for RISC-V.
Signed-off-by: Oleksii Kurochko
---
- [PATCH] move __read_mostly to xen/cache.h [2]
Right now, the patch series doesn't have a direct dependency on [2] and it
provides __read_mostly in the patch:
[PATCH v3 26/34] xen/riscv: add definition of __read_mostly
However, it will be dropped as soon as [2] is merged or at least when the
final version of the patch [2] is provided.
Considering that there is still no still final decision regarding patch [2] my
suggestion
is to merge RISC-V specific patch and just drop the changes in patch [2].
[2]
https://lore.kernel.org/xen-devel/f25eb5c9-7c14-6e23-8535-2c66772b3...@suse.com/
---
Changes in V9-V10:
- Only rebase was done.
---
Change in V8:
- update the footer after Signed-off.
---
Changes in V4-V7:
- Nothing changed. Only rebase.
---
xen/arch/riscv/include/asm/cache.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/xen/arch/riscv/include/asm/cache.h
b/xen/arch/riscv/include/asm/cache.h
index 69573eb051..94bd94db53 100644
--- a/xen/arch/riscv/include/asm/cache.h
+++ b/xen/arch/riscv/include/asm/cache.h
@@ -3,4 +3,6 @@
#ifndef _ASM_RISCV_CACHE_H
#define _ASM_RISCV_CACHE_H
+#define __read_mostly __section(".data.read_mostly")
+
#endif /* _ASM_RISCV_CACHE_H */
--
2.45.0
[PATCH v10 00/14] Enable build of full Xen for RISC-V
This patch series performs all of the additions necessary to drop the
build overrides for RISCV and enable the full Xen build. Except in cases
where compatibile implementations already exist (e.g. atomic.h and
bitops.h), the newly added definitions are simple.
The patch series is based on the following patch series:
- [PATCH 0/7] xen/bitops: Reduce the mess, starting with ffs() [1]
- [PATCH] move __read_mostly to xen/cache.h [2]
Right now, the patch series doesn't have a direct dependency on [2] and it
provides __read_mostly in the patch:
[PATCH v3 26/34] xen/riscv: add definition of __read_mostly
However, it will be dropped as soon as [2] is merged or at least when the
final version of the patch [2] is provided.
As an option, it can be considered to merge arch-specific patch and then just
rebase [2] and drop arch-specific changes.
[1]
https://lore.kernel.org/xen-devel/20240313172716.2325427-1-andrew.coop...@citrix.com/T/#t
[2]
https://lore.kernel.org/xen-devel/f25eb5c9-7c14-6e23-8535-2c66772b3...@suse.com/
---
Changes in V10:
- Patch was merged to staging:
- [PATCH v9 04/15] xen/bitops: put __ffs() into linux compatible header
- Other changes are specific to specific patches. Please look at changes for
specific patch.
---
Changes in V9:
- Patch was merged to staging:
- [PATCH v8 07/17] xen/riscv: introduce io.h
- [PATCH v7 14/19] xen/riscv: add minimal stuff to page.h to build full
Xen
- Other changes are specific to specific patches. Please look at changes for
specific patch.
---
Changes in V8:
- Patch was merged to staging:
- [PATCH v7 01/19] automation: introduce fixed randconfig for RISC-V
- [PATCH v7 03/19] xen/riscv: introduce extenstion support check by compiler
- Other changes are specific to specific patches. Please look at changes for
specific patch.
- Update the commit message:
- drop the dependency from STATIC_ASSERT_UNREACHABLE() implementation.
- Add suggestion to merge arch-specific changes related to __read_mostly.
---
Changes in V7:
- Patch was merged to staging:
[PATCH v6 15/20] xen/riscv: add minimal stuff to processor.h to build full
Xen.
- Other changes are specific to specific patches. Please look at changes for
specific patch.
---
Changes in V6:
- Update the cover letter message: drop already merged dependecies and add
a new one.
- Patches were merged to staging:
- [PATCH v5 02/23] xen/riscv: use some asm-generic headers ( even v4 was
merged to staging branch, I just wasn't apply changes on top of the latest
staging branch )
- [PATCH v5 03/23] xen/riscv: introduce nospec.h
- [PATCH v5 10/23] xen/riscv: introduces acrquire, release and full barriers
- Introduce new patches:
- xen/riscv: introduce extenstion support check by compiler
- xen/bitops: put __ffs() and ffz() into linux compatible header
- xen/bitops: implement fls{l}() in common logic
- The following patches were dropped:
- drop some patches related to bitops operations as they were introduced in
another
patch series [...]
- introduce new version for generic __ffs(), ffz() and fls{l}().
- Merge patch from patch series "[PATCH v9 0/7] Introduce generic headers" to
this patch
series as only one patch left in the generic headers patch series and it is
more about
RISC-V.
- Other changes are specific to specific patches. please look at specific
patch.
---
Changes in V5:
- Update the cover letter as one of the dependencies were merged to staging.
- Was introduced asm-generic for atomic ops and separate patches for
asm-generic bit ops
- Moved fence.h to separate patch to deal with some patches dependecies on
fence.h
- Patches were dropped as they were merged to staging:
* [PATCH v4 03/30] xen: add support in public/hvm/save.h for PPC and RISC-V
* [PATCH v4 04/30] xen/riscv: introduce cpufeature.h
* [PATCH v4 05/30] xen/riscv: introduce guest_atomics.h
* [PATCH v4 06/30] xen: avoid generation of empty asm/iommu.h
* [PATCH v4 08/30] xen/riscv: introduce setup.h
* [PATCH v4 10/30] xen/riscv: introduce flushtlb.h
* [PATCH v4 11/30] xen/riscv: introduce smp.h
* [PATCH v4 15/30] xen/riscv: introduce irq.h
* [PATCH v4 16/30] xen/riscv: introduce p2m.h
* [PATCH v4 17/30] xen/riscv: introduce regs.h
* [PATCH v4 18/30] xen/riscv: introduce time.h
* [PATCH v4 19/30] xen/riscv: introduce event.h
* [PATCH v4 22/30] xen/riscv: define an address of frame table
- Other changes are specific to specific patches. please look at specific patch
---
Changes in V4:
- Update the cover letter message: new patch series dependencies.
- Some patches were merged to staging, so they were dropped in this patch
series:
[PATCH v3 09/34] xen/riscv: introduce system.h
[PATCH v3 18/34] xen/riscv: introduce domain.h
[PATCH v3 19/34] xen/riscv: introduce guest_access.h
- Was sent out of this patch series:
[PATCH v3 16/34] xen/lib: introduce generic find next bit operations
[PATCH v10 02/14] xen: introduce generic non-atomic test_*bit()
The following generic functions were introduced:
* test_bit
* generic__test_and_set_bit
* generic__test_and_clear_bit
* generic__test_and_change_bit
These functions and macros can be useful for architectures
that don't have corresponding arch-specific instructions.
Also, the patch introduces the following generics which are
used by the functions mentioned above:
* BITOP_BITS_PER_WORD
* BITOP_MASK
* BITOP_WORD
* BITOP_TYPE
The following approach was chosen for generic*() and arch*() bit
operation functions:
If the bit operation function that is going to be generic starts
with the prefix "__", then the corresponding generic/arch function
will also contain the "__" prefix. For example:
* test_bit() will be defined using arch_test_bit() and
generic_test_bit().
* __test_and_set_bit() will be defined using
arch__test_and_set_bit() and generic__test_and_set_bit().
Signed-off-by: Oleksii Kurochko
---
The context ("* Find First Set bit. Bits are labelled from 1." in
xen/bitops.h )
suggests there's a dependency on an uncommitted patch. It happens becuase
the current patch
series is based on Andrew's patch series (
https://lore.kernel.org/xen-devel/20240313172716.2325427-1-andrew.coop...@citrix.com/T/#t
),
but if everything is okay with the current one patch it can be merged
without Andrew's patch series being merged.
---
Changes in V10:
- update the commit message. ( re-order paragraphs and add explanation usage
of prefix "__" in bit
operation function names )
- add parentheses around the whole expression of bitop_bad_size() macros.
- move macros bitop_bad_size() above asm/bitops.h as it is not arch-specific
anymore and there is
no need for overriding it.
- drop macros check_bitop_size() and use "if ( bitop_bad_size(addr) )
__bitop_bad_size();" implictly
where it is needed.
- in use 'int' as a first parameter for __test_and_*(),
generic__test_and_*() to be
consistent with how the mentioned functions were declared in the original
per-arch functions.
- add 'const' to p variable in generic_test_bit().
- move definition of BITOP_BITS_PER_WORD and bitop_uint_t to xen/bitops.h as
we don't allow for arch
overrides these definitions anymore.
---
Changes in V9:
- move up xen/bitops.h in ppc/asm/page.h.
- update defintion of arch_check_bitop_size.
And drop correspondent macros from x86/asm/bitops.h
- drop parentheses in generic__test_and_set_bit() for definition of
local variable p.
- fix indentation inside #ifndef BITOP_TYPE...#endif
- update the commit message.
---
Changes in V8:
- drop __pure for function which uses volatile.
- drop unnessary () in generic__test_and_change_bit() for addr casting.
- update prototype of generic_test_bit() and test_bit(): now it returns bool
instead of int.
- update generic_test_bit() to use BITOP_MASK().
- Deal with fls{l} changes: it should be in the patch with introduced generic
fls{l}.
- add a footer with explanation of dependency on an uncommitted patch after
Signed-off.
- abstract bitop_size().
- move BITOP_TYPE define to .
---
Changes in V7:
- move everything to xen/bitops.h to follow the same approach for all generic
bit ops.
- put together BITOP_BITS_PER_WORD and bitops_uint_t.
- make BITOP_MASK more generic.
- drop #ifdef ... #endif around BITOP_MASK, BITOP_WORD as they are generic
enough.
- drop "_" for generic__{test_and_set_bit,...}().
- drop " != 0" for functions which return bool.
- add volatile during the cast for generic__{...}().
- update the commit message.
- update arch related code to follow the proposed generic approach.
---
Changes in V6:
- Nothing changed ( only rebase )
---
Changes in V5:
- new patch
---
xen/arch/arm/arm64/livepatch.c| 1 -
xen/arch/arm/include/asm/bitops.h | 67 ---
xen/arch/ppc/include/asm/bitops.h | 52
xen/arch/ppc/include/asm/page.h | 2 +-
xen/arch/ppc/mm-radix.c | 2 +-
xen/arch/x86/include/asm/bitops.h | 31 ++-
xen/include/xen/bitops.h | 131 ++
7 files changed, 142 insertions(+), 144 deletions(-)
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index df2cebedde..4bc8ed9be5 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -10,7 +10,6 @@
#include
#include
-#include
#include
#include
#include
diff --git a/xen/arch/arm/include/asm/bitops.h
b/xen/arch/arm/include/asm/bitops.h
index 5104334e48..8e16335e76 100644
--- a/xen/arch/arm/include/asm/bitops.h
+++ b/xen/arch/arm/include/asm/bitops.h
@@ -22,9 +22,6 @@
#define __set_bit(n,p)set_bit(n,p)
#define __clear_bit(n,p) clear_bit(n,p)
-#define BITOP_BITS_PER_WORD 32
-#define BITOP_MASK(nr) (1UL << ((nr) % BITOP_BITS_PER_WORD))
-#define BITOP_WORD(nr) ((nr) / BITOP_BITS_PER_WORD)
#define BITS_PER_BYTE 8
#define ADDR (*(volatile int *)
Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl
> On 17 May 2024, at 14:33, Roger Pau Monne wrote: > > Enabling it using an HVM param is fragile, and complicates the logic when > deciding whether options that interact with altp2m can also be enabled. > > Leave the HVM param value for consumption by the guest, but prevent it from > being set. Enabling is now done using and additional altp2m specific field in > xen_domctl_createdomain. > > Note that albeit only currently implemented in x86, altp2m could be > implemented > in other architectures, hence why the field is added to > xen_domctl_createdomain > instead of xen_arch_domainconfig. > > Signed-off-by: Roger Pau Monné > --- > Changes since v2: > - Introduce a new altp2m field in xen_domctl_createdomain. > > Changes since v1: > - New in this version. > --- > tools/libs/light/libxl_create.c | 23 ++- > tools/libs/light/libxl_x86.c| 26 -- > tools/ocaml/libs/xc/xenctrl_stubs.c | 2 +- > xen/arch/arm/domain.c | 6 ++ > xen/arch/x86/domain.c | 22 ++ > xen/arch/x86/hvm/hvm.c | 23 ++- > xen/include/public/domctl.h | 20 +++- > xen/include/public/hvm/params.h | 9 ++--- > 8 files changed, 106 insertions(+), 25 deletions(-) > > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c > b/tools/ocaml/libs/xc/xenctrl_stubs.c > index 2b6d3c09dfa0..c6da9bb09137 100644 > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > @@ -257,7 +257,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value > wanted_domid, value co > #if defined(__i386__) || defined(__x86_64__) > > /* Quick & dirty check for ABI changes. */ > - BUILD_BUG_ON(sizeof(cfg) != 64); > + BUILD_BUG_ON(sizeof(cfg) != 68); > Acked-by: Christian Lindig
[PATCH for-4.19 v3 3/3] xen/x86: remove foreign mappings from the p2m on teardown
Iterate over the p2m up to the maximum recorded gfn and remove any foreign
mappings, in order to drop the underlying page references and thus don't keep
extra page references if a domain is destroyed while still having foreign
mappings on it's p2m.
The logic is similar to the one used on Arm.
Note that foreign mappings cannot be created by guests that have altp2m or
nested HVM enabled, as p2ms different than the host one are not currently
scrubbed when destroyed in order to drop references to any foreign maps.
It's unclear whether the right solution is to take an extra reference when
foreign maps are added to p2ms different than the host one, or just rely on the
host p2m already having a reference. The mapping being removed from the host
p2m should cause it to be dropped on all domain p2ms.
Signed-off-by: Roger Pau Monné
Reviewed-by: Jan Beulich
---
Changes since v2:
- Fix arch_acquire_resource_check() condition.
Changes since v1:
- Use existing p2m max_mapped_pfn field.
- Prevent creating foreign mappings by guests that have altp2m or nestedhvm
enabled.
---
CHANGELOG.md | 1 +
xen/arch/x86/domain.c | 6 +++
xen/arch/x86/include/asm/p2m.h | 26 +++--
xen/arch/x86/mm/p2m-basic.c| 18 +
xen/arch/x86/mm/p2m.c | 68 --
5 files changed, 103 insertions(+), 16 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c43c45d8d4bd..59aad04e9364 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@ The format is based on [Keep a
Changelog](https://keepachangelog.com/en/1.0.0/)
- xl/libxl configures vkb=[] for HVM domains with priority over vkb_device.
- Increase the maximum number of CPUs Xen can be built for from 4095 to
16383.
+ - Allow HVM/PVH domains to map foreign pages.
### Added
- On x86:
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index a89ceff9ae93..de5f6b3212fb 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2386,6 +2386,7 @@ int domain_relinquish_resources(struct domain *d)
enum {
PROG_iommu_pagetables = 1,
PROG_shared,
+PROG_mappings,
PROG_paging,
PROG_vcpu_pagetables,
PROG_xen,
@@ -2434,6 +2435,11 @@ int domain_relinquish_resources(struct domain *d)
}
#endif
+PROGRESS(mappings):
+ret = relinquish_p2m_mapping(d);
+if ( ret )
+return ret;
+
PROGRESS(paging):
/* Tear down paging-assistance stuff. */
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 107b9f260848..c1478ffc3647 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -383,6 +383,8 @@ struct p2m_domain {
/* Number of foreign mappings. */
unsigned long nr_foreign;
+/* Cursor for iterating over the p2m on teardown. */
+unsigned long teardown_gfn;
#endif /* CONFIG_HVM */
};
@@ -395,16 +397,7 @@ struct p2m_domain {
#endif
#include
-static inline bool arch_acquire_resource_check(struct domain *d)
-{
-/*
- * FIXME: Until foreign pages inserted into the P2M are properly
- * reference counted, it is unsafe to allow mapping of
- * resource pages unless the caller is the hardware domain
- * (see set_foreign_p2m_entry()).
- */
-return !paging_mode_translate(d) || is_hardware_domain(d);
-}
+bool arch_acquire_resource_check(const struct domain *d);
/*
* Updates vCPU's n2pm to match its np2m_base in VMCx12 and returns that np2m.
@@ -720,6 +713,10 @@ p2m_pod_offline_or_broken_hit(struct page_info *p);
void
p2m_pod_offline_or_broken_replace(struct page_info *p);
+/* Perform cleanup of p2m mappings ahead of teardown. */
+int
+relinquish_p2m_mapping(struct domain *d);
+
#else
static inline bool
@@ -748,6 +745,11 @@ static inline void
p2m_pod_offline_or_broken_replace(struct page_info *p)
ASSERT_UNREACHABLE();
}
+static inline int relinquish_p2m_mapping(struct domain *d)
+{
+return 0;
+}
+
#endif
@@ -1043,7 +1045,7 @@ static inline int p2m_entry_modify(struct p2m_domain
*p2m, p2m_type_t nt,
break;
case p2m_map_foreign:
-if ( !mfn_valid(nfn) )
+if ( !mfn_valid(nfn) || p2m != p2m_get_hostp2m(p2m->domain) )
{
ASSERT_UNREACHABLE();
return -EINVAL;
@@ -1068,7 +1070,7 @@ static inline int p2m_entry_modify(struct p2m_domain
*p2m, p2m_type_t nt,
break;
case p2m_map_foreign:
-if ( !mfn_valid(ofn) )
+if ( !mfn_valid(ofn) || p2m != p2m_get_hostp2m(p2m->domain) )
{
ASSERT_UNREACHABLE();
return -EINVAL;
diff --git a/xen/arch/x86/mm/p2m-basic.c b/xen/arch/x86/mm/p2m-basic.c
index 8599bd15c61a..25d27a0a9f56 100644
--- a/xen/arch/x86/mm/p2m-basic.c
+++ b/xen/arch/x86/mm/p2m-basic.c
@@ -13,6 +13,8 @@
#include
#include
+#include
+#include
#include
#include "mm-locks.h"
[PATCH for-4.19 v3 1/3] xen/x86: account number of foreign mappings in the p2m
Such information will be needed in order to remove foreign mappings during
teardown for HVM guests.
Right now the introduced counter is not consumed.
Signed-off-by: Roger Pau Monné
Reviewed-by: Jan Beulich
---
Changes since v1:
- Drop max_gfn accounting.
---
xen/arch/x86/include/asm/p2m.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 111badf89a6e..107b9f260848 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -380,6 +380,9 @@ struct p2m_domain {
unsigned int flags;
unsigned long entry_count;
} ioreq;
+
+/* Number of foreign mappings. */
+unsigned long nr_foreign;
#endif /* CONFIG_HVM */
};
@@ -1049,6 +1052,8 @@ static inline int p2m_entry_modify(struct p2m_domain
*p2m, p2m_type_t nt,
if ( !page_get_owner_and_reference(mfn_to_page(nfn)) )
return -EBUSY;
+p2m->nr_foreign++;
+
break;
default:
@@ -1069,6 +1074,7 @@ static inline int p2m_entry_modify(struct p2m_domain
*p2m, p2m_type_t nt,
return -EINVAL;
}
put_page(mfn_to_page(ofn));
+p2m->nr_foreign--;
break;
default:
--
2.44.0
[PATCH for-4.19 v3 0/3] xen/x86: support foreign mappings for HVM/PVH
Hello, The following series attempts to solve a shortcoming of HVM/PVH guests with the lack of support for foreign mappings. Such lack of support prevents using PVH based guests as stubdomains for example. Add support in a way similar to how it's done on Arm, by iterating over the p2m based on the maximum gfn. Patch 2 is not strictly needed. Moving the enablement of altp2m from an HVM param to a create domctl flag avoids any possible race with the HVM param changing after it's been evaluated. Note the param can only be set by the control domain, and libxl currently sets it at domain create. Also altp2m enablement is different from activation, as activation does happen during runtime of the domain. Thanks, Roger. Roger Pau Monne (3): xen/x86: account number of foreign mappings in the p2m xen/x86: enable altp2m at create domain domctl xen/x86: remove foreign mappings from the p2m on teardown CHANGELOG.md| 1 + tools/libs/light/libxl_create.c | 23 +- tools/libs/light/libxl_x86.c| 26 +-- tools/ocaml/libs/xc/xenctrl_stubs.c | 2 +- xen/arch/arm/domain.c | 6 +++ xen/arch/x86/domain.c | 28 xen/arch/x86/hvm/hvm.c | 23 +- xen/arch/x86/include/asm/p2m.h | 32 +- xen/arch/x86/mm/p2m-basic.c | 18 xen/arch/x86/mm/p2m.c | 68 +++-- xen/include/public/domctl.h | 20 - xen/include/public/hvm/params.h | 9 +--- 12 files changed, 215 insertions(+), 41 deletions(-) -- 2.44.0
[PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl
Enabling it using an HVM param is fragile, and complicates the logic when
deciding whether options that interact with altp2m can also be enabled.
Leave the HVM param value for consumption by the guest, but prevent it from
being set. Enabling is now done using and additional altp2m specific field in
xen_domctl_createdomain.
Note that albeit only currently implemented in x86, altp2m could be implemented
in other architectures, hence why the field is added to xen_domctl_createdomain
instead of xen_arch_domainconfig.
Signed-off-by: Roger Pau Monné
---
Changes since v2:
- Introduce a new altp2m field in xen_domctl_createdomain.
Changes since v1:
- New in this version.
---
tools/libs/light/libxl_create.c | 23 ++-
tools/libs/light/libxl_x86.c| 26 --
tools/ocaml/libs/xc/xenctrl_stubs.c | 2 +-
xen/arch/arm/domain.c | 6 ++
xen/arch/x86/domain.c | 22 ++
xen/arch/x86/hvm/hvm.c | 23 ++-
xen/include/public/domctl.h | 20 +++-
xen/include/public/hvm/params.h | 9 ++---
8 files changed, 106 insertions(+), 25 deletions(-)
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 41252ec55370..edeadd57ef5a 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -372,7 +372,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(_info->u.hvm.viridian, false);
libxl_defbool_setdefault(_info->u.hvm.hpet, true);
libxl_defbool_setdefault(_info->u.hvm.vpt_align, true);
-libxl_defbool_setdefault(_info->u.hvm.altp2m, false);
libxl_defbool_setdefault(_info->u.hvm.usb,false);
libxl_defbool_setdefault(_info->u.hvm.vkb_device, true);
libxl_defbool_setdefault(_info->u.hvm.xen_platform_pci, true);
@@ -678,6 +677,28 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config
*d_config,
if (info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
create.iommu_opts |= XEN_DOMCTL_IOMMU_no_sharept;
+LOG(DETAIL, "altp2m: %s", libxl_altp2m_mode_to_string(b_info->altp2m));
+switch(b_info->altp2m) {
+case LIBXL_ALTP2M_MODE_MIXED:
+create.altp2m_opts |=
+XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_mixed);
+break;
+
+case LIBXL_ALTP2M_MODE_EXTERNAL:
+create.altp2m_opts |=
+XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_external);
+break;
+
+case LIBXL_ALTP2M_MODE_LIMITED:
+create.altp2m_opts |=
+XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_limited);
+break;
+
+case LIBXL_ALTP2M_MODE_DISABLED:
+/* Nothing to do - altp2m disabled is signaled as mode == 0. */
+break;
+}
+
/* Ultimately, handle is an array of 16 uint8_t, same as uuid */
libxl_uuid_copy(ctx, (libxl_uuid *), >uuid);
diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index a50ec37eb3eb..60643d6f5376 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -407,19 +407,9 @@ static int hvm_set_conf_params(libxl__gc *gc, uint32_t
domid,
libxl_ctx *ctx = libxl__gc_owner(gc);
xc_interface *xch = ctx->xch;
int ret = ERROR_FAIL;
-unsigned int altp2m = info->altp2m;
switch(info->type) {
case LIBXL_DOMAIN_TYPE_HVM:
-/* The config parameter "altp2m" replaces the parameter "altp2mhvm".
For
- * legacy reasons, both parameters are accepted on x86 HVM guests.
- *
- * If the legacy field info->u.hvm.altp2m is set, activate altp2m.
- * Otherwise set altp2m based on the field info->altp2m. */
-if (info->altp2m == LIBXL_ALTP2M_MODE_DISABLED &&
-libxl_defbool_val(info->u.hvm.altp2m))
-altp2m = libxl_defbool_val(info->u.hvm.altp2m);
-
if (xc_hvm_param_set(xch, domid, HVM_PARAM_HPET_ENABLED,
libxl_defbool_val(info->u.hvm.hpet))) {
LOG(ERROR, "Couldn't set HVM_PARAM_HPET_ENABLED");
@@ -444,10 +434,6 @@ static int hvm_set_conf_params(libxl__gc *gc, uint32_t
domid,
LOG(ERROR, "Couldn't set HVM_PARAM_TIMER_MODE");
goto out;
}
-if (xc_hvm_param_set(xch, domid, HVM_PARAM_ALTP2M, altp2m)) {
-LOG(ERROR, "Couldn't set HVM_PARAM_ALTP2M");
-goto out;
-}
break;
default:
@@ -818,6 +804,18 @@ int libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(_info->acpi, true);
libxl_defbool_setdefault(_info->arch_x86.msr_relaxed, false);
+/*
+ * The config parameter "altp2m" replaces the parameter "altp2mhvm".
+ * For legacy reasons, both parameters are accepted on x86
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 5/17/24 04:08, Chen, Jiqian wrote:
> On 2024/5/16 21:08, Jan Beulich wrote:
>> On 16.05.2024 11:52, Jiqian Chen wrote:
>>> @@ -67,6 +68,41 @@ ret_t pci_physdev_op(int cmd,
>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>> +pcidevs_lock();
>>> +pdev = pci_get_pdev(NULL, sbdf);
>>> +if ( !pdev )
>>> +{
>>> +pcidevs_unlock();
>>> +ret = -ENODEV;
>>> +break;
>>> +}
>>> +
>>> +write_lock(>domain->pci_lock);
>>> +ret = vpci_reset_device_state(pdev);
>>> +write_unlock(>domain->pci_lock);
>>> +pcidevs_unlock();
>>
>> Can't this be done right after the write_lock()?
> You are right, I will move it in next version.
So that we are clear on the proposed order of calls here:
+write_lock(>domain->pci_lock);
+pcidevs_unlock();
+ret = vpci_reset_device_state(pdev);
+write_unlock(>domain->pci_lock);
>>> --- a/xen/drivers/vpci/vpci.c
>>> +++ b/xen/drivers/vpci/vpci.c
>>> @@ -115,6 +115,16 @@ int vpci_assign_device(struct pci_dev *pdev)
>>>
>>> return rc;
>>> }
>>> +
>>> +int vpci_reset_device_state(struct pci_dev *pdev)
>>> +{
>>> +ASSERT(pcidevs_locked());
>>
>> Is this necessary for ...
>>
>>> +ASSERT(rw_is_write_locked(>domain->pci_lock));
>>> +
>>> +vpci_deassign_device(pdev);
>>> +return vpci_assign_device(pdev);
>>
>> ... either of these calls? Both functions do themselves only have the
>> 2nd of the asserts you add.
> I checked codes again; I will remove the two asserts here in next version.
Nit: I think it's okay to keep the
ASSERT(rw_is_write_locked(>domain->pci_lock));
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 17.05.2024 13:14, Chen, Jiqian wrote:
> On 2024/5/17 18:51, Jan Beulich wrote:
>> On 17.05.2024 12:45, Chen, Jiqian wrote:
>>> On 2024/5/16 22:01, Jan Beulich wrote:
On 16.05.2024 11:52, Jiqian Chen wrote:
> +if ( gsi >= nr_irqs_gsi )
> +{
> +ret = -EINVAL;
> +break;
> +}
> +
> +if ( !irq_access_permitted(current->domain, gsi) ||
I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
source overrides may be surfaced by ACPI?
>>> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>>
>> They are, but there's not necessarily a 1:1 mapping.
> Oh, so do I need to add a new gsi_caps to store granted gsi?
Probably not. You ought to be able to translate between GSI and IRQ,
and then continue to record in / check against IRQ permissions.
Jan
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 17.05.2024 13:01, Chen, Jiqian wrote:
> On 2024/5/17 18:31, Jan Beulich wrote:
>> On 17.05.2024 12:00, Chen, Jiqian wrote:
>>> On 2024/5/17 17:50, Jan Beulich wrote:
On 17.05.2024 11:28, Chen, Jiqian wrote:
> On 2024/5/17 16:20, Jan Beulich wrote:
>> On 17.05.2024 10:08, Chen, Jiqian wrote:
>>> On 2024/5/16 21:08, Jan Beulich wrote:
On 16.05.2024 11:52, Jiqian Chen wrote:
> struct physdev_pci_device {
> /* IN */
> uint16_t seg;
Is re-using this struct for this new sub-op sufficient? IOW are all
possible resets equal, and hence it doesn't need specifying what kind
of
reset was done? For example, other than FLR most reset variants reset
all
functions in one go aiui. Imo that would better require only a single
hypercall, just to avoid possible confusion. It also reads as if FLR
would
not reset as many registers as other reset variants would.
>>> If I understood correctly that you mean in this hypercall it needs to
>>> support resetting both one function and all functions of a slot(dev)?
>>> But it can be done for caller to use a cycle to call this reset
>>> hypercall for each slot function.
>>
>> It could, yes, but since (aiui) there needs to be an indication of the
>> kind of reset anyway, we can as well avoid relying on the caller doing
>> so (and at the same time simplify what the caller needs to do).
> Since the corresponding kernel patch has been merged into linux_next
> branch
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20240515=b272722511d5e8ae580f01830687b8a6b2717f01,
> if it's not very mandatory and necessary, just let the caller handle it
> temporarily.
As also mentioned for the other patch having a corresponding kernel one:
The kernel patch would imo better not be merged until the new sub-op is
actually finalized.
>>> OK, what should I do next step?
>>> Upstream a patch to revert the merged patch on kernel side?
>>>
> Or it can add a new hypercall to reset all functions in one go in future
> potential requirement, like PHYSDEVOP_pci_device_state_reset_all_func.
I disagree. We shouldn't introduce incomplete sub-ops. At the very least,
if you want to stick to the present form, I'd expect you to supply reasons
why distinguishing different reset forms is not necessary (now or later).
>>> OK, if want to distinguish different reset, is it acceptable to add a
>>> parameter, like "u8 flag", and reset every function if corresponding bit is
>>> 1?
>>
>> I'm afraid a boolean won't do, at least not long term. I think it wants to
>> be an enumeration (i.e. a set of enumeration-like #define-s). And just to
>> stress it again: The extra argument is _not_ primarily for the looping over
>> all functions. It is to convey the kind of reset that was done. The single
>> vs all function(s) aspect is just a useful side effect this will have.
> Do you mean, like:
> enum RESET_DEVICE_STATE {
> RESET_DEVICE_SINGLE_FUNC,
> RESET_DEVICE_ALL_FUNC,
> Others
> };
No. What we need to be able to tell apart in the hypervisor is (at least)
FLR and Conventional Reset. I can't tell right away whether the sub-forms
of the latter also may need telling apart. I expect you to dive into that
and make a good proposal.
Jan
[PATCH] USB: xen-hcd: Traverse host/ when CONFIG_USB_XEN_HCD is selected
If no other USB HCDs are selected when compiling a small pure virutal
machine, the Xen HCD driver cannot be built.
Fix it by traversing down host/ if CONFIG_USB_XEN_HCD is selected.
Fixes: 494ed3997d75 ("usb: Introduce Xen pvUSB frontend (xen hcd)")
Cc: sta...@vger.kernel.org # v5.17+
Signed-off-by: John Ernberg
---
drivers/usb/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 3a9a0dd4be70..949eca0adebe 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_USB_R8A66597_HCD)+= host/
obj-$(CONFIG_USB_FSL_USB2) += host/
obj-$(CONFIG_USB_FOTG210_HCD) += host/
obj-$(CONFIG_USB_MAX3421_HCD) += host/
+obj-$(CONFIG_USB_XEN_HCD) += host/
obj-$(CONFIG_USB_C67X00_HCD) += c67x00/
--
2.45.0
[linux-linus test] 186023: regressions - FAIL
flight 186023 linux-linus real [real] flight 186029 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/186023/ http://logs.test-lab.xenproject.org/osstest/logs/186029/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit2 10 host-ping-check-xen fail in 186029 REGR. vs. 186018 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-credit2 8 xen-bootfail pass in 186029-retest test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass in 186029-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-arndale 8 xen-boot fail like 186010 test-armhf-armhf-xl-multivcpu 8 xen-boot fail like 186010 test-armhf-armhf-xl 8 xen-boot fail like 186010 test-armhf-armhf-xl-raw 8 xen-boot fail like 186010 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 186018 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 186018 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 186018 test-armhf-armhf-xl-credit1 8 xen-boot fail like 186018 test-armhf-armhf-examine 8 reboot fail like 186018 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 186018 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 186018 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 186018 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 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-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-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-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-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-armhf-armhf-xl-qcow214 migrate-support-checkfail never pass test-armhf-armhf-xl-qcow215 saverestore-support-checkfail never pass version targeted for testing: linuxea5f6ad9ad9645733b72ab53a98e719b460d36a6 baseline version: linux3c999d1ae3c75991902a1a7dad0cb62c2a3008b4 Last test of basis 186018 2024-05-16 15:33:02 Z0 days Testing same since 186023 2024-05-17 01:13:59 Z0 days1 attempts People who touched revisions under test: Aapo Vienamo AceLan Kao Adrian Hunter Ai Chao Aleksandr Burakov Alex Volkov Alexander Stein Andreas Helbech Kleist Andrzej Pietrasiewicz Andy Shevchenko Andy Shevchenko Angelo Dureghello Arend van Spriel Armin Wolf Arnd Bergmann Arunpravin Paneer Selvam Arvid Norlander Avri Altman Basavaraj Natikar Bastien Curutchet Ben Fradella Benjamin Gaignard Bingbu Cao Bjorn Helgaas Bryan O'Donoghue Changhuang Liang Claudiu Beznea Colin Ian King Conor Dooley Dan Carpenter Dave Airlie Dave Stevenson David E. Box Denis Arefev
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/5/17 18:51, Jan Beulich wrote:
> On 17.05.2024 12:45, Chen, Jiqian wrote:
>> On 2024/5/16 22:01, Jan Beulich wrote:
>>> On 16.05.2024 11:52, Jiqian Chen wrote:
+if ( gsi >= nr_irqs_gsi )
+{
+ret = -EINVAL;
+break;
+}
+
+if ( !irq_access_permitted(current->domain, gsi) ||
>>>
>>> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
>>> source overrides may be surfaced by ACPI?
>> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>
> They are, but there's not necessarily a 1:1 mapping.
Oh, so do I need to add a new gsi_caps to store granted gsi?
And Dom0 defaults to own all gsis permission?
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 2024/5/17 18:31, Jan Beulich wrote:
> On 17.05.2024 12:00, Chen, Jiqian wrote:
>> On 2024/5/17 17:50, Jan Beulich wrote:
>>> On 17.05.2024 11:28, Chen, Jiqian wrote:
On 2024/5/17 16:20, Jan Beulich wrote:
> On 17.05.2024 10:08, Chen, Jiqian wrote:
>> On 2024/5/16 21:08, Jan Beulich wrote:
>>> On 16.05.2024 11:52, Jiqian Chen wrote:
struct physdev_pci_device {
/* IN */
uint16_t seg;
>>>
>>> Is re-using this struct for this new sub-op sufficient? IOW are all
>>> possible resets equal, and hence it doesn't need specifying what kind of
>>> reset was done? For example, other than FLR most reset variants reset
>>> all
>>> functions in one go aiui. Imo that would better require only a single
>>> hypercall, just to avoid possible confusion. It also reads as if FLR
>>> would
>>> not reset as many registers as other reset variants would.
>> If I understood correctly that you mean in this hypercall it needs to
>> support resetting both one function and all functions of a slot(dev)?
>> But it can be done for caller to use a cycle to call this reset
>> hypercall for each slot function.
>
> It could, yes, but since (aiui) there needs to be an indication of the
> kind of reset anyway, we can as well avoid relying on the caller doing
> so (and at the same time simplify what the caller needs to do).
Since the corresponding kernel patch has been merged into linux_next branch
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20240515=b272722511d5e8ae580f01830687b8a6b2717f01,
if it's not very mandatory and necessary, just let the caller handle it
temporarily.
>>>
>>> As also mentioned for the other patch having a corresponding kernel one:
>>> The kernel patch would imo better not be merged until the new sub-op is
>>> actually finalized.
>> OK, what should I do next step?
>> Upstream a patch to revert the merged patch on kernel side?
>>
>>>
Or it can add a new hypercall to reset all functions in one go in future
potential requirement, like PHYSDEVOP_pci_device_state_reset_all_func.
>>>
>>> I disagree. We shouldn't introduce incomplete sub-ops. At the very least,
>>> if you want to stick to the present form, I'd expect you to supply reasons
>>> why distinguishing different reset forms is not necessary (now or later).
>> OK, if want to distinguish different reset, is it acceptable to add a
>> parameter, like "u8 flag", and reset every function if corresponding bit is
>> 1?
>
> I'm afraid a boolean won't do, at least not long term. I think it wants to
> be an enumeration (i.e. a set of enumeration-like #define-s). And just to
> stress it again: The extra argument is _not_ primarily for the looping over
> all functions. It is to convey the kind of reset that was done. The single
> vs all function(s) aspect is just a useful side effect this will have.
Do you mean, like:
enum RESET_DEVICE_STATE {
RESET_DEVICE_SINGLE_FUNC,
RESET_DEVICE_ALL_FUNC,
Others
};
If caller pass in RESET_DEVICE_SINGLE_FUNC, I call what I add in this patch, as
for other types call other functions if added in future?
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 17.05.2024 12:45, Chen, Jiqian wrote:
> On 2024/5/16 22:01, Jan Beulich wrote:
>> On 16.05.2024 11:52, Jiqian Chen wrote:
>>> +if ( gsi >= nr_irqs_gsi )
>>> +{
>>> +ret = -EINVAL;
>>> +break;
>>> +}
>>> +
>>> +if ( !irq_access_permitted(current->domain, gsi) ||
>>
>> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
>> source overrides may be surfaced by ACPI?
> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
They are, but there's not necessarily a 1:1 mapping.
Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/5/16 22:01, Jan Beulich wrote:
> On 16.05.2024 11:52, Jiqian Chen wrote:
>> Some type of domain don't have PIRQ, like PVH, when
>> passthrough a device to guest on PVH dom0, callstack
>> pci_add_dm_done->XEN_DOMCTL_irq_permission will failed
>> at domain_pirq_to_irq.
>>
>> So, add a new hypercall to grant/revoke gsi permission
>> when dom0 is not PV or dom0 has not PIRQ flag.
>
> Honestly I find this hard to follow, and thus not really making clear why
> no other existing mechanism could be used.
Sorry, I will describe more clearly in next version.
>
>> Signed-off-by: Huang Rui
>> Signed-off-by: Jiqian Chen
>> ---
>
> Below here in an RFC patch you typically would want to put specific items
> you're seeking feedback on. Without that it's hard to tell why this is
> marked RFC.
OK, I will add " RFC: wait for the third patch on kernel side is accepted." in
next version.
>
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -237,6 +237,37 @@ long arch_do_domctl(
>> break;
>> }
>>
>> +case XEN_DOMCTL_gsi_permission:
>> +{
>> +unsigned int gsi = domctl->u.gsi_permission.gsi;
>> +int allow = domctl->u.gsi_permission.allow_access;
>
> bool?
Will change.
>
>> +if ( is_pv_domain(current->domain) || has_pirq(current->domain) )
>> +{
>> +ret = -EOPNOTSUPP;
>> +break;
>> +}
>
> Such a restriction imo wants explaining in a comment.
Will add in next version.
>
>> +if ( gsi >= nr_irqs_gsi )
>> +{
>> +ret = -EINVAL;
>> +break;
>> +}
>> +
>> +if ( !irq_access_permitted(current->domain, gsi) ||
>
> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
> source overrides may be surfaced by ACPI?
All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>
>> + xsm_irq_permission(XSM_HOOK, d, gsi, allow) )
>
> Here I'm pretty sure you can't very well re-use an existing hook, as the
> value of interest is in a different numbering space, and a possible hook
> function has no way of knowing which one it is. Daniel?
>
>> +{
>> +ret = -EPERM;
>> +break;
>> +}
>> +
>> +if ( allow )
>> +ret = irq_permit_access(d, gsi);
>> +else
>> +ret = irq_deny_access(d, gsi);
>
> As above I'm afraid you can't assume IRQ == GSI.
>
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -447,6 +447,13 @@ struct xen_domctl_irq_permission {
>> };
>>
>>
>> +/* XEN_DOMCTL_gsi_permission */
>> +struct xen_domctl_gsi_permission {
>> +uint32_t gsi;
>> +uint8_t allow_access;/* flag to specify enable/disable of x86 gsi
>> access */
>> +};
>
> Explicit padding please, including a check that it's zero on input.
Thanks, I will add in next version.
>
>> +
>> +
>> /* XEN_DOMCTL_iomem_permission */
>
> No double blank lines please. In fact you will want to break the double blank
> lines in leading context, inserting in the middle.
Will remove one.
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [PATCH v3 3/4] tools/hotplug/Linux: Add bridge VLAN support
Hi Jason,
On 2024-05-17 03:19, Jason Andryuk wrote:
On Thu, May 16, 2024 at 6:56 AM Leigh Brown
wrote:
Update add_to_bridge shell function to read the vlan parameter from
xenstore and set the bridge VLAN configuration for the VID.
Add additional helper functions to parse the vlan specification,
which consists of one or more of the following:
a) single VLAN (e.g. 10).
b) contiguous range of VLANs (e.g. 10-15).
c) discontiguous range with base, increment and count
(e.g. 100+10x9 which gives VLAN IDs 100, 110, ... 190).
A single VLAN can be suffixed with "p" to indicate the PVID, or
"u" to indicate untagged. A range of VLANs can be suffixed with
"u" to indicate untagged. A complex example would be:
vlan=1p/10-15/20-25u
This capability requires the iproute2 bridge command to be
installed. An error will be generated if the vlan parameter is
set and the bridge command is not available.
Signed-off-by: Leigh Brown
---
tools/hotplug/Linux/xen-network-common.sh | 103
++
1 file changed, 103 insertions(+)
diff --git a/tools/hotplug/Linux/xen-network-common.sh
b/tools/hotplug/Linux/xen-network-common.sh
index 42fa704e8d..fa7615ce0f 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
+_vif_vlan_setup() {
+# References vlans and dev variable from the calling function
+local vid cmd
+
+bridge vlan del dev "$dev" vid 1
Is vid 1 always added, so you need to remove it before customizing?
Correct. I will add a comment to that effect.
+for vid in ${!vlans[@]} ;do
+cmd="bridge vlan add dev '$dev' vid $vid"
+case ${vlans[$vid]} in
+ p) cmd="$cmd pvid untagged" ;;
+ u) cmd="$cmd untagged" ;;
+ t) ;;
+esac
+eval "$cmd"
Sorry if I missed this last time, but `eval` shouldn't be necessary.
local args
case ${vlans[$vid]} in
p) args="pvid untagged" ;;
u) args="untagged" ;;
t) ;;
esac
bridge vlan add dev "$dev" vid "$vid" $args
args unquoted so it expands into maybe two args. You could also make
args an array and do "${args[@]}"
I will make that change, using an array.
+done
+}
+
+_vif_vlan_membership() {
+# The vlans, pvid and dev variables are used by sub-functions
+local -A vlans=()
+local -a terms=()
+local -i i pvid=0
+local dev=$1
+
+# Split the vlan specification string into its terms
+readarray -d / -t terms <<<$2
+for (( i=0; i<${#terms[@]}; ++i )) ;do
+_vif_vlan_parse_term ${terms[$i]%%[[:space:]]}
Because terms is not in double quotes, wouldn't it expand out? Then
any whitespace would be dropped when calling _vif_vlan_parse_term.
Your %% only drops trailing spaces too. Maybe you want
"${terms//[[:space:]]/}" to remove all spaces?
That is a hack because readarray adds a newline at the end of the
last element (not sure why). I will change the code just to fix up
the last element before the loop, and it will be clearer.
Stylistically, you use (( )) more than I would. I'd do:
local term
for term in "${terms[@]}" ; do
_vif_vlan_parse_term "$term"
But you can keep it your way if you prefer.
You guessed correctly that like using (( )) but in this case your
suggestion is simpler, so I will make that change.
Regards,
Jason
I am making the changes then will test, after which I will send an
updated version.
Regards,
Leigh.
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 17.05.2024 12:00, Chen, Jiqian wrote:
> On 2024/5/17 17:50, Jan Beulich wrote:
>> On 17.05.2024 11:28, Chen, Jiqian wrote:
>>> On 2024/5/17 16:20, Jan Beulich wrote:
On 17.05.2024 10:08, Chen, Jiqian wrote:
> On 2024/5/16 21:08, Jan Beulich wrote:
>> On 16.05.2024 11:52, Jiqian Chen wrote:
>>> struct physdev_pci_device {
>>> /* IN */
>>> uint16_t seg;
>>
>> Is re-using this struct for this new sub-op sufficient? IOW are all
>> possible resets equal, and hence it doesn't need specifying what kind of
>> reset was done? For example, other than FLR most reset variants reset all
>> functions in one go aiui. Imo that would better require only a single
>> hypercall, just to avoid possible confusion. It also reads as if FLR
>> would
>> not reset as many registers as other reset variants would.
> If I understood correctly that you mean in this hypercall it needs to
> support resetting both one function and all functions of a slot(dev)?
> But it can be done for caller to use a cycle to call this reset hypercall
> for each slot function.
It could, yes, but since (aiui) there needs to be an indication of the
kind of reset anyway, we can as well avoid relying on the caller doing
so (and at the same time simplify what the caller needs to do).
>>> Since the corresponding kernel patch has been merged into linux_next branch
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20240515=b272722511d5e8ae580f01830687b8a6b2717f01,
>>> if it's not very mandatory and necessary, just let the caller handle it
>>> temporarily.
>>
>> As also mentioned for the other patch having a corresponding kernel one:
>> The kernel patch would imo better not be merged until the new sub-op is
>> actually finalized.
> OK, what should I do next step?
> Upstream a patch to revert the merged patch on kernel side?
>
>>
>>> Or it can add a new hypercall to reset all functions in one go in future
>>> potential requirement, like PHYSDEVOP_pci_device_state_reset_all_func.
>>
>> I disagree. We shouldn't introduce incomplete sub-ops. At the very least,
>> if you want to stick to the present form, I'd expect you to supply reasons
>> why distinguishing different reset forms is not necessary (now or later).
> OK, if want to distinguish different reset, is it acceptable to add a
> parameter, like "u8 flag", and reset every function if corresponding bit is 1?
I'm afraid a boolean won't do, at least not long term. I think it wants to
be an enumeration (i.e. a set of enumeration-like #define-s). And just to
stress it again: The extra argument is _not_ primarily for the looping over
all functions. It is to convey the kind of reset that was done. The single
vs all function(s) aspect is just a useful side effect this will have.
Jan
[XEN PATCH] automation/eclair_analysis: set MISRA C Rule 10.2 as clean
This rule has no more violations in the codebase, so it can be
set as clean.
No functional change.
Signed-off-by: Nicola Vetrini
---
automation/eclair_analysis/ECLAIR/tagging.ecl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl
b/automation/eclair_analysis/ECLAIR/tagging.ecl
index acea15f486a1..b7a9f75b275b 100644
--- a/automation/eclair_analysis/ECLAIR/tagging.ecl
+++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
@@ -19,7 +19,7 @@
-doc_begin="Clean guidelines: new violations for these guidelines are not
accepted."
--service_selector={clean_guidelines_common,
"MC3R1.D1.1||MC3R1.D2.1||MC3R1.D4.1||MC3R1.D4.11||MC3R1.D4.14||MC3R1.R11.7||MC3R1.R11.9||MC3R1.R12.5||MC3R1.R1.1||MC3R1.R1.3||MC3R1.R1.4||MC3R1.R14.1||MC3R1.R16.7||MC3R1.R17.1||MC3R1.R17.3||MC3R1.R17.4||MC3R1.R17.5||MC3R1.R17.6||MC3R1.R20.13||MC3R1.R20.14||MC3R1.R20.4||MC3R1.R20.9||MC3R1.R21.10||MC3R1.R21.13||MC3R1.R21.19||MC3R1.R21.21||MC3R1.R21.9||MC3R1.R2.2||MC3R1.R22.2||MC3R1.R22.4||MC3R1.R22.5||MC3R1.R22.6||MC3R1.R2.6||MC3R1.R3.1||MC3R1.R3.2||MC3R1.R4.1||MC3R1.R4.2||MC3R1.R5.1||MC3R1.R5.2||MC3R1.R5.4||MC3R1.R5.6||MC3R1.R6.1||MC3R1.R6.2||MC3R1.R7.1||MC3R1.R7.2||MC3R1.R7.4||MC3R1.R8.1||MC3R1.R8.10||MC3R1.R8.12||MC3R1.R8.14||MC3R1.R8.2||MC3R1.R8.5||MC3R1.R8.6||MC3R1.R8.8||MC3R1.R9.2||MC3R1.R9.3||MC3R1.R9.4||MC3R1.R9.5"
+-service_selector={clean_guidelines_common,
"MC3R1.D1.1||MC3R1.D2.1||MC3R1.D4.1||MC3R1.D4.11||MC3R1.D4.14||MC3R1.R10.2||MC3R1.R11.7||MC3R1.R11.9||MC3R1.R12.5||MC3R1.R1.1||MC3R1.R1.3||MC3R1.R1.4||MC3R1.R14.1||MC3R1.R16.7||MC3R1.R17.1||MC3R1.R17.3||MC3R1.R17.4||MC3R1.R17.5||MC3R1.R17.6||MC3R1.R20.13||MC3R1.R20.14||MC3R1.R20.4||MC3R1.R20.9||MC3R1.R21.10||MC3R1.R21.13||MC3R1.R21.19||MC3R1.R21.21||MC3R1.R21.9||MC3R1.R2.2||MC3R1.R22.2||MC3R1.R22.4||MC3R1.R22.5||MC3R1.R22.6||MC3R1.R2.6||MC3R1.R3.1||MC3R1.R3.2||MC3R1.R4.1||MC3R1.R4.2||MC3R1.R5.1||MC3R1.R5.2||MC3R1.R5.4||MC3R1.R5.6||MC3R1.R6.1||MC3R1.R6.2||MC3R1.R7.1||MC3R1.R7.2||MC3R1.R7.4||MC3R1.R8.1||MC3R1.R8.10||MC3R1.R8.12||MC3R1.R8.14||MC3R1.R8.2||MC3R1.R8.5||MC3R1.R8.6||MC3R1.R8.8||MC3R1.R9.2||MC3R1.R9.3||MC3R1.R9.4||MC3R1.R9.5"
}
-setq=target,getenv("XEN_TARGET_ARCH")
--
2.34.1
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
Hi Juergen:
On 2024/5/17 18:03, Jürgen Groß wrote:
> On 17.05.24 11:50, Jan Beulich wrote:
>> On 17.05.2024 11:28, Chen, Jiqian wrote:
>>> On 2024/5/17 16:20, Jan Beulich wrote:
On 17.05.2024 10:08, Chen, Jiqian wrote:
> On 2024/5/16 21:08, Jan Beulich wrote:
>> On 16.05.2024 11:52, Jiqian Chen wrote:
>>> struct physdev_pci_device {
>>> /* IN */
>>> uint16_t seg;
>>
>> Is re-using this struct for this new sub-op sufficient? IOW are all
>> possible resets equal, and hence it doesn't need specifying what kind of
>> reset was done? For example, other than FLR most reset variants reset all
>> functions in one go aiui. Imo that would better require only a single
>> hypercall, just to avoid possible confusion. It also reads as if FLR
>> would
>> not reset as many registers as other reset variants would.
> If I understood correctly that you mean in this hypercall it needs to
> support resetting both one function and all functions of a slot(dev)?
> But it can be done for caller to use a cycle to call this reset hypercall
> for each slot function.
It could, yes, but since (aiui) there needs to be an indication of the
kind of reset anyway, we can as well avoid relying on the caller doing
so (and at the same time simplify what the caller needs to do).
>>> Since the corresponding kernel patch has been merged into linux_next branch
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20240515=b272722511d5e8ae580f01830687b8a6b2717f01,
>>> if it's not very mandatory and necessary, just let the caller handle it
>>> temporarily.
>>
>> As also mentioned for the other patch having a corresponding kernel one:
>> The kernel patch would imo better not be merged until the new sub-op is
>> actually finalized.
>
> Oh, sorry to have overlooked that the interfcae change isn't yet committed on
> Xen side.
>
> I'll drop the patch from my linux-next branch.
Thanks. I will modify my code according to Jan's requirements and send a new
version soon.
>
>
> Juergen
>
--
Best regards,
Jiqian Chen.
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 17.05.24 11:50, Jan Beulich wrote:
On 17.05.2024 11:28, Chen, Jiqian wrote:
On 2024/5/17 16:20, Jan Beulich wrote:
On 17.05.2024 10:08, Chen, Jiqian wrote:
On 2024/5/16 21:08, Jan Beulich wrote:
On 16.05.2024 11:52, Jiqian Chen wrote:
struct physdev_pci_device {
/* IN */
uint16_t seg;
Is re-using this struct for this new sub-op sufficient? IOW are all
possible resets equal, and hence it doesn't need specifying what kind of
reset was done? For example, other than FLR most reset variants reset all
functions in one go aiui. Imo that would better require only a single
hypercall, just to avoid possible confusion. It also reads as if FLR would
not reset as many registers as other reset variants would.
If I understood correctly that you mean in this hypercall it needs to support
resetting both one function and all functions of a slot(dev)?
But it can be done for caller to use a cycle to call this reset hypercall for
each slot function.
It could, yes, but since (aiui) there needs to be an indication of the
kind of reset anyway, we can as well avoid relying on the caller doing
so (and at the same time simplify what the caller needs to do).
Since the corresponding kernel patch has been merged into linux_next branch
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20240515=b272722511d5e8ae580f01830687b8a6b2717f01,
if it's not very mandatory and necessary, just let the caller handle it
temporarily.
As also mentioned for the other patch having a corresponding kernel one:
The kernel patch would imo better not be merged until the new sub-op is
actually finalized.
Oh, sorry to have overlooked that the interfcae change isn't yet committed on
Xen side.
I'll drop the patch from my linux-next branch.
Juergen
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 2024/5/17 17:50, Jan Beulich wrote:
> On 17.05.2024 11:28, Chen, Jiqian wrote:
>> On 2024/5/17 16:20, Jan Beulich wrote:
>>> On 17.05.2024 10:08, Chen, Jiqian wrote:
On 2024/5/16 21:08, Jan Beulich wrote:
> On 16.05.2024 11:52, Jiqian Chen wrote:
>> struct physdev_pci_device {
>> /* IN */
>> uint16_t seg;
>
> Is re-using this struct for this new sub-op sufficient? IOW are all
> possible resets equal, and hence it doesn't need specifying what kind of
> reset was done? For example, other than FLR most reset variants reset all
> functions in one go aiui. Imo that would better require only a single
> hypercall, just to avoid possible confusion. It also reads as if FLR would
> not reset as many registers as other reset variants would.
If I understood correctly that you mean in this hypercall it needs to
support resetting both one function and all functions of a slot(dev)?
But it can be done for caller to use a cycle to call this reset hypercall
for each slot function.
>>>
>>> It could, yes, but since (aiui) there needs to be an indication of the
>>> kind of reset anyway, we can as well avoid relying on the caller doing
>>> so (and at the same time simplify what the caller needs to do).
>> Since the corresponding kernel patch has been merged into linux_next branch
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20240515=b272722511d5e8ae580f01830687b8a6b2717f01,
>> if it's not very mandatory and necessary, just let the caller handle it
>> temporarily.
>
> As also mentioned for the other patch having a corresponding kernel one:
> The kernel patch would imo better not be merged until the new sub-op is
> actually finalized.
OK, what should I do next step?
Upstream a patch to revert the merged patch on kernel side?
>
>> Or it can add a new hypercall to reset all functions in one go in future
>> potential requirement, like PHYSDEVOP_pci_device_state_reset_all_func.
>
> I disagree. We shouldn't introduce incomplete sub-ops. At the very least,
> if you want to stick to the present form, I'd expect you to supply reasons
> why distinguishing different reset forms is not necessary (now or later).
OK, if want to distinguish different reset, is it acceptable to add a
parameter, like "u8 flag", and reset every function if corresponding bit is 1?
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [PATCH v9 03/15] xen/bitops: implement fls{l}() in common logic
On 17.05.2024 11:06, Oleksii K. wrote:
> On Wed, 2024-05-15 at 11:09 +0200, Jan Beulich wrote:
>> But this then needs carrying through to ...
>>
>>> --- a/xen/arch/arm/include/asm/arm64/bitops.h
>>> +++ b/xen/arch/arm/include/asm/arm64/bitops.h
>>> @@ -22,17 +22,15 @@ static /*__*/always_inline unsigned long
>>> __ffs(unsigned long word)
>>> */
>>> #define ffz(x) __ffs(~(x))
>>>
>>> -static inline int flsl(unsigned long x)
>>> +static inline int arch_flsl(unsigned long x)
>>
>> ... e.g. here. You don't want to introduce signed/unsigned
>> mismatches.
> Is it critical for x86 to return int for flsl() and fls() or I can
> update the return type for x86 too?
The return types ought to be changed imo, for everything to end up
consistent.
>static always_inline int arch_flsl(unsigned long x)
>{
>long r;
>
>asm ( "bsr %1,%0\n\t"
> "jnz 1f\n\t"
> "mov $-1,%0\n"
> "1:" : "=r" (r) : "rm" (x));
>return (int)r+1;
>}
>#define arch_flsl arch_flsl
>
>static always_inline int arch_fls(unsigned int x)
>{
>int r;
>
>asm ( "bsr %1,%0\n\t"
> "jnz 1f\n\t"
> "mov $-1,%0\n"
> "1:" : "=r" (r) : "rm" (x));
>return r + 1;
>}
>#define arch_fls arch_fls
>
> Any specific reason why 'long' and 'int' types for r are used?
I don't think so. I expect it merely fits with no-one having cared about
signedness back at the time.
Jan
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 17.05.2024 11:28, Chen, Jiqian wrote:
> On 2024/5/17 16:20, Jan Beulich wrote:
>> On 17.05.2024 10:08, Chen, Jiqian wrote:
>>> On 2024/5/16 21:08, Jan Beulich wrote:
On 16.05.2024 11:52, Jiqian Chen wrote:
> struct physdev_pci_device {
> /* IN */
> uint16_t seg;
Is re-using this struct for this new sub-op sufficient? IOW are all
possible resets equal, and hence it doesn't need specifying what kind of
reset was done? For example, other than FLR most reset variants reset all
functions in one go aiui. Imo that would better require only a single
hypercall, just to avoid possible confusion. It also reads as if FLR would
not reset as many registers as other reset variants would.
>>> If I understood correctly that you mean in this hypercall it needs to
>>> support resetting both one function and all functions of a slot(dev)?
>>> But it can be done for caller to use a cycle to call this reset hypercall
>>> for each slot function.
>>
>> It could, yes, but since (aiui) there needs to be an indication of the
>> kind of reset anyway, we can as well avoid relying on the caller doing
>> so (and at the same time simplify what the caller needs to do).
> Since the corresponding kernel patch has been merged into linux_next branch
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20240515=b272722511d5e8ae580f01830687b8a6b2717f01,
> if it's not very mandatory and necessary, just let the caller handle it
> temporarily.
As also mentioned for the other patch having a corresponding kernel one:
The kernel patch would imo better not be merged until the new sub-op is
actually finalized.
> Or it can add a new hypercall to reset all functions in one go in future
> potential requirement, like PHYSDEVOP_pci_device_state_reset_all_func.
I disagree. We shouldn't introduce incomplete sub-ops. At the very least,
if you want to stick to the present form, I'd expect you to supply reasons
why distinguishing different reset forms is not necessary (now or later).
Jan
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 2024/5/17 16:20, Jan Beulich wrote:
> On 17.05.2024 10:08, Chen, Jiqian wrote:
>> On 2024/5/16 21:08, Jan Beulich wrote:
>>> On 16.05.2024 11:52, Jiqian Chen wrote:
struct physdev_pci_device {
/* IN */
uint16_t seg;
>>>
>>> Is re-using this struct for this new sub-op sufficient? IOW are all
>>> possible resets equal, and hence it doesn't need specifying what kind of
>>> reset was done? For example, other than FLR most reset variants reset all
>>> functions in one go aiui. Imo that would better require only a single
>>> hypercall, just to avoid possible confusion. It also reads as if FLR would
>>> not reset as many registers as other reset variants would.
>> If I understood correctly that you mean in this hypercall it needs to
>> support resetting both one function and all functions of a slot(dev)?
>> But it can be done for caller to use a cycle to call this reset hypercall
>> for each slot function.
>
> It could, yes, but since (aiui) there needs to be an indication of the
> kind of reset anyway, we can as well avoid relying on the caller doing
> so (and at the same time simplify what the caller needs to do).
Since the corresponding kernel patch has been merged into linux_next branch
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20240515=b272722511d5e8ae580f01830687b8a6b2717f01,
if it's not very mandatory and necessary, just let the caller handle it
temporarily.
Or it can add a new hypercall to reset all functions in one go in future
potential requirement, like PHYSDEVOP_pci_device_state_reset_all_func.
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [PATCH v9 03/15] xen/bitops: implement fls{l}() in common logic
On Wed, 2024-05-15 at 11:09 +0200, Jan Beulich wrote:
> But this then needs carrying through to ...
>
> > --- a/xen/arch/arm/include/asm/arm64/bitops.h
> > +++ b/xen/arch/arm/include/asm/arm64/bitops.h
> > @@ -22,17 +22,15 @@ static /*__*/always_inline unsigned long
> > __ffs(unsigned long word)
> > */
> > #define ffz(x) __ffs(~(x))
> >
> > -static inline int flsl(unsigned long x)
> > +static inline int arch_flsl(unsigned long x)
>
> ... e.g. here. You don't want to introduce signed/unsigned
> mismatches.
Is it critical for x86 to return int for flsl() and fls() or I can
update the return type for x86 too?
static always_inline int arch_flsl(unsigned long x)
{
long r;
asm ( "bsr %1,%0\n\t"
"jnz 1f\n\t"
"mov $-1,%0\n"
"1:" : "=r" (r) : "rm" (x));
return (int)r+1;
}
#define arch_flsl arch_flsl
static always_inline int arch_fls(unsigned int x)
{
int r;
asm ( "bsr %1,%0\n\t"
"jnz 1f\n\t"
"mov $-1,%0\n"
"1:" : "=r" (r) : "rm" (x));
return r + 1;
}
#define arch_fls arch_fls
Any specific reason why 'long' and 'int' types for r are used?
~ Oleksii
Re: [XEN PATCH v8 3/5] x86/pvh: Add PHYSDEVOP_setup_gsi for PVH dom0
On 17.05.2024 11:00, Chen, Jiqian wrote: > On 2024/5/16 21:49, Jan Beulich wrote: >> On 16.05.2024 11:52, Jiqian Chen wrote: >>> --- a/xen/arch/x86/hvm/hypercall.c >>> +++ b/xen/arch/x86/hvm/hypercall.c >>> @@ -76,6 +76,11 @@ long hvm_physdev_op(int cmd, >>> XEN_GUEST_HANDLE_PARAM(void) arg) >>> case PHYSDEVOP_unmap_pirq: >>> break; >>> >>> +case PHYSDEVOP_setup_gsi: >>> +if ( !is_hardware_domain(currd) ) >>> +return -EOPNOTSUPP; >>> +break; >>> + >>> case PHYSDEVOP_eoi: >>> case PHYSDEVOP_irq_status_query: >>> case PHYSDEVOP_get_free_pirq: >> >> Below here we have a hardware-domain-only block already. Any reason not >> to add to there? Yes, that uses -ENOSYS. Imo that wants changing anyway, >> but even without that to me it would seem more consistent overall to have >> the new case simply added there. > Ah yes, I remembered you suggest me to use EOPNOTSUPP in v4, if change to > ENOSYS is also fine, I will move to below in next version. > >> >> Just to double check: Is there a respective Linux patch already (if so, >> cross-linking the patches may be helpful)? > Yes, my corresponding kernel patch: > https://lore.kernel.org/lkml/20240515065011.13797-1-jiqian.c...@amd.com/T/#mc56b111562d7c67886da905e306f12b3ef8076b4 > > Do you mean I need to add this link into the commit message once the kernel > patch is accepted? Not necessarily the commit message itself, but below the --- marker. If the kernel patch was accepted earlier than the Xen one (which imo it better shouldn't be), you'd likely want to move to pointing at the resulting commit. Jan
Re: [XEN PATCH v8 3/5] x86/pvh: Add PHYSDEVOP_setup_gsi for PVH dom0
On 2024/5/16 21:49, Jan Beulich wrote: > On 16.05.2024 11:52, Jiqian Chen wrote: >> --- a/xen/arch/x86/hvm/hypercall.c >> +++ b/xen/arch/x86/hvm/hypercall.c >> @@ -76,6 +76,11 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) >> arg) >> case PHYSDEVOP_unmap_pirq: >> break; >> >> +case PHYSDEVOP_setup_gsi: >> +if ( !is_hardware_domain(currd) ) >> +return -EOPNOTSUPP; >> +break; >> + >> case PHYSDEVOP_eoi: >> case PHYSDEVOP_irq_status_query: >> case PHYSDEVOP_get_free_pirq: > > Below here we have a hardware-domain-only block already. Any reason not > to add to there? Yes, that uses -ENOSYS. Imo that wants changing anyway, > but even without that to me it would seem more consistent overall to have > the new case simply added there. Ah yes, I remembered you suggest me to use EOPNOTSUPP in v4, if change to ENOSYS is also fine, I will move to below in next version. > > Just to double check: Is there a respective Linux patch already (if so, > cross-linking the patches may be helpful)? Yes, my corresponding kernel patch: https://lore.kernel.org/lkml/20240515065011.13797-1-jiqian.c...@amd.com/T/#mc56b111562d7c67886da905e306f12b3ef8076b4 Do you mean I need to add this link into the commit message once the kernel patch is accepted? > Or does PVH Linux invoke the sub-op already anyway, just that so far it > fails? > > Jan -- Best regards, Jiqian Chen.
Re: [XEN PATCH v8 2/5] x86/pvh: Allow (un)map_pirq when dom0 is PVH
On 2024/5/16 21:29, Jan Beulich wrote:
> On 16.05.2024 11:52, Jiqian Chen wrote:
>> If run Xen with PVH dom0 and hvm domU, hvm will map a pirq for
>> a passthrough device by using gsi, see
>> xen_pt_realize->xc_physdev_map_pirq and
>> pci_add_dm_done->xc_physdev_map_pirq.
>
> xen_pt_realize() is in qemu, which imo wants saying here (for being a
> different
> repo), the more that pci_add_dm_done() is in libxl.
OK, I will describe more here(in qemu and in libxl).
>
>> --- a/xen/arch/x86/hvm/hypercall.c
>> +++ b/xen/arch/x86/hvm/hypercall.c
>> @@ -74,6 +74,8 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
>> arg)
>> {
>> case PHYSDEVOP_map_pirq:
>> case PHYSDEVOP_unmap_pirq:
>> +break;
>
> I think this could do with a comment as to why it's permitted as well as
> giving
> a reference to where further restrictions are enforced (or simply mentioning
> the constraint of this only being permitted for management of other domains).
Thanks, will add
/*
* Only being permitted for management of other domains.
* Further restrictions are enforced in do_physdev_op.
*/
>
>> --- a/xen/arch/x86/physdev.c
>> +++ b/xen/arch/x86/physdev.c
>> @@ -305,11 +305,23 @@ ret_t do_physdev_op(int cmd,
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>> case PHYSDEVOP_map_pirq: {
>> physdev_map_pirq_t map;
>> struct msi_info msi;
>> +struct domain *d;
>>
>> ret = -EFAULT;
>> if ( copy_from_guest(, arg, 1) != 0 )
>> break;
>>
>> +d = rcu_lock_domain_by_any_id(map.domid);
>> +if ( d == NULL )
>> +return -ESRCH;
>> +/* If caller is the same HVM guest as current, check pirq flag */
>
> The caller is always current. What I think you mean is "caller is same as
> the subject domain".
Yes, I want to prevent self-map when subject domain(domU) doesn't have
X86_EMU_USE_PIRQ flag.
> I'm also having trouble with seeing the usefulness of saying "check pirq
> flag". Instead I think you want to state the
> restriction here that you actually mean to enforce (which would also mean
> mentioning PVH in some way, to distinguish from the "normal HVM" case).
Yes, PVH and the HVM without X86_EMU_USE_PIRQ flag,
If a HVM has X86_EMU_USE_PIRQ flag, map_pirq should be permitted.
I will change this comment to be:
/* Prevent self-map when domain has no X86_EMU_USE_PIRQ flag */
>
>> +if ( !is_pv_domain(d) && !has_pirq(d) && map.domid == DOMID_SELF )
>
> You exclude DOMID_SELF but not the domain's ID? Why not simply check d
> being current->domain, thus covering both cases?
> Plus you could use rcu_lock_domain_by_id() to exclude DOMID_SELF, and you
> could use
> rcu_lock_remote_domain_by_id() to exclude the local domain altogether.
But there is a case that hvm hold PIRQ flag and DOMID_SELF id will do this
pirq_map, see code
physdev_map_pirq.
I think change to check d being current->domain is more suitable.
> Finally I'm not even sure you need the RCU lock here (else you could
> use knownalive_domain_from_domid()). But perhaps that's better to cover
> the qemu-in-stubdom case, which we have to consider potentially malicious.
Yes, for potential safety reasons, let's keep the RCU lock.
>
> I'm also inclined to suggest to use is_hvm_domain() here in favor of
> !is_pv_domain().
OK, will change to is_hvm_domain in next version.
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [PATCH v9 02/15] xen: introduce generic non-atomic test_*bit()
On Thu, 2024-05-16 at 12:49 +0200, Jan Beulich wrote:
> > Anyway, I am not sure I understand which approach I should use in
> > this
> > patch. You mentioned that possibly test_and_() can't have a generic
> > form, meaning it won't be a set of arch_test_and_() functions.
> >
> > So, can I rename arch__test_() and generic__test_() to arch_test_()
> > and
> > generic_test_(), respectively, and use the renamed functions in
> > _test_and*() in xen/bitops.h? Is my understanding correct?
>
> You could. You could also stick to what you have now - as said, I can
> accept that with the worked out explanation. Or you could switch to
> using arch__test_bit() and generic__test_bit(), thus having the
> double
> inner underscores identify "internal to the implementation"
> functions.
> My preference would be in backwards order of what I have just
> enumerated
> as possible options. I wonder whether really no-one else has any
> opinion
> here ...
I see that __test_bit() doesn't exist now and perhaps won't exist at
all, but in this patch we are providing the generic for test_bit(), not
__test_bit(). Thereby according to provided by me naming for test_bit()
should be defined using {generic, arch}_test_bit().
~ Oleksii
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 17.05.2024 10:08, Chen, Jiqian wrote:
> On 2024/5/16 21:08, Jan Beulich wrote:
>> On 16.05.2024 11:52, Jiqian Chen wrote:
>>> struct physdev_pci_device {
>>> /* IN */
>>> uint16_t seg;
>>
>> Is re-using this struct for this new sub-op sufficient? IOW are all
>> possible resets equal, and hence it doesn't need specifying what kind of
>> reset was done? For example, other than FLR most reset variants reset all
>> functions in one go aiui. Imo that would better require only a single
>> hypercall, just to avoid possible confusion. It also reads as if FLR would
>> not reset as many registers as other reset variants would.
> If I understood correctly that you mean in this hypercall it needs to support
> resetting both one function and all functions of a slot(dev)?
> But it can be done for caller to use a cycle to call this reset hypercall for
> each slot function.
It could, yes, but since (aiui) there needs to be an indication of the
kind of reset anyway, we can as well avoid relying on the caller doing
so (and at the same time simplify what the caller needs to do).
Jan
Re: [XEN PATCH v8 1/5] xen/vpci: Clear all vpci status of device
On 2024/5/16 21:08, Jan Beulich wrote:
> On 16.05.2024 11:52, Jiqian Chen wrote:
>> @@ -67,6 +68,41 @@ ret_t pci_physdev_op(int cmd,
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>> break;
>> }
>>
>> +case PHYSDEVOP_pci_device_state_reset: {
>> +struct physdev_pci_device dev;
>> +struct pci_dev *pdev;
>> +pci_sbdf_t sbdf;
>> +
>> +if ( !is_pci_passthrough_enabled() )
>> +return -EOPNOTSUPP;
>> +
>> +ret = -EFAULT;
>> +if ( copy_from_guest(, arg, 1) != 0 )
>> +break;
>> +sbdf = PCI_SBDF(dev.seg, dev.bus, dev.devfn);
>> +
>> +ret = xsm_resource_setup_pci(XSM_PRIV, sbdf.sbdf);
>> +if ( ret )
>> +break;
>
> Daniel, is re-use of this hook appropriate here?
In the v2 of this series, Daniel and Roger have agreed that reusing
xsm_resource_setup_pci is enough.
>
>> +pcidevs_lock();
>> +pdev = pci_get_pdev(NULL, sbdf);
>> +if ( !pdev )
>> +{
>> +pcidevs_unlock();
>> +ret = -ENODEV;
>> +break;
>> +}
>> +
>> +write_lock(>domain->pci_lock);
>> +ret = vpci_reset_device_state(pdev);
>> +write_unlock(>domain->pci_lock);
>> +pcidevs_unlock();
>
> Can't this be done right after the write_lock()?
You are right, I will move it in next version.
>
>> +if ( ret )
>> +printk(XENLOG_ERR "%pp: failed to reset PCI device state\n",
>> );
>
> s/PCI/vPCI/ (at least as long as nothing else is done here)
OK, will change in next version.
>
>> --- a/xen/drivers/vpci/vpci.c
>> +++ b/xen/drivers/vpci/vpci.c
>> @@ -115,6 +115,16 @@ int vpci_assign_device(struct pci_dev *pdev)
>>
>> return rc;
>> }
>> +
>> +int vpci_reset_device_state(struct pci_dev *pdev)
>> +{
>> +ASSERT(pcidevs_locked());
>
> Is this necessary for ...
>
>> +ASSERT(rw_is_write_locked(>domain->pci_lock));
>> +
>> +vpci_deassign_device(pdev);
>> +return vpci_assign_device(pdev);
>
> ... either of these calls? Both functions do themselves only have the
> 2nd of the asserts you add.
I checked codes again; I will remove the two asserts here in next version.
>
>> --- a/xen/include/public/physdev.h
>> +++ b/xen/include/public/physdev.h
>> @@ -296,6 +296,13 @@ DEFINE_XEN_GUEST_HANDLE(physdev_pci_device_add_t);
>> */
>> #define PHYSDEVOP_prepare_msix 30
>> #define PHYSDEVOP_release_msix 31
>> +/*
>> + * Notify the hypervisor that a PCI device has been reset, so that any
>> + * internally cached state is regenerated. Should be called after any
>> + * device reset performed by the hardware domain.
>> + */
>> +#define PHYSDEVOP_pci_device_state_reset 32
>
> Nit: Just a single blank as a separator will do here, for going past the
> columnar arrangement of other #define-s anyway.
OK.
>
>> struct physdev_pci_device {
>> /* IN */
>> uint16_t seg;
>
> Is re-using this struct for this new sub-op sufficient? IOW are all
> possible resets equal, and hence it doesn't need specifying what kind of
> reset was done? For example, other than FLR most reset variants reset all
> functions in one go aiui. Imo that would better require only a single
> hypercall, just to avoid possible confusion. It also reads as if FLR would
> not reset as many registers as other reset variants would.
If I understood correctly that you mean in this hypercall it needs to support
resetting both one function and all functions of a slot(dev)?
But it can be done for caller to use a cycle to call this reset hypercall for
each slot function.
>
> This may seem to not matter right now, but this is a stable interface you
> add, and hence modifying it later will be cumbersome, if possible at all.
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [PATCH v2 1/4] LICENSES: Add MIT-0 (MIT No Attribution)
> On 16 May 2024, at 19:58, Andrew Cooper wrote: > > We are about to import code licensed under MIT-0. It's compatible for us to > use, so identify it as a permitted license. > > Signed-off-by: Andrew Cooper > --- > CC: Anthony PERARD > CC: Juergen Gross > CC: George Dunlap > CC: Jan Beulich > CC: Stefano Stabellini > CC: Julien Grall > CC: Christian Lindig > CC: Edwin Török Acked-by: Christian Lindig
Re: [PATCH v2 for-4.19 0/4] Drop libsystemd
> On 16 May 2024, at 19:58, Andrew Cooper wrote:
>
> On advise from the systemd leadership.
>
> v2:
> * Import the free-standing example and use that to retain existing
> functionality.
>
> Andrew Cooper (4):
> LICENSES: Add MIT-0 (MIT No Attribution)
> tools: Import standalone sd_notify() implementation from systemd
> tools/{c,o}xenstored: Don't link against libsystemd
> tools: Drop libsystemd as a dependency
>
> LICENSES/MIT-0| 31 +++
> automation/build/archlinux/current.dockerfile | 1 +
> .../build/suse/opensuse-leap.dockerfile | 1 +
> .../build/suse/opensuse-tumbleweed.dockerfile | 1 +
> automation/build/ubuntu/focal.dockerfile | 1 +
> config/Tools.mk.in| 2 -
> m4/systemd.m4 | 9 -
> tools/configure | 256 --
> tools/include/xen-sd-notify.h | 98 +++
> tools/ocaml/xenstored/Makefile| 2 -
> tools/ocaml/xenstored/systemd_stubs.c | 2 +-
> tools/xenstored/Makefile | 5 -
> tools/xenstored/posix.c | 4 +-
> 13 files changed, 136 insertions(+), 277 deletions(-)
> create mode 100644 LICENSES/MIT-0
> create mode 100644 tools/include/xen-sd-notify.h
>
>
> base-commit: 977d98e67c2e929c62aa1f495fc4c6341c45abb5
> --
> 2.30.2
>
Acked-by: Christian Lindig
[xen-unstable-smoke test] 186024: tolerable all pass - PUSHED
flight 186024 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/186024/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 21611c68702dae2e18cb519a6e166cdceeaff4ca baseline version: xen 53dc37829c31edf02e8fc56aeccca8d60f6e2f5e Last test of basis 186022 2024-05-16 23:00:22 Z0 days Testing same since 186024 2024-05-17 04:02:10 Z0 days1 attempts People who touched revisions under test: Jan Beulich Stefano Stabellini Stefano Stabellini jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 53dc37829c..21611c6870 21611c68702dae2e18cb519a6e166cdceeaff4ca -> smoke
Re: [PATCH for-4.19?] xen/x86: pretty print interrupt CPU affinity masks
On 16.05.2024 19:13, Andrew Cooper wrote:
> On 15/05/2024 4:29 pm, Roger Pau Monne wrote:
>> Print the CPU affinity masks as numeric ranges instead of plain hexadecimal
>> bitfields.
>>
>> Signed-off-by: Roger Pau Monné
>> ---
>> xen/arch/x86/irq.c | 10 +-
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
>> index 80ba8d9fe912..3b951d81bd6d 100644
>> --- a/xen/arch/x86/irq.c
>> +++ b/xen/arch/x86/irq.c
>> @@ -1934,10 +1934,10 @@ void do_IRQ(struct cpu_user_regs *regs)
>> if ( ~irq < nr_irqs && irq_desc_initialized(desc) )
>> {
>> spin_lock(>lock);
>> -printk("IRQ%d a=%04lx[%04lx,%04lx] v=%02x[%02x] t=%s
>> s=%08x\n",
>> - ~irq, *cpumask_bits(desc->affinity),
>> - *cpumask_bits(desc->arch.cpu_mask),
>> - *cpumask_bits(desc->arch.old_cpu_mask),
>> +printk("IRQ%d a={%*pbl}[{%*pbl},{%*pbl}] v=%02x[%02x]
>> t=%s s=%08x\n",
>
> Looking at this more closely, there's still some information obfuscation
> going on.
>
> How about "... a={} o={} n={} v=..."
>
> so affinity, old and new masks are all stated explicitly, instead of
> having to remember what the square brackets mean, and in particular that
> the masks are backwards?
Just one question: Why put old ahead of new? Aiui that's what you refer to
with "backwards", yet I don't see what's backwards about it. Old would
possibly matter only when the IRQ was recently moved, whereas new (actually:
Why "new"?) would matter at all times. I'd see "... a={} m={} o={} v=..."
as more appropriate.
Jan
Re: [PATCH] Revert "evtchn: refuse EVTCHNOP_status for Xen-bound event channels"
On 17.05.2024 03:22, Daniel P. Smith wrote: > On 5/16/24 02:41, Jan Beulich wrote: >> On 14.05.2024 11:25, Jan Beulich wrote: >>> On 03.04.2024 08:16, Jan Beulich wrote: On 02.04.2024 19:06, Andrew Cooper wrote: > The commit makes a claim without any kind of justification. Well, what does "have no business" leave open? > The claim is false, and the commit broke lsevtchn in dom0. Or alternatively lsevtchn was doing something that was never meant to work (from Xen's perspective). > It is also quite > obvious from XSM_TARGET that it has broken device model stubdoms too. Why would that be "obvious"? What business would a stubdom have to look at Xen's side of an evtchn? > Whether to return information about a xen-owned evtchn is a matter of > policy, > and it's not acceptable to short circuit the XSM on the matter. I can certainly accept this as one possible view point. As in so many cases I'm afraid I dislike you putting it as if it was the only possible one. In summary: The supposed justification you claim is missing in the original change is imo also missing here then: What business would any entity in the system have to look at Xen's side of an event channel? Back at the time, 3 people agreed that it's "none". >>> >>> You've never responded to this reply of mine, or its follow-up. You also >>> didn't chime in on the discussion Daniel and I were having. I consider my >>> objections unaddressed, and in fact I continue to consider the change to >>> be wrong. Therefore it was inappropriate for you to commit it; it needs >>> reverting asap. If you're not going to do so, I will. >> >> For the record: With Andrew having clarified that in his revert's >> description: >> >> "The claim is false; it broke lsevtchn in dom0, a debugging utility which >> absolutely does care about all of the domain's event channels." >> >> "all of the domain's event channels" means to include event channels Xen >> uses for its own, and merely puts in the domain's table for ease of >> handling, I've agreed that - in the absence of any better debugging >> means - the revert may stay in place. For context, my prior understanding >> of the issue was that lsevtchn simply stops probing further channels when >> getting back any kind of error from EVTCHNOP_status (which I continue to >> think wants addressing there, by a future version of a patch that was >> already sent). However, there are follow-on concerns I have: >> >> 1) In the discussion George claimed that exposing status information in >> an uncontrolled manner is okay. I'm afraid I have to disagree, seeing >> how a similar assumption by CPU designers has led to a flood of >> vulnerabilities over the last 6+ years. Information exposure imo is never >> okay, unless it can be _proven_ that absolutely nothing "useful" can be >> inferred from it. (I'm having difficulty seeing how such a proof might >> look like.) > > Jan, I would agree with you that any resources exposed to the guest, > even if that resource is status information, should be behind an XSM > check. I would, however, have to disagree the position over proving > absolutely nothing "useful" can be inferred. Prior to spectre becoming > commonly aware, no one considered proving there needed to be protections > against out-of-order instruction execution being used to bypass branch > conditions. Interesting perspective. > Predicting the future will more often than not fail, but > with an XSM check in place, the dummy/default policy can just be updated > with the general safe decision and others can use FLASK to provide fine > grain access. I have to admit I have difficulty seeing how one would adjust dummy to do restrict things by default, at least as long as XSM_TARGET is used. >> 2) Me pointing out that the XSM hook might similarly get in the way of >> debugging, Andrew suggested that this is not an issue because any sensible >> XSM policy used in such an environment would grant sufficient privilege to >> Dom0. Yet that then still doesn't cover why DomU-s also can obtain status >> for Xen-internal event channels. The debugging argument then becomes weak, >> as in that case the XSM hook is possibly going to get in the way. > > And this is where we have a fundamental difference. Event channels are > XSM labeled objects that are always connected to a guest. If they were > truly Xen-internal, then a guest would have no way to get > access/reference them. And that's what I'd like to achieve. The fact that Xen puts these event channels in the guest's table is a mere implementation detail. They could as well be kept elsewhere, just that handling then would (likely) be more cumbersome. > Again, I never disagreed with whether the guest > should or should not be able to access them. I did ask for a better > explanation than, "has no business", which is a statement of opinion not > of
Re: [PATCH v2 6/8] xen/arm: Add XEN_DOMCTL_dt_overlay DOMCTL and related operations
On 17.05.2024 03:36, Henry Wang wrote:
> On 5/16/2024 8:31 PM, Jan Beulich wrote:
>> On 16.05.2024 12:03, Henry Wang wrote:
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -1190,6 +1190,17 @@ struct xen_domctl_vmtrace_op {
>>> typedef struct xen_domctl_vmtrace_op xen_domctl_vmtrace_op_t;
>>> DEFINE_XEN_GUEST_HANDLE(xen_domctl_vmtrace_op_t);
>>>
>>> +#if defined(__arm__) || defined (__aarch64__)
>> Nit: Consistent use of blanks please (also again below).
>
> Good catch. Will fix it.
>
>>> +struct xen_domctl_dt_overlay {
>>> +XEN_GUEST_HANDLE_64(const_void) overlay_fdt; /* IN: overlay fdt. */
>>> +uint32_t overlay_fdt_size; /* IN: Overlay dtb size. */
>>> +#define XEN_DOMCTL_DT_OVERLAY_ATTACH3
>>> +#define XEN_DOMCTL_DT_OVERLAY_DETACH4
>> While the numbers don't really matter much, picking 3 and 4 rather than,
>> say, 1 and 2 still looks a little odd.
>
> Well although I agree with you it is indeed a bit odd, the problem of
> this is that, in current implementation I reused the libxl_dt_overlay()
> (with proper backward compatible) to deliver the sysctl and domctl
> depend on the op, and we have:
> #define LIBXL_DT_OVERLAY_ADD 1
> #define LIBXL_DT_OVERLAY_REMOVE 2
> #define LIBXL_DT_OVERLAY_ATTACH 3
> #define LIBXL_DT_OVERLAY_DETACH 4
>
> Then the op-number is passed from the toolstack to Xen, and checked in
> dt_overlay_domctl(). So with this implementation the attach/detach op
> number should be 3 and 4 since 1 and 2 have different meanings.
>
> But I realized that I can also implement a similar API, say
> libxl_dt_overlay_domain() and that way we can reuse 1 and 2 and there is
> not even need to provide backward compatible of libxl_dt_overlay(). So
> would you mind sharing your preference on which approach would you like
> more? Thanks!
While I think tying together libxl and domctl values isn't a very good idea,
if you really want to do so, you'll want to add suitable checking somewhere,
alongside comments. The comments ought to keep people from changing the
values then, while the checks would need to be there for people not paying
attention.
Jan
[ovmf test] 186026: all pass - PUSHED
flight 186026 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/186026/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 284dbac43da752ee34825c8b3f6f9e8281cb5a19 baseline version: ovmf 558a25366d66e9ff96784ed3aab476f808531bf3 Last test of basis 186000 2024-05-15 01:41:14 Z2 days Testing same since 186026 2024-05-17 05:43:05 Z0 days1 attempts People who touched revisions under test: Pakkirisamy ShanmugavelX Shanmugavel Pakkirisamy 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 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 558a25366d..284dbac43d 284dbac43da752ee34825c8b3f6f9e8281cb5a19 -> xen-tested-master
Re: [PATCH] Revert "evtchn: refuse EVTCHNOP_status for Xen-bound event channels"
On 17.05.2024 03:21, Stefano Stabellini wrote: > On Thu, 16 May 2024, Jan Beulich wrote: >> 1) In the discussion George claimed that exposing status information in >> an uncontrolled manner is okay. I'm afraid I have to disagree, seeing >> how a similar assumption by CPU designers has led to a flood of >> vulnerabilities over the last 6+ years. Information exposure imo is never >> okay, unless it can be _proven_ that absolutely nothing "useful" can be >> inferred from it. (I'm having difficulty seeing how such a proof might >> look like.) > > Many would agree that it is better not to expose status information in > an uncontrolled manner. Anyway, let's focus on the actionable. > > >> 2) Me pointing out that the XSM hook might similarly get in the way of >> debugging, Andrew suggested that this is not an issue because any sensible >> XSM policy used in such an environment would grant sufficient privilege to >> Dom0. Yet that then still doesn't cover why DomU-s also can obtain status >> for Xen-internal event channels. The debugging argument then becomes weak, >> as in that case the XSM hook is possibly going to get in the way. >> >> 3) In the discussion Andrew further gave the impression that evtchn_send() >> had no XSM check. Yet it has; the difference to evtchn_status() is that >> the latter uses XSM_TARGET while the former uses XSM_HOOK. (Much like >> evtchn_status() may indeed be useful for debugging, evtchn_send() may be >> similarly useful to allow getting a stuck channel unstuck.) >> >> In summary I continue to think that an outright revert was inappropriate. >> DomU-s should continue to be denied status information on Xen-internal >> event channels, unconditionally and independent of whether dummy, silo, or >> Flask is in use. > > I think DomU-s should continue to be denied status information on > Xen-internal event channels *based on the default dummy, silo, or Flask > policy*. It is not up to us to decide the security policy, only to > enforce it and provide sensible defaults. > > In any case, the XSM_TARGET check in evtchn_status seems to do what we > want? No. XSM_TARGET permits the "owning" (not really, but it's its table) domain access. See xsm_default_action() in xsm/dummy.h. Jan > evtchn_send uses XSM_HOOK, which is weaker, but it doesn't seem to be an > issue because (ignoring the consumer_is_xen check) there is a if(!lchn) > check that would fail on invalid event channels?
[xen-unstable test] 186021: tolerable FAIL - PUSHED
flight 186021 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/186021/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 186013 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 186013 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 186013 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 186013 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 186013 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 186013 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail 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-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-qcow214 migrate-support-checkfail never pass test-armhf-armhf-xl-qcow215 saverestore-support-checkfail never pass test-armhf-armhf-xl-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-vhd 15 saverestore-support-checkfail never pass version targeted for testing: xen ae7584f63678cd9adc1c2f3a1e813b67a6b24544 baseline version: xen 762a848223e0fd61aa87b4e040c83dd2c490fb79 Last test of basis 186013 2024-05-16 08:20:07 Z0 days Testing same since 186021 2024-05-16 21:08:45 Z0 days1 attempts People who touched revisions under test: Jan Beulich Michal Orzel Oleksii Kurochko Rahul Singh Sergiy Kibrik Shawn Anastasio jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-arm64 pass build-armhf pass build-i386
Re: [PATCH] Revert "evtchn: refuse EVTCHNOP_status for Xen-bound event channels"
On 16.05.2024 21:15, Oleksii K. wrote: > I am not deeply familiar with the technical details surrounding XSM, > but if I understand Daniel's point correctly, the original change > violates the access control framework. This suggests to me that the > revert should be merged. > > However, I have a question: if we merge this revert, does it mean that > using channels a user ( domain ) will be able to get information about > certain events such as EVTCHNSTAT_unbound, EVTCHNSTAT_interdomain, > EVTCHNSTAT_pirq, EVTCHNSTAT_virq, and EVTCHNSTAT_IPI (based on the code > of lseventch.c)? Is this information really so critical that it cannot > be exposed for some time until a patch that changes the XSM policy > consistently is provided and merged? > > If this information is indeed critical and should not be exposed, I > think we can consider Daniel's suggestion to add a check to the dummy > XSM policy as a solution. The question isn't whether it's critical. As pointed out elsewhere, my view is that any exposure of information needs to come with a proof that nothing undue can be derived from that information. I see Daniel has responded there, so we'll continue the discussion there. Jan
Re: [PATCH for-4.19?] xen/x86: pretty print interrupt CPU affinity masks
On Thu, May 16, 2024 at 06:13:29PM +0100, Andrew Cooper wrote:
> On 15/05/2024 4:29 pm, Roger Pau Monne wrote:
> > Print the CPU affinity masks as numeric ranges instead of plain hexadecimal
> > bitfields.
> >
> > Signed-off-by: Roger Pau Monné
> > ---
> > xen/arch/x86/irq.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> > index 80ba8d9fe912..3b951d81bd6d 100644
> > --- a/xen/arch/x86/irq.c
> > +++ b/xen/arch/x86/irq.c
> > @@ -1934,10 +1934,10 @@ void do_IRQ(struct cpu_user_regs *regs)
> > if ( ~irq < nr_irqs && irq_desc_initialized(desc) )
> > {
> > spin_lock(>lock);
> > -printk("IRQ%d a=%04lx[%04lx,%04lx] v=%02x[%02x] t=%s
> > s=%08x\n",
> > - ~irq, *cpumask_bits(desc->affinity),
> > - *cpumask_bits(desc->arch.cpu_mask),
> > - *cpumask_bits(desc->arch.old_cpu_mask),
> > +printk("IRQ%d a={%*pbl}[{%*pbl},{%*pbl}] v=%02x[%02x]
> > t=%s s=%08x\n",
>
> Looking at this more closely, there's still some information obfuscation
> going on.
>
> How about "... a={} o={} n={} v=..."
>
> so affinity, old and new masks are all stated explicitly, instead of
> having to remember what the square brackets mean, and in particular that
> the masks are backwards?
>
> Happy to adjust on commit.
Sure, I guess I got used to it and didn't think of adjusting the
format. The only risk is anyone having an automated parser to consume
that information, but I think it's unlikely.
Thanks, Roger.
Re: [PATCH v2 5/8] xen/arm/gic: Allow routing/removing interrupt to running VMs
On 5/16/2024 6:03 PM, Henry Wang wrote:
From: Vikram Garhwal
Currently, routing/removing physical interrupts are only allowed at
the domain creation/destroy time. For use cases such as dynamic device
tree overlay adding/removing, the routing/removing of physical IRQ to
running domains should be allowed.
Removing the above-mentioned domain creation/dying check. Since this
will introduce interrupt state unsync issues for cases when the
interrupt is active or pending in the guest, therefore for these cases
we simply reject the operation. Do it for both new and old vGIC
implementations.
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Signed-off-by: Henry Wang
---
v2:
- Reject the case where the IRQ is active or pending in guest.
---
xen/arch/arm/gic-vgic.c | 8 ++--
xen/arch/arm/gic.c | 15 ---
xen/arch/arm/vgic/vgic.c | 5 +++--
3 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
index 56490dbc43..d1608415f8 100644
--- a/xen/arch/arm/gic-vgic.c
+++ b/xen/arch/arm/gic-vgic.c
@@ -444,14 +444,18 @@ int vgic_connect_hw_irq(struct domain *d, struct vcpu *v,
unsigned int virq,
{
/* The VIRQ should not be already enabled by the guest */
if ( !p->desc &&
- !test_bit(GIC_IRQ_GUEST_ENABLED, >status) )
+ !test_bit(GIC_IRQ_GUEST_ENABLED, >status) &&
+ !test_bit(GIC_IRQ_GUEST_VISIBLE, >status) &&
+ !test_bit(GIC_IRQ_GUEST_ACTIVE, >status) )
p->desc = desc;
else
ret = -EBUSY;
}
else
{
-if ( desc && p->desc != desc )
+if ( desc && p->desc != desc &&
+ (test_bit(GIC_IRQ_GUEST_VISIBLE, >status) ||
+ test_bit(GIC_IRQ_GUEST_ACTIVE, >status)) )
This should be
+if ( (desc && p->desc != desc) ||
+ test_bit(GIC_IRQ_GUEST_VISIBLE, >status) ||
+ test_bit(GIC_IRQ_GUEST_ACTIVE, >status) )
@@ -887,7 +887,8 @@ int vgic_connect_hw_irq(struct domain *d, struct vcpu *vcpu,
}
else/* remove a mapped IRQ */
{
-if ( desc && irq->hwintid != desc->irq )
+if ( desc && irq->hwintid != desc->irq &&
+ (irq->active || irq->pending_latch) )
Same here, this should be
+if ( (desc && irq->hwintid != desc->irq) ||
+ irq->active || irq->pending_latch )
Kind regards,
Henry
