Re: [Qemu-devel] [PATCH 2/3] usb-redir: Call qemu_chr_guest_open/close
Hi, On 08/07/2011 11:30 PM, Anthony Liguori wrote: On 08/07/2011 12:41 PM, Hans de Goede wrote: Hi, On 08/07/2011 05:52 PM, Anthony Liguori wrote: On 08/07/2011 08:21 AM, Hans de Goede wrote: To let the chardev now we're ready start receiving data. This is necessary with the spicevmc chardev to get it registered with the spice-server. Signed-off-by: Hans de Goede --- usb-redir.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/usb-redir.c b/usb-redir.c index e212993..ec88c0b 100644 --- a/usb-redir.c +++ b/usb-redir.c @@ -809,6 +809,8 @@ static int usbredir_initfn(USBDevice *udev) qemu_chr_add_handlers(dev->cs, usbredir_chardev_can_read, usbredir_chardev_read, usbredir_chardev_event, dev); + /* Let the other side know we are ready */ + qemu_chr_guest_open(dev->cs); You should do guest_open before adding handlers. Erm, no, guest_open may lead to a callback in the chardev, to which it may respond by immediately queuing a few writes / doing a read. So after my char-flow changes, you won't be allowed to set handlers unless you've called open. Why not do it the other way around? So don't allow open until the handlers are set. My reasoning behind this is that eventually we will want to have a struct describing a pipe endpoint, which will contain handlers (by then identical for both sides) and besides the struct a priv / user_data pointer which will get passed by the handlers when called. Then we will have a chardev_create or pipe_create call which will take a struct + user data ptr for both ends (so twice). This matches what currently our set handlers call does. But I would expect the open to come after the creation of the pipe. At least to me it is much more logical to first set the handlers (which are really part of object creation) and then later do the open, this matches the common programming paradigm of having an init/create function and an open function. Also forcing the set handlers after the open does not work well with virtio_console, as these are not open until the port inside the guest is opened. So then it would need to delay its set handlers till the first open, and what should it do at close, do a set handlers NULL before doing the actual close ?? Regards, Hans
Re: [Qemu-devel] Build broken
Am 05.08.2011 18:49, schrieb malc: > On Fri, 5 Aug 2011, Stefan Hajnoczi wrote: > >> On Fri, Aug 5, 2011 at 7:22 AM, malc wrote: >>> >>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function >>> 'coroutine_new': >>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' >>> may be used uninitialized in this function >>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' >>> was declared here >>> >>> diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c >>> index 41c2379..42dc3e2 100644 >>> --- a/coroutine-ucontext.c >>> +++ b/coroutine-ucontext.c >>> @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void) >>> CoroutineUContext *co; >>> ucontext_t old_uc, uc; >>> jmp_buf old_env; >>> -union cc_arg arg; >>> +union cc_arg arg = {0}; >>> >>> /* The ucontext functions preserve signal masks which incurs a system >>> call >>> * overhead. setjmp()/longjmp() does not preserve signal masks but only >>> >>> I guess gcc should yell not only here on ppc32 but on any machine where >>> pointer size is less than the size of two ints. >> >> Makes sense. Are you going to commit a fix or send a signed-off-by patch? >> > > If the author(s)(you and Kevin? just you?) agree with the above i can just > push it. Feel free to push it. (Original code was by Anthony, then heavily modified by me, and after that modified again by Stefan) Acked-by: Kevin Wolf
Re: [Qemu-devel] Safely reopening image files by stashing fds
Am 08.08.2011 09:02, schrieb Supriya Kannery: > On 08/05/2011 09:19 PM, Anthony Liguori wrote: >> On 08/05/2011 10:43 AM, Kevin Wolf wrote: >>> Am 05.08.2011 17:24, schrieb Stefan Hajnoczi: On Fri, Aug 5, 2011 at 3:28 PM, Christoph Hellwig wrote: > On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote: >>> Because you cannot change O_DIRECT on an open fd :(. This is why >>> we're going through this pain. >> >> Hmm, I remember hearing that before, but looking at the current >> fcntl() >> manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps >> this >> is a newish feature, but it'd be nicer to use it if possible ? > > It's been there since day 1 of O_DIRECT support. Sorry, my bad. So for Linux we could just use fcntl for block_set_hostcache and not bother with reopening. However, we will need to reopen should we wish to support changing O_DSYNC. >>> >>> We do wish to support that. >>> >>> Anthony thinks that allowing the guest to toggle WCE is a prerequisite >>> for making cache=writeback the default. And this is something that I >>> definitely want to do for 1.0. >> >> Indeed. >> > > We discussed the following so far... > 1. How to safely reopen image files > 2. Dynamic hostcache change > 3. Support for dynamic change of O_DSYNC > > Since 2 is independent of 1, shall I go ahead implementing > hostcache change using fcntl. > > Implementation for safely reopening image files using "BDRVReopenState" > can be done separately as a pre-requisite before implementing 3 Doing it separately means that we would introduce yet another callback that is used just to change O_DIRECT. In the end we want it to use bdrv_reopen(), too, so I'm not sure if there is a need for a temporary solution. Actually, once we know what we really want (I haven't seen many comments on the BDRVReopenState suggestion yet), it should be pretty easy to implement. Kevin
Re: [Qemu-devel] [PATCH] Permit -mem-path without sync mmu
On 08/08/2011 09:03 AM, David Gibson wrote: Second, if userspace qemu passing hugepages to kvm can cause (host) kernel memory corruption, that is clearly a host kernel bug. So am I correct in thinking this is basically just a safety feature if qemu is run on a buggy kernel. Seems so, yes. 2.6.2[456] are exploitable. We only found out after these were all released. Presumably this bug was corrected at some point? Is the presence of the SYNC_MMU feature just being used as a proxy for "is this kernel recent enough to have the corruption bug fixed"? SYNC_MMU actually fixes the bug. In any case this test sure as hell needs a big comment next to it explaining this context. Yes. > Why are mmu notifiers not implemented for PPC again? It's just not done yet; we're working on it. (That is, mmu notifiers are certainly present on PPC, it's just they're not wired up to kvm, yet). If ppc doesn't have this issue even without SYNC_MMU, we can make the check x86 specific. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] kvm PCI assignment & VFIO ramblings
On 08/03/2011 05:04 AM, David Gibson wrote: I still don't understand the distinction you're making. We're saying the group is "owned" by a given user or guest in the sense that no-one else may use anything in the group (including host drivers). At that point none, some or all of the devices in the group may actually be used by the guest. You seem to be making a distinction between "owned by" and "assigned to" and "used by" and I really don't see what it is. Alex (and I) think that we should work with device/function granularity, as is common with other archs, and that the group thing is just a constraint on which functions may be assigned where, while you think that we should work at group granularity, with 1-function groups for archs which don't have constraints. Is this an accurate way of putting it? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 13/16] usb-hid: add hid_has_events()
On Thu, Aug 4, 2011 at 23:10, Gerd Hoffmann wrote: > Add hid_has_events function, use it to figure whenever there are pending > events instead of checking and updating USBHIDState->changed. > > Setting ->changed to 1 on init is removed, that should have absolutely > no effect as the initial state of ->idle is 0 so we report hid state > anyway until the guest configures some idle time. Also should clear > ->idle on reset. > > Signed-off-by: Gerd Hoffmann > --- > hw/usb-hid.c | 16 +++- > 1 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/hw/usb-hid.c b/hw/usb-hid.c > index 870cc66..b730692 100644 > --- a/hw/usb-hid.c > +++ b/hw/usb-hid.c > @@ -88,7 +88,6 @@ typedef struct USBHIDState { > int32_t protocol; > uint8_t idle; > int64_t next_idle_clock; > - int changed; > void *datain_opaque; > void (*datain)(void *); > } USBHIDState; > @@ -444,12 +443,15 @@ static const uint8_t usb_hid_usage_keys[0x100] = { > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > }; > > +static bool hid_has_events(HIDState *hs) > +{ > + return hs->n > 0; > +} > + > static void usb_hid_changed(HIDState *hs) > { > USBHIDState *us = container_of(hs, USBHIDState, hid); > > - us->changed = 1; > - > if (us->datain) { > us->datain(us->datain_opaque); > } > @@ -742,6 +744,7 @@ static void usb_hid_handle_reset(USBDevice *dev) > > hid_handle_reset(&us->hid); > us->protocol = 1; > + us->idle = 0; > } > > static void usb_hid_set_next_idle(USBHIDState *s, int64_t curtime) > @@ -798,7 +801,6 @@ static int usb_hid_handle_control(USBDevice *dev, > USBPacket *p, > } else if (hs->kind == HID_KEYBOARD) { > ret = hid_keyboard_poll(hs, data, length); > } > - us->changed = hs->n > 0; > break; > case SET_REPORT: > if (hs->kind == HID_KEYBOARD) { > @@ -849,7 +851,7 @@ static int usb_hid_handle_data(USBDevice *dev, USBPacket > *p) > case USB_TOKEN_IN: > if (p->devep == 1) { > int64_t curtime = qemu_get_clock_ns(vm_clock); > - if (!us->changed && > + if (!hid_has_events(hs) && > (!us->idle || us->next_idle_clock - curtime > 0)) { > return USB_RET_NAK; > } > @@ -860,7 +862,6 @@ static int usb_hid_handle_data(USBDevice *dev, USBPacket > *p) > ret = hid_keyboard_poll(hs, buf, p->iov.size); > } > usb_packet_copy(p, buf, ret); > - us->changed = hs->n > 0; > } else { > goto fail; > } > @@ -914,9 +915,6 @@ static int usb_hid_initfn(USBDevice *dev, int kind) > > usb_desc_init(dev); > hid_init(&us->hid, kind, usb_hid_changed); > - > - /* Force poll routine to be run and grab input the first time. */ > - us->changed = 1; USB tablet does not work on the winxp guest. I think this code can't be removed. > return 0; > } > > -- > 1.7.1 > > >
Re: [Qemu-devel] [PATCH v3] XBZRLE delta for live migration of large memory apps
> -Original Message- > From: Stefan Hajnoczi [mailto:stefa...@gmail.com] > Sent: Tuesday, August 02, 2011 9:06 PM > To: Shribman, Aidan > Cc: qemu-devel@nongnu.org; Anthony Liguori > Subject: Re: [PATCH v3] XBZRLE delta for live migration of > large memory apps > > On Tue, Aug 02, 2011 at 03:45:56PM +0200, Shribman, Aidan wrote: > > Subject: [PATCH v3] XBZRLE delta for live migration of > large memory apps > > From: Aidan Shribman > > > > By using XBZRLE (Xor Binary Zero Run-Length-Encoding) we > can reduce VM downtime > > and total live-migration time for VMs running memory write > intensive workloads > > typical of large enterprise applications such as SAP ERP > Systems, and generally > > speaking for representative of any application with a > sparse memory update pattern. > > > > On the sender side XBZRLE is used as a compact delta > encoding of page updates, > > retrieving the old page content from an LRU cache (default > size of 64 MB). The > > receiving side uses the existing page content and XBZRLE to > decode the new page > > content. > > > > Work was originally based on research results published VEE > 2011: Evaluation of > > Delta Compression Techniques for Efficient Live Migration > of Large Virtual > > Machines by Benoit, Svard, Tordsson and Elmroth. > Additionally the delta encoder > > XBRLE was improved further using XBZRLE instead. > > > > XBZRLE has a sustained bandwidth of 1.5-2.2 GB/s for > typical workloads making it > > ideal for in-line, real-time encoding such as is needed for > live-migration. > > What is the CPU cost of xbzrle live migration on the source host? I'm > thinking about a graph showing CPU utilization (e.g. from mpstat(1)) > that has two datasets: migration without xbzrle and migration with > xbzrle. > zbzrle.out indicates that xbzrle is using 50% of the compute capacity during the xbzrle live-migration (which completed is few seconds), In vanilla.out between 30%-60% of compute is directed toward the live-migration itself - in this case live-migration is not able to complete. - root@ilrsh01:~# root@ilrsh01:~# cat xbzrle.out Linux 2.6.35-22-server (ilrsh01)08/07/2011 _x86_64_(2 CPU) 10:55:37 AM CPU%usr %nice%sys %iowait%irq %soft %steal %guest%idle 10:55:38 AM all 40.500.001.001.500.009.000.00 0.00 48.00 10:55:38 AM00.000.001.003.000.000.000.00 0.00 96.00 10:55:38 AM1 81.000.001.000.000.00 18.000.00 0.000.00 10:55:38 AM CPU%usr %nice%sys %iowait%irq %soft %steal %guest %idle 10:55:39 AM all 47.000.001.500.000.002.000.00 0.00 49.50 10:55:39 AM00.000.001.000.000.000.000.00 0.00 99.00 10:55:39 AM1 94.000.002.000.000.004.000.00 0.000.00 10:55:39 AM CPU%usr %nice%sys %iowait%irq %soft %steal %guest %idle 10:55:40 AM all 50.000.000.500.000.000.000.00 0.00 49.50 10:55:40 AM00.000.001.000.000.000.000.00 0.00 99.00 10:55:40 AM1 100.000.000.000.000.000.000.00 0.000.00 10:55:40 AM CPU%usr %nice%sys %iowait%irq %soft %steal %guest %idle 10:55:41 AM all 49.750.001.990.000.000.000.00 0.00 48.26 10:55:41 AM0 10.000.000.000.000.000.000.00 0.00 90.00 10:55:41 AM1 89.110.003.960.000.000.000.00 0.006.93 10:55:41 AM CPU%usr %nice%sys %iowait%irq %soft %steal %guest%idle 10:55:42 AM all 47.260.008.960.000.001.990.00 0.00 41.79 10:55:42 AM0 51.000.000.000.000.000.000.00 0.00 49.00 10:55:42 AM1 43.560.00 17.820.000.003.960.00 0.00 34.65 10:55:42 AM CPU%usr %nice%sys %iowait%irq %soft %steal %guest%idle 10:55:43 AM all 50.000.00 11.502.000.001.000.00 0.00 35.50 10:55:43 AM0 100.000.000.000.000.000.000.00 0.000.00 10:55:43 AM10.000.00 23.004.000.002.000.00 0.00 71.00 10:55:43 AM CPU%usr %nice%sys %iowait%irq %soft %steal %guest%idle 10:55:44 AM all 50.000.00 22.000.000.000.000.00 0.00 28.00 10:55:44 AM0 100.000.000.000.000.000.000.00 0.000.00 10:55:44 AM10.000.00 44.000.000.000.000.00 0.00 56.00 10:55:44 AM CPU%usr %nice%sys %iowait%irq %soft %steal %guest %idle 10:55:45 AM all 49.500.00 23.500.000.002.000.00 0.00 25.00 10:55:45 AM0 99.000.001.000.000.000.000.00 0.000.00
[Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
Subject: [PATCH v4] XBZRLE delta for live migration of large memory apps From: Aidan Shribman By using XBZRLE (Xor Binary Zero Run-Length-Encoding) we can reduce VM downtime and total live-migration time of VMs running memory write intensive workloads typical of large enterprise applications such as SAP ERP Systems, and generally speaking for any application with a sparse memory update pattern. On the sender side XBZRLE is used as a compact delta encoding of page updates, retrieving the old page content from an LRU cache (default size of 64 MB). The receiving side uses the existing page content and XBZRLE to decode the new page content. Work was originally based on research results published VEE 2011: Evaluation of Delta Compression Techniques for Efficient Live Migration of Large Virtual Machines by Benoit, Svard, Tordsson and Elmroth. Additionally the delta encoder XBRLE was improved further using XBZRLE instead. XBZRLE has a sustained bandwidth of 2-2.5 GB/s for typical workloads making it ideal for in-line, real-time encoding such as is needed for live-migration. A typical usage scenario: {qemu} migrate_set_cachesize 256m {qemu} migrate -x -d tcp:destination.host: {qemu} info migrate ... transferred ram-duplicate: A kbytes transferred ram-duplicate: B pages transferred ram-normal: C kbytes transferred ram-normal: D pages transferred ram-xbrle: E kbytes transferred ram-xbrle: F pages overflow ram-xbrle: G pages cache-hit ram-xbrle: H pages cache-lookup ram-xbrle: J pages Testing: live migration with XBZRLE completed in 110 seconds, without live migration was not able to complete. A simple synthetic memory r/w load generator: ..include ..include ..int main() ..{ ..char *buf = (char *) calloc(4096, 4096); ..while (1) { ..int i; ..for (i = 0; i < 4096 * 4; i++) { ..buf[i * 4096 / 4]++; ..} ..printf("."); ..} ..} Signed-off-by: Benoit Hudzia Signed-off-by: Petter Svard Signed-off-by: Aidan Shribman -- Makefile.target |1 + arch_init.c | 351 ++-- block-migration.c |3 +- hash.h| 72 +++ hmp-commands.hx | 36 -- hw/hw.h |3 +- lru.c | 142 + lru.h | 13 ++ migration-exec.c |6 +- migration-fd.c|6 +- migration-tcp.c |6 +- migration-unix.c |6 +- migration.c | 119 +- migration.h | 25 +++- qmp-commands.hx | 43 ++- savevm.c | 13 ++- sysemu.h | 13 ++- xbzrle.c | 126 +++ xbzrle.h | 12 ++ 19 files changed, 917 insertions(+), 79 deletions(-) diff --git a/Makefile.target b/Makefile.target index 2800f47..b3215de 100644 --- a/Makefile.target +++ b/Makefile.target @@ -186,6 +186,7 @@ endif #CONFIG_BSD_USER ifdef CONFIG_SOFTMMU obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o +obj-y += lru.o xbzrle.o # virtio has to be here due to weird dependency between PCI and virtio-net. # need to fix this properly obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o diff --git a/arch_init.c b/arch_init.c old mode 100644 new mode 100755 index 4486925..d67dc82 --- a/arch_init.c +++ b/arch_init.c @@ -40,6 +40,17 @@ #include "net.h" #include "gdbstub.h" #include "hw/smbios.h" +#include "lru.h" +#include "xbzrle.h" + +//#define DEBUG_ARCH_INIT +#ifdef DEBUG_ARCH_INIT +#define DPRINTF(fmt, ...) \ +do { fprintf(stdout, "arch_init: " fmt, ## __VA_ARGS__); } while (0) +#else +#define DPRINTF(fmt, ...) \ +do { } while (0) +#endif #ifdef TARGET_SPARC int graphic_width = 1024; @@ -88,6 +99,161 @@ const uint32_t arch_type = QEMU_ARCH; #define RAM_SAVE_FLAG_PAGE 0x08 #define RAM_SAVE_FLAG_EOS 0x10 #define RAM_SAVE_FLAG_CONTINUE 0x20 +#define RAM_SAVE_FLAG_XBZRLE0x40 + +/***/ +/* RAM Migration State */ +typedef struct ArchMigrationState { +int use_xbrle; +int64_t xbrle_cache_size; +} ArchMigrationState; + +static ArchMigrationState arch_mig_state; + +void arch_set_params(int blk_enable, int shared_base, int use_xbrle, +int64_t xbrle_cache_size, void *opaque) +{ +arch_mig_state.use_xbrle = use_xbrle; +arch_mig_state.xbrle_cache_size = xbrle_cache_size; +} + +#define BE16_MAGIC 0x0123 + +/***/ +/* XBZRLE (Xor Binary Zero Run-Length Encoding) */ +typedef struct XBZRLEHeader { +uint32_t xh_cksum; /* not used */ +uint16_t xh_magic; +uint16_t xh_len; +uint8_t xh_flags; +} XBZRLEHeader; + +static uint8_t dup_buf[TARGET_PAGE_SIZE]; + +/***/ +/* accounting */ +typedef struct AccountingInfo{ +uint64_t dup_pages; +
Re: [Qemu-devel] [PATCH 2/3] Introduce a 'client_add' monitor command accepting an open FD
On Sat, Aug 06, 2011 at 09:38:03AM -0500, Anthony Liguori wrote: > On 06/23/2011 07:31 AM, Daniel P. Berrange wrote: > >Allow client connections for VNC and socket based character > >devices to be passed in over the monitor using SCM_RIGHTS. > > > >One intended usage scenario is to start QEMU with VNC on a > >UNIX domain socket. An unprivileged user which cannot access > >the UNIX domain socket, can then connect to QEMU's VNC server > >by passing an open FD to libvirt, which passes it onto QEMU. > > > > { "execute": "get_fd", "arguments": { "fdname": "myclient" } } > > { "return": {} } > > { "execute": "add_client", "arguments": { "protocol": "vnc", > >"fdname": "myclient", > >"skipauth": true } } > > { "return": {} } > > > >In this case 'protocol' can be 'vnc' or 'spice', or the name > >of a character device (eg from -chardev id=) > > > >The 'skipauth' parameter can be used to skip any configured > >VNC authentication scheme, which is useful if the mgmt layer > >talking to the monitor has already authenticated the client > >in another way. > > > >* console.h: Define 'vnc_display_add_client' method > >* monitor.c: Implement 'client_add' command > >* qemu-char.c, qemu-char.h: Add 'qemu_char_add_client' method > > I don't feel all that good about this anymore. The notion of adding > a fd to an arbitrary character device is a big layering violation > and doesn't make much sense conceptually. > > A chardev cannot have multiple connections. It seems like the > use-case is much better suited to having an fd: protocol to create > char devices with. The same idea of being able to configure a VNC server to listen on a UNIX socket path by default, but be able to inject the client connection via libvirt + FD passing, also applies to chardevs IMHO. So having a fd: protocol chardev doesn't seem right to me. Even if we don't support multiple connections, it is still useful to be able to pass in the initial connection, for TCP/UNIX based chardevs. > I'd like to partially revert this removing the qemu_chr_add_client > interface. The VNC bit was the most immediately useful part to me, so having the chardev bits now is not critical. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] build failure with coroutine-gthread
LINK qemu-ga coroutine-gthread.o: In function `coroutine_init': /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined reference to `g_thread_init' collect2: ld returned 1 exit status The below patch fix the failure. I also added the patch in the VirtFS pull request. commit 4b76a481ee28166d5f415ef97833c624f4fc0792 Author: Stefan Hajnoczi Date: Mon Aug 8 13:04:05 2011 +0530 coroutine: add gthread dependency Commit 1fc7bd4a86a2bfeafcec29445871eb97469a2699 removed the gthread and gio dependency since qemu-ga did not require it. Coroutines require gthread, so add it back in. Signed-off-by: Stefan Hajnoczi diff --git a/configure b/configure index 0c67a4a..a6687f7 100755 --- a/configure +++ b/configure @@ -1844,16 +1844,14 @@ fi ## # glib support probe -if test "$guest_agent" != "no" ; then -if $pkg_config --modversion glib-2.0 > /dev/null 2>&1 ; then -glib_cflags=`$pkg_config --cflags glib-2.0 2>/dev/null` -glib_libs=`$pkg_config --libs glib-2.0 2>/dev/null` -libs_softmmu="$glib_libs $libs_softmmu" -libs_tools="$glib_libs $libs_tools" -else -echo "glib-2.0 required to compile QEMU" -exit 1 -fi +if $pkg_config --modversion gthread-2.0 > /dev/null 2>&1 ; then +glib_cflags=`$pkg_config --cflags gthread-2.0 2>/dev/null` +glib_libs=`$pkg_config --libs gthread-2.0 2>/dev/null` +libs_softmmu="$glib_libs $libs_softmmu" +libs_tools="$glib_libs $libs_tools" +else +echo "glib-2.0 required to compile QEMU" +exit 1 fi ##
[Qemu-devel] [PULL] VirtFS coroutine changes
The following changes since commit 23ddf2bb1e4bfe2b72a726fe5e828807b65941ad: Fix forcing multicast msgs to loopback on OpenBSD. (2011-08-07 11:06:43 +) are available in the git repository at: git://repo.or.cz/qemu/v9fs.git for-upstream-1 Aneesh Kumar K.V (32): hw/9pfs: Add yield support for readdir related coroutines hw/9pfs: Update v9fs_readdir to use coroutines hw/9pfs: Add yield support to statfs coroutine hw/9pfs: Update v9fs_statfs to use coroutines hw/9pfs: Add yield support to lstat coroutine hw/9pfs: Update v9fs_getattr to use coroutines hw/9pfs: Add yield support to setattr related coroutines hw/9pfs: Update v9fs_setattr to use coroutines hw/9pfs: Add yield support to xattr related coroutine hw/9pfs: Update v9fs_xattrwalk to coroutines hw/9pfs: Update v9fs_xattrcreate to use coroutines hw/9pfs: Add yield support to mknod coroutine hw/9pfs: Update v9fs_mknod to use coroutines hw/9pfs: Add yeild support to rename coroutine hw/9pfs: Update vfs_rename to use coroutines hw/9pfs: Add yeild support for fstat coroutine hw/9pfs: Update v9fs_lock to use coroutines hw/9pfs: Update v9fs_getlock to use coroutines hw/9pfs: Add yield support for open and opendir coroutine hw/9pfs: Update v9fs_open to use coroutines hw/9pfs: Update v9fs_stat to use coroutines hw/9pfs: Update v9fs_walk to use coroutines hw/9pfs: Add yeild support for clunk related coroutine hw/9pfs: Update v9fs_clunk to use coroutines hw/9pfs: Add yield support for fsync coroutine hw/9pfs: Update v9fs_fsync to use coroutines hw/9pfs: Add yield support for pwritev coroutine hw/9pfs: Update v9fs_write to use coroutines hw/9pfs: Update v9fs_wstat to use coroutines hw/9pfs: Update v9fs_attach to use coroutines hw/9pfs: Add yield support for preadv coroutine hw/9pfs: Update v9fs_read to use coroutines Harsh Prateek Bora (1): use readdir_r instead of readdir for reentrancy Stefan Hajnoczi (1): coroutine: add gthread dependency Venkateswararao Jujjuri (JV) (20): [virtio-9p] Add infrastructure to support glib threads and coroutines. [virtio-9p] Change all pdu handlers to coroutines. [virtio-9p] Remove post functions for v9fs_readlink. [virtio-9p] clean up v9fs_readlink. [virtio-9p] coroutines for readlink [virtio-9p] Remove post functions for v9fs_mkdir. [virtio-9p] clean up v9fs_mkdir. [virtio-9p] coroutine and threading for mkdir [virtio-9p] Remove post functions for v9fs_remove [virtio-9p] clean up v9fs_remove. [virtio-9p] coroutine and threading for remove/unlink [virtio-9p] Remove post functions for v9fs_lcreate [virtio-9p] clean up v9fs_lcreate [virtio-9p] coroutine and threading for open2 [virtio-9p] Remove post functions for v9fs_create [virtio-9p] clean up v9fs_create Rearrange the code [virtio-9p] Remove post functions for v9fs_symlink [virtio-9p] clean up v9fs_symlink [virtio-9p] coroutine and threading for v9fs_do_symlink [virtio-9p] coroutine and threading for v9fs_do_link Makefile.objs |3 + configure | 18 +- fsdev/file-op-9p.h |2 +- hw/9pfs/codir.c| 117 ++ hw/9pfs/cofile.c | 163 +++ hw/9pfs/cofs.c | 191 +++ hw/9pfs/coxattr.c | 84 ++ hw/9pfs/virtio-9p-coth.c | 102 ++ hw/9pfs/virtio-9p-coth.h | 96 ++ hw/9pfs/virtio-9p-device.c |7 +- hw/9pfs/virtio-9p-local.c |7 +- hw/9pfs/virtio-9p.c| 3052 hw/9pfs/virtio-9p.h| 155 +--- 13 files changed, 1848 insertions(+), 2149 deletions(-) create mode 100644 hw/9pfs/codir.c create mode 100644 hw/9pfs/cofile.c create mode 100644 hw/9pfs/cofs.c create mode 100644 hw/9pfs/coxattr.c create mode 100644 hw/9pfs/virtio-9p-coth.c create mode 100644 hw/9pfs/virtio-9p-coth.h
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
On 08/08/2011 06:24 AM, Isaku Yamahata wrote: This mail is on "Yabusame: Postcopy Live Migration for Qemu/KVM" on which we'll give a talk at KVM-forum. The purpose of this mail is to letting developers know it in advance so that we can get better feedback on its design/implementation approach early before our starting to implement it. Background == * What's is postcopy livemigration It is is yet another live migration mechanism for Qemu/KVM, which implements the migration technique known as "postcopy" or "lazy" migration. Just after the "migrate" command is invoked, the execution host of a VM is instantaneously switched to a destination host. The benefit is, total migration time is shorter because it transfer a page only once. On the other hand precopy may repeat sending same pages again and again because they can be dirtied. The switching time from the source to the destination is several hunderds mili seconds so that it enables quick load balancing. For details, please refer to the papers. We believe this is useful for others so that we'd like to merge this feature into the upstream qemu/kvm. The existing implementation that we have right now is very ad-hoc because it's for academic research. For the upstream merge, we're starting to re-design/implement it and we'd like to get feedback early. Although many improvements/optimizations are possible, we should implement/merge the simple/clean, but extensible as well, one at first and then improve/optimize it later. postcopy livemigration will be introduced as optional feature. The existing precopy livemigration remains as default behavior. * related links: project page http://sites.google.com/site/grivonhome/quick-kvm-migration Enabling Instantaneous Relocation of Virtual Machines with a Lightweight VMM Extension, (proof-of-concept, ad-hoc prototype. not a new design) http://grivon.googlecode.com/svn/pub/docs/ccgrid2010-hirofuchi-paper.pdf http://grivon.googlecode.com/svn/pub/docs/ccgrid2010-hirofuchi-talk.pdf Reactive consolidation of virtual machines enabled by postcopy live migration (advantage for VM consolidation) http://portal.acm.org/citation.cfm?id=1996125 http://www.emn.fr/x-info/ascola/lib/exe/fetch.php?media=internet:vtdc-postcopy.pdf Qemu wiki http://wiki.qemu.org/Features/PostCopyLiveMigration Design/Implementation = The basic idea of postcopy livemigration is to use a sort of distributed shared memory between the migration source and destination. The migration procedure looks like - start migration stop the guest VM on the source and send the machine states except guest RAM to the destination - resume the guest VM on the destination without guest RAM contents - Hook guest access to pages, and pull page contents from the source This continues until all the pages are pulled to the destination The big picture is depicted at http://wiki.qemu.org/File:Postcopy-livemigration.png That's terrific (nice video also)! Orit and myself had the exact same idea too (now we can't patent it..). Advantages: - No down time due to memory copying. - Efficient, reduce needed traffic no need to re-send pages. - Reduce overall RAM consumption of the source and destination as opposed from current live migration (both the source and the destination allocate the memory until the live migration completes). We can free copied memory once the destination guest received it and save RAM. - Increase parallelism for SMP guests we can have multiple virtual CPU handle their demand paging . Less time to hold a global lock, less thread contention. - Virtual machines are using more and more memory resources , for a virtual machine with very large working set doing live migration with reasonable down time is impossible today. Disadvantageous: - During the live migration the guest will run slower than in today's live migration. We need to remember that even today guests suffer from performance penalty on the source during the COW stage (memory copy). - Failure of the source or destination or the network will cause us to lose the running virtual machine. Those failures are very rare. In case there is shared storage we can store a copy of the memory there , that can be recovered in case of such failure . Overall, it looks like a better approach for the vast majority of cases. Hope it will get merged to kvm and become the default way. There are several design points. - who takes care of pulling page contents. an independent daemon vs a thread in qemu The daemon approach is preferable because an independent daemon would easy for debug postcopy memory mechanism without qemu. If required, it wouldn't be difficult to convert a daemon into a thread in qemu - connection between the source and the destinati
Re: [Qemu-devel] build failure with coroutine-gthread
On Mon, Aug 8, 2011 at 10:01 AM, Aneesh Kumar K.V wrote: > > > LINK qemu-ga > coroutine-gthread.o: In function `coroutine_init': > /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined > reference to `g_thread_init' > collect2: ld returned 1 exit status > > The below patch fix the failure. I also added the patch in the VirtFS > pull request. It appears coroutine-core v7 was merged instead of v8. The differences are minor: * Bisectability: introduce gthread implementation before ucontext/fibers * Depend on gthread, not just glib, for multithreaded programming Here is the patchwork link to the gthread dependency patch in case a committer wants to merge it immediately: http://patchwork.ozlabs.org/patch/106810/ Stefan
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
On Mon, Aug 8, 2011 at 4:24 AM, Isaku Yamahata wrote: > This mail is on "Yabusame: Postcopy Live Migration for Qemu/KVM" > on which we'll give a talk at KVM-forum. I'm curious if this approach is compatible with asynchronous page faults? The idea there was to tell the guest about a page fault so it can continue to do useful work in the meantime (if the fault was in guest userspace). Stefan
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
On 08/08/2011 12:20, Dor Laor wrote: On 08/08/2011 06:24 AM, Isaku Yamahata wrote: This mail is on "Yabusame: Postcopy Live Migration for Qemu/KVM" on which we'll give a talk at KVM-forum. The purpose of this mail is to letting developers know it in advance so that we can get better feedback on its design/implementation approach early before our starting to implement it. Background == * What's is postcopy livemigration It is is yet another live migration mechanism for Qemu/KVM, which implements the migration technique known as "postcopy" or "lazy" migration. Just after the "migrate" command is invoked, the execution host of a VM is instantaneously switched to a destination host. The benefit is, total migration time is shorter because it transfer a page only once. On the other hand precopy may repeat sending same pages again and again because they can be dirtied. The switching time from the source to the destination is several hunderds mili seconds so that it enables quick load balancing. For details, please refer to the papers. We believe this is useful for others so that we'd like to merge this feature into the upstream qemu/kvm. The existing implementation that we have right now is very ad-hoc because it's for academic research. For the upstream merge, we're starting to re-design/implement it and we'd like to get feedback early. Although many improvements/optimizations are possible, we should implement/merge the simple/clean, but extensible as well, one at first and then improve/optimize it later. postcopy livemigration will be introduced as optional feature. The existing precopy livemigration remains as default behavior. * related links: project page http://sites.google.com/site/grivonhome/quick-kvm-migration Enabling Instantaneous Relocation of Virtual Machines with a Lightweight VMM Extension, (proof-of-concept, ad-hoc prototype. not a new design) http://grivon.googlecode.com/svn/pub/docs/ccgrid2010-hirofuchi-paper.pdf http://grivon.googlecode.com/svn/pub/docs/ccgrid2010-hirofuchi-talk.pdf Reactive consolidation of virtual machines enabled by postcopy live migration (advantage for VM consolidation) http://portal.acm.org/citation.cfm?id=1996125 http://www.emn.fr/x-info/ascola/lib/exe/fetch.php?media=internet:vtdc-postcopy.pdf Qemu wiki http://wiki.qemu.org/Features/PostCopyLiveMigration Design/Implementation = The basic idea of postcopy livemigration is to use a sort of distributed shared memory between the migration source and destination. The migration procedure looks like - start migration stop the guest VM on the source and send the machine states except guest RAM to the destination - resume the guest VM on the destination without guest RAM contents - Hook guest access to pages, and pull page contents from the source This continues until all the pages are pulled to the destination The big picture is depicted at http://wiki.qemu.org/File:Postcopy-livemigration.png That's terrific (nice video also)! Orit and myself had the exact same idea too (now we can't patent it..). Advantages: - No down time due to memory copying. - Efficient, reduce needed traffic no need to re-send pages. - Reduce overall RAM consumption of the source and destination as opposed from current live migration (both the source and the destination allocate the memory until the live migration completes). We can free copied memory once the destination guest received it and save RAM. - Increase parallelism for SMP guests we can have multiple virtual CPU handle their demand paging . Less time to hold a global lock, less thread contention. - Virtual machines are using more and more memory resources , for a virtual machine with very large working set doing live migration with reasonable down time is impossible today. Disadvantageous: - During the live migration the guest will run slower than in today's live migration. We need to remember that even today guests suffer from performance penalty on the source during the COW stage (memory copy). - Failure of the source or destination or the network will cause us to lose the running virtual machine. Those failures are very rare. I highly doubt that's acceptable in enterprise deployments. In case there is shared storage we can store a copy of the memory there , that can be recovered in case of such failure . Overall, it looks like a better approach for the vast majority of cases. Hope it will get merged to kvm and become the default way. There are several design points. - who takes care of pulling page contents. an independent daemon vs a thread in qemu The daemon approach is preferable because an independent daemon would easy for debug postcopy memory mechanism without qemu. If required, it wouldn'
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
On Mon, Aug 08, 2011 at 10:38:35AM +0100, Stefan Hajnoczi wrote: > On Mon, Aug 8, 2011 at 4:24 AM, Isaku Yamahata wrote: > > This mail is on "Yabusame: Postcopy Live Migration for Qemu/KVM" > > on which we'll give a talk at KVM-forum. > > I'm curious if this approach is compatible with asynchronous page > faults? The idea there was to tell the guest about a page fault so it > can continue to do useful work in the meantime (if the fault was in > guest userspace). Yes. It's quite possible to inject async page fault into the guest when the faulted page isn't available on the destination. At the same time the page will be requested to the source of the migration. I think it's not so difficult. -- yamahata
Re: [Qemu-devel] [PULL 0/7] ARM patch queue (for master)
Ping? On 27 July 2011 14:26, Peter Maydell wrote: > This is a pull request for various outstanding ARM related > patches; they've been on the list for a week or so. > > Some of these are bug fixes which I want to get into 0.15; I'm > going to do a parallel pullreq for the 0.15 patches. > > Thanks > -- PMM > > > The following changes since commit c886edfb851c0c590d4e77f058f2ec8ed95ad1b5: > > Let users select their pythons (2011-07-25 16:50:12 +) > > are available in the git repository at: > git://git.linaro.org/people/pmaydell/qemu-arm.git for-upstream > > Jamie Iles (2): > target-arm: make VMSAv7 remapping and AP dependent on V6K > target-arm: support for ARM1176JZF-s cores > > Peter Maydell (5): > target-arm: Mark 1136r1 as a v6K core > target-arm: Support v6 barriers in linux-user mode > target-arm: Handle UNDEF and UNPREDICTABLE cases for VLDM, VSTM > target-arm: UNDEF on a VCVTT/VCVTB UNPREDICTABLE to avoid TCG assert > target-arm: Don't print debug messages for various UNDEF cases > > target-arm/cpu.h | 2 + > target-arm/helper.c | 47 ++- > target-arm/translate.c | 114 +++ > 3 files changed, 121 insertions(+), 42 deletions(-)
Re: [Qemu-devel] [PATCH v3 01/39] virtio-pci: get config on init
On Fri, Aug 05, 2011 at 08:52:25AM -0500, Anthony Liguori wrote: > On 08/04/2011 08:05 AM, Avi Kivity wrote: > >From: "Michael S. Tsirkin" > > > >We originally did get config on map, so that > >following write accesses are done on an updated config. > >New memory API doesn't give us a callback > >on map, and arguably, devices don't know when > >cpu really can access there. So updating on > >init seems cleaner. > > > >Signed-off-by: Michael S. Tsirkin > >Signed-off-by: Avi Kivity > >--- > > hw/virtio-pci.c |7 --- > > 1 files changed, 4 insertions(+), 3 deletions(-) > > > >diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > >index d685243..ca1f12f 100644 > >--- a/hw/virtio-pci.c > >+++ b/hw/virtio-pci.c > >@@ -506,9 +506,6 @@ static void virtio_map(PCIDevice *pci_dev, int > >region_num, > > register_ioport_read(addr, config_len, 1, virtio_pci_config_readb, > > proxy); > > register_ioport_read(addr, config_len, 2, virtio_pci_config_readw, > > proxy); > > register_ioport_read(addr, config_len, 4, virtio_pci_config_readl, > > proxy); > >- > >-if (vdev->config_len) > >-vdev->get_config(vdev, vdev->config); > > } > > > > static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, > >@@ -689,6 +686,10 @@ void virtio_init_pci(VirtIOPCIProxy *proxy, > >VirtIODevice *vdev) > > proxy->host_features |= 0x1<< VIRTIO_F_NOTIFY_ON_EMPTY; > > proxy->host_features |= 0x1<< VIRTIO_F_BAD_FEATURE; > > proxy->host_features = vdev->get_features(vdev, proxy->host_features); > >+ > >+if (vdev->config_len) { > >+vdev->get_config(vdev, vdev->config); > >+} > > Thinking more closely, I don't think this right. > > Updating on map ensured that the config was refreshed after each > time the bar was mapped. In the very least, the config needs to be > refreshed during reset because the guest may write to the guest > space which should get cleared after reset. > > Regards, > > Anthony Liguori Not sure I understand. Which register, for example, do you have in mind? Could you clarify please? > > } > > > > static int virtio_blk_init_pci(PCIDevice *pci_dev)
Re: [Qemu-devel] Build broken
On Mon, 8 Aug 2011, Kevin Wolf wrote: > Am 05.08.2011 18:49, schrieb malc: > > On Fri, 5 Aug 2011, Stefan Hajnoczi wrote: > > [..snip..] > > Feel free to push it. (Original code was by Anthony, then heavily > modified by me, and after that modified again by Stefan) > > Acked-by: Kevin Wolf Pushed. -- mailto:av1...@comtv.ru
Re: [Qemu-devel] build failure with coroutine-gthread
On Mon, 8 Aug 2011 10:29:04 +0100, Stefan Hajnoczi wrote: > On Mon, Aug 8, 2011 at 10:01 AM, Aneesh Kumar K.V > wrote: > > > > > > LINK qemu-ga > > coroutine-gthread.o: In function `coroutine_init': > > /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: > > undefined reference to `g_thread_init' > > collect2: ld returned 1 exit status > > > > The below patch fix the failure. I also added the patch in the VirtFS > > pull request. > > It appears coroutine-core v7 was merged instead of v8. The > differences are minor: > * Bisectability: introduce gthread implementation before ucontext/fibers > * Depend on gthread, not just glib, for multithreaded programming > > Here is the patchwork link to the gthread dependency patch in case a > committer wants to merge it immediately: > http://patchwork.ozlabs.org/patch/106810/ > I added a version that applies to master in the pull request http://article.gmane.org/gmane.comp.emulators.qemu/112648 Message-id:"87bow09sw1@skywalker.in.ibm.com" http://repo.or.cz/w/qemu/v9fs.git/commit/4b76a481ee28166d5f415ef97833c624f4fc0792 -aneesh
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
> >* What's is postcopy livemigration > >It is is yet another live migration mechanism for Qemu/KVM, which > >implements the migration technique known as "postcopy" or "lazy" > >migration. Just after the "migrate" command is invoked, the execution > >host of a VM is instantaneously switched to a destination host. Sounds like a cool idea. > >The benefit is, total migration time is shorter because it transfer > >a page only once. On the other hand precopy may repeat sending same pages > >again and again because they can be dirtied. > >The switching time from the source to the destination is several > >hunderds mili seconds so that it enables quick load balancing. > >For details, please refer to the papers. While these are the obvious benefits, the possible downside (that, as always, depends on the workload) is the amount of time that the guest workload runs more slowly than usual, waiting for pages it needs to continue. There are a whole spectrum between the guest pausing completely (which would solve all the problems of migration, but is often considered unacceptible) and running at full-speed. Is it acceptable that the guest runs at 90% speed during the migration? 50%? 10%? I guess we could have nothing to lose from having both options, and choosing the most appropriate technique for each guest! > That's terrific (nice video also)! > Orit and myself had the exact same idea too (now we can't patent it..). I think new implementation is not the only reason why you cannot patent this idea :-) Demand-paged migration has actually been discussed (and done) for nearly a quarter of a century (!) in the area of *process* migration. The first use I'm aware of was in CMU's Accent 1987 - see [1]. Another paper, [2], written in 1991, discusses how process migration is done in UCB's Sprite operating system, and evaluates the various alternatives common at the time (20 years ago), including what it calls "lazy copying" is more-or-less the same thing as "post copy". Mosix (a project which, in some sense, is still alive to day) also used some sort of cross between pre-copying (of dirty pages) and copying on-demand of clean pages (from their backing store on the source machine). References [1] "Attacking the Process Migration Bottleneck" http://www.nd.edu/~dthain/courses/cse598z/fall2004/papers/accent.pdf [2] "Transparent Process Migration: Design Alternatives and the Sprite Implementation" http://nd.edu/~dthain/courses/cse598z/fall2004/papers/sprite-migration.pdf > Advantages: > - Virtual machines are using more and more memory resources , > for a virtual machine with very large working set doing live > migration with reasonable down time is impossible today. If a guest actually constantly uses (working set) most of its allocated memory, it will basically be unable to do any significant amount of work on the destination VM until this large working set is transfered to the destination. So in this scenario, "post copying" doesn't give any significant advantages over plain-old "pause guest and send it to the destination". Or am I missing something? > Disadvantageous: > - During the live migration the guest will run slower than in > today's live migration. We need to remember that even today > guests suffer from performance penalty on the source during the > COW stage (memory copy). I wonder if something like asynchronous page faults can help somewhat with multi-process guest workloads (and modified (PV) guest OS). > - Failure of the source or destination or the network will cause > us to lose the running virtual machine. Those failures are very > rare. How is this different from a VM running on a single machine that fails? Just that the small probability of failure (roughly) doubles for the relatively-short duration of the transfer? -- Nadav Har'El| Monday, Aug 8 2011, 8 Av 5771 n...@math.technion.ac.il |- Phone +972-523-790466, ICQ 13349191 |If glory comes after death, I'm not in a http://nadav.harel.org.il |hurry. (Latin proverb)
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On 08/04/2011 01:17 PM, Jan Kiszka wrote: > > Why "QemuState"? In general, "qemu" can be inferred from the fact that > we're in qemu.git. Suggest "RunState". > > Second, these states can coexist. A user may pause the VM > simultaneously with the watchdog firing or entering premigrate state. > In fact, with multiple monitors, each monitor can pause and resume the > vm independently. > > So I think we should keep a reference count instead of just a start/stop > state. Perhaps > > vm_stop(QemuState s) > { > ++stopcount[s]; > } > > vm_is_stopped() > { > for (s in states) >if (stopcount[s]) >return true; > return false; > } I don't think this makes sense nor is user-friendly. If one command channel suspends the machine, others have the chance to subscribe for that event. It's inherently racy. Maintaining a suspension counter would mean you also need a channel to query its value. Why? IMHO, there is also no use for defining stopped orthogonally to premigrate and other states that imply that the machine is stopped. Basically they mean "stopped for/because of X". We just need to avoid that you can enter plain stopped state from them by issuing the corresponding monitor command. The other way around might be possible, though, if there are race windows. I'm worried about the following race: stop (qemu stopped for internal reason) stop comment processed resume The (qemu stopped for internal reason) part is lost. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH] posix-aio-compat: fix latency issues
In certain circumstances, posix-aio-compat can incur a lot of latency: - threads are created by vcpu threads, so if vcpu affinity is set, aio threads inherit vcpu affinity. This can cause many aio threads to compete for one cpu. - we can create up to max_threads (64) aio threads in one go; since a pthread_create can take around 30μs, we have up to 2ms of cpu time under a global lock. Fix by: - moving thread creation to the main thread, so we inherit the main thread's affinity instead of the vcpu thread's affinity. - if a thread is currently being created, and we need to create yet another thread, let thread being born create the new thread, reducing the amount of time we spend under the main thread. - drop the local lock while creating a thread (we may still hold the global mutex, though) Note this doesn't eliminate latency completely; scheduler artifacts or lack of host cpu resources can still cause it. We may want pre-allocated threads when this cannot be tolerated. Thanks to Uli Obergfell of Red Hat for his excellent analysis and suggestions. Signed-off-by: Avi Kivity --- posix-aio-compat.c | 48 ++-- 1 files changed, 46 insertions(+), 2 deletions(-) diff --git a/posix-aio-compat.c b/posix-aio-compat.c index 8dc00cb..aa30673 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -30,6 +30,7 @@ #include "block/raw-posix-aio.h" +static void do_spawn_thread(void); struct qemu_paiocb { BlockDriverAIOCB common; @@ -64,6 +65,9 @@ static pthread_attr_t attr; static int max_threads = 64; static int cur_threads = 0; static int idle_threads = 0; +static int new_threads = 0; /* backlog of threads we need to create */ +static int pending_threads = 0; /* threads created but not running yet */ +static QEMUBH *new_thread_bh; static QTAILQ_HEAD(, qemu_paiocb) request_list; #ifdef CONFIG_PREADV @@ -311,6 +315,13 @@ static void *aio_thread(void *unused) pid = getpid(); +mutex_lock(&lock); +if (new_threads) { +do_spawn_thread(); +} +pending_threads--; +mutex_unlock(&lock); + while (1) { struct qemu_paiocb *aiocb; ssize_t ret = 0; @@ -381,11 +392,18 @@ static void *aio_thread(void *unused) return NULL; } -static void spawn_thread(void) +static void do_spawn_thread(void) { sigset_t set, oldset; -cur_threads++; +if (!new_threads) { +return; +} + +new_threads--; +pending_threads++; + +mutex_unlock(&lock); /* block all signals */ if (sigfillset(&set)) die("sigfillset"); @@ -394,6 +412,31 @@ static void spawn_thread(void) thread_create(&thread_id, &attr, aio_thread, NULL); if (sigprocmask(SIG_SETMASK, &oldset, NULL)) die("sigprocmask restore"); + +mutex_lock(&lock); +} + +static void spawn_thread_bh_fn(void *opaque) +{ +mutex_lock(&lock); +do_spawn_thread(); +mutex_unlock(&lock); +} + +static void spawn_thread(void) +{ +cur_threads++; +new_threads++; +/* If there are threads being created, they will spawn new workers, so + * we don't spend time creating many threads in a loop holding a mutex or + * starving the current vcpu. + * + * If there are no idle threads, ask the main thread to create one, so we + * inherit the correct affinity instead of the vcpu affinity. + */ +if (!pending_threads) { +qemu_bh_schedule(new_thread_bh); +} } static void qemu_paio_submit(struct qemu_paiocb *aiocb) @@ -665,6 +708,7 @@ int paio_init(void) die2(ret, "pthread_attr_setdetachstate"); QTAILQ_INIT(&request_list); +new_thread_bh = qemu_bh_new(spawn_thread_bh_fn, NULL); posix_aio_state = s; return 0; -- 1.7.5.3
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
On 08/08/2011 01:59 PM, Nadav Har'El wrote: * What's is postcopy livemigration It is is yet another live migration mechanism for Qemu/KVM, which implements the migration technique known as "postcopy" or "lazy" migration. Just after the "migrate" command is invoked, the execution host of a VM is instantaneously switched to a destination host. Sounds like a cool idea. The benefit is, total migration time is shorter because it transfer a page only once. On the other hand precopy may repeat sending same pages again and again because they can be dirtied. The switching time from the source to the destination is several hunderds mili seconds so that it enables quick load balancing. For details, please refer to the papers. While these are the obvious benefits, the possible downside (that, as always, depends on the workload) is the amount of time that the guest workload runs more slowly than usual, waiting for pages it needs to continue. There are a whole spectrum between the guest pausing completely (which would solve all the problems of migration, but is often considered unacceptible) and running at full-speed. Is it acceptable that the guest runs at 90% speed during the migration? 50%? 10%? I guess we could have nothing to lose from having both options, and choosing the most appropriate technique for each guest! +1 That's terrific (nice video also)! Orit and myself had the exact same idea too (now we can't patent it..). I think new implementation is not the only reason why you cannot patent this idea :-) Demand-paged migration has actually been discussed (and done) for nearly a quarter of a century (!) in the area of *process* migration. The first use I'm aware of was in CMU's Accent 1987 - see [1]. Another paper, [2], written in 1991, discusses how process migration is done in UCB's Sprite operating system, and evaluates the various alternatives common at the time (20 years ago), including what it calls "lazy copying" is more-or-less the same thing as "post copy". Mosix (a project which, in some sense, is still alive to day) also used some sort of cross between pre-copying (of dirty pages) and copying on-demand of clean pages (from their backing store on the source machine). References [1] "Attacking the Process Migration Bottleneck" http://www.nd.edu/~dthain/courses/cse598z/fall2004/papers/accent.pdf w/o reading the internals, patents enable you to implement an existing idea on a new field. Anyway, there won't be no patent in this case. Still let's have the kvm innovation merged. [2] "Transparent Process Migration: Design Alternatives and the Sprite Implementation" http://nd.edu/~dthain/courses/cse598z/fall2004/papers/sprite-migration.pdf Advantages: - Virtual machines are using more and more memory resources , for a virtual machine with very large working set doing live migration with reasonable down time is impossible today. If a guest actually constantly uses (working set) most of its allocated memory, it will basically be unable to do any significant amount of work on the destination VM until this large working set is transfered to the destination. So in this scenario, "post copying" doesn't give any significant advantages over plain-old "pause guest and send it to the destination". Or am I missing something? There is one key advantage in this scheme/use case - if you have a guest with a very large working set, you'll need a very large downtime in order to migrate it with today's algorithm. With post copy (aka streaming/demand paging), the guest won't have any downtime but will run slower than expected. There are guests today that is impractical to really live migrate them. btw: Even today, marking pages RO also carries some performance penalty. Disadvantageous: - During the live migration the guest will run slower than in today's live migration. We need to remember that even today guests suffer from performance penalty on the source during the COW stage (memory copy). I wonder if something like asynchronous page faults can help somewhat with multi-process guest workloads (and modified (PV) guest OS). They should come in to play for some extent. Note that only newer Linux guest will enjoy of them. - Failure of the source or destination or the network will cause us to lose the running virtual machine. Those failures are very rare. How is this different from a VM running on a single machine that fails? Just that the small probability of failure (roughly) doubles for the relatively-short duration of the transfer? Exactly my point, this is not a major disadvantage because of this low probability.
[Qemu-devel] [PATCH] qemu-img: Use qemu_blockalign
Now that you can use cache=none for the output file in qemu-img, we should properly align our buffers so that raw-posix doesn't have to use its (smaller) bounce buffer. Signed-off-by: Kevin Wolf --- qemu-img.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 5ee207b..27e07e9 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -814,7 +814,7 @@ static int img_convert(int argc, char **argv) bs_i = 0; bs_offset = 0; bdrv_get_geometry(bs[0], &bs_sectors); -buf = qemu_malloc(IO_BUF_SIZE); +buf = qemu_blockalign(out_bs, IO_BUF_SIZE); if (compress) { ret = bdrv_get_info(out_bs, &bdi); @@ -986,7 +986,7 @@ out: qemu_progress_end(); free_option_parameters(create_options); free_option_parameters(param); -qemu_free(buf); +qemu_vfree(buf); if (out_bs) { bdrv_delete(out_bs); } @@ -1353,8 +1353,8 @@ static int img_rebase(int argc, char **argv) uint8_t * buf_new; float local_progress; -buf_old = qemu_malloc(IO_BUF_SIZE); -buf_new = qemu_malloc(IO_BUF_SIZE); +buf_old = qemu_blockalign(bs, IO_BUF_SIZE); +buf_new = qemu_blockalign(bs, IO_BUF_SIZE); bdrv_get_geometry(bs, &num_sectors); @@ -1410,8 +1410,8 @@ static int img_rebase(int argc, char **argv) qemu_progress_print(local_progress, 100); } -qemu_free(buf_old); -qemu_free(buf_new); +qemu_vfree(buf_old); +qemu_vfree(buf_new); } /* -- 1.7.6
Re: [Qemu-devel] [PATCH] qcow2: fix typo in documentation for qcow2_get_cluster_offset()
Am 08.08.2011 01:47, schrieb Devin Nakamura: > Documentation states the num is measured in clusters, but its > actually measured in sectors > > Signed-off-by: Devin Nakamura Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] posix-aio-compat: fix latency issues
On 08/08/2011 06:37 AM, Avi Kivity wrote: In certain circumstances, posix-aio-compat can incur a lot of latency: - threads are created by vcpu threads, so if vcpu affinity is set, aio threads inherit vcpu affinity. This can cause many aio threads to compete for one cpu. - we can create up to max_threads (64) aio threads in one go; since a pthread_create can take around 30μs, we have up to 2ms of cpu time under a global lock. Fix by: - moving thread creation to the main thread, so we inherit the main thread's affinity instead of the vcpu thread's affinity. - if a thread is currently being created, and we need to create yet another thread, let thread being born create the new thread, reducing the amount of time we spend under the main thread. - drop the local lock while creating a thread (we may still hold the global mutex, though) Note this doesn't eliminate latency completely; scheduler artifacts or lack of host cpu resources can still cause it. We may want pre-allocated threads when this cannot be tolerated. Thanks to Uli Obergfell of Red Hat for his excellent analysis and suggestions. Do you have a scenario where you can measure the benefits of this change? The idle time in the thread pool is rather large, it surprises me that it'd be an issue in practice. Regards, Anthony Liguori Signed-off-by: Avi Kivity --- posix-aio-compat.c | 48 ++-- 1 files changed, 46 insertions(+), 2 deletions(-) diff --git a/posix-aio-compat.c b/posix-aio-compat.c index 8dc00cb..aa30673 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -30,6 +30,7 @@ #include "block/raw-posix-aio.h" +static void do_spawn_thread(void); struct qemu_paiocb { BlockDriverAIOCB common; @@ -64,6 +65,9 @@ static pthread_attr_t attr; static int max_threads = 64; static int cur_threads = 0; static int idle_threads = 0; +static int new_threads = 0; /* backlog of threads we need to create */ +static int pending_threads = 0; /* threads created but not running yet */ +static QEMUBH *new_thread_bh; static QTAILQ_HEAD(, qemu_paiocb) request_list; #ifdef CONFIG_PREADV @@ -311,6 +315,13 @@ static void *aio_thread(void *unused) pid = getpid(); +mutex_lock(&lock); +if (new_threads) { +do_spawn_thread(); +} +pending_threads--; +mutex_unlock(&lock); + while (1) { struct qemu_paiocb *aiocb; ssize_t ret = 0; @@ -381,11 +392,18 @@ static void *aio_thread(void *unused) return NULL; } -static void spawn_thread(void) +static void do_spawn_thread(void) { sigset_t set, oldset; -cur_threads++; +if (!new_threads) { +return; +} + +new_threads--; +pending_threads++; + +mutex_unlock(&lock); /* block all signals */ if (sigfillset(&set)) die("sigfillset"); @@ -394,6 +412,31 @@ static void spawn_thread(void) thread_create(&thread_id,&attr, aio_thread, NULL); if (sigprocmask(SIG_SETMASK,&oldset, NULL)) die("sigprocmask restore"); + +mutex_lock(&lock); +} + +static void spawn_thread_bh_fn(void *opaque) +{ +mutex_lock(&lock); +do_spawn_thread(); +mutex_unlock(&lock); +} + +static void spawn_thread(void) +{ +cur_threads++; +new_threads++; +/* If there are threads being created, they will spawn new workers, so + * we don't spend time creating many threads in a loop holding a mutex or + * starving the current vcpu. + * + * If there are no idle threads, ask the main thread to create one, so we + * inherit the correct affinity instead of the vcpu affinity. + */ +if (!pending_threads) { +qemu_bh_schedule(new_thread_bh); +} } static void qemu_paio_submit(struct qemu_paiocb *aiocb) @@ -665,6 +708,7 @@ int paio_init(void) die2(ret, "pthread_attr_setdetachstate"); QTAILQ_INIT(&request_list); +new_thread_bh = qemu_bh_new(spawn_thread_bh_fn, NULL); posix_aio_state = s; return 0;
Re: [Qemu-devel] [PULL] VirtFS coroutine changes
On 08/08/2011 04:03 AM, Aneesh Kumar K.V wrote: The following changes since commit 23ddf2bb1e4bfe2b72a726fe5e828807b65941ad: Fix forcing multicast msgs to loopback on OpenBSD. (2011-08-07 11:06:43 +) This need to be sent to the list first as patches for review before you can do a pull request. Regards, Anthony Liguori
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
On 08/08/2011 06:24 AM, Isaku Yamahata wrote: This mail is on "Yabusame: Postcopy Live Migration for Qemu/KVM" on which we'll give a talk at KVM-forum. The purpose of this mail is to letting developers know it in advance so that we can get better feedback on its design/implementation approach early before our starting to implement it. Interesting; what is the impact of increased latency on memory reads? There are several design points. - who takes care of pulling page contents. an independent daemon vs a thread in qemu The daemon approach is preferable because an independent daemon would easy for debug postcopy memory mechanism without qemu. If required, it wouldn't be difficult to convert a daemon into a thread in qemu Isn't this equivalent to touching each page in sequence? Care must be taken that we don't post too many requests, or it could affect the latency of synchronous accesses by the guest. - connection between the source and the destination The connection for live migration can be re-used after sending machine state. - transfer protocol The existing protocol that exists today can be extended. - hooking guest RAM access Introduce a character device to handle page fault. When page fault occurs, it queues page request up to user space daemon at the destination. And the daemon pulls page contents from the source and serves it into the character device. Then the page fault is resovlved. This doesn't play well with host swapping, transparent hugepages, or ksm, does it? I see you note this later on. * More on hooking guest RAM access There are several candidate for the implementation. Our preference is character device approach. - inserting hooks into everywhere in qemu/kvm This is impractical - backing store for guest ram a block device or a file can be used to back guest RAM. Thus hook the guest ram access. pros - new device driver isn't needed. cons - future improvement would be difficult - some KVM host feature(KSM, THP) wouldn't work - character device qemu mmap() the dedicated character device, and then hook page fault. pros - straght forward approach - future improvement would be easy cons - new driver is needed - some KVM host feature(KSM, THP) wouldn't work They checks if a given VMA is anonymous. This can be fixed. - swap device When creating guest, it is set up as if all the guest RAM is swapped out to a dedicated swap device, which may be nbd disk (or some kind of user space block device, BUSE?). When the VM tries to access memory, swap-in is triggered and IO to the swap device is issued. Then the IO to swap is routed to the daemon in user space with nbd protocol (or BUSE, AOE, iSCSI...). The daemon pulls pages from the migration source and services the IO request. pros - After the page transfer is complete, everything is same as normal case. - no new device driver isn't needed cons - future improvement would be difficult - administration: setting up nbd, swap device Using a swap device would be my preference. We'd still be using anonymous memory so thp/ksm/ordinary swap still work. It would need to be a special kind of swap device since we only want to swap in, and never out, to that device. We'd also need a special way of telling the kernel that memory comes from that device. In that it's similar your second option. Maybe we should use a backing file (using nbd) and have a madvise() call that converts the vma to anonymous memory once the migration is finished. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] posix-aio-compat: fix latency issues
On 08/08/2011 03:34 PM, Anthony Liguori wrote: On 08/08/2011 06:37 AM, Avi Kivity wrote: In certain circumstances, posix-aio-compat can incur a lot of latency: - threads are created by vcpu threads, so if vcpu affinity is set, aio threads inherit vcpu affinity. This can cause many aio threads to compete for one cpu. - we can create up to max_threads (64) aio threads in one go; since a pthread_create can take around 30μs, we have up to 2ms of cpu time under a global lock. Fix by: - moving thread creation to the main thread, so we inherit the main thread's affinity instead of the vcpu thread's affinity. - if a thread is currently being created, and we need to create yet another thread, let thread being born create the new thread, reducing the amount of time we spend under the main thread. - drop the local lock while creating a thread (we may still hold the global mutex, though) Note this doesn't eliminate latency completely; scheduler artifacts or lack of host cpu resources can still cause it. We may want pre-allocated threads when this cannot be tolerated. Thanks to Uli Obergfell of Red Hat for his excellent analysis and suggestions. Do you have a scenario where you can measure the benefits of this change? It's a customer scenario, so I can't share it. Not that I know exactly what happened there in terms of workload. The idle time in the thread pool is rather large, it surprises me that it'd be an issue in practice. Just starting up a virtio guest will fill the queue with > max_threads requests, and if the vcpu is pinned, all 64 thread creations and executions will have to run on the same cpu, and will likely preempt the vcpu since it's classified as a "cpu hog" by some schedulers. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
On 08/08/2011 04:20 AM, Dor Laor wrote: On 08/08/2011 06:24 AM, Isaku Yamahata wrote: This mail is on "Yabusame: Postcopy Live Migration for Qemu/KVM" on which we'll give a talk at KVM-forum. The purpose of this mail is to letting developers know it in advance so that we can get better feedback on its design/implementation approach early before our starting to implement it. Background == * What's is postcopy livemigration It is is yet another live migration mechanism for Qemu/KVM, which implements the migration technique known as "postcopy" or "lazy" migration. Just after the "migrate" command is invoked, the execution host of a VM is instantaneously switched to a destination host. The benefit is, total migration time is shorter because it transfer a page only once. On the other hand precopy may repeat sending same pages again and again because they can be dirtied. The switching time from the source to the destination is several hunderds mili seconds so that it enables quick load balancing. For details, please refer to the papers. We believe this is useful for others so that we'd like to merge this feature into the upstream qemu/kvm. The existing implementation that we have right now is very ad-hoc because it's for academic research. For the upstream merge, we're starting to re-design/implement it and we'd like to get feedback early. Although many improvements/optimizations are possible, we should implement/merge the simple/clean, but extensible as well, one at first and then improve/optimize it later. postcopy livemigration will be introduced as optional feature. The existing precopy livemigration remains as default behavior. * related links: project page http://sites.google.com/site/grivonhome/quick-kvm-migration Enabling Instantaneous Relocation of Virtual Machines with a Lightweight VMM Extension, (proof-of-concept, ad-hoc prototype. not a new design) http://grivon.googlecode.com/svn/pub/docs/ccgrid2010-hirofuchi-paper.pdf http://grivon.googlecode.com/svn/pub/docs/ccgrid2010-hirofuchi-talk.pdf Reactive consolidation of virtual machines enabled by postcopy live migration (advantage for VM consolidation) http://portal.acm.org/citation.cfm?id=1996125 http://www.emn.fr/x-info/ascola/lib/exe/fetch.php?media=internet:vtdc-postcopy.pdf Qemu wiki http://wiki.qemu.org/Features/PostCopyLiveMigration Design/Implementation = The basic idea of postcopy livemigration is to use a sort of distributed shared memory between the migration source and destination. The migration procedure looks like - start migration stop the guest VM on the source and send the machine states except guest RAM to the destination - resume the guest VM on the destination without guest RAM contents - Hook guest access to pages, and pull page contents from the source This continues until all the pages are pulled to the destination The big picture is depicted at http://wiki.qemu.org/File:Postcopy-livemigration.png That's terrific (nice video also)! Orit and myself had the exact same idea too (now we can't patent it..). Advantages: - No down time due to memory copying. But non-deterministic down time due to network latency while trying to satisfy a page fault. - Efficient, reduce needed traffic no need to re-send pages. It's not quite that simple. Post-copy needs to introduce a protocol capable of requesting pages. I think in presenting something like this, it's important to collect quite a bit of performance data. I'd suggest doing runs while running jitterd in the guest to attempt to quantify the actual downtime experienced too. http://git.codemonkey.ws/cgit/jitterd.git/ There's a lot of potential in something like this, but it's not obvious to me whether it's a net win. Should make for a very interesting presentation :-) - Reduce overall RAM consumption of the source and destination as opposed from current live migration (both the source and the destination allocate the memory until the live migration completes). We can free copied memory once the destination guest received it and save RAM. - Increase parallelism for SMP guests we can have multiple virtual CPU handle their demand paging . Less time to hold a global lock, less thread contention. - Virtual machines are using more and more memory resources , for a virtual machine with very large working set doing live migration with reasonable down time is impossible today. This is really just a limitation of our implementation. In theory, pre-copy allows you to exert fine grain resource control over the guest which you can use to encourage convergence. Disadvantageous: - During the live migration the guest will run slower than in today's live migration. We need to remember that even today guests suffer from performance penalty on the source during the COW stage (memory copy). - Failure of the source or destination or the network will cause us to lose the running virtual machine. Those failures are very rare. In case
Re: [Qemu-devel] [PATCH v3 01/39] virtio-pci: get config on init
On 08/08/2011 05:36 AM, Michael S. Tsirkin wrote: Thinking more closely, I don't think this right. Updating on map ensured that the config was refreshed after each time the bar was mapped. In the very least, the config needs to be refreshed during reset because the guest may write to the guest space which should get cleared after reset. Regards, Anthony Liguori Not sure I understand. Which register, for example, do you have in mind? Could you clarify please? Actually, you never need to call config_get() AFAICT. It's called in every read/write access. So I think the code you changed is extraneous now. Regards, Anthony Liguori } static int virtio_blk_init_pci(PCIDevice *pci_dev)
Re: [Qemu-devel] [PATCH v3 01/39] virtio-pci: get config on init
On 08/08/2011 03:45 PM, Anthony Liguori wrote: Actually, you never need to call config_get() AFAICT. It's called in every read/write access. So I think the code you changed is extraneous now. Ok; I'll drop this patch and report (and just remove the code in virtio_map()). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] posix-aio-compat: fix latency issues
2011/8/8 Avi Kivity : > In certain circumstances, posix-aio-compat can incur a lot of latency: > - threads are created by vcpu threads, so if vcpu affinity is set, > aio threads inherit vcpu affinity. This can cause many aio threads > to compete for one cpu. > - we can create up to max_threads (64) aio threads in one go; since a > pthread_create can take around 30μs, we have up to 2ms of cpu time > under a global lock. > > Fix by: > - moving thread creation to the main thread, so we inherit the main > thread's affinity instead of the vcpu thread's affinity. > - if a thread is currently being created, and we need to create yet > another thread, let thread being born create the new thread, reducing > the amount of time we spend under the main thread. > - drop the local lock while creating a thread (we may still hold the > global mutex, though) > > Note this doesn't eliminate latency completely; scheduler artifacts or > lack of host cpu resources can still cause it. We may want pre-allocated > threads when this cannot be tolerated. > > Thanks to Uli Obergfell of Red Hat for his excellent analysis and suggestions. > > Signed-off-by: Avi Kivity Why not calling pthread_attr_setaffinity_np (where available) before thread creation or shed_setaffinity at thread start instead of telling another thread to create a thread for us just to get affinity cleared? Regards Frediano > --- > posix-aio-compat.c | 48 ++-- > 1 files changed, 46 insertions(+), 2 deletions(-) > > diff --git a/posix-aio-compat.c b/posix-aio-compat.c > index 8dc00cb..aa30673 100644 > --- a/posix-aio-compat.c > +++ b/posix-aio-compat.c > @@ -30,6 +30,7 @@ > > #include "block/raw-posix-aio.h" > > +static void do_spawn_thread(void); > > struct qemu_paiocb { > BlockDriverAIOCB common; > @@ -64,6 +65,9 @@ static pthread_attr_t attr; > static int max_threads = 64; > static int cur_threads = 0; > static int idle_threads = 0; > +static int new_threads = 0; /* backlog of threads we need to create */ > +static int pending_threads = 0; /* threads created but not running yet */ > +static QEMUBH *new_thread_bh; > static QTAILQ_HEAD(, qemu_paiocb) request_list; > > #ifdef CONFIG_PREADV > @@ -311,6 +315,13 @@ static void *aio_thread(void *unused) > > pid = getpid(); > > + mutex_lock(&lock); > + if (new_threads) { > + do_spawn_thread(); > + } > + pending_threads--; > + mutex_unlock(&lock); > + > while (1) { > struct qemu_paiocb *aiocb; > ssize_t ret = 0; > @@ -381,11 +392,18 @@ static void *aio_thread(void *unused) > return NULL; > } > > -static void spawn_thread(void) > +static void do_spawn_thread(void) > { > sigset_t set, oldset; > > - cur_threads++; > + if (!new_threads) { > + return; > + } > + > + new_threads--; > + pending_threads++; > + > + mutex_unlock(&lock); > > /* block all signals */ > if (sigfillset(&set)) die("sigfillset"); > @@ -394,6 +412,31 @@ static void spawn_thread(void) > thread_create(&thread_id, &attr, aio_thread, NULL); > > if (sigprocmask(SIG_SETMASK, &oldset, NULL)) die("sigprocmask restore"); > + > + mutex_lock(&lock); > +} > + > +static void spawn_thread_bh_fn(void *opaque) > +{ > + mutex_lock(&lock); > + do_spawn_thread(); > + mutex_unlock(&lock); > +} > + > +static void spawn_thread(void) > +{ > + cur_threads++; > + new_threads++; > + /* If there are threads being created, they will spawn new workers, so > + * we don't spend time creating many threads in a loop holding a mutex or > + * starving the current vcpu. > + * > + * If there are no idle threads, ask the main thread to create one, so we > + * inherit the correct affinity instead of the vcpu affinity. > + */ > + if (!pending_threads) { > + qemu_bh_schedule(new_thread_bh); > + } > } > > static void qemu_paio_submit(struct qemu_paiocb *aiocb) > @@ -665,6 +708,7 @@ int paio_init(void) > die2(ret, "pthread_attr_setdetachstate"); > > QTAILQ_INIT(&request_list); > + new_thread_bh = qemu_bh_new(spawn_thread_bh_fn, NULL); > > posix_aio_state = s; > return 0; > -- > 1.7.5.3 > > >
Re: [Qemu-devel] [PATCH 2/3] usb-redir: Call qemu_chr_guest_open/close
On 08/08/2011 03:01 AM, Hans de Goede wrote: Hi, On 08/07/2011 11:30 PM, Anthony Liguori wrote: On 08/07/2011 12:41 PM, Hans de Goede wrote: Hi, On 08/07/2011 05:52 PM, Anthony Liguori wrote: On 08/07/2011 08:21 AM, Hans de Goede wrote: To let the chardev now we're ready start receiving data. This is necessary with the spicevmc chardev to get it registered with the spice-server. Signed-off-by: Hans de Goede --- usb-redir.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/usb-redir.c b/usb-redir.c index e212993..ec88c0b 100644 --- a/usb-redir.c +++ b/usb-redir.c @@ -809,6 +809,8 @@ static int usbredir_initfn(USBDevice *udev) qemu_chr_add_handlers(dev->cs, usbredir_chardev_can_read, usbredir_chardev_read, usbredir_chardev_event, dev); + /* Let the other side know we are ready */ + qemu_chr_guest_open(dev->cs); You should do guest_open before adding handlers. Erm, no, guest_open may lead to a callback in the chardev, to which it may respond by immediately queuing a few writes / doing a read. So after my char-flow changes, you won't be allowed to set handlers unless you've called open. Why not do it the other way around? So don't allow open until the handlers are set. My reasoning behind this is that eventually we will want to have a struct describing a pipe endpoint, which will contain handlers (by then identical for both sides) and besides the struct a priv / user_data pointer which will get passed by the handlers when called. Then we will have a chardev_create or pipe_create call which will take a struct + user data ptr for both ends (so twice). This matches what currently our set handlers call does. But I would expect the open to come after the creation of the pipe. BTW, I'm 90% of the way there in my queue: http://repo.or.cz/w/qemu/aliguori.git/shortlog/refs/heads/char-flow My plan is to have a CharPipe structure that has two CharDriverStates embedded in it. The backend/frontends need to attach themselves to the CharDriverState. I see that as open(). At least to me it is much more logical to first set the handlers (which are really part of object creation) and then later do the open, this matches the common programming paradigm of having an init/create function and an open function. But you need to change the handlers all of the time to implement flow control. Today we overload the setting of handlers to have semantic meaning beyond setting the callbacks for various events. The paradigm I think of is open()'ing a file, and then select()'ing on a file descriptor. Also forcing the set handlers after the open does not work well with virtio_console, as these are not open until the port inside the guest is opened. So then it would need to delay its set handlers till the first open, Right, what's the problem with this? and what should it do at close, do a set handlers NULL before doing the actual close ?? No, close will automatically remove any added handlers. Regards, Anthony Liguori Regards, Hans
Re: [Qemu-devel] [PATCH] posix-aio-compat: fix latency issues
On 08/08/2011 03:49 PM, Frediano Ziglio wrote: 2011/8/8 Avi Kivity: > In certain circumstances, posix-aio-compat can incur a lot of latency: >- threads are created by vcpu threads, so if vcpu affinity is set, > aio threads inherit vcpu affinity. This can cause many aio threads > to compete for one cpu. >- we can create up to max_threads (64) aio threads in one go; since a > pthread_create can take around 30μs, we have up to 2ms of cpu time > under a global lock. > > Fix by: >- moving thread creation to the main thread, so we inherit the main > thread's affinity instead of the vcpu thread's affinity. >- if a thread is currently being created, and we need to create yet > another thread, let thread being born create the new thread, reducing > the amount of time we spend under the main thread. >- drop the local lock while creating a thread (we may still hold the > global mutex, though) > > Note this doesn't eliminate latency completely; scheduler artifacts or > lack of host cpu resources can still cause it. We may want pre-allocated > threads when this cannot be tolerated. > > Thanks to Uli Obergfell of Red Hat for his excellent analysis and suggestions. > > Signed-off-by: Avi Kivity Why not calling pthread_attr_setaffinity_np (where available) before thread creation or shed_setaffinity at thread start instead of telling another thread to create a thread for us just to get affinity cleared? The entire qemu process may be affined to a subset of the host cpus; we don't want to break that. For example: taskset 0xf0 qemu (qemu) info cpus -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v3 01/39] virtio-pci: get config on init
On Mon, Aug 08, 2011 at 07:45:19AM -0500, Anthony Liguori wrote: > On 08/08/2011 05:36 AM, Michael S. Tsirkin wrote: > >>Thinking more closely, I don't think this right. > >> > >>Updating on map ensured that the config was refreshed after each > >>time the bar was mapped. In the very least, the config needs to be > >>refreshed during reset because the guest may write to the guest > >>space which should get cleared after reset. > >> > >>Regards, > >> > >>Anthony Liguori > > > >Not sure I understand. Which register, for example, > >do you have in mind? > >Could you clarify please? > > Actually, you never need to call config_get() AFAICT. It's called > in every read/write access. Every read, yes. But every write? Are you sure? > So I think the code you changed is > extraneous now. > > Regards, > > Anthony Liguori -- MST
[Qemu-devel] [PATCH] Introduce short names for fixed width integer types
QEMU deals with a lot of fixed width integer types; their names (uint64_t etc) are clumsy to use and take up a lot of space. Following Linux, introduce shorter names, for example U64 for uint64_t. Signed-off-by: Avi Kivity --- qemu-common.h |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/qemu-common.h b/qemu-common.h index 0fdecf1..52a2300 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -112,6 +112,15 @@ static inline char *realpath(const char *path, char *resolved_path) int qemu_main(int argc, char **argv, char **envp); #endif +typedef int8_t S8; +typedef uint8_t U8; +typedef int16_t S16; +typedef uint16_t U16; +typedef int32_t S32; +typedef uint32_t U32; +typedef int64_t S64; +typedef uint64_t U64; + /* bottom halves */ typedef void QEMUBHFunc(void *opaque); -- 1.7.5.3
Re: [Qemu-devel] [PATCH] Introduce short names for fixed width integer types
On 08/08/2011 07:56 AM, Avi Kivity wrote: QEMU deals with a lot of fixed width integer types; their names (uint64_t etc) are clumsy to use and take up a lot of space. Following Linux, introduce shorter names, for example U64 for uint64_t. Except Linux uses lower case letters. I personally think Linux style is wrong here. The int8_t types are standard types. Besides, we save lots of characters by using 4-space tabs instead of 8-space tabs. We can afford to spend some of those saved characters on using proper type names :-) Regards, Anthony Liguori Signed-off-by: Avi Kivity --- qemu-common.h |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/qemu-common.h b/qemu-common.h index 0fdecf1..52a2300 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -112,6 +112,15 @@ static inline char *realpath(const char *path, char *resolved_path) int qemu_main(int argc, char **argv, char **envp); #endif +typedef int8_t S8; +typedef uint8_t U8; +typedef int16_t S16; +typedef uint16_t U16; +typedef int32_t S32; +typedef uint32_t U32; +typedef int64_t S64; +typedef uint64_t U64; + /* bottom halves */ typedef void QEMUBHFunc(void *opaque);
Re: [Qemu-devel] [PATCH 2/3] usb-redir: Call qemu_chr_guest_open/close
Hi, On 08/08/2011 02:52 PM, Anthony Liguori wrote: On 08/08/2011 03:01 AM, Hans de Goede wrote: So after my char-flow changes, you won't be allowed to set handlers unless you've called open. Why not do it the other way around? So don't allow open until the handlers are set. My reasoning behind this is that eventually we will want to have a struct describing a pipe endpoint, which will contain handlers (by then identical for both sides) and besides the struct a priv / user_data pointer which will get passed by the handlers when called. Then we will have a chardev_create or pipe_create call which will take a struct + user data ptr for both ends (so twice). This matches what currently our set handlers call does. But I would expect the open to come after the creation of the pipe. BTW, I'm 90% of the way there in my queue: http://repo.or.cz/w/qemu/aliguori.git/shortlog/refs/heads/char-flow My plan is to have a CharPipe structure that has two CharDriverStates embedded in it. The backend/frontends need to attach themselves to the CharDriverState. I see that as open(). So the attaching will cause the other end to see an open() (if the other end is already attached) ? Or will their still be a separate send open event call? And should that call be made before or after the attach? Regards, Hans
Re: [Qemu-devel] [PATCH v3 01/39] virtio-pci: get config on init
On 08/08/2011 07:56 AM, Michael S. Tsirkin wrote: On Mon, Aug 08, 2011 at 07:45:19AM -0500, Anthony Liguori wrote: On 08/08/2011 05:36 AM, Michael S. Tsirkin wrote: Thinking more closely, I don't think this right. Updating on map ensured that the config was refreshed after each time the bar was mapped. In the very least, the config needs to be refreshed during reset because the guest may write to the guest space which should get cleared after reset. Regards, Anthony Liguori Not sure I understand. Which register, for example, do you have in mind? Could you clarify please? Actually, you never need to call config_get() AFAICT. It's called in every read/write access. Every read, yes. But every write? Are you sure? Yeah, not on write, but I think this is a bug. get_config() should be called before doing the memcpy() in order to have a proper RMW. Regards, Anthony Liguori So I think the code you changed is extraneous now. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 2/3] usb-redir: Call qemu_chr_guest_open/close
On 08/08/2011 08:03 AM, Hans de Goede wrote: Hi, On 08/08/2011 02:52 PM, Anthony Liguori wrote: On 08/08/2011 03:01 AM, Hans de Goede wrote: So after my char-flow changes, you won't be allowed to set handlers unless you've called open. Why not do it the other way around? So don't allow open until the handlers are set. My reasoning behind this is that eventually we will want to have a struct describing a pipe endpoint, which will contain handlers (by then identical for both sides) and besides the struct a priv / user_data pointer which will get passed by the handlers when called. Then we will have a chardev_create or pipe_create call which will take a struct + user data ptr for both ends (so twice). This matches what currently our set handlers call does. But I would expect the open to come after the creation of the pipe. BTW, I'm 90% of the way there in my queue: http://repo.or.cz/w/qemu/aliguori.git/shortlog/refs/heads/char-flow My plan is to have a CharPipe structure that has two CharDriverStates embedded in it. The backend/frontends need to attach themselves to the CharDriverState. I see that as open(). So the attaching will cause the other end to see an open() (if the other end is already attached) ? Or will their still be a separate send open event call? And should that call be made before or after the attach? Doing an open() will result in an open event being generated. Neither side should ever be directly involved in sending open/close events. Regards, Anthony Liguori Regards, Hans
[Qemu-devel] [PATCH v4 01/39] memory: rename PORTIO_END to PORTIO_END_OF_LIST
For consistency with other _END_OF_LIST macros. Signed-off-by: Avi Kivity --- memory.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/memory.h b/memory.h index 4e518b2..da00a3b 100644 --- a/memory.h +++ b/memory.h @@ -133,7 +133,7 @@ struct MemoryRegionPortio { IOPortWriteFunc *write; }; -#define PORTIO_END { } +#define PORTIO_END_OF_LIST() { } /** * memory_region_init: Initialize a memory region -- 1.7.5.3
[Qemu-devel] [PATCH v4 19/39] ivshmem: convert to memory API
excluding msix. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/ivshmem.c | 148 -- 1 files changed, 50 insertions(+), 98 deletions(-) diff --git a/hw/ivshmem.c b/hw/ivshmem.c index 3055dd2..f80e7b6 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -56,11 +56,15 @@ typedef struct IVShmemState { CharDriverState **eventfd_chr; CharDriverState *server_chr; -int ivshmem_mmio_io_addr; +MemoryRegion ivshmem_mmio; pcibus_t mmio_addr; -pcibus_t shm_pci_addr; -uint64_t ivshmem_offset; +/* We might need to register the BAR before we actually have the memory. + * So prepare a container MemoryRegion for the BAR immediately and + * add a subregion when we have the memory. + */ +MemoryRegion bar; +MemoryRegion ivshmem; uint64_t ivshmem_size; /* size of shared memory region */ int shm_fd; /* shared memory file descriptor */ @@ -96,23 +100,6 @@ static inline bool is_power_of_two(uint64_t x) { return (x & (x - 1)) == 0; } -static void ivshmem_map(PCIDevice *pci_dev, int region_num, -pcibus_t addr, pcibus_t size, int type) -{ -IVShmemState *s = DO_UPCAST(IVShmemState, dev, pci_dev); - -s->shm_pci_addr = addr; - -if (s->ivshmem_offset > 0) { -cpu_register_physical_memory(s->shm_pci_addr, s->ivshmem_size, -s->ivshmem_offset); -} - -IVSHMEM_DPRINTF("guest pci addr = %" FMT_PCIBUS ", guest h/w addr = %" -PRIu64 ", size = %" FMT_PCIBUS "\n", addr, s->ivshmem_offset, size); - -} - /* accessing registers - based on rtl8139 */ static void ivshmem_update_irq(IVShmemState *s, int val) { @@ -168,15 +155,8 @@ static uint32_t ivshmem_IntrStatus_read(IVShmemState *s) return ret; } -static void ivshmem_io_writew(void *opaque, target_phys_addr_t addr, -uint32_t val) -{ - -IVSHMEM_DPRINTF("We shouldn't be writing words\n"); -} - -static void ivshmem_io_writel(void *opaque, target_phys_addr_t addr, -uint32_t val) +static void ivshmem_io_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned size) { IVShmemState *s = opaque; @@ -219,20 +199,8 @@ static void ivshmem_io_writel(void *opaque, target_phys_addr_t addr, } } -static void ivshmem_io_writeb(void *opaque, target_phys_addr_t addr, -uint32_t val) -{ -IVSHMEM_DPRINTF("We shouldn't be writing bytes\n"); -} - -static uint32_t ivshmem_io_readw(void *opaque, target_phys_addr_t addr) -{ - -IVSHMEM_DPRINTF("We shouldn't be reading words\n"); -return 0; -} - -static uint32_t ivshmem_io_readl(void *opaque, target_phys_addr_t addr) +static uint64_t ivshmem_io_read(void *opaque, target_phys_addr_t addr, +unsigned size) { IVShmemState *s = opaque; @@ -265,23 +233,14 @@ static uint32_t ivshmem_io_readl(void *opaque, target_phys_addr_t addr) return ret; } -static uint32_t ivshmem_io_readb(void *opaque, target_phys_addr_t addr) -{ -IVSHMEM_DPRINTF("We shouldn't be reading bytes\n"); - -return 0; -} - -static CPUReadMemoryFunc * const ivshmem_mmio_read[3] = { -ivshmem_io_readb, -ivshmem_io_readw, -ivshmem_io_readl, -}; - -static CPUWriteMemoryFunc * const ivshmem_mmio_write[3] = { -ivshmem_io_writeb, -ivshmem_io_writew, -ivshmem_io_writel, +static const MemoryRegionOps ivshmem_mmio_ops = { +.read = ivshmem_io_read, +.write = ivshmem_io_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.impl = { +.min_access_size = 4, +.max_access_size = 4, +}, }; static void ivshmem_receive(void *opaque, const uint8_t *buf, int size) @@ -371,12 +330,12 @@ static void create_shared_memory_BAR(IVShmemState *s, int fd) { ptr = mmap(0, s->ivshmem_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); -s->ivshmem_offset = qemu_ram_alloc_from_ptr(&s->dev.qdev, "ivshmem.bar2", -s->ivshmem_size, ptr); +memory_region_init_ram_ptr(&s->ivshmem, &s->dev.qdev, "ivshmem.bar2", + s->ivshmem_size, ptr); +memory_region_add_subregion(&s->bar, 0, &s->ivshmem); /* region for shared memory */ -pci_register_bar(&s->dev, 2, s->ivshmem_size, -PCI_BASE_ADDRESS_SPACE_MEMORY, ivshmem_map); +pci_register_bar_region(&s->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar); } static void close_guest_eventfds(IVShmemState *s, int posn) @@ -401,8 +360,12 @@ static void setup_ioeventfds(IVShmemState *s) { for (i = 0; i <= s->max_peer; i++) { for (j = 0; j < s->peers[i].nb_eventfds; j++) { -kvm_set_ioeventf
[Qemu-devel] [PATCH v4 23/39] lsi53c895a: convert to memory API
An optimization that fast-pathed DMA reads from the SCRIPTS memory was removed int the process. Likely it breaks with iommus anyway. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/lsi53c895a.c | 258 --- 1 files changed, 56 insertions(+), 202 deletions(-) diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c index e9904c4..0ab8c78 100644 --- a/hw/lsi53c895a.c +++ b/hw/lsi53c895a.c @@ -185,9 +185,9 @@ typedef struct lsi_request { typedef struct { PCIDevice dev; -int mmio_io_addr; -int ram_io_addr; -uint32_t script_ram_base; +MemoryRegion mmio_io; +MemoryRegion ram_io; +MemoryRegion io_io; int carry; /* ??? Should this be an a visible register somewhere? */ int status; @@ -391,10 +391,9 @@ static inline uint32_t read_dword(LSIState *s, uint32_t addr) { uint32_t buf; -/* Optimize reading from SCRIPTS RAM. */ -if ((addr & 0xe000) == s->script_ram_base) { -return s->script_ram[(addr & 0x1fff) >> 2]; -} +/* XXX: an optimization here used to fast-path the read from scripts + * memory. But that bypasses any iommu. + */ cpu_physical_memory_read(addr, (uint8_t *)&buf, 4); return cpu_to_le32(buf); } @@ -1899,232 +1898,90 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val) #undef CASE_SET_REG32 } -static void lsi_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) +static void lsi_mmio_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned size) { LSIState *s = opaque; lsi_reg_writeb(s, addr & 0xff, val); } -static void lsi_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) -{ -LSIState *s = opaque; - -addr &= 0xff; -lsi_reg_writeb(s, addr, val & 0xff); -lsi_reg_writeb(s, addr + 1, (val >> 8) & 0xff); -} - -static void lsi_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) -{ -LSIState *s = opaque; - -addr &= 0xff; -lsi_reg_writeb(s, addr, val & 0xff); -lsi_reg_writeb(s, addr + 1, (val >> 8) & 0xff); -lsi_reg_writeb(s, addr + 2, (val >> 16) & 0xff); -lsi_reg_writeb(s, addr + 3, (val >> 24) & 0xff); -} - -static uint32_t lsi_mmio_readb(void *opaque, target_phys_addr_t addr) +static uint64_t lsi_mmio_read(void *opaque, target_phys_addr_t addr, + unsigned size) { LSIState *s = opaque; return lsi_reg_readb(s, addr & 0xff); } -static uint32_t lsi_mmio_readw(void *opaque, target_phys_addr_t addr) -{ -LSIState *s = opaque; -uint32_t val; - -addr &= 0xff; -val = lsi_reg_readb(s, addr); -val |= lsi_reg_readb(s, addr + 1) << 8; -return val; -} - -static uint32_t lsi_mmio_readl(void *opaque, target_phys_addr_t addr) -{ -LSIState *s = opaque; -uint32_t val; -addr &= 0xff; -val = lsi_reg_readb(s, addr); -val |= lsi_reg_readb(s, addr + 1) << 8; -val |= lsi_reg_readb(s, addr + 2) << 16; -val |= lsi_reg_readb(s, addr + 3) << 24; -return val; -} - -static CPUReadMemoryFunc * const lsi_mmio_readfn[3] = { -lsi_mmio_readb, -lsi_mmio_readw, -lsi_mmio_readl, -}; - -static CPUWriteMemoryFunc * const lsi_mmio_writefn[3] = { -lsi_mmio_writeb, -lsi_mmio_writew, -lsi_mmio_writel, +static const MemoryRegionOps lsi_mmio_ops = { +.read = lsi_mmio_read, +.write = lsi_mmio_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.impl = { +.min_access_size = 1, +.max_access_size = 1, +}, }; -static void lsi_ram_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) +static void lsi_ram_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned size) { LSIState *s = opaque; uint32_t newval; +uint32_t mask; int shift; -addr &= 0x1fff; newval = s->script_ram[addr >> 2]; shift = (addr & 3) * 8; -newval &= ~(0xff << shift); +mask = ((uint64_t)1 << (size * 8)) - 1; +newval &= ~(mask << shift); newval |= val << shift; s->script_ram[addr >> 2] = newval; } -static void lsi_ram_writew(void *opaque, target_phys_addr_t addr, uint32_t val) -{ -LSIState *s = opaque; -uint32_t newval; - -addr &= 0x1fff; -newval = s->script_ram[addr >> 2]; -if (addr & 2) { -newval = (newval & 0x) | (val << 16); -} else { -newval = (newval & 0x) | val; -} -s->script_ram[addr >> 2] = newval; -} - - -static void lsi_ram_writel(void *opaque, target_phys_addr_t addr, uint32_t val) -{ -LSIState *s = opaque; - -addr &= 0x1fff; -s->script_ram[addr >> 2] = val; -} - -static uint32_t lsi_ram_readb(void *opaque, target_phys_addr_t addr) +static uint64_t lsi_ram_read(void *opaque, target_phys_addr_t addr, + unsigned size) { LSIState *s = opaque; uint32_t val; +uint32_t mask;
[Qemu-devel] [PATCH v4 14/39] ac97: convert to memory API
fixes BAR sizing as well. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/ac97.c | 89 +++- 1 files changed, 52 insertions(+), 37 deletions(-) diff --git a/hw/ac97.c b/hw/ac97.c index 0b59896..52f0f0d 100644 --- a/hw/ac97.c +++ b/hw/ac97.c @@ -160,8 +160,9 @@ typedef struct AC97LinkState { SWVoiceIn *voice_mc; int invalid_freq[3]; uint8_t silence[128]; -uint32_t base[2]; int bup_flag; +MemoryRegion io_nam; +MemoryRegion io_nabm; } AC97LinkState; enum { @@ -583,7 +584,7 @@ static uint32_t nam_readw (void *opaque, uint32_t addr) { AC97LinkState *s = opaque; uint32_t val = ~0U; -uint32_t index = addr - s->base[0]; +uint32_t index = addr; s->cas = 0; val = mixer_load (s, index); return val; @@ -611,7 +612,7 @@ static void nam_writeb (void *opaque, uint32_t addr, uint32_t val) static void nam_writew (void *opaque, uint32_t addr, uint32_t val) { AC97LinkState *s = opaque; -uint32_t index = addr - s->base[0]; +uint32_t index = addr; s->cas = 0; switch (index) { case AC97_Reset: @@ -714,7 +715,7 @@ static uint32_t nabm_readb (void *opaque, uint32_t addr) { AC97LinkState *s = opaque; AC97BusMasterRegs *r = NULL; -uint32_t index = addr - s->base[1]; +uint32_t index = addr; uint32_t val = ~0U; switch (index) { @@ -769,7 +770,7 @@ static uint32_t nabm_readw (void *opaque, uint32_t addr) { AC97LinkState *s = opaque; AC97BusMasterRegs *r = NULL; -uint32_t index = addr - s->base[1]; +uint32_t index = addr; uint32_t val = ~0U; switch (index) { @@ -798,7 +799,7 @@ static uint32_t nabm_readl (void *opaque, uint32_t addr) { AC97LinkState *s = opaque; AC97BusMasterRegs *r = NULL; -uint32_t index = addr - s->base[1]; +uint32_t index = addr; uint32_t val = ~0U; switch (index) { @@ -848,7 +849,7 @@ static void nabm_writeb (void *opaque, uint32_t addr, uint32_t val) { AC97LinkState *s = opaque; AC97BusMasterRegs *r = NULL; -uint32_t index = addr - s->base[1]; +uint32_t index = addr; switch (index) { case PI_LVI: case PO_LVI: @@ -904,7 +905,7 @@ static void nabm_writew (void *opaque, uint32_t addr, uint32_t val) { AC97LinkState *s = opaque; AC97BusMasterRegs *r = NULL; -uint32_t index = addr - s->base[1]; +uint32_t index = addr; switch (index) { case PI_SR: case PO_SR: @@ -924,7 +925,7 @@ static void nabm_writel (void *opaque, uint32_t addr, uint32_t val) { AC97LinkState *s = opaque; AC97BusMasterRegs *r = NULL; -uint32_t index = addr - s->base[1]; +uint32_t index = addr; switch (index) { case PI_BDBAR: case PO_BDBAR: @@ -1230,31 +1231,33 @@ static const VMStateDescription vmstate_ac97 = { } }; -static void ac97_map (PCIDevice *pci_dev, int region_num, - pcibus_t addr, pcibus_t size, int type) -{ -AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, pci_dev); -PCIDevice *d = &s->dev; - -if (!region_num) { -s->base[0] = addr; -register_ioport_read (addr, 256 * 1, 1, nam_readb, d); -register_ioport_read (addr, 256 * 2, 2, nam_readw, d); -register_ioport_read (addr, 256 * 4, 4, nam_readl, d); -register_ioport_write (addr, 256 * 1, 1, nam_writeb, d); -register_ioport_write (addr, 256 * 2, 2, nam_writew, d); -register_ioport_write (addr, 256 * 4, 4, nam_writel, d); -} -else { -s->base[1] = addr; -register_ioport_read (addr, 64 * 1, 1, nabm_readb, d); -register_ioport_read (addr, 64 * 2, 2, nabm_readw, d); -register_ioport_read (addr, 64 * 4, 4, nabm_readl, d); -register_ioport_write (addr, 64 * 1, 1, nabm_writeb, d); -register_ioport_write (addr, 64 * 2, 2, nabm_writew, d); -register_ioport_write (addr, 64 * 4, 4, nabm_writel, d); -} -} +static const MemoryRegionPortio nam_portio[] = { +{ 0, 256 * 1, 1, .read = nam_readb, }, +{ 0, 256 * 2, 2, .read = nam_readw, }, +{ 0, 256 * 4, 4, .read = nam_readl, }, +{ 0, 256 * 1, 1, .write = nam_writeb, }, +{ 0, 256 * 2, 2, .write = nam_writew, }, +{ 0, 256 * 4, 4, .write = nam_writel, }, +PORTIO_END_OF_LIST(), +}; + +static const MemoryRegionOps ac97_io_nam_ops = { +.old_portio = nam_portio, +}; + +static const MemoryRegionPortio nabm_portio[] = { +{ 0, 64 * 1, 1, .read = nabm_readb, }, +{ 0, 64 * 2, 2, .read = nabm_readw, }, +{ 0, 64 * 4, 4, .read = nabm_readl, }, +{ 0, 64 * 1, 1, .write = nabm_writeb, }, +{ 0, 64 * 2, 2, .write = nabm_writew, }, +{ 0, 64 * 4, 4, .write = nabm_writel, }, +PORTIO_END_OF_LIST() +}; + +static const MemoryRegionOps ac97_io_nabm_ops = { +.old_portio = nabm_portio, +}; static void ac97_on_reset (void *opaque) { @@ -1311,15 +1314,26 @@ static
[Qemu-devel] [PATCH v4 18/39] ide: convert to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/ide/cmd646.c | 208 +++ hw/ide/pci.c| 25 --- hw/ide/pci.h| 19 - hw/ide/piix.c | 64 + hw/ide/via.c| 65 + 5 files changed, 261 insertions(+), 120 deletions(-) diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c index 56302b5..13e6f2f 100644 --- a/hw/ide/cmd646.c +++ b/hw/ide/cmd646.c @@ -44,35 +44,95 @@ static void cmd646_update_irq(PCIIDEState *d); -static void ide_map(PCIDevice *pci_dev, int region_num, -pcibus_t addr, pcibus_t size, int type) +static uint64_t cmd646_cmd_read(void *opaque, target_phys_addr_t addr, +unsigned size) { -PCIIDEState *d = DO_UPCAST(PCIIDEState, dev, pci_dev); -IDEBus *bus; - -if (region_num <= 3) { -bus = &d->bus[(region_num >> 1)]; -if (region_num & 1) { -register_ioport_read(addr + 2, 1, 1, ide_status_read, bus); -register_ioport_write(addr + 2, 1, 1, ide_cmd_write, bus); +CMD646BAR *cmd646bar = opaque; + +if (addr != 2 || size != 1) { +return ((uint64_t)1 << (size * 8)) - 1; +} +return ide_status_read(cmd646bar->bus, addr + 2); +} + +static void cmd646_cmd_write(void *opaque, target_phys_addr_t addr, + uint64_t data, unsigned size) +{ +CMD646BAR *cmd646bar = opaque; + +if (addr != 2 || size != 1) { +return; +} +ide_cmd_write(cmd646bar->bus, addr + 2, data); +} + +static MemoryRegionOps cmd646_cmd_ops = { +.read = cmd646_cmd_read, +.write = cmd646_cmd_write, +.endianness = DEVICE_LITTLE_ENDIAN, +}; + +static uint64_t cmd646_data_read(void *opaque, target_phys_addr_t addr, + unsigned size) +{ +CMD646BAR *cmd646bar = opaque; + +if (size == 1) { +return ide_ioport_read(cmd646bar->bus, addr); +} else if (addr == 0) { +if (size == 2) { +return ide_data_readw(cmd646bar->bus, addr); } else { -register_ioport_write(addr, 8, 1, ide_ioport_write, bus); -register_ioport_read(addr, 8, 1, ide_ioport_read, bus); - -/* data ports */ -register_ioport_write(addr, 2, 2, ide_data_writew, bus); -register_ioport_read(addr, 2, 2, ide_data_readw, bus); -register_ioport_write(addr, 4, 4, ide_data_writel, bus); -register_ioport_read(addr, 4, 4, ide_data_readl, bus); +return ide_data_readl(cmd646bar->bus, addr); } } +return ((uint64_t)1 << (size * 8)) - 1; } -static uint32_t bmdma_readb_common(PCIIDEState *pci_dev, BMDMAState *bm, - uint32_t addr) +static void cmd646_data_write(void *opaque, target_phys_addr_t addr, + uint64_t data, unsigned size) { +CMD646BAR *cmd646bar = opaque; + +if (size == 1) { +return ide_ioport_write(cmd646bar->bus, addr, data); +} else if (addr == 0) { +if (size == 2) { +return ide_data_writew(cmd646bar->bus, addr, data); +} else { +return ide_data_writel(cmd646bar->bus, addr, data); +} +} +} + +static MemoryRegionOps cmd646_data_ops = { +.read = cmd646_data_read, +.write = cmd646_data_write, +.endianness = DEVICE_LITTLE_ENDIAN, +}; + +static void setup_cmd646_bar(PCIIDEState *d, int bus_num) +{ +IDEBus *bus = &d->bus[bus_num]; +CMD646BAR *bar = &d->cmd646_bar[bus_num]; + +bar->bus = bus; +bar->pci_dev = d; +memory_region_init_io(&bar->cmd, &cmd646_cmd_ops, bar, "cmd646-cmd", 4); +memory_region_init_io(&bar->data, &cmd646_data_ops, bar, "cmd646-data", 8); +} + +static uint64_t bmdma_read(void *opaque, target_phys_addr_t addr, + unsigned size) +{ +BMDMAState *bm = opaque; +PCIIDEState *pci_dev = bm->pci_dev; uint32_t val; +if (size != 1) { +return ((uint64_t)1 << (size * 8)) - 1; +} + switch(addr & 3) { case 0: val = bm->cmd; @@ -100,31 +160,22 @@ static uint32_t bmdma_readb_common(PCIIDEState *pci_dev, BMDMAState *bm, return val; } -static uint32_t bmdma_readb_0(void *opaque, uint32_t addr) +static void bmdma_write(void *opaque, target_phys_addr_t addr, +uint64_t val, unsigned size) { -PCIIDEState *pci_dev = opaque; -BMDMAState *bm = &pci_dev->bmdma[0]; - -return bmdma_readb_common(pci_dev, bm, addr); -} +BMDMAState *bm = opaque; +PCIIDEState *pci_dev = bm->pci_dev; -static uint32_t bmdma_readb_1(void *opaque, uint32_t addr) -{ -PCIIDEState *pci_dev = opaque; -BMDMAState *bm = &pci_dev->bmdma[1]; - -return bmdma_readb_common(pci_dev, bm, addr); -} +if (size != 1) { +return; +} -static void bmdma_writeb_common(PCIIDEState *pci_dev, BMDMAState *bm, -
[Qemu-devel] [PATCH v4 25/39] ne2000: convert to memory API
Reviewed-by: Richard Henderson Signed-off-by: Avi Kivity --- hw/ne2000-isa.c | 13 ++--- hw/ne2000.c | 77 +- hw/ne2000.h |8 + 3 files changed, 58 insertions(+), 40 deletions(-) diff --git a/hw/ne2000-isa.c b/hw/ne2000-isa.c index e41dbba..756ed5c 100644 --- a/hw/ne2000-isa.c +++ b/hw/ne2000-isa.c @@ -27,6 +27,7 @@ #include "qdev.h" #include "net.h" #include "ne2000.h" +#include "exec-memory.h" typedef struct ISANE2000State { ISADevice dev; @@ -66,19 +67,11 @@ static int isa_ne2000_initfn(ISADevice *dev) ISANE2000State *isa = DO_UPCAST(ISANE2000State, dev, dev); NE2000State *s = &isa->ne2000; -register_ioport_write(isa->iobase, 16, 1, ne2000_ioport_write, s); -register_ioport_read(isa->iobase, 16, 1, ne2000_ioport_read, s); +ne2000_setup_io(s, 0x20); isa_init_ioport_range(dev, isa->iobase, 16); - -register_ioport_write(isa->iobase + 0x10, 1, 1, ne2000_asic_ioport_write, s); -register_ioport_read(isa->iobase + 0x10, 1, 1, ne2000_asic_ioport_read, s); -register_ioport_write(isa->iobase + 0x10, 2, 2, ne2000_asic_ioport_write, s); -register_ioport_read(isa->iobase + 0x10, 2, 2, ne2000_asic_ioport_read, s); isa_init_ioport_range(dev, isa->iobase + 0x10, 2); - -register_ioport_write(isa->iobase + 0x1f, 1, 1, ne2000_reset_ioport_write, s); -register_ioport_read(isa->iobase + 0x1f, 1, 1, ne2000_reset_ioport_read, s); isa_init_ioport(dev, isa->iobase + 0x1f); +memory_region_add_subregion(get_system_io(), isa->iobase, &s->io); isa_init_irq(dev, &s->irq, isa->isairq); diff --git a/hw/ne2000.c b/hw/ne2000.c index f8acaae..5b76acf 100644 --- a/hw/ne2000.c +++ b/hw/ne2000.c @@ -297,7 +297,7 @@ ssize_t ne2000_receive(VLANClientState *nc, const uint8_t *buf, size_t size_) return size_; } -void ne2000_ioport_write(void *opaque, uint32_t addr, uint32_t val) +static void ne2000_ioport_write(void *opaque, uint32_t addr, uint32_t val) { NE2000State *s = opaque; int offset, page, index; @@ -394,7 +394,7 @@ void ne2000_ioport_write(void *opaque, uint32_t addr, uint32_t val) } } -uint32_t ne2000_ioport_read(void *opaque, uint32_t addr) +static uint32_t ne2000_ioport_read(void *opaque, uint32_t addr) { NE2000State *s = opaque; int offset, page, ret; @@ -544,7 +544,7 @@ static inline void ne2000_dma_update(NE2000State *s, int len) } } -void ne2000_asic_ioport_write(void *opaque, uint32_t addr, uint32_t val) +static void ne2000_asic_ioport_write(void *opaque, uint32_t addr, uint32_t val) { NE2000State *s = opaque; @@ -564,7 +564,7 @@ void ne2000_asic_ioport_write(void *opaque, uint32_t addr, uint32_t val) } } -uint32_t ne2000_asic_ioport_read(void *opaque, uint32_t addr) +static uint32_t ne2000_asic_ioport_read(void *opaque, uint32_t addr) { NE2000State *s = opaque; int ret; @@ -612,12 +612,12 @@ static uint32_t ne2000_asic_ioport_readl(void *opaque, uint32_t addr) return ret; } -void ne2000_reset_ioport_write(void *opaque, uint32_t addr, uint32_t val) +static void ne2000_reset_ioport_write(void *opaque, uint32_t addr, uint32_t val) { /* nothing to do (end of reset pulse) */ } -uint32_t ne2000_reset_ioport_read(void *opaque, uint32_t addr) +static uint32_t ne2000_reset_ioport_read(void *opaque, uint32_t addr) { NE2000State *s = opaque; ne2000_reset(s); @@ -676,27 +676,55 @@ static const VMStateDescription vmstate_pci_ne2000 = { } }; -/***/ -/* PCI NE2000 definitions */ +static uint64_t ne2000_read(void *opaque, target_phys_addr_t addr, +unsigned size) +{ +NE2000State *s = opaque; -static void ne2000_map(PCIDevice *pci_dev, int region_num, - pcibus_t addr, pcibus_t size, int type) +if (addr < 0x10 && size == 1) { +return ne2000_ioport_read(s, addr); +} else if (addr == 0x10) { +if (size <= 2) { +return ne2000_asic_ioport_read(s, addr); +} else { +return ne2000_asic_ioport_readl(s, addr); +} +} else if (addr == 0x1f && size == 1) { +return ne2000_reset_ioport_read(s, addr); +} +return ((uint64_t)1 << (size * 8)) - 1; +} + +static void ne2000_write(void *opaque, target_phys_addr_t addr, + uint64_t data, unsigned size) { -PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev); -NE2000State *s = &d->ne2000; +NE2000State *s = opaque; + +if (addr < 0x10 && size == 1) { +return ne2000_ioport_write(s, addr, data); +} else if (addr == 0x10) { +if (size <= 2) { +return ne2000_asic_ioport_write(s, addr, data); +} else { +return ne2000_asic_ioport_writel(s, addr, data); +} +} else if (addr == 0x1f && size == 1) { +return ne2000_reset_ioport_write(s, addr,
Re: [Qemu-devel] [PATCH] Introduce short names for fixed width integer types
On 08/08/2011 04:00 PM, Anthony Liguori wrote: On 08/08/2011 07:56 AM, Avi Kivity wrote: QEMU deals with a lot of fixed width integer types; their names (uint64_t etc) are clumsy to use and take up a lot of space. Following Linux, introduce shorter names, for example U64 for uint64_t. Except Linux uses lower case letters. I personally think Linux style is wrong here. The int8_t types are standard types. Besides, we save lots of characters by using 4-space tabs instead of 8-space tabs. We can afford to spend some of those saved characters on using proper type names :-) It's not about saving space, it's about improving readability. We have about 21k uses of these types, they deserve short names. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v3 01/39] virtio-pci: get config on init
On Mon, Aug 08, 2011 at 08:02:08AM -0500, Anthony Liguori wrote: > On 08/08/2011 07:56 AM, Michael S. Tsirkin wrote: > >On Mon, Aug 08, 2011 at 07:45:19AM -0500, Anthony Liguori wrote: > >>On 08/08/2011 05:36 AM, Michael S. Tsirkin wrote: > Thinking more closely, I don't think this right. > > Updating on map ensured that the config was refreshed after each > time the bar was mapped. In the very least, the config needs to be > refreshed during reset because the guest may write to the guest > space which should get cleared after reset. > > Regards, > > Anthony Liguori > >>> > >>>Not sure I understand. Which register, for example, > >>>do you have in mind? > >>>Could you clarify please? > >> > >>Actually, you never need to call config_get() AFAICT. It's called > >>in every read/write access. > > > >Every read, yes. But every write? Are you sure? > > Yeah, not on write, but I think this is a bug. get_config() should > be called before doing the memcpy() in order to have a proper RMW. > > Regards, > > Anthony Liguori Probably not noticeable because guests don't do the RMW in practice. We also send the config over on migration. That's probably a bug as well ... -- MST
Re: [Qemu-devel] [PATCH v3 01/39] virtio-pci: get config on init
On 08/08/2011 08:14 AM, Michael S. Tsirkin wrote: Probably not noticeable because guests don't do the RMW in practice. We also send the config over on migration. That's probably a bug as well ... It's probably unnecessary, but I don't think it's a bug.. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] Introduce short names for fixed width integer types
On 08/08/2011 08:12 AM, Avi Kivity wrote: On 08/08/2011 04:00 PM, Anthony Liguori wrote: On 08/08/2011 07:56 AM, Avi Kivity wrote: QEMU deals with a lot of fixed width integer types; their names (uint64_t etc) are clumsy to use and take up a lot of space. Following Linux, introduce shorter names, for example U64 for uint64_t. Except Linux uses lower case letters. I personally think Linux style is wrong here. The int8_t types are standard types. Besides, we save lots of characters by using 4-space tabs instead of 8-space tabs. We can afford to spend some of those saved characters on using proper type names :-) It's not about saving space, it's about improving readability. We have about 21k uses of these types, they deserve short names. This is one of the few areas that we're actually consistent with today. Introducing a new set of types will just create inconsistency. Most importantly, these are standard types. Every modern library and C program should be using them. TBH, having short names is just a bad case of NIH. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] Introduce short names for fixed width integer types
On 8 August 2011 13:56, Avi Kivity wrote: > QEMU deals with a lot of fixed width integer types; their names > (uint64_t etc) are clumsy to use and take up a lot of space. > > Following Linux, introduce shorter names, for example U64 for > uint64_t. Strongly disagree. uint64_t &c are standard types and it's immediately clear to a competent C programmer what they are. Random qemu-specific funny named types just introduces an unnecessary level of indirection. We only just recently managed to get rid of the nonstandard typenames for these from fpu/... -- PMM
Re: [Qemu-devel] [PATCH] Introduce short names for fixed width integer types
On 08/08/2011 04:17 PM, Anthony Liguori wrote: This is one of the few areas that we're actually consistent with today. Introducing a new set of types will just create inconsistency. Most importantly, these are standard types. Every modern library and C program should be using them. TBH, having short names is just a bad case of NIH. Those are exactly the same types, compatible with all the libraries. NIH would be redefining them ourselves (and breaking pointer compatibility etc.) -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] posix-aio-compat: fix latency issues
2011/8/8 Avi Kivity : > On 08/08/2011 03:49 PM, Frediano Ziglio wrote: >> >> 2011/8/8 Avi Kivity: >> > In certain circumstances, posix-aio-compat can incur a lot of latency: >> > - threads are created by vcpu threads, so if vcpu affinity is set, >> > aio threads inherit vcpu affinity. This can cause many aio threads >> > to compete for one cpu. >> > - we can create up to max_threads (64) aio threads in one go; since a >> > pthread_create can take around 30μs, we have up to 2ms of cpu time >> > under a global lock. >> > >> > Fix by: >> > - moving thread creation to the main thread, so we inherit the main >> > thread's affinity instead of the vcpu thread's affinity. >> > - if a thread is currently being created, and we need to create yet >> > another thread, let thread being born create the new thread, >> > reducing >> > the amount of time we spend under the main thread. >> > - drop the local lock while creating a thread (we may still hold the >> > global mutex, though) >> > >> > Note this doesn't eliminate latency completely; scheduler artifacts or >> > lack of host cpu resources can still cause it. We may want >> > pre-allocated >> > threads when this cannot be tolerated. >> > >> > Thanks to Uli Obergfell of Red Hat for his excellent analysis and >> > suggestions. >> > >> > Signed-off-by: Avi Kivity >> >> Why not calling pthread_attr_setaffinity_np (where available) before >> thread creation or shed_setaffinity at thread start instead of telling >> another thread to create a thread for us just to get affinity cleared? >> > > The entire qemu process may be affined to a subset of the host cpus; we > don't want to break that. > > For example: > > taskset 0xf0 qemu > (qemu) info cpus > > > Just call sched_getaffinity at program start, save to a global variable and then set this affinity for io threads. I didn't use affinity that much but from manual it seems that if you own process you can set affinity as you like. IMHO this patch introduce a delay in io thread creation due to posting thread creation to another thread just to set different affinity. Frediano
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On Mon, 08 Aug 2011 14:22:14 +0300 Avi Kivity wrote: > On 08/04/2011 01:17 PM, Jan Kiszka wrote: > > > > > > Why "QemuState"? In general, "qemu" can be inferred from the fact that > > > we're in qemu.git. Suggest "RunState". > > > > > > Second, these states can coexist. A user may pause the VM > > > simultaneously with the watchdog firing or entering premigrate state. > > > In fact, with multiple monitors, each monitor can pause and resume the > > > vm independently. > > > > > > So I think we should keep a reference count instead of just a start/stop > > > state. Perhaps > > > > > > vm_stop(QemuState s) > > > { > > > ++stopcount[s]; > > > } > > > > > > vm_is_stopped() > > > { > > > for (s in states) > > >if (stopcount[s]) > > >return true; > > > return false; > > > } > > > > I don't think this makes sense nor is user-friendly. If one command > > channel suspends the machine, others have the chance to subscribe for > > that event. > > It's inherently racy. > > > Maintaining a suspension counter would mean you also need a > > channel to query its value. > > Why? > > > IMHO, there is also no use for defining stopped orthogonally to > > premigrate and other states that imply that the machine is stopped. > > Basically they mean "stopped for/because of X". We just need to avoid > > that you can enter plain stopped state from them by issuing the > > corresponding monitor command. The other way around might be possible, > > though, if there are race windows. > > > > I'm worried about the following race: > >stop >(qemu stopped for internal reason) >stop comment processed > >resume > > The (qemu stopped for internal reason) part is lost. If the "stop" you're referring to happens through vm_stop(), then no, it won't be lost because do_vm_stop() doesn't allow qemu to be stopped twice.
Re: [Qemu-devel] [PATCH] posix-aio-compat: fix latency issues
On 08/08/2011 04:21 PM, Frediano Ziglio wrote: > > The entire qemu process may be affined to a subset of the host cpus; we > don't want to break that. > > For example: > > taskset 0xf0 qemu > (qemu) info cpus > > > Just call sched_getaffinity at program start, save to a global variable and then set this affinity for io threads. This affinity may change later on. I didn't use affinity that much but from manual it seems that if you own process you can set affinity as you like. IMHO this patch introduce a delay in io thread creation due to posting thread creation to another thread just to set different affinity. It does. But aio threads have a long life, so this happens very rarely. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] RFC: moving fsfreeze support from the userland guest agent to the guest kernel
On Sun, 07 Aug 2011 21:28:17 +0300 Ronen Hod wrote: > Well, we want to support Microsoft's VSS, and that requires a guest > agent that communicates with all the "writers" (applications), waiting > for them to flush their app data in order to generate a consistent > app-level snapshot. The VSS platform does most of the work. > Still, at the bottom line, the agent's role is only to find the right > moment in time. This moment can be relayed back to libvirt, and from > there do it according to your suggestion, so that the guest agent does > not do the freeze, and it is actually not a mandatory component. I think this discussion has reached the point where patches will speak louder than words.
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On 08/08/2011 04:25 PM, Luiz Capitulino wrote: > > I'm worried about the following race: > > stop > (qemu stopped for internal reason) > stop comment processed > > resume > > The (qemu stopped for internal reason) part is lost. If the "stop" you're referring to happens through vm_stop(), then no, it won't be lost because do_vm_stop() doesn't allow qemu to be stopped twice. What happens then? The user sees an error? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On Mon, 08 Aug 2011 16:27:05 +0300 Avi Kivity wrote: > On 08/08/2011 04:25 PM, Luiz Capitulino wrote: > > > > > > I'm worried about the following race: > > > > > > stop > > > (qemu stopped for internal reason) > > > stop comment processed > > > > > > resume > > > > > > The (qemu stopped for internal reason) part is lost. > > > > If the "stop" you're referring to happens through vm_stop(), then no, > > it won't be lost because do_vm_stop() doesn't allow qemu to be stopped > > twice. > > What happens then? The user sees an error? It's ignored.
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/08/2011 03:42 AM, Shribman, Aidan wrote: Subject: [PATCH v4] XBZRLE delta for live migration of large memory apps From: Aidan Shribman By using XBZRLE (Xor Binary Zero Run-Length-Encoding) we can reduce VM downtime and total live-migration time of VMs running memory write intensive workloads typical of large enterprise applications such as SAP ERP Systems, and generally speaking for any application with a sparse memory update pattern. On the sender side XBZRLE is used as a compact delta encoding of page updates, retrieving the old page content from an LRU cache (default size of 64 MB). The receiving side uses the existing page content and XBZRLE to decode the new page content. Work was originally based on research results published VEE 2011: Evaluation of Delta Compression Techniques for Efficient Live Migration of Large Virtual Machines by Benoit, Svard, Tordsson and Elmroth. Additionally the delta encoder XBRLE was improved further using XBZRLE instead. XBZRLE has a sustained bandwidth of 2-2.5 GB/s for typical workloads making it ideal for in-line, real-time encoding such as is needed for live-migration. A typical usage scenario: {qemu} migrate_set_cachesize 256m {qemu} migrate -x -d tcp:destination.host: {qemu} info migrate ... transferred ram-duplicate: A kbytes transferred ram-duplicate: B pages transferred ram-normal: C kbytes transferred ram-normal: D pages transferred ram-xbrle: E kbytes transferred ram-xbrle: F pages overflow ram-xbrle: G pages cache-hit ram-xbrle: H pages cache-lookup ram-xbrle: J pages Testing: live migration with XBZRLE completed in 110 seconds, without live migration was not able to complete. A simple synthetic memory r/w load generator: ..include ..include ..int main() ..{ ..char *buf = (char *) calloc(4096, 4096); ..while (1) { ..int i; ..for (i = 0; i< 4096 * 4; i++) { ..buf[i * 4096 / 4]++; ..} ..printf("."); ..} ..} Signed-off-by: Benoit Hudzia Signed-off-by: Petter Svard Signed-off-by: Aidan Shribman One thing that strikes me about this algorithm is that it's very good for a particular type of workload--shockingly good really. I think workload aware migration compression is possible for a lot of different types of workloads. That makes me a bit wary of QEMU growing quite a lot of compression mechanisms. It makes me think that this logic may really belong at a higher level where more information is known about the workload. For instance, I can imagine XBZRLE living in something like libvirt. Today, parsing migration traffic is pretty horrible but I think we're pretty strongly committed to fixing that in 1.0. That makes me wonder if it would be nicer architecturally for a higher level tool to own something like this. Originally, when I added migration, I had the view that we would have transport plugins based on the exec: protocol. That hasn't really happened since libvirt really owns migration but I think having XBZRLE as a transport plugin for libvirt is something worth considering. I'm curious what people think about this type of approach. CC'ing libvirt to get their input. Regards, Anthony Liguori -- Makefile.target |1 + arch_init.c | 351 ++-- block-migration.c |3 +- hash.h| 72 +++ hmp-commands.hx | 36 -- hw/hw.h |3 +- lru.c | 142 + lru.h | 13 ++ migration-exec.c |6 +- migration-fd.c|6 +- migration-tcp.c |6 +- migration-unix.c |6 +- migration.c | 119 +- migration.h | 25 +++- qmp-commands.hx | 43 ++- savevm.c | 13 ++- sysemu.h | 13 ++- xbzrle.c | 126 +++ xbzrle.h | 12 ++ 19 files changed, 917 insertions(+), 79 deletions(-) diff --git a/Makefile.target b/Makefile.target index 2800f47..b3215de 100644 --- a/Makefile.target +++ b/Makefile.target @@ -186,6 +186,7 @@ endif #CONFIG_BSD_USER ifdef CONFIG_SOFTMMU obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o +obj-y += lru.o xbzrle.o # virtio has to be here due to weird dependency between PCI and virtio-net. # need to fix this properly obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o diff --git a/arch_init.c b/arch_init.c old mode 100644 new mode 100755 index 4486925..d67dc82 --- a/arch_init.c +++ b/arch_init.c @@ -40,6 +40,17 @@ #include "net.h" #include "gdbstub.h" #include "hw/smbios.h" +#include "lru.h" +#include "xbzrle.h" + +//#define DEBUG_ARCH_INIT +#ifdef DEBUG_ARCH_INIT +#define DPRINTF(fmt, ...) \ +do { fprintf(stdout, "arch_init: " fmt, ## __VA_ARGS__); } while (0) +#else +#define DPRINTF(fmt, ...) \ +do { } wh
[Qemu-devel] [PATCH v4 03/39] vmsvga: don't remember pci BAR address in callback any more
We're going to remove the callback, so we can't use it to save the address. Use the pci API instead. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/vmware_vga.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 354c221..190b005 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -52,8 +52,6 @@ struct vmsvga_state_s { int on; } cursor; -target_phys_addr_t vram_base; - int index; int scratch_size; uint32_t *scratch; @@ -761,8 +759,11 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t address) case SVGA_REG_BYTES_PER_LINE: return ((s->depth + 7) >> 3) * s->new_width; -case SVGA_REG_FB_START: -return s->vram_base; +case SVGA_REG_FB_START: { +struct pci_vmsvga_state_s *pci_vmsvga += container_of(s, struct pci_vmsvga_state_s, chip); +return pci_get_bar_addr(&pci_vmsvga->card, 1); +} case SVGA_REG_FB_OFFSET: return 0x0; @@ -1247,14 +1248,13 @@ static void pci_vmsvga_map_mem(PCIDevice *pci_dev, int region_num, struct vmsvga_state_s *s = &d->chip; ram_addr_t iomemtype; -s->vram_base = addr; #ifdef DIRECT_VRAM iomemtype = cpu_register_io_memory(vmsvga_vram_read, vmsvga_vram_write, s, DEVICE_NATIVE_ENDIAN); #else iomemtype = s->vga.vram_offset | IO_MEM_RAM; #endif -cpu_register_physical_memory(s->vram_base, s->vga.vram_size, +cpu_register_physical_memory(addr, s->vga.vram_size, iomemtype); s->vga.map_addr = addr; -- 1.7.5.3
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On 08/08/2011 04:28 PM, Luiz Capitulino wrote: On Mon, 08 Aug 2011 16:27:05 +0300 Avi Kivity wrote: > On 08/08/2011 04:25 PM, Luiz Capitulino wrote: > > > > > > I'm worried about the following race: > > > > > > stop > > > (qemu stopped for internal reason) > > > stop comment processed > > > > > > resume > > > > > > The (qemu stopped for internal reason) part is lost. > > > > If the "stop" you're referring to happens through vm_stop(), then no, > > it won't be lost because do_vm_stop() doesn't allow qemu to be stopped > > twice. > > What happens then? The user sees an error? It's ignored. Well, then, the user won't know something happened and will happily resume the guest, like I outlined above. When you ignore something in the first set, something breaks in the third. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08.08.2011, at 15:29, Anthony Liguori wrote: > On 08/08/2011 03:42 AM, Shribman, Aidan wrote: >> Subject: [PATCH v4] XBZRLE delta for live migration of large memory apps >> From: Aidan Shribman >> >> By using XBZRLE (Xor Binary Zero Run-Length-Encoding) we can reduce VM >> downtime >> and total live-migration time of VMs running memory write intensive workloads >> typical of large enterprise applications such as SAP ERP Systems, and >> generally >> speaking for any application with a sparse memory update pattern. >> >> On the sender side XBZRLE is used as a compact delta encoding of page >> updates, >> retrieving the old page content from an LRU cache (default size of 64 MB). >> The >> receiving side uses the existing page content and XBZRLE to decode the new >> page >> content. >> >> Work was originally based on research results published VEE 2011: Evaluation >> of >> Delta Compression Techniques for Efficient Live Migration of Large Virtual >> Machines by Benoit, Svard, Tordsson and Elmroth. Additionally the delta >> encoder >> XBRLE was improved further using XBZRLE instead. >> >> XBZRLE has a sustained bandwidth of 2-2.5 GB/s for typical workloads making >> it >> ideal for in-line, real-time encoding such as is needed for live-migration. >> >> A typical usage scenario: >> {qemu} migrate_set_cachesize 256m >> {qemu} migrate -x -d tcp:destination.host: >> {qemu} info migrate >> ... >> transferred ram-duplicate: A kbytes >> transferred ram-duplicate: B pages >> transferred ram-normal: C kbytes >> transferred ram-normal: D pages >> transferred ram-xbrle: E kbytes >> transferred ram-xbrle: F pages >> overflow ram-xbrle: G pages >> cache-hit ram-xbrle: H pages >> cache-lookup ram-xbrle: J pages >> >> Testing: live migration with XBZRLE completed in 110 seconds, without live >> migration was not able to complete. >> >> A simple synthetic memory r/w load generator: >> ..include >> ..include >> ..int main() >> ..{ >> ..char *buf = (char *) calloc(4096, 4096); >> ..while (1) { >> ..int i; >> ..for (i = 0; i< 4096 * 4; i++) { >> ..buf[i * 4096 / 4]++; >> ..} >> ..printf("."); >> ..} >> ..} >> >> Signed-off-by: Benoit Hudzia >> Signed-off-by: Petter Svard >> Signed-off-by: Aidan Shribman > > One thing that strikes me about this algorithm is that it's very good for a > particular type of workload--shockingly good really. > > I think workload aware migration compression is possible for a lot of > different types of workloads. That makes me a bit wary of QEMU growing quite > a lot of compression mechanisms. > > It makes me think that this logic may really belong at a higher level where > more information is known about the workload. For instance, I can imagine > XBZRLE living in something like libvirt. > > Today, parsing migration traffic is pretty horrible but I think we're pretty > strongly committed to fixing that in 1.0. That makes me wonder if it would > be nicer architecturally for a higher level tool to own something like this. > > Originally, when I added migration, I had the view that we would have > transport plugins based on the exec: protocol. That hasn't really happened > since libvirt really owns migration but I think having XBZRLE as a transport > plugin for libvirt is something worth considering. > > I'm curious what people think about this type of approach. CC'ing libvirt to > get their input. In general, I believe it's a good idea to keep looking at libvirt as a vm management layer and only a vm management layer. Directly working with the migration protocol basically ties us to libvirt if we want to do migration, killing competition in the management stack. Just look at how xm is tied to xen - it's one of the major points I dislike about it :). Alex
[Qemu-devel] [PATCH v4 16/39] eepro100: convert to memory API
Note: the existing code aliases the flash BAR into the MMIO bar. This is probably a bug. This patch does not correct the problem. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/eepro100.c | 182 - 1 files changed, 37 insertions(+), 145 deletions(-) diff --git a/hw/eepro100.c b/hw/eepro100.c index 9b6f4a5..04723f3 100644 --- a/hw/eepro100.c +++ b/hw/eepro100.c @@ -228,13 +228,14 @@ typedef struct { PCIDevice dev; /* Hash register (multicast mask array, multiple individual addresses). */ uint8_t mult[8]; -int mmio_index; +MemoryRegion mmio_bar; +MemoryRegion io_bar; +MemoryRegion flash_bar; NICState *nic; NICConf conf; uint8_t scb_stat; /* SCB stat/ack byte */ uint8_t int_stat; /* PCI interrupt status */ /* region must not be saved by nic_save. */ -uint32_t region1; /* PCI region 1 address */ uint16_t mdimem[32]; eeprom_t *eeprom; uint32_t device;/* device variant */ @@ -1584,147 +1585,36 @@ static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val) } } -/* - * - * Port mapped I/O. - * - / - -static uint32_t ioport_read1(void *opaque, uint32_t addr) -{ -EEPRO100State *s = opaque; -#if 0 -logout("addr=%s\n", regname(addr)); -#endif -return eepro100_read1(s, addr - s->region1); -} - -static uint32_t ioport_read2(void *opaque, uint32_t addr) -{ -EEPRO100State *s = opaque; -return eepro100_read2(s, addr - s->region1); -} - -static uint32_t ioport_read4(void *opaque, uint32_t addr) -{ -EEPRO100State *s = opaque; -return eepro100_read4(s, addr - s->region1); -} - -static void ioport_write1(void *opaque, uint32_t addr, uint32_t val) -{ -EEPRO100State *s = opaque; -#if 0 -logout("addr=%s val=0x%02x\n", regname(addr), val); -#endif -eepro100_write1(s, addr - s->region1, val); -} - -static void ioport_write2(void *opaque, uint32_t addr, uint32_t val) -{ -EEPRO100State *s = opaque; -eepro100_write2(s, addr - s->region1, val); -} - -static void ioport_write4(void *opaque, uint32_t addr, uint32_t val) -{ -EEPRO100State *s = opaque; -eepro100_write4(s, addr - s->region1, val); -} - -/***/ -/* PCI EEPRO100 definitions */ - -static void pci_map(PCIDevice * pci_dev, int region_num, -pcibus_t addr, pcibus_t size, int type) -{ -EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev); - -TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", " - "size=0x%08"FMT_PCIBUS", type=%d\n", - region_num, addr, size, type)); - -assert(region_num == 1); -register_ioport_write(addr, size, 1, ioport_write1, s); -register_ioport_read(addr, size, 1, ioport_read1, s); -register_ioport_write(addr, size, 2, ioport_write2, s); -register_ioport_read(addr, size, 2, ioport_read2, s); -register_ioport_write(addr, size, 4, ioport_write4, s); -register_ioport_read(addr, size, 4, ioport_read4, s); - -s->region1 = addr; -} - -/* - * - * Memory mapped I/O. - * - / - -static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) -{ -EEPRO100State *s = opaque; -#if 0 -logout("addr=%s val=0x%02x\n", regname(addr), val); -#endif -eepro100_write1(s, addr, val); -} - -static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) +static uint64_t eepro100_read(void *opaque, target_phys_addr_t addr, + unsigned size) { EEPRO100State *s = opaque; -#if 0 -logout("addr=%s val=0x%02x\n", regname(addr), val); -#endif -eepro100_write2(s, addr, val); -} -static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) -{ -EEPRO100State *s = opaque; -#if 0 -logout("addr=%s val=0x%02x\n", regname(addr), val); -#endif -eepro100_write4(s, addr, val); -} - -static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr) -{ -EEPRO100State *s = opaque; -#if 0 -logout("addr=%s\n", regname(addr)); -#endif -return eepro100_read1(s, addr); +switch (size) { +case 1: return eepro100_read1(s, addr); +case 2: return eepro100_read2(s, addr); +case 4: return eepro100_read4(s, addr); +default: abort(); +} } -static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr) +static void eepro100_write(void *opaque, target_phys_addr_t addr, + uint64_t data, unsigned size) { EEPRO100State *s = opaque; -#if 0 -logout("addr=%s\n", regname(addr)); -#e
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/08/2011 08:41 AM, Alexander Graf wrote: On 08.08.2011, at 15:29, Anthony Liguori wrote: One thing that strikes me about this algorithm is that it's very good for a particular type of workload--shockingly good really. I think workload aware migration compression is possible for a lot of different types of workloads. That makes me a bit wary of QEMU growing quite a lot of compression mechanisms. It makes me think that this logic may really belong at a higher level where more information is known about the workload. For instance, I can imagine XBZRLE living in something like libvirt. Today, parsing migration traffic is pretty horrible but I think we're pretty strongly committed to fixing that in 1.0. That makes me wonder if it would be nicer architecturally for a higher level tool to own something like this. Originally, when I added migration, I had the view that we would have transport plugins based on the exec: protocol. That hasn't really happened since libvirt really owns migration but I think having XBZRLE as a transport plugin for libvirt is something worth considering. I'm curious what people think about this type of approach. CC'ing libvirt to get their input. In general, I believe it's a good idea to keep looking at libvirt as a vm management layer and only a vm management layer. Directly working with the migration protocol basically ties us to libvirt if we want to do migration, killing competition in the management stack. Just look at how xm is tied to xen - it's one of the major points I dislike about it :). The way I originally envisioned things, you'd have: (qemu) migrate xbzrle://destination?opt1=value1&opt2=value2 Which would in turn be equivalent to: (qemu) migrate exec:///usr/libexec/qemu/migration-helper-xbzrle --opt1=value1 --opt2=value2 But even if we supported that, it wouldn't get exposed via libvirt unless the libvirt guys exposed QEMU URIs directly. So I think the open question is, how do we do transport plugins in a way that makes libvirt and QEMU both happy? Regards, Anthony Liguori Alex
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On Mon, 08 Aug 2011 16:40:17 +0300 Avi Kivity wrote: > On 08/08/2011 04:28 PM, Luiz Capitulino wrote: > > On Mon, 08 Aug 2011 16:27:05 +0300 > > Avi Kivity wrote: > > > > > On 08/08/2011 04:25 PM, Luiz Capitulino wrote: > > > > > > > > > > I'm worried about the following race: > > > > > > > > > > stop > > > > > (qemu stopped for internal reason) > > > > > stop comment processed > > > > > > > > > > resume > > > > > > > > > > The (qemu stopped for internal reason) part is lost. > > > > > > > > If the "stop" you're referring to happens through vm_stop(), then no, > > > > it won't be lost because do_vm_stop() doesn't allow qemu to be stopped > > > > twice. > > > > > > What happens then? The user sees an error? > > > > It's ignored. > > Well, then, the user won't know something happened and will happily > resume the guest, like I outlined above. I think it makes sense to return an error in the monitor if the user tries to stop qemu when it's already stopped. Not sure if it will do what you think it should do, but we should always tell the user when we're unable to carry his/her orders. But it does make sense to me to not allow stopping twice. First because it doesn't make sense to stop something which is not moving and second because what else can stop the vm if it's already stopped? Maybe vm_stop() should return an error, but I think this goes beyond this series. > > When you ignore something in the first set, something breaks in the third. >
Re: [Qemu-devel] [PATCH] qemu-img: Use qemu_blockalign
On Mon, Aug 8, 2011 at 1:14 PM, Kevin Wolf wrote: > Now that you can use cache=none for the output file in qemu-img, we should > properly align our buffers so that raw-posix doesn't have to use its (smaller) > bounce buffer. > > Signed-off-by: Kevin Wolf > --- > qemu-img.c | 12 ++-- > 1 files changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Stefan Hajnoczi
[Qemu-devel] [PATCH v4 26/39] pcnet: convert to memory API
Also related chips. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/lance.c | 31 ++- hw/pcnet-pci.c | 74 +-- hw/pcnet.h |4 ++- 3 files changed, 61 insertions(+), 48 deletions(-) diff --git a/hw/lance.c b/hw/lance.c index ddb1cbb..8e20360 100644 --- a/hw/lance.c +++ b/hw/lance.c @@ -55,8 +55,8 @@ static void parent_lance_reset(void *opaque, int irq, int level) pcnet_h_reset(&d->state); } -static void lance_mem_writew(void *opaque, target_phys_addr_t addr, - uint32_t val) +static void lance_mem_write(void *opaque, target_phys_addr_t addr, +uint64_t val, unsigned size) { SysBusPCNetState *d = opaque; @@ -64,7 +64,8 @@ static void lance_mem_writew(void *opaque, target_phys_addr_t addr, pcnet_ioport_writew(&d->state, addr, val & 0x); } -static uint32_t lance_mem_readw(void *opaque, target_phys_addr_t addr) +static uint64_t lance_mem_read(void *opaque, target_phys_addr_t addr, + unsigned size) { SysBusPCNetState *d = opaque; uint32_t val; @@ -74,16 +75,14 @@ static uint32_t lance_mem_readw(void *opaque, target_phys_addr_t addr) return val & 0x; } -static CPUReadMemoryFunc * const lance_mem_read[3] = { -NULL, -lance_mem_readw, -NULL, -}; - -static CPUWriteMemoryFunc * const lance_mem_write[3] = { -NULL, -lance_mem_writew, -NULL, +static const MemoryRegionOps lance_mem_ops = { +.read = lance_mem_read, +.write = lance_mem_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 2, +.max_access_size = 2, +}, }; static void lance_cleanup(VLANClientState *nc) @@ -117,13 +116,11 @@ static int lance_init(SysBusDevice *dev) SysBusPCNetState *d = FROM_SYSBUS(SysBusPCNetState, dev); PCNetState *s = &d->state; -s->mmio_index = -cpu_register_io_memory(lance_mem_read, lance_mem_write, d, - DEVICE_NATIVE_ENDIAN); +memory_region_init_io(&s->mmio, &lance_mem_ops, s, "lance-mmio", 4); qdev_init_gpio_in(&dev->qdev, parent_lance_reset, 1); -sysbus_init_mmio(dev, 4, s->mmio_index); +sysbus_init_mmio_region(dev, &s->mmio); sysbus_init_irq(dev, &s->irq); diff --git a/hw/pcnet-pci.c b/hw/pcnet-pci.c index 216cf81..a25f565 100644 --- a/hw/pcnet-pci.c +++ b/hw/pcnet-pci.c @@ -46,6 +46,7 @@ typedef struct { PCIDevice pci_dev; PCNetState state; +MemoryRegion io_bar; } PCIPCNetState; static void pcnet_aprom_writeb(void *opaque, uint32_t addr, uint32_t val) @@ -69,25 +70,41 @@ static uint32_t pcnet_aprom_readb(void *opaque, uint32_t addr) return val; } -static void pcnet_ioport_map(PCIDevice *pci_dev, int region_num, - pcibus_t addr, pcibus_t size, int type) +static uint64_t pcnet_ioport_read(void *opaque, target_phys_addr_t addr, + unsigned size) { -PCNetState *d = &DO_UPCAST(PCIPCNetState, pci_dev, pci_dev)->state; +PCNetState *d = opaque; -#ifdef PCNET_DEBUG_IO -printf("pcnet_ioport_map addr=0x%04"FMT_PCIBUS" size=0x%04"FMT_PCIBUS"\n", - addr, size); -#endif +if (addr < 16 && size == 1) { +return pcnet_aprom_readb(d, addr); +} else if (addr >= 0x10 && addr < 0x20 && size == 2) { +return pcnet_ioport_readw(d, addr); +} else if (addr >= 0x10 && addr < 0x20 && size == 4) { +return pcnet_ioport_readl(d, addr); +} +return ((uint64_t)1 << (size * 8)) - 1; +} -register_ioport_write(addr, 16, 1, pcnet_aprom_writeb, d); -register_ioport_read(addr, 16, 1, pcnet_aprom_readb, d); +static void pcnet_ioport_write(void *opaque, target_phys_addr_t addr, + uint64_t data, unsigned size) +{ +PCNetState *d = opaque; -register_ioport_write(addr + 0x10, 0x10, 2, pcnet_ioport_writew, d); -register_ioport_read(addr + 0x10, 0x10, 2, pcnet_ioport_readw, d); -register_ioport_write(addr + 0x10, 0x10, 4, pcnet_ioport_writel, d); -register_ioport_read(addr + 0x10, 0x10, 4, pcnet_ioport_readl, d); +if (addr < 16 && size == 1) { +return pcnet_aprom_writeb(d, addr, data); +} else if (addr >= 0x10 && addr < 0x20 && size == 2) { +return pcnet_ioport_writew(d, addr, data); +} else if (addr >= 0x10 && addr < 0x20 && size == 4) { +return pcnet_ioport_writel(d, addr, data); +} } +static const MemoryRegionOps pcnet_io_ops = { +.read = pcnet_ioport_read, +.write = pcnet_ioport_write, +.endianness = DEVICE_NATIVE_ENDIAN, +}; + static void pcnet_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) { PCNetState *d = opaque; @@ -202,16 +219,12 @@ static const VMStateDescription vmstate_pci_pcnet = { /* PCI interface */ -static CPUWriteMemoryFun
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/08/2011 04:41 PM, Alexander Graf wrote: In general, I believe it's a good idea to keep looking at libvirt as a vm management layer and only a vm management layer. Very much yes. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/08/2011 04:29 PM, Anthony Liguori wrote: One thing that strikes me about this algorithm is that it's very good for a particular type of workload--shockingly good really. Poking bytes at random places in memory is fairly generic. If you have a lot of small objects, and modify a subset of them, this is the pattern you get. I think workload aware migration compression is possible for a lot of different types of workloads. That makes me a bit wary of QEMU growing quite a lot of compression mechanisms. It makes me think that this logic may really belong at a higher level where more information is known about the workload. For instance, I can imagine XBZRLE living in something like libvirt. A better model would be plugin based. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On 08/08/2011 04:47 PM, Luiz Capitulino wrote: > > Well, then, the user won't know something happened and will happily > resume the guest, like I outlined above. I think it makes sense to return an error in the monitor if the user tries to stop qemu when it's already stopped. Not sure if it will do what you think it should do, but we should always tell the user when we're unable to carry his/her orders. But it does make sense to me to not allow stopping twice. First because it doesn't make sense to stop something which is not moving and second because what else can stop the vm if it's already stopped? Maybe vm_stop() should return an error, but I think this goes beyond this series. This is why I suggested a reference count. In this case, we can always stop the guest "twice", because we don't lost information when we resume. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v4 13/39] rtl8139: convert to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/rtl8139.c | 72 ++--- 1 files changed, 38 insertions(+), 34 deletions(-) diff --git a/hw/rtl8139.c b/hw/rtl8139.c index 5214b8c..f07af35 100644 --- a/hw/rtl8139.c +++ b/hw/rtl8139.c @@ -474,7 +474,6 @@ typedef struct RTL8139State { NICState *nic; NICConf conf; -int rtl8139_mmio_io_addr; /* C ring mode */ uint32_t currTxDesc; @@ -506,6 +505,9 @@ typedef struct RTL8139State { QEMUTimer *timer; int64_t TimerExpire; +MemoryRegion bar_io; +MemoryRegion bar_mem; + /* Support migration to/from old versions */ int rtl8139_mmio_io_addr_dummy; } RTL8139State; @@ -3283,7 +3285,7 @@ static void rtl8139_pre_save(void *opaque) rtl8139_set_next_tctr_time(s, current_time); s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY, get_ticks_per_sec()); -s->rtl8139_mmio_io_addr_dummy = s->rtl8139_mmio_io_addr; +s->rtl8139_mmio_io_addr_dummy = 0; } static const VMStateDescription vmstate_rtl8139 = { @@ -3379,31 +3381,35 @@ static const VMStateDescription vmstate_rtl8139 = { /***/ /* PCI RTL8139 definitions */ -static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num, - pcibus_t addr, pcibus_t size, int type) -{ -RTL8139State *s = DO_UPCAST(RTL8139State, dev, pci_dev); - -register_ioport_write(addr, 0x100, 1, rtl8139_ioport_writeb, s); -register_ioport_read( addr, 0x100, 1, rtl8139_ioport_readb, s); - -register_ioport_write(addr, 0x100, 2, rtl8139_ioport_writew, s); -register_ioport_read( addr, 0x100, 2, rtl8139_ioport_readw, s); - -register_ioport_write(addr, 0x100, 4, rtl8139_ioport_writel, s); -register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl, s); -} +static const MemoryRegionPortio rtl8139_portio[] = { +{ 0, 0x100, 1, .read = rtl8139_ioport_readb, }, +{ 0, 0x100, 1, .write = rtl8139_ioport_writeb, }, +{ 0, 0x100, 2, .read = rtl8139_ioport_readw, }, +{ 0, 0x100, 2, .write = rtl8139_ioport_writew, }, +{ 0, 0x100, 4, .read = rtl8139_ioport_readl, }, +{ 0, 0x100, 4, .write = rtl8139_ioport_writel, }, +PORTIO_END_OF_LIST() +}; -static CPUReadMemoryFunc * const rtl8139_mmio_read[3] = { -rtl8139_mmio_readb, -rtl8139_mmio_readw, -rtl8139_mmio_readl, +static const MemoryRegionOps rtl8139_io_ops = { +.old_portio = rtl8139_portio, +.endianness = DEVICE_LITTLE_ENDIAN, }; -static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = { -rtl8139_mmio_writeb, -rtl8139_mmio_writew, -rtl8139_mmio_writel, +static const MemoryRegionOps rtl8139_mmio_ops = { +.old_mmio = { +.read = { +rtl8139_mmio_readb, +rtl8139_mmio_readw, +rtl8139_mmio_readl, +}, +.write = { +rtl8139_mmio_writeb, +rtl8139_mmio_writew, +rtl8139_mmio_writel, +}, +}, +.endianness = DEVICE_LITTLE_ENDIAN, }; static void rtl8139_timer(void *opaque) @@ -3432,7 +3438,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev) { RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev); -cpu_unregister_io_memory(s->rtl8139_mmio_io_addr); +memory_region_destroy(&s->bar_io); +memory_region_destroy(&s->bar_mem); if (s->cplus_txbuffer) { qemu_free(s->cplus_txbuffer); s->cplus_txbuffer = NULL; @@ -3462,15 +3469,12 @@ static int pci_rtl8139_init(PCIDevice *dev) * list bit in status register, and offset 0xdc seems unused. */ pci_conf[PCI_CAPABILITY_LIST] = 0xdc; -/* I/O handler for memory-mapped I/O */ -s->rtl8139_mmio_io_addr = -cpu_register_io_memory(rtl8139_mmio_read, rtl8139_mmio_write, s, - DEVICE_LITTLE_ENDIAN); - -pci_register_bar(&s->dev, 0, 0x100, - PCI_BASE_ADDRESS_SPACE_IO, rtl8139_ioport_map); - -pci_register_bar_simple(&s->dev, 1, 0x100, 0, s->rtl8139_mmio_io_addr); +memory_region_init_io(&s->bar_io, &rtl8139_io_ops, s, "rtl8139", 0x100); +memory_region_init_io(&s->bar_mem, &rtl8139_mmio_ops, s, "rtl8139", 0x100); +pci_register_bar_region(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, +&s->bar_io); +pci_register_bar_region(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, +&s->bar_mem); qemu_macaddr_default_if_unset(&s->conf.macaddr); -- 1.7.5.3
Re: [Qemu-devel] support for Freescale MPC8xx (850/860) processors/platforms
On 08.08.2011, at 03:36, Brendan Simon (eTRIX) wrote: > Hi, > > Anyone working on Freescale MPC8xx (embedded PowerPC) processors ?? > > I'm trying to ascertain if the MPC8xx (MPC850) processors are supported, and > if not, how much effort would be required to get it working. I'm not aware of anyone working on MPC8xx CPU support. In fact, I don't really know what they look like - I guess they're 601 derived? Ben, any idea how much different 8xx is? Alex > > Thanks for any help. > Brendan. > > > On 5/08/11 9:31 PM, Brendan Simon (eTRIX) wrote: >> >> Hello, >> >> Does QEMU support the Freescale MPC8xx (MPC850) processors or platforms (e.g >> FADS860) ?? >> >> Googling shows some code that suggests that it is not supported. >> > cpu_abort(env, "MPC8xx MMU model is not implemented\n"); >> >> If not supported, does anyone have any idea how much work there would be to >> add support for MPC850/860 and to create a platform that has supports DRAM, >> Flash, the CPM peripherals (SMC/USARTS, SCC/UARTS, SCC/Ethernet, BRG timers, >> etc) ?? >> >> The platform I would like to build has 1 x Ethernet (using SCC2), 1 x UART >> (on SMC1), 1 x UART (on SCC3), 1 x UART (external UART chip), and an Altera >> CPLD with digital I/O. >> >> Are there any other similar platforms I could look at, to either port or use >> as a reference, to build an MPC850 platform as described ?? >> >> Thanks, >> Brendan. >> >
[Qemu-devel] [PATCH v4 05/39] cirrus: simplify mmio BAR access functions
Make use of the memory API's ability to satisfy multi-byte accesses via multiple single-byte accesses. Reviewed-by: Richard Henderson Signed-off-by: Avi Kivity --- hw/cirrus_vga.c | 78 +- 1 files changed, 8 insertions(+), 70 deletions(-) diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index ad23c4a..4f57b92 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -2827,12 +2827,11 @@ static void cirrus_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val) * ***/ -static uint32_t cirrus_mmio_readb(void *opaque, target_phys_addr_t addr) +static uint64_t cirrus_mmio_read(void *opaque, target_phys_addr_t addr, + unsigned size) { CirrusVGAState *s = opaque; -addr &= CIRRUS_PNPMMIO_SIZE - 1; - if (addr >= 0x100) { return cirrus_mmio_blt_read(s, addr - 0x100); } else { @@ -2840,33 +2839,11 @@ static uint32_t cirrus_mmio_readb(void *opaque, target_phys_addr_t addr) } } -static uint32_t cirrus_mmio_readw(void *opaque, target_phys_addr_t addr) -{ -uint32_t v; - -v = cirrus_mmio_readb(opaque, addr); -v |= cirrus_mmio_readb(opaque, addr + 1) << 8; -return v; -} - -static uint32_t cirrus_mmio_readl(void *opaque, target_phys_addr_t addr) -{ -uint32_t v; - -v = cirrus_mmio_readb(opaque, addr); -v |= cirrus_mmio_readb(opaque, addr + 1) << 8; -v |= cirrus_mmio_readb(opaque, addr + 2) << 16; -v |= cirrus_mmio_readb(opaque, addr + 3) << 24; -return v; -} - -static void cirrus_mmio_writeb(void *opaque, target_phys_addr_t addr, - uint32_t val) +static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned size) { CirrusVGAState *s = opaque; -addr &= CIRRUS_PNPMMIO_SIZE - 1; - if (addr >= 0x100) { cirrus_mmio_blt_write(s, addr - 0x100, val); } else { @@ -2874,53 +2851,14 @@ static void cirrus_mmio_writeb(void *opaque, target_phys_addr_t addr, } } -static void cirrus_mmio_writew(void *opaque, target_phys_addr_t addr, - uint32_t val) -{ -cirrus_mmio_writeb(opaque, addr, val & 0xff); -cirrus_mmio_writeb(opaque, addr + 1, (val >> 8) & 0xff); -} - -static void cirrus_mmio_writel(void *opaque, target_phys_addr_t addr, - uint32_t val) -{ -cirrus_mmio_writeb(opaque, addr, val & 0xff); -cirrus_mmio_writeb(opaque, addr + 1, (val >> 8) & 0xff); -cirrus_mmio_writeb(opaque, addr + 2, (val >> 16) & 0xff); -cirrus_mmio_writeb(opaque, addr + 3, (val >> 24) & 0xff); -} - - -static uint64_t cirrus_mmio_read(void *opaque, target_phys_addr_t addr, - unsigned size) -{ -CirrusVGAState *s = opaque; - -switch (size) { -case 1: return cirrus_mmio_readb(s, addr); -case 2: return cirrus_mmio_readw(s, addr); -case 4: return cirrus_mmio_readl(s, addr); -default: abort(); -} -}; - -static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr, - uint64_t data, unsigned size) -{ -CirrusVGAState *s = opaque; - -switch (size) { -case 1: return cirrus_mmio_writeb(s, addr, data); -case 2: return cirrus_mmio_writew(s, addr, data); -case 4: return cirrus_mmio_writel(s, addr, data); -default: abort(); -} -}; - static const MemoryRegionOps cirrus_mmio_io_ops = { .read = cirrus_mmio_read, .write = cirrus_mmio_write, .endianness = DEVICE_LITTLE_ENDIAN, +.impl = { +.min_access_size = 1, +.max_access_size = 1, +}, }; /* load/save state */ -- 1.7.5.3
Re: [Qemu-devel] [PATCH v3] XBZRLE delta for live migration of large memory apps
On Mon, Aug 8, 2011 at 9:42 AM, Shribman, Aidan wrote: >> -Original Message- >> From: Stefan Hajnoczi [mailto:stefa...@gmail.com] >> Sent: Tuesday, August 02, 2011 9:06 PM >> To: Shribman, Aidan >> Cc: qemu-devel@nongnu.org; Anthony Liguori >> Subject: Re: [PATCH v3] XBZRLE delta for live migration of >> large memory apps >> >> On Tue, Aug 02, 2011 at 03:45:56PM +0200, Shribman, Aidan wrote: >> > Subject: [PATCH v3] XBZRLE delta for live migration of >> large memory apps >> > From: Aidan Shribman >> > >> > By using XBZRLE (Xor Binary Zero Run-Length-Encoding) we >> can reduce VM downtime >> > and total live-migration time for VMs running memory write >> intensive workloads >> > typical of large enterprise applications such as SAP ERP >> Systems, and generally >> > speaking for representative of any application with a >> sparse memory update pattern. >> > >> > On the sender side XBZRLE is used as a compact delta >> encoding of page updates, >> > retrieving the old page content from an LRU cache (default >> size of 64 MB). The >> > receiving side uses the existing page content and XBZRLE to >> decode the new page >> > content. >> > >> > Work was originally based on research results published VEE >> 2011: Evaluation of >> > Delta Compression Techniques for Efficient Live Migration >> of Large Virtual >> > Machines by Benoit, Svard, Tordsson and Elmroth. >> Additionally the delta encoder >> > XBRLE was improved further using XBZRLE instead. >> > >> > XBZRLE has a sustained bandwidth of 1.5-2.2 GB/s for >> typical workloads making it >> > ideal for in-line, real-time encoding such as is needed for >> live-migration. >> >> What is the CPU cost of xbzrle live migration on the source host? I'm >> thinking about a graph showing CPU utilization (e.g. from mpstat(1)) >> that has two datasets: migration without xbzrle and migration with >> xbzrle. >> > > zbzrle.out indicates that xbzrle is using 50% of the compute capacity during > the xbzrle live-migration (which completed is few seconds), In vanilla.out > between 30%-60% of compute is directed toward the live-migration itself - in > this case live-migration is not able to complete. > > - > > root@ilrsh01:~# > root@ilrsh01:~# cat xbzrle.out > Linux 2.6.35-22-server (ilrsh01) 08/07/2011 _x86_64_ (2 > CPU) > > 10:55:37 AM CPU %usr %nice %sys %iowait %irq %soft %steal > %guest %idle > 10:55:38 AM all 40.50 0.00 1.00 1.50 0.00 9.00 0.00 > 0.00 48.00 > 10:55:38 AM 0 0.00 0.00 1.00 3.00 0.00 0.00 0.00 > 0.00 96.00 > 10:55:38 AM 1 81.00 0.00 1.00 0.00 0.00 18.00 0.00 > 0.00 0.00 Too bad mpstat %guest is not being displayed correctly here. That would make it much easier to see how much CPU is spent executing guest code and how much doing live migration. Old system? >> > +int xbzrle_encode(uint8_t *xbzrle, const uint8_t *old, >> const uint8_t *curr, >> > + const size_t max_compressed_len) >> > +{ >> > + int compressed_len; >> > + >> > + xor_encode_word(xor_buf, old, curr); >> > + compressed_len = rle_encode((uint64_t *)xor_buf, >> > + sizeof(xor_buf)/sizeof(uint64_t), xbzrle_buf, >> > + sizeof(xbzrle_buf)); >> > + if (compressed_len > max_compressed_len) { >> > + return -1; >> > + } >> > + memcpy(xbzrle, xbzrle_buf, compressed_len); >> >> Why the intermediate xbrzle_buf buffer and why the memcpy()? > > xbzrle encoding may take up to 150% in a rare worst case scenario - to avoid > having to check during each xbzrle iteration or alternatively adding a loop > that checks for overflow potential during the xbzrle encoding I use the > xbzrle_buf as working area. memcpy() is a factor faster than xbzrle so it's > slow-down is in-significant. I missed that the encode/decode functions do not check their dlen parameter. dlen is unused and should be removed. Stefan
[Qemu-devel] [PATCH v4 37/39] pci: fold BAR mapping function into its caller
There is only one function, so no need for a function pointer. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/pci.c | 25 + hw/pci.h |1 - 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 62b34d4..aa17395 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -881,18 +881,6 @@ static int pci_unregister_device(DeviceState *dev) return 0; } -static void pci_simple_bar_mapfunc_region(PCIDevice *pci_dev, int region_num, - pcibus_t addr, pcibus_t size, - int type) -{ -PCIIORegion *r = &pci_dev->io_regions[region_num]; - -memory_region_add_subregion_overlap(r->address_space, -addr, -r->memory, -1); -} - void pci_register_bar_region(PCIDevice *pci_dev, int region_num, uint8_t type, MemoryRegion *memory) { @@ -914,7 +902,6 @@ void pci_register_bar_region(PCIDevice *pci_dev, int region_num, r->size = size; r->filtered_size = size; r->type = type; -r->map_func = pci_simple_bar_mapfunc_region; r->memory = NULL; wmask = ~(size - 1); @@ -1102,10 +1089,16 @@ static void pci_update_mappings(PCIDevice *d) * addr & (size - 1) != 0. */ if (r->type & PCI_BASE_ADDRESS_SPACE_IO) { -r->map_func(d, i, r->addr, r->filtered_size, r->type); +memory_region_add_subregion_overlap(r->address_space, +r->addr, +r->memory, +1); } else { -r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr), -r->filtered_size, r->type); +memory_region_add_subregion_overlap(r->address_space, +pci_to_cpu_addr(d->bus, +r->addr), +r->memory, +1); } } } diff --git a/hw/pci.h b/hw/pci.h index 8028176..8d1662a 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -92,7 +92,6 @@ typedef struct PCIIORegion { pcibus_t size; pcibus_t filtered_size; uint8_t type; -PCIMapIORegionFunc *map_func; MemoryRegion *memory; MemoryRegion *address_space; } PCIIORegion; -- 1.7.5.3
Re: [Qemu-devel] [libvirt] [PATCH v4] XBZRLE delta for live migration of large memory apps
On Mon, Aug 08, 2011 at 08:29:51AM -0500, Anthony Liguori wrote: > On 08/08/2011 03:42 AM, Shribman, Aidan wrote: > >Subject: [PATCH v4] XBZRLE delta for live migration of large memory apps > >From: Aidan Shribman > > > >By using XBZRLE (Xor Binary Zero Run-Length-Encoding) we can reduce VM > >downtime > >and total live-migration time of VMs running memory write intensive workloads > >typical of large enterprise applications such as SAP ERP Systems, and > >generally > >speaking for any application with a sparse memory update pattern. [snip] > One thing that strikes me about this algorithm is that it's very > good for a particular type of workload--shockingly good really. > > I think workload aware migration compression is possible for a lot > of different types of workloads. That makes me a bit wary of QEMU > growing quite a lot of compression mechanisms. > > It makes me think that this logic may really belong at a higher > level where more information is known about the workload. For > instance, I can imagine XBZRLE living in something like libvirt. > > Today, parsing migration traffic is pretty horrible but I think > we're pretty strongly committed to fixing that in 1.0. That makes > me wonder if it would be nicer architecturally for a higher level > tool to own something like this. > > Originally, when I added migration, I had the view that we would > have transport plugins based on the exec: protocol. That hasn't > really happened since libvirt really owns migration but I think > having XBZRLE as a transport plugin for libvirt is something worth > considering. NB I've not been much of a fan of the exec: migration code, since it has proved rather buggy in practice when we used it for 'save/restore to/from file' support. It has been hard to diagnose when things go wrong, and difficult for QEMU to report any useful error messages. Even with the tcp: protocol, QEMU is seemingly unable to provide any useful error reporting even of things as simple as "unable to connect to remote host". So with one exception, current libvirt now uses the 'fd:' protocol for everything, and the last exception will be removed soon too. > I'm curious what people think about this type of approach. CC'ing > libvirt to get their input. In "normal" migration though, even when using fd:, we don't make any attempt to touch the data stream. We just pass a pre-connected TCP socket into QEMU and let it write directly to it. This avoids extra data copying via libvirt. In our alternative "tunnelled" migration mode, libvirt does touch the data stream, passing a pipe FD into QEMU, and copying the data from the pipe into packets to be sent over libvirtd's existing secure RPC stream, and then copying it back to QEMU on the destination. The downside here is that we've added several extra data copies. In our "save/restore to file" code, we use 'fd:' and always have to send the data via a filter program. For example, we have the ability to compress/decompress data via gzip, bzip, xz, and lzop, for which instead pass QEMU as pipe FD to the external compression helper program. We also have another new option where we send data via another I/O helper program that uses O_DIRECT, so save/restore does not pollute the page cache. With this kind of existing precedent, I won't strongly argue against libvirt adding a filter to support this XBZRLE encoding scheme for migration, or indeed save/restore too, if it proves better than lzop which is our current optimal speed/compression winner. My main concern with all these scenarios where libvirt touches the actual data stream though is that we're introducing extra data copies into the migration path which potentially waste CPU cycles. If QEMU can directly XBZRLE encode data into the FD passed via 'fd:' then we minimize data copies. Whether this is a big enough benefit to offset the burden of having to maintain various compression code options in QEMU I can't answer. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On Mon, 08 Aug 2011 16:54:16 +0300 Avi Kivity wrote: > On 08/08/2011 04:47 PM, Luiz Capitulino wrote: > > > > > > Well, then, the user won't know something happened and will happily > > > resume the guest, like I outlined above. > > > > I think it makes sense to return an error in the monitor if the user > > tries to stop qemu when it's already stopped. Not sure if it will do what > > you > > think it should do, but we should always tell the user when we're unable to > > carry his/her orders. > > > > But it does make sense to me to not allow stopping twice. First because it > > doesn't make sense to stop something which is not moving and second because > > what else can stop the vm if it's already stopped? > > > > Maybe vm_stop() should return an error, but I think this goes beyond this > > series. > > > > This is why I suggested a reference count. In this case, we can always > stop the guest "twice", because we don't lost information when we resume. I could give it a try in the near future, as I really think it's independent from this series, but I still don't understand what can stop an already stopped VM besides the user. This is a real question, is it really possible? If only the user can do that, then the refcount is overkill as just returning an error will do it.
Re: [Qemu-devel] support for Freescale MPC8xx (850/860) processors/platforms
On Mon, 2011-08-08 at 15:57 +0200, Alexander Graf wrote: > > Hi, > > > > Anyone working on Freescale MPC8xx (embedded PowerPC) processors ?? > > > > I'm trying to ascertain if the MPC8xx (MPC850) processors are > > supported, and if not, how much effort would be required to get it > > working. > > > > > I'm not aware of anyone working on MPC8xx CPU support. In fact, I > don't really know what they look like - I guess they're 601 derived? > > Ben, any idea how much different 8xx is? Gothic horrors :-) They have an MMU of their own. SW loaded TLB but different from anything else. They have a lot of other "quirks" too. Cheers, Ben.
Re: [Qemu-devel] [PATCH] Introduce short names for fixed width integer types
Am 08.08.2011 15:00, schrieb Anthony Liguori: > On 08/08/2011 07:56 AM, Avi Kivity wrote: >> QEMU deals with a lot of fixed width integer types; their names >> (uint64_t etc) are clumsy to use and take up a lot of space. >> >> Following Linux, introduce shorter names, for example U64 for >> uint64_t. > > Except Linux uses lower case letters. > > I personally think Linux style is wrong here. The int8_t types are > standard types. I fully agree, we should use the standard types. > Besides, we save lots of characters by using 4-space tabs instead of > 8-space tabs. We can afford to spend some of those saved characters on > using proper type names :-) Heh, I like this reasoning. :-) Kevin
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/08/2011 08:51 AM, Avi Kivity wrote: On 08/08/2011 04:29 PM, Anthony Liguori wrote: One thing that strikes me about this algorithm is that it's very good for a particular type of workload--shockingly good really. Poking bytes at random places in memory is fairly generic. If you have a lot of small objects, and modify a subset of them, this is the pattern you get. I think workload aware migration compression is possible for a lot of different types of workloads. That makes me a bit wary of QEMU growing quite a lot of compression mechanisms. It makes me think that this logic may really belong at a higher level where more information is known about the workload. For instance, I can imagine XBZRLE living in something like libvirt. A better model would be plugin based. exec helpers are plugins. They just live in a different address space and a channel to exchange data (pipe). If we did .so plugins, which I'm really not opposed to, I'd want the interface to be something like: typedef struct MigrationTransportClass { ssize_t (*writev)(MigrationTransport *obj, struct iovec *iov, int iovcnt); } MigrationTransportClass; I think it's useful to use an interface like this because it makes it easy to put the transport in a dedicated thread that didn't hold qemu_mutex (which is sort of equivalent to using a fork'd helper but is zero-copy at the expense of less isolation). Regards, Anthony Liguori
[Qemu-devel] [PATCH v4 17/39] es1370: convert to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/es1370.c | 43 +-- 1 files changed, 25 insertions(+), 18 deletions(-) diff --git a/hw/es1370.c b/hw/es1370.c index 1ed62b7..4e43c4a 100644 --- a/hw/es1370.c +++ b/hw/es1370.c @@ -268,6 +268,7 @@ struct chan { typedef struct ES1370State { PCIDevice dev; QEMUSoundCard card; +MemoryRegion io; struct chan chan[NB_CHANNELS]; SWVoiceOut *dac_voice[2]; SWVoiceIn *adc_voice; @@ -775,7 +776,6 @@ IO_READ_PROTO (es1370_readl) return val; } - static void es1370_transfer_audio (ES1370State *s, struct chan *d, int loop_sel, int max, int *irq) { @@ -906,23 +906,20 @@ static void es1370_adc_callback (void *opaque, int avail) es1370_run_channel (s, ADC_CHANNEL, avail); } -static void es1370_map (PCIDevice *pci_dev, int region_num, -pcibus_t addr, pcibus_t size, int type) -{ -ES1370State *s = DO_UPCAST (ES1370State, dev, pci_dev); - -(void) region_num; -(void) size; -(void) type; - -register_ioport_write (addr, 0x40 * 4, 1, es1370_writeb, s); -register_ioport_write (addr, 0x40 * 2, 2, es1370_writew, s); -register_ioport_write (addr, 0x40, 4, es1370_writel, s); +static const MemoryRegionPortio es1370_portio[] = { +{ 0, 0x40 * 4, 1, .write = es1370_writeb, }, +{ 0, 0x40 * 2, 2, .write = es1370_writew, }, +{ 0, 0x40, 4, .write = es1370_writel, }, +{ 0, 0x40 * 4, 1, .read = es1370_readb, }, +{ 0, 0x40 * 2, 2, .read = es1370_readw, }, +{ 0, 0x40, 4, .read = es1370_readl, }, +PORTIO_END_OF_LIST() +}; -register_ioport_read (addr, 0x40 * 4, 1, es1370_readb, s); -register_ioport_read (addr, 0x40 * 2, 2, es1370_readw, s); -register_ioport_read (addr, 0x40, 4, es1370_readl, s); -} +static const MemoryRegionOps es1370_io_ops = { +.old_portio = es1370_portio, +.endianness = DEVICE_LITTLE_ENDIAN, +}; static const VMStateDescription vmstate_es1370_channel = { .name = "es1370_channel", @@ -1011,7 +1008,8 @@ static int es1370_initfn (PCIDevice *dev) c[PCI_MIN_GNT] = 0x0c; c[PCI_MAX_LAT] = 0x80; -pci_register_bar (&s->dev, 0, 256, PCI_BASE_ADDRESS_SPACE_IO, es1370_map); +memory_region_init_io (&s->io, &es1370_io_ops, s, "es1370", 256); +pci_register_bar_region (&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io); qemu_register_reset (es1370_on_reset, s); AUD_register_card ("es1370", &s->card); @@ -1019,6 +1017,14 @@ static int es1370_initfn (PCIDevice *dev) return 0; } +static int es1370_exitfn(PCIDevice *dev) +{ +ES1370State *s = DO_UPCAST (ES1370State, dev, dev); + +memory_region_destroy (&s->io); +return 0; +} + int es1370_init (PCIBus *bus) { pci_create_simple (bus, -1, "ES1370"); @@ -1031,6 +1037,7 @@ static PCIDeviceInfo es1370_info = { .qdev.size= sizeof (ES1370State), .qdev.vmsd= &vmstate_es1370, .init = es1370_initfn, +.exit = es1370_exitfn, .vendor_id= PCI_VENDOR_ID_ENSONIQ, .device_id= PCI_DEVICE_ID_ENSONIQ_ES1370, .class_id = PCI_CLASS_MULTIMEDIA_AUDIO, -- 1.7.5.3
[Qemu-devel] [PATCH v4 35/39] pci: convert pci rom to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/pci.c | 20 +++- hw/pci.h |3 ++- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 7a70037..f885d4e 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1855,11 +1855,6 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id, return next; } -static void pci_map_option_rom(PCIDevice *pdev, int region_num, pcibus_t addr, pcibus_t size, int type) -{ -cpu_register_physical_memory(addr, size, pdev->rom_offset); -} - /* Patch the PCI vendor and device ids in a PCI rom image if necessary. This is needed for an option rom which is used for more than one device. */ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size) @@ -1963,9 +1958,9 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) snprintf(name, sizeof(name), "%s.rom", pdev->qdev.info->vmsd->name); else snprintf(name, sizeof(name), "%s.rom", pdev->qdev.info->name); -pdev->rom_offset = qemu_ram_alloc(&pdev->qdev, name, size); - -ptr = qemu_get_ram_ptr(pdev->rom_offset); +pdev->has_rom = true; +memory_region_init_ram(&pdev->rom, &pdev->qdev, name, size); +ptr = memory_region_get_ram_ptr(&pdev->rom); load_image(path, ptr); qemu_free(path); @@ -1976,19 +1971,18 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) qemu_put_ram_ptr(ptr); -pci_register_bar(pdev, PCI_ROM_SLOT, size, - 0, pci_map_option_rom); +pci_register_bar_region(pdev, PCI_ROM_SLOT, 0, &pdev->rom); return 0; } static void pci_del_option_rom(PCIDevice *pdev) { -if (!pdev->rom_offset) +if (!pdev->has_rom) return; -qemu_ram_free(pdev->rom_offset); -pdev->rom_offset = 0; +memory_region_destroy(&pdev->rom); +pdev->has_rom = false; } /* diff --git a/hw/pci.h b/hw/pci.h index 25e28b1..6e2bcea 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -191,7 +191,8 @@ struct PCIDevice { /* Location of option rom */ char *romfile; -ram_addr_t rom_offset; +bool has_rom; +MemoryRegion rom; uint32_t rom_bar; }; -- 1.7.5.3
[Qemu-devel] [PATCH v4 04/39] vga: convert vga and its derivatives to the memory API
Convert all vga memory to the memory API. Note we need to fall back to get_system_memory(), since the various buses don't pass the vga window as a memory region. We no longer need to sync the dirty bitmap of the cirrus mapped memory banks, since the memory API takes care of that for us. [jan: fix vga-pci logging] Reviewed-by: Richard Henderson Signed-off-by: Avi Kivity --- hw/cirrus_vga.c | 342 --- hw/qxl-render.c |2 +- hw/qxl.c| 135 -- hw/qxl.h|6 +- hw/vga-isa-mm.c | 46 +--- hw/vga-isa.c| 10 +- hw/vga-pci.c| 28 + hw/vga.c| 146 +++- hw/vga_int.h| 14 +-- hw/vmware_vga.c | 143 --- 10 files changed, 437 insertions(+), 435 deletions(-) diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index f39d1f8..ad23c4a 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -32,6 +32,7 @@ #include "console.h" #include "vga_int.h" #include "loader.h" +#include "exec-memory.h" /* * TODO: @@ -200,9 +201,14 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s, typedef struct CirrusVGAState { VGACommonState vga; -int cirrus_linear_io_addr; -int cirrus_linear_bitblt_io_addr; -int cirrus_mmio_io_addr; +MemoryRegion cirrus_linear_io; +MemoryRegion cirrus_linear_bitblt_io; +MemoryRegion cirrus_mmio_io; +MemoryRegion pci_bar; +bool linear_vram; /* vga.vram mapped over cirrus_linear_io */ +MemoryRegion low_mem_container; /* container for 0xa-0xc */ +MemoryRegion low_mem; /* always mapped, overridden by: */ +MemoryRegion *cirrus_bank[2]; /* aliases at 0xa-0xb */ uint32_t cirrus_addr_mask; uint32_t linear_mmio_mask; uint8_t cirrus_shadow_gr0; @@ -612,7 +618,7 @@ static void cirrus_invalidate_region(CirrusVGAState * s, int off_begin, off_cur_end = (off_cur + bytesperline) & s->cirrus_addr_mask; off_cur &= TARGET_PAGE_MASK; while (off_cur < off_cur_end) { - cpu_physical_memory_set_dirty(s->vga.vram_offset + off_cur); + memory_region_set_dirty(&s->vga.vram, off_cur); off_cur += TARGET_PAGE_SIZE; } off_begin += off_pitch; @@ -1177,12 +1183,6 @@ static void cirrus_update_bank_ptr(CirrusVGAState * s, unsigned bank_index) } if (limit > 0) { -/* Thinking about changing bank base? First, drop the dirty bitmap information - * on the current location, otherwise we lose this pointer forever */ -if (s->vga.lfb_vram_mapped) { -target_phys_addr_t base_addr = isa_mem_base + 0xa + bank_index * 0x8000; -cpu_physical_sync_dirty_bitmap(base_addr, base_addr + 0x8000); -} s->cirrus_bank_base[bank_index] = offset; s->cirrus_bank_limit[bank_index] = limit; } else { @@ -1921,8 +1921,8 @@ static void cirrus_mem_writeb_mode4and5_8bpp(CirrusVGAState * s, val <<= 1; dst++; } -cpu_physical_memory_set_dirty(s->vga.vram_offset + offset); -cpu_physical_memory_set_dirty(s->vga.vram_offset + offset + 7); +memory_region_set_dirty(&s->vga.vram, offset); +memory_region_set_dirty(&s->vga.vram, offset + 7); } static void cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s, @@ -1946,8 +1946,8 @@ static void cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s, val <<= 1; dst += 2; } -cpu_physical_memory_set_dirty(s->vga.vram_offset + offset); -cpu_physical_memory_set_dirty(s->vga.vram_offset + offset + 15); +memory_region_set_dirty(&s->vga.vram, offset); +memory_region_set_dirty(&s->vga.vram, offset + 15); } /*** @@ -2057,8 +2057,7 @@ static void cirrus_vga_mem_writeb(void *opaque, target_phys_addr_t addr, mode = s->vga.gr[0x05] & 0x7; if (mode < 4 || mode > 5 || ((s->vga.gr[0x0B] & 0x4) == 0)) { *(s->vga.vram_ptr + bank_offset) = mem_value; - cpu_physical_memory_set_dirty(s->vga.vram_offset + - bank_offset); + memory_region_set_dirty(&s->vga.vram, bank_offset); } else { if ((s->vga.gr[0x0B] & 0x14) != 0x14) { cirrus_mem_writeb_mode4and5_8bpp(s, mode, @@ -2099,16 +2098,37 @@ static void cirrus_vga_mem_writel(void *opaque, target_phys_addr_t addr, uint32_ cirrus_vga_mem_writeb(opaque, addr + 3, (val >> 24) & 0xff); } -static CPUReadMemoryFunc * const cirrus_vga_mem_read[3] = { -cirrus_vga_mem_readb, -cirrus_vga_mem_readw, -cirrus_vga_mem_readl, +static uint64_t cirrus_vga_mem_read(void *opaque, +target_phys_addr_t addr, +uint32_t size) +{ +CirrusVGAState *s = opaque; + +switch (size) { +case 1: retur
[Qemu-devel] [PATCH v4 24/39] ppc: convert to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/cuda.c |6 ++- hw/escc.c | 42 +-- hw/escc.h |2 +- hw/heathrow_pic.c | 29 -- hw/ide.h |2 +- hw/ide/macio.c| 36 --- hw/mac_dbdma.c| 32 ++-- hw/mac_dbdma.h|4 ++- hw/mac_nvram.c| 39 ++--- hw/macio.c| 74 +++- hw/openpic.c | 81 + hw/openpic.h |2 +- hw/ppc_mac.h | 16 ++ hw/ppc_newworld.c | 30 +-- hw/ppc_oldworld.c | 23 +++ 15 files changed, 201 insertions(+), 217 deletions(-) diff --git a/hw/cuda.c b/hw/cuda.c index 065c362..5c92d81 100644 --- a/hw/cuda.c +++ b/hw/cuda.c @@ -117,6 +117,7 @@ typedef struct CUDATimer { } CUDATimer; typedef struct CUDAState { +MemoryRegion mem; /* cuda registers */ uint8_t b; /* B-side data */ uint8_t a; /* A-side data */ @@ -722,7 +723,7 @@ static void cuda_reset(void *opaque) set_counter(s, &s->timers[1], 0x); } -void cuda_init (int *cuda_mem_index, qemu_irq irq) +void cuda_init (MemoryRegion **cuda_mem, qemu_irq irq) { struct tm tm; CUDAState *s = &cuda_state; @@ -738,8 +739,9 @@ void cuda_init (int *cuda_mem_index, qemu_irq irq) s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET; s->adb_poll_timer = qemu_new_timer_ns(vm_clock, cuda_adb_poll, s); -*cuda_mem_index = cpu_register_io_memory(cuda_read, cuda_write, s, +cpu_register_io_memory(cuda_read, cuda_write, s, DEVICE_NATIVE_ENDIAN); +*cuda_mem = &s->mem; vmstate_register(NULL, -1, &vmstate_cuda, s); qemu_register_reset(cuda_reset, s); } diff --git a/hw/escc.c b/hw/escc.c index f6fd919..bea5873 100644 --- a/hw/escc.c +++ b/hw/escc.c @@ -126,7 +126,7 @@ struct SerialState { SysBusDevice busdev; struct ChannelState chn[2]; uint32_t it_shift; -int mmio_index; +MemoryRegion mmio; uint32_t disabled; uint32_t frequency; }; @@ -490,7 +490,8 @@ static void escc_update_parameters(ChannelState *s) qemu_chr_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp); } -static void escc_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) +static void escc_mem_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned size) { SerialState *serial = opaque; ChannelState *s; @@ -592,7 +593,8 @@ static void escc_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) } } -static uint32_t escc_mem_readb(void *opaque, target_phys_addr_t addr) +static uint64_t escc_mem_read(void *opaque, target_phys_addr_t addr, + unsigned size) { SerialState *serial = opaque; ChannelState *s; @@ -627,6 +629,16 @@ static uint32_t escc_mem_readb(void *opaque, target_phys_addr_t addr) return 0; } +static const MemoryRegionOps escc_mem_ops = { +.read = escc_mem_read, +.write = escc_mem_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 1, +.max_access_size = 1, +}, +}; + static int serial_can_receive(void *opaque) { ChannelState *s = opaque; @@ -668,18 +680,6 @@ static void serial_event(void *opaque, int event) serial_receive_break(s); } -static CPUReadMemoryFunc * const escc_mem_read[3] = { -escc_mem_readb, -NULL, -NULL, -}; - -static CPUWriteMemoryFunc * const escc_mem_write[3] = { -escc_mem_writeb, -NULL, -NULL, -}; - static const VMStateDescription vmstate_escc_chn = { .name ="escc_chn", .version_id = 2, @@ -712,7 +712,7 @@ static const VMStateDescription vmstate_escc = { } }; -int escc_init(target_phys_addr_t base, qemu_irq irqA, qemu_irq irqB, +MemoryRegion *escc_init(target_phys_addr_t base, qemu_irq irqA, qemu_irq irqB, CharDriverState *chrA, CharDriverState *chrB, int clock, int it_shift) { @@ -737,7 +737,7 @@ int escc_init(target_phys_addr_t base, qemu_irq irqA, qemu_irq irqB, } d = FROM_SYSBUS(SerialState, s); -return d->mmio_index; +return &d->mmio; } static const uint8_t keycodes[128] = { @@ -901,7 +901,6 @@ void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq, static int escc_init1(SysBusDevice *dev) { SerialState *s = FROM_SYSBUS(SerialState, dev); -int io; unsigned int i; s->chn[0].disabled = s->disabled; @@ -918,10 +917,9 @@ static int escc_init1(SysBusDevice *dev) s->chn[0].otherchn = &s->chn[1]; s->chn[1].otherchn = &s->chn[0]; -io = cpu_register_io_memory(escc_mem_read, escc_mem_write, s, -DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, ESCC_SIZE <
[Qemu-devel] [PATCH v4 36/39] pci: remove pci_register_bar()
Superceded by pci_register_bar_region(). The implementations are folded together. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/pci.c | 42 +- hw/pci.h |3 --- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index f885d4e..62b34d4 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -881,13 +881,25 @@ static int pci_unregister_device(DeviceState *dev) return 0; } -void pci_register_bar(PCIDevice *pci_dev, int region_num, -pcibus_t size, uint8_t type, -PCIMapIORegionFunc *map_func) +static void pci_simple_bar_mapfunc_region(PCIDevice *pci_dev, int region_num, + pcibus_t addr, pcibus_t size, + int type) +{ +PCIIORegion *r = &pci_dev->io_regions[region_num]; + +memory_region_add_subregion_overlap(r->address_space, +addr, +r->memory, +1); +} + +void pci_register_bar_region(PCIDevice *pci_dev, int region_num, + uint8_t type, MemoryRegion *memory) { PCIIORegion *r; uint32_t addr; uint64_t wmask; +pcibus_t size = memory_region_size(memory); assert(region_num >= 0); assert(region_num < PCI_NUM_REGIONS); @@ -902,7 +914,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, r->size = size; r->filtered_size = size; r->type = type; -r->map_func = map_func; +r->map_func = pci_simple_bar_mapfunc_region; r->memory = NULL; wmask = ~(size - 1); @@ -920,29 +932,9 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, pci_set_long(pci_dev->wmask + addr, wmask & 0x); pci_set_long(pci_dev->cmask + addr, 0x); } -} - -static void pci_simple_bar_mapfunc_region(PCIDevice *pci_dev, int region_num, - pcibus_t addr, pcibus_t size, - int type) -{ -PCIIORegion *r = &pci_dev->io_regions[region_num]; - -memory_region_add_subregion_overlap(r->address_space, -addr, -r->memory, -1); -} - -void pci_register_bar_region(PCIDevice *pci_dev, int region_num, - uint8_t attr, MemoryRegion *memory) -{ -pci_register_bar(pci_dev, region_num, memory_region_size(memory), - attr, - pci_simple_bar_mapfunc_region); pci_dev->io_regions[region_num].memory = memory; pci_dev->io_regions[region_num].address_space -= attr & PCI_BASE_ADDRESS_SPACE_IO += type & PCI_BASE_ADDRESS_SPACE_IO ? pci_dev->bus->address_space_io : pci_dev->bus->address_space_mem; } diff --git a/hw/pci.h b/hw/pci.h index 6e2bcea..8028176 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -201,9 +201,6 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name, PCIConfigReadFunc *config_read, PCIConfigWriteFunc *config_write); -void pci_register_bar(PCIDevice *pci_dev, int region_num, -pcibus_t size, uint8_t type, -PCIMapIORegionFunc *map_func); void pci_register_bar_region(PCIDevice *pci_dev, int region_num, uint8_t attr, MemoryRegion *memory); pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num); -- 1.7.5.3
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/08/2011 05:15 PM, Anthony Liguori wrote: I think workload aware migration compression is possible for a lot of different types of workloads. That makes me a bit wary of QEMU growing quite a lot of compression mechanisms. It makes me think that this logic may really belong at a higher level where more information is known about the workload. For instance, I can imagine XBZRLE living in something like libvirt. A better model would be plugin based. exec helpers are plugins. They just live in a different address space and a channel to exchange data (pipe). libvirt isn't an exec helper. If we did .so plugins, which I'm really not opposed to, I'd want the interface to be something like: typedef struct MigrationTransportClass { ssize_t (*writev)(MigrationTransport *obj, struct iovec *iov, int iovcnt); } MigrationTransportClass; I think it's useful to use an interface like this because it makes it easy to put the transport in a dedicated thread that didn't hold qemu_mutex (which is sort of equivalent to using a fork'd helper but is zero-copy at the expense of less isolation). If we have a shared object helper, the thread should be maintained by qemu proper, not the plugin. I wouldn't call it "migration transport", but instead a compression/decompression plugin. I don't think it merits a plugin at all though. There's limited scope for compression and it best sits in qemu proper. If anything, it needs to be more integrated (for example turning itself off if it doesn't match enough). -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v4 00/39] Memory API, batch 2: PCI devices
This is a mostly mindless conversion of all QEMU PCI devices to the memory API. After this patchset is applied, it is no longer possible to create a PCI device using the old API. An immediate benefit is that PCI BARs that overlap each other are now handled correctly: currently, the sequence map BAR 0 map BAR 1 at an overlapping address unmap either BAR 0 or BAR 1 will leave a hole where the overlap exists. With the patchset, the memory map is restored correctly. Note that overlaps of PCI BARs with memory or non-PCI resources are still not resolved correctly; this will be fixed later on. The vga patches have ugly intermediate states; however the result is fairly clean. Changes from v3: - dropped virtio-pci config patch; will be fixed outside this patchset if necessary - minor style fixes Changes from v2: - added patch from Michael simplifying virtio-pci config setup Changes from v1: - cmd646 type fix - folded a fixlet into its parent Avi Kivity (39): memory: rename PORTIO_END to PORTIO_END_OF_LIST pci: add API to get a BAR's mapped address vmsvga: don't remember pci BAR address in callback any more vga: convert vga and its derivatives to the memory API cirrus: simplify mmio BAR access functions cirrus: simplify bitblt BAR access functions cirrus: simplify vga window mmio access functions vga: simplify vga window mmio access functions cirrus: simplify linear framebuffer access functions Integrate I/O memory regions into qemu pci: pass I/O address space to new PCI bus pci: allow I/O BARs to be registered with pci_register_bar_region() rtl8139: convert to memory API ac97: convert to memory API e1000: convert to memory API eepro100: convert to memory API es1370: convert to memory API ide: convert to memory API ivshmem: convert to memory API virtio-pci: convert to memory API ahci: convert to memory API intel-hda: convert to memory API lsi53c895a: convert to memory API ppc: convert to memory API ne2000: convert to memory API pcnet: convert to memory API i6300esb: convert to memory API isa-mmio: convert to memory API sun4u: convert to memory API ehci: convert to memory API uhci: convert to memory API xen-platform: convert to memory API msix: convert to memory API pci: remove pci_register_bar_simple() pci: convert pci rom to memory API pci: remove pci_register_bar() pci: fold BAR mapping function into its caller pci: rename pci_register_bar_region() to pci_register_bar() pci: remove support for pre memory API BARs exec-memory.h |5 + exec.c | 10 ++ hw/ac97.c | 88 ++- hw/apb_pci.c |1 + hw/bonito.c|1 + hw/cirrus_vga.c| 459 --- hw/cuda.c |6 +- hw/e1000.c | 113 ++ hw/eepro100.c | 181 - hw/es1370.c| 43 +++-- hw/escc.c | 42 +++--- hw/escc.h |2 +- hw/grackle_pci.c |8 +- hw/gt64xxx.c |4 +- hw/heathrow_pic.c | 29 ++-- hw/ide.h |2 +- hw/ide/ahci.c | 31 ++-- hw/ide/ahci.h |2 +- hw/ide/cmd646.c| 204 +++- hw/ide/ich.c |3 +- hw/ide/macio.c | 36 +++-- hw/ide/pci.c | 25 ++-- hw/ide/pci.h | 19 ++- hw/ide/piix.c | 63 ++-- hw/ide/via.c | 64 ++-- hw/intel-hda.c | 35 +++-- hw/isa.h |2 + hw/isa_mmio.c | 29 ++-- hw/ivshmem.c | 158 +++ hw/lance.c | 31 ++-- hw/lsi53c895a.c| 257 +++--- hw/mac_dbdma.c | 32 ++-- hw/mac_dbdma.h |4 +- hw/mac_nvram.c | 39 ++--- hw/macio.c | 73 - hw/msix.c | 64 +++- hw/msix.h |6 +- hw/ne2000-isa.c| 13 +-- hw/ne2000.c| 77 ++--- hw/ne2000.h|8 +- hw/openpic.c | 81 +- hw/openpic.h |2 +- hw/pc.h|4 +- hw/pc_piix.c |6 +- hw/pci.c | 133 +--- hw/pci.h | 26 ++-- hw/pci_internals.h |3 +- hw/pcnet-pci.c | 74 + hw/pcnet.h |4 +- hw/piix_pci.c | 14 +- hw/ppc4xx_pci.c|1 + hw/ppc_mac.h | 27 ++-- hw/ppc_newworld.c | 34 ++-- hw/ppc_oldworld.c | 27 ++-- hw/ppc_prep.c |2 +- hw/ppce500_pci.c |7 +- hw/prep_pci.c |8 +- hw/prep_pci.h |4 +- hw/qxl-render.c|2 +- hw/qxl.c | 129 ++-- hw/qxl.h |6 +- hw/rtl8139.c | 70 hw/sh_pci.c|4 +- hw/sun4u.c | 53 +++ hw/unin_pci.c | 16 ++- hw/usb-ehci.c | 36 +--- hw/usb-ohci.c |2 +- hw/usb-uhci.c | 41 +++-- hw/versatile_pci.c |2 +- hw/vga-isa-mm.c| 46 -- hw/vga-isa.c | 10 +- hw/vga-pci.c | 27 +---
[Qemu-devel] [PATCH v4 22/39] intel-hda: convert to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/intel-hda.c | 35 +++ 1 files changed, 19 insertions(+), 16 deletions(-) diff --git a/hw/intel-hda.c b/hw/intel-hda.c index 5a2bc3a..1e4c71e 100644 --- a/hw/intel-hda.c +++ b/hw/intel-hda.c @@ -177,7 +177,7 @@ struct IntelHDAState { IntelHDAStream st[8]; /* state */ -int mmio_addr; +MemoryRegion mmio; uint32_t rirb_count; int64_t wall_base_ns; @@ -1084,16 +1084,20 @@ static uint32_t intel_hda_mmio_readl(void *opaque, target_phys_addr_t addr) return intel_hda_reg_read(d, reg, 0x); } -static CPUReadMemoryFunc * const intel_hda_mmio_read[3] = { -intel_hda_mmio_readb, -intel_hda_mmio_readw, -intel_hda_mmio_readl, -}; - -static CPUWriteMemoryFunc * const intel_hda_mmio_write[3] = { -intel_hda_mmio_writeb, -intel_hda_mmio_writew, -intel_hda_mmio_writel, +static const MemoryRegionOps intel_hda_mmio_ops = { +.old_mmio = { +.read = { +intel_hda_mmio_readb, +intel_hda_mmio_readw, +intel_hda_mmio_readl, +}, +.write = { +intel_hda_mmio_writeb, +intel_hda_mmio_writew, +intel_hda_mmio_writel, +}, +}, +.endianness = DEVICE_NATIVE_ENDIAN, }; /* - */ @@ -1130,10 +1134,9 @@ static int intel_hda_init(PCIDevice *pci) /* HDCTL off 0x40 bit 0 selects signaling mode (1-HDA, 0 - Ac97) 18.1.19 */ conf[0x40] = 0x01; -d->mmio_addr = cpu_register_io_memory(intel_hda_mmio_read, - intel_hda_mmio_write, d, - DEVICE_NATIVE_ENDIAN); -pci_register_bar_simple(&d->pci, 0, 0x4000, 0, d->mmio_addr); +memory_region_init_io(&d->mmio, &intel_hda_mmio_ops, d, + "intel-hda", 0x4000); +pci_register_bar_region(&d->pci, 0, 0, &d->mmio); if (d->msi) { msi_init(&d->pci, 0x50, 1, true, false); } @@ -1149,7 +1152,7 @@ static int intel_hda_exit(PCIDevice *pci) IntelHDAState *d = DO_UPCAST(IntelHDAState, pci, pci); msi_uninit(&d->pci); -cpu_unregister_io_memory(d->mmio_addr); +memory_region_destroy(&d->mmio); return 0; } -- 1.7.5.3
[Qemu-devel] [PATCH v4 08/39] vga: simplify vga window mmio access functions
Make use of the memory API's ability to satisfy multi-byte accesses via multiple single-byte accesses. We have to keep vga_mem_{read,write}b() since they're used by cirrus. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/cirrus_vga.c |4 +- hw/vga.c| 56 +++--- hw/vga_int.h|4 +- 3 files changed, 12 insertions(+), 52 deletions(-) diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 5ded1ff..2a9bd25 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -1966,7 +1966,7 @@ static uint64_t cirrus_vga_mem_read(void *opaque, uint32_t val; if ((s->vga.sr[0x07] & 0x01) == 0) { - return vga_mem_readb(s, addr); +return vga_mem_readb(&s->vga, addr); } if (addr < 0x1) { @@ -2011,7 +2011,7 @@ static void cirrus_vga_mem_write(void *opaque, unsigned mode; if ((s->vga.sr[0x07] & 0x01) == 0) { - vga_mem_writeb(s, addr, mem_value); +vga_mem_writeb(&s->vga, addr, mem_value); return; } diff --git a/hw/vga.c b/hw/vga.c index 8b6e6b6..33dc478 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -708,9 +708,8 @@ static void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val) #endif /* called for accesses between 0xa and 0xc */ -uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr) +uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr) { -VGACommonState *s = opaque; int memory_map_mode, plane; uint32_t ret; @@ -764,28 +763,9 @@ uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr) return ret; } -static uint32_t vga_mem_readw(void *opaque, target_phys_addr_t addr) -{ -uint32_t v; -v = vga_mem_readb(opaque, addr); -v |= vga_mem_readb(opaque, addr + 1) << 8; -return v; -} - -static uint32_t vga_mem_readl(void *opaque, target_phys_addr_t addr) -{ -uint32_t v; -v = vga_mem_readb(opaque, addr); -v |= vga_mem_readb(opaque, addr + 1) << 8; -v |= vga_mem_readb(opaque, addr + 2) << 16; -v |= vga_mem_readb(opaque, addr + 3) << 24; -return v; -} - /* called for accesses between 0xa and 0xc */ -void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) +void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val) { -VGACommonState *s = opaque; int memory_map_mode, plane, write_mode, b, func_select, mask; uint32_t write_mask, bit_mask, set_mask; @@ -917,20 +897,6 @@ void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) } } -static void vga_mem_writew(void *opaque, target_phys_addr_t addr, uint32_t val) -{ -vga_mem_writeb(opaque, addr, val & 0xff); -vga_mem_writeb(opaque, addr + 1, (val >> 8) & 0xff); -} - -static void vga_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val) -{ -vga_mem_writeb(opaque, addr, val & 0xff); -vga_mem_writeb(opaque, addr + 1, (val >> 8) & 0xff); -vga_mem_writeb(opaque, addr + 2, (val >> 16) & 0xff); -vga_mem_writeb(opaque, addr + 3, (val >> 24) & 0xff); -} - typedef void vga_draw_glyph8_func(uint8_t *d, int linesize, const uint8_t *font_ptr, int h, uint32_t fgcol, uint32_t bgcol); @@ -2105,12 +2071,7 @@ static uint64_t vga_mem_read(void *opaque, target_phys_addr_t addr, { VGACommonState *s = opaque; -switch (size) { -case 1: return vga_mem_readb(s, addr); -case 2: return vga_mem_readw(s, addr); -case 4: return vga_mem_readl(s, addr); -default: abort(); -} +return vga_mem_readb(s, addr); } static void vga_mem_write(void *opaque, target_phys_addr_t addr, @@ -2118,18 +2079,17 @@ static void vga_mem_write(void *opaque, target_phys_addr_t addr, { VGACommonState *s = opaque; -switch (size) { -case 1: return vga_mem_writeb(s, addr, data); -case 2: return vga_mem_writew(s, addr, data); -case 4: return vga_mem_writel(s, addr, data); -default: abort(); -} +return vga_mem_writeb(s, addr, data); } const MemoryRegionOps vga_mem_ops = { .read = vga_mem_read, .write = vga_mem_write, .endianness = DEVICE_LITTLE_ENDIAN, +.impl = { +.min_access_size = 1, +.max_access_size = 1, +}, }; static int vga_common_post_load(void *opaque, int version_id) diff --git a/hw/vga_int.h b/hw/vga_int.h index 4592d2c..100d98c 100644 --- a/hw/vga_int.h +++ b/hw/vga_int.h @@ -198,8 +198,8 @@ void vga_dirty_log_restart(VGACommonState *s); extern const VMStateDescription vmstate_vga_common; uint32_t vga_ioport_read(void *opaque, uint32_t addr); void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val); -uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr); -void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val); +uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr); +void vga_mem_writeb(VGACommonSta
[Qemu-devel] [PATCH v4 33/39] msix: convert to memory API
The msix table is defined as a subregion, to allow for a BAR that mixes device specific regions with the msix table. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/ivshmem.c| 11 + hw/msix.c | 64 +++ hw/msix.h |6 +--- hw/pci.h|2 +- hw/virtio-pci.c | 16 - hw/virtio-pci.h |1 + 6 files changed, 42 insertions(+), 58 deletions(-) diff --git a/hw/ivshmem.c b/hw/ivshmem.c index f80e7b6..bacba60 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -65,6 +65,7 @@ typedef struct IVShmemState { */ MemoryRegion bar; MemoryRegion ivshmem; +MemoryRegion msix_bar; uint64_t ivshmem_size; /* size of shared memory region */ int shm_fd; /* shared memory file descriptor */ @@ -540,11 +541,11 @@ static void ivshmem_setup_msi(IVShmemState * s) { /* allocate the MSI-X vectors */ -if (!msix_init(&s->dev, s->vectors, 1, 0)) { -pci_register_bar(&s->dev, 1, - msix_bar_size(&s->dev), - PCI_BASE_ADDRESS_SPACE_MEMORY, - msix_mmio_map); +memory_region_init(&s->msix_bar, "ivshmem-msix", 4096); +if (!msix_init(&s->dev, s->vectors, &s->msix_bar, 1, 0)) { +pci_register_bar_region(&s->dev, 1, +PCI_BASE_ADDRESS_SPACE_MEMORY, +&s->msix_bar); IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors); } else { IVSHMEM_DPRINTF("msix initialization failed\n"); diff --git a/hw/msix.c b/hw/msix.c index e67e700..8536c3f 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -82,7 +82,8 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries, return 0; } -static uint32_t msix_mmio_readl(void *opaque, target_phys_addr_t addr) +static uint64_t msix_mmio_read(void *opaque, target_phys_addr_t addr, + unsigned size) { PCIDevice *dev = opaque; unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3; @@ -91,12 +92,6 @@ static uint32_t msix_mmio_readl(void *opaque, target_phys_addr_t addr) return pci_get_long(page + offset); } -static uint32_t msix_mmio_read_unallowed(void *opaque, target_phys_addr_t addr) -{ -fprintf(stderr, "MSI-X: only dword read is allowed!\n"); -return 0; -} - static uint8_t msix_pending_mask(int vector) { return 1 << (vector % 8); @@ -169,8 +164,8 @@ void msix_write_config(PCIDevice *dev, uint32_t addr, } } -static void msix_mmio_writel(void *opaque, target_phys_addr_t addr, - uint32_t val) +static void msix_mmio_write(void *opaque, target_phys_addr_t addr, +uint64_t val, unsigned size) { PCIDevice *dev = opaque; unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3; @@ -179,37 +174,25 @@ static void msix_mmio_writel(void *opaque, target_phys_addr_t addr, msix_handle_mask_update(dev, vector); } -static void msix_mmio_write_unallowed(void *opaque, target_phys_addr_t addr, - uint32_t val) -{ -fprintf(stderr, "MSI-X: only dword write is allowed!\n"); -} - -static CPUWriteMemoryFunc * const msix_mmio_write[] = { -msix_mmio_write_unallowed, msix_mmio_write_unallowed, msix_mmio_writel -}; - -static CPUReadMemoryFunc * const msix_mmio_read[] = { -msix_mmio_read_unallowed, msix_mmio_read_unallowed, msix_mmio_readl +static const MemoryRegionOps msix_mmio_ops = { +.read = msix_mmio_read, +.write = msix_mmio_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 4, +.max_access_size = 4, +}, }; -/* Should be called from device's map method. */ -void msix_mmio_map(PCIDevice *d, int region_num, - pcibus_t addr, pcibus_t size, int type) +static void msix_mmio_setup(PCIDevice *d, MemoryRegion *bar) { uint8_t *config = d->config + d->msix_cap; uint32_t table = pci_get_long(config + PCI_MSIX_TABLE); uint32_t offset = table & ~(MSIX_PAGE_SIZE - 1); /* TODO: for assigned devices, we'll want to make it possible to map * pending bits separately in case they are in a separate bar. */ -int table_bir = table & PCI_MSIX_FLAGS_BIRMASK; -if (table_bir != region_num) -return; -if (size <= offset) -return; -cpu_register_physical_memory(addr + offset, size - offset, - d->msix_mmio_index); +memory_region_add_subregion(bar, offset, &d->msix_mmio); } static void msix_mask_all(struct PCIDevice *dev, unsigned nentries) @@ -225,6 +208,7 @@ static void msix_mask_all(struct PCIDevice *dev, unsigned nentries) /* Initialize the MSI-X structures. Note: if MSI-X is supported, BAR size is * modified, it should be retrieved with msix_bar_size. */ int msix_init(struct PCIDevice *dev, unsigned
Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type
On 08/08/2011 05:06 PM, Luiz Capitulino wrote: > > This is why I suggested a reference count. In this case, we can always > stop the guest "twice", because we don't lost information when we resume. I could give it a try in the near future, as I really think it's independent from this series, but I still don't understand what can stop an already stopped VM besides the user. This is a real question, is it really possible? If only the user can do that, then the refcount is overkill as just returning an error will do it. Most of the reasons in QemuState are asynchronous and can happen at any time while the guest is running. Because they're asynchronous, they can trigger before a monitor stop is issued but caught after it is processed. We could possibly synchronize during user stop, but that lets us capture only the first non-user reason. And even if we return an error, that only pushes the refcounting to the user; you probably don't want to return a "vm is already stopped" error to the user, he'll just ask "why are you telling me that". -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v4 31/39] uhci: convert to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/usb-uhci.c | 42 -- 1 files changed, 28 insertions(+), 14 deletions(-) diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c index 824e3a5..ea38169 100644 --- a/hw/usb-uhci.c +++ b/hw/usb-uhci.c @@ -129,6 +129,7 @@ typedef struct UHCIPort { struct UHCIState { PCIDevice dev; +MemoryRegion io_bar; USBBus bus; /* Note unused when we're a companion controller */ uint16_t cmd; /* cmd register */ uint16_t status; @@ -1096,18 +1097,19 @@ static void uhci_frame_timer(void *opaque) qemu_mod_timer(s->frame_timer, s->expire_time); } -static void uhci_map(PCIDevice *pci_dev, int region_num, -pcibus_t addr, pcibus_t size, int type) -{ -UHCIState *s = (UHCIState *)pci_dev; - -register_ioport_write(addr, 32, 2, uhci_ioport_writew, s); -register_ioport_read(addr, 32, 2, uhci_ioport_readw, s); -register_ioport_write(addr, 32, 4, uhci_ioport_writel, s); -register_ioport_read(addr, 32, 4, uhci_ioport_readl, s); -register_ioport_write(addr, 32, 1, uhci_ioport_writeb, s); -register_ioport_read(addr, 32, 1, uhci_ioport_readb, s); -} +static const MemoryRegionPortio uhci_portio[] = { +{ 0, 32, 2, .write = uhci_ioport_writew, }, +{ 0, 32, 2, .read = uhci_ioport_readw, }, +{ 0, 32, 4, .write = uhci_ioport_writel, }, +{ 0, 32, 4, .read = uhci_ioport_readl, }, +{ 0, 32, 1, .write = uhci_ioport_writeb, }, +{ 0, 32, 1, .read = uhci_ioport_readb, }, +PORTIO_END_OF_LIST() +}; + +static const MemoryRegionOps uhci_ioport_ops = { +.old_portio = uhci_portio, +}; static USBPortOps uhci_port_ops = { .attach = uhci_attach, @@ -1154,10 +1156,11 @@ static int usb_uhci_common_initfn(PCIDevice *dev) qemu_register_reset(uhci_reset, s); +memory_region_init_io(&s->io_bar, &uhci_ioport_ops, s, "uhci", 0x20); /* Use region 4 for consistency with real hardware. BSD guests seem to rely on this. */ -pci_register_bar(&s->dev, 4, 0x20, - PCI_BASE_ADDRESS_SPACE_IO, uhci_map); +pci_register_bar_region(&s->dev, 4, +PCI_BASE_ADDRESS_SPACE_IO, &s->io_bar); return 0; } @@ -1177,6 +1180,14 @@ static int usb_uhci_vt82c686b_initfn(PCIDevice *dev) return usb_uhci_common_initfn(dev); } +static int usb_uhci_exit(PCIDevice *dev) +{ +UHCIState *s = DO_UPCAST(UHCIState, dev, dev); + +memory_region_destroy(&s->io_bar); +return 0; +} + static Property uhci_properties[] = { DEFINE_PROP_STRING("masterbus", UHCIState, masterbus), DEFINE_PROP_UINT32("firstport", UHCIState, firstport, 0), @@ -1189,6 +1200,7 @@ static PCIDeviceInfo uhci_info[] = { .qdev.size= sizeof(UHCIState), .qdev.vmsd= &vmstate_uhci, .init = usb_uhci_common_initfn, +.exit = usb_uhci_exit, .vendor_id= PCI_VENDOR_ID_INTEL, .device_id= PCI_DEVICE_ID_INTEL_82371SB_2, .revision = 0x01, @@ -1199,6 +1211,7 @@ static PCIDeviceInfo uhci_info[] = { .qdev.size= sizeof(UHCIState), .qdev.vmsd= &vmstate_uhci, .init = usb_uhci_common_initfn, +.exit = usb_uhci_exit, .vendor_id= PCI_VENDOR_ID_INTEL, .device_id= PCI_DEVICE_ID_INTEL_82371AB_2, .revision = 0x01, @@ -1209,6 +1222,7 @@ static PCIDeviceInfo uhci_info[] = { .qdev.size= sizeof(UHCIState), .qdev.vmsd= &vmstate_uhci, .init = usb_uhci_vt82c686b_initfn, +.exit = usb_uhci_exit, .vendor_id= PCI_VENDOR_ID_VIA, .device_id= PCI_DEVICE_ID_VIA_UHCI, .revision = 0x01, -- 1.7.5.3
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/08/2011 09:23 AM, Avi Kivity wrote: On 08/08/2011 05:15 PM, Anthony Liguori wrote: If we did .so plugins, which I'm really not opposed to, I'd want the interface to be something like: typedef struct MigrationTransportClass { ssize_t (*writev)(MigrationTransport *obj, struct iovec *iov, int iovcnt); } MigrationTransportClass; I think it's useful to use an interface like this because it makes it easy to put the transport in a dedicated thread that didn't hold qemu_mutex (which is sort of equivalent to using a fork'd helper but is zero-copy at the expense of less isolation). If we have a shared object helper, the thread should be maintained by qemu proper, not the plugin. I wouldn't call it "migration transport", but instead a compression/decompression plugin. I don't think it merits a plugin at all though. There's limited scope for compression and it best sits in qemu proper. If anything, it needs to be more integrated (for example turning itself off if it doesn't match enough). That adds a tremendous amount of complexity to QEMU. If we're going to change our compression algorithm, we would need to use a single algorithm that worked well for a wide variety of workloads. We struggle enough with migration as it is, it only would get worse if we have 10 different algorithms that we were dynamically enabling/disabling. The other option is to allow 1-off compression algorithms in the form of plugins. I think in this case, plugins are a pretty good compromise in terms of isolating complexity while allowing something that at least works very well for one particular type of workload. Regards, Anthony Liguori
[Qemu-devel] [PATCH v4 20/39] virtio-pci: convert to memory API
except msix. [jan: fix build] Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/virtio-pci.c | 71 +-- hw/virtio-pci.h |2 +- 2 files changed, 28 insertions(+), 45 deletions(-) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index f3b3293..5df380d 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -162,7 +162,8 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy, { VirtQueue *vq = virtio_get_queue(proxy->vdev, n); EventNotifier *notifier = virtio_queue_get_host_notifier(vq); -int r; +int r = 0; + if (assign) { r = event_notifier_init(notifier, 1); if (r < 0) { @@ -170,24 +171,11 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy, __func__, r); return r; } -r = kvm_set_ioeventfd_pio_word(event_notifier_get_fd(notifier), - proxy->addr + VIRTIO_PCI_QUEUE_NOTIFY, - n, assign); -if (r < 0) { -error_report("%s: unable to map ioeventfd: %d", - __func__, r); -event_notifier_cleanup(notifier); -} +memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2, + true, n, event_notifier_get_fd(notifier)); } else { -r = kvm_set_ioeventfd_pio_word(event_notifier_get_fd(notifier), - proxy->addr + VIRTIO_PCI_QUEUE_NOTIFY, - n, assign); -if (r < 0) { -error_report("%s: unable to unmap ioeventfd: %d", - __func__, r); -return r; -} - +memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2, + true, n, event_notifier_get_fd(notifier)); /* Handle the race condition where the guest kicked and we deassigned * before we got around to handling the kick. */ @@ -424,7 +412,6 @@ static uint32_t virtio_pci_config_readb(void *opaque, uint32_t addr) { VirtIOPCIProxy *proxy = opaque; uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev); -addr -= proxy->addr; if (addr < config) return virtio_ioport_read(proxy, addr); addr -= config; @@ -435,7 +422,6 @@ static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr) { VirtIOPCIProxy *proxy = opaque; uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev); -addr -= proxy->addr; if (addr < config) return virtio_ioport_read(proxy, addr); addr -= config; @@ -446,7 +432,6 @@ static uint32_t virtio_pci_config_readl(void *opaque, uint32_t addr) { VirtIOPCIProxy *proxy = opaque; uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev); -addr -= proxy->addr; if (addr < config) return virtio_ioport_read(proxy, addr); addr -= config; @@ -457,7 +442,6 @@ static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val) { VirtIOPCIProxy *proxy = opaque; uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev); -addr -= proxy->addr; if (addr < config) { virtio_ioport_write(proxy, addr, val); return; @@ -470,7 +454,6 @@ static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val) { VirtIOPCIProxy *proxy = opaque; uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev); -addr -= proxy->addr; if (addr < config) { virtio_ioport_write(proxy, addr, val); return; @@ -483,7 +466,6 @@ static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val) { VirtIOPCIProxy *proxy = opaque; uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev); -addr -= proxy->addr; if (addr < config) { virtio_ioport_write(proxy, addr, val); return; @@ -492,30 +474,26 @@ static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val) virtio_config_writel(proxy->vdev, addr, val); } -static void virtio_map(PCIDevice *pci_dev, int region_num, - pcibus_t addr, pcibus_t size, int type) -{ -VirtIOPCIProxy *proxy = container_of(pci_dev, VirtIOPCIProxy, pci_dev); -VirtIODevice *vdev = proxy->vdev; -unsigned config_len = VIRTIO_PCI_REGION_SIZE(pci_dev) + vdev->config_len; - -proxy->addr = addr; - -register_ioport_write(addr, config_len, 1, virtio_pci_config_writeb, proxy); -register_ioport_write(addr, config_len, 2, virtio_pci_config_writew, proxy); -register_ioport_write(addr, config_len, 4, virtio_pci_config_writel, proxy); -register_ioport_read(addr, config_len, 1, virtio_pci_config_readb, proxy); -register_ioport_read(addr, config_len, 2, virtio_pci_config_readw, proxy); -register_ioport_read(addr, config_len, 4, virtio_pci_config_readl,
[Qemu-devel] [PATCH v4 06/39] cirrus: simplify bitblt BAR access functions
Make use of the memory API's ability to satisfy multi-byte accesses via multiple single-byte accesses. Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/cirrus_vga.c | 81 +-- 1 files changed, 13 insertions(+), 68 deletions(-) diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 4f57b92..c39acb9 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -2446,37 +2446,23 @@ static void cirrus_linear_write(void *opaque, target_phys_addr_t addr, ***/ -static uint32_t cirrus_linear_bitblt_readb(void *opaque, target_phys_addr_t addr) +static uint64_t cirrus_linear_bitblt_read(void *opaque, + target_phys_addr_t addr, + unsigned size) { +CirrusVGAState *s = opaque; uint32_t ret; /* XXX handle bitblt */ +(void)s; ret = 0xff; return ret; } -static uint32_t cirrus_linear_bitblt_readw(void *opaque, target_phys_addr_t addr) -{ -uint32_t v; - -v = cirrus_linear_bitblt_readb(opaque, addr); -v |= cirrus_linear_bitblt_readb(opaque, addr + 1) << 8; -return v; -} - -static uint32_t cirrus_linear_bitblt_readl(void *opaque, target_phys_addr_t addr) -{ -uint32_t v; - -v = cirrus_linear_bitblt_readb(opaque, addr); -v |= cirrus_linear_bitblt_readb(opaque, addr + 1) << 8; -v |= cirrus_linear_bitblt_readb(opaque, addr + 2) << 16; -v |= cirrus_linear_bitblt_readb(opaque, addr + 3) << 24; -return v; -} - -static void cirrus_linear_bitblt_writeb(void *opaque, target_phys_addr_t addr, -uint32_t val) +static void cirrus_linear_bitblt_write(void *opaque, + target_phys_addr_t addr, + uint64_t val, + unsigned size) { CirrusVGAState *s = opaque; @@ -2489,55 +2475,14 @@ static void cirrus_linear_bitblt_writeb(void *opaque, target_phys_addr_t addr, } } -static void cirrus_linear_bitblt_writew(void *opaque, target_phys_addr_t addr, -uint32_t val) -{ -cirrus_linear_bitblt_writeb(opaque, addr, val & 0xff); -cirrus_linear_bitblt_writeb(opaque, addr + 1, (val >> 8) & 0xff); -} - -static void cirrus_linear_bitblt_writel(void *opaque, target_phys_addr_t addr, -uint32_t val) -{ -cirrus_linear_bitblt_writeb(opaque, addr, val & 0xff); -cirrus_linear_bitblt_writeb(opaque, addr + 1, (val >> 8) & 0xff); -cirrus_linear_bitblt_writeb(opaque, addr + 2, (val >> 16) & 0xff); -cirrus_linear_bitblt_writeb(opaque, addr + 3, (val >> 24) & 0xff); -} - -static uint64_t cirrus_linear_bitblt_read(void *opaque, - target_phys_addr_t addr, - unsigned size) -{ -CirrusVGAState *s = opaque; - -switch (size) { -case 1: return cirrus_linear_bitblt_readb(s, addr); -case 2: return cirrus_linear_bitblt_readw(s, addr); -case 4: return cirrus_linear_bitblt_readl(s, addr); -default: abort(); -} -}; - -static void cirrus_linear_bitblt_write(void *opaque, - target_phys_addr_t addr, - uint64_t data, - unsigned size) -{ -CirrusVGAState *s = opaque; - -switch (size) { -case 1: return cirrus_linear_bitblt_writeb(s, addr, data); -case 2: return cirrus_linear_bitblt_writew(s, addr, data); -case 4: return cirrus_linear_bitblt_writel(s, addr, data); -default: abort(); -} -}; - static const MemoryRegionOps cirrus_linear_bitblt_io_ops = { .read = cirrus_linear_bitblt_read, .write = cirrus_linear_bitblt_write, .endianness = DEVICE_LITTLE_ENDIAN, +.impl = { +.min_access_size = 1, +.max_access_size = 1, +}, }; static void unmap_bank(CirrusVGAState *s, unsigned bank) -- 1.7.5.3
Re: [Qemu-devel] [PULL] VirtFS coroutine changes
On Mon, 08 Aug 2011 07:36:48 -0500, Anthony Liguori wrote: > On 08/08/2011 04:03 AM, Aneesh Kumar K.V wrote: > > > > The following changes since commit 23ddf2bb1e4bfe2b72a726fe5e828807b65941ad: > > > >Fix forcing multicast msgs to loopback on OpenBSD. (2011-08-07 11:06:43 > > +) > > This need to be sent to the list first as patches for review before you > can do a pull request. The patch series was sent to the list before Message-id:1306367597-797-1-git-send-email-jv...@linux.vnet.ibm.com http://thread.gmane.org/gmane.comp.emulators.qemu/104120 -aneesh
[Qemu-devel] [PATCH v4 30/39] ehci: convert to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/usb-ehci.c | 36 +--- 1 files changed, 9 insertions(+), 27 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 2b43895..6ef7798 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -370,8 +370,7 @@ struct EHCIState { PCIDevice dev; USBBus bus; qemu_irq irq; -target_phys_addr_t mem_base; -int mem; +MemoryRegion mem; int companion_count; /* properties */ @@ -2179,29 +2178,15 @@ static void ehci_frame_timer(void *opaque) qemu_mod_timer(ehci->frame_timer, expire_time); } -static CPUReadMemoryFunc *ehci_readfn[3]={ -ehci_mem_readb, -ehci_mem_readw, -ehci_mem_readl -}; -static CPUWriteMemoryFunc *ehci_writefn[3]={ -ehci_mem_writeb, -ehci_mem_writew, -ehci_mem_writel +static const MemoryRegionOps ehci_mem_ops = { +.old_mmio = { +.read = { ehci_mem_readb, ehci_mem_readw, ehci_mem_readl }, +.write = { ehci_mem_writeb, ehci_mem_writew, ehci_mem_writel }, +}, +.endianness = DEVICE_LITTLE_ENDIAN, }; -static void ehci_map(PCIDevice *pci_dev, int region_num, - pcibus_t addr, pcibus_t size, int type) -{ -EHCIState *s =(EHCIState *)pci_dev; - -DPRINTF("ehci_map: region %d, addr %08" PRIx64 ", size %" PRId64 ", s->mem %08X\n", -region_num, addr, size, s->mem); -s->mem_base = addr; -cpu_register_physical_memory(addr, size, s->mem); -} - static int usb_ehci_initfn(PCIDevice *dev); static USBPortOps ehci_port_ops = { @@ -2316,11 +2301,8 @@ static int usb_ehci_initfn(PCIDevice *dev) qemu_register_reset(ehci_reset, s); -s->mem = cpu_register_io_memory(ehci_readfn, ehci_writefn, s, -DEVICE_LITTLE_ENDIAN); - -pci_register_bar(&s->dev, 0, MMIO_SIZE, PCI_BASE_ADDRESS_SPACE_MEMORY, -ehci_map); +memory_region_init_io(&s->mem, &ehci_mem_ops, s, "ehci", MMIO_SIZE); +pci_register_bar_region(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem); fprintf(stderr, "*** EHCI support is under development ***\n"); -- 1.7.5.3
[Qemu-devel] [PATCH v4 21/39] ahci: convert to memory API
Reviewed-by: Richard Henderson Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity --- hw/ide/ahci.c | 31 +-- hw/ide/ahci.h |2 +- hw/ide/ich.c |3 +-- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 1f008a3..e207ca0 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -276,12 +276,12 @@ static void ahci_port_write(AHCIState *s, int port, int offset, uint32_t val) } } -static uint32_t ahci_mem_readl(void *ptr, target_phys_addr_t addr) +static uint64_t ahci_mem_read(void *opaque, target_phys_addr_t addr, + unsigned size) { -AHCIState *s = ptr; +AHCIState *s = opaque; uint32_t val = 0; -addr = addr & 0xfff; if (addr < AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR) { switch (addr) { case HOST_CAP: @@ -314,10 +314,10 @@ static uint32_t ahci_mem_readl(void *ptr, target_phys_addr_t addr) -static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val) +static void ahci_mem_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned size) { -AHCIState *s = ptr; -addr = addr & 0xfff; +AHCIState *s = opaque; /* Only aligned reads are allowed on AHCI */ if (addr & 3) { @@ -364,16 +364,10 @@ static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val) } -static CPUReadMemoryFunc * const ahci_readfn[3]={ -ahci_mem_readl, -ahci_mem_readl, -ahci_mem_readl -}; - -static CPUWriteMemoryFunc * const ahci_writefn[3]={ -ahci_mem_writel, -ahci_mem_writel, -ahci_mem_writel +static MemoryRegionOps ahci_mem_ops = { +.read = ahci_mem_read, +.write = ahci_mem_write, +.endianness = DEVICE_LITTLE_ENDIAN, }; static void ahci_reg_init(AHCIState *s) @@ -1131,8 +1125,8 @@ void ahci_init(AHCIState *s, DeviceState *qdev, int ports) s->ports = ports; s->dev = qemu_mallocz(sizeof(AHCIDevice) * ports); ahci_reg_init(s); -s->mem = cpu_register_io_memory(ahci_readfn, ahci_writefn, s, -DEVICE_LITTLE_ENDIAN); +/* XXX BAR size should be 1k, but that breaks, so bump it to 4k for now */ +memory_region_init_io(&s->mem, &ahci_mem_ops, s, "ahci", 0x1000); irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports); for (i = 0; i < s->ports; i++) { @@ -1151,6 +1145,7 @@ void ahci_init(AHCIState *s, DeviceState *qdev, int ports) void ahci_uninit(AHCIState *s) { +memory_region_destroy(&s->mem); qemu_free(s->dev); } diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index dc86951..e456193 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -289,7 +289,7 @@ struct AHCIDevice { typedef struct AHCIState { AHCIDevice *dev; AHCIControlRegs control_regs; -int mem; +MemoryRegion mem; int ports; qemu_irq irq; } AHCIState; diff --git a/hw/ide/ich.c b/hw/ide/ich.c index d241ea8..698b5f6 100644 --- a/hw/ide/ich.c +++ b/hw/ide/ich.c @@ -98,8 +98,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev) msi_init(dev, 0x50, 1, true, false); d->ahci.irq = d->card.irq[0]; -/* XXX BAR size should be 1k, but that breaks, so bump it to 4k for now */ -pci_register_bar_simple(&d->card, 5, 0x1000, 0, d->ahci.mem); +pci_register_bar_region(&d->card, 5, 0, &d->ahci.mem); return 0; } -- 1.7.5.3