Re: [Qemu-devel] [PATCH 3/7] dataplane: add virtqueue vring code
On 15/11/12 16:19, Stefan Hajnoczi wrote:
> +#include "trace.h"
> +#include "hw/dataplane/vring.h"
> +
> +/* Map target physical address to host address
> + */
> +static inline void *phys_to_host(Vring *vring, hwaddr phys)
> +{
> +/* Adjust for 3.6-4 GB PCI memory range */
> +if (phys >= 0x1) {
> +phys -= 0x1 - 0xe000;
> +} else if (phys >= 0xe000) {
> +fprintf(stderr, "phys_to_host bad physical address in "
> +"PCI range %#lx\n", phys);
> +exit(1);
> +}
I think non-pci virtio also wants to use dataplane. Any chance to move such pci
specific things out of the main code?
Christian
Re: [Qemu-devel] [PATCH] iscsi: fix deadlock during login
Il 15/11/2012 19:28, Peter Lieven ha scritto: >>> >> I dont know if we should switch to use synchronous code here. >>> >> It is much nicer if all code is async. >> > >> > bdrv_open is generally synchronous, so I think Peter's patch is ok. > if all is sync wouldn't it be best to have all code in iscsi_open sync > then and convert the iscsi_inquiry and iscsi_readcapacity commands also to > sync? Indeed, there is no real advantage in using qemu_aio_wait(). I didn't know libiscsi also had sync APIs. :) Paolo
Re: [Qemu-devel] [PATCH 7/7] virtio-blk: add x-data-plane=on|off performance feature
Il 15/11/2012 22:08, Anthony Liguori ha scritto: > Stefan Hajnoczi writes: > >> The virtio-blk-data-plane feature is easy to integrate into >> hw/virtio-blk.c. The data plane can be started and stopped similar to >> vhost-net. >> >> Users can take advantage of the virtio-blk-data-plane feature using the >> new -device virtio-blk-pci,x-data-plane=on property. > > I don't think this should be a property of virtio-blk-pci but rather a > separate device. Since this is all temporary and we want it to become the default (perhaps keeping the old code for ioeventfd=false only), I don't think it really matters. Paolo
[Qemu-devel] [Bug 1077806] Re: Integrate Virtualbox/Qemu Guest booting as a desktop environment listing (request)
** Changed in: pantheon-greeter Importance: Undecided => Wishlist -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1077806 Title: Integrate Virtualbox/Qemu Guest booting as a desktop environment listing (request) Status in Light Display Manager: New Status in Pantheon Login Screen: New Status in QEMU: New Bug description: I had seen this new way to install Chromium OS and "boot" it using LightDM's session select menu, and it made me think of an idea: What if you were able to boot virtual machines in the same manner? It would simplify the Ubuntu user's life GREATLY if they had easy access to a Windows VM that can synchronize their files to and fro (Guest additions) and not require a reboot to use it. Modern computers have more than enough power to do something like this, and it should be even easier if the system is using a dedicated virtual machine environment rather than a full blown desktop. This can transform LightDM from being just another login screen to a full-blown glorified host for all of your operating systems and remote logins. I think this would make using Ubuntu a LOT less of a hassle for the new user who came from Windows :) To manage notifications about this bug go to: https://bugs.launchpad.net/lightdm/+bug/1077806/+subscriptions
[Qemu-devel] [PATCH] trace: Remove "info trace" from documents
commit 88affa1c monitor: remove unused do_info_trace has removed "info trace" function from monitor, so remove it from documents. Signed-off-by: Liming Wang --- docs/tracing.txt |9 - hmp-commands.hx |7 --- 2 files changed, 16 deletions(-) diff --git a/docs/tracing.txt b/docs/tracing.txt index c541133..98e9480 100644 --- a/docs/tracing.txt +++ b/docs/tracing.txt @@ -185,15 +185,6 @@ records the char* pointer value instead of the string that is pointed to. Monitor commands -* info trace - Display the contents of trace buffer. This command dumps the trace buffer - with simple formatting. For full pretty-printing, use the simpletrace.py - script on a binary trace file. - - The trace buffer is written into until full. The full trace buffer is - flushed and emptied. This means the 'info trace' will display few or no - entries if the buffer has just been flushed. - * trace-file on|off|flush|set Enable/disable/flush the trace file or set the trace file name. diff --git a/hmp-commands.hx b/hmp-commands.hx index b74ef75..010b8c9 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1573,13 +1573,6 @@ show roms @end table ETEXI -#ifdef CONFIG_TRACE_SIMPLE -STEXI -@item info trace -show contents of trace buffer -ETEXI -#endif - STEXI @item info trace-events show available trace events and their state -- 1.7.9.5
[Qemu-devel] [PATCH] free the memory malloced by load_at()
Signed-off-by: Olivia Yin --- hw/elf_ops.h |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/elf_ops.h b/hw/elf_ops.h index db4630e..4495932 100644 --- a/hw/elf_ops.h +++ b/hw/elf_ops.h @@ -178,6 +178,8 @@ static int glue(load_symbols, SZ)(struct elfhdr *ehdr, int fd, int must_swab, s->disas_strtab = str; s->next = syminfos; syminfos = s; +g_free(syms); +g_free(str); g_free(shdr_table); return 0; fail: -- 1.7.0.4
Re: [Qemu-devel] [PATCH 3/7] dataplane: add virtqueue vring code
On Thu, Nov 15, 2012 at 9:09 PM, Anthony Liguori wrote:
> Stefan Hajnoczi writes:
>
>> The virtio-blk-data-plane cannot access memory using the usual QEMU
>> functions since it executes outside the global mutex and the memory APIs
>> are this time are not thread-safe.
>>
>> This patch introduces a virtqueue module based on the kernel's vhost
>> vring code. The trick is that we map guest memory ahead of time and
>> access it cheaply outside the global mutex.
>>
>> Once the hardware emulation code can execute outside the global mutex it
>> will be possible to drop this code.
>>
>> Signed-off-by: Stefan Hajnoczi
>> ---
>> hw/Makefile.objs | 2 +-
>> hw/dataplane/Makefile.objs | 3 +
>> hw/dataplane/vring.c | 321
>> +
>> hw/dataplane/vring.h | 54
>> trace-events | 3 +
>> 5 files changed, 382 insertions(+), 1 deletion(-)
>> create mode 100644 hw/dataplane/Makefile.objs
>> create mode 100644 hw/dataplane/vring.c
>> create mode 100644 hw/dataplane/vring.h
>>
>> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
>> index af4ab0c..da8ef0c 100644
>> --- a/hw/Makefile.objs
>> +++ b/hw/Makefile.objs
>> @@ -1,4 +1,4 @@
>> -common-obj-y = usb/ ide/
>> +common-obj-y = usb/ ide/ dataplane/
>> common-obj-y += loader.o
>> common-obj-$(CONFIG_VIRTIO) += virtio-console.o
>> common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
>> diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs
>> new file mode 100644
>> index 000..b58544f
>> --- /dev/null
>> +++ b/hw/dataplane/Makefile.objs
>> @@ -0,0 +1,3 @@
>> +ifeq ($(CONFIG_VIRTIO), y)
>> +common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o
>> +endif
>> diff --git a/hw/dataplane/vring.c b/hw/dataplane/vring.c
>> new file mode 100644
>> index 000..6aacce8
>> --- /dev/null
>> +++ b/hw/dataplane/vring.c
>> @@ -0,0 +1,321 @@
>> +/* Copyright 2012 Red Hat, Inc.
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Based on Linux vhost code:
>> + * Copyright (C) 2009 Red Hat, Inc.
>> + * Copyright (C) 2006 Rusty Russell IBM Corporation
>> + *
>> + * Author: Michael S. Tsirkin
>> + * Stefan Hajnoczi
>> + *
>> + * Inspiration, some code, and most witty comments come from
>> + * Documentation/virtual/lguest/lguest.c, by Rusty Russell
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.
>> + */
>> +
>> +#include "trace.h"
>> +#include "hw/dataplane/vring.h"
>> +
>> +/* Map target physical address to host address
>> + */
>> +static inline void *phys_to_host(Vring *vring, hwaddr phys)
>> +{
>> +/* Adjust for 3.6-4 GB PCI memory range */
>> +if (phys >= 0x1) {
>> +phys -= 0x1 - 0xe000;
>> +} else if (phys >= 0xe000) {
>> +fprintf(stderr, "phys_to_host bad physical address in "
>> +"PCI range %#lx\n", phys);
>> +exit(1);
>> +}
>> +return vring->phys_mem_zero_host_ptr + phys;
>> +}
>
> This is too dirty to merge. This code should use a slot mechanism
> similar to what vhost uses. Then it has a plausible chance of working
> on other architectures.
[...]
> You should be able to use a MemoryListener to keep the memory slot table
> up to date. You'll need to use a mutex to protect it but again since
> it's mostly uncontended, that shouldn't be a problem.
Anthony, Paolo,
You're right, we should use MemoryListener. Will fix for v2.
Stefan
Re: [Qemu-devel] [PATCH 7/7] virtio-blk: add x-data-plane=on|off performance feature
On Thu, Nov 15, 2012 at 10:08 PM, Anthony Liguori wrote: > Stefan Hajnoczi writes: > >> The virtio-blk-data-plane feature is easy to integrate into >> hw/virtio-blk.c. The data plane can be started and stopped similar to >> vhost-net. >> >> Users can take advantage of the virtio-blk-data-plane feature using the >> new -device virtio-blk-pci,x-data-plane=on property. > > I don't think this should be a property of virtio-blk-pci but rather a > separate device. The hw/virtio-blk.c code still needs to be used since hw/dataplane/virtio-blk.c is only a subset of virtio-blk. So we're talking about adding a new virtio-blk-data-plane-pci device type to hw/virtio-pci.c? Stefan
Re: [Qemu-devel] [PATCH 1/7] raw-posix: add raw_get_aio_fd() for virtio-blk-data-plane
On Thu, Nov 15, 2012 at 9:03 PM, Anthony Liguori wrote: > Stefan Hajnoczi writes: > >> The raw_get_aio_fd() function allows virtio-blk-data-plane to get the >> file descriptor of a raw image file with Linux AIO enabled. This >> interface is really a layering violation that can be resolved once the >> block layer is able to run outside the global mutex - at that point >> virtio-blk-data-plane will switch from custom Linux AIO code to using >> the block layer. >> >> Signed-off-by: Stefan Hajnoczi > > I think this creates user confusion because virtio-blk-data-plane can't > actually take a BDS. > > So why not just make a string 'filename' property and open it directly > in virtio-blk-data-plane? Then it's at least clear to the user and > management tools what the device is capable of doing. There are some benefits to raw_get_aio_fd(): 1. virtio-blk-data-plane is only a subset virtio-blk implementation, it still needs a regular virtio-blk-pci device (with BDS) in order to run. If we use a filename the user would have to specify it twice. 2. Fetching the file descriptor in this way ensures that the image file is format=raw. 3. virtio-blk-data-plane uses Linux AIO and raw-posix.c has checks which I don't want to duplicate - we can simply check s->use_aio in raw_get_aio_fd() to confirm that Linux AIO can be used. If we open a file directly then we lose these benefits. Do you still think we should open a filename? Stefan
Re: [Qemu-devel] [RFC PATCH v4 2/3] use uimage_reset to reload uimage
Hi Stuart,
load_uimage() is a public function which is called by below files:
hw/dummy_m68k.c:kernel_size = load_uimage(kernel_filename,
&entry, NULL, NULL);
hw/an5206.c:kernel_size = load_uimage(kernel_filename, &entry,
NULL, NULL);
hw/ppc440_bamboo.c:success = load_uimage(kernel_filename,
&entry, &loadaddr, NULL);
hw/openrisc_sim.c:kernel_size =
load_uimage(kernel_filename, entry, NULL, NULL);
hw/ppc/e500.c:kernel_size =
load_uimage(params->kernel_filename, &entry, &loadaddr, NULL);
hw/arm_boot.c:kernel_size = load_uimage(info->kernel_filename,
&entry, NULL, &is_linux);
hw/microblaze_boot.c:kernel_size =
load_uimage(kernel_filename, &uentry, &loadaddr, 0);
hw/mcf5208.c:kernel_size = load_uimage(kernel_filename, &entry,
NULL, NULL);
Most value of *is_linux is NULL, only arm_boot.c will check the value of
is_linux.
I just define an static variable is_linux other than include it into structure
ImageFile.
To define static variable kernel_loaded because function
uimage_physical_loader() will
be called twice.
1) load_uimage() which will run only one time when startup.
2) uimage_reset() which will run once reset, so it maybe run many times.
Since "ROM images must be loaded at startup", it means rom_add_*() should not
be called by any reset handlers.
So I use variable kernel_loaded to decide the booting is startup or reset.
Best Regards,
Olivia
> -Original Message-
> From: Stuart Yoder [mailto:b08...@gmail.com]
> Sent: Friday, November 16, 2012 4:46 AM
> To: Yin Olivia-R63875
> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org
> Subject: Re: [Qemu-devel] [RFC PATCH v4 2/3] use uimage_reset to reload
> uimage
>
> On Wed, Nov 14, 2012 at 2:28 PM, Olivia Yin
> wrote:
> > Signed-off-by: Olivia Yin
> > ---
> > hw/loader.c | 64 ++-
> ---
> > 1 files changed, 46 insertions(+), 18 deletions(-)
> >
> > diff --git a/hw/loader.c b/hw/loader.c index a8a0a09..1a909d0 100644
> > --- a/hw/loader.c
> > +++ b/hw/loader.c
> > @@ -55,6 +55,8 @@
> > #include
> >
> > static int roms_loaded;
> > +static int kernel_loaded;
> > +static int *is_linux;
>
> Why do we need these 2 new variables?
>
> > /* return the size or -1 if error */
> > int get_image_size(const char *filename) @@ -472,15 +474,14 @@ static
> > ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
> > return dstbytes;
> > }
> >
> > -/* Load a U-Boot image. */
> > -int load_uimage(const char *filename, hwaddr *ep,
> > -hwaddr *loadaddr, int *is_linux)
> > +/* write uimage into memory */
> > +static int uimage_physical_loader(const char *filename, uint8_t **data,
> > + hwaddr *loadaddr, int *is_linux)
> > {
> > int fd;
> > int size;
> > uboot_image_header_t h;
> > uboot_image_header_t *hdr = &h;
> > -uint8_t *data = NULL;
> > int ret = -1;
> >
> > fd = open(filename, O_RDONLY | O_BINARY); @@ -521,10 +522,9 @@
> > int load_uimage(const char *filename, hwaddr *ep,
> > *is_linux = 0;
> > }
> >
> > -*ep = hdr->ih_ep;
> > -data = g_malloc(hdr->ih_size);
> > +*data = g_malloc(hdr->ih_size);
> >
> > -if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
> > +if (read(fd, *data, hdr->ih_size) != hdr->ih_size) {
> > fprintf(stderr, "Error reading file\n");
> > goto out;
> > }
> > @@ -534,11 +534,11 @@ int load_uimage(const char *filename, hwaddr *ep,
> > size_t max_bytes;
> > ssize_t bytes;
> >
> > -compressed_data = data;
> > +compressed_data = *data;
> > max_bytes = UBOOT_MAX_GUNZIP_BYTES;
> > -data = g_malloc(max_bytes);
> > +*data = g_malloc(max_bytes);
> >
> > -bytes = gunzip(data, max_bytes, compressed_data, hdr->ih_size);
> > +bytes = gunzip(*data, max_bytes, compressed_data,
> > + hdr->ih_size);
> > g_free(compressed_data);
> > if (bytes < 0) {
> > fprintf(stderr, "Unable to decompress gzipped image!\n");
> > @@ -547,7 +547,9 @@ int load_uimage(const char *filename, hwaddr *ep,
> > hdr->ih_size = bytes;
> > }
> >
> > -rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
> > +if (!kernel_loaded) {
> > +rom_add_blob_fixed(filename, *data, hdr->ih_size, hdr-
> >ih_load);
> > +}
>
> Why do we need to keep the rom_add_blob_fixed() call? I thought the
> point of this was to remove adding roms.
>
> > if (loadaddr)
> > *loadaddr = hdr->ih_load;
> > @@ -555,12 +557,41 @@ int load_uimage(const char *filename, hwaddr *ep,
> > ret = hdr->ih_size;
> >
> > out:
> > -if (data)
> > -g_free(data);
> > close(fd);
> > return ret;
> > }
> >
> > +static void uimage_reset(void *opaque) {
> > +ImageFile *im
Re: [Qemu-devel] [RFC PATCH v4 1/3] use image_file_reset to reload initrd image
Hi Stuart,
I want to keep the Memory Region of rom which will be inserted to the queue
roms.
And then we can use "info roms" command to check the rom information.
ROM images must be loaded at startup, so rom_add_* functions could be called
only
one time before all the reset handlers.
Exactly all the memory malloced to rom->data will be free when rom_reset() no
matter
it is rom (rom->isrom = 1) or ram (rom->isrom =0).
@@ -708,11 +739,8 @@ static void rom_reset(void *unused)
continue;
}
cpu_physical_memory_write_rom(rom->addr, rom->data, rom->romsize);
-if (rom->isrom) {
-/* rom needs to be written only once */
-g_free(rom->data);
-rom->data = NULL;
-}
+g_free(rom->data);
+rom->data = NULL;
}
}
For example, to load initrd image, both reset handlers (rom_reset() and
image_file_reset()
will write the image into the same physical address. So it will not malloc more
memory.
cpu_physical_memory_write_rom(rom->addr, rom->data, rom->romsize);
cpu_physical_memory_write(image->addr, (uint8_t *)content, size);
The uImage is another case, I'll explain in the reply of patch 2/3.
Best Regards,
Olivia
> -Original Message-
> From: Stuart Yoder [mailto:b08...@gmail.com]
> Sent: Friday, November 16, 2012 3:52 AM
> To: Yin Olivia-R63875
> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org
> Subject: Re: [Qemu-devel] [RFC PATCH v4 1/3] use image_file_reset to
> reload initrd image
>
> > /* read()-like version */
> > ssize_t read_targphys(const char *name,
> >int fd, hwaddr dst_addr, size_t nbytes) @@
> > -113,6 +146,12 @@ int load_image_targphys(const char *filename,
> > }
> > if (size > 0) {
> > rom_add_file_fixed(filename, addr, -1);
> > +ImageFile *image;
> > +image = g_malloc0(sizeof(*image));
> > +image->name = g_strdup(filename);
> > +image->addr = addr;
> > +
> > +qemu_register_reset(image_file_reset, image);
>
> You need to remove the call to rom_add_file_fixed(), no?
>
> Stuart
[Qemu-devel] [PATCH 2/2] Buildsystem clean tests directory clearly
Currently make clean only clean tests/tcg and hard to extend. This patch added command make check-clean, which clean all generated files used in tests. With this command root Makefile do not care tests clean method any more, it simply calls the command to do it, so any more clean script could be added in tests/Makefile make it easier to extend. Signed-off-by: Wenchao Xia --- Makefile |2 +- configure |2 +- tests/Makefile |7 +++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f40885b..8201e80 100644 --- a/Makefile +++ b/Makefile @@ -251,7 +251,7 @@ clean: rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp) rm -rf qapi-generated rm -rf qga/qapi-generated - $(MAKE) -C tests/tcg clean + MAKEFILES=./tests/Makefile $(MAKE) check-clean for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \ if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \ rm -f $$d/qemu-options.def; \ diff --git a/configure b/configure index f847ee2..a18e267 100755 --- a/configure +++ b/configure @@ -4163,7 +4163,7 @@ DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas" DIRS="$DIRS roms/seabios roms/vgabios" DIRS="$DIRS qapi-generated" DIRS="$DIRS libcacard libcacard/libcacard libcacard/trace" -FILES="Makefile tests/tcg/Makefile qdict-test-data.txt" +FILES="Makefile tests/Makefile tests/tcg/Makefile qdict-test-data.txt" FILES="$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit" FILES="$FILES tests/tcg/lm32/Makefile libcacard/Makefile" FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps" diff --git a/tests/Makefile b/tests/Makefile index 9bf0765..a286622 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -94,6 +94,7 @@ check-help: @echo " make check-unit Run qobject tests" @echo " make check-block Run block tests" @echo " make check-report.htmlGenerates an HTML test report" + @echo " make check-clean Clean the tests" @echo @echo "Please note that HTML reports do not regenerate if the unit tests" @echo "has not changed." @@ -148,4 +149,10 @@ check-unit: $(patsubst %,check-%, $(check-unit-y)) check-block: $(patsubst %,check-%, $(check-block-y)) check: check-unit check-qtest +check-clean: + $(MAKE) -C tests/tcg clean + rm -f $(check-unit-y) + rm -f $(check-qtest-i386-y) $(check-qtest-x86_64-y) $(check-qtest-sparc64-y) $(check-qtest-sparc-y) + rm -f tests/*.o + -include $(wildcard tests/*.d) -- 1.7.1
[Qemu-devel] [PATCH 1/2] Buildsystem fix distclean error in pixman
Currently if pixman have no config.log inside, make file still try to clean it resulting error. This patch fix it. Signed-off-by: Wenchao Xia --- Makefile |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 81c660f..f40885b 100644 --- a/Makefile +++ b/Makefile @@ -278,7 +278,10 @@ distclean: clean for d in $(TARGET_DIRS) $(QEMULIBS); do \ rm -rf $$d || exit 1 ; \ done - test -f pixman/config.log && make -C pixman distclean + @if test -f pixman/config.log; \ + then \ + make -C pixman distclean;\ + fi KEYMAPS=da en-gb et fr fr-ch is lt modifiers no pt-br sv \ ar de en-us fi fr-be hr it lv nl pl ru th \ -- 1.7.1
[Qemu-devel] [PATCH 0/2] Buildsystem fix
Following patch fix a problem and adjust build system, to make it more modulized and easier to extend. Wenchao Xia (2): Buildsystem fix distclean error in pixman Buildsystem clean tests directory clearly Makefile |7 +-- configure |2 +- tests/Makefile |7 +++ 3 files changed, 13 insertions(+), 3 deletions(-)
Re: [Qemu-devel] [PATCH v6 0/7] TCG global variables clean-up
On 11/12/2012 01:27 PM, Evgeny Voevodin wrote: This set of patches moves global variables to tcg_ctx: gen_opc_ptr gen_opparam_ptr gen_opc_buf gen_opparam_buf Build tested for all targets. Execution tested on Exynos4210 target. After this patchset was aplied, I noticed 0.7% speed-up of code generation. Probably, this is due to better data caching. Here is the test procedure: 1. Boot Linux Kernel 5 times. 2. For each iteration wait while "JIT cycles" is stable for ~10 seconds 3. Write down the "cycles/op" Here are the results (tested on gcc-4.6): Before clean-up: min: 731.5 max: 734.8 avg: 733.0 standard deviation: ~2 ~= 0.2% Average cycles/op = 733 +- 2 After clean-up: min: 725.0 max: 730.5 avg: 727.8 standard deviation: ~3 ~= 0.4% Average cycles/op = 728 +- 3 Speed-up of TCG code generation = 0.7% Changelog: v5->v6: Fixed broken patches. Rebased. v4->v5: Rebased. Fixed authorship. All patches are reviewed-by Richard Henderson v3->v4: Rebased. Added target-cris/translate.c: Code style clean-up v2->v3: Removed tcg_cur_ctx since it gives slow-down on gcc-4.5. Rebased. v1->v2: Introduced TCGContext *tcg_cur_ctx global to use in those places where we don't have an interface to pass pointer to tcg_ctx. Code style clean-up Evgeny Voevodin (7): target-cris/translate.c: Code style clean-up tcg/tcg.h: Duplicate global TCG variables in TCGContext TCG: Use gen_opc_ptr from context instead of global variable. TCG: Use gen_opparam_ptr from context instead of global variable. TCG: Use gen_opc_buf from context instead of global variable. TCG: Use gen_opparam_buf from context instead of global variable. TCG: Remove unused global variables gen-icount.h |2 +- target-alpha/translate.c | 10 +- target-arm/translate.c| 10 +- target-cris/translate.c | 5040 + target-i386/translate.c | 10 +- target-lm32/translate.c | 13 +- target-m68k/translate.c | 10 +- target-microblaze/translate.c | 13 +- target-mips/translate.c | 11 +- target-openrisc/translate.c | 13 +- target-ppc/translate.c| 11 +- target-s390x/translate.c | 11 +- target-sh4/translate.c| 10 +- target-sparc/translate.c | 10 +- target-unicore32/translate.c | 10 +- target-xtensa/translate.c |8 +- tcg/optimize.c| 62 +- tcg/tcg-op.h | 324 +-- tcg/tcg.c | 85 +- tcg/tcg.h | 10 +- translate-all.c |3 - 21 files changed, 2859 insertions(+), 2817 deletions(-) Ping? -- Kind regards, Evgeny Voevodin, Technical Leader, Mobile Group, Samsung Moscow Research Center, e-mail: e.voevo...@samsung.com
[Qemu-devel] [PATCH] target-mips: Add comments on POOL32Axf encoding
Hi all,
Current QEMU MIPS POOL32AXF encoding comes from microMIPS32
and microMIPS32 DSP. Add comment here to help reading.
Please review, thanks.
Regards,
chenwj
Signed-off-by: Chen Wei-Ren
---
target-mips/translate.c | 17 +
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 01b48fa..9d4b2c3 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -10359,6 +10359,19 @@ enum {
/* POOL32AXF encoding of minor opcode field extension */
+/*
+ * 1. MIPS Architecture for Programmers Volume II-B:
+ * The microMIPS32 Instruction Set (Revision 3.05)
+ *
+ * Table 6.5 POOL32Axf Encoding of Minor Opcode Extension Field
+ *
+ * 2. MIPS Architecture for Programmers VolumeIV-e:
+ * The MIPS DSP Application-Specific Extension
+ * to the microMIPS32 Architecture (Revision 2.34)
+ *
+ * Table 5.5 POOL32Axf Encoding of Minor Opcode Extension Field
+ */
+
enum {
/* bits 11..6 */
TEQ = 0x00,
@@ -10371,6 +10384,8 @@ enum {
MFC0 = 0x03,
MTC0 = 0x0b,
+/* begin of microMIPS32 DSP */
+
/* bits 13..12 for 0x01 */
MFHI_ACC = 0x0,
MFLO_ACC = 0x1,
@@ -10387,6 +10402,8 @@ enum {
MULT_ACC = 0x0,
MULTU_ACC = 0x1,
+/* end of microMIPS32 DSP */
+
/* bits 15..12 for 0x2c */
SEB = 0x2,
SEH = 0x3,
--
1.7.3.4
Re: [Qemu-devel] Question about comment on MIPS POOL32AXF encoding
> Please refer to the following document. > > MIPS® Architecture for Programmers VolumeIV-e: The MIPS® DSP > Application-Specific Extension to the microMIPS32™ Architecture > > http://www.mips.com/secure-download/index.dot?product_name=/auth/MD00764-2B-microMIPS32DSP-AFP-02.34.pdf Thanks, Eric. I would like to add a comment here to remind me where they come. :) Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
[Qemu-devel] [PATCH v2] target-mips: Clean up microMIPS32 major opcode
Hi all,
I check MIPS microMIPS manual [1], and found the major opcode might be
wrong. I add a comment to explicitly indicate what manual I am refering
to, and according that manual I remove microMIPS32 major opcodes 0x1f.
As for others, like 0x16, 0x17, 0x36 and 0x37, they are for higher-order
MIPS ISA level or new revision of this microMIPS architecture. Quote
from Johnson, they are belong MIPS64 [2].
Please review, thanks.
[1] http://www.mips.com/products/architectures/micromips/#specifications
MIPS Architecture for Programmers Volume II-B:
The microMIPS32 Instruction Set (Revision 3.05)
MD00582-2B-microMIPS-AFP-03.05.pdf
[2] http://www.mips.com/products/architectures/mips64/
MIPS Architecture For Programmers
Volume II-A: The MIPS64 Instruction Set
MD00087-2B-MIPS64BIS-AFP-03.51.pdf
Signed-off-by: Chen Wei-Ren
---
target-mips/translate.c | 24 +---
1 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 01b48fa..1c0ff65 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -10239,9 +10239,19 @@ static int decode_mips16_opc (CPUMIPSState *env,
DisasContext *ctx,
return n_bytes;
}
-/* microMIPS extension to MIPS32 */
+/* microMIPS extension to MIPS32/MIPS64 */
-/* microMIPS32 major opcodes */
+/*
+ * microMIPS32/microMIPS64 major opcodes
+ *
+ * 1. MIPS Architecture for Programmers Volume II-B:
+ * The microMIPS32 Instruction Set (Revision 3.05)
+ *
+ * Table 6.2 microMIPS32 Encoding of Major Opcode Field
+ *
+ * 2. MIPS Architecture For Programmers Volume II-A:
+ * The MIPS64 Instruction Set (Revision 3.51)
+ */
enum {
POOL32A = 0x00,
@@ -10268,9 +10278,10 @@ enum {
POOL16D = 0x13,
ORI32 = 0x14,
POOL32F = 0x15,
-POOL32S = 0x16,
-DADDIU32 = 0x17,
+POOL32S = 0x16, /* MIPS64 */
+DADDIU32 = 0x17, /* MIPS64 */
+/* 0x1f is reserved */
POOL32C = 0x18,
LWGP16 = 0x19,
LW16 = 0x1a,
@@ -10278,7 +10289,6 @@ enum {
XORI32 = 0x1c,
JALS32 = 0x1d,
ADDIUPC = 0x1e,
-POOL48A = 0x1f,
/* 0x20 is reserved */
RES_20 = 0x20,
@@ -10307,8 +10317,8 @@ enum {
B16 = 0x33,
ANDI32 = 0x34,
J32 = 0x35,
-SD32 = 0x36,
-LD32 = 0x37,
+SD32 = 0x36, /* MIPS64 */
+LD32 = 0x37, /* MPIS64 */
/* 0x38 and 0x39 are reserved */
RES_38 = 0x38,
--
1.7.3.4
Re: [Qemu-devel] [PATCH v2 000/147] target-s390 reorg
Ping. I've updated the branch at git://repo.or.cz/qemu/rth.git s390-reorg to rebase vs mainline (ce34cf72fe508b27a78f83c184142e8d1e6a048a). There are minor changes due to Aurel's TCG_CALL_NO_* rewrite, but is otherwise unchanged. I can re-post the 140+ patches if required... r~ On 2012-09-27 15:39, Richard Henderson wrote: > Tons of changes since v1. I believe I incorproated all of the feedback > that I received. The patch set is relative to the "TCGCond" patch set > that I sent out earlier this week. Both trees are available at > > git://repo.or.cz/qemu/rth.git tcg-cond > git://repo.or.cz/qemu/rth.git s390-reorg > > This passes the gcc testsuite in userland with no unexpected failures > (considering anything that touches threads to be "expected"). It boots > a debian system image as well as (or as poorly as) mainline -- agraf > and I had a rather long session re those failures on IRC the other day. > > Note that patches 2-11 are bug fixes for the *existing* code base. I > found that I had to fix those in order to be able to properly bisect > problems in the later patches. > > The last 7 patches are new, and improve the generated code for a > variety of stuff I noticed while staring at dumps. Not that the > last 25 patches or so made it to the list last time... > > > r~ > > > Alexander Graf (1): > s390x: fix -initrd in virtio machine > > Richard Henderson (146): > tcg: Add TCGV_IS_UNUSED_* > target-s390: Disassemble more z10 and z196 opcodes > target-s390: Fix disassembly of cpsdr > target-s390: Fix gdbstub > target-s390: Add missing temp_free in gen_op_calc_cc > target-s390: Use TCG registers for FPR > target-s390: Register helpers > target-s390: Fix SACF exit > target-s390: Fix BCR > target-s390: Tidy unconditional BRCL > target-s390: Fix PSW_MASK handling > target-s390: Add format based disassassmbly infrastructure > target-s390: Split out disas_jcc > target-s390: Reorg exception handling > target-s390: Convert ADD HALFWORD > target-s390: Implement SUBTRACT HALFWORD > target-s390: Implement ADD LOGICAL WITH SIGNED IMMEDIATE > target-s390: Convert MULTIPLY HALFWORD, SINGLE > target-s390: Convert 32-bit MULTIPLY, MULTIPLY LOGICAL > target-s390: Convert 64-bit MULTIPLY LOGICAL > target-s390: Convert AND, OR, XOR > target-s390: Convert COMPARE, COMPARE LOGICAL > target-s390: Convert LOAD, LOAD LOGICAL > target-s390: Convert LOAD ADDRESS > target-s390: Convert LOAD (LOGICAL) BYTE, CHARACTER, HALFWORD > target-s390: Convert LOAD AND TEST > target-s390: Convert LOAD LOGICAL IMMEDIATE > target-s390: Convert LOAD COMPLIMENT, POSITIVE, NEGATIVE > target-s390: Convert AND, OR, XOR, INSERT IMMEDIATE > target-s390: Convert STORE > target-s390: Convert ADD LOGICAL CARRY and SUBTRACT LOGICAL BORROW > target-s390: Convert BRANCH AND SAVE > target-s390: Convert BRANCH ON CONDITION > target-s390: Convert BRANCH ON COUNT > target-s390: Convert DIVIDE > target-s390: Send signals for divide > target-s390: Convert TEST UNDER MASK > target-s390: Convert SET ADDRESSING MODE > target-s390: Convert SUPERVISOR CALL > target-s390: Convert MOVE LONG > target-s390: Convert FP LOAD > target-s390: Convert INSERT CHARACTER > target-s390: Cleanup cc computation helpers > target-s390: Convert INSERT CHARACTERS UNDER MASK > target-s390: Convert EXECUTE > target-s390: Convert FP STORE > target-s390: Convert CONVERT TO DECIMAL > target-s390: Convert SET SYSTEM MASK > target-s390: Convert LOAD PSW > target-s390: Convert DIAGNOSE > target-s390: Convert SHIFT, ROTATE SINGLE > target-s390: Convert SHIFT DOUBLE > target-s390: Convert LOAD, STORE MULTIPLE > target-s390: Convert MOVE > target-s390: Convert NI, XI, OI > target-s390: Convert STNSM, STOSM > target-s390: Convert LAM, STAM > target-s390: Convert CLCLE, MVCLE > target-s390: Convert MVC > target-s390: Convert NC, XC, OC, TR, UNPK > target-s390: Convert CLC > target-s390: Convert MVCP, MVCS > target-s390: Convert LRA > target-s390: Convert SIGP > target-s390: Convert EFPC, STFPC > target-s390: Convert LCTL, STCTL > target-s390: Convert COMPARE AND SWAP > target-s390: Convert CLM > target-s390: Convert STCM > target-s390: Convert TPROT > target-s390: Convert LOAD CONTROL, part 2 > target-s390: Convert LOAD REVERSED > target-s390: Convert STORE REVERSED > target-s390: Convert LLGT > target-s390: Convert FP ADD, COMPARE, LOAD TEST/ROUND/LENGTHENED > target-s390: Convert FP SUBTRACT > target-s390: Convert FP DIVIDE > target-s390: Convert FP MULTIPLY > target-s390: Convert MULTIPLY AND ADD, SUBTRACT > target-s390: Convert TEST DATA CLASS > target-s390: Convert FP LOAD COMPLIMENT, NEGATIVE, POSITIVE > target-s390: Convert FP SQUARE ROOT > target-s390: Convert LOAD ZERO > target-s390: Convert CONVERT TO FIXED > target-s390: Convert CONVERT FROM FIXED > target-s390: Convert FLOGR > target-s390: C
Re: [Qemu-devel] [PATCH v6 1/8] atomic: introduce atomic operations
On 2012-11-14 23:47, liu ping fan wrote: > Probably I made a mistake here, in vhost, log = > __sync_fetch_and_and(from, 0) is used to fetch 64bits atomically in > the case 32bits qemu running on 64bits linux. Right? But how can > we read 32bits twice in atomic? Seem that no instruction like "_lock > xchg" for this ops. So I guess _sync_fetch_and_and() based on > something like spinlock. ... or for gcc 4.7 and later, log = __atomic_load_n(from, memory_model) For i386, we will not perform 2 32-bit reads of course. Paulo suggests using cmpxchg8b, but that's a tad slow. Instead we'll perform a 64-bit read into either the fpu or the sse units, and from there copy the data wherever it's needed. Such 64-bit aligned reads are guaranteed to be atomic for i586 (pentium) and later. For other 32-bit architectures other possibilities exist. Recent arm can use its ldrexd insn. Many of the 32-bit linux architectures have special kernel entry points or schemes to perform atomic operations. These are generally based on the assumption of a single-processor system, and are arranged to either disable interrupts or notice that no interrupt occurred, while executing a code region. As an ultimate fallback, yes we would use locks. But none of the host architectures that QEMU supports needs to do so. r~
Re: [Qemu-devel] [PATCH v2 05/39] fdsets: use weak aliases instead of qemu-tool.c/qemu-user.c
Am 15.11.2012 21:52, schrieb Paolo Bonzini: Il 15/11/2012 19:01, Stefan Weil ha scritto: Hi Paolo, this patch breaks QEMU on 32 and 64 bit hosts, native and with Wine. It's easy to reproduce the SIGSEGV crash: just add a -snapshot option. Obviously the critical code is executed only when this option was used. I cannot reproduce this, so it must be an assembler or linker bug. Can you try the alternative code that is used for Mac OS X? Paolo The code which is used for Mac OS X also compiles and results in the same run-time bug with Wine: wine: Unhandled page fault on write access to 0x0004 at address 0x7b845d6e (thread 001b), starting debugger... (immediately after BIOS says "Booting from hard disk...") This was the modification used: diff --git a/compiler.h b/compiler.h index 55d7d74..62427e4 100644 --- a/compiler.h +++ b/compiler.h @@ -50,11 +50,12 @@ # define __printf__ __gnu_printf__ # endif # endif -# if defined(__APPLE__) +# if defined(__APPLE__) || defined(_WIN32) # define QEMU_WEAK_ALIAS(newname, oldname) \ static typeof(oldname) weak_##newname __attribute__((unused, weakref(#oldname))) # define QEMU_WEAK_REF(newname, oldname) (weak_##newname ? weak_##newname : oldname) # else +#error # define QEMU_WEAK_ALIAS(newname, oldname) \ typeof(oldname) newname __attribute__((weak, alias (#oldname))) # define QEMU_WEAK_REF(newname, oldname) newname These are my Debian packages (only the 32 bit ones are needed for the test): ii binutils-mingw-w64-i686 2.22-7+2 Cross-binutils for Win32 (x86) using MinGW-w64 ii binutils-mingw-w64-x86-64 2.22-7+2 Cross-binutils for Win64 (x64) using MinGW-w64 ii gcc-mingw-w64 4.6.3-8+7 GNU C compiler for MinGW-w64 ii gcc-mingw-w64-base 4.6.3-8+7 GNU Compiler Collection for MinGW-w64 (base package) ii gcc-mingw-w64-i686 4.6.3-8+7 GNU C compiler for MinGW-w64 targeting Win32 ii gcc-mingw-w64-x86-64 4.6.3-8+7 GNU C compiler for MinGW-w64 targeting Win64 ii mingw-w64 3.0~svn4933-1 Development environment targetting 32- and 64-bit Windows ii mingw-w64-dev 3.0~svn4933-1 Development files for MinGW-w64 ii mingw-w64-tools 3.0~svn4933-1 Development tools for 32- and 64-bit Windows On Windows, I used a rather new MinGW standard installation. I'll run more tests with other Linux distributions tomorrow. Cheers, Stefan Here is a simple command line using Wine: wine i386-softmmu/qemu-system-i386 -L pc-bios -snapshot Makefile The disk image does not matter, so I just selected QEMU's Makefile. It looks like weak symbols are not really working with MinGW (Blue Swirl previously pointed out that only ELF and a.out are officially supported). I can see in the debugger that QEMU wants to call monitor_fdset_dup_fd_find from qemu_close. In previous versions, this was just a dummy function returning 0. Now, it is the function in monitor.c, but the address does not match exactly, so the code addresses lines near the beginning of monitor_fdset_dup_fd_find which does not work of course. A trivial workaround is calling default_fdset_dup_fd_find which restores the old behaviour. I expect that all other weak functions would show the same problem if they were used. Regards, Stefan
[Qemu-devel] [Bug 1077838] Re: qemu-nbd -r -c taints device for subsequent usage, even after -d
Hello Robert, or anyone else affected, Accepted qemu-kvm into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/qemu-kvm/1.0+noroms- 0ubuntu14.4 in a few hours, and then in the -proposed repository. Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users. If this package fixes the bug for you, please change the bug tag from verification-needed to verification-done. If it does not, change the tag to verification-failed. In either case, details of your testing will help us make a better decision. Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance! ** Changed in: qemu-kvm (Ubuntu Precise) Status: In Progress => Fix Committed ** Tags added: verification-needed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1077838 Title: qemu-nbd -r -c taints device for subsequent usage, even after -d Status in QEMU: In Progress Status in “qemu-kvm” package in Ubuntu: Fix Released Status in “qemu-kvm” source package in Precise: Fix Committed Status in “qemu-kvm” source package in Quantal: In Progress Bug description: Something about qemu-nbd -r -c /dev/nbd0 someimg leaves cruft behind - subsequent connections get marked readonly. This is on quantal, haven't checked precise or raring. To demonstrate: # use one image qemu-img create -f qcow2 /tmp/1.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/1.qcow2 sudo mkfs -t ext4 /dev/nbd2 sudo qemu-nbd -d /dev/nbd2 # use a second one on the same nbd device, shows that reuse works: qemu-img create -f qcow2 /tmp/2.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/2.qcow2 sudo mkfs -t ext4 /dev/nbd2 sudo qemu-nbd -d /dev/nbd2 # connect an image in read only mode sudo qemu-nbd -r -c /dev/nbd2 /tmp/2.qcow2 sudo dumpe2fs /dev/nbd2 | head -n 3 sudo qemu-nbd -d /dev/nbd2 # now try to reuse in read-write mode again: qemu-img create -f qcow2 /tmp/3.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/3.qcow2 sudo mkfs -t ext4 /dev/nbd2 # here it goes boom: mke2fs 1.42.5 (29-Jul-2012) /dev/nbd2: Operation not permitted while setting up superblock # still need to cleanup sudo qemu-nbd -d /dev/nbd2 === SRU Justification: 1. Impact: mounting an nbd device as read-write after doing so read-only will cause the mount to erroneously (and quietly) be read-only. 2. Development fix: have qemu-nbd set the device to read-write when asked, rather than only setting read-only. 3. Stable fix: same as development fix 4. Test case: See above 5. Regression potential: The patch is localized to the handling of read-only flag in qemu-nbd, so any regression should not affect anything else. === To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1077838/+subscriptions
Re: [Qemu-devel] [PATCH 7/7] virtio-blk: add x-data-plane=on|off performance feature
Khoa Huynh writes: > "Michael S. Tsirkin" wrote on 11/15/2012 12:48:49 PM: > >> From: "Michael S. Tsirkin" >> To: Stefan Hajnoczi , >> Cc: qemu-devel@nongnu.org, Anthony Liguori/Austin/IBM@IBMUS, Paolo >> Bonzini , Kevin Wolf , Asias >> He , Khoa Huynh/Austin/IBM@IBMUS >> Date: 11/15/2012 12:46 PM >> Subject: Re: [PATCH 7/7] virtio-blk: add x-data-plane=on|off >> performance feature >> >> On Thu, Nov 15, 2012 at 04:19:06PM +0100, Stefan Hajnoczi wrote: >> > The virtio-blk-data-plane feature is easy to integrate into >> > hw/virtio-blk.c. The data plane can be started and stopped similar to >> > vhost-net. >> > >> > Users can take advantage of the virtio-blk-data-plane feature using the >> > new -device virtio-blk-pci,x-data-plane=on property. >> > >> > The x-data-plane name was chosen because at this stage the feature is >> > experimental and likely to see changes in the future. >> > >> > If the VM configuration does not support virtio-blk-data-plane an error >> > message is printed. Although we could fall back to regular virtio-blk, >> > I prefer the explicit approach since it prompts the user to fix their >> > configuration if they want the performance benefit of >> > virtio-blk-data-plane. >> > >> > Limitations: >> > * Only format=raw is supported >> > * Live migration is not supported >> > * Block jobs, hot unplug, and other operations fail with -EBUSY >> > * I/O throttling limits are ignored >> > * Only Linux hosts are supported due to Linux AIO usage >> > >> > Signed-off-by: Stefan Hajnoczi >> >> >> Would be very interested in learning about the performance >> impact of this. How does this compare to current model and >> to vhost-blk? > > I plan to do a complete evaluation of this patchset in the coming days. > However, I have done quite a bit of performance testing on earlier versions > of the data-plane and vhost-blk code bits. Here's what I found: > > 1) The existing kvm/qemu code can only handle up to about 150,000 IOPS for > a single KVM guest. The bottleneck here is the global qemu mutex. > > 2) With performance tuning, I was able to achieve 1.33 million IOPS for a > single KVM guest with data-plane. This is very close to the > 1.4-million-IOPS > limit of my storage setup. >From my POV, if we can get this close to bare metal with virtio-blk-dataplane, there's absolutely no reason to merge vhost-blk support. We simply lose too much with a kernel-based solution. I'm sure there's more we can do to improve the userspace implementation too like a hypercall-based notify and adaptive polling. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 7/7] virtio-blk: add x-data-plane=on|off performance feature
Stefan Hajnoczi writes:
> The virtio-blk-data-plane feature is easy to integrate into
> hw/virtio-blk.c. The data plane can be started and stopped similar to
> vhost-net.
>
> Users can take advantage of the virtio-blk-data-plane feature using the
> new -device virtio-blk-pci,x-data-plane=on property.
I don't think this should be a property of virtio-blk-pci but rather a
separate device.
It's an alternative interface with a slightly different guest view.
Regards,
Anthony Liguori
>
> The x-data-plane name was chosen because at this stage the feature is
> experimental and likely to see changes in the future.
>
> If the VM configuration does not support virtio-blk-data-plane an error
> message is printed. Although we could fall back to regular virtio-blk,
> I prefer the explicit approach since it prompts the user to fix their
> configuration if they want the performance benefit of
> virtio-blk-data-plane.
>
> Limitations:
> * Only format=raw is supported
> * Live migration is not supported
> * Block jobs, hot unplug, and other operations fail with -EBUSY
> * I/O throttling limits are ignored
> * Only Linux hosts are supported due to Linux AIO usage
>
> Signed-off-by: Stefan Hajnoczi
> ---
> hw/virtio-blk.c | 59
> -
> hw/virtio-blk.h | 1 +
> hw/virtio-pci.c | 3 +++
> 3 files changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index e25cc96..7f6004e 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -17,6 +17,8 @@
> #include "hw/block-common.h"
> #include "blockdev.h"
> #include "virtio-blk.h"
> +#include "hw/dataplane/virtio-blk.h"
> +#include "migration.h"
> #include "scsi-defs.h"
> #ifdef __linux__
> # include
> @@ -33,6 +35,8 @@ typedef struct VirtIOBlock
> VirtIOBlkConf *blk;
> unsigned short sector_mask;
> DeviceState *qdev;
> +VirtIOBlockDataPlane *dataplane;
> +Error *migration_blocker;
> } VirtIOBlock;
>
> static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
> @@ -407,6 +411,14 @@ static void virtio_blk_handle_output(VirtIODevice *vdev,
> VirtQueue *vq)
> .num_writes = 0,
> };
>
> +/* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
> + * dataplane here instead of waiting for .set_status().
> + */
> +if (s->dataplane) {
> +virtio_blk_data_plane_start(s->dataplane);
> +return;
> +}
> +
> while ((req = virtio_blk_get_request(s))) {
> virtio_blk_handle_request(req, &mrb);
> }
> @@ -446,8 +458,13 @@ static void virtio_blk_dma_restart_cb(void *opaque, int
> running,
> {
> VirtIOBlock *s = opaque;
>
> -if (!running)
> +if (!running) {
> +/* qemu_drain_all() doesn't know about data plane, quiesce here */
> +if (s->dataplane) {
> +virtio_blk_data_plane_drain(s->dataplane);
> +}
> return;
> +}
>
> if (!s->bh) {
> s->bh = qemu_bh_new(virtio_blk_dma_restart_bh, s);
> @@ -538,6 +555,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev,
> uint8_t status)
> VirtIOBlock *s = to_virtio_blk(vdev);
> uint32_t features;
>
> +if (s->dataplane && !(status & VIRTIO_CONFIG_S_DRIVER)) {
> +virtio_blk_data_plane_stop(s->dataplane);
> +}
> +
> if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> return;
> }
> @@ -604,6 +625,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> VirtIOBlkConf *blk)
> {
> VirtIOBlock *s;
> static int virtio_blk_id;
> +int fd = -1;
>
> if (!blk->conf.bs) {
> error_report("drive property not set");
> @@ -619,6 +641,21 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> VirtIOBlkConf *blk)
> return NULL;
> }
>
> +if (blk->data_plane) {
> +if (blk->scsi) {
> +error_report("device is incompatible with x-data-plane, "
> + "use scsi=off");
> +return NULL;
> +}
> +
> +fd = raw_get_aio_fd(blk->conf.bs);
> +if (fd < 0) {
> +error_report("drive is incompatible with x-data-plane, "
> + "use format=raw,cache=none,aio=native");
> +return NULL;
> +}
> +}
> +
> s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
>sizeof(struct virtio_blk_config),
>sizeof(VirtIOBlock));
> @@ -636,6 +673,17 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> VirtIOBlkConf *blk)
>
> s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
>
> +if (fd >= 0) {
> +s->dataplane = virtio_blk_data_plane_create(&s->vdev, fd);
> +
> +/* Prevent block operations that conflict with data plane thread */
> +bdrv_set_in_use(s->bs, 1);
> +
> +error_setg(&s->migration_blocker,
> + "x-data-plane do
Re: [Qemu-devel] [PATCH v2 05/39] fdsets: use weak aliases instead of qemu-tool.c/qemu-user.c
Il 15/11/2012 19:01, Stefan Weil ha scritto: > Hi Paolo, > > this patch breaks QEMU on 32 and 64 bit hosts, native and with Wine. > It's easy to reproduce the SIGSEGV crash: just add a -snapshot option. > Obviously the critical code is executed only when this option was used. I cannot reproduce this, so it must be an assembler or linker bug. Can you try the alternative code that is used for Mac OS X? Paolo > Here is a simple command line using Wine: > > wine i386-softmmu/qemu-system-i386 -L pc-bios -snapshot Makefile > > The disk image does not matter, so I just selected QEMU's Makefile. > > It looks like weak symbols are not really working with MinGW > (Blue Swirl previously pointed out that only ELF and a.out are > officially supported). > > I can see in the debugger that QEMU wants to call monitor_fdset_dup_fd_find > from qemu_close. > > In previous versions, this was just a dummy function returning 0. > Now, it is the function in monitor.c, but the address does not match > exactly, so the code addresses lines near the beginning of > monitor_fdset_dup_fd_find which does not work of course. > > A trivial workaround is calling default_fdset_dup_fd_find which > restores the old behaviour. I expect that all other weak functions > would show the same problem if they were used. > > Regards, > > Stefan > >
Re: [Qemu-devel] [RFC PATCH v4 2/3] use uimage_reset to reload uimage
On Wed, Nov 14, 2012 at 2:28 PM, Olivia Yin wrote:
> Signed-off-by: Olivia Yin
> ---
> hw/loader.c | 64 ++
> 1 files changed, 46 insertions(+), 18 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index a8a0a09..1a909d0 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -55,6 +55,8 @@
> #include
>
> static int roms_loaded;
> +static int kernel_loaded;
> +static int *is_linux;
Why do we need these 2 new variables?
> /* return the size or -1 if error */
> int get_image_size(const char *filename)
> @@ -472,15 +474,14 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t
> *src,
> return dstbytes;
> }
>
> -/* Load a U-Boot image. */
> -int load_uimage(const char *filename, hwaddr *ep,
> -hwaddr *loadaddr, int *is_linux)
> +/* write uimage into memory */
> +static int uimage_physical_loader(const char *filename, uint8_t **data,
> + hwaddr *loadaddr, int *is_linux)
> {
> int fd;
> int size;
> uboot_image_header_t h;
> uboot_image_header_t *hdr = &h;
> -uint8_t *data = NULL;
> int ret = -1;
>
> fd = open(filename, O_RDONLY | O_BINARY);
> @@ -521,10 +522,9 @@ int load_uimage(const char *filename, hwaddr *ep,
> *is_linux = 0;
> }
>
> -*ep = hdr->ih_ep;
> -data = g_malloc(hdr->ih_size);
> +*data = g_malloc(hdr->ih_size);
>
> -if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
> +if (read(fd, *data, hdr->ih_size) != hdr->ih_size) {
> fprintf(stderr, "Error reading file\n");
> goto out;
> }
> @@ -534,11 +534,11 @@ int load_uimage(const char *filename, hwaddr *ep,
> size_t max_bytes;
> ssize_t bytes;
>
> -compressed_data = data;
> +compressed_data = *data;
> max_bytes = UBOOT_MAX_GUNZIP_BYTES;
> -data = g_malloc(max_bytes);
> +*data = g_malloc(max_bytes);
>
> -bytes = gunzip(data, max_bytes, compressed_data, hdr->ih_size);
> +bytes = gunzip(*data, max_bytes, compressed_data, hdr->ih_size);
> g_free(compressed_data);
> if (bytes < 0) {
> fprintf(stderr, "Unable to decompress gzipped image!\n");
> @@ -547,7 +547,9 @@ int load_uimage(const char *filename, hwaddr *ep,
> hdr->ih_size = bytes;
> }
>
> -rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
> +if (!kernel_loaded) {
> +rom_add_blob_fixed(filename, *data, hdr->ih_size, hdr->ih_load);
> +}
Why do we need to keep the rom_add_blob_fixed() call? I thought the
point of this was to remove adding roms.
> if (loadaddr)
> *loadaddr = hdr->ih_load;
> @@ -555,12 +557,41 @@ int load_uimage(const char *filename, hwaddr *ep,
> ret = hdr->ih_size;
>
> out:
> -if (data)
> -g_free(data);
> close(fd);
> return ret;
> }
>
> +static void uimage_reset(void *opaque)
> +{
> +ImageFile *image = opaque;
> +uint8_t *data = NULL;
> +int size;
> +
> +size = uimage_physical_loader(image->name, &data, &image->addr,
> is_linux);
Not sure why we need is_linux here. I think you should just set that
parameter to NULL.
Nothing will look at is_linux.
Stuart
[Qemu-devel] Fwd: buildbot failure in qemu on ubuntu-default
Original Message Subject: buildbot failure in qemu on ubuntu-default Date: Thu, 15 Nov 2012 17:54:06 +0100 From: build...@spunk.home.kraxel.org To: kraxel...@gmail.com The Buildbot has detected a failed build on builder ubuntu-default while building qemu. Full details are available at: http://www.kraxel.org/bb/builders/ubuntu-default/builds/760 Buildbot URL: http://www.kraxel.org/bb/ Buildslave for this Build: ubuntu-32 Build Reason: scheduler Build Source Stamp: [branch master] 6801038bc52d61f81ac8a25fbe392f1bad982887 Blamelist: Aurelien Jarno ,陳韋任 (Wei-Ren Chen) BUILD FAILED: failed test sincerely, -The Buildbot == log tail == /i386/fdc/no_media_on_start: OK /i386/fdc/read_without_media:OK /i386/fdc/media_change: OK /i386/fdc/sense_interrupt: OK /i386/fdc/relative_seek: OK /i386/fdc/fuzz-registers:OK PASS: tests/fdc-test TEST: tests/hd-geo-test... (pid=29558) /i386/hd-geo/ide/none: OK /i386/hd-geo/ide/drive/mbr/blank:OK /i386/hd-geo/ide/drive/mbr/lba: OK /i386/hd-geo/ide/drive/mbr/chs: OK /i386/hd-geo/ide/drive/user/chs: OK /i386/hd-geo/ide/drive/user/chst:OK /i386/hd-geo/ide/drive/cd_0: OK /i386/hd-geo/ide/device/mbr/blank: OK /i386/hd-geo/ide/device/mbr/lba: OK /i386/hd-geo/ide/device/mbr/chs: OK /i386/hd-geo/ide/device/user/chs:OK /i386/hd-geo/ide/device/user/chst: OK PASS: tests/hd-geo-test TEST: tests/rtc-test... (pid=29619) /i386/rtc/bcd/check-time:OK /i386/rtc/dec/check-time:OK /i386/rtc/alarm-time:OK /i386/rtc/set-year/20xx: ** ERROR:tests/rtc-test.c:209:set_year_20xx: assertion failed (cmos_read(RTC_HOURS) == 0x02): (25 == 2) FAIL GTester: last random seed: R02S4d9d0b80b9b0a18ebdf4ffd3e739eb52 (pid=29629) /i386/rtc/set-year/1980: OK /i386/rtc/fuzz-registers: == full log == http://www.kraxel.org/bb/builders/ubuntu-default/builds/760/steps/test/logs/stdio
Re: [Qemu-devel] [PATCH 3/7] dataplane: add virtqueue vring code
Stefan Hajnoczi writes:
> The virtio-blk-data-plane cannot access memory using the usual QEMU
> functions since it executes outside the global mutex and the memory APIs
> are this time are not thread-safe.
>
> This patch introduces a virtqueue module based on the kernel's vhost
> vring code. The trick is that we map guest memory ahead of time and
> access it cheaply outside the global mutex.
>
> Once the hardware emulation code can execute outside the global mutex it
> will be possible to drop this code.
>
> Signed-off-by: Stefan Hajnoczi
> ---
> hw/Makefile.objs | 2 +-
> hw/dataplane/Makefile.objs | 3 +
> hw/dataplane/vring.c | 321
> +
> hw/dataplane/vring.h | 54
> trace-events | 3 +
> 5 files changed, 382 insertions(+), 1 deletion(-)
> create mode 100644 hw/dataplane/Makefile.objs
> create mode 100644 hw/dataplane/vring.c
> create mode 100644 hw/dataplane/vring.h
>
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index af4ab0c..da8ef0c 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -1,4 +1,4 @@
> -common-obj-y = usb/ ide/
> +common-obj-y = usb/ ide/ dataplane/
> common-obj-y += loader.o
> common-obj-$(CONFIG_VIRTIO) += virtio-console.o
> common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
> diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs
> new file mode 100644
> index 000..b58544f
> --- /dev/null
> +++ b/hw/dataplane/Makefile.objs
> @@ -0,0 +1,3 @@
> +ifeq ($(CONFIG_VIRTIO), y)
> +common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o
> +endif
> diff --git a/hw/dataplane/vring.c b/hw/dataplane/vring.c
> new file mode 100644
> index 000..6aacce8
> --- /dev/null
> +++ b/hw/dataplane/vring.c
> @@ -0,0 +1,321 @@
> +/* Copyright 2012 Red Hat, Inc.
> + * Copyright IBM, Corp. 2012
> + *
> + * Based on Linux vhost code:
> + * Copyright (C) 2009 Red Hat, Inc.
> + * Copyright (C) 2006 Rusty Russell IBM Corporation
> + *
> + * Author: Michael S. Tsirkin
> + * Stefan Hajnoczi
> + *
> + * Inspiration, some code, and most witty comments come from
> + * Documentation/virtual/lguest/lguest.c, by Rusty Russell
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.
> + */
> +
> +#include "trace.h"
> +#include "hw/dataplane/vring.h"
> +
> +/* Map target physical address to host address
> + */
> +static inline void *phys_to_host(Vring *vring, hwaddr phys)
> +{
> +/* Adjust for 3.6-4 GB PCI memory range */
> +if (phys >= 0x1) {
> +phys -= 0x1 - 0xe000;
> +} else if (phys >= 0xe000) {
> +fprintf(stderr, "phys_to_host bad physical address in "
> +"PCI range %#lx\n", phys);
> +exit(1);
> +}
> +return vring->phys_mem_zero_host_ptr + phys;
> +}
This is too dirty to merge. This code should use a slot mechanism
similar to what vhost uses. Then it has a plausible chance of working
on other architectures.
> +
> +/* Setup for cheap target physical to host address conversion
> + *
> + * This is a hack for direct access to guest memory, we're not really allowed
> + * to do this.
> + */
> +static void setup_phys_to_host(Vring *vring)
> +{
> +hwaddr len = 4096; /* RAM is really much larger but we cheat */
> +vring->phys_mem_zero_host_ptr = cpu_physical_memory_map(0, &len, 0);
> +if (!vring->phys_mem_zero_host_ptr) {
> +fprintf(stderr, "setup_phys_to_host failed\n");
> +exit(1);
> +}
> +}
You should be able to use a MemoryListener to keep the memory slot table
up to date. You'll need to use a mutex to protect it but again since
it's mostly uncontended, that shouldn't be a problem.
Regards,
Anthony Liguori
> +
> +/* Map the guest's vring to host memory
> + *
> + * This is not allowed but we know the ring won't move.
> + */
> +void vring_setup(Vring *vring, VirtIODevice *vdev, int n)
> +{
> +setup_phys_to_host(vring);
> +
> +vring_init(&vring->vr, virtio_queue_get_num(vdev, n),
> + phys_to_host(vring, virtio_queue_get_ring_addr(vdev, n)),
> 4096);
> +
> +vring->last_avail_idx = 0;
> +vring->last_used_idx = 0;
> +vring->signalled_used = 0;
> +vring->signalled_used_valid = false;
> +
> +trace_vring_setup(virtio_queue_get_ring_addr(vdev, n),
> + vring->vr.desc, vring->vr.avail, vring->vr.used);
> +}
> +
> +/* Toggle guest->host notifies */
> +void vring_set_notification(VirtIODevice *vdev, Vring *vring, bool enable)
> +{
> +if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
> +if (enable) {
> +vring_avail_event(&vring->vr) = vring->vr.avail->idx;
> +}
> +} else if (enable) {
> +vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY;
> +} else {
> +vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY;
> +}
> +}
> +
> +/* This is stolen from linux/drivers/vhost/vhost.c:vhost_notify() */
> +bool vring_should_notify(Vir
Re: [Qemu-devel] [PATCH 1/7] raw-posix: add raw_get_aio_fd() for virtio-blk-data-plane
Stefan Hajnoczi writes:
> The raw_get_aio_fd() function allows virtio-blk-data-plane to get the
> file descriptor of a raw image file with Linux AIO enabled. This
> interface is really a layering violation that can be resolved once the
> block layer is able to run outside the global mutex - at that point
> virtio-blk-data-plane will switch from custom Linux AIO code to using
> the block layer.
>
> Signed-off-by: Stefan Hajnoczi
I think this creates user confusion because virtio-blk-data-plane can't
actually take a BDS.
So why not just make a string 'filename' property and open it directly
in virtio-blk-data-plane? Then it's at least clear to the user and
management tools what the device is capable of doing.
Regards,
Anthony Liguori
> ---
> block.h | 9 +
> block/raw-posix.c | 34 ++
> 2 files changed, 43 insertions(+)
>
> diff --git a/block.h b/block.h
> index 722c620..2dc6aaf 100644
> --- a/block.h
> +++ b/block.h
> @@ -365,6 +365,15 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
> void bdrv_set_in_use(BlockDriverState *bs, int in_use);
> int bdrv_in_use(BlockDriverState *bs);
>
> +#ifdef CONFIG_LINUX_AIO
> +int raw_get_aio_fd(BlockDriverState *bs);
> +#else
> +static inline int raw_get_aio_fd(BlockDriverState *bs)
> +{
> +return -ENOTSUP;
> +}
> +#endif
> +
> enum BlockAcctType {
> BDRV_ACCT_READ,
> BDRV_ACCT_WRITE,
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index f2f0404..fc04981 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1768,6 +1768,40 @@ static BlockDriver bdrv_host_cdrom = {
> };
> #endif /* __FreeBSD__ */
>
> +#ifdef CONFIG_LINUX_AIO
> +/**
> + * Return the file descriptor for Linux AIO
> + *
> + * This function is a layering violation and should be removed when it
> becomes
> + * possible to call the block layer outside the global mutex. It allows the
> + * caller to hijack the file descriptor so I/O can be performed outside the
> + * block layer.
> + */
> +int raw_get_aio_fd(BlockDriverState *bs)
> +{
> +BDRVRawState *s;
> +
> +if (!bs->drv) {
> +return -ENOMEDIUM;
> +}
> +
> +if (bs->drv == bdrv_find_format("raw")) {
> +bs = bs->file;
> +}
> +
> +/* raw-posix has several protocols so just check for raw_aio_readv */
> +if (bs->drv->bdrv_aio_readv != raw_aio_readv) {
> +return -ENOTSUP;
> +}
> +
> +s = bs->opaque;
> +if (!s->use_aio) {
> +return -ENOTSUP;
> +}
> +return s->fd;
> +}
> +#endif /* CONFIG_LINUX_AIO */
> +
> static void bdrv_file_init(void)
> {
> /*
> --
> 1.8.0
Re: [Qemu-devel] [RFC PATCH v4 1/3] use image_file_reset to reload initrd image
> /* read()-like version */
> ssize_t read_targphys(const char *name,
>int fd, hwaddr dst_addr, size_t nbytes)
> @@ -113,6 +146,12 @@ int load_image_targphys(const char *filename,
> }
> if (size > 0) {
> rom_add_file_fixed(filename, addr, -1);
> +ImageFile *image;
> +image = g_malloc0(sizeof(*image));
> +image->name = g_strdup(filename);
> +image->addr = addr;
> +
> +qemu_register_reset(image_file_reset, image);
You need to remove the call to rom_add_file_fixed(), no?
Stuart
Re: [Qemu-devel] [PATCH 7/7] virtio-blk: add x-data-plane=on|off performance feature
"Michael S. Tsirkin" wrote on 11/15/2012 12:48:49 PM:
> From: "Michael S. Tsirkin"
> To: Stefan Hajnoczi ,
> Cc: qemu-devel@nongnu.org, Anthony Liguori/Austin/IBM@IBMUS, Paolo
> Bonzini , Kevin Wolf , Asias
> He , Khoa Huynh/Austin/IBM@IBMUS
> Date: 11/15/2012 12:46 PM
> Subject: Re: [PATCH 7/7] virtio-blk: add x-data-plane=on|off
> performance feature
>
> On Thu, Nov 15, 2012 at 04:19:06PM +0100, Stefan Hajnoczi wrote:
> > The virtio-blk-data-plane feature is easy to integrate into
> > hw/virtio-blk.c. The data plane can be started and stopped similar to
> > vhost-net.
> >
> > Users can take advantage of the virtio-blk-data-plane feature using the
> > new -device virtio-blk-pci,x-data-plane=on property.
> >
> > The x-data-plane name was chosen because at this stage the feature is
> > experimental and likely to see changes in the future.
> >
> > If the VM configuration does not support virtio-blk-data-plane an error
> > message is printed. Although we could fall back to regular virtio-blk,
> > I prefer the explicit approach since it prompts the user to fix their
> > configuration if they want the performance benefit of
> > virtio-blk-data-plane.
> >
> > Limitations:
> > * Only format=raw is supported
> > * Live migration is not supported
> > * Block jobs, hot unplug, and other operations fail with -EBUSY
> > * I/O throttling limits are ignored
> > * Only Linux hosts are supported due to Linux AIO usage
> >
> > Signed-off-by: Stefan Hajnoczi
>
>
> Would be very interested in learning about the performance
> impact of this. How does this compare to current model and
> to vhost-blk?
I plan to do a complete evaluation of this patchset in the coming days.
However, I have done quite a bit of performance testing on earlier versions
of the data-plane and vhost-blk code bits. Here's what I found:
1) The existing kvm/qemu code can only handle up to about 150,000 IOPS for
a single KVM guest. The bottleneck here is the global qemu mutex.
2) With performance tuning, I was able to achieve 1.33 million IOPS for a
single KVM guest with data-plane. This is very close to the
1.4-million-IOPS
limit of my storage setup.
3) Similarly, I was able to get up to 1.34 million IOPS for a single KVM
guest with an earlier prototype of vhost-blk (using Linux AIO, from Liu
Yuan).
This shows that bypassing the global qemu mutex would allow us to achieve
much higher I/O rates. In fact, these I/O rates would handily beat claims
from all other competing hypervisors.
I am currently evaluating the latest vhost-blk code bits from Asias He and
will report results soon. I have already got past 1 million IOPS per guest
with Asias' code and is trying to see if I could get even higher I/O rates.
And as I mentioned earlier, I'll also evaluate this latest data-plane code
from Stefan real soon. Please stay tuned...
-Khoa
>
>
> > ---
> > hw/virtio-blk.c | 59
> -
> > hw/virtio-blk.h | 1 +
> > hw/virtio-pci.c | 3 +++
> > 3 files changed, 62 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> > index e25cc96..7f6004e 100644
> > --- a/hw/virtio-blk.c
> > +++ b/hw/virtio-blk.c
> > @@ -17,6 +17,8 @@
> > #include "hw/block-common.h"
> > #include "blockdev.h"
> > #include "virtio-blk.h"
> > +#include "hw/dataplane/virtio-blk.h"
> > +#include "migration.h"
> > #include "scsi-defs.h"
> > #ifdef __linux__
> > # include
> > @@ -33,6 +35,8 @@ typedef struct VirtIOBlock
> > VirtIOBlkConf *blk;
> > unsigned short sector_mask;
> > DeviceState *qdev;
> > +VirtIOBlockDataPlane *dataplane;
> > +Error *migration_blocker;
> > } VirtIOBlock;
> >
> > static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
> > @@ -407,6 +411,14 @@ static void virtio_blk_handle_output
> (VirtIODevice *vdev, VirtQueue *vq)
> > .num_writes = 0,
> > };
> >
> > +/* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so
start
> > + * dataplane here instead of waiting for .set_status().
> > + */
> > +if (s->dataplane) {
> > +virtio_blk_data_plane_start(s->dataplane);
> > +return;
> > +}
> > +
> > while ((req = virtio_blk_get_request(s))) {
> > virtio_blk_handle_request(req, &mrb);
> > }
> > @@ -446,8 +458,13 @@ static void virtio_blk_dma_restart_cb(void
> *opaque, int running,
> > {
> > VirtIOBlock *s = opaque;
> >
> > -if (!running)
> > +if (!running) {
> > +/* qemu_drain_all() doesn't know about data plane, quiesce
here */
> > +if (s->dataplane) {
> > +virtio_blk_data_plane_drain(s->dataplane);
> > +}
> > return;
> > +}
> >
> > if (!s->bh) {
> > s->bh = qemu_bh_new(virtio_blk_dma_restart_bh, s);
> > @@ -538,6 +555,10 @@ static void virtio_blk_set_status
> (VirtIODevice *vdev, uint8_t status)
> > VirtIOBlock *s = to_virtio_blk(vdev);
> > uint32_t features;
> >
> > +if (s->dataplane && !(st
Re: [Qemu-devel] Question about comment on MIPS POOL32AXF encoding
> -Original Message- > From: qemu-devel-bounces+ericj=mips@nongnu.org [mailto:qemu-devel- > bounces+ericj=mips@nongnu.org] On Behalf Of ??? (Wei-Ren Chen) > Sent: Thursday, November 15, 2012 1:03 AM > To: qemu-devel@nongnu.org > Cc: Jia Liu > Subject: [Qemu-devel] Question about comment on MIPS POOL32AXF encoding > > Hi all, > > I am reading POOL32AXF encoding in target-mips/translate.c, > and can't understand what the comment says. For example, > > /* bits 13..12 for 0x01 */ > MFHI_ACC = 0x0, > MFLO_ACC = 0x1, > MTHI_ACC = 0x2, > MTLO_ACC = 0x3, > > I compare this with microMIPS32 manual [1] Table 6.5, but I still > don't understand why they're encoded in such way. The comment says > "bits 13..12 for 0x01", I can roughly understand the enum list > bits 13..12 here, but what "for 0x01" exactly means? Please refer to the following document. MIPS® Architecture for Programmers VolumeIV-e: The MIPS® DSP Application-Specific Extension to the microMIPS32™ Architecture http://www.mips.com/secure-download/index.dot?product_name=/auth/MD00764-2B-microMIPS32DSP-AFP-02.34.pdf -Eric
Re: [Qemu-devel] [PATCH 7/7] virtio-blk: add x-data-plane=on|off performance feature
On Thu, Nov 15, 2012 at 04:19:06PM +0100, Stefan Hajnoczi wrote:
> The virtio-blk-data-plane feature is easy to integrate into
> hw/virtio-blk.c. The data plane can be started and stopped similar to
> vhost-net.
>
> Users can take advantage of the virtio-blk-data-plane feature using the
> new -device virtio-blk-pci,x-data-plane=on property.
>
> The x-data-plane name was chosen because at this stage the feature is
> experimental and likely to see changes in the future.
>
> If the VM configuration does not support virtio-blk-data-plane an error
> message is printed. Although we could fall back to regular virtio-blk,
> I prefer the explicit approach since it prompts the user to fix their
> configuration if they want the performance benefit of
> virtio-blk-data-plane.
>
> Limitations:
> * Only format=raw is supported
> * Live migration is not supported
> * Block jobs, hot unplug, and other operations fail with -EBUSY
> * I/O throttling limits are ignored
> * Only Linux hosts are supported due to Linux AIO usage
>
> Signed-off-by: Stefan Hajnoczi
Would be very interested in learning about the performance
impact of this. How does this compare to current model and
to vhost-blk?
> ---
> hw/virtio-blk.c | 59
> -
> hw/virtio-blk.h | 1 +
> hw/virtio-pci.c | 3 +++
> 3 files changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index e25cc96..7f6004e 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -17,6 +17,8 @@
> #include "hw/block-common.h"
> #include "blockdev.h"
> #include "virtio-blk.h"
> +#include "hw/dataplane/virtio-blk.h"
> +#include "migration.h"
> #include "scsi-defs.h"
> #ifdef __linux__
> # include
> @@ -33,6 +35,8 @@ typedef struct VirtIOBlock
> VirtIOBlkConf *blk;
> unsigned short sector_mask;
> DeviceState *qdev;
> +VirtIOBlockDataPlane *dataplane;
> +Error *migration_blocker;
> } VirtIOBlock;
>
> static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
> @@ -407,6 +411,14 @@ static void virtio_blk_handle_output(VirtIODevice *vdev,
> VirtQueue *vq)
> .num_writes = 0,
> };
>
> +/* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
> + * dataplane here instead of waiting for .set_status().
> + */
> +if (s->dataplane) {
> +virtio_blk_data_plane_start(s->dataplane);
> +return;
> +}
> +
> while ((req = virtio_blk_get_request(s))) {
> virtio_blk_handle_request(req, &mrb);
> }
> @@ -446,8 +458,13 @@ static void virtio_blk_dma_restart_cb(void *opaque, int
> running,
> {
> VirtIOBlock *s = opaque;
>
> -if (!running)
> +if (!running) {
> +/* qemu_drain_all() doesn't know about data plane, quiesce here */
> +if (s->dataplane) {
> +virtio_blk_data_plane_drain(s->dataplane);
> +}
> return;
> +}
>
> if (!s->bh) {
> s->bh = qemu_bh_new(virtio_blk_dma_restart_bh, s);
> @@ -538,6 +555,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev,
> uint8_t status)
> VirtIOBlock *s = to_virtio_blk(vdev);
> uint32_t features;
>
> +if (s->dataplane && !(status & VIRTIO_CONFIG_S_DRIVER)) {
> +virtio_blk_data_plane_stop(s->dataplane);
> +}
> +
> if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> return;
> }
> @@ -604,6 +625,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> VirtIOBlkConf *blk)
> {
> VirtIOBlock *s;
> static int virtio_blk_id;
> +int fd = -1;
>
> if (!blk->conf.bs) {
> error_report("drive property not set");
> @@ -619,6 +641,21 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> VirtIOBlkConf *blk)
> return NULL;
> }
>
> +if (blk->data_plane) {
> +if (blk->scsi) {
> +error_report("device is incompatible with x-data-plane, "
> + "use scsi=off");
> +return NULL;
> +}
> +
> +fd = raw_get_aio_fd(blk->conf.bs);
> +if (fd < 0) {
> +error_report("drive is incompatible with x-data-plane, "
> + "use format=raw,cache=none,aio=native");
> +return NULL;
> +}
> +}
> +
> s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
>sizeof(struct virtio_blk_config),
>sizeof(VirtIOBlock));
> @@ -636,6 +673,17 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> VirtIOBlkConf *blk)
>
> s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
>
> +if (fd >= 0) {
> +s->dataplane = virtio_blk_data_plane_create(&s->vdev, fd);
> +
> +/* Prevent block operations that conflict with data plane thread */
> +bdrv_set_in_use(s->bs, 1);
> +
> +error_setg(&s->migration_blocker,
> + "x-data-plane does not
Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c
On 15 November 2012 18:42, Eduardo Habkost wrote: > DeviceState CPUs have to be reset too, and DeviceState uses the > reset-handler system to make sure DeviceState objects are reset, so it > won't be softmmu-specific. > > An alternative is to make empty stubs for qemu_[un]register_reset(), and > not move the reset-handler system to *-user. But somehow I feel better > having a working qemu_register_reset() function (and then adding a > qemu_devices_reset() call to *-user) than having a fake > qemu_register_reset() function and having to use a different API to > reset the CPUs on on *-user. cf the pretty nasty stuff in linux-user/main.c which currently calls cpu_reset() but only for some targets. -- PMM
Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c
On Thu, Nov 15, 2012 at 02:54:57AM +0100, Andreas Färber wrote:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > The core qdev code uses the reset handler list from vl.c, so move
> > qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> > to qdev.c.
> >
> > The function declarations were moved to a new qdev-reset.h file, that is
> > included by hw.h to keep compatibility, so we don't need to change all
> > files that use qemu_register_reset().
> >
> > Signed-off-by: Eduardo Habkost
> > ---
> > hw/hw.h | 6 +-
> > hw/qdev-reset.h | 11 +++
> > hw/qdev.c | 41 +
> > hw/qdev.h | 1 +
> > sysemu.h| 1 -
> > vl.c| 40
> > 6 files changed, 54 insertions(+), 46 deletions(-)
> > create mode 100644 hw/qdev-reset.h
> >
> > diff --git a/hw/hw.h b/hw/hw.h
> > index f530f6f..622a157 100644
> > --- a/hw/hw.h
> > +++ b/hw/hw.h
> > @@ -14,6 +14,7 @@
> > #include "qemu-file.h"
> > #include "vmstate.h"
> > #include "qemu-log.h"
> > +#include "qdev-reset.h"
> >
> > #ifdef NEED_CPU_H
> > #if TARGET_LONG_BITS == 64
> > @@ -37,11 +38,6 @@
> > #endif
> > #endif
> >
> > -typedef void QEMUResetHandler(void *opaque);
> > -
> > -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > -
> > /* handler to set the boot_device order for a specific type of QEMUMachine
> > */
> > /* return 0 if success */
> > typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> > diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> > new file mode 100644
> > index 000..40ae9a5
> > --- /dev/null
> > +++ b/hw/qdev-reset.h
> > @@ -0,0 +1,11 @@
> > +/* Device reset handler function registration, used by qdev */
> > +#ifndef QDEV_RESET_H
> > +#define QDEV_RESET_H
> > +
> > +typedef void QEMUResetHandler(void *opaque);
> > +
> > +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > +void qemu_devices_reset(void);
> > +
> > +#endif /* QDEV_RESET_H */
> > diff --git a/hw/qdev.c b/hw/qdev.c
> > index 2cc6434..c242097 100644
> > --- a/hw/qdev.c
> > +++ b/hw/qdev.c
> > @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
> > static bool qdev_hot_added = false;
> > static bool qdev_hot_removed = false;
> >
> > +typedef struct QEMUResetEntry {
> > +QTAILQ_ENTRY(QEMUResetEntry) entry;
> > +QEMUResetHandler *func;
> > +void *opaque;
> > +} QEMUResetEntry;
> > +
> > +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > +QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > +
> > +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > +{
> > +QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > +
> > +re->func = func;
> > +re->opaque = opaque;
> > +QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > +}
> > +
> > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > +{
> > +QEMUResetEntry *re;
> > +
> > +QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > +if (re->func == func && re->opaque == opaque) {
> > +QTAILQ_REMOVE(&reset_handlers, re, entry);
> > +g_free(re);
> > +return;
> > +}
> > +}
> > +}
>
> My tired mind does not like this move and the naming qdev-reset.h.
> The reset handling infrastructure is not limited to DeviceState (qdev),
> it takes an opaque and is limited to softmmu whereas qdev prefers its
> own DeviceClass::reset hook.
True, it isn't qdev-specific. But it doesn't belong to vl.c either.
Should we create a reset.o file just for those few lines of code, or is
there any obvious place where this code can go?
DeviceState CPUs have to be reset too, and DeviceState uses the
reset-handler system to make sure DeviceState objects are reset, so it
won't be softmmu-specific.
An alternative is to make empty stubs for qemu_[un]register_reset(), and
not move the reset-handler system to *-user. But somehow I feel better
having a working qemu_register_reset() function (and then adding a
qemu_devices_reset() call to *-user) than having a fake
qemu_register_reset() function and having to use a different API to
reset the CPUs on on *-user.
>
> Andreas
>
> > +
> > +void qemu_devices_reset(void)
> > +{
> > +QEMUResetEntry *re, *nre;
> > +
> > +/* reset all devices */
> > +QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > +re->func(re->opaque);
> > +}
> > +}
> > +
> > const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> > {
> > DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > diff --git a/hw/qdev.h b/hw/qdev.h
> > index 365b8d6..2487b3b 100644
> > --- a/hw/qdev.h
> > +++ b/hw/qdev.h
> > @@ -5,5 +5,6 @@
> > #include "qdev-core.h"
> > #include "qdev-properties.h"
> > #include "qdev-monitor.h"
> > +#include "qdev-reset.h"
> >
> >
Re: [Qemu-devel] slow virtio network with vhost=on and multiple cores
Dietmar Maurer wrote: >> > You can also reproduce the problem with RHEL6.2 as guest But it seems >> > RHEL 6.3 fixed it. >> >> RHEL6.2 on ubuntu host? > > I only tested with RHEL6.3 kernel on host. can you check if there is a difference on interrupt delivery between those two? cat /proc/interrupts should be sufficient after some traffic has flown. peter > > >
Re: [Qemu-devel] [PATCH v2] qga/channel-posix.c: include headers it needs
On Thu, Nov 15, 2012 at 02:19:31AM +0100, Igor Mammedov wrote: > From: Eduardo Habkost > > Include: > - for errno > - & for fcntl() > - "qemu-stdio.h" for qemu_open() > > Some of those headers were probably being included by accident because > some other headers were including qemu-common.h, but those headers > should eventually stop including qemu-common.h. > > Signed-off-by: Eduardo Habkost > Signed-off-by: Igor Mammedov > --- > v2 > - include qemu-common.h for EXIT_FAILURE, exit and qemu_open definitions Thanks! The commit message above needs to be edited, too. :-) > --- > qga/channel-posix.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/qga/channel-posix.c b/qga/channel-posix.c > index d152827..cc9 100644 > --- a/qga/channel-posix.c > +++ b/qga/channel-posix.c > @@ -1,7 +1,11 @@ > #include > #include > +#include > +#include > +#include > #include "qemu_socket.h" > #include "qga/channel.h" > +#include "qemu-common.h" > > #ifdef CONFIG_SOLARIS > #include > -- > 1.7.11.7 > -- Eduardo
Re: [Qemu-devel] [PATCH] iscsi: fix deadlock during login
Paolo Bonzini wrote: > Il 15/11/2012 15:57, ronnie sahlberg ha scritto: >> I dont know if we should switch to use synchronous code here. >> It is much nicer if all code is async. > > bdrv_open is generally synchronous, so I think Peter's patch is ok. if all is sync wouldn't it be best to have all code in iscsi_open sync then and convert the iscsi_inquiry and iscsi_readcapacity commands also to sync? Peter > > Paolo > >> Is it possible to add a timeout instead that would break out if the >> connect/login has not completed within a certain amount of time? > > >
Re: [Qemu-devel] [PATCH v2 05/39] fdsets: use weak aliases instead of qemu-tool.c/qemu-user.c
Am 31.10.2012 16:30, schrieb Paolo Bonzini:
Signed-off-by: Paolo Bonzini
---
cutils.c | 5 -
osdep.c | 30 ++
qemu-common.h | 1 -
qemu-tool.c | 20
qemu-user.c | 20
5 file modificati, 30 inserzioni(+), 46 rimozioni(-)
diff --git a/cutils.c b/cutils.c
index 6f9f799..4f0692f 100644
--- a/cutils.c
+++ b/cutils.c
@@ -280,11 +280,6 @@ int qemu_parse_fd(const char *param)
return fd;
}
-int qemu_parse_fdset(const char *param)
-{
-return qemu_parse_fd(param);
-}
-
/* round down to the nearest power of 2*/
int64_t pow2floor(int64_t value)
{
diff --git a/osdep.c b/osdep.c
index 3b25297..0061f74 100644
--- a/osdep.c
+++ b/osdep.c
@@ -144,6 +144,11 @@ fail:
errno = serrno;
return -1;
}
+
+static int qemu_parse_fdset(const char *param)
+{
+return qemu_parse_fd(param);
+}
#endif
/*
@@ -404,3 +409,28 @@ bool fips_get_state(void)
{
return fips_enabled;
}
+
+
+static int default_fdset_get_fd(int64_t fdset_id, int flags)
+{
+return -1;
+}
+QEMU_WEAK_ALIAS(monitor_fdset_get_fd, default_fdset_get_fd);
+
+static int default_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+{
+return -1;
+}
+QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_add, default_fdset_dup_fd_add);
+
+static int default_fdset_dup_fd_remove(int dup_fd)
+{
+return -1;
+}
+QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_remove);
+
+static int default_fdset_dup_fd_find(int dup_fd)
+{
+return -1;
+}
+QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_find, default_fdset_dup_fd_find);
diff --git a/qemu-common.h b/qemu-common.h
index b54612b..36ce522 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -167,7 +167,6 @@ int qemu_fls(int i);
int qemu_fdatasync(int fd);
int fcntl_setfl(int fd, int flag);
int qemu_parse_fd(const char *param);
-int qemu_parse_fdset(const char *param);
/*
* strtosz() suffixes used to specify the default treatment of an
diff --git a/qemu-tool.c b/qemu-tool.c
index f2f9813..84273ae 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -68,26 +68,6 @@ void monitor_protocol_event(MonitorEvent event, QObject
*data)
{
}
-int monitor_fdset_get_fd(int64_t fdset_id, int flags)
-{
-return -1;
-}
-
-int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
-{
-return -1;
-}
-
-int monitor_fdset_dup_fd_remove(int dup_fd)
-{
-return -1;
-}
-
-int monitor_fdset_dup_fd_find(int dup_fd)
-{
-return -1;
-}
-
int64_t cpu_get_clock(void)
{
return qemu_get_clock_ns(rt_clock);
diff --git a/qemu-user.c b/qemu-user.c
index 13fb9ae..08ccb0f 100644
--- a/qemu-user.c
+++ b/qemu-user.c
@@ -35,23 +35,3 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list
ap)
void monitor_set_error(Monitor *mon, QError *qerror)
{
}
-
-int monitor_fdset_get_fd(int64_t fdset_id, int flags)
-{
-return -1;
-}
-
-int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
-{
-return -1;
-}
-
-int monitor_fdset_dup_fd_remove(int dup_fd)
-{
-return -1;
-}
-
-int monitor_fdset_dup_fd_find(int dup_fd)
-{
-return -1;
-}
Hi Paolo,
this patch breaks QEMU on 32 and 64 bit hosts, native and with Wine.
It's easy to reproduce the SIGSEGV crash: just add a -snapshot option.
Obviously the critical code is executed only when this option was used.
Here is a simple command line using Wine:
wine i386-softmmu/qemu-system-i386 -L pc-bios -snapshot Makefile
The disk image does not matter, so I just selected QEMU's Makefile.
It looks like weak symbols are not really working with MinGW
(Blue Swirl previously pointed out that only ELF and a.out are
officially supported).
I can see in the debugger that QEMU wants to call monitor_fdset_dup_fd_find
from qemu_close.
In previous versions, this was just a dummy function returning 0.
Now, it is the function in monitor.c, but the address does not match
exactly, so the code addresses lines near the beginning of
monitor_fdset_dup_fd_find which does not work of course.
A trivial workaround is calling default_fdset_dup_fd_find which
restores the old behaviour. I expect that all other weak functions
would show the same problem if they were used.
Regards,
Stefan
Re: [Qemu-devel] [PATCH] block: vpc support for ~2 TB disks
I'm not sure I understand your question. In XenServer blktap2 we set CHS to 65535*16*255 in the VHD metadata for disks larger than 127GB. We don't really care about these values, we just store them in the VHD metadata. > -Original Message- > From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Sent: 14 November 2012 16:36 > To: Thanos Makatos > Cc: Stefano Stabellini; Charles Arnold; qemu-devel@nongnu.org; Kevin > Wolf; stefa...@redhat.com; xen-de...@lists.xensource.com > Subject: Re: [PATCH] block: vpc support for ~2 TB disks > > Il 14/11/2012 17:25, Thanos Makatos ha scritto: > > We don't use qemu's VHD driver in XenServer. Instead, we use blktap2 > > to create a block device in dom0 serving the VHD file in question, > and > > have qemu open that block device instead of the VHD file itself. > > Yes, the question is how you handle disks bigger than 127GB, so that > QEMU can do the same. > > Paolo
Re: [Qemu-devel] [PATCH] target-mips: Clean up microMIPS32 major opcode
> -Original Message- > From: 陳韋任 (Wei-Ren Chen) [mailto:che...@iis.sinica.edu.tw] > Sent: Wednesday, November 14, 2012 9:51 PM > To: Johnson, Eric > Cc: qemu-devel@nongnu.org; qemu-triv...@nongnu.org; Jia Liu; Aurelien > Jarno > Subject: Re: [Qemu-devel] [PATCH] target-mips: Clean up microMIPS32 major > opcode > > On Thu, Nov 15, 2012 at 04:01:32AM +, Johnson, Eric wrote: > > Hi Chen, > > > > Sorry I must have made a copy paste error. I access the documents > internally. I'll double check the link tomorrow. > > > > The document I referenced is the MIPS64 not the microMIPS64. > > > > Do not change the names. The LD32 and SD32 are microMIPS specific. The > assembler LD and SD opcodes work for either MIPS64 or microMIPS64. > > O.K., thanks for the help. :) > > How about DADDIU32, should I keep the 32 suffix, too? > I still can't find where POOL32S is. > > Regards, > chenwj The only opcode on the list that is incorrect is the POOL48A opcode. It should be removed. If the others are removed or renamed, I or someone else would just have to fix them at some point in the future. It seems the microMIPS64 Instruction Set manual is not one the external website. I'll ask about that. -Eric
Re: [Qemu-devel] [PATCH] target-mips: Clean up microMIPS32 major opcode
> -Original Message- > From: Aurelien Jarno [mailto:aurel...@aurel32.net] > Sent: Thursday, November 15, 2012 6:04 AM > To: Johnson, Eric > Cc: 陳韋任 (Wei-Ren Chen); qemu-devel@nongnu.org; qemu-triv...@nongnu.org; > che...@cs.nctu.edu.tw; Jia Liu > Subject: Re: [Qemu-devel] [PATCH] target-mips: Clean up microMIPS32 major > opcode > > On Thu, Nov 15, 2012 at 02:34:31AM +, Johnson, Eric wrote: > > Hi Chen, > > > > Please only remove the POOL48A opcode. > > > > The others are documented in the microMIPS64 Instruction Set manual ( > http://www.mips.com/secure-download/index.dot?product_name=/auth/MD00087- > 2B-MIPS64BIS-AFP-03.51.pdf ). See > http://www.mips.com/products/architectures/mips64/ for other relavent > docs. > > > > Instead of removing them please surround the POOL32S, DADDIU32, SD32, > and LD32 opcodes with > > #if defined(TARGET_MIPS64) > > > > I don't think a #if is necessary there, this makes the code more > difficult to read, while it doesn't change anything on the generated > code. Agreed. -Eric
[Qemu-devel] [Bug 1077838] Re: qemu-nbd -r -c taints device for subsequent usage, even after -d
** Changed in: qemu-kvm (Ubuntu Precise) Status: Triaged => In Progress ** Changed in: qemu-kvm (Ubuntu Quantal) Status: Triaged => In Progress -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1077838 Title: qemu-nbd -r -c taints device for subsequent usage, even after -d Status in QEMU: In Progress Status in “qemu-kvm” package in Ubuntu: Fix Released Status in “qemu-kvm” source package in Precise: In Progress Status in “qemu-kvm” source package in Quantal: In Progress Bug description: Something about qemu-nbd -r -c /dev/nbd0 someimg leaves cruft behind - subsequent connections get marked readonly. This is on quantal, haven't checked precise or raring. To demonstrate: # use one image qemu-img create -f qcow2 /tmp/1.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/1.qcow2 sudo mkfs -t ext4 /dev/nbd2 sudo qemu-nbd -d /dev/nbd2 # use a second one on the same nbd device, shows that reuse works: qemu-img create -f qcow2 /tmp/2.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/2.qcow2 sudo mkfs -t ext4 /dev/nbd2 sudo qemu-nbd -d /dev/nbd2 # connect an image in read only mode sudo qemu-nbd -r -c /dev/nbd2 /tmp/2.qcow2 sudo dumpe2fs /dev/nbd2 | head -n 3 sudo qemu-nbd -d /dev/nbd2 # now try to reuse in read-write mode again: qemu-img create -f qcow2 /tmp/3.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/3.qcow2 sudo mkfs -t ext4 /dev/nbd2 # here it goes boom: mke2fs 1.42.5 (29-Jul-2012) /dev/nbd2: Operation not permitted while setting up superblock # still need to cleanup sudo qemu-nbd -d /dev/nbd2 === SRU Justification: 1. Impact: mounting an nbd device as read-write after doing so read-only will cause the mount to erroneously (and quietly) be read-only. 2. Development fix: have qemu-nbd set the device to read-write when asked, rather than only setting read-only. 3. Stable fix: same as development fix 4. Test case: See above 5. Regression potential: The patch is localized to the handling of read-only flag in qemu-nbd, so any regression should not affect anything else. === To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1077838/+subscriptions
Re: [Qemu-devel] [PATCH] block: vpc support for ~2 TB disks
Il 15/11/2012 17:46, Charles Arnold ha scritto: >>> We don't use qemu's VHD driver in XenServer. Instead, we use blktap2 >>> >> to create a block device in dom0 serving the VHD file in question, >>> >> and have qemu open that block device instead of the VHD file itself. >> > >> > Yes, the question is how you handle disks bigger than 127GB, so that >> > QEMU can do the same. >> > > In analyzing a 160 GB VHD fixed disk image created on Windows 2008 R2, it > appears that > MS is also ignoring the CHS values in the footer geometry field in whatever > driver they use for accessing the image. The CHS values are set at > 65535,16,255 > which obviously doesn't represent an image size of 160 GB. Thanks, this would have been useful in the commit message. The patch looks good, Reviewed-by: Paolo Bonzini Paolo > This patch only extends the existing qemu driver to allow a larger image by > allowing > more heads. On real hardware, only 4 bits would be allowed for heads but we > don't > have that restriction in qemu.
[Qemu-devel] [Bug 1077838] Re: qemu-nbd -r -c taints device for subsequent usage, even after -d
** Also affects: qemu-kvm (Ubuntu Precise) Importance: Undecided Status: New ** Also affects: qemu-kvm (Ubuntu Quantal) Importance: Undecided Status: New ** Changed in: qemu-kvm (Ubuntu Precise) Importance: Undecided => High ** Changed in: qemu-kvm (Ubuntu Quantal) Importance: Undecided => High ** Changed in: qemu-kvm (Ubuntu Quantal) Status: New => Triaged ** Changed in: qemu-kvm (Ubuntu Precise) Status: New => Triaged ** Description changed: Something about qemu-nbd -r -c /dev/nbd0 someimg leaves cruft behind - subsequent connections get marked readonly. This is on quantal, haven't checked precise or raring. To demonstrate: # use one image qemu-img create -f qcow2 /tmp/1.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/1.qcow2 sudo mkfs -t ext4 /dev/nbd2 sudo qemu-nbd -d /dev/nbd2 # use a second one on the same nbd device, shows that reuse works: qemu-img create -f qcow2 /tmp/2.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/2.qcow2 sudo mkfs -t ext4 /dev/nbd2 sudo qemu-nbd -d /dev/nbd2 # connect an image in read only mode sudo qemu-nbd -r -c /dev/nbd2 /tmp/2.qcow2 sudo dumpe2fs /dev/nbd2 | head -n 3 sudo qemu-nbd -d /dev/nbd2 # now try to reuse in read-write mode again: qemu-img create -f qcow2 /tmp/3.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/3.qcow2 sudo mkfs -t ext4 /dev/nbd2 # here it goes boom: mke2fs 1.42.5 (29-Jul-2012) /dev/nbd2: Operation not permitted while setting up superblock # still need to cleanup sudo qemu-nbd -d /dev/nbd2 + + === + SRU Justification: + 1. Impact: mounting an nbd device as read-write after doing so read-only + will cause the mount to erroneously (and quietly) be read-only. + 2. Development fix: have qemu-nbd set the device to read-write when asked, + rather than only setting read-only. + 3. Stable fix: same as development fix + 4. Test case: See above + 5. Regression potential: The patch is localized to the handling of read-only + flag in qemu-nbd, so any regression should not affect anything else. + === -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1077838 Title: qemu-nbd -r -c taints device for subsequent usage, even after -d Status in QEMU: In Progress Status in “qemu-kvm” package in Ubuntu: Fix Released Status in “qemu-kvm” source package in Precise: In Progress Status in “qemu-kvm” source package in Quantal: Triaged Bug description: Something about qemu-nbd -r -c /dev/nbd0 someimg leaves cruft behind - subsequent connections get marked readonly. This is on quantal, haven't checked precise or raring. To demonstrate: # use one image qemu-img create -f qcow2 /tmp/1.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/1.qcow2 sudo mkfs -t ext4 /dev/nbd2 sudo qemu-nbd -d /dev/nbd2 # use a second one on the same nbd device, shows that reuse works: qemu-img create -f qcow2 /tmp/2.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/2.qcow2 sudo mkfs -t ext4 /dev/nbd2 sudo qemu-nbd -d /dev/nbd2 # connect an image in read only mode sudo qemu-nbd -r -c /dev/nbd2 /tmp/2.qcow2 sudo dumpe2fs /dev/nbd2 | head -n 3 sudo qemu-nbd -d /dev/nbd2 # now try to reuse in read-write mode again: qemu-img create -f qcow2 /tmp/3.qcow2 100M sudo qemu-nbd -c /dev/nbd2 /tmp/3.qcow2 sudo mkfs -t ext4 /dev/nbd2 # here it goes boom: mke2fs 1.42.5 (29-Jul-2012) /dev/nbd2: Operation not permitted while setting up superblock # still need to cleanup sudo qemu-nbd -d /dev/nbd2 === SRU Justification: 1. Impact: mounting an nbd device as read-write after doing so read-only will cause the mount to erroneously (and quietly) be read-only. 2. Development fix: have qemu-nbd set the device to read-write when asked, rather than only setting read-only. 3. Stable fix: same as development fix 4. Test case: See above 5. Regression potential: The patch is localized to the handling of read-only flag in qemu-nbd, so any regression should not affect anything else. === To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1077838/+subscriptions
Re: [Qemu-devel] Question on Xen management of qemu ram blocks and memory regions
On Nov 15, 2012, at 9:33 AM, Stefano Stabellini wrote: > On Wed, 14 Nov 2012, Andres Lagar-Cavilla wrote: >> Stefano, and Xen-qemu team, I have a question. >> >> The standard Xen-qemu workflow has Xen manage the physmap for a VM, and >> allocate all the backing memory for valid pfns, regardless of whether they >> are MMIO, RAM, etc. On save/migrate, when using upstream qemu, a special >> monitor command is used to save the device model state, while the >> save/restore code blocks in libxc takes care of the memory. >> >> Qemu has a chain of ram blocks with offsets, each of which is further >> subdivided into memory regions that map to specific chunks of the physmap. >> >> AFAICT, the restore code in libxc has no knowledge of qemu's ram blocks and >> offsets. My question is, how is a mismatch avoided? >> >> How does the workflow ensure that all the sub regions in each ram block map >> to the same physmap chunks on restore? Is this an implicit guarantee from >> qemu when building the VM (with the same command line) on the restore side? >> >> Are the regions and physmap offsets contained in the device state that is >> saved? >> >> If, for example, I were to save/restore a VM with four e1000 emulated >> devices, how does the workflow guarantee that each physmap region backing >> each e1000 ROM gets reconstructed with exactly the same ram block, offset, >> and physmap chunk coordinates? >> >> Code inspection seems to suggest qemu will lay out things deterministically >> given the command line. I want to make sure I am not missing anything. > > Yes, it does. Moreover QEMU is going to save everything it needs to > restore the state of the devices exactly the way it was, MMIO regions > addresses and sizes included. > > The only issue is the videoram: even though it is an MMIO region, it is > saved by Xen because it looks like normal ram to the hypervisor. > To solve the problem QEMU writes the location and the size of the > videoram to xenstore and keeps the records up to date. > The toolstack reads those records and adds them to the save file. > Thanks Stefano. That matches my read. IIUC, the xen MemoryListener callbacks will write to xenstore any ram block that is relocated, for restore's benefit. It just happens to only be the cirrus vram in the standard workflow. Cheers! Andres > At restore time the toolstack writes back the records to xenstore and > QEMU at boot time uses them to populate a list of physmap regions, see > xen_read_physmap.
Re: [Qemu-devel] [PATCH] block: vpc support for ~2 TB disks
>>> On 11/14/2012 at 09:35 AM, in message <50a3c853.4010...@redhat.com>, Paolo Bonzini wrote: > Il 14/11/2012 17:25, Thanos Makatos ha scritto: >> We don't use qemu's VHD driver in XenServer. Instead, we use blktap2 >> to create a block device in dom0 serving the VHD file in question, >> and have qemu open that block device instead of the VHD file itself. > > Yes, the question is how you handle disks bigger than 127GB, so that > QEMU can do the same. > In analyzing a 160 GB VHD fixed disk image created on Windows 2008 R2, it appears that MS is also ignoring the CHS values in the footer geometry field in whatever driver they use for accessing the image. The CHS values are set at 65535,16,255 which obviously doesn't represent an image size of 160 GB. This patch only extends the existing qemu driver to allow a larger image by allowing more heads. On real hardware, only 4 bits would be allowed for heads but we don't have that restriction in qemu. - Charles
Re: [Qemu-devel] [PATCH] iscsi: fix deadlock during login
Il 15/11/2012 17:13, ronnie sahlberg ha scritto: > On Thu, Nov 15, 2012 at 7:54 AM, Paolo Bonzini wrote: >> Il 15/11/2012 15:57, ronnie sahlberg ha scritto: >>> I dont know if we should switch to use synchronous code here. >>> It is much nicer if all code is async. >> >> bdrv_open is generally synchronous, so I think Peter's patch is ok. > > I was thinking about the case where you disconnect/reconnect a device > at runtime. Like swapping the medium in a CDROM. > If bdrv_open() is synchronous and blocks for a long time, would that > not impact the rest of QEMU? Yes, it's not optimal, but VCPUs would still run until they request I/O. But usually iscsi devices should be non-removable, no? That leaves hotplug as the only problematic case. Paolo > > Otherwise: > Acked-by: ronniesahlb...@gmail.com > >> >> Paolo >> >>> Is it possible to add a timeout instead that would break out if the >>> connect/login has not completed within a certain amount of time? >>
Re: [Qemu-devel] [PATCH] iscsi: fix deadlock during login
On Thu, Nov 15, 2012 at 7:54 AM, Paolo Bonzini wrote: > Il 15/11/2012 15:57, ronnie sahlberg ha scritto: >> I dont know if we should switch to use synchronous code here. >> It is much nicer if all code is async. > > bdrv_open is generally synchronous, so I think Peter's patch is ok. I was thinking about the case where you disconnect/reconnect a device at runtime. Like swapping the medium in a CDROM. If bdrv_open() is synchronous and blocks for a long time, would that not impact the rest of QEMU? Otherwise: Acked-by: ronniesahlb...@gmail.com > > Paolo > >> Is it possible to add a timeout instead that would break out if the >> connect/login has not completed within a certain amount of time? >
[Qemu-devel] Fwd: buildbot failure in qemu on rhel6-default
Original Message Subject: buildbot failure in qemu on rhel6-default Date: Thu, 15 Nov 2012 16:25:41 +0100 From: build...@spunk.home.kraxel.org To: kraxel...@gmail.com The Buildbot has detected a failed build on builder rhel6-default while building qemu. Full details are available at: http://www.kraxel.org/bb/builders/rhel6-default/builds/1030 Buildbot URL: http://www.kraxel.org/bb/ Buildslave for this Build: rhel6.xeni Build Reason: scheduler Build Source Stamp: [branch master] 6801038bc52d61f81ac8a25fbe392f1bad982887 Blamelist: Aurelien Jarno ,陳韋任 (Wei-Ren Chen) BUILD FAILED: failed test sincerely, -The Buildbot == log tail == PASS: tests/fdc-test TEST: tests/hd-geo-test... (pid=2355) /x86_64/hd-geo/ide/none: OK /x86_64/hd-geo/ide/drive/mbr/blank: OK /x86_64/hd-geo/ide/drive/mbr/lba:OK /x86_64/hd-geo/ide/drive/mbr/chs:OK /x86_64/hd-geo/ide/drive/user/chs: OK /x86_64/hd-geo/ide/drive/user/chst: OK /x86_64/hd-geo/ide/drive/cd_0: OK /x86_64/hd-geo/ide/device/mbr/blank: OK /x86_64/hd-geo/ide/device/mbr/lba: OK /x86_64/hd-geo/ide/device/mbr/chs: OK /x86_64/hd-geo/ide/device/user/chs: OK /x86_64/hd-geo/ide/device/user/chst: OK PASS: tests/hd-geo-test TEST: tests/rtc-test... (pid=2405) /x86_64/rtc/bcd/check-time: OK /x86_64/rtc/dec/check-time: OK /x86_64/rtc/alarm-time: OK /x86_64/rtc/set-year/20xx: OK /x86_64/rtc/set-year/1980: OK /x86_64/rtc/fuzz-registers: OK PASS: tests/rtc-test QTEST_QEMU_BINARY=sparc-softmmu/qemu-system-sparc gtester -k --verbose -m=quick tests/m48t59-test TEST: tests/m48t59-test... (pid=2412) /sparc/rtc/bcd/check-time: OK /sparc/rtc/fuzz-registers: OK PASS: tests/m48t59-test QTEST_QEMU_BINARY=sparc64-softmmu/qemu-system-sparc64 gtester -k --verbose -m=quick tests/m48t59-test TEST: tests/m48t59-test... (pid=2417) /sparc64/rtc/bcd/check-time: OK /sparc64/rtc/fuzz-registers: == full log == http://www.kraxel.org/bb/builders/rhel6-default/builds/1030/steps/test/logs/stdio
[Qemu-devel] Fwd: buildbot failure in qemu on openbsd-default
Original Message Subject: buildbot failure in qemu on openbsd-default Date: Thu, 15 Nov 2012 16:06:14 +0100 From: build...@spunk.home.kraxel.org To: kraxel...@gmail.com The Buildbot has detected a failed build on builder openbsd-default while building qemu. Full details are available at: http://www.kraxel.org/bb/builders/openbsd-default/builds/911 Buildbot URL: http://www.kraxel.org/bb/ Buildslave for this Build: openbsd Build Reason: scheduler Build Source Stamp: [branch kraxel-usb] b2d3b122e900de745b4dce620d0c85ec3557cfac Blamelist: Gerd Hoffmann ,Hans de Goede ,Jan Kiszka BUILD FAILED: failed test sincerely, -The Buildbot == log tail == /usr/local/lib/libcurl.so.22.0: warning: strcat() is almost always misused, please use strlcat() /usr/local/lib/libglib-2.0.so.2992.0: warning: sprintf() is often misused, please use snprintf() gcc -I/home/buildbot/slave-spunk/openbsd-default/build/slirp -I. -I/home/buildbot/slave-spunk/openbsd-default/build -I/home/buildbot/slave-spunk/openbsd-default/build/fpu -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -Wno-redundant-decls -fstack-protector-all -Wendif-labels -Wmissing-include-dirs -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wold-style-definition -I/usr/local/include/libpng -I/usr/X11R6/include/pixman-1 -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -I/home/buildbot/slave-spunk/openbsd-default/build/include -MMD -MP -MT tests/hd-geo-test.o -MF tests/hd-geo-test.d -O2 -D_FORTIFY_SOURCE=2 -c -o tests/hd-geo-test.o tests/hd-geo-test.c gcc -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -Wno-redundant-decls -fstack-protector-all -Wendif-labels -Wmissing-include-dirs -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wold-style-definition -I/usr/local/include/libpng -I/usr/X11R6/include/pixman-1 -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -I/home/buildbot/slave-spunk/openbsd-default/build/include -O2 -D_FORTIFY_SOURCE=2 -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -o tests/hd-geo-test cutils.o osdep.o oslib-posix.o qemu-thread-posix.o qemu-timer-common.o tests/hd-geo-test.o tests/libqtest.o trace.o trace/control.o trace/default.o -L/usr/local/lib -pthread -lgthread-2.0 -lglib-2.0 -lintl -liconv -lz -L/usr/local/lib -lcurl /usr/local/lib/libglib-2.0.so.2992.0: warning: vsprintf() is often misused, please use vsnprintf() /usr/local/lib/libglib-2.0.so.2992.0: warning: stpcpy() is dangerous GNU crap; don't use it /usr/local/lib/libglib-2.0.so.2992.0: warning: strcpy() is almost always misused, please use strlcpy() /usr/local/lib/libcurl.so.22.0: warning: strcat() is almost always misused, please use strlcat() /usr/local/lib/libglib-2.0.so.2992.0: warning: sprintf() is often misused, please use snprintf() gcc -I/home/buildbot/slave-spunk/openbsd-default/build/slirp -I. -I/home/buildbot/slave-spunk/openbsd-default/build -I/home/buildbot/slave-spunk/openbsd-default/build/fpu -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -Wno-redundant-decls -fstack-protector-all -Wendif-labels -Wmissing-include-dirs -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wold-style-definition -I/usr/local/include/libpng -I/usr/X11R6/include/pixman-1 -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -I/home/buildbot/slave-spunk/openbsd-default/build/include -MMD -MP -MT tests/rtc-test.o -MF tests/rtc-test.d -O2 -D_FORTIFY_SOURCE=2 -c -o tests/rtc-test.o tests/rtc-test.c gcc -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -Wno-redundant-decls -fstack-protector-all -Wendif-labels -Wmissing-include-dirs -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wold-style-definition -I/usr/local/include/libpng -I/usr/X11R6/include/pixman-1 -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -I/home/buildbot/slave-spunk/openbsd-default/build/include -O2 -D_FORTIFY_SOURCE=2 -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -o tests/rtc-test cutils.o osdep.o oslib-posix.o qemu-thread-posix.o qemu-timer-common.o tests/libqtest.o tests/rtc-test.o trace.o trace/control.o trace/default.o -L/usr/local/lib -pthread -lgthread-2.0 -lglib-2.0 -lintl -liconv -lz -L/usr/local/lib -lcurl /usr/local/lib/libglib-2.0.so.2992.0: warning: vsprintf() is often misused, please use vsnprintf() /usr/local/li
[Qemu-devel] [PATCH 5/7] dataplane: add Linux AIO request queue
The IOQueue has a pool of iocb structs and a function to add new
read/write requests. Multiple requests can be added before calling the
submit function to actually tell the host kernel to begin I/O. This
allows callers to batch requests and submit them in one go.
The actual I/O is performed using Linux AIO.
Signed-off-by: Stefan Hajnoczi
---
hw/dataplane/Makefile.objs | 2 +-
hw/dataplane/ioq.c | 118 +
hw/dataplane/ioq.h | 57 ++
3 files changed, 176 insertions(+), 1 deletion(-)
create mode 100644 hw/dataplane/ioq.c
create mode 100644 hw/dataplane/ioq.h
diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs
index a01ad05..5f3e371 100644
--- a/hw/dataplane/Makefile.objs
+++ b/hw/dataplane/Makefile.objs
@@ -1,3 +1,3 @@
ifeq ($(CONFIG_VIRTIO), y)
-common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o event-poll.o
+common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o event-poll.o ioq.o
endif
diff --git a/hw/dataplane/ioq.c b/hw/dataplane/ioq.c
new file mode 100644
index 000..7adeb5d
--- /dev/null
+++ b/hw/dataplane/ioq.c
@@ -0,0 +1,118 @@
+/*
+ * Linux AIO request queue
+ *
+ * Copyright 2012 IBM, Corp.
+ * Copyright 2012 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Stefan Hajnoczi
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/dataplane/ioq.h"
+
+void ioq_init(IOQueue *ioq, int fd, unsigned int max_reqs)
+{
+int rc;
+
+ioq->fd = fd;
+ioq->max_reqs = max_reqs;
+
+memset(&ioq->io_ctx, 0, sizeof ioq->io_ctx);
+rc = io_setup(max_reqs, &ioq->io_ctx);
+if (rc != 0) {
+fprintf(stderr, "ioq io_setup failed %d\n", rc);
+exit(1);
+}
+
+rc = event_notifier_init(&ioq->io_notifier, 0);
+if (rc != 0) {
+fprintf(stderr, "ioq io event notifier creation failed %d\n", rc);
+exit(1);
+}
+
+ioq->freelist = g_malloc0(sizeof ioq->freelist[0] * max_reqs);
+ioq->freelist_idx = 0;
+
+ioq->queue = g_malloc0(sizeof ioq->queue[0] * max_reqs);
+ioq->queue_idx = 0;
+}
+
+void ioq_cleanup(IOQueue *ioq)
+{
+g_free(ioq->freelist);
+g_free(ioq->queue);
+
+event_notifier_cleanup(&ioq->io_notifier);
+io_destroy(ioq->io_ctx);
+}
+
+EventNotifier *ioq_get_notifier(IOQueue *ioq)
+{
+return &ioq->io_notifier;
+}
+
+struct iocb *ioq_get_iocb(IOQueue *ioq)
+{
+if (unlikely(ioq->freelist_idx == 0)) {
+fprintf(stderr, "ioq underflow\n");
+exit(1);
+}
+struct iocb *iocb = ioq->freelist[--ioq->freelist_idx];
+ioq->queue[ioq->queue_idx++] = iocb;
+return iocb;
+}
+
+void ioq_put_iocb(IOQueue *ioq, struct iocb *iocb)
+{
+if (unlikely(ioq->freelist_idx == ioq->max_reqs)) {
+fprintf(stderr, "ioq overflow\n");
+exit(1);
+}
+ioq->freelist[ioq->freelist_idx++] = iocb;
+}
+
+struct iocb *ioq_rdwr(IOQueue *ioq, bool read, struct iovec *iov,
+ unsigned int count, long long offset)
+{
+struct iocb *iocb = ioq_get_iocb(ioq);
+
+if (read) {
+io_prep_preadv(iocb, ioq->fd, iov, count, offset);
+} else {
+io_prep_pwritev(iocb, ioq->fd, iov, count, offset);
+}
+io_set_eventfd(iocb, event_notifier_get_fd(&ioq->io_notifier));
+return iocb;
+}
+
+int ioq_submit(IOQueue *ioq)
+{
+int rc = io_submit(ioq->io_ctx, ioq->queue_idx, ioq->queue);
+ioq->queue_idx = 0; /* reset */
+return rc;
+}
+
+int ioq_run_completion(IOQueue *ioq, IOQueueCompletion *completion,
+ void *opaque)
+{
+struct io_event events[ioq->max_reqs];
+int nevents, i;
+
+nevents = io_getevents(ioq->io_ctx, 0, ioq->max_reqs, events, NULL);
+if (unlikely(nevents < 0)) {
+fprintf(stderr, "io_getevents failed %d\n", nevents);
+exit(1);
+}
+
+for (i = 0; i < nevents; i++) {
+ssize_t ret = ((uint64_t)events[i].res2 << 32) | events[i].res;
+
+completion(events[i].obj, ret, opaque);
+ioq_put_iocb(ioq, events[i].obj);
+}
+return nevents;
+}
diff --git a/hw/dataplane/ioq.h b/hw/dataplane/ioq.h
new file mode 100644
index 000..890db22
--- /dev/null
+++ b/hw/dataplane/ioq.h
@@ -0,0 +1,57 @@
+/*
+ * Linux AIO request queue
+ *
+ * Copyright 2012 IBM, Corp.
+ * Copyright 2012 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Stefan Hajnoczi
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef IOQ_H
+#define IOQ_H
+
+#include
+#include "event_notifier.h"
+
+typedef struct {
+int fd; /* file descriptor */
+unsigned int max_reqs; /* max length of freelist and queue */
+
+io_context_t io_ctx;/* Linux AIO context */
+EventNotifier io_notifier; /* Linux AIO eventfd */
+
+/* Reque
[Qemu-devel] [PATCH 2/7] configure: add CONFIG_VIRTIO_BLK_DATA_PLANE
The virtio-blk-data-plane feature only works with Linux AIO. Therefore add a ./configure option and necessary checks to implement this dependency. Signed-off-by: Stefan Hajnoczi --- configure | 21 + 1 file changed, 21 insertions(+) diff --git a/configure b/configure index f847ee2..0460589 100755 --- a/configure +++ b/configure @@ -223,6 +223,7 @@ libiscsi="" coroutine="" seccomp="" glusterfs="" +virtio_blk_data_plane="" # parse CC options first for opt do @@ -871,6 +872,10 @@ for opt do ;; --enable-glusterfs) glusterfs="yes" ;; + --disable-virtio-blk-data-plane) virtio_blk_data_plane="no" + ;; + --enable-virtio-blk-data-plane) virtio_blk_data_plane="yes" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -2233,6 +2238,17 @@ EOF fi ## +# adjust virtio-blk-data-plane based on linux-aio + +if test "$virtio_blk_data_plane" = "yes" -a \ + "$linux_aio" != "yes" ; then + echo "Error: virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio" + exit 1 +elif test -z "$virtio_blk_data_plane" ; then + virtio_blk_data_plane=$linux_aio +fi + +## # attr probe if test "$attr" != "no" ; then @@ -3235,6 +3251,7 @@ echo "build guest agent $guest_agent" echo "seccomp support $seccomp" echo "coroutine backend $coroutine_backend" echo "GlusterFS support $glusterfs" +echo "virtio-blk-data-plane $virtio_blk_data_plane" if test "$sdl_too_old" = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" @@ -3581,6 +3598,10 @@ if test "$glusterfs" = "yes" ; then echo "CONFIG_GLUSTERFS=y" >> $config_host_mak fi +if test "$virtio_blk_data_plane" = "yes" ; then + echo "CONFIG_VIRTIO_BLK_DATA_PLANE=y" >> $config_host_mak +fi + # USB host support case "$usb" in linux) -- 1.8.0
[Qemu-devel] [PATCH 4/7] dataplane: add event loop
Outside the safety of the global mutex we need to poll on file
descriptors. I found epoll(2) is a convenient way to do that, although
other options could replace this module in the future (such as an
AioContext-based loop or glib's GMainLoop).
One important feature of this small event loop implementation is that
the loop can be terminated in a thread-safe way. This allows QEMU to
stop the data plane thread cleanly.
Signed-off-by: Stefan Hajnoczi
---
hw/dataplane/Makefile.objs | 2 +-
hw/dataplane/event-poll.c | 109 +
hw/dataplane/event-poll.h | 40 +
3 files changed, 150 insertions(+), 1 deletion(-)
create mode 100644 hw/dataplane/event-poll.c
create mode 100644 hw/dataplane/event-poll.h
diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs
index b58544f..a01ad05 100644
--- a/hw/dataplane/Makefile.objs
+++ b/hw/dataplane/Makefile.objs
@@ -1,3 +1,3 @@
ifeq ($(CONFIG_VIRTIO), y)
-common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o
+common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o event-poll.o
endif
diff --git a/hw/dataplane/event-poll.c b/hw/dataplane/event-poll.c
new file mode 100644
index 000..4a53d48
--- /dev/null
+++ b/hw/dataplane/event-poll.c
@@ -0,0 +1,109 @@
+/*
+ * Event loop with file descriptor polling
+ *
+ * Copyright 2012 IBM, Corp.
+ * Copyright 2012 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Stefan Hajnoczi
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include
+#include "hw/dataplane/event-poll.h"
+
+/* Add an event notifier and its callback for polling */
+void event_poll_add(EventPoll *poll, EventHandler *handler,
+EventNotifier *notifier, EventCallback *callback)
+{
+struct epoll_event event = {
+.events = EPOLLIN,
+.data.ptr = handler,
+};
+handler->notifier = notifier;
+handler->callback = callback;
+if (epoll_ctl(poll->epoll_fd, EPOLL_CTL_ADD,
+ event_notifier_get_fd(notifier), &event) != 0) {
+fprintf(stderr, "failed to add event handler to epoll: %m\n");
+exit(1);
+}
+}
+
+/* Event callback for stopping the event_poll_run() loop */
+static bool handle_stop(EventHandler *handler)
+{
+return false; /* stop event loop */
+}
+
+void event_poll_init(EventPoll *poll)
+{
+/* Create epoll file descriptor */
+poll->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
+if (poll->epoll_fd < 0) {
+fprintf(stderr, "epoll_create1 failed: %m\n");
+exit(1);
+}
+
+/* Set up stop notifier */
+if (event_notifier_init(&poll->stop_notifier, 0) < 0) {
+fprintf(stderr, "failed to init stop notifier\n");
+exit(1);
+}
+event_poll_add(poll, &poll->stop_handler,
+ &poll->stop_notifier, handle_stop);
+}
+
+void event_poll_cleanup(EventPoll *poll)
+{
+event_notifier_cleanup(&poll->stop_notifier);
+close(poll->epoll_fd);
+poll->epoll_fd = -1;
+}
+
+/* Block until the next event and invoke its callback
+ *
+ * Signals must be masked, EINTR should never happen. This is true for QEMU
+ * threads.
+ */
+static bool event_poll(EventPoll *poll)
+{
+EventHandler *handler;
+struct epoll_event event;
+int nevents;
+
+/* Wait for the next event. Only do one event per call to keep the
+ * function simple, this could be changed later. */
+nevents = epoll_wait(poll->epoll_fd, &event, 1, -1);
+if (unlikely(nevents != 1)) {
+fprintf(stderr, "epoll_wait failed: %m\n");
+exit(1); /* should never happen */
+}
+
+/* Find out which event handler has become active */
+handler = event.data.ptr;
+
+/* Clear the eventfd */
+event_notifier_test_and_clear(handler->notifier);
+
+/* Handle the event */
+return handler->callback(handler);
+}
+
+void event_poll_run(EventPoll *poll)
+{
+while (event_poll(poll)) {
+/* do nothing */
+}
+}
+
+/* Stop the event_poll_run() loop
+ *
+ * This function can be used from another thread.
+ */
+void event_poll_stop(EventPoll *poll)
+{
+event_notifier_set(&poll->stop_notifier);
+}
diff --git a/hw/dataplane/event-poll.h b/hw/dataplane/event-poll.h
new file mode 100644
index 000..5e1771f
--- /dev/null
+++ b/hw/dataplane/event-poll.h
@@ -0,0 +1,40 @@
+/*
+ * Event loop with file descriptor polling
+ *
+ * Copyright 2012 IBM, Corp.
+ * Copyright 2012 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Stefan Hajnoczi
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef EVENT_POLL_H
+#define EVENT_POLL_H
+
+#include "event_notifier.h"
+
+typedef struct EventHandler EventHandler;
+typedef bool EventCallback(EventHandler *handler);
+struct EventHandler {
+EventNotifier *notifier;/* eventfd */
+EventC
Re: [Qemu-devel] [PATCH] iscsi: fix segfault in url parsing
Il 15/11/2012 15:42, Peter Lieven ha scritto:
> If an invalid URL is specified iscsi_get_error(iscsi) is called
> with iscsi == NULL.
>
> Signed-off-by: Peter Lieven
Applied to scsi-next branch, but next time please disable format=flowed
if you want to send patches with Thunderbird. It mangles whitespace.
Paolo
> ---
> block/iscsi.c |3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index d0b1a10..b5c3161 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -947,8 +947,7 @@ static int iscsi_open(BlockDriverState *bs, const
> char *filename, int flags)
>
> iscsi_url = iscsi_parse_full_url(iscsi, filename);
> if (iscsi_url == NULL) {
> -error_report("Failed to parse URL : %s %s", filename,
> - iscsi_get_error(iscsi));
> +error_report("Failed to parse URL : %s", filename);
> ret = -EINVAL;
> goto out;
> }
Re: [Qemu-devel] "usb: uhci: Look up queue by address, not token"
Hi Hans, On 2012-11-15 16:19, Hans de Goede wrote: > Hi Jan, > > I just saw your $subject patch in Gerd's usb-next tree, and I've a question > about it. The token should be enough to uniquely identify a device + ep, > and unless a guest uses multiple qhs for a singe ep, that _should_ be enough. But what disallows that the guest issues multiple requests (QH + series of TDs) for a single endpoint? I'm not finding any trace in the spec that disallows this. And my special guest is stumbling over that limitation in QEMU. > > So I'm wondering if you can give a (short) description of exactly what you > were seeing which is fixed by this patch ? And were you seeing this with > 1.2, or with master ? I'm on git master. > > The reason why I want to know, is that identifying the queue by qh has a > disadvantage, to be precise I believe the following can then happen: > > 1) The guest queues up multiple requests for a queue > 2) We execute one, go async > 3) The guest changes it mind an unlinks the qh > 4) The guest will think the queue is cancelled after frindex has > changed by 1, but we keep it around for 64 frames (which sucks, > and I want to improve on this, but we need to for various reasons) > 5) The guests submits some new requests to the queue, using a > new qh (which possibly has a different address). > 6) We see the new qh, and execute a request from there > 7) The 1st request on the old qh completes, and we execute the next > 8) Now things are a mess as we're executing requests from the old > (cancelled from the guest pov) and new queue intermixed... > > Using the token to identify the queue fixes this, cause we will > find the same queue for the old and new qh, and uhci_queue_verify() > will then fail because of the qh addr change, and then we cancel > the old queue. IOW then we do the right thing. I was afraid of triggering some conflict but, as pointed out above, the current code also does something wrong. It associates two different active QH with the same queue and then triggers a failure for the first QH as it wrongly thinks the guest has unlinked it. > > So I'm wondering if there is another, potentially better fix for > what you are seeing? /me too. I'm not fully understanding your scenario above yet, specifically when and how the guest can correctly remove an active QH, so I cannot provide good suggestions at this point. > > Regards, > > Hans > > p.s. > > Did you send this directly to Gerd, without CC-ing qemu-devel, or > did I just miss it on qemu-devel ? If the former, please add qemu-devel > to the CC next time. http://permalink.gmane.org/gmane.comp.emulators.qemu/180483 Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH 1/7] raw-posix: add raw_get_aio_fd() for virtio-blk-data-plane
The raw_get_aio_fd() function allows virtio-blk-data-plane to get the
file descriptor of a raw image file with Linux AIO enabled. This
interface is really a layering violation that can be resolved once the
block layer is able to run outside the global mutex - at that point
virtio-blk-data-plane will switch from custom Linux AIO code to using
the block layer.
Signed-off-by: Stefan Hajnoczi
---
block.h | 9 +
block/raw-posix.c | 34 ++
2 files changed, 43 insertions(+)
diff --git a/block.h b/block.h
index 722c620..2dc6aaf 100644
--- a/block.h
+++ b/block.h
@@ -365,6 +365,15 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
void bdrv_set_in_use(BlockDriverState *bs, int in_use);
int bdrv_in_use(BlockDriverState *bs);
+#ifdef CONFIG_LINUX_AIO
+int raw_get_aio_fd(BlockDriverState *bs);
+#else
+static inline int raw_get_aio_fd(BlockDriverState *bs)
+{
+return -ENOTSUP;
+}
+#endif
+
enum BlockAcctType {
BDRV_ACCT_READ,
BDRV_ACCT_WRITE,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index f2f0404..fc04981 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1768,6 +1768,40 @@ static BlockDriver bdrv_host_cdrom = {
};
#endif /* __FreeBSD__ */
+#ifdef CONFIG_LINUX_AIO
+/**
+ * Return the file descriptor for Linux AIO
+ *
+ * This function is a layering violation and should be removed when it becomes
+ * possible to call the block layer outside the global mutex. It allows the
+ * caller to hijack the file descriptor so I/O can be performed outside the
+ * block layer.
+ */
+int raw_get_aio_fd(BlockDriverState *bs)
+{
+BDRVRawState *s;
+
+if (!bs->drv) {
+return -ENOMEDIUM;
+}
+
+if (bs->drv == bdrv_find_format("raw")) {
+bs = bs->file;
+}
+
+/* raw-posix has several protocols so just check for raw_aio_readv */
+if (bs->drv->bdrv_aio_readv != raw_aio_readv) {
+return -ENOTSUP;
+}
+
+s = bs->opaque;
+if (!s->use_aio) {
+return -ENOTSUP;
+}
+return s->fd;
+}
+#endif /* CONFIG_LINUX_AIO */
+
static void bdrv_file_init(void)
{
/*
--
1.8.0
Re: [Qemu-devel] [PATCH] iscsi: fix deadlock during login
Il 15/11/2012 15:57, ronnie sahlberg ha scritto: > I dont know if we should switch to use synchronous code here. > It is much nicer if all code is async. bdrv_open is generally synchronous, so I think Peter's patch is ok. Paolo > Is it possible to add a timeout instead that would break out if the > connect/login has not completed within a certain amount of time?
[Qemu-devel] [PATCH 7/7] virtio-blk: add x-data-plane=on|off performance feature
The virtio-blk-data-plane feature is easy to integrate into
hw/virtio-blk.c. The data plane can be started and stopped similar to
vhost-net.
Users can take advantage of the virtio-blk-data-plane feature using the
new -device virtio-blk-pci,x-data-plane=on property.
The x-data-plane name was chosen because at this stage the feature is
experimental and likely to see changes in the future.
If the VM configuration does not support virtio-blk-data-plane an error
message is printed. Although we could fall back to regular virtio-blk,
I prefer the explicit approach since it prompts the user to fix their
configuration if they want the performance benefit of
virtio-blk-data-plane.
Limitations:
* Only format=raw is supported
* Live migration is not supported
* Block jobs, hot unplug, and other operations fail with -EBUSY
* I/O throttling limits are ignored
* Only Linux hosts are supported due to Linux AIO usage
Signed-off-by: Stefan Hajnoczi
---
hw/virtio-blk.c | 59 -
hw/virtio-blk.h | 1 +
hw/virtio-pci.c | 3 +++
3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index e25cc96..7f6004e 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -17,6 +17,8 @@
#include "hw/block-common.h"
#include "blockdev.h"
#include "virtio-blk.h"
+#include "hw/dataplane/virtio-blk.h"
+#include "migration.h"
#include "scsi-defs.h"
#ifdef __linux__
# include
@@ -33,6 +35,8 @@ typedef struct VirtIOBlock
VirtIOBlkConf *blk;
unsigned short sector_mask;
DeviceState *qdev;
+VirtIOBlockDataPlane *dataplane;
+Error *migration_blocker;
} VirtIOBlock;
static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
@@ -407,6 +411,14 @@ static void virtio_blk_handle_output(VirtIODevice *vdev,
VirtQueue *vq)
.num_writes = 0,
};
+/* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
+ * dataplane here instead of waiting for .set_status().
+ */
+if (s->dataplane) {
+virtio_blk_data_plane_start(s->dataplane);
+return;
+}
+
while ((req = virtio_blk_get_request(s))) {
virtio_blk_handle_request(req, &mrb);
}
@@ -446,8 +458,13 @@ static void virtio_blk_dma_restart_cb(void *opaque, int
running,
{
VirtIOBlock *s = opaque;
-if (!running)
+if (!running) {
+/* qemu_drain_all() doesn't know about data plane, quiesce here */
+if (s->dataplane) {
+virtio_blk_data_plane_drain(s->dataplane);
+}
return;
+}
if (!s->bh) {
s->bh = qemu_bh_new(virtio_blk_dma_restart_bh, s);
@@ -538,6 +555,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev,
uint8_t status)
VirtIOBlock *s = to_virtio_blk(vdev);
uint32_t features;
+if (s->dataplane && !(status & VIRTIO_CONFIG_S_DRIVER)) {
+virtio_blk_data_plane_stop(s->dataplane);
+}
+
if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
return;
}
@@ -604,6 +625,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
VirtIOBlkConf *blk)
{
VirtIOBlock *s;
static int virtio_blk_id;
+int fd = -1;
if (!blk->conf.bs) {
error_report("drive property not set");
@@ -619,6 +641,21 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
VirtIOBlkConf *blk)
return NULL;
}
+if (blk->data_plane) {
+if (blk->scsi) {
+error_report("device is incompatible with x-data-plane, "
+ "use scsi=off");
+return NULL;
+}
+
+fd = raw_get_aio_fd(blk->conf.bs);
+if (fd < 0) {
+error_report("drive is incompatible with x-data-plane, "
+ "use format=raw,cache=none,aio=native");
+return NULL;
+}
+}
+
s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
sizeof(struct virtio_blk_config),
sizeof(VirtIOBlock));
@@ -636,6 +673,17 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
VirtIOBlkConf *blk)
s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
+if (fd >= 0) {
+s->dataplane = virtio_blk_data_plane_create(&s->vdev, fd);
+
+/* Prevent block operations that conflict with data plane thread */
+bdrv_set_in_use(s->bs, 1);
+
+error_setg(&s->migration_blocker,
+ "x-data-plane does not support migration");
+migrate_add_blocker(s->migration_blocker);
+}
+
qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
s->qdev = dev;
register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
@@ -652,6 +700,15 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
VirtIOBlkConf *blk)
void virtio_blk_exit(VirtIODevice *vdev)
{
VirtIOBlock *s = to_virtio_blk(vdev);
+
+if (s->dataplane) {
+migrate_del_blocker(s->migration_blocker)
[Qemu-devel] [PATCH 1/2] usb-host: update tracing
Now that we have separate status and length fields in USBPacket
update the completion tracepoint to log both.
Signed-off-by: Gerd Hoffmann
---
hw/usb/host-linux.c | 20
trace-events|2 +-
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index b17e1dc..e3d394f 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -385,10 +385,12 @@ static void async_complete(void *opaque)
}
if (aurb->urb.type == USBDEVFS_URB_TYPE_CONTROL) {
-trace_usb_host_req_complete(s->bus_num, s->addr, p, p->status);
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status,
aurb->urb.actual_length);
usb_generic_async_ctrl_complete(&s->dev, p);
} else if (!aurb->more) {
-trace_usb_host_req_complete(s->bus_num, s->addr, p, p->status);
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status,
aurb->urb.actual_length);
usb_packet_complete(&s->dev, p);
}
}
@@ -863,8 +865,9 @@ static void usb_host_handle_data(USBDevice *dev, USBPacket
*p)
p->ep->nr, p->iov.size);
if (!is_valid(s, p->pid, p->ep->nr)) {
-trace_usb_host_req_complete(s->bus_num, s->addr, p, USB_RET_NAK);
p->status = USB_RET_NAK;
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status, p->actual_length);
return;
}
@@ -879,8 +882,9 @@ static void usb_host_handle_data(USBDevice *dev, USBPacket
*p)
ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &arg);
if (ret < 0) {
perror("USBDEVFS_CLEAR_HALT");
-trace_usb_host_req_complete(s->bus_num, s->addr, p, USB_RET_NAK);
p->status = USB_RET_NAK;
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status, p->actual_length);
return;
}
clear_halt(s, p->pid, p->ep->nr);
@@ -936,15 +940,15 @@ static void usb_host_handle_data(USBDevice *dev,
USBPacket *p)
switch(errno) {
case ETIMEDOUT:
-trace_usb_host_req_complete(s->bus_num, s->addr, p,
-USB_RET_NAK);
p->status = USB_RET_NAK;
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status, p->actual_length);
break;
case EPIPE:
default:
-trace_usb_host_req_complete(s->bus_num, s->addr, p,
-USB_RET_STALL);
p->status = USB_RET_STALL;
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status, p->actual_length);
}
return;
}
diff --git a/trace-events b/trace-events
index 913e00b..02cdabc 100644
--- a/trace-events
+++ b/trace-events
@@ -431,7 +431,7 @@ usb_host_claim_interfaces(int bus, int addr, int config,
int nif) "dev %d:%d, co
usb_host_release_interfaces(int bus, int addr) "dev %d:%d"
usb_host_req_control(int bus, int addr, void *p, int req, int value, int
index) "dev %d:%d, packet %p, req 0x%x, value %d, index %d"
usb_host_req_data(int bus, int addr, void *p, int in, int ep, int size) "dev
%d:%d, packet %p, in %d, ep %d, size %d"
-usb_host_req_complete(int bus, int addr, void *p, int status) "dev %d:%d,
packet %p, status %d"
+usb_host_req_complete(int bus, int addr, void *p, int status, int length) "dev
%d:%d, packet %p, status %d, length %d"
usb_host_req_emulated(int bus, int addr, void *p, int status) "dev %d:%d,
packet %p, status %d"
usb_host_req_canceled(int bus, int addr, void *p) "dev %d:%d, packet %p"
usb_host_urb_submit(int bus, int addr, void *aurb, int length, int more) "dev
%d:%d, aurb %p, length %d, more %d"
--
1.7.1
Re: [Qemu-devel] [PATCH 3/7] dataplane: add virtqueue vring code
Il 15/11/2012 16:19, Stefan Hajnoczi ha scritto:
> +/* Map target physical address to host address
> + */
> +static inline void *phys_to_host(Vring *vring, hwaddr phys)
> +{
> +/* Adjust for 3.6-4 GB PCI memory range */
> +if (phys >= 0x1) {
> +phys -= 0x1 - 0xe000;
> +} else if (phys >= 0xe000) {
> +fprintf(stderr, "phys_to_host bad physical address in "
> +"PCI range %#lx\n", phys);
> +exit(1);
> +}
> +return vring->phys_mem_zero_host_ptr + phys;
> +}
> +
Hmm, perhaps *this* is not quite ready. :)
What we want is lockless address_space_map. We're not far from it, but
not there either.
Can you add, at least for now, a weak function that does a 1:1 mapping,
and override it with the above code in hw/pc.c? The prototype then would be
static inline void *dataplane_phys_to_host(void *base, hwaddr phys)
{
}
or something like that.
Paolo
[Qemu-devel] [PATCH 6/7] dataplane: add virtio-blk data plane code
virtio-blk-data-plane is a subset implementation of virtio-blk. It only
handles read, write, and flush requests. It does this using a dedicated
thread that executes an epoll(2)-based event loop and processes I/O
using Linux AIO.
This approach performs very well but can be used for raw image files
only. The number of IOPS achieved has been reported to be several times
higher than the existing virtio-blk implementation.
Eventually it should be possible to unify virtio-blk-data-plane with the
main body of QEMU code once the block layer and hardware emulation is
able to run outside the global mutex.
Signed-off-by: Stefan Hajnoczi
---
hw/dataplane/Makefile.objs | 2 +-
hw/dataplane/virtio-blk.c | 414 +
hw/dataplane/virtio-blk.h | 41 +
trace-events | 6 +
4 files changed, 462 insertions(+), 1 deletion(-)
create mode 100644 hw/dataplane/virtio-blk.c
create mode 100644 hw/dataplane/virtio-blk.h
diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs
index 5f3e371..d56796f 100644
--- a/hw/dataplane/Makefile.objs
+++ b/hw/dataplane/Makefile.objs
@@ -1,3 +1,3 @@
ifeq ($(CONFIG_VIRTIO), y)
-common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o event-poll.o ioq.o
+common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o event-poll.o ioq.o
virtio-blk.o
endif
diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
new file mode 100644
index 000..e18fcb8
--- /dev/null
+++ b/hw/dataplane/virtio-blk.c
@@ -0,0 +1,414 @@
+/*
+ * Dedicated thread for virtio-blk I/O processing
+ *
+ * Copyright 2012 IBM, Corp.
+ * Copyright 2012 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Stefan Hajnoczi
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "trace.h"
+#include "event-poll.h"
+#include "qemu-thread.h"
+#include "vring.h"
+#include "ioq.h"
+#include "hw/virtio-blk.h"
+#include "hw/dataplane/virtio-blk.h"
+
+enum {
+SEG_MAX = 126, /* maximum number of I/O segments */
+VRING_MAX = SEG_MAX + 2,/* maximum number of vring descriptors */
+REQ_MAX = VRING_MAX,/* maximum number of requests in the vring,
+ * is VRING_MAX / 2 with traditional and
+ * VRING_MAX with indirect descriptors */
+};
+
+typedef struct {
+struct iocb iocb; /* Linux AIO control block */
+unsigned char *status; /* virtio block status code */
+unsigned int head; /* vring descriptor index */
+} VirtIOBlockRequest;
+
+struct VirtIOBlockDataPlane {
+bool started;
+QEMUBH *start_bh;
+QemuThread thread;
+
+int fd; /* image file descriptor */
+
+VirtIODevice *vdev;
+Vring vring;/* virtqueue vring */
+EventNotifier *guest_notifier; /* irq */
+
+EventPoll event_poll; /* event poller */
+EventHandler io_handler;/* Linux AIO completion handler */
+EventHandler notify_handler;/* virtqueue notify handler */
+
+IOQueue ioqueue;/* Linux AIO queue (should really be per
+ dataplane thread) */
+VirtIOBlockRequest requests[REQ_MAX]; /* pool of requests, managed by the
+ queue */
+
+unsigned int num_reqs;
+QemuMutex num_reqs_lock;
+QemuCond no_reqs_cond;
+};
+
+/* Raise an interrupt to signal guest, if necessary */
+static void notify_guest(VirtIOBlockDataPlane *s)
+{
+if (!vring_should_notify(s->vdev, &s->vring)) {
+return;
+}
+
+event_notifier_set(s->guest_notifier);
+}
+
+static void complete_request(struct iocb *iocb, ssize_t ret, void *opaque)
+{
+VirtIOBlockDataPlane *s = opaque;
+VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
+int len;
+
+if (likely(ret >= 0)) {
+*req->status = VIRTIO_BLK_S_OK;
+len = ret;
+} else {
+*req->status = VIRTIO_BLK_S_IOERR;
+len = 0;
+}
+
+trace_virtio_blk_data_plane_complete_request(s, req->head, ret);
+
+/* According to the virtio specification len should be the number of bytes
+ * written to, but for virtio-blk it seems to be the number of bytes
+ * transferred plus the status bytes.
+ */
+vring_push(&s->vring, req->head, len + sizeof(*req->status));
+
+qemu_mutex_lock(&s->num_reqs_lock);
+if (--s->num_reqs == 0) {
+qemu_cond_broadcast(&s->no_reqs_cond);
+}
+qemu_mutex_unlock(&s->num_reqs_lock);
+}
+
+static void process_request(IOQueue *ioq, struct iovec iov[],
+unsigned int out_num, unsigned int in_num,
+unsigned int head)
+{
+/* Virtio block requests look like this: */
+struct virtio_blk_outhdr *outhdr; /* iov[0] */
+/* dat
[Qemu-devel] [PATCH] vmstate: drop orphan register_device_unmigratable() prototype
commit 38e0735eb76a1479917ef3501a208d4f70998494 ("ivshmem: use migration
blockers to prevent live migration in peer mode (v2)") removed the
definition of register_device_unmigratable() but forgot to remove the
declaration in the header file.
Signed-off-by: Stefan Hajnoczi
---
vmstate.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/vmstate.h b/vmstate.h
index 623af0a..af10596 100644
--- a/vmstate.h
+++ b/vmstate.h
@@ -56,9 +56,6 @@ int register_savevm_live(DeviceState *dev,
void *opaque);
void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque);
-void register_device_unmigratable(DeviceState *dev, const char *idstr,
-void *opaque);
-
typedef struct VMStateInfo VMStateInfo;
typedef struct VMStateDescription VMStateDescription;
--
1.8.0
Re: [Qemu-devel] [PATCH 1/2] usb-host: update tracing
ACK series.
Regards,
Hans
On 11/15/2012 04:19 PM, Gerd Hoffmann wrote:
Now that we have separate status and length fields in USBPacket
update the completion tracepoint to log both.
Signed-off-by: Gerd Hoffmann
---
hw/usb/host-linux.c | 20
trace-events|2 +-
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index b17e1dc..e3d394f 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -385,10 +385,12 @@ static void async_complete(void *opaque)
}
if (aurb->urb.type == USBDEVFS_URB_TYPE_CONTROL) {
-trace_usb_host_req_complete(s->bus_num, s->addr, p, p->status);
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status,
aurb->urb.actual_length);
usb_generic_async_ctrl_complete(&s->dev, p);
} else if (!aurb->more) {
-trace_usb_host_req_complete(s->bus_num, s->addr, p, p->status);
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status,
aurb->urb.actual_length);
usb_packet_complete(&s->dev, p);
}
}
@@ -863,8 +865,9 @@ static void usb_host_handle_data(USBDevice *dev, USBPacket
*p)
p->ep->nr, p->iov.size);
if (!is_valid(s, p->pid, p->ep->nr)) {
-trace_usb_host_req_complete(s->bus_num, s->addr, p, USB_RET_NAK);
p->status = USB_RET_NAK;
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status, p->actual_length);
return;
}
@@ -879,8 +882,9 @@ static void usb_host_handle_data(USBDevice *dev, USBPacket
*p)
ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &arg);
if (ret < 0) {
perror("USBDEVFS_CLEAR_HALT");
-trace_usb_host_req_complete(s->bus_num, s->addr, p, USB_RET_NAK);
p->status = USB_RET_NAK;
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status, p->actual_length);
return;
}
clear_halt(s, p->pid, p->ep->nr);
@@ -936,15 +940,15 @@ static void usb_host_handle_data(USBDevice *dev,
USBPacket *p)
switch(errno) {
case ETIMEDOUT:
-trace_usb_host_req_complete(s->bus_num, s->addr, p,
-USB_RET_NAK);
p->status = USB_RET_NAK;
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status, p->actual_length);
break;
case EPIPE:
default:
-trace_usb_host_req_complete(s->bus_num, s->addr, p,
-USB_RET_STALL);
p->status = USB_RET_STALL;
+trace_usb_host_req_complete(s->bus_num, s->addr, p,
+p->status, p->actual_length);
}
return;
}
diff --git a/trace-events b/trace-events
index 913e00b..02cdabc 100644
--- a/trace-events
+++ b/trace-events
@@ -431,7 +431,7 @@ usb_host_claim_interfaces(int bus, int addr, int config, int
nif) "dev %d:%d, co
usb_host_release_interfaces(int bus, int addr) "dev %d:%d"
usb_host_req_control(int bus, int addr, void *p, int req, int value, int index)
"dev %d:%d, packet %p, req 0x%x, value %d, index %d"
usb_host_req_data(int bus, int addr, void *p, int in, int ep, int size) "dev
%d:%d, packet %p, in %d, ep %d, size %d"
-usb_host_req_complete(int bus, int addr, void *p, int status) "dev %d:%d, packet
%p, status %d"
+usb_host_req_complete(int bus, int addr, void *p, int status, int length) "dev
%d:%d, packet %p, status %d, length %d"
usb_host_req_emulated(int bus, int addr, void *p, int status) "dev %d:%d, packet
%p, status %d"
usb_host_req_canceled(int bus, int addr, void *p) "dev %d:%d, packet %p"
usb_host_urb_submit(int bus, int addr, void *aurb, int length, int more) "dev
%d:%d, aurb %p, length %d, more %d"
[Qemu-devel] [PATCH 2/2] usb-host: fix splitted transfers
USBPacket->actual_length wasn't updated correctly for USBPackets
splitted into multiple urbs. Fix it.
Signed-off-by: Gerd Hoffmann
---
hw/usb/host-linux.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index e3d394f..aa77b77 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -366,8 +366,11 @@ static void async_complete(void *opaque)
if (p) {
switch (aurb->urb.status) {
case 0:
-p->actual_length = aurb->urb.actual_length;
-p->status = USB_RET_SUCCESS; /* Clear previous ASYNC status */
+p->actual_length += aurb->urb.actual_length;
+if (!aurb->more) {
+/* Clear previous ASYNC status */
+p->status = USB_RET_SUCCESS;
+}
break;
case -EPIPE:
--
1.7.1
[Qemu-devel] [PATCH 3/7] dataplane: add virtqueue vring code
The virtio-blk-data-plane cannot access memory using the usual QEMU
functions since it executes outside the global mutex and the memory APIs
are this time are not thread-safe.
This patch introduces a virtqueue module based on the kernel's vhost
vring code. The trick is that we map guest memory ahead of time and
access it cheaply outside the global mutex.
Once the hardware emulation code can execute outside the global mutex it
will be possible to drop this code.
Signed-off-by: Stefan Hajnoczi
---
hw/Makefile.objs | 2 +-
hw/dataplane/Makefile.objs | 3 +
hw/dataplane/vring.c | 321 +
hw/dataplane/vring.h | 54
trace-events | 3 +
5 files changed, 382 insertions(+), 1 deletion(-)
create mode 100644 hw/dataplane/Makefile.objs
create mode 100644 hw/dataplane/vring.c
create mode 100644 hw/dataplane/vring.h
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index af4ab0c..da8ef0c 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y = usb/ ide/
+common-obj-y = usb/ ide/ dataplane/
common-obj-y += loader.o
common-obj-$(CONFIG_VIRTIO) += virtio-console.o
common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs
new file mode 100644
index 000..b58544f
--- /dev/null
+++ b/hw/dataplane/Makefile.objs
@@ -0,0 +1,3 @@
+ifeq ($(CONFIG_VIRTIO), y)
+common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o
+endif
diff --git a/hw/dataplane/vring.c b/hw/dataplane/vring.c
new file mode 100644
index 000..6aacce8
--- /dev/null
+++ b/hw/dataplane/vring.c
@@ -0,0 +1,321 @@
+/* Copyright 2012 Red Hat, Inc.
+ * Copyright IBM, Corp. 2012
+ *
+ * Based on Linux vhost code:
+ * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2006 Rusty Russell IBM Corporation
+ *
+ * Author: Michael S. Tsirkin
+ * Stefan Hajnoczi
+ *
+ * Inspiration, some code, and most witty comments come from
+ * Documentation/virtual/lguest/lguest.c, by Rusty Russell
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ */
+
+#include "trace.h"
+#include "hw/dataplane/vring.h"
+
+/* Map target physical address to host address
+ */
+static inline void *phys_to_host(Vring *vring, hwaddr phys)
+{
+/* Adjust for 3.6-4 GB PCI memory range */
+if (phys >= 0x1) {
+phys -= 0x1 - 0xe000;
+} else if (phys >= 0xe000) {
+fprintf(stderr, "phys_to_host bad physical address in "
+"PCI range %#lx\n", phys);
+exit(1);
+}
+return vring->phys_mem_zero_host_ptr + phys;
+}
+
+/* Setup for cheap target physical to host address conversion
+ *
+ * This is a hack for direct access to guest memory, we're not really allowed
+ * to do this.
+ */
+static void setup_phys_to_host(Vring *vring)
+{
+hwaddr len = 4096; /* RAM is really much larger but we cheat */
+vring->phys_mem_zero_host_ptr = cpu_physical_memory_map(0, &len, 0);
+if (!vring->phys_mem_zero_host_ptr) {
+fprintf(stderr, "setup_phys_to_host failed\n");
+exit(1);
+}
+}
+
+/* Map the guest's vring to host memory
+ *
+ * This is not allowed but we know the ring won't move.
+ */
+void vring_setup(Vring *vring, VirtIODevice *vdev, int n)
+{
+setup_phys_to_host(vring);
+
+vring_init(&vring->vr, virtio_queue_get_num(vdev, n),
+ phys_to_host(vring, virtio_queue_get_ring_addr(vdev, n)), 4096);
+
+vring->last_avail_idx = 0;
+vring->last_used_idx = 0;
+vring->signalled_used = 0;
+vring->signalled_used_valid = false;
+
+trace_vring_setup(virtio_queue_get_ring_addr(vdev, n),
+ vring->vr.desc, vring->vr.avail, vring->vr.used);
+}
+
+/* Toggle guest->host notifies */
+void vring_set_notification(VirtIODevice *vdev, Vring *vring, bool enable)
+{
+if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
+if (enable) {
+vring_avail_event(&vring->vr) = vring->vr.avail->idx;
+}
+} else if (enable) {
+vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY;
+} else {
+vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY;
+}
+}
+
+/* This is stolen from linux/drivers/vhost/vhost.c:vhost_notify() */
+bool vring_should_notify(VirtIODevice *vdev, Vring *vring)
+{
+uint16_t old, new;
+bool v;
+/* Flush out used index updates. This is paired
+ * with the barrier that the Guest executes when enabling
+ * interrupts. */
+smp_mb();
+
+if ((vdev->guest_features & VIRTIO_F_NOTIFY_ON_EMPTY) &&
+unlikely(vring->vr.avail->idx == vring->last_avail_idx)) {
+return true;
+}
+
+if (!(vdev->guest_features & VIRTIO_RING_F_EVENT_IDX)) {
+return !(vring->vr.avail->flags & VRING_AVAIL_F_NO_INTERRUPT);
+}
+old = vring->signalled_used;
+v = vring->signalled_used_valid;
+new = vring->signalled_used = vring->last_used_idx;
+vring->signalled_used_
[Qemu-devel] [PATCH 0/7] virtio: virtio-blk data plane
This series adds the -device virtio-blk-pci,x-data-plane=on property that enables a high performance I/O codepath. A dedicated thread is used to process virtio-blk requests outside the global mutex and without going through the QEMU block layer. Khoa Huynh reported an increase from 140,000 IOPS to 600,000 IOPS for a single VM using virtio-blk-data-plane in July: http://comments.gmane.org/gmane.comp.emulators.kvm.devel/94580 The virtio-blk-data-plane approach was originally presented at Linux Plumbers Conference 2010. The following slides contain a brief overview: http://linuxplumbersconf.org/2010/ocw/system/presentations/651/original/Optimizing_the_QEMU_Storage_Stack.pdf The basic approach is: 1. Each virtio-blk device has a thread dedicated to handling ioeventfd signalling when the guest kicks the virtqueue. 2. Requests are processed without going through the QEMU block layer using Linux AIO directly. 3. Completion interrupts are injected via irqfd from the dedicated thread. To try it out: qemu -drive if=none,id=drive0,cache=none,aio=native,format=raw,file=... -device virtio-blk-pci,drive=drive0,scsi=off,x-data-plane=on Limitations: * Only format=raw is supported * Live migration is not supported * Block jobs, hot unplug, and other operations fail with -EBUSY * I/O throttling limits are ignored * Only Linux hosts are supported due to Linux AIO usage The code has reached a stage where I feel it is ready to merge. Users have been playing with it for some time and want the significant performance boost. We are refactoring QEMU to get rid of the global mutex. I believe that virtio-blk-data-plane can eventually become the default mode of operation. Instead of waiting for global mutex removal efforts to finish, I want to use virtio-blk-data-plane as an example device for AioContext and threaded hw dispatch refactoring. This means: 1. When the block layer can bind to an AioContext and execute I/O outside the global mutex, virtio-blk-data-plane can use this (and gain image format support). 2. When hw dispatch no longer needs the global mutex we can use hw/virtio.c again and perhaps run a pool of iothreads instead of dedicated data plane threads. But in the meantime, I have cleaned up the virtio-blk-data-plane code so that it can be merged as an experimental feature. Changes from the RFC v9: * Add x-data-plane=on|off option and coexist with regular virtio-blk code * Create thread from BH so it inherits iothread cpusets * Drain requests on vm_stop() so stopped guest does not access image file * Add migration blocker * Add bdrv_in_use() to prevent block jobs and other operations that can interfere * Drop IOQueue request merging for simplicity * Drop ioctl interrupt injection and always use irqfd for simplicity * Major cleanup to split up source files * Rebase from qemu-kvm.git onto qemu.git * Address Michael Tsirkin's review comments Stefan Hajnoczi (7): raw-posix: add raw_get_aio_fd() for virtio-blk-data-plane configure: add CONFIG_VIRTIO_BLK_DATA_PLANE dataplane: add virtqueue vring code dataplane: add event loop dataplane: add Linux AIO request queue dataplane: add virtio-blk data plane code virtio-blk: add x-data-plane=on|off performance feature block.h| 9 + block/raw-posix.c | 34 configure | 21 +++ hw/Makefile.objs | 2 +- hw/dataplane/Makefile.objs | 3 + hw/dataplane/event-poll.c | 109 hw/dataplane/event-poll.h | 40 + hw/dataplane/ioq.c | 118 + hw/dataplane/ioq.h | 57 +++ hw/dataplane/virtio-blk.c | 414 + hw/dataplane/virtio-blk.h | 41 + hw/dataplane/vring.c | 321 +++ hw/dataplane/vring.h | 54 ++ hw/virtio-blk.c| 59 ++- hw/virtio-blk.h| 1 + hw/virtio-pci.c| 3 + trace-events | 9 + 17 files changed, 1293 insertions(+), 2 deletions(-) create mode 100644 hw/dataplane/Makefile.objs create mode 100644 hw/dataplane/event-poll.c create mode 100644 hw/dataplane/event-poll.h create mode 100644 hw/dataplane/ioq.c create mode 100644 hw/dataplane/ioq.h create mode 100644 hw/dataplane/virtio-blk.c create mode 100644 hw/dataplane/virtio-blk.h create mode 100644 hw/dataplane/vring.c create mode 100644 hw/dataplane/vring.h -- 1.8.0
Re: [Qemu-devel] "usb: uhci: Look up queue by address, not token"
Hi Jan, I just saw your $subject patch in Gerd's usb-next tree, and I've a question about it. The token should be enough to uniquely identify a device + ep, and unless a guest uses multiple qhs for a singe ep, that _should_ be enough. So I'm wondering if you can give a (short) description of exactly what you were seeing which is fixed by this patch ? And were you seeing this with 1.2, or with master ? The reason why I want to know, is that identifying the queue by qh has a disadvantage, to be precise I believe the following can then happen: 1) The guest queues up multiple requests for a queue 2) We execute one, go async 3) The guest changes it mind an unlinks the qh 4) The guest will think the queue is cancelled after frindex has changed by 1, but we keep it around for 64 frames (which sucks, and I want to improve on this, but we need to for various reasons) 5) The guests submits some new requests to the queue, using a new qh (which possibly has a different address). 6) We see the new qh, and execute a request from there 7) The 1st request on the old qh completes, and we execute the next 8) Now things are a mess as we're executing requests from the old (cancelled from the guest pov) and new queue intermixed... Using the token to identify the queue fixes this, cause we will find the same queue for the old and new qh, and uhci_queue_verify() will then fail because of the qh addr change, and then we cancel the old queue. IOW then we do the right thing. So I'm wondering if there is another, potentially better fix for what you are seeing? Regards, Hans p.s. Did you send this directly to Gerd, without CC-ing qemu-devel, or did I just miss it on qemu-devel ? If the former, please add qemu-devel to the CC next time.
Re: [Qemu-devel] [PATCH] iscsi: fix deadlock during login
Am 15.11.2012 um 15:57 schrieb ronnie sahlberg :
> I dont know if we should switch to use synchronous code here.
> It is much nicer if all code is async.
Of course, but its just the initial login after which qemu should exit if it
fails.
>
> Is it possible to add a timeout instead that would break out if the
> connect/login has not completed within a certain amount of time?
I think it is, but this has to be done by qemu or not?
I found switching to synchronous code fixed 2 issues at once, the issue
that it hangs if the connection is interrupted during login and another
issue that i submitted a patch for earlier:
https://github.com/sahlberg/libiscsi/commit/a9257d52a7577b607e237e209b9868c5ce78a927
it might be that this fix introduced the new issue.
Peter
>
>
> regards
> ronnie sahlberg
>
> On Thu, Nov 15, 2012 at 6:50 AM, Peter Lieven wrote:
>> If the connection is interrupted before the first login is successfully
>> completed qemu-kvm is waiting forever in qemu_aio_wait().
>>
>> This is fixed by performing an sync login to the target. If the
>> connection breaks after the first successful login errors are
>> handled internally by libiscsi.
>>
>> Signed-off-by: Peter Lieven
>> ---
>> block/iscsi.c | 56
>> +---
>> 1 file changed, 21 insertions(+), 35 deletions(-)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index b5c3161..f44bb57 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -798,30 +798,6 @@ iscsi_inquiry_cb(struct iscsi_context *iscsi, int
>> status, void *command_data,
>> }
>> }
>>
>> -static void
>> -iscsi_connect_cb(struct iscsi_context *iscsi, int status, void
>> *command_data,
>> - void *opaque)
>> -{
>> -struct IscsiTask *itask = opaque;
>> -struct scsi_task *task;
>> -
>> -if (status != 0) {
>> -itask->status = 1;
>> -itask->complete = 1;
>> -return;
>> -}
>> -
>> -task = iscsi_inquiry_task(iscsi, itask->iscsilun->lun,
>> - 0, 0, 36,
>> - iscsi_inquiry_cb, opaque);
>> -if (task == NULL) {
>> -error_report("iSCSI: failed to send inquiry command.");
>> -itask->status = 1;
>> -itask->complete = 1;
>> -return;
>> -}
>> -}
>> -
>> static int parse_chap(struct iscsi_context *iscsi, const char *target)
>> {
>> QemuOptsList *list;
>> @@ -934,7 +910,8 @@ static int iscsi_open(BlockDriverState *bs, const char
>> *filename, int flags)
>> IscsiLun *iscsilun = bs->opaque;
>> struct iscsi_context *iscsi = NULL;
>> struct iscsi_url *iscsi_url = NULL;
>> -struct IscsiTask task;
>> +struct IscsiTask itask;
>> +struct scsi_task *task;
>> char *initiator_name = NULL;
>> int ret;
>>
>> @@ -997,27 +974,36 @@ static int iscsi_open(BlockDriverState *bs, const char
>> *filename, int flags)
>> /* check if we got HEADER_DIGEST via the options */
>> parse_header_digest(iscsi, iscsi_url->target);
>>
>> -task.iscsilun = iscsilun;
>> -task.status = 0;
>> -task.complete = 0;
>> -task.bs = bs;
>> +if (iscsi_full_connect_sync(iscsi, iscsi_url->portal, iscsi_url->lun)
>> != 0) {
>> +error_report("iSCSI: Failed to connect to LUN : %s",
>> +iscsi_get_error(iscsi));
>> +ret = -EINVAL;
>> +goto out;
>> +}
>> +
>> +itask.iscsilun = iscsilun;
>> +itask.status = 0;
>> +itask.complete = 0;
>> +itask.bs = bs;
>>
>> iscsilun->iscsi = iscsi;
>> iscsilun->lun = iscsi_url->lun;
>>
>> -if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
>> - iscsi_connect_cb, &task)
>> -!= 0) {
>> -error_report("iSCSI: Failed to start async connect.");
>> +task = iscsi_inquiry_task(iscsi, iscsilun->lun,
>> + 0, 0, 36,
>> + iscsi_inquiry_cb, &itask);
>> +if (task == NULL) {
>> +error_report("iSCSI: failed to send inquiry command.");
>> ret = -EINVAL;
>> goto out;
>> }
>>
>> -while (!task.complete) {
>> +while (!itask.complete) {
>> iscsi_set_events(iscsilun);
>> qemu_aio_wait();
>> }
>> -if (task.status != 0) {
>> +
>> +if (itask.status != 0) {
>> error_report("iSCSI: Failed to connect to LUN : %s",
>> iscsi_get_error(iscsi));
>> ret = -EINVAL;
>> --
>> 1.7.9.5
Re: [Qemu-devel] [PATCH] iscsi: fix deadlock during login
I dont know if we should switch to use synchronous code here.
It is much nicer if all code is async.
Is it possible to add a timeout instead that would break out if the
connect/login has not completed within a certain amount of time?
regards
ronnie sahlberg
On Thu, Nov 15, 2012 at 6:50 AM, Peter Lieven wrote:
> If the connection is interrupted before the first login is successfully
> completed qemu-kvm is waiting forever in qemu_aio_wait().
>
> This is fixed by performing an sync login to the target. If the
> connection breaks after the first successful login errors are
> handled internally by libiscsi.
>
> Signed-off-by: Peter Lieven
> ---
> block/iscsi.c | 56
> +---
> 1 file changed, 21 insertions(+), 35 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index b5c3161..f44bb57 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -798,30 +798,6 @@ iscsi_inquiry_cb(struct iscsi_context *iscsi, int
> status, void *command_data,
> }
> }
>
> -static void
> -iscsi_connect_cb(struct iscsi_context *iscsi, int status, void
> *command_data,
> - void *opaque)
> -{
> -struct IscsiTask *itask = opaque;
> -struct scsi_task *task;
> -
> -if (status != 0) {
> -itask->status = 1;
> -itask->complete = 1;
> -return;
> -}
> -
> -task = iscsi_inquiry_task(iscsi, itask->iscsilun->lun,
> - 0, 0, 36,
> - iscsi_inquiry_cb, opaque);
> -if (task == NULL) {
> -error_report("iSCSI: failed to send inquiry command.");
> -itask->status = 1;
> -itask->complete = 1;
> -return;
> -}
> -}
> -
> static int parse_chap(struct iscsi_context *iscsi, const char *target)
> {
> QemuOptsList *list;
> @@ -934,7 +910,8 @@ static int iscsi_open(BlockDriverState *bs, const char
> *filename, int flags)
> IscsiLun *iscsilun = bs->opaque;
> struct iscsi_context *iscsi = NULL;
> struct iscsi_url *iscsi_url = NULL;
> -struct IscsiTask task;
> +struct IscsiTask itask;
> +struct scsi_task *task;
> char *initiator_name = NULL;
> int ret;
>
> @@ -997,27 +974,36 @@ static int iscsi_open(BlockDriverState *bs, const char
> *filename, int flags)
> /* check if we got HEADER_DIGEST via the options */
> parse_header_digest(iscsi, iscsi_url->target);
>
> -task.iscsilun = iscsilun;
> -task.status = 0;
> -task.complete = 0;
> -task.bs = bs;
> +if (iscsi_full_connect_sync(iscsi, iscsi_url->portal, iscsi_url->lun)
> != 0) {
> +error_report("iSCSI: Failed to connect to LUN : %s",
> +iscsi_get_error(iscsi));
> +ret = -EINVAL;
> +goto out;
> +}
> +
> +itask.iscsilun = iscsilun;
> +itask.status = 0;
> +itask.complete = 0;
> +itask.bs = bs;
>
> iscsilun->iscsi = iscsi;
> iscsilun->lun = iscsi_url->lun;
>
> -if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
> - iscsi_connect_cb, &task)
> -!= 0) {
> -error_report("iSCSI: Failed to start async connect.");
> +task = iscsi_inquiry_task(iscsi, iscsilun->lun,
> + 0, 0, 36,
> + iscsi_inquiry_cb, &itask);
> +if (task == NULL) {
> +error_report("iSCSI: failed to send inquiry command.");
> ret = -EINVAL;
> goto out;
> }
>
> -while (!task.complete) {
> +while (!itask.complete) {
> iscsi_set_events(iscsilun);
> qemu_aio_wait();
> }
> -if (task.status != 0) {
> +
> +if (itask.status != 0) {
> error_report("iSCSI: Failed to connect to LUN : %s",
> iscsi_get_error(iscsi));
> ret = -EINVAL;
> --
> 1.7.9.5
Re: [Qemu-devel] [PATCH] iscsi: fix segfault in url parsing
Acked-By: ronniesahlb...@gmail.com
On Thu, Nov 15, 2012 at 6:42 AM, Peter Lieven wrote:
> If an invalid URL is specified iscsi_get_error(iscsi) is called
> with iscsi == NULL.
>
> Signed-off-by: Peter Lieven
> ---
> block/iscsi.c |3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index d0b1a10..b5c3161 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -947,8 +947,7 @@ static int iscsi_open(BlockDriverState *bs, const char
> *filename, int flags)
>
> iscsi_url = iscsi_parse_full_url(iscsi, filename);
> if (iscsi_url == NULL) {
> -error_report("Failed to parse URL : %s %s", filename,
> - iscsi_get_error(iscsi));
> +error_report("Failed to parse URL : %s", filename);
> ret = -EINVAL;
> goto out;
> }
> --
> 1.7.9.5
>
[Qemu-devel] [PATCH] iscsi: fix deadlock during login
If the connection is interrupted before the first login is successfully
completed qemu-kvm is waiting forever in qemu_aio_wait().
This is fixed by performing an sync login to the target. If the
connection breaks after the first successful login errors are
handled internally by libiscsi.
Signed-off-by: Peter Lieven
---
block/iscsi.c | 56
+---
1 file changed, 21 insertions(+), 35 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index b5c3161..f44bb57 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -798,30 +798,6 @@ iscsi_inquiry_cb(struct iscsi_context *iscsi, int
status, void *command_data,
}
}
-static void
-iscsi_connect_cb(struct iscsi_context *iscsi, int status, void
*command_data,
- void *opaque)
-{
-struct IscsiTask *itask = opaque;
-struct scsi_task *task;
-
-if (status != 0) {
-itask->status = 1;
-itask->complete = 1;
-return;
-}
-
-task = iscsi_inquiry_task(iscsi, itask->iscsilun->lun,
- 0, 0, 36,
- iscsi_inquiry_cb, opaque);
-if (task == NULL) {
-error_report("iSCSI: failed to send inquiry command.");
-itask->status = 1;
-itask->complete = 1;
-return;
-}
-}
-
static int parse_chap(struct iscsi_context *iscsi, const char *target)
{
QemuOptsList *list;
@@ -934,7 +910,8 @@ static int iscsi_open(BlockDriverState *bs, const
char *filename, int flags)
IscsiLun *iscsilun = bs->opaque;
struct iscsi_context *iscsi = NULL;
struct iscsi_url *iscsi_url = NULL;
-struct IscsiTask task;
+struct IscsiTask itask;
+struct scsi_task *task;
char *initiator_name = NULL;
int ret;
@@ -997,27 +974,36 @@ static int iscsi_open(BlockDriverState *bs, const
char *filename, int flags)
/* check if we got HEADER_DIGEST via the options */
parse_header_digest(iscsi, iscsi_url->target);
-task.iscsilun = iscsilun;
-task.status = 0;
-task.complete = 0;
-task.bs = bs;
+if (iscsi_full_connect_sync(iscsi, iscsi_url->portal,
iscsi_url->lun) != 0) {
+error_report("iSCSI: Failed to connect to LUN : %s",
+iscsi_get_error(iscsi));
+ret = -EINVAL;
+goto out;
+}
+
+itask.iscsilun = iscsilun;
+itask.status = 0;
+itask.complete = 0;
+itask.bs = bs;
iscsilun->iscsi = iscsi;
iscsilun->lun = iscsi_url->lun;
-if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
- iscsi_connect_cb, &task)
-!= 0) {
-error_report("iSCSI: Failed to start async connect.");
+task = iscsi_inquiry_task(iscsi, iscsilun->lun,
+ 0, 0, 36,
+ iscsi_inquiry_cb, &itask);
+if (task == NULL) {
+error_report("iSCSI: failed to send inquiry command.");
ret = -EINVAL;
goto out;
}
-while (!task.complete) {
+while (!itask.complete) {
iscsi_set_events(iscsilun);
qemu_aio_wait();
}
-if (task.status != 0) {
+
+if (itask.status != 0) {
error_report("iSCSI: Failed to connect to LUN : %s",
iscsi_get_error(iscsi));
ret = -EINVAL;
--
1.7.9.5
[Qemu-devel] [PATCH] iscsi: fix segfault in url parsing
If an invalid URL is specified iscsi_get_error(iscsi) is called
with iscsi == NULL.
Signed-off-by: Peter Lieven
---
block/iscsi.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index d0b1a10..b5c3161 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -947,8 +947,7 @@ static int iscsi_open(BlockDriverState *bs, const
char *filename, int flags)
iscsi_url = iscsi_parse_full_url(iscsi, filename);
if (iscsi_url == NULL) {
-error_report("Failed to parse URL : %s %s", filename,
- iscsi_get_error(iscsi));
+error_report("Failed to parse URL : %s", filename);
ret = -EINVAL;
goto out;
}
--
1.7.9.5
Re: [Qemu-devel] Question on Xen management of qemu ram blocks and memory regions
On Wed, 14 Nov 2012, Andres Lagar-Cavilla wrote: > Stefano, and Xen-qemu team, I have a question. > > The standard Xen-qemu workflow has Xen manage the physmap for a VM, and > allocate all the backing memory for valid pfns, regardless of whether they > are MMIO, RAM, etc. On save/migrate, when using upstream qemu, a special > monitor command is used to save the device model state, while the > save/restore code blocks in libxc takes care of the memory. > > Qemu has a chain of ram blocks with offsets, each of which is further > subdivided into memory regions that map to specific chunks of the physmap. > > AFAICT, the restore code in libxc has no knowledge of qemu's ram blocks and > offsets. My question is, how is a mismatch avoided? > > How does the workflow ensure that all the sub regions in each ram block map > to the same physmap chunks on restore? Is this an implicit guarantee from > qemu when building the VM (with the same command line) on the restore side? > > Are the regions and physmap offsets contained in the device state that is > saved? > > If, for example, I were to save/restore a VM with four e1000 emulated > devices, how does the workflow guarantee that each physmap region backing > each e1000 ROM gets reconstructed with exactly the same ram block, offset, > and physmap chunk coordinates? > > Code inspection seems to suggest qemu will lay out things deterministically > given the command line. I want to make sure I am not missing anything. Yes, it does. Moreover QEMU is going to save everything it needs to restore the state of the devices exactly the way it was, MMIO regions addresses and sizes included. The only issue is the videoram: even though it is an MMIO region, it is saved by Xen because it looks like normal ram to the hypervisor. To solve the problem QEMU writes the location and the size of the videoram to xenstore and keeps the records up to date. The toolstack reads those records and adds them to the savefile. At restore time the toolstack writes back the records to xenstore and QEMU at boot time uses them to populate a list of physmap regions, see xen_read_physmap.
Re: [Qemu-devel] [PATCH] target-mips: Clean up microMIPS32 major opcode
On Thu, Nov 15, 2012 at 02:34:31AM +, Johnson, Eric wrote: > Hi Chen, > > Please only remove the POOL48A opcode. > > The others are documented in the microMIPS64 Instruction Set manual ( > http://www.mips.com/secure-download/index.dot?product_name=/auth/MD00087-2B-MIPS64BIS-AFP-03.51.pdf > ). See http://www.mips.com/products/architectures/mips64/ for other > relavent docs. > > Instead of removing them please surround the POOL32S, DADDIU32, SD32, and > LD32 opcodes with > #if defined(TARGET_MIPS64) > I don't think a #if is necessary there, this makes the code more difficult to read, while it doesn't change anything on the generated code. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] mips/malta: fix CBUS UART interrupt pin
On Wed, Nov 14, 2012 at 07:45:02PM +, Johnson, Eric wrote: > > -Original Message- > > From: qemu-devel-bounces+ericj=mips@nongnu.org [mailto:qemu-devel- > > bounces+ericj=mips@nongnu.org] On Behalf Of Aurelien Jarno > > Sent: Wednesday, November 14, 2012 6:38 AM > > To: qemu-devel@nongnu.org > > Cc: Aurelien Jarno > > Subject: [Qemu-devel] [PATCH] mips/malta: fix CBUS UART interrupt pin > > > > According to the MIPS Malta Developement Platform User's Manual, the > > i8259 interrupt controller is supposed to be connected to the hardware > > IRQ0, and the CBUS UART to the hardware interrupt 2. > > > > In QEMU they are both connected to hardware interrupt 0, the CBUS UART > > interrupt being wrong. This patch fixes that. It should be noted that > > the irq array in QEMU includes the software interrupts, hence > > env->irq[2] is the first hardware interrupt. > > > > Cc: Ralf Baechle > > Signed-off-by: Aurelien Jarno > > --- > > hw/mips_malta.c |3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/hw/mips_malta.c b/hw/mips_malta.c > > index 0571d58..4d2464a 100644 > > --- a/hw/mips_malta.c > > +++ b/hw/mips_malta.c > > @@ -861,7 +861,8 @@ void mips_malta_init(QEMUMachineInitArgs *args) > > be = 0; > > #endif > > /* FPGA */ > > -malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[2], > > serial_hds[2]); > > +/* The CBUS UART is attached to the MIPS CPU INT2 pin, ie interrupt 4 > > */ > > +malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[4], > > serial_hds[2]); > > > > /* Load firmware in flash / BIOS. */ > > dinfo = drive_get(IF_PFLASH, 0, fl_idx); > > -- > > 1.7.10.4 > > > > I double checked with a Malta expert here. He verified that the CBUS UART is > connected to the HW2 interrupt pin. > > Reviewed-by: Eric Johnson > Thanks for the review, I have applied the patch. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] KVM Forum 2012 Block BoF minutes
Attendees: Kevin Wolf Stefan Hajnoczi Jeff Cody Markus Armbruster and a few people dropping in and out. Minutes are basically a TODO list. Could be a bit terse in places; if anything's unclear, please ask. Block layer data structure cleanup: - Split block backend off BlockDriverState - Turn block backend into a proper ADT (besides other good things, this makes it possible to replace its implementation by a test harness for unit testing of its customers) - Get rid of BlockDriverState opaque (driver private state), inherit the common part into driver state Convert open to QemuOpts (or perhaps QDict?) Avoid double open for probing (update: patches posted) Block jobs: - Rate limiting is broken in general (it works for the single BlockDriverState case) - Block jobs should be jobs: generally available instead of tied to a (single) BlockDriverState; get rid of the block job pointer in BlockDriverState - Migration should probably be a job Block migration needs to die after its replacement is ready BlockDriverState member in_use and DriveInfo member refcount are a mess: - in_use is used to avoid running certain things concurrently, but the rules are unclear, except they're clearly incomplete; possible rules could be about need for consistent views of image contents - refcount is only used together with in_use, and appears to be for avoiding premature deletion AHCI: - Same IDE device models connect to different kinds of controllers just fine - Use links to connect controller device model and drive device models; the link serves as generic address replacing bus, unit, index - Make -drive if!=none true sugar for suitable -device (details depend on machine) - Checking legacy bus, unit, index needs to be delayed until machine's limits are known - Add "enough" -hdX to cover common usage (got 4 now, want 6 for q35) - Some AHCI controllers can optionally mimick a legacy IDE controller, and expose some of their ports there (controlled by BIOS for real hardware, by controller qdev property for us) - Convenience machine option to control that qdev property of an onboard controller would be nice (should not apply to additional controllers plugged in with -device) - if=ahci is not necessary
[Qemu-devel] [PATCH 2/3] usb-redir: Only add actually in flight packets to the in flight queue
Packets which are queued up, but not yet handed over to the device, are
*not* in flight.
Signed-off-by: Hans de Goede
---
hw/usb/redirect.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index d9236c5..4cd32d6 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -352,7 +352,9 @@ static void
usbredir_fill_already_in_flight_from_ep(USBRedirDevice *dev,
if (p->combined && p != p->combined->first) {
continue;
}
-packet_id_queue_add(&dev->already_in_flight, p->id);
+if (p->state == USB_PACKET_ASYNC) {
+packet_id_queue_add(&dev->already_in_flight, p->id);
+}
}
}
--
1.7.12.1
[Qemu-devel] [PATCH 3/3] usb-redir: Set default debug level to warning
The previous default of 0 means that even errors and warnings would not
get printed, which is really not a good default.
Signed-off-by: Hans de Goede
---
hw/usb/redirect.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 4cd32d6..c8e2718 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1985,7 +1985,7 @@ static const VMStateDescription usbredir_vmstate = {
static Property usbredir_properties[] = {
DEFINE_PROP_CHR("chardev", USBRedirDevice, cs),
-DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, 0),
+DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, usbredirparser_warning),
DEFINE_PROP_STRING("filter", USBRedirDevice, filter_str),
DEFINE_PROP_INT32("bootindex", USBRedirDevice, bootindex, -1),
DEFINE_PROP_END_OF_LIST(),
--
1.7.12.1
[Qemu-devel] [PATCH 1/3] ehci: Don't verify the next pointer for periodic qh-s and qtd-s
While testing the move to async packet handling for interrupt endpoints I
noticed that Windows-XP likes to play tricks with the next pointer for
periodic qh-s, so we should not fail qh / qtd verification when it changes.
Signed-off-by: Hans de Goede
---
hw/usb/hcd-ehci.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 89b7520..287a066 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1550,8 +1550,10 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci,
int async)
endp= get_field(qh.epchar, QH_EPCHAR_EP);
if ((devaddr != get_field(q->qh.epchar, QH_EPCHAR_DEVADDR)) ||
(endp!= get_field(q->qh.epchar, QH_EPCHAR_EP)) ||
-(memcmp(&qh.current_qtd, &q->qh.current_qtd,
- 9 * sizeof(uint32_t)) != 0) ||
+(qh.current_qtd != q->qh.current_qtd) ||
+(q->async && qh.next_qtd != q->qh.next_qtd) ||
+(memcmp(&qh.altnext_qtd, &q->qh.altnext_qtd,
+ 7 * sizeof(uint32_t)) != 0) ||
(q->dev != NULL && q->dev->addr != devaddr)) {
if (ehci_reset_queue(q) > 0) {
ehci_trace_guest_bug(ehci, "guest updated active QH");
@@ -1719,7 +1721,8 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
p = QTAILQ_FIRST(&q->packets);
if (p != NULL) {
if (p->qtdaddr != q->qtdaddr ||
-(!NLPTR_TBIT(p->qtd.next) && (p->qtd.next != qtd.next)) ||
+(q->async && !NLPTR_TBIT(p->qtd.next) &&
+(p->qtd.next != qtd.next)) ||
(!NLPTR_TBIT(p->qtd.altnext) && (p->qtd.altnext != qtd.altnext)) ||
p->qtd.bufptr[0] != qtd.bufptr[0]) {
ehci_cancel_queue(q);
--
1.7.12.1
[Qemu-devel] [USB Fixes for 1.3]: ehci + usb-redir fixes for 1.3
As discussed here is one more ehci fix + 2 small usb-redir fixes for 1.3. Regards, Hans
Re: [Qemu-devel] [PATCH] ehci: handle dma errors
Looks good, ack.
On 11/15/2012 01:22 PM, Gerd Hoffmann wrote:
Starting with commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d dma
transfers can actually fail. This patch makes ehci keep track
of the busmaster bit in pci config space, by setting/clearing the
dma_context pointer. Attempts to dma without context will result
in raising HSE (Host System Error) interrupt and stopping the host
controller.
This patch fixes WinXP not booting with a usb stick attached to ehci.
Root cause is seabios activating ehci so you can boot from the stick,
and WinXP clearing the busmaster bit before resetting the host
controller, leading to ehci actually trying dma while it is disabled.
Signed-off-by: Gerd Hoffmann
---
hw/usb/hcd-ehci-pci.c | 17 +
hw/usb/hcd-ehci.c | 63 ++--
trace-events |1 +
3 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index fe45a1f..5887eab 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -17,6 +17,7 @@
#include "hw/usb/hcd-ehci.h"
#include "hw/pci.h"
+#include "range.h"
typedef struct EHCIPCIState {
PCIDevice pcidev;
@@ -79,6 +80,21 @@ static int usb_ehci_pci_initfn(PCIDevice *dev)
return 0;
}
+static void usb_ehci_pci_write_config(PCIDevice *dev, uint32_t addr,
+ uint32_t val, int l)
+{
+EHCIPCIState *i = DO_UPCAST(EHCIPCIState, pcidev, dev);
+bool busmaster;
+
+pci_default_write_config(dev, addr, val, l);
+
+if (!range_covers_byte(addr, l, PCI_COMMAND)) {
+return;
+}
+busmaster = pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER;
+i->ehci.dma = busmaster ? pci_dma_context(dev) : NULL;
+}
+
static Property ehci_pci_properties[] = {
DEFINE_PROP_UINT32("maxframes", EHCIPCIState, ehci.maxframes, 128),
DEFINE_PROP_END_OF_LIST(),
@@ -106,6 +122,7 @@ static void ehci_class_init(ObjectClass *klass, void *data)
k->device_id = i->device_id;
k->revision = i->revision;
k->class_id = PCI_CLASS_SERIAL_USB;
+k->config_write = usb_ehci_pci_write_config;
dc->vmsd = &vmstate_ehci_pci;
dc->props = ehci_pci_properties;
}
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 57da76c..df3aaed 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1000,21 +1000,25 @@ static void ehci_opreg_write(void *ptr, hwaddr addr,
*mmio, old);
}
-
-// TODO : Put in common header file, duplication from usb-ohci.c
-
/* Get an array of dwords from main memory */
static inline int get_dwords(EHCIState *ehci, uint32_t addr,
uint32_t *buf, int num)
{
int i;
+if (!ehci->dma) {
+ehci_raise_irq(ehci, USBSTS_HSE);
+ehci->usbcmd &= ~USBCMD_RUNSTOP;
+trace_usb_ehci_dma_error();
+return -1;
+}
+
for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
dma_memory_read(ehci->dma, addr, buf, sizeof(*buf));
*buf = le32_to_cpu(*buf);
}
-return 1;
+return num;
}
/* Put an array of dwords in to main memory */
@@ -1023,12 +1027,19 @@ static inline int put_dwords(EHCIState *ehci, uint32_t
addr,
{
int i;
+if (!ehci->dma) {
+ehci_raise_irq(ehci, USBSTS_HSE);
+ehci->usbcmd &= ~USBCMD_RUNSTOP;
+trace_usb_ehci_dma_error();
+return -1;
+}
+
for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
uint32_t tmp = cpu_to_le32(*buf);
dma_memory_write(ehci->dma, addr, &tmp, sizeof(tmp));
}
-return 1;
+return num;
}
/*
@@ -1440,8 +1451,10 @@ static int ehci_state_waitlisthead(EHCIState *ehci, int
async)
/* Find the head of the list (4.9.1.1) */
for(i = 0; i < MAX_QH; i++) {
-get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh,
- sizeof(EHCIqh) >> 2);
+if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh,
+ sizeof(EHCIqh) >> 2) < 0) {
+return 0;
+}
ehci_trace_qh(NULL, NLPTR_GET(entry), &qh);
if (qh.epchar & QH_EPCHAR_H) {
@@ -1538,8 +1551,11 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci,
int async)
goto out;
}
-get_dwords(ehci, NLPTR_GET(q->qhaddr),
- (uint32_t *) &qh, sizeof(EHCIqh) >> 2);
+if (get_dwords(ehci, NLPTR_GET(q->qhaddr),
+ (uint32_t *) &qh, sizeof(EHCIqh) >> 2) < 0) {
+q = NULL;
+goto out;
+}
ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &qh);
/*
@@ -1626,8 +1642,10 @@ static int ehci_state_fetchitd(EHCIState *ehci, int
async)
assert(!async);
entry = ehci_get_fetch_addr(ehci, async);
-get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd,
- sizeof(EHCIitd) >> 2);
+if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd,
+
[Qemu-devel] [PATCH v2 2/2] block: Avoid second open for format probing
This fixes problems that are caused by the additional open/close cycle
of the existing format probing, for example related to qemu-nbd without
-t option or file descriptor passing.
Signed-off-by: Kevin Wolf
---
block.c | 66 --
1 files changed, 38 insertions(+), 28 deletions(-)
diff --git a/block.c b/block.c
index 95fd00e..1be667f 100644
--- a/block.c
+++ b/block.c
@@ -518,22 +518,16 @@ BlockDriver *bdrv_find_protocol(const char *filename)
return NULL;
}
-static int find_image_format(const char *filename, BlockDriver **pdrv)
+static int find_image_format(BlockDriverState *bs, const char *filename,
+ BlockDriver **pdrv)
{
-int ret, score, score_max;
+int score, score_max;
BlockDriver *drv1, *drv;
uint8_t buf[2048];
-BlockDriverState *bs;
-
-ret = bdrv_file_open(&bs, filename, 0);
-if (ret < 0) {
-*pdrv = NULL;
-return ret;
-}
+int ret = 0;
/* Return the raw BlockDriver * to scsi-generic devices or empty drives */
if (bs->sg || !bdrv_is_inserted(bs)) {
-bdrv_delete(bs);
drv = bdrv_find_format("raw");
if (!drv) {
ret = -ENOENT;
@@ -543,7 +537,6 @@ static int find_image_format(const char *filename,
BlockDriver **pdrv)
}
ret = bdrv_pread(bs, 0, buf, sizeof(buf));
-bdrv_delete(bs);
if (ret < 0) {
*pdrv = NULL;
return ret;
@@ -657,7 +650,8 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
/*
* Common part for opening disk images and files
*/
-static int bdrv_open_common(BlockDriverState *bs, const char *filename,
+static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
+const char *filename,
int flags, BlockDriver *drv)
{
int ret, open_flags;
@@ -691,12 +685,16 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
/* Open the image, either directly or using a protocol */
if (drv->bdrv_file_open) {
-ret = drv->bdrv_file_open(bs, filename, open_flags);
-} else {
-ret = bdrv_file_open(&bs->file, filename, open_flags);
-if (ret >= 0) {
-ret = drv->bdrv_open(bs, open_flags);
+if (file != NULL) {
+bdrv_swap(file, bs);
+ret = 0;
+} else {
+ret = drv->bdrv_file_open(bs, filename, open_flags);
}
+} else {
+assert(file != NULL);
+bs->file = file;
+ret = drv->bdrv_open(bs, open_flags);
}
if (ret < 0) {
@@ -716,10 +714,7 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
return 0;
free_and_fail:
-if (bs->file) {
-bdrv_delete(bs->file);
-bs->file = NULL;
-}
+bs->file = NULL;
g_free(bs->opaque);
bs->opaque = NULL;
bs->drv = NULL;
@@ -741,7 +736,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char
*filename, int flags)
}
bs = bdrv_new("");
-ret = bdrv_open_common(bs, filename, flags, drv);
+ret = bdrv_open_common(bs, NULL, filename, flags, drv);
if (ret < 0) {
bdrv_delete(bs);
return ret;
@@ -795,6 +790,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
{
int ret;
char tmp_filename[PATH_MAX];
+BlockDriverState *file = NULL;
if (flags & BDRV_O_SNAPSHOT) {
BlockDriverState *bs1;
@@ -854,25 +850,36 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
bs->is_temporary = 1;
}
+/* Open image file without format layer */
+if (flags & BDRV_O_RDWR) {
+flags |= BDRV_O_ALLOW_RDWR;
+}
+
+ret = bdrv_file_open(&file, filename, bdrv_open_flags(bs, flags));
+if (ret < 0) {
+return ret;
+}
+
/* Find the right image format driver */
if (!drv) {
-ret = find_image_format(filename, &drv);
+ret = find_image_format(file, filename, &drv);
}
if (!drv) {
goto unlink_and_fail;
}
-if (flags & BDRV_O_RDWR) {
-flags |= BDRV_O_ALLOW_RDWR;
-}
-
/* Open the image */
-ret = bdrv_open_common(bs, filename, flags, drv);
+ret = bdrv_open_common(bs, file, filename, flags, drv);
if (ret < 0) {
goto unlink_and_fail;
}
+if (bs->file != file) {
+bdrv_delete(file);
+file = NULL;
+}
+
/* If there is a backing file, use it */
if ((flags & BDRV_O_NO_BACKING) == 0) {
ret = bdrv_open_backing_file(bs);
@@ -894,6 +901,9 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
return 0;
unlink_and_fail:
+if (file != NULL) {
+bdrv_delete(file);
+}
if (bs->is_temporary) {
unlink(filename);
}
--
1.7.6.5
[Qemu-devel] [PATCH v2 1/2] block: Factor out bdrv_open_flags
Signed-off-by: Kevin Wolf
---
block.c | 35 +--
1 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/block.c b/block.c
index bf0747f..95fd00e 100644
--- a/block.c
+++ b/block.c
@@ -634,6 +634,26 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
bs->copy_on_read--;
}
+static int bdrv_open_flags(BlockDriverState *bs, int flags)
+{
+int open_flags = flags | BDRV_O_CACHE_WB;
+
+/*
+ * Clear flags that are internal to the block layer before opening the
+ * image.
+ */
+open_flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+
+/*
+ * Snapshots should be writable.
+ */
+if (bs->is_temporary) {
+open_flags |= BDRV_O_RDWR;
+}
+
+return open_flags;
+}
+
/*
* Common part for opening disk images and files
*/
@@ -665,20 +685,7 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
bs->opaque = g_malloc0(drv->instance_size);
bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB);
-open_flags = flags | BDRV_O_CACHE_WB;
-
-/*
- * Clear flags that are internal to the block layer before opening the
- * image.
- */
-open_flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
-
-/*
- * Snapshots should be writable.
- */
-if (bs->is_temporary) {
-open_flags |= BDRV_O_RDWR;
-}
+open_flags = bdrv_open_flags(bs, flags);
bs->read_only = !(open_flags & BDRV_O_RDWR);
--
1.7.6.5
[Qemu-devel] [PATCH v2 0/2] block: Avoid second open for format probing
Kevin Wolf (2): block: Factor out bdrv_open_flags block: Avoid second open for format probing block.c | 101 -- 1 files changed, 59 insertions(+), 42 deletions(-) -- 1.7.6.5
Re: [Qemu-devel] [RFC] libqblock OOM issue
Il 15/11/2012 13:21, Wenchao Xia ha scritto: > >>>Personally agree, but I want to add a simple wrapper to let libqblock >>> get user faster. In this way I guess best choice now is making rpc >>> client and server not mirrored in implemention, server provides >>> r/w/info retrieving capabilities via XDR protocol, client keeps >>> the API unchanged but implement a bit different than server, which >>> may archieve the same effect as invoking qemu-img inside without string >>> parsing. >> >> I think the result would really not be simple... >> > yes it would not be very simple, I guess reimplement API on client > side with RPC for only R/W/info APIs, would be relative easier than > design a transparent and balanced RPC layer for every API. So just let the libvirt people use it. Then you will understand their requirements. Do not try to solve the world's problems at the first attempt. Paolo
Re: [Qemu-devel] [RFC] libqblock OOM issue
> Il 15/11/2012 05:18, Wenchao Xia ha scritto: Personally agree, but I want to add a simple wrapper to let libqblock get user faster. In this way I guess best choice now is making rpc client and server not mirrored in implemention, server provides r/w/info retrieving capabilities via XDR protocol, client keeps the API unchanged but implement a bit different than server, which may archieve the same effect as invoking qemu-img inside without string parsing. I think the result would really not be simple... yes it would not be very simple, I guess reimplement API on client side with RPC for only R/W/info APIs, would be relative easier than design a transparent and balanced RPC layer for every API. Not sure if libqblock now is blocked by OOM issue as 1st version to be reviewed. If you think it must be resolved first, I would add this wrapper quickly. No, I was lost with the build problems. Please repost the last version you have. Paolo OK, I'll rebase and repost it. -- Best Regards Wenchao Xia
[Qemu-devel] [PATCH] ehci: handle dma errors
Starting with commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d dma
transfers can actually fail. This patch makes ehci keep track
of the busmaster bit in pci config space, by setting/clearing the
dma_context pointer. Attempts to dma without context will result
in raising HSE (Host System Error) interrupt and stopping the host
controller.
This patch fixes WinXP not booting with a usb stick attached to ehci.
Root cause is seabios activating ehci so you can boot from the stick,
and WinXP clearing the busmaster bit before resetting the host
controller, leading to ehci actually trying dma while it is disabled.
Signed-off-by: Gerd Hoffmann
---
hw/usb/hcd-ehci-pci.c | 17 +
hw/usb/hcd-ehci.c | 63 ++--
trace-events |1 +
3 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index fe45a1f..5887eab 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -17,6 +17,7 @@
#include "hw/usb/hcd-ehci.h"
#include "hw/pci.h"
+#include "range.h"
typedef struct EHCIPCIState {
PCIDevice pcidev;
@@ -79,6 +80,21 @@ static int usb_ehci_pci_initfn(PCIDevice *dev)
return 0;
}
+static void usb_ehci_pci_write_config(PCIDevice *dev, uint32_t addr,
+ uint32_t val, int l)
+{
+EHCIPCIState *i = DO_UPCAST(EHCIPCIState, pcidev, dev);
+bool busmaster;
+
+pci_default_write_config(dev, addr, val, l);
+
+if (!range_covers_byte(addr, l, PCI_COMMAND)) {
+return;
+}
+busmaster = pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER;
+i->ehci.dma = busmaster ? pci_dma_context(dev) : NULL;
+}
+
static Property ehci_pci_properties[] = {
DEFINE_PROP_UINT32("maxframes", EHCIPCIState, ehci.maxframes, 128),
DEFINE_PROP_END_OF_LIST(),
@@ -106,6 +122,7 @@ static void ehci_class_init(ObjectClass *klass, void *data)
k->device_id = i->device_id;
k->revision = i->revision;
k->class_id = PCI_CLASS_SERIAL_USB;
+k->config_write = usb_ehci_pci_write_config;
dc->vmsd = &vmstate_ehci_pci;
dc->props = ehci_pci_properties;
}
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 57da76c..df3aaed 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1000,21 +1000,25 @@ static void ehci_opreg_write(void *ptr, hwaddr addr,
*mmio, old);
}
-
-// TODO : Put in common header file, duplication from usb-ohci.c
-
/* Get an array of dwords from main memory */
static inline int get_dwords(EHCIState *ehci, uint32_t addr,
uint32_t *buf, int num)
{
int i;
+if (!ehci->dma) {
+ehci_raise_irq(ehci, USBSTS_HSE);
+ehci->usbcmd &= ~USBCMD_RUNSTOP;
+trace_usb_ehci_dma_error();
+return -1;
+}
+
for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
dma_memory_read(ehci->dma, addr, buf, sizeof(*buf));
*buf = le32_to_cpu(*buf);
}
-return 1;
+return num;
}
/* Put an array of dwords in to main memory */
@@ -1023,12 +1027,19 @@ static inline int put_dwords(EHCIState *ehci, uint32_t
addr,
{
int i;
+if (!ehci->dma) {
+ehci_raise_irq(ehci, USBSTS_HSE);
+ehci->usbcmd &= ~USBCMD_RUNSTOP;
+trace_usb_ehci_dma_error();
+return -1;
+}
+
for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
uint32_t tmp = cpu_to_le32(*buf);
dma_memory_write(ehci->dma, addr, &tmp, sizeof(tmp));
}
-return 1;
+return num;
}
/*
@@ -1440,8 +1451,10 @@ static int ehci_state_waitlisthead(EHCIState *ehci, int
async)
/* Find the head of the list (4.9.1.1) */
for(i = 0; i < MAX_QH; i++) {
-get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh,
- sizeof(EHCIqh) >> 2);
+if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh,
+ sizeof(EHCIqh) >> 2) < 0) {
+return 0;
+}
ehci_trace_qh(NULL, NLPTR_GET(entry), &qh);
if (qh.epchar & QH_EPCHAR_H) {
@@ -1538,8 +1551,11 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci,
int async)
goto out;
}
-get_dwords(ehci, NLPTR_GET(q->qhaddr),
- (uint32_t *) &qh, sizeof(EHCIqh) >> 2);
+if (get_dwords(ehci, NLPTR_GET(q->qhaddr),
+ (uint32_t *) &qh, sizeof(EHCIqh) >> 2) < 0) {
+q = NULL;
+goto out;
+}
ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &qh);
/*
@@ -1626,8 +1642,10 @@ static int ehci_state_fetchitd(EHCIState *ehci, int
async)
assert(!async);
entry = ehci_get_fetch_addr(ehci, async);
-get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd,
- sizeof(EHCIitd) >> 2);
+if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd,
+ sizeof(EHCIitd) >> 2) < 0) {
+return -1;
+}
ehci_trace_itd(ehci, entry, &it
Re: [Qemu-devel] [PATCH 00/10] ehci: various fixes
Hi, >> Ok, so it looks like 1-3 are clear bugfixes which are needed in 1.3 >> > Moving it forward in the patch-set will cause a conflict though, I'll go > and > do the reshuffle myself, throw in 2 usb-redir fixes and then send a new set Oh, and I've picked 1-3 already, no need to resend them. cheers, Gerd
Re: [Qemu-devel] [PATCH 00/10] ehci: various fixes
On 11/15/12 12:58, Hans de Goede wrote: > Hi, > > On 11/15/2012 09:55 AM, Gerd Hoffmann wrote: >> On 11/14/12 17:21, Hans de Goede wrote: >>> While working on moving usb-redir and usb-hid over to using async packet >>> handling for their interrupt input endpoints. I've found and fixed quite >>> a few ehci bugs. >>> >>> Unfortunately the moving to async for interrupt endpoints turns out >>> to be a >>> bad idea, as it causes issues for migration, an async completed >>> packet will >>> not getting written back to guest memory until the next poll time, >>> and if a >>> migration happens in between it will get lost! >> >> Ok, so it looks like 1-3 are clear bugfixes which are needed in 1.3 >> > > Ack, I think that "ehci: Don't verify the next pointer for periodic qh-s" > belongs in 1.3 too, since this could be hit with host-redirection which > does use async completion for interrupt endpoints. > > Moving it forward in the patch-set will cause a conflict though, I'll go > and > do the reshuffle myself, throw in 2 usb-redir fixes and then send a new set > targetting 1.3. Thanks. > Do you also want a separate set on top for 1.4 / > usb-next, or > do you want me to keep the rest in my own tree for now ? Keep it in your tree for now. cheers, Gerd
Re: [Qemu-devel] Virtio refactoring.
On Thu, 15 Nov 2012 11:32:13 +0100 KONRAD Frédéric wrote: > For the qtree structure we have eg for virtio block : > > bus: main-system-bus >type System >dev: pcihost, id "" > bus: pci.0 >type PCI >dev: virtio-blk-pci, id "" > ... > My current virtio-ccw layout is: bus: main-system-bus type System dev: virtio-ccw-bridge, id "" bus: virtio-ccw type virtio-ccw-bus dev: virtio-blk-ccw, id "" ... > And it would become : > > bus: main-system-bus >type System >dev: pcihost, id "" > bus: pci.0 >type PCI >dev: virtio-pci, id "" > bus: virtio.0 >type VIRTIO >dev: virtio-blk, id "" > ... > I guess we could have the following for virtio-ccw: bus: main-system-bus type System dev: css, id "" bus: css.fe type ccw dev: virtio-ccw, id "" bus: virtio.0 type virtio dev: virtio-blk, id "" ... IOW, we introduce a channel subsystem (css) analogous to pcihost, have a ccw bus which can hold all kinds of channel devices, introduce a bus per channel subsystem image (here: the virtual css fe) and stick the virtio bus under it. If we want to support passthrough of real channel devices some day, they get their own bus for their channel subsystem (css.0, css.1, ...) and have the devices show up under it. (Just thinking aloud. I'll probably need to play with that idea a bit.) > > > Is it the right approach ? Do I miss something ? > >>> What of the alias handling? Can this be killed once everything has been > >>> converted? > >> Which alias ? > > The alias stuff in hw/qdev-monitor.c that lets you specify either > > virtio-- or virtio-. > > > So it would break the alias, we must find a solution for that. I think only virtio-xxx-pci and s390-virtio-xxx need to get backward compatibility; virtio-mmio and virtio-ccw should not need to deal with old interfaces.
Re: [Qemu-devel] [PATCH 00/10] ehci: various fixes
Hi, On 11/15/2012 09:55 AM, Gerd Hoffmann wrote: On 11/14/12 17:21, Hans de Goede wrote: While working on moving usb-redir and usb-hid over to using async packet handling for their interrupt input endpoints. I've found and fixed quite a few ehci bugs. Unfortunately the moving to async for interrupt endpoints turns out to be a bad idea, as it causes issues for migration, an async completed packet will not getting written back to guest memory until the next poll time, and if a migration happens in between it will get lost! Ok, so it looks like 1-3 are clear bugfixes which are needed in 1.3 Ack, I think that "ehci: Don't verify the next pointer for periodic qh-s" belongs in 1.3 too, since this could be hit with host-redirection which does use async completion for interrupt endpoints. Moving it forward in the patch-set will cause a conflict though, I'll go and do the reshuffle myself, throw in 2 usb-redir fixes and then send a new set targetting 1.3. Do you also want a separate set on top for 1.4 / usb-next, or do you want me to keep the rest in my own tree for now ? I'd tend to schedule the other ones for the 1.4 devel cycle. They are clearly nice cleanups, but as we are not going the async-for-intr route I think they are not important enougth to push them post-freeze. Ok. Patch 8 doesn't pass checkpatch (yes, it just shuffles around existing code, but it's still nice to fix style issues while touching those bits). Agreed, will fix. Regards, Hans
Re: [Qemu-devel] Fwd: buildbot failure in qemu on rhel5-default
On 15 November 2012 08:22, Gerd Hoffmann wrote: > CCs390x-softmmu/hw/s390x/event-facility.o > cc1: warnings being treated as errors > /home/buildbot/slave-spunk/rhel5-default/build/hw/s390x/event-facility.c: In > function ‘command_handler’: > /home/buildbot/slave-spunk/rhel5-default/build/hw/s390x/event-facility.c:110: > warning: ‘rc’ may be used uninitialized in this function > make[1]: *** [hw/s390x/event-facility.o] Error 1 > make: *** [subdir-s390x-softmmu] Error 2 handle_write_event_buf() doesn't consider the case of the qbus.children list being empty, in which case it will return an uninitialized value. Introduced in commit 559a17a14. -- PMM
Re: [Qemu-devel] [PATCH v6 1/8] atomic: introduce atomic operations
Il 15/11/2012 08:47, liu ping fan ha scritto:
RCU prototype required pointer-sized access, which you cannot make type-
>>> But I think that your RCU prototype should rely on atomic of CPU, not
>>> gcc‘s atomic.
>>
>> What's the difference? gcc's atomic produces the same instructions as
>> hand-written assembly (or should).
>>
> Probably I made a mistake here, in vhost, log =
> __sync_fetch_and_and(from, 0) is used to fetch 64bits atomically in
> the case 32bits qemu running on 64bits linux. Right? But how can
> we read 32bits twice in atomic?
You can use cmpxchg8b.
>>> Otherwise, it could be slow (I guess something like spinlock there).
>>
>> Not sure what you mean. I'm using two things: 1) write barriers; 2)
>> atomic_xchg of a pointer for fast, wait-free enqueuing of call_rcu
>> callbacks.
>
> Got your main idea. Will go through your prototype. Just one more
> question, why here atomic_xchg needed? Could not the sequence "read
> old pointer, set new pointer" satisfy your requirement?
No, it would be racy. Say you have this:
wrong right
-
ref(new) ref(new)
old = tailold = new
tail = newxchg(&old, &tail)
old->next = new old->next = new
up(&sem) up(&sem)
where sem and tail are global, while old and new are local. There can
be any number of producers as in the above scheme, and one consumer. In
the consumer, iteration of the list starts from the second element (the
first element is dummy):
dummy = new Node;
tail = dummy;
for(;;) {
down(&sem);
node = dummy->next;
unref(dummy);
visit node;
dummy = node;
}
Then the invariants are:
- the number of elements N in the list (starting from the dummy node and
following ->next) is always >1. Because of this, you never have to
touch head when adding an element.
- the number of excess signals in the semaphore sem is always <= N-1.
Because of this, it doesn't matter that there is an instant in time
where tail is not reachable from head. The element is really in the
list (as far as the consumer goes) only after the semaphore is signaled.
In the wrong version, two threads could read the same pointer:
x = tail |
y = tail |
tail = new_x |
tail = new_y |
x->next = new_x | old_tail->next = new_x
up(&sem) |
y->next = new_y | old_tail->next = new_x
up(&sem) |
No node points to new_x, so you have 2 nodes reachable from the head
(the dummy node, and new_y) with 2 signals on the semaphore,
contradicting the second invariant.
This instead works:
x = new_x |
y = new_y|
xchg(&x, &tail) | tail = x, x = old_tail
xchg(&y, &tail) | tail = y, y = x
x->next = new_x | old_tail->next = new_x
up(&sem) |
y->next = new_y | x->next = new_y
up(&sem) |
because you have 3 nodes and 2 signals.
Paolo
Re: [Qemu-devel] Virtio refactoring.
Hi, On 13/11/2012 19:09, Cornelia Huck wrote: On Tue, 13 Nov 2012 17:31:40 +0100 KONRAD Frédéric wrote: We'd go from system bus -> virtio transport bridge dev (virtio-xxx-bridge) -> virtio transport bus (virtio-xxx-bus) -> virtio transport dev (virtio--xxx) to system bus -> virtio transport bridge dev (virtio-bridge-xxx) -> virtio bus (virtio-bus-xxx) -> virtio dev (virtio--xxx) ? I'm not sure of what you mean,.. do you mean for s390 ? for the moment we have e.g : virtio-blk-pci ( in virtio-pci.c ) and we want virtio-pci -> virtio-bus -> virtio-blk. ( or virtio-mmio -> virtio-bus -> virtio-blk. for pci-less system. ) I meant the structure you see in 'info qtree'. We might be talking about the same thing :) For the qtree structure we have eg for virtio block : bus: main-system-bus type System dev: pcihost, id "" bus: pci.0 type PCI dev: virtio-blk-pci, id "" ... And it would become : bus: main-system-bus type System dev: pcihost, id "" bus: pci.0 type PCI dev: virtio-pci, id "" bus: virtio.0 type VIRTIO dev: virtio-blk, id "" ... Is it the right approach ? Do I miss something ? What of the alias handling? Can this be killed once everything has been converted? Which alias ? The alias stuff in hw/qdev-monitor.c that lets you specify either virtio-- or virtio-. So it would break the alias, we must find a solution for that. Fred
Re: [Qemu-devel] [RFC] libqblock OOM issue
Il 15/11/2012 05:18, Wenchao Xia ha scritto: > Personally agree, but I want to add a simple wrapper to let libqblock > get user faster. In this way I guess best choice now is making rpc > client and server not mirrored in implemention, server provides > r/w/info retrieving capabilities via XDR protocol, client keeps > the API unchanged but implement a bit different than server, which > may archieve the same effect as invoking qemu-img inside without string > parsing. I think the result would really not be simple... > Not sure if libqblock now is blocked by OOM issue as 1st version to > be reviewed. If you think it must be resolved first, I would add this > wrapper quickly. No, I was lost with the build problems. Please repost the last version you have. Paolo
Re: [Qemu-devel] [PATCH] Add domain-search option to slirp's DHCP server
On 2012-10-27 19:53, Klaus Stengel wrote:
> This patch will allow the user to include the domain-search option in
> replies from the built-in DHCP server. The domain suffixes can be
> specified by adding dnssearch= entries to the "-net user" parameter.
>
> Signed-off-by: Klaus Stengel
> ---
> net/slirp.c | 35 +-
> qapi-schema.json|4 +
> qemu-options.hx | 18 +++-
> slirp/Makefile.objs |2 +-
> slirp/bootp.c | 12 ++
> slirp/dnssearch.c | 320
> +++
> slirp/libslirp.h|3 +-
> slirp/slirp.c |8 +-
> slirp/slirp.h |5 +
> 9 files changed, 398 insertions(+), 9 deletions(-)
> create mode 100644 slirp/dnssearch.c
>
> diff --git a/net/slirp.c b/net/slirp.c
> index bf86a44..72ecc5e 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -136,7 +136,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
>const char *vhostname, const char *tftp_export,
>const char *bootfile, const char *vdhcp_start,
>const char *vnameserver, const char *smb_export,
> - const char *vsmbserver)
> + const char *vsmbserver, const char **dnssearch)
> {
> /* default settings according to historic slirp */
> struct in_addr net = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
> @@ -242,7 +242,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> s = DO_UPCAST(SlirpState, nc, nc);
>
> s->slirp = slirp_init(restricted, net, mask, host, vhostname,
> - tftp_export, bootfile, dhcp, dns, s);
> + tftp_export, bootfile, dhcp, dns, dnssearch, s);
> QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
>
> for (config = slirp_configs; config; config = config->next) {
> @@ -699,6 +699,31 @@ net_init_slirp_configs(const StringList *fwd, int flags)
> }
> }
>
> +static const char**
> +slirp_dnssearch(const StringList *dnsname) {
> +const StringList *c = dnsname;
> +size_t i = 0, num_opts = 0;
> +const char **ret;
> +
> +while (c) {
> +num_opts++;
> +c = c->next;
> +}
> +
> +if (num_opts == 0) {
> +return NULL;
> +}
> +
> +ret = g_malloc((num_opts + 1) * sizeof(*ret));
> +c = dnsname;
> +while (c) {
> +ret[i++] = c->value->str;
> +c = c->next;
> +}
> +ret[i] = NULL;
> +return ret;
> +}
> +
> int net_init_slirp(const NetClientOptions *opts, const char *name,
> NetClientState *peer)
> {
> @@ -706,6 +731,7 @@ int net_init_slirp(const NetClientOptions *opts, const
> char *name,
> char *vnet;
> int ret;
> const NetdevUserOptions *user;
> +const char **dnssearch;
>
> assert(opts->kind == NET_CLIENT_OPTIONS_KIND_USER);
> user = opts->user;
> @@ -714,6 +740,8 @@ int net_init_slirp(const NetClientOptions *opts, const
> char *name,
> user->has_ip ? g_strdup_printf("%s/24", user->ip) :
> NULL;
>
> +dnssearch = slirp_dnssearch(user->dnssearch);
> +
> /* all optional fields are initialized to "all bits zero" */
>
> net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
> @@ -722,7 +750,7 @@ int net_init_slirp(const NetClientOptions *opts, const
> char *name,
> ret = net_slirp_init(peer, "user", name, user->q_restrict, vnet,
> user->host, user->hostname, user->tftp,
> user->bootfile, user->dhcpstart, user->dns,
> user->smb,
> - user->smbserver);
> + user->smbserver, dnssearch);
>
> while (slirp_configs) {
> config = slirp_configs;
> @@ -731,6 +759,7 @@ int net_init_slirp(const NetClientOptions *opts, const
> char *name,
> }
>
> g_free(vnet);
> +g_free(dnssearch);
>
> return ret;
> }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 6fd263e..b24ce95 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2297,6 +2297,9 @@
> #
> # @dns: #optional guest-visible address of the virtual nameserver
> #
> +# @dnssearch: #optional list of DNS suffixes to search, passed as DHCP option
> +# to the guest
> +#
> # @smb: #optional root directory of the built-in SMB server
> #
> # @smbserver: #optional IP address of the built-in SMB server
> @@ -2319,6 +2322,7 @@
> '*bootfile': 'str',
> '*dhcpstart': 'str',
> '*dns': 'str',
> +'*dnssearch': ['String'],
> '*smb': 'str',
> '*smbserver': 'str',
> '*hostfwd': ['String'],
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 46f0539..a6efc56 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1282,8 +1282,8 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
> "create a new Network Interface Card and connect it to
> VLAN 'n'\n"
>
Re: [Qemu-devel] [PATCH v2] slirp: Don't crash on packets from 0.0.0.0/8.
On 2012-11-12 17:59, Nickolai Zeldovich wrote:
> LWIP can generate packets with a source of 0.0.0.0, which triggers an
> assertion failure in arp_table_add(). Instead of crashing, simply return
> to avoid adding an invalid ARP table entry.
>
> Signed-off-by: Nickolai Zeldovich
> ---
> slirp/arp_table.c |4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Change from v1: adhere to qemu's code style (put braces around all
> indentation blocks).
>
>
> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> index 5d7b8ac..bf698c1 100644
> --- a/slirp/arp_table.c
> +++ b/slirp/arp_table.c
> @@ -38,7 +38,9 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t
> ethaddr[ETH_ALEN])
> ethaddr[3], ethaddr[4], ethaddr[5]));
>
> /* Check 0.0.0.0/8 invalid source-only addresses */
> -assert((ip_addr & htonl(~(0xf << 28))) != 0);
> +if ((ip_addr & htonl(~(0xf << 28))) == 0) {
> +return;
> +}
>
> if (ip_addr == 0x || ip_addr == broadcast_addr) {
> /* Do not register broadcast addresses */
>
Thanks, applied to slirp queue.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] usb: host-linux: Ignore parsing errors of the device descriptors
On 11/15/12 09:31, Jan Kiszka wrote: >> > I'd prefer to keep the error jump target to handle the parse error here. >> > Dumping the reset there is fine with me, but I'd prefer this event being >> > logged (trace point or stderr message or both) to ease trouble shooting >> > in case a device doesn't behave as expected. > That would be "over-logging" as we already record the individual > reasons. There is simply no code to jump to after the refactoring. Oh, right, we trace that already. Ok then. Patch added to usb patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH] usb: uhci: Look up queue by address, not token
On 11/15/12 09:20, Jan Kiszka wrote:
> -if (queue->token == token) {
> +if (queue->qh_addr == qh_addr) {
Patch added to usb patch queue.
thanks,
Gerd
