Re: [Xen-devel] [PATCH v3] PCI: Add guard to avoid mapping a invalid msix base address
[+cc Jan] On Thu, Jan 29, 2015 at 11:54:43AM +0800, Yijing Wang wrote: Sometimes, a pci bridge device BAR was not assigned properly. After we call pci_bus_assign_resources(), the resource of the BAR would be reseted. So if we try to enable msix for this device, it will map a invalid resource as the msix base address, and a warning call trace will report. pci_bus_assign_resources() __pci_bus_assign_resources() pbus_assign_resources_sorted() __assign_resources_sorted() assign_requested_resources_sorted() pci_assign_resource() --fail reset_resource() --res-start/end/flags = 0 pcie_port_device_register() init_service_irqs() pcie_port_enable_msix() ... msix_capability_init() msix_map_region() phys_addr = pci_resource_start(dev, bir) + table_offset; If BAR(index=bir) was not assign properly, pci_resource_start(dev, bir) here would return 0, so phys_addr is a invalid physical address of msix. [ 43.094087] [ cut here ] [ 43.097418] WARNING: CPU: 1 PID: 1800 at arch/arm64/mm/ioremap.c:58 __ioremap_caller+0xd4/0xe8() ... [ 43.121694] CPU: 1 PID: 1800 Comm: insmod Tainted: G O 3.16.0 #5 [ 43.127374] Call trace: [ 43.128522] [ffc891d4] dump_backtrace+0x0/0x130 [ 43.132637] [ffc89314] show_stack+0x10/0x1c [ 43.136402] [ffc0004db040] dump_stack+0x74/0x94 [ 43.140166] [ffc986f8] warn_slowpath_common+0x8c/0xb4 [ 43.144804] [ffc987e4] warn_slowpath_null+0x14/0x20 [ 43.149266] [ffc95474] __ioremap_caller+0xd0/0xe8 [ 43.153555] [ffc95498] __ioremap+0xc/0x18 [ 43.157145] [ffc0002cc62c] pci_enable_msix+0x238/0x44c [ 43.161521] [ffc0002cc874] pci_enable_msix_range+0x34/0x80 [ 43.166243] [ffc0002c946c] pcie_port_device_register+0x104/0x480 [ 43.171491] [ffc0002c99f8] pcie_portdrv_probe+0x38/0xa0 [ 43.175952] [ffc0002bd6c8] pci_device_probe+0x78/0xd4 [ 43.180238] [ffc00030e68c] really_probe+0x6c/0x22c [ 43.184265] [ffc00030e8a8] __device_attach+0x5c/0x6c [ 43.188466] [ffc00030cb68] bus_for_each_drv+0x50/0x94 [ 43.192755] [ffc00030e5fc] device_attach+0x9c/0xc0 [ 43.196780] [ffc0002b4c54] pci_bus_add_device+0x38/0x80 [ 43.201243] [ffc0002b5064] pci_bus_add_devices+0x4c/0xd4 [ 43.205791] [ffc93d70] pci_common_init+0x274/0x378 [ 43.210170] [ffbff18e4b8c] $x+0xb8c/0xc88 [pcie] [ 43.214024] [ffc00031013c] platform_drv_probe+0x20/0x58 [ 43.218483] [ffc00030e6e4] really_probe+0xc4/0x22c [ 43.222510] [ffc00030e958] __driver_attach+0xa0/0xa8 [ 43.226708] [ffc00030cab0] bus_for_each_dev+0x54/0x98 [ 43.231000] [ffc00030e234] driver_attach+0x1c/0x28 [ 43.235024] [ffc00030deb0] bus_add_driver+0x14c/0x204 [ 43.239309] [ffc00030f038] driver_register+0x64/0x130 [ 43.243598] [ffc000310110] __platform_driver_register+0x5c/0x68 [ 43.248757] [ffc0003101b4] platform_driver_probe+0x28/0xac [ 43.253485] [ffbff18e4cb8] pcie_init+0x30/0x3c [pcie] [ 43.258293] [ffc815dc] do_one_initcall+0x88/0x19c [ 43.262585] [ffcf6b74] load_module+0xc2c/0xf2c [ 43.266609] [ffcf6fd0] SyS_finit_module+0x78/0x88 [ 43.270897] ---[ end trace ea5eb60837afb5aa ]--- Reported-by: Zhang Jukuo zhangju...@huawei.com Tested-by: Zhang Jukuo zhangju...@huawei.com Signed-off-by: Yijing Wang wangyij...@huawei.com I applied this to pci/msi for v3.20, thanks! I reworked the changelog as follows; let me know if it still doesn't make things clear: commit 6a878e5085fe97bd1e222b7883a1b815fcbbe4ed Author: Yijing Wang wangyij...@huawei.com Date: Wed Jan 28 09:52:17 2015 +0800 PCI: Fail MSI-X mappings if there's no space assigned to MSI-X BAR Unlike MSI, which is configured via registers in the MSI capability in Configuration Space, MSI-X is configured via tables in Memory Space. These MSI-X tables are mapped by a device BAR, and if no Memory Space has been assigned to the BAR, MSI-X cannot be used. Fail MSI-X setup if no space has been assigned for the BAR. Previously, we ioremapped the MSI-X table even if the resource hadn't been assigned. In this case, the resource address is undefined (and is often zero), which may lead to warnings or oopses in this path: pci_enable_msix msix_capability_init msix_map_region ioremap_nocache The PCI core sets resource flags to zero when it can't assign space for the resource (see reset_resource()). There are also some cases where it sets the IORESOURCE_UNSET flag, e.g.,
[Xen-devel] [qemu-upstream-unstable bisection] complete test-amd64-i386-freebsd10-i386
branch xen-unstable xen branch xen-unstable job test-amd64-i386-freebsd10-i386 test guest-localmigrate Tree: linux git://xenbits.xen.org/linux-pvops.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/staging/qemu-xen-unstable.git Tree: qemuu git://xenbits.xen.org/staging/qemu-upstream-unstable.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: qemuu git://xenbits.xen.org/staging/qemu-upstream-unstable.git Bug introduced: 7665d6ba98e20fb05c420de947c1750fd47e5c07 Bug not present: 9026dca821a61b72983778e3dcd92cfc34b02e8b commit 7665d6ba98e20fb05c420de947c1750fd47e5c07 Author: Paul Durrant paul.durr...@citrix.com Date: Tue Jan 20 11:06:19 2015 + Xen: Use the ioreq-server API when available The ioreq-server API added to Xen 4.5 offers better security than the existing Xen/QEMU interface because the shared pages that are used to pass emulation request/results back and forth are removed from the guest's memory space before any requests are serviced. This prevents the guest from mapping these pages (they are in a well known location) and attempting to attack QEMU by synthesizing its own request structures. Hence, this patch modifies configure to detect whether the API is available, and adds the necessary code to use the API if it is. upstream-commit-id: 3996e85c1822e05c50250f8d2d1e57b6bea1229d Signed-off-by: Paul Durrant paul.durr...@citrix.com Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com For bisection revision-tuple graph see: http://www.chiark.greenend.org.uk/~xensrcts/results/bisect.qemu-upstream-unstable.test-amd64-i386-freebsd10-i386.guest-localmigrate.html Revision IDs in each graph node refer, respectively, to the Trees above. Searching for failure / basis pass: 34011 fail [host=grain-weevil] / 33488 [host=scape-moth] 32024 [host=scape-moth] 31848 ok. Failure / basis pass flights: 34011 / 31848 (tree in latest but not in basispass: ovmf) (tree with no url: seabios) Tree: linux git://xenbits.xen.org/linux-pvops.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/staging/qemu-xen-unstable.git Tree: qemuu git://xenbits.xen.org/staging/qemu-upstream-unstable.git Tree: xen git://xenbits.xen.org/xen.git Latest c3b70f0bbb6a883f4afa639286043d3f71fbbddf c530a75c1e6a472b0eb9558310b518f0dfcd8860 b0d42741f8e9a00854c3b3faca1da84bfc69bf22 be11dc1e9172f91e798a8f831b30c14b479e08e8 7106c691a6332cffab4037186d1caa3012ae051e Basis pass d7892a4c389d54bccb9bce8e65eb053a33bbe290 c530a75c1e6a472b0eb9558310b518f0dfcd8860 b0d42741f8e9a00854c3b3faca1da84bfc69bf22 a230ec3101ddda868252c036ea960af2b2d6cd5a 6913fa31fa898f45ecc3b00e2397b8ebc75c8df4 Generating revisions with ./adhoc-revtuple-generator git://xenbits.xen.org/linux-pvops.git#d7892a4c389d54bccb9bce8e65eb053a33bbe290-c3b70f0bbb6a883f4afa639286043d3f71fbbddf git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/staging/qemu-xen-unstable.git#b0d42741f8e9a00854c3b3faca1da84bfc69bf22-b0d42741f8e9a00854c3b3faca1da84bfc69bf22 git://xenbits.xen.org/staging/qemu-upstream-unstable.git#a230ec3101ddda868252c036ea960af2b2d6cd5a-be11dc1e9172f91e798a8f831b30c14b479e08e8 git://xenbits.xen.org/xen.git#6913fa31fa898f45ecc3b00e2397b8ebc75c8df4-7106c691a6332cffab4037186d1caa3012ae051e + exec + sh -xe + cd /export/home/osstest/repos/linux-pvops + git remote set-url origin git://drall.uk.xensource.com:9419/git://xenbits.xen.org/linux-pvops.git + git fetch -p origin +refs/heads/*:refs/remotes/origin/* + exec + sh -xe + cd /export/home/osstest/repos/qemu-upstream-unstable + git remote set-url origin git://drall.uk.xensource.com:9419/git://xenbits.xen.org/staging/qemu-upstream-unstable.git + git fetch -p origin +refs/heads/*:refs/remotes/origin/* + exec + sh -xe + cd /export/home/osstest/repos/xen + git remote set-url origin git://drall.uk.xensource.com:9419/git://xenbits.xen.org/xen.git + git fetch -p origin +refs/heads/*:refs/remotes/origin/* + exec + sh -xe + cd /export/home/osstest/repos/linux-pvops + git remote set-url origin git://drall.uk.xensource.com:9419/git://xenbits.xen.org/linux-pvops.git + git fetch -p origin +refs/heads/*:refs/remotes/origin/* + exec + sh -xe + cd /export/home/osstest/repos/qemu-upstream-unstable + git remote set-url origin git://drall.uk.xensource.com:9419/git://xenbits.xen.org/staging/qemu-upstream-unstable.git + git fetch -p origin +refs/heads/*:refs/remotes/origin/* + exec + sh -xe + cd /export/home/osstest/repos/xen + git remote set-url origin git://drall.uk.xensource.com:9419/git://xenbits.xen.org/xen.git + git fetch
Re: [Xen-devel] [PATCH 0/6] xen/arm: Move in/out code to/from init section
On 02/02/15 11:15, Jan Beulich wrote: On 02.02.15 at 11:58, ian.campb...@citrix.com wrote: On Fri, 2015-01-30 at 11:33 +, Julien Grall wrote: On 30/01/15 11:30, Ian Campbell wrote: On Thu, 2015-01-29 at 18:32 +, Julien Grall wrote: Hello, Ping? Any more review for this version of this series? I was awaiting a version with the declarations in the right places as pointed out by Andy (including his point about definition vs. prototype I'm afraid, which is the style we use). I'm not convince about your last point. We have many places (if not all on ARM) where __init is used in the header. Those are mistakes, it seems. Some of them was even added by you ;). So I would prefer if we keep them, it's more readable. FWIW Linux does it too. I personally don't particularly care where these things go, but other hypervisor maintainers seem to and we should aim for the code base to be consistent where possible. If we want to change this coding style then we should do that directly and explicitly, not through the backdoor by just ignoring the policy. And I think we should strive to keep pointless redundancy down: There's no point in repeating attributes - either they belong on the declaration (when producer and consumers care), or they belong on the definition (when only the producer cares). Repeating them on the definition when the declaration already has them only results in needless churn when such an attribute later gets changed/ removed, and having producer-relevant attributes on declarations means needless extra work by the compiler (however little that may be - it adds up). I think we should allow the annotation in both place. When a developer is looking for the prototype of the function, it will likely look at the headers and not the implementation itself. This may lead to call an init function in code running after the boot. I don't think we can catch it easily during review. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 3/7] stubdom: don't look for mini-os source file during configure
Mini-os source code will be fetched during build. Please rerun autogen.sh after applying this patch. Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com --- stubdom/configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/stubdom/configure.ac b/stubdom/configure.ac index 6468203..424b24f 100644 --- a/stubdom/configure.ac +++ b/stubdom/configure.ac @@ -4,7 +4,6 @@ AC_PREREQ([2.67]) AC_INIT([Xen Hypervisor Stub Domains], m4_esyscmd([../version.sh ../xen/Makefile]), [xen-devel@lists.xen.org], [xen], [http://www.xen.org/]) -AC_CONFIG_SRCDIR([../extras/mini-os/kernel.c]) AC_CONFIG_FILES([../config/Stubdom.mk]) AC_CONFIG_AUX_DIR([../]) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 5/7] Mini-OS: standalone build
In order to keep the tree bisectable all the changes are done in one single commit. Things done in this commit: 1. Import necessary .mk files from Xen. 2. Move all XEN_ related variables to MINIOS_ namespace. 3. Import Xen public header files. 4. Import BSD's list.h and helper script. Mini-OS's vanilla Config.mk is modified to contain some macros copied from Xen's Config.mk. It also contains compatibility handling logic for Xen's stubdom build environment. Files modified: Config.mk Makefile arch/x86/Makefile arch/x86/arch.mk minios.mk All other files are just imported from Xen. Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com --- .gitignore |1 - extras/mini-os/Config.mk | 50 +- extras/mini-os/Makefile| 16 +- extras/mini-os/arch/x86/Makefile |5 +- extras/mini-os/arch/x86/arch.mk|8 +- extras/mini-os/config/MiniOS.mk| 10 + extras/mini-os/config/StdGNU.mk| 47 + extras/mini-os/config/arm32.mk | 22 + extras/mini-os/config/arm64.mk | 19 + extras/mini-os/config/x86_32.mk| 20 + extras/mini-os/config/x86_64.mk| 33 + [ import output trimmed ] extras/mini-os/minios.mk |4 +- 76 files changed, 16511 insertions(+), 23 deletions(-) [ import output trimmed ] diff --git a/.gitignore b/.gitignore index 13ee05b..cdbdca7 100644 --- a/.gitignore +++ b/.gitignore @@ -48,7 +48,6 @@ docs/pdf/ docs/txt/ extras/mini-os/include/mini-os extras/mini-os/include/x86/mini-os -extras/mini-os/include/xen extras/mini-os/include/list.h extras/mini-os/mini-os* install/* diff --git a/extras/mini-os/Config.mk b/extras/mini-os/Config.mk index 4852443..e5d8ade 100644 --- a/extras/mini-os/Config.mk +++ b/extras/mini-os/Config.mk @@ -1,7 +1,49 @@ -# Set mini-os root path, used in mini-os.mk. +# +# Compare $(1) and $(2) and replace $(2) with $(1) if they differ +# +# Typically $(1) is a newly generated file and $(2) is the target file +# being regenerated. This prevents changing the timestamp of $(2) only +# due to being auto regenereated with the same contents. +define move-if-changed +if ! cmp -s $(1) $(2); then mv -f $(1) $(2); else rm -f $(1); fi +endef + +# cc-option: Check if compiler supports first option, else fall back to second. +# +# This is complicated by the fact that unrecognised -Wno-* options: +# (a) are ignored unless the compilation emits a warning; and +# (b) even then produce a warning rather than an error +# To handle this we do a test compile, passing the option-under-test, on a code +# fragment that will always produce a warning (integer assigned to pointer). +# We then grep for the option-under-test in the compiler's output, the presence +# of which would indicate an unrecognized command-line option warning/error. +# +# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) +cc-option = $(shell if test -z `echo 'void*p=1;' | \ + $(1) $(2) -S -o /dev/null -x c - 21 | grep -- $(2) -`; \ + then echo $(2); else echo $(3); fi ;) + +# Compatibility with Xen's stubdom build environment. If we are building +# stubdom, some XEN_ variables are set, set MINIOS_ variables accordingly. +# +ifneq ($(XEN_ROOT),) MINI-OS_ROOT=$(XEN_ROOT)/extras/mini-os +else +MINI-OS_ROOT=$(TOPLEVEL_DIR) +endif export MINI-OS_ROOT +ifneq ($(XEN_TARGET_ARCH),) +MINIOS_TARGET_ARCH = $(XEN_TARGET_ARCH) +else +MINIOS_COMPILE_ARCH?= $(shell uname -m | sed -e s/i.86/x86_32/ \ +-e s/i86pc/x86_32/ -e s/amd64/x86_64/ \ +-e s/armv7.*/arm32/ -e s/armv8.*/arm64/ \ +-e s/aarch64/arm64/) + +MINIOS_TARGET_ARCH ?= $(MINIOS_COMPILE_ARCH) +endif + libc = $(stubdom) XEN_INTERFACE_VERSION := 0x00030205 @@ -9,11 +51,11 @@ export XEN_INTERFACE_VERSION # Try to find out the architecture family TARGET_ARCH_FAM. # First check whether x86_... is contained (for x86_32, x86_32y, x86_64). -# If not x86 then use $(XEN_TARGET_ARCH) -ifeq ($(findstring x86_,$(XEN_TARGET_ARCH)),x86_) +# If not x86 then use $(MINIOS_TARGET_ARCH) +ifeq ($(findstring x86_,$(MINIOS_TARGET_ARCH)),x86_) TARGET_ARCH_FAM = x86 else -TARGET_ARCH_FAM = $(XEN_TARGET_ARCH) +TARGET_ARCH_FAM = $(MINIOS_TARGET_ARCH) endif # The architecture family directory below mini-os. diff --git a/extras/mini-os/Makefile b/extras/mini-os/Makefile index 6d6537e..f16520e 100644 --- a/extras/mini-os/Makefile +++ b/extras/mini-os/Makefile @@ -4,9 +4,8 @@ # Makefile and a arch.mk. # -export XEN_ROOT = $(CURDIR)/../.. -include $(XEN_ROOT)/Config.mk -OBJ_DIR ?= $(CURDIR) +OBJ_DIR=$(CURDIR) +TOPLEVEL_DIR=$(CURDIR) ifeq ($(MINIOS_CONFIG),) include
[Xen-devel] [PATCH 4/7] git-checkout.sh: use mkdir -p
Otherwise mkdir extras/mini-os fails because extras doesn't exist. Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com --- scripts/git-checkout.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/git-checkout.sh b/scripts/git-checkout.sh index 15b3ce9..20ae31f 100755 --- a/scripts/git-checkout.sh +++ b/scripts/git-checkout.sh @@ -13,7 +13,7 @@ set -e if test \! -d $DIR-remote; then rm -rf $DIR-remote $DIR-remote.tmp - mkdir $DIR-remote.tmp; rmdir $DIR-remote.tmp + mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp $GIT clone $TREE $DIR-remote.tmp if test $TAG ; then cd $DIR-remote.tmp -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 6/7] build system: stubdom targets now depends on mini-os target
Provide mini-os url and revision in Config.mk
Introduce Makefile.mini-os which contains mini-os specific targets.
Target mini-os-dir clones mini-os tree from upstream.
Make stubdom targets depend on mini-os-dir target. Make
subtree-force-update{,-all} depend on mini-os-dir-force-update.
Also make mktarball script generate mini-os archive.
Original mini-os directory is renamed to mini-os-intree to help reduce
patch length. That directory will be deleted in a separate patch.
Signed-off-by: Wei Liu wei.l...@citrix.com
Cc: Ian Campbell ian.campb...@citrix.com
Cc: Ian Jackson ian.jack...@eu.citrix.com
---
.gitignore| 6 ++
Config.mk | 3 +++
Makefile | 15 +--
Makefile.mini-os | 15 +++
[ rename output trimmed ]
stubdom/Makefile | 5 +
tools/misc/mktarball | 4 +++-
227 files changed, 37 insertions(+), 11 deletions(-)
create mode 100644 Makefile.mini-os
[ rename output trimmed ]
diff --git a/.gitignore b/.gitignore
index cdbdca7..4979018 100644
--- a/.gitignore
+++ b/.gitignore
@@ -46,10 +46,8 @@ docs/man1/
docs/man5/
docs/pdf/
docs/txt/
-extras/mini-os/include/mini-os
-extras/mini-os/include/x86/mini-os
-extras/mini-os/include/list.h
-extras/mini-os/mini-os*
+extras/mini-os
+extras/mini-os-remote
install/*
stubdom/autom4te.cache/
stubdom/binutils-*
diff --git a/Config.mk b/Config.mk
index 6324237..aea3450 100644
--- a/Config.mk
+++ b/Config.mk
@@ -245,14 +245,17 @@ OVMF_UPSTREAM_URL ?=
http://xenbits.xen.org/git-http/ovmf.git
QEMU_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/qemu-upstream-unstable.git
QEMU_TRADITIONAL_URL ?= http://xenbits.xen.org/git-http/qemu-xen-unstable.git
SEABIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/seabios.git
+MINIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/mini-os.git
else
OVMF_UPSTREAM_URL ?= git://xenbits.xen.org/ovmf.git
QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/qemu-upstream-unstable.git
QEMU_TRADITIONAL_URL ?= git://xenbits.xen.org/qemu-xen-unstable.git
SEABIOS_UPSTREAM_URL ?= git://xenbits.xen.org/seabios.git
+MINIOS_UPSTREAM_URL ?= git://xenbits.xen.org/mini-os.git
endif
OVMF_UPSTREAM_REVISION ?= 447d264115c476142f884af0be287622cd244423
QEMU_UPSTREAM_REVISION ?= master
+MINIOS_UPSTREAM_REVISION ?= master
SEABIOS_UPSTREAM_REVISION ?= rel-1.7.5
# Thu May 22 16:59:16 2014 -0400
# python3 fixes for vgabios and csm builds.
diff --git a/Makefile b/Makefile
index ad6f917..d4d0f74 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,8 @@ TARGS_DISTCLEAN=$(patsubst %, distclean-%, $(SUBSYSTEMS))
export XEN_ROOT=$(CURDIR)
include Config.mk
+include Makefile.mini-os
+
SUBARCH := $(subst x86_32,i386,$(XEN_TARGET_ARCH))
export XEN_TARGET_ARCH SUBARCH
export DESTDIR
@@ -37,7 +39,7 @@ build-tools:
$(MAKE) -C tools build
.PHONY: build-stubdom
-build-stubdom:
+build-stubdom: mini-os-dir
$(MAKE) -C stubdom build
ifeq (x86_64,$(XEN_TARGET_ARCH))
XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom pv-grub
@@ -84,7 +86,7 @@ install-tools:
$(MAKE) -C tools install
.PHONY: install-stubdom
-install-stubdom: install-tools
+install-stubdom: install-tools mini-os-dir
$(MAKE) -C stubdom install
ifeq (x86_64,$(XEN_TARGET_ARCH))
XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom install-grub
@@ -125,11 +127,11 @@ rpmball: dist
bash ./tools/misc/mkrpm $(XEN_ROOT) $$($(MAKE) -C xen xenversion
--no-print-directory)
.PHONY: subtree-force-update
-subtree-force-update:
+subtree-force-update: mini-os-dir-force-update
$(MAKE) -C tools subtree-force-update
.PHONY: subtree-force-update-all
-subtree-force-update-all:
+subtree-force-update-all: mini-os-dir-force-update
$(MAKE) -C tools subtree-force-update-all
# Make a source tarball, including qemu sub-trees.
@@ -161,7 +163,7 @@ clean-tools:
$(MAKE) -C tools clean
.PHONY: clean-stubdom
-clean-stubdom:
+clean-stubdom: mini-os-dir
$(MAKE) -C stubdom crossclean
ifeq (x86_64,$(XEN_TARGET_ARCH))
XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom crossclean
@@ -187,11 +189,12 @@ distclean-tools:
$(MAKE) -C tools distclean
.PHONY: distclean-stubdom
-distclean-stubdom:
+distclean-stubdom: mini-os-dir
$(MAKE) -C stubdom distclean
ifeq (x86_64,$(XEN_TARGET_ARCH))
XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom distclean
endif
+ rm -rf extras/mini-os extras/mini-os-remote
.PHONY: distclean-docs
distclean-docs:
diff --git a/Makefile.mini-os b/Makefile.mini-os
new file mode 100644
index 000..46b1d80
--- /dev/null
+++ b/Makefile.mini-os
@@ -0,0 +1,15 @@
+.PHONY: mini-os-dir
+mini-os-dir:
+ GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh \
+ $(MINIOS_UPSTREAM_URL) \
+
[Xen-devel] [PATCH v2] xen/evtchn: Alter the alloc/free xen event channel functions to take a domain
The resource behind an event channel is domain centric rather than vcpu
centric.
This change allows mem_event_disable() to avoid arbitrarily referencing
d-vcpu[0] just to pass the domain.
No functional change.
Signed-off-by: Andrew Cooper andrew.coop...@citrix.com
CC: Keir Fraser k...@xen.org
CC: Jan Beulich jbeul...@suse.com
CC: Tim Deegan t...@xen.org
---
v2: Adjust alloc_unbound_xen_event_channel() as well (suggested by Jan)
---
xen/arch/x86/hvm/hvm.c | 20 +++-
xen/common/event_channel.c | 19 ---
xen/common/mem_event.c |4 ++--
xen/include/xen/event.h|5 ++---
4 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index fd2314e..6a7d8a9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -627,7 +627,7 @@ static int hvm_ioreq_server_add_vcpu(struct
hvm_ioreq_server *s,
spin_lock(s-lock);
-rc = alloc_unbound_xen_event_channel(v, s-domid, NULL);
+rc = alloc_unbound_xen_event_channel(v-domain, v-vcpu_id, s-domid,
NULL);
if ( rc 0 )
goto fail2;
@@ -637,7 +637,8 @@ static int hvm_ioreq_server_add_vcpu(struct
hvm_ioreq_server *s,
{
struct domain *d = s-domain;
-rc = alloc_unbound_xen_event_channel(v, s-domid, NULL);
+rc = alloc_unbound_xen_event_channel(v-domain, v-vcpu_id,
+ s-domid, NULL);
if ( rc 0 )
goto fail3;
@@ -658,7 +659,7 @@ static int hvm_ioreq_server_add_vcpu(struct
hvm_ioreq_server *s,
return 0;
fail3:
-free_xen_event_channel(v, sv-ioreq_evtchn);
+free_xen_event_channel(v-domain, sv-ioreq_evtchn);
fail2:
spin_unlock(s-lock);
@@ -685,9 +686,9 @@ static void hvm_ioreq_server_remove_vcpu(struct
hvm_ioreq_server *s,
list_del(sv-list_entry);
if ( v-vcpu_id == 0 s-bufioreq.va != NULL )
-free_xen_event_channel(v, s-bufioreq_evtchn);
+free_xen_event_channel(v-domain, s-bufioreq_evtchn);
-free_xen_event_channel(v, sv-ioreq_evtchn);
+free_xen_event_channel(v-domain, sv-ioreq_evtchn);
xfree(sv);
break;
@@ -712,9 +713,9 @@ static void hvm_ioreq_server_remove_all_vcpus(struct
hvm_ioreq_server *s)
list_del(sv-list_entry);
if ( v-vcpu_id == 0 s-bufioreq.va != NULL )
-free_xen_event_channel(v, s-bufioreq_evtchn);
+free_xen_event_channel(v-domain, s-bufioreq_evtchn);
-free_xen_event_channel(v, sv-ioreq_evtchn);
+free_xen_event_channel(v-domain, sv-ioreq_evtchn);
xfree(sv);
}
@@ -1338,13 +1339,14 @@ static int hvm_replace_event_channel(struct vcpu *v,
domid_t remote_domid,
{
int old_port, new_port;
-new_port = alloc_unbound_xen_event_channel(v, remote_domid, NULL);
+new_port = alloc_unbound_xen_event_channel(v-domain, v-vcpu_id,
+ remote_domid, NULL);
if ( new_port 0 )
return new_port;
/* xchg() ensures that only we call free_xen_event_channel(). */
old_port = xchg(p_port, new_port);
-free_xen_event_channel(v, old_port);
+free_xen_event_channel(v-domain, old_port);
return 0;
}
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 4a52f69..b756d7b 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1139,42 +1139,39 @@ long do_event_channel_op(int cmd,
XEN_GUEST_HANDLE_PARAM(void) arg)
int alloc_unbound_xen_event_channel(
-struct vcpu *local_vcpu, domid_t remote_domid,
+struct domain *ld, unsigned int lvcpu, domid_t remote_domid,
xen_event_channel_notification_t notification_fn)
{
struct evtchn *chn;
-struct domain *d = local_vcpu-domain;
intport, rc;
-spin_lock(d-event_lock);
+spin_lock(ld-event_lock);
-rc = get_free_port(d);
+rc = get_free_port(ld);
if ( rc 0 )
goto out;
port = rc;
-chn = evtchn_from_port(d, port);
+chn = evtchn_from_port(ld, port);
-rc = xsm_evtchn_unbound(XSM_TARGET, d, chn, remote_domid);
+rc = xsm_evtchn_unbound(XSM_TARGET, ld, chn, remote_domid);
if ( rc )
goto out;
chn-state = ECS_UNBOUND;
chn-xen_consumer = get_xen_consumer(notification_fn);
-chn-notify_vcpu_id = local_vcpu-vcpu_id;
+chn-notify_vcpu_id = lvcpu;
chn-u.unbound.remote_domid = remote_domid;
out:
-spin_unlock(d-event_lock);
+spin_unlock(ld-event_lock);
return rc 0 ? rc : port;
}
-void free_xen_event_channel(
-struct vcpu *local_vcpu, int port)
+void free_xen_event_channel(struct domain *d, int port)
{
struct evtchn *chn;
-struct domain *d = local_vcpu-domain;
spin_lock(d-event_lock);
diff --git a/xen/common/mem_event.c b/xen/common/mem_event.c
index 16ebdb5..0cb2334 100644
--- a/xen/common/mem_event.c
+++ b/xen/common/mem_event.c
Re: [Xen-devel] [OSSTEST PATCH v4 8/9] make-flight: factor out do_pv_debian_tests
On Wed, 2014-12-10 at 22:21 +, Wei Liu wrote:
Signed-off-by: Wei Liu wei.l...@citrix.com
---
make-flight | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/make-flight b/make-flight
index 9963a46..35904be 100755
--- a/make-flight
+++ b/make-flight
@@ -281,17 +281,24 @@ do_passthrough_tests () {
done
}
-test_matrix_do_one () {
-
- # Basic PV Linux test with xl
+do_pv_debian_test_one () {
+ toolstack=$1
- job_create_test test-$xenarch$kern-$dom0arch-xl test-debian xl \
+ job_create_test test-$xenarch$kern-$dom0arch-$toolstack test-debian
$toolstack \
$xenarch $dom0arch \
$debian_runvars all_hostflags=$most_hostflags
+}
- job_create_test test-$xenarch$kern-$dom0arch-libvirt test-debian libvirt \
-$xenarch $dom0arch \
-$debian_runvars all_hostflags=$most_hostflags
+do_pv_debian_tests () {
+ for toolstack in xl libvirt; do
+do_pv_debian_test_one $toolstack
+ done
+}
+
+test_matrix_do_one () {
+
+ # Basic PV Linux test with xl
This comment isn't true, since libvirt is included too.
Did you confirm no changes to runvars after this change? If so then
please say so in the commit log, then:
Acked-by: Ian Campbell ian.campb...@citrix.com
+ do_pv_debian_tests
# No further arm tests at the moment
if [ $dom0arch = armhf ]; then
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] docs: create reproducible html
Olaf Hering writes (Re: [PATCH] docs: create reproducible html): On Mon, Feb 02, Ian Jackson wrote: What do you think ? Thats overkill. The maintainers of make removed the internal sort with 3.82, likely for performance reasons. Sorted lists are required only in a few places. The Xen source has only two or three places where the sort is actually required. I have patches for all of them. There aren't any places in the Xen build system where the performance impact of sorting is relevant. So we should be going with whatever is easiest to understand and maintain. A rule that $(wildcard ) in Makefiles should always be accompanied by $(sort ) is simple, clear, easy to understand, and directly addresses the unpredictability at source. Personally I think that make(1) should have an option or env variable or something to make $(wildcard ) always sort. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH OSSTEST] ts-xen-build-prep: install nasm
On Mon, 2015-02-02 at 10:38 +, Wei Liu wrote: Ian Campbell ian.campb...@citrix.com writes: On Sat, 2015-01-31 at 22:51 +, Wei Liu wrote: OVMF requires nasm to build. I suppose this is a new requirement added by the mainline branch we just added? Yes. The log in the latest two failed runs suggests so. I also tested building latest OVMF HEAD it does require nasm to build. Thanks, applied to pretest after discussion with Ian J. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] support for more than 32 VCPUs when migrating PVHVM guest
On Mon, Feb 02, 2015 at 12:03:28PM +0100, Vitaly Kuznetsov wrote: Andrew Cooper andrew.coop...@citrix.com writes: On 02/02/15 10:47, Vitaly Kuznetsov wrote: Hi Konrad, I just hit an issue with PVHVM guests after save/restore (or migration), if a PVHVM guest has 32 VCPUs it hangs. Turns out, you saw it almost a year ago and even wrote patches to call VCPUOP_register_vcpu_info after resume. Unfortunately these patches never made it to xen/kernel. Do you have a plan to pick this up? What were the arguments against your suggestion? 32 VCPUs is the legacy limit for HVM guests, but should not have any remaining artefacts these days. Do you know why the hang occurs? I can't spot anything in the legacy migration code which would enforce such a limit. What is the subject of the thread you reference so I can search for it? Sorry, I should have send the link: http://lists.xen.org/archives/html/xen-devel/2014-04/msg00794.html Konrad's patches: http://lists.xen.org/archives/html/xen-devel/2014-04/msg01199.html The issue is that we don't call VCPUOP_register_vcpu_info after suspend/resume (or migration) and it is mandatory. The issues I saw were that with the enablement of that everything (which is what Jan requested) seems to work - except that I , ah here it is: http://lists.xen.org/archives/html/xen-devel/2014-04/msg02875.html err: http://lists.xen.org/archives/html/xen-devel/2014-04/msg02945.html The VCPUOP_send_nmi did cause the HVM to get an NMI and it spitted out 'Dazed and confused'. It also noticed corruption: [3.611742] Corrupted low memory at c000fffc (fffc phys) = 00029b00 [2.386785] Corrupted low memory at 8800fff8 (fff8 phys) = 29900 Which is odd because there does not seem to be anything in the path of hypervisor that would cause this. Indeed. This looks a little like a segment descriptor got modified here with a descriptor table base of zero and a selector of 0xfff8. That corruption needs to be hunted down in any case before enabling VCPUOP_send_nmi for HVM. I did not get a chance to hunt down that pesky issue. That is the only thing holding this patchset. Said patch is in my queue of patches to upstream (amongts 30 other ones) - and I am working through the review/issues - but it will take me quite some time - so if you feel like taking a stab at this - please do! ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/5] IOMMU/x86: correct page_list_first() use
On 02/02/15 11:20, Jan Beulich wrote:
Comparing its result against NULL is unsafe when page lists use normal
list entries for linking together - page_list_empty() needs to be used
instead.
Signed-off-by: Jan Beulich jbeul...@suse.com
Reviewed-by: Andrew Cooper andrew.coop...@citrix.com
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -85,8 +85,9 @@ int arch_iommu_populate_page_table(struc
* first few entries.
*/
page_list_move(d-page_list, d-arch.relmem_list);
-while ( (page = page_list_first(d-page_list)) != NULL
-(page-count_info (PGC_state|PGC_broken)) )
+while ( !page_list_empty(d-page_list)
+(page = page_list_first(d-page_list),
+ (page-count_info (PGC_state|PGC_broken))) )
{
page_list_del(page, d-page_list);
page_list_add_tail(page, d-arch.relmem_list);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/6] xen/arm: Move in/out code to/from init section
On 02.02.15 at 13:52, julien.gr...@linaro.org wrote: On 02/02/15 11:15, Jan Beulich wrote: On 02.02.15 at 11:58, ian.campb...@citrix.com wrote: On Fri, 2015-01-30 at 11:33 +, Julien Grall wrote: On 30/01/15 11:30, Ian Campbell wrote: On Thu, 2015-01-29 at 18:32 +, Julien Grall wrote: Hello, Ping? Any more review for this version of this series? I was awaiting a version with the declarations in the right places as pointed out by Andy (including his point about definition vs. prototype I'm afraid, which is the style we use). I'm not convince about your last point. We have many places (if not all on ARM) where __init is used in the header. Those are mistakes, it seems. Some of them was even added by you ;). So I would prefer if we keep them, it's more readable. FWIW Linux does it too. I personally don't particularly care where these things go, but other hypervisor maintainers seem to and we should aim for the code base to be consistent where possible. If we want to change this coding style then we should do that directly and explicitly, not through the backdoor by just ignoring the policy. And I think we should strive to keep pointless redundancy down: There's no point in repeating attributes - either they belong on the declaration (when producer and consumers care), or they belong on the definition (when only the producer cares). Repeating them on the definition when the declaration already has them only results in needless churn when such an attribute later gets changed/ removed, and having producer-relevant attributes on declarations means needless extra work by the compiler (however little that may be - it adds up). I think we should allow the annotation in both place. When a developer is looking for the prototype of the function, it will likely look at the headers and not the implementation itself. This may lead to call an init function in code running after the boot. I don't think we can catch it easily during review. This example in particular makes clear that looking at just the declaration is insufficient in certain cases. Whoever is looking for specific properties of a functions needs to know which ones (s)he cares about, and then look in the appropriate place. Unless a rule gets put in place overriding my personal opinion on this, I'm not going to ack or otherwise accept needless code duplication. If the ARM maintainers feel differently, so be it. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 17/29] Ovmf/Xen: refactor XenBusDxe hypercall implementation
On 01/27/15 14:10, Ard Biesheuvel wrote:
On 27 January 2015 at 12:46, Ard Biesheuvel ard.biesheu...@linaro.org wrote:
On 27 January 2015 at 12:43, Stefano Stabellini
stefano.stabell...@eu.citrix.com wrote:
On Mon, 26 Jan 2015, Ard Biesheuvel wrote:
This refactors the Xen hypercall implementation that is part of the
XenBusDxe driver, in preparation of splitting it off entirely into
a XenHypercallLib library. This involves:
- removing the dependency on XENBUS_DEVICE* pointers in the XenHypercall()
prototypes
- moving the discovered hyperpage address to a global variable
- moving XenGetSharedInfoPage() to its only user XenBusDxe.c (the shared
info
page is not strictly part of the Xen hypercall interface, and is not used
by other expected users of XenHypercallLib such as the Xen console
version
of SerialPortLib
- reimplement XenHypercall2() in C and move the indexing of the hyperpage
there; the existing asm implementations are renamed to __XenHypercall2()
and
invoked from the new C implementation.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
---
OvmfPkg/XenBusDxe/EventChannel.c | 11 +++
OvmfPkg/XenBusDxe/GrantTable.c| 4 ++--
OvmfPkg/XenBusDxe/Ia32/hypercall.nasm | 6 +++---
OvmfPkg/XenBusDxe/X64/hypercall.nasm | 6 +++---
OvmfPkg/XenBusDxe/XenBusDxe.c | 44
+++-
OvmfPkg/XenBusDxe/XenBusDxe.h | 1 -
OvmfPkg/XenBusDxe/XenHypercall.c | 50
++
OvmfPkg/XenBusDxe/XenHypercall.h | 28 +++-
OvmfPkg/XenBusDxe/XenStore.c | 4 ++--
9 files changed, 73 insertions(+), 81 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/EventChannel.c
b/OvmfPkg/XenBusDxe/EventChannel.c
index 03efaf9cb904..a86323e6adfd 100644
--- a/OvmfPkg/XenBusDxe/EventChannel.c
+++ b/OvmfPkg/XenBusDxe/EventChannel.c
@@ -28,7 +28,7 @@ XenEventChannelNotify (
evtchn_send_t Send;
Send.port = Port;
- ReturnCode = XenHypercallEventChannelOp (Dev, EVTCHNOP_send, Send);
+ ReturnCode = XenHypercallEventChannelOp (EVTCHNOP_send, Send);
return (UINT32)ReturnCode;
}
@@ -40,15 +40,12 @@ XenBusEventChannelAllocate (
OUT evtchn_port_t *Port
)
{
- XENBUS_PRIVATE_DATA *Private;
evtchn_alloc_unbound_t Parameter;
UINT32 ReturnCode;
- Private = XENBUS_PRIVATE_DATA_FROM_THIS (This);
-
Parameter.dom = DOMID_SELF;
Parameter.remote_dom = DomainId;
- ReturnCode = (UINT32)XenHypercallEventChannelOp (Private-Dev,
+ ReturnCode = (UINT32)XenHypercallEventChannelOp (
EVTCHNOP_alloc_unbound,
Parameter);
if (ReturnCode != 0) {
@@ -79,10 +76,8 @@ XenBusEventChannelClose (
IN evtchn_port_t Port
)
{
- XENBUS_PRIVATE_DATA *Private;
evtchn_close_t Close;
- Private = XENBUS_PRIVATE_DATA_FROM_THIS (This);
Close.port = Port;
- return (UINT32)XenHypercallEventChannelOp (Private-Dev,
EVTCHNOP_close, Close);
+ return (UINT32)XenHypercallEventChannelOp (EVTCHNOP_close, Close);
}
diff --git a/OvmfPkg/XenBusDxe/GrantTable.c
b/OvmfPkg/XenBusDxe/GrantTable.c
index 8405edc51bc4..53cb99f0e004 100644
--- a/OvmfPkg/XenBusDxe/GrantTable.c
+++ b/OvmfPkg/XenBusDxe/GrantTable.c
@@ -161,7 +161,7 @@ XenGrantTableInit (
Parameters.idx = Index;
Parameters.space = XENMAPSPACE_grant_table;
Parameters.gpfn = (xen_pfn_t) ((UINTN) GrantTable EFI_PAGE_SHIFT)
+ Index;
-ReturnCode = XenHypercallMemoryOp (Dev, XENMEM_add_to_physmap,
Parameters);
+ReturnCode = XenHypercallMemoryOp (XENMEM_add_to_physmap,
Parameters);
if (ReturnCode != 0) {
DEBUG ((EFI_D_ERROR, Xen GrantTable, add_to_physmap hypercall
error: %d\n, ReturnCode));
}
@@ -184,7 +184,7 @@ XenGrantTableDeinit (
Parameters.domid = DOMID_SELF;
Parameters.gpfn = (xen_pfn_t) ((UINTN) GrantTable EFI_PAGE_SHIFT)
+ Index;
DEBUG ((EFI_D_INFO, Xen GrantTable, removing %X\n,
Parameters.gpfn));
-ReturnCode = XenHypercallMemoryOp (Dev, XENMEM_remove_from_physmap,
Parameters);
+ReturnCode = XenHypercallMemoryOp (XENMEM_remove_from_physmap,
Parameters);
if (ReturnCode != 0) {
DEBUG ((EFI_D_ERROR, Xen GrantTable, remove_from_physmap hypercall
error: %d\n, ReturnCode));
}
diff --git a/OvmfPkg/XenBusDxe/Ia32/hypercall.nasm
b/OvmfPkg/XenBusDxe/Ia32/hypercall.nasm
index 8547c30b81ee..e0fa71bb5ba8 100644
--- a/OvmfPkg/XenBusDxe/Ia32/hypercall.nasm
+++ b/OvmfPkg/XenBusDxe/Ia32/hypercall.nasm
@@ -2,13 +2,13 @@ SECTION .text
; INTN
; EFIAPI
-; XenHypercall2 (
+; __XenHypercall2 (
; IN VOID *HypercallAddr,
; IN OUT INTN Arg1,
; IN OUT INTN Arg2
; );
-global ASM_PFX(XenHypercall2)
-ASM_PFX(XenHypercall2):
+global ASM_PFX(__XenHypercall2)
+ASM_PFX(__XenHypercall2):
Re: [Xen-devel] apic-v reduce network performance in my test case
Hi Jan, Thanks for the reply. On 2015/2/2 18:12, Jan Beulich wrote: On 31.01.15 at 11:29, john.liuqim...@huawei.com wrote: Recently I met an odd performance problem: when I turn on APIC Virtualization feature (apicv=1), the network performance of a windows guest become worse. My test case like this: host only have one windows 2008 R2 HVM guest running,and this guest has a SR-IOV VF network passthrough to it. Guest using this network access a NAS device. No fontend or backend of network and storage, all data transfered through network. The xentrace data shows: the mainly difference between apicv and non-apicv, is the way guest write apic registers, and EXIT_REASON_MSR_WRITE vmexit cost much more time than EXIT_REASON_APIC_WRITE, but when using WRMSR, the PAUSE vmexit is much less than using APIC-v. There being heavier use of the pause VMEXIT doesn't by itself say anything, I'm afraid. It may suggest that you have a C-state exit latency problem - try lowering the maximum C-state allowed, or disabling use of C-states altogether. Sorry, I forgot to mention my test scenario: Its a video test suite,I am not sure what the logic inside the tools exactly (not opensource tool). The basic flow is: 1) test suite start several thread to read video file from disk (from NAS through network in my case) 2) decode these video data as a frame one by one 3) if any frame delay more than 40ms, then mark as lost test result: apicv=1, there can be 15 thread running at the same time without lost frame apicv=0, there can be 22 thread running at the same time without lost frame so when I'm saying apicv reduce the performance, I got the conclusion from the test result not from what xentrace shows. In commit 7f2e992b824ec62a2818e64390ac2ccfbd74e6b7 VMX/Viridian: suppress MSR-based APIC suggestion when having APIC-V, msr based apic is disabled when apic-v is on, I wonder can they co-exist in some way? seems for windows guest msr-based apic has better performance. The whole purpose is to avoid the costly MSR access exits. Why would you want to reintroduce that overhead? Jan I agree to avoid the MSR access vmexit by using apicv, I just do not know what's the side effect. Because from the test result, apicv replacing msr-based access brings performance reduction. . ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] docs: create reproducible html
The Makefile uses wildcard to collect a list of files. The resulting list of files is in directory order, which is random. As a result the generated html files will differ when build on different hosts. Use the built-in sort function to get a stable list of files. Signed-off-by: Olaf Hering o...@aepfle.de Cc: Ian Jackson ian.jack...@eu.citrix.com Cc: Stefano Stabellini stefano.stabell...@eu.citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Wei Liu wei.l...@citrix.com --- docs/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/Makefile b/docs/Makefile index 2c0903b..73a61a5 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -110,7 +110,7 @@ install-html: html txt figs install: install-man-pages install-html html/index.html: $(DOC_HTML) $(CURDIR)/gen-html-index INDEX - $(PERL) -w -- $(CURDIR)/gen-html-index -i INDEX html $(DOC_HTML) + $(PERL) -w -- $(CURDIR)/gen-html-index -i INDEX html $(sort $(DOC_HTML)) html/%.html: %.markdown $(INSTALL_DIR) $(@D) @@ -152,8 +152,8 @@ html/hypercall/%/index.html: $(CURDIR)/xen-headers Makefile $(INSTALL_DIR) $(@D) $(PERL) -w $(CURDIR)/xen-headers -O $(@D) \ -T 'arch-$* - Xen public headers' \ - $(patsubst %,-X arch-%,$(filter-out $*,$(DOC_ARCHES))) \ - $(patsubst %,-X xen-%,$(filter-out $*,$(DOC_ARCHES))) \ + $(sort $(patsubst %,-X arch-%,$(filter-out $*,$(DOC_ARCHES \ + $(sort $(patsubst %,-X xen-%,$(filter-out $*,$(DOC_ARCHES \ $(EXTRA_EXCLUDE) \ $(XEN_ROOT)/xen include/public include/xen/errno.h ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH v4 8/9] make-flight: factor out do_pv_debian_tests
On Mon, 2015-02-02 at 13:18 +, Ian Campbell wrote:
On Wed, 2014-12-10 at 22:21 +, Wei Liu wrote:
Signed-off-by: Wei Liu wei.l...@citrix.com
---
make-flight | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/make-flight b/make-flight
index 9963a46..35904be 100755
--- a/make-flight
+++ b/make-flight
@@ -281,17 +281,24 @@ do_passthrough_tests () {
done
}
-test_matrix_do_one () {
-
- # Basic PV Linux test with xl
+do_pv_debian_test_one () {
+ toolstack=$1
- job_create_test test-$xenarch$kern-$dom0arch-xl test-debian xl \
+ job_create_test test-$xenarch$kern-$dom0arch-$toolstack test-debian
$toolstack \
$xenarch $dom0arch \
$debian_runvars all_hostflags=$most_hostflags
+}
- job_create_test test-$xenarch$kern-$dom0arch-libvirt test-debian libvirt
\
-$xenarch $dom0arch \
-$debian_runvars all_hostflags=$most_hostflags
+do_pv_debian_tests () {
+ for toolstack in xl libvirt; do
+do_pv_debian_test_one $toolstack
+ done
+}
+
+test_matrix_do_one () {
+
+ # Basic PV Linux test with xl
This comment isn't true, since libvirt is included too.
Did you confirm no changes to runvars after this change? If so then
please say so in the commit log,
I just saw the diff in 0/9 which didn't imply any changes due to this
patch. Still useful to explicitly state that in the commit log though I
guess.
then:
Acked-by: Ian Campbell ian.campb...@citrix.com
+ do_pv_debian_tests
# No further arm tests at the moment
if [ $dom0arch = armhf ]; then
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH v4 0/9] XSM test case for OSSTest
On Wed, 2014-12-10 at 22:21 +, Wei Liu wrote: Hi all This patch series attempts to duplicate some Debian test cases for XSM. This is version 4 of this series. Apart from any rebasing issues relating to its age I think this series looks good. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] rcu_sched self-detect stall when disable vif device
Hi David, On 30/01/15 16:04, David Vrabel wrote: How about this? This is working for me. Thanks! 8-- xen-netback: stop the guest rx thread after a fatal error After commit e9d8b2c2968499c1f96563e6522c56958d5a1d0d (xen-netback: disable rogue vif in kthread context), a fatal (protocol) error would leave the guest Rx thread spinning, wasting CPU time. Commit ecf08d2dbb96d5a4b4bcc53a39e8d29cc8fef02e (xen-netback: reintroduce guest Rx stall detection) made this even worse by removing a cond_resched() from this path. A fatal error is non-recoverable so just allow the guest Rx thread to exit. This requires taking additional refs to the task so the thread exiting early is handled safely. Signed-off-by: David Vrabel david.vra...@citrix.com Reported-by: Julien Grall julien.gr...@linaro.org Tested-by: Julien Grall julien.gr...@linaro.org Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST] README.dev: Updates to commissioning process
- mkpxedir requires the resource to be allocated
- mention the need to bless the hosts for comissioning run, and to add
proper blessings afterwards.
- run mg-execute-flight in screen
Signed-off-by: Ian Campbell ian.campb...@citrix.com
---
README.dev | 17 -
mg-hosts |3 ++-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/README.dev b/README.dev
index 3303e77..b26f8df 100644
--- a/README.dev
+++ b/README.dev
@@ -34,6 +34,9 @@ Base it on an existing machine:
$ mg-hosts create-like marilith-n5 marilith-n4
+Allocate it for the duration of setup
+$ mg-allocate HOST
+
Set up the props and flags
$ mg-hosts setprops ...
$ mg-hosts setflags ...
@@ -52,9 +55,21 @@ $ echo $FLIGHT
Clone an existing flight, e.g. 20855 was previous successful flight
$ OSSTEST_CONFIG=production-config ./cs-adjust-flight $FLIGHT copy 20855
-Run it:
+commission is a blessing, hosts should be blessed with it, e.g.
+
+$ mg-hosts setflags HOSTA HOSTB -- blessed-commission
+
+Can use anything e.g. commission-$classofmachine if doing multiple
+commissions in parallel.
+
+Run it, e.g. in a screen session:
OSSTEST_CONFIG=production-config ./mg-execute-flight -Bcommission
-eian.campb...@citrix.com -f20855 $FLIGHT
+Once machines are ready for production use remove the commission
+blessing and add the production ones, e.g.
+
+$ mf-hosts setflags HOSTA HOSTB -- \!blessed-commission
blessed-{real,play,adhoc}
+
Shutting down
=
diff --git a/mg-hosts b/mg-hosts
index c49be79..a6f6e6e 100755
--- a/mg-hosts
+++ b/mg-hosts
@@ -21,7 +21,8 @@
#
# ./mg-hosts mkpxedir HOST...
# Create directories for pxeboot as expected by the rest
-# of osstest. Will use sudo.
+# of osstest. Will use sudo. The HOST(s) must be
+# allocated (via mg-allocate HOST).
#
# ./mg-hosts create-like SOURCE-HOST NEW-HOST[,NEW-HOST...]
# Create new host(s). This does NOT copy the
--
1.7.2.5
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] sched: credit2: respect per-vcpu hard affinity
On Sat, 2015-01-31 at 20:51 -1000, Justin Weaver wrote: On Mon, Jan 19, 2015 at 9:21 PM, Justin Weaver jtwea...@hawaii.edu wrote: On Mon, Jan 12, 2015 at 8:05 AM, Dario Faggioli For example... I start a guest with one vcpu with hard affinity 8 - 15 and xl vcpu-list says it's running on pcpu 15 I run xl vcpu-pin 1 0 8 to change it to hard affinity only with pcpu 8 When it gets to vcpu_wake, it tests vcpu_runnable(v) which is false because _VPF_blocked is set, so it skips the call to SCHED_OP(VCPU2OP(v), wake, v); and so does not get a runq_tickle xl vcpu-list now shows --- for the state and I cannot console into it What I don't understand though is if I then enter xl vcpu-pin 1 0 15 it reports that _VPF_blocked is NOT set, vcpu_wake calls credit2's wake, it gets a runq_tickle and everything is fine again Why did the value of the _VPF_blocked flag change after I entered xl vcpu-pin the second time?? I dove deep in the code and could not figure it out. As promised, I am having a look. As I recalled, affinity setting/changing should not involve fiddling with _VPF_blocked, so there should be something else going on. This seems to me to be confirmed by the fact that you can make the system working again via another call to 'vcpu-pin'. I'm playing a little bit with your code, with and without the call to migrate(), to try and see better what's happening. If you happen to have an updated version of it, even if still work-in progress, and not ready for being the actual v2, and you want to share it, so that I can try that one, please go ahead (just attach it to your reply, it's for debugging, so no necessary for it to be a proper submission). I'll let you know if I find something interesting. Regards, Dario -- This happens because I choose it to happen! (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems RD Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH OSSTEST] README.dev: Updates to commissioning process
On Mon, 2015-02-02 at 14:22 +, Ian Jackson wrote:
Ian Campbell writes ([PATCH OSSTEST] README.dev: Updates to commissioning
process):
- mkpxedir requires the resource to be allocated
- mention the need to bless the hosts for comissioning run, and to add
proper blessings afterwards.
- run mg-execute-flight in screen
...
+$ mf-hosts setflags HOSTA HOSTB -- \!blessed-commission
blessed-{real,play,adhoc}
YM mg-hosts
I do, yes, updated locally.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH v4 4/9] mfi-common: create build-$arch-xsm job
On Wed, 2014-12-10 at 22:21 +, Wei Liu wrote: Signed-off-by: Wei Liu wei.l...@citrix.com Acked-by:Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH v4 7/9] ts-xen-install: install Xen with XSM support if requested
On Wed, 2014-12-10 at 22:21 +, Wei Liu wrote: Signed-off-by: Wei Liu wei.l...@citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC] libxl: set disk defaults in remove/destroy functions
On Mon, 2015-01-26 at 16:14 -0700, Jim Fehlig wrote: Cc-ing the other toolstack maintainers, both of whom have more familiarity with this part of libxl than I. The attached patch is a hack I cooked up to fix one of the libvirt-TCK Xen failures. The test (200-disk-hotplug.t) attempts to hot add and remove a disk from a running domain. The add works fine, but remove fails with libxl: debug: libxl.c:3858:libxl_device_disk_remove: ao 0x7f9b9c0015a0: create: how=(nil) callback=(nil) poller= 0x7f9ba0004590 libxl: error: libxl.c:2399:libxl__device_from_disk: unrecognized disk backend type: 0 The test does not define a backend type, in which case the libvirt libxl driver allows libxl to choose an appropriate backend via libxl__device_disk_set_backend(). The backend type is never set on a remove operation, hence it fails with the above error. I spent some time trying to figure out the best place to set backend type on remove, but in the end could only come up with this hack. It wouldn't feel so gross if I could have simply added 'libxl__device_##type##_setdefault(gc, type);' to the existing DEFINE_DEVICE_REMOVE macro, but alas libxl__device_nic_setdefault() has a different prototype than the other devices. Better suggestions welcomed! One I considered was fixing this in libvirt. But the Xen community suggested allowing libxl to choose a suitable backend when not specified, so I think this recommendation should be symmetrical in the add and remove operations. Regards, Jim ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] docs: create reproducible html
On Mon, 2015-02-02 at 14:40 +0100, Olaf Hering wrote: On Mon, Feb 02, Ian Jackson wrote: What do you think ? Thats overkill. The maintainers of make removed the internal sort with 3.82, likely for performance reasons. Aren't you adding uses of it here? Sorted lists are required only in a few places. The Xen source has only two or three places where the sort is actually required. I have patches for all of them. Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry.
On 02/02/15 05:14, Jan Beulich wrote: On 02.02.15 at 11:06, paul.durr...@citrix.com wrote: -Original Message- From: Jan Beulich [mailto:jbeul...@suse.com] Sent: 02 February 2015 10:04 To: Paul Durrant; Don Slutz Cc: Andrew Cooper; George Dunlap; Ian Campbell; Ian Jackson; Stefano Stabellini; Wei Liu; xen-devel@lists.xen.org; Keir (Xen.org) Subject: RE: [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry. On 02.02.15 at 10:51, paul.durr...@citrix.com wrote: From: Don Slutz [mailto:dsl...@verizon.com] --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2507,9 +2507,45 @@ int hvm_buffered_io_send(ioreq_t *p) return 1; } -bool_t hvm_has_dm(struct domain *d) +static bool_t hvm_complete_assist_req(ioreq_t *p) You'll need to change this to a void return as Jan requested. I don't think I did. Oh, sorry. I thought you suggested that the constant return of 1 from hvm_complete_assist_req() was a bad idea. Oh, I see, but that was in another thread, and I really suggested to remove the function altogether (folding it back into its only caller). I was planing to also do this. This is why the code was not in patch form. -Don Slutz Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/6] xen/arm: Move in/out code to/from init section
On Mon, 2015-02-02 at 12:48 +, Julien Grall wrote: On 02/02/15 10:58, Ian Campbell wrote: On Fri, 2015-01-30 at 11:33 +, Julien Grall wrote: Hi Ian, On 30/01/15 11:30, Ian Campbell wrote: On Thu, 2015-01-29 at 18:32 +, Julien Grall wrote: Hello, Ping? Any more review for this version of this series? I was awaiting a version with the declarations in the right places as pointed out by Andy (including his point about definition vs. prototype I'm afraid, which is the style we use). I'm not convince about your last point. We have many places (if not all on ARM) where __init is used in the header. Those are mistakes, it seems. Some of them was even added by you ;). So I would prefer if we keep them, it's more readable. FWIW Linux does it too. I personally don't particularly care where these things go, but other hypervisor maintainers seem to and we should aim for the code base to be consistent where possible. If we want to change this coding style then we should do that directly and explicitly, not through the backdoor by just ignoring the policy. Which policy are you talking about? AFAICT, this seems to be an unwritten rule. It is let to the appreciation of the person who review the patches... Regardless of whether it is currently written down it is something which Jan has been consistent about requesting for ages. We should probably add it to some sort of coding style document though. Actually, it doesn't seem to have a consensus between maintainers. ARM people seems to allow attributes on prototype and not x86 people. This ARM person cares more about consistency with common code and other arches than his own personal feelings on the matter, which are meh. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH v4 9/9] mfi-common, make-flight: create XSM test jobs
On Wed, 2014-12-10 at 22:21 +, Wei Liu wrote: Duplicate Debian PV and HVM test jobs for XSM testing. Signed-off-by: Wei Liu wei.l...@citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry.
On 02/02/15 03:36, Jan Beulich wrote:
On 30.01.15 at 20:19, dsl...@verizon.com wrote:
Soon after sending this, I came up with a 2nd way for fix.
Change hvm_has_dm() to use hvm_select_ioreq_server(). Then
the correct answer will be found, and so will not retry.
To avoid 2 calls to hvm_select_ioreq_server(), it makes
snes to me to return s. Also adding a call to hvm_complete_assist_req()
would help. So based on all this is is a possible v2:
Looks reasonable (awaiting Paul's opinion though) except for
mechanical aspects:
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2507,9 +2507,45 @@ int hvm_buffered_io_send(ioreq_t *p)
return 1;
}
-bool_t hvm_has_dm(struct domain *d)
+static bool_t hvm_complete_assist_req(ioreq_t *p)
{
...
+}
+
+void *hvm_has_dm(struct domain *d, ioreq_t *p)
I can't see why this needs to return void * instead of a properly typed
pointer.
Using struct hvm_ioreq_server *s did not build:
In file included from /home/don/xen-master/xen/include/asm/hvm/irq.h:26:0,
from /home/don/xen-master/xen/include/asm/hvm/vpt.h:32,
from /home/don/xen-master/xen/include/asm/hvm/vlapic.h:27,
from /home/don/xen-master/xen/include/asm/hvm/vcpu.h:25,
from /home/don/xen-master/xen/include/asm/domain.h:7,
from /home/don/xen-master/xen/include/xen/domain.h:6,
from /home/don/xen-master/xen/include/xen/sched.h:10,
from x86_64/asm-offsets.c:10:
/home/don/xen-master/xen/include/asm/hvm/hvm.h:231:35: error: 'struct
hvm_ioreq_server' declared inside parameter list [-Werror]
/home/don/xen-master/xen/include/asm/hvm/hvm.h:231:35: error: its scope
is only this definition or declaration, which is probably not what you
want [-Werror]
cc1: all warnings being treated as errors
So I went with void * to check the that the code would pass my unit
testing.
No clue why I did not say:
+struct hvm_ioreq_server;
+bool_t hvm_send_assist_req(struct hvm_ioreq_server *s, ioreq_t *p);
Unless I hear otherwise, this is the way I will go.
@@ -2571,44 +2607,15 @@ bool_t hvm_send_assist_req_to_ioreq_server(struct
hvm_ioreq_server *s,
return 0;
}
-static bool_t hvm_complete_assist_req(ioreq_t *p)
-{
...
-return 0; /* implicitly bins the i/o operation */
-}
Moving hvm_has_dm() down here would make the patch smaller and
hence quite a bit easier to review (as one doesn't need to manually
check the differences between the much larger added/removed
pieces of code.
Will do.
-Don Slutz
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH OSSTEST] README.dev: Updates to commissioning process
Ian Campbell writes ([PATCH OSSTEST] README.dev: Updates to commissioning
process):
- mkpxedir requires the resource to be allocated
- mention the need to bless the hosts for comissioning run, and to add
proper blessings afterwards.
- run mg-execute-flight in screen
...
+$ mf-hosts setflags HOSTA HOSTB -- \!blessed-commission
blessed-{real,play,adhoc}
YM mg-hosts
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry.
On 02.02.15 at 11:06, paul.durr...@citrix.com wrote: -Original Message- From: Jan Beulich [mailto:jbeul...@suse.com] Sent: 02 February 2015 10:04 To: Paul Durrant; Don Slutz Cc: Andrew Cooper; George Dunlap; Ian Campbell; Ian Jackson; Stefano Stabellini; Wei Liu; xen-devel@lists.xen.org; Keir (Xen.org) Subject: RE: [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry. On 02.02.15 at 10:51, paul.durr...@citrix.com wrote: From: Don Slutz [mailto:dsl...@verizon.com] --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2507,9 +2507,45 @@ int hvm_buffered_io_send(ioreq_t *p) return 1; } -bool_t hvm_has_dm(struct domain *d) +static bool_t hvm_complete_assist_req(ioreq_t *p) You'll need to change this to a void return as Jan requested. I don't think I did. Oh, sorry. I thought you suggested that the constant return of 1 from hvm_complete_assist_req() was a bad idea. Oh, I see, but that was in another thread, and I really suggested to remove the function altogether (folding it back into its only caller). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: Fix rtds scheduler for arm
Hi Ian,
The issue observed on credit2 scheduler is similar to the rt scheduler on
arm platform. The root cause is that interrupts are disabled in the
beginning of arm context_switch, thus spin_lock_irq is failing in
ASSERT(local_irq_is_enabled()). I propose to change both credit2 and rt
scheduler to run on arm platform as well and re-run the regression with
scheduler patches.
On Sat, Jan 31, 2015 at 12:50 PM, Ian Campbell ian.campb...@citrix.com
wrote:
On Fri, 2015-01-30 at 18:19 +0200, Denys Drozdov wrote:
since context_save executed right from IRQ level. The arm interrupt
handling differs from x86. ARM is handling context_saved with IRQ
disabled in CPSR, though x86 has IRQs on. Thus it is failing on ASSERT
inside spin_lock_irq when running on ARM. I guess it should work on
x86, so this issue is ARM platform specific.
FWIW I was waiting for it to happen to a xen-unstable run but the latest
osstest gate run at
http://www.chiark.greenend.org.uk/~xensrcts/logs/33915/ which included
Dario's patches to rationalize the schedulr tests vs. archs also
resulted in a similar sounding failure on credit2:
http://www.chiark.greenend.org.uk/~xensrcts/logs/33915/test-armhf-armhf-xl-credit2/info.html
http://www.chiark.greenend.org.uk/~xensrcts/logs/33915/test-armhf-armhf-xl-credit2/serial-marilith-n5.txt
[Thu Jan 29 13:29:28 2015](XEN) Assertion 'local_irq_is_enabled()'
failed at spinlock.c:137
[Thu Jan 29 13:29:28 2015](XEN) [ Xen-4.6-unstable arm32
debug=y Not tainted ]
[Thu Jan 29 13:29:28 2015](XEN) CPU:0
[Thu Jan 29 13:29:28 2015](XEN) PC: 00229734
_spin_lock_irq+0x18/0x94
[Thu Jan 29 13:29:28 2015](XEN) CPSR: 20da MODE:Hypervisor
[Thu Jan 29 13:29:28 2015](XEN) R0: 4000823c R1: R2:
02faf080 R3: 60da
[Thu Jan 29 13:29:28 2015](XEN) R4: 4000823c R5: 4000d000 R6:
4000823c R7: 002ee020
[Thu Jan 29 13:29:28 2015](XEN) R8: 4000f218 R9:
R10:0026fe08 R11:7ffcfefc R12:0002
[Thu Jan 29 13:29:28 2015](XEN) HYP: SP: 7ffcfeec LR: 0021f34c
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN) VTCR_EL2: 80003558
[Thu Jan 29 13:29:28 2015](XEN) VTTBR_EL2: 00010002b9ffc000
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN) SCTLR_EL2: 30cd187f
[Thu Jan 29 13:29:28 2015](XEN)HCR_EL2: 0038643f
[Thu Jan 29 13:29:28 2015](XEN) TTBR0_EL2: ff6e8000
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN)ESR_EL2:
[Thu Jan 29 13:29:28 2015](XEN) HPFAR_EL2:
[Thu Jan 29 13:29:28 2015](XEN) HDFAR:
[Thu Jan 29 13:29:28 2015](XEN) HIFAR:
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN) Xen stack trace from sp=7ffcfeec:
[Thu Jan 29 13:29:28 2015](XEN)0024d068 002f0328
7ffcff2c 0021f34c 6591e5c1
[Thu Jan 29 13:29:28 2015](XEN) 4000d000 4000d000
7ffcff3c
[Thu Jan 29 13:29:28 2015](XEN)002285dc 7fff
7ffcff4c 00242614 7ffcff54
[Thu Jan 29 13:29:28 2015](XEN)002427c8 00242b6c
2800
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN) 27a0
01d3
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN) Xen call trace:
[Thu Jan 29 13:29:28 2015](XEN)[00229734]
_spin_lock_irq+0x18/0x94 (PC)
[Thu Jan 29 13:29:28 2015](XEN)[0021f34c]
csched2_context_saved+0x44/0x18c (LR)
[Thu Jan 29 13:29:28 2015](XEN)[0021f34c]
csched2_context_saved+0x44/0x18c
[Thu Jan 29 13:29:28 2015](XEN)[002285dc]
context_saved+0x58/0x80
[Thu Jan 29 13:29:28 2015](XEN)[00242614]
schedule_tail+0x148/0x2f0
[Thu Jan 29 13:29:28 2015](XEN)[002427c8]
continue_new_vcpu+0xc/0x70
[Thu Jan 29 13:29:28 2015](XEN)[00242b6c]
context_switch+0x74/0x7c
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN)
[Thu Jan 29 13:29:28 2015](XEN) Panic on CPU 0:
[Thu Jan 29 13:29:28 2015](XEN) Assertion 'local_irq_is_enabled()'
failed at spinlock.c:137
[Thu Jan 29 13:29:28
[Xen-devel] [PATCH] xen-blkfront: fix accounting of reqs when migrating
Current migration code uses blk_put_request in order to finish a request before requeuing it. This function doesn't update the statistics of the queue, which completely screws accounting. Use blk_end_request_all instead which properly updates the statistics of the queue. Signed-off-by: Roger Pau Monné roger@citrix.com Reported-and-Tested-by: Ouyang Zhaowei (Charles) ouyangzhao...@huawei.com Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com Cc: Boris Ostrovsky boris.ostrov...@oracle.com Cc: David Vrabel david.vra...@citrix.com Cc: xen-de...@lists.xenproject.org --- drivers/block/xen-blkfront.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 5ac312f..aac41c1 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1493,7 +1493,7 @@ static int blkif_recover(struct blkfront_info *info) merge_bio.tail = copy[i].request-biotail; bio_list_merge(bio_list, merge_bio); copy[i].request-bio = NULL; - blk_put_request(copy[i].request); + blk_end_request_all(copy[i].request, 0); } kfree(copy); @@ -1516,7 +1516,7 @@ static int blkif_recover(struct blkfront_info *info) req-bio = NULL; if (req-cmd_flags (REQ_FLUSH | REQ_FUA)) pr_alert(diskcache flush request found!\n); - __blk_put_request(info-rq, req); + __blk_end_request_all(req, 0); } spin_unlock_irq(info-io_lock); -- 1.9.3 (Apple Git-50) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 3/5] x86/mm: allow for building without shadow mode support
Considering the complexity of the code, it seems to be a reasonable
thing to allow people to disable that code entirely even outside the
immediate need for this by the next patch.
Signed-off-by: Jan Beulich jbeul...@suse.com
---
v2: Set mode table in shadow_vcpu_init() stub. Convert BUG()/BUG_ON()
to ASSERT()/ASSERT_UNREACHABLE() and make various of the stub
functions macros or inline. Hide opt_dom0_shadow when
!CONFIG_SHADOW_PAGING. Adjust Makefile modification.
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -32,9 +32,13 @@ x86 := y
x86_32 := n
x86_64 := y
+shadow-paging ?= y
+
CFLAGS += -mno-red-zone -mno-sse -fpic
CFLAGS += -fno-asynchronous-unwind-tables
# -fvisibility=hidden reduces -fpic cost, if it's available
ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
endif
+
+CFLAGS-$(shadow-paging) += -DCONFIG_SHADOW_PAGING
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -128,8 +128,10 @@ struct vcpu *__init alloc_dom0_vcpu0(str
return alloc_vcpu(dom0, 0, 0);
}
+#ifdef CONFIG_SHADOW_PAGING
static bool_t __initdata opt_dom0_shadow;
boolean_param(dom0_shadow, opt_dom0_shadow);
+#endif
static char __initdata opt_dom0_ioports_disable[200] = ;
string_param(dom0_ioports_disable, opt_dom0_ioports_disable);
@@ -1399,6 +1401,7 @@ int __init construct_dom0(
regs-esi = vstartinfo_start;
regs-eflags = X86_EFLAGS_IF;
+#ifdef CONFIG_SHADOW_PAGING
if ( opt_dom0_shadow )
{
if ( is_pvh_domain(d) )
@@ -1409,6 +1412,7 @@ int __init construct_dom0(
if ( paging_enable(d, PG_SH_enable) == 0 )
paging_update_paging_modes(v);
}
+#endif
/*
* PVH Fixme: XENFEAT_supervisor_mode_kernel has been reused in PVH with a
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -635,16 +635,16 @@ int paging_domain_init(struct domain *d,
* don't want to leak any active log-dirty bitmaps */
d-arch.paging.log_dirty.top = _mfn(INVALID_MFN);
-/* The order of the *_init calls below is important, as the later
- * ones may rewrite some common fields. Shadow pagetables are the
- * default... */
-shadow_domain_init(d, domcr_flags);
-
-/* ... but we will use hardware assistance if it's available. */
+/*
+ * Shadow pagetables are the default, but we will use
+ * hardware assistance if it's available and enabled.
+ */
if ( hap_enabled(d) )
hap_domain_init(d);
+else
+rc = shadow_domain_init(d, domcr_flags);
-return 0;
+return rc;
}
/* vcpu paging struct initialization goes here */
@@ -822,12 +822,16 @@ int paging_enable(struct domain *d, u32
* and therefore its pagetables will soon be discarded */
void pagetable_dying(struct domain *d, paddr_t gpa)
{
+#ifdef CONFIG_SHADOW_PAGING
struct vcpu *v;
ASSERT(paging_mode_shadow(d));
v = d-vcpu[0];
v-arch.paging.mode-shadow.pagetable_dying(v, gpa);
+#else
+BUG();
+#endif
}
/* Print paging-assistance info to the console */
--- a/xen/arch/x86/mm/shadow/Makefile
+++ b/xen/arch/x86/mm/shadow/Makefile
@@ -1,4 +1,8 @@
-obj-$(x86_64) += common.o guest_2.o guest_3.o guest_4.o
+ifeq ($(shadow-paging),y)
+obj-y += common.o guest_2.o guest_3.o guest_4.o
+else
+obj-y += none.o
+endif
guest_%.o: multi.c Makefile
$(CC) $(CFLAGS) -DGUEST_PAGING_LEVELS=$* -c $ -o $@
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -47,7 +47,7 @@ static void sh_clean_dirty_bitmap(struct
/* Set up the shadow-specific parts of a domain struct at start of day.
* Called for every domain from arch_domain_create() */
-void shadow_domain_init(struct domain *d, unsigned int domcr_flags)
+int shadow_domain_init(struct domain *d, unsigned int domcr_flags)
{
INIT_PAGE_LIST_HEAD(d-arch.paging.shadow.freelist);
INIT_PAGE_LIST_HEAD(d-arch.paging.shadow.pinned_shadows);
@@ -61,6 +61,8 @@ void shadow_domain_init(struct domain *d
d-arch.paging.shadow.oos_off = (domcr_flags DOMCRF_oos_off) ? 1 : 0;
#endif
d-arch.paging.shadow.pagetable_dying_op = 0;
+
+return 0;
}
/* Setup the shadow-specfic parts of a vcpu struct. Note: The most important
--- /dev/null
+++ b/xen/arch/x86/mm/shadow/none.c
@@ -0,0 +1,78 @@
+#include xen/mm.h
+#include asm/shadow.h
+
+static int _enable_log_dirty(struct domain *d, bool_t log_global)
+{
+ASSERT(is_pv_domain(d));
+return -EOPNOTSUPP;
+}
+
+static int _disable_log_dirty(struct domain *d)
+{
+ASSERT(is_pv_domain(d));
+return -EOPNOTSUPP;
+}
+
+static void _clean_dirty_bitmap(struct domain *d)
+{
+ASSERT(is_pv_domain(d));
+}
+
+int shadow_domain_init(struct domain *d, unsigned int domcr_flags)
+{
+paging_log_dirty_init(d, _enable_log_dirty,
+ _disable_log_dirty, _clean_dirty_bitmap);
+return is_pv_domain(d) ? 0 : -EOPNOTSUPP;
+}
+
+static int _page_fault(struct vcpu *v, unsigned long va,
Re: [Xen-devel] [PATCH linux-2.6.18] xen: mark pvscsi frontend request consumed only after last read
On 02/02/2015 08:52 AM, Jan Beulich wrote: On 30.01.15 at 14:51, jgr...@suse.com.non-mime.internet wrote: A request in the ring buffer mustn't be read after it has been marked as consumed. Otherwise it might already have been reused by the frontend without violating the ring protocol. This is irrelevant, as the -req_cons is a backend private field (if it was a shared one, a barrier would have been needed between copying and updating that field). Hmm, you are right. Interesting, I've always thought req_cons would be used by the frontend, too. Thanks for updating my protocol know-how. :-) To avoid inconsistencies in the backend only work on a private copy of the request. This will ensure a malicious guest not being able to bypass consistency checks of the backend by modifying an active request. Hence - also considering with my earlier reply - I don't view the change as necessary. Agreed. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC V10 0/4] domain snapshot document
On Sun, 2015-02-01 at 20:55 +, Dave Scott wrote: On 29 Jan 2015, at 16:36, Ian Campbell ian.campb...@citrix.com wrote: On Mon, 2015-01-26 at 11:25 +0800, Chunyan Liu wrote: Changes to V9: This looks good to me, thanks. Ian/Wei do you have any comments? xapi folks? The API looks sensible to me. Xapi normally manages disk snapshots through it’s own plug-in mechanism. Will it still be possible to * libxl_domain_suspend * do custom snapshot stuff involving tapdisk/ ceph/ ... * libxl_domain_create_restore ? If so then everything is fine for us. I think so, but I'll let Chunyan confirm for sure. The main thing which you cannot do without going through libxl is operations which involve talking to a live domain, i.e. anything which might talk QMP to the associate qemu process. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PCI-Passthrough Error: transmit queue timed out
On 31.01.15 at 09:49, open...@126.com wrote: 2015-01-31T04:53:45.005161-05:00 linux-gbze kernel: [63331.820079] [ cut here ] 2015-01-31T04:53:45.005187-05:00 linux-gbze kernel: [63331.820163] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:264 dev_watchdog+0x266/0x270() 2015-01-31T04:53:45.005190-05:00 linux-gbze kernel: [63331.820196] NETDEV WATCHDOG: eth0 (ixgbe): transmit queue 1 timed out Please keep in mind that Intel officially doesn't support passing through physical functions of NICs to guests; only SR-IOV VF pass-through is supported from what they've been telling us. This may well have to do with (wild guess!) them knowing their PF drivers not working reliably in unprivileged guests - in any event your report seems to hint towards a driver issue. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] apic-v reduce network performance in my test case
On 31.01.15 at 11:29, john.liuqim...@huawei.com wrote: Recently I met an odd performance problem: when I turn on APIC Virtualization feature (apicv=1), the network performance of a windows guest become worse. My test case like this: host only have one windows 2008 R2 HVM guest running,and this guest has a SR-IOV VF network passthrough to it. Guest using this network access a NAS device. No fontend or backend of network and storage, all data transfered through network. The xentrace data shows: the mainly difference between apicv and non-apicv, is the way guest write apic registers, and EXIT_REASON_MSR_WRITE vmexit cost much more time than EXIT_REASON_APIC_WRITE, but when using WRMSR, the PAUSE vmexit is much less than using APIC-v. There being heavier use of the pause VMEXIT doesn't by itself say anything, I'm afraid. It may suggest that you have a C-state exit latency problem - try lowering the maximum C-state allowed, or disabling use of C-states altogether. In commit 7f2e992b824ec62a2818e64390ac2ccfbd74e6b7 VMX/Viridian: suppress MSR-based APIC suggestion when having APIC-V, msr based apic is disabled when apic-v is on, I wonder can they co-exist in some way? seems for windows guest msr-based apic has better performance. The whole purpose is to avoid the costly MSR access exits. Why would you want to reintroduce that overhead? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/5] x86/shadow: tidy up fragmentary page lists in multi‑page shadows
From: Tim Deegan t...@xen.org
Multi-page shadows are linked together using the 'list' field. When
those shadows are in the pinned list, the list fragments are spliced
into the pinned list; otherwise they have no associated list head.
Rework the code that handles these fragments to use the page_list
interface rather than manipulating the fields directly. This makes
the code cleaner, and allows the 'list' field to be either the
compact pdx form or a normal list_entry.
Signed-off-by: Tim Deegan t...@xen.org
Introduce sh_terminate_list() and make it use LIST_POISON*.
Move helper array of shadow_size() into common.c.
Signed-off-by: Jan Beulich jbeul...@suse.com
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1196,6 +1196,26 @@ int shadow_cmpxchg_guest_entry(struct vc
* the free pool.
*/
+const u32 sh_type_to_size[] = {
+1, /* SH_type_none */
+2, /* SH_type_l1_32_shadow */
+2, /* SH_type_fl1_32_shadow */
+4, /* SH_type_l2_32_shadow */
+1, /* SH_type_l1_pae_shadow */
+1, /* SH_type_fl1_pae_shadow */
+1, /* SH_type_l2_pae_shadow */
+1, /* SH_type_l2h_pae_shadow */
+1, /* SH_type_l1_64_shadow */
+1, /* SH_type_fl1_64_shadow */
+1, /* SH_type_l2_64_shadow */
+1, /* SH_type_l2h_64_shadow */
+1, /* SH_type_l3_64_shadow */
+1, /* SH_type_l4_64_shadow */
+1, /* SH_type_p2m_table */
+1, /* SH_type_monitor_table */
+1 /* SH_type_oos_snapshot */
+};
+
/* Figure out the least acceptable quantity of shadow memory.
* The minimum memory requirement for always being able to free up a
* chunk of memory is very small -- only three max-order chunks per
@@ -1218,33 +1238,6 @@ static unsigned int shadow_min_acceptabl
return (vcpu_count * 128);
}
-/* Figure out the size (in pages) of a given shadow type */
-static inline u32
-shadow_size(unsigned int shadow_type)
-{
-static const u32 type_to_size[SH_type_unused] = {
-1, /* SH_type_none */
-2, /* SH_type_l1_32_shadow */
-2, /* SH_type_fl1_32_shadow */
-4, /* SH_type_l2_32_shadow */
-1, /* SH_type_l1_pae_shadow */
-1, /* SH_type_fl1_pae_shadow */
-1, /* SH_type_l2_pae_shadow */
-1, /* SH_type_l2h_pae_shadow */
-1, /* SH_type_l1_64_shadow */
-1, /* SH_type_fl1_64_shadow */
-1, /* SH_type_l2_64_shadow */
-1, /* SH_type_l2h_64_shadow */
-1, /* SH_type_l3_64_shadow */
-1, /* SH_type_l4_64_shadow */
-1, /* SH_type_p2m_table */
-1, /* SH_type_monitor_table */
-1 /* SH_type_oos_snapshot */
-};
-ASSERT(shadow_type SH_type_unused);
-return type_to_size[shadow_type];
-}
-
/* Dispatcher function: call the per-mode function that will unhook the
* non-Xen mappings in this top-level shadow mfn. With user_only == 1,
* unhooks only the user-mode mappings. */
@@ -1487,9 +1480,6 @@ mfn_t shadow_alloc(struct domain *d,
break;
}
-/* Page lists don't have pointers back to the head structure, so
- * it's safe to use a head structure on the stack to link the pages
- * together. */
INIT_PAGE_LIST_HEAD(tmp_list);
/* Init page info fields and clear the pages */
@@ -1523,6 +1513,9 @@ mfn_t shadow_alloc(struct domain *d,
if ( shadow_type = SH_type_min_shadow
shadow_type = SH_type_max_shadow )
sp-u.sh.head = 1;
+
+sh_terminate_list(tmp_list);
+
return page_to_mfn(sp);
}
@@ -1531,6 +1524,7 @@ mfn_t shadow_alloc(struct domain *d,
void shadow_free(struct domain *d, mfn_t smfn)
{
struct page_info *next = NULL, *sp = mfn_to_page(smfn);
+struct page_list_head *pin_list;
unsigned int pages;
u32 shadow_type;
int i;
@@ -1542,6 +1536,7 @@ void shadow_free(struct domain *d, mfn_t
ASSERT(shadow_type != SH_type_none);
ASSERT(sp-u.sh.head || (shadow_type SH_type_max_shadow));
pages = shadow_size(shadow_type);
+pin_list = d-arch.paging.shadow.pinned_shadows;
for ( i = 0; i pages; i++ )
{
@@ -1562,7 +1557,7 @@ void shadow_free(struct domain *d, mfn_t
#endif
/* Get the next page before we overwrite the list header */
if ( i pages - 1 )
-next = pdx_to_page(sp-list.next);
+next = page_list_next(sp, pin_list);
/* Strip out the type: this is now a free shadow page */
sp-u.sh.type = sp-u.sh.head = 0;
/* Remember the TLB timestamp so we will know whether to flush
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -428,20 +428,20 @@ sh_guest_get_eff_l1e(struct vcpu *v, uns
/* From one page of a multi-page shadow, find the next one */
static inline mfn_t sh_next_page(mfn_t smfn)
{
-mfn_t next;
-struct page_info *pg = mfn_to_page(smfn);
+struct page_info *pg = mfn_to_page(smfn), *next;
+struct page_list_head h =
[Xen-devel] [PATCH v2 2/5] x86/shadow: don't needlessly expose internal functions
... and drop an unused one.
Signed-off-by: Jan Beulich jbeul...@suse.com
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2383,7 +2383,7 @@ int sh_remove_write_access_from_sl1p(str
/* Remove all mappings of a guest frame from the shadow tables.
* Returns non-zero if we need to flush TLBs. */
-int sh_remove_all_mappings(struct vcpu *v, mfn_t gmfn)
+static int sh_remove_all_mappings(struct vcpu *v, mfn_t gmfn)
{
struct page_info *page = mfn_to_page(gmfn);
--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -387,6 +387,11 @@ int shadow_write_guest_entry(struct vcpu
int shadow_cmpxchg_guest_entry(struct vcpu *v, intpte_t *p,
intpte_t *old, intpte_t new, mfn_t gmfn);
+/* Update all the things that are derived from the guest's CR0/CR3/CR4.
+ * Called to initialize paging structures if the paging mode
+ * has changed, and when bringing up a VCPU for the first time. */
+void shadow_update_paging_modes(struct vcpu *v);
+
/* Unhook the non-Xen mappings in this top-level shadow mfn.
* With user_only == 1, unhooks only the user-mode mappings. */
void shadow_unhook_mappings(struct vcpu *v, mfn_t smfn, int user_only);
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -77,23 +77,6 @@ void shadow_teardown(struct domain *d);
/* Call once all of the references to the domain have gone away */
void shadow_final_teardown(struct domain *d);
-/* Update all the things that are derived from the guest's CR0/CR3/CR4.
- * Called to initialize paging structures if the paging mode
- * has changed, and when bringing up a VCPU for the first time. */
-void shadow_update_paging_modes(struct vcpu *v);
-
-
-/* Remove all mappings of the guest page from the shadows.
- * This is called from common code. It does not flush TLBs. */
-int sh_remove_all_mappings(struct vcpu *v, mfn_t target_mfn);
-static inline void
-shadow_drop_references(struct domain *d, struct page_info *p)
-{
-if ( unlikely(shadow_mode_enabled(d)) )
-/* See the comment about locking in sh_remove_all_mappings */
-sh_remove_all_mappings(d-vcpu[0], _mfn(page_to_mfn(p)));
-}
-
/* Remove all shadows of the guest mfn. */
void sh_remove_shadows(struct vcpu *v, mfn_t gmfn, int fast, int all);
static inline void shadow_remove_all_shadows(struct vcpu *v, mfn_t gmfn)
x86/shadow: don't needlessly expose internal functions
... and drop an unused one.
Signed-off-by: Jan Beulich jbeul...@suse.com
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2383,7 +2383,7 @@ int sh_remove_write_access_from_sl1p(str
/* Remove all mappings of a guest frame from the shadow tables.
* Returns non-zero if we need to flush TLBs. */
-int sh_remove_all_mappings(struct vcpu *v, mfn_t gmfn)
+static int sh_remove_all_mappings(struct vcpu *v, mfn_t gmfn)
{
struct page_info *page = mfn_to_page(gmfn);
--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -387,6 +387,11 @@ int shadow_write_guest_entry(struct vcpu
int shadow_cmpxchg_guest_entry(struct vcpu *v, intpte_t *p,
intpte_t *old, intpte_t new, mfn_t gmfn);
+/* Update all the things that are derived from the guest's CR0/CR3/CR4.
+ * Called to initialize paging structures if the paging mode
+ * has changed, and when bringing up a VCPU for the first time. */
+void shadow_update_paging_modes(struct vcpu *v);
+
/* Unhook the non-Xen mappings in this top-level shadow mfn.
* With user_only == 1, unhooks only the user-mode mappings. */
void shadow_unhook_mappings(struct vcpu *v, mfn_t smfn, int user_only);
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -77,23 +77,6 @@ void shadow_teardown(struct domain *d);
/* Call once all of the references to the domain have gone away */
void shadow_final_teardown(struct domain *d);
-/* Update all the things that are derived from the guest's CR0/CR3/CR4.
- * Called to initialize paging structures if the paging mode
- * has changed, and when bringing up a VCPU for the first time. */
-void shadow_update_paging_modes(struct vcpu *v);
-
-
-/* Remove all mappings of the guest page from the shadows.
- * This is called from common code. It does not flush TLBs. */
-int sh_remove_all_mappings(struct vcpu *v, mfn_t target_mfn);
-static inline void
-shadow_drop_references(struct domain *d, struct page_info *p)
-{
-if ( unlikely(shadow_mode_enabled(d)) )
-/* See the comment about locking in sh_remove_all_mappings */
-sh_remove_all_mappings(d-vcpu[0], _mfn(page_to_mfn(p)));
-}
-
/* Remove all shadows of the guest mfn. */
void sh_remove_shadows(struct vcpu *v, mfn_t gmfn, int fast, int all);
static inline void shadow_remove_all_shadows(struct vcpu *v, mfn_t gmfn)
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry.
-Original Message-
From: Don Slutz [mailto:dsl...@verizon.com]
Sent: 30 January 2015 19:19
To: Don Slutz; Jan Beulich; Paul Durrant
Cc: Andrew Cooper; Ian Campbell; Wei Liu; George Dunlap; Ian Jackson;
Stefano Stabellini; xen-devel@lists.xen.org; Keir (Xen.org)
Subject: Re: [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server
failed do not retry.
On 01/30/15 13:17, Don Slutz wrote:
On 01/30/15 05:23, Jan Beulich wrote:
On 30.01.15 at 01:52, dsl...@verizon.com wrote:
I.E. do just what no backing DM does.
_If_ this is correct, the if() modified here should be folded with the
one a few lines up.
Ok, will fold with the one a few lines up. As expected without this
change the guest will hang (more details below).
But looking at the description of the commit that
introduced this (bac0999325 x86 hvm: Do not incorrectly retire an
instruction emulation..., almost immediately modified by f20f3c8ece
x86 hvm: On failed hvm_send_assist_req(), io emulation...) I doubt
this is really what we want, or at the very least your change
description should explain what was wrong with the original commit.
Looking at these 2 commits, it looks to me like I need to handle these
cases also. So it looks like having hvm_send_assist_req() return an int
0, 1, 2 is the simpler way. V2 on the way soon.
Soon after sending this, I came up with a 2nd way for fix.
Change hvm_has_dm() to use hvm_select_ioreq_server(). Then
the correct answer will be found, and so will not retry.
To avoid 2 calls to hvm_select_ioreq_server(), it makes
snes to me to return s. Also adding a call to hvm_complete_assist_req()
would help. So based on all this is is a possible v2:
Yes, I think this approach does make a lot of sense.
commit 24eb5a839427ba80c1adf16ab656c19729f906be
Author: Don Slutz dsl...@verizon.com
Date: Fri Jan 30 13:54:43 2015 -0500
fixup use of hvm_send_assist_req
Signed-off-by: Don Slutz dsl...@verizon.com
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 2ed4344..7c3c654 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -218,8 +218,11 @@ static int hvmemul_do_io(
vio-io_state = HVMIO_handle_mmio_awaiting_completion;
break;
case X86EMUL_UNHANDLEABLE:
+{
+void *s = hvm_has_dm(curr-domain, p);
+
/* If there is no backing DM, just ignore accesses */
-if ( !hvm_has_dm(curr-domain) )
+if ( !s )
{
rc = X86EMUL_OKAY;
vio-io_state = HVMIO_none;
@@ -227,11 +230,12 @@ static int hvmemul_do_io(
else
{
rc = X86EMUL_RETRY;
-if ( !hvm_send_assist_req(p) )
+if ( !hvm_send_assist_req(s, p) )
vio-io_state = HVMIO_none;
else if ( p_data == NULL )
rc = X86EMUL_OKAY;
}
+}
break;
default:
BUG();
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 984af81..21d4a73 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2507,9 +2507,45 @@ int hvm_buffered_io_send(ioreq_t *p)
return 1;
}
-bool_t hvm_has_dm(struct domain *d)
+static bool_t hvm_complete_assist_req(ioreq_t *p)
You'll need to change this to a void return as Jan requested.
{
-return !list_empty(d-arch.hvm_domain.ioreq_server.list);
+HVMTRACE_1D(COMPLETE_ASSIST, p-type);
+ASSERT(p-type != IOREQ_TYPE_PCI_CONFIG);
+switch ( p-type )
+{
+case IOREQ_TYPE_COPY:
+case IOREQ_TYPE_PIO:
+if ( p-dir == IOREQ_READ )
+{
+if ( !p-data_is_ptr )
+p-data = ~0ul;
+else
+{
+int i, step = p-df ? -p-size : p-size;
+uint32_t data = ~0;
+
+for ( i = 0; i p-count; i++ )
+hvm_copy_to_guest_phys(p-data + step * i, data,
+ p-size);
+}
+}
+/* FALLTHRU */
+default:
+p-state = STATE_IORESP_READY;
+hvm_io_assist(p);
+break;
+}
+
+return 1;
+}
+
+void *hvm_has_dm(struct domain *d, ioreq_t *p)
+{
+struct hvm_ioreq_server *s = hvm_select_ioreq_server(d, p);
+
+if ( !s )
+hvm_complete_assist_req(p);
If you're going to complete the I/O here then...
+return (void *)s;
}
bool_t hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
@@ -2571,44 +2607,15 @@ bool_t
hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
return 0;
}
-static bool_t hvm_complete_assist_req(ioreq_t *p)
-{
-HVMTRACE_1D(COMPLETE_ASSIST, p-type);
-ASSERT(p-type != IOREQ_TYPE_PCI_CONFIG);
-switch ( p-type )
-{
-case IOREQ_TYPE_COPY:
-case IOREQ_TYPE_PIO:
-if ( p-dir == IOREQ_READ )
-{
-
Re: [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough
On Mon, 2015-02-02 at 09:17 +0800, Tiejun Chen wrote:
When we're working to support IGD GFX passthrough with qemu
upstream, instead of -gfx_passthru we'd like to make that
a machine option, -machine xxx,-igd-passthru=on. This need
to bring a change on tool side.
From which Qemu version is this new option accepted? What will a verison
of qemu prior to then do when presented with the new option?
(nb: you have an extra '-' in the description I think)
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
---
v2:
* Based on some discussions with Wei we'd like to keep both old
option, -gfx_passthru, and new machine property option,
-machine xxx,-igd-passthru=on at the same time but deprecate
the old one. Then finally we remove the old one at that point
that to give downstream (in this case, Xen) time to cope with the
change.
tools/libxl/libxl_dm.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index c2b0487..8405f0b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -701,6 +701,11 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_append(dm_args, -net);
flexarray_append(dm_args, none);
}
+/*
+ * Although we already introduce 'igd-passthru', but we'd like
+ * to remove this until we give downstream time to cope with
+ * the change.
+ */
if (libxl_defbool_val(b_info-u.hvm.gfx_passthru)) {
flexarray_append(dm_args, -gfx_passthru);
}
@@ -748,6 +753,11 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
machinearg, max_ram_below_4g);
}
}
+
+if (libxl_defbool_val(b_info-u.hvm.gfx_passthru)) {
+machinearg = libxl__sprintf(gc, %s,igd-passthru=on,
machinearg);
+}
+
flexarray_append(dm_args, machinearg);
for (i = 0; b_info-extra_hvm b_info-extra_hvm[i] != NULL; i++)
flexarray_append(dm_args, b_info-extra_hvm[i]);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] libxl_set_memory_target: retain the same maxmem offset on top of the current target
On Thu, 2015-01-29 at 15:08 +, Stefano Stabellini wrote:
@@ -4775,6 +4781,14 @@ retry_transaction:
new_target_memkb = current_target_memkb + target_memkb;
} else
new_target_memkb = target_memkb - videoram;
+
+if (new_target_memkb = 0) {
+LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+cannot set memory target to 0 or less than 0.\n);
new_target_memkb is uint32 so it can't be less than zero.
In fact, there looks to be some under/overflow bugs hidden in this
function. e.g. in new_target_memkb = target_memkb (since target is
signed)
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/2] xen/arm: vgic: Keep track of vIRQ used by a domain
On Thu, 2015-01-29 at 15:51 +, Julien Grall wrote:
- Move the retry after looking for first/end. I keep the goto
rather than a loop because it's more clear that we retry because
we were unable to set the bit
Then I think a do {} while (!successfully allocated) is what is
wanted, maybe with a comment.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] time: widen wallclock seconds to 64 bits
On 02.02.15 at 15:40, ian.campb...@citrix.com wrote: On Thu, 2015-01-29 at 15:39 +, Jan Beulich wrote: [...] A note on the conditional suppressing the xen_wc_sec_hi helper macro definition in the ix86 case for hypervisor and tools: Neither of the two actually need this, and its presence causes the tools to fail to build (due to the inclusion of both the x86-64 and x86-32 variants of the header). [...] --- a/tools/include/xen-foreign/Makefile +++ b/tools/include/xen-foreign/Makefile @@ -17,7 +17,7 @@ clean: distclean: clean checker: checker.c $(headers) -$(HOSTCC) $(HOSTCFLAGS) -o $@ $ +$(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -o $@ $ This is to make int64_aligned_t available I think? (Which isn't a problem just wanted to check it wasn't something more subtle) The primary purpose really is to make the thing build - see the description paragraph that I kept quoted above, which is related to this even if that may not be obvious. int64_aligned_t should be available regardless, as it only mirrors uint64_aligned_t (which quite obviously needs to already be available). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/7] build system: stubdom targets now depends on mini-os target
On Mon, Feb 02, 2015 at 03:01:44PM +, Ian Campbell wrote: On Mon, 2015-02-02 at 12:59 +, Wei Liu wrote: Provide mini-os url and revision in Config.mk Introduce Makefile.mini-os which contains mini-os specific targets. I'm not sure it has enough content to warrant that, those two targets could easily be inlined. +ifeq ($(wildcard $(MINI_OS)/Config.mk),) +$(error Please run `make mini-os-dir' in top-level directory) +endif This makes me think that maybe my earlier suggestion to move the clone under stubdom/ might be better done right away. In fact, that would avoid the need to play tricks with renaming extras/ too, since you could build both for a commit. I think I made this hack not because of the location, but because I couldn't get it to clone in a sensible way without breaking stubdom build. I'm not GNU Make expert so I'm open to suggestion on this issue. So in this Makefile: 6 export stubdom=y 7 export debug=y 8 include $(XEN_ROOT)/Config.mk Line 8 includes $(XEN_ROOT)/Config.mk, which in turn includes $(MINI_OS)/Config.mk. Mini-os's Config.mk contains a bunch of flags. In GNU Make's manual: If an included makefile cannot be found in any of these directories, a warning message is generated, but it is not an immediately fatal error; processing of the makefile containing the include continues. Once it has finished reading makefiles, make will try to remake any that are out of date or don’t exist. See How Makefiles Are Remade. Only after it has tried to find a way to remake a makefile and failed, will make diagnose the missing makefile as a fatal error. So I once had something like: $(XEN_ROOT)/Config.mk: mini-os-dir Mini-os got clone and the build proceeded. But stubdom build still broke miserably, because *FLAGS in stubdom/Makefile are not reevaluated. Is there a solution to this? Wei. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] xen/arm: Automatically find a PPI for the DOM0 event channel interrupt
On Thu, 2015-01-29 at 15:51 +, Julien Grall wrote: Use the new vgic interface to know which virtual PPI is free and use it for the event channel code. At the DOM0 creation time, Xen doesn't know which vIRQ will be free. All the vIRQ will be reserved when we parse the device tree. So we can allocate the vIRQ just after the device tree has been parsed. It's safe to defer the allocation because no vIRQ can be injected as long as the vCPU is not online. As the device tree node hypervisor containing the description of the event channel interrupt is created earlier, add a placeholder which will be fix up once Xen has allocated the PPI. Also correct the check in arch_domain_create to use is_hardware_domain. Signed-off-by: Julien Grall julien.gr...@linaro.org Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 18/29] Ovmf/Xen: move XenBusDxe hypercall code to separate library
one important comment
On 01/26/15 20:03, Ard Biesheuvel wrote:
This moves all of the Xen hypercall code that was private to XenBusDxe
to a new library class XenHypercallLib. This will allow us to reimplement
it for ARM, and to export the Xen hypercall functionality to other parts
of the code, such as a Xen console SerialPortLib driver.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
---
OvmfPkg/{XenBusDxe/XenHypercall.h = Include/Library/XenHypercallLib.h} | 16
++-
OvmfPkg/{XenBusDxe = Library/XenHypercallLib}/Ia32/hypercall.nasm | 0
OvmfPkg/{XenBusDxe = Library/XenHypercallLib}/X64/hypercall.nasm | 0
OvmfPkg/{XenBusDxe = Library/XenHypercallLib}/XenHypercall.c | 37
++
OvmfPkg/Library/XenHypercallLib/XenHypercallIntel.c | 77
+++
OvmfPkg/Library/XenHypercallLib/XenHypercallLibIntel.inf| 52
OvmfPkg/OvmfPkg.dec | 4
OvmfPkg/OvmfPkgIa32.dsc | 1
+
OvmfPkg/OvmfPkgIa32X64.dsc | 1
+
OvmfPkg/OvmfPkgX64.dsc | 1
+
OvmfPkg/XenBusDxe/EventChannel.c| 3
++-
OvmfPkg/XenBusDxe/GrantTable.c | 2
+-
OvmfPkg/XenBusDxe/XenBusDxe.c | 9
+
OvmfPkg/XenBusDxe/XenBusDxe.inf | 11
+--
OvmfPkg/XenBusDxe/XenStore.c| 2
+-
15 files changed, 146 insertions(+), 70 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/XenHypercall.h
b/OvmfPkg/Include/Library/XenHypercallLib.h
similarity index 82%
rename from OvmfPkg/XenBusDxe/XenHypercall.h
rename to OvmfPkg/Include/Library/XenHypercallLib.h
index 9d49e33eb5af..dc2c5424683c 100644
--- a/OvmfPkg/XenBusDxe/XenHypercall.h
+++ b/OvmfPkg/Include/Library/XenHypercallLib.h
@@ -13,8 +13,8 @@
**/
-#ifndef __XENBUS_DXE_HYPERCALL_H__
-#define __XENBUS_DXE_HYPERCALL_H__
+#ifndef __XEN_HYPERCALL_LIB_H_
+#define __XEN_HYPERCALL_LIB_H_
I guess if you lead it with __, then you should also trail it with
__. :)
Other than that, it looks good to me.
Reviewed-by: Laszlo Ersek ler...@redhat.com
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] tools/libxc: Don't leave scratch_pfn uninitialised if the domain has no memory
On Wed, 2015-01-28 at 15:52 +, Andrew Cooper wrote: c/s 5b5c40c0d1 libxc: introduce a per architecture scratch pfn for temporary grant mapping accidentally an issue whereby there were two paths out of xc_core_arch_get_scratch_gpfn() which returned 0, but only one of which assigned a value to the gpfn parameter. xc_domain_maximum_gpfn() can validly return 0, at which point gpfn 1 is a valid scratch page to use. In addition, widen rc before adding 1 and possibly overflowing. Signed-off-by: Andrew Cooper andrew.coop...@citrix.com CC: Julien Grall julien.gr...@linaro.org CC: Jan Beulich jbeul...@suse.com CC: Ian Campbell ian.campb...@citrix.com CC: Ian Jackson ian.jack...@eu.citrix.com CC: Wei Liu wei.l...@citrix.com Acked + applied, thanks. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] ocaml/xenctrl: Make failwith_xc() thread safe
On Fri, 2015-01-30 at 14:24 +, Dave Scott wrote: On 30 Jan 2015, at 14:20, Wei Liu wei.l...@citrix.com wrote: On Fri, Jan 30, 2015 at 02:19:53PM +, Dave Scott wrote: Looks ok to me. Signed-off-by: David Scott dave.sc...@citrix.com I think this should be an Acked-by. Sorry, you’re completely right. Muscle memory strikes again! :) ;-) Acked-by: David Scott dave.sc...@citrix.com Thanks, Applied ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 05/15] xen/arm: vgic-v3: Correctly implement read into GICR_NSACR
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: The 32-bit register GICR_NSACR is RAZ/WI on non-secure state. Therefore we should not inject a data abort to the guest. Signed-off-by: Julien Grall julien.gr...@linaro.org Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] time: widen wallclock seconds to 64 bits
On Thu, 2015-01-29 at 15:39 +, Jan Beulich wrote: Linux is in the process of converting their seconds representation to 64 bits, so in order to support it consistently we should follow suit (which at some point in quite a few years we'd have to do anyway). To represent this in struct shared_info we leverage a 32-bit hole in x86-64's and arm's variant of the structure; for x86-32 guests the only (reasonable) choice we have is to put the extension in struct arch_shared_info. A note on the conditional suppressing the xen_wc_sec_hi helper macro definition in the ix86 case for hypervisor and tools: Neither of the two actually need this, and its presence causes the tools to fail to build (due to the inclusion of both the x86-64 and x86-32 variants of the header). As a secondary change, x86's do_platform_op() gets a pointless initializer as well as a pointless assignment of that same variable dropped. Signed-off-by: Jan Beulich jbeul...@suse.com --- a/tools/include/xen-foreign/Makefile +++ b/tools/include/xen-foreign/Makefile @@ -17,7 +17,7 @@ clean: distclean: clean checker: checker.c $(headers) - $(HOSTCC) $(HOSTCFLAGS) -o $@ $ + $(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -o $@ $ This is to make int64_aligned_t available I think? (Which isn't a problem just wanted to check it wasn't something more subtle) Everything else looks ok to me. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/7] stubdom: don't look for mini-os source file during configure
On Mon, Feb 02, 2015 at 03:09:32PM +, Ian Jackson wrote: Wei Liu writes ([PATCH 3/7] stubdom: don't look for mini-os source file during configure): Mini-os source code will be fetched during build. ... -AC_CONFIG_SRCDIR([../extras/mini-os/kernel.c]) You should replace this with a different file, surely. Maybe AC_CONFIG_SRCDIR([xenstore-minios.cfg]) ? Ack. Wei. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/15] xen/arm: vgic-v3: Correctly handle RAZ/WI registers
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: Some of the registers are accessible via multiple size (see GICD_IPRIORITYR*). Thoses registers are misimplemented when they should be RAZ. Only Those and incorrectly implemented. word-access size are currently allowed for them. To avoid further issues, introduce different label following the access-size of the registers: - read_as_zero_64 and write_ignore_64: Used for registers accessible via a double-word. - read_as_zero_32 and write_ignore_32: Used for registers accessible via a word. 5.1.3 suggests there are at least some 64-bit registers where it ought to be possible to read the upper and lower halves independently. BTW, a reference to 5.1.3 in the changelog would be handy. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] tools/xenctrl: correct some function declarations
On Fri, 2015-01-30 at 12:40 +, Wei Liu wrote: On Fri, Jan 30, 2015 at 03:32:26PM +0800, Tiejun Chen wrote: When commit 6865e52b78f4, PCI multi-seg: adjust domctl interface, is introduced, we missed to sync that head file. Signed-off-by: Tiejun Chen tiejun.c...@intel.com Acked-by: Wei Liu wei.l...@citrix.com Applied, thanks. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] apic-v reduce network performance in my test case
On Mon, Feb 02, 2015 at 09:58:57PM +0800, Liuqiming (John) wrote: Hi Jan, Thanks for the reply. On 2015/2/2 18:12, Jan Beulich wrote: On 31.01.15 at 11:29, john.liuqim...@huawei.com wrote: Recently I met an odd performance problem: when I turn on APIC Virtualization feature (apicv=1), the network performance of a windows guest become worse. My test case like this: host only have one windows 2008 R2 HVM guest running,and this guest has a SR-IOV VF network passthrough to it. Guest using this network access a NAS device. No fontend or backend of network and storage, all data transfered through network. The xentrace data shows: the mainly difference between apicv and non-apicv, is the way guest write apic registers, and EXIT_REASON_MSR_WRITE vmexit cost much more time than EXIT_REASON_APIC_WRITE, but when using WRMSR, the PAUSE vmexit is much less than using APIC-v. There being heavier use of the pause VMEXIT doesn't by itself say anything, I'm afraid. It may suggest that you have a C-state exit latency problem - try lowering the maximum C-state allowed, or disabling use of C-states altogether. Sorry, I forgot to mention my test scenario: Its a video test suite,I am not sure what the logic inside the tools exactly (not opensource tool). The basic flow is: 1) test suite start several thread to read video file from disk (from NAS through network in my case) 2) decode these video data as a frame one by one 3) if any frame delay more than 40ms, then mark as lost test result: apicv=1, there can be 15 thread running at the same time without lost frame apicv=0, there can be 22 thread running at the same time without lost frame so when I'm saying apicv reduce the performance, I got the conclusion from the test result not from what xentrace shows. In commit 7f2e992b824ec62a2818e64390ac2ccfbd74e6b7 VMX/Viridian: suppress MSR-based APIC suggestion when having APIC-V, msr based apic is disabled when apic-v is on, I wonder can they co-exist in some way? seems for windows guest msr-based apic has better performance. The whole purpose is to avoid the costly MSR access exits. Why would you want to reintroduce that overhead? Jan I agree to avoid the MSR access vmexit by using apicv, I just do not know what's the side effect. Because from the test result, apicv replacing msr-based access brings performance reduction. It sounds like it brings latency disruption, not neccessarily performance reduction. What happens if you run with the cpufreq turned to performance, or as Jan suggested - with disabling C-states? . ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 2/3] hvm_complete_assist_req: We should not be able to get here on IOREQ_TYPE_PCI_CONFIG
Suggested-by: Paul Durrant paul.durr...@citrix.com
Signed-off-by: Don Slutz dsl...@verizon.com
---
v2:
Add Suggested-by
xen/arch/x86/hvm/hvm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c7984d1..6f7b407 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2573,6 +2573,7 @@ bool_t hvm_send_assist_req_to_ioreq_server(struct
hvm_ioreq_server *s,
static bool_t hvm_complete_assist_req(ioreq_t *p)
{
+ASSERT(p-type != IOREQ_TYPE_PCI_CONFIG);
switch ( p-type )
{
case IOREQ_TYPE_COPY:
--
1.8.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 0/3] Skip unneeded VMENTRY VMEXIT
Changes v2: Paul Durrant: I think the two changes only make sense in combination. folded old #3 and old #5 into #3. Actually that comment is not right. The operation is not binned; it's just already been done. Comment has been dropped. I think you can ASSERT(s) here, as you should never get here ... Dropped call to hvm_complete_assist_req(), added ASSERT. So basically collapses down to just this call. Yes, so changed to call on hvm_send_assist_req_to_ioreq_server() and removed hvm_send_assist_req() which had 1 caller. Jan Beulich: The change on what to do when hvm_send_assist_req() fails is bad. That is correct. Made hvm_has_dm() do full checking so that the extra VMEXIT and VMENTRY can be skipped. Add Suggested-by to old #4 Done (Now #2) hvm_complete_assist_req() be a separate function yet having only a single caller ... Folded hvm_complete_assist_req() in to hvm_has_dm() which is the only place it is called. Using a new enough QEMU that has: commit 7665d6ba98e20fb05c420de947c1750fd47e5c07 Author: Paul Durrant paul.durr...@citrix.com Date: Tue Jan 20 11:06:19 2015 + Xen: Use the ioreq-server API when available means that hvmloader and the guest will do pci config accesses that will not be sent to QEMU. However hvm_complete_assist_req() (which is the routine that does the work) returns a 1 which hvm_send_assist_req() returns to the caller which means that the request was sent to QEMU and need to be waited for. This is not the case. hvm_complete_assist_req() has called on hvm_io_assist() which has changed the io_state from HVMIO_awaiting_completion to HVMIO_completed if needed. But hvmemul_do_io() thinks that it needs to wait on the IOREQ_READ case, and returns X86EMUL_RETRY. [PATCH 1/5] DEBUG: xentrace: Add debug of HANDLE_PIO -- No need to apply [PATCH 2/5] xentrace: Adjust IOPORT MMIO format -- Simple bugfix. xentrace_format is not converting these correctly [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do -- Simple bugfix. Do the same thing as when hvm_has_dm() returns a zero. [PATCH 4/5] hvm_complete_assist_req: We should not be able to get -- This ASSERT was sugested by Paul Durrant. Since I was in the file, just add it. [PATCH 5/5] hvm_complete_assist_req: Tell caller we failed to send -- The real fix. Does depend on [PATCH 3/5] hvmemul_do_io: If the send ... Here is the before xentrace output using the [PATCH 1/5] DEBUG: xentrace: Add debug of HANDLE_PIO (which I do not expect to be applied. It is first because that is the order you need to use to reproduce the xentrace output): CPU0 685992932190 (+2292) VMEXIT [ exitcode = 0x001e, rIP = 0x00101581 ] CPU0 0 (+ 0) HANDLE_PIO [ port = 0x0cfe size = 2 dir = 1 io_state = 5 ret = 0 ] CPU0 685992934938 (+2748) vlapic_accept_pic_intr [ i8259_target = 1, accept_pic_int = 1 ] CPU0 685992935526 (+ 588) VMENTRY CPU0 685992937650 (+2124) VMEXIT [ exitcode = 0x001e, rIP = 0x00101581 ] CPU0 0 (+ 0) IOPORT_READ [ port = 0x0cfe, data = 0x ] CPU0 0 (+ 0) HANDLE_PIO [ port = 0x0cfe size = 2 dir = 1 io_state = 0 ret = 1 ] CPU0 685992940248 (+2598) vlapic_accept_pic_intr [ i8259_target = 1, accept_pic_int = 1 ] CPU0 685992940968 (+ 720) VMENTRY And after: CPU2 1028654638584 (+2388) VMEXIT [ exitcode = 0x001e, rIP = 0x00101581 ] CPU2 0 (+ 0) IOPORT_READ [ port = 0x0cfe, data = 0x ] CPU2 0 (+ 0) HANDLE_PIO [ port = 0x0cfe size = 2 dir = 1 io_state = 0 ret = 1 ] CPU2 1028654641932 (+3348) vlapic_accept_pic_intr [ i8259_target = 1, accept_pic_int = 1 ] CPU2 1028654642586 (+ 654) VMENTRY Don Slutz (3): xentrace: Adjust IOPORT MMIO format hvm_complete_assist_req: We should not be able to get here on IOREQ_TYPE_PCI_CONFIG hvm_has_dm: Do a full check for backing DM tools/xentrace/formats| 8 xen/arch/x86/hvm/emulate.c| 8 ++-- xen/arch/x86/hvm/hvm.c| 26 +- xen/include/asm-x86/hvm/hvm.h | 6 -- 4 files changed, 23 insertions(+), 25 deletions(-) -- 1.8.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] ocaml/xenctrl: Fix stub_xc_readconsolering()
On Fri, 2015-01-30 at 14:30 +, Dave Scott wrote: On 30 Jan 2015, at 14:11, Andrew Cooper andrew.coop...@citrix.com wrote: The Ocaml stub to retrieve the hypervisor console ring had a few problems. * A single 32k buffer would truncate a large console ring. * The buffer was static and not under the protection of the Ocaml GC lock so could be clobbered by concurrent accesses. * Embedded NUL characters would cause caml_copy_string() (which is strlen() based) to truncate the buffer. The function is rewritten from scratch, using the same algorithm as the python stubs, but uses the protection of the Ocaml GC lock to maintain a static running total of the ring size, to avoid redundant realloc()ing in future calls. As far as the OCaml FFI goes, this looks fine to me. I can’t spot any problems with the more regular C parts either, although someone who is more used to spotting bugs in C code should probably take a look. It looks ok to me and the fact it is based on some existing code helps too. Acked-by: David Scott dave.sc...@citrix.com likewise + applied. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 19/29] Ovmf/Xen: introduce XENIO_PROTOCOL
On 01/26/15 20:03, Ard Biesheuvel wrote:
This introduces the abstract XENIO_PROTOCOL that will be used to
communicate the Xen grant table address to drivers supporting this
protocol. Primary purpose is allowing us to change the XenBusDxe
implementation so that it can support non-PCI Xen implementations
such as Xen on ARM.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
---
OvmfPkg/Include/Protocol/XenIo.h | 48
OvmfPkg/OvmfPkg.dec | 1 +
2 files changed, 49 insertions(+)
diff --git a/OvmfPkg/Include/Protocol/XenIo.h
b/OvmfPkg/Include/Protocol/XenIo.h
new file mode 100644
index ..510391f3b3e8
--- /dev/null
+++ b/OvmfPkg/Include/Protocol/XenIo.h
@@ -0,0 +1,48 @@
+/** @file
+ XenIo protocol to abstract arch specific details
+
+ The Xen implementations for the Intel and ARM archictures differ in the way
+ the base address of the grant table is communicated to the guest. The
former
+ uses a virtual PCI device, while the latter uses a device tree node.
+ In order to allow the XenBusDxe UEFI driver to be reused for the non-PCI
+ Xen implementation, this abstract protocol can be installed on a handle
+ with the appropriate base address.
+
+ Copyright (C) 2014, Linaro Ltd.
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD
License
+ which accompanies this distribution. The full text of the license may be
found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN AS IS BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
IMPLIED.
+
+**/
+
+#ifndef __PROTOCOL_XENIO_H__
+#define __PROTOCOL_XENIO_H__
+
+#include IndustryStandard/Xen/xen.h
+
+#define XENIO_PROTOCOL_GUID \
+ {0x6efac84f, 0x0ab0, 0x4747, {0x81, 0xbe, 0x85, 0x55, 0x62, 0x59, 0x04,
0x49}}
+
+///
+/// Forward declaration
+///
+typedef struct _XENIO_PROTOCOL XENIO_PROTOCOL;
+
+///
+/// Protocol structure
+///
+struct _XENIO_PROTOCOL {
+ //
+ // Protocol data fields
+ //
+ EFI_PHYSICAL_ADDRESS GrantTableAddress;
+};
+
+extern EFI_GUID gXenIoProtocolGuid;
+
+#endif
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 30a9fb1e9b42..3711fa922311 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -58,6 +58,7 @@
gVirtioDeviceProtocolGuid = {0xfa920010, 0x6785, 0x4941, {0xb6,
0xec, 0x49, 0x8c, 0x57, 0x9f, 0x16, 0x0a}}
gBlockMmioProtocolGuid = {0x6b558ce3, 0x69e5, 0x4c67, {0xa6,
0x34, 0xf7, 0xfe, 0x72, 0xad, 0xbe, 0x84}}
gXenBusProtocolGuid = {0x3d3ca290, 0xb9a5, 0x11e3, {0xb7,
0x5d, 0xb8, 0xac, 0x6f, 0x7d, 0x65, 0xe6}}
+ gXenIoProtocolGuid = {0x6efac84f, 0x0ab0, 0x4747, {0x81,
0xbe, 0x85, 0x55, 0x62, 0x59, 0x04, 0x49}}
[PcdsFixedAtBuild]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|0x0|UINT32|0
Reviewed-by: Laszlo Ersek ler...@redhat.com
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 3/3] hvm_has_dm: Do a full check for backing DM
This saves a VMENTRY and a VMEXIT since we not longer retry the
ioport read on backing DM not handling a given ioreq.
To only call on hvm_select_ioreq_server() once in this code path, return s.
Also switch to hvm_send_assist_req_to_ioreq_server().
Signed-off-by: Don Slutz dsl...@verizon.com
---
v2:
Paul Durrant:
I think the two changes only make sense in combination.
folded old #3 and old #5 into #3.
Actually that comment is not right. The operation is not binned;
it's just already been done.
Comment has been dropped.
I think you can ASSERT(s) here, as you should never get here ...
Dropped call to hvm_complete_assist_req(), added ASSERT.
So basically collapses down to just this call.
Yes, so changed to call on hvm_send_assist_req_to_ioreq_server()
and removed hvm_send_assist_req() which had 1 caller.
Jan Beulich:
The change on what to do when hvm_send_assist_req() fails is bad.
That is correct. Made hvm_has_dm() do full checking so
that the extra VMEXIT and VMENTRY can be skipped.
hvm_complete_assist_req() be a separate function yet having only
a single caller ...
Folded hvm_complete_assist_req() in to hvm_has_dm() which
is the only place it is called.
xen/arch/x86/hvm/emulate.c| 8 ++--
xen/arch/x86/hvm/hvm.c| 25 -
xen/include/asm-x86/hvm/hvm.h | 6 --
3 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 2ed4344..dca6d2e 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -218,8 +218,11 @@ static int hvmemul_do_io(
vio-io_state = HVMIO_handle_mmio_awaiting_completion;
break;
case X86EMUL_UNHANDLEABLE:
+{
+struct hvm_ioreq_server *s = hvm_has_dm(curr-domain, p);
+
/* If there is no backing DM, just ignore accesses */
-if ( !hvm_has_dm(curr-domain) )
+if ( !s )
{
rc = X86EMUL_OKAY;
vio-io_state = HVMIO_none;
@@ -227,11 +230,12 @@ static int hvmemul_do_io(
else
{
rc = X86EMUL_RETRY;
-if ( !hvm_send_assist_req(p) )
+if ( !hvm_send_assist_req_to_ioreq_server(s, p) )
vio-io_state = HVMIO_none;
else if ( p_data == NULL )
rc = X86EMUL_OKAY;
}
+}
break;
default:
BUG();
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 6f7b407..81bdf18 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2507,11 +2507,6 @@ int hvm_buffered_io_send(ioreq_t *p)
return 1;
}
-bool_t hvm_has_dm(struct domain *d)
-{
-return !list_empty(d-arch.hvm_domain.ioreq_server.list);
-}
-
bool_t hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
ioreq_t *proto_p)
{
@@ -2519,6 +2514,7 @@ bool_t hvm_send_assist_req_to_ioreq_server(struct
hvm_ioreq_server *s,
struct domain *d = curr-domain;
struct hvm_ioreq_vcpu *sv;
+ASSERT(s);
if ( unlikely(!vcpu_start_shutdown_deferral(curr)) )
return 0; /* implicitly bins the i/o operation */
@@ -2571,8 +2567,13 @@ bool_t hvm_send_assist_req_to_ioreq_server(struct
hvm_ioreq_server *s,
return 0;
}
-static bool_t hvm_complete_assist_req(ioreq_t *p)
+struct hvm_ioreq_server *hvm_has_dm(struct domain *d, ioreq_t *p)
{
+struct hvm_ioreq_server *s = hvm_select_ioreq_server(current-domain, p);
+
+if ( s )
+return s;
+
ASSERT(p-type != IOREQ_TYPE_PCI_CONFIG);
switch ( p-type )
{
@@ -2599,17 +2600,7 @@ static bool_t hvm_complete_assist_req(ioreq_t *p)
break;
}
-return 1;
-}
-
-bool_t hvm_send_assist_req(ioreq_t *p)
-{
-struct hvm_ioreq_server *s = hvm_select_ioreq_server(current-domain, p);
-
-if ( !s )
-return hvm_complete_assist_req(p);
-
-return hvm_send_assist_req_to_ioreq_server(s, p);
+return NULL;
}
void hvm_broadcast_assist_req(ioreq_t *p)
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index e3d2d9a..4bcc61c 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -228,7 +228,9 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v);
void hvm_vcpu_cacheattr_destroy(struct vcpu *v);
void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip);
-bool_t hvm_send_assist_req(ioreq_t *p);
+struct hvm_ioreq_server;
+bool_t hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
+ ioreq_t *p);
void hvm_broadcast_assist_req(ioreq_t *p);
void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat);
@@ -359,7 +361,7 @@ void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
unsigned int *ecx, unsigned int *edx);
void
[Xen-devel] [PATCH v2 1/3] xentrace: Adjust IOPORT MMIO format
The 1st item is not data, but the port (address). The 2nd item is the data. Signed-off-by: Don Slutz dsl...@verizon.com --- tools/xentrace/formats | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/xentrace/formats b/tools/xentrace/formats index da658bf..5d7b72a 100644 --- a/tools/xentrace/formats +++ b/tools/xentrace/formats @@ -68,10 +68,10 @@ 0x00082014 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) INVLPG [ is invlpga? = %(1)d, virt = 0x%(2)08x ] 0x00082114 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) INVLPG [ is invlpga? = %(1)d, virt = 0x%(3)08x%(2)08x ] 0x00082015 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) MCE -0x00082016 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) IOPORT_READ [ data = 0x%(1)04x ] -0x00082216 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) IOPORT_WRITE [ data = 0x%(1)04x ] -0x00082017 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) MMIO_READ [ data = 0x%(1)04x ] -0x00082217 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) MMIO_WRITE [ data = 0x%(1)04x ] +0x00082016 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) IOPORT_READ [ port = 0x%(1)04x, data = 0x%(2)08x ] +0x00082216 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) IOPORT_WRITE [ port = 0x%(1)04x, data = 0x%(2)08x ] +0x00082017 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) MMIO_READ [ port = 0x%(1)08x, data = 0x%(2)08x ] +0x00082217 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) MMIO_WRITE [ port = 0x%(1)08x, data = 0x%(2)08x ] 0x00082018 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) CLTS 0x00082019 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) LMSW[ value = 0x%(1)08x ] 0x00082119 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) LMSW[ value = 0x%(2)08x%(1)08x ] -- 1.8.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/7] stubdom: fix make build
On Mon, 2015-02-02 at 12:58 +, Wei Liu wrote: Cross compiling libxc requires some symlinks to exist. Note that make -C tools/include requires running tools/configure. But at least now the error message is much better than just a file not found error. Signed-off-by: Wei Liu wei.l...@citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/7] Makefile: refactor build/clean/distclean targets
On Mon, 2015-02-02 at 12:58 +, Wei Liu wrote: Factor out per-subsystem build/clean/distclean-% targets, so that we can build subsystems independently in top level directory. The motive behind this is after splitting out mini-os from Xen tree, stubdom is in effect a downstream of mini-os. I would like to have the ability to build it independently and instrument OSSTest to test it. Signed-off-by: Wei Liu wei.l...@citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/7] Mini-OS: standalone build
On Mon, 2015-02-02 at 12:59 +, Wei Liu wrote: In order to keep the tree bisectable all the changes are done in one single commit. Things done in this commit: 1. Import necessary .mk files from Xen. 2. Move all XEN_ related variables to MINIOS_ namespace. 3. Import Xen public header files. 4. Import BSD's list.h and helper script. Mini-OS's vanilla Config.mk is modified to contain some macros copied from Xen's Config.mk. It also contains compatibility handling logic for Xen's stubdom build environment. Files modified: Config.mk Makefile arch/x86/Makefile arch/x86/arch.mk minios.mk All other files are just imported from Xen. Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com I've not reviewed in depth, but: Acked-by: Ian Campbell ian.campb...@citrix.com --- .gitignore |1 - extras/mini-os/Config.mk | 50 +- extras/mini-os/Makefile| 16 +- extras/mini-os/arch/x86/Makefile |5 +- extras/mini-os/arch/x86/arch.mk|8 +- extras/mini-os/config/MiniOS.mk| 10 + extras/mini-os/config/StdGNU.mk| 47 + extras/mini-os/config/arm32.mk | 22 + extras/mini-os/config/arm64.mk | 19 + extras/mini-os/config/x86_32.mk| 20 + extras/mini-os/config/x86_64.mk| 33 + [ import output trimmed ] extras/mini-os/minios.mk |4 +- 76 files changed, 16511 insertions(+), 23 deletions(-) [ import output trimmed ] diff --git a/.gitignore b/.gitignore index 13ee05b..cdbdca7 100644 --- a/.gitignore +++ b/.gitignore @@ -48,7 +48,6 @@ docs/pdf/ docs/txt/ extras/mini-os/include/mini-os extras/mini-os/include/x86/mini-os -extras/mini-os/include/xen extras/mini-os/include/list.h extras/mini-os/mini-os* install/* diff --git a/extras/mini-os/Config.mk b/extras/mini-os/Config.mk index 4852443..e5d8ade 100644 --- a/extras/mini-os/Config.mk +++ b/extras/mini-os/Config.mk @@ -1,7 +1,49 @@ -# Set mini-os root path, used in mini-os.mk. +# +# Compare $(1) and $(2) and replace $(2) with $(1) if they differ +# +# Typically $(1) is a newly generated file and $(2) is the target file +# being regenerated. This prevents changing the timestamp of $(2) only +# due to being auto regenereated with the same contents. +define move-if-changed +if ! cmp -s $(1) $(2); then mv -f $(1) $(2); else rm -f $(1); fi +endef + +# cc-option: Check if compiler supports first option, else fall back to second. +# +# This is complicated by the fact that unrecognised -Wno-* options: +# (a) are ignored unless the compilation emits a warning; and +# (b) even then produce a warning rather than an error +# To handle this we do a test compile, passing the option-under-test, on a code +# fragment that will always produce a warning (integer assigned to pointer). +# We then grep for the option-under-test in the compiler's output, the presence +# of which would indicate an unrecognized command-line option warning/error. +# +# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) +cc-option = $(shell if test -z `echo 'void*p=1;' | \ + $(1) $(2) -S -o /dev/null -x c - 21 | grep -- $(2) -`; \ + then echo $(2); else echo $(3); fi ;) + +# Compatibility with Xen's stubdom build environment. If we are building +# stubdom, some XEN_ variables are set, set MINIOS_ variables accordingly. +# +ifneq ($(XEN_ROOT),) MINI-OS_ROOT=$(XEN_ROOT)/extras/mini-os +else +MINI-OS_ROOT=$(TOPLEVEL_DIR) +endif export MINI-OS_ROOT +ifneq ($(XEN_TARGET_ARCH),) +MINIOS_TARGET_ARCH = $(XEN_TARGET_ARCH) +else +MINIOS_COMPILE_ARCH?= $(shell uname -m | sed -e s/i.86/x86_32/ \ +-e s/i86pc/x86_32/ -e s/amd64/x86_64/ \ +-e s/armv7.*/arm32/ -e s/armv8.*/arm64/ \ +-e s/aarch64/arm64/) + +MINIOS_TARGET_ARCH ?= $(MINIOS_COMPILE_ARCH) +endif + libc = $(stubdom) XEN_INTERFACE_VERSION := 0x00030205 @@ -9,11 +51,11 @@ export XEN_INTERFACE_VERSION # Try to find out the architecture family TARGET_ARCH_FAM. # First check whether x86_... is contained (for x86_32, x86_32y, x86_64). -# If not x86 then use $(XEN_TARGET_ARCH) -ifeq ($(findstring x86_,$(XEN_TARGET_ARCH)),x86_) +# If not x86 then use $(MINIOS_TARGET_ARCH) +ifeq ($(findstring x86_,$(MINIOS_TARGET_ARCH)),x86_) TARGET_ARCH_FAM = x86 else -TARGET_ARCH_FAM = $(XEN_TARGET_ARCH) +TARGET_ARCH_FAM = $(MINIOS_TARGET_ARCH) endif # The architecture family directory below mini-os. diff --git a/extras/mini-os/Makefile b/extras/mini-os/Makefile index 6d6537e..f16520e 100644 ---
Re: [Xen-devel] [PATCH 4/7] git-checkout.sh: use mkdir -p
On Mon, 2015-02-02 at 12:59 +, Wei Liu wrote: Otherwise mkdir extras/mini-os fails because extras doesn't exist. Signed-off-by: Wei Liu wei.l...@citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com As a future change perhaps we should clone mini-os under stubdom/* somewhere instead of in extras? Cc: Ian Jackson ian.jack...@eu.citrix.com --- scripts/git-checkout.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/git-checkout.sh b/scripts/git-checkout.sh index 15b3ce9..20ae31f 100755 --- a/scripts/git-checkout.sh +++ b/scripts/git-checkout.sh @@ -13,7 +13,7 @@ set -e if test \! -d $DIR-remote; then rm -rf $DIR-remote $DIR-remote.tmp - mkdir $DIR-remote.tmp; rmdir $DIR-remote.tmp + mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp $GIT clone $TREE $DIR-remote.tmp if test $TAG ; then cd $DIR-remote.tmp ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/7] build system: stubdom targets now depends on mini-os target
Wei Liu writes (Re: [PATCH 6/7] build system: stubdom targets now depends on mini-os target): So I once had something like: $(XEN_ROOT)/Config.mk: mini-os-dir Mini-os got clone and the build proceeded. But stubdom build still broke miserably, because *FLAGS in stubdom/Makefile are not reevaluated. This seems wrong to me because I would expect make to actually read the relevant file it has just built. But given how bad the xen.git/stubdom makefiles are, I'm not convinced that trying to debug this will be very fruitful :-/. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] libxl: Wait for ballooning if free memory is increasing
On Monday, February 02, 2015 02:35:39 PM Ian Campbell wrote:
On Fri, 2015-01-30 at 14:01 -0700, Mike Latimer wrote:
During domain startup, all required memory ballooning must complete
within a maximum window of 33 seconds (3 retries, 11 seconds of delay).
If not, domain creation is aborted with a 'failed to free memory' error.
In order to accommodate large domains or slower hardware (which require
substantially longer to balloon memory) the free memory process should
continue retrying if the amount of free memory is increasing on each
iteration of the loop.
Sorry for not spotting this earlier, but to accept a patch into the Xen
code base we need a Signed-off-by:
http://wiki.xen.org/wiki/Submitting_Xen_Patches#Signing_off_a_patch
You can provide one in this thread rather than resending if that is more
convenient.
Sorry, I should have caught that myself. If's it's okay to provide it here:
Signed-off-by: Mike Latimer mlati...@suse.com
For my part: Acked-by: Ian Campbell ian.campb...@citrix.com
Thanks,
Mike
---
tools/libxl/xl_cmdimpl.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 0b02a6c..9ff3c4f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2194,8 +2194,9 @@ static int preserve_domain(uint32_t *r_domid,
libxl_event *event,
static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
{
-int rc, retries = 3;
-uint32_t need_memkb, free_memkb;
+int rc, retries;
+const int MAX_RETRIES = 3;
+uint32_t need_memkb, free_memkb, free_memkb_prev = 0;
if (!autoballoon)
return 0;
@@ -2204,6 +2205,7 @@ static int freemem(uint32_t domid,
libxl_domain_build_info *b_info)
if (rc 0)
return rc;
+retries = MAX_RETRIES;
do {
rc = libxl_get_free_memory(ctx, free_memkb);
if (rc 0)
@@ -2228,7 +2230,16 @@ static int freemem(uint32_t domid,
libxl_domain_build_info *b_info)
if (rc 0)
return rc;
-retries--;
+/*
+ * If the amount of free mem has increased on this iteration
(i.e.
+ * some progress has been made) then reset the retry counter.
+ */
+if (free_memkb free_memkb_prev) {
+retries = MAX_RETRIES;
+free_memkb_prev = free_memkb;
+} else {
+retries--;
+}
} while (retries 0);
return ERROR_NOMEM;
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH] dpci: Put the dpci back on the list if running on another CPU.
On 02.02.15 at 15:29, konrad.w...@oracle.com wrote:
On Tue, Jan 13, 2015 at 10:20:00AM +, Jan Beulich wrote:
On 12.01.15 at 17:45, konrad.w...@oracle.com wrote:
There is race when we clear the STATE_SCHED in the softirq
- which allows the 'raise_softirq_for' to progress and
schedule another dpci. During that time the other CPU could
receive an interrupt and calls 'raise_softirq_for' and put
the dpci on its per-cpu list. There would be two 'dpci_softirq'
running at the same time (on different CPUs) where the
dpci state is STATE_RUN (and STATE_SCHED is cleared). This
ends up hitting:
if ( test_and_set_bit(STATE_RUN, pirq_dpci-state) )
BUG()
Instead of that put the dpci back on the per-cpu list to deal
with later.
The reason we can get his with this is when an interrupt
affinity is set over multiple CPUs.
Another potential fix would be to add a guard in the raise_softirq_for
to check for 'STATE_RUN' bit being set and not schedule the
dpci until that bit has been cleared.
I indeed think this should be investigated, because it would make
explicit what ...
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -804,7 +804,17 @@ static void dpci_softirq(void)
d = pirq_dpci-dom;
smp_mb(); /* 'd' MUST be saved before we set/clear the bits. */
if ( test_and_set_bit(STATE_RUN, pirq_dpci-state) )
-BUG();
+{
+unsigned long flags;
+
+/* Put back on the list and retry. */
+local_irq_save(flags);
+list_add_tail(pirq_dpci-softirq_list, this_cpu(dpci_list));
+local_irq_restore(flags);
+
+raise_softirq(HVM_DPCI_SOFTIRQ);
+continue;
+}
... this does implicitly - spin until the bad condition cleared.
I still haven't gotten to trying to reproduce the issues Malcolm
saw which is easier for me to do (use Intel super-fast storage
in a Windows guest) - since I've one of those boxes.
However in lieu of that, here is a patch that does pass my testing
of SR-IOV, and I believe should work fine. I _think_ it covers
what you want it to have Jan, but of course please correct me
if I made a mistake in the logic.
Since the code quoted above is still there in the new patch, the new
patch can at best be half of what I suggested.
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -63,10 +63,37 @@ enum {
static void raise_softirq_for(struct hvm_pirq_dpci *pirq_dpci)
{
unsigned long flags;
+unsigned long old, new, val = pirq_dpci-state;
-if ( test_and_set_bit(STATE_SCHED, pirq_dpci-state) )
-return;
+/*
+ * This cmpxch is a more clear version of:
+ * if ( test_and_set_bit(STATE_SCHED, pirq_dpci-state) )
+ * return;
+ * since it also checks for STATE_RUN conditions.
+ */
+for ( ;; )
+{
+new = 1 STATE_SCHED;
+if ( val )
+new |= val;
Why the if()?
+old = cmpxchg(pirq_dpci-state, val, new);
+switch ( old )
+{
+case (1 STATE_SCHED):
+case (1 STATE_RUN) | (1 STATE_SCHED):
+/*
+ * Whenever STATE_SCHED is set we MUST not schedule it.
+ */
+return;
+}
+/*
+ * If the 'state' is 0 or (1 STATE_RUN) we can schedule it.
+ */
Really? Wasn't the intention to _not_ schedule when STATE_RUN is
set? (Also the above is a only line comment, i.e. wants different style.)
I.e. with what you do now you could as well keep the old code.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/7] build system: stubdom targets now depends on mini-os target
On Mon, Feb 02, 2015 at 03:15:07PM +, Ian Jackson wrote: Wei Liu writes (Re: [PATCH 6/7] build system: stubdom targets now depends on mini-os target): So I once had something like: $(XEN_ROOT)/Config.mk: mini-os-dir Mini-os got clone and the build proceeded. But stubdom build still broke miserably, because *FLAGS in stubdom/Makefile are not reevaluated. This seems wrong to me because I would expect make to actually read the relevant file it has just built. But given how bad the xen.git/stubdom makefiles are, I'm not convinced that trying to debug this will be very fruitful :-/. FWIW I did this test myself. == Config.mk include New.mk == end Config.mk == Makefile include Config.mk .PHONY: all @echo $(VAR) New.mk: @echo VAR = 123 New.mk VAR += ABC == end Makefile And the final result is VAR = 123 ABC. However this is not the case for stubdom. :-/ I agree it won't be fruitful in debugging this. Wei. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl_set_memory_target: only remove videoram from absolute targets
On Wed, 2015-01-28 at 14:47 +, Stefano Stabellini wrote: On Wed, 28 Jan 2015, Ian Campbell wrote: On Wed, 2015-01-28 at 14:24 +, Stefano Stabellini wrote: On Wed, 28 Jan 2015, Ian Campbell wrote: On Mon, 2015-01-26 at 16:47 +, Stefano Stabellini wrote: If the new target is relative to the current target, do not remove videoram again: it has already been removed from the current target. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com How does this relate to libxl_set_memory_target: retain the same maxmem offset on top of the current target? Doesn't that also achieve the same aim, since maxmem accounts for vram but target doesn't? The two patches are different: this one affects the memory target while the other one affects maxmem. The only relationship is that this issue was found out by looking at the other patch. Not even a textual relationship? (i.e. one needs to go first to avoid conflicts) Yep, there is a textual relationship. Sorry I failed to mention it. This one needs to go first. Acked + applied then, thanks. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 07/15] xen/arm: vgic-v3: Use a struct to describe contiguous rdist regions
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: Also update the different comment to make clear that we register one MMIO region per contiguous regions and not per re-distributor. Signed-off-by: Julien Grall julien.gr...@linaro.org Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH] dpci: Put the dpci back on the list if running on another CPU.
On Tue, Jan 13, 2015 at 10:20:00AM +, Jan Beulich wrote:
On 12.01.15 at 17:45, konrad.w...@oracle.com wrote:
There is race when we clear the STATE_SCHED in the softirq
- which allows the 'raise_softirq_for' to progress and
schedule another dpci. During that time the other CPU could
receive an interrupt and calls 'raise_softirq_for' and put
the dpci on its per-cpu list. There would be two 'dpci_softirq'
running at the same time (on different CPUs) where the
dpci state is STATE_RUN (and STATE_SCHED is cleared). This
ends up hitting:
if ( test_and_set_bit(STATE_RUN, pirq_dpci-state) )
BUG()
Instead of that put the dpci back on the per-cpu list to deal
with later.
The reason we can get his with this is when an interrupt
affinity is set over multiple CPUs.
Another potential fix would be to add a guard in the raise_softirq_for
to check for 'STATE_RUN' bit being set and not schedule the
dpci until that bit has been cleared.
I indeed think this should be investigated, because it would make
explicit what ...
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -804,7 +804,17 @@ static void dpci_softirq(void)
d = pirq_dpci-dom;
smp_mb(); /* 'd' MUST be saved before we set/clear the bits. */
if ( test_and_set_bit(STATE_RUN, pirq_dpci-state) )
-BUG();
+{
+unsigned long flags;
+
+/* Put back on the list and retry. */
+local_irq_save(flags);
+list_add_tail(pirq_dpci-softirq_list, this_cpu(dpci_list));
+local_irq_restore(flags);
+
+raise_softirq(HVM_DPCI_SOFTIRQ);
+continue;
+}
... this does implicitly - spin until the bad condition cleared.
I still haven't gotten to trying to reproduce the issues Malcolm
saw which is easier for me to do (use Intel super-fast storage
in a Windows guest) - since I've one of those boxes.
However in lieu of that, here is a patch that does pass my testing
of SR-IOV, and I believe should work fine. I _think_ it covers
what you want it to have Jan, but of course please correct me
if I made a mistake in the logic.
From 20766b144e0b11daf3227ae978880bacd686ea2b Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Date: Mon, 12 Jan 2015 11:35:03 -0500
Subject: [PATCH] dpci: schedule properly the dpci when it is in STATE_RUN on
any per-cpu list.
There is race when we clear the STATE_SCHED in the softirq
- which allows the 'raise_softirq_for' to progress and
schedule another dpci. During that time the other CPU could
receive an interrupt and calls 'raise_softirq_for' and put
the dpci on its per-cpu list. There would be two 'dpci_softirq'
running at the same time (on different CPUs) where the
dpci state is STATE_RUN (and STATE_SCHED is cleared). This
ends up hitting:
if ( test_and_set_bit(STATE_RUN, pirq_dpci-state) )
BUG()
The reason we can get his with this is when an interrupt
affinity is set over multiple CPUs.
Instead of the BUG() we can put the dpci back on the per-cpu
list to deal with later (when the softirq are activated again).
This putting the 'dpci' back on the per-cpu list is an spin
until the bad condition clear.
We also expand the test-and-set(STATE_SCHED) in raise_softirq_for
to detect for 'STATE_RUN' bit being set and schedule the dpci.
The cases in which an dpci is NOT going to be scheduled is
whenever the 'STATE_SCHED' bit is set (regardless whether
STATE_RUN is set or not).
Reported-by: Sander Eikelenboom li...@eikelenboom.it
Reported-by: Malcolm Crossley malcolm.cross...@citrix.com
Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
xen/drivers/passthrough/io.c | 43 ---
1 file changed, 40 insertions(+), 3 deletions(-)
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index ae050df..b61b73c 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -63,10 +63,37 @@ enum {
static void raise_softirq_for(struct hvm_pirq_dpci *pirq_dpci)
{
unsigned long flags;
+unsigned long old, new, val = pirq_dpci-state;
-if ( test_and_set_bit(STATE_SCHED, pirq_dpci-state) )
-return;
+/*
+ * This cmpxch is a more clear version of:
+ * if ( test_and_set_bit(STATE_SCHED, pirq_dpci-state) )
+ * return;
+ * since it also checks for STATE_RUN conditions.
+ */
+for ( ;; )
+{
+new = 1 STATE_SCHED;
+if ( val )
+new |= val;
+old = cmpxchg(pirq_dpci-state, val, new);
+switch ( old )
+{
+case (1 STATE_SCHED):
+case (1 STATE_RUN) | (1 STATE_SCHED):
+/*
+ * Whenever STATE_SCHED is set we MUST not schedule it.
+ */
+return;
+}
+/*
+ * If the 'state' is 0 or (1 STATE_RUN) we can schedule it.
+ */
+
Re: [Xen-devel] [OSSTEST PATCH v4 3/9] ts-xen-build: build with XSM support if requested
On Mon, Feb 02, 2015 at 01:14:34PM +, Ian Campbell wrote: On Wed, 2014-12-10 at 22:21 +, Wei Liu wrote: Signed-off-by: Wei Liu wei.l...@citrix.com Inconsistent whitespace in the final hunk, but: Acked-by: Ian Campbell ian.campb...@citrix.com This will override the default XSM_ENABLE always, even if no runvar is present, I guess we think that is correct? I'm not sure what's correct. I originally treated it as the same as debug=. But now since there is doubt about what is the correct behaviour I will make it only set XSM_ENABLE when runvar is set in next version. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/7] Split off mini-os to a separate tree
On Mon, 2015-02-02 at 12:58 +, Wei Liu wrote: As far as I can tell there is no incoming mini-os patches at the moment. So I suggest we commence once we get that last mini-os patch in staging pushed to master. Is the new xenbits tree (the proper one, not your people one) for mini-os all setup and in place (but perhaps not populated?) I was just about to suggest that I apply patch #1, #2 and #4 as precursors. It sounds like you would prefer that I do not. I use following runes to split off mini-os: git filter-branch --tag-name-filter cat \ --subdirectory-filter extras/mini-os/ -- --all # There is already a tag name 4.3.0-rc2 which points to the same commit. git tag -d xen-4.3.0-rc2 I wonder if we should just nuke that, it always breaks my tab completion and is very annoying ;-) Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH v4 3/9] ts-xen-build: build with XSM support if requested
On Mon, 2015-02-02 at 14:30 +, Wei Liu wrote: On Mon, Feb 02, 2015 at 01:14:34PM +, Ian Campbell wrote: On Wed, 2014-12-10 at 22:21 +, Wei Liu wrote: Signed-off-by: Wei Liu wei.l...@citrix.com Inconsistent whitespace in the final hunk, but: Acked-by: Ian Campbell ian.campb...@citrix.com This will override the default XSM_ENABLE always, even if no runvar is present, I guess we think that is correct? I'm not sure what's correct. I originally treated it as the same as debug=. But now since there is doubt about what is the correct behaviour I will make it only set XSM_ENABLE when runvar is set in next version. That's probably going to have the least potential to be problematic in the future. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 7/7] Remove in-tree mini-os directory
On Mon, 2015-02-02 at 12:59 +, Wei Liu wrote: Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com [ output trimmed ] Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/15] xen/arm: vgic-v3: Correctly handle RAZ/WI registers
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: Some of the registers are accessible via multiple size (see GICD_IPRIORITYR*). Thoses registers are misimplemented when they should be RAZ. Only Those and incorrectly implemented. word-access size are currently allowed for them. To avoid further issues, introduce different label following the access-size of the registers: - read_as_zero_64 and write_ignore_64: Used for registers accessible via a double-word. - read_as_zero_32 and write_ignore_32: Used for registers accessible via a word. 5.1.3 suggests there are at least some 64-bit registers where it ought to be possible to read the upper and lower halves independently. Don't you need to support that? BTW, a reference to 5.1.3 in the changelog would be handy. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH] dpci: Put the dpci back on the list if running on another CPU.
On Mon, Feb 02, 2015 at 03:19:33PM +, Jan Beulich wrote:
On 02.02.15 at 15:29, konrad.w...@oracle.com wrote:
On Tue, Jan 13, 2015 at 10:20:00AM +, Jan Beulich wrote:
On 12.01.15 at 17:45, konrad.w...@oracle.com wrote:
There is race when we clear the STATE_SCHED in the softirq
- which allows the 'raise_softirq_for' to progress and
schedule another dpci. During that time the other CPU could
receive an interrupt and calls 'raise_softirq_for' and put
the dpci on its per-cpu list. There would be two 'dpci_softirq'
running at the same time (on different CPUs) where the
dpci state is STATE_RUN (and STATE_SCHED is cleared). This
ends up hitting:
if ( test_and_set_bit(STATE_RUN, pirq_dpci-state) )
BUG()
Instead of that put the dpci back on the per-cpu list to deal
with later.
The reason we can get his with this is when an interrupt
affinity is set over multiple CPUs.
Another potential fix would be to add a guard in the raise_softirq_for
to check for 'STATE_RUN' bit being set and not schedule the
dpci until that bit has been cleared.
I indeed think this should be investigated, because it would make
explicit what ...
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -804,7 +804,17 @@ static void dpci_softirq(void)
d = pirq_dpci-dom;
smp_mb(); /* 'd' MUST be saved before we set/clear the bits. */
if ( test_and_set_bit(STATE_RUN, pirq_dpci-state) )
-BUG();
+{
+unsigned long flags;
+
+/* Put back on the list and retry. */
+local_irq_save(flags);
+list_add_tail(pirq_dpci-softirq_list,
this_cpu(dpci_list));
+local_irq_restore(flags);
+
+raise_softirq(HVM_DPCI_SOFTIRQ);
+continue;
+}
... this does implicitly - spin until the bad condition cleared.
I still haven't gotten to trying to reproduce the issues Malcolm
saw which is easier for me to do (use Intel super-fast storage
in a Windows guest) - since I've one of those boxes.
However in lieu of that, here is a patch that does pass my testing
of SR-IOV, and I believe should work fine. I _think_ it covers
what you want it to have Jan, but of course please correct me
if I made a mistake in the logic.
Since the code quoted above is still there in the new patch, the new
patch can at best be half of what I suggested.
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -63,10 +63,37 @@ enum {
static void raise_softirq_for(struct hvm_pirq_dpci *pirq_dpci)
{
unsigned long flags;
+unsigned long old, new, val = pirq_dpci-state;
-if ( test_and_set_bit(STATE_SCHED, pirq_dpci-state) )
-return;
+/*
+ * This cmpxch is a more clear version of:
+ * if ( test_and_set_bit(STATE_SCHED, pirq_dpci-state) )
+ * return;
+ * since it also checks for STATE_RUN conditions.
+ */
+for ( ;; )
+{
+new = 1 STATE_SCHED;
+if ( val )
+new |= val;
Why the if()?
To 'or' the variable new with '1 STATE_RUN' in case 'val' changed from
the first read ('val = pirq_dpci-state') to the moment when
we do the cmpxchg.
+old = cmpxchg(pirq_dpci-state, val, new);
+switch ( old )
+{
+case (1 STATE_SCHED):
+case (1 STATE_RUN) | (1 STATE_SCHED):
+/*
+ * Whenever STATE_SCHED is set we MUST not schedule it.
+ */
+return;
+}
+/*
+ * If the 'state' is 0 or (1 STATE_RUN) we can schedule it.
+ */
Really? Wasn't the intention to _not_ schedule when STATE_RUN is
set? (Also the above is a only line comment, i.e. wants different style.)
I must confess I must have misread your last review. You said:
Here is a patch that does this. I don't yet have an setup to test
the failing scenario (working on that). I removed the part in
the softirq because with this patch I cannot see a way it would
ever get there (in the softirq hitting the BUG).
Hmm, but how do you now schedule the second instance that needed ...
+case (1 STATE_RUN):
... in this case?
which to me implied you still want to schedule an 'dpci' when STATE_RUN is set?
My thinking is that we should still schedule an 'dpci' when STATE_RUN is set
as that is inline with what the old tasklet code did. It would
schedule the tasklet the moment said tasklet was off the global list (but
it would not add it in the global list - that would be the job of the
tasklet function wrapper to detect and insert said tasklet back on
the global list).
I.e. with what you do now you could as well keep the old code.
Jan
___
Re: [Xen-devel] [PATCH v2 06/15] xen/arm: vgic-v3: Set stride during domain initialization
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote:
The stride may not be set if the hardware GIC is using the default
layout. It happens on the Foundation model.
On GICv3, the default stride is 2 * 64K. Therefore it's possible to avoid
checking at every redistributor MMIO access if the stride is not set.
Can this defaulting not be pulled further to the initialisation of
gicv3.rdist_stride?
This is only happening for DOM0,
Please say instead Because domU uses a static stride configuration this
only happens for dom0... or similar (i.e. include the reason why domU
is excluded)
so we can move this code in
gicv_v3_init. Take the opportunity to move the stride setting a bit ealier
earlier.
because the loop to set regions will require the stride.
Also, use 2 * 64K rather than 128K and explain the reason.
Signed-off-by: Julien Grall julien.gr...@linaro.org
---
I wasn't not sure where to move this code. I find very confusion the
splitting between vgic and gicv. Maybe we should introduce a
hwdom_gicv_init and giccc_map callbacks. Then move most of the
initialization in the vgic one.
Changes in v2:
- Patch added
---
xen/arch/arm/gic-v3.c | 11 ++-
xen/arch/arm/vgic-v3.c | 6 +-
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 47452ca..7b33ff7 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -897,12 +897,21 @@ static int gicv_v3_init(struct domain *d)
{
d-arch.vgic.dbase = gicv3.dbase;
d-arch.vgic.dbase_size = gicv3.dbase_size;
+
+d-arch.vgic.rdist_stride = gicv3.rdist_stride;
+/*
+ * If the stride is not set, the default stride for GICv3 is 2 * 64K:
+ * - first 64k page for Control and Physical LPIs
+ * - second 64k page for Control and Generation of SGIs
+ */
+if ( !d-arch.vgic.rdist_stride )
+d-arch.vgic.rdist_stride = 2 * SZ_64K;
+
for ( i = 0; i gicv3.rdist_count; i++ )
{
d-arch.vgic.rbase[i] = gicv3.rdist_regions[i].base;
d-arch.vgic.rbase_size[i] = gicv3.rdist_regions[i].size;
}
-d-arch.vgic.rdist_stride = gicv3.rdist_stride;
d-arch.vgic.rdist_count = gicv3.rdist_count;
}
else
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 2c14717..db6b514 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -625,11 +625,7 @@ static int vgic_v3_rdistr_mmio_read(struct vcpu *v,
mmio_info_t *info)
Why not the write case too?
perfc_incr(vgicr_reads);
-if ( v-domain-arch.vgic.rdist_stride != 0 )
-offset = info-gpa (v-domain-arch.vgic.rdist_stride - 1);
-else
-/* If stride is not set. Default 128K */
-offset = info-gpa (SZ_128K - 1);
+offset = info-gpa (v-domain-arch.vgic.rdist_stride - 1);
if ( offset SZ_64K )
return __vgic_v3_rdistr_rd_mmio_read(v, info, offset);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/15] xen/arm: vgic-v3: Correctly handle RAZ/WI registers
Hi Ian, On 02/02/15 15:24, Ian Campbell wrote: On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: Some of the registers are accessible via multiple size (see GICD_IPRIORITYR*). Thoses registers are misimplemented when they should be RAZ. Only Those and incorrectly implemented. word-access size are currently allowed for them. To avoid further issues, introduce different label following the access-size of the registers: - read_as_zero_64 and write_ignore_64: Used for registers accessible via a double-word. - read_as_zero_32 and write_ignore_32: Used for registers accessible via a word. 5.1.3 suggests there are at least some 64-bit registers where it ought to be possible to read the upper and lower halves independently. Don't you need to support that? Only when the system is supporting AArch32. If the system only supports AArch64, 64-bit registers can only be read via a 64-bit access. I don't think we actually support AArch32 on the vGICv3 drivers. And we don't emulate 32-bits access on 64-bit registers. I will give a look to it. BTW, a reference to 5.1.3 in the changelog would be handy. I will also mention the version of the document as this paragraph doesn't exists on the previous version. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 08/15] xen/arm: vgic-v3: Emulate correctly the re-distributor
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: There is a one-to-one mapping between each re-distributors and processors. Each re-distributors can be accessed by any processor at any time. For instance during the initialization of the GIC, the drivers will browse the re-distributor to find the one associated to the current processor (via GICR_TYPER). So each re-distributor has its own MMIO region. The current implementation of the vGICv3 emulation assumes that the re-distributor region is banked. Therefore, the processor won't be able to access other re-distributor. While this is working fine for Linux, a processor will only access GICR_TYPER to find the associated re-distributor, we should have a correct implementation for the other operating system. You could state something stronger here, which is that it is wrong and should be fixed as a matter of principal. The fact that we happen to get away with it for some versions of Linux is an aside (although worth mentioning) All emulated registers of the re-distributors take a vCPU in parameter and necessary lock. Therefore concurrent access is already properly handled. The missing bit is retrieving the right vCPU following the region accessed. Retrieving the right vCPU could be slow, so it has been divided in 2 paths: - fast path: The current vCPU is accessing its own re-distributor - slow path: The current vCPU is accessing an other re-distributor another. As the processor needs to initialize itself, the former case is very common. To handle the access quickly, the base address of the re-distributor is computed and stored per-vCPU during the vCPU initialization. The latter is less common and more complicate to handle. The re-distributors can be spread accross multiple regions in the memory. across +/* + * Find the region where the re-distributor lives. For this purpose, + * we look one region ahead as only MMIO range for redistributors + * traps here. + * Note: We assume that the region are ordered. You could also check base+size vs gpa to avoid this limitation. + +/* + * Note: We are assuming that d-vcpu[vcpu_id] is never NULL. If + * it's the case, the guest will receive a data abort and won't be + * able to boot. Is cpu hotplug a factor here? Do we support guests booting with offline cpus yet? +/* + * Safety check mostly for DOM0. It's possible to have more vCPU + * than the number of physical CPU. Maybe we should deny this case? In general it's allowed, if a bit silly. Mainly for e.g. people working on PV spinlock code who want to force contention... Unlikely for dom0 I suppose. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 11/15] xen/arm: vgic-v2: Correctly handle RAZ/WI registers
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote:
Some of the registers are accessible via multiple size (see GICD_IPRIORITYR*).
They are byte accessible, but are they half word accessible? I suspect
not.
Thoses registers are misimplemented when they should be RAZ. Only
Same typoes as the v3 version.
word-access size are currently allowed for them.
To avoid further issues, introduce different label following the access-size
of the registers:
- read_as_zero_32 and write_ignore_32: Used for registers accessible
via a word.
- read_as_zero: Used when we don't have to check the access size.
The latter is used when the access size has already been checked in the
register emulation and/or when the register offset is
reserved/implementation defined.
Signed-off-by: Julien Grall julien.gr...@linaro.org
---
This patch should be backported to Xen 4.4 and 4.5 as it fixes
byte-access to GICD_ITARGET0.
Although, this patch won't apply directly to Xen 4.4.
Changes in v2:
- This patch replaces https://patches.linaro.org/43318/. A new
approach has been taken to explicitly use the size in the goto
label and have one version which don't check the access size. It's
useful for reserved registers and register we already check the access
size.
---
xen/arch/arm/vgic-v2.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 5faef12..6530ecc 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -74,7 +74,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v,
mmio_info_t *info)
case GICD_IGROUPR ... GICD_IGROUPRN:
/* We do not implement security extensions for guests, read zero */
-goto read_as_zero;
+goto read_as_zero_32;
case GICD_ISENABLER ... GICD_ISENABLERN:
if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -166,7 +166,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v,
mmio_info_t *info)
case GICD_NSACR ... GICD_NSACRN:
/* We do not implement security extensions for guests, read zero */
-goto read_as_zero;
+goto read_as_zero_32;
case GICD_SGIR:
if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -226,8 +226,9 @@ bad_width:
domain_crash_synchronous();
return 0;
-read_as_zero:
+read_as_zero_32:
if ( dabt.size != DABT_WORD ) goto bad_width;
+read_as_zero:
*r = 0;
return 1;
}
@@ -286,7 +287,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info)
/* R/O -- write ignored */
case GICD_TYPER:
case GICD_IIDR:
-goto write_ignore;
+goto write_ignore_32;
/* Implementation defined -- write ignored */
case 0x020 ... 0x03c:
@@ -294,7 +295,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info)
case GICD_IGROUPR ... GICD_IGROUPRN:
/* We do not implement security extensions for guests, write ignore
*/
-goto write_ignore;
+goto write_ignore_32;
case GICD_ISENABLER ... GICD_ISENABLERN:
if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -360,7 +361,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info)
case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
/* SGI/PPI target is read only */
-goto write_ignore;
+goto write_ignore_32;
case GICD_ITARGETSR + 8 ... GICD_ITARGETSRN:
{
@@ -433,10 +434,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info)
return 1;
case GICD_ICFGR: /* SGIs */
-goto write_ignore;
+goto write_ignore_32;
case GICD_ICFGR + 1: /* PPIs */
/* It is implementation defined if these are writeable. We chose not
*/
-goto write_ignore;
+goto write_ignore_32;
case GICD_ICFGR + 2 ... GICD_ICFGRN: /* SPIs */
if ( dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
@@ -448,7 +449,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info)
case GICD_NSACR ... GICD_NSACRN:
/* We do not implement security extensions for guests, write ignore
*/
-goto write_ignore;
+goto write_ignore_32;
case GICD_SGIR:
if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -474,7 +475,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info)
/* R/O -- write ignore */
case GICD_ICPIDR2:
-goto write_ignore;
+goto write_ignore_32;
/* Implementation defined -- write ignored */
case 0xfec ... 0xffc:
@@ -503,8 +504,9 @@ bad_width:
domain_crash_synchronous();
return 0;
-write_ignore:
+write_ignore_32:
if ( dabt.size != DABT_WORD )
Re: [Xen-devel] Did you get my e-mail about DRBD with Xen 4.4 on Jessie ?
Tomasz, you don't have to subscribe, but I think you used the wrong email address. If you don't subscript your mail gets moderated and it may take a few hours for a moderator to pick it up. From your mail it says you used xen-us...@list.xen.org, whereas the right email address is xen-de...@lists.xenproject.org or xen-us...@lists.xenproject.org (lists.xen.org also works). But it appears you used list. instead of lists. ... see http://www.xenproject.org/help/mailing-list.html for more info Regards Lars On 2 Feb 2015, at 13:56, Olaf Hering o...@aepfle.de wrote: On Mon, Feb 02, tomasz.grzegorc...@szu.pl wrote: Please help me with my problem i stuck with this on few days. I think you have to subscribe to xen-users at http://lists.xen.org/cgi-bin/mailman/listinfo/xen-users Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 06/15] xen/arm: vgic-v3: Set stride during domain initialization
On 02/02/15 15:40, Ian Campbell wrote:
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote:
The stride may not be set if the hardware GIC is using the default
layout. It happens on the Foundation model.
On GICv3, the default stride is 2 * 64K. Therefore it's possible to avoid
checking at every redistributor MMIO access if the stride is not set.
Can this defaulting not be pulled further to the initialisation of
gicv3.rdist_stride?
With the upcoming GICv4, the stride may be different for each
distributor (see the check on GICR_TYPER.VLPIS in gicv3_populate_rdist).
So I'd like to avoid the check of rdist_stride.
This is only happening for DOM0,
Please say instead Because domU uses a static stride configuration this
only happens for dom0... or similar (i.e. include the reason why domU
is excluded)
I will do.
so we can move this code in
gicv_v3_init. Take the opportunity to move the stride setting a bit ealier
earlier.
because the loop to set regions will require the stride.
Also, use 2 * 64K rather than 128K and explain the reason.
Signed-off-by: Julien Grall julien.gr...@linaro.org
---
I wasn't not sure where to move this code. I find very confusion the
splitting between vgic and gicv. Maybe we should introduce a
hwdom_gicv_init and giccc_map callbacks. Then move most of the
initialization in the vgic one.
Changes in v2:
- Patch added
---
xen/arch/arm/gic-v3.c | 11 ++-
xen/arch/arm/vgic-v3.c | 6 +-
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 47452ca..7b33ff7 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -897,12 +897,21 @@ static int gicv_v3_init(struct domain *d)
{
d-arch.vgic.dbase = gicv3.dbase;
d-arch.vgic.dbase_size = gicv3.dbase_size;
+
+d-arch.vgic.rdist_stride = gicv3.rdist_stride;
+/*
+ * If the stride is not set, the default stride for GICv3 is 2 *
64K:
+ * - first 64k page for Control and Physical LPIs
+ * - second 64k page for Control and Generation of SGIs
+ */
+if ( !d-arch.vgic.rdist_stride )
+d-arch.vgic.rdist_stride = 2 * SZ_64K;
+
for ( i = 0; i gicv3.rdist_count; i++ )
{
d-arch.vgic.rbase[i] = gicv3.rdist_regions[i].base;
d-arch.vgic.rbase_size[i] = gicv3.rdist_regions[i].size;
}
-d-arch.vgic.rdist_stride = gicv3.rdist_stride;
d-arch.vgic.rdist_count = gicv3.rdist_count;
}
else
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 2c14717..db6b514 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -625,11 +625,7 @@ static int vgic_v3_rdistr_mmio_read(struct vcpu *v,
mmio_info_t *info)
Why not the write case too?
By mistake it has been dropped in a following patch (Emulate correctly
the re-distributor). I will move the changes here.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC] libxl: set disk defaults in remove/destroy functions
Wei Liu writes (Re: [Xen-devel] [PATCH RFC] libxl: set disk defaults in remove/destroy functions): FWIW xl block-detach calls libxl_vdev_to_device_disk to convert a vdev to disk. That function reads xenstore to get the actual backend of that specific vdev. Don't know how useful it is to libvirt though. Maybe we should look up disk's backend in libxl_device_disk_remove? Not sure what's the best approach. libxl_device_disk_remove should definitely not require the caller to do what xl currently does. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 15/15] xen/arm: gic-v3: Update some comments in the code
Hi Ian, On 02/02/15 16:05, Ian Campbell wrote: On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: - Drop wrong comment about the default stride. It's not always 2 * SZ_64K What other defaults are possible and under what circumstances? 128K, when the re-distributor supports VLPIs. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1 net] xen-netback: stop the guest rx thread after a fatal error
On Mon, Feb 02, 2015 at 04:57:51PM +, David Vrabel wrote:
After commit e9d8b2c2968499c1f96563e6522c56958d5a1d0d (xen-netback:
disable rogue vif in kthread context), a fatal (protocol) error would
leave the guest Rx thread spinning, wasting CPU time. Commit
ecf08d2dbb96d5a4b4bcc53a39e8d29cc8fef02e (xen-netback: reintroduce
guest Rx stall detection) made this even worse by removing a
cond_resched() from this path.
Since a fatal error is non-recoverable, just allow the guest Rx thread
to exit. This requires taking additional refs to the task so the
thread exiting early is handled safely.
Signed-off-by: David Vrabel david.vra...@citrix.com
Reported-by: Julien Grall julien.gr...@linaro.org
Tested-by: Julien Grall julien.gr...@linaro.org
Acked-by: Wei Liu wei.l...@citrix.com
---
drivers/net/xen-netback/interface.c |2 ++
drivers/net/xen-netback/netback.c |3 +--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/xen-netback/interface.c
b/drivers/net/xen-netback/interface.c
index 9259a73..037f74f 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -578,6 +578,7 @@ int xenvif_connect(struct xenvif_queue *queue, unsigned
long tx_ring_ref,
goto err_rx_unbind;
}
queue-task = task;
+ get_task_struct(task);
task = kthread_create(xenvif_dealloc_kthread,
(void *)queue, %s-dealloc, queue-name);
@@ -634,6 +635,7 @@ void xenvif_disconnect(struct xenvif *vif)
if (queue-task) {
kthread_stop(queue-task);
+ put_task_struct(queue-task);
queue-task = NULL;
}
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 908e65e..c8ce701 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -2109,8 +2109,7 @@ int xenvif_kthread_guest_rx(void *data)
*/
if (unlikely(vif-disabled queue-id == 0)) {
xenvif_carrier_off(vif);
- xenvif_rx_queue_purge(queue);
- continue;
+ break;
}
if (!skb_queue_empty(queue-rx_queue))
--
1.7.10.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/15] xen/arm: vgic-v3: Correctly handle RAZ/WI registers
On Mon, 2015-02-02 at 15:59 +, Julien Grall wrote: Hi Ian, On 02/02/15 15:24, Ian Campbell wrote: On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: Some of the registers are accessible via multiple size (see GICD_IPRIORITYR*). Thoses registers are misimplemented when they should be RAZ. Only Those and incorrectly implemented. word-access size are currently allowed for them. To avoid further issues, introduce different label following the access-size of the registers: - read_as_zero_64 and write_ignore_64: Used for registers accessible via a double-word. - read_as_zero_32 and write_ignore_32: Used for registers accessible via a word. 5.1.3 suggests there are at least some 64-bit registers where it ought to be possible to read the upper and lower halves independently. Don't you need to support that? Only when the system is supporting AArch32. If the system only supports AArch64, 64-bit registers can only be read via a 64-bit access. I don't think we actually support AArch32 on the vGICv3 drivers. And we don't emulate 32-bits access on 64-bit registers. It's certainly our intention in general to support arm32 guest kernels on arm64, the v3 vgic may not reach that aspiration though. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC] libxl: set disk defaults in remove/destroy functions
On Mon, Feb 02, 2015 at 01:27:13PM +, Ian Campbell wrote: On Mon, 2015-01-26 at 16:14 -0700, Jim Fehlig wrote: Cc-ing the other toolstack maintainers, both of whom have more familiarity with this part of libxl than I. The attached patch is a hack I cooked up to fix one of the libvirt-TCK Xen failures. The test (200-disk-hotplug.t) attempts to hot add and remove a disk from a running domain. The add works fine, but remove fails with libxl: debug: libxl.c:3858:libxl_device_disk_remove: ao 0x7f9b9c0015a0: create: how=(nil) callback=(nil) poller= 0x7f9ba0004590 libxl: error: libxl.c:2399:libxl__device_from_disk: unrecognized disk backend type: 0 The test does not define a backend type, in which case the libvirt libxl driver allows libxl to choose an appropriate backend via libxl__device_disk_set_backend(). The backend type is never set on a remove operation, hence it fails with the above error. I spent some time trying to figure out the best place to set backend type on remove, but in the end could only come up with this hack. It wouldn't feel so gross if I could have simply added 'libxl__device_##type##_setdefault(gc, type);' to the existing DEFINE_DEVICE_REMOVE macro, but alas libxl__device_nic_setdefault() has a different prototype than the other devices. Better suggestions welcomed! One I considered was fixing this in libvirt. But the Xen community suggested allowing libxl to choose a suitable backend when not specified, so I think this recommendation should be symmetrical in the add and remove operations. FWIW xl block-detach calls libxl_vdev_to_device_disk to convert a vdev to disk. That function reads xenstore to get the actual backend of that specific vdev. Don't know how useful it is to libvirt though. Maybe we should look up disk's backend in libxl_device_disk_remove? Not sure what's the best approach. Wei. Regards, Jim ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 11/15] xen/arm: vgic-v2: Correctly handle RAZ/WI registers
On Mon, 2015-02-02 at 16:36 +, Julien Grall wrote: Hi Ian, On 02/02/15 16:02, Ian Campbell wrote: On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: Some of the registers are accessible via multiple size (see GICD_IPRIORITYR*). They are byte accessible, but are they half word accessible? I suspect not. Only byte accessible. I think we might need a read_as_zero_8_32 then, i.e. explicitly list the sizes which are allowed, and perhaps omit the un-suffixed version for so it's clear exactly what is what, although that might be more churn than you want to have here. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] PV USB
Hi All! What is the current state of PV USB drivers and where can I get actual info about it? I already have read this wiki page http://wiki.xen.org/wiki/Xen_USB_Passthrough but instead of clearness I have got a bit confusion. Could anybody clarify this theme for me? With best regards, -- *Vitaly Chernooky | Senior Developer - Product Engineering and Development* GlobalLogic P *+380.44.4929695 ext.1136* M *+380.98.7920568* S cvv_2k www.globallogic.com http://www.globallogic.com/email_disclaimer.txt ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 09/15] xen/arm: vgic-v3: Clarify which distributor is used in the common emulation
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: The messages in the common emulation doesn't specify which distributor (redistributor or distributor) is used. This make difficult to find the correct registers. Signed-off-by: Julien Grall julien.gr...@linaro.org Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] WebCAM via PV-USB
Hi All! Is there known any USB webcam to be working via PV USB? With best regards, -- *Vitaly Chernooky | Senior Developer - Product Engineering and Development* GlobalLogic P *+380.44.4929695 ext.1136* M *+380.98.7920568* S cvv_2k www.globallogic.com http://www.globallogic.com/email_disclaimer.txt ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 13/15] xen/arm: vgic-v2: GICD_I{S, C}PENDR* are only word-accessible
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: Signed-off-by: Julien Grall julien.gr...@linaro.org Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 14/15] xen/arm: vgic: Drop iactive, ipend, pendsgi field
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: The current VGIC code doesn't support to change the pending and active status of an IRQ via the (re-)distributor. If we plan to support it in the future, it will unlikely require a specific bitfield as we already store the status per vIRQ. I think we would want to track the state like we do for enabled, since walking 32 virqs to construct the value would be a bit expensive. Stefano, what do you think? Rather than wasting memory for nothing, drop thoses field. Any read to those these registers will be RAZ and any write will print an error and inject a data abort to the guest. Signed-off-by: Julien Grall julien.gr...@linaro.org --- Changes in v2: - Patch added --- xen/arch/arm/vgic-v2.c | 71 + xen/arch/arm/vgic-v3.c | 125 +++-- xen/include/asm-arm/vgic.h | 2 +- 3 files changed, 55 insertions(+), 143 deletions(-) diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c index 1a02541..3cf67a9 100644 --- a/xen/arch/arm/vgic-v2.c +++ b/xen/arch/arm/vgic-v2.c @@ -94,41 +94,15 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info) vgic_unlock_rank(v, rank, flags); return 1; +/* Read the pending status of an IRQ via GICD is not supported */ case GICD_ISPENDR ... GICD_ISPENDRN: -if ( dabt.size != DABT_WORD ) goto bad_width; -rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISPENDR, DABT_WORD); -if ( rank == NULL) goto read_as_zero; -vgic_lock_rank(v, rank, flags); -*r = vgic_byte_read(rank-ipend, dabt.sign, gicd_reg); -vgic_unlock_rank(v, rank, flags); -return 1; - case GICD_ICPENDR ... GICD_ICPENDRN: -if ( dabt.size != DABT_WORD ) goto bad_width; -rank = vgic_rank_offset(v, 0, gicd_reg - GICD_ICPENDR, DABT_WORD); -if ( rank == NULL) goto read_as_zero; -vgic_lock_rank(v, rank, flags); -*r = vgic_byte_read(rank-ipend, dabt.sign, gicd_reg); -vgic_unlock_rank(v, rank, flags); -return 1; +goto read_as_zero; +/* Read the active status of an IRQ via GICD is not supported */ case GICD_ISACTIVER ... GICD_ISACTIVERN: -if ( dabt.size != DABT_WORD ) goto bad_width; -rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD); -if ( rank == NULL) goto read_as_zero; -vgic_lock_rank(v, rank, flags); -*r = rank-iactive; -vgic_unlock_rank(v, rank, flags); -return 1; - case GICD_ICACTIVER ... GICD_ICACTIVERN: -if ( dabt.size != DABT_WORD ) goto bad_width; -rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD); -if ( rank == NULL) goto read_as_zero; -vgic_lock_rank(v, rank, flags); -*r = rank-iactive; -vgic_unlock_rank(v, rank, flags); -return 1; +goto read_as_zero; case GICD_ITARGETSR ... GICD_ITARGETSRN: if ( dabt.size != DABT_BYTE dabt.size != DABT_WORD ) goto bad_width; @@ -174,23 +148,10 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info) *r = 0xdeadbeef; return 1; +/* Setting/Clearing the SGI pending bit via GICD is not supported */ case GICD_CPENDSGIR ... GICD_CPENDSGIRN: -if ( dabt.size != DABT_BYTE dabt.size != DABT_WORD ) goto bad_width; -rank = vgic_rank_offset(v, 1, gicd_reg - GICD_CPENDSGIR, DABT_WORD); -if ( rank == NULL) goto read_as_zero; -vgic_lock_rank(v, rank, flags); -*r = vgic_byte_read(rank-pendsgi, dabt.sign, gicd_reg); -vgic_unlock_rank(v, rank, flags); -return 1; - case GICD_SPENDSGIR ... GICD_SPENDSGIRN: -if ( dabt.size != DABT_BYTE dabt.size != DABT_WORD ) goto bad_width; -rank = vgic_rank_offset(v, 1, gicd_reg - GICD_SPENDSGIR, DABT_WORD); -if ( rank == NULL) goto read_as_zero; -vgic_lock_rank(v, rank, flags); -*r = vgic_byte_read(rank-pendsgi, dabt.sign, gicd_reg); -vgic_unlock_rank(v, rank, flags); -return 1; +goto read_as_zero; /* Implementation defined -- read as zero */ case 0xfd0 ... 0xfe4: @@ -346,21 +307,17 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info) case GICD_ISACTIVER ... GICD_ISACTIVERN: if ( dabt.size != DABT_WORD ) goto bad_width; -rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD); -if ( rank == NULL) goto write_ignore; -vgic_lock_rank(v, rank, flags); -rank-iactive = ~*r; -vgic_unlock_rank(v, rank, flags); -return 1; +printk(XENLOG_G_ERR + %pv: vGICD: unhandled word write %#PRIregister to ISACTIVER%d\n, + v, *r, gicd_reg - GICD_ISACTIVER); +
Re: [Xen-devel] [PATCH v2 15/15] xen/arm: gic-v3: Update some comments in the code
On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote:
- Drop wrong comment about the default stride. It's not always 2 * SZ_64K
What other defaults are possible and under what circumstances?
- Explain why SZ_64K * 2
Signed-off-by: Julien Grall julien.gr...@linaro.org
---
Changes in v2:
- Patch added
---
xen/arch/arm/gic-v3.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 1b7ddb3..9035e3b 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -638,7 +638,7 @@ static int __init gicv3_populate_rdist(void)
ptr += gicv3.rdist_stride;
else
{
-ptr += SZ_64K * 2;
+ptr += SZ_64K * 2; /* Skip RD_base + SGI_base */
if ( typer GICR_TYPER_VLPIS )
ptr += SZ_64K * 2; /* Skip VLPI_base + reserved page */
}
@@ -1238,7 +1238,6 @@ static int __init gicv3_init(struct dt_device_node
*node, const void *data)
rdist_regs[i].size = rdist_size;
}
-/* If stride is not set in dt. Set default to 2 * SZ_64K */
if ( !dt_property_read_u32(node, redistributor-stride,
gicv3.rdist_stride) )
gicv3.rdist_stride = 0;
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/15] xen/arm: vgic-v3: Correctly handle RAZ/WI registers
On 02/02/15 16:08, Ian Campbell wrote: On Mon, 2015-02-02 at 15:59 +, Julien Grall wrote: Hi Ian, On 02/02/15 15:24, Ian Campbell wrote: On Thu, 2015-01-29 at 18:25 +, Julien Grall wrote: Some of the registers are accessible via multiple size (see GICD_IPRIORITYR*). Thoses registers are misimplemented when they should be RAZ. Only Those and incorrectly implemented. word-access size are currently allowed for them. To avoid further issues, introduce different label following the access-size of the registers: - read_as_zero_64 and write_ignore_64: Used for registers accessible via a double-word. - read_as_zero_32 and write_ignore_32: Used for registers accessible via a word. 5.1.3 suggests there are at least some 64-bit registers where it ought to be possible to read the upper and lower halves independently. Don't you need to support that? Only when the system is supporting AArch32. If the system only supports AArch64, 64-bit registers can only be read via a 64-bit access. I don't think we actually support AArch32 on the vGICv3 drivers. And we don't emulate 32-bits access on 64-bit registers. It's certainly our intention in general to support arm32 guest kernels on arm64, the v3 vgic may not reach that aspiration though. AFAICT, only the vGIC v3 is using 64-bit access. So we are fine for now. Linux seems to allow to build GICv3 for ARM32. I guess we should support it in the future. It would be useful for booting 32 bit guest on GICv3 only platform, assuming a such platform will exists. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
