[Qemu-devel] VFIO: Not require to make VFIO_IOMMU_MAP_DMA for MMAPED PCI bars
Hi Alex Williamson, I am able (with some hacks :)) to directly assign the e1000 PCI device to KVM guest using VFIO on Freescale device. One of the problem I am facing is about the DMA mapping in IOMMU for PCI device BARs. On Freescale devices, the mmaped PCI device BARs are not required to be mapped in IOMMU. Typically the flow of in/out transaction (from CPU) is: Incoming flow: -| |--| |---| |-| CORE |-| IOMMU|--| PCI-Controller|---| PCI device | -| |--| |---| |-| Outgoing Flow: IOMMU is bypassed for out transactions -| |--| |---| |-| CORE |--| | IOMMU|---| PCI-Controller|---| PCI device | -| | |--| ^ |---| |-| | | |--| Also because of some hardware limitations on our IOMMU it is difficult to map these BAR regions with RAM (DDR) regions. So on Freescale device we want the VFIO_IOMMU_MAP_DMA ioctl to be called for RAM regions (DDR) only and _not_ for these mmaped ram regions of PCI device bars. I can understand that we need to add these mmaped PCI bars as RAM type in qemu memory_region_*(). So for that I tried to skip these regions in VFIO memory_listeners. Below changes which works for me. I am not sure whether this is correct approach, please suggest. - diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c index c51ae67..63728d8 100644 --- a/hw/vfio_pci.c +++ b/hw/vfio_pci.c @@ -1115,9 +1115,35 @@ static int vfio_dma_map(VFIOContainer *container, hwaddr iova, return -errno; } -static bool vfio_listener_skipped_section(MemoryRegionSection *section) +static int memory_region_is_mmap_bars(VFIOContainer *container, + MemoryRegionSection *section) { -return !memory_region_is_ram(section-mr); +VFIOGroup *group; +VFIODevice *vdev; +int i; + +QLIST_FOREACH(group, container-group_list, next) { +QLIST_FOREACH(vdev, group-device_list, next) { +if (vdev-msix-mmap_mem.ram_addr == section-mr-ram_addr) +return 1; +for (i = 0; i PCI_ROM_SLOT; i++) { +VFIOBAR *bar = vdev-bars[i]; +if (bar-mmap_mem.ram_addr == section-mr-ram_addr) +return 1; +} +} +} + +return 0; +} + +static bool vfio_listener_skipped_section(VFIOContainer *container, + MemoryRegionSection *section) +{ +if (!memory_region_is_ram(section-mr)) + return 1; + +return memory_region_is_mmap_bars(container, section); } static void vfio_listener_region_add(MemoryListener *listener, @@ -1129,7 +1155,7 @@ static void vfio_listener_region_add(MemoryListener *listener, void *vaddr; int ret; -if (vfio_listener_skipped_section(section)) { +if (vfio_listener_skipped_section(container, section)) { DPRINTF(vfio: SKIPPING region_add %HWADDR_PRIx - %PRIx64\n, section-offset_within_address_space, section-offset_within_address_space + section-size - 1); @@ -1173,7 +1199,7 @@ static void vfio_listener_region_del(MemoryListener *listener, hwaddr iova, end; int ret; -if (vfio_listener_skipped_section(section)) { +if (vfio_listener_skipped_section(container, section)) { DPRINTF(vfio: SKIPPING region_del %HWADDR_PRIx - %PRIx64\n, section-offset_within_address_space, section-offset_within_address_space + section-size - 1); - Thanks -Bharat
Re: [Qemu-devel] [PATCH 0/2] block: refuse negative iops and bps values
On Wed, Feb 13, 2013 at 04:53:41PM +0100, Stefan Hajnoczi wrote: These patches report an error if negative values are given for I/O throttling iops or bps. Patch 1 gets do_check_io_limits() into shape so that we can add checks. Patch 2 adds the negative check. Stefan Hajnoczi (2): block: use Error in do_check_io_limits() block: refuse negative iops and bps values blockdev.c | 24 ++-- 1 file changed, 18 insertions(+), 6 deletions(-) -- 1.8.1.2 Applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan
Re: [Qemu-devel] [PATCH v11 0/4] Add subcommand compare for qemu-img
On Wed, Feb 13, 2013 at 09:09:38AM +0100, Miroslav Rezanina wrote: This is 11th version of patch adding compare subcommand that compares two images. Compare has following criteria: - only data part is compared - unallocated sectors are not read - qemu-img returns: - 0 if images are identical - 1 if images differ - 2 error on execution v11: - add minor enhancements to qemu-img compare test (patch 4) v10: - rebased to kwolf's block-next branch due to conflict patch (patch 2) - exit status updates (patch 3) - minor non-functional fixes v9: - Merged patch 3 and 4 (subcommmand implementation and documentation) of v8 - Added basic test for qemu-img compare - Fixed incorrect indentation - add bdrv_is_allocated_above() return value check - Add description of exit codes into the documentation - minor non-functional changes v8: - minor grammar fixes (no behavior change) v7: - split patch into pieces - Quiet mode added for all relevant subcommands - check non-shared part of disk after shared one - minor docummentation and naming fixes v6: - added handling -?, -h options for compare subcommand v5 (only minor changes): - removed redundant comment - removed dead code (goto after help()) - set final total_sectors on first assignment v4: - Fixed various typos - Added functions for empty sector check and sector-to-bytes offset conversion - Fixed command-line parameters processing v3: - options -f/-F are orthogonal - documentation updated to new syntax and behavior - used byte offset instead of sector number for output v2: - changed option for second image format to -F - changed handling of -f and -F [1] - added strict mode (-s) - added quiet mode (-q) - improved output messages [2] - rename variables for larger image handling - added man page content Signed-off-by: Miroslav Rezanina mreza...@redhat.com Miroslav Rezanina (4): block: Add synchronous wrapper for bdrv_co_is_allocated_above qemu-img: Add Quiet mode option qemu-img: Add compare subcommand qemu-iotests: Add qemu-img compare test block.c| 51 +- blockdev.c |6 +- include/block/block.h |5 +- qemu-img-cmds.hx | 34 ++-- qemu-img.c | 441 +++- qemu-img.texi | 56 ++ tests/qemu-iotests/048 | 78 tests/qemu-iotests/048.out | 31 +++ tests/qemu-iotests/group |1 + 9 files changed, 633 insertions(+), 70 deletions(-) create mode 100755 tests/qemu-iotests/048 create mode 100644 tests/qemu-iotests/048.out Fixups applied. Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan
[Qemu-devel] [PATCH moxie 5/5] Add top level changes for moxie port
Signed-off-by: Anthony Green gr...@moxielogic.com --- hw/moxie/Makefile.objs | 5 ++ hw/moxiesim.c | 200 + include/sysemu/arch_init.h | 1 + 3 files changed, 206 insertions(+) create mode 100644 hw/moxie/Makefile.objs create mode 100644 hw/moxiesim.c diff --git a/hw/moxie/Makefile.objs b/hw/moxie/Makefile.objs new file mode 100644 index 000..2963363 --- /dev/null +++ b/hw/moxie/Makefile.objs @@ -0,0 +1,5 @@ +# moxie boards +obj-y = moxiesim.o serial.o mc146818rtc.o vga.o +obj-$(CONFIG_FDT) += device_tree.o + +obj-y := $(addprefix ../,$(obj-y)) diff --git a/hw/moxiesim.c b/hw/moxiesim.c new file mode 100644 index 000..feae538 --- /dev/null +++ b/hw/moxiesim.c @@ -0,0 +1,200 @@ +/* + * QEMU/moxiesim emulation + * + * Emulates a very simple machine model similiar to the one use by the + * GDB moxie simulator. + * + * Copyright (c) 2008, 2009, 2010 Anthony Green + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include sysbus.h +#include hw.h +#include pc.h +#include isa.h +#include net/net.h +#include sysemu/sysemu.h +#include boards.h +#include loader.h +#include serial.h +#include exec/address-spaces.h + +#define PHYS_MEM_BASE 0x8000 + +static struct _loaderparams { +int ram_size; +const char *kernel_filename; +const char *kernel_cmdline; +const char *initrd_filename; +} loaderparams; + +static void load_kernel (CPUMoxieState *env) +{ +uint64_t entry, kernel_low, kernel_high; +long kernel_size; +long initrd_size; +ram_addr_t initrd_offset; + +kernel_size = load_elf(loaderparams.kernel_filename, NULL, NULL, + entry, kernel_low, kernel_high, 1, ELF_MACHINE, 0); +if (kernel_size = 0) + env-pc = (unsigned) entry; +else + { +fprintf(stderr, qemu: could not load kernel '%s'\n, +loaderparams.kernel_filename); +exit(1); + } + +/* load initrd */ +initrd_size = 0; +initrd_offset = 0; +if (loaderparams.initrd_filename) { +initrd_size = get_image_size (loaderparams.initrd_filename); +if (initrd_size 0) { +initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) TARGET_PAGE_MASK; +if (initrd_offset + initrd_size loaderparams.ram_size) { +fprintf(stderr, +qemu: memory too small for initial ram disk '%s'\n, +loaderparams.initrd_filename); +exit(1); +} +initrd_size = load_image_targphys(loaderparams.initrd_filename, + initrd_offset, + ram_size); +} +if (initrd_size == (target_ulong) -1) { +fprintf(stderr, qemu: could not load initial ram disk '%s'\n, +loaderparams.initrd_filename); +exit(1); +} +} +} + +static void main_cpu_reset(void *opaque) +{ +MoxieCPU *cpu = opaque; + +cpu_reset(CPU(cpu)); +} + +static inline DeviceState * +moxie_intc_create(hwaddr base, qemu_irq irq, int kind_of_intr) +{ +DeviceState *dev; + +dev = qdev_create(NULL, moxie,intc); +qdev_prop_set_uint32(dev, kind-of-intr, kind_of_intr); +qdev_init_nofail(dev); +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq); +return dev; +} + +static void moxiesim_init(QEMUMachineInitArgs *args) +{ +MoxieCPU *cpu = NULL; +ram_addr_t ram_size = args-ram_size; +const char *cpu_model = args-cpu_model; +const char *kernel_filename = args-kernel_filename; +const char *kernel_cmdline = args-kernel_cmdline; +const char *initrd_filename = args-initrd_filename; +CPUMoxieState *env; +MemoryRegion *address_space_mem = get_system_memory(); +MemoryRegion *ram = g_new(MemoryRegion, 1); +MemoryRegion *rom = g_new(MemoryRegion,
Re: [Qemu-devel] [PATCH] hw/mc146818rtc.c: Fix reading and writing of time registers
Il 14/02/2013 08:54, Antoine Mathys ha scritto: This patch consolidates the bit twidling involved in reading and writing the time registers to four functions that are used consistently. This also has the effect of fixing bug 1090558. Signed-off-by: Antoine Mathys barsa...@gmail.com --- Nice. Do you have a testcase? Please also test this patch with the two rtc-test patches at http://thread.gmane.org/gmane.comp.emulators.qemu/188244. Thanks, Paolo
[Qemu-devel] [PATCH moxie 2/5] Add moxie disassembler
This patch adds the disassembler logic for moxie. From 57158c29b2956d03c5948b530fa476e26c893000 Mon Sep 17 00:00:00 2001 From: Anthony Green gr...@moxielogic.com Date: Wed, 13 Feb 2013 16:44:45 -0500 Subject: [PATCH 2/5] Add moxie disassembler Signed-off-by: Anthony Green gr...@moxielogic.com --- disas.c | 6 + disas/Makefile.objs | 1 + disas/moxie.c | 369 include/disas/bfd.h | 2 + 4 files changed, 378 insertions(+) create mode 100644 disas/moxie.c diff --git a/disas.c b/disas.c index a46faee..74d3ba0 100644 --- a/disas.c +++ b/disas.c @@ -256,6 +256,9 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code, #elif defined(TARGET_MICROBLAZE) s.info.mach = bfd_arch_microblaze; print_insn = print_insn_microblaze; +#elif defined(TARGET_MOXIE) +s.info.mach = bfd_arch_moxie; +print_insn = print_insn_moxie; #elif defined(TARGET_LM32) s.info.mach = bfd_mach_lm32; print_insn = print_insn_lm32; @@ -462,6 +465,9 @@ void monitor_disas(Monitor *mon, CPUArchState *env, #elif defined(TARGET_S390X) s.info.mach = bfd_mach_s390_64; print_insn = print_insn_s390; +#elif defined(TARGET_MOXIE) +s.info.mach = bfd_arch_moxie; +print_insn = print_insn_moxie; #elif defined(TARGET_LM32) s.info.mach = bfd_mach_lm32; print_insn = print_insn_lm32; diff --git a/disas/Makefile.objs b/disas/Makefile.objs index ed75f9a..3b1e77a 100644 --- a/disas/Makefile.objs +++ b/disas/Makefile.objs @@ -7,6 +7,7 @@ common-obj-$(CONFIG_IA64_DIS) += ia64.o common-obj-$(CONFIG_M68K_DIS) += m68k.o common-obj-$(CONFIG_MICROBLAZE_DIS) += microblaze.o common-obj-$(CONFIG_MIPS_DIS) += mips.o +common-obj-$(CONFIG_MOXIE_DIS) += moxie.o common-obj-$(CONFIG_PPC_DIS) += ppc.o common-obj-$(CONFIG_S390_DIS) += s390.o common-obj-$(CONFIG_SH4_DIS) += sh4.o diff --git a/disas/moxie.c b/disas/moxie.c new file mode 100644 index 000..20ae0eb --- /dev/null +++ b/disas/moxie.c @@ -0,0 +1,369 @@ +/* Disassemble moxie instructions. + Copyright 2009 + Free Software Foundation, Inc. + + This file is part of the GNU opcodes library. + + This library is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3, or (at your option) + any later version. + + It is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public + License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, + MA 02110-1301, USA. */ + +#include stdio.h +#define STATIC_TABLE +#define DEFINE_TABLE + +#include disas/bfd.h + +static void *stream; + +/* Form 1 instructions come in different flavors: + + Some have no arguments (MOXIE_F1_NARG) + Some only use the A operand (MOXIE_F1_A) + Some use A and B registers (MOXIE_F1_AB) + Some use A and consume a 4 byte immediate value (MOXIE_F1_A4) + Some use just a 4 byte immediate value (MOXIE_F1_4) + Some use just a 4 byte memory address (MOXIE_F1_M) + Some use B and an indirect A(MOXIE_F1_AiB) + Some use A and an indirect B(MOXIE_F1_ABi) + Some consume a 4 byte immediate value and use X (MOXIE_F1_4A) + Some use B and an indirect A plus 4 bytes (MOXIE_F1_AiB4) + Some use A and an indirect B plus 4 bytes (MOXIE_F1_ABi4) + + Form 2 instructions also come in different flavors: + + Some have no arguments (MOXIE_F2_NARG) + Some use the A register and an 8-bit value (MOXIE_F2_A8V) + + Form 3 instructions also come in different flavors: + + Some have no arguments (MOXIE_F3_NARG) + Some have a 10-bit PC relative operand (MOXIE_F3_PCREL). */ + +#define MOXIE_F1_NARG 0x100 +#define MOXIE_F1_A0x101 +#define MOXIE_F1_AB 0x102 +/* #define MOXIE_F1_ABC 0x103 */ +#define MOXIE_F1_A4 0x104 +#define MOXIE_F1_40x105 +#define MOXIE_F1_AiB 0x106 +#define MOXIE_F1_ABi 0x107 +#define MOXIE_F1_4A 0x108 +#define MOXIE_F1_AiB4 0x109 +#define MOXIE_F1_ABi4 0x10a +#define MOXIE_F1_M0x10b + +#define MOXIE_F2_NARG 0x200 +#define MOXIE_F2_A8V 0x201 + +#define MOXIE_F3_NARG 0x300 +#define MOXIE_F3_PCREL 0x301 + +typedef struct moxie_opc_info_t { + short opcode; + unsigned itype; + const char * name; +} moxie_opc_info_t; + +extern const moxie_opc_info_t moxie_form1_opc_info[64]; +extern const moxie_opc_info_t moxie_form2_opc_info[4]; +extern const moxie_opc_info_t moxie_form3_opc_info[16]; + +/* The moxie processor's 16-bit
Re: [Qemu-devel] [RFC PATCH v2 13/23] qcow2: handle_copied(): Implement non-zero host_offset
Am 13.02.2013 22:17, schrieb Blue Swirl: On Wed, Feb 13, 2013 at 1:22 PM, Kevin Wolf kw...@redhat.com wrote: Look only for clusters that start at a given physical offset. Signed-off-by: Kevin Wolf kw...@redhat.com --- block/qcow2-cluster.c | 26 ++ 1 files changed, 18 insertions(+), 8 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 5ce2c88..90fe36c 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -827,8 +827,6 @@ static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset, * the length of the area that can be written to. * * -errno: in error cases - * - * TODO Make non-zero host_offset behave like describe above */ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset, uint64_t *host_offset, uint64_t *bytes, QCowL2Meta **m) @@ -843,7 +841,6 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset, trace_qcow2_handle_copied(qemu_coroutine_self(), guest_offset, *host_offset, *bytes); -assert(*host_offset == 0); /* * Calculate the number of clusters to look for. We stop at L2 table @@ -867,6 +864,15 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset, if (qcow2_get_cluster_type(cluster_offset) == QCOW2_CLUSTER_NORMAL (cluster_offset QCOW_OFLAG_COPIED)) { +/* If a specific host_offset is required, check it */ +if (*host_offset != 0 + (cluster_offset L2E_OFFSET_MASK) != *host_offset) +{ Braces should cuddle with the previous line. Can we get rid of this rule for multiline ifs? Having the second/third/... line of the condition and the first line of the then branch with no clear separation severely hurts readability in my opinion. +*bytes = 0; +ret = 0; +goto out; +} + /* We keep all QCOW_OFLAG_COPIED clusters */ keep_clusters = count_contiguous_clusters(nb_clusters, s-cluster_size, Kevin
Re: [Qemu-devel] [PATCH v2 00/10] Cleanup bitops vs host-utils
On 14 February 2013 01:47, Richard Henderson r...@twiddle.net wrote: Version 1 merely tried to adjust bitops_flsl, here I instead eliminate it all from bitops.h, and standardizes on the routines from host-utils.h. I was hoping we'd be able to get rid of host-utils.h instead, since Utility compute operations used by translated code is now a completely irrelevant categorisation... -- PMM
Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently
[Note cc: +Laszlo, +Anthony, -qemu-trivial] Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 08 Feb 2013 20:34:20 +0100 Markus Armbruster arm...@redhat.com wrote: The real problem here is that the k, M, G suffixes, for example, are not good to be reported by QMP. So maybe we should refactor the code in a way that we separate what's done in QMP from what is done in HMP/command-line. Isn't it separated already? parse_option_size() is used when parsing key=value,... Such strings should not exist in QMP. If they do, it's a design bug. No and no. Such strings don't exist in QMP as far as can tell (see bugs below though), but parse_option_size() is theoretically present in a possible QMP call stack: qemu_opts_from_dict_1() qemu_opt_set_err() opt_set() qemu_opt_paser() parse_option_size() I can't tell if this will ever happen because qemu_opts_from_dict_1() restricts the call to qemu_opt_set_err() to certain values, but the fact that it's not clear is an indication that a better separation is necessary. Permit me two detours. One, qemu_opt_set_err() is a confusing name. I doesn't set an error. It attempts to set an option value, using the conventional Error ** parameter for error reporting. Its buddy qemu_opt_set() does the same, except it reports errors to the user and returns only success/failure. Two, qemu_opts_from_qdict() should be taken out and shot (I may say that, because I created it). I created it because I had to go from QDicts I get from QMP to internal APIs that want QemuOpts, specifically qdev_device_add(). qdev_device_add() takes QemuOpts because that's the only tool we had at the time for passing dictionaries around. Made enough sense then: command line gives us QemuOpts already, so why not just pass them on. Still made sense for the human monitor command: give it an argument just like -device's option argument, parse it the same way, done. It became awkward with QMP. Perhaps I should've switched qdev_device_add() to QDict then, so the QMP command handler can use it directly. Instead, I simply converted from QDict to QemuOpts. Now, to build hmp_device_add() on top of qmp_device_add(), we'd have to convert first from QemuOpts to QDict and then back to QemuOpts. Blech. Instead, we made do_device_add() go straight to qdev_device_add(). Same for hmp_netdev_add(): it goes straight to netdev_add(). QemuOpts is a bad fit for general interfaces, because it's really designed for accumulating command line options for later use. Features useful there get in the way, e.g. how QemuOptsList serves both as schema and as the single list of QemuOpts. Features not needed there are missing, e.g. signed and floating-point numbers. End of detours, back to your questions. I guess weird things can happen with qemu_opts_from_qdict(), at least in theory. For each (key, value) in the QDict, qemu_opts_from_qdict() converts value to string according to its qtype_code. The string then gets parsed according to key's QemuOptType. Yes, that means you can pass a QString to QEMU_OPT_SIZE option, and get the suffixes interpreted. However, we've only used qemu_opts_from_qdict() with QemuOptsLists that have empty desc[]! Specifically: qemu_netdev_opts and qemu_device_opts. Without desc, qemu_opt_parse() does nothing, and QemuOpts holds just string values. Actual parsing left to the consumer. Two consumers: qdev_device_add() and netdev_add(). netdev_add() uses QAPI wizardry to create the appropriate object from the QemuOpts. The parsing is done by visitors. Things get foggy for me from there on, but it looks like one of them, opts_type_size(), can parse size suffixes, which is inappropriate for QMP. A quick test confirms that this is indeed possible: {execute: netdev_add,arguments: {type:tap,id:net.1, sndbuf: 8k }} Sets NetdevTapOptions member sndbuf to 8192. To sum up, we go from QDict delivered by the JSON parser via QemuOpts to QAPI object, with a few cock-ups along the way. Ugh. By the way, the JSON schema reads { 'command': 'netdev_add', 'data': {'type': 'str', 'id': 'str', '*props': '**'}, 'gen': 'no' } I'll be hanged if I know what '**' means. qdev_device_add() uses a few well-known options itself, and they're all strings. The others it passes on to qdev_prop_parse(). This permits some weirdness like passing PCI device's addr property as number in QMP, even though it's nominally a string of the form SLOT[.FN]. No JSON schema, because device_add hasn't been qapified (Laszlo's working on it now). Now, I think I've found at least two bugs. The first one is the netdev_add doc in the schema, which states that we do accept key=value strings. The problem is here is that that's about the C API, on the wire we act as before (ie. accepting each key as a separate argument). The qapi-schame.json is more or less format-independent, so I'm not exactly sure what's the best way to describe commands using
Re: [Qemu-devel] [PATCH] hw/mc146818rtc.c: Fix reading and writing of time registers
On 02/14/2013 10:30 AM, Paolo Bonzini wrote: Nice. Do you have a testcase? No, but the refactoring makes the code very easy to audit. Please also test this patch with the two rtc-test patches at http://thread.gmane.org/gmane.comp.emulators.qemu/188244. I did and the tests pass.
[Qemu-devel] [PATCH moxie 1/5] New port contribution
Hello qemu maintainers, I have been maintaining a qemu port for moxie on github for a few years now, and would now like to submit it upstream. Moxie is a soft-core architecture, similar to lm32 and microblaze. The GNU toolchain has supported moxie for several years now. The qemu port is very basic, but sufficient to bring up a uClinux kernel port. Thank you, Anthony Green From aaa3802f785954ddafd696a1ed61ea40ea59db5f Mon Sep 17 00:00:00 2001 From: Anthony Green gr...@moxielogic.com Date: Wed, 13 Feb 2013 16:43:46 -0500 Subject: [PATCH 1/5] Add myself as moxie maintainer Signed-off-by: Anthony Green gr...@moxielogic.com --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 21043e4..b970159 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -91,6 +91,11 @@ M: Aurelien Jarno aurel...@aurel32.net S: Odd Fixes F: target-mips/ +Moxie +M: Anthony Green gr...@moxielogic.com +S: Maintained +F: target-moxie/ + PowerPC M: Alexander Graf ag...@suse.de L: qemu-...@nongnu.org -- 1.8.1.2
Re: [Qemu-devel] [PATCH moxie 5/5] Add top level changes for moxie port
On 13 February 2013 22:28, Anthony Green gr...@moxielogic.com wrote: Signed-off-by: Anthony Green gr...@moxielogic.com --- [snip] extern const uint32_t arch_type; -- 1.8.1.2 [green@localhost qemu]$ cat 0005-Add-top-level-changes-for-moxie.patch From 43eb752f105a3c1554234acb54df4e3954d97946 Mon Sep 17 00:00:00 2001 From: Anthony Green gr...@moxielogic.com Date: Wed, 13 Feb 2013 17:07:14 -0500 Subject: [PATCH 5/5] Add top level changes for moxie There's something weird going on with whatever you're using to create these patch emails. Can you redo your next series in the standard '0/0 cover letter + each patch mail as a followup to the cover letter' format, please? git format-patch and git send-email should be able to generate these for you. thanks -- PMM
Re: [Qemu-devel] [PATCH moxie 5/5] Add top level changes for moxie port
On Thu, Feb 14, 2013 at 1:56 PM, Peter Maydell peter.mayd...@linaro.org wrote: On 13 February 2013 22:28, Anthony Green gr...@moxielogic.com wrote: Signed-off-by: Anthony Green gr...@moxielogic.com [...] There's something weird going on with whatever you're using to create these patch emails. Can you redo your next series in the standard '0/0 cover letter + each patch mail as a followup to the cover letter' format, please? git There's also a lot of coding standard violations, can you run your patches through scripts/checkpatch.pl and fix its findings? -- Thanks. -- Max
Re: [Qemu-devel] [PATCH for-1.4 0/7] Option doc fixes for -help and qemu-doc
On Wed, Feb 13, 2013 at 07:49:36PM +0100, Markus Armbruster wrote: Hope it's not too late for -help and doc fixes. Markus Armbruster (7): help: Drop bogus help on -qtest and -qtest-log doc: Fix some option entries in qemu-doc's function index doc: Fill some option doc gaps in manual page and qemu-doc doc: Fix texinfo @table markup in qemu-options.hx help: Fix markup of heading USB options so it appears in -help doc help: A few options are under inappropriate headings, fix doc help: Collect block device stuff under its own heading qemu-options.hx | 754 +--- 1 file changed, 384 insertions(+), 370 deletions(-) -- 1.7.11.7 Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [PATCH v2] qemu-iotests: Test qcow2 image creation options
On Wed, Feb 13, 2013 at 03:05:28PM +0100, Kevin Wolf wrote: Just create lots of images and try out each of the creation options that qcow2 provides (except backing_file/fmt for now) I'm not totally happy with the behaviour of qemu-img in each of the cases, but let's be explicit and update the test when we do change things later. Signed-off-by: Kevin Wolf kw...@redhat.com --- v2: - Filter out the path for images, /home/kwolf/... shouldn't be in there - Commit 7216ae3 removed helpful error messages, updated golden output tests/qemu-iotests/049 | 123 + tests/qemu-iotests/049.out | 212 tests/qemu-iotests/group |1 + 3 files changed, 336 insertions(+), 0 deletions(-) create mode 100755 tests/qemu-iotests/049 create mode 100644 tests/qemu-iotests/049.out Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan
Re: [Qemu-devel] [PATCH] hw/mc146818rtc.c: Fix reading and writing of time registers
Il 14/02/2013 10:45, Antoine Mathys ha scritto: Nice. Do you have a testcase? No, but the refactoring makes the code very easy to audit. I asked because I tried to reproduce the launchpad bug and failed. Please also test this patch with the two rtc-test patches at http://thread.gmane.org/gmane.comp.emulators.qemu/188244. I did and the tests pass. Cool. Paolo
Re: [Qemu-devel] [PATCH][QEMU] vmxcap: Open MSR file in unbuffered mode
Am 14.02.2013 08:55, schrieb Gleb Natapov: On Wed, Feb 13, 2013 at 12:43:10PM +0100, Jan Kiszka wrote: Python may otherwise decide to to read larger chunks, applying the seek only on the software buffer. This will return results from the wrong MSRs. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Applied, thanks. Could you please fix the to to? :) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] using -net dump with tap networking
On Thu, Feb 14, 2013 at 11:51:42AM +1100, Alexey Kardashevskiy wrote: On 14/02/13 05:02, Laszlo Ersek wrote: On 02/13/13 15:48, Alexey Kardashevskiy wrote: Hi! I am running qemu as: qemu/ppc64-softmmu/qemu-system-ppc64 -m 1024 -M pseries -trace events=trace_events -netdev user,id=virtnet,hostfwd=tcp::5000-:22 -device virtio-net-pci,netdev=virtnet -nographic -vga none -enable-kvm -kernel vml36_64k -initrd 1.cpio Now I want to enable network dump. With the old -net syntax I could do that with -net dump but I cannot with the new syntax, tried many variants, none works. What would the correct syntax be for the case above? Ugh, I'm a bit confused, but if I say something stupid that should still help ignite the discussion. So, in general there are two ways to specify this: (1) -net dump,id=dump0,vlan=VLAN_ID,len=SIZE_LIMIT,file=PATHNAME (2) -netdev dump,id=dump0,len=SIZE_LIMIT,file=PATHNAME I believe the first option (legacy) should work. The second one will not work; actually I think it will trigger an assert. The generic init code in net_client_init1() [net/net.c] says: NetClientState *peer = NULL; /* Do not add to a vlan if it's a -netdev or a nic with a netdev= * parameter. */ if (!is_netdev (opts-kind != NET_CLIENT_OPTIONS_KIND_NIC || !opts-nic-has_netdev)) { peer = net_hub_add_port(u.net-has_vlan ? u.net-vlan : 0, NULL); } if (net_client_init_fun[opts-kind](opts, name, peer) 0) { So in (2) we don't add the dump netdev to any hub/vlan; however the specific code (net_init_dump(), [net/dump.c]) asserts (peer != NULL). Otherwise I think the idea would be to add the dump netdev *afterwards* to a vlan/hub, by changing its vlan property. See set_vlan() in [hw/qdev-properties-system.c]; it calls net_hub_port_find() [net/hub.c] whose task is to Find a available port on a hub; otherwise create one new port. See http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg03182.html. Hence I think you're back to (1), the legacy format. Assuming qemu doesn't barf on that option immediately, I believe you *also* have to add your -netdev user to the same hub as the dumper is on. In total you have to create both netdevs (a, b) and assign both to a common hub/vlan (c, d). Again, unfortunately the dump netdev only works with the legacy format, but that already includes the assignment to the hub (a, c). So you have to take care of creating the other netdev (-netdev user, b), and assign it through its vlan qdev property to the same hub (d), so that data can flow from it to the dump netdev. Hm... Looks like you can't do that directly on -netdev user (it seems to have no such property). virtio-net-pci does have it however. At least in a quick info qtree check: bus: main-system-bus type System dev: i440FX-pcihost, id bus: pci.0 type PCI dev: virtio-net-pci, id net0 dev-prop: vlan = null Also confirmed by qemu-system-x86_64 -device virtio-net-pci,help. So -netdev user,id=virtnet,hostfwd=tcp::5000-:22 \ -device virtio-net-pci,netdev=virtnet,vlan=2 \ -net dump,vlan=2,len=SIZE_LIMIT,file=PATHNAME Or some such... Ok. So, there is user device (interface to the world) and 2 QEMU network devices - virtio and dump, attached to the same virtual bridge within the QEMU. Now let's make it more fun. Actually I want to trace a tap config (I put it into the subject but later changed the actual example in a hope that it makes things simpler but I was wrong :-) ): qemu-impreza/ppc64-softmmu/qemu-system-ppc64 -m 1024 -M pseries -nographic -vga none -enable-kvm -kernel vml36_64k -initrd 1.cpio -netdev tap,id=tapnet,ifname=tap0,script=qemu-ifup.sh -device virtio-net-pci,netdev=tapnet,vlan=100 -net dump,vlan=100,file=./dump.lan.qemu.virtio So I have a virtual bridge but it is in the host, not in the QEMU. To the command line above QEMU says: Warning: vlan 100 is not connected to host network Warning: netdev tapnet has no peer qemu -help says -net tap accepts vlan=n but -netdev tap,vlan=100,... generates an error (Invalid parameter 'vlan'). Those options don't make sense because: 1. A -netdev does not have a peer or a vlan. It simply defines a host net device that can been peered with -device ...,netdev=id. 2. A -device ...,netdev=tapnet,vlan=100 doesn't make sense since netdev= specifies the peer and so does vlan=100. Either it can be connected directly (peered) to tapnet or it can be connected to vlan 100. You can't have both. I would do: qemu-system-ppc64 ... -net tap,ifname=tap0,script=qemu-ifup.sh -net nic,model=virtio -net dump,file=./dump.lan.qemu.virtio Sure I can run tcpdump on the host with the tap0 interface but I would like to catch trafic between virtio-net-pci and tap0 if it is possible. Is it? -nic dump kind of
Re: [Qemu-devel] Google Summer of Code 2013 ideas wiki open
On Tue, Feb 12, 2013 at 5:21 AM, Stefan Hajnoczi stefa...@gmail.com wrote: On Thu, Feb 7, 2013 at 4:19 PM, Stefan Hajnoczi stefa...@gmail.com wrote: I believe Google will announce GSoC again this year (there is no guarantee though) and I have created the wiki page so we can begin organizing project ideas that students can choose from. Google Summer of Code 2013 has just been announced! http://google-opensource.blogspot.de/2013/02/flip-bits-not-burgers-google-summer-of.html Some project ideas have already been discussed on IRC or private emails. Please go ahead and put them on the project ideas wiki page: http://wiki.qemu.org/Google_Summer_of_Code_2013 I am a senior student and wanna do some jobs about storage in Libvirt in GSOC 2013. I wonder whether Libvirt and QEMU will join GSOC 2013 together. If true, i will focus on http://wiki.qemu.org/Google_Summer_of_Code_2013 and add myself introductions to QEMU links said by Stefan Hajnoczi. Could anyone give me some suggestions? Thanks in advance. -- Thanks Harry Wei
Re: [Qemu-devel] [PATCH moxie 1/5] New processor port
Hello, Am 14.02.2013 02:06, schrieb Anthony Green: Hello qemu maintainers, I have been maintaining a qemu port for moxie on github for a few years now, and would now like to submit it upstream. Moxie is a soft-core architecture, similar to lm32 and microblaze. The GNU toolchain has supported moxie for several years now. The qemu port is very basic, but sufficient to bring up a uClinux kernel port. Thank you, Anthony Green Signed-off-by: Anthony Green gr...@moxielogic.com --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) Please adhere to our patch submission conventions: Please thread patches of a series together so that they stay together (standard git-send-email settings) and use a cover letter to describe the series and any changes between versions. http://wiki.qemu.org/Contribute/SubmitAPatch There's no reason to use [PATCH moxie n/m] if this series applies to master. For the commit messages, please leave out any personal messages such as Hello and I and roughly follow https://live.gnome.org/Git/CommitMessages using a target-moxie: Yada yada scheme where applicable. This patch 1/5 adds no code to target-moxie/ but describes it in MAINTAINERS. Please just make this change in a patch that actually adds such code. There's duplicate patches on the list. If you resend for whatever reason please use --subject-prefix=PATCH v2 etc. to distinguish (NB: moxie left out intentionally). It looks like this series is lacking changes to configure? Also a heads-up that we are in the middle of a CPU code refactoring and that some CPU-related functions will change argument type and fields their location once the 1.5 merge window opens. Regards, Andreas diff --git a/MAINTAINERS b/MAINTAINERS index 21043e4..b970159 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -91,6 +91,11 @@ M: Aurelien Jarno aurel...@aurel32.net S: Odd Fixes F: target-mips/ +Moxie +M: Anthony Green gr...@moxielogic.com +S: Maintained +F: target-moxie/ + PowerPC M: Alexander Graf ag...@suse.de L: qemu-...@nongnu.org -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH v4 0/3] AER-KVM: Error containment of VFIO devices assigned to KVM guests
Add support for error containment when a VFIO device assigned to a KVM guest encounters an error. This is for PCIe devices/drivers that support AER functionality. When the host OS is notified of an error in a device either through the firmware first approach or through an interrupt handled by the AER root port driver, the error handler registered by the vfio-pci driver gets invoked. The qemu process is signaled through an eventfd registered per VFIO device by the qemu process. In the eventfd handler, qemu decides on what action to take. In this implementation, guest is brought down to contain the error. v4: - Stop the guest instead of terminating - Remove unwanted returns from functions - Incorporate other feedback v3: - Removed PCI_AER* flags from device info ioctl. - Incorporated feedback v2: - Rebased to latest upstream stable bits - Changed the new ioctl to be part of VFIO_SET_IRQs ioctl - Added a new patch to get/put reference to a vfio device from struct device - Incorporated all other feedback. --- Vijay Mohan Pandarathil(3): [PATCH 1/3] VFIO: Wrapper to get reference to vfio_device from device [PATCH 2/3] VFIO-AER: Vfio-pci driver changes for supporting AER [PATCH 3/3] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices Kernel files changed drivers/vfio/vfio.c | 47 ++- include/linux/vfio.h | 3 +++ 2 files changed, 37 insertions(+), 13 deletions(-) drivers/vfio/pci/vfio_pci.c | 44 +- drivers/vfio/pci/vfio_pci_intrs.c | 47 + drivers/vfio/pci/vfio_pci_private.h | 1 + include/uapi/linux/vfio.h | 1 + 4 files changed, 92 insertions(+), 1 deletion(-) Qemu files changed hw/vfio_pci.c | 112 + linux-headers/linux/vfio.h | 1 + 2 files changed, 113 insertions(+)
[Qemu-devel] [PATCH v4 1/3] VFIO: Wrapper for getting reference to vfio_device from device
- Added vfio_device_get_from_dev() as wrapper to get reference to vfio_device from struct device. - Added vfio_device_data() as a wrapper to get device_data from vfio_device. Signed-off-by: Vijay Mohan Pandarathil vijaymohan.pandarat...@hp.com --- drivers/vfio/vfio.c | 47 ++- include/linux/vfio.h | 3 +++ 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 12c264d..863d1d3 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -407,12 +407,13 @@ static void vfio_device_release(struct kref *kref) } /* Device reference always implies a group reference */ -static void vfio_device_put(struct vfio_device *device) +void vfio_device_put(struct vfio_device *device) { struct vfio_group *group = device-group; kref_put_mutex(device-kref, vfio_device_release, group-device_lock); vfio_group_put(group); } +EXPORT_SYMBOL_GPL(vfio_device_put); static void vfio_device_get(struct vfio_device *device) { @@ -642,8 +643,12 @@ int vfio_add_group_dev(struct device *dev, } EXPORT_SYMBOL_GPL(vfio_add_group_dev); -/* Test whether a struct device is present in our tracking */ -static bool vfio_dev_present(struct device *dev) +/** + * This does a get on the vfio_device from device. + * Callers of this function will have to call vfio_put_device() to + * remove the reference. + */ +struct vfio_device *vfio_device_get_from_dev(struct device *dev) { struct iommu_group *iommu_group; struct vfio_group *group; @@ -651,25 +656,41 @@ static bool vfio_dev_present(struct device *dev) iommu_group = iommu_group_get(dev); if (!iommu_group) - return false; + return NULL; group = vfio_group_get_from_iommu(iommu_group); if (!group) { iommu_group_put(iommu_group); - return false; + return NULL; } device = vfio_group_get_device(group, dev); - if (!device) { - vfio_group_put(group); - iommu_group_put(iommu_group); - return false; - } - - vfio_device_put(device); vfio_group_put(group); iommu_group_put(iommu_group); - return true; + return device; +} +EXPORT_SYMBOL_GPL(vfio_device_get_from_dev); + +/* + * Caller must hold a reference to the vfio_device + */ +void *vfio_device_data(struct vfio_device *device) +{ + return device-device_data; +} +EXPORT_SYMBOL_GPL(vfio_device_data); + +/* Test whether a struct device is present in our tracking */ +static bool vfio_dev_present(struct device *dev) +{ + struct vfio_device *device; + + device = vfio_device_get_from_dev(dev); + if (device) { + vfio_device_put(device); + return true; + } else + return false; } /* diff --git a/include/linux/vfio.h b/include/linux/vfio.h index ab9e862..ac8d488 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -45,6 +45,9 @@ extern int vfio_add_group_dev(struct device *dev, void *device_data); extern void *vfio_del_group_dev(struct device *dev); +extern struct vfio_device *vfio_device_get_from_dev(struct device *dev); +extern void vfio_device_put(struct vfio_device *device); +extern void *vfio_device_data(struct vfio_device *device); /** * struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks -- 1.7.11.3
Re: [Qemu-devel] [RFC PATCH v2 00/23] qcow2: Delayed COW
On Wed, Feb 13, 2013 at 02:21:50PM +0100, Kevin Wolf wrote: iozone results with and without this series show significant difference for allocating writes: random random KB reclen write rewritereadrereadread write base65536 817271945125461253924491836 patch 65536 819341949122631252124631796 base 1048576 256 22344 38437 105823 106135 37743 32167 patch 1048576 256 35989 38542 105231 105994 38301 33036 I imagine the benefit is even greater for 4 KB or 8 KB blocksize? Stefan
Re: [Qemu-devel] [RFC PATCH v2 00/23] qcow2: Delayed COW
Am 14.02.2013 11:43, schrieb Stefan Hajnoczi: On Wed, Feb 13, 2013 at 02:21:50PM +0100, Kevin Wolf wrote: iozone results with and without this series show significant difference for allocating writes: random random KB reclen write rewritereadrereadread write base65536 817271945125461253924491836 patch 65536 819341949122631252124631796 base 1048576 256 22344 38437 105823 106135 37743 32167 patch 1048576 256 35989 38542 105231 105994 38301 33036 I imagine the benefit is even greater for 4 KB or 8 KB blocksize? The upper two lines are 8k block size. I suspect that the benefit would be the greatest for misaligned 64k writes, then you have one overlapping allocation for each issued request. Kevin
[Qemu-devel] [PATCH v4 2/3] VFIO-AER: Vfio-pci driver changes for supporting AER
- New VFIO_SET_IRQ ioctl option to pass the eventfd that is signaled when an error occurs in the vfio_pci_device - Register pci_error_handler for the vfio_pci driver - When the device encounters an error, the error handler registered by the vfio_pci driver gets invoked by the AER infrastructure - In the error handler, signal the eventfd registered for the device. - This results in the qemu eventfd handler getting invoked and appropriate action taken for the guest. Signed-off-by: Vijay Mohan Pandarathil vijaymohan.pandarat...@hp.com --- drivers/vfio/pci/vfio_pci.c | 44 +- drivers/vfio/pci/vfio_pci_intrs.c | 47 + drivers/vfio/pci/vfio_pci_private.h | 1 + include/uapi/linux/vfio.h | 1 + 4 files changed, 92 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index b28e66c..d70bd58 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -196,7 +196,9 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type) return (flags PCI_MSIX_FLAGS_QSIZE) + 1; } - } + } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX) + if (pci_is_pcie(vdev-pdev)) + return 1; return 0; } @@ -302,6 +304,17 @@ static long vfio_pci_ioctl(void *device_data, if (info.argsz minsz || info.index = VFIO_PCI_NUM_IRQS) return -EINVAL; + switch (info.index) { + case VFIO_PCI_INTX_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX: + break; + case VFIO_PCI_ERR_IRQ_INDEX: + if (pci_is_pcie(vdev-pdev)) + break; + /* pass thru to return error */ + default: + return -EINVAL; + } + info.flags = VFIO_IRQ_INFO_EVENTFD; info.count = vfio_pci_get_irq_count(vdev, info.index); @@ -538,11 +551,40 @@ static void vfio_pci_remove(struct pci_dev *pdev) kfree(vdev); } +static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev, + pci_channel_state_t state) +{ + struct vfio_pci_device *vpdev; + struct vfio_device *vdev; + + vdev = vfio_device_get_from_dev(pdev-dev); + if (vdev == NULL) + return PCI_ERS_RESULT_DISCONNECT; + + vpdev = vfio_device_data(vdev); + if (vpdev == NULL) { + vfio_device_put(vdev); + return PCI_ERS_RESULT_DISCONNECT; + } + + if (vpdev-err_trigger) + eventfd_signal(vpdev-err_trigger, 1); + + vfio_device_put(vdev); + + return PCI_ERS_RESULT_CAN_RECOVER; +} + +static struct pci_error_handlers vfio_err_handlers = { + .error_detected = vfio_pci_aer_err_detected, +}; + static struct pci_driver vfio_pci_driver = { .name = vfio-pci, .id_table = NULL, /* only dynamic ids */ .probe = vfio_pci_probe, .remove = vfio_pci_remove, + .err_handler= vfio_err_handlers, }; static void __exit vfio_pci_cleanup(void) diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 3639371..8036f3a 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -745,6 +745,46 @@ static int vfio_pci_set_msi_trigger(struct vfio_pci_device *vdev, return 0; } +static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev, + unsigned index, unsigned start, + unsigned count, uint32_t flags, void *data) +{ + int32_t fd = *(int32_t *)data; + + if ((index != VFIO_PCI_ERR_IRQ_INDEX) || + !(flags VFIO_IRQ_SET_DATA_TYPE_MASK)) + return -EINVAL; + + /* DATA_NONE/DATA_BOOL enables loopback testing */ + + if (flags VFIO_IRQ_SET_DATA_NONE) { + if (vdev-err_trigger) + eventfd_signal(vdev-err_trigger, 1); + return 0; + } else if (flags VFIO_IRQ_SET_DATA_BOOL) { + uint8_t trigger = *(uint8_t *)data; + if (trigger vdev-err_trigger) + eventfd_signal(vdev-err_trigger, 1); + return 0; + } + + /* Handle SET_DATA_EVENTFD */ + + if (fd == -1) { + if (vdev-err_trigger) + eventfd_ctx_put(vdev-err_trigger); + vdev-err_trigger = NULL; + return 0; + } else if (fd = 0) { + struct eventfd_ctx *efdctx; + efdctx = eventfd_ctx_fdget(fd); + if (IS_ERR(efdctx)) + return PTR_ERR(efdctx); +
[Qemu-devel] [PATCH v4 3/3] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices
- Create eventfd per vfio device assigned to a guest and register an event handler - This fd is passed to the vfio_pci driver through the SET_IRQ ioctl - When the device encounters an error, the eventfd is signalled and the qemu eventfd handler gets invoked. - In the handler decide what action to take. Current action taken is to stop the guest. Signed-off-by: Vijay Mohan Pandarathil vijaymohan.pandarat...@hp.com --- hw/vfio_pci.c | 112 + linux-headers/linux/vfio.h | 1 + 2 files changed, 113 insertions(+) diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c index c51ae67..05da53b 100644 --- a/hw/vfio_pci.c +++ b/hw/vfio_pci.c @@ -38,6 +38,7 @@ #include qemu/error-report.h #include qemu/queue.h #include qemu/range.h +#include sysemu/sysemu.h /* #define DEBUG_VFIO */ #ifdef DEBUG_VFIO @@ -129,7 +130,9 @@ typedef struct VFIODevice { PCIHostDeviceAddress host; QLIST_ENTRY(VFIODevice) next; struct VFIOGroup *group; +EventNotifier err_notifier; bool reset_works; +bool pci_aer; } VFIODevice; typedef struct VFIOGroup { @@ -1802,6 +1805,7 @@ static int vfio_get_device(VFIOGroup *group, const char *name, VFIODevice *vdev) { struct vfio_device_info dev_info = { .argsz = sizeof(dev_info) }; struct vfio_region_info reg_info = { .argsz = sizeof(reg_info) }; +struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) }; int ret, i; ret = ioctl(group-fd, VFIO_GROUP_GET_DEVICE_FD, name); @@ -1901,6 +1905,15 @@ static int vfio_get_device(VFIOGroup *group, const char *name, VFIODevice *vdev) vdev-config_size = reg_info.size; vdev-config_offset = reg_info.offset; +irq_info.index = VFIO_PCI_ERR_IRQ_INDEX; + +ret = ioctl(vdev-fd, VFIO_DEVICE_GET_IRQ_INFO, irq_info); +if (ret) { +error_report(vfio: Error getting IRQ info: %m\n); +goto error; +} +if (irq_info.count == 1) +vdev-pci_aer = true; error: if (ret) { QLIST_REMOVE(vdev, next); @@ -1922,6 +1935,102 @@ static void vfio_put_device(VFIODevice *vdev) } } +static void vfio_err_notifier_handler(void *opaque) +{ +VFIODevice *vdev = opaque; + +if (!event_notifier_test_and_clear(vdev-err_notifier)) { +return; +} + +/* + * TBD. Retrieve the error details and decide what action + * needs to be taken. One of the actions could be to pass + * the error to the guest and have the guest driver recover + * from the error. This requires that PCIe capabilities be + * exposed to the guest. For now, we just terminate the + * guest to contain the error. + */ + +error_report(%s (%04x:%02x:%02x.%x) +Unrecoverable error detected...\n +Please inestigate/collect any data required and then kill the quest, +__func__, vdev-host.domain, vdev-host.bus, +vdev-host.slot, vdev-host.function); + +vm_stop(RUN_STATE_IO_ERROR); +} + +static void vfio_register_err_notifier(VFIODevice *vdev) +{ +int ret; +int argsz; +struct vfio_irq_set *irq_set; +int32_t *pfd; + +if (!vdev-pci_aer) { +return; +} + +if (event_notifier_init(vdev-err_notifier, 0)) { +error_report(vfio: Warning: Unable to init event notifier for error detection\n); +return; +} + +argsz = sizeof(*irq_set) + sizeof(*pfd); + +irq_set = g_malloc0(argsz); +irq_set-argsz = argsz; +irq_set-flags = VFIO_IRQ_SET_DATA_EVENTFD | + VFIO_IRQ_SET_ACTION_TRIGGER; +irq_set-index = VFIO_PCI_ERR_IRQ_INDEX; +irq_set-start = 0; +irq_set-count = 1; +pfd = (int32_t *)irq_set-data; + +*pfd = event_notifier_get_fd(vdev-err_notifier); +qemu_set_fd_handler(*pfd, vfio_err_notifier_handler, NULL, vdev); + +ret = ioctl(vdev-fd, VFIO_DEVICE_SET_IRQS, irq_set); +if (ret) { +error_report(vfio: Failed to set up error notification\n); +qemu_set_fd_handler(*pfd, NULL, NULL, vdev); +event_notifier_cleanup(vdev-err_notifier); +} +g_free(irq_set); +} +static void vfio_unregister_err_notifier(VFIODevice *vdev) +{ +int argsz; +struct vfio_irq_set *irq_set; +int32_t *pfd; +int ret; + +if (!vdev-pci_aer) { +return; +} + +argsz = sizeof(*irq_set) + sizeof(*pfd); + +irq_set = g_malloc0(argsz); +irq_set-argsz = argsz; +irq_set-flags = VFIO_IRQ_SET_DATA_EVENTFD | + VFIO_IRQ_SET_ACTION_TRIGGER; +irq_set-index = VFIO_PCI_ERR_IRQ_INDEX; +irq_set-start = 0; +irq_set-count = 1; +pfd = (int32_t *)irq_set-data; +*pfd = -1; + +ret = ioctl(vdev-fd, VFIO_DEVICE_SET_IRQS, irq_set); +if (ret) { +error_report(vfio: Failed to de-assign error fd: %d\n, ret); +} +g_free(irq_set); +qemu_set_fd_handler(event_notifier_get_fd(vdev-err_notifier), +NULL, NULL, vdev); +
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
On 14 December 2012 22:53, Antoine Mathys barsa...@gmail.com wrote: The proper mapping between 24 hours and 12 hours modes is: 0 12 AM 1-111-11 AM 12 12 PM 13-23 1-11 PM Fix code accordingly. Signed-off-by: Antoine Mathys barsa...@gmail.com Sorry, I missed this patch earlier. Reviewed-by: Peter Maydell peter.mayd...@linaro.org (and I've had confirmation from somebody that this change makes us match the hardware behaviour where AM/PM is toggled when we go from 11:59 to 12:00). I tweaked the commit message a little (added hw/ds1338: prefix to the summary and removed the hardcoded tabs) and have queued it in arm-devs.next. thanks -- PMM
Re: [Qemu-devel] Testing Linux user-mode with LTP
On 8 February 2013 12:32, Peter Maydell peter.mayd...@linaro.org wrote: Hi; I spent yesterday working out how to run the Linux Test Project's syscall test suite under QEMU's linux-user mode, and I wrote up the instructions here for other peoples' benefit: http://wiki.qemu.org/Testing/LTP Currently ARM manages 86 failures out of 959 total tests run. If anybody would like to have a go at automating doing these test runs and producing pretty webpages of progressions and regressions that would be cool :-) Thanks to Fathi Boudra, Linaro is now running armhf LTP tests automatically nightly based on qemu upstream master: no pretty webpages or notification of progression/regression, but the raw pass/fail test output reports appear here: http://snapshots.linaro.org/components/toolchain/qemu-ltp (git commit hash used for the test run is shown at the very bottom of the log). -- PMM
Re: [Qemu-devel] [PATCH moxie 3/5] moxie target code
Am 14.02.2013 02:23, schrieb Anthony Green: This patch adds all of the target-moxie code to the new moxie port. Signed-off-by: Anthony Green gr...@moxielogic.com --- target-moxie/Makefile.objs |2 + target-moxie/cpu-qom.h | 70 +++ target-moxie/cpu.c | 84 +++ target-moxie/cpu.h | 181 +++ target-moxie/exec.h| 28 + target-moxie/helper.c | 136 + target-moxie/helper.h |6 + target-moxie/machine.c | 15 + target-moxie/mmu.c | 41 ++ target-moxie/mmu.h | 20 + target-moxie/op_helper.c | 81 +++ target-moxie/translate.c | 1222 12 files changed, 1886 insertions(+) create mode 100644 target-moxie/Makefile.objs create mode 100644 target-moxie/cpu-qom.h create mode 100644 target-moxie/cpu.c create mode 100644 target-moxie/cpu.h create mode 100644 target-moxie/exec.h create mode 100644 target-moxie/helper.c create mode 100644 target-moxie/helper.h create mode 100644 target-moxie/machine.c create mode 100644 target-moxie/mmu.c create mode 100644 target-moxie/mmu.h create mode 100644 target-moxie/op_helper.c create mode 100644 target-moxie/translate.c Some general comments: For a new target, please do things the new way, then you won't need a cpu-qom.h file, cf. target-openrisc/cpu.h. Please take a look at the patches queued on qom-cpu-next for the next merge window: https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next You should rebase on that (then using [PATCH qom-cpu-next n/m] or [PATCH qom-cpu n/m]) and change cpu_moxie_init() accordingly so that it sets realized = true and doesn't do CPU reset, vCPU init or TCG initialization from there and move the remaining cpu_moxie_init() to your cpu.c. Please drop the #if 0'ed bogus cpu_reset() implementation in translate.c. Please see my previous messages on the topic of using CPUMoxieState vs. MoxieCPU, with a preference for MoxieCPU except for TCG helpers: http://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg05885.html Your gen_intermediate_code_internal() for instance would be a candidate for MoxieCPU, and it should use bool type (and true/false) for search_pc argument, same for any other yes/no logic. You don't implement migration support (no CPU_SAVE_VERSION in cpu.h at least), so please set dc-vmsd to a VMStateDescription marked as unmigratable, as done for virtually all such targets for 1.4. Please drop machine.c if you don't #define CPU_SAVE_VERSION, this bug was recently fixed: http://git.qemu.org/?p=qemu.git;a=commit;h=3ce8b2bcbff6445f84db53ef38dbc4e5dd102676 Are you sure register struct asm(AREG0) in exec.h is needed??? Blue had reworked targets to not rely on a fixed asm register incompatible with clang/LLVM, I thought. Please run checkpatch.pl and drop any spaces before parenthesis for instance and use a consistent 4-char indentation. Please use inline functions and macros to avoid hardcoded printf()s, e.g., in cpu_moxie_handle_mmu_fault(). Old code uses DPRINTF() macros, as a heads-up a newer proposal was made recently for tmp105.c file: http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg01573.html The general idea is to keep format strings from bitrotting when #ifdef'ed out, details may change in response to review feedback. I also recommend against #ifdef'ing blocks of code that touch on TCG- or tree-wide fields that may get refactored, breaking such debug code (e.g., DEBUG_DISAS using env). If the corresponding log level is not set, it won't execute anyway. You can use unlikely(qemu_loglevel_mask(...)) to optimize the common case. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available
On Wed, Feb 13, 2013 at 05:17:44PM +0100, Igor Mammedov wrote: qemu boots from disk image 3 times faster than direct kernel load. That's surprising. Do you have any idea why that happens? (CCing qemu-devel in case other QEMU developers can explain it) Use grub2-mkrescue tool to create boot image with test kernel if available and do image boot then. On FC17 it reduces 1 VM run from ~15 sec to ~5sec. Signed-off-by: Igor Mammedov imamm...@redhat.com --- qemu/tests/cpuid.py| 11 +-- shared/deps/cpuid_test_kernel/Makefile | 18 -- shared/deps/cpuid_test_kernel/grub.cfg |8 3 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 shared/deps/cpuid_test_kernel/grub.cfg diff --git a/qemu/tests/cpuid.py b/qemu/tests/cpuid.py index 5065c6a..731411c 100644 --- a/qemu/tests/cpuid.py +++ b/qemu/tests/cpuid.py @@ -118,13 +118,20 @@ def run_cpuid(test, params, env): cpuid_test_kernel) os.chdir(test_kernel_dir) utils.make(cpuid_dump_kernel.bin) +utils.make(boot.img, ignore_status=True) +cmdres = utils.run(cd %s ls boot.img % test_kernel_dir, ignore_status=True) Why fork a shell just to check if a file exists? You can simply use os.path.exists('%s/boot.img' % (test_kernel_dir)) or os.path.exists(os.path.join(test_kernel_dir, 'boot.img')). The rest of the patch looks good to me. I wonder if we could make this a generic boot kernel image function, that uses grub2-mkrescue if available, and -kernel otherwise. I believe other test cases may benefit for a general mechanism to boot test kernels. vm_name = params.get('main_vm') params_b = params.copy() -params_b[kernel] = os.path.join(test_kernel_dir, cpuid_dump_kernel.bin) +if cmdres.exit_status == 0: +params_b[image_name_custom] = os.path.join(test_kernel_dir, boot.img) +params_b[image_format_custom] = +params_b[images] = custom +else: +params_b[kernel] = os.path.join(test_kernel_dir, cpuid_dump_kernel.bin) +del params_b[images] params_b[cpu_model] = cpu_model params_b[cpu_model_flags] = feature -del params_b[images] del params_b[nics] env_process.preprocess_vm(self, params_b, env, vm_name) vm = env.get_vm(vm_name) diff --git a/shared/deps/cpuid_test_kernel/Makefile b/shared/deps/cpuid_test_kernel/Makefile index 0e34c43..5999804 100644 --- a/shared/deps/cpuid_test_kernel/Makefile +++ b/shared/deps/cpuid_test_kernel/Makefile @@ -1,10 +1,24 @@ CFLAGS := -m32 -fno-stack-protector -fno-builtin -nostdinc -O -g -Wall -I. LDFLAGS := -nostdlib -Wl,-N -Wl,-Ttext -Wl,10 -all: cpuid_dump_kernel.bin +boot := $(shell sh -c 'which grub2-mkrescue 2/dev/null echo grub2boot.img') + +all: cpuid_dump_kernel.bin boot.img cpuid_dump_kernel.bin: boot.S main.c test.c $(CC) -o $@ $(CFLAGS) $^ $(LDFLAGS) + ln -s $@ kernel.bin + +boot.img: $(boot) + + +grub2boot.img: + mkdir -p iso/boot/grub + cp -f grub.cfg iso/boot/grub + cp -f kernel.bin iso/ + grub2-mkrescue -o boot.img iso clean: - rm -f *.o *.bin + rm -rf *.o *.bin iso *.img + +.PHONY: grub2boot.img diff --git a/shared/deps/cpuid_test_kernel/grub.cfg b/shared/deps/cpuid_test_kernel/grub.cfg new file mode 100644 index 000..aeb5b2b --- /dev/null +++ b/shared/deps/cpuid_test_kernel/grub.cfg @@ -0,0 +1,8 @@ +set timeout=0 +set default=0 + +menuentry test { + set root=(hd0) + multiboot /kernel.bin + boot +} -- 1.7.1 -- Eduardo
Re: [Qemu-devel] [RFC v5] target-i386: Slim conversion to X86CPU subclasses + KVM subclasses
On Wed, Feb 13, 2013 at 04:33:33PM +0100, Igor Mammedov wrote: From: Andreas Färber afaer...@suse.de Depends on http://lists.gnu.org/archive/html/qemu-devel/2013-02/msg00677.html Move x86_def_t definition to header and embed into X86CPUClass. Register types per built-in model definition. Move version initialization from x86_cpudef_setup() to class_init(). Move default setting of CPUID_EXT_HYPERVISOR to class_init(). Move KVM specific built-in CPU defaults overrides in a kvm specific x86_cpu_kvm_def_class_init(). And select TCG vs KVM class of CPU to create at runtime in x86_cpu_class_by_name() when kvm_enable() is available. Inline cpu_x86_register() into the X86CPU initfn. Since instance_init cannot reports errors, die there if some of default values are incorrect, instead of ignoring errors. Replace cpu_x86_find_by_name() with x86_cpu_class_by_name(). Move handling of KVM host vendor override from cpu_x86_find_by_name() to the kvm_arch_init() and class_init(). Use TYPE_X86_CPU class to communicate kvm specific defaults to other sub-classes. Register host-kvm-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs and only when KVM is enabled to avoid workarounds in name to class lookup code in x86_cpu_class_by_name(). Make kvm_cpu_fill_host() into a host specific class_init and inline cpu_x86_fill_model_id(). Let kvm_check_features_against_host() obtain host-kvm-{i386,86_64}-cpu for comparison. Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Igor Mammedov imamm...@redhat.com --- v5: * remove special case for 'host' CPU check in x86_cpu_class_by_name(), due to 'host' CPU will not find anything if not in KVM mode or return 'host' CPU class in KVM mode, i.e. treat it as regular CPUs. * register KVM specific subclasses for built-in CPU models. * abort() in instance_init() if property setter fails to set default value. v4: * set error if cpu model is not found and goto out; * copy vendor override from 'host' CPU class in sub-class'es class_init() if 'host' CPU class is available. * register type TYPE_HOST_X86_CPU in kvm_arch_init(), this type should be available only in KVM mode and we haven't printed it in -cpu ? output so far, so we can continue doing so. It's not really confusing to show 'host' cpu (even if we do it) when KVM is not enabled. --- target-i386/cpu-qom.h | 24 target-i386/cpu.c | 348 +++-- target-i386/cpu.h |5 +- target-i386/kvm.c | 72 ++ 4 files changed, 232 insertions(+), 217 deletions(-) [...] diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index 48e6b54..c8f320d 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -30,6 +30,27 @@ #define TYPE_X86_CPU i386-cpu #endif +#define TYPE_HOST_X86_CPU host-kvm- TYPE_X86_CPU + [...] if (name == NULL) { -return -1; -} -if (kvm_enabled() strcmp(name, host) == 0) { -kvm_cpu_fill_host(x86_cpu_def); -return 0; +return NULL; } -for (i = 0; i ARRAY_SIZE(builtin_x86_defs); i++) { -def = builtin_x86_defs[i]; -if (strcmp(name, def-name) == 0) { -memcpy(x86_cpu_def, def, sizeof(*def)); -/* sysenter isn't supported in compatibility mode on AMD, - * syscall isn't supported in compatibility mode on Intel. - * Normally we advertise the actual CPU vendor, but you can - * override this using the 'vendor' property if you want to use - * KVM's sysenter/syscall emulation in compatibility mode and - * when doing cross vendor migration - */ -if (kvm_enabled()) { -uint32_t ebx = 0, ecx = 0, edx = 0; -host_cpuid(0, 0, NULL, ebx, ecx, edx); -x86_cpu_vendor_words2str(x86_cpu_def-vendor, ebx, edx, ecx); -} -return 0; -} +if (kvm_enabled()) { +typename = g_strdup_printf(%s-kvm- TYPE_X86_CPU, name); * Could we use the -tcg suffix for the non-KVM mode? * Can we make the code fall back to no-suffix CPU names? We need to add the -kvm suffix (and maybe a -tcg suffix) to keep compatibility with existing CPU models, but maybe we should deprecate the automatic -tcg/-kvm suffixing and ask users to specify the full CPU name. [...] @@ -2234,7 +2075,57 @@ static void x86_cpu_initfn(Object *obj) cpu_set_debug_excp_handler(breakpoint_handler); #endif } + +if (error) { +fprintf(stderr, %s\n, error_get_pretty(error)); +error_free(error); +abort(); +} Good idea, we should have done that a long time ago, to avoid surprises if one day the property setting fails. + +} + +static void x86_cpu_def_class_init(ObjectClass *oc, void *data) +{ +X86CPUClass *xcc = X86_CPU_CLASS(oc);
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
Am 14.02.2013 12:32, schrieb Peter Maydell: On 14 February 2013 11:29, Andreas Färber afaer...@suse.de wrote: Am 14.02.2013 12:11, schrieb Peter Maydell: On 14 December 2012 22:53, Antoine Mathys barsa...@gmail.com wrote: The proper mapping between 24 hours and 12 hours modes is: [snip] (and I've had confirmation from somebody that this change makes us match the hardware behaviour where AM/PM is toggled when we go from 11:59 to 12:00). I may be repeating myself here, but adding qtests as requested would've caught this. Er, only if the tests were testing for the right thing, which seems unlikely since you can't run a qtest against real hardware. What seems more likely is that we would have written a qtest which checked for the same wrong behaviour we incorrectly put into the code, which doesn't help anybody. Still my point is we should stop accepting RTC bugfixes without test cases or we'll never get things testable... Paolo and me can't write the qtests for everyone, that doesn't scale. In particular in this case the same author is touching on the other RTC as well and refusing to supply a test case for the pre-existing rtc-test, which is not nice. And yes, it's always possible a test case is wrong, then it notices a change in behavior by starting to fail and can be updated along with the change. Remember, maintainers are supposed to run `make check`. ;) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
On 14 February 2013 11:44, Andreas Färber afaer...@suse.de wrote: Am 14.02.2013 12:32, schrieb Peter Maydell: On 14 February 2013 11:29, Andreas Färber afaer...@suse.de wrote: I may be repeating myself here, but adding qtests as requested would've caught this. Er, only if the tests were testing for the right thing, which seems unlikely since you can't run a qtest against real hardware. What seems more likely is that we would have written a qtest which checked for the same wrong behaviour we incorrectly put into the code, which doesn't help anybody. Still my point is we should stop accepting RTC bugfixes without test cases or we'll never get things testable... Paolo and me can't write the qtests for everyone, that doesn't scale. I'm not saying test cases are bad, I'm just saying I think you're wrong when you claim test cases would have made us notice the need for this bugfix. -- PMM
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
On 14 February 2013 11:29, Andreas Färber afaer...@suse.de wrote: Am 14.02.2013 12:11, schrieb Peter Maydell: On 14 December 2012 22:53, Antoine Mathys barsa...@gmail.com wrote: The proper mapping between 24 hours and 12 hours modes is: [snip] (and I've had confirmation from somebody that this change makes us match the hardware behaviour where AM/PM is toggled when we go from 11:59 to 12:00). I may be repeating myself here, but adding qtests as requested would've caught this. Er, only if the tests were testing for the right thing, which seems unlikely since you can't run a qtest against real hardware. What seems more likely is that we would have written a qtest which checked for the same wrong behaviour we incorrectly put into the code, which doesn't help anybody. -- PMM
Re: [Qemu-devel] uhci: cancel delay for unregistered queues
Hi, On 02/12/2013 11:01 PM, Hans de Goede wrote: Hi, On 02/12/2013 05:46 PM, Gerd Hoffmann wrote: On 02/12/13 15:38, Jan Kiszka wrote: Hi, was just debugging a memory corruption of my USB driver inside QEMU - and so far only there: I have a queue registered with the UHCI controller on an input endpoint that continuously generates data. At some point my driver decides to stop reading and removes the QH (with a lot of TDs attached) from the schedule. The driver waits for the next frame, then releases the QH and its TDs. QEMU apparently takes a few more frames to consider this queue dead. In the meantime, it seems to happily fill the TD buffers with data. But those buffers are long returned to the guest pool of free memory, causing corruptions there. Try setting QH_VALID to 1. That should fix it, but has a high chance to break iso transfers. I guess we'll need different QH_VALID values depending on transfer type. Hans? Agree?n Er, this is tricky. The biggest problem here is that Linux unlinks and then *relinks* bulk-transfers on UHCI in some cases, and we should not cancel those, so as much I would like to set QH_VALID to 1, that is not really an option, as it *will* break Linux guests. OTOH guests may indeed assume that we stop processing a queue after 1 frame. The only fix I can think of is allocating a receive buffer inside the UHCI code for read transfers, and copy the result over to the guest memory when we are re-scanning the schedule, encounter the completed td and are going to signal its completion to the guest. Since we then do the write to guest mem from the schedule scan, that means we won't do it if the qh got unlinked, but we did not cancel the queue yet, avoiding the memory corruption. The extra memcpy this introduces should not be a big deal given the low speed of USB-1, The memory allocations / frees may have some measurable performance impact, but I think we can live with that. I could take a shot at writing a patch with the proposed fix, but I was sort of waiting on a reaction on the proposal first. Gerd what do you think of my proposed fix ? Regards, Hans
[Qemu-devel] [Bug 1082292] Re: [ARM] pflash_write: Unimplemented flash cmd sequence
As of commit 5928023c these pflash_cfi01 messages are now suppressed, so this bug will be fixed in the next release of QEMU (1.4.0) and qemu- linaro (2012.03). ** Changed in: qemu-linaro Status: New = Fix Committed ** Changed in: qemu Status: New = Fix Committed ** Changed in: qemu-linaro Milestone: None = 2013.03 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1082292 Title: [ARM] pflash_write: Unimplemented flash cmd sequence Status in QEMU: Fix Committed Status in Linaro QEMU: Fix Committed Bug description: I find the bug already reported on qemu-devel mailing-list [1] w/o replies also on other sources. Tested on Darwin (Mac OS X 10.7.5) and Scientific Linux 6.3 hosts. Also using the latest 1.2.X and 1.3.0-rc0 and qemu-linaro. On all host operating systems and with different versions on qemu results are the same. I was following the official Ubuntu ARM instructions [2] how to test OS on ARM emulator. oss: Could not initialize DAC oss: Failed to open `/dev/dsp' oss: Reason: No such file or directory oss: Could not initialize DAC oss: Failed to open `/dev/dsp' oss: Reason: No such file or directory audio: Failed to create voice `lm4549.out' Uncompressing Linux. done, booting the kernel. pflash_write: Unimplemented flash cmd sequence (offset , wcycle 0x0 cmd 0x0 value 0xf000f0) pflash_write: Unimplemented flash cmd sequence (offset , wcycle 0x0 cmd 0x0 value 0xf0) And it freezes, but still eats 40-50% of CPU core. ## Instructions ## wget -c http://w3.impa.br/~gabrield/data/ubuntu-arm-development-rootfs.tar.bz2 tar jxfv ubuntu-arm-development-rootfs.tar.bz2 chmod +x run.sh ./run.sh The user to login is ubuntu and password temppwd. More details on [2]. - - - [1] http://lists.gnu.org/archive/html/qemu-devel/2012-11/msg00863.html [2] https://wiki.ubuntu.com/ARM/RootfsFromScratch To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1082292/+subscriptions
Re: [Qemu-devel] [PATCH][QEMU] vmxcap: Open MSR file in unbuffered mode
On Thu, Feb 14, 2013 at 11:25:05AM +0100, Andreas Färber wrote: Am 14.02.2013 08:55, schrieb Gleb Natapov: On Wed, Feb 13, 2013 at 12:43:10PM +0100, Jan Kiszka wrote: Python may otherwise decide to to read larger chunks, applying the seek only on the software buffer. This will return results from the wrong MSRs. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Applied, thanks. Could you please fix the to to? :) Too too late :( Pushed already. -- Gleb.
Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently
On Thu, 14 Feb 2013 10:45:22 +0100 Markus Armbruster arm...@redhat.com wrote: [Note cc: +Laszlo, +Anthony, -qemu-trivial] Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 08 Feb 2013 20:34:20 +0100 Markus Armbruster arm...@redhat.com wrote: The real problem here is that the k, M, G suffixes, for example, are not good to be reported by QMP. So maybe we should refactor the code in a way that we separate what's done in QMP from what is done in HMP/command-line. Isn't it separated already? parse_option_size() is used when parsing key=value,... Such strings should not exist in QMP. If they do, it's a design bug. No and no. Such strings don't exist in QMP as far as can tell (see bugs below though), but parse_option_size() is theoretically present in a possible QMP call stack: qemu_opts_from_dict_1() qemu_opt_set_err() opt_set() qemu_opt_paser() parse_option_size() I can't tell if this will ever happen because qemu_opts_from_dict_1() restricts the call to qemu_opt_set_err() to certain values, but the fact that it's not clear is an indication that a better separation is necessary. Permit me two detours. One, qemu_opt_set_err() is a confusing name. I doesn't set an error. It takes an Error ** object and it was introduced to avoid having to convert qemu_opt_set() to take an Error ** object, as this would multiply the work required to get netdev_add converted to the qapi. Now, I pass the bikeshed :) Now, to build hmp_device_add() on top of qmp_device_add(), we'd have to convert first from QemuOpts to QDict and then back to QemuOpts. Blech. Instead, we made do_device_add() go straight to qdev_device_add(). Same for hmp_netdev_add(): it goes straight to netdev_add(). Yes, the idea was to have netdev_add() and add frontends to hmp and qmp. More on this below. netdev_add() uses QAPI wizardry to create the appropriate object from the QemuOpts. The parsing is done by visitors. Things get foggy for me from there on, but it looks like one of them, opts_type_size(), can parse size suffixes, which is inappropriate for QMP. A quick test confirms that this is indeed possible: {execute: netdev_add,arguments: {type:tap,id:net.1, sndbuf: 8k }} Sets NetdevTapOptions member sndbuf to 8192. Well, I've just tested this with commit 783e9b4, which is before netdev_add conversion to the qapi, and the command above just works. Didn't check if sndbuf is actually set to 8192, but this shows that we've always accepted such a command. To sum up, we go from QDict delivered by the JSON parser via QemuOpts to QAPI object, with a few cock-ups along the way. Ugh. By the way, the JSON schema reads { 'command': 'netdev_add', 'data': {'type': 'str', 'id': 'str', '*props': '**'}, 'gen': 'no' } I'll be hanged if I know what '**' means. For QMP on the wire it means that the command accepts a bunch of arguments not specified in the schema. Yes, it's a dirty trick. Long story short: we decide to maintain QemuOpts usage in both HMP and QMP to speed up netdev_add conversion. qdev_device_add() uses a few well-known options itself, and they're all strings. The others it passes on to qdev_prop_parse(). This permits some weirdness like passing PCI device's addr property as number in QMP, even though it's nominally a string of the form SLOT[.FN]. No JSON schema, because device_add hasn't been qapified (Laszlo's working on it now). Now, I think I've found at least two bugs. The first one is the netdev_add doc in the schema, which states that we do accept key=value strings. The problem is here is that that's about the C API, on the wire we act as before (ie. accepting each key as a separate argument). The qapi-schame.json is more or less format-independent, so I'm not exactly sure what's the best way to describe commands using QemuOpts the way QMP uses it. Netdev could be done without this key=value business in the schema. We have just a few backends, and they're well-known, so we could just collect them all in a union, like Gerd did for chardev backends. I honestly don't know if this is a good idea, but should be possible to change it if you're willing to. Devices are hairier. For a union approach, we'd have to include a schema for every device model. Dunno. IMHO, right now going fast is more important than doing things with perfection (unless you can do perfection in no time, of course), because the qapi conversions already took a lot more than expected and it's delaying very good stuff (like the new qmp server, and dropping the *.hx files and old QMP code). So, I wouldn't bother doing old crap commands perfect. Specially when replacements are on the way. The second bug is that I entirely ignored how set_option_paramater() handles errors when doing parse_option_size() conversion to Error ** and also when converting bdrv_img_create(). The end result
Re: [Qemu-devel] [PATCH moxie 4/5] Add sample moxie system
Am 13.02.2013 23:27, schrieb Anthony Green: Add a simple moxie target, similar to what we have in the gdb simulator today. Signed-off-by: Anthony Green gr...@moxielogic.com --- hw/moxie/Makefile.objs | 5 ++ hw/moxiesim.c | 200 + include/sysemu/arch_init.h | 1 + 3 files changed, 206 insertions(+) create mode 100644 hw/moxie/Makefile.objs create mode 100644 hw/moxiesim.c Patch has Coding Style issues and is line-wrapped, i.e. broken. Please place your moxie-specific board in hw/moxie/. In Makefile.objs just place obj-y = moxiesim.o below the $(addprefix ...) line. Also respect C99 by not using underscore at the beginning of identifiers (e.g., struct _loaderparams - struct LoaderParams). Are you aware that your loaderparams truncate ram_size from ram_addr_t to int? If you don't want to use ram_addr_t there you might want to use [u]int64_t. Again some overuses of CPUMoxieState, please fix. Please make your QEMUMachine static, it is not needed elsewhere. Please drop semicolon after machine_init() macro, it's not a statement. Please add a comma after QEMU_ARCH_MOXIE enum entry so that further architectures can more easily be added. Are pic_info and irq_info really needed? If so, they would be candidates to place in stubs/ directory instead. There's #if 0'ed code. If there's a question about that, please label as RFC rather than PATCH. Otherwise please drop. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
Am 14.02.2013 12:11, schrieb Peter Maydell: On 14 December 2012 22:53, Antoine Mathys barsa...@gmail.com wrote: The proper mapping between 24 hours and 12 hours modes is: 0 12 AM 1-111-11 AM 12 12 PM 13-23 1-11 PM Fix code accordingly. Signed-off-by: Antoine Mathys barsa...@gmail.com Sorry, I missed this patch earlier. Reviewed-by: Peter Maydell peter.mayd...@linaro.org (and I've had confirmation from somebody that this change makes us match the hardware behaviour where AM/PM is toggled when we go from 11:59 to 12:00). I tweaked the commit message a little (added hw/ds1338: prefix to the summary and removed the hardcoded tabs) and have queued it in arm-devs.next. I may be repeating myself here, but adding qtests as requested would've caught this. And they should even more be added to avoid regressions. There's even templates to copy for RTC, and the new Big Endian-safe {read,write}[bwlq]() functions for 1.5 are on the list and got a Reviewed-by from Anthony in the unpolished version, so only implementation details might still change. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available
Il 14/02/2013 12:18, Eduardo Habkost ha scritto: qemu boots from disk image 3 times faster than direct kernel load. That's surprising. Do you have any idea why that happens? Because kernel load uses MMIO (from fw_cfg), while booting from disk uses at worst PCI DMA and at best virtio. Paolo
[Qemu-devel] [Bug 1094950] Re: crash at qemu_iohandler_poll (iohandler.c:124) on macos 10.8.2
Just a note that IME trying to debug QEMU under gdb on MacOS doesn't work very well. In particular as far as I can tell gdb breaks sigwait() such that the sigwait() in sigwait_compat() can return 0 without setting the int* sig. This causes QEMU to write an uninitialized value into the qemu_signalfd_siginfo struct it sends down the pipe, and then sigfd_handler() calls sigaction() with this bogus data as the signal number. Since sigfd_handler() doesn't check the return value from sigaction() we then proceed to leap off into nowhere. sigfd_handler() should probably be checking the return value from sigaction() but the underlying problem is MacOS and/or its gdb breaking sigwait() behaviour somehow. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1094950 Title: crash at qemu_iohandler_poll (iohandler.c:124) on macos 10.8.2 Status in QEMU: New Bug description: I'm seeing consistent hangs / crashes on MacOS 10.8.2 with 1.3.0. I've tried both gcc-4.2 and clang. I've tried a half a dozen different images/kernels. I configured qemu like this: ./configure --disable-sdl --disable-kvm --enable-cocoa --cc=gcc-4.2 --host-cc=gcc-4.2 --enable-debug --extra-cflags=-g --extra- ldflags=-g And ran it like this: qemu-system-arm -nographic -M versatilepb -kernel vmlinuz-2.6.32-5-versatile -initrd initrd.img-2.6.32-5-versatile -hda debian_squeeze_armel_standard.qcow2 -append root=/dev/sda1 console=ttyAMA0 With images, kernel, and initrd described here: http://psellos.com/2012/08/2012.08.qemu-arm-osx.html And I get: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x00010142f2d0 0x00010142f2d0 in ?? () (gdb) bt #0 0x00010142f2d0 in ?? () #1 0x00010016e209 in qemu_iohandler_poll (readfds=0x10097ca00, writefds=0x10097ca80, xfds=0x10097cb00, ret=4) at iohandler.c:124 #2 0x000100172acf in main_loop_wait (nonblocking=0) at main-loop.c:418 #3 0x000100207bbf in main_loop () at vl.c:1765 #4 0x00010020e7b0 in qemu_main (argc=12, argv=0x7fff5fbff360, envp=0x7fff5fbff3c8) at vl.c:3992 #5 0x0001001d6013 in main (argc=12, argv=0x7fff5fbff360) at ui/cocoa.m:884 (gdb) frame 1 #1 0x00010016e209 in qemu_iohandler_poll (readfds=0x10097ca00, writefds=0x10097ca80, xfds=0x10097cb00, ret=4) at iohandler.c:124 124 ioh-fd_read(ioh-opaque); Current language: auto; currently c (gdb) p ioh $1 = (IOHandlerRecord *) 0x10142f110 (gdb) p *ioh $2 = { fd_read_poll = 0, fd_read = 0x10017212b sigfd_handler, fd_write = 0, opaque = 0x3, next = { le_next = 0x0, le_prev = 0x105d00bc0 }, fd = 3, deleted = false } To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1094950/+subscriptions
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
Il 14/02/2013 12:44, Andreas Färber ha scritto: What seems more likely is that we would have written a qtest which checked for the same wrong behaviour we incorrectly put into the code, which doesn't help anybody. Hmm, that's not how I write tests... You write them as black boxes, and if things don't match what you expect, you either replicate the qtest on real hardware or check the datasheet. Paolo
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
Il 14/02/2013 12:29, Andreas Färber ha scritto: I may be repeating myself here, but adding qtests as requested would've caught this. And they should even more be added to avoid regressions. There's even templates to copy for RTC, and the new Big Endian-safe {read,write}[bwlq]() functions for 1.5 are on the list and got a Reviewed-by from Anthony in the unpolished version, so only implementation details might still change. When this patch was posted, the i2c libqos was not committed yet. Also, the rules we gave ourselves at QEMU Summit (BTW where are the minutes? still on etherpad only?) are that qtests will be needed only for new devices. So it's left to the good will of the maintainers to add new qtests for old devices. Paolo
[Qemu-devel] [Bug 1079080] Re: ARM instruction srs wrong behaviour
It looks like this is only a problem in Thumb mode; the equivalent bug in ARM mode was fixed in commit c67b6b71 back in 2009. Can you make the test case dtb and image available? That would help in testing... ** Changed in: qemu Status: New = Confirmed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1079080 Title: ARM instruction srs wrong behaviour Status in QEMU: Confirmed Bug description: Quote from ARM Architecture Reference Manual ARMv7-A and ARMv7-R : Store Return State stores the LR and SPSR of the current mode to the stack of a specified mode Problem: When executing this instruction, the register stored is CPSR instead of SPSR. Context: Using QEMU 1.2.0 to simulate a Zynq application (processor Cortex-a9 mpcore) with the following command line: qemu-system-arm -M xilinx-zynq-a9 -m 512 -serial null -serial mon:stdio -dtb /home/vcesson/workspace/xilinx_zynq.dtb -kernel install/tests/io/serial/current/tests/serial2 -S -s -nographic To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1079080/+subscriptions
Re: [Qemu-devel] uhci: cancel delay for unregistered queues
Hi, The only fix I can think of is allocating a receive buffer inside the UHCI code for read transfers, and copy the result over to the guest memory when we are re-scanning the schedule, encounter the completed td and are going to signal its completion to the guest. I could take a shot at writing a patch with the proposed fix, but I was sort of waiting on a reaction on the proposal first. Gerd what do you think of my proposed fix ? That should fix it indeed. Yes, for uhci we can live with the overhead, it's low bandwidth anyway. No, I don't have a better idea. Patch is very welcome. Luckily ehci + xhci have a better hardware design so the issue doesn't exist there in the first place. ehci has the IAAD handshake for deactivating queue heads, which we are using to make sure any requests are canceled before the guest releases the buffers. xhci is completely different anyway (and has control requests for deactivating endpoints which also cancel all inflight transfers). cheers, Gerd
[Qemu-devel] [Bug 1056668] Re: qemu-system-arm crash at startup with -usbdevice
I believe this bug was fixed on master by commit c128d6a6 in November, and that fix made it into QEMU 1.3.0, so I'm closing this bug as fix released. ** Changed in: qemu Status: New = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1056668 Title: qemu-system-arm crash at startup with -usbdevice Status in QEMU: Fix Released Bug description: This happens with official 1.2.0 version, but I reproduce the same issue with the main git branch On my host: :~$ lsusb | grep Logi Bus 004 Device 002: ID 046d:c219 Logitech, Inc. Cordless RumblePad 2 :~$ qemu/arm-softmmu/qemu-system-arm -kernel output/images/uImage -initrd output/images/rootfs.cpio -M vexpress-a9 -serial stdio -append console=ttyAMA0 -net nic -net tap,script=$SCRIPTS/qemu-ifup,downscript=$SCRIPTS/qemu-ifdown -m 1024 -usbdevice host:046d:c219 Segmentation fault To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1056668/+subscriptions
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
Am 14.02.2013 13:17, schrieb Paolo Bonzini: Il 14/02/2013 12:44, Andreas Färber ha scritto: What seems more likely is that we would have written a qtest which checked for the same wrong behaviour we incorrectly put into the code, which doesn't help anybody. Hmm, that's not how I write tests... You write them as black boxes, and if things don't match what you expect, you either replicate the qtest on real hardware or check the datasheet. We seem in violent agreement once again. :) My point was, if there's an error in the device, then either the test case had a thinko or there's no test case covering that particular code path (which I was just lobbying against). You're saying, if one writes one's test cases right, then there's no bugs in those code paths. True. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC PATCH 02/10] Fix errors and warnings while compiling with c++ compilier
On Thu, 14 Feb 2013 15:10:36 +0900 Tomoki Sekiyama tomoki.sekiyama...@hitachi.com wrote: Rename 'class' member in class_info of PciDeviceInfo to 'dev_class', and add some casts to avoid errors from c++ compiler. [...] # # @class_info.desc: #optional a string description of the device's class # -# @class_info.class: the class code of the device +# @class_info.dev_class: the class code of the device # # @id.device: the PCI device id # @@ -1171,7 +1171,7 @@ ## { 'type': 'PciDeviceInfo', 'data': {'bus': 'int', 'slot': 'int', 'function': 'int', - 'class_info': {'*desc': 'str', 'class': 'int'}, + 'class_info': {'*desc': 'str', 'dev_class': 'int'}, 'id': {'device': 'int', 'vendor': 'int'}, '*irq': 'int', 'qdev_id': 'str', '*pci_bridge': 'PciBridgeInfo', 'regions': ['PciMemoryRegion']} } The right way of doing this is to add 'class' to the set of reserved words in scripts/qapi.py:c_var(). Then you'll have to adapt the code to use the 'q_' prefix. Now, is using C++ required? Why can't you use plain C?
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
On 14 February 2013 12:17, Paolo Bonzini pbonz...@redhat.com wrote: Il 14/02/2013 12:44, Andreas Färber ha scritto: What seems more likely is that we would have written a qtest which checked for the same wrong behaviour we incorrectly put into the code, which doesn't help anybody. Hmm, that's not how I write tests... You write them as black boxes, and if things don't match what you expect, you either replicate the qtest on real hardware or check the datasheet. In this case the data sheet was ambiguous. The only way to avoid the bug would be to examine actual hardware behaviour, or to have different people write the test and the code. If the patch author had written the test case they'd just have resolved the ambiguity in the same (wrong) way in both code and test. -- PMM
[Qemu-devel] [Bug 1028260] Re: ARM: stellaris lm3s6965evb machine model broken
Fix went out in QEMU 1.3.0. ** Changed in: qemu Status: Fix Committed = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1028260 Title: ARM: stellaris lm3s6965evb machine model broken Status in QEMU: Fix Released Bug description: The Stellaris lm3s6965evb Machine model is broken: qemu-system-arm -M lm3s6965evb -kernel qs_ek-lm3s6965.bin GNU gdb (GDB) 7.1-ubuntu Copyright (C) 2010 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as x86_64-linux-gnu. For bug reporting instructions, please see: http://www.gnu.org/software/gdb/bugs/... Reading symbols from /home/peterc/Petalogix/Internal/plgx_install/qemu-upstream-regression/petalinux-vx.x/tools/linux-i386/petalogix/bin/qemu-system-arm...done. (gdb) r Starting program: /home/peterc/Petalogix/Internal/plgx_install/qemu-upstream-regression/petalinux-vx.x/tools/linux-i386/petalogix/bin/qemu-system-arm -M lm3s6965evb -kernel qs_ek-lm3s6965.bin [Thread debugging using libthread_db enabled] warning: no loadable sections found in added symbol-file in-memory [New Thread 0x763fc700 (LWP 15156)] qemu-system-arm: /home/peterc/Petalogix/Internal/plgx_src/qemu/hw/qdev.c:309: qdev_get_gpio_in: Assertion `n = 0 n dev-num_gpio_in' failed. Program received signal SIGABRT, Aborted. 0x006f4a2b in raise (sig=value optimised out) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:42 42../nptl/sysdeps/unix/sysv/linux/pt-raise.c: No such file or directory. in ../nptl/sysdeps/unix/sysv/linux/pt-raise.c (gdb) bt #0 0x006f4a2b in raise (sig=value optimised out) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:42 #1 0x007089a0 in abort () #2 0x00702c15 in __assert_fail () #3 0x00463ffa in qdev_get_gpio_in () #4 0x00588f94 in armv7m_init () #5 0x0060c62a in stellaris_init () #6 0x0060cc4e in lm3s6965evb_init () #7 0x004fdb11 in main () (gdb) Bisection points at this commit: commit 1e8cae4dfea2bcc91d3820dcf4f9284e7b0abb28 Author: Peter Maydell peter.mayd...@linaro.org Date: Wed May 2 16:49:42 2012 + hw/armv7m_nvic: Make the NVIC a freestanding class Rearrange the GIC and NVIC so both are straightforward subclasses of a common class, rather than having the NVIC source file textually include arm_gic.c. Signed-off-by: Peter Maydell peter.mayd...@linaro.org To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1028260/+subscriptions
[Qemu-devel] [Bug 929638] Re: qemu 1.0 unable to compile on the pandaboard ES
Closing as fixed, since the original build failure problems were fixed some time ago, and the most recent quoted error message (garbage in a system header file) strongly indicates either corrupt filesystem or hardware issues with the build machine. ** Changed in: qemu Status: Incomplete = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/929638 Title: qemu 1.0 unable to compile on the pandaboard ES Status in QEMU: Fix Released Bug description: root@omap:/home/mario/Scrivania/dati/os/qemu# uname -a Linux omap 3.1.6-x6 #1 SMP Thu Dec 22 11:17:51 UTC 2011 armv7l armv7l armv7l GNU/Linux It's running Ubuntu 11.10... root@omap:/home/mario/Scrivania/dati/os/qemu# ./configure --disable-kvm --enable-tcg-interpreter --enable-curses --enable-sdl --enable-vnc --enable-debug-tcg --enable-vhost-net Install prefix/usr/local BIOS directory/usr/local/share/qemu binary directory /usr/local/bin library directory /usr/local/lib include directory /usr/local/include config directory /usr/local/etc Manual directory /usr/local/share/man ELF interp prefix /usr/gnemul/qemu-%M Source path /home/mario/Scrivania/dati/os/qemu C compilergcc Host C compiler gcc CFLAGS-O2 -g QEMU_CFLAGS -Werror -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fstack-protector-all -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -I/usr/include/libpng12 LDFLAGS -Wl,--warn-common -g make make install install pythonpython smbd /usr/sbin/smbd host CPU arm host big endian no target list i386-softmmu x86_64-softmmu alpha-softmmu arm-softmmu cris-softmmu lm32-softmmu m68k-softmmu microblaze-softmmu microblazeel-softmmu mips-softmmu mipsel-softmmu mips64-softmmu mips64el-softmmu ppc-softmmu ppcemb-softmmu ppc64-softmmu sh4-softmmu sh4eb-softmmu sparc-softmmu sparc64-softmmu s390x-softmmu xtensa-softmmu xtensaeb-softmmu i386-linux-user x86_64-linux-user alpha-linux-user arm-linux-user armeb-linux-user cris-linux-user m68k-linux-user microblaze-linux-user microblazeel-linux-user mips-linux-user mipsel-linux-user ppc-linux-user ppc64-linux-user ppc64abi32-linux-user sh4-linux-user sh4eb-linux-user sparc-linux-user sparc64-linux-user sparc32plus-linux-user unicore32-linux-user s390x-linux-user tcg debug enabled yes Mon debug enabled no gprof enabled no sparse enabledno strip binariesyes profiler no static build no -Werror enabled yes SDL support yes curses supportyes curl support no mingw32 support no Audio drivers oss Extra audio cards ac97 es1370 sb16 hda Block whitelist Mixer emulation no VNC support yes VNC TLS support no VNC SASL support no VNC JPEG support no VNC PNG support yes VNC threadno xen support no brlapi supportno bluez supportno Documentation yes NPTL support yes GUEST_BASEyes PIE no vde support no Linux AIO support no ATTR/XATTR support yes Install blobs yes KVM support no TCG interpreter yes fdt support no preadv supportyes fdatasync yes madvise yes posix_madvise yes uuid support no libcap-ng support no vhost-net support yes Trace backend nop Trace output file trace-pid spice support no rbd support no xfsctl supportno nss used no usb net redir no OpenGL supportyes libiscsi support no build guest agent yes root@omap:/home/mario/Scrivania/dati/os/qemu# make GEN i386-softmmu/config-devices.mak GEN x86_64-softmmu/config-devices.mak GEN alpha-softmmu/config-devices.mak GEN arm-softmmu/config-devices.mak GEN cris-softmmu/config-devices.mak GEN lm32-softmmu/config-devices.mak GEN m68k-softmmu/config-devices.mak GEN microblaze-softmmu/config-devices.mak GEN microblazeel-softmmu/config-devices.mak GEN mips-softmmu/config-devices.mak CCppc-softmmu/op_helper.o CCppc-softmmu/helper.o /home/mario/Scrivania/dati/os/qemu/target-ppc/helper.c: In function ‘booke206_tlb_to_page_size’: /home/mario/Scrivania/dati/os/qemu/target-ppc/helper.c:1296:14: error: variable ‘tlbncfg’ set but not used [-Werror=unused-but-set-variable] cc1: all warnings being treated as errors make[1]: *** [helper.o] Errore 1 make: *** [subdir-ppc-softmmu] Errore 2 To manage notifications about this
[Qemu-devel] [Bug 1015226] Re: arm realview pbx hung
I'm closing this bug as invalid since it has sat with insufficient information to reproduce for more than six months with no further info or followup from the original bug submitter. If you'd like to provide the info I asked for, please feel free to reopen the bug and do so. ** Changed in: qemu Status: Incomplete = Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1015226 Title: arm realview pbx hung Status in QEMU: Invalid Bug description: host(pc):ubuntu 10.1 qemu:qemu-1.1.0-1 target:realview-pbx-a9 kernel:2.6.34.10 gcc:4.4.6 command line, and output as follow: - majun@majun-Lenovo-Product:~/qemu$ qemu-system-arm -M realview-pbx-a9 -kernel ~/emb_linux/trunk/bsp/versatile_qemu/kernel/linux-2.6.34.10/arch/arm/boot/zImage -initrd rootfs.ext2 -nographic -append console=ttyAMA0 earlyprintk Uncompressing Linux... done, booting the kernel. Linux version 2.6.34.10 (majun@majun-Lenovo-Product) (gcc version 4.4.6 (crosstool-NG 1.13.2 - hsan-5115) ) #40 SMP Tue Jun 19 18:53:13 BST 2012 CPU: ARMv7 Processor [410fc090] revision 0 (ARMv7), cr=10c53c7f CPU: VIPT nonaliasing data cache, VIPT nonaliasing instruction cache Machine: ARM-RealView PBX Ignoring unrecognised tag 0x bootconsole [earlycon0] enabled Memory policy: ECC disabled, Data cache writealloc Ignoring RAM at 8000-8fff (vmalloc region overlap). realview_pbx_map_io:117 realview_pbx_map_io:119 hung here QEMU: Terminated - modify code to add some debug info - arch/arm/mach-realview/realview_pbx.c line 115: static void __init realview_pbx_map_io(void) { printk(%s:%d\r\n, __FUNCTION__, __LINE__); iotable_init(realview_pbx_io_desc, ARRAY_SIZE(realview_pbx_io_desc)); printk(%s:%d\r\n, __FUNCTION__, __LINE__);//!!! the last printk output if (core_tile_pbx11mp() || core_tile_pbxa9mp()) //!!! hung here (here is a read to system control register 0x10XX) !! { printk(%s:%d\r\n, __FUNCTION__, __LINE__); iotable_init(realview_local_io_desc, ARRAY_SIZE(realview_local_io_desc)); printk(%s:%d\r\n, __FUNCTION__, __LINE__); } } - To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1015226/+subscriptions
[Qemu-devel] [Bug 928204] Re: build failure gentoo
Closing as invalid since this is a build failure for an old version of QEMU, appears to be a compiler/toolchain issue, and has had no followup from the reporter in over a year. ** Changed in: qemu Status: New = Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/928204 Title: build failure gentoo Status in QEMU: Invalid Bug description: gcc 4.5.3, gentoo linux ./configure --prefix=/usr --sysconfdir=/etc --disable-strip --disable-werror --disable-kvm --disable-libiscsi --enable-nptl --enable-uuid --enable-linux-user --extra-ldflags=-Wl,-z,execheap --enable-linux-aio --enable-bluez --disable-brlapi --disable-curl --disable-fdt --disable-pie --enable-vnc-jpeg --enable-curses --disable-smartcard-nss --enable-opengl --enable-vnc-png --disable-rbd --disable-vnc-sasl --enable-sdl --disable-spice --enable-vnc-tls --disable-vnc-thread --disable-vde --enable-vhost-net --disable-xen --disable-attr --disable-xfsctl --disable-darwin-user --disable-bsd-user --audio-card-list=ac97 es1370 sb16 cs4231a adlib gus hda --audio-drv-list=sdl alsa oss --target-list= i386-softmmu x86_64-softmmu alpha-softmmu arm-softmmu cris-softmmu m68k-softmmu microblaze-softmmu microblazeel-softmmu mips-softmmu mipsel-softmmu ppc-softmmu ppc64-softmmu sh4-softmmu sh4eb-softmmu sparc-softmmu sparc64-softmmu s390x-softmmu lm32-softmmu mips64-softmmu mips64el-softmmu ppcemb-softmmu xtensa-softmmu xtensaeb-softmmu i386-linux-user x86_64-linux-user alpha-linux-user arm-linux-user cris-linux-user m68k-linux-user microblaze-linux-user microblazeel-linux-user mips-linux-user mipsel-linux-user ppc-linux-user ppc64-linux-user sh4-linux-user sh4eb-linux-user sparc-linux-user sparc64-linux-user s390x-linux-user armeb-linux-user ppc64abi32-linux-user sparc32plus-linux-user unicore32-linux-user --cc=x86_64-pc-linux-gnu-gcc --host-cc=x86_64-pc-linux-gnu-gcc Install prefix/usr BIOS directory/usr/share/qemu binary directory /usr/bin library directory /usr/lib include directory /usr/include config directory /etc Manual directory /usr/share/man ELF interp prefix /usr/gnemul/qemu-%M Source path /var/tmp/portage/app-emulation/qemu-/work/qemu- C compilerx86_64-pc-linux-gnu-gcc Host C compiler x86_64-pc-linux-gnu-gcc CFLAGS-O2 -g -O2 -pipe QEMU_CFLAGS -m64 -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fstack-protector-all -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -I/usr/include/libpng15 LDFLAGS -Wl,--warn-common -m64 -g -Wl,-z,execheap -Wl,-O1 -Wl,--as-needed make make install install pythonpython smbd /usr/sbin/smbd host CPU x86_64 host big endian no target listi386-softmmu x86_64-softmmu alpha-softmmu arm-softmmu cris-softmmu m68k-softmmu microblaze-softmmu microblazeel-softmmu mips-softmmu mipsel-softmmu ppc-softmmu ppc64-softmmu sh4-softmmu sh4eb-softmmu sparc-softmmu sparc64-softmmu s390x-softmmu lm32-softmmu mips64-softmmu mips64el-softmmu ppcemb-softmmu xtensa-softmmu xtensaeb-softmmu i386-linux-user x86_64-linux-user alpha-linux-user arm-linux-user cris-linux-user m68k-linux-user microblaze-linux-user microblazeel-linux-user mips-linux-user mipsel-linux-user ppc-linux-user ppc64-linux-user sh4-linux-user sh4eb-linux-user sparc-linux-user sparc64-linux-user s390x-linux-user armeb-linux-user ppc64abi32-linux-user sparc32plus-linux-user unicore32-linux-user tcg debug enabled no Mon debug enabled no gprof enabled no sparse enabledno strip binariesno profiler no static build no -Werror enabled no SDL support yes curses supportyes curl support no mingw32 support no Audio drivers sdl alsa oss Extra audio cards ac97 es1370 sb16 cs4231a adlib gus hda Block whitelist Mixer emulation no VNC support yes VNC TLS support yes VNC SASL support no VNC JPEG support yes VNC PNG support yes VNC threadno xen support no brlapi supportno bluez supportyes Documentation yes NPTL support yes GUEST_BASEyes PIE no vde support no Linux AIO support yes ATTR/XATTR support no Install blobs yes KVM support no TCG interpreter no fdt support no preadv supportyes fdatasync yes madvise yes posix_madvise yes uuid support yes libcap-ng support no vhost-net support yes Trace backend nop Trace output file trace-pid
[Qemu-devel] [Bug 925412] Re: Cannot build on Mac using Xcode 4 and LLVM
QEMU now builds fine on MacOS, with LLVM-gcc and clang (we fixed both the typedef problems and the global-fixed-register variable). ** Changed in: qemu Status: New = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/925412 Title: Cannot build on Mac using Xcode 4 and LLVM Status in QEMU: Fix Released Bug description: As detailed in the mailing-list and the brew project (see below), QEMU currently either doesn't build with LLVM or builds and crashes upon runtime on Mac OS X Lion (or Snow Leopard if you've upgraded your compiler from gcc-4.2). This seems to be tied to the internal representation of UINT16, but effectively means that you currently cannot run QEMU 1.0 or HEAD (for any target arch - I'm focusing on ARM and Intel) on a Mac. References: [1]: http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg01330.html [2]: https://github.com/mxcl/homebrew/pull/9520 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/925412/+subscriptions
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
On Sun, Feb 10, 2013 at 10:10 PM, David Gibson da...@gibson.dropbear.id.au wrote: On Thu, Feb 07, 2013 at 09:28:20AM -0200, Erlon Cruz wrote: From: Erlon Cruz erlon.c...@br.flextronics.com This h_call is useful for DLPAR in future amongst other things. Given an index it fetches the corresponding PTE stored in the htab. Nice. It would be good to add in this little bit of PAPR compliance. Couple of small nits in the implementation: Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com --- hw/spapr_hcall.c | 58 ++ 1 file changed, 58 insertions(+) diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c index 2889742..5ba07e5 100644 --- a/hw/spapr_hcall.c +++ b/hw/spapr_hcall.c @@ -323,6 +323,63 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr, return H_SUCCESS; } +static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr, +target_ulong opcode, target_ulong *args) +{ +CPUPPCState *env = cpu-env; +target_ulong flags = args[0]; +target_ulong pte_index = args[1]; +uint8_t *hpte; + +if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) { +return H_PARAMETER; +} + +if (!(flags H_READ_4)) { It would be nice to combine the H_READ_4 and !H_READ_4 paths together, since except for the masking and stopping sooner the !H_READ_4 path is just like the H_READ_4 path. Ok. +target_ulong v, r; +target_ulong *pteh = args[0]; +target_ulong *ptel = args[1]; + +hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64); + +v = ldq_p(hpte); +r = ldq_p(hpte + (HASH_PTE_SIZE_64/2)); + +if (flags H_R_XLATE) { +/* FIXME: include a valid logical page num in the pte */ This comment is misleading. Since you do copy out both words of the hpte, and qemu stores the external_htab in terms of guest physical (== logical) addresses, in fact you're *always* supplying a valid logical page num. So you've already correctly implemented the flag as a no-op. I believe that flag is included for the benefit of a true hypervisor, where the native htab would be stored as true physical addresses, and it might be expensive for the hypervisor to recompute the logical address. Ok, then I think we can just skip this checking. That said, I actually wrote such helpers about 15 minutes ago as part of my MMU cleanup series. Should I wait for the helpers to send It again? -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
[Qemu-devel] [Bug 821289] Re: qemu mac segfault
It only segfaults when built with LLVM. This was because of QEMU's use of a global-fixed-register variable. This usage has now been removed and QEMU builds and runs fine with LLVM as well as pure gcc. ** Changed in: qemu Status: New = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/821289 Title: qemu mac segfault Status in QEMU: Fix Released Bug description: qemu-0.14.1 ./configure --target-list=i386-softmmu,x86_64-softmmu,arm-softmmu qemu(-system-(x86_64|arm)) Segmentation fault: 11 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/821289/+subscriptions
[Qemu-devel] [Bug 571432] Re: qemu-system-arm crashed with SIGSEGV in subpage_register()
Closing as invalid for QEMU because it's an Incomplete bug against an ancient QEMU version. ** Changed in: qemu Status: Incomplete = Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/571432 Title: qemu-system-arm crashed with SIGSEGV in subpage_register() Status in QEMU: Invalid Status in “qemu-kvm” package in Ubuntu: Expired Bug description: Binary package hint: qemu-kvm i think this is the crash behind Bug #570588 not sure why apport did not trigger before ProblemType: Crash DistroRelease: Ubuntu 10.04 Package: qemu-kvm-extras 0.12.3+noroms-0ubuntu9 ProcVersionSignature: Ubuntu 2.6.32-21.32-generic 2.6.32.11+drm33.2 Uname: Linux 2.6.32-21-generic x86_64 NonfreeKernelModules: openafs Architecture: amd64 Date: Wed Apr 28 21:30:13 2010 ExecutablePath: /usr/bin/qemu-system-arm InstallationMedia: Ubuntu 9.10 Karmic Koala - Release amd64 (20091027) KvmCmdLine: Error: command ['ps', '-C', 'kvm', '-F'] failed with exit code 1: UIDPID PPID CSZ RSS PSR STIME TTY TIME CMD ProcCmdLine: BOOT_IMAGE=/boot/vmlinuz-2.6.32-21-generic root=UUID=52d7f930-7148-4978-825e-71fcb9243ac6 ro quiet splash ProcCmdline: qemu-system-arm -M versatilepb -cpu cortex-a8 -kernel /tmp/tmp.B2CtSo2g2u/qemu-vmlinuz -no-reboot -nographic -pidfile /tmp/tmp.B2CtSo2g2u/qemu.pid -drive file=/tmp/tmp.B2CtSo2g2u/qemu-armel-201004282122.img,aio=native,cache=none -m 512 -append console=ttyAMA0,115200n8\ root=/dev/sda\ rw\ mem=256M\ devtmpfs.mount=0\ init=/bin/installer\ quiet ProcEnviron: SHELL=/bin/bash LANG=en_GB.UTF-8 SegvAnalysis: Segfault happened at: 0x51058e subpage_register+158: cmpq $0x0,(%rdx) PC (0x0051058e) ok source $0x0 ok destination (%rdx) (0x40cc28c0) not located in a known VMA region (needed writable region)! SegvReason: writing unknown VMA Signal: 11 SourcePackage: qemu-kvm StacktraceTop: subpage_register (mmio=0x7f841b26d010, start=value optimised out, subpage_init (base=268500992, phys=0x1d47400, cpu_register_physical_memory_offset ( smc91c111_init (nd=0xc41b60, base=1087121600, versatile_init (ram_size=value optimised out, Title: qemu-system-arm crashed with SIGSEGV in subpage_register() UserGroups: dmi.bios.date: 11/07/2007 dmi.bios.vendor: Phoenix Technologies LTD dmi.bios.version: 6.00 dmi.board.name: S2696 dmi.board.vendor: Tyan Computer Corporation dmi.chassis.type: 6 dmi.modalias: dmi:bvnPhoenixTechnologiesLTD:bvr6.00:bd11/07/2007:svn:pn:pvr:rvnTyanComputerCorporation:rnS2696:rvr:cvn:ct6:cvr: To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/571432/+subscriptions
[Qemu-devel] [PATCH RFC] memory: drop _overlap variant
overlap flag in the region is currently unused, most devices have no idea whether their region overlaps with anything, so drop it, assume that all regions can overlap and always require priority. It's also not clear how should devices allocate priorities. As a first step, define a set of symbolic priority names so it's easier to grep for. The result of this patch is that it's easy to see who uses a given priority. To avoid breaking build, this patch should be combined with a patch updating all devices. I have it working but am not sending this yet to avoid unnecessary load on list, so this patch shoul dnot be applied as is. Will send the full version if people don't object to this one. Signed-off-by: Michael S. Tsirkin m...@redhat.com Cc: avi.kiv...@gmail.com --- diff --git a/hw/sysbus.c b/hw/sysbus.c index 6d9d1df..f039cb8 100644 --- a/hw/sysbus.c +++ b/hw/sysbus.c @@ -61,9 +61,8 @@ void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr) memory_region_del_subregion(get_system_memory(), dev-mmio[n].memory); } dev-mmio[n].addr = addr; -memory_region_add_subregion(get_system_memory(), -addr, -dev-mmio[n].memory); +memory_region_add_subregion(get_system_memory(), addr, +dev-mmio[n].memory, MEMORY_PRIO_LOWEST); } @@ -215,16 +214,10 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev) } void sysbus_add_memory(SysBusDevice *dev, hwaddr addr, - MemoryRegion *mem) -{ -memory_region_add_subregion(get_system_memory(), addr, mem); -} - -void sysbus_add_memory_overlap(SysBusDevice *dev, hwaddr addr, - MemoryRegion *mem, unsigned priority) + MemoryRegion *mem, MemoryRegionPriority priority) { -memory_region_add_subregion_overlap(get_system_memory(), addr, mem, -priority); +memory_region_add_subregion(get_system_memory(), addr, mem, +priority); } void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem) @@ -235,7 +228,8 @@ void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem) void sysbus_add_io(SysBusDevice *dev, hwaddr addr, MemoryRegion *mem) { -memory_region_add_subregion(get_system_io(), addr, mem); +memory_region_add_subregion(get_system_io(), addr, mem, +MEMORY_PRIO_LOWEST); } void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem) diff --git a/hw/sysbus.h b/hw/sysbus.h index a7fcded..4059c03 100644 --- a/hw/sysbus.h +++ b/hw/sysbus.h @@ -57,9 +57,7 @@ void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size); void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq); void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr); void sysbus_add_memory(SysBusDevice *dev, hwaddr addr, - MemoryRegion *mem); -void sysbus_add_memory_overlap(SysBusDevice *dev, hwaddr addr, - MemoryRegion *mem, unsigned priority); + MemoryRegion *mem, MemoryRegionPriority priority); void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem); void sysbus_add_io(SysBusDevice *dev, hwaddr addr, MemoryRegion *mem); diff --git a/include/exec/memory.h b/include/exec/memory.h index 2322732..af1e0fa 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -147,6 +147,14 @@ struct MemoryRegion { MemoryRegionIoeventfd *ioeventfds; }; +typedef enum MemoryRegionPriority { +MEMORY_PRIO_LOWEST = 0x0, +MEMORY_PRIO_LOW = 0x1, +MEMORY_PRIO_MEDIUM = 0x2, +MEMORY_PRIO_HIGH = 1000, +MEMORY_PRIO_HIGHEST = UINT_MAX, +} MemoryRegionPriority; + struct MemoryRegionPortio { uint32_t offset; uint32_t len; @@ -628,27 +636,9 @@ void memory_region_del_eventfd(MemoryRegion *mr, /** * memory_region_add_subregion: Add a subregion to a container. * - * Adds a subregion at @offset. The subregion may not overlap with other - * subregions (except for those explicitly marked as overlapping). A region - * may only be added once as a subregion (unless removed with - * memory_region_del_subregion()); use memory_region_init_alias() if you - * want a region to be a subregion in multiple locations. - * - * @mr: the region to contain the new subregion; must be a container - * initialized with memory_region_init(). - * @offset: the offset relative to @mr where @subregion is added. - * @subregion: the subregion to be added. - */ -void memory_region_add_subregion(MemoryRegion *mr, - hwaddr offset, - MemoryRegion *subregion); -/** - * memory_region_add_subregion_overlap: Add a subregion to a container - * with overlap. - * * Adds a subregion at @offset. The subregion may overlap with other * subregions.
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On 14 February 2013 12:45, Michael S. Tsirkin m...@redhat.com wrote: overlap flag in the region is currently unused, most devices have no idea whether their region overlaps with anything, so drop it, assume that all regions can overlap and always require priority. Devices themselves shouldn't care, for the most part -- they just provide a memory region and it's their parent that has to map it and know whether it overlaps or not. Similarly, parents should generally be in control of the container they're mapping the memory region into, and know whether it will be an overlapping map or not. It's also not clear how should devices allocate priorities. Up to the parent which controls the region being mapped into. I definitely don't like making the priority argument mandatory: this is just introducing pointless boilerplate for the common case where nothing overlaps and you know nothing overlaps. Maybe we should take the printf() about subregion collisions in memory_region_add_subregion_common() out of the #if 0 that it currently sits in? -- PMM
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On Thu, Feb 14, 2013 at 12:56:02PM +, Peter Maydell wrote: On 14 February 2013 12:45, Michael S. Tsirkin m...@redhat.com wrote: overlap flag in the region is currently unused, most devices have no idea whether their region overlaps with anything, so drop it, assume that all regions can overlap and always require priority. Devices themselves shouldn't care, for the most part -- they just provide a memory region and it's their parent that has to map it and know whether it overlaps or not. Similarly, parents should generally be in control of the container they're mapping the memory region into, and know whether it will be an overlapping map or not. It's also not clear how should devices allocate priorities. Up to the parent which controls the region being mapped into. We could just assume same priority as parent but what happens if it has to be different? There are also aliases so a region can have multiple parents. Presumably it will have to have different priorities depending on what the parent does? Here's a list of instances using priority != 0. hw/armv7m_nvic.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:s-low_mem_container, MEMORY_PRIO_LOW); hw/kvm/pci-assign.c: r_dev-mmio, MEMORY_PRIO_LOW); hw/kvmvapic.c:memory_region_add_subregion(as, rom_paddr, s-rom, MEMORY_PRIO_HIGH); hw/lpc_ich9.c:MEMORY_PRIO_LOW); hw/onenand.c:s-mapped_ram, MEMORY_PRIO_LOW); hw/pam.c:MEMORY_PRIO_LOW); hw/pc.c:MEMORY_PRIO_LOW); hw/pc_sysfw.c:isa_bios, MEMORY_PRIO_LOW); hw/pc_sysfw.c:isa_bios, MEMORY_PRIO_LOW); hw/pci/pci.c:MEMORY_PRIO_LOW); hw/pci/pci_bridge.c:memory_region_add_subregion(parent_space, base, alias, MEMORY_PRIO_LOW); hw/piix_pci.c:MEMORY_PRIO_LOW); hw/piix_pci.c:d-rcr_mem, MEMORY_PRIO_LOW); hw/q35.c:mch-smram_region, MEMORY_PRIO_LOW); hw/vga-isa.c:MEMORY_PRIO_LOW); hw/vga.c:MEMORY_PRIO_MEDIUM); hw/vga.c:vga_io_memory, MEMORY_PRIO_LOW); hw/xen_pt_msi.c:MEMORY_PRIO_MEDIUM); /* Priority: pci default + 1 Making priority relative to parent but not the same just seems like a recipe for disaster. I definitely don't like making the priority argument mandatory: this is just introducing pointless boilerplate for the common case where nothing overlaps and you know nothing overlaps. Non overlapping is not a common case at all. E.g. with normal PCI devices you have no way to know nothing overlaps - addresses are guest programmable. See also recent discussion about 64 bit BARs. We could add a wrapper for MEMORY_PRIO_LOWEST - will that address your concern? Maybe we should take the printf() about subregion collisions in memory_region_add_subregion_common() out of the #if 0 that it currently sits in? -- PMM This is just a debugging tool, it won't fix anything. -- MST
Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available
On Thu, 14 Feb 2013 09:18:53 -0200 Eduardo Habkost ehabk...@redhat.com wrote: On Wed, Feb 13, 2013 at 05:17:44PM +0100, Igor Mammedov wrote: qemu boots from disk image 3 times faster than direct kernel load. That's surprising. Do you have any idea why that happens? (CCing qemu-devel in case other QEMU developers can explain it) Use grub2-mkrescue tool to create boot image with test kernel if available and do image boot then. On FC17 it reduces 1 VM run from ~15 sec to ~5sec. Signed-off-by: Igor Mammedov imamm...@redhat.com --- qemu/tests/cpuid.py| 11 +-- shared/deps/cpuid_test_kernel/Makefile | 18 -- shared/deps/cpuid_test_kernel/grub.cfg |8 3 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 shared/deps/cpuid_test_kernel/grub.cfg diff --git a/qemu/tests/cpuid.py b/qemu/tests/cpuid.py index 5065c6a..731411c 100644 --- a/qemu/tests/cpuid.py +++ b/qemu/tests/cpuid.py @@ -118,13 +118,20 @@ def run_cpuid(test, params, env): cpuid_test_kernel) os.chdir(test_kernel_dir) utils.make(cpuid_dump_kernel.bin) +utils.make(boot.img, ignore_status=True) +cmdres = utils.run(cd %s ls boot.img % test_kernel_dir, ignore_status=True) Why fork a shell just to check if a file exists? You can simply use os.path.exists('%s/boot.img' % (test_kernel_dir)) or os.path.exists(os.path.join(test_kernel_dir, 'boot.img')). Thanks, I'll fix it. The rest of the patch looks good to me. I wonder if we could make this a generic boot kernel image function, that uses grub2-mkrescue if available, and -kernel otherwise. I believe other test cases may benefit for a general mechanism to boot test kernels. I'd make something like make_boot_image() a generic, which would * take kernel, (optional initrd, kernel options), root tree directory * create boot image using host available tools (which is greatly distro specific depending on available tools) and leave decision what boot method to use to test cases. But due to lack of free time, I went through easy road. I'd glad to amend patch, if there are suggestions how to generalize it and improve create boot image process for distros that doesn't have grub2-mkrescue. vm_name = params.get('main_vm') params_b = params.copy() -params_b[kernel] = os.path.join(test_kernel_dir, cpuid_dump_kernel.bin) +if cmdres.exit_status == 0: +params_b[image_name_custom] = os.path.join(test_kernel_dir, boot.img) +params_b[image_format_custom] = +params_b[images] = custom +else: +params_b[kernel] = os.path.join(test_kernel_dir, cpuid_dump_kernel.bin) +del params_b[images] params_b[cpu_model] = cpu_model params_b[cpu_model_flags] = feature -del params_b[images] del params_b[nics] env_process.preprocess_vm(self, params_b, env, vm_name) vm = env.get_vm(vm_name) diff --git a/shared/deps/cpuid_test_kernel/Makefile b/shared/deps/cpuid_test_kernel/Makefile index 0e34c43..5999804 100644 --- a/shared/deps/cpuid_test_kernel/Makefile +++ b/shared/deps/cpuid_test_kernel/Makefile @@ -1,10 +1,24 @@ CFLAGS := -m32 -fno-stack-protector -fno-builtin -nostdinc -O -g -Wall -I. LDFLAGS := -nostdlib -Wl,-N -Wl,-Ttext -Wl,10 -all: cpuid_dump_kernel.bin +boot := $(shell sh -c 'which grub2-mkrescue 2/dev/null echo grub2boot.img') + +all: cpuid_dump_kernel.bin boot.img cpuid_dump_kernel.bin: boot.S main.c test.c $(CC) -o $@ $(CFLAGS) $^ $(LDFLAGS) + ln -s $@ kernel.bin + +boot.img: $(boot) + + +grub2boot.img: + mkdir -p iso/boot/grub + cp -f grub.cfg iso/boot/grub + cp -f kernel.bin iso/ + grub2-mkrescue -o boot.img iso clean: - rm -f *.o *.bin + rm -rf *.o *.bin iso *.img + +.PHONY: grub2boot.img diff --git a/shared/deps/cpuid_test_kernel/grub.cfg b/shared/deps/cpuid_test_kernel/grub.cfg new file mode 100644 index 000..aeb5b2b --- /dev/null +++ b/shared/deps/cpuid_test_kernel/grub.cfg @@ -0,0 +1,8 @@ +set timeout=0 +set default=0 + +menuentry test { + set root=(hd0) + multiboot /kernel.bin + boot +} -- 1.7.1
Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available
On Thu, Feb 14, 2013 at 01:13:18PM +0100, Paolo Bonzini wrote: Il 14/02/2013 12:18, Eduardo Habkost ha scritto: qemu boots from disk image 3 times faster than direct kernel load. That's surprising. Do you have any idea why that happens? Because kernel load uses MMIO (from fw_cfg), while booting from disk uses at worst PCI DMA and at best virtio. Is it something worth trying to optimize, or a reasonable solution would be so similar to having a disk+bootloader that's easier to simply recommend people to set up a real disk with a real bootloader if they care about speed? -- Eduardo
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On 14 February 2013 13:09, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 14, 2013 at 12:56:02PM +, Peter Maydell wrote: Up to the parent which controls the region being mapped into. We could just assume same priority as parent Er, no. I mean the code in control of the parent MR sets the priority, when it calls memory_region_add_subregion_overlap(). but what happens if it has to be different? There are also aliases so a region can have multiple parents. The alias has its own priority. Presumably it will have to have different priorities depending on what the parent does? Here's a list of instances using priority != 0. hw/armv7m_nvic.c:MEMORY_PRIO_LOW); So this one I know about, and it's a good example of what I'm talking about. This function sets up a container memory region (nvic), and it is in total control of what is mapped into that container. Specifically, it puts in a nvic_sysregs background region which covers the whole 0x1000 size of the container (at an implicit priority of zero). It then layers over that an alias of the GIC registers (nvic-gic) at a specific address and with a priority of 1 so it appears above the background region. Nobody else ever puts anything in this container, so the only thing we care about is that the priority of the nvic-gic region is higher than that of the nvic_sysregs region; and it's clear from the code that we do that. Priority is a local question whose meaning is only relevant within a particular container region, not system-wide, and having system-wide MEMORY_PRIO_ defines obscures that IMHO. I definitely don't like making the priority argument mandatory: this is just introducing pointless boilerplate for the common case where nothing overlaps and you know nothing overlaps. Non overlapping is not a common case at all. E.g. with normal PCI devices you have no way to know nothing overlaps - addresses are guest programmable. That means PCI is a special case :-) If the guest can program overlap then presumably PCI specifies semantics for what happens then, and there need to be PCI specific wrappers that enforce those semantics and they can call the relevant _overlap functions when mapping things. In any case this isn't a concern for the PCI *device*, which can just provide its memory regions. It's a problem the PCI *host adaptor* has to deal with when it's figuring out how to map those regions into the container which corresponds to its area of the address space. We could add a wrapper for MEMORY_PRIO_LOWEST - will that address your concern? Well, I'm entirely happy with the memory API we have at the moment, and I'm trying to figure out why you want to change it... Maybe we should take the printf() about subregion collisions in memory_region_add_subregion_common() out of the #if 0 that it currently sits in? This is just a debugging tool, it won't fix anything. It might tell us what bits of code are currently erroneously mapping regions that overlap without using the _overlap() function. Then we could fix them. -- PMM
Re: [Qemu-devel] [RFC PATCH 04/10] qemu-ga: Add Windows VSS provider to quiesce applications on fsfreeze
Il 14/02/2013 07:10, Tomoki Sekiyama ha scritto: diff --git a/qga/vss-win32-provider/Makefile b/qga/vss-win32-provider/Makefile new file mode 100644 index 000..1f213f2 --- /dev/null +++ b/qga/vss-win32-provider/Makefile @@ -0,0 +1,30 @@ +-include ../../config-host.mak +-include ../../rules.mak Please try to use a non-recursive makefile style. See libcacard/Makefile for an example. +# To build .tlb from .idl, WindowsSDK and C++ must be installed +MIDL=midl +WINSDK=C:\\Program Files\\Microsoft SDKs\\Windows\\v7.1\\Include When cross-compiling, does it work to use Wine's widl implementation? widl needs a -t flag when creating a .tlb, but otherwise should be compatible. And unlike some other wine tools, it has no dependency on the Wine runtime. It would be great to choose between midl or widl in configure. It's okay to require a specific option (like --midl=FOO) when compiling under Windows, and to require installation of Wine tools to build the VSS stuff under non-Windows. Paolo +#include ../vss-win32.h +#include inc/win2003/vscoordint.h +#include ../vss-win32-provider.h + +#include comadmin.h +#include wbemidl.h +#include comutil.h + +extern HINSTANCE g_hinstDll; + +const GUID CLSID_COMAdminCatalog = +{0xF618C514, 0xDFB8, 0x11d1, {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35}}; +const GUID IID_ICOMAdminCatalog = +{0xDD662187, 0xDFC2, 0x11d1, {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35}}; +const GUID CLSID_WbemLocator = +{0x4590f811, 0x1d3a, 0x11d0, {0x89, 0x1f, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24}}; +const GUID IID_IWbemLocator = +{0xdc12a687, 0x737f, 0x11cf, {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24}}; + +static void errmsg(DWORD err, const char *text) +{ +char *msg = NULL, *nul = strchr(text, '('); +int len = nul ? nul - text : -1; + +FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (char *)msg, 0, NULL); +printf(%.*s. (Error: %lx) %s\n, len, text, err, msg); +LocalFree(msg); +} + +static void errmsg_dialog(DWORD err, const char *text, const char *opt = ) +{ +char *msg, buf[512]; + +FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (char *)msg, 0, NULL); +snprintf(buf, sizeof(buf), %s%s. (Error: %lx) %s\n, text, opt, err, msg); +MessageBox(NULL, buf, Error from QGA_PROVIDER_NAME, MB_OK|MB_ICONERROR); +LocalFree(msg); +} + +#define _chk(hr, status, msg, err_label) \ +do {\ +hr = (status); \ +if (FAILED(hr)) { \ +errmsg(hr, msg);\ +goto err_label; \ +} \ +} while(0) + +#define chk(status) _chk(hr, status, Failed to #status, out) + +templateclass T +HRESULT put_Value(ICatalogObject *pObj, LPCWSTR name, T val) +{ +return pObj-put_Value(_bstr_t(name), _variant_t(val)); +} + +/* Check whether this OS version supports VSS providers */ +STDAPI VSSCheckOSVersion(void) +{ +OSVERSIONINFO OSver; + +OSver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); +GetVersionEx(OSver); +if((OSver.dwMajorVersion == 5 OSver.dwMinorVersion = 2) || + OSver.dwMajorVersion 5) { +return S_OK; +} +return S_FALSE; +} + +/* Lookup Administrators group name from winmgmt */ +static HRESULT GetAdminName(_bstr_t name) +{ +HRESULT hr; +IWbemLocator *pLoc = NULL; +IWbemServices *pSvc = NULL; +IEnumWbemClassObject *pEnum = NULL; +IWbemClassObject *pWobj = NULL; +ULONG returned; +_variant_t var; + +chk( CoCreateInstance(CLSID_WbemLocator, NULL, CLSCTX_INPROC_SERVER, + IID_IWbemLocator, (LPVOID*)pLoc) ); +chk( pLoc-ConnectServer(_bstr_t(LROOT\\CIMV2), NULL, NULL, NULL, + 0, 0, 0, pSvc) ); +chk( CoSetProxyBlanket(pSvc, RPC_C_AUTHN_WINNT, RPC_C_AUTHZ_NONE, + NULL, RPC_C_AUTHN_LEVEL_CALL, + RPC_C_IMP_LEVEL_IMPERSONATE, NULL, EOAC_NONE) ); +chk( pSvc-ExecQuery( + _bstr_t(LWQL), + _bstr_t(Lselect * from Win32_Account where + SID='S-1-5-32-544' and localAccount=TRUE), + WBEM_FLAG_RETURN_IMMEDIATELY | WBEM_FLAG_FORWARD_ONLY, + NULL, pEnum) ); +if (!pEnum) { +errmsg(E_FAIL, Failed to query for Administrators); +goto out; +} +chk( pEnum-Next(WBEM_INFINITE, 1, pWobj, returned) ); +if (returned == 0) { +errmsg(E_FAIL,
Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available
Il 14/02/2013 14:24, Eduardo Habkost ha scritto: On Thu, Feb 14, 2013 at 01:13:18PM +0100, Paolo Bonzini wrote: Il 14/02/2013 12:18, Eduardo Habkost ha scritto: qemu boots from disk image 3 times faster than direct kernel load. That's surprising. Do you have any idea why that happens? Because kernel load uses MMIO (from fw_cfg), while booting from disk uses at worst PCI DMA and at best virtio. Is it something worth trying to optimize I think that, within the limits of what the spec makes legal, Gleb optimized all that he could out of it. The alternative is to make fw_cfg do DMA, which in the past was rejected because it doesn't look like what real ISA hardware would do. , or a reasonable solution would be so similar to having a disk+bootloader that's easier to simply recommend people to set up a real disk with a real bootloader if they care about speed? In the end it's a pity, but yeah that's the easiest thing to do with distro kernels and big all-drivers initrd. -kernel is still useful and fast enough if you have a custom-built kernel, possibly with no initrd at all. Paolo
Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently
[Some quoted material restored] Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 14 Feb 2013 10:45:22 +0100 Markus Armbruster arm...@redhat.com wrote: [Note cc: +Laszlo, +Anthony, -qemu-trivial] Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 08 Feb 2013 20:34:20 +0100 Markus Armbruster arm...@redhat.com wrote: The real problem here is that the k, M, G suffixes, for example, are not good to be reported by QMP. So maybe we should refactor the code in a way that we separate what's done in QMP from what is done in HMP/command-line. Isn't it separated already? parse_option_size() is used when parsing key=value,... Such strings should not exist in QMP. If they do, it's a design bug. No and no. Such strings don't exist in QMP as far as can tell (see bugs below though), but parse_option_size() is theoretically present in a possible QMP call stack: qemu_opts_from_dict_1() qemu_opt_set_err() opt_set() qemu_opt_paser() parse_option_size() I can't tell if this will ever happen because qemu_opts_from_dict_1() restricts the call to qemu_opt_set_err() to certain values, but the fact that it's not clear is an indication that a better separation is necessary. Permit me two detours. One, qemu_opt_set_err() is a confusing name. I doesn't set an error. It takes an Error ** object and it was introduced to avoid having to convert qemu_opt_set() to take an Error ** object, as this would multiply the work required to get netdev_add converted to the qapi. Now, I pass the bikeshed :) I get why it's there, I just find its name confusing. [...] Now, to build hmp_device_add() on top of qmp_device_add(), we'd have to convert first from QemuOpts to QDict and then back to QemuOpts. Blech. Instead, we made do_device_add() go straight to qdev_device_add(). Same for hmp_netdev_add(): it goes straight to netdev_add(). Yes, the idea was to have netdev_add() and add frontends to hmp and qmp. More on this below. [...] I guess weird things can happen with qemu_opts_from_qdict(), at least in theory. For each (key, value) in the QDict, qemu_opts_from_qdict() converts value to string according to its qtype_code. The string then gets parsed according to key's QemuOptType. Yes, that means you can pass a QString to QEMU_OPT_SIZE option, and get the suffixes interpreted. However, we've only used qemu_opts_from_qdict() with QemuOptsLists that have empty desc[]! Specifically: qemu_netdev_opts and qemu_device_opts. Without desc, qemu_opt_parse() does nothing, and QemuOpts holds just string values. Actual parsing left to the consumer. Two consumers: qdev_device_add() and netdev_add(). netdev_add() uses QAPI wizardry to create the appropriate object from the QemuOpts. The parsing is done by visitors. Things get foggy for me from there on, but it looks like one of them, opts_type_size(), can parse size suffixes, which is inappropriate for QMP. A quick test confirms that this is indeed possible: {execute: netdev_add,arguments: {type:tap,id:net.1, sndbuf: 8k }} Sets NetdevTapOptions member sndbuf to 8192. Well, I've just tested this with commit 783e9b4, which is before netdev_add conversion to the qapi, and the command above just works. Didn't check if sndbuf is actually set to 8192, but this shows that we've always accepted such a command. Plausible. Nevertheless, we really shouldn't. To sum up, we go from QDict delivered by the JSON parser via QemuOpts to QAPI object, with a few cock-ups along the way. Ugh. By the way, the JSON schema reads { 'command': 'netdev_add', 'data': {'type': 'str', 'id': 'str', '*props': '**'}, 'gen': 'no' } I'll be hanged if I know what '**' means. For QMP on the wire it means that the command accepts a bunch of arguments not specified in the schema. Thanks! Is the schema language documented anywhere? Yes, it's a dirty trick. Long story short: we decide to maintain QemuOpts usage in both HMP and QMP to speed up netdev_add conversion. I don't think '**' is as dirty as you make it sound. It's simply a way to relax the rigidity of the schema. I got no problem with that. qdev_device_add() uses a few well-known options itself, and they're all strings. The others it passes on to qdev_prop_parse(). This permits some weirdness like passing PCI device's addr property as number in QMP, even though it's nominally a string of the form SLOT[.FN]. No JSON schema, because device_add hasn't been qapified (Laszlo's working on it now). Now, I think I've found at least two bugs. The first one is the netdev_add doc in the schema, which states that we do accept key=value strings. The problem is here is that that's about the C API, on the wire we act as before (ie. accepting each key as a separate argument). The qapi-schame.json is more or less format-independent, so I'm not exactly sure what's the
Re: [Qemu-devel] [RFC PATCH 04/10] qemu-ga: Add Windows VSS provider to quiesce applications on fsfreeze
Il 14/02/2013 07:10, Tomoki Sekiyama ha scritto: diff --git a/qga/vss-win32-provider/qga-provider.idl b/qga/vss-win32-provider/qga-provider.idl new file mode 100644 index 000..17abca0 --- /dev/null +++ b/qga/vss-win32-provider/qga-provider.idl @@ -0,0 +1,20 @@ +import oaidl.idl; +import ocidl.idl; + +[ +uuid(103B8142-6CE5-48A7-BDE1-794D3192FCF1), +version(1.0), +helpstring(QGAVSSProvider Type Library) +] +library QGAVSSHWProviderLib +{ +importlib(stdole2.tlb); +[ +uuid(6E6A3492-8D4D-440C-9619-5E5D0CC31CA8), +helpstring(QGAVSSProvider Class) +] +coclass QGAVSSHWProvider +{ +[default] interface IUnknown; +}; +}; Ok, I checked widl and it chokes on the importlib line. If that can be removed, it's fine to use widl. The invocation is widl -m32/-m64 -o qga-provider.tlb -t qga-provider.idl where code to choose between -m32 and -m64 is already in the configure script (search for `case $cpu`). Paolo
[Qemu-devel] [Bug 569760] Re: Error in i386 cmpxchg instruction emulation
** Changed in: qemu Status: Fix Committed = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/569760 Title: Error in i386 cmpxchg instruction emulation Status in QEMU: Fix Released Bug description: As reported in http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=42158 programs using pthreads and fork() under NetBSD/i386 hang when the NetBSD system is run within qemu. This problem affects every version of qemu I have tested, including 0.12.3. I have now tracked down the cause of the problem to a bug in qemu's emulation of the cmpxchg instruction. Quoting the above bug report: In a physical i386 CPU, the cmpxchg instruction performs a comparison and read-modify-write memory cycle. In the case where the comparison outcome is unequal, the read-modify-write cycle is an effective no-op, writing back the same value that was read, and the value of the source operand is loaded into the accumulator. Qemu attempts to emulate this behavior including the redundant memory write. To be precise, qemu first loads the accumulator and then does the redundant memory write. If a page fault occurs during the write, the cmpxchg instruction will be restarted after handling the page fault, but because the accumulator has already been changed, the comparison will now incorrectly yield a result of equal, causing the memory write to write the value from the source operand instead of re-writing the original memory contents. I assume fork() triggers the bug because it write protects pages to implement copy-on-write, thereby producing a situation where the read part of the cmpxchg read-modify-write cycle succeeds but the write part causes a page fault. Patching qemu to only change the accumulator after performing the redundant write fixes the problem for me. I will attach a patch against qemu 0.12.3 shortly. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/569760/+subscriptions
Re: [Qemu-devel] [RFC PATCH v2 00/23] qcow2: Delayed COW
On Thu, Feb 14, 2013 at 11:56:34AM +0100, Kevin Wolf wrote: Am 14.02.2013 11:43, schrieb Stefan Hajnoczi: On Wed, Feb 13, 2013 at 02:21:50PM +0100, Kevin Wolf wrote: iozone results with and without this series show significant difference for allocating writes: random random KB reclen write rewritereadrereadread write base65536 817271945125461253924491836 patch 65536 819341949122631252124631796 base 1048576 256 22344 38437 105823 106135 37743 32167 patch 1048576 256 35989 38542 105231 105994 38301 33036 I imagine the benefit is even greater for 4 KB or 8 KB blocksize? The upper two lines are 8k block size. I suspect that the benefit would be the greatest for misaligned 64k writes, then you have one overlapping allocation for each issued request. I see. I haven't looked at IOzone output for too long, forgot how to interpret it. Stefan
Re: [Qemu-devel] [PATCH moxie 2/5] Add moxie disassembler
On 02/13/2013 03:21 PM, Anthony Green wrote: This patch adds the disassembler logic for moxie. When sending a patch series, it is better if you thread each message as a reply to a cover letter, rather than each message as a top-level thread ('git send-email -5 --cover-letter' is one way to easily do this). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [Bug 393430] Re: kvm: use PulseAudio instead of ALSA
(ancient distro packaging bug so never valid for QEMU upstream itself; marking Invalid there) ** Changed in: qemu Status: Incomplete = Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/393430 Title: kvm: use PulseAudio instead of ALSA Status in QEMU: Invalid Status in “kvm” package in Ubuntu: Fix Released Status in “kvm” package in Debian: Fix Released Bug description: Binary package hint: kvm kvm in jaunty prefers to use OSS drivers rather than ALSA: # dpkg -l | grep kvm ii kvm1:84+dfsg-0ubuntu11Full virtualization on i386 and amd64 hardwa # kvm -audio-help | grep ^Name Name: oss Name: alsa Name: sdl Name: pa Name: none Name: wav # Because of that, once a virtual machine with working audio card starts, the kvm grabs the audio card entirely for itself. Any subsequent operations on audio card either from the host machine, or from the other audio-enabled virtual machines lock up (apparently awaiting on on access to the audio card). The bug is echo of the corresponding debian bug: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508018 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/393430/+subscriptions
Re: [Qemu-devel] 3 new x86 instructions
Richard Henderson r...@twiddle.net writes: Yes, and -cpu Haswell will enable the BMI extensions. I have used git://repo.or.cz/qemu/rth.git x86-next checked out around 15 January for a while now. The host system is Debian GNU/Linux amd64 and the guest is FreeBSD 9.1 amd64. I now updated x86-next in order to get support for adox/adcx. The current sources causes a problem inside FreeBSD's sshd. Debugging a bit reveals that it gets a SIGFPE at a 'div' instruction inside /lib/libcrypto.so.6's BN_div function. The instruction is correct in raising SIGFPE (or whatever the instruction-level counterpart is called), since the dividend / divisor is too large to fit a 64-bit quotient (or said differently: the high word in rdx is = than the divisor [in r13]). I have not tracked down why execution leads to these invalid operands. I cannot therefore state with any certainty that this is a bug in the qemu variant used. -- Torbjörn
Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available
(CCing virt-tools list in case they have any existing solutions or ideas on how to make this easier for everybody) On Thu, Feb 14, 2013 at 02:20:45PM +0100, Igor Mammedov wrote: On Thu, 14 Feb 2013 09:18:53 -0200 Eduardo Habkost ehabk...@redhat.com wrote: On Wed, Feb 13, 2013 at 05:17:44PM +0100, Igor Mammedov wrote: qemu boots from disk image 3 times faster than direct kernel load. That's surprising. Do you have any idea why that happens? (CCing qemu-devel in case other QEMU developers can explain it) Use grub2-mkrescue tool to create boot image with test kernel if available and do image boot then. On FC17 it reduces 1 VM run from ~15 sec to ~5sec. Signed-off-by: Igor Mammedov imamm...@redhat.com --- qemu/tests/cpuid.py| 11 +-- shared/deps/cpuid_test_kernel/Makefile | 18 -- shared/deps/cpuid_test_kernel/grub.cfg |8 3 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 shared/deps/cpuid_test_kernel/grub.cfg diff --git a/qemu/tests/cpuid.py b/qemu/tests/cpuid.py index 5065c6a..731411c 100644 --- a/qemu/tests/cpuid.py +++ b/qemu/tests/cpuid.py @@ -118,13 +118,20 @@ def run_cpuid(test, params, env): cpuid_test_kernel) os.chdir(test_kernel_dir) utils.make(cpuid_dump_kernel.bin) +utils.make(boot.img, ignore_status=True) +cmdres = utils.run(cd %s ls boot.img % test_kernel_dir, ignore_status=True) Why fork a shell just to check if a file exists? You can simply use os.path.exists('%s/boot.img' % (test_kernel_dir)) or os.path.exists(os.path.join(test_kernel_dir, 'boot.img')). Thanks, I'll fix it. The rest of the patch looks good to me. I wonder if we could make this a generic boot kernel image function, that uses grub2-mkrescue if available, and -kernel otherwise. I believe other test cases may benefit for a general mechanism to boot test kernels. I'd make something like make_boot_image() a generic, which would * take kernel, (optional initrd, kernel options), root tree directory * create boot image using host available tools (which is greatly distro specific depending on available tools) and leave decision what boot method to use to test cases. But due to lack of free time, I went through easy road. I understand. Maybe one day somebody will volunteer to write a more generic solution. :-) I'd glad to amend patch, if there are suggestions how to generalize it and improve create boot image process for distros that doesn't have grub2-mkrescue. A reusable tool/script would be useful even outside virt-test. Anybody who uses -kernel/-initrd today may want a more efficient solution (IIRC, virt-install uses -kernel/-initrd when installing from a Fedora repository URL). But if we made that an independent tool we would have the additional problem of having the new tool packaged and installed on the host. So having something specific inside virt-test makes sense by now. -- Eduardo
Re: [Qemu-devel] uhci: cancel delay for unregistered queues
Hi, On 02/14/2013 01:23 PM, Gerd Hoffmann wrote: Hi, The only fix I can think of is allocating a receive buffer inside the UHCI code for read transfers, and copy the result over to the guest memory when we are re-scanning the schedule, encounter the completed td and are going to signal its completion to the guest. I could take a shot at writing a patch with the proposed fix, but I was sort of waiting on a reaction on the proposal first. Gerd what do you think of my proposed fix ? That should fix it indeed. Yes, for uhci we can live with the overhead, it's low bandwidth anyway. No, I don't have a better idea. Patch is very welcome. Ok, I've put this on my todo list, I expect to be able to start working on this next Tuesday (and hopefully will finish it in 1 day). Regards, Hans
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On Thu, Feb 14, 2013 at 01:22:18PM +, Peter Maydell wrote: On 14 February 2013 13:09, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 14, 2013 at 12:56:02PM +, Peter Maydell wrote: Up to the parent which controls the region being mapped into. We could just assume same priority as parent Er, no. I mean the code in control of the parent MR sets the priority, when it calls memory_region_add_subregion_overlap(). but what happens if it has to be different? There are also aliases so a region can have multiple parents. The alias has its own priority. Well that's the status quo. One of the issues is, you have no idea what else uses each priority. With this change, at least you can grep for it. Presumably it will have to have different priorities depending on what the parent does? Here's a list of instances using priority != 0. hw/armv7m_nvic.c:MEMORY_PRIO_LOW); So this one I know about, and it's a good example of what I'm talking about. This function sets up a container memory region (nvic), and it is in total control of what is mapped into that container. Specifically, it puts in a nvic_sysregs background region which covers the whole 0x1000 size of the container (at an implicit priority of zero). It then layers over that an alias of the GIC registers (nvic-gic) at a specific address and with a priority of 1 so it appears above the background region. Nobody else ever puts anything in this container, so the only thing we care about is that the priority of the nvic-gic region is higher than that of the nvic_sysregs region; and it's clear from the code that we do that. Priority is a local question whose meaning is only relevant within a particular container region, not system-wide, and having system-wide MEMORY_PRIO_ defines obscures that IMHO. Well that's not how it seems to work, and I don't see how it *could* work. Imagine the specific example: ioapic and pci devices. ioapic has an address within the pci hole but it is not a subregion. If priority has no meaning how would you decide which one to use? Also, on a PC many addresses are guest programmable. We need to behave in some defined way if guest programs addresses to something silly. The only reason it works sometimes is because some systems use fixes addresses which never overlap. I definitely don't like making the priority argument mandatory: this is just introducing pointless boilerplate for the common case where nothing overlaps and you know nothing overlaps. Non overlapping is not a common case at all. E.g. with normal PCI devices you have no way to know nothing overlaps - addresses are guest programmable. That means PCI is a special case :-) If the guest can program overlap then presumably PCI specifies semantics for what happens then, and there need to be PCI specific wrappers that enforce those semantics and they can call the relevant _overlap functions when mapping things. In any case this isn't a concern for the PCI *device*, which can just provide its memory regions. It's a problem the PCI *host adaptor* has to deal with when it's figuring out how to map those regions into the container which corresponds to its area of the address space. Issue is, a PCI device overlapping something else suddenly becomes this something else's problem. We could add a wrapper for MEMORY_PRIO_LOWEST - will that address your concern? Well, I'm entirely happy with the memory API we have at the moment, and I'm trying to figure out why you want to change it... I am guessing your systems all have hardcoded addresses not controlled by guest. Maybe we should take the printf() about subregion collisions in memory_region_add_subregion_common() out of the #if 0 that it currently sits in? This is just a debugging tool, it won't fix anything. It might tell us what bits of code are currently erroneously mapping regions that overlap without using the _overlap() function. Then we could fix them. -- PMM When there is a single guest programmable device, any address can be overlapped by it. We could invent rules like 'non overlappable is higher priority' but it seems completely arbitrary, a single priority is clearer. -- MST
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On Thu, Feb 14, 2013 at 3:09 PM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 14, 2013 at 12:56:02PM +, Peter Maydell wrote: On 14 February 2013 12:45, Michael S. Tsirkin m...@redhat.com wrote: overlap flag in the region is currently unused, most devices have no idea whether their region overlaps with anything, so drop it, assume that all regions can overlap and always require priority. Devices themselves shouldn't care, for the most part -- they just provide a memory region and it's their parent that has to map it and know whether it overlaps or not. Similarly, parents should generally be in control of the container they're mapping the memory region into, and know whether it will be an overlapping map or not. It's also not clear how should devices allocate priorities. Up to the parent which controls the region being mapped into. We could just assume same priority as parent but what happens if it has to be different? Priority is only considered relative to siblings. The parent's priority is only considered wrt the parent's siblings, not its children. There are also aliases so a region can have multiple parents. Presumably it will have to have different priorities depending on what the parent does? The alias region has its own priority Here's a list of instances using priority != 0. hw/armv7m_nvic.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:s-low_mem_container, MEMORY_PRIO_LOW); hw/kvm/pci-assign.c: r_dev-mmio, MEMORY_PRIO_LOW); hw/kvmvapic.c:memory_region_add_subregion(as, rom_paddr, s-rom, MEMORY_PRIO_HIGH); hw/lpc_ich9.c:MEMORY_PRIO_LOW); hw/onenand.c:s-mapped_ram, MEMORY_PRIO_LOW); hw/pam.c:MEMORY_PRIO_LOW); hw/pc.c:MEMORY_PRIO_LOW); hw/pc_sysfw.c:isa_bios, MEMORY_PRIO_LOW); hw/pc_sysfw.c:isa_bios, MEMORY_PRIO_LOW); hw/pci/pci.c:MEMORY_PRIO_LOW); hw/pci/pci_bridge.c:memory_region_add_subregion(parent_space, base, alias, MEMORY_PRIO_LOW); hw/piix_pci.c:MEMORY_PRIO_LOW); hw/piix_pci.c:d-rcr_mem, MEMORY_PRIO_LOW); hw/q35.c:mch-smram_region, MEMORY_PRIO_LOW); hw/vga-isa.c:MEMORY_PRIO_LOW); hw/vga.c:MEMORY_PRIO_MEDIUM); hw/vga.c:vga_io_memory, MEMORY_PRIO_LOW); hw/xen_pt_msi.c:MEMORY_PRIO_MEDIUM); /* Priority: pci default + 1 Making priority relative to parent but not the same just seems like a recipe for disaster. I definitely don't like making the priority argument mandatory: this is just introducing pointless boilerplate for the common case where nothing overlaps and you know nothing overlaps. Non overlapping is not a common case at all. E.g. with normal PCI devices you have no way to know nothing overlaps - addresses are guest programmable. Non overlapping is mostly useful for embedded platforms.
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On Thu, Feb 14, 2013 at 4:02 PM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 14, 2013 at 01:22:18PM +, Peter Maydell wrote: On 14 February 2013 13:09, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 14, 2013 at 12:56:02PM +, Peter Maydell wrote: Up to the parent which controls the region being mapped into. We could just assume same priority as parent Er, no. I mean the code in control of the parent MR sets the priority, when it calls memory_region_add_subregion_overlap(). but what happens if it has to be different? There are also aliases so a region can have multiple parents. The alias has its own priority. Well that's the status quo. One of the issues is, you have no idea what else uses each priority. With this change, at least you can grep for it. The question what priorities do aliases of this region have is not an interesting question. Priority is a local attribute, not an attribute of the region being prioritized. Presumably it will have to have different priorities depending on what the parent does? Here's a list of instances using priority != 0. hw/armv7m_nvic.c:MEMORY_PRIO_LOW); So this one I know about, and it's a good example of what I'm talking about. This function sets up a container memory region (nvic), and it is in total control of what is mapped into that container. Specifically, it puts in a nvic_sysregs background region which covers the whole 0x1000 size of the container (at an implicit priority of zero). It then layers over that an alias of the GIC registers (nvic-gic) at a specific address and with a priority of 1 so it appears above the background region. Nobody else ever puts anything in this container, so the only thing we care about is that the priority of the nvic-gic region is higher than that of the nvic_sysregs region; and it's clear from the code that we do that. Priority is a local question whose meaning is only relevant within a particular container region, not system-wide, and having system-wide MEMORY_PRIO_ defines obscures that IMHO. Well that's not how it seems to work, and I don't see how it *could* work. Imagine the specific example: ioapic and pci devices. ioapic has an address within the pci hole but it is not a subregion. If priority has no meaning how would you decide which one to use? Like PMM said. You look at the semantics of the hardware, and map that onto the API. If the pci controller says that BARs hide the ioapic, then you give them higher priority. If it says that the ioapic hides BARs, then that gets higher priority. If it doesn't say anything, take your pick (or give them the same priority). Also, on a PC many addresses are guest programmable. We need to behave in some defined way if guest programs addresses to something silly. That's why _overlap exists. The only reason it works sometimes is because some systems use fixes addresses which never overlap. That's why the no overlap API exists. I definitely don't like making the priority argument mandatory: this is just introducing pointless boilerplate for the common case where nothing overlaps and you know nothing overlaps. Non overlapping is not a common case at all. E.g. with normal PCI devices you have no way to know nothing overlaps - addresses are guest programmable. That means PCI is a special case :-) If the guest can program overlap then presumably PCI specifies semantics for what happens then, and there need to be PCI specific wrappers that enforce those semantics and they can call the relevant _overlap functions when mapping things. In any case this isn't a concern for the PCI *device*, which can just provide its memory regions. It's a problem the PCI *host adaptor* has to deal with when it's figuring out how to map those regions into the container which corresponds to its area of the address space. Issue is, a PCI device overlapping something else suddenly becomes this something else's problem. It is not a problem at all. Maybe we should take the printf() about subregion collisions in memory_region_add_subregion_common() out of the #if 0 that it currently sits in? This is just a debugging tool, it won't fix anything. It might tell us what bits of code are currently erroneously mapping regions that overlap without using the _overlap() function. Then we could fix them. -- PMM When there is a single guest programmable device, any address can be overlapped by it. No. Only addresses within the same container. Other containers work fine without overlap. We could invent rules like 'non overlappable is higher priority' but it seems completely arbitrary, a single priority is clearer. It's just noise for the xx% of cases which don't need it.
Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available
On 02/14/2013 11:41 AM, Eduardo Habkost wrote: I'd glad to amend patch, if there are suggestions how to generalize it and improve create boot image process for distros that doesn't have grub2-mkrescue. A reusable tool/script would be useful even outside virt-test. Anybody who uses -kernel/-initrd today may want a more efficient solution (IIRC, virt-install uses -kernel/-initrd when installing from a Fedora repository URL). But if we made that an independent tool we would have the additional problem of having the new tool packaged and installed on the host. So having something specific inside virt-test makes sense by now. Absolutely, agreed.
Re: [Qemu-devel] [RFC PATCH v2 01/23] qcow2: Handle dependencies earlier
On Wed, Feb 13, 2013 at 02:21:51PM +0100, Kevin Wolf wrote: @@ -882,20 +876,55 @@ int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset, trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset, n_start, n_end); -/* Find L2 entry for the first involved cluster */ again: -ret = get_cluster_table(bs, offset, l2_table, l2_index); -if (ret 0) { -return ret; -} - /* * Calculate the number of clusters to look for. We stop at L2 table * boundaries to keep things simple. */ +l2_index = offset_to_l2_index(s, offset); nb_clusters = MIN(size_to_clusters(s, n_end BDRV_SECTOR_BITS), s-l2_size - l2_index); +/* + * Now start gathering as many contiguous clusters as possible: + * + * 1. Check for overlaps with in-flight allocations + * + * a) Overlap not in the first cluster - shorten this request and let + * the caller handle the rest in its next loop iteration. + * + * b) Real overlaps of two requests. Yield and restart the search for + * contiguous clusters (the situation could have changed while we + * were sleeping) + * + * c) TODO: Request starts in the same cluster as the in-flight + * allocation ends. Shorten the COW of the in-fight allocation, set + * cluster_offset to write to the same cluster and set up the right + * synchronisation between the in-flight request and the new one. + * + * 2. Count contiguous COPIED clusters. + *TODO: Consider cluster_offset if set in step 1c. + * + * 3. If the request still hasn't completed, allocate new clusters, + *considering any cluster_offset of steps 1c or 2. + */ +ret = handle_dependencies(bs, offset, nb_clusters); +if (ret == -EAGAIN) { +goto again; Now we might be able to replace the again label with a while loop. do { l2_index = offset_to_l2_index(s, offset); nb_clusters = MIN(size_to_clusters(s, n_end BDRV_SECTOR_BITS), s-l2_size - l2_index); } while ((ret = handle_dependencies(bs, offset, nb_clusters)) == -EAGAIN); Not essential, but perhaps clearer.
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On 14 February 2013 14:02, Michael S. Tsirkin m...@redhat.com wrote: Well that's the status quo. One of the issues is, you have no idea what else uses each priority. With this change, at least you can grep for it. No, because most of the code you find will be setting priorities for completely irrelevant containers (for instance PCI doesn't care at all about priorities used by the v7m NVIC). Imagine the specific example: ioapic and pci devices. ioapic has an address within the pci hole but it is not a subregion. If priority has no meaning how would you decide which one to use? I don't know about the specifics of the PC's memory layout, but *something* has to manage the address space that is being set up. I would expect something like: * PCI host controller has a memory region (container) which all the PCI devices are mapped into as per guest programming * ioapic has a memory region * there is another container which contains both these memory regions. The code that controls and sets up that container [which is probably the pc board model] gets to decide priorities, which are purely local to it (It's possible that at the moment the another container is the get_system_memory() system address space. If it makes life easier you can always invent another container to give you a fresh level of indirection.) Also, on a PC many addresses are guest programmable. We need to behave in some defined way if guest programs addresses to something silly. Yes, this is the job of the code controlling the container(s) into which those memory regions may be mapped. If the guest can program overlap then presumably PCI specifies semantics for what happens then, and there need to be PCI specific wrappers that enforce those semantics and they can call the relevant _overlap functions when mapping things. In any case this isn't a concern for the PCI *device*, which can just provide its memory regions. It's a problem the PCI *host adaptor* has to deal with when it's figuring out how to map those regions into the container which corresponds to its area of the address space. Issue is, a PCI device overlapping something else suddenly becomes this something else's problem. Nope, because the PCI host controller model should be in complete control of the container all the PCI devices live in, and it is the thing doing the mapping and unmapping so it gets to set priorities and mark things as OK to overlap. Also, memory.c permits overlap if either of the two memory regions in question is marked as may-overlap; they don't both have to be marked. We could add a wrapper for MEMORY_PRIO_LOWEST - will that address your concern? Well, I'm entirely happy with the memory API we have at the moment, and I'm trying to figure out why you want to change it... I am guessing your systems all have hardcoded addresses not controlled by guest. Nope. omap_gpmc.c for instance has guest programmable subregions; it uses a container so the guest's manipulation of these can't leak out and cause weird things to happen to other bits of QEMU. [I think we don't implement the correct guest-facing behaviour when the guest asks for overlapping regions, but we shouldn't hit the memory.c overlapping-region issue, or if we do it's a bug to be fixed.] There's also PCI on the versatilepb, but PCI devices can't just appear anywhere, the PCI memory windows are at known addresses and the PCI device can't escape from the wrong side of the PCI controller. Maybe we should take the printf() about subregion collisions in memory_region_add_subregion_common() out of the #if 0 that it currently sits in? This is just a debugging tool, it won't fix anything. It might tell us what bits of code are currently erroneously mapping regions that overlap without using the _overlap() function. Then we could fix them. When there is a single guest programmable device, any address can be overlapped by it. Do we really have an example of a guest programmable device where the *device itself* decides where it lives in the address space, rather than the guest being able to program a host controller/bus fabric/equivalent thing to specify where the device should live, or the device effectively negotiating with its bus controller? That seems very implausible to me just because hardware itself generally has some kind of hierarchy of buses and it's not really possible for a leaf node to make itself appear anywhere in the hierarchy; all it can do is by agreement with the thing above it appear at some different address at the same level. [of course there are trivial systems with a totally flat bus but that's just a degenerate case of the above where there's only one thing (the board) managing a single layer, and typically those systems have everything at a fixed address anyhow.] -- PMM
Re: [Qemu-devel] [RFC PATCH 03/10] qemu-ga: Add an configure option to specify path to Windows VSS SDK
Il 14/02/2013 07:10, Tomoki Sekiyama ha scritto: To enable VSS support in qemu-ga for Windows, header files included in VSS SDK is required. The VSS support is enabled when the option like below: ./configure --with-vss-sdk=/pass/to/VSS SDK VSS SDK is available from: http://www.microsoft.com/en-us/download/details.aspx?id=23490 To cross-compilie using mingw32 for Linux, you need to setup the SDK on Windows environments to extract headers. You can also use wine to run the setup of SDK on Linux etc. You can also use msitools (https://live.gnome.org/msitools; right now they are not packaged for any distro, but will be in Fedora soon): - #! /bin/bash # extract-vsssdk-headers # Author: Paolo Bonzini pbonz...@redhat.com set -e if test $# = 0 || ! test -f $1; then echo 'Usage: extract-vsssdk-headers /path/to/setup.exe exit 1 fi # Extract .MSI file in the .exe, looking for the OLE compound # document signature. Extra data at the end does not matter. export LC_ALL=C MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1' offset=`grep -abom1 $MAGIC setup.exe | sed -n 's/:/\n/; P' ` (dd of=/dev/null skip=$offset bs=1 count=0; cat) $1 vsssdk.msi # Now extract the files. tmpdir=tmp$$ mkdir $tmpdir msiextract -C $tmpdir vsssdk.msi mv $tmpdir/Program Files/Microsoft/VSSSDK72/inc inc rm -rf $tmpdir vsssdk.msi exit 0 - Can you add this in scripts/extract-vsssdk-headers please? ## +# check if we have VSS SDK headers for win + +if test $mingw32 = yes -a $guest_agent = yes ; then + case $vss_win32_sdk in +) vss_win32_include= ;; +*\ *) # The SDK is installed in Program Files by default, but we cannot + # handle path with spaces. So we copy the headers into .sdk/sdk. + vss_win32_include=-I$source_path/.sdk/vss + symlink $vss_win32_sdk/inc $source_path/.sdk/vss/inc + ;; +*)vss_win32_include=-I$vss_win32_sdk + esac Please also add support for these: --with-vss-sdk=no and --without-vss-sdk to disable VSS --with-vss-sdk (with no path) is the same as --with-vss-sdk=, but should fail if the program does not compile. The default should be what you have now, i.e. test and proceed according to the result. + cat $TMPC EOF +#define __MIDL_user_allocate_free_DEFINED__ +#include inc/win2003/vss.h +int main(void) { return VSS_CTX_BACKUP; } +EOF + if compile_prog $vss_win32_include ; then +guest_agent_with_vss=yes +QEMU_CFLAGS=$QEMU_CFLAGS $vss_win32_include +libs_qga=-lole32 -loleaut32 -lshlwapi -luuid -lstdc++ -Wl,--enable-stdcall-fixup $libs_qga + else +if test $vss_win32_sdk != ; then + echo ERROR: Please download and install Microsoft VSS SDK from + echo ERROR: http://www.microsoft.com/en-us/download/details.aspx?id=23490; Please add a note here detailing how to extract the headers on POSIX systems. Paolo + feature_not_found VSS support +fi +guest_agent_with_vss=no + fi +fi + +## ## # check if we have fdatasync @@ -3343,6 +3380,7 @@ echo usb net redir $usb_redir echo OpenGL support$opengl echo libiscsi support $libiscsi echo build guest agent $guest_agent +echo QGA VSS support $guest_agent_with_vss echo seccomp support $seccomp echo coroutine backend $coroutine_backend echo GlusterFS support $glusterfs @@ -3404,6 +3442,9 @@ if test $mingw32 = yes ; then version_micro=0 echo CONFIG_FILEVERSION=$version_major,$version_minor,$version_subminor,$version_micro $config_host_mak echo CONFIG_PRODUCTVERSION=$version_major,$version_minor,$version_subminor,$version_micro $config_host_mak + if test $guest_agent_with_vss = yes ; then +echo CONFIG_QGA_VSS=y $config_host_mak + fi else echo CONFIG_POSIX=y $config_host_mak fi
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On Thu, Feb 14, 2013 at 04:14:39PM +0200, Avi Kivity wrote: On Thu, Feb 14, 2013 at 3:09 PM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 14, 2013 at 12:56:02PM +, Peter Maydell wrote: On 14 February 2013 12:45, Michael S. Tsirkin m...@redhat.com wrote: overlap flag in the region is currently unused, most devices have no idea whether their region overlaps with anything, so drop it, assume that all regions can overlap and always require priority. Devices themselves shouldn't care, for the most part -- they just provide a memory region and it's their parent that has to map it and know whether it overlaps or not. Similarly, parents should generally be in control of the container they're mapping the memory region into, and know whether it will be an overlapping map or not. It's also not clear how should devices allocate priorities. Up to the parent which controls the region being mapped into. We could just assume same priority as parent but what happens if it has to be different? Priority is only considered relative to siblings. The parent's priority is only considered wrt the parent's siblings, not its children. But some parents are system created and shared by many devices so children for such have no idea who their siblings are. Please take a look at the typical map in this mail: '[BUG] Guest OS hangs on boot when 64bit BAR present' system overlap 0 pri 0 [0x0 - 0x7fff] kvmvapic-rom overlap 1 pri 1000 [0xca000 - 0xcd000] pc.ram overlap 0 pri 0 [0xca000 - 0xcd000] ++ pc.ram [0xca000 - 0xcd000] is added to view smram-region overlap 1 pri 1 [0xa - 0xc] pci overlap 0 pri 0 [0xa - 0xc] cirrus-lowmem-container overlap 1 pri 1 [0xa - 0xc] cirrus-low-memory overlap 0 pri 0 [0xa - 0xc] ++cirrus-low-memory [0xa - 0xc] is added to view kvm-ioapic overlap 0 pri 0 [0xfec0 - 0xfec01000] ++kvm-ioapic [0xfec0 - 0xfec01000] is added to view pci-hole64 overlap 0 pri 0 [0x1 - 0x4001] pci overlap 0 pri 0 [0x1 - 0x4001] pci-hole overlap 0 pri 0 [0x7d00 - 0x1] pci overlap 0 pri 0 [0x7d00 - 0x1] ivshmem-bar2-container overlap 1 pri 1 [0xfe00 - 0x1] ivshmem.bar2 overlap 0 pri 0 [0xfe00 - 0x1] ++ivshmem.bar2 [0xfe00 - 0xfec0] is added to view ++ivshmem.bar2 [0xfec01000 - 0x1] is added to view As you see, ioapic at 0xfec0 overlaps pci-hole. ioapic is guest programmable in theory - should use _overlap? pci-hole is not but can overlap with ioapic. So also _overlap? Let's imagine someone writes a guest programmable device for ARM. Now we should update all ARM devices from regular to _overlap? There are also aliases so a region can have multiple parents. Presumably it will have to have different priorities depending on what the parent does? The alias region has its own priority Here's a list of instances using priority != 0. hw/armv7m_nvic.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:MEMORY_PRIO_LOW); hw/cirrus_vga.c:s-low_mem_container, MEMORY_PRIO_LOW); hw/kvm/pci-assign.c: r_dev-mmio, MEMORY_PRIO_LOW); hw/kvmvapic.c:memory_region_add_subregion(as, rom_paddr, s-rom, MEMORY_PRIO_HIGH); hw/lpc_ich9.c:MEMORY_PRIO_LOW); hw/onenand.c:s-mapped_ram, MEMORY_PRIO_LOW); hw/pam.c:MEMORY_PRIO_LOW); hw/pc.c:MEMORY_PRIO_LOW); hw/pc_sysfw.c:isa_bios, MEMORY_PRIO_LOW); hw/pc_sysfw.c:isa_bios, MEMORY_PRIO_LOW); hw/pci/pci.c:MEMORY_PRIO_LOW); hw/pci/pci_bridge.c:memory_region_add_subregion(parent_space, base, alias, MEMORY_PRIO_LOW); hw/piix_pci.c:MEMORY_PRIO_LOW); hw/piix_pci.c:d-rcr_mem, MEMORY_PRIO_LOW); hw/q35.c:mch-smram_region, MEMORY_PRIO_LOW); hw/vga-isa.c:MEMORY_PRIO_LOW); hw/vga.c:MEMORY_PRIO_MEDIUM); hw/vga.c:vga_io_memory, MEMORY_PRIO_LOW); hw/xen_pt_msi.c:MEMORY_PRIO_MEDIUM); /* Priority: pci default + 1 Making priority relative to parent but not the same just seems like a recipe for disaster. I definitely don't like making the priority argument mandatory: this is just introducing
Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available
On Thu, Feb 14, 2013 at 02:31:38PM +0100, Paolo Bonzini wrote: Il 14/02/2013 14:24, Eduardo Habkost ha scritto: On Thu, Feb 14, 2013 at 01:13:18PM +0100, Paolo Bonzini wrote: Il 14/02/2013 12:18, Eduardo Habkost ha scritto: qemu boots from disk image 3 times faster than direct kernel load. That's surprising. Do you have any idea why that happens? Because kernel load uses MMIO (from fw_cfg), while booting from disk uses at worst PCI DMA and at best virtio. Is it something worth trying to optimize I think that, within the limits of what the spec makes legal, Gleb optimized all that he could out of it. The alternative is to make fw_cfg do DMA, which in the past was rejected because it doesn't look like what real ISA hardware would do. , or a reasonable solution would be so similar to having a disk+bootloader that's easier to simply recommend people to set up a real disk with a real bootloader if they care about speed? In the end it's a pity, but yeah that's the easiest thing to do with distro kernels and big all-drivers initrd. -kernel is still useful and fast enough if you have a custom-built kernel, possibly with no initrd at all. The patch that originated this thread wasn't even for distro kernels and big initrds. Our test case lods a very small test kernel (11 KB), and it is taking almost 15 seconds to boot. Maybe our test case should create a custom BIOS image to be loaded on ROM, instead? -- Eduardo
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On Thu, Feb 14, 2013 at 02:34:20PM +, Peter Maydell wrote: On 14 February 2013 14:02, Michael S. Tsirkin m...@redhat.com wrote: Well that's the status quo. One of the issues is, you have no idea what else uses each priority. With this change, at least you can grep for it. No, because most of the code you find will be setting priorities for completely irrelevant containers (for instance PCI doesn't care at all about priorities used by the v7m NVIC). Imagine the specific example: ioapic and pci devices. ioapic has an address within the pci hole but it is not a subregion. If priority has no meaning how would you decide which one to use? I don't know about the specifics of the PC's memory layout, but *something* has to manage the address space that is being set up. I would expect something like: * PCI host controller has a memory region (container) which all the PCI devices are mapped into as per guest programming * ioapic has a memory region * there is another container which contains both these memory regions. The code that controls and sets up that container [which is probably the pc board model] gets to decide priorities, which are purely local to it This assumes we set up devices in code. We are trying to move away from that, and have APIs that let you set up boards from command line. (It's possible that at the moment the another container is the get_system_memory() system address space. If it makes life easier you can always invent another container to give you a fresh level of indirection.) Also, on a PC many addresses are guest programmable. We need to behave in some defined way if guest programs addresses to something silly. Yes, this is the job of the code controlling the container(s) into which those memory regions may be mapped. Some containers don't know what is mapped into them. If the guest can program overlap then presumably PCI specifies semantics for what happens then, and there need to be PCI specific wrappers that enforce those semantics and they can call the relevant _overlap functions when mapping things. In any case this isn't a concern for the PCI *device*, which can just provide its memory regions. It's a problem the PCI *host adaptor* has to deal with when it's figuring out how to map those regions into the container which corresponds to its area of the address space. Issue is, a PCI device overlapping something else suddenly becomes this something else's problem. Nope, because the PCI host controller model should be in complete control of the container all the PCI devices live in, and it is the thing doing the mapping and unmapping so it gets to set priorities and mark things as OK to overlap. Also, memory.c permits overlap if either of the two memory regions in question is marked as may-overlap; they don't both have to be marked. That's undocumented, isn't it? And then which one wins? We could add a wrapper for MEMORY_PRIO_LOWEST - will that address your concern? Well, I'm entirely happy with the memory API we have at the moment, and I'm trying to figure out why you want to change it... I am guessing your systems all have hardcoded addresses not controlled by guest. Nope. omap_gpmc.c for instance has guest programmable subregions; it uses a container so the guest's manipulation of these can't leak out and cause weird things to happen to other bits of QEMU. [I think we don't implement the correct guest-facing behaviour when the guest asks for overlapping regions, but we shouldn't hit the memory.c overlapping-region issue, or if we do it's a bug to be fixed.] There's also PCI on the versatilepb, but PCI devices can't just appear anywhere, the PCI memory windows are at known addresses and the PCI device can't escape from the wrong side of the PCI controller. But, there are devices who's addresses can overlap the PCI window. Maybe we should take the printf() about subregion collisions in memory_region_add_subregion_common() out of the #if 0 that it currently sits in? This is just a debugging tool, it won't fix anything. It might tell us what bits of code are currently erroneously mapping regions that overlap without using the _overlap() function. Then we could fix them. When there is a single guest programmable device, any address can be overlapped by it. Do we really have an example of a guest programmable device where the *device itself* decides where it lives in the address space, rather than the guest being able to program a host controller/bus fabric/equivalent thing to specify where the device should live, or the device effectively negotiating with its bus controller? That seems very implausible to me just because hardware itself generally has some kind of hierarchy of buses and it's not really possible for a leaf node to make itself appear anywhere in the hierarchy; all it can do is by
Re: [Qemu-devel] [RFC PATCH v2 03/23] qcow2: Change handle_dependency to byte granularity
On Wed, Feb 13, 2013 at 02:21:53PM +0100, Kevin Wolf wrote: Signed-off-by: Kevin Wolf kw...@redhat.com --- block/qcow2-cluster.c | 40 block/qcow2.h | 11 +++ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 0e804ba..a3b2447 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -756,31 +756,41 @@ out: * Check if there already is an AIO write request in flight which allocates * the same cluster. In this case we need to wait until the previous * request has completed and updated the L2 table accordingly. + * + * Returns: + * 0 if there was no dependency. *cur_bytes indicates the number of + * bytes from guest_offset that can be read before the next + * dependency must be processed (or the request is complete) s/read/accessed/ IMO read is too specific, the doc comment doesn't need to contain knowledge of what the caller will do at guest_offset. + * + * -EAGAIN if we had to wait for another request, previously gathered + * information on cluster allocation may be invalid now. The caller + * must start over anyway, so consider *cur_bytes undefined. */ static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset, -unsigned int *nb_clusters) +uint64_t *cur_bytes) { BDRVQcowState *s = bs-opaque; QCowL2Meta *old_alloc; +uint64_t bytes = *cur_bytes; QLIST_FOREACH(old_alloc, s-cluster_allocs, next_in_flight) { -uint64_t start = guest_offset s-cluster_bits; -uint64_t end = start + *nb_clusters; -uint64_t old_start = old_alloc-offset s-cluster_bits; -uint64_t old_end = old_start + old_alloc-nb_clusters; +uint64_t start = guest_offset; I'm not sure this is safe. Previously the function caught write requests which overlap at cluster granularity, now it will allow them? Stefan
Re: [Qemu-devel] [PATCH moxie 2/5] Add moxie disassembler
Il 13/02/2013 23:21, Anthony Green ha scritto: +/* Disassemble moxie instructions. + Copyright 2009 + Free Software Foundation, Inc. + + This file is part of the GNU opcodes library. + + This library is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3, or (at your option) + any later version. + + It is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public + License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, + MA 02110-1301, USA. */ + +#include stdio.h Unfortunately, large part of QEMU are uner the GPLv2. Are you the sole author of this file, so that (per the FSF copyright agreement) you can relicense it to GPLv2+? Paolo
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On Thu, Feb 14, 2013 at 4:40 PM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 14, 2013 at 04:14:39PM +0200, Avi Kivity wrote: But some parents are system created and shared by many devices so children for such have no idea who their siblings are. Please take a look at the typical map in this mail: '[BUG] Guest OS hangs on boot when 64bit BAR present' system overlap 0 pri 0 [0x0 - 0x7fff] kvmvapic-rom overlap 1 pri 1000 [0xca000 - 0xcd000] pc.ram overlap 0 pri 0 [0xca000 - 0xcd000] ++ pc.ram [0xca000 - 0xcd000] is added to view smram-region overlap 1 pri 1 [0xa - 0xc] pci overlap 0 pri 0 [0xa - 0xc] cirrus-lowmem-container overlap 1 pri 1 [0xa - 0xc] cirrus-low-memory overlap 0 pri 0 [0xa - 0xc] ++cirrus-low-memory [0xa - 0xc] is added to view kvm-ioapic overlap 0 pri 0 [0xfec0 - 0xfec01000] ++kvm-ioapic [0xfec0 - 0xfec01000] is added to view pci-hole64 overlap 0 pri 0 [0x1 - 0x4001] pci overlap 0 pri 0 [0x1 - 0x4001] pci-hole overlap 0 pri 0 [0x7d00 - 0x1] pci overlap 0 pri 0 [0x7d00 - 0x1] ivshmem-bar2-container overlap 1 pri 1 [0xfe00 - 0x1] ivshmem.bar2 overlap 0 pri 0 [0xfe00 - 0x1] ++ivshmem.bar2 [0xfe00 - 0xfec0] is added to view ++ivshmem.bar2 [0xfec01000 - 0x1] is added to view As you see, ioapic at 0xfec0 overlaps pci-hole. ioapic is guest programmable in theory - should use _overlap? pci-hole is not but can overlap with ioapic. So also _overlap? It's a bug. The ioapic is in the pci address space, not the system address space. And yes it's overlappable. Let's imagine someone writes a guest programmable device for ARM. Now we should update all ARM devices from regular to _overlap? It's sufficient to update the programmable device. Non overlapping is not a common case at all. E.g. with normal PCI devices you have no way to know nothing overlaps - addresses are guest programmable. Non overlapping is mostly useful for embedded platforms. Maybe it should have a longer name like _nonoverlap then? Current API makes people assume _overlap is only for special cases and default should be non overlap. The assumption is correct.
Re: [Qemu-devel] [PATCH 03/10] hbitmap: Use non-bitops ctzl
Il 14/02/2013 02:47, Richard Henderson ha scritto: Both uses of ctz have already eliminated zero, and thus the difference in edge conditions between the two routines is irrelevant. Signed-off-by: Richard Henderson r...@twiddle.net --- include/qemu/hbitmap.h | 3 ++- util/hbitmap.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h index 250de03..550d7ce 100644 --- a/include/qemu/hbitmap.h +++ b/include/qemu/hbitmap.h @@ -16,6 +16,7 @@ #include stdint.h #include stdbool.h #include bitops.h +#include host-utils.h typedef struct HBitmap HBitmap; typedef struct HBitmapIter HBitmapIter; @@ -170,7 +171,7 @@ static inline int64_t hbitmap_iter_next(HBitmapIter *hbi) /* The next call will resume work from the next bit. */ hbi-cur[HBITMAP_LEVELS - 1] = cur (cur - 1); -item = ((uint64_t)hbi-pos BITS_PER_LEVEL) + bitops_ctzl(cur); +item = ((uint64_t)hbi-pos BITS_PER_LEVEL) + ctzl(cur); return item hbi-granularity; } diff --git a/util/hbitmap.c b/util/hbitmap.c index a0df5d3..d936831 100644 --- a/util/hbitmap.c +++ b/util/hbitmap.c @@ -126,7 +126,8 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi) * The index of this word's least significant set bit provides * the low-order bits. */ -pos = (pos BITS_PER_LEVEL) + bitops_ctzl(cur); +assert(cur); +pos = (pos BITS_PER_LEVEL) + ctzl(cur); hbi-cur[i] = cur (cur - 1); /* Set up next level for iteration. */ Acked-by: Paolo Bonzini pbonz...@redhat.com
Re: [Qemu-devel] [RFC v5] target-i386: Slim conversion to X86CPU subclasses + KVM subclasses
On Thu, 14 Feb 2013 09:44:59 -0200 Eduardo Habkost ehabk...@redhat.com wrote: On Wed, Feb 13, 2013 at 04:33:33PM +0100, Igor Mammedov wrote: From: Andreas Färber afaer...@suse.de Depends on http://lists.gnu.org/archive/html/qemu-devel/2013-02/msg00677.html Move x86_def_t definition to header and embed into X86CPUClass. Register types per built-in model definition. Move version initialization from x86_cpudef_setup() to class_init(). Move default setting of CPUID_EXT_HYPERVISOR to class_init(). Move KVM specific built-in CPU defaults overrides in a kvm specific x86_cpu_kvm_def_class_init(). And select TCG vs KVM class of CPU to create at runtime in x86_cpu_class_by_name() when kvm_enable() is available. Inline cpu_x86_register() into the X86CPU initfn. Since instance_init cannot reports errors, die there if some of default values are incorrect, instead of ignoring errors. Replace cpu_x86_find_by_name() with x86_cpu_class_by_name(). Move handling of KVM host vendor override from cpu_x86_find_by_name() to the kvm_arch_init() and class_init(). Use TYPE_X86_CPU class to communicate kvm specific defaults to other sub-classes. Register host-kvm-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs and only when KVM is enabled to avoid workarounds in name to class lookup code in x86_cpu_class_by_name(). Make kvm_cpu_fill_host() into a host specific class_init and inline cpu_x86_fill_model_id(). Let kvm_check_features_against_host() obtain host-kvm-{i386,86_64}-cpu for comparison. Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Igor Mammedov imamm...@redhat.com --- v5: * remove special case for 'host' CPU check in x86_cpu_class_by_name(), due to 'host' CPU will not find anything if not in KVM mode or return 'host' CPU class in KVM mode, i.e. treat it as regular CPUs. * register KVM specific subclasses for built-in CPU models. * abort() in instance_init() if property setter fails to set default value. v4: * set error if cpu model is not found and goto out; * copy vendor override from 'host' CPU class in sub-class'es class_init() if 'host' CPU class is available. * register type TYPE_HOST_X86_CPU in kvm_arch_init(), this type should be available only in KVM mode and we haven't printed it in -cpu ? output so far, so we can continue doing so. It's not really confusing to show 'host' cpu (even if we do it) when KVM is not enabled. --- target-i386/cpu-qom.h | 24 target-i386/cpu.c | 348 +++-- target-i386/cpu.h |5 +- target-i386/kvm.c | 72 ++ 4 files changed, 232 insertions(+), 217 deletions(-) [...] diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index 48e6b54..c8f320d 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -30,6 +30,27 @@ #define TYPE_X86_CPU i386-cpu #endif +#define TYPE_HOST_X86_CPU host-kvm- TYPE_X86_CPU + [...] if (name == NULL) { -return -1; -} -if (kvm_enabled() strcmp(name, host) == 0) { -kvm_cpu_fill_host(x86_cpu_def); -return 0; +return NULL; } -for (i = 0; i ARRAY_SIZE(builtin_x86_defs); i++) { -def = builtin_x86_defs[i]; -if (strcmp(name, def-name) == 0) { -memcpy(x86_cpu_def, def, sizeof(*def)); -/* sysenter isn't supported in compatibility mode on AMD, - * syscall isn't supported in compatibility mode on Intel. - * Normally we advertise the actual CPU vendor, but you can - * override this using the 'vendor' property if you want to use - * KVM's sysenter/syscall emulation in compatibility mode and - * when doing cross vendor migration - */ -if (kvm_enabled()) { -uint32_t ebx = 0, ecx = 0, edx = 0; -host_cpuid(0, 0, NULL, ebx, ecx, edx); -x86_cpu_vendor_words2str(x86_cpu_def-vendor, ebx, edx, ecx); -} -return 0; -} +if (kvm_enabled()) { +typename = g_strdup_printf(%s-kvm- TYPE_X86_CPU, name); * Could we use the -tcg suffix for the non-KVM mode? sure, I'll add it for the next re-spin. * Can we make the code fall back to no-suffix CPU names? We need to add the -kvm suffix (and maybe a -tcg suffix) to keep compatibility with existing CPU models, but maybe we should deprecate the automatic -tcg/-kvm suffixing and ask users to specify the full CPU name. I'm not sure that I got you right, Have you meant something like this: if (kvm) { typename = name-kvm-... } else { typename = name-tcg-... } oc = object_class_by_name(typename) if (!oc) { oc = object_class_by_name(name) } [...] @@ -2234,7 +2075,57 @@ static void
Re: [Qemu-devel] Google Summer of Code 2013 ideas wiki open
On Thu, Feb 14, 2013 at 11:39 AM, harryxiyou harryxi...@gmail.com wrote: On Tue, Feb 12, 2013 at 5:21 AM, Stefan Hajnoczi stefa...@gmail.com wrote: On Thu, Feb 7, 2013 at 4:19 PM, Stefan Hajnoczi stefa...@gmail.com wrote: I believe Google will announce GSoC again this year (there is no guarantee though) and I have created the wiki page so we can begin organizing project ideas that students can choose from. Google Summer of Code 2013 has just been announced! http://google-opensource.blogspot.de/2013/02/flip-bits-not-burgers-google-summer-of.html Some project ideas have already been discussed on IRC or private emails. Please go ahead and put them on the project ideas wiki page: http://wiki.qemu.org/Google_Summer_of_Code_2013 I am a senior student and wanna do some jobs about storage in Libvirt in GSOC 2013. I wonder whether Libvirt and QEMU will join GSOC 2013 together. If true, i will focus on http://wiki.qemu.org/Google_Summer_of_Code_2013 and add myself introductions to QEMU links said by Stefan Hajnoczi. Could anyone give me some suggestions? Thanks in advance. Hi Harry, Thanks for your interest. You can begin thinking about ideas but please keep in mind that we are still in the very early stages of GSoC preparation. Google will publish the list of accepted organizations on April 8th. Then there is a period of over 3 weeks to discuss your project idea with the organization. In the meantime, the best thing to do is to get familiar with the code bases and see if you can find/fix a bug. Contributing patches is a great way to get noticed. There is always a chance that QEMU and/or libvirt may not be among the list of accepted organizations, so don't put all your eggs in one basket :). Stefan
Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output
CC'ing some more people from the debug output revamp RFC discussion. Am 11.02.2013 20:01, schrieb Andreas Färber: From: Andreas Färber andreas.faer...@web.de Signed-off-by: Andreas Färber andreas.faer...@web.de --- hw/tmp105.c | 27 +-- 1 Datei geändert, 25 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-) diff --git a/hw/tmp105.c b/hw/tmp105.c index 3ad2d2f..5dafa37 100644 --- a/hw/tmp105.c +++ b/hw/tmp105.c @@ -23,6 +23,18 @@ #include tmp105.h #include qapi/visitor.h +#undef DEBUG_TMP105 + +static inline void GCC_FMT_ATTR(1, 2) DPRINTF(const char *fmt, ...) Being an inline function, I think it would be better to name this tmp105_dprintf() and alias via: #define DPRINTF tmp105_dprintf Assuming we want an uppercase conditional-output statement in the first place? dprintf() as used in some files (sheepdog, sPAPR, SPICE, target-{i386,ppc,s390x}/kvm.c) is already used by system headers on some platforms, so I'd rather avoid it. Any other comments or suggestions? I'm preparing to respin the above series in a similar, less-invasive style. Thanks, Andreas +{ +#ifdef DEBUG_TMP105 +va_list ap; +va_start(ap, fmt); +vfprintf(stderr, fmt, ap); +va_end(ap); +#endif +} + static void tmp105_interrupt_update(TMP105State *s) { qemu_set_irq(s-pin, s-alarm ^ ((~s-config 2) 1));/* POL */ @@ -115,10 +127,16 @@ static void tmp105_read(TMP105State *s) s-buf[s-len ++] = ((uint16_t) s-limit[1]) 0; break; } + +DPRINTF(%s: %02 PRIx8 %02 PRIx8 %02 PRIx8 \n, +__func__, s-pointer, s-buf[0], s-buf[1]); } static void tmp105_write(TMP105State *s) { +DPRINTF(%s: %02 PRIx8 %02 PRIx8 %02 PRIx8 \n, +__func__, s-pointer, s-buf[0], s-buf[1]); + switch (s-pointer 3) { case TMP105_REG_TEMPERATURE: break; @@ -144,18 +162,23 @@ static void tmp105_write(TMP105State *s) static int tmp105_rx(I2CSlave *i2c) { TMP105State *s = TMP105(i2c); +int ret; if (s-len 2) { -return s-buf[s-len ++]; +ret = s-buf[s-len++]; } else { -return 0xff; +ret = 0xff; } +DPRINTF(%s: - %02x\n, __func__, ret); +return ret; } static int tmp105_tx(I2CSlave *i2c, uint8_t data) { TMP105State *s = TMP105(i2c); +DPRINTF(%s: - %02 PRIx8 \n, __func__, data); + if (s-len == 0) { s-pointer = data; s-len++; -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH for-1.4 07/19] target-sparc: Fix debug output for DEBUG_MMU
Blue, Am 27.01.2013 14:32, schrieb Andreas Färber: Signed-off-by: Andreas Färber afaer...@suse.de --- target-sparc/ldst_helper.c |2 +- 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-) diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c index cf1bddf..7decd66 100644 --- a/target-sparc/ldst_helper.c +++ b/target-sparc/ldst_helper.c @@ -1850,7 +1850,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val, DPRINTF_MMU(LSU change: 0x% PRIx64 - 0x% PRIx64 \n, oldreg, env-lsu); #ifdef DEBUG_MMU -dump_mmu(stdout, fprintf, env1); +dump_mmu(stdout, fprintf, env); #endif tlb_flush(env, 1); } This patch was not applied to master. Was that an oversight or is something wrong with it? (Preparing a v2 series for 1.5.) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC v5] target-i386: Slim conversion to X86CPU subclasses + KVM subclasses
On Thu, Feb 14, 2013 at 04:13:22PM +0100, Igor Mammedov wrote: On Thu, 14 Feb 2013 09:44:59 -0200 Eduardo Habkost ehabk...@redhat.com wrote: [...] +if (kvm_enabled()) { +typename = g_strdup_printf(%s-kvm- TYPE_X86_CPU, name); * Could we use the -tcg suffix for the non-KVM mode? sure, I'll add it for the next re-spin. * Can we make the code fall back to no-suffix CPU names? We need to add the -kvm suffix (and maybe a -tcg suffix) to keep compatibility with existing CPU models, but maybe we should deprecate the automatic -tcg/-kvm suffixing and ask users to specify the full CPU name. I'm not sure that I got you right, Have you meant something like this: if (kvm) { typename = name-kvm-... } else { typename = name-tcg-... } oc = object_class_by_name(typename) if (!oc) { oc = object_class_by_name(name) } Yes, exactly. [...] + +} + +static void x86_cpu_def_class_init(ObjectClass *oc, void *data) +{ +X86CPUClass *xcc = X86_CPU_CLASS(oc); +x86_def_t *def = data; +int i; +static const char *versioned_models[] = { qemu32, qemu64, athlon }; + +memcpy(xcc-info, def, sizeof(x86_def_t)); +xcc-info.ext_features |= CPUID_EXT_HYPERVISOR; If this is TCG-specific now, we could make the class match the reality and mask TCG_FEATURES/TCG_EXT_FEATURES/etc here. That's what happens in practice, e.g.: -cpu SandyBridge in TCG mode is a completely different CPU, today. well, this function is shared between TCG and KVM, I mean, it's common code for both. Which asks for one more TCG class_init function for TCG specific behavior. Having both TCG-specific and KVM-specific subclasses instead of making the KVM class inherit from the TCG class would make sense to me, as we may have TCG-specific behavior in other places too. Or we could do memcpy() again on the KVM class_init function. Or we could set a different void* pointer on the TCG class, with already-filtered x86_def_t object. There are multiple possible solutions. But could it be a separate patch (i.e. fixing TCG filtering), I think just moving tcg filtering might cause regression. And need to worked on separately. I'm OK with doing that in a separate patch, as it is not a bug fix or behavior change. But it would be nice to do that before we introduce the feature properties, to make the reported defaults right since the beginning. [...] +#ifdef CONFIG_KVM +static void x86_cpu_kvm_def_class_init(ObjectClass *oc, void *data) +{ +uint32_t eax, ebx, ecx, edx; +X86CPUClass *xcc = X86_CPU_CLASS(oc); + +x86_cpu_def_class_init(oc, data); +/* sysenter isn't supported in compatibility mode on AMD, + * syscall isn't supported in compatibility mode on Intel. + * Normally we advertise the actual CPU vendor, but you can + * override this using the 'vendor' property if you want to use + * KVM's sysenter/syscall emulation in compatibility mode and + * when doing cross vendor migration + */ +host_cpuid(0x0, 0, eax, ebx, ecx, edx); Cool, this is exactly what I was going to suggest, to avoid depending on the host CPU class and KVM initialization. I see two options when making the vendor property static, and I don't know which one is preferable: One solution is the one in this patch: to call host_cpuid() here in class_init, and have a different property default depending on the host CPU. I prefer this one ^^^. Another solution is to have default to vendor=host, and have instance_init understand vendor=host as use the host CPU vendor. This would make the property default really static (not depending on the host CPU). I am inclined for the latter, because I am assuming that the QOM design expects class_init results to be completely static. This is not as bad as depending on KVM initialization, but it may be an unexpected surprise, or even something not allowed by QOM. That's where I have to disagree :) You might have a point with 'static' if our goal is for introspection for source level to work. But we have a number of issues with that: * model_id is not static for some cpus, 'vendor' is the same just for different set of classes model_id is static for a given QEMU version, isn't it? This may be OK (but I am _also_ not sure about this case). * we generate sub-classes at runtime dynamically, which makes source level introspection impossible for them. It's not source-level introspection I am looking for, but runtime introspection that won't surprise other components in the system by changing unexpectedly (e.g. when the host CPU changes). I think that QOM is aiming towards run-time introspection of classes/objects. And that allows us to define defaults (even generate them dynamically) at class_init() time, they don't have to be static constants. Yes, QOM introspection is run-time based. What
[Qemu-devel] single step not working after hit a break point
Hi All I use the following code to insert a breakpoint in physical address 0×160CPUArchState *cpu = first_cpu; hwaddr addr; sscanf(command + 2, %ld, addr); int err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL );qemu successfully hit the breakpoint and stop, then i try to single-step by the following code:CPUArchState *cpu = first_cpu; cpu_single_step(cpu, sstep_flags); vm_start();Nothing happened, the EIP still stay in 0×160, but if i delete the breakpoint, the single step just work again. Am I missed something? thanks
Re: [Qemu-devel] [PATCH v2 00/10] Cleanup bitops vs host-utils
On 02/13/2013 06:47 PM, Richard Henderson wrote: Version 1 merely tried to adjust bitops_flsl, here I instead eliminate it all from bitops.h, and standardizes on the routines from host-utils.h. r~ Richard Henderson (10): host-utils: Add host long specific aliases for clz, ctz, ctpop host-utils: Fix coding style and add comments hbitmap: Use non-bitops ctzl bitops: Use non-bitops ctzl memory: Use non-bitops ctzl bitops: Write bitops_flsl in terms of clzl target-i386: Inline bitops_flsl bitops: Inline bitops_flsl bitops: Replace bitops_ctol with ctzl bitops: Remove routines redundant with host-utils include/qemu/bitops.h | 75 - include/qemu/hbitmap.h| 3 +- include/qemu/host-utils.h | 119 +++--- memory.c | 4 +- target-i386/topology.h| 6 +-- util/bitops.c | 6 +-- util/hbitmap.c| 3 +- 7 files changed, 112 insertions(+), 104 deletions(-) Series: Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently
On Thu, 14 Feb 2013 14:31:50 +0100 Markus Armbruster arm...@redhat.com wrote: [Some quoted material restored] Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 14 Feb 2013 10:45:22 +0100 Markus Armbruster arm...@redhat.com wrote: [Note cc: +Laszlo, +Anthony, -qemu-trivial] Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 08 Feb 2013 20:34:20 +0100 Markus Armbruster arm...@redhat.com wrote: The real problem here is that the k, M, G suffixes, for example, are not good to be reported by QMP. So maybe we should refactor the code in a way that we separate what's done in QMP from what is done in HMP/command-line. Isn't it separated already? parse_option_size() is used when parsing key=value,... Such strings should not exist in QMP. If they do, it's a design bug. No and no. Such strings don't exist in QMP as far as can tell (see bugs below though), but parse_option_size() is theoretically present in a possible QMP call stack: qemu_opts_from_dict_1() qemu_opt_set_err() opt_set() qemu_opt_paser() parse_option_size() I can't tell if this will ever happen because qemu_opts_from_dict_1() restricts the call to qemu_opt_set_err() to certain values, but the fact that it's not clear is an indication that a better separation is necessary. Permit me two detours. One, qemu_opt_set_err() is a confusing name. I doesn't set an error. It takes an Error ** object and it was introduced to avoid having to convert qemu_opt_set() to take an Error ** object, as this would multiply the work required to get netdev_add converted to the qapi. Now, I pass the bikeshed :) I get why it's there, I just find its name confusing. [...] Now, to build hmp_device_add() on top of qmp_device_add(), we'd have to convert first from QemuOpts to QDict and then back to QemuOpts. Blech. Instead, we made do_device_add() go straight to qdev_device_add(). Same for hmp_netdev_add(): it goes straight to netdev_add(). Yes, the idea was to have netdev_add() and add frontends to hmp and qmp. More on this below. [...] I guess weird things can happen with qemu_opts_from_qdict(), at least in theory. For each (key, value) in the QDict, qemu_opts_from_qdict() converts value to string according to its qtype_code. The string then gets parsed according to key's QemuOptType. Yes, that means you can pass a QString to QEMU_OPT_SIZE option, and get the suffixes interpreted. However, we've only used qemu_opts_from_qdict() with QemuOptsLists that have empty desc[]! Specifically: qemu_netdev_opts and qemu_device_opts. Without desc, qemu_opt_parse() does nothing, and QemuOpts holds just string values. Actual parsing left to the consumer. Two consumers: qdev_device_add() and netdev_add(). netdev_add() uses QAPI wizardry to create the appropriate object from the QemuOpts. The parsing is done by visitors. Things get foggy for me from there on, but it looks like one of them, opts_type_size(), can parse size suffixes, which is inappropriate for QMP. A quick test confirms that this is indeed possible: {execute: netdev_add,arguments: {type:tap,id:net.1, sndbuf: 8k }} Sets NetdevTapOptions member sndbuf to 8192. Well, I've just tested this with commit 783e9b4, which is before netdev_add conversion to the qapi, and the command above just works. Didn't check if sndbuf is actually set to 8192, but this shows that we've always accepted such a command. Plausible. Nevertheless, we really shouldn't. Agreed. I would be willing to break compatibility to fix the suffix part of the problem, but there's another issue: sndbuf shouldn't be a string in QMP, and that's unfixable. To sum up, we go from QDict delivered by the JSON parser via QemuOpts to QAPI object, with a few cock-ups along the way. Ugh. By the way, the JSON schema reads { 'command': 'netdev_add', 'data': {'type': 'str', 'id': 'str', '*props': '**'}, 'gen': 'no' } I'll be hanged if I know what '**' means. For QMP on the wire it means that the command accepts a bunch of arguments not specified in the schema. Thanks! Is the schema language documented anywhere? Unfortunately not. Yes, it's a dirty trick. Long story short: we decide to maintain QemuOpts usage in both HMP and QMP to speed up netdev_add conversion. I don't think '**' is as dirty as you make it sound. It's simply a way to relax the rigidity of the schema. I got no problem with that. Problem is, I don't know how to make it better if we want to. I think we could use it for the old commands that depend on QemuOpts so that we don't make conversions too hard, but we shouldn't use it for new commands. qdev_device_add() uses a few well-known options itself, and they're all strings. The others it passes on to qdev_prop_parse(). This
Re: [Qemu-devel] [RFC PATCH v2 03/23] qcow2: Change handle_dependency to byte granularity
On Thu, Feb 14, 2013 at 03:48:51PM +0100, Stefan Hajnoczi wrote: On Wed, Feb 13, 2013 at 02:21:53PM +0100, Kevin Wolf wrote: Signed-off-by: Kevin Wolf kw...@redhat.com --- block/qcow2-cluster.c | 40 block/qcow2.h | 11 +++ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 0e804ba..a3b2447 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -756,31 +756,41 @@ out: * Check if there already is an AIO write request in flight which allocates * the same cluster. In this case we need to wait until the previous * request has completed and updated the L2 table accordingly. + * + * Returns: + * 0 if there was no dependency. *cur_bytes indicates the number of + * bytes from guest_offset that can be read before the next + * dependency must be processed (or the request is complete) s/read/accessed/ IMO read is too specific, the doc comment doesn't need to contain knowledge of what the caller will do at guest_offset. Right. In fact, read is even wrong, if anything it should be written to. + * + * -EAGAIN if we had to wait for another request, previously gathered + * information on cluster allocation may be invalid now. The caller + * must start over anyway, so consider *cur_bytes undefined. */ static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset, -unsigned int *nb_clusters) +uint64_t *cur_bytes) { BDRVQcowState *s = bs-opaque; QCowL2Meta *old_alloc; +uint64_t bytes = *cur_bytes; QLIST_FOREACH(old_alloc, s-cluster_allocs, next_in_flight) { -uint64_t start = guest_offset s-cluster_bits; -uint64_t end = start + *nb_clusters; -uint64_t old_start = old_alloc-offset s-cluster_bits; -uint64_t old_end = old_start + old_alloc-nb_clusters; +uint64_t start = guest_offset; I'm not sure this is safe. Previously the function caught write requests which overlap at cluster granularity, now it will allow them? At this point in the series, old_start and old_end are still aligned to cluster boundaries. So the cases in which an overlap is detected stay exactly the same with this patch. This is just a more precise description of what we really wanted to compare. Kevin
[Qemu-devel] test images for musicpal and milkymist?
Hi; I'm looking for test images (and command lines) for the musicpal and milkymist targets, because I have a patchset which makes some changes to them [I'm trying to get rid of sysbus_add_memory()]. Does anybody have any to hand? thanks -- PMM
Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant
On Thu, Feb 14, 2013 at 05:07:02PM +0200, Avi Kivity wrote: On Thu, Feb 14, 2013 at 4:40 PM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 14, 2013 at 04:14:39PM +0200, Avi Kivity wrote: But some parents are system created and shared by many devices so children for such have no idea who their siblings are. Please take a look at the typical map in this mail: '[BUG] Guest OS hangs on boot when 64bit BAR present' system overlap 0 pri 0 [0x0 - 0x7fff] kvmvapic-rom overlap 1 pri 1000 [0xca000 - 0xcd000] pc.ram overlap 0 pri 0 [0xca000 - 0xcd000] ++ pc.ram [0xca000 - 0xcd000] is added to view smram-region overlap 1 pri 1 [0xa - 0xc] pci overlap 0 pri 0 [0xa - 0xc] cirrus-lowmem-container overlap 1 pri 1 [0xa - 0xc] cirrus-low-memory overlap 0 pri 0 [0xa - 0xc] ++cirrus-low-memory [0xa - 0xc] is added to view kvm-ioapic overlap 0 pri 0 [0xfec0 - 0xfec01000] ++kvm-ioapic [0xfec0 - 0xfec01000] is added to view pci-hole64 overlap 0 pri 0 [0x1 - 0x4001] pci overlap 0 pri 0 [0x1 - 0x4001] pci-hole overlap 0 pri 0 [0x7d00 - 0x1] pci overlap 0 pri 0 [0x7d00 - 0x1] ivshmem-bar2-container overlap 1 pri 1 [0xfe00 - 0x1] ivshmem.bar2 overlap 0 pri 0 [0xfe00 - 0x1] ++ivshmem.bar2 [0xfe00 - 0xfec0] is added to view ++ivshmem.bar2 [0xfec01000 - 0x1] is added to view As you see, ioapic at 0xfec0 overlaps pci-hole. ioapic is guest programmable in theory - should use _overlap? pci-hole is not but can overlap with ioapic. So also _overlap? It's a bug. The ioapic is in the pci address space, not the system address space. And yes it's overlappable. So you want to put it where? Under pci-hole? And we'll have to teach all machine types creating pci-hole about it? Let's imagine someone writes a guest programmable device for ARM. Now we should update all ARM devices from regular to _overlap? It's sufficient to update the programmable device. Then the device can be higher priority (works for apic) but not lower priority. Make priority signed? Non overlapping is not a common case at all. E.g. with normal PCI devices you have no way to know nothing overlaps - addresses are guest programmable. Non overlapping is mostly useful for embedded platforms. Maybe it should have a longer name like _nonoverlap then? Current API makes people assume _overlap is only for special cases and default should be non overlap. The assumption is correct.