Re: [Xen-devel] [v8][PATCH 02/17] introduce XEN_DOMCTL_set_rdm
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:24 PM This should be based on a new parameter globally, 'pci_rdmforce'. pci_rdmforce = 1 = Of course this should be 0 by default. '1' means we should force check to reserve all ranges. If failed VM wouldn't be created successfully. This also can give user a chance to work well with later hotplug, even if not a device assignment while creating VM. But we can override that by one specific pci device: pci = ['AA:BB.CC,rdmforce=0/1] But this 'rdmforce' should be 1 by default since obviously any passthrough device always need to do this. Actually no one really want to set as '0' so it may be unnecessary but I'd like to leave this as a potential approach. since no one requires it, why bother adding it? better to just keep global option. So this domctl provides an approach to control how to populate reserved device memory by tools. Note we always post a message to user about this once we owns RMRR. Signed-off-by: Tiejun Chen tiejun.c...@intel.com --- docs/man/xl.cfg.pod.5 | 6 + docs/misc/vtd.txt | 15 tools/libxc/include/xenctrl.h | 6 + tools/libxc/xc_domain.c| 28 +++ tools/libxl/libxl_create.c | 3 +++ tools/libxl/libxl_dm.c | 47 ++ tools/libxl/libxl_internal.h | 4 tools/libxl/libxl_types.idl| 2 ++ tools/libxl/libxlu_pci.c | 2 ++ tools/libxl/xl_cmdimpl.c | 10 xen/drivers/passthrough/pci.c | 39 +++ xen/drivers/passthrough/vtd/dmar.c | 8 +++ xen/include/asm-x86/hvm/domain.h | 4 xen/include/public/domctl.h| 21 + xen/xsm/flask/hooks.c | 1 + 15 files changed, 196 insertions(+) diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 index 622ea53..9adc41e 100644 --- a/docs/man/xl.cfg.pod.5 +++ b/docs/man/xl.cfg.pod.5 @@ -645,6 +645,12 @@ dom0 without confirmation. Please use with care. D0-D3hot power management states for the PCI device. False (0) by default. +=item Brdmforce=BOOLEAN + +(HVM/x86 only) Specifies that the VM would force to check and try to +reserve all reserved device memory, like RMRR, associated to the PCI +device. False (0) by default. + =back =back diff --git a/docs/misc/vtd.txt b/docs/misc/vtd.txt index 9af0e99..23544d5 100644 --- a/docs/misc/vtd.txt +++ b/docs/misc/vtd.txt @@ -111,6 +111,21 @@ in the config file: To override for a specific device: pci = [ '01:00.0,msitranslate=0', '03:00.0' ] +RDM, 'reserved device memory', for PCI Device Passthrough +- + +The BIOS controls some devices in terms of some reginos of memory used for +these devices. This kind of region should be reserved before creating a VM +to make sure they are not occupied by RAM/MMIO to conflict, and also we can +create necessary IOMMU table successfully. + +To enable this globally, add pci_rdmforce in the config file: + + pci_rdmforce = 1 (default is 0) + +Or just enable for a specific device: + pci = [ '01:00.0,rdmforce=1', '03:00.0' ] + Caveat on Conventional PCI Device Passthrough - diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 0ad8b8d..84012fe 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -2038,6 +2038,12 @@ int xc_assign_device(xc_interface *xch, uint32_t domid, uint32_t machine_bdf); +int xc_domain_device_setrdm(xc_interface *xch, +uint32_t domid, +uint32_t num_pcidevs, +uint32_t pci_rdmforce, +struct xen_guest_pcidev_info *pcidevs); + int xc_get_device_group(xc_interface *xch, uint32_t domid, uint32_t machine_bdf, diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index b864872..7fd43e9 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -1633,6 +1633,34 @@ int xc_assign_device( return do_domctl(xch, domctl); } +int xc_domain_device_setrdm(xc_interface *xch, +uint32_t domid, +uint32_t num_pcidevs, +uint32_t pci_rdmforce, +struct xen_guest_pcidev_info *pcidevs) +{ +int ret; +DECLARE_DOMCTL; +DECLARE_HYPERCALL_BOUNCE(pcidevs, + num_pcidevs*sizeof(xen_guest_pcidev_info_t), + XC_HYPERCALL_BUFFER_BOUNCE_IN); + +if ( xc_hypercall_bounce_pre(xch, pcidevs) ) +return -1; + +domctl.cmd = XEN_DOMCTL_set_rdm; +domctl.domain =
Re: [Xen-devel] [v8][PATCH 04/17] update the existing hypercall to support XEN_DOMCTL_set_rdm
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:24 PM After we intend to expost that hypercall explicitly based on XEN_DOMCTL_set_rdm, we need this rebase. I hope we can squash this into that previous patch once Jan Ack this. better to merge together, since it's the right thing to do based on previous discussion. one question about 'd-arch.hvm_domain.pci_force'. My impression is that this flag enables force check, and while enabled, you'll always do selected BDF filtering by default. However from below code, seems pci_force is used to whether report all or selected regions. Am I reading it wrong? Signed-off-by: Tiejun Chen tiejun.c...@intel.com --- xen/common/compat/memory.c | 75 ++ xen/common/memory.c| 71 +--- xen/drivers/passthrough/vtd/dmar.c | 32 xen/include/public/memory.h| 5 +++ xen/include/xen/iommu.h| 2 +- xen/include/xen/pci.h | 2 + 6 files changed, 148 insertions(+), 39 deletions(-) diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c index 60512fa..e6a256e 100644 --- a/xen/common/compat/memory.c +++ b/xen/common/compat/memory.c @@ -22,27 +22,66 @@ struct get_reserved_device_memory { unsigned int used_entries; }; -static int get_reserved_device_memory(xen_pfn_t start, - xen_ulong_t nr, void *ctxt) +static int get_reserved_device_memory(xen_pfn_t start, xen_ulong_t nr, + u32 id, void *ctxt) { struct get_reserved_device_memory *grdm = ctxt; +struct domain *d; +unsigned int i; +u32 sbdf; +struct compat_reserved_device_memory rdm = { +.start_pfn = start, .nr_pages = nr +}; -if ( grdm-used_entries grdm-map.nr_entries ) -{ -struct compat_reserved_device_memory rdm = { -.start_pfn = start, .nr_pages = nr -}; +if ( rdm.start_pfn != start || rdm.nr_pages != nr ) +return -ERANGE; -if ( rdm.start_pfn != start || rdm.nr_pages != nr ) -return -ERANGE; +d = rcu_lock_domain_by_any_id(grdm-map.domid); +if ( d == NULL ) +return -ESRCH; -if ( __copy_to_compat_offset(grdm-map.buffer, grdm-used_entries, - rdm, 1) ) -return -EFAULT; +if ( d ) +{ +if ( d-arch.hvm_domain.pci_force ) +{ +if ( grdm-used_entries grdm-map.nr_entries ) +{ +if ( __copy_to_compat_offset(grdm-map.buffer, + grdm-used_entries, + rdm, 1) ) +{ +rcu_unlock_domain(d); +return -EFAULT; +} +} +++grdm-used_entries; +} +else +{ +for ( i = 0; i d-arch.hvm_domain.num_pcidevs; i++ ) +{ +sbdf = PCI_SBDF2(d-arch.hvm_domain.pcidevs[i].seg, + d-arch.hvm_domain.pcidevs[i].bus, + d-arch.hvm_domain.pcidevs[i].devfn); +if ( sbdf == id ) +{ +if ( grdm-used_entries grdm-map.nr_entries ) +{ +if ( __copy_to_compat_offset(grdm-map.buffer, + grdm-used_entries, + rdm, 1) ) +{ +rcu_unlock_domain(d); +return -EFAULT; +} +} +++grdm-used_entries; +} +} +} } -++grdm-used_entries; - +rcu_unlock_domain(d); return 0; } #endif @@ -319,9 +358,13 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) if ( !rc grdm.map.nr_entries grdm.used_entries ) rc = -ENOBUFS; + grdm.map.nr_entries = grdm.used_entries; -if ( __copy_to_guest(compat, grdm.map, 1) ) -rc = -EFAULT; +if ( grdm.map.nr_entries ) +{ +if ( __copy_to_guest(compat, grdm.map, 1) ) +rc = -EFAULT; +} return rc; } diff --git a/xen/common/memory.c b/xen/common/memory.c index 4788acc..9ce82b1 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -698,24 +698,63 @@ struct get_reserved_device_memory { unsigned int used_entries; }; -static int get_reserved_device_memory(xen_pfn_t start, - xen_ulong_t nr, void *ctxt) +static int get_reserved_device_memory(xen_pfn_t start, xen_ulong_t nr, + u32 id, void *ctxt)
Re: [Xen-devel] [v8][PATCH 06/17] tools/libxc: check if modules space is overlapping with reserved device memory
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:24 PM In case of reserved device memory overlapping with ram, it also probably overlap with modules space so we need to check these reserved device memory as well. Signed-off-by: Tiejun Chen tiejun.c...@intel.com Reviewed-by: Kevin Tian kevin.t...@intel.com, with one small comment --- tools/libxc/xc_hvm_build_x86.c | 94 +++--- 1 file changed, 79 insertions(+), 15 deletions(-) diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c index c81a25b..ddcf06d 100644 --- a/tools/libxc/xc_hvm_build_x86.c +++ b/tools/libxc/xc_hvm_build_x86.c @@ -54,9 +54,82 @@ #define VGA_HOLE_SIZE (0x20) +/* + * Check whether there exists mmio hole in the specified memory range. + * Returns 1 if exists, else returns 0. + */ +static int check_mmio_hole(uint64_t start, uint64_t memsize, + uint64_t mmio_start, uint64_t mmio_size) +{ +if ( start + memsize = mmio_start || start = mmio_start + mmio_size ) +return 0; +else +return 1; +} + +/* Getting all reserved device memory map info. */ +static struct xen_reserved_device_memory +*xc_get_reserved_device_memory_map(xc_interface *xch, unsigned int nr_entries, + uint32_t dom) +{ +struct xen_reserved_device_memory *xrdm = NULL; +int rc = xc_reserved_device_memory_map(xch, dom, xrdm, nr_entries); + +if ( rc 0 ) +{ +if ( errno == ENOBUFS ) +{ +if ( (xrdm = malloc(nr_entries * + sizeof(xen_reserved_device_memory_t))) == NULL ) +{ +PERROR(Could not allocate memory.); +return 0; +} +rc = xc_reserved_device_memory_map(xch, dom, xrdm, nr_entries); +if ( rc ) +{ +PERROR(Could not get reserved device memory maps.); +free(xrdm); +return 0; +} +} +else +PERROR(Could not get reserved device memory maps.); +} + +return xrdm; +} + +static int xc_check_modules_space(xc_interface *xch, uint64_t *mstart_out, + uint64_t *mend_out, uint32_t dom) +{ +unsigned int i = 0, nr_entries = 0; +uint64_t rdm_start = 0, rdm_end = 0; +struct xen_reserved_device_memory *rdm_map = +xc_get_reserved_device_memory_map(xch, nr_entries, dom); + +for ( i = 0; i nr_entries; i++ ) +{ +rdm_start = (uint64_t)rdm_map[i].start_pfn XC_PAGE_SHIFT; +rdm_end = rdm_start + ((uint64_t)rdm_map[i].nr_pages XC_PAGE_SHIFT); + +/* Just use check_mmio_hole() to check modules ranges. */ +if ( check_mmio_hole(rdm_start, + rdm_end - rdm_start, then you don't need a rdm_end variable here, since only size is wanted. + *mstart_out, *mend_out) ) +return -1; +} + +free(rdm_map); + +return 0; +} + static int modules_init(struct xc_hvm_build_args *args, uint64_t vend, struct elf_binary *elf, -uint64_t *mstart_out, uint64_t *mend_out) +uint64_t *mstart_out, uint64_t *mend_out, +xc_interface *xch, +uint32_t dom) { #define MODULE_ALIGN 1UL 7 #define MB_ALIGN 1UL 20 @@ -80,6 +153,10 @@ static int modules_init(struct xc_hvm_build_args *args, if ( *mend_out vend ) return -1; +/* Is it overlapping with reserved device memory? */ +if ( xc_check_modules_space(xch, mstart_out, mend_out, dom) ) +return -1; + if ( args-acpi_module.length != 0 ) args-acpi_module.guest_addr_out = *mstart_out; if ( args-smbios_module.length != 0 ) @@ -226,19 +303,6 @@ static int loadmodules(xc_interface *xch, return rc; } -/* - * Check whether there exists mmio hole in the specified memory range. - * Returns 1 if exists, else returns 0. - */ -static int check_mmio_hole(uint64_t start, uint64_t memsize, - uint64_t mmio_start, uint64_t mmio_size) -{ -if ( start + memsize = mmio_start || start = mmio_start + mmio_size ) -return 0; -else -return 1; -} - static int setup_guest(xc_interface *xch, uint32_t dom, struct xc_hvm_build_args *args, char *image, unsigned long image_size) @@ -282,7 +346,7 @@ static int setup_guest(xc_interface *xch, goto error_out; } -if ( modules_init(args, v_end, elf, m_start, m_end) != 0 ) +if ( modules_init(args, v_end, elf, m_start, m_end, xch, dom) != 0 ) { ERROR(Insufficient space to load modules.); goto error_out; -- 1.9.1
Re: [Xen-devel] [PATCH v4] x86: add p2m_mmio_write_dm
On 02.12.14 at 08:48, yu.c.zh...@linux.intel.com wrote: On 12/1/2014 8:31 PM, Jan Beulich wrote: On 01.12.14 at 13:13, t...@xen.org wrote: At 11:17 + on 01 Dec (1417429027), Jan Beulich wrote: On 01.12.14 at 11:30, t...@xen.org wrote: At 09:32 + on 01 Dec (1417422746), Jan Beulich wrote: On 01.12.14 at 09:49, yu.c.zh...@linux.intel.com wrote: To my understanding, pages with p2m_ram_ro are not supposed to be modified by guest. So in __hvm_copy(), when p2m type of a page is p2m_ram_rom, no copy will occur. However, to our usage, we just wanna this page to be write protected, so that our device model can be triggered to do some emulation. The content written to this page is supposed not to be dropped. This way, if sequentially a read operation is performed by guest to this page, the guest will still see its anticipated value. __hvm_copy() is only a helper function, and doesn't write to mmio_dm space either; instead its (indirect) callers would invoke hvmemul_do_mmio() upon seeing HVMCOPY_bad_gfn_to_mfn returns. The question hence is about the apparent inconsistency resulting from writes to ram_ro being dropped here but getting passed to the DM by hvm_hap_nested_page_fault(). Tim - is that really intentional? No - and AFAICT it shouldn't be happening. It _is_ how it was implemented originally, because it involved fewer moving parts and didn't need to be efficient (and after all, writes to entirely missing addresses go to the device model too). But the code was later updated to log and discard writes to read-only memory (see 4d8aa29 from Trolle Selander). Early version of p2m_ram_ro were documented in the internal headers as sending the writes to the DM, but the public interface (HVMMEM_ram_ro) has always said that writes are discarded. Hmm, so which way do you recommend resolving the inconsistency then - match what the public interface says or what the apparent original intention for the internal type was? Presumably we need to follow the public interface mandated model, and hence require the new type to be introduced. Sorry, I was unclear -- there isn't an inconsistency; both internal and public headers currently say that writes are discarded and AFAICT that is what the code does. Not for hvm_hap_nested_page_fault() afaict - the forwarding to DM there contradicts the writes are discarded model that other code paths follow. Thanks, Jan. By inconsistency, do you mean the p2m_ram_ro shall not trigger the handle_mmio_with_translation() in hvm_hap_nested_page_fault()? Yes, pending Tim's agreement. I'm also a bit confused with the writes are discarded/dropped comments in the code. Does this mean writes to the p2m_ram_ro pages should be abandoned without going to the dm, or going to the dm and ignored later? The code seems to be the second one. And it would seem to me that it should be the former. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] PVHVM drivers in upstream linux kernel
Hi, looking into the upstream linux sources I realized that the PVHVM drivers of XEN are only available with the pvops kernel. Is this on purpose? Shouldn't the frontend drivers, xen/platform-pci.c etc. be configurable without having to enable CONFIG_PARAVIRT? Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v8][PATCH 09/17] hvmloader/ram: check if guest memory is out of reserved device memory maps
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:24 PM We need to check to reserve all reserved device memory maps in e820 to avoid any potential guest memory conflict. Currently, if we can't insert RDM entries directly, we may need to handle several ranges as follows: a. Fixed Ranges -- BUG() lowmem_reserved_base-0xA: reserved by BIOS implementation, BIOS region, RESERVED_MEMBASE ~ 0x1, b. RAM or RAM:Hole - Try to reserve Signed-off-by: Tiejun Chen tiejun.c...@intel.com --- tools/firmware/hvmloader/e820.c | 168 1 file changed, 168 insertions(+) diff --git a/tools/firmware/hvmloader/e820.c b/tools/firmware/hvmloader/e820.c index 2e05e93..ef87e41 100644 --- a/tools/firmware/hvmloader/e820.c +++ b/tools/firmware/hvmloader/e820.c @@ -22,6 +22,7 @@ #include config.h #include util.h +#include xen/memory.h void dump_e820_table(struct e820entry *e820, unsigned int nr) { @@ -68,12 +69,173 @@ void dump_e820_table(struct e820entry *e820, unsigned int nr) } } +extern struct xen_reserved_device_memory *rdm_map; +static unsigned int construct_rdm_e820_maps(unsigned int next_e820_entry_index, +uint32_t nr_map, +struct xen_reserved_device_memory *map, +struct e820entry *e820, +unsigned int lowmem_reserved_base, +unsigned int bios_image_base) +{ +unsigned int i, j, sum_nr; +uint64_t start, end, next_start, rdm_start, rdm_end; +uint32_t type; +int err = 0; + +for ( i = 0; i nr_map; i++ ) +{ +rdm_start = (uint64_t)map[i].start_pfn PAGE_SHIFT; +rdm_end = rdm_start + ((uint64_t)map[i].nr_pages PAGE_SHIFT); + +for ( j = 0; j next_e820_entry_index - 1; j++ ) +{ +sum_nr = next_e820_entry_index + nr_map; need a check whether sum_nr exceeds max #entries... +start = e820[j].addr; +end = e820[j].addr + e820[j].size; +type = e820[j].type; +next_start = e820[j+1].addr; + +if ( rdm_start = start rdm_start = end ) +{ what about other confliction pattern here, e.g. rdm_end between [start,end]? +/* + * lowmem_reserved_base-0xA: reserved by BIOS + * implementation. + * Or BIOS region. + */ +if ( (lowmem_reserved_base 0xA +start == lowmem_reserved_base) || + start == bios_image_base ) +{ +err = -1; +break; +} +} + +/* Just amid those remaining e820 entries. */ +if ( (rdm_start end) (rdm_end next_start) ) = and =? +{ +memmove(e820[j+2], e820[j+1], +(sum_nr - j - 1) * sizeof(struct e820entry)); seems you just need (next_e820_entry_index - j - 1) + +/* Then fill RMRR into that entry. */ +e820[j+1].addr = rdm_start; +e820[j+1].size = rdm_end - rdm_start; +e820[j+1].type = E820_RESERVED; +next_e820_entry_index++; +continue; continue-break? otherwise the newly added rmrr entry will be checked next for same rmrr entry, and then catch an unnecessary confliction. +} + +/* Already at the end. */ +if ( (rdm_start end) !next_start ) +{ +e820[next_e820_entry_index].addr = rdm_start; +e820[next_e820_entry_index].size = rdm_end - rdm_start; +e820[next_e820_entry_index].type = E820_RESERVED; +next_e820_entry_index++; +continue; break though it's the last one. +} + +if ( type == E820_RAM ) +{ +/* If coincide with one RAM range. */ +if ( rdm_start == start rdm_end == end) +{ +e820[j].type = E820_RESERVED; +continue; +} + +/* If we're just aligned with start of one RAM range. */ +if ( rdm_start == start rdm_end end ) +{ +memmove(e820[j+1], e820[j], +(sum_nr - j) * sizeof(struct e820entry)); + +e820[j+1].addr = rdm_end; +e820[j+1].size = e820[j].addr + e820[j].size - rdm_end; +e820[j+1].type = E820_RAM; +next_e820_entry_index++; + +e820[j].addr = rdm_start; +e820[j].size = rdm_end - rdm_start; +
Re: [Xen-devel] [v8][PATCH 10/17] hvmloader/mem_hole_alloc: skip any overlap with reserved device memory
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:24 PM In some cases like igd_opregion_pgbase, guest will use mem_hole_alloc to allocate some memory to use in runtime cycle, so we alsoe need to make sure all reserved device memory don't overlap such a region. OK, seems you meant to use this patch to address opregion confliction. when it works, I think it's better to still add opregion detection in e820, as a modulo suggestion. It's not good to make assumption in one module about how other module works. Now opregion is allocated dynamically, but it may be fixed somewhere in the future. So you always need to detect confliction, purely in e820 world, regardless of how a range is actually allocated. Signed-off-by: Tiejun Chen tiejun.c...@intel.com --- tools/firmware/hvmloader/util.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c index 8767897..f3723c7 100644 --- a/tools/firmware/hvmloader/util.c +++ b/tools/firmware/hvmloader/util.c @@ -416,9 +416,29 @@ static uint32_t alloc_down = RESERVED_MEMORY_DYNAMIC_END; xen_pfn_t mem_hole_alloc(uint32_t nr_mfns) { +unsigned int i, num = hvm_get_reserved_device_memory_map(); +uint64_t rdm_start, rdm_end; +uint32_t alloc_start, alloc_end; + alloc_down -= nr_mfns PAGE_SHIFT; +alloc_start = alloc_down; +alloc_end = alloc_start + (nr_mfns PAGE_SHIFT); +for ( i = 0; i num; i++ ) +{ +rdm_start = (uint64_t)rdm_map[i].start_pfn PAGE_SHIFT; +rdm_end = rdm_start + ((uint64_t)rdm_map[i].nr_pages PAGE_SHIFT); +if ( check_rdm_hole_conflict((uint64_t)alloc_start, + (uint64_t)alloc_end, + rdm_start, rdm_end - rdm_start) ) +{ +alloc_end = rdm_start; +alloc_start = alloc_end - (nr_mfns PAGE_SHIFT); +BUG_ON(alloc_up = alloc_start); +} +} + BUG_ON(alloc_up = alloc_down); -return alloc_down PAGE_SHIFT; +return alloc_start PAGE_SHIFT; } this patch is required, but I'd prefer to have an initialization phase check to have a sane alloc_up/down, so you don't bother detection for every run-time call. void *mem_alloc(uint32_t size, uint32_t align) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5] call xc_domain_irq_permission before xc_domain_irq_permission
On 01.12.14 at 20:22, stefano.stabell...@eu.citrix.com wrote: xc_physdev_unmap_pirq might revoke the permission to map the irq from the domain causing the following xc_domain_irq_permission call to fail and return error (domain_pirq_to_irq returns 0). Apart from the patch title being rather confusing, isn't the root problem then xc_physdev_unmap_pirq() removes permissions? At least after the strict splitting of permission granting and mapping for MMIO (commit 0561e1f0) it would seem that if the underlying hypercall here behaves similarly to the original behavior there, it ought to be fixed in an analogous way. Jan Call xc_domain_irq_permission first to prevent this from happening (xc_physdev_unmap_pirq calls irq_deny_access, that doesn't fail if the permission is already removed). This is a simple bug fix and I think should go in 4.5. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index 316643c..904d255 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -1288,14 +1290,14 @@ skip1: goto out; } if ((fscanf(f, %u, irq) == 1) irq) { -rc = xc_physdev_unmap_pirq(ctx-xch, domid, irq); -if (rc 0) { -LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_physdev_unmap_pirq irq=%d, irq); -} rc = xc_domain_irq_permission(ctx-xch, domid, irq, 0); if (rc 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_domain_irq_permission irq=%d, irq); } +rc = xc_physdev_unmap_pirq(ctx-xch, domid, irq); +if (rc 0) { +LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_physdev_unmap_pirq irq=%d, irq); +} } fclose(f); } ___ 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] [v8][PATCH 11/17] xen/x86/p2m: reject populating for reserved device memory mapping
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:24 PM We need to reject to populate reserved device memory mapping, and then make sure all reserved device memory can't be accessed by any !iommu approach. Signed-off-by: Tiejun Chen tiejun.c...@intel.com --- xen/arch/x86/mm/p2m.c | 59 +-- xen/include/asm-x86/p2m.h | 9 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index efa49dd..607ecd0 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -556,6 +556,40 @@ guest_physmap_remove_page(struct domain *d, unsigned long gfn, gfn_unlock(p2m, gfn, page_order); } +/* Check if we are accessing rdm. */ +int p2m_check_reserved_device_memory(xen_pfn_t start, xen_ulong_t nr, + u32 id, void *ctxt) +{ +xen_pfn_t end = start + nr; +unsigned int i; +u32 sbdf; +struct p2m_get_reserved_device_memory *pgrdm = ctxt; +struct domain *d = pgrdm-domain; + +if ( d-arch.hvm_domain.pci_force ) +{ +if ( pgrdm-gfn = start pgrdm-gfn end ) +return 1; +} +else +{ +for ( i = 0; i d-arch.hvm_domain.num_pcidevs; i++ ) +{ +sbdf = PCI_SBDF2(d-arch.hvm_domain.pcidevs[i].seg, + d-arch.hvm_domain.pcidevs[i].bus, + d-arch.hvm_domain.pcidevs[i].devfn); + +if ( sbdf == id ) +{ +if ( pgrdm-gfn = start pgrdm-gfn end ) +return 1; +} +} +} + +return 0; +} + int guest_physmap_add_entry(struct domain *d, unsigned long gfn, unsigned long mfn, unsigned int page_order, @@ -568,6 +602,7 @@ guest_physmap_add_entry(struct domain *d, unsigned long gfn, mfn_t omfn; int pod_count = 0; int rc = 0; +struct p2m_get_reserved_device_memory pgrdm; if ( !paging_mode_translate(d) ) { @@ -686,8 +721,28 @@ guest_physmap_add_entry(struct domain *d, unsigned long gfn, /* Now, actually do the two-way mapping */ if ( mfn_valid(_mfn(mfn)) ) { -rc = p2m_set_entry(p2m, gfn, _mfn(mfn), page_order, t, - p2m-default_access); +pgrdm.gfn = gfn; +pgrdm.domain = d; +if ( !is_hardware_domain(d) iommu_use_hap_pt(d) ) why is this check only for shared case? +{ +rc = iommu_get_reserved_device_memory(p2m_check_reserved_device_memory, + pgrdm); +/* We always avoid populating reserved device memory. */ +if ( rc == 1 ) +{ +rc = -EBUSY; +goto out; +} +else if ( rc 0 ) +{ +printk(XENLOG_G_WARNING + Can't check reserved device memory for Dom%d.\n, + d-domain_id); +goto out; +} +} + +rc = p2m_set_entry(p2m, gfn, _mfn(mfn), page_order, t, p2m-default_access); if ( rc ) goto out; /* Failed to update p2m, bail without updating m2p. */ diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h index 5f7fe71..99f7fb7 100644 --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -709,6 +709,15 @@ static inline unsigned int p2m_get_iommu_flags(p2m_type_t p2mt) return flags; } +struct p2m_get_reserved_device_memory { +unsigned long gfn; +struct domain *domain; +}; + +/* Check if we are accessing rdm. */ +extern int p2m_check_reserved_device_memory(xen_pfn_t start, xen_ulong_t nr, +u32 id, void *ctxt); + #endif /* _XEN_P2M_H */ /* -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v8][PATCH 12/17] xen/x86/ept: handle reserved device memory in ept_handle_violation
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:25 PM We always reserve these ranges since we never allow any stuff to poke them. But in theory some untrusted VM can maliciously access them. So we need to intercept this approach. But we just don't want to leak anything or introduce any side affect since other OSs may touch them by careless behavior, so its enough to have a lightweight way, and it shouldn't be same as those broken pages which cause domain crush. So we just need to return with next eip then let VM/OS itself handle such a scenario as its own logic. Signed-off-by: Tiejun Chen tiejun.c...@intel.com --- xen/arch/x86/hvm/vmx/vmx.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 2907afa..3ee884a 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -2403,6 +2403,7 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa) p2m_type_t p2mt; int ret; struct domain *d = current-domain; +struct p2m_get_reserved_device_memory pgrdm; /* * We treat all write violations also as read violations. @@ -2438,6 +2439,23 @@ static void ept_handle_violation(unsigned long qualification, paddr_t gpa) __trace_var(TRC_HVM_NPF, 0, sizeof(_d), _d); } +/* This means some untrusted VM can maliciously access reserved + * device memory. But we just don't want to leak anything or + * introduce any side affect since other OSs may touch them by + * careless behavior, so its enough to have a lightweight way. + * Here we just need to return with next eip then let VM/OS itself + * handle such a scenario as its own logic. + */ +pgrdm.gfn = gfn; +pgrdm.domain = d; +ret = iommu_get_reserved_device_memory(p2m_check_reserved_device_memory, + pgrdm); can this be optimized to not walk rmrr map if no device is assigned? +if ( ret ) +{ +update_guest_eip(); +return; +} + if ( qualification EPT_GLA_VALID ) { __vmread(GUEST_LINEAR_ADDRESS, gla); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v8][PATCH 14/17] xen/x86/p2m: introduce set_identity_p2m_entry
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:25 PM We will create RMRR mapping as follows: If gfn space unoccupied, we just set that. If space already occupy by 1:1 RMRR mapping do thing. Others should be failed. Signed-off-by: Tiejun Chen tiejun.c...@intel.com Reviewed-by: Kevin Tian kevin.t...@intel.com --- xen/arch/x86/mm/p2m.c | 28 xen/include/asm-x86/p2m.h | 4 2 files changed, 32 insertions(+) diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 607ecd0..c415521 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -913,6 +913,34 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn) return set_typed_p2m_entry(d, gfn, mfn, p2m_mmio_direct); } +int set_identity_p2m_entry(struct domain *d, unsigned long gfn, + p2m_access_t p2ma) +{ +p2m_type_t p2mt; +p2m_access_t a; +mfn_t mfn; +struct p2m_domain *p2m = p2m_get_hostp2m(d); +int ret = -EBUSY; + +gfn_lock(p2m, gfn, 0); + +mfn = p2m-get_entry(p2m, gfn, p2mt, a, 0, NULL); + +if ( !mfn_valid(mfn) ) +ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K, p2m_mmio_direct, +p2ma); +else if ( mfn_x(mfn) == gfn p2mt == p2m_mmio_direct a == p2ma ) +ret = 0; +else +printk(XENLOG_G_WARNING + Cannot identity map d%d:%lx, already mapped to %lx.\n, + d-domain_id, gfn, mfn_x(mfn)); + +gfn_unlock(p2m, gfn, 0); + +return ret; +} + /* Returns: 0 for success, -errno for failure */ int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn) { diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h index 99f7fb7..26cf0cc 100644 --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -509,6 +509,10 @@ int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start, int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn); int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn); +/* Set identity addresses in the p2m table (for pass-through) */ +int set_identity_p2m_entry(struct domain *d, unsigned long gfn, + p2m_access_t p2ma); + /* Add foreign mapping to the guest's p2m table. */ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn, unsigned long gpfn, domid_t foreign_domid); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v8][PATCH 15/17] xen:vtd: create RMRR mapping
From: Chen, Tiejun Sent: Monday, December 01, 2014 5:25 PM intel_iommu_map_page() does nothing if VT-d shares EPT page table. So rmrr_identity_mapping() never create RMRR mapping but in some cases like some GFX drivers it still need to access RMRR. Here we will create those RMRR mappings even in shared EPT case. Signed-off-by: Tiejun Chen tiejun.c...@intel.com Acked-by: Kevin Tian kevin.t...@intel.com --- xen/drivers/passthrough/vtd/iommu.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index a38f201..a54c6eb 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1856,10 +1856,15 @@ static int rmrr_identity_mapping(struct domain *d, bool_t map, while ( base_pfn end_pfn ) { -int err = intel_iommu_map_page(d, base_pfn, base_pfn, - IOMMUF_readable|IOMMUF_writable); - -if ( err ) +int err = 0; +if ( iommu_use_hap_pt(d) ) +{ +ASSERT(!iommu_passthrough || !is_hardware_domain(d)); +if ( (err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw)) ) +return err; +} +else if ( (err = intel_iommu_map_page(d, base_pfn, base_pfn, + IOMMUF_readable|IOMMUF_writable)) ) return err; base_pfn++; } -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant table state
Rename lock to maptrack_lock and use it to protect maptrack state. The new rwlock is used to prevent readers from accessing inconsistent grant table state such as current version, partially initialized active table pages, etc. Signed-off-by: Matt Wilson m...@amazon.com [chegger: ported to xen-staging, split into multiple commits] Signed-off-by: Christoph Egger cheg...@amazon.de Cc: Jan Beulich jbeul...@suse.com Cc: Keir Fraser k...@xen.org --- docs/misc/grant-tables.txt| 28 - xen/arch/x86/mm.c |4 +- xen/common/grant_table.c | 135 - xen/include/xen/grant_table.h |9 ++- 4 files changed, 115 insertions(+), 61 deletions(-) diff --git a/docs/misc/grant-tables.txt b/docs/misc/grant-tables.txt index 19db4ec..c9ae8f2 100644 --- a/docs/misc/grant-tables.txt +++ b/docs/misc/grant-tables.txt @@ -74,7 +74,33 @@ is complete. matching map track entry is then removed, as if unmap had been invoked. These are not used by the transfer mechanism. map-domid : owner of the mapped frame - map-ref_and_flags : grant reference, ro/rw, mapped for host or device access + map-ref : grant reference + map-flags : ro/rw, mapped for host or device access + + + Locking + ~~~ + Xen uses several locks to serialize access to the internal grant table state. + + grant_table-lock : rwlock used to prevent readers from accessing + inconsistent grant table state such as current + version, partially initialized active table pages, + etc. + grant_table-maptrack_lock : spinlock used to protect the maptrack state + + The primary lock for the grant table is a read/write spinlock. All + functions that access members of struct grant_table must acquire a + read lock around critical sections. Any modification to the members + of struct grant_table (e.g., nr_status_frames, nr_grant_frames, + active frames, etc.) must only be made if the write lock is + held. These elements are read-mostly, and read critical sections can + be large, which makes a rwlock a good choice. + + The maptrack state is protected by its own spinlock. Any access (read + or write) of struct grant_table members that have a maptrack_ + prefix must be made while holding the maptrack lock. The maptrack + state can be rapidly modified under some workloads, and the critical + sections are very small, thus we use a spinlock to protect them. diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 522c43d..37c13b1 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4565,7 +4565,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-shared_info); break; case XENMAPSPACE_grant_table: -spin_lock(d-grant_table-lock); +write_lock(d-grant_table-lock); if ( d-grant_table-gt_version == 0 ) d-grant_table-gt_version = 1; @@ -4587,7 +4587,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-grant_table-shared_raw[idx]); } -spin_unlock(d-grant_table-lock); +write_unlock(d-grant_table-lock); break; case XENMAPSPACE_gmfn_range: case XENMAPSPACE_gmfn: diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 8fba923..018b5b6 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -227,23 +227,23 @@ double_gt_lock(struct grant_table *lgt, struct grant_table *rgt) { if ( lgt rgt ) { -spin_lock(lgt-lock); -spin_lock(rgt-lock); +spin_lock(lgt-maptrack_lock); +spin_lock(rgt-maptrack_lock); } else { if ( lgt != rgt ) -spin_lock(rgt-lock); -spin_lock(lgt-lock); +spin_lock(rgt-maptrack_lock); +spin_lock(lgt-maptrack_lock); } } static inline void double_gt_unlock(struct grant_table *lgt, struct grant_table *rgt) { -spin_unlock(lgt-lock); +spin_unlock(lgt-maptrack_lock); if ( lgt != rgt ) -spin_unlock(rgt-lock); +spin_unlock(rgt-maptrack_lock); } static inline int @@ -261,10 +261,10 @@ static inline void put_maptrack_handle( struct grant_table *t, int handle) { -spin_lock(t-lock); +spin_lock(t-maptrack_lock); maptrack_entry(t, handle).ref = t-maptrack_head; t-maptrack_head = handle; -spin_unlock(t-lock); +spin_unlock(t-maptrack_lock); } static inline int @@ -276,7 +276,7 @@ get_maptrack_handle( struct grant_mapping *new_mt; unsigned int new_mt_limit, nr_frames; -spin_lock(lgt-lock); +spin_lock(lgt-maptrack_lock); while ( unlikely((handle = __get_maptrack_handle(lgt)) == -1) ) { @@
[Xen-devel] [PATCH 0/2] gnttab: Improve scaleability
This patch series changes the grant table locking to a more fain grained locking protocol. The result is a performance boost measured with blkfront/blkback. Document the locking protocol. [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant [PATCH 2/2] gnttab: refactor locking for scalability ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/2] gnttab: refactor locking for scalability
From: Matt Wilson m...@amazon.com This patch refactors grant table locking. It splits the previous single spinlock per grant table into multiple locks. The heavily modified components of the grant table (the maptrack state and the active entries) are now protected by their own spinlocks. The remaining elements of the grant table are read-mostly, so the main grant table lock is modified to be a rwlock to improve concurrency. Workloads with high grant table operation rates, especially map/unmap operations as used by blkback/blkfront when persistent grants are not supported, show marked improvement with these changes. A domU with 24 VBDs in a streaming 2M write workload achieved 1,400 MB/sec before this change. Performance more than doubles with this patch, reaching 3,000 MB/sec before tuning and 3,600 MB/sec after adjusting event channel vCPU bindings. Signed-off-by: Matt Wilson m...@amazon.com [chegger: ported to xen-staging, split into multiple commits] Signed-off-by: Christoph Egger cheg...@amazon.de Cc: Anthony Liguori aligu...@amazon.com Cc: Jan Beulich jbeul...@suse.com Cc: Keir Fraser k...@xen.org --- docs/misc/grant-tables.txt | 21 + xen/common/grant_table.c | 213 2 files changed, 157 insertions(+), 77 deletions(-) diff --git a/docs/misc/grant-tables.txt b/docs/misc/grant-tables.txt index c9ae8f2..1ada018 100644 --- a/docs/misc/grant-tables.txt +++ b/docs/misc/grant-tables.txt @@ -63,6 +63,7 @@ is complete. act-domid : remote domain being granted rights act-frame : machine frame being granted act-pin : used to hold reference counts + act-lock : spinlock used to serialize access to active entry state Map tracking @@ -87,6 +88,8 @@ is complete. version, partially initialized active table pages, etc. grant_table-maptrack_lock : spinlock used to protect the maptrack state + active_grant_entry-lock : spinlock used to serialize modifications to + active entries The primary lock for the grant table is a read/write spinlock. All functions that access members of struct grant_table must acquire a @@ -102,6 +105,24 @@ is complete. state can be rapidly modified under some workloads, and the critical sections are very small, thus we use a spinlock to protect them. + Active entries are obtained by calling active_entry_acquire(gt, ref). + This function returns a pointer to the active entry after locking its + spinlock. The caller must hold the rwlock for the gt in question + before calling active_entry_acquire(). This is because the grant + table can be dynamically extended via gnttab_grow_table() while a + domain is running and must be fully initialized. Once all access to + the active entry is complete, release the lock by calling + active_entry_release(act). + + Summary of rules for locking: + active_entry_acquire() and active_entry_release() can only be + called when holding the relevant grant table's lock. I.e.: +read_lock(gt-lock); +act = active_entry_acquire(gt, ref); +... +active_entry_release(act); +read_unlock(gt-lock); + Granting a foreign domain access to frames diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 018b5b6..6b59627 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -151,10 +151,13 @@ struct active_grant_entry { in the page. */ unsigned length:16; /* For sub-page grants, the length of the grant.*/ +spinlock_tlock; /* lock to protect access of this entry. +see docs/misc/grant-tables.txt for +locking protocol */ }; #define ACGNT_PER_PAGE (PAGE_SIZE / sizeof(struct active_grant_entry)) -#define active_entry(t, e) \ +#define _active_entry(t, e) \ ((t)-active[(e)/ACGNT_PER_PAGE][(e)%ACGNT_PER_PAGE]) static inline void gnttab_flush_tlb(const struct domain *d) @@ -182,6 +185,29 @@ nr_active_grant_frames(struct grant_table *gt) return num_act_frames_from_sha_frames(nr_grant_frames(gt)); } +static inline struct active_grant_entry * +active_entry_acquire(struct grant_table *t, grant_ref_t e) +{ +struct active_grant_entry *act; + +#ifndef NDEBUG +/* not perfect, but better than nothing for a debug build + * sanity check + */ +BUG_ON(!rw_is_locked(t-lock)); +#endif + +act = _active_entry(t, e); +spin_lock(act-lock); + +return act; +} + +static inline void active_entry_release(struct active_grant_entry *act) +{ +spin_unlock(act-lock); +} + /* Check if the page has been paged out, or needs unsharing. If rc == GNTST_okay, *page contains the page struct with a ref
Re: [Xen-devel] [PATCH v4] Fixes for PCI backend for 3.19
On 21.11.14 at 23:17, konrad.w...@oracle.com wrote: Konrad Rzeszutek Wilk (7): xen/pciback: Don't deadlock when unbinding. driver core: Provide an wrapper around the mutex to do lockdep warnings xen/pciback: Include the domain id if removing the device whilst still in use xen/pciback: Print out the domain owning the device. xen/pciback: Remove tons of dereferences PCI: Expose pci_load_saved_state for public consumption. xen/pciback: Restore configuration space when detaching from a guest. So my xen-pciback: drop SR-IOV VFs when PF driver unloads isn't among them, nor is there any alternative. What's the status of that patch (or the problem that prompted its creation)? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] x86: add p2m_mmio_write_dm
At 12:31 + on 01 Dec (1417433464), Jan Beulich wrote: On 01.12.14 at 13:13, t...@xen.org wrote: At 11:17 + on 01 Dec (1417429027), Jan Beulich wrote: On 01.12.14 at 11:30, t...@xen.org wrote: At 09:32 + on 01 Dec (1417422746), Jan Beulich wrote: On 01.12.14 at 09:49, yu.c.zh...@linux.intel.com wrote: To my understanding, pages with p2m_ram_ro are not supposed to be modified by guest. So in __hvm_copy(), when p2m type of a page is p2m_ram_rom, no copy will occur. However, to our usage, we just wanna this page to be write protected, so that our device model can be triggered to do some emulation. The content written to this page is supposed not to be dropped. This way, if sequentially a read operation is performed by guest to this page, the guest will still see its anticipated value. __hvm_copy() is only a helper function, and doesn't write to mmio_dm space either; instead its (indirect) callers would invoke hvmemul_do_mmio() upon seeing HVMCOPY_bad_gfn_to_mfn returns. The question hence is about the apparent inconsistency resulting from writes to ram_ro being dropped here but getting passed to the DM by hvm_hap_nested_page_fault(). Tim - is that really intentional? No - and AFAICT it shouldn't be happening. It _is_ how it was implemented originally, because it involved fewer moving parts and didn't need to be efficient (and after all, writes to entirely missing addresses go to the device model too). But the code was later updated to log and discard writes to read-only memory (see 4d8aa29 from Trolle Selander). Early version of p2m_ram_ro were documented in the internal headers as sending the writes to the DM, but the public interface (HVMMEM_ram_ro) has always said that writes are discarded. Hmm, so which way do you recommend resolving the inconsistency then - match what the public interface says or what the apparent original intention for the internal type was? Presumably we need to follow the public interface mandated model, and hence require the new type to be introduced. Sorry, I was unclear -- there isn't an inconsistency; both internal and public headers currently say that writes are discarded and AFAICT that is what the code does. Not for hvm_hap_nested_page_fault() afaict - the forwarding to DM there contradicts the writes are discarded model that other code paths follow. It calls handle_mmio() to emulate the instruction for it, but handle_mmio() ought not to send anything to the DM. hvmemul_write() will call hvm_copy(), which will drop the write and report success. The shadow code has its own emulator framework for emulting PT writes, which does much the same thing. Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5] call xc_domain_irq_permission before xc_domain_irq_permission
On Tue, 2 Dec 2014, Jan Beulich wrote: On 01.12.14 at 20:22, stefano.stabell...@eu.citrix.com wrote: xc_physdev_unmap_pirq might revoke the permission to map the irq from the domain causing the following xc_domain_irq_permission call to fail and return error (domain_pirq_to_irq returns 0). Apart from the patch title being rather confusing, isn't the root problem then xc_physdev_unmap_pirq() removes permissions? At least after the strict splitting of permission granting and mapping for MMIO (commit 0561e1f0) it would seem that if the underlying hypercall here behaves similarly to the original behavior there, it ought to be fixed in an analogous way. The big difference is that 0561e1f0 makes XEN_DOMCTL_memory_mapping stricker, while you are suggesting to make xc_physdev_unmap_pirq laxer. Are you sure that is a good idea? What if a third party caller of xc_physdev_unmap_pirq relies on that behaviour? Call xc_domain_irq_permission first to prevent this from happening (xc_physdev_unmap_pirq calls irq_deny_access, that doesn't fail if the permission is already removed). This is a simple bug fix and I think should go in 4.5. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index 316643c..904d255 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -1288,14 +1290,14 @@ skip1: goto out; } if ((fscanf(f, %u, irq) == 1) irq) { -rc = xc_physdev_unmap_pirq(ctx-xch, domid, irq); -if (rc 0) { -LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_physdev_unmap_pirq irq=%d, irq); -} rc = xc_domain_irq_permission(ctx-xch, domid, irq, 0); if (rc 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_domain_irq_permission irq=%d, irq); } +rc = xc_physdev_unmap_pirq(ctx-xch, domid, irq); +if (rc 0) { +LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_physdev_unmap_pirq irq=%d, irq); +} } fclose(f); } ___ 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 0/2] gnttab: Improve scaleability
On 02/12/14 10:06, Christoph Egger wrote: This patch series changes the grant table locking to a more fain grained locking protocol. The result is a performance boost measured with blkfront/blkback. Document the locking protocol. [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant [PATCH 2/2] gnttab: refactor locking for scalability XenServer has been using the original version of this patch (forward ported a little), and it makes significant improvements to our scalability. I highly recommend this for inclusion, although for 4.6 at this point. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Poor network performance between DomU with multiqueue support
On Tue, Dec 02, 2014 at 04:30:49PM +0800, zhangleiqiang wrote: Hi, all I am testing the performance of xen netfront-netback driver that with multi-queues support. The throughput from domU to remote dom0 is 9.2Gb/s, but the throughput from domU to remote domU is only 3.6Gb/s, I think the bottleneck is the throughput from dom0 to local domU. However, we have done some testing and found the throughput from dom0 to local domU is 5.8Gb/s. And if we send packets from one DomU to other 3 DomUs on different host simultaneously, the sum of throughout can reach 9Gbps. It seems like the bottleneck is the receiver? After some analysis, I found that even the max_queue of netfront/back is set to 4, there are some strange results as follows: 1. In domU, only one rx queue deal with softirq Try to bind irq to different vcpus? 2. In dom0, only two netback queues process are scheduled, other two process aren't scheduled. How many Dom0 vcpu do you have? If it only has two then there will only be two processes running at a time. Are there any issues in my test? In theory, can we achieve 9~10Gb/s between DomUs on different hosts using netfront/netback? The testing environment details are as follows: 1. Hardware a. CPU: Intel(R) Xeon(R) CPU E5645 @ 2.40GHz, 2 CPU 6 Cores with Hyper Thread enabled b. NIC: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ Network Connection (rev 01) 2. Sofware: a. HostOS: SLES 12 (Kernel 3.16-7,git commit d0335e4feea0d3f7a8af3116c5dc166239da7521 ) And this is a SuSE kernel? b. NIC Driver: IXGBE 3.21.2 c. OVS: 2.1.3 d. MTU: 1600 e. Dom0:6U6G f. queue number: 4 g. xen 4.4 h. DomU: 4U4G 3. Networking Environment: a. All network flows are transmit/receive through OVS b. Sender server and receiver server are connected directly between 10GE NIC 4. Testing Tools: a. Sender: netperf b. Receiver: netserver -- zhangleiqiang (Trump) Best Regards ___ 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] PVHVM drivers in upstream linux kernel
On 12/02/2014 11:54 AM, David Vrabel wrote: On 02/12/14 09:39, Juergen Gross wrote: Hi, looking into the upstream linux sources I realized that the PVHVM drivers of XEN are only available with the pvops kernel. Is this on purpose? Shouldn't the frontend drivers, xen/platform-pci.c etc. be configurable without having to enable CONFIG_PARAVIRT? I suppose that would be possible but I don't think it's a useful configuration because you would lose PV spinlocks for example. Having no pv spinlocks on a single vcpu HVM guest isn't so bad. Having no pv-drivers for disks and networking is worse. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] x86: add p2m_mmio_write_dm
On 02.12.14 at 11:37, t...@xen.org wrote: At 12:31 + on 01 Dec (1417433464), Jan Beulich wrote: On 01.12.14 at 13:13, t...@xen.org wrote: At 11:17 + on 01 Dec (1417429027), Jan Beulich wrote: On 01.12.14 at 11:30, t...@xen.org wrote: At 09:32 + on 01 Dec (1417422746), Jan Beulich wrote: On 01.12.14 at 09:49, yu.c.zh...@linux.intel.com wrote: To my understanding, pages with p2m_ram_ro are not supposed to be modified by guest. So in __hvm_copy(), when p2m type of a page is p2m_ram_rom, no copy will occur. However, to our usage, we just wanna this page to be write protected, so that our device model can be triggered to do some emulation. The content written to this page is supposed not to be dropped. This way, if sequentially a read operation is performed by guest to this page, the guest will still see its anticipated value. __hvm_copy() is only a helper function, and doesn't write to mmio_dm space either; instead its (indirect) callers would invoke hvmemul_do_mmio() upon seeing HVMCOPY_bad_gfn_to_mfn returns. The question hence is about the apparent inconsistency resulting from writes to ram_ro being dropped here but getting passed to the DM by hvm_hap_nested_page_fault(). Tim - is that really intentional? No - and AFAICT it shouldn't be happening. It _is_ how it was implemented originally, because it involved fewer moving parts and didn't need to be efficient (and after all, writes to entirely missing addresses go to the device model too). But the code was later updated to log and discard writes to read-only memory (see 4d8aa29 from Trolle Selander). Early version of p2m_ram_ro were documented in the internal headers as sending the writes to the DM, but the public interface (HVMMEM_ram_ro) has always said that writes are discarded. Hmm, so which way do you recommend resolving the inconsistency then - match what the public interface says or what the apparent original intention for the internal type was? Presumably we need to follow the public interface mandated model, and hence require the new type to be introduced. Sorry, I was unclear -- there isn't an inconsistency; both internal and public headers currently say that writes are discarded and AFAICT that is what the code does. Not for hvm_hap_nested_page_fault() afaict - the forwarding to DM there contradicts the writes are discarded model that other code paths follow. It calls handle_mmio() to emulate the instruction for it, but handle_mmio() ought not to send anything to the DM. hvmemul_write() will call hvm_copy(), which will drop the write and report success. Oh, of course - it's really just the wording of the comment there that mislead me to assume the operation gets passed on for actual handling. Thanks for clarifying! Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PVHVM drivers in upstream linux kernel
On Tue, 2014-12-02 at 10:54 +, David Vrabel wrote: On 02/12/14 09:39, Juergen Gross wrote: Hi, looking into the upstream linux sources I realized that the PVHVM drivers of XEN are only available with the pvops kernel. Is this on purpose? Shouldn't the frontend drivers, xen/platform-pci.c etc. be configurable without having to enable CONFIG_PARAVIRT? I suppose that would be possible but I don't think it's a useful configuration because you would lose PV spinlocks for example. IIRC the reason this hasn't been implemented until now is that refactoring would be required to the various bits of driver code which assumes PAE + PARAVIRT when they aren't strictly needed, e.g. grant table code. Whether its worth the churn at this stage is debatable, but I think the (in)ability to use PV spinlocks is a red-herring. Adding PV drivers to an HVM guest is a useful thing to do, even without PV spinlocks. PV IO gets you far more incremental benefit than the locks do, adding PV IO paths is the number 1 thing which should be done to any guest. One actual usecase is installing from a distro installer which isn't PAE, let alone PARAVIRT enabled[0], to get far enough that you can install a more capable PVHVM kernel with more bells and whistles. If there were distros around who refused wholesale to enable PARAVIRT even in a non-default kernel then it would be more likely that they could be convinced to enable a set of PV IO drivers, since they have 0 impact on a non-PARAVIRT system, and still give significant benefit to Xen users. I don't know of any of the major distros are refusing PARAVIRT in this way though. Ian. [0] The default i386 Debian installer falls into this camp, but you can use the special PV Xen variant to install as PVHVM too so it's not so critical. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2] gnttab: Improve scaleability
On 2014/12/02 12:00, Andrew Cooper wrote: On 02/12/14 10:06, Christoph Egger wrote: This patch series changes the grant table locking to a more fain grained locking protocol. The result is a performance boost measured with blkfront/blkback. Document the locking protocol. [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant [PATCH 2/2] gnttab: refactor locking for scalability XenServer has been using the original version of this patch Maybe the one sent by Matt Wilson some time ago? (forward ported a little), This version is against staging tree. and it makes significant improvements to our scalability. Thanks for confirmation. I highly recommend this for inclusion, although for 4.6 at this point. I am fine with this. I would be happy to see a backport to xen 4.5 and 4.4 then. Christoph ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5] call xc_domain_irq_permission before xc_domain_irq_permission
On 02.12.14 at 11:55, stefano.stabell...@eu.citrix.com wrote: On Tue, 2 Dec 2014, Jan Beulich wrote: On 01.12.14 at 20:22, stefano.stabell...@eu.citrix.com wrote: xc_physdev_unmap_pirq might revoke the permission to map the irq from the domain causing the following xc_domain_irq_permission call to fail and return error (domain_pirq_to_irq returns 0). Apart from the patch title being rather confusing, isn't the root problem then xc_physdev_unmap_pirq() removes permissions? At least after the strict splitting of permission granting and mapping for MMIO (commit 0561e1f0) it would seem that if the underlying hypercall here behaves similarly to the original behavior there, it ought to be fixed in an analogous way. The big difference is that 0561e1f0 makes XEN_DOMCTL_memory_mapping stricker, while you are suggesting to make xc_physdev_unmap_pirq laxer. Are you sure that is a good idea? What if a third party caller of xc_physdev_unmap_pirq relies on that behaviour? Yes, I think this would be correct. We expect tool stacks to be in sync with the hypervisor. That said, I'm not sure it would actually work, since if we did that change, the implicit access granting in the map operation would also need to be dropped. Yet the map operation is a prerequisite for XEN_DOMCTL_irq_permission to work at all right now, as it needs to look up the pIRQ-IRQ translation (which looks wrong to me now anyway, as it makes little sense for the same pIRQ to be associated with the same IRQ in two different domains, i.e. I think the pirq_{permit,deny}_access() calls there really need to be irq_{permit,deny}_access(), handed the IRQ translated from the passed in pIRQ). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant table state
On 02/12/14 10:06, Christoph Egger wrote: Rename lock to maptrack_lock and use it to protect maptrack state. The new rwlock is used to prevent readers from accessing inconsistent grant table state such as current version, partially initialized active table pages, etc. I would suggest phrasing this slightly differently, as it isn't a simple rename. What you are doing is taking the existing grant table lock, splitting it in two to separate the grant locking from the maptrack locking, and changing the resulting grant lock to be a rwlock. With this noted, it would certainly be easier to review if this patch was split in two; one patch to split the spinlocks and one patch to change the grant lock from a spinlock to a rwlock. diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 8fba923..018b5b6 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -2501,8 +2510,19 @@ gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) uop, return i; if ( unlikely(__copy_from_guest(op, uop, 1)) ) return -EFAULT; -op.status = __gnttab_swap_grant_ref(op.ref_a, op.ref_b); -if ( unlikely(__copy_field_to_guest(uop, op, status)) ) +if ( unlikely(op.ref_a == op.ref_b) ) +{ +/* noop, so avoid acquiring the same active entry + * twice in __gnttab_swap_grant_ref(), which would + * case a deadlock. + */ +op.status = GNTST_okay; +} +else +{ +op.status = __gnttab_swap_grant_ref(op.ref_a, op.ref_b); +} +if ( unlikely(__copy_to_guest_offset(uop, i, op, 1)) ) I believe this comment only applies to the changes in active locking introduced in the subsequent patch? Either way, I think the a == b check should be in __gnttab_swap_grant_ref() make any caller safe, not just the this one. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PVHVM drivers in upstream linux kernel
On 12/02/2014 12:05 PM, Ian Campbell wrote: On Tue, 2014-12-02 at 10:54 +, David Vrabel wrote: On 02/12/14 09:39, Juergen Gross wrote: Hi, looking into the upstream linux sources I realized that the PVHVM drivers of XEN are only available with the pvops kernel. Is this on purpose? Shouldn't the frontend drivers, xen/platform-pci.c etc. be configurable without having to enable CONFIG_PARAVIRT? I suppose that would be possible but I don't think it's a useful configuration because you would lose PV spinlocks for example. IIRC the reason this hasn't been implemented until now is that refactoring would be required to the various bits of driver code which assumes PAE + PARAVIRT when they aren't strictly needed, e.g. grant table code. Whether its worth the churn at this stage is debatable, but I think the (in)ability to use PV spinlocks is a red-herring. Adding PV drivers to an HVM guest is a useful thing to do, even without PV spinlocks. PV IO gets you far more incremental benefit than the locks do, adding PV IO paths is the number 1 thing which should be done to any guest. I take this as an ack to change this. :-) One actual usecase is installing from a distro installer which isn't PAE, let alone PARAVIRT enabled[0], to get far enough that you can install a more capable PVHVM kernel with more bells and whistles. If there were distros around who refused wholesale to enable PARAVIRT even in a non-default kernel then it would be more likely that they could be convinced to enable a set of PV IO drivers, since they have 0 impact on a non-PARAVIRT system, and still give significant benefit to Xen users. I don't know of any of the major distros are refusing PARAVIRT in this way though. I think we have customers wanting to run a default kernel as domU. So it isn't always the distro refusing paravirt, it might be the user... Okay, how do the current config settings regarding Xen look like? We have: - XEN depending on PARAVIRT - XEN_DOM0 depending on XEN and others - XEN_BACKEND depending on XEN_DOM0 - various backend drivers depending on XEN_BACKEND - XEN_PVHVM depending on XEN - various frontend drivers depending on XEN (even if some are not depending on XEN according to Kconfig, they do as the complete drivers/xen directory is made only if CONFIG_XEN is defined) To sort things out I'd suggest to: - make XEN independent from PARAVIRT - let XEN_DOM0 select XEN_BACKEND, PARAVIRT, XEN - let XEN_BACKEND select PARAVIRT, XEN (I'd like to be able to build a driver domain without XEN_DOM0) - introduce XEN_FRONTEND, let it select XEN - let frontend drivers and drivers needed by those depend on XEN_FRONTEND - let XEN_PVHVM select XEN_FRONTEND - don't skip drivers/xen on make, as XEN might be selected via a config item in that directory Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PVHVM drivers in upstream linux kernel
On Tue, 2014-12-02 at 12:33 +0100, Juergen Gross wrote: On 12/02/2014 12:05 PM, Ian Campbell wrote: On Tue, 2014-12-02 at 10:54 +, David Vrabel wrote: On 02/12/14 09:39, Juergen Gross wrote: Hi, looking into the upstream linux sources I realized that the PVHVM drivers of XEN are only available with the pvops kernel. Is this on purpose? Shouldn't the frontend drivers, xen/platform-pci.c etc. be configurable without having to enable CONFIG_PARAVIRT? I suppose that would be possible but I don't think it's a useful configuration because you would lose PV spinlocks for example. IIRC the reason this hasn't been implemented until now is that refactoring would be required to the various bits of driver code which assumes PAE + PARAVIRT when they aren't strictly needed, e.g. grant table code. Whether its worth the churn at this stage is debatable, but I think the (in)ability to use PV spinlocks is a red-herring. Adding PV drivers to an HVM guest is a useful thing to do, even without PV spinlocks. PV IO gets you far more incremental benefit than the locks do, adding PV IO paths is the number 1 thing which should be done to any guest. I take this as an ack to change this. :-) It's a best IMHO being able to do this is a useful thing. I can't ack the actual final patch, a) I'm not a relevant maintainer and b) I've not seen the patch. Okay, how do the current config settings regarding Xen look like? ...I'll leave the mechanics down to you and the maintainers to thrash out. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PVHVM drivers in upstream linux kernel
On 12/02/2014 12:36 PM, Ian Campbell wrote: On Tue, 2014-12-02 at 12:33 +0100, Juergen Gross wrote: On 12/02/2014 12:05 PM, Ian Campbell wrote: On Tue, 2014-12-02 at 10:54 +, David Vrabel wrote: On 02/12/14 09:39, Juergen Gross wrote: Hi, looking into the upstream linux sources I realized that the PVHVM drivers of XEN are only available with the pvops kernel. Is this on purpose? Shouldn't the frontend drivers, xen/platform-pci.c etc. be configurable without having to enable CONFIG_PARAVIRT? I suppose that would be possible but I don't think it's a useful configuration because you would lose PV spinlocks for example. IIRC the reason this hasn't been implemented until now is that refactoring would be required to the various bits of driver code which assumes PAE + PARAVIRT when they aren't strictly needed, e.g. grant table code. Whether its worth the churn at this stage is debatable, but I think the (in)ability to use PV spinlocks is a red-herring. Adding PV drivers to an HVM guest is a useful thing to do, even without PV spinlocks. PV IO gets you far more incremental benefit than the locks do, adding PV IO paths is the number 1 thing which should be done to any guest. I take this as an ack to change this. :-) It's a best IMHO being able to do this is a useful thing. I can't ack the actual final patch, a) I'm not a relevant maintainer and b) I've not seen the patch. It was not meant to be taken as an ack for a not yet written patch. :-) Okay, how do the current config settings regarding Xen look like? ...I'll leave the mechanics down to you and the maintainers to thrash out. Yeah, stay tuned. :-) Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] x86: add p2m_mmio_write_dm
At 15:38 +0800 on 02 Dec (1417531126), Yu, Zhang wrote: On 12/1/2014 8:13 PM, Tim Deegan wrote: At 11:17 + on 01 Dec (1417429027), Jan Beulich wrote: On 01.12.14 at 11:30, t...@xen.org wrote: During this bit of archaeology I realised that either this new type should _not_ be made part of P2M_RO_TYPES, or, better, we need a new class of P2M types (P2M_DISCARD_WRITE_TYPES, say) that should be used for these paths in emulate_gva_to_mfn() and __hvm_copy(), containing just p2m_ram_ro and p2m_grant_map_ro. And I suppose in that latter case the new type could be made part of P2M_RO_TYPES()? Yes indeed, as P2M_RO_TYPES is defined as must have the _PAGE_RW bit clear in their PTEs. Thanks Tim. Following are my understanding of the P2M_RO_TYPES and your comments. Not sure if I get it right. Please correct me if anything wrong: 1 The P2M_RO_TYPES now bears 2 meanings: one is w bit is clear in the pte; and another being to discard the write operations; 2 We better define another class to bear the second meaning. Yes, that's what I meant. Answering your other questions in reverse order: 2 p2m_grant_map_ro is also supposed to be discarded? Will handling of this type of pages goes into __hvm_copy()/__hvm_clear(), or should? I think so, yes. At the moment we inject #GP when the guest writes to a read-only grant, which is OK: the guest really ought to know better. But I think we'll probably end up with neater code if we handle read-only grants the same way as p2m_ram_ro. Anyone else have an opinion on the right thing to do here? Also some questions for the new p2m class, say P2M_DISCARD_WRITE_TYPES, and the new predicates, say p2m_is_discard_write: 1 You mentioned emulate_gva_to_mfn() and __hvm_copy() should discard the write ops, yet I also noticed many other places using the p2m_is_readonly, or only the p2mt == p2m_ram_ro judgement(in __hvm_copy/__hvm_clear). Among all these other places, is there any ones also supposed to use the p2m_is_discard_write? I've just had a look through them all, and I can see exactly four places that should be using the new p2m_is_discard_write() test: - emulate_gva_to_mfn() (Though in fact it's a no-op as shadow-mode guests never have p2m_ram_shared or p2m_ram_logdirty mappings.) - __hvm_copy() - __hvm_clear() and - hvm_hap_nested_page_fault() (where you should also remove the explicit handling of p2m_grant_map_ro below.) Looking through that turned up a few other oddities, which I'm listing here to remind myself to look at them later (i.e. you don't need to worry about them for this patch): - nsvm_get_nvmcb_page() and nestedhap_walk_L0_p2m() need to handle p2m_ram_logdirty or they might spuriously fail duiring live migration. - __hvm_copy() and __hvm_clear are probably over-strict in their failure to handle grant types. - P2M_UNMAP_TYPES in vmce.c is a mess. It's not the right place to define this, since it definitely won't be seen my anyone adding a new type, and it already has an 'XXX' comment that says it doesn't cover a lot of cases. :( I'll have a look at those another time. Cheers, Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl_set_memory_target: retain the same maxmem offset on top of the current target
On Mon, 1 Dec 2014, Don Slutz wrote: On 11/25/14 12:54, Stefano Stabellini wrote: In libxl_set_memory_target when setting the new maxmem, retain the same offset on top of the current target. The offset includes memory allocated by QEMU for rom files. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index de23fec..8381c3e 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -4767,10 +4767,12 @@ retry_transaction: %s/memory/videoram, dompath)); videoram = videoram_s ? atoi(videoram_s) : 0; -if (enforce) { -memorykb = new_target_memkb; -rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb + -LIBXL_MAXMEM_CONSTANT); +libxl_dominfo_init(ptr); +xcinfo2xlinfo(ctx, info, ptr); This fills ptr with uninitialized data. You need to call xc_domain_getinfolist() before this. However calling xc_domain_getinfolist() here and retry of the xenstore transaction, you will adjust this more then one time. So I think that xc_domain_getinfolist() needs to be called before the label retry_transaction. However rc is a mess in this routine. rc = 1 is the normal return (since rc = xc_domain_getinfolist() is the last setting of rc). So all the rest of rc = needs to be adjusted someway. Instead of calling xc_domain_getinfolist, I think it's best to call libxl_domain_info. You are right that I need to call it before retry_transaction. Thanks for the review! -Don Slutz + +if (enforce new_target_memkb 0) { +memorykb = ptr.max_memkb - current_target_memkb + new_target_memkb; +rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb); if (rc != 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_domain_setmaxmem domid=%d memkb=%d failed @@ -4800,8 +4802,6 @@ retry_transaction: goto out; } -libxl_dominfo_init(ptr); -xcinfo2xlinfo(ctx, info, ptr); uuid = libxl__uuid2string(gc, ptr.uuid); libxl__xs_write(gc, t, libxl__sprintf(gc, /vm/%s/memory, uuid), %PRIu32, new_target_memkb / 1024); ___ 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 v4] x86: add p2m_mmio_write_dm
On 02.12.14 at 12:40, t...@xen.org wrote: At 15:38 +0800 on 02 Dec (1417531126), Yu, Zhang wrote: 2 p2m_grant_map_ro is also supposed to be discarded? Will handling of this type of pages goes into __hvm_copy()/__hvm_clear(), or should? I think so, yes. At the moment we inject #GP when the guest writes to a read-only grant, which is OK: the guest really ought to know better. But I think we'll probably end up with neater code if we handle read-only grants the same way as p2m_ram_ro. Anyone else have an opinion on the right thing to do here? I actually came to the same conclusion. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Poor network performance between DomU with multiqueue support
-Original Message- From: xen-devel-boun...@lists.xen.org [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Wei Liu Sent: Tuesday, December 02, 2014 7:02 PM To: zhangleiqiang Cc: wei.l...@citrix.com; xen-devel@lists.xen.org Subject: Re: [Xen-devel] Poor network performance between DomU with multiqueue support On Tue, Dec 02, 2014 at 04:30:49PM +0800, zhangleiqiang wrote: Hi, all I am testing the performance of xen netfront-netback driver that with multi-queues support. The throughput from domU to remote dom0 is 9.2Gb/s, but the throughput from domU to remote domU is only 3.6Gb/s, I think the bottleneck is the throughput from dom0 to local domU. However, we have done some testing and found the throughput from dom0 to local domU is 5.8Gb/s. And if we send packets from one DomU to other 3 DomUs on different host simultaneously, the sum of throughout can reach 9Gbps. It seems like the bottleneck is the receiver? After some analysis, I found that even the max_queue of netfront/back is set to 4, there are some strange results as follows: 1. In domU, only one rx queue deal with softirq Try to bind irq to different vcpus? Do you mean we try to bind irq to different vcpus in DomU? I will try it now. 2. In dom0, only two netback queues process are scheduled, other two process aren't scheduled. How many Dom0 vcpu do you have? If it only has two then there will only be two processes running at a time. Dom0 has 6 vcpus, and 6G memory. There are only one DomU running in Dom0 and so four netback processes are running in Dom0 (because the max_queue param of netback kernel module is set to 4). The phenomenon is that only 2 of these four netback process were running with about 70% cpu usage, and another two use little CPU. Is there a hash algorithm to determine which netback process to handle the input packet? Are there any issues in my test? In theory, can we achieve 9~10Gb/s between DomUs on different hosts using netfront/netback? The testing environment details are as follows: 1. Hardware a. CPU: Intel(R) Xeon(R) CPU E5645 @ 2.40GHz, 2 CPU 6 Cores with Hyper Thread enabled b. NIC: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ Network Connection (rev 01) 2. Sofware: a. HostOS: SLES 12 (Kernel 3.16-7,git commit d0335e4feea0d3f7a8af3116c5dc166239da7521 ) And this is a SuSE kernel? No, I just compile Dom0 and DomU kernel using 3.16-7 tag from kernel.org. b. NIC Driver: IXGBE 3.21.2 c. OVS: 2.1.3 d. MTU: 1600 e. Dom0:6U6G f. queue number: 4 g. xen 4.4 h. DomU: 4U4G 3. Networking Environment: a. All network flows are transmit/receive through OVS b. Sender server and receiver server are connected directly between 10GE NIC 4. Testing Tools: a. Sender: netperf b. Receiver: netserver -- zhangleiqiang (Trump) Best Regards ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Poor network performance between DomU with multiqueue support
-Original Message- From: xen-devel-boun...@lists.xen.org [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of David Vrabel Sent: Tuesday, December 02, 2014 6:57 PM To: zhangleiqiang; xen-devel@lists.xen.org Subject: Re: [Xen-devel] Poor network performance between DomU with multiqueue support On 02/12/14 08:30, zhangleiqiang wrote: Hi, all I am testing the performance of xen netfront-netback driver that with multi-queues support. The throughput from domU to remote dom0 is 9.2Gb/s, but the throughput from domU to remote domU is only 3.6Gb/s, I think the bottleneck is the throughput from dom0 to local domU. However, we have done some testing and found the throughput from dom0 to local domU is 5.8Gb/s. And if we send packets from one DomU to other 3 DomUs on different host simultaneously, the sum of throughout can reach 9Gbps. It seems like the bottleneck is the receiver? After some analysis, I found that even the max_queue of netfront/back is set to 4, there are some strange results as follows: 1. In domU, only one rx queue deal with softirq 2. In dom0, only two netback queues process are scheduled, other two process aren't scheduled. Multiqueue only has benefits if you have multiple flows since the source/destination addresses are hashed to a queue number. This probably explains why only some of the queues are being used in your test. The hash method you mentioned is used for selection of netback process or netfront rx queue? Indeed, there are 4 netback processes running in Dom0, because there are only one DomU running in Dom0 and so four netback processes are running in Dom0 (the max_queue param of netback kernel module is set to 4). The phenomenon is that only 2 of these four netback process were running with about 70% cpu usage, and another two use little CPU. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 for-4.6] libxl_set_memory_target: retain the same maxmem offset on top of the current target
In libxl_set_memory_target when setting the new maxmem, retain the same offset on top of the current target. The offset includes memory allocated by QEMU for rom files. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- Changes in v2: - call libxl_domain_info instead of libxl_dominfo_init; - call libxl_domain_info before retry_transaction. diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index de23fec..569a32a 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -4694,6 +4694,9 @@ int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, char *uuid; xs_transaction_t t; +if (libxl_domain_info(ctx, ptr, domid) 0) +goto out_no_transaction; + retry_transaction: t = xs_transaction_start(ctx-xsh); @@ -4767,10 +4770,9 @@ retry_transaction: %s/memory/videoram, dompath)); videoram = videoram_s ? atoi(videoram_s) : 0; -if (enforce) { -memorykb = new_target_memkb; -rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb + -LIBXL_MAXMEM_CONSTANT); +if (enforce new_target_memkb 0) { +memorykb = ptr.max_memkb - current_target_memkb + new_target_memkb; +rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb); if (rc != 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_domain_setmaxmem domid=%d memkb=%d failed @@ -4800,8 +4802,6 @@ retry_transaction: goto out; } -libxl_dominfo_init(ptr); -xcinfo2xlinfo(ctx, info, ptr); uuid = libxl__uuid2string(gc, ptr.uuid); libxl__xs_write(gc, t, libxl__sprintf(gc, /vm/%s/memory, uuid), %PRIu32, new_target_memkb / 1024); ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Poor network performance between DomU with multiqueue support
On Tue, Dec 02, 2014 at 11:50:59AM +, Zhangleiqiang (Trump) wrote: -Original Message- From: xen-devel-boun...@lists.xen.org [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Wei Liu Sent: Tuesday, December 02, 2014 7:02 PM To: zhangleiqiang Cc: wei.l...@citrix.com; xen-devel@lists.xen.org Subject: Re: [Xen-devel] Poor network performance between DomU with multiqueue support On Tue, Dec 02, 2014 at 04:30:49PM +0800, zhangleiqiang wrote: Hi, all I am testing the performance of xen netfront-netback driver that with multi-queues support. The throughput from domU to remote dom0 is 9.2Gb/s, but the throughput from domU to remote domU is only 3.6Gb/s, I think the bottleneck is the throughput from dom0 to local domU. However, we have done some testing and found the throughput from dom0 to local domU is 5.8Gb/s. And if we send packets from one DomU to other 3 DomUs on different host simultaneously, the sum of throughout can reach 9Gbps. It seems like the bottleneck is the receiver? After some analysis, I found that even the max_queue of netfront/back is set to 4, there are some strange results as follows: 1. In domU, only one rx queue deal with softirq Try to bind irq to different vcpus? Do you mean we try to bind irq to different vcpus in DomU? I will try it now. Yes. Given the fact that you have two backend threads running while only one DomU vcpu is busy, it smells like misconfiguration in DomU. If this phenomenon persists after correctly binding irqs, you might want to check traffic is steering correctly to different queues. 2. In dom0, only two netback queues process are scheduled, other two process aren't scheduled. How many Dom0 vcpu do you have? If it only has two then there will only be two processes running at a time. Dom0 has 6 vcpus, and 6G memory. There are only one DomU running in Dom0 and so four netback processes are running in Dom0 (because the max_queue param of netback kernel module is set to 4). The phenomenon is that only 2 of these four netback process were running with about 70% cpu usage, and another two use little CPU. Is there a hash algorithm to determine which netback process to handle the input packet? I think that's whatever default algorithm Linux kernel is using. We don't currently support other algorithms. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 31983: regressions - FAIL
flight 31983 qemu-mainline real [real] http://www.chiark.greenend.org.uk/~xensrcts/logs/31983/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemut-debianhvm-amd64 5 xen-boot fail REGR. vs. 31947 Regressions which are regarded as allowable (not blocking): test-amd64-i386-pair17 guest-migrate/src_host/dst_host fail like 31947 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 9 guest-start fail never pass test-armhf-armhf-xl 10 migrate-support-checkfail never pass test-amd64-i386-libvirt 9 guest-start fail never pass test-amd64-amd64-xl-win7-amd64 14 guest-stop fail never pass test-amd64-i386-xl-win7-amd64 14 guest-stop fail never pass test-amd64-amd64-xl-winxpsp3 14 guest-stop fail never pass test-amd64-i386-xl-qemut-winxpsp3-vcpus1 14 guest-stop fail never pass test-amd64-amd64-xl-qemut-winxpsp3 14 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 14 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 14 guest-stop fail never pass test-amd64-i386-xl-qemut-winxpsp3 14 guest-stopfail never pass test-amd64-i386-xl-qemut-win7-amd64 14 guest-stop fail never pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 14 guest-stop fail never pass test-amd64-amd64-libvirt 9 guest-start fail never pass test-amd64-amd64-xl-pcipt-intel 9 guest-start fail never pass test-amd64-amd64-xl-qemuu-winxpsp3 14 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 14 guest-stop fail never pass test-amd64-i386-xl-winxpsp3-vcpus1 14 guest-stop fail never pass test-amd64-i386-xl-qemuu-winxpsp3 14 guest-stopfail never pass test-amd64-i386-xl-winxpsp3 14 guest-stop fail never pass version targeted for testing: qemuub19ca188022d720e6cdf87c43c27cb68bac32f6a baseline version: qemuudb12451decf7dfe0f083564183e135f2095228b9 People who touched revisions under test: Gonglei arei.gong...@huawei.com Peter Maydell peter.mayd...@linaro.org jobs: 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 pass test-amd64-i386-rhel6hvm-amd pass test-amd64-i386-qemut-rhel6hvm-amd pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemut-debianhvm-amd64pass test-amd64-i386-xl-qemut-debianhvm-amd64 fail 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-qemut-win7-amd64 fail test-amd64-i386-xl-qemut-win7-amd64 fail test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-win7-amd64 fail test-amd64-i386-xl-win7-amd64fail test-amd64-i386-xl-credit2 pass test-amd64-i386-freebsd10-i386 pass test-amd64-amd64-xl-pcipt-intel fail test-amd64-i386-rhel6hvm-intel pass test-amd64-i386-qemut-rhel6hvm-intel pass test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-libvirt fail
Re: [Xen-devel] [Qemu-devel] [PATCH] increase maxmem before calling xc_domain_populate_physmap
On Mon, 1 Dec 2014, Don Slutz wrote: On 12/01/14 10:37, Stefano Stabellini wrote: On Mon, 1 Dec 2014, Don Slutz wrote: On 11/27/14 05:48, Stefano Stabellini wrote: [...] Works fine in both claim modes and with PoD used (maxmem memory). Do not know how to test with tmem. I do not see how it would be worse then current code that does not auto increase. I.E. even without a xen change, I think something like this could be done. OK, good to know. I am OK with increasing maxmem only if it is strictly necessary. My testing shows a free 32 pages that I am not sure where they come from. But the code about is passing my 8 nics of e1000. I think that raising maxmem a bit higher than necessary is not too bad. If we really care about it, we could lower the limit after QEMU's initialization is completed. Ok. I did find the 32 it is VGA_HOLE_SIZE. So here is what I have which includes a lot of extra printf. In QEMU I would prefer not to assume that libxl increased maxmem for the vga hole. I would rather call xc_domain_setmaxmem twice for the vga hole than tie QEMU to a particular maxmem allocation scheme in libxl. Ok. The area we are talking about is 0x000a to 0x000c. It is in libxc (xc_hvm_build_x86), not libxl. I have no issue with a name change to some thing like QEMU_EXTRA_FREE_PAGES. Sorry, I was thinking about the relocated videoram region, the one allocated by xen_add_to_physmap in QEMU. Regarding 0x000a-0x000c, according to this comment: /* * Allocate memory for HVM guest, skipping VGA hole 0xA-0xC. * xc_hvm_build_x86 shouldn't be allocating memory for it. In fact if I remember correctly 0xa-0xc is trapped and emulated. My testing has shows that some of these 32 pages are used outside of QEMU. I am seeing just 23 free pages using a standalone program to display the same info after a CentOS 6.3 guest is done booting. In libxl I would like to avoid increasing mamxem for anything QEMU will allocate later, that includes rom and vga vram. I am not sure how to make that work with older QEMU versions that don't call xc_domain_setmaxmem by themselves yet though. Maybe we could check the specific QEMU version in libxl and decide based on that. Or we could export a feature flag in QEMU. Yes, it would be nice to adjust libxl to not increase maxmem. However since videoram is included in memory (and maxmem), making the change related to vram is a bigger issue. the rom change is much simpler. Currently I do not know of a way to do different things based on the QEMU version and/or features (this includes getting the QEMU version in libxl). I have been going with: 1) change QEMU 1st. 2) Wait for an upstream version of QEMU with this. 3) change xen to optionally use a feature in the latest QEMU. Let's try to take this approach for the videoram too --- a/xen-hvm.c +++ b/xen-hvm.c @@ -67,6 +67,7 @@ static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t *shared_page, int vcpu) #endif #define BUFFER_IO_MAX_DELAY 100 +#define VGA_HOLE_SIZE (0x20) typedef struct XenPhysmap { hwaddr start_addr; @@ -219,6 +220,11 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr) xen_pfn_t *pfn_list; int i; xc_dominfo_t info; +unsigned long max_pages, free_pages, real_free; +long need_pages; +uint64_t tot_pages, pod_cache_pages, pod_entries; + +trace_xen_ram_alloc(ram_addr, size, mr-name); if (runstate_check(RUN_STATE_INMIGRATE)) { /* RAM already populated in Xen */ @@ -232,13 +238,6 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr) return; } -fprintf(stderr, %s: alloc RAM_ADDR_FMT - bytes (%ld Kib) of ram at RAM_ADDR_FMT - mr.name=%s\n, -__func__, size, (long)(size10), ram_addr, mr-name); - -trace_xen_ram_alloc(ram_addr, size); - nr_pfn = size TARGET_PAGE_BITS; pfn_list = g_malloc(sizeof (*pfn_list) * nr_pfn); @@ -246,11 +245,38 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr) pfn_list[i] = (ram_addr TARGET_PAGE_BITS) + i; } -if (xc_domain_getinfo(xen_xc, xen_domid, 1, info) 0) { +if ((xc_domain_getinfo(xen_xc, xen_domid, 1, info) != 1) || I think it's best to call xc_domain_getinfolist. + (info.domid != xen_domid)) { hw_error(xc_domain_getinfo failed); } -if (xc_domain_setmaxmem(xen_xc, xen_domid, info.max_memkb + -(nr_pfn * XC_PAGE_SIZE / 1024)) 0) { +max_pages = info.max_memkb * 1024 / XC_PAGE_SIZE; +free_pages = max_pages - info.nr_pages; +real_free = free_pages; +
Re: [Xen-devel] [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant table state
Hi Christoph, On 02/12/14 10:06, Christoph Egger wrote: diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 522c43d..37c13b1 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4565,7 +4565,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-shared_info); break; case XENMAPSPACE_grant_table: -spin_lock(d-grant_table-lock); +write_lock(d-grant_table-lock); if ( d-grant_table-gt_version == 0 ) d-grant_table-gt_version = 1; @@ -4587,7 +4587,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-grant_table-shared_raw[idx]); } -spin_unlock(d-grant_table-lock); +write_unlock(d-grant_table-lock); break; case XENMAPSPACE_gmfn_range: case XENMAPSPACE_gmfn: You forgot to modify the ARM bits which is using the spinlock. See arch/arm/mm.c Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PVHVM drivers in upstream linux kernel
On 12/02/2014 12:59 PM, David Vrabel wrote: On 02/12/14 11:33, Juergen Gross wrote: I think we have customers wanting to run a default kernel as domU. So it isn't always the distro refusing paravirt, it might be the user... I don't think this is a sensible use case but I'm not adverse to improving the set of Xen config options. To sort things out I'd suggest to: - make XEN independent from PARAVIRT - let XEN_DOM0 select XEN_BACKEND, PARAVIRT, XEN - let XEN_BACKEND select PARAVIRT, XEN (I'd like to be able to build a driver domain without XEN_DOM0) - introduce XEN_FRONTEND, let it select XEN - let frontend drivers and drivers needed by those depend on XEN_FRONTEND - let XEN_PVHVM select XEN_FRONTEND Rather than looking at the current set of configuration options, can you look at what user-visible functionality or use cases need to be covered by a (potentially new) set of configuration options? I'd see: - XEN_PV (selects PARAVIRT, XEN_FRONTEND): be able to run as pv-domain (x86 only) - XEN_PVHVM (selects XEN_FRONTEND): be able to run as hvm-domain with pv-drivers - XEN_BACKEND (selects PARAVIRT if x86): be able to run as driver domain (dom0 or other) - XEN_DOM0 (selects PARAVIRT if x86, XEN_BACKEND): be able to run as dom0 - XEN_FRONTEND: be able to run as domU with pv-drivers Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant table state
On 2014/12/02 13:35, Julien Grall wrote: Hi Christoph, On 02/12/14 10:06, Christoph Egger wrote: diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 522c43d..37c13b1 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4565,7 +4565,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-shared_info); break; case XENMAPSPACE_grant_table: -spin_lock(d-grant_table-lock); +write_lock(d-grant_table-lock); if ( d-grant_table-gt_version == 0 ) d-grant_table-gt_version = 1; @@ -4587,7 +4587,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-grant_table-shared_raw[idx]); } -spin_unlock(d-grant_table-lock); +write_unlock(d-grant_table-lock); break; case XENMAPSPACE_gmfn_range: case XENMAPSPACE_gmfn: You forgot to modify the ARM bits which is using the spinlock. See arch/arm/mm.c I can do the change. But I don't have ARM hardware nor the build infrastructure. I need your help with compiling and testing on/for ARM. Alternatively you can send me a patch I can add to or squash into my patch series. Christoph ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant table state
Hi, On 02/12/14 13:03, Egger, Christoph wrote: On 2014/12/02 13:35, Julien Grall wrote: Hi Christoph, On 02/12/14 10:06, Christoph Egger wrote: diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 522c43d..37c13b1 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4565,7 +4565,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-shared_info); break; case XENMAPSPACE_grant_table: -spin_lock(d-grant_table-lock); +write_lock(d-grant_table-lock); if ( d-grant_table-gt_version == 0 ) d-grant_table-gt_version = 1; @@ -4587,7 +4587,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-grant_table-shared_raw[idx]); } -spin_unlock(d-grant_table-lock); +write_unlock(d-grant_table-lock); break; case XENMAPSPACE_gmfn_range: case XENMAPSPACE_gmfn: You forgot to modify the ARM bits which is using the spinlock. See arch/arm/mm.c I can do the change. But I don't have ARM hardware nor the build infrastructure. I need your help with compiling and testing on/for ARM. Alternatively you can send me a patch I can add to or squash into my patch series. I agree that testing the ARM part may require a bit of setup. But you can easily build testing. only a cross-compiler for ARM is required for this step. See: http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions#Building_Xen_on_ARM Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant table state
On 2014/12/02 14:08, Julien Grall wrote: Hi, On 02/12/14 13:03, Egger, Christoph wrote: On 2014/12/02 13:35, Julien Grall wrote: Hi Christoph, On 02/12/14 10:06, Christoph Egger wrote: diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 522c43d..37c13b1 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4565,7 +4565,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-shared_info); break; case XENMAPSPACE_grant_table: -spin_lock(d-grant_table-lock); +write_lock(d-grant_table-lock); if ( d-grant_table-gt_version == 0 ) d-grant_table-gt_version = 1; @@ -4587,7 +4587,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-grant_table-shared_raw[idx]); } -spin_unlock(d-grant_table-lock); +write_unlock(d-grant_table-lock); break; case XENMAPSPACE_gmfn_range: case XENMAPSPACE_gmfn: You forgot to modify the ARM bits which is using the spinlock. See arch/arm/mm.c I can do the change. But I don't have ARM hardware nor the build infrastructure. I need your help with compiling and testing on/for ARM. Alternatively you can send me a patch I can add to or squash into my patch series. I agree that testing the ARM part may require a bit of setup. But you can easily build testing. only a cross-compiler for ARM is required for this step. See: http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions#Building_Xen_on_ARM Thank you for this pointer. I just compared xenmem_add_to_physmap_one() x86 and arm versions. The real architectural functions are virt_to_mfn(), mfn_to_page() and friends. Everything else in this function is just common code and can be moved into xen/common/ . Christoph ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Fix building libxlu_cfg_y.y with bison 3.0
On Mon, 2014-12-01 at 12:19 +, Wei Liu wrote: On Mon, Dec 01, 2014 at 09:42:13AM +, Ian Campbell wrote: On Sat, 2014-11-29 at 21:23 -0800, Ed Swierk wrote: - Use %lex-param instead of obsolete YYLEX_PARAM to override lex scanner parameter - Change deprecated %name-prefix= to %name-prefix Tested against bison 2.4.1 and 3.0.2. Signed-off-by: Ed Swierk eswi...@skyportsystems.com Copying Ian J who is the bison guy among the toolstack maintainers. FWIW I can confirm that libxlu_cfg_y.y won't build in Debian Jessie (bison 3.0.2) as is. And this patch fixes the problem for me. That would seem like a pretty strong case for 4.5, *except* we ship the generated files so it should be possible to build anywhere without requiring any version of bison at all. If Bison is installed then ./configure BISON=/bin/true or some such might be needed to stop it trying to regenerate. Konrad, any thoughts. Wei. --- tools/libxl/libxlu_cfg_y.y | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxlu_cfg_y.y b/tools/libxl/libxlu_cfg_y.y index aa9f787..5acd438 100644 --- a/tools/libxl/libxlu_cfg_y.y +++ b/tools/libxl/libxlu_cfg_y.y @@ -17,7 +17,7 @@ */ %{ -#define YYLEX_PARAM ctx-scanner +#define ctx_scanner ctx-scanner #include libxlu_cfg_i.h #include libxlu_cfg_l.h %} @@ -31,9 +31,9 @@ %pure-parser %defines %error-verbose -%name-prefix=xlu__cfg_yy +%name-prefix xlu__cfg_yy %parse-param { CfgParseContext *ctx } -%lex-param { void *scanner } +%lex-param { ctx_scanner } %token stringIDENT STRING NUMBER NEWLINE %type stringatom ___ 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/arm: Handle platforms with edge-triggered virtual timer
On Fri, 2014-11-28 at 15:17 +, Julien Grall wrote: Some platforms (such as Xgene and ARMv8 models) use an edge-triggered interrupt for the virtual timer. Even if the timer output signal is masked in the context switch, the GIC will keep track that of any interrupts raised while IRQs are disabled. As soon as IRQs are re-enabled, the virtual interrupt timer will be injected to Xen. If an idle vVCPU was scheduled next then the interrupt handler doesn't expect to the receive the IRQ and will crash: (XEN)[00228388] _spin_lock_irqsave+0x28/0x94 (PC) (XEN)[00228380] _spin_lock_irqsave+0x20/0x94 (LR) (XEN)[00250510] vgic_vcpu_inject_irq+0x40/0x1b0 (XEN)[0024bcd0] vtimer_interrupt+0x4c/0x54 (XEN)[00247010] do_IRQ+0x1a4/0x220 (XEN)[00244864] gic_interrupt+0x50/0xec (XEN)[0024fbac] do_trap_irq+0x20/0x2c (XEN)[00255240] hyp_irq+0x5c/0x60 (XEN)[00241084] context_switch+0xb8/0xc4 (XEN)[0022482c] schedule+0x684/0x6d0 (XEN)[0022785c] __do_softirq+0xcc/0xe8 (XEN)[002278d4] do_softirq+0x14/0x1c (XEN)[00240fac] idle_loop+0x134/0x154 (XEN)[0024c160] start_secondary+0x14c/0x15c (XEN)[0001] 0001 The proper solution is to context switch the virtual interrupt state at the GIC level. This would also avoid masking the output signal which requires specific handling in the guest OS and more complex code in Xen to deal with EOIs, and so is desirable for that reason too. Sadly, this solution requires some refactoring which would not be suitable for a freeze exception for the Xen 4.5 release. For now implement a temporary solution which ignores the virtual timer interrupt when the idle VCPU is running. When we reschedule the vcpu which caused the spurious interrupt, the IRQ will definitely trigger again for real, right? Signed-off-by: Julien Grall julien.gr...@linaro.org Acked-by: Ian Campbell ian.campb...@citrix.com I'll defer applying until you've said Yes to the above question. diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c index a6436f1..471d7a9 100644 --- a/xen/arch/arm/time.c +++ b/xen/arch/arm/time.c @@ -169,6 +169,19 @@ static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) static void vtimer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) { +/* + * Edge-triggered interrupt can be used for the virtual timer. Even interrupts + * if the timer output signal is masked in the context switch, the + * GIC will keep track that of any interrupts raised while IRQS as s/as/are/ I'll fix those on commit. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Fix building libxlu_cfg_y.y with bison 3.0
On 02/12/14 13:47, Ian Campbell wrote: On Mon, 2014-12-01 at 12:19 +, Wei Liu wrote: On Mon, Dec 01, 2014 at 09:42:13AM +, Ian Campbell wrote: On Sat, 2014-11-29 at 21:23 -0800, Ed Swierk wrote: - Use %lex-param instead of obsolete YYLEX_PARAM to override lex scanner parameter - Change deprecated %name-prefix= to %name-prefix Tested against bison 2.4.1 and 3.0.2. Signed-off-by: Ed Swierk eswi...@skyportsystems.com Copying Ian J who is the bison guy among the toolstack maintainers. FWIW I can confirm that libxlu_cfg_y.y won't build in Debian Jessie (bison 3.0.2) as is. And this patch fixes the problem for me. That would seem like a pretty strong case for 4.5, *except* we ship the generated files so it should be possible to build anywhere without requiring any version of bison at all. If Bison is installed then ./configure BISON=/bin/true or some such might be needed to stop it trying to regenerate. Konrad, any thoughts. The automatically generating doesn't actually work. Depending on the relative timestamps caused by a SCM checkout, or a tarball extraction, the files will be attempted to be regenerated. These files are regenerated in the XenServer build, simply because of their order in the archived tarball. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: Handle platforms with edge-triggered virtual timer
Hi Ian, On 02/12/14 13:54, Ian Campbell wrote: On Fri, 2014-11-28 at 15:17 +, Julien Grall wrote: Some platforms (such as Xgene and ARMv8 models) use an edge-triggered interrupt for the virtual timer. Even if the timer output signal is masked in the context switch, the GIC will keep track that of any interrupts raised while IRQs are disabled. As soon as IRQs are re-enabled, the virtual interrupt timer will be injected to Xen. If an idle vVCPU was scheduled next then the interrupt handler doesn't expect to the receive the IRQ and will crash: (XEN)[00228388] _spin_lock_irqsave+0x28/0x94 (PC) (XEN)[00228380] _spin_lock_irqsave+0x20/0x94 (LR) (XEN)[00250510] vgic_vcpu_inject_irq+0x40/0x1b0 (XEN)[0024bcd0] vtimer_interrupt+0x4c/0x54 (XEN)[00247010] do_IRQ+0x1a4/0x220 (XEN)[00244864] gic_interrupt+0x50/0xec (XEN)[0024fbac] do_trap_irq+0x20/0x2c (XEN)[00255240] hyp_irq+0x5c/0x60 (XEN)[00241084] context_switch+0xb8/0xc4 (XEN)[0022482c] schedule+0x684/0x6d0 (XEN)[0022785c] __do_softirq+0xcc/0xe8 (XEN)[002278d4] do_softirq+0x14/0x1c (XEN)[00240fac] idle_loop+0x134/0x154 (XEN)[0024c160] start_secondary+0x14c/0x15c (XEN)[0001] 0001 The proper solution is to context switch the virtual interrupt state at the GIC level. This would also avoid masking the output signal which requires specific handling in the guest OS and more complex code in Xen to deal with EOIs, and so is desirable for that reason too. Sadly, this solution requires some refactoring which would not be suitable for a freeze exception for the Xen 4.5 release. For now implement a temporary solution which ignores the virtual timer interrupt when the idle VCPU is running. When we reschedule the vcpu which caused the spurious interrupt, the IRQ will definitely trigger again for real, right? Xen arms a timer when the domain is saved. As we received an unexpected interrupt that means the timer expires which will make Xen injected the virtual timer interrupt (see virt_timer_expired). Signed-off-by: Julien Grall julien.gr...@linaro.org Acked-by: Ian Campbell ian.campb...@citrix.com I'll defer applying until you've said Yes to the above question. diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c index a6436f1..471d7a9 100644 --- a/xen/arch/arm/time.c +++ b/xen/arch/arm/time.c @@ -169,6 +169,19 @@ static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) static void vtimer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) { +/* + * Edge-triggered interrupt can be used for the virtual timer. Even interrupts + * if the timer output signal is masked in the context switch, the + * GIC will keep track that of any interrupts raised while IRQS as s/as/are/ I'll fix those on commit. Thanks! Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PVHVM drivers in upstream linux kernel
On 02/12/14 13:00, Juergen Gross wrote: I'd see: - XEN_PV (selects PARAVIRT, XEN_FRONTEND): be able to run as pv-domain (x86 only) Depends on PARAVIRT perhaps? - XEN_PVHVM (selects XEN_FRONTEND): be able to run as hvm-domain with pv-drivers - XEN_BACKEND (selects PARAVIRT if x86): be able to run as driver domain (dom0 or other) Does not need to select PARAVIRT -- HVM domains can run backends. - XEN_DOM0 (selects PARAVIRT if x86, XEN_BACKEND): be able to run as dom0 XEN_DOM0 depends on XEN_PV or XEN_PVH (if x86) and whatever ARM needs. - XEN_FRONTEND: be able to run as domU with pv-drivers It may also be interesting to consider splitting the PV MMU stuff under a PARAVIRT_MMU option. This might address a reason why people want to disable PARAVIRT completely. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 for-4.5 2/2] xl: fix two memory leaks
On Mon, 2014-12-01 at 11:31 +, Wei Liu wrote: Free strings returned by libxl_basename after used. Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com --- tools/libxl/xl_cmdimpl.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 0e754e7..fe3034f 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -920,6 +920,7 @@ static void parse_config_data(const char *config_source, int pci_permissive = 0; int pci_seize = 0; int i, e; +char *basename; For some reason on arm32 (only) this causes: xl_cmdimpl.c: In function ‘parse_config_data’: xl_cmdimpl.c:929:11: error: declaration of ‘basename’ shadows a global declaration [-Werror=shadow] basename(3) is defined by libgen.h which we don't include, so I suspect this is a libc issue on armhf (unless Ian has any other ideas?). How about I s/basename/kernel_basename/ or some such on commit? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 for-4.5 2/2] xl: fix two memory leaks
On Tue, Dec 02, 2014 at 02:19:49PM +, Ian Campbell wrote: On Mon, 2014-12-01 at 11:31 +, Wei Liu wrote: Free strings returned by libxl_basename after used. Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com --- tools/libxl/xl_cmdimpl.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 0e754e7..fe3034f 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -920,6 +920,7 @@ static void parse_config_data(const char *config_source, int pci_permissive = 0; int pci_seize = 0; int i, e; +char *basename; For some reason on arm32 (only) this causes: xl_cmdimpl.c: In function ‘parse_config_data’: xl_cmdimpl.c:929:11: error: declaration of ‘basename’ shadows a global declaration [-Werror=shadow] basename(3) is defined by libgen.h which we don't include, so I suspect this is a libc issue on armhf (unless Ian has any other ideas?). How about I s/basename/kernel_basename/ or some such on commit? That's OK. Wei. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 for-4.6] libxl_set_memory_target: retain the same maxmem offset on top of the current target
On Tue, 2 Dec 2014, Don Slutz wrote: On 12/02/14 06:53, Stefano Stabellini wrote: In libxl_set_memory_target when setting the new maxmem, retain the same offset on top of the current target. The offset includes memory allocated by QEMU for rom files. Signed-off-by: Stefano Stabellinistefano.stabell...@eu.citrix.com --- Changes in v2: - call libxl_domain_info instead of libxl_dominfo_init; - call libxl_domain_info before retry_transaction. diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index de23fec..569a32a 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -4694,6 +4694,9 @@ int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, char *uuid; xs_transaction_t t; +if (libxl_domain_info(ctx, ptr, domid) 0) +goto out_no_transaction; + retry_transaction: t = xs_transaction_start(ctx-xsh); @@ -4767,10 +4770,9 @@ retry_transaction: %s/memory/videoram, dompath)); videoram = videoram_s ? atoi(videoram_s) : 0; -if (enforce) { -memorykb = new_target_memkb; -rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb + -LIBXL_MAXMEM_CONSTANT); +if (enforce new_target_memkb 0) { +memorykb = ptr.max_memkb - current_target_memkb + new_target_memkb; +rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb); if (rc != 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_domain_setmaxmem domid=%d memkb=%d failed You need to remove LIBXL_MAXMEM_CONSTANT here also. I don't think so: LIBXL_MAXMEM_CONSTANT is supposed to be a safety buffer and we should keep it as is across libxl_set_memory_target calls. Arguably LIBXL_MAXMEM_CONSTANT could be removed entirely with the proposed QEMU changes but that is a separate matter. @@ -4800,8 +4802,6 @@ retry_transaction: goto out; } -libxl_dominfo_init(ptr); -xcinfo2xlinfo(ctx, info, ptr); uuid = libxl__uuid2string(gc, ptr.uuid); libxl__xs_write(gc, t, libxl__sprintf(gc, /vm/%s/memory, uuid), %PRIu32, new_target_memkb / 1024); ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: Handle platforms with edge-triggered virtual timer
On 02/12/14 14:21, Ian Campbell wrote: On Tue, 2014-12-02 at 14:08 +, Julien Grall wrote: Hi Ian, On 02/12/14 13:54, Ian Campbell wrote: On Fri, 2014-11-28 at 15:17 +, Julien Grall wrote: Some platforms (such as Xgene and ARMv8 models) use an edge-triggered interrupt for the virtual timer. Even if the timer output signal is masked in the context switch, the GIC will keep track that of any interrupts raised while IRQs are disabled. As soon as IRQs are re-enabled, the virtual interrupt timer will be injected to Xen. If an idle vVCPU was scheduled next then the interrupt handler doesn't expect to the receive the IRQ and will crash: (XEN)[00228388] _spin_lock_irqsave+0x28/0x94 (PC) (XEN)[00228380] _spin_lock_irqsave+0x20/0x94 (LR) (XEN)[00250510] vgic_vcpu_inject_irq+0x40/0x1b0 (XEN)[0024bcd0] vtimer_interrupt+0x4c/0x54 (XEN)[00247010] do_IRQ+0x1a4/0x220 (XEN)[00244864] gic_interrupt+0x50/0xec (XEN)[0024fbac] do_trap_irq+0x20/0x2c (XEN)[00255240] hyp_irq+0x5c/0x60 (XEN)[00241084] context_switch+0xb8/0xc4 (XEN)[0022482c] schedule+0x684/0x6d0 (XEN)[0022785c] __do_softirq+0xcc/0xe8 (XEN)[002278d4] do_softirq+0x14/0x1c (XEN)[00240fac] idle_loop+0x134/0x154 (XEN)[0024c160] start_secondary+0x14c/0x15c (XEN)[0001] 0001 The proper solution is to context switch the virtual interrupt state at the GIC level. This would also avoid masking the output signal which requires specific handling in the guest OS and more complex code in Xen to deal with EOIs, and so is desirable for that reason too. Sadly, this solution requires some refactoring which would not be suitable for a freeze exception for the Xen 4.5 release. For now implement a temporary solution which ignores the virtual timer interrupt when the idle VCPU is running. When we reschedule the vcpu which caused the spurious interrupt, the IRQ will definitely trigger again for real, right? Xen arms a timer when the domain is saved. As we received an unexpected interrupt that means the timer expires which will make Xen injected the virtual timer interrupt (see virt_timer_expired). Are we sure there is no race here where the software timer doesn't fire because it appears to be in the past or something? That would correspond to the sequence: disable interrupts h/w timer expires, interrupt raised but masked calculate timeout for s/w timeout = -ve. The s/w timers contains the absolute value of the deadline that will be compared to NOW(). Perhaps Xen s/w timers in the past fire immediately? The s/w timer is added in the heap and a SOFTIRQ is raised. When executed, the softirq will notice that the timer has to be fired and therefore an interrupt will be injected to the guest. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: Handle platforms with edge-triggered virtual timer
On Tue, 2014-12-02 at 14:32 +, Julien Grall wrote: On 02/12/14 14:21, Ian Campbell wrote: On Tue, 2014-12-02 at 14:08 +, Julien Grall wrote: Hi Ian, On 02/12/14 13:54, Ian Campbell wrote: On Fri, 2014-11-28 at 15:17 +, Julien Grall wrote: Some platforms (such as Xgene and ARMv8 models) use an edge-triggered interrupt for the virtual timer. Even if the timer output signal is masked in the context switch, the GIC will keep track that of any interrupts raised while IRQs are disabled. As soon as IRQs are re-enabled, the virtual interrupt timer will be injected to Xen. If an idle vVCPU was scheduled next then the interrupt handler doesn't expect to the receive the IRQ and will crash: (XEN)[00228388] _spin_lock_irqsave+0x28/0x94 (PC) (XEN)[00228380] _spin_lock_irqsave+0x20/0x94 (LR) (XEN)[00250510] vgic_vcpu_inject_irq+0x40/0x1b0 (XEN)[0024bcd0] vtimer_interrupt+0x4c/0x54 (XEN)[00247010] do_IRQ+0x1a4/0x220 (XEN)[00244864] gic_interrupt+0x50/0xec (XEN)[0024fbac] do_trap_irq+0x20/0x2c (XEN)[00255240] hyp_irq+0x5c/0x60 (XEN)[00241084] context_switch+0xb8/0xc4 (XEN)[0022482c] schedule+0x684/0x6d0 (XEN)[0022785c] __do_softirq+0xcc/0xe8 (XEN)[002278d4] do_softirq+0x14/0x1c (XEN)[00240fac] idle_loop+0x134/0x154 (XEN)[0024c160] start_secondary+0x14c/0x15c (XEN)[0001] 0001 The proper solution is to context switch the virtual interrupt state at the GIC level. This would also avoid masking the output signal which requires specific handling in the guest OS and more complex code in Xen to deal with EOIs, and so is desirable for that reason too. Sadly, this solution requires some refactoring which would not be suitable for a freeze exception for the Xen 4.5 release. For now implement a temporary solution which ignores the virtual timer interrupt when the idle VCPU is running. When we reschedule the vcpu which caused the spurious interrupt, the IRQ will definitely trigger again for real, right? Xen arms a timer when the domain is saved. As we received an unexpected interrupt that means the timer expires which will make Xen injected the virtual timer interrupt (see virt_timer_expired). Are we sure there is no race here where the software timer doesn't fire because it appears to be in the past or something? That would correspond to the sequence: disable interrupts h/w timer expires, interrupt raised but masked calculate timeout for s/w timeout = -ve. The s/w timers contains the absolute value of the deadline that will be compared to NOW(). Perhaps Xen s/w timers in the past fire immediately? The s/w timer is added in the heap and a SOFTIRQ is raised. When executed, the softirq will notice that the timer has to be fired and therefore an interrupt will be injected to the guest. Perfect, thanks. Regards, ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v8][PATCH 13/17] xen/mem_access: don't allow accessing reserved device memory
Hi, CC Tamas as he did some work on memaccess for ARM. On 01/12/14 09:24, Tiejun Chen wrote: We can't expost those reserved device memory in case of mem_access s/expost/expose/ since any access may corrupt device usage. Signed-off-by: Tiejun Chen tiejun.c...@intel.com --- xen/common/mem_access.c | 41 + 1 file changed, 41 insertions(+) diff --git a/xen/common/mem_access.c b/xen/common/mem_access.c index 6c2724b..72a807a 100644 --- a/xen/common/mem_access.c +++ b/xen/common/mem_access.c @@ -55,6 +55,43 @@ void mem_access_resume(struct domain *d) } } +/* We can't expose reserved device memory. */ +static int mem_access_check_rdm(struct domain *d, uint64_aligned_t start, +uint32_t nr) +{ +uint32_t i; +struct p2m_get_reserved_device_memory pgrdm; p2m_get_reserved_device_memory is only defined on x86. This will fail to compile on ARM when memaccess is enabled. +int rc = 0; + +if ( !is_hardware_domain(d) iommu_use_hap_pt(d) ) +{ +for ( i = 0; i nr; i++ ) +{ +pgrdm.gfn = start + i; +pgrdm.domain = d; +rc = iommu_get_reserved_device_memory(p2m_check_reserved_device_memory, + pgrdm); Same here. Overall, I'm not sure if it's worth to introduce this code in the common part has it doesn't seem useful for ARM. In any case, you have to at least stub those bits for ARM. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/2] gnttab: Introduce rwlock to protect updates to grant table state
Split grant table lock into two separate locks. One to protect maptrack state and change the other into a rwlock. The rwlock is used to prevent readers from accessing inconsistent grant table state such as current version, partially initialized active table pages, etc. Signed-off-by: Matt Wilson m...@amazon.com [chegger: ported to xen-staging, split into multiple commits] v2: * Add arm part per request from Julien Grall Signed-off-by: Christoph Egger cheg...@amazon.de Cc: Jan Beulich jbeul...@suse.com Cc: Keir Fraser k...@xen.org Cc: Julien Grall julien.gr...@linaro.org --- docs/misc/grant-tables.txt| 28 - xen/arch/arm/mm.c |4 +- xen/arch/x86/mm.c |4 +- xen/common/grant_table.c | 135 - xen/include/xen/grant_table.h |9 ++- 5 files changed, 117 insertions(+), 63 deletions(-) diff --git a/docs/misc/grant-tables.txt b/docs/misc/grant-tables.txt index 19db4ec..c9ae8f2 100644 --- a/docs/misc/grant-tables.txt +++ b/docs/misc/grant-tables.txt @@ -74,7 +74,33 @@ is complete. matching map track entry is then removed, as if unmap had been invoked. These are not used by the transfer mechanism. map-domid : owner of the mapped frame - map-ref_and_flags : grant reference, ro/rw, mapped for host or device access + map-ref : grant reference + map-flags : ro/rw, mapped for host or device access + + + Locking + ~~~ + Xen uses several locks to serialize access to the internal grant table state. + + grant_table-lock : rwlock used to prevent readers from accessing + inconsistent grant table state such as current + version, partially initialized active table pages, + etc. + grant_table-maptrack_lock : spinlock used to protect the maptrack state + + The primary lock for the grant table is a read/write spinlock. All + functions that access members of struct grant_table must acquire a + read lock around critical sections. Any modification to the members + of struct grant_table (e.g., nr_status_frames, nr_grant_frames, + active frames, etc.) must only be made if the write lock is + held. These elements are read-mostly, and read critical sections can + be large, which makes a rwlock a good choice. + + The maptrack state is protected by its own spinlock. Any access (read + or write) of struct grant_table members that have a maptrack_ + prefix must be made while holding the maptrack lock. The maptrack + state can be rapidly modified under some workloads, and the critical + sections are very small, thus we use a spinlock to protect them. diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 7d4ba0c..2765683 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1037,7 +1037,7 @@ int xenmem_add_to_physmap_one( switch ( space ) { case XENMAPSPACE_grant_table: -spin_lock(d-grant_table-lock); +write_lock(d-grant_table-lock); if ( d-grant_table-gt_version == 0 ) d-grant_table-gt_version = 1; @@ -1067,7 +1067,7 @@ int xenmem_add_to_physmap_one( t = p2m_ram_rw; -spin_unlock(d-grant_table-lock); +write_unlock(d-grant_table-lock); break; case XENMAPSPACE_shared_info: if ( idx != 0 ) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 522c43d..37c13b1 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4565,7 +4565,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-shared_info); break; case XENMAPSPACE_grant_table: -spin_lock(d-grant_table-lock); +write_lock(d-grant_table-lock); if ( d-grant_table-gt_version == 0 ) d-grant_table-gt_version = 1; @@ -4587,7 +4587,7 @@ int xenmem_add_to_physmap_one( mfn = virt_to_mfn(d-grant_table-shared_raw[idx]); } -spin_unlock(d-grant_table-lock); +write_unlock(d-grant_table-lock); break; case XENMAPSPACE_gmfn_range: case XENMAPSPACE_gmfn: diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 8fba923..018b5b6 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -227,23 +227,23 @@ double_gt_lock(struct grant_table *lgt, struct grant_table *rgt) { if ( lgt rgt ) { -spin_lock(lgt-lock); -spin_lock(rgt-lock); +spin_lock(lgt-maptrack_lock); +spin_lock(rgt-maptrack_lock); } else { if ( lgt != rgt ) -spin_lock(rgt-lock); -spin_lock(lgt-lock); +spin_lock(rgt-maptrack_lock); +spin_lock(lgt-maptrack_lock); } } static inline void
Re: [Xen-devel] [PATCH for-xen-4.5] console: increase initial conring size
On 02/12/14 14:57, Daniel Kiper wrote: In general initial conring size is sufficient. However, if log level is increased on platforms which have e.g. huge number of memory regions (I have an IBM System x3550 M2 with 8 GiB RAM which has more than 200 entries in EFI memory map) then some of earlier messages in console ring are overwritten. It means that in case of issues deeper analysis can be hindered. Sadly conring_size argument does not help because new console buffer is allocated late on heap. It means that it is not possible to allocate larger ring earlier. So, in this situation initial conring size should be increased. My experiments showed that even on not so big machines more than 26 KiB of free space are needed for initial messages. In theory we could increase conring size buffer to 32 KiB. However, I think that this value could be too small for huge machines with large number of ACPI tables and EFI memory regions. Hence, this patch increases initial conring size to 64 KiB. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com For what it is worth, XenServer has been been using a 64k default console size for a very long time now. However, a change line this must include a change to docs/misc/xen-command-line.markdown ~Andrew --- xen/drivers/char/console.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index 2f03259..429d296 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -67,7 +67,7 @@ custom_param(console_timestamps, parse_console_timestamps); static uint32_t __initdata opt_conring_size; size_param(conring_size, opt_conring_size); -#define _CONRING_SIZE 16384 +#define _CONRING_SIZE 65536 #define CONRING_IDX_MASK(i) ((i)(conring_size-1)) static char __initdata _conring[_CONRING_SIZE]; static char *__read_mostly conring = _conring; ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 for-4.6] libxl_set_memory_target: retain the same maxmem offset on top of the current target
On Tue, 2 Dec 2014, Don Slutz wrote: On 12/02/14 09:26, Stefano Stabellini wrote: On Tue, 2 Dec 2014, Don Slutz wrote: On 12/02/14 06:53, Stefano Stabellini wrote: In libxl_set_memory_target when setting the new maxmem, retain the same offset on top of the current target. The offset includes memory allocated by QEMU for rom files. Signed-off-by: Stefano Stabellinistefano.stabell...@eu.citrix.com --- Changes in v2: - call libxl_domain_info instead of libxl_dominfo_init; - call libxl_domain_info before retry_transaction. diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index de23fec..569a32a 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -4694,6 +4694,9 @@ int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, char *uuid; xs_transaction_t t; +if (libxl_domain_info(ctx, ptr, domid) 0) +goto out_no_transaction; + retry_transaction: t = xs_transaction_start(ctx-xsh); @@ -4767,10 +4770,9 @@ retry_transaction: %s/memory/videoram, dompath)); videoram = videoram_s ? atoi(videoram_s) : 0; -if (enforce) { -memorykb = new_target_memkb; -rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb + -LIBXL_MAXMEM_CONSTANT); +if (enforce new_target_memkb 0) { +memorykb = ptr.max_memkb - current_target_memkb + new_target_memkb; +rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb); if (rc != 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_domain_setmaxmem domid=%d memkb=%d failed You need to remove LIBXL_MAXMEM_CONSTANT here also. I don't think so: LIBXL_MAXMEM_CONSTANT is supposed to be a safety buffer and we should keep it as is across libxl_set_memory_target calls. Arguably LIBXL_MAXMEM_CONSTANT could be removed entirely with the proposed QEMU changes but that is a separate matter. I was talking about the line: rc=%d\n, domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc); (which is missing from the diff but is part of the LIBXL__LOG_ERRNO call). The error message no longer matches what xc_domain_setmaxmem() was called with. Yep, you are right, I'll fix. @@ -4800,8 +4802,6 @@ retry_transaction: goto out; } -libxl_dominfo_init(ptr); -xcinfo2xlinfo(ctx, info, ptr); uuid = libxl__uuid2string(gc, ptr.uuid); libxl__xs_write(gc, t, libxl__sprintf(gc, /vm/%s/memory, uuid), %PRIu32, new_target_memkb / 1024); ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5] pygrub: Fix regression from c/s d1b93ea, attempt 2
On Tue, Dec 02, 2014 at 02:02:58PM +, Ian Campbell wrote: On Tue, 2014-12-02 at 02:43 +, Andrew Cooper wrote: On 01/12/2014 20:30, Konrad Rzeszutek Wilk wrote: On Fri, Nov 28, 2014 at 11:31:24AM +, Ian Campbell wrote: On Tue, 2014-11-25 at 11:11 -0500, Boris Ostrovsky wrote: c/s d1b93ea causes substantial functional regressions in pygrub's ability to parse bootloader configuration files. c/s d1b93ea itself changed an an interface which previously used exclusively integers, to using strings in the case of a grub configuration with explicit default set, along with changing the code calling the interface to require a string. The default value for default remained as an integer. As a result, any Extlinux or Lilo configuration (which drives this interface exclusively with integers), or Grub configuration which doesn't explicitly declare a default will die with an AttributeError when attempting to call self.cf.default.isdigit() where default is an integer. Sadly, this AttributeError gets swallowed by the blanket ignore in the loop which searches partitions for valid bootloader configurations, causing the issue to be reported as Unable to find partition containing kernel We should explicitly check type of default in image_index() and process it appropriately. Reported-by: Andrew Cooper andrew.coop...@citrix.com Signed-off-by: Boris Ostrovsky boris.ostrov...@oracle.com Acked-by: Ian Campbell ian.campb...@citrix.com In general I would prefer the first line of the commit message to be a short description of the actual functional change and not a reference to fixing some other commit, which is basically meaningless to anyone reading the log even now, nevermind in six months. I think I'm going to start picking up on this more frequently from now on. CCing Konrad for RM input. I'm much less worried about corner cases etc wrt the freeze etc than I was with the larger fix proposed earlier. I am bit lost. I thought Andrew NACKed this? I didn't, as I am not in a position to. I have not tested the result, but believe it is sufficient to fix the exact error at hand. If the maintainers think it is the best solution then so be it, but I am still of the opinion that it is is still a hack upon a hack. At this point in a freeze I'm much happier with: tools/pygrub/src/pygrub |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) The same here. This is now the season of handing out band-aids. As such Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com than tools/pygrub/src/ExtLinuxConf.py |6 +++--- tools/pygrub/src/GrubConf.py |7 ++- tools/pygrub/src/LiloConf.py |6 +++--- 3 files changed, 8 insertions(+), 11 deletions(-) Plus Boris' patch is far easier to reason about in isolation in a dynamically/duck typed language. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] PVHVM drivers in upstream linux kernel
On Tue, Dec 02, 2014 at 11:05:14AM +, Ian Campbell wrote: On Tue, 2014-12-02 at 10:54 +, David Vrabel wrote: On 02/12/14 09:39, Juergen Gross wrote: Hi, looking into the upstream linux sources I realized that the PVHVM drivers of XEN are only available with the pvops kernel. Is this on purpose? Shouldn't the frontend drivers, xen/platform-pci.c etc. be configurable without having to enable CONFIG_PARAVIRT? I suppose that would be possible but I don't think it's a useful configuration because you would lose PV spinlocks for example. IIRC the reason this hasn't been implemented until now is that refactoring would be required to the various bits of driver code which assumes PAE + PARAVIRT when they aren't strictly needed, e.g. grant table code. Whether its worth the churn at this stage is debatable, but I think the (in)ability to use PV spinlocks is a red-herring. Adding PV drivers to an HVM guest is a useful thing to do, even without PV spinlocks. PV IO gets you far more incremental benefit than the locks do, adding PV IO paths is the number 1 thing which should be done to any guest. One actual usecase is installing from a distro installer which isn't PAE, let alone PARAVIRT enabled[0], to get far enough that you can install a more capable PVHVM kernel with more bells and whistles. If there were distros around who refused wholesale to enable PARAVIRT even in a non-default kernel then it would be more likely that they could be convinced to enable a set of PV IO drivers, since they have 0 impact on a non-PARAVIRT system, and still give significant benefit to Xen users. I don't know of any of the major distros are refusing PARAVIRT in this way though. Ian. [0] The default i386 Debian installer falls into this camp, but you can use the special PV Xen variant to install as PVHVM too so it's not so critical. And the Fedora 21 LiveISO (32-bit) does too. ___ 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] [Xen-users] 4.5 git: regression in xen systemd shutdown hangs the OS
On Tue, Dec 02, Ian Campbell wrote: On Mon, 2014-12-01 at 23:41 +, Mark Pryor wrote: list, Thanks. If you've identified a buggy changeset then it is fine to post to the devel lists. I've added a CC. I've also CCd everyone listed in the commit which you've fingered. Olaf, does this suggested change look correct? If so then can you turn it into a patch please. Yes, something like this (sed -i 's@socket@service@g' *.in): diff --git a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in index 4d4cb23..3befadc 100644 --- a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in +++ b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in @@ -1,7 +1,7 @@ [Unit] Description=xen-init-dom0, initialise Dom0 configuration (xenstore nodes, JSON configuration stub) -Requires=xenstored.socket proc-xen.mount -After=xenstored.socket proc-xen.mount +Requires=xenstored.service proc-xen.mount +After=xenstored.service proc-xen.mount ConditionPathExists=/proc/xen/capabilities [Service] diff --git a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in index 6b9c96e..0a5807a 100644 --- a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in +++ b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in @@ -1,7 +1,7 @@ [Unit] Description=qemu for xen dom0 disk backend -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket xenconsoled.service +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service xenconsoled.service Before=xendomains.service libvirtd.service libvirt-guests.service RefuseManualStop=true ConditionPathExists=/proc/xen/capabilities diff --git a/tools/hotplug/Linux/systemd/xenconsoled.service.in b/tools/hotplug/Linux/systemd/xenconsoled.service.in index 2c5d99f..cb44cd6 100644 --- a/tools/hotplug/Linux/systemd/xenconsoled.service.in +++ b/tools/hotplug/Linux/systemd/xenconsoled.service.in @@ -1,7 +1,7 @@ [Unit] Description=Xenconsoled - handles logging from guest consoles and hypervisor -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service ConditionPathExists=/proc/xen/capabilities [Service] diff --git a/tools/hotplug/Linux/systemd/xendomains.service.in b/tools/hotplug/Linux/systemd/xendomains.service.in index 757278f..9962671 100644 --- a/tools/hotplug/Linux/systemd/xendomains.service.in +++ b/tools/hotplug/Linux/systemd/xendomains.service.in @@ -1,7 +1,7 @@ [Unit] Description=Xendomains - start and stop guests on boot and shutdown -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket xenconsoled.service xen-init-dom0.service +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service xenconsoled.service xen-init-dom0.service ConditionPathExists=/proc/xen/capabilities [Service] Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH for-xen-4.5 2/3] gitignore: ignore some files generated by configure
Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- .gitignore |3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index b24e905..2b51d5f 100644 --- a/.gitignore +++ b/.gitignore @@ -141,12 +141,15 @@ tools/flask/utils/flask-set-bool tools/flask/utils/flask-label-pci tools/hotplug/common/hotplugpath.sh tools/hotplug/FreeBSD/rc.d/xencommons +tools/hotplug/Linux/init.d/sysconfig.xencommons tools/hotplug/Linux/init.d/xen-watchdog +tools/hotplug/Linux/init.d/xencommons tools/hotplug/Linux/init.d/xendomains tools/hotplug/Linux/vif-setup tools/hotplug/Linux/xen-backend.rules tools/hotplug/Linux/xen-hotplug-common.sh tools/hotplug/Linux/xendomains +tools/hotplug/NetBSD/rc.d/xencommons tools/include/xen/* tools/include/xen-foreign/*.(c|h|size) tools/include/xen-foreign/checker -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH for-xen-4.5 1/3] tools/hotplug: distclean target should remove files generated by configure
Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- tools/Makefile |3 +++ tools/hotplug/Makefile |5 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/Makefile b/tools/Makefile index af9798a..19b24f3 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -261,6 +261,9 @@ subdir-all-debugger/kdd: .phony subdir-distclean-firmware: .phony $(MAKE) -C firmware distclean +subdir-distclean-hotplug: .phony + $(MAKE) -C hotplug distclean + subtree-force-update: ifeq ($(CONFIG_QEMU_XEN),y) $(MAKE) qemu-xen-dir-force-update diff --git a/tools/hotplug/Makefile b/tools/hotplug/Makefile index 14ae9a8..a29a522 100644 --- a/tools/hotplug/Makefile +++ b/tools/hotplug/Makefile @@ -6,5 +6,8 @@ SUBDIRS-$(CONFIG_NetBSD) += NetBSD SUBDIRS-$(CONFIG_Linux) += Linux SUBDIRS-$(CONFIG_FreeBSD) += FreeBSD -.PHONY: all clean install +.PHONY: all clean distclean install all clean install: %: subdirs-% + +distclean: + rm -f Linux/init.d/sysconfig.xencommons Linux/init.d/xencommons NetBSD/rc.d/xencommons -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] libxc: check in xc_get_tot_pages() that the proper domain is reported
XEN_DOMCTL_getdomaininfo, which is being used by xc_domain_getinfo(), has strange interface: it reports first domain which has domid = requested domid so all callers are supposed to check that the proper domain(s) was queried by checking domid. xc_get_tot_pages() doesn't do that. In case the requested domain was destroyed it will report first domain with domid requested domid which is apparently misleading as there is no way xc_get_tot_pages() callers can figure out that they got tot_pages for some other domain. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com --- tools/libxc/xc_private.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c index 1c214dd..e2441ad 100644 --- a/tools/libxc/xc_private.c +++ b/tools/libxc/xc_private.c @@ -613,8 +613,10 @@ int xc_get_pfn_list(xc_interface *xch, long xc_get_tot_pages(xc_interface *xch, uint32_t domid) { xc_dominfo_t info; -return (xc_domain_getinfo(xch, domid, 1, info) != 1) ? --1 : info.nr_pages; +if ( (xc_domain_getinfo(xch, domid, 1, info) != 1) || + (info.domid != domid) ) +return -1; +return info.nr_pages; } int xc_copy_to_domain_page(xc_interface *xch, -- 1.9.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH for-xen-4.5 3/3] gitignore: group tools/hotplug files in one place
Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- .gitignore |9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 2b51d5f..8c8c06f 100644 --- a/.gitignore +++ b/.gitignore @@ -145,6 +145,10 @@ tools/hotplug/Linux/init.d/sysconfig.xencommons tools/hotplug/Linux/init.d/xen-watchdog tools/hotplug/Linux/init.d/xencommons tools/hotplug/Linux/init.d/xendomains +tools/hotplug/Linux/systemd/*.conf +tools/hotplug/Linux/systemd/*.mount +tools/hotplug/Linux/systemd/*.socket +tools/hotplug/Linux/systemd/*.service tools/hotplug/Linux/vif-setup tools/hotplug/Linux/xen-backend.rules tools/hotplug/Linux/xen-hotplug-common.sh @@ -329,8 +333,3 @@ tools/xenstore/xenstore-watch docs/txt/misc/*.txt docs/txt/man/*.txt docs/figs/*.png - -tools/hotplug/Linux/systemd/*.conf -tools/hotplug/Linux/systemd/*.mount -tools/hotplug/Linux/systemd/*.socket -tools/hotplug/Linux/systemd/*.service -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH for-xen-4.5 0/3] tools: build system fixes and cleanups
Hi, Here are three tools build system patches: - tools/hotplug: distclean target should remove files generated by configure, - gitignore: ignore some files generated by configure, - gitignore: group tools/hotplug files in one place. First two are real fixes. Last one is a cleanup which is nice to have but not required at this stage of 4.5 development. Daniel .gitignore | 12 +++- tools/Makefile |3 +++ tools/hotplug/Makefile |5 - 3 files changed, 14 insertions(+), 6 deletions(-) Daniel Kiper (3): tools/hotplug: distclean target should remove files generated by configure gitignore: ignore some files generated by configure gitignore: group tools/hotplug files in one place ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 for-4.6] libxl_set_memory_target: retain the same maxmem offset on top of the current target
On 12/02/14 09:59, Don Slutz wrote: On 12/02/14 09:26, Stefano Stabellini wrote: On Tue, 2 Dec 2014, Don Slutz wrote: On 12/02/14 06:53, Stefano Stabellini wrote: In libxl_set_memory_target when setting the new maxmem, retain the same offset on top of the current target. The offset includes memory allocated by QEMU for rom files. Signed-off-by: Stefano Stabellinistefano.stabell...@eu.citrix.com --- Changes in v2: - call libxl_domain_info instead of libxl_dominfo_init; - call libxl_domain_info before retry_transaction. diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index de23fec..569a32a 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -4694,6 +4694,9 @@ int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, char *uuid; xs_transaction_t t; +if (libxl_domain_info(ctx, ptr, domid) 0) +goto out_no_transaction; + retry_transaction: t = xs_transaction_start(ctx-xsh); @@ -4767,10 +4770,9 @@ retry_transaction: %s/memory/videoram, dompath)); videoram = videoram_s ? atoi(videoram_s) : 0; -if (enforce) { -memorykb = new_target_memkb; -rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb + -LIBXL_MAXMEM_CONSTANT); +if (enforce new_target_memkb 0) { +memorykb = ptr.max_memkb - current_target_memkb + new_target_memkb; My testing shows that this should be: memorykb = ptr.max_memkb - (current_target_memkb + videoram) + new_target_memkb; As far as I can tell the reason for this is that memory/target (aka current_target_memkb) was set based on: new_target_memkb -= videoram; -Don Slutz +rc = xc_domain_setmaxmem(ctx-xch, domid, memorykb); if (rc != 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, xc_domain_setmaxmem domid=%d memkb=%d failed You need to remove LIBXL_MAXMEM_CONSTANT here also. I don't think so: LIBXL_MAXMEM_CONSTANT is supposed to be a safety buffer and we should keep it as is across libxl_set_memory_target calls. Arguably LIBXL_MAXMEM_CONSTANT could be removed entirely with the proposed QEMU changes but that is a separate matter. I was talking about the line: rc=%d\n, domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc); (which is missing from the diff but is part of the LIBXL__LOG_ERRNO call). The error message no longer matches what xc_domain_setmaxmem() was called with. -Don Slutz @@ -4800,8 +4802,6 @@ retry_transaction: goto out; } -libxl_dominfo_init(ptr); -xcinfo2xlinfo(ctx, info, ptr); uuid = libxl__uuid2string(gc, ptr.uuid); libxl__xs_write(gc, t, libxl__sprintf(gc, /vm/%s/memory, uuid), %PRIu32, new_target_memkb / 1024); ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH for-4.5] systemd: use pkg-config to determine systemd library availability
AC_CHECK_LIB fails on Debian Jessie since the ld flag it generates is incorrect, even in the event systemd library is available. Use PKG_CHECK_MODULES instead. Tested on Debian Jessie and Arch Linux. Please rerun autogen.sh after applying this patch. Reported-by: Mark Pryor tlvie...@yahoo.com Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com Cc: Anthony Perard anthony.per...@citrix.com Cc: Luis R. Rodriguez mcg...@do-not-panic.com Cc: Mark Pryor tlvie...@yahoo.com --- m4/systemd.m4 | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/m4/systemd.m4 b/m4/systemd.m4 index a832d59..b04964b 100644 --- a/m4/systemd.m4 +++ b/m4/systemd.m4 @@ -42,13 +42,6 @@ AC_DEFUN([AX_ALLOW_SYSTEMD_OPTS], [ ]) AC_DEFUN([AX_CHECK_SYSTEMD_LIBS], [ - AC_CHECK_HEADER([systemd/sd-daemon.h], [ - AC_CHECK_LIB([systemd-daemon], [sd_listen_fds], [libsystemddaemon=y]) - ]) - AS_IF([test x$libsystemddaemon = x], [ - AC_MSG_ERROR([Unable to find a suitable libsystemd-daemon library]) - ]) - PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon]) dnl pkg-config older than 0.24 does not set these for dnl PKG_CHECK_MODULES() worth also noting is that as of version 208 @@ -98,9 +91,8 @@ AC_DEFUN([AX_CHECK_SYSTEMD], [ ]) AC_DEFUN([AX_CHECK_SYSTEMD_ENABLE_AVAILABLE], [ - AC_CHECK_HEADER([systemd/sd-daemon.h], [ - AC_CHECK_LIB([systemd-daemon], [sd_listen_fds], [systemd=y]) - ]) + PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon], [systemd=y], + [systemd=n]) ]) dnl Enables systemd by default and requires a --disable-systemd option flag -- 2.1.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2] Add support for Xen ARM guest on FreeBSD
Hello Andrew, On 28/11/2014 13:57, Andrew Turner wrote: On Sun, 23 Nov 2014 22:35:36 + Julien Grall julien.gr...@linaro.org wrote: Major changes in this new version: * Add Device Tree support via Linux Boot ABI * Add zImage support * Netfront support * Blkfront fixes * DOM0 support (separate branch see below) The former item is very hackish. I was wondering if there is another way to do it? Or maybe we should support FreeBSD Bootloader in ARM guest? I think using the loader is the correct way to handle booting in Xen. It allows us to relocate the dtb as required. It look like a zImage then use the Xen console to interact with the user. Thanks, I will give a look to this solution. The patch series is divided in X parts: * #1 - #14: Clean up and bug fixes for Xen. They can be applied without the rest of the series * #15 - #19: Update Xen interface to 4.4 and fix compilation. It's required for ARM. * #20 - #26: Update Xen code to support ARM * #27 - #33: Rework the event channel code for supporting ARM. I will work with Royger to come with a common interface with x86 * #34 - #36: Add support for ARM in Xen code * #37 - #46: ARM bug fixes and new features. Some of thoses patches (#37 - #40) could be applied without the rest of the series * #47 - #48: Add Xen ARM platform I have committed patches 30 and 40 as they look good. Thanks! I'm not familiar with the code to review 37 or 38, however from my quick look at 38 I appears _bus_dmamap_load_buffer does take in to account buflen and dmat-maxsegsz when setting sgsize just not dmat-alignment. Right, I guess I could just keep the roundup2. ... TODO: * Add SMP/PSCI support in FreeBSD. Could be useful other platform too Adding PSCI support is on my TODO lost for arm64, however I don't expect to get on ti in until early next year. BTW, what is the actual status of the ARM64 port? I plan to give a look for adding Xen support too. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxc: check in xc_get_tot_pages() that the proper domain is reported
On 12/02/14 10:18, Vitaly Kuznetsov wrote: XEN_DOMCTL_getdomaininfo, which is being used by xc_domain_getinfo(), has strange interface: it reports first domain which has domid = requested domid so all callers are supposed to check that the proper domain(s) was queried by checking domid. xc_get_tot_pages() doesn't do that. In case the requested domain was destroyed it will report first domain with domid requested domid which is apparently misleading as there is no way xc_get_tot_pages() callers can figure out that they got tot_pages for some other domain. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com --- tools/libxc/xc_private.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c index 1c214dd..e2441ad 100644 --- a/tools/libxc/xc_private.c +++ b/tools/libxc/xc_private.c @@ -613,8 +613,10 @@ int xc_get_pfn_list(xc_interface *xch, long xc_get_tot_pages(xc_interface *xch, uint32_t domid) { xc_dominfo_t info; -return (xc_domain_getinfo(xch, domid, 1, info) != 1) ? --1 : info.nr_pages; +if ( (xc_domain_getinfo(xch, domid, 1, info) != 1) || + (info.domid != domid) ) +return -1; +return info.nr_pages; } int xc_copy_to_domain_page(xc_interface *xch, Looks good to me. Reviewed-by: Don Slutz dsl...@verizon.com -Don Slutz ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [COVERITY ACCESS] Request for access to Coverity
On Tue, 2014-12-02 at 15:10 +, George Dunlap wrote: On Thu, Nov 27, 2014 at 3:11 PM, Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: On Nov 27, 2014 6:59 AM, Tim Deegan t...@xen.org wrote: At 11:39 + on 27 Nov (1417084797), George Dunlap wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 I agree to the conditions in the XenProject Coverity contribution guidelines [1]. I'm a developer working for Citrix Systems UK, Ltd. I've been active in the Xen community since 2006; I was release coordinator for the 4.3 and 4.4 releases, and I am currently maintainer of the Xen scheduling subsystem. I would like access primarily to be able to write and speak intelligently about Xen and Coverity in blog postings and conference talks. I figure it would be easier to go through the stats and history myself, rather than trying to get some other developer to do it for me. (Although of course once I have access I'll probably end up doing some work looking at scans anyway.) +1 +1 too. OK, that's +4 and no minuses after 5 days. How long to I have to wait? :-) http://www.xenproject.org/help/contribution-guidelines.html says seven days... Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5] systemd: use pkg-config to determine systemd library availability
On Tue, 2014-12-02 at 15:11 +, Wei Liu wrote: AC_CHECK_LIB fails on Debian Jessie since the ld flag it generates is incorrect, even in the event systemd library is available. Use PKG_CHECK_MODULES instead. Tested on Debian Jessie and Arch Linux. Please rerun autogen.sh after applying this patch. Reported-by: Mark Pryor tlvie...@yahoo.com Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Ian Campbell ian.campb...@citrix.com Cc: Ian Jackson ian.jack...@eu.citrix.com Cc: Anthony Perard anthony.per...@citrix.com Cc: Luis R. Rodriguez mcg...@do-not-panic.com Cc: Mark Pryor tlvie...@yahoo.com Acked-by: Ian Campbell ian.campb...@citrix.com I think you should make reference in the commit log to the fact that the library has change name upstream but that a compatibility pkg-config is provided for the old name. It'd be nice if there was a less repetitive way to handle defaults + overriding in the autoconf stuff, but for a freeze exception this is the way to go. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] xl pci-attach silently fails the first time
On Mon, Dec 01, Sander Eikelenboom wrote: Monday, December 1, 2014, 3:34:09 PM, you wrote: actually be a workaround for the double pci-attach bug. Don't know about that bug. You just replied to it. ;-) Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] xl pci-attach silently fails the first time
On Mon, Dec 01, Konrad Rzeszutek Wilk wrote: That is odd - I see any device 'hot-plugged' being added at 00:05 and further. Does this by any chance depend on the guest?! I mean, how is the guest notified that a PCI device is gone (by unplug)? Maybe the pvops case just happens to work because the unplug happens early, perhaps before PCI discovery?! Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 for-4.5 0/2] xl/libxl: fix API and two memory leaks
On Mon, 2014-12-01 at 16:54 -0500, Konrad Rzeszutek Wilk wrote: On Mon, Dec 01, 2014 at 11:31:11AM +, Wei Liu wrote: Return value of libxl_basename was erroneously marked as const. This series removes that const and fixes two memory leaks in xl. I think these fixes should be included in 4.5, given that they fix real issues and are very straight foward to reason about. Thank you for catching those! Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com Applied both, including the modification discussed in my reply to the second. Wei. Wei Liu (2): libxl: un-constify return value of libxl_basename xl: fix two memory leaks tools/libxl/libxl.h | 10 ++ tools/libxl/libxl_utils.c |5 - tools/libxl/libxl_utils.h |6 +- tools/libxl/xl_cmdimpl.c |8 ++-- 4 files changed, 25 insertions(+), 4 deletions(-) -- 1.7.10.4 ___ 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] xenstore: Clarify xs_open() semantics
On Mon, 2014-12-01 at 16:16 -0500, Konrad Rzeszutek Wilk wrote: On Fri, Nov 28, 2014 at 12:31:34PM +, Ian Jackson wrote: Razvan Cojocaru writes ([PATCH] xenstore: Clarify xs_open() semantics): Added to the xs_open() comments in xenstore.h. The text has been taken almost verbatim from a xen-devel email by Ian Campbell, and confirmed as accurate by Ian Jackson. Signed-off-by: Razvan Cojocaru rcojoc...@bitdefender.com Suggested-off-by: Ian Campbell ian.campb...@citrix.com Acked-by: Ian Jackson ian.jack...@eu.citrix.com It is documentation per say so it can go in .. Applied. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 答复: Re: [PATCH] missing chunk of HVM direct kernel boot patch
On Mon, 2014-12-01 at 16:51 -0500, Konrad Rzeszutek Wilk wrote: On Mon, Dec 01, 2014 at 09:21:24AM +, Ian Campbell wrote: On Mon, 2014-12-01 at 01:21 -0700, Chun Yan Liu wrote: Ian Campbell ian.campb...@citrix.com 2014-11-28 下午 20:01 On Fri, 2014-11-28 at 13:55 +0800, Chunyan Liu wrote: Found by Stefano, this chunk of the patch was never applied to xen-unstable (commit 11dffa2359e8a2629490c14c029c7c7c777b3e47), see http://marc.info/?l=qemu-develm=140471493425353w=2. How strange, git am usually makes it pretty difficult to miss hunks. Sorry about this. Signed-off-by: Chunyan Liu cy...@suse.com Acked-by: Ian Campbell ian.campb...@citrix.com I'm afraid that despite the circumstances this still needs a release ack from Konrad. Obviously the upside is fixing a partially implemented feature, but I'm not sure what the downsides are. Has this been tested with stubdoms, including when this feature is not used? My biggest concern is that because this function is also used to build the command line for the stubdom and the stubdom is PV and hence has at least a -kernel setting then this new code might break that use case, by adding these options when they are not wanted. This path is all a bit tangled so I'm not 100% sure if those fields are actually set or not. '-kernel' is only added to qemu command line under HVM case. PV would not be affected. And only added when device model is upstream qemu, but not old qemu-xen. About stubdom, tested before, when stubdom is using old qemu-xen, would not be affected. Ah yes, I'd forgotten we didn't have upstream stubdom yet, obviously any issues here will become apparent whenever that gets implemented. nods Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com Applied, thanks. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] fix migration failure with xl migrate --debug
On Mon, 2014-12-01 at 15:25 -0500, Konrad Rzeszutek Wilk wrote: On Fri, Nov 28, 2014 at 11:52:26AM +, Ian Jackson wrote: Ian Campbell writes (Re: [PATCH v2] fix migration failure with xl migrate --debug): Acked-by: Ian Campbell ian.campb...@citrix.com Thanks for reviewing it :-). It's pretty big but a large chunk is pretty mechanical. Were you intending this for 4.5? (Konrad CCd). I think (based on reading the thread but not the code) that this ought to be in 4.5. Correct. Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com Applied. thanks. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Xen-users] 4.5 git: regression in xen systemd shutdown hangs the OS
On Tue, 2014-12-02 at 16:17 +0100, Olaf Hering wrote: On Tue, Dec 02, Ian Campbell wrote: On Mon, 2014-12-01 at 23:41 +, Mark Pryor wrote: list, Thanks. If you've identified a buggy changeset then it is fine to post to the devel lists. I've added a CC. I've also CCd everyone listed in the commit which you've fingered. Olaf, does this suggested change look correct? If so then can you turn it into a patch please. Yes, something like this (sed -i 's@socket@service@g' *.in): Can you submit to -devel with a commit log and S-o-b etc please. diff --git a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in index 4d4cb23..3befadc 100644 --- a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in +++ b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in @@ -1,7 +1,7 @@ [Unit] Description=xen-init-dom0, initialise Dom0 configuration (xenstore nodes, JSON configuration stub) -Requires=xenstored.socket proc-xen.mount -After=xenstored.socket proc-xen.mount +Requires=xenstored.service proc-xen.mount +After=xenstored.service proc-xen.mount ConditionPathExists=/proc/xen/capabilities [Service] diff --git a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in index 6b9c96e..0a5807a 100644 --- a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in +++ b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in @@ -1,7 +1,7 @@ [Unit] Description=qemu for xen dom0 disk backend -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket xenconsoled.service +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service xenconsoled.service Before=xendomains.service libvirtd.service libvirt-guests.service RefuseManualStop=true ConditionPathExists=/proc/xen/capabilities diff --git a/tools/hotplug/Linux/systemd/xenconsoled.service.in b/tools/hotplug/Linux/systemd/xenconsoled.service.in index 2c5d99f..cb44cd6 100644 --- a/tools/hotplug/Linux/systemd/xenconsoled.service.in +++ b/tools/hotplug/Linux/systemd/xenconsoled.service.in @@ -1,7 +1,7 @@ [Unit] Description=Xenconsoled - handles logging from guest consoles and hypervisor -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service ConditionPathExists=/proc/xen/capabilities [Service] diff --git a/tools/hotplug/Linux/systemd/xendomains.service.in b/tools/hotplug/Linux/systemd/xendomains.service.in index 757278f..9962671 100644 --- a/tools/hotplug/Linux/systemd/xendomains.service.in +++ b/tools/hotplug/Linux/systemd/xendomains.service.in @@ -1,7 +1,7 @@ [Unit] Description=Xendomains - start and stop guests on boot and shutdown -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket xenconsoled.service xen-init-dom0.service +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service xenconsoled.service xen-init-dom0.service ConditionPathExists=/proc/xen/capabilities [Service] Olaf ___ 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] libxl: Don't derefence null new_name pointer in libxl_domain_rename()
On Mon, 2014-12-01 at 17:02 -0500, Konrad Rzeszutek Wilk wrote: On Mon, Dec 01, 2014 at 02:50:56PM +, Ian Campbell wrote: On Mon, 2014-12-01 at 14:27 +, Euan Harris wrote: libxl__domain_rename() unconditionally dereferences its new_name parameter, to check whether it is an empty string. Add a check to avoid a segfault if new_name is null. Signed-off-by: Euan Harris euan.har...@citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com I think this is a good fix to have for 4.5, Konrad CCd. Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com Applied, fixing the typo in the subject as I went. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: libxl_domain_info: fix typo in error message
On Mon, 2014-12-01 at 11:15 +, Ian Campbell wrote: On Mon, 2014-12-01 at 10:47 +, Euan Harris wrote: Signed-off-by: Euan Harris euan.har...@citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com This is so trivial as to not need a release ack IMHO, I'll apply next time I'm doing such things unless someone beats me to it... Done. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] tools/hotplug: update systemd dependency to use service instead of socket
On Tue, 2014-12-02 at 16:39 +0100, Olaf Hering wrote: Since commit 4542ae340d75bd6319e3fcd94e6c9336e210aeef (tools/hotplug: systemd xenstored dependencies) all service files use the .socket unit as startup dependency. While this happens to work for boot it fails for shutdown because a .socket does not seem to enforce ordering. When xendomains.service runs during shutdown then systemd will stop xenstored.service at the same time. Change all xenstored.socket to xenstored.service to let systemd know that xenstored has to be shutdown after everything else. Reported-by: Mark Pryor tlvie...@yahoo.com Signed-off-by: Olaf Hering o...@aepfle.de Cc: Ian Jackson ian.jack...@eu.citrix.com Cc: Stefano Stabellini stefano.stabell...@eu.citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com Cc: Wei Liu wei.l...@citrix.com --- This should go into 4.5 to fix xendomains.service. CCing Konrad... tools/hotplug/Linux/systemd/xen-init-dom0.service.in | 4 ++-- tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in | 4 ++-- tools/hotplug/Linux/systemd/xenconsoled.service.in| 4 ++-- tools/hotplug/Linux/systemd/xendomains.service.in | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in index 4d4cb23..3befadc 100644 --- a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in +++ b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in @@ -1,7 +1,7 @@ [Unit] Description=xen-init-dom0, initialise Dom0 configuration (xenstore nodes, JSON configuration stub) -Requires=xenstored.socket proc-xen.mount -After=xenstored.socket proc-xen.mount +Requires=xenstored.service proc-xen.mount +After=xenstored.service proc-xen.mount ConditionPathExists=/proc/xen/capabilities [Service] diff --git a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in index 6b9c96e..0a5807a 100644 --- a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in +++ b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in @@ -1,7 +1,7 @@ [Unit] Description=qemu for xen dom0 disk backend -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket xenconsoled.service +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service xenconsoled.service Before=xendomains.service libvirtd.service libvirt-guests.service RefuseManualStop=true ConditionPathExists=/proc/xen/capabilities diff --git a/tools/hotplug/Linux/systemd/xenconsoled.service.in b/tools/hotplug/Linux/systemd/xenconsoled.service.in index 2c5d99f..cb44cd6 100644 --- a/tools/hotplug/Linux/systemd/xenconsoled.service.in +++ b/tools/hotplug/Linux/systemd/xenconsoled.service.in @@ -1,7 +1,7 @@ [Unit] Description=Xenconsoled - handles logging from guest consoles and hypervisor -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service ConditionPathExists=/proc/xen/capabilities [Service] diff --git a/tools/hotplug/Linux/systemd/xendomains.service.in b/tools/hotplug/Linux/systemd/xendomains.service.in index 757278f..9962671 100644 --- a/tools/hotplug/Linux/systemd/xendomains.service.in +++ b/tools/hotplug/Linux/systemd/xendomains.service.in @@ -1,7 +1,7 @@ [Unit] Description=Xendomains - start and stop guests on boot and shutdown -Requires=proc-xen.mount xenstored.socket -After=proc-xen.mount xenstored.socket xenconsoled.service xen-init-dom0.service +Requires=proc-xen.mount xenstored.service +After=proc-xen.mount xenstored.service xenconsoled.service xen-init-dom0.service ConditionPathExists=/proc/xen/capabilities [Service] ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 03/18] Pass host to toolstack()
This will be needed in a future patch. Everywhere already has a $ho in hand. Also cache the answer as $ho-{Toolstack}. I scanned the source with: find -name \*.pm -exec perl -c {} \; for i in ts-* ; do perl -c $i; done which reported Not enough arguments for Osstest::TestSupport::toolstack for each callsite which needed changing. Also don't pass the toolstack command name directly to host_get_free_memory(). Look it up instead. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/TestSupport.pm | 16 ++-- ts-debian-fixup| 2 +- ts-debian-hvm-install | 4 ++-- ts-guest-localmigrate | 2 +- ts-guest-migrate | 2 +- ts-guest-saverestore | 8 ts-guest-start | 4 ++-- ts-guest-stop | 2 +- ts-logs-capture| 2 +- ts-migrate-support-check | 4 ++-- ts-redhat-install | 2 +- ts-rumpuserxen-demo-xenstorels | 4 ++-- ts-windows-install | 2 +- ts-xen-install | 10 +- 14 files changed, 34 insertions(+), 30 deletions(-) diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm index 2ac490f..b7887bc 100644 --- a/Osstest/TestSupport.pm +++ b/Osstest/TestSupport.pm @@ -954,8 +954,9 @@ END }); } -sub host_get_free_memory($$) { -my ($ho,$toolstack) = @_; +sub host_get_free_memory($) { +my ($ho) = @_; +my $toolstack = toolstack($ho)-{Command}; # The line is as followed: # free_memory : my $info = target_cmd_output_root($ho, $toolstack info, 10); @@ -1318,7 +1319,7 @@ sub guest_await_shutdown ($$$) { sub guest_destroy ($$) { my ($ho,$gho) = @_; -target_cmd_root($ho, toolstack()-{Command}. destroy $gho-{Name}, 40); +target_cmd_root($ho, toolstack($ho)-{Command}. destroy $gho-{Name}, 40); } sub guest_create ($$) { @@ -1609,7 +1610,7 @@ sub guest_check_up ($) { sub guest_get_state ($$) { my ($ho,$gho) = @_; -my $domains= target_cmd_output_root($ho, toolstack()-{Command}. list); +my $domains= target_cmd_output_root($ho, toolstack($ho)-{Command}. list); $domains =~ s/^Name.*\n//; foreach my $l (split /\n/, $domains) { $l =~ m/^(\S+) (?: \s+ \d+ ){3} \s+ ([-a-z]+) \s/x or die $l ?; @@ -1814,7 +1815,7 @@ sub guest_find_domid ($$) { my ($ho,$gho) = @_; return if defined $gho-{Domid}; my $list= target_cmd_output_root($ho, -toolstack()-{Command}. list $gho-{Name}); +toolstack($ho)-{Command}. list $gho-{Name}); $list =~ m/^(?!Name\s)(\S+)\s+(\d+)\s+(\d+)+(\d+)\s.*$/m or die domain list: $list; $1 eq $gho-{Name} or die domain list name $1 expected $gho-{Name}; @@ -1884,7 +1885,9 @@ our %toolstacks= }, ); -sub toolstack () { +sub toolstack ($) { +my ($ho) = @_; +return $ho-{Toolstack} if $ho-{Toolstack}; my $tsname= $r{toolstack}; $tsname= 'xend' if !defined $tsname; my $ts= $toolstacks{$tsname}; @@ -1893,6 +1896,7 @@ sub toolstack () { logm(toolstack $tsname); $ts-{Name}= $tsname; } +$ho-{Toolstack} = $ts; return $ts; } diff --git a/ts-debian-fixup b/ts-debian-fixup index f001418..11e956c 100755 --- a/ts-debian-fixup +++ b/ts-debian-fixup @@ -142,7 +142,7 @@ sub otherfixupcfg () { if (@pcipt) { logm(checking passthrough device(s) are assignable: @pcipt); my @assignables= split /\n/, -target_cmd_output_root($ho, toolstack()-{Command}. +target_cmd_output_root($ho, toolstack($ho)-{Command}. pci-assignable-list); foreach my $pcipt (@pcipt) { die not assignable: $pcipt (not in: @assignables) diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install index 37eade2..0148eef 100755 --- a/ts-debian-hvm-install +++ b/ts-debian-hvm-install @@ -41,7 +41,7 @@ our $disk_mb= 1; our $guesthost= $gn.guest.osstest; our $gho; -our $toolstack= toolstack()-{Command}; +our $toolstack= toolstack($ho)-{Command}; sub preseed () { @@ -171,7 +171,7 @@ sub prep () { # If host has 8G free memory, create a guest with 4G memory to catch # any error that triggers cross 4G boundary -my $host_freemem_mb = host_get_free_memory($ho, $toolstack); +my $host_freemem_mb = host_get_free_memory($ho); my $ram_minslop = 100; my $ram_lots = 5000; if ($host_freemem_mb $ram_lots * 2 + $ram_minslop) { diff --git a/ts-guest-localmigrate b/ts-guest-localmigrate index e50e93a..f3381da 100755 --- a/ts-guest-localmigrate +++ b/ts-guest-localmigrate @@ -33,7 +33,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub migrate () { guest_checkrunning($ho,$gho) or die $gho-{Name}; target_cmd_root($ho, - toolstack()-{Command} + toolstack($ho)-{Command} . migrate $gho-{Name} localhost, $timeout{Migrate}); } diff --git
[Xen-devel] [PATCH OSSTEST v2 10/18] Toolstack: Refactor shutdown support
Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/Toolstack/libvirt.pm | 6 ++ Osstest/Toolstack/xend.pm| 1 + Osstest/Toolstack/xl.pm | 7 +++ ts-guest-stop| 5 + 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm index 54d2a6d..d039c06 100644 --- a/Osstest/Toolstack/libvirt.pm +++ b/Osstest/Toolstack/libvirt.pm @@ -57,4 +57,10 @@ sub consolecmd ($$) { return virsh console $gn; } +sub shutdown_wait ($$) { +my ($self,$gho) = @_; +my $gn = $gho-{Name}; +die libvirt shutdown wait not implemented yet. +} + 1; diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm index 896d949..d0e1113 100644 --- a/Osstest/Toolstack/xend.pm +++ b/Osstest/Toolstack/xend.pm @@ -37,5 +37,6 @@ sub new { sub destroy { return Osstest::Toolstack::xl::destroy; } sub create { return Osstest::Toolstack::xl::create; } sub consolecmd { return Osstest::Toolstack::xl::consolecmd; } +sub shutdown_wait { return Osstest::Toolstack::xl::shutdown_wait; } 1; diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm index 4997775..ce2456b 100644 --- a/Osstest/Toolstack/xl.pm +++ b/Osstest/Toolstack/xl.pm @@ -51,4 +51,11 @@ sub consolecmd ($$) { return $self-{Command}. console $gn; } +sub shutdown_wait ($$) { +my ($self,$gho) = @_; +my $ho = $self-{Host}; +my $gn = $gho-{Name}; +target_cmd_root($ho,$self-{Command} shutdown -w $gn, 200); +} + 1; diff --git a/ts-guest-stop b/ts-guest-stop index 0e3a863..5a10755 100755 --- a/ts-guest-stop +++ b/ts-guest-stop @@ -26,10 +26,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub stop () { guest_checkrunning($ho, $gho) or die $gho-{Name} not running; -target_cmd_root($ho, - toolstack($ho)-{Command} - . shutdown -w - .$gho-{Name}, 200); +toolstack($ho)-shutdown_wait($gho); guest_checkrunning($ho, $gho) and die $gho-{Name}; } -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 02/18] ts-logs-capture: Collect some libvirt logs and capabilities
Signed-off-by: Ian Campbell ian.campb...@citrix.com --- ts-logs-capture | 4 1 file changed, 4 insertions(+) diff --git a/ts-logs-capture b/ts-logs-capture index 21974a9..6cf51c1 100755 --- a/ts-logs-capture +++ b/ts-logs-capture @@ -117,6 +117,9 @@ sub fetch_logs_host_guests () { /var/log/xen/osstest* /var/log/xen/xenstored* + /var/log/libvirt/libvirtd.log + /var/log/libvirt/libxl/* + /var/run/xenstored* /var/log/xenstored* @@ -161,6 +164,7 @@ sub fetch_logs_host_guests () { 'lspci -vvv', 'lspci -tv', 'cat /proc/partitions', + 'virsh capabilities', ) { try_cmd_output_save($cmd); } -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxc: check in xc_get_tot_pages() that the proper domain is reported
On Tue, Dec 02, 2014 at 04:18:08PM +0100, Vitaly Kuznetsov wrote: XEN_DOMCTL_getdomaininfo, which is being used by xc_domain_getinfo(), has strange interface: it reports first domain which has domid = requested domid so all callers are supposed to check that the proper domain(s) was queried by checking domid. xc_get_tot_pages() doesn't do that. In case the requested domain was destroyed it will report first domain with domid requested domid which is apparently misleading as there is no way xc_get_tot_pages() callers can figure out that they got tot_pages for some other domain. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com Acked-by: Wei Liu wei.l...@citrix.com --- tools/libxc/xc_private.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c index 1c214dd..e2441ad 100644 --- a/tools/libxc/xc_private.c +++ b/tools/libxc/xc_private.c @@ -613,8 +613,10 @@ int xc_get_pfn_list(xc_interface *xch, long xc_get_tot_pages(xc_interface *xch, uint32_t domid) { xc_dominfo_t info; -return (xc_domain_getinfo(xch, domid, 1, info) != 1) ? --1 : info.nr_pages; +if ( (xc_domain_getinfo(xch, domid, 1, info) != 1) || + (info.domid != domid) ) +return -1; +return info.nr_pages; } int xc_copy_to_domain_page(xc_interface *xch, -- 1.9.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 04/18] ts-rumpuserxen-demo-xenstorels: Use standard functions for things
Specifically guest_create and guest_find_domid. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- ts-rumpuserxen-demo-xenstorels | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ts-rumpuserxen-demo-xenstorels b/ts-rumpuserxen-demo-xenstorels index a2a6a77..6698848 100755 --- a/ts-rumpuserxen-demo-xenstorels +++ b/ts-rumpuserxen-demo-xenstorels @@ -40,11 +40,9 @@ sub arrangepreserve () { } sub start () { -my $cmd= toolstack($ho)-{Command}. create . -$r{ $gho-{Guest}.'_'. toolstack($ho)-{CfgPathVar} }; -target_cmd_root($ho, $cmd, 30); +guest_create($gho,toolstack($ho)); -$domid = target_cmd_output_root($ho,xl domid $gho-{Guest}); +$domid = guest_find_domid($ho, $gho); } sub await_end () { -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 11/18] Toolstack: Refactor migration support check.
Not implemented for libvirt (the check itself that is, the hook is present). Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/Toolstack/libvirt.pm | 5 + Osstest/Toolstack/xend.pm| 3 +++ Osstest/Toolstack/xl.pm | 9 + ts-migrate-support-check | 10 +- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm index d039c06..75e49d0 100644 --- a/Osstest/Toolstack/libvirt.pm +++ b/Osstest/Toolstack/libvirt.pm @@ -63,4 +63,9 @@ sub shutdown_wait ($$) { die libvirt shutdown wait not implemented yet. } +sub migrate_check ($) { +my ($self) = @_; +die Migration check is not yet supported on libvirt.; +} + 1; diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm index d0e1113..e69bb1f 100644 --- a/Osstest/Toolstack/xend.pm +++ b/Osstest/Toolstack/xend.pm @@ -39,4 +39,7 @@ sub create { return Osstest::Toolstack::xl::create; } sub consolecmd { return Osstest::Toolstack::xl::consolecmd; } sub shutdown_wait { return Osstest::Toolstack::xl::shutdown_wait; } +# xend always supported migration +sub migrate_check ($) { return 0; } + 1; diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm index ce2456b..3c59da3 100644 --- a/Osstest/Toolstack/xl.pm +++ b/Osstest/Toolstack/xl.pm @@ -58,4 +58,13 @@ sub shutdown_wait ($$) { target_cmd_root($ho,$self-{Command} shutdown -w $gn, 200); } +sub migrate_check ($) { +my ($self) = @_; +my $ho = $self-{Host}; +my $help = target_cmd_output_root($ho, $self-{Command}. help); +my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1; +logm(rc=$rc); +return $rc; +} + 1; diff --git a/ts-migrate-support-check b/ts-migrate-support-check index c70b77a..cd41f68 100755 --- a/ts-migrate-support-check +++ b/ts-migrate-support-check @@ -24,12 +24,4 @@ tsreadconfig(); our $ho = selecthost($ARGV[0]); -# all xend/xm platforms support migration -exit(0) if toolstack($ho)-{Command} eq xm; - -my $help = target_cmd_output_root($ho, toolstack($ho)-{Command}. help); - -my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1; - -logm(rc=$rc); -exit($rc); +exit(toolstack($ho)-migrate_check()); -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxc: check in xc_get_tot_pages() that the proper domain is reported
On 02/12/14 15:18, Vitaly Kuznetsov wrote: XEN_DOMCTL_getdomaininfo, which is being used by xc_domain_getinfo(), has strange interface: it reports first domain which has domid = requested domid so all callers are supposed to check that the proper domain(s) was queried by checking domid. xc_get_tot_pages() doesn't do that. In case the requested domain was destroyed it will report first domain with domid requested domid which is apparently misleading as there is no way xc_get_tot_pages() callers can figure out that they got tot_pages for some other domain. Signed-off-by: Vitaly Kuznetsov vkuzn...@redhat.com Reviewed-by: Andrew Cooper andrew.coop...@citrix.com CCing Konrad as this really should get a 4.5 release ack. It is a straight bugfix. --- tools/libxc/xc_private.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c index 1c214dd..e2441ad 100644 --- a/tools/libxc/xc_private.c +++ b/tools/libxc/xc_private.c @@ -613,8 +613,10 @@ int xc_get_pfn_list(xc_interface *xch, long xc_get_tot_pages(xc_interface *xch, uint32_t domid) { xc_dominfo_t info; -return (xc_domain_getinfo(xch, domid, 1, info) != 1) ? --1 : info.nr_pages; +if ( (xc_domain_getinfo(xch, domid, 1, info) != 1) || + (info.domid != domid) ) +return -1; +return info.nr_pages; } int xc_copy_to_domain_page(xc_interface *xch, ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Xen-users] 4.5 git: regression in xen systemd shutdown hangs the OS
On Tue, Dec 02, Ian Campbell wrote: On Tue, 2014-12-02 at 16:17 +0100, Olaf Hering wrote: On Tue, Dec 02, Ian Campbell wrote: On Mon, 2014-12-01 at 23:41 +, Mark Pryor wrote: list, Thanks. If you've identified a buggy changeset then it is fine to post to the devel lists. I've added a CC. I've also CCd everyone listed in the commit which you've fingered. Olaf, does this suggested change look correct? If so then can you turn it into a patch please. Yes, something like this (sed -i 's@socket@service@g' *.in): Can you submit to -devel with a commit log and S-o-b etc please. Yes, I did already. Looks like git send-email does not process a Reported-by tag so Mark did not actually got a copy of that submission. But the actual patch did not change so what I sent him earlier can be used as is. Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/Toolstack/libvirt.pm | 6 ++ Osstest/Toolstack/xend.pm| 1 + Osstest/Toolstack/xl.pm | 6 ++ ts-logs-capture | 2 +- 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm index ea83995..54d2a6d 100644 --- a/Osstest/Toolstack/libvirt.pm +++ b/Osstest/Toolstack/libvirt.pm @@ -51,4 +51,10 @@ sub create ($$) { target_cmd_root($ho, virsh create --file $cfg.xml, 100); } +sub consolecmd ($$) { +my ($self,$gho) = @_; +my $gn = $gho-{Name}; +return virsh console $gn; +} + 1; diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm index c921c20..896d949 100644 --- a/Osstest/Toolstack/xend.pm +++ b/Osstest/Toolstack/xend.pm @@ -36,5 +36,6 @@ sub new { # Defer to xl driver for most things sub destroy { return Osstest::Toolstack::xl::destroy; } sub create { return Osstest::Toolstack::xl::create; } +sub consolecmd { return Osstest::Toolstack::xl::consolecmd; } 1; diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm index 12417ca..4997775 100644 --- a/Osstest/Toolstack/xl.pm +++ b/Osstest/Toolstack/xl.pm @@ -45,4 +45,10 @@ sub create ($$) { target_cmd_root($self-{Host}, $self-{Command}. create $cfg, 100); } +sub consolecmd ($$) { +my ($self,$gho) = @_; +my $gn = $gho-{Name}; +return $self-{Command}. console $gn; +} + 1; diff --git a/ts-logs-capture b/ts-logs-capture index 841ad5a..dbca13a 100755 --- a/ts-logs-capture +++ b/ts-logs-capture @@ -195,7 +195,7 @@ sub fetch_logs_guest ($) { logm(cannot find domid: $@); return; } -my $consolecmd= toolstack($ho)-{Command}. console $gho-{Name}; +my $consolecmd= toolstack($ho)-consolecmd($gho); try_cmd_output_save(sleep 1 | $consolecmd | cat, guest-$gho-{Name}-console); -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 06/18] TestSupport: always use xl for generic operations.
Unless the toolstack is xend (for compatibility with pre-xl Xen versions), when we use xm. For several operations in TestSupport.pm the actual toolstack isn't really relevant, since we want info straight from Xen. For simplicity just use xl (or xm) in these cases, to avoid needing to implement the following specially for each toolstack: - host_get_free_memory - guest_get_state - guest_find_domid - listing assignable pci devices Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/TestSupport.pm | 13 ++--- ts-debian-fixup| 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm index df05d8a..3f63273 100644 --- a/Osstest/TestSupport.pm +++ b/Osstest/TestSupport.pm @@ -936,6 +936,13 @@ sub compress_stashed($) { #-- other stuff -- +sub common_toolstack () { +my $tsname= $r{toolstack} || 'xend'; +my $ts = 'xl'; +$ts = 'xm' if $tsname eq 'xend'; +return $ts; +} + sub host_reboot ($) { my ($ho) = @_; target_reboot($ho); @@ -956,7 +963,7 @@ END sub host_get_free_memory($) { my ($ho) = @_; -my $toolstack = toolstack($ho)-{Command}; +my $toolstack = common_toolstack(); # The line is as followed: # free_memory : my $info = target_cmd_output_root($ho, $toolstack info, 10); @@ -1610,7 +1617,7 @@ sub guest_check_up ($) { sub guest_get_state ($$) { my ($ho,$gho) = @_; -my $domains= target_cmd_output_root($ho, toolstack($ho)-{Command}. list); +my $domains= target_cmd_output_root($ho, common_toolstack(). list); $domains =~ s/^Name.*\n//; foreach my $l (split /\n/, $domains) { $l =~ m/^(\S+) (?: \s+ \d+ ){3} \s+ ([-a-z]+) \s/x or die $l ?; @@ -1815,7 +1822,7 @@ sub guest_find_domid ($$) { my ($ho,$gho) = @_; return if defined $gho-{Domid}; my $list= target_cmd_output_root($ho, -toolstack($ho)-{Command}. list $gho-{Name}); +common_toolstack(). list $gho-{Name}); $list =~ m/^(?!Name\s)(\S+)\s+(\d+)\s+(\d+)+(\d+)\s.*$/m or die domain list: $list; $1 eq $gho-{Name} or die domain list name $1 expected $gho-{Name}; diff --git a/ts-debian-fixup b/ts-debian-fixup index 11e956c..0dc2d07 100755 --- a/ts-debian-fixup +++ b/ts-debian-fixup @@ -142,7 +142,7 @@ sub otherfixupcfg () { if (@pcipt) { logm(checking passthrough device(s) are assignable: @pcipt); my @assignables= split /\n/, -target_cmd_output_root($ho, toolstack($ho)-{Command}. +target_cmd_output_root($ho, common_toolstack(). pci-assignable-list); foreach my $pcipt (@pcipt) { die not assignable: $pcipt (not in: @assignables) -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle.
Implement destory/create as per toolstack methods, including implementing the libvirt version which previously didn't work. To do this we use the virsh capability to convert an xl/xm style config file into the correct XML. xend basically calls into the xl helper since they are compatible. xl/x, uses -{Command} which will eventually become private. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/TestSupport.pm | 5 ++--- Osstest/Toolstack/libvirt.pm | 20 Osstest/Toolstack/xend.pm| 5 + Osstest/Toolstack/xl.pm | 13 + ts-guest-saverestore | 2 +- ts-guest-start | 4 +--- ts-windows-install | 7 +-- 7 files changed, 43 insertions(+), 13 deletions(-) diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm index 616f7ed..16ab4c6 100644 --- a/Osstest/TestSupport.pm +++ b/Osstest/TestSupport.pm @@ -1326,13 +1326,12 @@ sub guest_await_shutdown ($$$) { sub guest_destroy ($$) { my ($ho,$gho) = @_; -target_cmd_root($ho, toolstack($ho)-{Command}. destroy $gho-{Name}, 40); +toolstack($ho)-destroy($gho); } sub guest_create ($$) { my ($ho,$gho) = @_; -my $ts= toolstack($ho); -target_cmd_root($ho, $ts-{Command}. create $gho-{CfgPath}, 100); +toolstack($ho)-create($gho-{CfgPath}); } diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm index 90fe434..ea83995 100644 --- a/Osstest/Toolstack/libvirt.pm +++ b/Osstest/Toolstack/libvirt.pm @@ -20,15 +20,35 @@ package Osstest::Toolstack::libvirt; use strict; use warnings; +use Osstest::TestSupport; + sub new { my ($class, $ho, $methname,$asset) = @_; return bless { Name = libvirt, Host = $ho, NewDaemons = [qw(libvirtd)], Dom0MemFixed = 1, + CfgPathVar = 'cfgpath', Command = 'virsh', ExtraPackages = [qw(libnl1 libavahi-client3)], }, $class; } +sub destroy ($$) { +my ($self,$gho) = @_; +my $gn = $gho-{Name}; +target_cmd_root($self-{Host}, virsh destroy $gn, 40); +} + +sub create ($$) { +my ($self,$cfg) = @_; +my $ho = $self-{Host}; +my $lcfg = $cfg; +$lcfg =~ s,/,-,g; +$lcfg = $ho-{Name}--$lcfg; +target_cmd_root($ho, virsh domxml-from-native xen-xm $cfg $cfg.xml, 30); +target_getfile_root($ho,60,$cfg.xml, $stash/$lcfg); +target_cmd_root($ho, virsh create --file $cfg.xml, 100); +} + 1; diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm index 881417d..c921c20 100644 --- a/Osstest/Toolstack/xend.pm +++ b/Osstest/Toolstack/xend.pm @@ -19,6 +19,7 @@ package Osstest::Toolstack::xend; use strict; use warnings; +use Osstest::Toolstack::xl; sub new { my ($class, $ho, $methname,$asset) = @_; @@ -32,4 +33,8 @@ sub new { }, $class; } +# Defer to xl driver for most things +sub destroy { return Osstest::Toolstack::xl::destroy; } +sub create { return Osstest::Toolstack::xl::create; } + 1; diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm index 0b66201..12417ca 100644 --- a/Osstest/Toolstack/xl.pm +++ b/Osstest/Toolstack/xl.pm @@ -20,6 +20,8 @@ package Osstest::Toolstack::xl; use strict; use warnings; +use Osstest::TestSupport; + sub new { my ($class, $ho, $methname,$asset) = @_; return bless { Name = xl, @@ -32,4 +34,15 @@ sub new { }, $class; } +sub destroy ($$) { +my ($self,$gho) = @_; +my $gn = $gho-{Name}; +target_cmd_root($self-{Host}, $self-{Command}. destroy $gn, 40); +} + +sub create ($$) { +my ($self,$cfg) = @_; +target_cmd_root($self-{Host}, $self-{Command}. create $cfg, 100); +} + 1; diff --git a/ts-guest-saverestore b/ts-guest-saverestore index 9e04ae9..8911aed 100755 --- a/ts-guest-saverestore +++ b/ts-guest-saverestore @@ -38,7 +38,7 @@ sub restore () { toolstack($ho)-{Command} . restore .(toolstack($ho)-{RestoreNeedsConfig} ? - $r{ $gho-{Guest}.'_'. toolstack($ho)-{CfgPathVar} } : '') + $gho-{CfgPath} : '') . image, 200); target_ping_check_up($gho); } diff --git a/ts-guest-start b/ts-guest-start index bfbb734..fb6a174 100755 --- a/ts-guest-start +++ b/ts-guest-start @@ -26,9 +26,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub start () { guest_umount_lv($ho, $gho); -my $cmd= toolstack($ho)-{Command}. create . -$r{ $gho-{Guest}.'_'. toolstack($ho)-{CfgPathVar} }; -target_cmd_root($ho, $cmd, 30); +toolstack($ho)-create($r{ $gho-{Guest}.'_'. toolstack($ho)-{CfgPathVar} }); } sub checkstart () { diff --git a/ts-windows-install b/ts-windows-install index 4b06310..f12a4f4 100755 --- a/ts-windows-install +++ b/ts-windows-install @@ -49,13 +49,8 @@ END store_runvar($gho-{Guest}_pingbroken, 1); } -sub start () { -target_cmd_root($ho, toolstack($ho)-{Command}. -
Re: [Xen-devel] Poor network performance between DomU with multiqueue support
On Tue, Dec 02, 2014 at 02:46:36PM +, Zhangleiqiang (Trump) wrote: Thanks for your reply, Wei. I do the following testing just now and found the results as follows: There are three DomUs (4U4G) are running on Host A (6U6G) and one DomU (4U4G) is running on Host B (6U6G), I send packets from three DomUs to the DomU on Host B simultaneously. 1. The top output of Host B as follows: top - 09:42:11 up 1:07, 2 users, load average: 2.46, 1.90, 1.47 Tasks: 173 total, 4 running, 169 sleeping, 0 stopped, 0 zombie %Cpu0 : 0.0 us, 0.0 sy, 0.0 ni, 97.3 id, 0.0 wa, 0.0 hi, 0.8 si, 1.9 st %Cpu1 : 0.0 us, 27.0 sy, 0.0 ni, 63.1 id, 0.0 wa, 0.0 hi, 9.5 si, 0.4 st %Cpu2 : 0.0 us, 90.0 sy, 0.0 ni, 8.3 id, 0.0 wa, 0.0 hi, 1.7 si, 0.0 st %Cpu3 : 0.4 us, 1.4 sy, 0.0 ni, 95.4 id, 0.0 wa, 0.0 hi, 1.4 si, 1.4 st %Cpu4 : 0.0 us, 60.2 sy, 0.0 ni, 39.5 id, 0.0 wa, 0.0 hi, 0.3 si, 0.0 st %Cpu5 : 0.0 us, 2.8 sy, 0.0 ni, 89.4 id, 0.0 wa, 0.0 hi, 6.9 si, 0.9 st KiB Mem: 4517144 total, 3116480 used, 1400664 free, 876 buffers KiB Swap: 2103292 total,0 used, 2103292 free. 2374656 cached Mem PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ COMMAND 7440 root 20 0 0 0 0 R 71.10 0.000 8:15.38 vif4.0-q3-guest 7434 root 20 0 0 0 0 R 59.14 0.000 9:00.58 vif4.0-q0-guest 18 root 20 0 0 0 0 R 33.89 0.000 2:35.06 ksoftirqd/2 28 root 20 0 0 0 0 S 20.93 0.000 3:01.81 ksoftirqd/4 As shown above, only two netback related processes (vif4.0-*) are running with high cpu usage, and the other 2 netback processes are idle. The ps result of vif4.0-* processes as follows: root 7434 50.5 0.0 0 0 ?R09:23 11:29 [vif4.0-q0-guest] root 7435 0.0 0.0 0 0 ?S09:23 0:00 [vif4.0-q0-deall] root 7436 0.0 0.0 0 0 ?S09:23 0:00 [vif4.0-q1-guest] root 7437 0.0 0.0 0 0 ?S09:23 0:00 [vif4.0-q1-deall] root 7438 0.0 0.0 0 0 ?S09:23 0:00 [vif4.0-q2-guest] root 7439 0.0 0.0 0 0 ?S09:23 0:00 [vif4.0-q2-deall] root 7440 48.1 0.0 0 0 ?R09:23 10:55 [vif4.0-q3-guest] root 7441 0.0 0.0 0 0 ?S09:23 0:00 [vif4.0-q3-deall] root 9724 0.0 0.0 9244 1520 pts/0S+ 09:46 0:00 grep --color=auto 2. The rx related content in /proc/interupts in receiver DomU (on Host B): 73: 2 0 2925405 0 xen-dyn-event eth0-q0-rx 75: 43 93 0 118 xen-dyn-event eth0-q1-rx 77: 2 337614 1983 xen-dyn-event eth0-q2-rx 79: 2414666 0 9 0 xen-dyn-event eth0-q3-rx As shown above, it seems like that only q0 and q3 handles the interrupt triggered by packet receving. Any advise? Thanks. Netback selects queue based on the return value of skb_get_queue_mapping. The queue mapping is set by core driver or ndo_select_queue (if specified by individual driver). In this case netback doesn't have its implementation of ndo_select_queue, so it's up to core driver to decide which queue to dispatch the packet to. I think you need to inspect why Dom0 only steers traffic to these two queues but not all of them. Don't know which utility is handy for this job. Probably tc(8) is useful? Wei. -- zhangleiqiang (Trump) Best Regards -Original Message- From: Wei Liu [mailto:wei.l...@citrix.com] Sent: Tuesday, December 02, 2014 8:12 PM To: Zhangleiqiang (Trump) Cc: Wei Liu; zhangleiqiang; xen-devel@lists.xen.org; Luohao (brian); Xiaoding (B); Yuzhou (C); Zhuangyuxin Subject: Re: [Xen-devel] Poor network performance between DomU with multiqueue support On Tue, Dec 02, 2014 at 11:50:59AM +, Zhangleiqiang (Trump) wrote: -Original Message- From: xen-devel-boun...@lists.xen.org [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Wei Liu Sent: Tuesday, December 02, 2014 7:02 PM To: zhangleiqiang Cc: wei.l...@citrix.com; xen-devel@lists.xen.org Subject: Re: [Xen-devel] Poor network performance between DomU with multiqueue support On Tue, Dec 02, 2014 at 04:30:49PM +0800, zhangleiqiang wrote: Hi, all I am testing the performance of xen netfront-netback driver that with
[Xen-devel] [PATCH OSSTEST v2 17/18] ts-guest-start: Use guest_create
This allows us to abolish CfgPathVar which was inconsistently used, appears redundant with $gho-{CfgPath} and in any case never set to anything other than 'cfgpath'. I suppose it was intended to deal with toolstacks with a cfg format completely dissimilar to xm/xl's. I think if this arises in a future toolstack this functionality could be reintroduced pretty trivially via the toolstack abstraction which is added by this series. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/Toolstack/libvirt.pm | 1 - Osstest/Toolstack/xend.pm| 1 - Osstest/Toolstack/xl.pm | 1 - ts-guest-start | 2 +- 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm index e50238d..8030aeb 100644 --- a/Osstest/Toolstack/libvirt.pm +++ b/Osstest/Toolstack/libvirt.pm @@ -28,7 +28,6 @@ sub new { Host = $ho, NewDaemons = [qw(libvirtd)], Dom0MemFixed = 1, - CfgPathVar = 'cfgpath', ExtraPackages = [qw(libnl1 libavahi-client3)], }, $class; } diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm index 97c6da6..d76f650 100644 --- a/Osstest/Toolstack/xend.pm +++ b/Osstest/Toolstack/xend.pm @@ -28,7 +28,6 @@ sub new { NewDaemons = [qw(xend)], OldDaemonInitd = 'xend', _Command = 'xm', - CfgPathVar = 'cfgpath', Dom0MemFixed = 1, }, $class; } diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm index 5480f8f..cd6ba15 100644 --- a/Osstest/Toolstack/xl.pm +++ b/Osstest/Toolstack/xl.pm @@ -29,7 +29,6 @@ sub new { NewDaemons = [], Dom0MemFixed = 1, _Command = 'xl', - CfgPathVar = 'cfgpath', RestoreNeedsConfig = 1, }, $class; } diff --git a/ts-guest-start b/ts-guest-start index fb6a174..19d6830 100755 --- a/ts-guest-start +++ b/ts-guest-start @@ -26,7 +26,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub start () { guest_umount_lv($ho, $gho); -toolstack($ho)-create($r{ $gho-{Guest}.'_'. toolstack($ho)-{CfgPathVar} }); +guest_create($ho,$gho); } sub checkstart () { -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 14/18] libvirt: Implement initscript restart which has some hope of working.
Signed-off-by: Ian Campbell ian.campb...@citrix.com --- ts-libvirt-build | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ts-libvirt-build b/ts-libvirt-build index 940c034..878d4e2 100755 --- a/ts-libvirt-build +++ b/ts-libvirt-build @@ -140,8 +140,10 @@ case $1 in [ $VERBOSE != no ] log_end_msg $? ;; restart) - stop - start + [ $VERBOSE != no ] log_daemon_msg Restarting $DESC $DAEMON + start-stop-daemon --oknodo --stop --pidfile $PIDFILE --exec $DAEMON + start-stop-daemon --start --pidfile $PIDFILE --exec $DAEMON -- -d $libvirtd_opts + [ $VERBOSE != no ] log_end_msg $? ;; reload|force-reload) start-stop-daemon --stop --signal 1 --quiet --pidfile \ -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 07/18] TestSupport: guest_create takes a $ho.
And looks up the toolstack from it. This is now consistent with guest_destroy. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/TestSupport.pm | 5 +++-- ts-debian-hvm-install | 7 ++- ts-redhat-install | 7 ++- ts-rumpuserxen-demo-xenstorels | 2 +- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm index 3f63273..616f7ed 100644 --- a/Osstest/TestSupport.pm +++ b/Osstest/TestSupport.pm @@ -1330,8 +1330,9 @@ sub guest_destroy ($$) { } sub guest_create ($$) { -my ($gho,$toolstack) = @_; -target_cmd_root($gho-{Host}, $toolstack create $gho-{CfgPath}, 100); +my ($ho,$gho) = @_; +my $ts= toolstack($ho); +target_cmd_root($ho, $ts-{Command}. create $gho-{CfgPath}, 100); } diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install index 0148eef..f274630 100755 --- a/ts-debian-hvm-install +++ b/ts-debian-hvm-install @@ -41,9 +41,6 @@ our $disk_mb= 1; our $guesthost= $gn.guest.osstest; our $gho; -our $toolstack= toolstack($ho)-{Command}; - - sub preseed () { my $preseed_file = preseed_base('wheezy','',()); @@ -183,7 +180,7 @@ logm(Host has $host_freemem_mb MB free memory, setting guest memory size to $ra if (!$stage) { prep(); -guest_create($gho,$toolstack); +guest_create($ho,$gho); } else { $gho= selectguest($gn,$gho); } @@ -193,6 +190,6 @@ if ($stage2) { } guest_editconfig_nocd($gho,$emptyiso); -guest_create($gho,$toolstack); +guest_create($ho,$gho); guest_await_dhcp_tcp($gho,300); guest_check_up($gho); diff --git a/ts-redhat-install b/ts-redhat-install index a0b1fab..0b4ee17 100755 --- a/ts-redhat-install +++ b/ts-redhat-install @@ -37,9 +37,6 @@ our $disk_mb= 5; our $guesthost= $gn.guest.osstest; our $gho; -our $xl= toolstack($ho)-{Command}; - - sub kickstart () { my $cryptpw= '$6$anjRJmBbJcrNJGWN$rqvGUhu8ITjvErdIA5C//w2R6b/6wAjHbaF7XF8u3lZg1XI3StthPIE6UII08scOFwASMOepCGpgtsYeCpjqb.'; my $authkeys= authorized_keys(); @@ -152,7 +149,7 @@ sub prep () { if (!$stage) { prep(); -guest_create($gho,$xl); +guest_create($ho,$gho); } else { $gho= selectguest($gn,$gho); } @@ -162,6 +159,6 @@ if ($stage2) { } guest_editconfig_nocd($gho,$emptyiso); -guest_create($gho,$xl); +guest_create($ho,$gho); guest_await_dhcp_tcp($gho,300); guest_check_up($gho); diff --git a/ts-rumpuserxen-demo-xenstorels b/ts-rumpuserxen-demo-xenstorels index 6698848..69a2186 100755 --- a/ts-rumpuserxen-demo-xenstorels +++ b/ts-rumpuserxen-demo-xenstorels @@ -40,7 +40,7 @@ sub arrangepreserve () { } sub start () { -guest_create($gho,toolstack($ho)); +guest_create($ho, $gho); $domid = guest_find_domid($ho, $gho); } -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support.
Note that since the previous patch arranges for ts-migration-support-check to continue to fail for libvirt the libvirt code is not actually called yet (and will die if it is). This patch is mainly included to reduce the number of users of toolstack()-{Command} closer to zero. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/Toolstack/libvirt.pm | 5 + Osstest/Toolstack/xend.pm| 1 + Osstest/Toolstack/xl.pm | 9 + ts-guest-localmigrate| 5 + ts-guest-migrate | 5 + 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm index 75e49d0..7740a89 100644 --- a/Osstest/Toolstack/libvirt.pm +++ b/Osstest/Toolstack/libvirt.pm @@ -68,4 +68,9 @@ sub migrate_check ($) { die Migration check is not yet supported on libvirt.; } +sub migrate ($) { +my ($self,$gho,$dst,$to) = @_; +die Migration is not yet supported on libvirt.; +} + 1; diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm index e69bb1f..4b5482f 100644 --- a/Osstest/Toolstack/xend.pm +++ b/Osstest/Toolstack/xend.pm @@ -38,6 +38,7 @@ sub destroy { return Osstest::Toolstack::xl::destroy; } sub create { return Osstest::Toolstack::xl::create; } sub consolecmd { return Osstest::Toolstack::xl::consolecmd; } sub shutdown_wait { return Osstest::Toolstack::xl::shutdown_wait; } +sub migrate { return Osstest::Toolstack::xl::migrate; } # xend always supported migration sub migrate_check ($) { return 0; } diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm index 3c59da3..a6f04bc 100644 --- a/Osstest/Toolstack/xl.pm +++ b/Osstest/Toolstack/xl.pm @@ -67,4 +67,13 @@ sub migrate_check ($) { return $rc; } +sub migrate () { +my ($self,$gho,$dst,$to) = @_; +my $ho = $self-{Host}; +my $gn = $gho-{Name}; +target_cmd_root($ho, + $self-{Command}. migrate $gn $dst, + $to); +} + 1; diff --git a/ts-guest-localmigrate b/ts-guest-localmigrate index f3381da..caefa11 100755 --- a/ts-guest-localmigrate +++ b/ts-guest-localmigrate @@ -32,10 +32,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub migrate () { guest_checkrunning($ho,$gho) or die $gho-{Name}; -target_cmd_root($ho, - toolstack($ho)-{Command} - . migrate $gho-{Name} localhost, - $timeout{Migrate}); +toolstack($ho)-migrate($gho, localhost, $timeout{Migrate}); } guest_await_dhcp_tcp($gho, 5); diff --git a/ts-guest-migrate b/ts-guest-migrate index 65e7b42..55d2854 100755 --- a/ts-guest-migrate +++ b/ts-guest-migrate @@ -31,10 +31,7 @@ our $gho = selectguest($ARGV[2],$sho); sub migrate () { guest_checkrunning($sho,$gho) or die $gho-{Name}; my $err= guest_check_ip($gho); die $err $gho-{Name} if defined $err; -target_cmd_root($sho, - toolstack($sho)-{Command} - . migrate $gho-{Name} $dho-{Name}, - $timeout{Migrate}); +toolstack($sho)-migrate($gho, $dho-{Name}, $timeout{Migrate}); } guest_await_dhcp_tcp($gho, 5); -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] introduce grant copy for user land
This patch introduces the interface to allow user-space applications execute grant-copy operations. This is done by sending an ioctl to the grant device. Signed-off-by: Thanos Makatos thanos.maka...@citrix.com --- drivers/xen/gntdev.c | 171 + include/uapi/xen/gntdev.h | 69 ++ 2 files changed, 240 insertions(+) diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 51f4c95..7b4a8e0 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -705,6 +705,174 @@ static long gntdev_ioctl_notify(struct gntdev_priv *priv, void __user *u) return rc; } +static int gntdev_gcopy_batch(int nr_segments, unsigned long gcopy_cb, + struct gntdev_grant_copy_segment __user *__segments, int dir, + int src, int dst) { + + static const int batch_size = PAGE_SIZE / (sizeof(struct page*) + + sizeof(struct gnttab_copy) + sizeof(struct gntdev_grant_copy_segment)); + struct page **pages = (struct page **)gcopy_cb; + struct gnttab_copy *batch = (struct gnttab_copy *)((unsigned long)pages + + sizeof(struct page*) * batch_size); + struct gntdev_grant_copy_segment *segments = + (struct gntdev_grant_copy_segment *)((unsigned long)batch + + sizeof(struct gnttab_copy) * batch_size); + unsigned int nr_pinned = 0, nr_segs2cp = 0; + int err = 0, i; + const int write = dir == GNTCOPY_IOCTL_g2s; + + nr_segments = min(nr_segments, batch_size); + + if (unlikely(copy_from_user(segments, __segments, + sizeof(struct gntdev_grant_copy_segment) * nr_segments))) { + pr_debug(failed to copy %d segments from user, nr_segments); + err = -EFAULT; + goto out; + } + + for (i = 0; i nr_segments; i++) { + + xen_pfn_t pgaddr; + unsigned long start, offset; + struct gntdev_grant_copy_segment *seg = segments[i]; + + if (dir == GNTCOPY_IOCTL_s2g || dir == GNTCOPY_IOCTL_g2s) { + + start = (unsigned long)seg-self.iov.iov_base PAGE_MASK; + offset = (unsigned long)seg-self.iov.iov_base ~PAGE_MASK; + if (unlikely(offset + seg-self.iov.iov_len PAGE_SIZE)) { + pr_warn(segments crossing page boundaries not yet + implemented\n); + err = -ENOSYS; + goto out; + } + + err = get_user_pages_fast(start, 1, write, pages[i]); + if (unlikely(err != 1)) { + pr_debug(failed to get user page %lu, start); + err = -EFAULT; + goto out; + } + + nr_pinned++; + + pgaddr = pfn_to_mfn(page_to_pfn(pages[i])); + } + + nr_segs2cp++; + + switch (dir) { + case GNTCOPY_IOCTL_g2s: /* copy from guest */ + batch[i].len = seg-self.iov.iov_len; + batch[i].source.u.ref = seg-self.ref; + batch[i].source.domid = src; + batch[i].source.offset = seg-self.offset; + batch[i].dest.u.gmfn = pgaddr; + batch[i].dest.domid = DOMID_SELF; + batch[i].dest.offset = offset; + batch[i].flags = GNTCOPY_source_gref; + break; + case GNTCOPY_IOCTL_s2g: /* copy to guest */ + batch[i].len = seg-self.iov.iov_len; + batch[i].source.u.gmfn = pgaddr; + batch[i].source.domid = DOMID_SELF; + batch[i].source.offset = offset; + batch[i].dest.u.ref = seg-self.ref; + batch[i].dest.domid = dst; + batch[i].dest.offset = seg-self.offset; + batch[i].flags = GNTCOPY_dest_gref; + break; + case GNTCOPY_IOCTL_g2g: /* copy guest to guest */ + batch[i].len = seg-g2g.len; + batch[i].source.u.ref = seg-g2g.src.ref; + batch[i].source.domid = src; + batch[i].source.offset = seg-g2g.src.offset; + batch[i].dest.u.ref = seg-g2g.dst.ref; + batch[i].dest.domid = dst; + batch[i].dest.offset = seg-g2g.dst.offset; + batch[i].flags = GNTCOPY_source_gref | GNTCOPY_dest_gref; + break; + default: + pr_debug(invalid grant-copy direction %d\n,
[Xen-devel] [PATCH OSSTEST v2 16/18] Toolstack: Remove Command field for all toolstacks.
Nothing in generic code uses this now, so remove. xl+xend retain as _Command for internal use only. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/Toolstack/libvirt.pm | 1 - Osstest/Toolstack/xend.pm| 2 +- Osstest/Toolstack/xl.pm | 18 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm index c18f467..e50238d 100644 --- a/Osstest/Toolstack/libvirt.pm +++ b/Osstest/Toolstack/libvirt.pm @@ -29,7 +29,6 @@ sub new { NewDaemons = [qw(libvirtd)], Dom0MemFixed = 1, CfgPathVar = 'cfgpath', - Command = 'virsh', ExtraPackages = [qw(libnl1 libavahi-client3)], }, $class; } diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm index 6a5f9e6..97c6da6 100644 --- a/Osstest/Toolstack/xend.pm +++ b/Osstest/Toolstack/xend.pm @@ -27,7 +27,7 @@ sub new { Host = $ho, NewDaemons = [qw(xend)], OldDaemonInitd = 'xend', - Command = 'xm', + _Command = 'xm', CfgPathVar = 'cfgpath', Dom0MemFixed = 1, }, $class; diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm index adda0c7..5480f8f 100644 --- a/Osstest/Toolstack/xl.pm +++ b/Osstest/Toolstack/xl.pm @@ -28,7 +28,7 @@ sub new { Host = $ho, NewDaemons = [], Dom0MemFixed = 1, - Command = 'xl', + _Command = 'xl', CfgPathVar = 'cfgpath', RestoreNeedsConfig = 1, }, $class; @@ -37,31 +37,31 @@ sub new { sub destroy ($$) { my ($self,$gho) = @_; my $gn = $gho-{Name}; -target_cmd_root($self-{Host}, $self-{Command}. destroy $gn, 40); +target_cmd_root($self-{Host}, $self-{_Command}. destroy $gn, 40); } sub create ($$) { my ($self,$cfg) = @_; -target_cmd_root($self-{Host}, $self-{Command}. create $cfg, 100); +target_cmd_root($self-{Host}, $self-{_Command}. create $cfg, 100); } sub consolecmd ($$) { my ($self,$gho) = @_; my $gn = $gho-{Name}; -return $self-{Command}. console $gn; +return $self-{_Command}. console $gn; } sub shutdown_wait ($$) { my ($self,$gho) = @_; my $ho = $self-{Host}; my $gn = $gho-{Name}; -target_cmd_root($ho,$self-{Command} shutdown -w $gn, 200); +target_cmd_root($ho,$self-{_Command} shutdown -w $gn, 200); } sub migrate_check ($) { my ($self) = @_; my $ho = $self-{Host}; -my $help = target_cmd_output_root($ho, $self-{Command}. help); +my $help = target_cmd_output_root($ho, $self-{_Command}. help); my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1; logm(rc=$rc); return $rc; @@ -72,7 +72,7 @@ sub migrate () { my $ho = $self-{Host}; my $gn = $gho-{Name}; target_cmd_root($ho, - $self-{Command}. migrate $gn $dst, + $self-{_Command}. migrate $gn $dst, $to); } @@ -80,7 +80,7 @@ sub save () { my ($self,$gho,$f,$to) = @_; my $ho = $self-{Host}; my $gn = $gho-{Name}; -target_cmd_root($ho,$self-{Command}. save $gn $f, $to); +target_cmd_root($ho,$self-{_Command}. save $gn $f, $to); } sub restore ($) { @@ -88,7 +88,7 @@ sub restore ($) { my $ho = $self-{Host}; my $gn = $gho-{Name}; target_cmd_root($ho, - $self-{Command} + $self-{_Command} . restore .($self-{RestoreNeedsConfig} ? $cfg : '') . $f, $to); -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH OSSTEST v2 01/18] apt: lock osstest's usages of apt-get against each other
Currently we rely on all apt-get invocations being in a single ts-xen-build-prep job which can't run on a shared host. That is a bit inflexible so instead use our own lock. We wait indefinitely and rely on osstest's existing command timeout infrastructure to catch problems. target_install_packages*() previous estimated the time taken to install the packages based on the number of packages. This no longer applies because the install might get stuck behind some other large install. Use a 3000s (nearly an hour) timeout instead (I expect failures here to be unusual so erred on the big side) Signed-off-by: Ian Campbell ian.campb...@citrix.com --- Osstest/Debian.pm | 2 +- Osstest/TestSupport.pm | 7 --- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm index c8db601..b343f0f 100644 --- a/Osstest/Debian.pm +++ b/Osstest/Debian.pm @@ -492,7 +492,7 @@ d-i apt-setup/another boolean false d-i apt-setup/non-free boolean false d-i apt-setup/contrib boolean false -d-i pkgsel/include string openssh-server, ntp, ntpdate, ethtool, $extra_packages +d-i pkgsel/include string openssh-server, ntp, ntpdate, ethtool, chiark-utils-bin, $extra_packages $xopts{ExtraPreseed} diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm index e6c54bc..2ac490f 100644 --- a/Osstest/TestSupport.pm +++ b/Osstest/TestSupport.pm @@ -433,17 +433,18 @@ sub target_putfile_root (;$) { sub target_run_apt { my ($ho, $timeout, @aptopts) = @_; target_cmd_root($ho, - DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y apt-get @aptopts, + DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y \ +with-lock-ex -w /var/lock/osstest-apt apt-get @aptopts, $timeout); } sub target_install_packages { my ($ho, @packages) = @_; -target_run_apt($ho, 300 + 100 * @packages, +target_run_apt($ho, 3000, qw(-y install), @packages); } sub target_install_packages_norec { my ($ho, @packages) = @_; -target_run_apt($ho, 300 + 100 * @packages, +target_run_apt($ho, 3000, qw(--no-install-recommends -y install), @packages); } -- 2.1.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel