[Xen-devel] [ovmf test] 58963: tolerable FAIL - PUSHED
flight 58963 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/58963/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 58919 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 58919 version targeted for testing: ovmf 79d274b8b6b113248661c18f31c4be03c7da32de baseline version: ovmf 495ee9b85141dd9b65434d677b3a685fe166128d People who touched revisions under test: Laszlo Ersek ler...@redhat.com Maoming maoming.maom...@huawei.com Wei Liu wei.l...@citrix.com jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass test-amd64-amd64-xl-qemuu-winxpsp3 pass test-amd64-i386-xl-qemuu-winxpsp3pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=ovmf + revision=79d274b8b6b113248661c18f31c4be03c7da32de + . cri-lock-repos ++ . cri-common +++ . cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{Repos} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 79d274b8b6b113248661c18f31c4be03c7da32de + branch=ovmf + revision=79d274b8b6b113248661c18f31c4be03c7da32de + . cri-lock-repos ++ . cri-common +++ . cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{Repos} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . cri-common ++ . cri-getconfig ++ umask 002 + select_xenbranch + case $branch in + tree=ovmf + xenbranch=xen-unstable + '[' xovmf = xlinux ']' + linuxbranch= + '[' x = x ']' + qemuubranch=qemu-upstream-unstable + : tested/2.6.39.x + . ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{OsstestUpstream} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/staging/qemu-xen-unstable.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git +++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local
Re: [Xen-devel] [RFC PATCH v3 07/18] xen/arm: ITS: implement hw_irq_controller for LPIs
On 29/06/15 12:53, Ian Campbell wrote: In this case, I would prefer to see 2 callbacks (one for the host the other for the guest) which return the correct IRQ controller for a specific IRQ. I have in mind something like: get_guest_hw_irq_controller(unsigned int irq) { if ( !is_lpi ) return gicv3_guest_irq_controller else return gicv3_guest_lpi_controller } Same for the host irq controller. So the selection of the IRQ controller would be hidden from gic.c and keep the code a generic as possible. Yes, this is how I would expect it too. Alternatively I notice that the pattern today is: desc-handler = gic_hw_ops-gic_(host|guest)_irq_type; [set_bit(_IRQ_GUEST, desc-status) or not] gic_set_irq_properties(desc,[...]); So an alternative might be for the set_irq_properties hook in the ops to also setup the handler (based on desc-status_IRQ_GUEST and desc-irq), perhaps renaming it to something less property based. Both callers are git_route_irq_to_... so perhaps gic_route_irq? Sounds good for me. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/vm_event: toggle singlestep from vm_event response
Add an option to the vm_event response to toggle singlestepping on the vCPU. Singed-off-by: Tamas K Lengyel tleng...@novetta.com --- MAINTAINERS| 1 + xen/arch/x86/Makefile | 1 + xen/arch/x86/hvm/hvm.c | 8 xen/arch/x86/vm_event.c| 41 + xen/common/vm_event.c | 8 +++- xen/include/asm-arm/vm_event.h | 29 + xen/include/asm-x86/hvm/hvm.h | 3 +++ xen/include/asm-x86/vm_event.h | 25 + xen/include/public/vm_event.h | 11 --- 9 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 xen/arch/x86/vm_event.c create mode 100644 xen/include/asm-arm/vm_event.h create mode 100644 xen/include/asm-x86/vm_event.h diff --git a/MAINTAINERS b/MAINTAINERS index 6b1068e..59c0822 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -383,6 +383,7 @@ F: xen/common/vm_event.c F: xen/common/mem_access.c F: xen/arch/x86/hvm/event.c F: xen/arch/x86/monitor.c +F: xen/arch/x86/vm_event.c XENTRACE M: George Dunlap george.dun...@eu.citrix.com diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 37e547c..5f24951 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -60,6 +60,7 @@ obj-y += machine_kexec.o obj-y += crash.o obj-y += tboot.o obj-y += hpet.o +obj-y += vm_event.o obj-y += xstate.o obj-$(crash_debug) += gdbstub.o diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 535d622..2bfd1b0 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -6431,6 +6431,14 @@ int hvm_debug_op(struct vcpu *v, int32_t op) return rc; } +void hvm_toggle_singlestep(struct vcpu *v) +{ +if ( !cpu_has_monitor_trap_flag ) +return; + +v-arch.hvm_vcpu.single_step = !v-arch.hvm_vcpu.single_step; +} + int nhvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs) { if (hvm_funcs.nhvm_vcpu_hostrestore) diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c new file mode 100644 index 000..95b30ad --- /dev/null +++ b/xen/arch/x86/vm_event.c @@ -0,0 +1,41 @@ +/* + * arch/x86/vm_event.c + * + * Architecture-specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#include xen/sched.h +#include asm/hvm/hvm.h + +void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) +{ +if ( (v == current) || !is_hvm_domain(d) ) +return; + +hvm_toggle_singlestep(v); +} + +/* + * Local variables: + * mode: C + * c-file-style: BSD + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c index 120a78a..2ee27e2 100644 --- a/xen/common/vm_event.c +++ b/xen/common/vm_event.c @@ -27,6 +27,7 @@ #include xen/vm_event.h #include xen/mem_access.h #include asm/p2m.h +#include asm/vm_event.h #include xsm/xsm.h /* for public/io/ring.h macros */ @@ -399,9 +400,14 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved) }; -/* Unpause domain. */ if ( rsp.flags VM_EVENT_FLAG_VCPU_PAUSED ) +{ +if ( rsp.flags VM_EVENT_FLAG_TOGGLE_SINGLESTEP ) +vm_event_toggle_singlestep(d, v); + +/* Unpause domain. */ vm_event_vcpu_unpause(v); +} } } diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h new file mode 100644 index 000..a4cf4c6 --- /dev/null +++ b/xen/include/asm-arm/vm_event.h @@ -0,0 +1,29 @@ +/* + * vm_event.h: architecture specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
Re: [Xen-devel] [RFC PATCH v3 17/18] xen/arm: ITS: Generate ITS node for Dom0
On Mon, 2015-06-22 at 17:31 +0530, vijay.kil...@gmail.com wrote: From: Vijaya Kumar K vijaya.ku...@caviumnetworks.com Parse host dt and generate ITS node for Dom0. ITS node resides inside GIC node so when GIC node is encountered look for ITS node. Signed-off-by: Vijaya Kumar K vijaya.ku...@caviumnetworks.com --- xen/arch/arm/domain_build.c | 50 +++- xen/arch/arm/gic-v3-its.c | 57 + xen/include/asm-arm/gic-its.h |2 ++ 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index e9cb8a9..0de5a8b 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -20,6 +20,7 @@ #include asm/cpufeature.h #include asm/gic.h +#include asm/gic-its.h #include xen/irq.h #include kernel.h @@ -803,6 +804,34 @@ static int make_cpus_node(const struct domain *d, void *fdt, return res; } +static int make_its_node(const struct domain *d, void *fdt, + const struct dt_device_node *node) +{ +int res = 0; + +DPRINT(Create GIC ITS node\n); + +res = its_make_dt_node(d, node, fdt); +if ( res ) +return res; + +/* + * The value of the property phandle in the property interrupts + * to know on which interrupt controller the interrupt is wired. + */ +if ( node-phandle ) +{ +DPRINT( Set phandle = 0x%x\n, node-phandle); +res = fdt_property_cell(fdt, phandle, node-phandle); +if ( res ) +return res; +} + +res = fdt_end_node(fdt); + +return res; +} + static int make_gic_node(const struct domain *d, void *fdt, const struct dt_device_node *node) { @@ -1119,7 +1148,13 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, DT_MATCH_TIMER, { /* sentinel */ }, }; +static const struct dt_device_match gits_matches[] __initconst = +{ +DT_MATCH_GIC_ITS, +{ /* sentinel */ }, +}; struct dt_device_node *child; +struct dt_device_node *gic_child; int res; const char *name; const char *path; @@ -1143,7 +1178,20 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, /* Replace these nodes with our own. Note that the original may be * used_by DOMID_XEN so this check comes first. */ if ( device_get_class(node) == DEVICE_GIC ) -return make_gic_node(d, kinfo-fdt, node); +{ +if ( !make_gic_node(d, kinfo-fdt, node) ) +{ +dt_for_each_child_node(node, gic_child) +{ +if ( gic_child != NULL ) +{ +if ( dt_match_node(gits_matches, gic_child) ) +return make_its_node(d, kinfo-fdt, gic_child); This will create multiple ITS nodes, we only want one and we need the msi-parent properties everywhere else to be changed to point to it. +} +} +} +return 0; +} if ( dt_match_node(timer_matches, node) ) return make_timer_node(d, kinfo-fdt, node); diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 8aa1ec5..fc853d4 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -27,6 +27,8 @@ #include xen/sched.h #include xen/errno.h #include xen/delay.h +#include xen/device_tree.h +#include xen/libfdt/libfdt.h #include xen/list.h #include xen/sizes.h #include xen/vmap.h @@ -1205,6 +1207,61 @@ static void its_cpu_init_collection(void) spin_unlock(its_lock); } +int its_make_dt_node(const struct domain *d, + const struct dt_device_node *node, void *fdt) +{ +struct its_node *its; +const struct dt_device_node *gic; +const void *compatible = NULL; +u32 len; +__be32 *new_cells, *tmp; +int res = 0; + +/* Will pass only first ITS node info */ +/* TODO: Handle multi node */ +its = list_first_entry(its_nodes, struct its_node, entry); I think this should be done at the top level by not walking all children and by blacklisting all ITS nodes to be replaced by our own. It's also not clear why list_first_entry should be NULL for all but the first, but nevermind. +if ( !its ) +{ +dprintk(XENLOG_ERR, ITS node not found\n); +return -FDT_ERR_XEN(ENOENT); +} + +gic = its-dt_node; + +compatible = dt_get_property(gic, compatible, len); +if ( !compatible ) +{ +dprintk(XENLOG_ERR, Can't find compatible property for the its node\n); +return -FDT_ERR_XEN(ENOENT); +} + +res = fdt_begin_node(fdt, gic-its); +if ( res ) +return res; + +res = fdt_property(fdt, compatible, compatible, len); +if ( res ) +return res; + +
Re: [Xen-devel] [Xen-users] xen physical address(paddr)and machine address (maddr)
On Mon, 2015-06-29 at 18:35 +0800, xinyue wrote: Please don't top post. Please have another read of hte Etiquette section of http://wiki.xen.org/wiki/Asking_Xen_Devel_Questions . Thanks very much,I think it‘s very helpful and I'll try it. Does for an arbitrary guest page it is unlikely to be from the xenheap means that some guest pages can't be mapped to xen heap page? Any given page is either a domheap page or a xenheap page, so there is no mapping in the sense you seem to be thinking of. And I wonder in HVM doms with EPT supported, is there any difference if I won't monitor guest domains memory regions? I'm afraid I don't understand the question. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vm_event: toggle singlestep from vm_event response
On Mon, Jun 29, 2015 at 9:35 AM, Andrew Cooper andrew.coop...@citrix.com wrote: On 29/06/15 13:58, Tamas K Lengyel wrote: Add an option to the vm_event response to toggle singlestepping on the vCPU. Singed-off-by: Tamas K Lengyel tleng...@novetta.com --- MAINTAINERS| 1 + xen/arch/x86/Makefile | 1 + xen/arch/x86/hvm/hvm.c | 8 xen/arch/x86/vm_event.c| 41 + xen/common/vm_event.c | 8 +++- xen/include/asm-arm/vm_event.h | 29 + xen/include/asm-x86/hvm/hvm.h | 3 +++ xen/include/asm-x86/vm_event.h | 25 + xen/include/public/vm_event.h | 11 --- 9 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 xen/arch/x86/vm_event.c create mode 100644 xen/include/asm-arm/vm_event.h create mode 100644 xen/include/asm-x86/vm_event.h diff --git a/MAINTAINERS b/MAINTAINERS index 6b1068e..59c0822 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -383,6 +383,7 @@ F:xen/common/vm_event.c F: xen/common/mem_access.c F: xen/arch/x86/hvm/event.c F: xen/arch/x86/monitor.c +F: xen/arch/x86/vm_event.c XENTRACE M: George Dunlap george.dun...@eu.citrix.com diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 37e547c..5f24951 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -60,6 +60,7 @@ obj-y += machine_kexec.o obj-y += crash.o obj-y += tboot.o obj-y += hpet.o +obj-y += vm_event.o obj-y += xstate.o obj-$(crash_debug) += gdbstub.o diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 535d622..2bfd1b0 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -6431,6 +6431,14 @@ int hvm_debug_op(struct vcpu *v, int32_t op) return rc; } +void hvm_toggle_singlestep(struct vcpu *v) +{ +if ( !cpu_has_monitor_trap_flag ) monitor_trap_flag is a VMX feature. This will never be true on AMD systems. (its use in hvm_debug_op() is also dubious) Yes, this feature is only for Intel cpus. Reworking the use of the flag though is a bit out-of-scope for this patch itself. +return; + +v-arch.hvm_vcpu.single_step = !v-arch.hvm_vcpu.single_step; +} + int nhvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs) { if (hvm_funcs.nhvm_vcpu_hostrestore) diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c new file mode 100644 index 000..95b30ad --- /dev/null +++ b/xen/arch/x86/vm_event.c @@ -0,0 +1,41 @@ +/* + * arch/x86/vm_event.c + * + * Architecture-specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#include xen/sched.h +#include asm/hvm/hvm.h + +void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) +{ +if ( (v == current) || !is_hvm_domain(d) ) Why is 'current' excluded? Only to be consistent with the sanity check applied for XEN_DOMCTL_debug_op. +return; + +hvm_toggle_singlestep(v); +} + +/* + * Local variables: + * mode: C + * c-file-style: BSD + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c index 120a78a..2ee27e2 100644 --- a/xen/common/vm_event.c +++ b/xen/common/vm_event.c @@ -27,6 +27,7 @@ #include xen/vm_event.h #include xen/mem_access.h #include asm/p2m.h +#include asm/vm_event.h #include xsm/xsm.h /* for public/io/ring.h macros */ @@ -399,9 +400,14 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved) }; -/* Unpause domain. */ if ( rsp.flags VM_EVENT_FLAG_VCPU_PAUSED ) +{ +if ( rsp.flags VM_EVENT_FLAG_TOGGLE_SINGLESTEP ) +vm_event_toggle_singlestep(d, v); + +/* Unpause domain. */ I don't think this comment is useful to keep. Yea, agree. vm_event_vcpu_unpause(v); +} } } diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h new file mode 100644 index 000..a4cf4c6 ---
Re: [Xen-devel] pvUSB backend performance
On 06/29/2015 03:46 PM, David Vrabel wrote: On 24/06/15 13:06, Juergen Gross wrote: Hi, my qemu integrated pvUSB backend is now running stable enough to do some basic performance measurements. I've passed a memory-stick with about 90MB of data on it to a pv-domU. Then I read all the data on it with tar and looked how long this would take (elapsed time): in dom0: 5.2s in domU with kernel backend: 6.1s in domU with qemu backend: 8.2s So the qemu backend is about 30% slower than the kernel backend. Is this acceptable? Yes? Maybe? I assume you're adding this backend for a reason, so is it good enough for your workloads? We (SUSE) have a kernel based backend in SLE and openSUSE. I'm pretty sure it isn't used for really performance critical stuff right now. But having an upstream variant might change this, so I'd rather ask before adding the qemu variant and having to do the kernel solution afterwards as well. :-) IMO, It seems a reasonable trade off to not have a bunch of kernel code. Yeah, this would be my gut feeling, too. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d
-Original Message- From: Paul Durrant Sent: 29 June 2015 13:54 To: Paul Durrant; Don Slutz; xen-devel@lists.xen.org; Jan Beulich Subject: RE: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d -Original Message- From: xen-devel-boun...@lists.xen.org [mailto:xen-devel- boun...@lists.xen.org] On Behalf Of Paul Durrant Sent: 29 June 2015 10:42 To: Don Slutz; xen-devel@lists.xen.org; Jan Beulich Subject: Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d -Original Message- From: Don Slutz [mailto:don.sl...@gmail.com] Sent: 27 June 2015 22:02 To: xen-devel@lists.xen.org; Paul Durrant; Jan Beulich Subject: Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d commit 2df1aa01bef7366798248ac6d03cfb42048b003d Author: Paul Durrant paul.durr...@citrix.com Date: Tue Jun 23 18:07:49 2015 +0200 x86/hvm: remove hvm_io_pending() check in hvmemul_do_io() ... -rc = X86EMUL_RETRY; -if ( !hvm_send_assist_req(s, p) ) +rc = hvm_send_assist_req(s, p); +if ( rc != X86EMUL_RETRY ) ... if ( unlikely(!vcpu_start_shutdown_deferral(curr)) ) -return 0; /* implicitly bins the i/o operation */ +return X86EMUL_OKAY; So now X86EMUL_OKAY is returned from hvmemul_do_io() during shutdown. From Jan Beulich about this: Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry. https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=subject:%22Re%5C%3A+%5C%5BXen%5C- devel%5C%5D+%5C%5BPATCH+3%5C%2F5%5C%5D+hvmemul_do_io%5C%3 A+If+the+send+to+the+ioreq+server+failed+do+not+retry.%22o=newest Jan Beulich https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=from:%22Jan+Beulich%22 Fri, 30 Jan 2015 02:24:55 -0800 https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=date:20150130 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. 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. Jan going up the call stack: in handle_pio when hvmemul_do_pio_buffer() returns X86EMUL_OKAY, it returns 1. svm_vmexit_handler 2578 if ( handle_pio(port, bytes, dir) ) or vmx_vmexit_handler 3178 if ( handle_pio(port, bytes, dir) ) both update the IP in this case which is wrong during shutdown. If this is indeed the problem then it seems to me that the correct fix is to add a check in handle_pio() and return 0 if the cpu is shutting down. Actually, with some more thought, it sounds like we essentially want to unwind the I/O emulation if we're shutting down and I think this could be achieved by having hvm_send_assist_req() return X86EMUL_RETRY if it hits the shutdown deferral case, having hvm_emul_do_io() reset the emulation state back to 'none' if the domain is shutting down, and then having handle_pio() return 0 if sees X86EMUL_RETRY without any form of completion being requested. I think this patch should do it for now: diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index a4d7225..cc6130c 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -183,7 +183,7 @@ static int hvmemul_do_io( else { rc = hvm_send_assist_req(s, p); -if ( rc != X86EMUL_RETRY ) +if ( rc != X86EMUL_RETRY || curr-domain-is_shutting_down ) vio-io_state = HVMIO_none; else if ( data_is_addr || dir == IOREQ_WRITE ) rc = X86EMUL_OKAY; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 8226d8c..aba4ef6 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2547,7 +2547,7 @@ int hvm_send_assist_req(struct hvm_ioreq_server *s, ioreq_t *proto_p) ASSERT(s); if ( unlikely(!vcpu_start_shutdown_deferral(curr)) ) -return X86EMUL_OKAY; +return X86EMUL_RETRY; list_for_each_entry ( sv, s-ioreq_vcpu_list, Paul Paul So I think: rc = hvm_send_assist_req(s, p); if ( rc != X86EMUL_RETRY ) vio-io_state = HVMIO_none; needs to change to: rc = hvm_send_assist_req(s, p); if ( rc != X86EMUL_RETRY )
Re: [Xen-devel] [PATCH OSSTEST v1] mg-all-branch-statuses: Show how up to date each branch is
Ian Campbell writes (Re: [PATCH OSSTEST v1] mg-all-branch-statuses: Show how up to date each branch is): On Mon, 2015-06-29 at 13:25 +0100, Ian Jackson wrote: It's probably fairly straightforward to search through the database or history to find some relevant date. What do want to know ? 1. `Commit' timestamp of last pass ? 2. `Commit' timestamp of first commit after last pass ? 3. `Commit' timestamp of tip ? 2 sounds tricky, especially since there could potentially be several. Yes. Although `earliest-dated nontrivial descendant of the last pass commit' is unique, it ... In general commit timestamps can be a bit misleading, since various things preserve them even over long periods of time before they are published and they don't necessarily correspond at all with when something became visible in the tree one is looking at. ... is likely to exacerbate this problem. (Although I did say `commit timestamp', which in git is a separate timestamp updated by rebase, cherry pick, etc., as opposed to `author timestamp'.) Maybe better than nothing though. 4. Start date of last flight for current baseline ? 5. Start date of first flight after last flight for current baseline ? 6. Start date of first flight for current tip ? 5 ? You mean out of all 6 options you think 5 is all we need? Yes. The date of the of the flight which established the current baseline seems like a no-brainer (I suppose that is #4). That doesn't tell us how out of date it is. If the tree is updated rarely but the tip was updated yesterday it might look like the test has been failing for months. I'm not really sure what date of a flight relating to tip would be most useful, first try of something newer (#5?) vs most recent attempt (#6). The former tells you how long it has been failing. Maybe even the _number_ of bites of the cherry would be useful? Yes. You'd want both `number of times _this version_ has been tried' and `total number of flights since last pass'. Perhaps I should make a library function in Osstest::Executive which provides this date and count. After all they would be useful in the email reports. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] x86/arm/mm: use gfn instead of pfn in p2m_get_mem_access/p2m_set_mem_access
On 26/06/15 16:17, Vitaly Kuznetsov wrote: 'pfn' and 'start_pfn' are ambiguous, both these functions expect GFNs as input. On x86 the interface of p2m_set_mem_access() in p2m.c doesn't match the declaration in p2m-common.h as 'pfn' is being used instead of 'start_pfn'. On ARM both p2m_set_mem_access and p2m_get_mem_access interfaces don't match declarations from p2m-common.h: p2m_set_mem_access uses 'pfn' instead of 'start_pfn' and p2m_get_mem_access uses 'gpfn' instead of 'pfn'. There is also an issue in p2m_get_mem_access on x86: 'gfn' parameter passed to gfn_lock/gfn_unlock is not defined. This code compiles only because of a coincidence: gfn_lock/gfn_unlock are currently macros which don't use their second argument. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com --- Changes since v2: - Instead of adding start_ prefix on ARM remove it on x86 [Jan Beulich, Ian Campbell, Razvan Cojocaru] Changes since v1: - This patch is a successor of '[PATCH] x86/mm: use existing 'pfn' in p2m_get_mem_access', instead of fixing gfn_lock/gfn_unlock arguments we do s/pfn/gfn/g for both p2m_get_mem_access/p2m_set_mem_access [Andrew Cooper, Jan Beulich] P.S. - The patch was compile-tested on x86 only. --- xen/arch/arm/p2m.c | 12 ++-- xen/arch/x86/mm/p2m.c| 24 xen/include/xen/p2m-common.h | 12 ++-- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 903fa3f..54c238c 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1709,9 +1709,9 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec) /* * Set access type for a region of pfns. - * If start_pfn == -1ul, sets the default access type. + * If gfn == -1ul, sets the default access type. */ -long p2m_set_mem_access(struct domain *d, unsigned long pfn, uint32_t nr, +long p2m_set_mem_access(struct domain *d, unsigned long gfn, uint32_t nr, uint32_t start, uint32_t mask, xenmem_access_t access) { struct p2m_domain *p2m = p2m_get_hostp2m(d); @@ -1752,14 +1752,14 @@ long p2m_set_mem_access(struct domain *d, unsigned long pfn, uint32_t nr, p2m-mem_access_enabled = true; /* If request to set default access. */ -if ( pfn == ~0ul ) +if ( gfn == ~0ul ) Please here and everywhere else in the patch, use INVALID_GFN instead of ~0 or -1. With those changes and the comment style from Razvan, Reviewed-by: Andrew Cooper andrew.coop...@citrix.com However, if you feel up to it, it would be fantastic if you could substitute unsigned long for gfn_t, per the justification in e758ed1 and 177bd5f. c/s 24036a5 is an example of a different API I fixed up in a similar way. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d
-Original Message- From: xen-devel-boun...@lists.xen.org [mailto:xen-devel- boun...@lists.xen.org] On Behalf Of Paul Durrant Sent: 29 June 2015 10:42 To: Don Slutz; xen-devel@lists.xen.org; Jan Beulich Subject: Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d -Original Message- From: Don Slutz [mailto:don.sl...@gmail.com] Sent: 27 June 2015 22:02 To: xen-devel@lists.xen.org; Paul Durrant; Jan Beulich Subject: Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d commit 2df1aa01bef7366798248ac6d03cfb42048b003d Author: Paul Durrant paul.durr...@citrix.com Date: Tue Jun 23 18:07:49 2015 +0200 x86/hvm: remove hvm_io_pending() check in hvmemul_do_io() ... -rc = X86EMUL_RETRY; -if ( !hvm_send_assist_req(s, p) ) +rc = hvm_send_assist_req(s, p); +if ( rc != X86EMUL_RETRY ) ... if ( unlikely(!vcpu_start_shutdown_deferral(curr)) ) -return 0; /* implicitly bins the i/o operation */ +return X86EMUL_OKAY; So now X86EMUL_OKAY is returned from hvmemul_do_io() during shutdown. From Jan Beulich about this: Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry. https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=subject:%22Re%5C%3A+%5C%5BXen%5C- devel%5C%5D+%5C%5BPATCH+3%5C%2F5%5C%5D+hvmemul_do_io%5C%3 A+If+the+send+to+the+ioreq+server+failed+do+not+retry.%22o=newest Jan Beulich https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=from:%22Jan+Beulich%22 Fri, 30 Jan 2015 02:24:55 -0800 https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=date:20150130 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. 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. Jan going up the call stack: in handle_pio when hvmemul_do_pio_buffer() returns X86EMUL_OKAY, it returns 1. svm_vmexit_handler 2578 if ( handle_pio(port, bytes, dir) ) or vmx_vmexit_handler 3178 if ( handle_pio(port, bytes, dir) ) both update the IP in this case which is wrong during shutdown. If this is indeed the problem then it seems to me that the correct fix is to add a check in handle_pio() and return 0 if the cpu is shutting down. Actually, with some more thought, it sounds like we essentially want to unwind the I/O emulation if we're shutting down and I think this could be achieved by having hvm_send_assist_req() return X86EMUL_RETRY if it hits the shutdown deferral case, having hvm_emul_do_io() reset the emulation state back to 'none' if the domain is shutting down, and then having handle_pio() return 0 if sees X86EMUL_RETRY without any form of completion being requested. Paul Paul So I think: rc = hvm_send_assist_req(s, p); if ( rc != X86EMUL_RETRY ) vio-io_state = HVMIO_none; needs to change to: rc = hvm_send_assist_req(s, p); if ( rc != X86EMUL_RETRY ) { vio-io_state = HVMIO_none; if ( rc == X86EMUL_OKAY ) rc = X86EMUL_RETRY; } -Don Slutz ___ 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 OSSTEST v1] mg-all-branch-statuses: Show how up to date each branch is
On Mon, 2015-06-29 at 13:25 +0100, Ian Jackson wrote: Ian Campbell writes ([PATCH OSSTEST v1] mg-all-branch-statuses: Show how up to date each branch is): The osstest branch doesn't expose pretest (at least not to the machine I tested this on), so it ends up ignored here. Some branches (e.g. linux-next) do not record a baseline, so the script allows for that. Since everything serialises on the repo lock I didn't bother trying to parallelise anything. ... Example output: libvirt d10a5f58c75e7eb5943b44cc36a1e768adb2cdb0 = f7d8aa44b06e5cfa06abea4a72ee26b13754667d This is lacking date information which is IMO critical. Right. I started off simply wondering which branches were up to date or not, not wondering why that might be so... It's probably fairly straightforward to search through the database or history to find some relevant date. What do want to know ? 1. `Commit' timestamp of last pass ? 2. `Commit' timestamp of first commit after last pass ? 3. `Commit' timestamp of tip ? 2 sounds tricky, especially since there could potentially be several. In general commit timestamps can be a bit misleading, since various things preserve them even over long periods of time before they are published and they don't necessarily correspond at all with when something became visible in the tree one is looking at. Maybe better than nothing though. 4. Start date of last flight for current baseline ? 5. Start date of first flight after last flight for current baseline ? 6. Start date of first flight for current tip ? 5 ? You mean out of all 6 options you think 5 is all we need? The date of the of the flight which established the current baseline seems like a no-brainer (I suppose that is #4). I'm not really sure what date of a flight relating to tip would be most useful, first try of something newer (#5?) vs most recent attempt (#6). Maybe even the _number_ of bites of the cherry would be useful? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] pvUSB backend performance
On Wed, Jun 24, 2015 at 1:06 PM, Juergen Gross jgr...@suse.com wrote: Hi, my qemu integrated pvUSB backend is now running stable enough to do some basic performance measurements. I've passed a memory-stick with about 90MB of data on it to a pv-domU. Then I read all the data on it with tar and looked how long this would take (elapsed time): in dom0: 5.2s in domU with kernel backend: 6.1s in domU with qemu backend: 8.2s So the qemu backend is about 30% slower than the kernel backend. Is this acceptable? Just to be clear, you mean having qemu act as a pvusb backend (a la qdisk), not emulated, is that correct? I don't actually understand your question -- is the overhead acceptable for what? I think in an ideal world the toolstack will use the kernel backend if it's available, and fall back to a qemu backend if it's not available. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/7] xen: get rid of the SEDF scheduler
On Mon, 2015-06-29 at 12:56 +0100, Andrew Cooper wrote: On 26/06/15 17:19, Dario Faggioli wrote: --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -324,7 +324,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); /* XEN_DOMCTL_scheduler_op */ /* Scheduler types. */ -#define XEN_SCHEDULER_SEDF 4 Please instead use: /* #define XEN_SCHEDULER_SEDF 4 (Removed) */ To help keep bits of the history, and reduce the likelyhood of accidental reuse. Ok, good point. I just killed it as I did not see any trace of 0-3, which I suspect were the other previously existing schedulers, but, yes, I'll do as you suggest. Thanks and 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] x86/vm_event: toggle singlestep from vm_event response
On 29/06/15 13:58, Tamas K Lengyel wrote: Add an option to the vm_event response to toggle singlestepping on the vCPU. Singed-off-by: Tamas K Lengyel tleng...@novetta.com --- MAINTAINERS| 1 + xen/arch/x86/Makefile | 1 + xen/arch/x86/hvm/hvm.c | 8 xen/arch/x86/vm_event.c| 41 + xen/common/vm_event.c | 8 +++- xen/include/asm-arm/vm_event.h | 29 + xen/include/asm-x86/hvm/hvm.h | 3 +++ xen/include/asm-x86/vm_event.h | 25 + xen/include/public/vm_event.h | 11 --- 9 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 xen/arch/x86/vm_event.c create mode 100644 xen/include/asm-arm/vm_event.h create mode 100644 xen/include/asm-x86/vm_event.h diff --git a/MAINTAINERS b/MAINTAINERS index 6b1068e..59c0822 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -383,6 +383,7 @@ F:xen/common/vm_event.c F: xen/common/mem_access.c F: xen/arch/x86/hvm/event.c F: xen/arch/x86/monitor.c +F: xen/arch/x86/vm_event.c XENTRACE M: George Dunlap george.dun...@eu.citrix.com diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 37e547c..5f24951 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -60,6 +60,7 @@ obj-y += machine_kexec.o obj-y += crash.o obj-y += tboot.o obj-y += hpet.o +obj-y += vm_event.o obj-y += xstate.o obj-$(crash_debug) += gdbstub.o diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 535d622..2bfd1b0 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -6431,6 +6431,14 @@ int hvm_debug_op(struct vcpu *v, int32_t op) return rc; } +void hvm_toggle_singlestep(struct vcpu *v) +{ +if ( !cpu_has_monitor_trap_flag ) monitor_trap_flag is a VMX feature. This will never be true on AMD systems. (its use in hvm_debug_op() is also dubious) +return; + +v-arch.hvm_vcpu.single_step = !v-arch.hvm_vcpu.single_step; +} + int nhvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs) { if (hvm_funcs.nhvm_vcpu_hostrestore) diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c new file mode 100644 index 000..95b30ad --- /dev/null +++ b/xen/arch/x86/vm_event.c @@ -0,0 +1,41 @@ +/* + * arch/x86/vm_event.c + * + * Architecture-specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#include xen/sched.h +#include asm/hvm/hvm.h + +void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) +{ +if ( (v == current) || !is_hvm_domain(d) ) Why is 'current' excluded? +return; + +hvm_toggle_singlestep(v); +} + +/* + * Local variables: + * mode: C + * c-file-style: BSD + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c index 120a78a..2ee27e2 100644 --- a/xen/common/vm_event.c +++ b/xen/common/vm_event.c @@ -27,6 +27,7 @@ #include xen/vm_event.h #include xen/mem_access.h #include asm/p2m.h +#include asm/vm_event.h #include xsm/xsm.h /* for public/io/ring.h macros */ @@ -399,9 +400,14 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved) }; -/* Unpause domain. */ if ( rsp.flags VM_EVENT_FLAG_VCPU_PAUSED ) +{ +if ( rsp.flags VM_EVENT_FLAG_TOGGLE_SINGLESTEP ) +vm_event_toggle_singlestep(d, v); + +/* Unpause domain. */ I don't think this comment is useful to keep. vm_event_vcpu_unpause(v); +} } } diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h new file mode 100644 index 000..a4cf4c6 --- /dev/null +++ b/xen/include/asm-arm/vm_event.h @@ -0,0 +1,29 @@ +/* + * vm_event.h: architecture specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is
Re: [Xen-devel] pvUSB backend performance
On 24/06/15 13:06, Juergen Gross wrote: Hi, my qemu integrated pvUSB backend is now running stable enough to do some basic performance measurements. I've passed a memory-stick with about 90MB of data on it to a pv-domU. Then I read all the data on it with tar and looked how long this would take (elapsed time): in dom0: 5.2s in domU with kernel backend: 6.1s in domU with qemu backend: 8.2s So the qemu backend is about 30% slower than the kernel backend. Is this acceptable? Yes? Maybe? I assume you're adding this backend for a reason, so is it good enough for your workloads? IMO, It seems a reasonable trade off to not have a bunch of kernel code. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vm_event: toggle singlestep from vm_event response
On 29/06/15 14:42, Lengyel, Tamas wrote: --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -6431,6 +6431,14 @@ int hvm_debug_op(struct vcpu *v, int32_t op) return rc; } +void hvm_toggle_singlestep(struct vcpu *v) +{ +if ( !cpu_has_monitor_trap_flag ) monitor_trap_flag is a VMX feature. This will never be true on AMD systems. (its use in hvm_debug_op() is also dubious) Yes, this feature is only for Intel cpus. Reworking the use of the flag though is a bit out-of-scope for this patch itself. In which case you must make it very clear in the commit message that this is for Intel only and needs fixing up for AMD. Best also to have a comment to the same effect in this function. +return; + +v-arch.hvm_vcpu.single_step = !v-arch.hvm_vcpu.single_step; +} + int nhvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs) { if (hvm_funcs.nhvm_vcpu_hostrestore) diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c new file mode 100644 index 000..95b30ad --- /dev/null +++ b/xen/arch/x86/vm_event.c @@ -0,0 +1,41 @@ +/* + * arch/x86/vm_event.c + * + * Architecture-specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com mailto:ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#include xen/sched.h +#include asm/hvm/hvm.h + +void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) +{ +if ( (v == current) || !is_hvm_domain(d) ) Why is 'current' excluded? Only to be consistent with the sanity check applied for XEN_DOMCTL_debug_op. XEN_DOMCTL_debug_op needs the check because of vcpu_pause(). It is always run in the context of the hypercaller domain, and must never end up calling singlestep on itself. However in this case, we can only legitimately use this on an already-paused vcpu, which guarentees that Xen is not currently in the context of the target vcpu. It would probably be better to have a check against the vcpu pause count here (with early return), and hvm_toggle_singlestep() assert so. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vm_event: toggle singlestep from vm_event response
On Mon, Jun 29, 2015 at 9:55 AM, Andrew Cooper andrew.coop...@citrix.com wrote: On 29/06/15 14:42, Lengyel, Tamas wrote: --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -6431,6 +6431,14 @@ int hvm_debug_op(struct vcpu *v, int32_t op) return rc; } +void hvm_toggle_singlestep(struct vcpu *v) +{ +if ( !cpu_has_monitor_trap_flag ) monitor_trap_flag is a VMX feature. This will never be true on AMD systems. (its use in hvm_debug_op() is also dubious) Yes, this feature is only for Intel cpus. Reworking the use of the flag though is a bit out-of-scope for this patch itself. In which case you must make it very clear in the commit message that this is for Intel only and needs fixing up for AMD. Best also to have a comment to the same effect in this function. Sure. +return; + +v-arch.hvm_vcpu.single_step = !v-arch.hvm_vcpu.single_step; +} + int nhvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs) { if (hvm_funcs.nhvm_vcpu_hostrestore) diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c new file mode 100644 index 000..95b30ad --- /dev/null +++ b/xen/arch/x86/vm_event.c @@ -0,0 +1,41 @@ +/* + * arch/x86/vm_event.c + * + * Architecture-specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#include xen/sched.h +#include asm/hvm/hvm.h + +void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) +{ +if ( (v == current) || !is_hvm_domain(d) ) Why is 'current' excluded? Only to be consistent with the sanity check applied for XEN_DOMCTL_debug_op. XEN_DOMCTL_debug_op needs the check because of vcpu_pause(). It is always run in the context of the hypercaller domain, and must never end up calling singlestep on itself. However in this case, we can only legitimately use this on an already-paused vcpu, which guarentees that Xen is not currently in the context of the target vcpu. It would probably be better to have a check against the vcpu pause count here (with early return), and hvm_toggle_singlestep() assert so. I guess that's a fair sanity check to add in case the vm_event listener is buggy. ~Andrew Thanks, Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vm_event: toggle singlestep from vm_event response
On Mon, Jun 29, 2015 at 10:09 AM, Lengyel, Tamas tleng...@novetta.com wrote: On Mon, Jun 29, 2015 at 9:55 AM, Andrew Cooper andrew.coop...@citrix.com wrote: On 29/06/15 14:42, Lengyel, Tamas wrote: --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -6431,6 +6431,14 @@ int hvm_debug_op(struct vcpu *v, int32_t op) return rc; } +void hvm_toggle_singlestep(struct vcpu *v) +{ +if ( !cpu_has_monitor_trap_flag ) monitor_trap_flag is a VMX feature. This will never be true on AMD systems. (its use in hvm_debug_op() is also dubious) Yes, this feature is only for Intel cpus. Reworking the use of the flag though is a bit out-of-scope for this patch itself. In which case you must make it very clear in the commit message that this is for Intel only and needs fixing up for AMD. Best also to have a comment to the same effect in this function. Sure. +return; + +v-arch.hvm_vcpu.single_step = !v-arch.hvm_vcpu.single_step; +} + int nhvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs) { if (hvm_funcs.nhvm_vcpu_hostrestore) diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c new file mode 100644 index 000..95b30ad --- /dev/null +++ b/xen/arch/x86/vm_event.c @@ -0,0 +1,41 @@ +/* + * arch/x86/vm_event.c + * + * Architecture-specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#include xen/sched.h +#include asm/hvm/hvm.h + +void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) +{ +if ( (v == current) || !is_hvm_domain(d) ) Why is 'current' excluded? Only to be consistent with the sanity check applied for XEN_DOMCTL_debug_op. XEN_DOMCTL_debug_op needs the check because of vcpu_pause(). It is always run in the context of the hypercaller domain, and must never end up calling singlestep on itself. However in this case, we can only legitimately use this on an already-paused vcpu, which guarentees that Xen is not currently in the context of the target vcpu. It would probably be better to have a check against the vcpu pause count here (with early return), and hvm_toggle_singlestep() assert so. I guess that's a fair sanity check to add in case the vm_event listener is buggy. I was thinking ASSERT(v-vm_event_pause_count.counter); but that locks the function to only be usable through the vm_event response method. What do you think? Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v3 13/18] xen/arm: ITS: Add irq descriptors for LPIs
On Mon, 2015-06-22 at 17:31 +0530, vijay.kil...@gmail.com wrote: From: Vijaya Kumar K vijaya.ku...@caviumnetworks.com Add irq descriptors for LPIs and route This seems to also do interrupt injection for LPIs and more. Please check that your commit messages are accurately describing the contents of the patch. If it turns into a long list of unrelated sounding things then that might suggest the patch should be split up. Signed-off-by: Vijaya Kumar K vijaya.ku...@caviumnetworks.com --- xen/arch/arm/gic-v3.c |8 +++- xen/arch/arm/gic.c| 17 +++- xen/arch/arm/irq.c| 38 + xen/arch/arm/vgic-v3-its.c|9 + xen/arch/arm/vgic.c | 90 ++--- xen/include/asm-arm/domain.h |2 + xen/include/asm-arm/gic-its.h |6 +++ xen/include/asm-arm/gic.h |3 ++ xen/include/asm-arm/vgic.h|1 + 9 files changed, 157 insertions(+), 17 deletions(-) diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index 737646c..793f2f0 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -899,9 +899,13 @@ static void gicv3_update_lr(int lr, const struct pending_irq *p, val = (((uint64_t)state 0x3) GICH_LR_STATE_SHIFT) | grp; val |= ((uint64_t)p-priority 0xff) GICH_LR_PRIORITY_SHIFT; -val |= ((uint64_t)p-irq GICH_LR_VIRTUAL_MASK) GICH_LR_VIRTUAL_SHIFT; - if ( p-desc != NULL ) +if ( is_lpi(p-irq) ) +val |= ((uint64_t)p-irq GICH_LR_VIRTUAL_MASK) GICH_LR_VIRTUAL_SHIFT; +else +val |= ((uint64_t)p-irq GICH_LR_VIRTUAL_MASK) GICH_LR_VIRTUAL_SHIFT; Is there supposed to be something different between these two cases? (Or am I missing it?) + + if ( p-desc != NULL !(is_lpi(p-irq)) ) val |= GICH_LR_HW | (((uint64_t)p-desc-irq GICH_LR_PHYSICAL_MASK) GICH_LR_PHYSICAL_SHIFT); diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index cfc9c42..091f7e5 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -124,18 +124,31 @@ void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask, unsigned int priority) { ASSERT(priority = 0xff); /* Only 8 bits of priority */ -ASSERT(desc-irq gic_number_lines());/* Can't route interrupts that don't exist */ +/* Can't route interrupts that don't exist */ +ASSERT(desc-irq gic_number_lines() is_lpi(desc-irq)); ||, surely? Otherwise doesn't this hit for every possible irq? ASSERT(test_bit(_IRQ_DISABLED, desc-status)); ASSERT(spin_is_locked(desc-lock)); desc-handler = gic_hw_ops-gic_host_irq_type; -gic_set_irq_properties(desc, cpu_mask, priority); +if ( !is_lpi(desc-irq) ) +gic_set_irq_properties(desc, cpu_mask, priority); I think any lack of support for affinity or priority should be pushed down into the lower level drivers, rather than encoding that lack of current support in the pits driver into the generic code. int gic_route_irq_to_guest(struct domain *d, unsigned int virq, struct irq_desc *desc, unsigned int priority) { diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c index 9dbdf7d..105ef85 100644 --- a/xen/arch/arm/irq.c +++ b/xen/arch/arm/irq.c @@ -57,12 +57,22 @@ hw_irq_controller no_irq_type = { }; static irq_desc_t irq_desc[NR_IRQS]; +static irq_desc_t irq_desc_lpi[MAX_NR_LPIS]; static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc); irq_desc_t *__irq_to_desc(int irq) { +struct irq_desc *desc = NULL; if (irq NR_LOCAL_IRQS) return this_cpu(local_irq_desc)[irq]; -return irq_desc[irq-NR_LOCAL_IRQS]; +else if ( irq = NR_LOCAL_IRQS irq NR_IRQS) +return irq_desc[irq-NR_LOCAL_IRQS]; +else +{ +if ( is_lpi(irq) ) +return irq_desc_lpi[irq - NR_GIC_LPI]; +} + +return desc; I'd write this as just a chain of if (...) return, no need for else or {}s. +void vgic_vcpu_inject_lpi(struct domain *d, unsigned int irq) +{ +struct irq_desc *desc; +struct pending_irq *p; +struct its_device *dev; +struct vitt vitt_entry; +struct vdevice_table dt_entry; +uint32_t devid, col_id; +int event; + +desc = irq_to_desc(irq); +event = irq_to_virq(desc); + +dev = get_irq_device(desc); +devid = dev-device_id; +event = irq - dev-lpi_base; +if ( event 0 event dev-nr_lpis) +{ +dprintk(XENLOG_WARNING, + LPI %d received for dev 0x%x is not valid..dropping \n, + irq, devid); +return; +} + +/* validity of device is checheck on vitt entry request */ Typo Also trailing whitespace, please clean that up everywhere. +vits_get_vdevice_entry(d, devid, dt_entry); +if ( dt_entry.vitt_ipa != INVALID_PADDR ) +{ +
Re: [Xen-devel] Stop testing SEDF almost at all (at least until Xen 4.2) [was: Re: [PATCH v2] OSSTest: stop testing SEDF, start testing RTDS]
On Mon, 2015-06-29 at 12:07 +0100, George Dunlap wrote: On 06/26/2015 04:24 PM, Ian Jackson wrote: Dario Faggioli writes (Stop testing SEDF almost at all (at least until Xen 4.2) [was: Re: [PATCH v2] OSSTest: stop testing SEDF, start testing RTDS]): However, I honestly think that testing SEDF for earlier versions than xen-unstable (at least until 4.2) is also just a waste of test resources. ... Therefore, I'd argue for the attached patch, [...] With my osstest maintainer hat on the aim of this patch is fine by me. But it seems that sedf is not coming back, so if we are going to do this to osstest we should probably rip the code out. So I would welcome a followup which got rid of all the leftover traces in make-flight and allow.all. (`git-grep -i sedf'.) As to the substance, I would like to take advice from the relevant HV maintainers about this change. I agree with Dario that there's no point in testing it, and with you that we might as well rip the code out of osstest. Ok then, even better. Another patch to OSSTest to that effect will come shortly. With my tools maintainer hat on, we should consider removing the sedf-specific code from libxl. With the obvious caveat that old code with SEDF macros and such still needs to compile but return a runtime error if people try to use it. Yep, that's, in fact, what I think I've done in my 'get rid of SEDF' series, that I sent Friday afternoon. Thanks and 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] pvUSB backend performance
On 06/29/2015 03:22 PM, George Dunlap wrote: On Wed, Jun 24, 2015 at 1:06 PM, Juergen Gross jgr...@suse.com wrote: Hi, my qemu integrated pvUSB backend is now running stable enough to do some basic performance measurements. I've passed a memory-stick with about 90MB of data on it to a pv-domU. Then I read all the data on it with tar and looked how long this would take (elapsed time): in dom0: 5.2s in domU with kernel backend: 6.1s in domU with qemu backend: 8.2s So the qemu backend is about 30% slower than the kernel backend. Is this acceptable? Just to be clear, you mean having qemu act as a pvusb backend (a la qdisk), not emulated, is that correct? Yes. I don't actually understand your question -- is the overhead acceptable for what? Acceptable for the decision to build the backend in qemu only. When I posted the first draft of my kernel backend to lkml the question was raised why I couldn't do this in userland via libusb. I think in an ideal world the toolstack will use the kernel backend if it's available, and fall back to a qemu backend if it's not available. In case the performance is regarded to be sufficient I won't retry to push a kernel backend. So there will be none in the near future. If the performance is not good enough I'll give the kernel backend another try. If it's being accepted I probably won't do the qemu one. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-4.2
On Sat, Jun 27, 2015 at 11:47:49AM -0600, Jens Axboe wrote: On 06/23/2015 06:33 PM, Konrad Rzeszutek Wilk wrote: Hey Jens, Please git pull the following branch: git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/for-jens-4.2 in your 'for-4.2/drivers' branch. It is late - for which I am terrible sorry! The patches have been sitting in my branch for two weeks - except the last patch which was a fix and is now part of the branch. Please git pull at your convience. Pulled, thanks. But really, should be sent in before the last -rc of the previous window... Thankfully this isn't that large. Yes it should. I am sorry about that - we had some questions that took a bit of time to answer and they delayed me sending you this git pull (that and me destroying my GPG USB key in salad dressing). Thank you! ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 02/12] VMX: implement suppress #VE.
On Mon, Jun 22, 2015 at 7:56 PM, Ed White edmund.h.wh...@intel.com wrote: In preparation for selectively enabling #VE in a later patch, set suppress #VE on all EPTE's. Suppress #VE should always be the default condition for two reasons: it is generally not safe to deliver #VE into a guest unless that guest has been modified to receive it; and even then for most EPT violations only the hypervisor is able to handle the violation. Signed-off-by: Ed White edmund.h.wh...@intel.com --- xen/arch/x86/mm/p2m-ept.c | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index a6c9adf..5de3387 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -41,7 +41,7 @@ #define is_epte_superpage(ept_entry)((ept_entry)-sp) static inline bool_t is_epte_valid(ept_entry_t *e) { -return (e-epte != 0 e-sa_p2mt != p2m_invalid); +return ((e-epte ~(1ul 63)) != 0 e-sa_p2mt != p2m_invalid); So just getting up to speed here: Is it the case that if #VE is enabled in vmcs that a #VE will be delivered to the guest on any invalid epte entry that doesn't contain this flag? So we now need to actively choose a default which is different than the hardware? -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] tools/libxl: Fix build following c/s c3c8da9
On 29/06/15 15:09, Ian Jackson wrote: Andrew Cooper writes ([PATCH] tools/libxl: Fix build following c/s c3c8da9): c/s c3c8da9 libxl: ao: datacopier callback gets an rc caused libxl__domain_save_device_model() to pass its rc directly into the callback. However in the preexisting code, there were 3 goto out; paths which left rc uninitialised, which is cause by GCC 4.8's -Wmaybe-uninitialized The solution is not to initialise rc (to a bogus value) but to fix the goto out paths to explicitly set rc. Can you easily confirm that this fixes it ? It does indeed. Tested-by: Andrew Cooper andrew.coop...@citrix.com However, the problem with this style is that it is subverted by: rc = libxl__datacopier_start(dc); if (rc) goto out; out of context below, which cases rc to be initialised on all subsequent error paths, and thus miss further issues where it is set incorrectly. I would suggest introducing another int to hold the temporary from libxl__datacopier_start(). ~Andrew Ian. diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index bdc0465..1c9418a 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -2190,17 +2190,20 @@ void libxl__domain_save_device_model(libxl__egc *egc, dc-readfd = open(filename, O_RDONLY); if (dc-readfd 0) { LOGE(ERROR, unable to open %s, dc-readwhat); +rc = ERROR_FAIL; goto out; } if (fstat(dc-readfd, st)) { LOGE(ERROR, unable to fstat %s, dc-readwhat); +rc = ERROR_FAIL; goto out; } if (!S_ISREG(st.st_mode)) { LOG(ERROR, %s is not a plain file!, dc-readwhat); +rc = ERROR_FAIL; goto out; } ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Increase device model startup timeout to 1min.
On Fri, Jun 26, 2015 at 05:41:07PM +0100, Ian Campbell wrote: On Fri, 2015-06-26 at 12:57 +0100, Anthony PERARD wrote: On a busy host, QEMU may take more than 10s to start. Please show your working here so that in 2 years when we want to know why this value was chosen we don't have to go scrobbling around in the ML archives looking for the data you gathered. OK. What about: While running the whole stack of OpenStack (install via devstack on Ubuntu) on a single machine, sometime the device model fail to start on time. When this happen, a strace of QEMU shows that it still loading its libraries. You can find such strace on: http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03549.html What shows the strace is that between a mmap() syscall and the next syscall, 0.5s might have pass. Normaly, on this same machine it takes 0.32 second on average for QEMU to start, while running Tempest (which is the OpenStack test suite). Those QEMU are runned as backend for a PV guest. Signed-off-by: Anthony PERARD anthony.per...@citrix.com --- tools/libxl/libxl_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index e96d6b5..3a604f7 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -85,7 +85,7 @@ #define LIBXL_INIT_TIMEOUT 10 #define LIBXL_DESTROY_TIMEOUT 10 #define LIBXL_HOTPLUG_TIMEOUT 10 -#define LIBXL_DEVICE_MODEL_START_TIMEOUT 10 +#define LIBXL_DEVICE_MODEL_START_TIMEOUT 60 #define LIBXL_STUBDOM_START_TIMEOUT 30 #define LIBXL_QEMU_BODGE_TIMEOUT 2 #define LIBXL_XENCONSOLE_LIMIT 1048576 -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] tools/libxl: Fix build following c/s c3c8da9
c/s c3c8da9 libxl: ao: datacopier callback gets an rc caused libxl__domain_save_device_model() to pass its rc directly into the callback. However in the preexisting code, there were 3 goto out; paths which left rc uninitialised, which is cause by GCC 4.8's -Wmaybe-uninitialized Signed-off-by: Andrew Cooper andrew.coop...@citrix.com CC: Ian Campbell ian.campb...@citrix.com CC: Ian Jackson ian.jack...@eu.citrix.com CC: Wei Liu wei.l...@citrix.com --- tools/libxl/libxl_dom.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 600393d..d547eb5 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -2147,7 +2147,7 @@ void libxl__domain_save_device_model(libxl__egc *egc, STATE_AO_GC(dss-ao); struct stat st; uint32_t qemu_state_len; -int rc; +int rc = -1; dss-save_dm_callback = callback; -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] x86/arm/mm: use gfn instead of pfn in p2m_get_mem_access/p2m_set_mem_access
Andrew Cooper andrew.coop...@citrix.com writes: On 26/06/15 16:17, Vitaly Kuznetsov wrote: 'pfn' and 'start_pfn' are ambiguous, both these functions expect GFNs as input. On x86 the interface of p2m_set_mem_access() in p2m.c doesn't match the declaration in p2m-common.h as 'pfn' is being used instead of 'start_pfn'. On ARM both p2m_set_mem_access and p2m_get_mem_access interfaces don't match declarations from p2m-common.h: p2m_set_mem_access uses 'pfn' instead of 'start_pfn' and p2m_get_mem_access uses 'gpfn' instead of 'pfn'. There is also an issue in p2m_get_mem_access on x86: 'gfn' parameter passed to gfn_lock/gfn_unlock is not defined. This code compiles only because of a coincidence: gfn_lock/gfn_unlock are currently macros which don't use their second argument. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com --- Changes since v2: - Instead of adding start_ prefix on ARM remove it on x86 [Jan Beulich, Ian Campbell, Razvan Cojocaru] Changes since v1: - This patch is a successor of '[PATCH] x86/mm: use existing 'pfn' in p2m_get_mem_access', instead of fixing gfn_lock/gfn_unlock arguments we do s/pfn/gfn/g for both p2m_get_mem_access/p2m_set_mem_access [Andrew Cooper, Jan Beulich] P.S. - The patch was compile-tested on x86 only. --- xen/arch/arm/p2m.c | 12 ++-- xen/arch/x86/mm/p2m.c| 24 xen/include/xen/p2m-common.h | 12 ++-- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 903fa3f..54c238c 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1709,9 +1709,9 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec) /* * Set access type for a region of pfns. - * If start_pfn == -1ul, sets the default access type. + * If gfn == -1ul, sets the default access type. */ -long p2m_set_mem_access(struct domain *d, unsigned long pfn, uint32_t nr, +long p2m_set_mem_access(struct domain *d, unsigned long gfn, uint32_t nr, uint32_t start, uint32_t mask, xenmem_access_t access) { struct p2m_domain *p2m = p2m_get_hostp2m(d); @@ -1752,14 +1752,14 @@ long p2m_set_mem_access(struct domain *d, unsigned long pfn, uint32_t nr, p2m-mem_access_enabled = true; /* If request to set default access. */ -if ( pfn == ~0ul ) +if ( gfn == ~0ul ) Please here and everywhere else in the patch, use INVALID_GFN instead of ~0 or -1. With those changes and the comment style from Razvan, Reviewed-by: Andrew Cooper andrew.coop...@citrix.com However, if you feel up to it, it would be fantastic if you could substitute unsigned long for gfn_t, per the justification in e758ed1 and 177bd5f. c/s 24036a5 is an example of a different API I fixed up in a similar way. Sure, will do v4. ~Andrew -- Vitaly ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/12] x86/altp2m: basic data structures and support routines.
On 26/06/15 22:17, Ed White wrote: On 06/24/2015 03:29 AM, Andrew Cooper wrote: On 22/06/15 19:56, Ed White wrote: diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h index 3d8f4dc..a1529c0 100644 --- a/xen/include/asm-x86/hvm/vcpu.h +++ b/xen/include/asm-x86/hvm/vcpu.h @@ -118,6 +118,13 @@ struct nestedvcpu { #define vcpu_nestedhvm(v) ((v)-arch.hvm_vcpu.nvcpu) +struct altp2mvcpu { +uint16_tp2midx; /* alternate p2m index */ +uint64_tveinfo_gfn; /* #VE information page guest pfn */ Please use the recently-introduced pfn_t here. pfn is a more appropriate term than gfn in this case. Did you mean pfn_t, or xen_pfn_t? Actually I meant gfn_t, per the followup I sent shortly afterwards. I'm having a hard time figuring out how to use a pfn_t, I can't even assign INVALID_PFN to one. Documentation in c/s 177bd5f, example in c/s 24036a5. The point of this For now, it will require copious use of _gfn() and gfn_x() until the rest of the mm subsystem has been updated to use the new typesafe types. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 09/12] x86/altp2m: add remaining support routines.
On 26/06/15 17:30, Ed White wrote: On 06/24/2015 11:19 AM, Andrew Cooper wrote: On 24/06/15 18:47, Ed White wrote: This looks like some hoop jumping around the assertions in domain_pause() and vcpu_pause(). We should probably have some new helpers where the domain needs to be paused, possibly while in context. The current domain/vcpu_pause() are almost always used where it is definitely not safe to pause in context, hence the assertions. It is. I'd be happy to use new helpers, I don't feel qualified to write them. Ed Something like this? Only compile tested. In the meantime, I have an optimisation in mind for domain_pause() on domains with large numbers of vcpus, but that will have to wait a while. From: Andrew Cooper andrew.coop...@citrix.com Date: Wed, 24 Jun 2015 19:06:14 +0100 Subject: [PATCH] common/domain: Helpers to pause a domain while in context For use on codepaths which would need to use domain_pause() but might be in the target domain's context. In the case that the target domain is in context, all other vcpus are paused. Signed-off-by: Andrew Cooper andrew.coop...@citrix.com --- xen/common/domain.c | 28 xen/include/xen/sched.h |5 + 2 files changed, 33 insertions(+) diff --git a/xen/common/domain.c b/xen/common/domain.c index 3bc52e6..a1d27e3 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -1010,6 +1010,34 @@ int domain_unpause_by_systemcontroller(struct domain *d) return 0; } +void domain_pause_except_self(struct domain *d) +{ +struct vcpu *v, *curr = current; + +if ( curr-domain == d ) +{ +for_each_vcpu( d, v ) +if ( likely(v != curr) ) +vcpu_pause(v); +} +else +domain_pause(d); +} + +void domain_unpause_except_self(struct domain *d) +{ +struct vcpu *v, *curr = current; + +if ( curr-domain == d ) +{ +for_each_vcpu( d, v ) +if ( likely(v != curr) ) +vcpu_unpause(v); +} +else +domain_unpause(d); +} + int vcpu_reset(struct vcpu *v) { struct domain *d = v-domain; diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index b29d9e7..8e1345a 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -804,6 +804,11 @@ static inline int domain_pause_by_systemcontroller_nosync(struct domain *d) { return __domain_pause_by_systemcontroller(d, domain_pause_nosync); } + +/* domain_pause() but safe against trying to pause current. */ +void domain_pause_except_self(struct domain *d); +void domain_unpause_except_self(struct domain *d); + void cpu_init(void); struct scheduler; Did you commit this to staging? I am not a committer, so couldn't even if I wished to. IOW, can I apply it to my branch and assume it will already be in-tree when our patches are applied? You will be the first user of the patch, and as noted, I have only compile tested. Please take it and put it at the start of your series. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] traps.c:3227: GPF (0000): ffff82d080194a4d - ffff82d080239d85 and other dom0 induced log messages
On 26/06/15 16:51, Jan Beulich wrote: On 26.06.15 at 17:41, li...@eikelenboom.it wrote: from 3.16 to 3.19 we gained a lot of these, if i remember correctly related to perf being enabled in the kernel: + traps.c:2655:d0v0 Domain attempted WRMSR c081 from 0xe023e008 to 0x00230010. + traps.c:2655:d0v0 Domain attempted WRMSR c082 from 0x82d0b000 to 0x81bc2670. + traps.c:2655:d0v0 Domain attempted WRMSR c083 from 0x82d0b020 to 0x81bc4630. These are the SYSCALL (STAR) MSRs, which the kernel has no business touching when running on Xen. from 3.19 to 4.0 we gained: + d0 attempted to change d0v0's CR4 flags 0660 - 0760 + d0 attempted to change d0v1's CR4 flags 0660 - 0760 + d0 attempted to change d0v2's CR4 flags 0660 - 0760 + d0 attempted to change d0v3's CR4 flags 0660 - 0760 + d0 attempted to change d0v4's CR4 flags 0660 - 0760 + d0 attempted to change d0v5's CR4 flags 0660 - 0760 This is X86_CR4_PCE - not sure how to properly handle that. Andrew, you're fiddling with the CR4 handling right now anyway - any thoughts? We have no infrastructure whatsoever to allow PV guests to use rdpmc, and PCE is unconditionally clear in CR4. With Boris' perf series, and oprofile already having other PV interfaces to access performance counters, I don't see any reason to open this up to native use by PV guests. and from 4.0 to 4.1 we gained the ones you were interested in: + traps.c:3227: GPF (): 82d080194a4d - 82d080239d85 + traps.c:3227: GPF (): 82d080194a4d - 82d080239d85 + traps.c:3227: GPF (): 82d080194a4d - 82d080239d85 + traps.c:3227: GPF (): 82d080194a4d - 82d080239d85 + traps.c:3227: GPF (): 82d080194a4d - 82d080239d85 + traps.c:3227: GPF (): 82d080194a4d - 82d080239d85 For these to be meaningful you need to translate them to symbolic addresses. (And yes, we should see to make the code print them in a more useful manner.) For things like {wr,rd}msr_safe(), we should print ecx/eax/edx. I expect there are similar useful debug hints for other areas of code. Is it worth stashing some other information in the extable to aid generic debug printing of errors? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] tools/libxl: Fix build following c/s c3c8da9
Andrew Cooper writes ([PATCH] tools/libxl: Fix build following c/s c3c8da9): c/s c3c8da9 libxl: ao: datacopier callback gets an rc caused libxl__domain_save_device_model() to pass its rc directly into the callback. However in the preexisting code, there were 3 goto out; paths which left rc uninitialised, which is cause by GCC 4.8's -Wmaybe-uninitialized The solution is not to initialise rc (to a bogus value) but to fix the goto out paths to explicitly set rc. Can you easily confirm that this fixes it ? Ian. diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index bdc0465..1c9418a 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -2190,17 +2190,20 @@ void libxl__domain_save_device_model(libxl__egc *egc, dc-readfd = open(filename, O_RDONLY); if (dc-readfd 0) { LOGE(ERROR, unable to open %s, dc-readwhat); +rc = ERROR_FAIL; goto out; } if (fstat(dc-readfd, st)) { LOGE(ERROR, unable to fstat %s, dc-readwhat); +rc = ERROR_FAIL; goto out; } if (!S_ISREG(st.st_mode)) { LOG(ERROR, %s is not a plain file!, dc-readwhat); +rc = ERROR_FAIL; goto out; } ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Work-arounds in Xen code for Intel GFX?Re: Is: graphics corruption with 'xen: Support Xen pv-domains using PAT. Was:Re: [BUG] Characters on the screen are broken on Linux = 3.19 with VT-
On Sun, Jun 28, 2015 at 05:15:28AM +0800, Ting-Wei Lan wrote: 於 二,2015-06-16 於 11:58 +0200,Juergen Gross 提到: On 06/16/2015 11:32 AM, 藍挺瑋 wrote: 於 二,2015-06-16 於 11:06 +0200,Juergen Gross 提到: On 06/16/2015 10:55 AM, Ting-Wei Lan wrote: Juergen Gross 於 西元2015年06月16日 12:30 寫道: On 06/15/2015 09:03 PM, Ting-Wei Lan wrote: 於 一,2015-06-15 於 14:55 -0400,Konrad Rzeszutek Wilk 提到: On Sat, Jun 13, 2015 at 12:43:14AM +0800, Ting-Wei Lan wrote: When using Linux = 3.19 as the dom0 kernel, characters on the screen become broken after the graphic driver is loaded. The commit that breaks it is (found by git bisect): https://git.kernel.org/cgit/linux/kernel/git/torvalds/l inux .git/com mit/?id=47591df Lets CC Juergen Screenshot when the system run in single user mode: https://bugs.freedesktop.org/attachment.cgi?id=115079 Are those messages to be expected: (XEN) Scrubbing Free RAM on 1 nodes using 2 CPUs (XEN) [VT-D]DMAR:[DMA Write] Request device [:00:02.0] fault addr b61eef000, iommu reg = 82c000203000 (XEN) [VT-D]DMAR: reason 05 - PTE Write access is not set (XEN) . .done. I'm not familiar with VT-D internals, but seeing these messages for the video device during RAM scrubbing makes wonder if everything is correct regarding the VT-D and memory setup... ... I still remember that there was a similar problem found two years ago on the same hardware with similar broken screen output and it also crashed after Xorg was started, but I cannot confirm that they are the same problems. I don't know whether error messages are simliar. The old problem happens on Linux 3.7 ~ 3.10 with VT-d enabled. It only happened when not using Xen, so I added 'intel_iommu=off' to Linux boot arguments to workaround it. Hmm, do you see any chance in finding the commit which made it working again? Perhaps there was some workaround for this hardware which is missing in Xen now... After some tests, I found the information I provided before was incorrect. It seems the problem happens on all Linux = 3.7, including Linux 4.0.5, so the old problem was never fixed. Here are some 'dmesg | grep -i iommu' outputs. So a Xen-specific error is rather improbable, correct? But Linux provides 'intel_iommu=igfx_off' to workaround the problem. Does Xen provide similar things? Not that I know of. The question is whether you really need VT-d, and if yes, why. You could still switch the iommu off for dom0 by setting iommu=dom0-passthrough in the Xen command line (your hardware might not support it, though). iommu=dom0-passthrough doesn't work on my hardware. I found a workaround for my hardware: diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c index 69d29ab..b937ad0 100644 --- a/xen/drivers/passthrough/vtd/quirks.c +++ b/xen/drivers/passthrough/vtd/quirks.c @@ -74,6 +74,7 @@ int is_igd_vt_enabled_quirk(void) if ( !IS_ILK(ioh_id) ) return 1; +return 0; /* integrated graphics on Intel platforms is located at 0:2.0 */ ggc = pci_conf_read16(0, 0, IGD_DEV, 0, GGC); Lets CC the maintaners of said code. This workaround is silimar to intel_iommu=igfx_off in Linux. I found silimar code in function quirk_calpella_no_shadow_gtt in drivers/iommu/intel-iommu.c. I reported the graphics corruption problems on Linux = 3.7 here: https://bugs.freedesktop.org/show_bug.cgi?id=91127 And here is the bug link of Xen and Linux = 3.19 (the bug we are discussing here): https://bugs.freedesktop.org/show_bug.cgi?id=90037 I hope we can get a real fix. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v3 15/18] xen/arm: ITS: Add domain specific ITS initialization
On Mon, 2015-06-22 at 17:31 +0530, vijay.kil...@gmail.com wrote: From: Vijaya Kumar K vijaya.ku...@caviumnetworks.com Add Domain and vcpu specific ITS initialization Signed-off-by: Vijaya Kumar K vijaya.ku...@caviumnetworks.com --- xen/arch/arm/gic-v3-its.c | 17 xen/arch/arm/setup.c |1 + xen/arch/arm/vgic-v3-its.c| 45 + xen/arch/arm/vgic-v3.c|1 + xen/include/asm-arm/gic-its.h |3 +++ xen/include/asm-arm/vgic.h|1 + 6 files changed, 68 insertions(+) diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 4471669..8aa1ec5 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -1234,6 +1234,23 @@ static int its_force_quiescent(void __iomem *base) } } +void its_domain_init(struct domain *d) +{ +struct its_node *its; + +if ( is_hardware_domain(d) ) +{ +list_for_each_entry(its, its_nodes, entry) +{ +/* XXX: Assign only first physical ITS address */ I think we settled upon rewriting the msi-parent properties in the dts to all refer to a single vits, in which case putting that vits in the first pits' hole seem ok. Rather than an XXX (which is a TODO) a comment explaining what is going on would be preferable. Ian ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v3 00/18] Add ITS support
On Mon, 2015-06-22 at 17:31 +0530, vijay.kil...@gmail.com wrote: Vijaya Kumar K (18): xen/arm: Add bitmap_find_next_zero_area helper function xen: Add log2 functionality xen: console: Add ratelimit support for error message xen/arm: gicv3: Refactor redistributor information xen/arm: ITS: Port ITS driver to xen xen/arm: ITS: Add helper functions to manage its_devices xen/arm: ITS: implement hw_irq_controller for LPIs xen/arm: vITS: Add virtual ITS driver xen/arm: ITS: Add virtual ITS commands support xen/arm: ITS: Add APIs to add and assign device xen/arm: ITS: Add GITS registers emulation xen/arm: ITS: Add GICR register emulation xen/arm: ITS: Add irq descriptors for LPIs xen/arm: ITS: Initialize physical ITS xen/arm: ITS: Add domain specific ITS initialization xen/arm: ITS: Handle LPI interrupts xen/arm: ITS: Generate ITS node for Dom0 xen/arm: ITS: Map ITS translation space FWIW I think it would be more logical to order this as: * Preparation, precursor cleanups etc * PITS support * VITS support, which uses pits stuff * Domain vits init, including map the ITS translation page, which uses vits stuff * Device Tree mangling which finally exposes the result to the guest. But I think far more critical would be to make function which whould eventually be static be so when it is introduced (not later) and to include prototypes of functions in the patch which adds the function. Also lots more const correctness would be good. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vm_event: toggle singlestep from vm_event response
On 06/29/2015 03:58 PM, Tamas K Lengyel wrote: Add an option to the vm_event response to toggle singlestepping on the vCPU. Singed-off-by: Tamas K Lengyel tleng...@novetta.com --- MAINTAINERS| 1 + xen/arch/x86/Makefile | 1 + xen/arch/x86/hvm/hvm.c | 8 xen/arch/x86/vm_event.c| 41 + xen/common/vm_event.c | 8 +++- xen/include/asm-arm/vm_event.h | 29 + xen/include/asm-x86/hvm/hvm.h | 3 +++ xen/include/asm-x86/vm_event.h | 25 + xen/include/public/vm_event.h | 11 --- 9 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 xen/arch/x86/vm_event.c create mode 100644 xen/include/asm-arm/vm_event.h create mode 100644 xen/include/asm-x86/vm_event.h Looks good, and it definitely sounds useful. Acked-by: Razvan Cojocaru rcojoc...@bitdefender.com Cheers, Razvan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/events/fifo: Consume unprocessed events when a CPU dies
On 06/29/2015 06:19 AM, Ross Lagerwall wrote: On 06/19/2015 05:06 PM, David Vrabel wrote: On 19/06/15 17:02, Boris Ostrovsky wrote: On 06/19/2015 11:15 AM, Ross Lagerwall wrote: When a CPU is offlined, there may be unprocessed events on a port for that CPU. If the port is subsequently reused on a different CPU, it could be in an unexpected state with the link bit set, resulting in interrupts being missed. Fix this by consuming any unprocessed events for a particular CPU when that CPU dies. Signed-off-by: Ross Lagerwall ross.lagerw...@citrix.com --- drivers/xen/events/events_fifo.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c index 417415d..1dd0ba12 100644 --- a/drivers/xen/events/events_fifo.c +++ b/drivers/xen/events/events_fifo.c @@ -281,7 +281,8 @@ static void handle_irq_for_port(unsigned port) static void consume_one_event(unsigned cpu, struct evtchn_fifo_control_block *control_block, - unsigned priority, unsigned long *ready) + unsigned priority, unsigned long *ready, + bool drop) { struct evtchn_fifo_queue *q = per_cpu(cpu_queue, cpu); uint32_t head; @@ -313,13 +314,17 @@ static void consume_one_event(unsigned cpu, if (head == 0) clear_bit(priority, ready); -if (evtchn_fifo_is_pending(port) !evtchn_fifo_is_masked(port)) -handle_irq_for_port(port); +if (evtchn_fifo_is_pending(port) !evtchn_fifo_is_masked(port)) { +if (unlikely(drop)) +pr_warn(Dropping pending event for port %u\n, port); Maybe pr_info (or pr_notice)? We want a warning here because we think this shouldn't happen -- if it does we actually need to retrigger the event on its new CPU. Also, why not do this (testing for unprocessed events) in xen_evtchn_close()? We can't do anything about them when closing because they may be in the middle of a queue. (Sorry, I missed this) Why can't (actually, why doesn't) the cpu that is being offlined drain its queue? -boris David Ping. Is this change OK? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt test] 58119: regressions - FAIL
On Fri, Jun 26, 2015 at 05:44:59PM +0100, Ian Jackson wrote: Anthony PERARD writes (Re: [Xen-devel] [libvirt test] 58119: regressions - FAIL): FYI, I have looked at how long it takes for QEMU to start, from libxl point of view, and from strace point of view. For libxl, I have look at the time difference between a call to libxl__ev_xswatch_register('device-model/$domid/path') and libxl__qmp_initialize(): cat deltatime | sort | uniq -c 2754 0:00:00 1309 0:00:01 12 0:00:02 8 0:00:03 5 0:00:04 1 0:00:05 4 0:00:06 7 0:00:07 6 0:00:08 1 0:00:09 2 0:00:10 16 timeout: 0:00:10 Is this information from merlot ? No, this data is gathered on a local machine running OpenStack (the whole stack). From straces, it is the time between the execve() call and when QEMU respond to a QMP connection. The average is 0.316729, and the standard deviation is 0.460369 (The average and std deviation does not take into account the QEMUs that timed out). But, out of the 3386 QEMU startup, there are 26 run that took between 2s and 10s, and there are 14 more qemu run that have timed out. I'm going to send a patch to ask to increase the timeout. I think you have not explained why these long startup times are to be expected. If they are _not_ expected, we should not be covering up for them by increasing the timeout. I will try to answer to this in the patch description. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V5 2/7] libxl_read_file_contents: add new entry to read sysfs file
On 6/26/2015 at 09:05 PM, in message 21901.20009.85407.581...@mariner.uk.xensource.com, Ian Jackson ian.jack...@eu.citrix.com wrote: Chun Yan Liu writes (Re: [PATCH V5 2/7] libxl_read_file_contents: add new entry to read sysfs file): On 6/25/2015 at 07:09 PM, in message 21899.57676.368102.982...@mariner.uk.xensource.com, Ian Jackson ian.jack...@eu.citrix.com wrote: Chunyan Liu writes ([PATCH V5 2/7] libxl_read_file_contents: add new entry Add a new entry libxl_read_sysfs_file_contents to handle sysfs file specially. It would be used in later pvusb work. I think this still fails to detect a situation where the file is unexpectedly longer than the requested size ? +} else if (feof(f)) { +if (rs datalen tolerate_shrinking_file) { +datalen = rs; +} else { If the file is bigger than the requested size, it will fall to this branch and report error. I don't think this is true. I just applied your patch to my copy of staging to see what the code looks like and saw this: if (stab.st_size data_r) { data = malloc(datalen); if (!data) goto xe; rs = fread(data, 1, datalen, f); if (rs != datalen) { if (ferror(f)) { LOGE(ERROR, failed to read %s, filename); goto xe; } else if (feof(f)) { if (rs datalen tolerate_shrinking_file) { datalen = rs; } else { LOG(ERROR, %s changed size while we were reading it, filename); goto xe; } } else { abort(); } } } So I think in the case of a sysfs file which is 4096 bytes: - stat will succeed and give st_size == 4096 - fread(,,4096,) will read the first 4096 bytes and return 4096 - rs == datalen, so we don't take the `errors and special cases' branch - we return success having read 4096 bytes But please feel free to explain why I'm wrong. You are right. I'm confused about the original logic. The original code doesn't consider this case (size bigger than requested) at all. So, we need to add a check if (rs == datalen read end of file), if not, means bigger than requested, report error. - Chunyan Thanks, Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Problems with merlot* AMD Opteron 6376 systems (Was Re: stable trees
On Fri, 2015-06-26 at 21:07 +0100, Ian Campbell wrote: On Fri, 2015-06-26 at 15:36 -0400, Boris Ostrovsky wrote: On 06/26/2015 10:52 AM, Jan Beulich wrote: On 26.06.15 at 16:34, ian.campb...@citrix.com wrote: I did this using rdmsr from mst-tools instead, running on a native kernel gave: # for i in $(seq 0 31) ;do rdmsr -p $i MSR_K8_TOP_MEM2; done 0 [...] 0 Is MSR_K8_TOP_MEM2 defined somewhere in the shell? There is no $ there, so it wouldn't make any difference... I had foolishly assumed that rdmsr would either know the names of the MSRs or it would complain about a string it didn't understand which wasn't a number. Instead it just reads some random register which happens to be strtoul(MSR_K8_TOP_MEM2), how helpful. = https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=790075 Just to make sure, could you use explicit address, i.e. for i in $(seq 0 31) ;do rdmsr -p $i 0xc001001d; done It reported 43f00 on all processors on native (and only the first 8 on Xen due to limited dom0 vcpus). (and if they are still all zeroes, can you read 0xc0010010 (SYSCFG) as well?) It wasn't all zeroes, but anyway, it reported 74 on all processors on native (I forgot to run under Xen). Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v1] mg-all-branch-statuses: Show how up to date each branch is
The osstest branch doesn't expose pretest (at least not to the machine I tested this on), so it ends up ignored here. Some branches (e.g. linux-next) do not record a baseline, so the script allows for that. Since everything serialises on the repo lock I didn't bother trying to parallelise anything. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- I did wonder about adding this to some cronjob or something. Example output: libvirt d10a5f58c75e7eb5943b44cc36a1e768adb2cdb0 = f7d8aa44b06e5cfa06abea4a72ee26b13754667d linux-3.0e1c63f9f42393f7c1dc072db7e0d54e599e96b46 = 5dba9ddd98cbc7ad319d687887981a0ea0062c75 linux-3.10 28114597f84ea08d0f61f0a60aa23176ec36004a = OK linux-3.14 165797d05c15ab87ef7421c63a076ffa8477cbe4 = OK linux-3.16 162d64326176ee1916fb98323d810c78a7e3d042 = OK linux-3.18 d048c068d00da7d4cfa5ea7651933b99026958cf = OK linux-3.4bb4a05a0400ed6d2f1e13d1f82f289ff74300a70 = cf1b3dad6c5699b977273276bada8597636ef3e2 linux-arm-xen64972ceb0b0cafc91a09764bc731e1b7f0503b5c = OK linux-linus aefbef10e3ae6e2c6e3c54f906f10b34c73a2c66 = 4a10a91756ef381bced7b88cfb9232f660b92d93 linux-mingo-tip-master d935d0f77650fea53986811ca8a2f8c726fd9857 = b830b87013d814c38a61adcf7bcfd6d4000ba5b3 linux-next No baseline version = 043831b4a4e9a981c4ec6331b6d64b9f62285d5d ovmf 495ee9b85141dd9b65434d677b3a685fe166128d = 79d274b8b6b113248661c18f31c4be03c7da32de qemu-mainlinedc1e1350f8061021df765b396295329797d66933 = OK qemu-upstream-4.2-testingd2382550f9d563da371b79e1b97b2453c77b3c8e = OK qemu-upstream-4.3-testingefae5e0f79f77c77720185a0d8a49f3ba49071e7 = OK qemu-upstream-4.4-testing32226f429cca6c79826364d8d18acb2226f2f102 = OK qemu-upstream-4.5-testingd9552b0af21c27535cd3c8549bb31d26bbecd506 = OK qemu-upstream-unstable c4a962ec0c61aa9b860a3635c8424472e6c2cc2c = OK rumpuserxen 30d72f3fc5e35cd53afd82c8179cc0e0b11146ad = 3b91e44996ea6ae1276bce1cc44f38701c53ee6f seabios f24eb2f853d4aa28814761e0bbc7df78149f8029 = OK xen-4.0-testing 2692df2a2c6ca3c09ef6c3d064f36e3630ff9bdc = OK xen-4.1-testing 40feff8733e2ac27561a27e7c009a61ba3b320fe = OK xen-4.2-testing 38fcda225d6613ecc4bf44769806887252d7b2b1 = OK xen-4.3-testing e7c022977eb83822edb52919a3748ebfa5705b5d = OK xen-4.4-testing 05ab7714ff84003b9b4542d7816b4651efb67679 = 6c1cb3dba4ff97dd40909670755f24fcdf903012 xen-4.5-testing e3bd3cefba5f11062523701bd07051c92a47ef34 = OK xen-unstable bdf741bf4d014eec18f251576c1182ae264397bb = c40317f11b3f05e7c06a2213560c8471081f2662 --- mg-all-branch-statuses | 51 ++ 1 file changed, 51 insertions(+) create mode 100755 mg-all-branch-statuses diff --git a/mg-all-branch-statuses b/mg-all-branch-statuses new file mode 100755 index 000..75f4c72 --- /dev/null +++ b/mg-all-branch-statuses @@ -0,0 +1,51 @@ +#!/bin/bash +# +# Prints the status of each branch +# +# Usage: +#./mg-all-branch-statuses [BRANCH] +# +# If no BRANCHes specified, does all that are normally run by +# cr-daily-branch or out of crontab. + +# This is part of osstest, an automated testing framework for Xen. +# Copyright (C) 2009-2014 Citrix Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. + +set -e + +set -o pipefail + +mkdir -p tmp + +if [ $# = 0 ]; then + set `./mg-list-all-branches` +fi + +for branch in $@; do +old=`./ap-fetch-version-old $branch 2/dev/null || true` +new=`./ap-fetch-version $branch 2/dev/null || true` +if [ x$new = x ] ; then +# ??? osstest? +continue +elif [ x$old = x ] ; then +# e.g. linux-next doesn't record, or a new branch +printf %-32s %-40s = %s\n $branch 'No baseline version' $new +elif [ x$old != x$new ] ; then +printf %-32s %s = %s\n $branch $old $new +else +printf %-32s %s = OK\n $branch $old +fi +done -- 2.1.4 ___ Xen-devel mailing list
Re: [Xen-devel] [PATCH] Refactor ioreq server for better performance
-Original Message- From: Yu Zhang [mailto:yu.c.zh...@linux.intel.com] Sent: 26 June 2015 11:30 To: xen-de...@lists.xenproject.org; Paul Durrant; Andrew Cooper; jbeul...@suse.com; Kevin Tian; zhiyuan...@intel.com Subject: [PATCH] Refactor ioreq server for better performance XenGT leverages ioreq server to track and forward the accesses to GPU IO resources, e.g. the PPGTT(per-process graphic translation tables). Currently, ioreq server uses rangeset to track the BDF/ PIO/MMIO ranges to be emulated. To select an ioreq server, the rangeset is searched to see if the IO range is recorded. However, traversing the link list inside rangeset could be time consuming when number of ranges is too high. On HSW platform, number of PPGTTs for each vGPU could be several hundred. On BDW, this value could be several thousand. To increase the performance, a new data structure, rb_rangeset is defined. Compared with rangeset, which is based on doubly linked list with O(n) time complexity for searching, this rb_rangeset is based on red-black tree with O(log(n)) time complexity. Besides the underlying data structure difference with the rangeset, another one is that the rb_rangeset does not provide a spinlock, instead, it left this to users. Besides, NR_IO_RANGE_TYPES is changed to 8192 to accommodate more ranges. Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com --- xen/arch/x86/hvm/hvm.c | 52 - xen/common/Makefile | 1 + xen/common/rb_rangeset.c | 243 +++ xen/include/asm-x86/hvm/domain.h | 4 +- xen/include/xen/rb_rangeset.h| 45 5 files changed, 311 insertions(+), 34 deletions(-) create mode 100644 xen/common/rb_rangeset.c create mode 100644 xen/include/xen/rb_rangeset.h diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 535d622..be70925 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -37,6 +37,7 @@ #include xen/wait.h #include xen/mem_access.h #include xen/rangeset.h +#include xen/rb_rangeset.h #include asm/shadow.h #include asm/hap.h #include asm/current.h @@ -809,7 +810,7 @@ static void hvm_ioreq_server_unmap_pages(struct hvm_ioreq_server *s, } } -static void hvm_ioreq_server_free_rangesets(struct hvm_ioreq_server *s, +static void hvm_ioreq_server_free_rb_rangesets(struct hvm_ioreq_server *s, Did you need to change the name of the function here? bool_t is_default) { unsigned int i; @@ -818,10 +819,10 @@ static void hvm_ioreq_server_free_rangesets(struct hvm_ioreq_server *s, return; for ( i = 0; i NR_IO_RANGE_TYPES; i++ ) -rangeset_destroy(s-range[i]); +rb_rangeset_destroy(s-range[i]); } -static int hvm_ioreq_server_alloc_rangesets(struct hvm_ioreq_server *s, +static int hvm_ioreq_server_alloc_rb_rangesets(struct hvm_ioreq_server *s, Same here. bool_t is_default) { unsigned int i; @@ -832,33 +833,20 @@ static int hvm_ioreq_server_alloc_rangesets(struct hvm_ioreq_server *s, for ( i = 0; i NR_IO_RANGE_TYPES; i++ ) { -char *name; - -rc = asprintf(name, ioreq_server %d %s, s-id, - (i == HVMOP_IO_RANGE_PORT) ? port : - (i == HVMOP_IO_RANGE_MEMORY) ? memory : - (i == HVMOP_IO_RANGE_PCI) ? pci : - ); -if ( rc ) -goto fail; - -s-range[i] = rangeset_new(s-domain, name, - RANGESETF_prettyprint_hex); - -xfree(name); +s-range[i] = rb_rangeset_new(); I think assigning a name to the rangeset and having a debug-key dump is useful. Can you not duplicate that in your new implementation? rc = -ENOMEM; if ( !s-range[i] ) goto fail; -rangeset_limit(s-range[i], MAX_NR_IO_RANGES); +s-range[i]-nr_ranges = MAX_NR_IO_RANGES; I'd add a limit function rather than just stooging into the structure fields. } done: return 0; fail: -hvm_ioreq_server_free_rangesets(s, 0); +hvm_ioreq_server_free_rb_rangesets(s, 0); Without the name change this diff is gone. return rc; } @@ -934,7 +922,7 @@ static int hvm_ioreq_server_init(struct hvm_ioreq_server *s, struct domain *d, INIT_LIST_HEAD(s-ioreq_vcpu_list); spin_lock_init(s-bufioreq_lock); -rc = hvm_ioreq_server_alloc_rangesets(s, is_default); +rc = hvm_ioreq_server_alloc_rb_rangesets(s, is_default); And this one. if ( rc ) return rc; @@ -960,7 +948,7 @@ static int hvm_ioreq_server_init(struct hvm_ioreq_server *s, struct domain *d, hvm_ioreq_server_unmap_pages(s, is_default); fail_map: -hvm_ioreq_server_free_rangesets(s, is_default); +
Re: [Xen-devel] PCI Pass-through in Xen ARM - Draft 2.
On Mon, 2015-06-29 at 11:31 +0100, Julien Grall wrote: Hi Manish, On 28/06/15 19:38, Manish Jaggi wrote: 4.1 Holes in guest memory space Holes are added in the guest memory space for mapping pci device's BAR regions. These are defined in arch-arm.h /* For 32bit */ GUEST_MMIO_HOLE0_BASE, GUEST_MMIO_HOLE0_SIZE /* For 64bit */ GUEST_MMIO_HOLE1_BASE , GUEST_MMIO_HOLE1_SIZE The memory layout for 32bit and 64bit are exactly the same. Why do you need to differ here? I took this to be a hole under 4GB and a hole over 4GB IOW rather than specific to 32-bit/64-bit guests it is specific to 32/64 bit devices, the latter of which could be placed higher in RAM, which is useful since the space under 4GB is naturally more limited. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v3 05/18] xen/arm: ITS: Port ITS driver to xen
On Mon, 2015-06-22 at 17:31 +0530, vijay.kil...@gmail.com wrote: [...] +/* + * ITS command descriptors - parameters to be encoded in a command + * block. + */ +struct its_cmd_desc { +union { +struct { +struct its_collection *col; +u32 event_id; +u32 dev_id; +} its_inv_cmd; [...] +static struct its_collection *its_build_inv_cmd(its_cmd_block *cmd, +struct its_cmd_desc *desc) +{ +memset(cmd, 0x0, sizeof(its_cmd_block)); +cmd-inv.cmd = GITS_CMD_INV; +cmd-inv.devid = desc-its_inv_cmd.dev_id; +cmd-inv.event = desc-its_inv_cmd.event_id; + +#ifdef DEBUG_GIC_ITS +dump_cmd(cmd); +#endif + +return desc-its_inv_cmd.col; +} [...] +void its_send_inv(struct its_device *dev, struct its_collection *col, + u32 event_id) +{ +struct its_cmd_desc desc; + +desc.its_inv_cmd.dev_id = dev-device_id; +desc.its_inv_cmd.event_id = event_id; +desc.its_inv_cmd.col = col; + +its_send_single_command(dev-its, its_build_inv_cmd, desc); +} [...] +typedef struct __packed { +u64 cmd:8; +u64 res1:24; +u64 devid:32; +u64 event:32; +u64 res2:32; +u64 res3:64; +u64 res4:64; +}inv_cmd_t; (I've trimmed this to just the INV command, but it's the same for all of them) I suppose this is a mix of the way the Linux code was structured and my request to use a struct/union to encode these things, but I'm afraid this is not how I intended to suggest things be done. What I expected was something analogous to the hsr or lpae_t types, e.g. a single: union its_cmd { uint64_t bits[N]; struct { uint8_t cmd; uint8_t pad[...]; } hdr; struct { uint8_t cmd; uint8_t res1[3]; uint32_t devid; uint32_t event; uint64_t res2[2]; } inv; }; So its_send_single_command can take a union its_cmd * and its_send_inv should look like: void its_send_inv(struct its_device *dev, struct its_collection *col, u32 event_id) { union its_cmd cmd; /* memset perhaps, or sets .bits = {0,} */ cmd.inv.cmd = GITS_CMD_INV; cmd.inv.dev_id = dev-device_id; cmd.inv.event_id = event_id; cmd.inv.col = col; return its_send_single_command(dev-its, cmd); } I've omitted the its_ prefix and _cmd/_desc suffix where they aren't needed in the context they are used. (so not cmd.cmd_inv etc). I've also used proper types where possible instead of bitfields of u64 (although unsigned long bitfields should still be used for sub 8-bit fields). The hdr member of the union should contain any field which is global to all commands and which generic code (i.e. which isn't aware which command is in the cmd in its hand) can use. Off the top of my head that is just the cmd code itself. You should also consider doing the collection sync in the caller as appropriate instead of pushing it down into its_send_single_command. IMHO its_send_single_command should do just that, not optionally do some other command too. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/7] xen: get rid of the SEDF scheduler
On 26/06/15 17:19, Dario Faggioli wrote: --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -324,7 +324,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); /* XEN_DOMCTL_scheduler_op */ /* Scheduler types. */ -#define XEN_SCHEDULER_SEDF 4 Please instead use: /* #define XEN_SCHEDULER_SEDF 4 (Removed) */ To help keep bits of the history, and reduce the likelyhood of accidental reuse. #define XEN_SCHEDULER_CREDIT 5 #define XEN_SCHEDULER_CREDIT2 6 #define XEN_SCHEDULER_ARINC653 7 @@ -337,13 +336,6 @@ struct xen_domctl_scheduler_op { uint32_t sched_id; /* XEN_SCHEDULER_* */ uint32_t cmd; /* XEN_DOMCTL_SCHEDOP_* */ union { -struct xen_domctl_sched_sedf { -uint64_aligned_t period; -uint64_aligned_t slice; -uint64_aligned_t latency; -uint32_t extratime; -uint32_t weight; -} sedf; struct xen_domctl_sched_credit { uint16_t weight; uint16_t cap; diff --git a/xen/include/public/trace.h b/xen/include/public/trace.h index 5211ae7..36be196 100644 --- a/xen/include/public/trace.h +++ b/xen/include/public/trace.h @@ -75,7 +75,6 @@ /* Per-scheduler IDs, to identify scheduler specific events */ #define TRC_SCHED_CSCHED 0 #define TRC_SCHED_CSCHED2 1 -#define TRC_SCHED_SEDF 2 Same here. ~Andrew #define TRC_SCHED_ARINC653 3 #define TRC_SCHED_RTDS 4 diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h index 7cc25c6..dbe7cab 100644 --- a/xen/include/xen/sched-if.h +++ b/xen/include/xen/sched-if.h @@ -165,7 +165,6 @@ struct scheduler { void (*tick_resume) (const struct scheduler *, unsigned int); }; -extern const struct scheduler sched_sedf_def; extern const struct scheduler sched_credit_def; extern const struct scheduler sched_credit2_def; extern const struct scheduler sched_arinc653_def; ___ 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] [RFC PATCH v3 08/18] xen/arm: vITS: Add virtual ITS driver
On Mon, 2015-06-22 at 17:31 +0530, vijay.kil...@gmail.com wrote: +if ( !p2m_is_ram(p2mt) ) +{ +put_page(page); +dprintk(XENLOG_G_ERR, vITS:d%dv%d with wrong attributes\n, +d-domain_id, current-vcpu_id); d%dv%d should be done (everywhere) with the %pv format code passing current as the argument. See docs/misc/printk-formats.txt. +return -EINVAL; +return -EINVAL; +} + +p = __map_domain_page(page); +/* Offset within the mapped page */ +offset = dt_entry (PAGE_SIZE - 1); This should be ~PAGE_MASK please. + +if ( set ) +memcpy(p + offset, entry, sizeof(struct vdevice_table)); +else +memcpy(entry, p + offset, sizeof(struct vdevice_table)); Personally I'd have done this with *entry = *(struct ..*)(p + offset) and vice versa and let the compiler figure out how to achieve that. +/* dt_entry is validated when read */ The vits_get_vdevice_entry call is right above and in this implementation I don't see any checks in that function, so I don't think this comment is strictly true. Since it is very important that information living in this guest accessible memory is correctly validated before use I think these comments need to be 100% trustworthy. I think vits_get_vdevice_entry is indeed the right place for those checks as the comment suggests. If there are no actual checks needed (e.g. because everything is in terms of IPA) then there should be a comment in that function explaining exactly why nothing actually needs to be validated. +offset = event * sizeof(struct vitt); +if ( offset dt_entry.vitt_size ) +{ +dprintk(XENLOG_G_ERR, vITS:d%dv%d ITT out of range\n, +d-domain_id, current-vcpu_id); +return -EINVAL; ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 58964: tolerable FAIL - PUSHED
flight 58964 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/58964/ Failures :-/ but no regressions. Tests which did not succeed, including tests which could not be run: test-amd64-i386-libvirt-xsm 11 guest-start fail baseline untested test-amd64-i386-libvirt 11 guest-start fail baseline untested test-amd64-amd64-libvirt 11 guest-start fail baseline untested test-amd64-i386-xl6 xen-bootfail baseline untested test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-libvirt-xsm 11 guest-start fail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail never pass jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-armhf-armhf-xl pass test-amd64-i386-xl fail test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-libvirt-xsm fail test-armhf-armhf-libvirt-xsm pass test-amd64-i386-libvirt-xsm fail test-amd64-amd64-xl-xsm pass test-armhf-armhf-xl-xsm pass test-amd64-i386-xl-xsm pass test-amd64-amd64-xl-pvh-amd fail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-i386-freebsd10-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-armhf-armhf-xl-arndale pass test-amd64-amd64-xl-credit2 pass test-armhf-armhf-xl-credit2 pass test-armhf-armhf-xl-cubietruck pass test-amd64-i386-freebsd10-i386 pass test-amd64-amd64-xl-pvh-intelfail test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-libvirt fail test-armhf-armhf-libvirt pass test-amd64-i386-libvirt fail test-amd64-amd64-xl-multivcpupass test-armhf-armhf-xl-multivcpupass test-amd64-amd64-pairpass test-amd64-i386-pair pass
Re: [Xen-devel] [linux-3.4 test] 58961: regressions - FAIL
On Mon, 2015-06-29 at 06:55 +, osstest service user wrote: flight 58961 linux-3.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/58961/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemut-win7-amd64 6 xen-boot fail REGR. vs. 30511 From http://logs.test-lab.xenproject.org/osstest/logs/58961/test-amd64-amd64-xl-qemut-win7-amd64/serial-chardonnay1.log Jun 29 03:26:17.341011 [8.856477] ata7.00: ATA-8: WDC WD1003FBYZ-010FB0, 01.01V03, max UDMA/133 Jun 29 03:26:17.757027 [8.856497] ata7.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 31/32) Jun 29 03:26:17.765028 [8.857962] ata7.00: failed to IDENTIFY (device reports invalid type, err_mask=0x0) Jun 29 03:26:17.773040 [8.857971] ata7.00: revalidation failed (errno=-22) Jun 29 03:26:17.781011 [8.857979] ata7.00: limiting speed to UDMA/133:PIO3 Jun 29 03:26:17.781031 [ 14.013757] ata7.00: failed to IDENTIFY (device reports invalid type, err_mask=0x0) Jun 29 03:26:22.917074 [ 14.013776] ata7.00: revalidation failed (errno=-22) Jun 29 03:26:22.925002 [ 14.013783] ata7.00: disabled Jun 29 03:26:22.925054 Begin: Loading essential drivers ... done. Jun 29 03:26:33.013122 Begin: Running /scripts/init-premount ... done. Jun 29 03:26:33.013176 Begin: Mounting root file system ... Begin: Running /scripts/local-top ... Volume group chardonnay1 not found Jun 29 03:26:33.053058 Skipping volume group chardonnay1 Jun 29 03:26:33.053094 Unable to find LVM volume chardonnay1/root Jun 29 03:26:33.061015 Volume group chardonnay1 not found Jun 29 03:26:33.085071 Skipping volume group chardonnay1 Jun 29 03:26:33.093029 Unable to find LVM volume chardonnay1/swap_1 Jun 29 03:26:33.093064 done. Jun 29 03:26:33.093088 Begin: Waiting for root file system ... done. Jun 29 03:26:43.309116 Gave up waiting for root device. Common problems: Jun 29 03:26:43.309152 - Boot args (cat /proc/cmdline) Jun 29 03:26:43.317046- Check rootdelay= (did the system wait long enough?) Jun 29 03:26:43.317085- Check root= (did the system wait for the right device?) Jun 29 03:26:43.325043 - Missing modules (cat /proc/modules; ls /dev) Jun 29 03:26:43.333036 ALERT! /dev/mapper/chardonnay1-root does not exist. Dropping to a shell! http://logs.test-lab.xenproject.org/osstest/results/host/chardonnay1.html suggests this is specific to Linux 3.4 and not e.g. a failing disk (chardonnay0 has some similar fails in the same flight). Unfortunately due to 'don't bugger nd-seq seems to break umount sometimes'[0] and possibly 'config: Enable NEED_DMA_MAP_STATE by default when SWIOTLB is selected'[1] hitting 3.4.107 (under test here is 3.4.108, our baseline is 104) I suspect the bisector won't be able to find a basis pass which doesn't end up blaming one of those first. I've had a google for the message, but no hints. Ian. [0] 20150429122139.gn...@zeniv.linux.org.uk [1] 1434443826-4929-165-git-send-email-l...@kernel.org Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-sedf-pin 6 xen-boot fail in 58831 pass in 58798 test-amd64-amd64-pair10 xen-boot/dst_host fail pass in 58798 test-amd64-amd64-pair 9 xen-boot/src_host fail pass in 58798 test-amd64-i386-pair 10 xen-boot/dst_host fail pass in 58831 test-amd64-i386-pair 9 xen-boot/src_host fail pass in 58831 test-amd64-amd64-xl 6 xen-bootfail pass in 58941 test-amd64-i386-xl-qemuu-win7-amd64 9 windows-install fail pass in 58955 Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail baseline untested test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail baseline untested test-amd64-i386-libvirt-xsm 6 xen-bootfail baseline untested test-amd64-amd64-xl-multivcpu 6 xen-boot fail baseline untested test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 6 xen-boot fail baseline untested test-amd64-amd64-libvirt-xsm 6 xen-bootfail baseline untested test-amd64-amd64-xl-credit2 6 xen-bootfail baseline untested test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 6 xen-boot fail baseline untested test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 6 xen-boot fail baseline untested test-amd64-amd64-xl-sedf 6 xen-boot fail in 58831 like 30406 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail in 58955 baseline untested test-amd64-i386-libvirt 11 guest-start fail like 30511 test-amd64-amd64-libvirt 11 guest-start fail like 30511 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 30511 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop
[Xen-devel] [v3] nested EPT: fix the handling of nested EPT
If the host EPT entry is changed, the nested EPT should be updated. the current code does not do this, and it's wrong. I have tested this patch, the L2 guest can boot and run as normal. Signed-off-by: Liang Li liang.z...@intel.com Signed-off-by: Yang Zhang yang.z.zh...@intel.com Reported-by: Tim Deegan t...@xen.org Reviewed-by: Tim Deegan t...@xen.org --- xen/arch/x86/mm/p2m-ept.c | 4 xen/arch/x86/mm/p2m.c | 6 ++ 2 files changed, 10 insertions(+) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index 5133eb6..a28c6eb 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -26,6 +26,7 @@ #include asm/p2m.h #include asm/hvm/vmx/vmx.h #include asm/hvm/vmx/vmcs.h +#include asm/hvm/nestedhvm.h #include xen/iommu.h #include asm/mtrr.h #include asm/hvm/cacheattr.h @@ -1076,6 +1077,9 @@ void ept_sync_domain(struct p2m_domain *p2m) ASSERT(local_irq_is_enabled()); +if ( nestedhvm_enabled(d) !p2m_is_nestedp2m(p2m) ) +p2m_flush_nestedp2m(d); + /* * Flush active cpus synchronously. Flush others the next time this domain * is scheduled onto them. We accept the race of other CPUs adding to diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 1fd1194..b171491 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1742,6 +1742,12 @@ p2m_flush_table(struct p2m_domain *p2m) ASSERT(page_list_empty(p2m-pod.super)); ASSERT(page_list_empty(p2m-pod.single)); +if ( p2m-np2m_base == P2M_BASE_EADDR ) +{ +p2m_unlock(p2m); +return; +} + /* This is no longer a valid nested p2m for any address space */ p2m-np2m_base = P2M_BASE_EADDR; -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PCI Pass-through in Xen ARM - Draft 2.
Hi Manish, On 28/06/15 19:38, Manish Jaggi wrote: 4.1 Holes in guest memory space Holes are added in the guest memory space for mapping pci device's BAR regions. These are defined in arch-arm.h /* For 32bit */ GUEST_MMIO_HOLE0_BASE, GUEST_MMIO_HOLE0_SIZE /* For 64bit */ GUEST_MMIO_HOLE1_BASE , GUEST_MMIO_HOLE1_SIZE The memory layout for 32bit and 64bit are exactly the same. Why do you need to differ here? 4.2 New entries in xenstore for device BARs toolkit also updates the xenstore information for the device (virtualbar:physical bar). This information is read by xenpciback and returned to the pcifront driver configuration space accesses. Can you details what do you plan to put in xenstore and how? What about the expansion ROM? 4.3 Hypercall for bdf mapping notification to xen --- #define PHYSDEVOP_map_sbdf 43 typedef struct { u32 s; u8 b; u8 df; u16 res; } sbdf_t; struct physdev_map_sbdf { int domain_id; sbdf_tsbdf; sbdf_tgsbdf; }; Each domain has a pdev list, which contains the list of all pci devices. The pdev structure already has a sbdf information. The arch_pci_dev is updated to contain the gsbdf information. (gs- guest segment id) Whenever there is trap from guest or an interrupt has to be injected, the pdev list is iterated to find the gsbdf. Can you give more background for this section? i.e: - Why do you need this? - How xen will translate the gbdf to a vDeviceID? - Who will call this hypercall? - Why not setting the gsbdf when the device is assigned? Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Stop testing SEDF almost at all (at least until Xen 4.2) [was: Re: [PATCH v2] OSSTest: stop testing SEDF, start testing RTDS]
On 06/26/2015 04:24 PM, Ian Jackson wrote: Dario Faggioli writes (Stop testing SEDF almost at all (at least until Xen 4.2) [was: Re: [PATCH v2] OSSTest: stop testing SEDF, start testing RTDS]): However, I honestly think that testing SEDF for earlier versions than xen-unstable (at least until 4.2) is also just a waste of test resources. ... Therefore, I'd argue for the attached patch, [...] With my osstest maintainer hat on the aim of this patch is fine by me. But it seems that sedf is not coming back, so if we are going to do this to osstest we should probably rip the code out. So I would welcome a followup which got rid of all the leftover traces in make-flight and allow.all. (`git-grep -i sedf'.) As to the substance, I would like to take advice from the relevant HV maintainers about this change. I agree with Dario that there's no point in testing it, and with you that we might as well rip the code out of osstest. With my tools maintainer hat on, we should consider removing the sedf-specific code from libxl. With the obvious caveat that old code with SEDF macros and such still needs to compile but return a runtime error if people try to use it. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen-unstable: pci-passthrough of device using MSI-X interrupts not working after commit x86/MSI: track host and guest masking separately
On Fri, 26 Jun 2015, Jan Beulich wrote: On 26.06.15 at 17:04, jbeul...@suse.com wrote: On 26.06.15 at 15:38, li...@eikelenboom.it wrote: On 2015-06-26 14:41, Jan Beulich wrote: On 26.06.15 at 13:02, li...@eikelenboom.it wrote: Strange, i don't see *any* of your printk's being hit ... (xl dmesg attached). So does the guest (in the working case) use MSI-X at all for the device? I.e. it might be worth comparing the guest's /proc/interrupts from both cases, as the lack of any of the debug messages clearly suggests that such interrupts aren#t being set up. In the good case it uses one of them. (probably one per port and it has only one usb device connected at present) -- Sander CPU0 CPU1 CPU2 CPU3 0: 42 0 0 0 IO-APIC 2-edge timer [...] 83: 8 0 0 0 xen-dyn-event eth0-q3-rx 84: 2101 0 0 0 xen-pirq-msi-x xhci_hcd I think this explains it - you're running in PVHVM mode, which I never tried with those patches. I'd even have to go dig to see how they drive MSI-X in the first place in that case. I have an idea: In static unsigned int startup_msi_irq(struct irq_desc *desc) { bool_t guest_masked = (desc-status IRQ_GUEST) is_hvm_domain(desc-msi_desc-dev-domain); if ( unlikely(!msi_set_mask_bit(desc, 0, guest_masked)) ) WARN(); return 0; } I think we need to also exclude the emuirq case (which is what I understand backs the pvhvm interrupt in the guest - Stefano, please confirm). For testing purposes, could you try simply passing zero instead of guest_masked here? emuirq is an irq (or msi) that is generated by an emulated device. Obviously a guest cannot tell whether a device is emulated or passthrough, as a consequence when the guest remaps irqs (and msis) into event channels (arch/x86/pci/xen.c:acpi_register_gsi_xen_hvm and arch/x86/pci/xen.c:xen_hvm_setup_msi_irqs), some of the irqs (and msis) might actually be emulated. Xen uses the concept of emuirq to distinguish irqs (and msis) of a passthrough device from those belonging to an emulated device. However you should know that in a default configuration, without virtio, there are no emulated msis. Maybe emuirqs are not really the problem here. In the case of passthrough devices that generate MSI-X, the guest remaps MSIs into event channels by writing a magic number and the desired pirq in the MSI-X table (xen_hvm_setup_msi_irqs), then QEMU retrieves the pirq and calls xc_physdev_map_pirq_msi friends on its behalf. Afterwards Linux uses the regular xen_pirq_chip functions to enable/disable the pirq. Specifically masking the MSI, is done by masking the event channel. In the quoted function above, if the issue is finding out whether the msi is masked from the guest point of view, then you also need to check if it has been remapped into an event channel (you can use domain_irq_to_pirq for that), then check its status. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v3 07/18] xen/arm: ITS: implement hw_irq_controller for LPIs
On Tue, 2015-06-23 at 15:32 +0100, Julien Grall wrote: [...] +{ +struct its_collection *col; +struct its_device *its_dev = get_irq_device(desc); +u8 *cfg; +u32 virq = irq_to_virq(desc); + +ASSERT(virq its_dev-nr_lpis); + +cfg = gic_rdists-prop_page + desc-irq - NR_GIC_LPI; +if ( enable ) +*cfg |= LPI_PROP_ENABLED; +else +*cfg = ~LPI_PROP_ENABLED; + +/* + * Make the above write visible to the redistributors. + * And yes, we're flushing exactly: One. Single. Byte. + * Humpf... + */ +if ( gic_rdists-flags RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING ) +clean_and_invalidate_dcache_va_range(cfg, sizeof(*cfg)); +else +dsb(ishst); + +/* Get collection id for this event id */ +col = its_dev-its-collections[virq % num_online_cpus()]; This is fragile, you are assuming that num_online_cpus() will never change. Why don't you store the collection in every irq_desc? The original Linux code upon which this is based doesn't seem to need to lookup the collection here, why is flushing needed for us but not Linux? I'm also confused by the use of the variable name virq in a function called set_lpi_config which appears to be dealing with host level physical LPIs. It seems like this function would be broken for LPIs which were delivered to Xen and not to a guest, and that the irq_to_virq in here ought to be desc-irq, no? BTW, the Linux original calls what this calls virq id instead, which is much less confusing. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v3 09/18] xen/arm: ITS: Add virtual ITS commands support
On Mon, 2015-06-22 at 17:31 +0530, vijay.kil...@gmail.com wrote: +static int vgic_its_process_mapvi(struct vcpu *v, struct vgic_its *vits, + its_cmd_block *virt_cmd) +{ +struct vitt entry; +struct vits_device *vdev; +uint8_t vcol_id, cmd; +uint32_t vid, dev_id, event; + +vcol_id = virt_cmd-mapvi.col; +vid = virt_cmd-mapvi.phy_id; +dev_id = its_decode_devid(v-domain, virt_cmd); If you used the union its_cmd I proposed earlier for the virt_cmd argument then this would just be virt_command-mapvi.devid. [...] +static int vgic_its_parse_its_command(struct vcpu *v, struct vgic_its *vits, + its_cmd_block *virt_cmd) +{ +uint8_t cmd = its_decode_cmd(virt_cmd); and this would be cirt_cmd-hdr.cmd. [...] +memcpy(virt_cmd, p + offset, sizeof(its_cmd_block)); and this would be memcpy(virt_cmd.bits[0], p+offset, sizeof(union its_cmd)); Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v3 10/18] xen/arm: ITS: Add APIs to add and assign device
On Wed, 2015-06-24 at 12:38 +0100, Julien Grall wrote: [...] @@ -459,7 +475,7 @@ void its_send_inv(struct its_device *dev, struct its_collection *col, its_send_single_command(dev-its, its_build_inv_cmd, desc); } -void its_send_mapd(struct its_device *dev, int valid) +static void its_send_mapd(struct its_device *dev, int valid) I would prefer to see this static where the function has been introduced and delay the compilation until the end. Yes, please. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [ovmf test] 58956: regressions - FAIL
On Sun, 2015-06-28 at 22:32 +, osstest service user wrote: flight 58956 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/58956/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-win7-amd64 9 windows-installfail REGR. vs. 58919 http://logs.test-lab.xenproject.org/osstest/results/bisect/ovmf/test-amd64-i386-xl-qemuu-win7-amd64.windows-install.html smells like something in the range: $ git log --oneline 495ee9b85..79d274b8b 79d274b OvmfPkg: PlatformPei: invert MTRR setup in QemuInitializeRam() cfc80e2 OvmfPkg: PlatformPei: beautify memory HOB order in QemuInitializeRam() 86a14b0 OvmfPkg: PlatformPei: create the CPU HOB with dynamic memory space width bc89fe4 OvmfPkg: PlatformPei: enable larger permanent PEI RAM has made windows installs less reliable. From http://logs.test-lab.xenproject.org/osstest/results/history/test-amd64-i386-xl-qemuu-win7-amd64/ovmf.html they mostly seem to be okay, or else we've gotten unluckY 3 in a row. Ian, is the stickiness of the guest-stop (which is now an allowed fail again) a coincidence or should we do something to unstick? Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 58919 version targeted for testing: ovmf 79d274b8b6b113248661c18f31c4be03c7da32de baseline version: ovmf 495ee9b85141dd9b65434d677b3a685fe166128d People who touched revisions under test: Laszlo Ersek ler...@redhat.com Maoming maoming.maom...@huawei.com Wei Liu wei.l...@citrix.com jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass test-amd64-amd64-xl-qemuu-winxpsp3 pass test-amd64-i386-xl-qemuu-winxpsp3pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 79d274b8b6b113248661c18f31c4be03c7da32de Author: Laszlo Ersek ler...@redhat.com Date: Fri Jun 26 16:09:52 2015 + OvmfPkg: PlatformPei: invert MTRR setup in QemuInitializeRam() At the moment we work with a UC default MTRR type, and set three memory ranges to WB: - [0, 640 KB), - [1 MB, LowerMemorySize), - [4 GB, 4 GB + UpperMemorySize). Unfortunately, coverage for the third range can fail with a high likelihood. If the alignment of the base (ie. 4 GB) and the alignment of the size (UpperMemorySize) differ, then MtrrLib creates a series of variable MTRR entries, with power-of-two sized MTRR masks. And, it's really easy to run out of variable MTRR entries, dependent on the alignment difference. This is a problem because a Linux guest will loudly reject any high memory that is not covered my MTRR. So, let's follow the inverse pattern (loosely inspired by SeaBIOS): - flip the MTRR default type to WB, - set [0, 640 KB) to WB -- fixed MTRRs have precedence over the default type and variable MTRRs, so we can't avoid this, - set [640 KB, 1 MB) to UC -- implemented with fixed MTRRs, - set
Re: [Xen-devel] [PATCH v2] OSSTest: stop testing SEDF, start testing RTDS
On Mon, 2015-06-29 at 09:53 +0100, Ian Campbell wrote: On Sat, 2015-06-27 at 12:05 -0700, Meng Xu wrote: I want/hope to know when/how the RTDS scheduler fails the test so that I can also contribute to fix the issue. I think other than keeping an eye on the reposts on the reports on the list the only way would be to setup some filters which looked at the report and noticed rtds failures, the mails are pretty structured so if you are only interested in the rtds stuff that's likely not too hard. Thanks Ian. I was about to reply myself, basically saying the same thing. Meng, this is, as an example, one of the tests report: http://logs.test-lab.xenproject.org/osstest/logs/58938/ And the mails you should look for are the ones similar to this: [Xen-devel] [xen-unstable test] 58938: regressions - trouble: blocked/broken/fail/pass http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg04628.html (the subject changes a bit, depending on the actual tests results, but that's the patter. If something starts failing repeatedly then usually someone will end up CCing the appropriate maintainer. Yeah, I'll certainly keep an eye out for RTDS stuff going on in OSSTest and, at some point, I and/or George will be notified anyway, and will involve you, if that is what's necessary. However, if you also get used to look at the reports, we certainly will be able to identify and fix issues quickly, so feel free. :-D 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] Hypervisor can get PV guest's value?
On Sat, 2015-06-27 at 03:36 -0700, nicelhc13 wrote: I want to use guest's variables on Xen hypervisor. But I don't know how to do exactly. First, I was thinking that I could use grant table but grant-table is not related to this issue(cause grant-table is used inter-domain) Is there any way to get PV guest's variables? or is there no solution? Xen can certainly map and access a PV guest's memory but it generally doesn't have access to the guest symbol table to find individual variables without being explicitly told about them. I think it would be best if you described your end goal (i.e. what variables you think you want to access and why) so that people on this list can advise on the best approach to take. Ian. Thanks. -- View this message in context: http://xen.1045712.n5.nabble.com/Hypervisor-can-get-PV-guest-s-value-tp5727837.html Sent from the Xen - Dev mailing list archive at Nabble.com. ___ 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] xen/events/fifo: Consume unprocessed events when a CPU dies
On 06/19/2015 05:06 PM, David Vrabel wrote: On 19/06/15 17:02, Boris Ostrovsky wrote: On 06/19/2015 11:15 AM, Ross Lagerwall wrote: When a CPU is offlined, there may be unprocessed events on a port for that CPU. If the port is subsequently reused on a different CPU, it could be in an unexpected state with the link bit set, resulting in interrupts being missed. Fix this by consuming any unprocessed events for a particular CPU when that CPU dies. Signed-off-by: Ross Lagerwall ross.lagerw...@citrix.com --- drivers/xen/events/events_fifo.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c index 417415d..1dd0ba12 100644 --- a/drivers/xen/events/events_fifo.c +++ b/drivers/xen/events/events_fifo.c @@ -281,7 +281,8 @@ static void handle_irq_for_port(unsigned port) static void consume_one_event(unsigned cpu, struct evtchn_fifo_control_block *control_block, - unsigned priority, unsigned long *ready) + unsigned priority, unsigned long *ready, + bool drop) { struct evtchn_fifo_queue *q = per_cpu(cpu_queue, cpu); uint32_t head; @@ -313,13 +314,17 @@ static void consume_one_event(unsigned cpu, if (head == 0) clear_bit(priority, ready); -if (evtchn_fifo_is_pending(port) !evtchn_fifo_is_masked(port)) -handle_irq_for_port(port); +if (evtchn_fifo_is_pending(port) !evtchn_fifo_is_masked(port)) { +if (unlikely(drop)) +pr_warn(Dropping pending event for port %u\n, port); Maybe pr_info (or pr_notice)? We want a warning here because we think this shouldn't happen -- if it does we actually need to retrigger the event on its new CPU. Also, why not do this (testing for unprocessed events) in xen_evtchn_close()? We can't do anything about them when closing because they may be in the middle of a queue. David Ping. Is this change OK? -- Ross Lagerwall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V5 2/7] libxl_read_file_contents: add new entry to read sysfs file
Chun Yan Liu writes (Re: [PATCH V5 2/7] libxl_read_file_contents: add new entry to read sysfs file): On 6/26/2015 at 09:05 PM, in message 21901.20009.85407.581...@mariner.uk.xensource.com, Ian Jackson ian.jack...@eu.citrix.com wrote: But please feel free to explain why I'm wrong. You are right. I'm confused about the original logic. The original code doesn't consider this case (size bigger than requested) at all. Indeed. The original code assumes that files don't change size, and only detects them shrinking because that causes a short read which it has deal with somehow. But if we are intending to use this with sysfs files, where the reported size is known to be wrong, it seems to me that we should be more proactive. So, we need to add a check if (rs == datalen read end of file), if not, means bigger than requested, report error. To detect a growing file it will be necessary to actually attempt to read at least one byte more than expected. This is probably done most conveniently by making the buffer one byte bigger, too. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH OSSTEST v1] mg-all-branch-statuses: Show how up to date each branch is
On Mon, 2015-06-29 at 15:15 +0100, Ian Jackson wrote: Ian Campbell writes (Re: [PATCH OSSTEST v1] mg-all-branch-statuses: Show how up to date each branch is): On Mon, 2015-06-29 at 13:25 +0100, Ian Jackson wrote: It's probably fairly straightforward to search through the database or history to find some relevant date. What do want to know ? 1. `Commit' timestamp of last pass ? 2. `Commit' timestamp of first commit after last pass ? 3. `Commit' timestamp of tip ? 2 sounds tricky, especially since there could potentially be several. Yes. Although `earliest-dated nontrivial descendant of the last pass commit' is unique, it ... In general commit timestamps can be a bit misleading, since various things preserve them even over long periods of time before they are published and they don't necessarily correspond at all with when something became visible in the tree one is looking at. ... is likely to exacerbate this problem. (Although I did say `commit timestamp', which in git is a separate timestamp updated by rebase, cherry pick, etc., as opposed to `author timestamp'.) I knew there was two timestamps but not that they differed in this way, so good. Maybe better than nothing though. 4. Start date of last flight for current baseline ? 5. Start date of first flight after last flight for current baseline ? 6. Start date of first flight for current tip ? 5 ? You mean out of all 6 options you think 5 is all we need? Yes. The date of the of the flight which established the current baseline seems like a no-brainer (I suppose that is #4). That doesn't tell us how out of date it is. If the tree is updated rarely but the tip was updated yesterday it might look like the test has been failing for months. Ah yes, that's a useful way of thinking about things which I wasn't doing... I'm not really sure what date of a flight relating to tip would be most useful, first try of something newer (#5?) vs most recent attempt (#6). The former tells you how long it has been failing. Right and is therefore much more interesting. Maybe even the _number_ of bites of the cherry would be useful? Yes. You'd want both `number of times _this version_ has been tried' and `total number of flights since last pass'. True. Perhaps I should make a library function in Osstest::Executive which provides this date and count. After all they would be useful in the email reports. That would be good, thanks. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vm_event: toggle singlestep from vm_event response
On 29/06/15 15:17, Lengyel, Tamas wrote: On Mon, Jun 29, 2015 at 10:09 AM, Lengyel, Tamas tleng...@novetta.com mailto:tleng...@novetta.com wrote: On Mon, Jun 29, 2015 at 9:55 AM, Andrew Cooper andrew.coop...@citrix.com mailto:andrew.coop...@citrix.com wrote: On 29/06/15 14:42, Lengyel, Tamas wrote: --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -6431,6 +6431,14 @@ int hvm_debug_op(struct vcpu *v, int32_t op) return rc; } +void hvm_toggle_singlestep(struct vcpu *v) +{ +if ( !cpu_has_monitor_trap_flag ) monitor_trap_flag is a VMX feature. This will never be true on AMD systems. (its use in hvm_debug_op() is also dubious) Yes, this feature is only for Intel cpus. Reworking the use of the flag though is a bit out-of-scope for this patch itself. In which case you must make it very clear in the commit message that this is for Intel only and needs fixing up for AMD. Best also to have a comment to the same effect in this function. Sure. +return; + +v-arch.hvm_vcpu.single_step = !v-arch.hvm_vcpu.single_step; +} + int nhvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs) { if (hvm_funcs.nhvm_vcpu_hostrestore) diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c new file mode 100644 index 000..95b30ad --- /dev/null +++ b/xen/arch/x86/vm_event.c @@ -0,0 +1,41 @@ +/* + * arch/x86/vm_event.c + * + * Architecture-specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com mailto:ta...@tklengyel.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#include xen/sched.h +#include asm/hvm/hvm.h + +void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) +{ +if ( (v == current) || !is_hvm_domain(d) ) Why is 'current' excluded? Only to be consistent with the sanity check applied for XEN_DOMCTL_debug_op. XEN_DOMCTL_debug_op needs the check because of vcpu_pause(). It is always run in the context of the hypercaller domain, and must never end up calling singlestep on itself. However in this case, we can only legitimately use this on an already-paused vcpu, which guarentees that Xen is not currently in the context of the target vcpu. It would probably be better to have a check against the vcpu pause count here (with early return), and hvm_toggle_singlestep() assert so. I guess that's a fair sanity check to add in case the vm_event listener is buggy. I was thinking ASSERT(v-vm_event_pause_count.counter); but that locks the function to only be usable through the vm_event response method. What do you think? You want to use atomic_read() instead of peeking into .counter. vm_event_toggle_singlestep() can legitimately gate on v-vm_event_pause_count, whereas hvm_toggle_singlestep() can get away with looking at v-pause_count. See vmx_domain_emable_pml() as a similar example. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d
On 06/29/15 10:03, Paul Durrant wrote: -Original Message- From: Paul Durrant Sent: 29 June 2015 13:54 To: Paul Durrant; Don Slutz; xen-devel@lists.xen.org; Jan Beulich Subject: RE: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d -Original Message- From: xen-devel-boun...@lists.xen.org [mailto:xen-devel- boun...@lists.xen.org] On Behalf Of Paul Durrant Sent: 29 June 2015 10:42 To: Don Slutz; xen-devel@lists.xen.org; Jan Beulich Subject: Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d -Original Message- From: Don Slutz [mailto:don.sl...@gmail.com] Sent: 27 June 2015 22:02 To: xen-devel@lists.xen.org; Paul Durrant; Jan Beulich Subject: Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d commit 2df1aa01bef7366798248ac6d03cfb42048b003d Author: Paul Durrant paul.durr...@citrix.com Date: Tue Jun 23 18:07:49 2015 +0200 x86/hvm: remove hvm_io_pending() check in hvmemul_do_io() ... -rc = X86EMUL_RETRY; -if ( !hvm_send_assist_req(s, p) ) +rc = hvm_send_assist_req(s, p); +if ( rc != X86EMUL_RETRY ) ... if ( unlikely(!vcpu_start_shutdown_deferral(curr)) ) -return 0; /* implicitly bins the i/o operation */ +return X86EMUL_OKAY; So now X86EMUL_OKAY is returned from hvmemul_do_io() during shutdown. From Jan Beulich about this: Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry. https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=subject:%22Re%5C%3A+%5C%5BXen%5C- devel%5C%5D+%5C%5BPATCH+3%5C%2F5%5C%5D+hvmemul_do_io%5C%3 A+If+the+send+to+the+ioreq+server+failed+do+not+retry.%22o=newest Jan Beulich https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=from:%22Jan+Beulich%22 Fri, 30 Jan 2015 02:24:55 -0800 https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=date:20150130 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. 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. Jan going up the call stack: in handle_pio when hvmemul_do_pio_buffer() returns X86EMUL_OKAY, it returns 1. svm_vmexit_handler 2578 if ( handle_pio(port, bytes, dir) ) or vmx_vmexit_handler 3178 if ( handle_pio(port, bytes, dir) ) both update the IP in this case which is wrong during shutdown. If this is indeed the problem then it seems to me that the correct fix is to add a check in handle_pio() and return 0 if the cpu is shutting down. Actually, with some more thought, it sounds like we essentially want to unwind the I/O emulation if we're shutting down and I think this could be achieved by having hvm_send_assist_req() return X86EMUL_RETRY if it hits the shutdown deferral case, having hvm_emul_do_io() reset the emulation state back to 'none' if the domain is shutting down, and then having handle_pio() return 0 if sees X86EMUL_RETRY without any form of completion being requested. I think this patch should do it for now: diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index a4d7225..cc6130c 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -183,7 +183,7 @@ static int hvmemul_do_io( else { rc = hvm_send_assist_req(s, p); -if ( rc != X86EMUL_RETRY ) +if ( rc != X86EMUL_RETRY || curr-domain-is_shutting_down ) I do not know enough about is_shutting_down to agree. What is clear is that this test is not the same as !vcpu_start_shutdown_deferral(curr). -Don Slutz vio-io_state = HVMIO_none; else if ( data_is_addr || dir == IOREQ_WRITE ) rc = X86EMUL_OKAY; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 8226d8c..aba4ef6 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2547,7 +2547,7 @@ int hvm_send_assist_req(struct hvm_ioreq_server *s, ioreq_t *proto_p) ASSERT(s); if ( unlikely(!vcpu_start_shutdown_deferral(curr)) ) -return X86EMUL_OKAY; +return X86EMUL_RETRY; list_for_each_entry ( sv, s-ioreq_vcpu_list, Paul Paul So I think: rc = hvm_send_assist_req(s, p); if ( rc != X86EMUL_RETRY ) vio-io_state = HVMIO_none; needs to change to: rc = hvm_send_assist_req(s, p); if ( rc != X86EMUL_RETRY ) {
Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d
-Original Message- From: Don Slutz [mailto:don.sl...@gmail.com] Sent: 29 June 2015 16:14 To: Paul Durrant; xen-devel@lists.xen.org; Jan Beulich Subject: Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d On 06/29/15 10:03, Paul Durrant wrote: -Original Message- From: Paul Durrant Sent: 29 June 2015 13:54 To: Paul Durrant; Don Slutz; xen-devel@lists.xen.org; Jan Beulich Subject: RE: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d -Original Message- From: xen-devel-boun...@lists.xen.org [mailto:xen-devel- boun...@lists.xen.org] On Behalf Of Paul Durrant Sent: 29 June 2015 10:42 To: Don Slutz; xen-devel@lists.xen.org; Jan Beulich Subject: Re: [Xen-devel] Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d -Original Message- From: Don Slutz [mailto:don.sl...@gmail.com] Sent: 27 June 2015 22:02 To: xen-devel@lists.xen.org; Paul Durrant; Jan Beulich Subject: Migration bug added by commit 2df1aa01bef7366798248ac6d03cfb42048b003d commit 2df1aa01bef7366798248ac6d03cfb42048b003d Author: Paul Durrant paul.durr...@citrix.com Date: Tue Jun 23 18:07:49 2015 +0200 x86/hvm: remove hvm_io_pending() check in hvmemul_do_io() ... -rc = X86EMUL_RETRY; -if ( !hvm_send_assist_req(s, p) ) +rc = hvm_send_assist_req(s, p); +if ( rc != X86EMUL_RETRY ) ... if ( unlikely(!vcpu_start_shutdown_deferral(curr)) ) -return 0; /* implicitly bins the i/o operation */ +return X86EMUL_OKAY; So now X86EMUL_OKAY is returned from hvmemul_do_io() during shutdown. From Jan Beulich about this: Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq server failed do not retry. https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=subject:%22Re%5C%3A+%5C%5BXen%5C- devel%5C%5D+%5C%5BPATCH+3%5C%2F5%5C%5D+hvmemul_do_io%5C%3 A+If+the+send+to+the+ioreq+server+failed+do+not+retry.%22o=newest Jan Beulich https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=from:%22Jan+Beulich%22 Fri, 30 Jan 2015 02:24:55 -0800 https://www.mail-archive.com/search?l=xen- de...@lists.xen.orgq=date:20150130 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. 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. Jan going up the call stack: in handle_pio when hvmemul_do_pio_buffer() returns X86EMUL_OKAY, it returns 1. svm_vmexit_handler 2578 if ( handle_pio(port, bytes, dir) ) or vmx_vmexit_handler 3178 if ( handle_pio(port, bytes, dir) ) both update the IP in this case which is wrong during shutdown. If this is indeed the problem then it seems to me that the correct fix is to add a check in handle_pio() and return 0 if the cpu is shutting down. Actually, with some more thought, it sounds like we essentially want to unwind the I/O emulation if we're shutting down and I think this could be achieved by having hvm_send_assist_req() return X86EMUL_RETRY if it hits the shutdown deferral case, having hvm_emul_do_io() reset the emulation state back to 'none' if the domain is shutting down, and then having handle_pio() return 0 if sees X86EMUL_RETRY without any form of completion being requested. I think this patch should do it for now: diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index a4d7225..cc6130c 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -183,7 +183,7 @@ static int hvmemul_do_io( else { rc = hvm_send_assist_req(s, p); -if ( rc != X86EMUL_RETRY ) +if ( rc != X86EMUL_RETRY || curr-domain-is_shutting_down ) I do not know enough about is_shutting_down to agree. What is clear is that this test is not the same as !vcpu_start_shutdown_deferral(curr). It pretty much is. If you look at the code for vcpu_start_shutdown_deferral() then you can see that it will only return 0 in the case that the domain's is_shutting_down flag is true. Paul -Don Slutz vio-io_state = HVMIO_none; else if ( data_is_addr || dir == IOREQ_WRITE ) rc = X86EMUL_OKAY; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 8226d8c..aba4ef6 100644 --- a/xen/arch/x86/hvm/hvm.c +++
Re: [Xen-devel] Xen PV IOMMU interface draft C
On 29/06/15 15:52, Ian Campbell wrote: On Mon, 2015-06-29 at 10:40 -0400, Konrad Rzeszutek Wilk wrote: On Fri, Jun 26, 2015 at 12:03:44PM +0100, Ian Campbell wrote: +ARM devs. On Fri, 2015-06-26 at 11:23 +0100, Malcolm Crossley wrote: Hi All, I had a chat with Malcolm about this with respect to ARM. The upshot is that this does not help us to remove the dom0 1:1 workaround or associated swiotlb uses on systems without an SMMU, nor does it allow us to sensibly do passthrough on systems which lack an SMMU. What would? This Xen PV IOMMU interface. I guess Konrad is asking if this PV IOMMU interface doesn't solve these problems, what (other possible interface) would? I guess the answer is a SMMU? David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 COLOPre 03/26] tools/libxl: move domain resume code into libxl_dom_suspend.c
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: move domain resume code into libxl_dom_suspend.c. pure code motion except for making libxl__domain_resume_device_model() static. Signed-off-by: Yang Hongyang yan...@cn.fujitsu.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] race condition in xen-gntdev
On Mon, Jun 29, 2015 at 10:39:26AM -0400, Konrad Rzeszutek Wilk wrote: On Fri, Jun 26, 2015 at 03:28:24AM +0200, Marek Marczykowski-Górecki wrote: On Mon, Jun 22, 2015 at 03:14:16PM -0400, Daniel De Graaf wrote: The reason that gntdev_release didn't have a lock is because there are not supposed to be any references to the areas pointed to by priv-maps when it is called. However, since the MMU notifier has not yet been unregistered, it is apparently possible to race here; the comment on mmu_notifier_unregister seems to confirm this as a possibility (as do the backtraces). I think adding the lock will be sufficient. Ok, so here is the patch: Awesome! Since you are the one who has been seeing this particular fault - any chance you could give it some soak time? If I recall your emails correctly it takes about a week or so before you saw the crash? Sure. I've already installed patched kernel, will report back results later. ---8 From b876e14888bdafa112c3265e6420543fa74aa709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= marma...@invisiblethingslab.com Date: Fri, 26 Jun 2015 02:16:49 +0200 Subject: [PATCH] xen/grant: fix race condition in gntdev_release While gntdev_release is called, MMU notifier is still registered and can traverse priv-maps list even if no pages are mapped (which is the case - gntdev_release is called after all). But gntdev_release will clear that list, so make sure that only one of those things happens at the same time. Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- drivers/xen/gntdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 8927485..4bd23bb 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -568,12 +568,14 @@ static int gntdev_release(struct inode *inode, struct file *flip) pr_debug(priv %p\n, priv); + mutex_lock(priv-lock); while (!list_empty(priv-maps)) { map = list_entry(priv-maps.next, struct grant_map, next); list_del(map-next); gntdev_put_map(NULL /* already removed */, map); } WARN_ON(!list_empty(priv-freeable_maps)); + mutex_unlock(priv-lock); if (use_ptemod) mmu_notifier_unregister(priv-mn, priv-mm); -- 1.9.3 -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? pgpUPyLeHwMcQ.pgp Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 02/12] VMX: implement suppress #VE.
On Mon, Jun 29, 2015 at 3:31 PM, Andrew Cooper andrew.coop...@citrix.com wrote: On 29/06/15 15:20, George Dunlap wrote: On Mon, Jun 22, 2015 at 7:56 PM, Ed White edmund.h.wh...@intel.com wrote: In preparation for selectively enabling #VE in a later patch, set suppress #VE on all EPTE's. Suppress #VE should always be the default condition for two reasons: it is generally not safe to deliver #VE into a guest unless that guest has been modified to receive it; and even then for most EPT violations only the hypervisor is able to handle the violation. Signed-off-by: Ed White edmund.h.wh...@intel.com --- xen/arch/x86/mm/p2m-ept.c | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index a6c9adf..5de3387 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -41,7 +41,7 @@ #define is_epte_superpage(ept_entry)((ept_entry)-sp) static inline bool_t is_epte_valid(ept_entry_t *e) { -return (e-epte != 0 e-sa_p2mt != p2m_invalid); +return ((e-epte ~(1ul 63)) != 0 e-sa_p2mt != p2m_invalid); So just getting up to speed here: Is it the case that if #VE is enabled in vmcs that a #VE will be delivered to the guest on any invalid epte entry that doesn't contain this flag? There is a list of conditions which must be satisfied for a #VE to be injected instead of an EPT related VMexit. All EPT misconfiguration still exit to the hypervisor, but this suppress_ve bit allows the hypervisor to choose to whether a plain EPT permission violation exits to Xen, or injects a #VE. So we now need to actively choose a default which is different than the hardware? By default, setting suppress_ve on everything will cause everything to behave as before. Clearing suppress_ve is an optimisation to avoid a vmexit/vmentry for faults needing bouncing to an in-guest agent. So the short answer is, 'yes': The hardware will deliver #VEs for all non-misconfigured ept entries (which includes entries which are simply not present) unless you actively do something to suppress them; what we want is *not* to deliver #VEs unless the guest actively does something to cause them to be delivered for particular GPAs. Thanks, -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen PV IOMMU interface draft C
On 29/06/15 15:52, Ian Campbell wrote: On Mon, 2015-06-29 at 10:40 -0400, Konrad Rzeszutek Wilk wrote: On Fri, Jun 26, 2015 at 12:03:44PM +0100, Ian Campbell wrote: +ARM devs. On Fri, 2015-06-26 at 11:23 +0100, Malcolm Crossley wrote: Hi All, I had a chat with Malcolm about this with respect to ARM. The upshot is that this does not help us to remove the dom0 1:1 workaround or associated swiotlb uses on systems without an SMMU, nor does it allow us to sensibly do passthrough on systems which lack an SMMU. What would? This Xen PV IOMMU interface. You only get mediated device passthrough on system without IOMMU's. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v3 08/15] Suppress posting interrupts when 'SN' is set
On 24/06/15 06:18, Feng Wu wrote: Currently, we don't support urgent interrupt, all interrupts are recognized as non-urgent interrupt, so we cannot send posted-interrupt when 'SN' is set. Signed-off-by: Feng Wu feng...@intel.com --- v3: use cmpxchg to test SN/ON and set ON xen/arch/x86/hvm/vmx/vmx.c | 32 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 0837627..b94ef6a 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1686,6 +1686,8 @@ static void __vmx_deliver_posted_interrupt(struct vcpu *v) static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector) { +struct pi_desc old, new, prev; These should be moved into the else clause below, to reduce their scope. + if ( pi_test_and_set_pir(vector, v-arch.hvm_vmx.pi_desc) ) return; @@ -1698,13 +1700,35 @@ static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector) */ pi_set_on(v-arch.hvm_vmx.pi_desc); } -else if ( !pi_test_and_set_on(v-arch.hvm_vmx.pi_desc) ) +else { +prev.control = 0; + +do { +old.control = v-arch.hvm_vmx.pi_desc.control + ~(1 POSTED_INTR_ON | 1 POSTED_INTR_SN); Brackets around each of the 1 $NNN please. +new.control = v-arch.hvm_vmx.pi_desc.control | + 1 POSTED_INTR_ON; + +/* + * Currently, we don't support urgent interrupt, all + * interrupts are recognized as non-urgent interrupt, + * so we cannot send posted-interrupt when 'SN' is set. + * Besides that, if 'ON' is already set, we cannot set + * posted-interrupts as well. + */ +if ( prev.sn || prev.on ) +{ +vcpu_kick(v); +return; +} + +prev.control = cmpxchg(v-arch.hvm_vmx.pi_desc.control, + old.control, new.control); +} while ( prev.control != old.control ); + __vmx_deliver_posted_interrupt(v); -return; } - -vcpu_kick(v); This removes a vcpu_kick() from the eoi_exitmap_changed path, which I suspect is not what you intend. ~Andrew } static void vmx_sync_pir_to_irr(struct vcpu *v) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4] x86/arm/mm: use gfn instead of pfn in p2m_get_mem_access/p2m_set_mem_access
'pfn' and 'start_pfn' are ambiguous, both these functions expect GFNs as input. On x86 the interface of p2m_set_mem_access() in p2m.c doesn't match the declaration in p2m-common.h as 'pfn' is being used instead of 'start_pfn'. On ARM both p2m_set_mem_access and p2m_get_mem_access interfaces don't match declarations from p2m-common.h: p2m_set_mem_access uses 'pfn' instead of 'start_pfn' and p2m_get_mem_access uses 'gpfn' instead of 'pfn'. Convert p2m_get_mem_access/p2m_set_mem_access (and __p2m_get_mem_access on ARM) interfaces to using gft_t instead of unsigned long and update all users of these functions. There is also an issue in p2m_get_mem_access on x86: 'gfn' parameter passed to gfn_lock/gfn_unlock is not defined. This code compiles only because of a coincidence: gfn_lock/gfn_unlock are currently macros which don't use their second argument. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com --- Changes since v3: - Comment codying style fix [Razvan Cojocaru] - Use INVALID_GFN instead of ~0 and -1 [Andrew Cooper] - Convert p2m_get_mem_access/p2m_set_mem_access interfaces to using gfn_t [Andrew Cooper] Changes since v2: - Instead of adding start_ prefix on ARM remove it on x86 [Jan Beulich, Ian Campbell, Razvan Cojocaru] Changes since v1: - This patch is a successor of '[PATCH] x86/mm: use existing 'pfn' in p2m_get_mem_access', instead of fixing gfn_lock/gfn_unlock arguments we do s/pfn/gfn/g for both p2m_get_mem_access/p2m_set_mem_access [Andrew Cooper, Jan Beulich] P.S. - The patch was compile-tested on x86 and ARM64. --- xen/arch/arm/p2m.c | 33 + xen/arch/x86/mm/p2m.c| 36 xen/common/mem_access.c | 4 ++-- xen/include/xen/p2m-common.h | 13 ++--- 4 files changed, 45 insertions(+), 41 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 903fa3f..6b9ef33 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -436,7 +436,7 @@ static int p2m_create_table(struct domain *d, lpae_t *entry, return 0; } -static int __p2m_get_mem_access(struct domain *d, unsigned long gpfn, +static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t *access) { struct p2m_domain *p2m = p2m_get_hostp2m(d); @@ -465,14 +465,14 @@ static int __p2m_get_mem_access(struct domain *d, unsigned long gpfn, return 0; } -/* If request to get default access */ -if ( gpfn == ~0ul ) +/* If request to get default access. */ +if ( gfn_x(gfn) == INVALID_GFN ) { *access = memaccess[p2m-default_access]; return 0; } -i = radix_tree_lookup(p2m-mem_access_settings, gpfn); +i = radix_tree_lookup(p2m-mem_access_settings, gfn_x(gfn)); if ( !i ) { @@ -480,7 +480,7 @@ static int __p2m_get_mem_access(struct domain *d, unsigned long gpfn, * No setting was found in the Radix tree. Check if the * entry exists in the page-tables. */ -paddr_t maddr = p2m_lookup(d, gpfn PAGE_SHIFT, NULL); +paddr_t maddr = p2m_lookup(d, gfn_x(gfn) PAGE_SHIFT, NULL); if ( INVALID_PADDR == maddr ) return -ESRCH; @@ -1386,7 +1386,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag) * We do this first as this is faster in the default case when no * permission is set on the page. */ -rc = __p2m_get_mem_access(current-domain, paddr_to_pfn(ipa), xma); +rc = __p2m_get_mem_access(current-domain, _gfn(paddr_to_pfn(ipa)), xma); if ( rc 0 ) goto err; @@ -1590,7 +1590,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec) if ( !p2m-mem_access_enabled ) return true; -rc = p2m_get_mem_access(v-domain, paddr_to_pfn(gpa), xma); +rc = p2m_get_mem_access(v-domain, _gfn(paddr_to_pfn(gpa)), xma); if ( rc ) return true; @@ -1632,13 +1632,13 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec) /* First, handle rx2rw and n2rwx conversion automatically. */ if ( npfec.write_access xma == XENMEM_access_rx2rw ) { -rc = p2m_set_mem_access(v-domain, paddr_to_pfn(gpa), 1, +rc = p2m_set_mem_access(v-domain, _gfn(paddr_to_pfn(gpa)), 1, 0, ~0, XENMEM_access_rw); return false; } else if ( xma == XENMEM_access_n2rwx ) { -rc = p2m_set_mem_access(v-domain, paddr_to_pfn(gpa), 1, +rc = p2m_set_mem_access(v-domain, _gfn(paddr_to_pfn(gpa)), 1, 0, ~0, XENMEM_access_rwx); } @@ -1660,7 +1660,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec) { /* A listener is not required, so clear the access * restrictions. */ -rc = p2m_set_mem_access(v-domain,
[Xen-devel] [PATCH] libxl: Fix uninitialised rc in libxl__domain_save_device_model
c3c8da9 libxl: ao: datacopier callback gets an rc caused libxl__domain_save_device_model() to pass its rc directly into the callback. However in the preexisting code, there were 3 goto out; paths which left rc uninitialised. This causes a build failure with GCC 4.8's -Wmaybe-uninitialized. Set the rc explicitly on each goto out path. Reported-by: Andrew Cooper andrew.coop...@citrix.com Signed-off-by: Ian Jackson ian.jack...@eu.citrix.com Tested-by: Andrew Cooper andrew.coop...@citrix.com --- tools/libxl/libxl_dom.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index bdc0465..1c9418a 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -2190,17 +2190,20 @@ void libxl__domain_save_device_model(libxl__egc *egc, dc-readfd = open(filename, O_RDONLY); if (dc-readfd 0) { LOGE(ERROR, unable to open %s, dc-readwhat); +rc = ERROR_FAIL; goto out; } if (fstat(dc-readfd, st)) { LOGE(ERROR, unable to fstat %s, dc-readwhat); +rc = ERROR_FAIL; goto out; } if (!S_ISREG(st.st_mode)) { LOG(ERROR, %s is not a plain file!, dc-readwhat); +rc = ERROR_FAIL; goto out; } -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] race condition in xen-gntdev
On Fri, Jun 26, 2015 at 03:28:24AM +0200, Marek Marczykowski-Górecki wrote: On Mon, Jun 22, 2015 at 03:14:16PM -0400, Daniel De Graaf wrote: The reason that gntdev_release didn't have a lock is because there are not supposed to be any references to the areas pointed to by priv-maps when it is called. However, since the MMU notifier has not yet been unregistered, it is apparently possible to race here; the comment on mmu_notifier_unregister seems to confirm this as a possibility (as do the backtraces). I think adding the lock will be sufficient. Ok, so here is the patch: Awesome! Since you are the one who has been seeing this particular fault - any chance you could give it some soak time? If I recall your emails correctly it takes about a week or so before you saw the crash? ---8 From b876e14888bdafa112c3265e6420543fa74aa709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= marma...@invisiblethingslab.com Date: Fri, 26 Jun 2015 02:16:49 +0200 Subject: [PATCH] xen/grant: fix race condition in gntdev_release While gntdev_release is called, MMU notifier is still registered and can traverse priv-maps list even if no pages are mapped (which is the case - gntdev_release is called after all). But gntdev_release will clear that list, so make sure that only one of those things happens at the same time. Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- drivers/xen/gntdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 8927485..4bd23bb 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -568,12 +568,14 @@ static int gntdev_release(struct inode *inode, struct file *flip) pr_debug(priv %p\n, priv); + mutex_lock(priv-lock); while (!list_empty(priv-maps)) { map = list_entry(priv-maps.next, struct grant_map, next); list_del(map-next); gntdev_put_map(NULL /* already removed */, map); } WARN_ON(!list_empty(priv-freeable_maps)); + mutex_unlock(priv-lock); if (use_ptemod) mmu_notifier_unregister(priv-mn, priv-mm); -- 1.9.3 -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen PV IOMMU interface draft C
On Mon, 2015-06-29 at 10:40 -0400, Konrad Rzeszutek Wilk wrote: On Fri, Jun 26, 2015 at 12:03:44PM +0100, Ian Campbell wrote: +ARM devs. On Fri, 2015-06-26 at 11:23 +0100, Malcolm Crossley wrote: Hi All, I had a chat with Malcolm about this with respect to ARM. The upshot is that this does not help us to remove the dom0 1:1 workaround or associated swiotlb uses on systems without an SMMU, nor does it allow us to sensibly do passthrough on systems which lack an SMMU. What would? This Xen PV IOMMU interface. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Increase device model startup timeout to 1min.
On Mon, 2015-06-29 at 15:23 +0100, Anthony PERARD wrote: On Fri, Jun 26, 2015 at 05:41:07PM +0100, Ian Campbell wrote: On Fri, 2015-06-26 at 12:57 +0100, Anthony PERARD wrote: On a busy host, QEMU may take more than 10s to start. Please show your working here so that in 2 years when we want to know why this value was chosen we don't have to go scrobbling around in the ML archives looking for the data you gathered. OK. What about: I'm afraid none of that really explains why QEMU taking 10s is reasonable/expected under the circumstances or why 60s is a good choice now. Nor does it really answer Ian's question in 21901.33163.547929.321...@mariner.uk.xensource.com I think. Ian. While running the whole stack of OpenStack (install via devstack on Ubuntu) on a single machine, sometime the device model fail to start on time. When this happen, a strace of QEMU shows that it still loading its libraries. You can find such strace on: http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03549.html What shows the strace is that between a mmap() syscall and the next syscall, 0.5s might have pass. Normaly, on this same machine it takes 0.32 second on average for QEMU to start, while running Tempest (which is the OpenStack test suite). Those QEMU are runned as backend for a PV guest. Signed-off-by: Anthony PERARD anthony.per...@citrix.com --- tools/libxl/libxl_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index e96d6b5..3a604f7 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -85,7 +85,7 @@ #define LIBXL_INIT_TIMEOUT 10 #define LIBXL_DESTROY_TIMEOUT 10 #define LIBXL_HOTPLUG_TIMEOUT 10 -#define LIBXL_DEVICE_MODEL_START_TIMEOUT 10 +#define LIBXL_DEVICE_MODEL_START_TIMEOUT 60 #define LIBXL_STUBDOM_START_TIMEOUT 30 #define LIBXL_QEMU_BODGE_TIMEOUT 2 #define LIBXL_XENCONSOLE_LIMIT 1048576 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v3 06/15] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts
On 24/06/15 06:18, Feng Wu wrote: Extend struct pi_desc according to VT-d Posted-Interrupts Spec. Signed-off-by: Feng Wu feng...@intel.com Reviewed-by: Andrew Cooper andrew.coop...@citrix.com Although this, like many other patches in the series needs a VT-d maintainers ack/review. --- v3: - Use u32 instead of u64 for the bitfield in 'struct pi_desc' xen/include/asm-x86/hvm/vmx/vmcs.h | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 1104bda..dedfaef 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -81,8 +81,19 @@ struct vmx_domain { struct pi_desc { DECLARE_BITMAP(pir, NR_VECTORS); -u32 control; -u32 rsvd[7]; +union { +struct +{ +u16 on : 1, /* bit 256 - Outstanding Notification */ +sn : 1, /* bit 257 - Suppress Notification */ +rsvd_1 : 14; /* bit 271:258 - Reserved */ +u8 nv; /* bit 279:272 - Notification Vector */ +u8 rsvd_2; /* bit 287:280 - Reserved */ +u32 ndst;/* bit 319:288 - Notification Destination */ +}; +u64 control; +}; +u32 rsvd[6]; } __attribute__ ((aligned (64))); #define ept_get_wl(ept) ((ept)-ept_wl) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [libvirt test] 58967: regressions - FAIL
flight 58967 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/58967/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-xsm 11 guest-start fail REGR. vs. 58842 Regressions which are regarded as allowable (not blocking): test-amd64-i386-libvirt 11 guest-start fail like 58842 test-amd64-amd64-libvirt 11 guest-start fail like 58842 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass version targeted for testing: libvirt f7d8aa44b06e5cfa06abea4a72ee26b13754667d baseline version: libvirt d10a5f58c75e7eb5943b44cc36a1e768adb2cdb0 People who touched revisions under test: Boris Fiuczynski fiu...@linux.vnet.ibm.com Dmitry Guryanov dgurya...@parallels.com Eric Blake ebl...@redhat.com Jiri Denemark jdene...@redhat.com John Ferlan jfer...@redhat.com Laine Stump la...@laine.org Luyao Huang lhu...@redhat.com Martin Kletzander mklet...@redhat.com Michal Privoznik mpriv...@redhat.com Mikhail Feoktistov mfeoktis...@virtuozzo.com Nikolay Shirokovskiy nshirokovs...@parallels.com Nikolay Shirokovskiy nshirokovs...@virtuozzo.com Peter Krempa pkre...@redhat.com jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm pass test-amd64-i386-libvirt-xsm fail test-amd64-amd64-libvirt fail test-armhf-armhf-libvirt pass test-amd64-i386-libvirt fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 925 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 COLOPre 05/26] tools/libxl: move save/restore code into libxl_dom_save.c
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: This is purely code motion. On that basis: Signed-off-by: Yang Hongyang yan...@cn.fujitsu.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 v12 8/8] Add xentrace to vmware_port
On 28/06/15 00:27, Don Slutz wrote: From: Don Slutz dsl...@verizon.com Also added missing TRAP_DEBUG VLAPIC. Signed-off-by: Don Slutz dsl...@verizon.com Acked-by: Ian Campbell ian.campb...@citrix.com CC: Don Slutz don.sl...@gmail.com --- v12: Switch VMPORT_IGNORED to port, regs-_eax. v11: No change v10: Added Acked-by: Ian Campbell Added back in the trace point calls. Why is cmd in this patch? Because the trace points use it. v9: Dropped unneed VMPORT_UNHANDLED, VMPORT_DECODE. v7: Dropped some of the new traces. Added HVMTRACE_ND7. v6: Dropped the attempt to use svm_nextrip_insn_length via __get_instruction_length (added in v2). Just always look at upto 15 bytes on AMD. v5: exitinfo1 is used twice. Fixed. tools/xentrace/formats | 5 + xen/arch/x86/hvm/io.c| 3 +++ xen/arch/x86/hvm/vmware/vmport.c | 15 --- xen/include/asm-x86/hvm/trace.h | 22 ++ xen/include/public/trace.h | 3 +++ 5 files changed, 45 insertions(+), 3 deletions(-) diff --git a/tools/xentrace/formats b/tools/xentrace/formats index 5d7b72a..ac8800e 100644 --- a/tools/xentrace/formats +++ b/tools/xentrace/formats @@ -79,6 +79,11 @@ 0x00082020 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) INTR_WINDOW [ value = 0x%(1)08x ] 0x00082021 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) NPF [ gpa = 0x%(2)08x%(1)08x mfn = 0x%(4)08x%(3)08x qual = 0x%(5)04x p2mt = 0x%(6)04x ] 0x00082023 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) TRAP[ vector = 0x%(1)02x ] +0x00082024 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) TRAP_DEBUG [ exit_qualification = 0x%(1)08x ] +0x00082025 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VLAPIC +0x00082026 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_HANDLED [ cmd = %(1)d eax = 0x%(2)08x ebx = 0x%(3)08x ecx = 0x%(4)08x edx = 0x%(5)08x esi = 0x%(6)08x edi = 0x%(7)08x ] +0x00082027 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_IGNORED [ port = %(1)d eax = 0x%(2)08x ] +0x00082028 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_QEMU [ eax = 0x%(1)08x ebx = 0x%(2)08x ecx = 0x%(3)08x edx = 0x%(4)08x esi = 0x%(5)08x edi = 0x%(6)08x ] 0x0010f001 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) page_grant_map [ domid = %(1)d ] 0x0010f002 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) page_grant_unmap[ domid = %(1)d ] diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c index c1379bf..e2d947b 100644 --- a/xen/arch/x86/hvm/io.c +++ b/xen/arch/x86/hvm/io.c @@ -206,6 +206,9 @@ void hvm_io_assist(ioreq_t *p) regs-_edx = vr-edx; regs-_esi = vr-esi; regs-_edi = vr-edi; +HVMTRACE_ND(VMPORT_QEMU, 0, 1/*cycles*/, 6, +p-data, regs-_ebx, regs-_ecx, +regs-_edx, regs-_esi, regs-_edi); } } if ( vio-io_size == 4 ) /* Needs zero extension. */ diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c index 5e14402..de29e2f 100644 --- a/xen/arch/x86/hvm/vmware/vmport.c +++ b/xen/arch/x86/hvm/vmware/vmport.c @@ -16,6 +16,7 @@ #include xen/lib.h #include asm/hvm/hvm.h #include asm/hvm/support.h +#include asm/hvm/trace.h #include backdoor_def.h @@ -35,6 +36,7 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val) if ( port == BDOOR_PORT regs-_eax == BDOOR_MAGIC ) { uint32_t new_eax = ~0u; +uint16_t cmd = regs-_ecx; uint64_t value; struct vcpu *curr = current; struct domain *currd = curr-domain; @@ -45,7 +47,7 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val) * leaving the high 32-bits unchanged, unlike what one would * expect to happen. */ -switch ( regs-_ecx 0x ) +switch ( cmd ) { case BDOOR_CMD_GETMHZ: new_eax = currd-arch.tsc_khz / 1000; @@ -123,11 +125,18 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val) /* Let backing DM handle */ return X86EMUL_UNHANDLEABLE; } +HVMTRACE_ND7(VMPORT_HANDLED, 0, 0/*cycles*/, 7, + cmd, new_eax, regs-_ebx, regs-_ecx, + regs-_edx, regs-_esi, regs-_edi); if ( dir == IOREQ_READ ) *val = new_eax; } -else if ( dir == IOREQ_READ ) -*val = ~0u; +else +{ +HVMTRACE_2D(VMPORT_IGNORED, port, regs-_eax); +if ( dir == IOREQ_READ ) +*val = ~0u; +} return X86EMUL_OKAY; } diff --git a/xen/include/asm-x86/hvm/trace.h b/xen/include/asm-x86/hvm/trace.h index de802a6..0ad805f 100644 --- a/xen/include/asm-x86/hvm/trace.h +++ b/xen/include/asm-x86/hvm/trace.h @@ -54,6 +54,9 @@ #define DO_TRC_HVM_TRAP
Re: [Xen-devel] PCI Pass-through in Xen ARM - Draft 2.
On Mon, 2015-06-29 at 00:08 +0530, Manish Jaggi wrote: PCI Pass-through in Xen ARM -- Draft 2 Index 1. Background 2. Basic PCI Support in Xen ARM 2.1 pci_hostbridge and pci_hostbridge_ops 2.2 PHYSDEVOP_HOSTBRIDGE_ADD hypercall 3. Dom0 Access PCI devices 4. DomU assignment of PCI device 4.1 Holes in guest memory space 4.2 New entries in xenstore for device BARs 4.3 Hypercall for bdf mapping noification to xen 4.4 Change in Linux PCI FrontEnd - backend driver for MSI/X programming 5. NUMA and PCI passthrough 6. DomU pci device attach flow Revision History Changes from Draft 1 a) map_mmio hypercall removed from earlier draft b) device bar mapping into guest not 1:1 c) holes in guest address space 32bit / 64bit for MMIO virtual BARs d) xenstore device's BAR info addition. 1. Background of PCI passthrough Passthrough refers to assigning a pci device to a guest domain (domU) such that the guest has full control over the device.The MMIO space and interrupts are managed by the guest itself, close to how a bare kernel manages a device. Device's access to guest address space needs to be isolated and protected. SMMU (System MMU - IOMMU in ARM) is programmed by xen hypervisor to allow device access guest memory for data transfer and sending MSI/X interrupts. In case of MSI/X the device writes to GITS (ITS address space) Interrupt Translation Register. 2. Basic PCI Support for ARM The apis to read write from pci configuration space are based on segment:bdf. How the sbdf is mapped to a physical address is under the realm of the pci host controller. ARM PCI support in Xen, introduces pci host controller similar to what exists in Linux. Each drivers registers callbacks, which are invoked on matching the compatible property in pci device tree node. 2.1: The init function in the pci host driver calls to register hostbridge callbacks: int pci_hostbridge_register(pci_hostbridge_t *pcihb); struct pci_hostbridge_ops { u32 (*pci_conf_read)(struct pci_hostbridge*, u32 bus, u32 devfn, u32 reg, u32 bytes); void (*pci_conf_write)(struct pci_hostbridge*, u32 bus, u32 devfn, u32 reg, u32 bytes, u32 val); }; struct pci_hostbridge{ u32 segno; paddr_t cfg_base; paddr_t cfg_size; struct dt_device_node *dt_node; struct pci_hostbridge_ops ops; struct list_head list; }; A pci conf read function would internally be as follows: u32 pcihb_conf_read(u32 seg, u32 bus, u32 devfn,u32 reg, u32 bytes) { pci_hostbridge_t *pcihb; list_for_each_entry(pcihb, pci_hostbridge_list, list) { if(pcihb-segno == seg) return pcihb-ops.pci_conf_read(pcihb, bus, devfn, reg, bytes); } return -1; } 2.2 PHYSDEVOP_pci_host_bridge_add hypercall Xen code accesses PCI configuration space based on the sbdf received from the guest. The order in which the pci device tree node appear may not be the same order of device enumeration in dom0. Thus there needs to be a mechanism to bind the segment number assigned by dom0 to the pci host controller. The hypercall is introduced: #define PHYSDEVOP_pci_host_bridge_add44 struct physdev_pci_host_bridge_add { /* IN */ uint16_t seg; uint64_t cfg_base; uint64_t cfg_size; }; This hypercall is invoked before dom0 invokes the PHYSDEVOP_pci_device_add hypercall. The handler code invokes to update segment number in pci_hostbridge: int pci_hostbridge_setup(uint32_t segno, uint64_t cfg_base, uint64_t cfg_size); Subsequent calls to pci_conf_read/write are completed by the pci_hostbridge_ops of the respective pci_hostbridge. 3. Dom0 access PCI device - As per the design of xen hypervisor, dom0 enumerates the PCI devices. For each device the MMIO space has to be mapped in the Stage2 translation for dom0. Here device is really host bridge, isn't it? i.e. this is done by mapping the entire MMIO window of each host bridge, not the individual BAR registers of each device one at a time. IOW this is functionality of the pci host driver's intitial setup, not something which is driven from the dom0 enumeration of the bus. For dom0 xen maps the ranges in pci nodes in stage 2 translation. GITS_ITRANSLATER space (4k( must be programmed in Stage2 translation so that MSI/X must work. This is done in vits initialization in dom0/domU. This also happens at start of day, but what isn't mentioned is that (AIUI) the SMMU will need to be programmed to map each SBDF to the dom0 p2m as the devices are discovered and reported. Right? 4. DomU access / assignment PCI device -- In the flow of pci-attach device, the toolkit I assume you mean toolstack
Re: [Xen-devel] Xen PV IOMMU interface draft C
On Mon, 2015-06-29 at 16:24 +0100, David Vrabel wrote: On 29/06/15 15:52, Ian Campbell wrote: On Mon, 2015-06-29 at 10:40 -0400, Konrad Rzeszutek Wilk wrote: On Fri, Jun 26, 2015 at 12:03:44PM +0100, Ian Campbell wrote: +ARM devs. On Fri, 2015-06-26 at 11:23 +0100, Malcolm Crossley wrote: Hi All, I had a chat with Malcolm about this with respect to ARM. The upshot is that this does not help us to remove the dom0 1:1 workaround or associated swiotlb uses on systems without an SMMU, nor does it allow us to sensibly do passthrough on systems which lack an SMMU. What would? This Xen PV IOMMU interface. I guess Konrad is asking if this PV IOMMU interface doesn't solve these problems, what (other possible interface) would? Ah, I thought it was a response to the this in ... that this doest not I guess the answer is a SMMU? That's the only solution I've thought of, yes. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] x86/arm/mm: use gfn instead of pfn in p2m_get_mem_access/p2m_set_mem_access
On 29/06/15 16:45, Vitaly Kuznetsov wrote: 'pfn' and 'start_pfn' are ambiguous, both these functions expect GFNs as input. On x86 the interface of p2m_set_mem_access() in p2m.c doesn't match the declaration in p2m-common.h as 'pfn' is being used instead of 'start_pfn'. On ARM both p2m_set_mem_access and p2m_get_mem_access interfaces don't match declarations from p2m-common.h: p2m_set_mem_access uses 'pfn' instead of 'start_pfn' and p2m_get_mem_access uses 'gpfn' instead of 'pfn'. Convert p2m_get_mem_access/p2m_set_mem_access (and __p2m_get_mem_access on ARM) interfaces to using gft_t instead of unsigned long and update all users of these functions. There is also an issue in p2m_get_mem_access on x86: 'gfn' parameter passed to gfn_lock/gfn_unlock is not defined. This code compiles only because of a coincidence: gfn_lock/gfn_unlock are currently macros which don't use their second argument. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com Reviewed-by: Andrew Cooper andrew.coop...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 02/12] VMX: implement suppress #VE.
On 29/06/15 15:20, George Dunlap wrote: On Mon, Jun 22, 2015 at 7:56 PM, Ed White edmund.h.wh...@intel.com wrote: In preparation for selectively enabling #VE in a later patch, set suppress #VE on all EPTE's. Suppress #VE should always be the default condition for two reasons: it is generally not safe to deliver #VE into a guest unless that guest has been modified to receive it; and even then for most EPT violations only the hypervisor is able to handle the violation. Signed-off-by: Ed White edmund.h.wh...@intel.com --- xen/arch/x86/mm/p2m-ept.c | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index a6c9adf..5de3387 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -41,7 +41,7 @@ #define is_epte_superpage(ept_entry)((ept_entry)-sp) static inline bool_t is_epte_valid(ept_entry_t *e) { -return (e-epte != 0 e-sa_p2mt != p2m_invalid); +return ((e-epte ~(1ul 63)) != 0 e-sa_p2mt != p2m_invalid); So just getting up to speed here: Is it the case that if #VE is enabled in vmcs that a #VE will be delivered to the guest on any invalid epte entry that doesn't contain this flag? There is a list of conditions which must be satisfied for a #VE to be injected instead of an EPT related VMexit. All EPT misconfiguration still exit to the hypervisor, but this suppress_ve bit allows the hypervisor to choose to whether a plain EPT permission violation exits to Xen, or injects a #VE. So we now need to actively choose a default which is different than the hardware? By default, setting suppress_ve on everything will cause everything to behave as before. Clearing suppress_ve is an optimisation to avoid a vmexit/vmentry for faults needing bouncing to an in-guest agent. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen PV IOMMU interface draft C
On Fri, Jun 26, 2015 at 12:03:44PM +0100, Ian Campbell wrote: +ARM devs. On Fri, 2015-06-26 at 11:23 +0100, Malcolm Crossley wrote: Hi All, I had a chat with Malcolm about this with respect to ARM. The upshot is that this does not help us to remove the dom0 1:1 workaround or associated swiotlb uses on systems without an SMMU, nor does it allow us to sensibly do passthrough on systems which lack an SMMU. What would? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable test] 58965: tolerable FAIL
flight 58965 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/58965/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemuu-debianhvm-amd64 18 guest-start/debianhvm.repeat fail in 58948 pass in 58965 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 15 guest-localmigrate/x10 fail in 58958 pass in 58948 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 12 guest-localmigrate fail pass in 58958 Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stopfail blocked in 58958 test-amd64-i386-rumpuserxen-i386 15 rumpuserxen-demo-xenstorels/xenstorels.repeat fail in 58948 like 58938 test-amd64-i386-xl-qemuu-win7-amd64 9 windows-install fail in 58958 like 58948 test-amd64-i386-libvirt-xsm 11 guest-start fail like 58958 test-amd64-i386-libvirt 11 guest-start fail like 58958 test-amd64-amd64-libvirt-xsm 11 guest-start fail like 58958 test-amd64-amd64-libvirt 11 guest-start fail like 58958 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 58958 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-armhf-armhf-xl-rtds 11 guest-start fail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass version targeted for testing: xen c40317f11b3f05e7c06a2213560c8471081f2662 baseline version: xen c40317f11b3f05e7c06a2213560c8471081f2662 jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-oldkern pass build-i386-oldkern pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass build-amd64-rumpuserxen pass build-i386-rumpuserxen pass test-amd64-amd64-xl pass test-armhf-armhf-xl pass test-amd64-i386-xl pass test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemut-debianhvm-amd64-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsmfail test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm pass test-amd64-amd64-libvirt-xsm fail test-armhf-armhf-libvirt-xsm pass test-amd64-i386-libvirt-xsm fail test-amd64-amd64-xl-xsm pass test-armhf-armhf-xl-xsm pass test-amd64-i386-xl-xsm pass test-amd64-amd64-xl-pvh-amd fail test-amd64-i386-qemut-rhel6hvm-amd pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemut-debianhvm-amd64pass
Re: [Xen-devel] [v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor
On 24/06/15 06:18, Feng Wu wrote: This patch initializes the VT-d Posted-interrupt Descriptor. Signed-off-by: Feng Wu feng...@intel.com --- v3: - Move pi_desc_init() to xen/arch/x86/hvm/vmx/vmcs.c - Remove the 'inline' flag of pi_desc_init() xen/arch/x86/hvm/vmx/vmcs.c | 18 ++ xen/include/asm-x86/hvm/vmx/vmx.h | 2 ++ 2 files changed, 20 insertions(+) diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index 3aff365..11dc1b5 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -40,6 +40,7 @@ #include asm/flushtlb.h #include asm/shadow.h #include asm/tboot.h +#include asm/apic.h static bool_t __read_mostly opt_vpid_enabled = 1; boolean_param(vpid, opt_vpid_enabled); @@ -921,6 +922,20 @@ void virtual_vmcs_vmwrite(void *vvmcs, u32 vmcs_encoding, u64 val) virtual_vmcs_exit(vvmcs); } +static void pi_desc_init(struct vcpu *v) +{ +uint32_t dest; + +v-arch.hvm_vmx.pi_desc.nv = posted_intr_vector; + +dest = cpu_physical_id(v-processor); I am fairly sure that this is not a safe use of v-processor. Everything else in this patch looks fine, but I would like review from people more familiar with scheduling. ~Andrew + +if ( x2apic_enabled ) +v-arch.hvm_vmx.pi_desc.ndst = dest; +else +v-arch.hvm_vmx.pi_desc.ndst = MASK_INSR(dest, PI_xAPIC_NDST_MASK); +} + static int construct_vmcs(struct vcpu *v) { struct domain *d = v-domain; @@ -1054,6 +1069,9 @@ static int construct_vmcs(struct vcpu *v) if ( cpu_has_vmx_posted_intr_processing ) { +if ( iommu_intpost ) +pi_desc_init(v); + __vmwrite(PI_DESC_ADDR, virt_to_maddr(v-arch.hvm_vmx.pi_desc)); __vmwrite(POSTED_INTR_NOTIFICATION_VECTOR, posted_intr_vector); } diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h index 35f804a..5853563 100644 --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -89,6 +89,8 @@ typedef enum { #define EPT_EMT_WB 6 #define EPT_EMT_RSV27 +#define PI_xAPIC_NDST_MASK 0xFF00 + void vmx_asm_vmexit_handler(struct cpu_user_regs); void vmx_asm_do_vmentry(void); void vmx_intr_assist(void); ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 COLOPre 01/26] tools/libxl: rename libxl__domain_suspend to libxl__domain_save
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: Rename libxl__domain_suspend() to libxl__domain_save() since it actually do the save domain work. In response to Ian J's query on the last version Andrew said: The terminology used by libxc is more consistent in this area. suspend refers to quiescing the VM, so pausing qemu, making a remote_shutdown(SHUTDOWN_suspend) hypercall etc. save refers to the actions involved in actually shuffling the state of the VM, so xc_domain_save() etc. I believe that these are useful distinctions to maintain. libxl currently uses suspend to encapsulate both. Are your changes here staying true to those definitions? I think incorporating some of that (in particular the second and third paragraphs) into a code comment somewhere (likely libxl_internal.h) would make it much clearer why you were doing this, and give people like me a reminder of the distinction each time they trip over it in this series too. diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index e0f6e09..19ebaab 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -3264,8 +3264,8 @@ struct libxl__domain_create_state { /*- Domain suspend (save) functions -*/ ... here might be a good place to explain why/how suspend and save are different. /* calls dss-callback when done */ -_hidden void libxl__domain_suspend(libxl__egc *egc, - libxl__domain_suspend_state *dss); +_hidden void libxl__domain_save(libxl__egc *egc, +libxl__domain_suspend_state *dss); /* calls libxl__xc_domain_suspend_done when done */ ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 COLOPre 06/26] libxl/save: Refactor libxl__domain_suspend_state
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: [...] switch (type) { case LIBXL_DOMAIN_TYPE_HVM: { dss-hvm = 1; +dsps-hvm = 1; break; } case LIBXL_DOMAIN_TYPE_PV: dss-hvm = 0; +dsps-hvm = 0; break; [...] @@ -2913,9 +2914,27 @@ typedef struct libxl__logdirty_switch { } libxl__logdirty_switch; struct libxl__domain_suspend_state { +/* set by caller of domain_suspend_callback_common */ +libxl__ao *ao; + +uint32_t domid; +int hvm; +/* private */ +libxl__ev_evtchn guest_evtchn; +int guest_evtchn_lockfd; +int guest_responded; +libxl__xswait_state pvcontrol; +libxl__ev_xswatch guest_watch; +libxl__ev_time guest_timeout; +const char *dm_savefile; +void (*callback_common_done)(libxl__egc*, + struct libxl__domain_suspend_state*, int ok); +}; + +struct libxl__domain_save_state { /* set by caller of libxl__domain_suspend */ libxl__ao *ao; -libxl__domain_suspend_cb *callback; +libxl__domain_save_cb *callback; uint32_t domid; int fd; @@ -2924,22 +2943,14 @@ struct libxl__domain_suspend_state { int debug; const libxl_domain_remus_info *remus; /* private */ -libxl__ev_evtchn guest_evtchn; -int guest_evtchn_lockfd; +libxl__domain_suspend_state dsps; int hvm; I wonder if, given that any domain suspend must necessarily be contained within a domain save it would be preferable to have suspend code upcast the suspend_state to the containing save_state in order to look at -hvm, rather than duplicating it. Likewise domid. For ao I can imagine that the suspend and save might actually have separate ao lifetimes, in which case these do of course need to remain different. Would that make any sense at all? FYI it was the dual initialisation of hvm quoted above which lead me to think along these lines. Alternatively perhaps suspend_state should a separate init function to logically separate it from the save_state initialiser. Perhaps taking the latter as an argument? Or possibly just the relevant fields. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Unable to unregister netdevice after netfilter: bridge: forward IPv6 fragmented packets
Hi, I tried to run the latest Linux tree (4a10a91756ef381bced7b88cfb9232f660b92d93) as DOM0 Xen. After destroying a guest using network, I got the following lines in the DOM0 kernel log: unregister_netdevice: waiting for vif1.0 to become free. Usage count = 1 The bisector pointed the problem after the commit efb6de9b4ba0092b2c55f6a52d16294a8a698edd netfilter: bridge: forward IPv6 fragmented packets. This is happening on an xgene board. The bridge is configured using /etc/network/interfaces: # The primary network interface auto xenbr0 iface xenbr0 inet dhcp hostname pony bridge_ports eth0 bridge_stp off bridge_waitport 0 bridge_fd 0 bridge_maxwait 0 Xen tools is creating the interface vif1.0 and adding to the bridge xenbr0. Linux log: device eth0 entered promiscuous mode xenbr0: port 1(eth0) entered forwarding state xenbr0: port 1(eth0) entered forwarding state xenbr0: port 1(eth0) entered disabled state xgene-enet 1702.ethernet eth0: Link is Down random: nonblocking pool is initialized [] Configuring network interfaces... Waiting for a max of 0 seconds for eth0 to become available. Internet Systems Consortium DHCP Client 4.3.1 Copyright 2004-2014 Internet Systems Consortium. All rights reserved. For info, please visit https://www.isc.org/software/dhcp/ Listening on LPF/xenbr0/00:01:73:02:1a:00 Sending on LPF/xenbr0/00:01:73:02:1a:00 Sending on Socket/fallback DHCPDISCOVER on xenbr0 to 255.255.255.255 port 67 interval 7 xgene-enet 1702.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx xenbr0: port 1(eth0) entered forwarding state xenbr0: port 1(eth0) entered forwarding state DHCPDISCOVER on xenbr0 to 255.255.255.255 port 67 interval 13 DHCPREQUEST on xenbr0 to 255.255.255.255 port 67 DHCPOFFER from 10.80.224.1 DHCPACK from 10.80.224.1 bound to 10.80.229.126 -- renewal in 19083 seconds. done. [ ok ] Starting rpcbind daemon [ ok ] Starting NFS common utilities: statd idmapd. [ ok ] Cleaning up temporary files [info] Setting console screen modes. setterm: cannot (un)set powersave mode: Inappropriate ioctl for device [] Setting up console font and keymap...Couldn't get a file descriptor referring to the console Couldn't get a file descriptor referring to the console done. [ ok ] Setting up X socket directories... /tmp/.X11-unix /tmp/.ICE-unix. INIT: Entering runlevel: 2 [info] Using makefile-style concurrent boot in runlevel 2. [ ok ] Starting enhanced syslogd: rsyslogd. [ ok ] Starting deferred execution scheduler: atd. [ ok ] Starting periodic command scheduler: cron. [ ok ] Starting SMP IRQ Balancer: irqbalance. [ ok ] Starting NTP server: ntpd. [ ok ] Starting OpenBSD Secure Shell server: sshd. [ ok ] Starting system message bus: dbus. Starting /usr/local/sbin/xenstored... Setting domain 0 name, domid and JSON config... Done setting up Dom0 Starting xenconsoled... Starting QEMU as disk backend for dom0 Debian GNU/Linux 8 pony hvc0 pony login: device vif1.0 entered promiscuous mode IPv6: ADDRCONF(NETDEV_UP): vif1.0: link is not ready xen-blkback: ring-ref 8, event-channel 3, protocol 1 (arm-abi) persistent grants vif vif-1-0 vif1.0: Guest Rx ready IPv6: ADDRCONF(NETDEV_CHANGE): vif1.0: link becomes ready xenbr0: port 2(vif1.0) entered forwarding state xenbr0: port 2(vif1.0) entered forwarding state (XEN) mm.c:1251:d0v2 gnttab_mark_dirty not implemented yet xenbr0: port 2(vif1.0) entered disabled state xenbr0: port 2(vif1.0) entered disabled state device vif1.0 left promiscuous mode xenbr0: port 2(vif1.0) entered disabled state INIT: Id 1 respawning too fast: disabled for 5 minutes INIT: Id 3 respawning too fast: disabled for 5 minutes INIT: Id 5 respawning too fast: disabled for 5 minutes INIT: Id 2 respawning too fast: disabled for 5 minutes INIT: Id 6 respawning too fast: disabled for 5 minutes INIT: Id T0 respawning too fast: disabled for 5 minutes INIT: Id 4 respawning too fast: disabled for 5 minutes unregister_netdevice: waiting for vif1.0 to become free. Usage count = 1 unregister_netdevice: waiting for vif1.0 to become free. Usage count = 1 unregister_netdevice: waiting for vif1.0 to become free. Usage count = 1 Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 COLOPre 07/26] libxc/restore: fix error handle of process_record
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: ITYM handling in the subject. And perhaps change rather than fix? If the err is RECORD_NOT_PROCESSED, and it is an optional record, restore will still fail. There're two options to fix this, a, setting rc to 0 when it is an optional record. b, do the error handling in the caller. We choose b because: There will be another error type in COLO, which indicates a failover, that needs to be handled in restore(), so moving the error handling out to make the logic clearer...Otherwise, in process_record, RECORD_NOT_PROCESSED is handled, and in restore another error type returned from process_record is handled. Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com CC: Ian Jackson ian.jack...@eu.citrix.com CC: Wei Liu wei.l...@citrix.com CC: Andrew Cooper andrew.coop...@citrix.com --- tools/libxc/xc_sr_restore.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index fd45775..d5645e0 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -569,19 +569,6 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) free(rec-data); rec-data = NULL; -if ( rc == RECORD_NOT_PROCESSED ) -{ -if ( rec-type REC_TYPE_OPTIONAL ) -DPRINTF(Ignoring optional record %#x (%s), -rec-type, rec_type_to_str(rec-type)); -else -{ -ERROR(Mandatory record %#x (%s) not handled, - rec-type, rec_type_to_str(rec-type)); -rc = -1; -} -} - return rc; } @@ -669,7 +656,20 @@ static int restore(struct xc_sr_context *ctx) There is a second caller of process_record in handle_checkpoint, so this change will result in a change of behaviour for that case, I think. If that is intentional then it ought to be mentioned in the commit log, since otherwise it looks a lot like this change is supposed to result in no overall difference. Ian. else { rc = process_record(ctx, rec); -if ( rc ) +if ( rc == RECORD_NOT_PROCESSED ) +{ +if ( rec.type REC_TYPE_OPTIONAL ) +DPRINTF(Ignoring optional record %#x (%s), +rec.type, rec_type_to_str(rec.type)); +else +{ +ERROR(Mandatory record %#x (%s) not handled, + rec.type, rec_type_to_str(rec.type)); +rc = -1; +goto err; +} +} +else if ( rc ) goto err; } ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Increase device model startup timeout to 1min.
On Mon, Jun 29, 2015 at 03:51:57PM +0100, Ian Campbell wrote: On Mon, 2015-06-29 at 15:23 +0100, Anthony PERARD wrote: On Fri, Jun 26, 2015 at 05:41:07PM +0100, Ian Campbell wrote: On Fri, 2015-06-26 at 12:57 +0100, Anthony PERARD wrote: On a busy host, QEMU may take more than 10s to start. Please show your working here so that in 2 years when we want to know why this value was chosen we don't have to go scrobbling around in the ML archives looking for the data you gathered. OK. What about: I'm afraid none of that really explains why QEMU taking 10s is reasonable/expected under the circumstances or why 60s is a good choice now. Nor does it really answer Ian's question in 21901.33163.547929.321...@mariner.uk.xensource.com I think. I only know what happen, not why it happen. How could I investigate why qemu is taking so long after an mmap() syscall? I guest at that time, it is copying the library into memory. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 8/8] Add xentrace to vmware_port
On 06/29/15 10:54, Andrew Cooper wrote: On 28/06/15 00:27, Don Slutz wrote: From: Don Slutz dsl...@verizon.com Also added missing TRAP_DEBUG VLAPIC. Signed-off-by: Don Slutz dsl...@verizon.com Acked-by: Ian Campbell ian.campb...@citrix.com CC: Don Slutz don.sl...@gmail.com --- v12: Switch VMPORT_IGNORED to port, regs-_eax. v11: No change v10: Added Acked-by: Ian Campbell Added back in the trace point calls. Why is cmd in this patch? Because the trace points use it. v9: Dropped unneed VMPORT_UNHANDLED, VMPORT_DECODE. v7: Dropped some of the new traces. Added HVMTRACE_ND7. v6: Dropped the attempt to use svm_nextrip_insn_length via __get_instruction_length (added in v2). Just always look at upto 15 bytes on AMD. v5: exitinfo1 is used twice. Fixed. tools/xentrace/formats | 5 + xen/arch/x86/hvm/io.c| 3 +++ xen/arch/x86/hvm/vmware/vmport.c | 15 --- xen/include/asm-x86/hvm/trace.h | 22 ++ xen/include/public/trace.h | 3 +++ 5 files changed, 45 insertions(+), 3 deletions(-) diff --git a/tools/xentrace/formats b/tools/xentrace/formats index 5d7b72a..ac8800e 100644 --- a/tools/xentrace/formats +++ b/tools/xentrace/formats @@ -79,6 +79,11 @@ 0x00082020 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) INTR_WINDOW [ value = 0x%(1)08x ] 0x00082021 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) NPF [ gpa = 0x%(2)08x%(1)08x mfn = 0x%(4)08x%(3)08x qual = 0x%(5)04x p2mt = 0x%(6)04x ] 0x00082023 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) TRAP[ vector = 0x%(1)02x ] +0x00082024 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) TRAP_DEBUG [ exit_qualification = 0x%(1)08x ] +0x00082025 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VLAPIC +0x00082026 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_HANDLED [ cmd = %(1)d eax = 0x%(2)08x ebx = 0x%(3)08x ecx = 0x%(4)08x edx = 0x%(5)08x esi = 0x%(6)08x edi = 0x%(7)08x ] +0x00082027 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_IGNORED [ port = %(1)d eax = 0x%(2)08x ] +0x00082028 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_QEMU [ eax = 0x%(1)08x ebx = 0x%(2)08x ecx = 0x%(3)08x edx = 0x%(4)08x esi = 0x%(5)08x edi = 0x%(6)08x ] 0x0010f001 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) page_grant_map [ domid = %(1)d ] 0x0010f002 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) page_grant_unmap[ domid = %(1)d ] diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c index c1379bf..e2d947b 100644 --- a/xen/arch/x86/hvm/io.c +++ b/xen/arch/x86/hvm/io.c @@ -206,6 +206,9 @@ void hvm_io_assist(ioreq_t *p) regs-_edx = vr-edx; regs-_esi = vr-esi; regs-_edi = vr-edi; +HVMTRACE_ND(VMPORT_QEMU, 0, 1/*cycles*/, 6, +p-data, regs-_ebx, regs-_ecx, +regs-_edx, regs-_esi, regs-_edi); } } if ( vio-io_size == 4 ) /* Needs zero extension. */ diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c index 5e14402..de29e2f 100644 --- a/xen/arch/x86/hvm/vmware/vmport.c +++ b/xen/arch/x86/hvm/vmware/vmport.c @@ -16,6 +16,7 @@ #include xen/lib.h #include asm/hvm/hvm.h #include asm/hvm/support.h +#include asm/hvm/trace.h #include backdoor_def.h @@ -35,6 +36,7 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val) if ( port == BDOOR_PORT regs-_eax == BDOOR_MAGIC ) { uint32_t new_eax = ~0u; +uint16_t cmd = regs-_ecx; uint64_t value; struct vcpu *curr = current; struct domain *currd = curr-domain; @@ -45,7 +47,7 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val) * leaving the high 32-bits unchanged, unlike what one would * expect to happen. */ -switch ( regs-_ecx 0x ) +switch ( cmd ) { case BDOOR_CMD_GETMHZ: new_eax = currd-arch.tsc_khz / 1000; @@ -123,11 +125,18 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val) /* Let backing DM handle */ return X86EMUL_UNHANDLEABLE; } +HVMTRACE_ND7(VMPORT_HANDLED, 0, 0/*cycles*/, 7, + cmd, new_eax, regs-_ebx, regs-_ecx, + regs-_edx, regs-_esi, regs-_edi); if ( dir == IOREQ_READ ) *val = new_eax; } -else if ( dir == IOREQ_READ ) -*val = ~0u; +else +{ +HVMTRACE_2D(VMPORT_IGNORED, port, regs-_eax); +if ( dir == IOREQ_READ ) +*val = ~0u; +} return X86EMUL_OKAY; } diff --git a/xen/include/asm-x86/hvm/trace.h b/xen/include/asm-x86/hvm/trace.h index de802a6..0ad805f 100644 --- a/xen/include/asm-x86/hvm/trace.h +++ b/xen/include/asm-x86/hvm/trace.h @@ -54,6 +54,9 @@ #define DO_TRC_HVM_TRAP DEFAULT_HVM_MISC
Re: [Xen-devel] [PATCH v4] x86/arm/mm: use gfn instead of pfn in p2m_get_mem_access/p2m_set_mem_access
On 06/29/2015 06:45 PM, Vitaly Kuznetsov wrote: 'pfn' and 'start_pfn' are ambiguous, both these functions expect GFNs as input. On x86 the interface of p2m_set_mem_access() in p2m.c doesn't match the declaration in p2m-common.h as 'pfn' is being used instead of 'start_pfn'. On ARM both p2m_set_mem_access and p2m_get_mem_access interfaces don't match declarations from p2m-common.h: p2m_set_mem_access uses 'pfn' instead of 'start_pfn' and p2m_get_mem_access uses 'gpfn' instead of 'pfn'. Convert p2m_get_mem_access/p2m_set_mem_access (and __p2m_get_mem_access on ARM) interfaces to using gft_t instead of unsigned long and update all users of these functions. There is also an issue in p2m_get_mem_access on x86: 'gfn' parameter passed to gfn_lock/gfn_unlock is not defined. This code compiles only because of a coincidence: gfn_lock/gfn_unlock are currently macros which don't use their second argument. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com --- Changes since v3: - Comment codying style fix [Razvan Cojocaru] - Use INVALID_GFN instead of ~0 and -1 [Andrew Cooper] - Convert p2m_get_mem_access/p2m_set_mem_access interfaces to using gfn_t [Andrew Cooper] Changes since v2: - Instead of adding start_ prefix on ARM remove it on x86 [Jan Beulich, Ian Campbell, Razvan Cojocaru] Changes since v1: - This patch is a successor of '[PATCH] x86/mm: use existing 'pfn' in p2m_get_mem_access', instead of fixing gfn_lock/gfn_unlock arguments we do s/pfn/gfn/g for both p2m_get_mem_access/p2m_set_mem_access [Andrew Cooper, Jan Beulich] P.S. - The patch was compile-tested on x86 and ARM64. --- xen/arch/arm/p2m.c | 33 + xen/arch/x86/mm/p2m.c| 36 xen/common/mem_access.c | 4 ++-- xen/include/xen/p2m-common.h | 13 ++--- 4 files changed, 45 insertions(+), 41 deletions(-) Reviewed-by: Razvan Cojocaru rcojoc...@bitdefender.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 02/12] VMX: implement suppress #VE.
On Mon, Jun 29, 2015 at 3:31 PM, Andrew Cooper andrew.coop...@citrix.com wrote: On 29/06/15 15:20, George Dunlap wrote: On Mon, Jun 22, 2015 at 7:56 PM, Ed White edmund.h.wh...@intel.com wrote: In preparation for selectively enabling #VE in a later patch, set suppress #VE on all EPTE's. Suppress #VE should always be the default condition for two reasons: it is generally not safe to deliver #VE into a guest unless that guest has been modified to receive it; and even then for most EPT violations only the hypervisor is able to handle the violation. Signed-off-by: Ed White edmund.h.wh...@intel.com --- xen/arch/x86/mm/p2m-ept.c | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index a6c9adf..5de3387 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -41,7 +41,7 @@ #define is_epte_superpage(ept_entry)((ept_entry)-sp) static inline bool_t is_epte_valid(ept_entry_t *e) { -return (e-epte != 0 e-sa_p2mt != p2m_invalid); +return ((e-epte ~(1ul 63)) != 0 e-sa_p2mt != + p2m_invalid); So just getting up to speed here: Is it the case that if #VE is enabled in vmcs that a #VE will be delivered to the guest on any invalid epte entry that doesn't contain this flag? There is a list of conditions which must be satisfied for a #VE to be injected instead of an EPT related VMexit. All EPT misconfiguration still exit to the hypervisor, but this suppress_ve bit allows the hypervisor to choose to whether a plain EPT permission violation exits to Xen, or injects a #VE. So we now need to actively choose a default which is different than the hardware? By default, setting suppress_ve on everything will cause everything to behave as before. Clearing suppress_ve is an optimisation to avoid a vmexit/vmentry for faults needing bouncing to an in-guest agent. So the short answer is, 'yes': The hardware will deliver #VEs for all non-misconfigured ept entries (which includes entries which are simply not present) unless you actively do something to suppress them; what we want is *not* to deliver #VEs unless the guest actively does something to cause them to be delivered for particular GPAs. Ravi correct, by setting suppress-ve in the default EPTE we achieve that behavior of not delivering #VE (ie legacy behavior) unless the guest actively sets an altp2m policy for specific GPAs. Ravi Thanks, -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 COLOPre 10/26] migration/save: pass checkpointed_stream from libxl to libxc
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: Pass checkpointed_stream from libxl to libxc. Hrm, so now we seem to have a purely libxl internal usage of this new enum, but still nothing crossing the libxl.h boundary. I suspect I've missed something! It won't affact legacy migration because legacy migration won't use this param. Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com CC: Ian Campbell ian.campb...@citrix.com CC: Ian Jackson ian.jack...@eu.citrix.com CC: Wei Liu wei.l...@citrix.com CC: Andrew Cooper andrew.coop...@citrix.com --- tools/libxc/include/xenguest.h | 9 ++--- tools/libxc/xc_domain_save.c | 6 -- tools/libxc/xc_nomigrate.c | 3 ++- tools/libxc/xc_sr_common.h | 2 +- tools/libxc/xc_sr_save.c | 5 +++-- tools/libxl/libxl.c | 2 ++ tools/libxl/libxl_dom_save.c | 12 +--- tools/libxl/libxl_internal.h | 1 + tools/libxl/libxl_save_callout.c | 2 +- tools/libxl/libxl_save_helper.c | 3 ++- 10 files changed, 31 insertions(+), 14 deletions(-) diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h index b0d27ed..d7c8fe4 100644 --- a/tools/libxc/include/xenguest.h +++ b/tools/libxc/include/xenguest.h @@ -30,7 +30,6 @@ #define XCFLAGS_HVM (1 2) #define XCFLAGS_STDVGA(1 3) #define XCFLAGS_CHECKPOINT_COMPRESS(1 4) -#define XCFLAGS_CHECKPOINTED(1 5) #define X86_64_B_SIZE 64 #define X86_32_B_SIZE 32 @@ -85,16 +84,20 @@ struct save_callbacks { * @parm xch a handle to an open hypervisor interface * @parm fd the file descriptor to save a domain to * @parm dom the id of the domain + * @parm checkpointed_stream non-zero if the far end of the stream is using + * checkpointing * @return 0 on success, -1 on failure */ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, uint32_t max_factor, uint32_t flags /* XCFLAGS_xxx */, - struct save_callbacks* callbacks, int hvm); + struct save_callbacks* callbacks, int hvm, + int checkpointed_stream); /* Domain Save v2 */ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, uint32_t max_factor, uint32_t flags, -struct save_callbacks* callbacks, int hvm); +struct save_callbacks* callbacks, int hvm, +int checkpointed_stream); /* callbacks provided by xc_domain_restore */ struct restore_callbacks { diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c index 301e770..c76980e 100644 --- a/tools/libxc/xc_domain_save.c +++ b/tools/libxc/xc_domain_save.c @@ -802,7 +802,8 @@ static int save_tsc_info(xc_interface *xch, uint32_t dom, int io_fd) int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, uint32_t max_factor, uint32_t flags, - struct save_callbacks* callbacks, int hvm) + struct save_callbacks* callbacks, int hvm, + int checkpointed_stream) { xc_dominfo_t info; DECLARE_DOMCTL; @@ -897,7 +898,8 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter if ( getenv(XG_MIGRATION_V2) ) { return xc_domain_save2(xch, io_fd, dom, max_iters, - max_factor, flags, callbacks, hvm); + max_factor, flags, callbacks, hvm, + checkpointed_stream); } DPRINTF(%s: starting save of domid %u, __func__, dom); diff --git a/tools/libxc/xc_nomigrate.c b/tools/libxc/xc_nomigrate.c index 76978a0..374d5bf 100644 --- a/tools/libxc/xc_nomigrate.c +++ b/tools/libxc/xc_nomigrate.c @@ -23,7 +23,8 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, uint32_t max_factor, uint32_t flags, - struct save_callbacks* callbacks, int hvm) + struct save_callbacks* callbacks, int hvm, + int checkpointed_stream) { errno = ENOSYS; return -1; diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index 42af55b..04f32f5 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -175,7 +175,7 @@ struct xc_sr_context bool live; /* Plain VM, or checkpoints over time. */ -bool checkpointed; +int checkpointed; /* Further debugging information in the stream. */ bool debug; diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index d63b783..6102b66 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -820,7 +820,8 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) int
[Xen-devel] [linux-linus test] 58966: regressions - FAIL
flight 58966 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/58966/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-rumpuserxen-amd64 15 rumpuserxen-demo-xenstorels/xenstorels.repeat fail REGR. vs. 58902 Tests which are failing intermittently (not blocking): test-amd64-i386-rumpuserxen-i386 15 rumpuserxen-demo-xenstorels/xenstorels.repeat fail pass in 58944 test-amd64-i386-qemuu-rhel6hvm-amd 9 redhat-installfail pass in 58944 test-amd64-i386-qemut-rhel6hvm-amd 9 redhat-install fail pass in 58959-bisect Regressions which are regarded as allowable (not blocking): test-amd64-i386-freebsd10-amd64 9 freebsd-install fail like 58902 test-amd64-i386-freebsd10-i386 9 freebsd-install fail like 58902 test-amd64-amd64-libvirt 11 guest-start fail like 58902 test-amd64-i386-libvirt 11 guest-start fail like 58902 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 58902 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 58902 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 13 guest-saverestorefail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass test-armhf-armhf-xl-rtds 11 guest-start fail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass version targeted for testing: linux4a10a91756ef381bced7b88cfb9232f660b92d93 baseline version: linuxaefbef10e3ae6e2c6e3c54f906f10b34c73a2c66 981 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass build-amd64-rumpuserxen pass build-i386-rumpuserxen pass test-amd64-amd64-xl pass test-armhf-armhf-xl pass test-amd64-i386-xl pass test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemut-debianhvm-amd64-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm pass test-amd64-amd64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-xl-xsm pass test-armhf-armhf-xl-xsm pass test-amd64-i386-xl-xsm pass test-amd64-amd64-xl-pvh-amd fail test-amd64-i386-qemut-rhel6hvm-amd fail
Re: [Xen-devel] [v3 10/15] vt-d: Add API to update IRTE when VT-d PI is used
On 24/06/15 06:18, Feng Wu wrote: This patch adds an API which is used to update the IRTE for posted-interrupt when guest changes MSI/MSI-X information. Signed-off-by: Feng Wu feng...@intel.com --- v3: - Remove adding PDA_MASK() when updating 'pda_l' and 'pda_h' for IRTE. - Change the return type of pi_update_irte() to int. - Remove some pointless printk message in pi_update_irte(). - Use structure assignment instead of memcpy() for irte copy. xen/drivers/passthrough/vtd/intremap.c | 98 ++ xen/drivers/passthrough/vtd/iommu.h| 2 + xen/include/asm-x86/iommu.h| 2 + 3 files changed, 102 insertions(+) diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c index b7a42f6..401a9d1 100644 --- a/xen/drivers/passthrough/vtd/intremap.c +++ b/xen/drivers/passthrough/vtd/intremap.c @@ -900,3 +900,101 @@ void iommu_disable_x2apic_IR(void) for_each_drhd_unit ( drhd ) disable_qinval(drhd-iommu); } + +static inline void setup_posted_irte( No need for inline here. +struct iremap_entry *new_ire, struct pi_desc *pi_desc, uint8_t gvec) const struct pi_desc *pi_desc +{ +new_ire-post.urg = 0; I would start by memset()ing the entire structure to 0, then filling in the non-zero bits. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 COLOPre 08/26] tools/libxc: support to resume uncooperative HVM guests
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: From: Wen Congyang we...@cn.fujitsu.com 1. suspend a. PVHVM and PV: we use the same way to suspend the guest(send the suspend space between guest and the open parenthesis please. request to the guest). If the guest doesn't support evtchn, the xenstore variant will be used, suspending the guest via XenBus control node. b. pure HVM: we call xc_domain_shutdown(..., SHUTDOWN_suspend) to suspend the guest 2. Resume: a. fast path In this case, we don't change the guest's state. PV: modify the return code to 1, and than call the domctl: XEN_DOMCTL_resumedomain PVHVM: same with PV HVM: do nothing in modify_returncode, and than call the domctl: XEN_DOMCTL_resumedomain b. slow In this case, we have changed the guest's state. have or will? AIUI the latter would be more accurate. PV: update start info, and reset all secondary CPU states. Than call the domctl: XEN_DOMCTL_resumedomain PVHVM and HVM can not be resumed. I'm confused -- isn't the purpose of this patch to make PVHM support resume? For PVHVM, in my test, only call the domctl: XEN_DOMCTL_resumedomain can work. I am not sure if we should update start info and reset all secondary CPU states. For pure HVM guest, in my test, only call the domctl: XEN_DOMCTL_resumedomain can work. So we can call libxl__domain_resume(..., 1) if we don't change the guest state, otherwise call libxl__domain_resume(..., 0). Hrm, so it sounds here like the correctness of this new functionality requires the caller to have not messed with the domain's state? What sort of changes are to the guest state are we talking about here? Isn't that a new requirement for this call? If so then it should be documented somewhere, specifically what sorts of changes are and are not allowed and the types of guests which are affected. Signed-off-by: Wen Congyang we...@cn.fujitsu.com Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com --- tools/libxc/xc_resume.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tools/libxc/xc_resume.c b/tools/libxc/xc_resume.c index e67bebd..bd82334 100644 --- a/tools/libxc/xc_resume.c +++ b/tools/libxc/xc_resume.c @@ -109,6 +109,23 @@ static int xc_domain_resume_cooperative(xc_interface *xch, uint32_t domid) return do_domctl(xch, domctl); } +static int xc_domain_resume_hvm(xc_interface *xch, uint32_t domid) +{ +DECLARE_DOMCTL; + +/* + * If it is PVHVM, the hypercall return code is 0, because this + * is not a fast path resume, we do not modify_returncode as in + * xc_domain_resume_cooperative. + * (resuming it in a new domain context) + * + * If it is a HVM, the hypercall is a NOP. + */ +domctl.cmd = XEN_DOMCTL_resumedomain; +domctl.domain = domid; +return do_domctl(xch, domctl); There are already several open coded instances of this XEN_DOMCTL_resumedomain, and I think putting this particular one into a helper is actually more confusing than just inlining this at the caller. In particular when reading this function my first question was how do we know this is not a fast path resume, the answer being that the only caller is the slow path resume case, but that's not evident from the context (what if someone adds a second call?) So I think at least the comment ought to go at the callsite, at which point this function doesn't add much. (Ideally all the open coded do_domctl would go into a single do_domainresume or something, but I don't think you need to do that unless you really want to). ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 COLOPre 11/26] tools/libxl: introduce a new API libxl__domain_restore() to load qemu state
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: Secondary vm is running in colo mode. So we will do the following things again and again: 1. suspend both primay vm and secondary vm 2. sync the state 3. resume both primary vm and secondary vm We will send qemu's state each time in step2, and slave's qemu should read it each time before resuming secondary vm. Introduce a new API libxl__domain_restore() to do it. This API should be called before resuming secondary vm. I think before this patch the state was passed to qemu as a parameter when it was launched, is that correct? If so then that would be worth mentioning for completeness. Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com Signed-off-by: Wen Congyang we...@cn.fujitsu.com Cc: Anthony Perard anthony.per...@citrix.com --- tools/libxl/libxl_dom_save.c | 49 tools/libxl/libxl_internal.h | 3 +++ tools/libxl/libxl_qmp.c | 10 + 3 files changed, 62 insertions(+) diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c index 8fe1625..0ad2894 100644 --- a/tools/libxl/libxl_dom_save.c +++ b/tools/libxl/libxl_dom_save.c @@ -639,6 +639,55 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf, } return 0; } + +static int libxl__domain_restore_device_model(libxl__gc *gc, uint32_t domid); + +int libxl__domain_restore(libxl__gc *gc, uint32_t domid) We don't have any libxl__domain_save counterpart, but we do have libxl__domain_save_device_model, so I wonder if the upcoming callers ought to just call that direct? Especially given that this function isn't any kind of generic domain restore, but has rather specific functionality (in particular it fails for PV guests). ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] x86/arm/mm: use gfn instead of pfn in p2m_get_mem_access/p2m_set_mem_access
On Mon, 2015-06-29 at 17:45 +0200, Vitaly Kuznetsov wrote: 'pfn' and 'start_pfn' are ambiguous, both these functions expect GFNs as input. On x86 the interface of p2m_set_mem_access() in p2m.c doesn't match the declaration in p2m-common.h as 'pfn' is being used instead of 'start_pfn'. On ARM both p2m_set_mem_access and p2m_get_mem_access interfaces don't match declarations from p2m-common.h: p2m_set_mem_access uses 'pfn' instead of 'start_pfn' and p2m_get_mem_access uses 'gpfn' instead of 'pfn'. Convert p2m_get_mem_access/p2m_set_mem_access (and __p2m_get_mem_access on ARM) interfaces to using gft_t instead of unsigned long and update all users of these functions. There is also an issue in p2m_get_mem_access on x86: 'gfn' parameter passed to gfn_lock/gfn_unlock is not defined. This code compiles only because of a coincidence: gfn_lock/gfn_unlock are currently macros which don't use their second argument. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com For ARM: Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel