[Qemu-devel] [PULL] usb patch queue
Hi, More usb and hid bits. Fixes a usb tablet regression with windows xp. milkymist goes use the new, splitted hid code directly instead of (ab-)using the usb-kbd device, which in turn allows to kill the usb_hid_datain_cb callback as no users are left. please pull, Gerd The following changes since commit b9c6cbff76061537b722d55f0e321dde2a612a23: Merge remote-tracking branch 'pm-arm/for-upstream' into pm (2011-08-09 19:16:43 +0200) are available in the git repository at: git://git.kraxel.org/qemu usb.23 Gerd Hoffmann (2): usb/hid: add hid_pointer_activate, use it usb-hid: remove usb_hid_datain_cb Michael Walle (4): hid: register kbd hander in init() hid: introduce hid vmstate macros usb-hid: use hid vmstate macro milkymist-softusb: use hid code directly hw/hid.c | 76 +++-- hw/hid.h |1 + hw/hw.h| 20 hw/milkymist-softusb.c | 122 +++- hw/usb-hid.c | 58 ++- hw/usb.h |3 - 6 files changed, 134 insertions(+), 146 deletions(-)
[Qemu-devel] [PATCH 4/6] usb-hid: use hid vmstate macro
From: Michael Walle Use new hid vmstate macro. Version stays the same, because there is no reordering of the fields. Signed-off-by: Michael Walle Signed-off-by: Gerd Hoffmann --- hw/usb-hid.c | 41 ++--- 1 files changed, 2 insertions(+), 39 deletions(-) diff --git a/hw/usb-hid.c b/hw/usb-hid.c index 6f12751..6a75147 100644 --- a/hw/usb-hid.c +++ b/hw/usb-hid.c @@ -541,41 +541,13 @@ void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *)) s->datain = datain; } -static int usb_hid_post_load(void *opaque, int version_id) -{ -USBHIDState *s = opaque; - -if (s->hid.idle) { -hid_set_next_idle(&s->hid, qemu_get_clock_ns(vm_clock)); -} -return 0; -} - -static const VMStateDescription vmstate_usb_ptr_queue = { -.name = "usb-ptr-queue", -.version_id = 1, -.minimum_version_id = 1, -.fields = (VMStateField []) { -VMSTATE_INT32(xdx, HIDPointerEvent), -VMSTATE_INT32(ydy, HIDPointerEvent), -VMSTATE_INT32(dz, HIDPointerEvent), -VMSTATE_INT32(buttons_state, HIDPointerEvent), -VMSTATE_END_OF_LIST() -} -}; static const VMStateDescription vmstate_usb_ptr = { .name = "usb-ptr", .version_id = 1, .minimum_version_id = 1, -.post_load = usb_hid_post_load, .fields = (VMStateField []) { VMSTATE_USB_DEVICE(dev, USBHIDState), -VMSTATE_STRUCT_ARRAY(hid.ptr.queue, USBHIDState, QUEUE_LENGTH, 0, - vmstate_usb_ptr_queue, HIDPointerEvent), -VMSTATE_UINT32(hid.head, USBHIDState), -VMSTATE_UINT32(hid.n, USBHIDState), -VMSTATE_INT32(hid.protocol, USBHIDState), -VMSTATE_UINT8(hid.idle, USBHIDState), +VMSTATE_HID_POINTER_DEVICE(hid, USBHIDState), VMSTATE_END_OF_LIST() } }; @@ -584,18 +556,9 @@ static const VMStateDescription vmstate_usb_kbd = { .name = "usb-kbd", .version_id = 1, .minimum_version_id = 1, -.post_load = usb_hid_post_load, .fields = (VMStateField []) { VMSTATE_USB_DEVICE(dev, USBHIDState), -VMSTATE_UINT32_ARRAY(hid.kbd.keycodes, USBHIDState, QUEUE_LENGTH), -VMSTATE_UINT32(hid.head, USBHIDState), -VMSTATE_UINT32(hid.n, USBHIDState), -VMSTATE_UINT16(hid.kbd.modifiers, USBHIDState), -VMSTATE_UINT8(hid.kbd.leds, USBHIDState), -VMSTATE_UINT8_ARRAY(hid.kbd.key, USBHIDState, 16), -VMSTATE_INT32(hid.kbd.keys, USBHIDState), -VMSTATE_INT32(hid.protocol, USBHIDState), -VMSTATE_UINT8(hid.idle, USBHIDState), +VMSTATE_HID_KEYBOARD_DEVICE(hid, USBHIDState), VMSTATE_END_OF_LIST() } }; -- 1.7.1
[Qemu-devel] [PATCH 3/6] hid: introduce hid vmstate macros
From: Michael Walle Add VMSTATE macros to describe a HIDState. Based on usb-hid.c descriptions. Signed-off-by: Michael Walle Signed-off-by: Gerd Hoffmann --- hw/hid.c | 58 ++ hw/hw.h | 20 2 files changed, 78 insertions(+), 0 deletions(-) diff --git a/hw/hid.c b/hw/hid.c index 3dc4246..ec066cf 100644 --- a/hw/hid.c +++ b/hw/hid.c @@ -407,3 +407,61 @@ void hid_init(HIDState *hs, int kind, HIDEventFunc event) 1, "QEMU HID Tablet"); } } + +static int hid_post_load(void *opaque, int version_id) +{ +HIDState *s = opaque; + +if (s->idle) { +hid_set_next_idle(s, qemu_get_clock_ns(vm_clock)); +} +return 0; +} + +static const VMStateDescription vmstate_hid_ptr_queue = { +.name = "HIDPointerEventQueue", +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField[]) { +VMSTATE_INT32(xdx, HIDPointerEvent), +VMSTATE_INT32(ydy, HIDPointerEvent), +VMSTATE_INT32(dz, HIDPointerEvent), +VMSTATE_INT32(buttons_state, HIDPointerEvent), +VMSTATE_END_OF_LIST() +} +}; + +const VMStateDescription vmstate_hid_ptr_device = { +.name = "HIDPointerDevice", +.version_id = 1, +.minimum_version_id = 1, +.post_load = hid_post_load, +.fields = (VMStateField[]) { +VMSTATE_STRUCT_ARRAY(ptr.queue, HIDState, QUEUE_LENGTH, 0, + vmstate_hid_ptr_queue, HIDPointerEvent), +VMSTATE_UINT32(head, HIDState), +VMSTATE_UINT32(n, HIDState), +VMSTATE_INT32(protocol, HIDState), +VMSTATE_UINT8(idle, HIDState), +VMSTATE_END_OF_LIST(), +} +}; + +const VMStateDescription vmstate_hid_keyboard_device = { +.name = "HIDKeyboardDevice", +.version_id = 1, +.minimum_version_id = 1, +.post_load = hid_post_load, +.fields = (VMStateField[]) { +VMSTATE_UINT32_ARRAY(kbd.keycodes, HIDState, QUEUE_LENGTH), +VMSTATE_UINT32(head, HIDState), +VMSTATE_UINT32(n, HIDState), +VMSTATE_UINT16(kbd.modifiers, HIDState), +VMSTATE_UINT8(kbd.leds, HIDState), +VMSTATE_UINT8_ARRAY(kbd.key, HIDState, 16), +VMSTATE_INT32(kbd.keys, HIDState), +VMSTATE_INT32(protocol, HIDState), +VMSTATE_UINT8(idle, HIDState), +VMSTATE_END_OF_LIST(), +} +}; diff --git a/hw/hw.h b/hw/hw.h index df6ca65..a124da9 100644 --- a/hw/hw.h +++ b/hw/hw.h @@ -701,6 +701,26 @@ extern const VMStateDescription vmstate_ptimer; .offset = vmstate_offset_pointer(_state, _field, ptimer_state), \ } +extern const VMStateDescription vmstate_hid_keyboard_device; + +#define VMSTATE_HID_KEYBOARD_DEVICE(_field, _state) {\ +.name = (stringify(_field)), \ +.size = sizeof(HIDState), \ +.vmsd = &vmstate_hid_keyboard_device, \ +.flags = VMS_STRUCT,\ +.offset = vmstate_offset_value(_state, _field, HIDState),\ +} + +extern const VMStateDescription vmstate_hid_ptr_device; + +#define VMSTATE_HID_POINTER_DEVICE(_field, _state) { \ +.name = (stringify(_field)), \ +.size = sizeof(HIDState), \ +.vmsd = &vmstate_hid_ptr_device, \ +.flags = VMS_STRUCT,\ +.offset = vmstate_offset_value(_state, _field, HIDState),\ +} + /* _f : field name _f_n : num of elements field_name _n : num of elements -- 1.7.1
[Qemu-devel] [PULL] spice patch queue
Hi, Here comes the spice patch queue with two little fixes queued up and and extension of the "info spice" monitor command which reports the spice version too now. please pull, Gerd The following changes since commit b9c6cbff76061537b722d55f0e321dde2a612a23: Merge remote-tracking branch 'pm-arm/for-upstream' into pm (2011-08-09 19:16:43 +0200) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu spice.v41 Alon Levy (2): qxl: unbreak after memory API conversion ui/spice-core: report compiled-version in info spice/query-spice Yonit Halperin (1): qxl: allowing the command rings to be not empty when spice worker is stopped RHBZ #728984 hw/qxl.c| 13 ++--- ui/spice-core.c |8 2 files changed, 14 insertions(+), 7 deletions(-)
[Qemu-devel] [PATCH 2/3] qxl: unbreak after memory API conversion
From: Alon Levy Break is only noticable with newer spice-server library (0.8.2 release or 0.9.0 and newer on master branch). ioport_write's val was changed from uint32_t to uint64_t, this broke two printfs. Use PRId64 instead of %d. Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- hw/qxl.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index 7991e70..b34bccf 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1187,7 +1187,7 @@ async_common: } d->current_async = orig_io_port; qemu_mutex_unlock(&d->async_lock); -dprint(d, 2, "start async %d (%d)\n", io_port, val); +dprint(d, 2, "start async %d (%"PRId64")\n", io_port, val); break; default: break; @@ -1303,7 +1303,8 @@ async_common: break; } case QXL_IO_FLUSH_SURFACES_ASYNC: -dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC (%d) (%s, s#=%d, res#=%d)\n", +dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC" + " (%"PRId64") (%s, s#=%d, res#=%d)\n", val, qxl_mode_to_string(d->mode), d->guest_surfaces.count, d->num_free_res); qxl_spice_flush_surfaces_async(d); -- 1.7.1
[Qemu-devel] [PATCH 3/3] ui/spice-core: report compiled-version in info spice/query-spice
From: Alon Levy Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- ui/spice-core.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/ui/spice-core.c b/ui/spice-core.c index 3d77c01..8bb62ea 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -372,6 +372,8 @@ void do_info_spice_print(Monitor *mon, const QObject *data) monitor_printf(mon, " address: %s:%d [tls]\n", host, port); } monitor_printf(mon, "auth: %s\n", qdict_get_str(server, "auth")); +monitor_printf(mon, "compiled: %s\n", + qdict_get_str(server, "compiled-version")); channels = qdict_get_qlist(server, "channels"); if (qlist_empty(channels)) { @@ -388,6 +390,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data) QList *clist; const char *addr; int port, tls_port; +char version_string[20]; /* 12 = |255.255.255\0| is the max */ if (!spice_server) { *ret_data = qobject_from_jsonf("{ 'enabled': false }"); @@ -403,6 +406,11 @@ void do_info_spice(Monitor *mon, QObject **ret_data) qdict_put(server, "enabled", qbool_from_int(true)); qdict_put(server, "auth", qstring_from_str(auth)); qdict_put(server, "host", qstring_from_str(addr ? addr : "0.0.0.0")); +snprintf(version_string, sizeof(version_string), "%d.%d.%d", + (SPICE_SERVER_VERSION & 0xff) >> 16, + (SPICE_SERVER_VERSION & 0xff00) >> 8, + SPICE_SERVER_VERSION & 0xff); +qdict_put(server, "compiled-version", qstring_from_str(version_string)); if (port) { qdict_put(server, "port", qint_from_int(port)); } -- 1.7.1
[Qemu-devel] [PATCH 1/3] qxl: allowing the command rings to be not empty when spice worker is stopped RHBZ #728984
From: Yonit Halperin same as 8927cfbba232e28304734f7afd463c1b84134031, but for qxl_check_state, that was triggered by qxl_pre_load (which calls qxl_hard_reset, which calls qxl_soft_reset), and caused the migration target to crash. Signed-off-by: Gerd Hoffmann --- hw/qxl.c |8 +++- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index db7ae7a..7991e70 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -821,17 +821,15 @@ static void qxl_check_state(PCIQXLDevice *d) { QXLRam *ram = d->ram; -assert(SPICE_RING_IS_EMPTY(&ram->cmd_ring)); -assert(SPICE_RING_IS_EMPTY(&ram->cursor_ring)); +assert(!d->ssd.running || SPICE_RING_IS_EMPTY(&ram->cmd_ring)); +assert(!d->ssd.running || SPICE_RING_IS_EMPTY(&ram->cursor_ring)); } static void qxl_reset_state(PCIQXLDevice *d) { -QXLRam *ram = d->ram; QXLRom *rom = d->rom; -assert(!d->ssd.running || SPICE_RING_IS_EMPTY(&ram->cmd_ring)); -assert(!d->ssd.running || SPICE_RING_IS_EMPTY(&ram->cursor_ring)); +qxl_check_state(d); d->shadow_rom.update_id = cpu_to_le32(0); *rom = d->shadow_rom; qxl_rom_set_dirty(d); -- 1.7.1
Re: [Qemu-devel] [PATCH 2/2] i_generation / st_gen support for handle based fs driver
On 08/10/2011 08:47 PM, Stefan Hajnoczi wrote: On Fri, Aug 5, 2011 at 1:53 PM, Stefan Hajnoczi wrote: On Fri, Aug 5, 2011 at 12:32 PM, Aneesh Kumar K.V wrote: On Fri, 5 Aug 2011 10:24:42 +0100, Stefan Hajnoczi wrote: On Fri, Aug 5, 2011 at 7:40 AM, Aneesh Kumar K.V wrote: On Thu, 4 Aug 2011 22:57:34 +0100, Stefan Hajnoczi wrote: On Thu, Aug 4, 2011 at 7:45 PM, Aneesh Kumar K.V wrote: On Thu, 4 Aug 2011 15:31:08 +0100, Stefan Hajnoczi wrote: On Thu, Aug 4, 2011 at 1:03 PM, Aneesh Kumar K.V wrote: On Thu, 4 Aug 2011 12:47:42 +0100, Stefan Hajnoczi wrote: On Thu, Aug 4, 2011 at 12:20 PM, Aneesh Kumar K.V wrote: On Thu, 4 Aug 2011 11:21:05 +0100, Stefan Hajnoczi wrote: On Thu, Aug 4, 2011 at 11:06 AM, Harsh Prateek Bora wrote: This patch provides support for st_gen for handle based fs type server. Currently the support is provided for ext4, btrfs, reiserfs and xfs. Signed-off-by: Harsh Prateek Bora --- hw/9pfs/virtio-9p-handle.c | 30 ++ 1 files changed, 30 insertions(+), 0 deletions(-) Does handle-based file I/O really need to duplicate all this code? Is it possible to use either regular open or handle-based open from a single local fs codebase? The only details common between handle based and local based getversion callback is the ioctl. Moving that into a helper may not really help in this case ?. Aneesh, do you have a public virtfs tree that I can look at? In qemu.git we don't have virtio-9p-handle.c yet, so I can't give any specific feedback. http://repo.or.cz/w/qemu/v9fs.git for-upstream I should send the patchset to qemu list soon. Was waiting for the co-routine patches to go upstream. The handle code looks like a copy of the local backend minus security models. It just needs to use handle syscalls instead of using paths. If you treat the path as the "handle" and use regular openat(2), then the handle code could do what the local backend does today. Except compared to the local backend it would not have security models and be a bit slower due to extra syscalls. Is the plan to add security models to the handle backend? If so, then handle and local will be equivalent and duplicate code. handle require root user privileges to run. So security model with handle fs driver doesn't make sense. We added mapped security model to avoid requiring user to run as root. Does it really require root or is a specific set of capabilities enough? CAP_DAC_READ_SEARCH is needed. A feature that requires QEMU to run as root has really limited value. Unprivileged users cannot use the feature, so ad-hoc QEMU users are left behind. People don't want to deploy production guests as root, may not be allowed to, or might find that their management tool doesn't support that. So who will be able to use this feature? One of the main issue that handle based backend fix is the complexity involved in handling renames, both on the guest and on the host. I am also not sure how effective it would be to run the qemu as non root user when exporting a directory with VirtFS. In the mapped security model the user credentials with which the files are created are stored in xattr and that mostly implies host cannot look at the files the same way. My understanding is passthrough security model (which require qemu to run as root) will be used if somebody wants to export a directory on the host to guest. In my case I use none security model, simply because i don't want new xattr on the file created and I am ok even the files get created on the host with the credentials on qemu. With xattrs you have to mount the directory on the host in order to see the same view as the guest. How will that help ? There is nothing on the host that maps those xattr to mode/ownership bits currently. We will have to do something similar to fuse to make that work ? Sorry, what I suggested is not actually possible today. We only have a virtio-9p transport in the QEMU 9pfs code, not a TCP transport. I meant mount -t 9p on the host - don't access the backing directory directly, instead mount it using 9p on localhost. My understanding was passthrough will be preferred option. But i may be mistaken. If passthrough requires all of QEMU to run as root, then we need to find a way to run that code separately and drop privileges in QEMU. The chroot helper process patches that Mohan posted might be a solution. The chroot helper does all path and permissions-related operations in a separate process. File descriptor passing is used so that QEMU can perform read/write operations itself without copying data. Then we just need to make sure that QEMU itself runs unprivileged and the chroot helper is able to run as root for the passthrough security model. Harsh, any thoughts on this? Hi Stefan, I am still not sure if it is really a big concern for VirtFS users, and if really required, we can move the functionality to a privileged process and but that would require Qemu
[Qemu-devel] [PATCH 1/6] usb/hid: add hid_pointer_activate, use it
HID reorganziation broke the usb tablet in windows xp. The reason is that xp activates idle before it starts polling, which creates a chicken-and-egg issue: We don't call hid_pointer_poll because there are no pending events. We don't get any events because the activation code in hid_pointer_poll is never executed and thus all pointer events are routed to the PS/2 mouse by qemu. Fix this by creating a hid_pointer_activate function and call it from usb-hid when the guest sets the idle state. Signed-off-by: Gerd Hoffmann --- hw/hid.c | 13 + hw/hid.h |1 + hw/usb-hid.c |3 +++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/hw/hid.c b/hw/hid.c index 7b5ef5f..77339f7 100644 --- a/hw/hid.c +++ b/hw/hid.c @@ -218,16 +218,21 @@ static inline int int_clamp(int val, int vmin, int vmax) } } +void hid_pointer_activate(HIDState *hs) +{ +if (!hs->ptr.mouse_grabbed) { +qemu_activate_mouse_event_handler(hs->ptr.eh_entry); +hs->ptr.mouse_grabbed = 1; +} +} + int hid_pointer_poll(HIDState *hs, uint8_t *buf, int len) { int dx, dy, dz, b, l; int index; HIDPointerEvent *e; -if (!hs->ptr.mouse_grabbed) { -qemu_activate_mouse_event_handler(hs->ptr.eh_entry); -hs->ptr.mouse_grabbed = 1; -} +hid_pointer_activate(hs); /* When the buffer is empty, return the last event. Relative movements will all be zero. */ diff --git a/hw/hid.h b/hw/hid.h index 4a8fa5b..9ce03b1 100644 --- a/hw/hid.h +++ b/hw/hid.h @@ -51,6 +51,7 @@ void hid_free(HIDState *hs); bool hid_has_events(HIDState *hs); void hid_set_next_idle(HIDState *hs, int64_t curtime); +void hid_pointer_activate(HIDState *hs); int hid_pointer_poll(HIDState *hs, uint8_t *buf, int len); int hid_keyboard_poll(HIDState *hs, uint8_t *buf, int len); int hid_keyboard_write(HIDState *hs, uint8_t *buf, int len); diff --git a/hw/usb-hid.c b/hw/usb-hid.c index e5d57de..6f12751 100644 --- a/hw/usb-hid.c +++ b/hw/usb-hid.c @@ -454,6 +454,9 @@ static int usb_hid_handle_control(USBDevice *dev, USBPacket *p, case SET_IDLE: hs->idle = (uint8_t) (value >> 8); hid_set_next_idle(hs, qemu_get_clock_ns(vm_clock)); +if (hs->kind == HID_MOUSE || hs->kind == HID_TABLET) { +hid_pointer_activate(hs); +} ret = 0; break; default: -- 1.7.1
[Qemu-devel] [PATCH 5/6] milkymist-softusb: use hid code directly
From: Michael Walle Remove the dummy USB device and use the HID code directly. Use the HID code for the mouse support, too. Signed-off-by: Michael Walle Signed-off-by: Gerd Hoffmann --- hw/milkymist-softusb.c | 122 +++- 1 files changed, 38 insertions(+), 84 deletions(-) diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c index 75c85ae..fe4eedb 100644 --- a/hw/milkymist-softusb.c +++ b/hw/milkymist-softusb.c @@ -25,7 +25,7 @@ #include "sysbus.h" #include "trace.h" #include "console.h" -#include "usb.h" +#include "hid.h" #include "qemu-error.h" enum { @@ -46,9 +46,8 @@ enum { struct MilkymistSoftUsbState { SysBusDevice busdev; -USBBus usbbus; -USBPort usbport[2]; -USBDevice *usbdev; +HIDState hid_kbd; +HIDState hid_mouse; qemu_irq irq; @@ -62,13 +61,10 @@ struct MilkymistSoftUsbState { uint32_t regs[R_MAX]; /* mouse state */ -int mouse_dx; -int mouse_dy; -int mouse_dz; -uint8_t mouse_buttons_state; +uint8_t mouse_hid_buffer[4]; /* keyboard state */ -uint8_t kbd_usb_buffer[8]; +uint8_t kbd_hid_buffer[8]; }; typedef struct MilkymistSoftUsbState MilkymistSoftUsbState; @@ -177,16 +173,10 @@ static inline void softusb_write_pmem(MilkymistSoftUsbState *s, static void softusb_mouse_changed(MilkymistSoftUsbState *s) { uint8_t m; -uint8_t buf[4]; - -buf[0] = s->mouse_buttons_state; -buf[1] = s->mouse_dx; -buf[2] = s->mouse_dy; -buf[3] = s->mouse_dz; softusb_read_dmem(s, COMLOC_MEVT_PRODUCE, &m, 1); trace_milkymist_softusb_mevt(m); -softusb_write_dmem(s, COMLOC_MEVT_BASE + 4 * m, buf, 4); +softusb_write_dmem(s, COMLOC_MEVT_BASE + 4 * m, s->mouse_hid_buffer, 4); m = (m + 1) & 0xf; softusb_write_dmem(s, COMLOC_MEVT_PRODUCE, &m, 1); @@ -200,7 +190,7 @@ static void softusb_kbd_changed(MilkymistSoftUsbState *s) softusb_read_dmem(s, COMLOC_KEVT_PRODUCE, &m, 1); trace_milkymist_softusb_kevt(m); -softusb_write_dmem(s, COMLOC_KEVT_BASE + 8 * m, s->kbd_usb_buffer, 8); +softusb_write_dmem(s, COMLOC_KEVT_BASE + 8 * m, s->kbd_hid_buffer, 8); m = (m + 1) & 0x7; softusb_write_dmem(s, COMLOC_KEVT_PRODUCE, &m, 1); @@ -208,62 +198,42 @@ static void softusb_kbd_changed(MilkymistSoftUsbState *s) qemu_irq_pulse(s->irq); } -static void softusb_mouse_event(void *opaque, - int dx, int dy, int dz, int buttons_state) +static void softusb_kbd_hid_datain(HIDState *hs) { -MilkymistSoftUsbState *s = opaque; +MilkymistSoftUsbState *s = container_of(hs, MilkymistSoftUsbState, hid_kbd); +int len; /* if device is in reset, do nothing */ if (s->regs[R_CTRL] & CTRL_RESET) { return; } -trace_milkymist_softusb_mouse_event(dx, dy, dz, buttons_state); +len = hid_keyboard_poll(hs, s->kbd_hid_buffer, sizeof(s->kbd_hid_buffer)); -s->mouse_dx = dx; -s->mouse_dy = dy; -s->mouse_dz = dz; -s->mouse_buttons_state = buttons_state; - -softusb_mouse_changed(s); +if (len == 8) { +softusb_kbd_changed(s); +} } -static void softusb_usbdev_datain(void *opaque) +static void softusb_mouse_hid_datain(HIDState *hs) { -MilkymistSoftUsbState *s = opaque; - -USBPacket p; - -usb_packet_init(&p); -usb_packet_setup(&p, USB_TOKEN_IN, 0, 1); -usb_packet_addbuf(&p, s->kbd_usb_buffer, sizeof(s->kbd_usb_buffer)); -s->usbdev->info->handle_data(s->usbdev, &p); -usb_packet_cleanup(&p); - -softusb_kbd_changed(s); -} +MilkymistSoftUsbState *s = +container_of(hs, MilkymistSoftUsbState, hid_mouse); +int len; -static void softusb_attach(USBPort *port) -{ -} +/* if device is in reset, do nothing */ +if (s->regs[R_CTRL] & CTRL_RESET) { +return; +} -static void softusb_detach(USBPort *port) -{ -} +len = hid_pointer_poll(hs, s->mouse_hid_buffer, +sizeof(s->mouse_hid_buffer)); -static void softusb_child_detach(USBPort *port, USBDevice *child) -{ +if (len == 4) { +softusb_mouse_changed(s); +} } -static USBPortOps softusb_ops = { -.attach = softusb_attach, -.detach = softusb_detach, -.child_detach = softusb_child_detach, -}; - -static USBBusOps softusb_bus_ops = { -}; - static void milkymist_softusb_reset(DeviceState *d) { MilkymistSoftUsbState *s = @@ -273,11 +243,11 @@ static void milkymist_softusb_reset(DeviceState *d) for (i = 0; i < R_MAX; i++) { s->regs[i] = 0; } -s->mouse_dx = 0; -s->mouse_dy = 0; -s->mouse_dz = 0; -s->mouse_buttons_state = 0; -memset(s->kbd_usb_buffer, 0, sizeof(s->kbd_usb_buffer)); +memset(s->kbd_hid_buffer, 0, sizeof(s->kbd_hid_buffer)); +memset(s->mouse_hid_buffer, 0, sizeof(s->mouse_hid_buffer)); + +hid_reset(&s->hid_kbd); +hid_reset(&s->hid_mouse); /* defaults */ s->regs[R_CTRL] = CTRL_RESET; @@ -304,23 +274,8 @@ static int milky
Re: [Qemu-devel] Safely reopening image files by stashing fds
Am 10.08.2011 19:20, schrieb Blue Swirl: > On Wed, Aug 10, 2011 at 7:58 AM, Kevin Wolf wrote: >> Am 09.08.2011 21:39, schrieb Blue Swirl: >>> On Tue, Aug 9, 2011 at 12:24 PM, Kevin Wolf wrote: Am 09.08.2011 14:00, schrieb Stefan Hajnoczi: > I liked the idea of doing a generic FDStash type that the monitor and > bdrv_reopen() can use. Blue's idea to hook at the qemu_open() level > takes that further. Well, having something that works for the raw-posix, the monitor and maybe some more things is nice. Having something that works for raw-posix, Sheepdog and rbd is an absolute requirement and I can't see how FDStash solves that. >>> >>> For Sheepdog also network access functions would need to be hooked. >>> RBD seems to use librados functions for low level I/O, so that needs >>> some RBD specific wrappers. >>> Even raw-win32 doesn't have an int fd, but a HANDLE hfile for its backend. >>> >>> Just replace "int fd" with "FDStash fd" everywhere? >> >> And how is FDStash defined? The current proposed is this one: >> >> /* A stashed file descriptor */ >> typedef FDEntry { >>const char *name; >>int fd; >>QLIST_ENTRY(FDEntry) next; >> } FDEntry; >> >> /* A container for stashing file descriptors */ >> typedef struct FDStash { >>QLIST_HEAD(, FDEntry) fds; >> } FDStash; >> >> So there we have the int fd again. If you want something generic, you > > What's the problem: > typedef struct FDEntry { >const char *name; > #ifdef _WIN32 >HANDLE fd; > #else >int fd; > #endif >QLIST_ENTRY(FDEntry) next; > } FDEntry; > > Renaming 'fd' to something that does not immediately look like 'int' > may be useful. This isn't any more generic, it merely covers two special cases instead of only one. You have used the fact that raw-posix and raw-win32 are never compiled in at the same time, so that you can use an #ifdef to conditionally pull out parts of their internal data structures into a generic struct (which in itself is a layering violation) It doesn't help with the other backend drivers like curl, nbd, rbd, sheepdog and last but not least our special friend vvfat. >> would have to start letting block drivers extend FDEntry with their own >> fields (and how would the first bdrv_open work then?) and basically >> arrive at something like a BDRVReopenState. > > I'd not handle FDEntries at block level at all (or only at raw level), > then there would not be first mover problems. Well, but how does it solve the bdrv_reopen problem if it's not visible on the block level? > Extending the uses of FDEntries to for example network stuff > (Sheepdog) could need some kind of unified API for both file and net > operations which is not what we have now (bdrv_read/write etc. vs. > direct recv/send). Then this new API would use FDEntries instead of > fds, sockets or HANDLEs. The 'name' field could be either network > address or file path. Though maybe we are only interested in > open/connect part so unification would not be needed for other > operations. Now you have handled three special cases, but still haven't got a generic solution. But considering that you don't want to touch the block layer API and therefore I don't even have an idea of what you would use FDEntries for... Let's go back one step: What problem are you trying to solve, and what does the API look like that you're thinking of? It doesn't seem to be the same as Stefan suggested. Kevin
[Qemu-devel] [PATCH 2/2] memory: crack wide ioport accesses into smaller ones when needed
The memory API supports cracking wide accesses into narrower ones when needed; but this was no implemented for the pio address space, causing lsi53c895a's IO BAR to malfunction. Fix by correctly cracking wide accesses when needed. Signed-off-by: Avi Kivity --- memory.c | 11 +-- 1 files changed, 9 insertions(+), 2 deletions(-) diff --git a/memory.c b/memory.c index 3c18e70..81032b6 100644 --- a/memory.c +++ b/memory.c @@ -400,7 +400,11 @@ static void memory_region_iorange_read(IORange *iorange, } return; } -*data = mr->ops->read(mr->opaque, offset, width); +*data = 0; +access_with_adjusted_size(offset, data, width, + mr->ops->impl.min_access_size, + mr->ops->impl.max_access_size, + memory_region_read_accessor, mr); } static void memory_region_iorange_write(IORange *iorange, @@ -418,7 +422,10 @@ static void memory_region_iorange_write(IORange *iorange, } return; } -mr->ops->write(mr->opaque, offset, data, width); +access_with_adjusted_size(offset, &data, width, + mr->ops->impl.min_access_size, + mr->ops->impl.max_access_size, + memory_region_write_accessor, mr); } static const IORangeOps memory_region_iorange_ops = { -- 1.7.5.3
[Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
The memory API automatically cracks wide memory accesses into narrower (usually byte) accesses when needed. Unfortunately this wasn't implemented for ioports, leading to an lsi53c895a failure. This series implements cracking for ioports as well. Note that the dual implementation is due to the fact the memory API is layered on top of the original qemu API; eventually the same code will be used for both ioports and mmio. Avi Kivity (2): memory: abstract cracking of write access ops into a function memory: crack wide ioport accesses into smaller ones when needed memory.c | 120 +++-- 1 files changed, 77 insertions(+), 43 deletions(-) -- 1.7.5.3
[Qemu-devel] [PATCH 1/2] memory: abstract cracking of write access ops into a function
The memory API automatically cracks large reads and writes into smaller ones when needed. Factor out this mechanism, which is now duplicated between memory reads and memory writes, into a function. Signed-off-by: Avi Kivity --- memory.c | 109 ++--- 1 files changed, 68 insertions(+), 41 deletions(-) diff --git a/memory.c b/memory.c index beff98c..3c18e70 100644 --- a/memory.c +++ b/memory.c @@ -226,6 +226,65 @@ static void flatview_simplify(FlatView *view) } } +static void memory_region_read_accessor(void *opaque, +target_phys_addr_t addr, +uint64_t *value, +unsigned size, +unsigned shift, +uint64_t mask) +{ +MemoryRegion *mr = opaque; +uint64_t tmp; + +tmp = mr->ops->read(mr->opaque, addr, size); +*value |= (tmp & mask) << shift; +} + +static void memory_region_write_accessor(void *opaque, + target_phys_addr_t addr, + uint64_t *value, + unsigned size, + unsigned shift, + uint64_t mask) +{ +MemoryRegion *mr = opaque; +uint64_t tmp; + +tmp = (*value >> shift) & mask; +mr->ops->write(mr->opaque, addr, tmp, size); +} + +static void access_with_adjusted_size(target_phys_addr_t addr, + uint64_t *value, + unsigned size, + unsigned access_size_min, + unsigned access_size_max, + void (*access)(void *opaque, + target_phys_addr_t addr, + uint64_t *value, + unsigned size, + unsigned shift, + uint64_t mask), + void *opaque) +{ +uint64_t access_mask; +unsigned access_size; +unsigned i; + +if (!access_size_min) { +access_size_min = 1; +} +if (!access_size_max) { +access_size_max = 4; +} +access_size = MAX(MIN(size, access_size_max), access_size_min); +access_mask = -1ULL >> (64 - access_size * 8); +for (i = 0; i < size; i += access_size) { +/* FIXME: big-endian support */ +access(opaque, addr + i, value, access_size, i * 8, access_mask); +} +} + static void memory_region_prepare_ram_addr(MemoryRegion *mr); static void as_memory_range_add(AddressSpace *as, FlatRange *fr) @@ -744,10 +803,7 @@ static uint32_t memory_region_read_thunk_n(void *_mr, unsigned size) { MemoryRegion *mr = _mr; -unsigned access_size, access_size_min, access_size_max; -uint64_t access_mask; -uint32_t data = 0, tmp; -unsigned i; +uint64_t data = 0; if (!memory_region_access_valid(mr, addr, size)) { return -1U; /* FIXME: better signalling */ @@ -758,23 +814,10 @@ static uint32_t memory_region_read_thunk_n(void *_mr, } /* FIXME: support unaligned access */ - -access_size_min = mr->ops->impl.min_access_size; -if (!access_size_min) { -access_size_min = 1; -} -access_size_max = mr->ops->impl.max_access_size; -if (!access_size_max) { -access_size_max = 4; -} -access_size = MAX(MIN(size, access_size_max), access_size_min); -access_mask = -1ULL >> (64 - access_size * 8); -addr += mr->offset; -for (i = 0; i < size; i += access_size) { -/* FIXME: big-endian support */ -tmp = mr->ops->read(mr->opaque, addr + i, access_size); -data |= (tmp & access_mask) << (i * 8); -} +access_with_adjusted_size(addr + mr->offset, &data, size, + mr->ops->impl.min_access_size, + mr->ops->impl.max_access_size, + memory_region_read_accessor, mr); return data; } @@ -785,9 +828,6 @@ static void memory_region_write_thunk_n(void *_mr, uint64_t data) { MemoryRegion *mr = _mr; -unsigned access_size, access_size_min, access_size_max; -uint64_t access_mask; -unsigned i; if (!memory_region_access_valid(mr, addr, size)) { return; /* FIXME: better signalling */ @@ -799,23 +839,10 @@ static void memory_region_write_thunk_n(void *_mr, } /* FIXME: support unaligned access */ - -access_size_min = mr->ops->impl.min_access_size; -if (!access_size_min) { -access_size_min = 1; -
[Qemu-devel] [Bug 721793] Re: QEMU freezes on startup (100% CPU utilization)
QEMU 0.15.0 fixes the problem. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/721793 Title: QEMU freezes on startup (100% CPU utilization) Status in QEMU: New Bug description: 0.12.5 was the last version of QEMU that runs ok and boots any os image. 0.13.0-0.14.0 just freeze, and the only thing I see is a black screen and both of them make it use 100% of CPU also. Both kernels 2.6.35.11 and 2.6.37.1 with and without PAE support. tested commands: W2000: $ qemu -m 256 -localtime -net nic,model=rtl8139 -net tap -usbdevice host:0e21:0750 /var/opt/vm/w2000.img W2000: $ qemu /var/opt/vm/w2000.img OpenBSD 4.8: $ qemu -cdrom ~/cd48.iso -boot d empty-qcow2.img tried to use `-M pc-0.12` selector, different audio cards (I've found it caused infinite loop on startup once) -- no luck. tried to use recent seabios from git -- still no luck. attached strace log of 0.14.0. everything was tested on HP mini 311C with Intel Atom N270. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/721793/+subscriptions
Re: [Qemu-devel] [PATCH 00/11] Memory API conversion for ISA
On 08/11/2011 01:28 AM, Richard Henderson wrote: [ Sorry about the partial duplicates, Avi. ] This patchset is relative to git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory-region although I'm sure it could be applied on top of Avi's batch 4. This is not complete by any means. I'm interested in feedback for some of the trickier points, e.g. pckbd and fdc. The pckbd mmio interface to mips_jazz is similar to how serial and ide/mmio are structured. But there are way more uses of serial. Thanks, I've queued this on my branch. If we're going to work on this in parallel, we'd better coordinate. Right now I'm going though all files the contain qemu_ram_alloc alphabetically and converting them (now in 'O'). If anyone wants a letter, let me know. Anthony, can we step up the merge rate? There are now 70 patches in the queue and growing, at the current rate this will take months. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 2/6] hid: register kbd hander in init()
From: Michael Walle Register the keyboard event handler in hid's init() instead of its reset() function. Signed-off-by: Michael Walle Signed-off-by: Gerd Hoffmann --- hw/hid.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/hid.c b/hw/hid.c index 77339f7..3dc4246 100644 --- a/hw/hid.c +++ b/hw/hid.c @@ -364,7 +364,6 @@ void hid_reset(HIDState *hs) { switch (hs->kind) { case HID_KEYBOARD: -qemu_add_kbd_event_handler(hid_keyboard_event, hs); memset(hs->kbd.keycodes, 0, sizeof(hs->kbd.keycodes)); memset(hs->kbd.key, 0, sizeof(hs->kbd.key)); hs->kbd.keys = 0; @@ -398,7 +397,9 @@ void hid_init(HIDState *hs, int kind, HIDEventFunc event) hs->kind = kind; hs->event = event; -if (hs->kind == HID_MOUSE) { +if (hs->kind == HID_KEYBOARD) { +qemu_add_kbd_event_handler(hid_keyboard_event, hs); +} else if (hs->kind == HID_MOUSE) { hs->ptr.eh_entry = qemu_add_mouse_event_handler(hid_pointer_event, hs, 0, "QEMU HID Mouse"); } else if (hs->kind == HID_TABLET) { -- 1.7.1
Re: [Qemu-devel] [PATCH] pcie/slot: fix hotplug event
On Fri, Aug 05, 2011 at 06:22:03PM +0900, Isaku Yamahata wrote: > When slot status register is cleared, PCIDevice::exp.hpev_notify > needs to be cleared. > Otherwise, PCIDevice::exp.hpev_notify is never set to false resulting > in no more hot plug event once it's raised. > > Signed-off-by: Isaku Yamahata Thanks, applied. > --- > hw/pcie.c | 12 > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/hw/pcie.c b/hw/pcie.c > index 39607bf..5c9eb2f 100644 > --- a/hw/pcie.c > +++ b/hw/pcie.c > @@ -175,6 +175,14 @@ static void hotplug_event_notify(PCIDevice *dev) > } > } > > +static void hotplug_event_clear(PCIDevice *dev) > +{ > +hotplug_event_update_event_status(dev); > +if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) { > +qemu_set_irq(dev->irq[dev->exp.hpev_intx], 0); > +} > +} > + > /* > * A PCI Express Hot-Plug Event has occurred, so update slot status register > * and notify OS of the event if necessary. > @@ -320,6 +328,10 @@ void pcie_cap_slot_write_config(PCIDevice *dev, > uint8_t *exp_cap = dev->config + pos; > uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA); > > +if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) { > +hotplug_event_clear(dev); > +} > + > if (!ranges_overlap(addr, len, pos + PCI_EXP_SLTCTL, 2)) { > return; > } > -- > 1.7.1.1
Re: [Qemu-devel] [PATCH] pcie/aer: fix inject aer error command
On Fri, Aug 05, 2011 at 06:22:06PM +0900, Isaku Yamahata wrote: > various fixes to make aer inject error command work. > - wrong assert > - command line parser > - err.status needs initialization > > Signed-off-by: Isaku Yamahata Thanks, applied. > --- > hw/pcie_aer.c |9 + > 1 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c > index be019c7..4346345 100644 > --- a/hw/pcie_aer.c > +++ b/hw/pcie_aer.c > @@ -415,7 +415,7 @@ static void pcie_aer_update_log(PCIDevice *dev, const > PCIEAERErr *err) > int i; > > assert(err->status); > -assert(err->status & (err->status - 1)); > +assert(!(err->status & (err->status - 1))); > > errcap &= ~(PCI_ERR_CAP_FEP_MASK | PCI_ERR_CAP_TLP); > errcap |= PCI_ERR_CAP_FEP(first_bit); > @@ -495,7 +495,7 @@ static int pcie_aer_record_error(PCIDevice *dev, > int fep = PCI_ERR_CAP_FEP(errcap); > > assert(err->status); > -assert(err->status & (err->status - 1)); > +assert(!(err->status & (err->status - 1))); > > if (errcap & PCI_ERR_CAP_MHRE && > (pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS) & (1U << fep))) { > @@ -979,20 +979,21 @@ int do_pcie_aer_inejct_error(Monitor *mon, > if (pcie_aer_parse_error_string(error_name, &error_status, > &correctable)) { > char *e = NULL; > error_status = strtoul(error_name, &e, 0); > -correctable = !!qdict_get_int(qdict, "correctable"); > +correctable = qdict_get_try_bool(qdict, "correctable", 0); > if (!e || *e != '\0') { > monitor_printf(mon, "invalid error status value. \"%s\"", > error_name); > return -EINVAL; > } > } > +err.status = error_status; > err.source_id = (pci_bus_num(dev->bus) << 8) | dev->devfn; > > err.flags = 0; > if (correctable) { > err.flags |= PCIE_AER_ERR_IS_CORRECTABLE; > } > -if (qdict_get_int(qdict, "advisory_non_fatal")) { > +if (qdict_get_try_bool(qdict, "advisory_non_fatal", 0)) { > err.flags |= PCIE_AER_ERR_MAYBE_ADVISORY; > } > if (qdict_haskey(qdict, "header0")) { > -- > 1.7.1.1
Re: [Qemu-devel] [PATCH 00/10] SCSI scatter/gather support
On Thu, Aug 04, 2011 at 07:14:38PM +0200, Paolo Bonzini wrote: > Hi, > > this is the version of SCSI scatter/gather based on the existing > DMA helpers infrastructure. > > The infrastructure required a little update because I need to > know the residual amount of data upon short transfers. To this > end, my choice was to make QEMUSGList mutable and track the > current position in there. Any other ideas are welcome, the > reason for this choice is explained in patch 2. > > The patches are quite self-contained, but they depend on the > changes I posted yesterday. > > Patch 11 is the sample vmw_pvscsi device model that I used to > test the code. This is a good opportunity to rename is_write in dma-helpers because it is confusing. The problem is that bdrv_*() is_write indicates whether the I/O request is a read or write. But in cpu_physical_memory_map() is_write indicates whether we are writing to target memory. These two is_write use cases actually have opposite meanings, therefore the confusing code in dma-helpers.c today: mem = cpu_physical_memory_map(cur_addr, &cur_len, !dbs->is_write); ^^ Please use a DMA direction instead of is_write: DMA-to-device means target->device memory transfer DMA-from-device means device->target memory transfer This patch series is a good place to do the rename because it adds more instances of !dbs->is_write. Stefan
Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write
On Thu, Aug 04, 2011 at 07:14:43PM +0200, Paolo Bonzini wrote: > These helpers do a full transfer from an in-memory buffer to > target memory, with full support for MMIO areas. It will be used to store > the reply of an emulated command into a QEMUSGList provided by the > adapter. > > Signed-off-by: Paolo Bonzini > --- > cutils.c |8 +++--- > dma-helpers.c | 63 > + > dma.h |5 > 3 files changed, 72 insertions(+), 4 deletions(-) I don't understand this patch. If we have a memory buffer that needs to be transferred to target memory, then there is no need for bounce buffers or cpu_physical_memory_map(). Can we use cpu_physical_memory_rw() on each sglist element instead? No -EAGAIN necessary because the memory buffer already acts as the local bounce buffer. Stefan
[Qemu-devel] [PATCH 6/6] usb-hid: remove usb_hid_datain_cb
No users left, all migrated over to hw/hid.[ch]. Yea! Zap it! Signed-off-by: Gerd Hoffmann --- hw/usb-hid.c | 14 -- hw/usb.h |3 --- 2 files changed, 0 insertions(+), 17 deletions(-) diff --git a/hw/usb-hid.c b/hw/usb-hid.c index 6a75147..ba79466 100644 --- a/hw/usb-hid.c +++ b/hw/usb-hid.c @@ -45,8 +45,6 @@ typedef struct USBHIDState { USBDevice dev; HIDState hid; -void *datain_opaque; -void (*datain)(void *); } USBHIDState; enum { @@ -362,10 +360,6 @@ static void usb_hid_changed(HIDState *hs) { USBHIDState *us = container_of(hs, USBHIDState, hid); -if (us->datain) { -us->datain(us->datain_opaque); -} - usb_wakeup(&us->dev); } @@ -533,14 +527,6 @@ static int usb_keyboard_initfn(USBDevice *dev) return usb_hid_initfn(dev, HID_KEYBOARD); } -void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *)) -{ -USBHIDState *s = (USBHIDState *)dev; - -s->datain_opaque = opaque; -s->datain = datain; -} - static const VMStateDescription vmstate_usb_ptr = { .name = "usb-ptr", .version_id = 1, diff --git a/hw/usb.h b/hw/usb.h index 84d04df..d784448 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -316,9 +316,6 @@ USBDevice *usb_host_device_open(const char *devname); int usb_host_device_close(const char *devname); void usb_host_info(Monitor *mon); -/* usb-hid.c */ -void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *)); - /* usb-bt.c */ USBDevice *usb_bt_init(HCIInfo *hci); -- 1.7.1
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
> From: Anthony Liguori [mailto:anth...@codemonkey.ws] > Sent: Wednesday, August 10, 2011 10:28 PM > To: Avi Kivity > Cc: Blue Swirl; Stefan Hajnoczi; Shribman, Aidan; qemu-devel > Developers; libvir-l...@redhat.com > Subject: Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of > large memory apps > a) A query-migration-caps command that returns a dict with two lists of > strings. Something like: > > { 'execute': 'query-migration-caps' } > { 'return' : { 'capabilities': [ 'xbzrle' ], 'current': [] } } > > b) A set-migration-caps command that takes a list of strings. It > simply > takes the intersection of the capabilities set with the argument and > sets the current set to the result. Something like: > > { 'execute': 'set-migration-caps', 'arguments': { 'set': [ 'xbzrle' ] > }} > { 'return' : {} } We may want to further sub-divide capabilities into categories: { 'execute': 'query-migration-caps' } { 'return' : { 'encoding' : { 'current', 'asn.1', 'proto2', 'thrift', etc. } }, { 'delta' : { 'xbzrle', "xdelta", ...} }, { 'compression' : { 'snappy', 'lzo' } } } This would help libvirt/management to select features automatically or manually (via UI) without having to 'understand' the any given capability meaning. > Yes. But that negotiation needs to become part of the "protocol" for > migration. In the absence of that negotiation, we need to use the wire > protocol we use today. We cannot have ad-hoc feature negotiation for > every change we make to the wire protocol. Agreed. Therefore caps plus xbzrle could be added before ASN.1/v1.0 without breaking anything as long as when 'set-migration-caps' is not issued Qemu uses the current protocol. Aidan
Re: [Qemu-devel] [PATCH 00/11] Memory API conversion for ISA
On Thu, Aug 11, 2011 at 10:48:23AM +0300, Avi Kivity wrote: > On 08/11/2011 01:28 AM, Richard Henderson wrote: > >[ Sorry about the partial duplicates, Avi. ] > > > >This patchset is relative to > > git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory-region > > > >although I'm sure it could be applied on top of Avi's batch 4. > > > >This is not complete by any means. I'm interested in feedback > >for some of the trickier points, e.g. pckbd and fdc. > > > >The pckbd mmio interface to mips_jazz is similar to how serial and > >ide/mmio are structured. But there are way more uses of serial. > > > > Thanks, I've queued this on my branch. > > If we're going to work on this in parallel, we'd better coordinate. > Right now I'm going though all files the contain qemu_ram_alloc > alphabetically and converting them (now in 'O'). If anyone wants a > letter, let me know. Hi Avi, I was planing to convert the xilinx_* and etraxfs_* parts soon. Or have you already done any etrax* conversions? Cheers
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/10/2011 10:27 PM, Anthony Liguori wrote: This may be acceptable, wait until the entire migration cluster is xzbrle capable before enabling it. If not, add a monitor command. 1) xzbrle needs to be disabled by default. That way management tools don't unknowingly enable it by not passing -no-xzbrle. We could hook it to -M, though it's a bit gross. Otherwise we need to document this clearly in the management tool author's guide. 3) a management tool should be able to query the source and destination, and then enable xzbrle if both sides support it. You can argue that (3) could be static. A command could be added to toggle it dynamically through the monitor. But no matter what, someone has to touch libvirt and any other tool that works with QEMU to make this thing work. But this is a general problem. Any optional change to the migration protocol has exactly the same characteristics whether it's XZBRLE, XZBRLE v2 (if there is a v2), ASN.1, or any other form of compression that rolls around. If we have two-way communication we can do this transparently in the protocol itself. Instead of teaching management tools how to deal with all of these things, let's just fix this problem once. It just takes: a) A query-migration-caps command that returns a dict with two lists of strings. Something like: { 'execute': 'query-migration-caps' } { 'return' : { 'capabilities': [ 'xbzrle' ], 'current': [] } } b) A set-migration-caps command that takes a list of strings. It simply takes the intersection of the capabilities set with the argument and sets the current set to the result. Something like: { 'execute': 'set-migration-caps', 'arguments': { 'set': [ 'xbzrle' ] }} { 'return' : {} } c) An internal interface to register a capability and an internal interface to check if a capability is currently enabled. The xzbrle code just needs to disable itself if the capability isn't set. Then we teach libvirt (and other tools) to query the caps list on the source, set the destination, query the current set on the destination, and then set that set on the source. This is only if the capability has no side effect. As we introduce new things, like the next great compression protocol, or ASN.1, we don't need to touch libvirt again. libvirt can still know about the caps and selectively override QEMU if it's so inclined but it prevents us from reinventing the same mechanisms over and over again. Right. Yes. But that negotiation needs to become part of the "protocol" for migration. In the absence of that negotiation, we need to use the wire protocol we use today. We cannot have ad-hoc feature negotiation for every change we make to the wire protocol. Okay, as long as we have someone willing to implement it. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 00/11] Memory API conversion for ISA
On 08/11/2011 11:05 AM, Edgar E. Iglesias wrote: > > If we're going to work on this in parallel, we'd better coordinate. > Right now I'm going though all files the contain qemu_ram_alloc > alphabetically and converting them (now in 'O'). If anyone wants a > letter, let me know. Hi Avi, I was planing to convert the xilinx_* and etraxfs_* parts soon. Or have you already done any etrax* conversions? I converted etraxfs.c, which was then removed... I haven't touched xilinx. You can view the entire queue at git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory-region -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
Hello Avi, Thank for the fast fix. Unfortunatly it still doesn't work (but LSI BIOS is initialized correctly). I'm getting at boot time: qemu-system-x86_64: /qemu-kvm-test/memory.c:1168: memory_region_del_subregion: Assertion `subregion->parent == mr' failed. Thnx. Ciao, Gerhard -- http://www.wiesinger.com/ On Thu, 11 Aug 2011, Avi Kivity wrote: The memory API automatically cracks wide memory accesses into narrower (usually byte) accesses when needed. Unfortunately this wasn't implemented for ioports, leading to an lsi53c895a failure. This series implements cracking for ioports as well. Note that the dual implementation is due to the fact the memory API is layered on top of the original qemu API; eventually the same code will be used for both ioports and mmio. Avi Kivity (2): memory: abstract cracking of write access ops into a function memory: crack wide ioport accesses into smaller ones when needed memory.c | 120 +++-- 1 files changed, 77 insertions(+), 43 deletions(-) -- 1.7.5.3
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
On 08/11/2011 11:25 AM, Gerhard Wiesinger wrote: Hello Avi, Thank for the fast fix. Unfortunatly it still doesn't work (but LSI BIOS is initialized correctly). I'm getting at boot time: qemu-system-x86_64: /qemu-kvm-test/memory.c:1168: memory_region_del_subregion: Assertion `subregion->parent == mr' failed. What OS are you booting? What is your qemu command line? How early is the failure? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
On 08/11/2011 11:27 AM, Avi Kivity wrote: On 08/11/2011 11:25 AM, Gerhard Wiesinger wrote: Hello Avi, Thank for the fast fix. Unfortunatly it still doesn't work (but LSI BIOS is initialized correctly). I'm getting at boot time: qemu-system-x86_64: /qemu-kvm-test/memory.c:1168: memory_region_del_subregion: Assertion `subregion->parent == mr' failed. What OS are you booting? What is your qemu command line? How early is the failure? Alternatively, build with debug information (./configure --enable-debug) enable core dumps ('ulimit -c unlimited'), and post a backtrace: gdb qemu-system-x86_64 /path/to/core (gdb) bt It should be immediately apparent where the failure is. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
Hello Avi, #0 0x003a060328f5 in raise () from /lib64/libc.so.6 #1 0x003a060340d5 in abort () from /lib64/libc.so.6 #2 0x003a0602b8b5 in __assert_fail () from /lib64/libc.so.6 #3 0x00435339 in memory_region_del_subregion (mr=, subregion=)at /root/download/qemu/git/qemu-kvm-test/memory.c:1168 #4 0x0041eb9b in pci_update_mappings (d=0x1a90bc0) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1134 #5 0x00420a9c in pci_default_write_config (d=0x1a90bc0, addr=4, val=, l=) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1213 #6 0x004329a6 in kvm_handle_io (env=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/kvm-all.c:858 #7 kvm_cpu_exec (env=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/kvm-all.c:997 #8 0x0040bd4a in qemu_kvm_cpu_thread_fn (arg=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/cpus.c:806 #9 0x003a06807761 in start_thread () from /lib64/libpthread.so.0 #10 0x003a060e098d in clone () from /lib64/libc.so.6 Command line: qemu-system-x86_64 -drive file=gerhard.img,media=disk,if=scsi,bus=0,unit=0 -drive file=gerhard2.img,media=disk,if=scsi,bus=0,unit=1 -drive file=gerhard3.img,media=disk,if=scsi,bus=0,unit=2 -boot order=c -m 256 -k de -vga vmware -vnc :0 -bios bios.bin -option-rom 8xx_64.rom -net nic,model=pcnet,macaddr=1a:46:0b:cc:aa:bb -net tap,ifname=tap0,script=no,downscript=no BTW: Is the new memory API faster? Any ideas how to optimize (if not)? I don't know if you remember but I was looking for fast access for the following use cases for DOS legacy for KVM: 1.) Page switching in the area of 0xA-0xA (linear frame buffer mapping) through INT 0x10 function 2.) Access the memory page As far as I saw there are 2 different virtualization approaches (different in VMWare VGA and cirrus VGA): 1.) Just remember the page on the INT 0x10 function setter and virtualize each access to the page. Advantages: Fast page switching Disadvantages: Each access is virtualized which is slow (you pointed out that each switch from non virtualized to virtualized is very slow and requires thousands of CPU cycles, see archive) 2.) mapping in the INT 0x10 function through memory mapping functions and direct access to the mapped memory area without virtualization. Advantages: Fast direct access Disadvantages with old API: was very slow (was about 1000 switches per second or even lower as far as I remember) As far as I found it out it came from (maybe a linear list issue?): static int cpu_notify_sync_dirty_bitmap(target_phys_addr_t start, target_phys_addr_t end) { CPUPhysMemoryClient *client; QLIST_FOREACH(client, &memory_client_list, list) { int r = client->sync_dirty_bitmap(client, start, end); if (r < 0) return r; } return 0; } kvm_physical_sync_dirty_bitmap I think variant 2 is the preferred one but with optimized switching of mapping. Thnx. Ciao, Gerhard -- http://www.wiesinger.com/ On Thu, 11 Aug 2011, Avi Kivity wrote: Alternatively, build with debug information (./configure --enable-debug) enable core dumps ('ulimit -c unlimited'), and post a backtrace: gdb qemu-system-x86_64 /path/to/core (gdb) bt It should be immediately apparent where the failure is.
Re: [Qemu-devel] [libvirt] [PATCH v4] XBZRLE delta for live migration of large memory apps
On Thu, Aug 11, 2011 at 11:17:09AM +0300, Avi Kivity wrote: > On 08/10/2011 10:27 PM, Anthony Liguori wrote: > >>This may be acceptable, wait until the entire migration cluster is > >>xzbrle capable before enabling it. If not, add a monitor command. > > > > > >1) xzbrle needs to be disabled by default. That way management > >tools don't unknowingly enable it by not passing -no-xzbrle. > > We could hook it to -M, though it's a bit gross. That would needlessly prevent its use for any existing installed guests with a older machine type, which are running in a new QEMU Some kind of monitor capabilities seems good to me. 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] [libvirt] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/11/2011 12:16 PM, Daniel P. Berrange wrote: On Thu, Aug 11, 2011 at 11:17:09AM +0300, Avi Kivity wrote: > On 08/10/2011 10:27 PM, Anthony Liguori wrote: > >>This may be acceptable, wait until the entire migration cluster is > >>xzbrle capable before enabling it. If not, add a monitor command. > > > > > >1) xzbrle needs to be disabled by default. That way management > >tools don't unknowingly enable it by not passing -no-xzbrle. > > We could hook it to -M, though it's a bit gross. That would needlessly prevent its use for any existing installed guests with a older machine type, which are running in a new QEMU You could still enable it explicitly; I'm just trying to get it to be enabled by default. Some kind of monitor capabilities seems good to me. Live migration is probably mostly done in managed environments, so I think you're right. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [libvirt] [PATCH v4] XBZRLE delta for live migration of large memory apps
On Wed, Aug 10, 2011 at 02:27:41PM -0500, Anthony Liguori wrote: > On 08/10/2011 11:40 AM, Avi Kivity wrote: > Instead of teaching management tools how to deal with all of these > things, let's just fix this problem once. It just takes: > > a) A query-migration-caps command that returns a dict with two lists > of strings. Something like: > > { 'execute': 'query-migration-caps' } > { 'return' : { 'capabilities': [ 'xbzrle' ], 'current': [] } } > > b) A set-migration-caps command that takes a list of strings. It > simply takes the intersection of the capabilities set with the > argument and sets the current set to the result. Something like: > > { 'execute': 'set-migration-caps', 'arguments': { 'set': [ 'xbzrle' ] }} > { 'return' : {} } We have a number of commands to set migration parameters (bandwidth, max downtime, etc). One thing that has always troubled me with this, is that they are a global setting for QEMU, not simply the next migration command. This is fine if you expect that only a single 'migrate' command will ever be invoked during the lifetime of QEMU, but this doesn't hold true when we use 'migrate' as a means to implement saving of snapshots/save-to-file, or "core" dumps. For example, when libvirt does 'save to file', we want to set the max bandwidth to unlimited for that, but we don't want that 'unlimited' setting to apply to a future cross-host migration attempt. This means we have to send three commands migrate_set_speed 9223372036854775808 migrate file:/some/file migrate_set_speed 33554432 it doubly sucks because there is no way to reset the migration speed to the QEMU default, so we have to hardcoded (32 << 20) which is what QEMU currently uses. It would be more desirable if we could simply pass in the desired speed, compression algorithm, max downtime, etc, as parameters to the 'migrate' command. And then have 'migrate_set_speed' only affect the current active migration, not any future ones. So a 'query-migration-caps' command is nice, but I think having a set-migration-caps command is wrong. There should just be a 'caps' parameter for 'migrate'. 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 0/2] Fix wide ioport access cracking
On 08/11/2011 12:01 PM, Gerhard Wiesinger wrote: Hello Avi, #0 0x003a060328f5 in raise () from /lib64/libc.so.6 #1 0x003a060340d5 in abort () from /lib64/libc.so.6 #2 0x003a0602b8b5 in __assert_fail () from /lib64/libc.so.6 #3 0x00435339 in memory_region_del_subregion (mr=optimized out>, subregion=)at /root/download/qemu/git/qemu-kvm-test/memory.c:1168 #4 0x0041eb9b in pci_update_mappings (d=0x1a90bc0) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1134 #5 0x00420a9c in pci_default_write_config (d=0x1a90bc0, addr=4, val=, l=) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1213 #6 0x004329a6 in kvm_handle_io (env=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/kvm-all.c:858 #7 kvm_cpu_exec (env=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/kvm-all.c:997 #8 0x0040bd4a in qemu_kvm_cpu_thread_fn (arg=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/cpus.c:806 #9 0x003a06807761 in start_thread () from /lib64/libpthread.so.0 #10 0x003a060e098d in clone () from /lib64/libc.so.6 In frame 4, can you print out i, *r, and d->io_regions[0 through 6]? Some of them may be optimized out unfortunately. BTW: Is the new memory API faster? Any ideas how to optimize (if not)? Currently it has no effect on run time performance. I don't know if you remember but I was looking for fast access for the following use cases for DOS legacy for KVM: 1.) Page switching in the area of 0xA-0xA (linear frame buffer mapping) through INT 0x10 function 2.) Access the memory page As far as I saw there are 2 different virtualization approaches (different in VMWare VGA and cirrus VGA): 1.) Just remember the page on the INT 0x10 function setter and virtualize each access to the page. Advantages: Fast page switching Disadvantages: Each access is virtualized which is slow (you pointed out that each switch from non virtualized to virtualized is very slow and requires thousands of CPU cycles, see archive) 2.) mapping in the INT 0x10 function through memory mapping functions and direct access to the mapped memory area without virtualization. Advantages: Fast direct access Disadvantages with old API: was very slow (was about 1000 switches per second or even lower as far as I remember) As far as I found it out it came from (maybe a linear list issue?): static int cpu_notify_sync_dirty_bitmap(target_phys_addr_t start, target_phys_addr_t end) { CPUPhysMemoryClient *client; QLIST_FOREACH(client, &memory_client_list, list) { int r = client->sync_dirty_bitmap(client, start, end); if (r < 0) return r; } return 0; } kvm_physical_sync_dirty_bitmap I think variant 2 is the preferred one but with optimized switching of mapping. This should be faster today with really new kernels (the problem is not in qemu) but I'm not sure if it's fast enough. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 00/11] Memory API conversion for ISA
On Thu, Aug 11, 2011 at 11:20:04AM +0300, Avi Kivity wrote: > On 08/11/2011 11:05 AM, Edgar E. Iglesias wrote: > >> > >> If we're going to work on this in parallel, we'd better coordinate. > >> Right now I'm going though all files the contain qemu_ram_alloc > >> alphabetically and converting them (now in 'O'). If anyone wants a > >> letter, let me know. > > > >Hi Avi, > > > >I was planing to convert the xilinx_* and etraxfs_* parts soon. > >Or have you already done any etrax* conversions? > > > > I converted etraxfs.c, which was then removed... I've been planing to remove that board for a while, apparantly I did it to late. Sorry. > > I haven't touched xilinx. You can view the entire queue at > >git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory-region OK, thanks. I'll post etrax devs soon and do xilinx devs this weekend. Cheers
[Qemu-devel] [PATCH 0/5] etrax: Convert devices to MemoryRegion
From: "Edgar E. Iglesias" Mechanical conversion of ETRAX devices to use MemoryRegions. I've tested it lightly with the axis-dev88 board, seems to work fine. Note that eth and dma models predate CodingStyle and use tabs, I plan to change that with separate commits at some point. Edgar E. Iglesias (5): etrax-pic: Convert to MemoryRegion etrax-ser: Convert to MemoryRegion etrax-timer: Convert to MemoryRegion etrax-dma: Convert to MemoryRegion etrax-eth: Convert to MemoryRegion hw/etraxfs_dma.c | 43 +++ hw/etraxfs_eth.c | 30 -- hw/etraxfs_pic.c | 30 +++--- hw/etraxfs_ser.c | 33 ++--- hw/etraxfs_timer.c | 31 --- 5 files changed, 92 insertions(+), 75 deletions(-) -- 1.7.5.4
[Qemu-devel] [PATCH] TCG: Add preprocessor guards for optional tcg ops
While compiling current HEAD on a ppc64 box, I was confronted with the following compile errors: tcg/optimize.c: In function ‘tcg_constant_folding’: tcg/optimize.c:546: error: ‘INDEX_op_not_i32’ undeclared (first use in this function) tcg/optimize.c:546: error: (Each undeclared identifier is reported only once tcg/optimize.c:546: error: for each function it appears in.) tcg/optimize.c:546: error: ‘INDEX_op_not_i64’ undeclared (first use in this function) tcg/optimize.c:573: error: ‘INDEX_op_ext32u_i64’ undeclared (first use in this function) make[1]: *** [tcg/optimize.o] Error 1 Obviously, the optimize.c tries to use TCG opcode constants that are optional and thus not defined in some targets, such as ppc64. This patch guards them with the proper #ifdefs, so compilation works again. Signed-off-by: Alexander Graf --- tcg/optimize.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/tcg/optimize.c b/tcg/optimize.c index 7eb5eb1..7b4954c 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -543,7 +543,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, gen_args += 2; args += 2; break; +#if ((TCG_TARGET_REG_BITS == 32) && defined(TCG_TARGET_HAS_not_i32)) || \ +((TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_not_i32) && \ + defined(TCG_TARGET_HAS_not_i64)) CASE_OP_32_64(not): +#endif #ifdef TCG_TARGET_HAS_ext8s_i32 case INDEX_op_ext8s_i32: #endif @@ -568,8 +572,10 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, #ifdef TCG_TARGET_HAS_ext16u_i64 case INDEX_op_ext16u_i64: #endif -#if TCG_TARGET_REG_BITS == 64 +#if (TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_ext32s_i64) case INDEX_op_ext32s_i64: +#endif +#if (TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_ext32u_i64) case INDEX_op_ext32u_i64: #endif if (temps[args[1]].state == TCG_TEMP_CONST) { -- 1.6.0.2
Re: [Qemu-devel] [PATCH 00/11] Memory API conversion for ISA
On 08/11/2011 01:24 PM, Edgar E. Iglesias wrote: > > I converted etraxfs.c, which was then removed... I've been planing to remove that board for a while, apparantly I did it to late. Sorry. No problem, especially as I didn't coordinate this with anyone. > > I haven't touched xilinx. You can view the entire queue at > > git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory-region OK, thanks. I'll post etrax devs soon and do xilinx devs this weekend. Great. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] TCG: Add preprocessor guards for optional tcg ops
On Thu, 11 Aug 2011, Alexander Graf wrote: > While compiling current HEAD on a ppc64 box, I was confronted with the > following compile errors: > > tcg/optimize.c: In function ?tcg_constant_folding?: > tcg/optimize.c:546: error: ?INDEX_op_not_i32? undeclared (first use in this > function) > tcg/optimize.c:546: error: (Each undeclared identifier is reported only once > tcg/optimize.c:546: error: for each function it appears in.) > tcg/optimize.c:546: error: ?INDEX_op_not_i64? undeclared (first use in this > function) > tcg/optimize.c:573: error: ?INDEX_op_ext32u_i64? undeclared (first use in > this function) > make[1]: *** [tcg/optimize.o] Error 1 > > Obviously, the optimize.c tries to use TCG opcode constants that are optional > and thus not defined in some targets, such as ppc64. > > This patch guards them with the proper #ifdefs, so compilation works again. > > Signed-off-by: Alexander Graf > --- > tcg/optimize.c |8 +++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index 7eb5eb1..7b4954c 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -543,7 +543,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s, > uint16_t *tcg_opc_ptr, > gen_args += 2; > args += 2; > break; > +#if ((TCG_TARGET_REG_BITS == 32) && defined(TCG_TARGET_HAS_not_i32)) || \ > +((TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_not_i32) && \ > + defined(TCG_TARGET_HAS_not_i64)) > CASE_OP_32_64(not): > +#endif > #ifdef TCG_TARGET_HAS_ext8s_i32 > case INDEX_op_ext8s_i32: > #endif > @@ -568,8 +572,10 @@ static TCGArg *tcg_constant_folding(TCGContext *s, > uint16_t *tcg_opc_ptr, > #ifdef TCG_TARGET_HAS_ext16u_i64 > case INDEX_op_ext16u_i64: > #endif > -#if TCG_TARGET_REG_BITS == 64 > +#if (TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_ext32s_i64) > case INDEX_op_ext32s_i64: > +#endif > +#if (TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_ext32u_i64) > case INDEX_op_ext32u_i64: > #endif > if (temps[args[1]].state == TCG_TEMP_CONST) { > Or (not even compile tested): diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c index 02a6cb2..2669403 100644 --- a/tcg/ppc64/tcg-target.c +++ b/tcg/ppc64/tcg-target.c @@ -1449,6 +1449,11 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, const TCGArg *args, tcg_out32 (s, NEG | RT (args[0]) | RA (args[1])); break; +case INDEX_op_neg_i32: +case INDEX_op_not_i64: +tcg_out32 (s, NOR | SAB (args[1], args[0], args[1])); +break; + case INDEX_op_add_i64: if (const_args[2]) ppc_addi64 (s, args[0], args[1], args[2]); @@ -1553,6 +1558,10 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, const TCGArg *args, tcg_out32 (s, c | RS (args[1]) | RA (args[0])); break; +case INDEX_op_ext32u_i64: +tcg_out_rld (s, RLDICR, ret, ret, 0, 32); +break; + case INDEX_op_setcond_i32: tcg_out_setcond (s, TCG_TYPE_I32, args[3], args[0], args[1], args[2], const_args[2]); diff --git a/tcg/ppc64/tcg-target.h b/tcg/ppc64/tcg-target.h index 8a6db11..71a5fe9 100644 --- a/tcg/ppc64/tcg-target.h +++ b/tcg/ppc64/tcg-target.h @@ -76,7 +76,7 @@ enum { /* #define TCG_TARGET_HAS_ext16u_i32 */ /* #define TCG_TARGET_HAS_bswap16_i32 */ /* #define TCG_TARGET_HAS_bswap32_i32 */ -/* #define TCG_TARGET_HAS_not_i32 */ +#define TCG_TARGET_HAS_not_i32 #define TCG_TARGET_HAS_neg_i32 /* #define TCG_TARGET_HAS_andc_i32 */ /* #define TCG_TARGET_HAS_orc_i32 */ @@ -91,11 +91,11 @@ enum { #define TCG_TARGET_HAS_ext32s_i64 /* #define TCG_TARGET_HAS_ext8u_i64 */ /* #define TCG_TARGET_HAS_ext16u_i64 */ -/* #define TCG_TARGET_HAS_ext32u_i64 */ +#define TCG_TARGET_HAS_ext32u_i64 /* #define TCG_TARGET_HAS_bswap16_i64 */ /* #define TCG_TARGET_HAS_bswap32_i64 */ /* #define TCG_TARGET_HAS_bswap64_i64 */ -/* #define TCG_TARGET_HAS_not_i64 */ +#define TCG_TARGET_HAS_not_i64 #define TCG_TARGET_HAS_neg_i64 /* #define TCG_TARGET_HAS_andc_i64 */ /* #define TCG_TARGET_HAS_orc_i64 */ -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 2 July 2011 08:50, Jan Kiszka wrote: > From: Jan Kiszka > > Recent compilers look deep into cpu_exec, find longjmp as a noreturn > function and decide to smash some stack variables as they won't be used > again. This may lead to env becoming invalid after return from setjmp, > causing crashes. Fix it by reloading env from cpu_single_env in that > case. Can you give more details of what compiler/platform this was a problem for? My reading of the C standard is that the compiler isn't allowed to trash env across this longjmp, because it's a variable of automatic scope which isn't modified between the setjmp and the longjmp... (We've been looking at this because reloading from cpu_single_env is the wrong fix in the case of user-mode where there are multiple-threads.) Thanks -- PMM
Re: [Qemu-devel] [PATCH 0/5] etrax: Convert devices to MemoryRegion
On 08/11/2011 01:25 PM, Edgar E. Iglesias wrote: From: "Edgar E. Iglesias" Mechanical conversion of ETRAX devices to use MemoryRegions. I've tested it lightly with the axis-dev88 board, seems to work fine. Note that eth and dma models predate CodingStyle and use tabs, I plan to change that with separate commits at some point. Edgar E. Iglesias (5): etrax-pic: Convert to MemoryRegion etrax-ser: Convert to MemoryRegion etrax-timer: Convert to MemoryRegion etrax-dma: Convert to MemoryRegion etrax-eth: Convert to MemoryRegion Only 0/5 made it to the list. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] TCG: Add preprocessor guards for optional tcg ops
On 11.08.2011, at 13:24, malc wrote: > On Thu, 11 Aug 2011, Alexander Graf wrote: > >> While compiling current HEAD on a ppc64 box, I was confronted with the >> following compile errors: >> >> tcg/optimize.c: In function ?tcg_constant_folding?: >> tcg/optimize.c:546: error: ?INDEX_op_not_i32? undeclared (first use in this >> function) >> tcg/optimize.c:546: error: (Each undeclared identifier is reported only once >> tcg/optimize.c:546: error: for each function it appears in.) >> tcg/optimize.c:546: error: ?INDEX_op_not_i64? undeclared (first use in this >> function) >> tcg/optimize.c:573: error: ?INDEX_op_ext32u_i64? undeclared (first use in >> this function) >> make[1]: *** [tcg/optimize.o] Error 1 >> >> Obviously, the optimize.c tries to use TCG opcode constants that are optional >> and thus not defined in some targets, such as ppc64. >> >> This patch guards them with the proper #ifdefs, so compilation works again. >> >> Signed-off-by: Alexander Graf >> --- >> tcg/optimize.c |8 +++- >> 1 files changed, 7 insertions(+), 1 deletions(-) >> >> diff --git a/tcg/optimize.c b/tcg/optimize.c >> index 7eb5eb1..7b4954c 100644 >> --- a/tcg/optimize.c >> +++ b/tcg/optimize.c >> @@ -543,7 +543,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s, >> uint16_t *tcg_opc_ptr, >> gen_args += 2; >> args += 2; >> break; >> +#if ((TCG_TARGET_REG_BITS == 32) && defined(TCG_TARGET_HAS_not_i32)) || \ >> +((TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_not_i32) && \ >> + defined(TCG_TARGET_HAS_not_i64)) >> CASE_OP_32_64(not): >> +#endif >> #ifdef TCG_TARGET_HAS_ext8s_i32 >> case INDEX_op_ext8s_i32: >> #endif >> @@ -568,8 +572,10 @@ static TCGArg *tcg_constant_folding(TCGContext *s, >> uint16_t *tcg_opc_ptr, >> #ifdef TCG_TARGET_HAS_ext16u_i64 >> case INDEX_op_ext16u_i64: >> #endif >> -#if TCG_TARGET_REG_BITS == 64 >> +#if (TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_ext32s_i64) >> case INDEX_op_ext32s_i64: >> +#endif >> +#if (TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_ext32u_i64) >> case INDEX_op_ext32u_i64: >> #endif >> if (temps[args[1]].state == TCG_TEMP_CONST) { >> > > Or (not even compile tested): Sure, but then the next target would pop up with optionals not implemented. The real alternative would be to make these ops non-optional :) Alex > > diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c > index 02a6cb2..2669403 100644 > --- a/tcg/ppc64/tcg-target.c > +++ b/tcg/ppc64/tcg-target.c > @@ -1449,6 +1449,11 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, > const TCGArg *args, > tcg_out32 (s, NEG | RT (args[0]) | RA (args[1])); > break; > > +case INDEX_op_neg_i32: > +case INDEX_op_not_i64: > +tcg_out32 (s, NOR | SAB (args[1], args[0], args[1])); > +break; > + > case INDEX_op_add_i64: > if (const_args[2]) > ppc_addi64 (s, args[0], args[1], args[2]); > @@ -1553,6 +1558,10 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, > const TCGArg *args, > tcg_out32 (s, c | RS (args[1]) | RA (args[0])); > break; > > +case INDEX_op_ext32u_i64: > +tcg_out_rld (s, RLDICR, ret, ret, 0, 32); > +break; > + > case INDEX_op_setcond_i32: > tcg_out_setcond (s, TCG_TYPE_I32, args[3], args[0], args[1], args[2], > const_args[2]); > diff --git a/tcg/ppc64/tcg-target.h b/tcg/ppc64/tcg-target.h > index 8a6db11..71a5fe9 100644 > --- a/tcg/ppc64/tcg-target.h > +++ b/tcg/ppc64/tcg-target.h > @@ -76,7 +76,7 @@ enum { > /* #define TCG_TARGET_HAS_ext16u_i32 */ > /* #define TCG_TARGET_HAS_bswap16_i32 */ > /* #define TCG_TARGET_HAS_bswap32_i32 */ > -/* #define TCG_TARGET_HAS_not_i32 */ > +#define TCG_TARGET_HAS_not_i32 > #define TCG_TARGET_HAS_neg_i32 > /* #define TCG_TARGET_HAS_andc_i32 */ > /* #define TCG_TARGET_HAS_orc_i32 */ > @@ -91,11 +91,11 @@ enum { > #define TCG_TARGET_HAS_ext32s_i64 > /* #define TCG_TARGET_HAS_ext8u_i64 */ > /* #define TCG_TARGET_HAS_ext16u_i64 */ > -/* #define TCG_TARGET_HAS_ext32u_i64 */ > +#define TCG_TARGET_HAS_ext32u_i64 > /* #define TCG_TARGET_HAS_bswap16_i64 */ > /* #define TCG_TARGET_HAS_bswap32_i64 */ > /* #define TCG_TARGET_HAS_bswap64_i64 */ > -/* #define TCG_TARGET_HAS_not_i64 */ > +#define TCG_TARGET_HAS_not_i64 > #define TCG_TARGET_HAS_neg_i64 > /* #define TCG_TARGET_HAS_andc_i64 */ > /* #define TCG_TARGET_HAS_orc_i64 */ > > -- > mailto:av1...@comtv.ru
[Qemu-devel] [PATCH 1/5] etrax-pic: Convert to MemoryRegion
From: "Edgar E. Iglesias" Signed-off-by: Edgar E. Iglesias --- hw/etraxfs_pic.c | 30 +++--- 1 files changed, 15 insertions(+), 15 deletions(-) diff --git a/hw/etraxfs_pic.c b/hw/etraxfs_pic.c index 4feffda..47a56d7 100644 --- a/hw/etraxfs_pic.c +++ b/hw/etraxfs_pic.c @@ -39,6 +39,7 @@ struct etrax_pic { SysBusDevice busdev; +MemoryRegion mmio; void *interrupt_vector; qemu_irq parent_irq; qemu_irq parent_nmi; @@ -77,7 +78,8 @@ static void pic_update(struct etrax_pic *fs) qemu_set_irq(fs->parent_irq, !!vector); } -static uint32_t pic_readl (void *opaque, target_phys_addr_t addr) +static uint64_t +pic_read(void *opaque, target_phys_addr_t addr, unsigned int size) { struct etrax_pic *fs = opaque; uint32_t rval; @@ -87,8 +89,8 @@ static uint32_t pic_readl (void *opaque, target_phys_addr_t addr) return rval; } -static void -pic_writel (void *opaque, target_phys_addr_t addr, uint32_t value) +static void pic_write(void *opaque, target_phys_addr_t addr, + uint64_t value, unsigned int size) { struct etrax_pic *fs = opaque; D(printf("%s addr=%x val=%x\n", __func__, addr, value)); @@ -99,14 +101,14 @@ pic_writel (void *opaque, target_phys_addr_t addr, uint32_t value) } } -static CPUReadMemoryFunc * const pic_read[] = { -NULL, NULL, -&pic_readl, -}; - -static CPUWriteMemoryFunc * const pic_write[] = { -NULL, NULL, -&pic_writel, +static const MemoryRegionOps pic_ops = { +.read = pic_read, +.write = pic_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 4, +.max_access_size = 4 +} }; static void nmi_handler(void *opaque, int irq, int level) @@ -139,15 +141,13 @@ static void irq_handler(void *opaque, int irq, int level) static int etraxfs_pic_init(SysBusDevice *dev) { struct etrax_pic *s = FROM_SYSBUS(typeof (*s), dev); -int intr_vect_regs; qdev_init_gpio_in(&dev->qdev, irq_handler, 32); sysbus_init_irq(dev, &s->parent_irq); sysbus_init_irq(dev, &s->parent_nmi); -intr_vect_regs = cpu_register_io_memory(pic_read, pic_write, s, -DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, R_MAX * 4, intr_vect_regs); +memory_region_init_io(&s->mmio, &pic_ops, s, "etraxfs-pic", R_MAX * 4); +sysbus_init_mmio_region(dev, &s->mmio); return 0; } -- 1.7.5.4
[Qemu-devel] [PATCH 3/5] etrax-timer: Convert to MemoryRegion
From: "Edgar E. Iglesias" Signed-off-by: Edgar E. Iglesias --- hw/etraxfs_timer.c | 31 --- 1 files changed, 16 insertions(+), 15 deletions(-) diff --git a/hw/etraxfs_timer.c b/hw/etraxfs_timer.c index b08e574..57dc739 100644 --- a/hw/etraxfs_timer.c +++ b/hw/etraxfs_timer.c @@ -43,6 +43,7 @@ struct etrax_timer { SysBusDevice busdev; +MemoryRegion mmio; qemu_irq irq; qemu_irq nmi; @@ -72,7 +73,8 @@ struct etrax_timer { uint32_t r_masked_intr; }; -static uint32_t timer_readl (void *opaque, target_phys_addr_t addr) +static uint64_t +timer_read(void *opaque, target_phys_addr_t addr, unsigned int size) { struct etrax_timer *t = opaque; uint32_t r = 0; @@ -239,9 +241,11 @@ static inline void timer_watchdog_update(struct etrax_timer *t, uint32_t value) } static void -timer_writel (void *opaque, target_phys_addr_t addr, uint32_t value) +timer_write(void *opaque, target_phys_addr_t addr, +uint64_t val64, unsigned int size) { struct etrax_timer *t = opaque; +uint32_t value = val64; switch (addr) { @@ -281,14 +285,14 @@ timer_writel (void *opaque, target_phys_addr_t addr, uint32_t value) } } -static CPUReadMemoryFunc * const timer_read[] = { -NULL, NULL, -&timer_readl, -}; - -static CPUWriteMemoryFunc * const timer_write[] = { -NULL, NULL, -&timer_writel, +static const MemoryRegionOps timer_ops = { +.read = timer_read, +.write = timer_write, +.endianness = DEVICE_LITTLE_ENDIAN, +.valid = { +.min_access_size = 4, +.max_access_size = 4 +} }; static void etraxfs_timer_reset(void *opaque) @@ -307,7 +311,6 @@ static void etraxfs_timer_reset(void *opaque) static int etraxfs_timer_init(SysBusDevice *dev) { struct etrax_timer *t = FROM_SYSBUS(typeof (*t), dev); -int timer_regs; t->bh_t0 = qemu_bh_new(timer0_hit, t); t->bh_t1 = qemu_bh_new(timer1_hit, t); @@ -319,10 +322,8 @@ static int etraxfs_timer_init(SysBusDevice *dev) sysbus_init_irq(dev, &t->irq); sysbus_init_irq(dev, &t->nmi); -timer_regs = cpu_register_io_memory(timer_read, timer_write, t, -DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, 0x5c, timer_regs); - +memory_region_init_io(&t->mmio, &timer_ops, t, "etraxfs-timer", 0x5c); +sysbus_init_mmio_region(dev, &t->mmio); qemu_register_reset(etraxfs_timer_reset, t); return 0; } -- 1.7.5.4
[Qemu-devel] [PATCH 4/5] etrax-dma: Convert to MemoryRegion
From: "Edgar E. Iglesias" Signed-off-by: Edgar E. Iglesias --- hw/etraxfs_dma.c | 43 +++ 1 files changed, 27 insertions(+), 16 deletions(-) diff --git a/hw/etraxfs_dma.c b/hw/etraxfs_dma.c index c205ec1..8023fa1 100644 --- a/hw/etraxfs_dma.c +++ b/hw/etraxfs_dma.c @@ -24,6 +24,7 @@ #include #include #include "hw.h" +#include "exec-memory.h" #include "qemu-common.h" #include "sysemu.h" @@ -185,7 +186,7 @@ struct fs_dma_channel struct fs_dma_ctrl { - int map; + MemoryRegion mmio; int nr_channels; struct fs_dma_channel *channels; @@ -562,13 +563,17 @@ static uint32_t dma_rinvalid (void *opaque, target_phys_addr_t addr) return 0; } -static uint32_t -dma_readl (void *opaque, target_phys_addr_t addr) +static uint64_t +dma_read(void *opaque, target_phys_addr_t addr, unsigned int size) { struct fs_dma_ctrl *ctrl = opaque; int c; uint32_t r = 0; + if (size != 4) { + dma_rinvalid(opaque, addr); + } + /* Make addr relative to this channel and bounded to nr regs. */ c = fs_channel(addr); addr &= 0xff; @@ -608,11 +613,17 @@ dma_update_state(struct fs_dma_ctrl *ctrl, int c) } static void -dma_writel (void *opaque, target_phys_addr_t addr, uint32_t value) +dma_write(void *opaque, target_phys_addr_t addr, + uint64_t val64, unsigned int size) { struct fs_dma_ctrl *ctrl = opaque; + uint32_t value = val64; int c; + if (size != 4) { + dma_winvalid(opaque, addr, value); + } + /* Make addr relative to this channel and bounded to nr regs. */ c = fs_channel(addr); addr &= 0xff; @@ -668,16 +679,14 @@ dma_writel (void *opaque, target_phys_addr_t addr, uint32_t value) } } -static CPUReadMemoryFunc * const dma_read[] = { - &dma_rinvalid, - &dma_rinvalid, - &dma_readl, -}; - -static CPUWriteMemoryFunc * const dma_write[] = { - &dma_winvalid, - &dma_winvalid, - &dma_writel, +static const MemoryRegionOps dma_ops = { + .read = dma_read, + .write = dma_write, + .endianness = DEVICE_NATIVE_ENDIAN, + .valid = { + .min_access_size = 1, + .max_access_size = 4 + } }; static int etraxfs_dmac_run(void *opaque) @@ -750,7 +759,9 @@ void *etraxfs_dmac_init(target_phys_addr_t base, int nr_channels) ctrl->nr_channels = nr_channels; ctrl->channels = qemu_mallocz(sizeof ctrl->channels[0] * nr_channels); - ctrl->map = cpu_register_io_memory(dma_read, dma_write, ctrl, DEVICE_NATIVE_ENDIAN); - cpu_register_physical_memory(base, nr_channels * 0x2000, ctrl->map); + memory_region_init_io(&ctrl->mmio, &dma_ops, ctrl, "etraxfs-dma", + nr_channels * 0x2000); + memory_region_add_subregion(get_system_memory(), base, &ctrl->mmio); + return ctrl; } -- 1.7.5.4
[Qemu-devel] [PATCH 2/5] etrax-ser: Convert to MemoryRegion
From: "Edgar E. Iglesias" Signed-off-by: Edgar E. Iglesias --- hw/etraxfs_ser.c | 33 ++--- 1 files changed, 18 insertions(+), 15 deletions(-) diff --git a/hw/etraxfs_ser.c b/hw/etraxfs_ser.c index 28b86ea..f862231 100644 --- a/hw/etraxfs_ser.c +++ b/hw/etraxfs_ser.c @@ -47,6 +47,7 @@ struct etrax_serial { SysBusDevice busdev; +MemoryRegion mmio; CharDriverState *chr; qemu_irq irq; @@ -73,7 +74,8 @@ static void ser_update_irq(struct etrax_serial *s) qemu_set_irq(s->irq, !!s->regs[R_MASKED_INTR]); } -static uint32_t ser_readl (void *opaque, target_phys_addr_t addr) +static uint64_t +ser_read(void *opaque, target_phys_addr_t addr, unsigned int size) { struct etrax_serial *s = opaque; D(CPUState *env = s->env); @@ -108,10 +110,12 @@ static uint32_t ser_readl (void *opaque, target_phys_addr_t addr) } static void -ser_writel (void *opaque, target_phys_addr_t addr, uint32_t value) +ser_write(void *opaque, target_phys_addr_t addr, + uint64_t val64, unsigned int size) { struct etrax_serial *s = opaque; -unsigned char ch = value; +uint32_t value = val64; +unsigned char ch = val64; D(CPUState *env = s->env); D(qemu_log("%s " TARGET_FMT_plx "=%x\n", __func__, addr, value)); @@ -142,14 +146,14 @@ ser_writel (void *opaque, target_phys_addr_t addr, uint32_t value) ser_update_irq(s); } -static CPUReadMemoryFunc * const ser_read[] = { -NULL, NULL, -&ser_readl, -}; - -static CPUWriteMemoryFunc * const ser_write[] = { -NULL, NULL, -&ser_writel, +static const MemoryRegionOps ser_ops = { +.read = ser_read, +.write = ser_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 4, +.max_access_size = 4 +} }; static void serial_receive(void *opaque, const uint8_t *buf, int size) @@ -207,12 +211,11 @@ static void etraxfs_ser_reset(DeviceState *d) static int etraxfs_ser_init(SysBusDevice *dev) { struct etrax_serial *s = FROM_SYSBUS(typeof (*s), dev); -int ser_regs; sysbus_init_irq(dev, &s->irq); -ser_regs = cpu_register_io_memory(ser_read, ser_write, s, - DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, R_MAX * 4, ser_regs); +memory_region_init_io(&s->mmio, &ser_ops, s, "etraxfs-serial", R_MAX * 4); +sysbus_init_mmio_region(dev, &s->mmio); + s->chr = qdev_init_chardev(&dev->qdev); if (s->chr) qemu_chr_add_handlers(s->chr, -- 1.7.5.4
[Qemu-devel] [PATCH 5/5] etrax-eth: Convert to MemoryRegion
From: "Edgar E. Iglesias" Signed-off-by: Edgar E. Iglesias --- hw/etraxfs_eth.c | 30 -- 1 files changed, 16 insertions(+), 14 deletions(-) diff --git a/hw/etraxfs_eth.c b/hw/etraxfs_eth.c index 92d4eca..38b33bf 100644 --- a/hw/etraxfs_eth.c +++ b/hw/etraxfs_eth.c @@ -320,6 +320,7 @@ static void mdio_cycle(struct qemu_mdio *bus) struct fs_eth { SysBusDevice busdev; + MemoryRegion mmio; NICState *nic; NICConf conf; int ethregs; @@ -373,7 +374,8 @@ static void eth_validate_duplex(struct fs_eth *eth) } } -static uint32_t eth_readl (void *opaque, target_phys_addr_t addr) +static uint64_t +eth_read(void *opaque, target_phys_addr_t addr, unsigned int size) { struct fs_eth *eth = opaque; uint32_t r = 0; @@ -417,9 +419,11 @@ static void eth_update_ma(struct fs_eth *eth, int ma) } static void -eth_writel (void *opaque, target_phys_addr_t addr, uint32_t value) +eth_write(void *opaque, target_phys_addr_t addr, + uint64_t val64, unsigned int size) { struct fs_eth *eth = opaque; + uint32_t value = val64; addr >>= 2; switch (addr) @@ -553,14 +557,14 @@ static void eth_set_link(VLANClientState *nc) eth->phy.link = !nc->link_down; } -static CPUReadMemoryFunc * const eth_read[] = { - NULL, NULL, - ð_readl, -}; - -static CPUWriteMemoryFunc * const eth_write[] = { - NULL, NULL, - ð_writel, +static const MemoryRegionOps eth_ops = { + .read = eth_read, + .write = eth_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size = 4 + } }; static void eth_cleanup(VLANClientState *nc) @@ -589,7 +593,6 @@ static NetClientInfo net_etraxfs_info = { static int fs_eth_init(SysBusDevice *dev) { struct fs_eth *s = FROM_SYSBUS(typeof(*s), dev); - int eth_regs; if (!s->dma_out || !s->dma_in) { hw_error("Unconnected ETRAX-FS Ethernet MAC.\n"); @@ -600,9 +603,8 @@ static int fs_eth_init(SysBusDevice *dev) s->dma_in->client.opaque = s; s->dma_in->client.pull = NULL; - eth_regs = cpu_register_io_memory(eth_read, eth_write, s, - DEVICE_LITTLE_ENDIAN); - sysbus_init_mmio(dev, 0x5c, eth_regs); + memory_region_init_io(&s->mmio, ð_ops, s, "etraxfs-eth", 0x5c); + sysbus_init_mmio_region(dev, &s->mmio); qemu_macaddr_default_if_unset(&s->conf.macaddr); s->nic = qemu_new_nic(&net_etraxfs_info, &s->conf, -- 1.7.5.4
Re: [Qemu-devel] [PATCH 1/5] etrax-pic: Convert to MemoryRegion
On 08/11/2011 02:47 PM, Edgar E. Iglesias wrote: From: "Edgar E. Iglesias" Thanks, queued all 5 in the memory-region branch. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write
On 08/11/2011 09:58 AM, Stefan Hajnoczi wrote: On Thu, Aug 04, 2011 at 07:14:43PM +0200, Paolo Bonzini wrote: These helpers do a full transfer from an in-memory buffer to target memory, with full support for MMIO areas. It will be used to store the reply of an emulated command into a QEMUSGList provided by the adapter. Signed-off-by: Paolo Bonzini --- cutils.c |8 +++--- dma-helpers.c | 63 + dma.h |5 3 files changed, 72 insertions(+), 4 deletions(-) I don't understand this patch. If we have a memory buffer that needs to be transferred to target memory, then there is no need for bounce buffers or cpu_physical_memory_map(). Can we use cpu_physical_memory_rw() on each sglist element instead? No -EAGAIN necessary because the memory buffer already acts as the local bounce buffer. Doh, you're obviously right. I don't know what I was thinking. :) What do you think about passing the residual bytes for short transfers? Should I look into updating BlockDriverCompletionFunc, or is the approach of patch 2 okay? If I have an excuse to learn more about Coccinelle, that can be fun. :) Paolo
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 08/11/2011 01:30 PM, Peter Maydell wrote: > Recent compilers look deep into cpu_exec, find longjmp as a noreturn > function and decide to smash some stack variables as they won't be used > again. This may lead to env becoming invalid after return from setjmp, > causing crashes. Fix it by reloading env from cpu_single_env in that > case. Can you give more details of what compiler/platform this was a problem for? My reading of the C standard is that the compiler isn't allowed to trash env across this longjmp, because it's a variable of automatic scope which isn't modified between the setjmp and the longjmp... longjmp can destroy any non-volatile variable (-Wclobbered warns about this). Paolo
[Qemu-devel] [PATCH 2/5] spice-qemu-char: Generate chardev open/close events
Define a state callback and make that generate chardev open/close events when called by the spice-server. Note that for all but the newest spice-server versions (which have a fix for this) the code ignores these events for a spicevmc with a subtype of vdagent, this subtype specific knowledge is undesirable, but unavoidable for now, see: http://lists.freedesktop.org/archives/spice-devel/2011-July/004837.html Signed-off-by: Hans de Goede --- spice-qemu-char.c | 35 ++- 1 files changed, 34 insertions(+), 1 deletions(-) diff --git a/spice-qemu-char.c b/spice-qemu-char.c index 2b8aec4..d55e74a 100644 --- a/spice-qemu-char.c +++ b/spice-qemu-char.c @@ -89,11 +89,39 @@ static int vmc_read(SpiceCharDeviceInstance *sin, uint8_t *buf, int len) return bytes; } +static void vmc_state(SpiceCharDeviceInstance *sin, int connected) +{ +SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin); + +#if SPICE_SERVER_VERSION < 0x000901 +/* + * spice-server calls the state callback for the agent channel when the + * spice client connects / disconnects. Given that not the client but + * the server is doing the parsing of the messages this is wrong as the + * server is still listening. Worse, this causes the parser in the server + * to go out of sync, so we ignore state calls for subtype vdagent + * spicevmc chardevs. For the full story see: + * http://lists.freedesktop.org/archives/spice-devel/2011-July/004837.html + */ +if (strcmp(sin->subtype, "vdagent") == 0) { +return; +} +#endif + +if ((scd->chr->opened && connected) || +(!scd->chr->opened && !connected)) { +return; +} + +qemu_chr_event(scd->chr, connected ? CHR_EVENT_OPENED : CHR_EVENT_CLOSED); +} + static SpiceCharDeviceInterface vmc_interface = { .base.type = SPICE_INTERFACE_CHAR_DEVICE, .base.description = "spice virtual channel char device", .base.major_version = SPICE_INTERFACE_CHAR_DEVICE_MAJOR, .base.minor_version = SPICE_INTERFACE_CHAR_DEVICE_MINOR, +.state = vmc_state, .write = vmc_write, .read = vmc_read, }; @@ -222,7 +250,12 @@ int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr) chr->chr_guest_close = spice_chr_guest_close; s->unblock_timer = qemu_new_timer_ms(vm_clock, spice_chr_unblock, s); -qemu_chr_generic_open(chr); +#if SPICE_SERVER_VERSION < 0x000901 +/* See comment in vmc_state() */ +if (strcmp(subtype, "vdagent") == 0) { +qemu_chr_generic_open(chr); +} +#endif *_chr = chr; return 0; -- 1.7.5.1
[Qemu-devel] [PATCH 1/5] qemu-char: make qemu_chr_event public
Make qemu_chr_event public so that it can be used by chardev code which lives outside of qemu-char.c Signed-off-by: Hans de Goede --- qemu-char.c |2 +- qemu-char.h |1 + 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 8d39500..5d5a6d5 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -119,7 +119,7 @@ static void char_write_unblocked(void *opaque) chr->chr_write_unblocked(chr->handler_opaque); } -static void qemu_chr_event(CharDriverState *s, int event) +void qemu_chr_event(CharDriverState *s, int event) { /* Keep track if the char device is open */ switch (event) { diff --git a/qemu-char.h b/qemu-char.h index 68e7b5b..77ad62d 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -107,6 +107,7 @@ int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg); void qemu_chr_generic_open(CharDriverState *s); int qemu_chr_can_read(CharDriverState *s); void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len); +void qemu_chr_event(CharDriverState *s, int event); int qemu_chr_get_msgfd(CharDriverState *s); void qemu_chr_accept_input(CharDriverState *s); int qemu_chr_add_client(CharDriverState *s, int fd); -- 1.7.5.1
[Qemu-devel] [PATCH 5/5] usb-redir: Don't try to write to the chardev after a close event
Sicne we handle close async in a bh, do_write and thus write can get called after receiving a close event. This patch adds a check to the usb-redir write callback to not do a qemu_chr_write on a closed chardev. Signed-off-by: Hans de Goede --- usb-redir.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/usb-redir.c b/usb-redir.c index 6d8f986..732ddab 100644 --- a/usb-redir.c +++ b/usb-redir.c @@ -226,7 +226,7 @@ static int usbredir_write(void *priv, uint8_t *data, int count) USBRedirDevice *dev = priv; int r; -if (dev->cs->write_blocked) { +if (!dev->cs->opened || dev->cs->write_blocked) { return 0; } -- 1.7.5.1
[Qemu-devel] [PATCH 4/5] usb-redir: Device disconnect + re-connect robustness fixes
These fixes mainly target the other side sending some (error status) packets after a disconnect packet. In some cases these would get queued up and then reported to the controller when a new device gets connected. * Fully reset device state on disconnect * Don't allow a connect message when already connected * Ignore iso and interrupt status messages when disconnected Signed-off-by: Hans de Goede --- usb-redir.c | 22 +- 1 files changed, 21 insertions(+), 1 deletions(-) diff --git a/usb-redir.c b/usb-redir.c index 9ce2c8b..6d8f986 100644 --- a/usb-redir.c +++ b/usb-redir.c @@ -905,6 +905,11 @@ static void usbredir_device_connect(void *priv, { USBRedirDevice *dev = priv; +if (qemu_timer_pending(dev->attach_timer) || dev->dev.attached) { +ERROR("Received device connect while already connected\n"); +return; +} + switch (device_connect->speed) { case usb_redir_speed_low: DPRINTF("attaching low speed device\n"); @@ -933,19 +938,26 @@ static void usbredir_device_connect(void *priv, static void usbredir_device_disconnect(void *priv) { USBRedirDevice *dev = priv; +int i; /* Stop any pending attaches */ qemu_del_timer(dev->attach_timer); if (dev->dev.attached) { usb_device_detach(&dev->dev); -usbredir_cleanup_device_queues(dev); /* * Delay next usb device attach to give the guest a chance to see * see the detach / attach in case of quick close / open succession */ dev->next_attach_time = qemu_get_clock_ms(vm_clock) + 200; } + +/* Reset state so that the next dev connected starts with a clean slate */ +usbredir_cleanup_device_queues(dev); +memset(dev->endpoint, 0, sizeof(dev->endpoint)); +for (i = 0; i < MAX_ENDPOINTS; i++) { +QTAILQ_INIT(&dev->endpoint[i].bufpq); +} } static void usbredir_interface_info(void *priv, @@ -1037,6 +1049,10 @@ static void usbredir_iso_stream_status(void *priv, uint32_t id, DPRINTF("iso status %d ep %02X id %u\n", iso_stream_status->status, ep, id); +if (!dev->dev.attached) { +return; +} + dev->endpoint[EP2I(ep)].iso_error = iso_stream_status->status; if (iso_stream_status->status == usb_redir_stall) { DPRINTF("iso stream stopped by peer ep %02X\n", ep); @@ -1054,6 +1070,10 @@ static void usbredir_interrupt_receiving_status(void *priv, uint32_t id, DPRINTF("interrupt recv status %d ep %02X id %u\n", interrupt_receiving_status->status, ep, id); +if (!dev->dev.attached) { +return; +} + dev->endpoint[EP2I(ep)].interrupt_error = interrupt_receiving_status->status; if (interrupt_receiving_status->status == usb_redir_stall) { -- 1.7.5.1
[Qemu-devel] [PATCH 3/5] usb-redir: Call qemu_chr_guest_open/close
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 6932beb..9ce2c8b 100644 --- a/usb-redir.c +++ b/usb-redir.c @@ -839,6 +839,8 @@ static int usbredir_initfn(USBDevice *udev) /* We'll do the attach once we receive the speed from the usb-host */ udev->auto_attach = 0; +/* Let the other side know we are ready */ +qemu_chr_guest_open(dev->cs); qemu_chr_add_handlers(dev->cs, &usbredir_chr_handlers, dev); return 0; @@ -861,6 +863,7 @@ static void usbredir_handle_destroy(USBDevice *udev) { USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev); +qemu_chr_guest_close(dev->cs); qemu_chr_close(dev->cs); /* Note must be done after qemu_chr_close, as that causes a close event */ qemu_bh_delete(dev->open_close_bh); -- 1.7.5.1
Re: [Qemu-devel] [PATCH] TCG: Add preprocessor guards for optional tcg ops
On 08/11/2011 02:40 PM, Alexander Graf wrote: On 11.08.2011, at 13:24, malc wrote: > On Thu, 11 Aug 2011, Alexander Graf wrote: > >> While compiling current HEAD on a ppc64 box, I was confronted with the >> following compile errors: >> >> tcg/optimize.c: In function ?tcg_constant_folding?: >> tcg/optimize.c:546: error: ?INDEX_op_not_i32? undeclared (first use in this function) >> tcg/optimize.c:546: error: (Each undeclared identifier is reported only once >> tcg/optimize.c:546: error: for each function it appears in.) >> tcg/optimize.c:546: error: ?INDEX_op_not_i64? undeclared (first use in this function) >> tcg/optimize.c:573: error: ?INDEX_op_ext32u_i64? undeclared (first use in this function) >> make[1]: *** [tcg/optimize.o] Error 1 >> >> Obviously, the optimize.c tries to use TCG opcode constants that are optional >> and thus not defined in some targets, such as ppc64. >> >> This patch guards them with the proper #ifdefs, so compilation works again. >> >> Signed-off-by: Alexander Graf >> --- >> tcg/optimize.c |8 +++- >> 1 files changed, 7 insertions(+), 1 deletions(-) >> >> diff --git a/tcg/optimize.c b/tcg/optimize.c >> index 7eb5eb1..7b4954c 100644 >> --- a/tcg/optimize.c >> +++ b/tcg/optimize.c >> @@ -543,7 +543,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, >> gen_args += 2; >> args += 2; >> break; >> +#if ((TCG_TARGET_REG_BITS == 32)&& defined(TCG_TARGET_HAS_not_i32)) || \ >> +((TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_not_i32)&& \ >> + defined(TCG_TARGET_HAS_not_i64)) >> CASE_OP_32_64(not): >> +#endif >> #ifdef TCG_TARGET_HAS_ext8s_i32 >> case INDEX_op_ext8s_i32: >> #endif >> @@ -568,8 +572,10 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, >> #ifdef TCG_TARGET_HAS_ext16u_i64 >> case INDEX_op_ext16u_i64: >> #endif >> -#if TCG_TARGET_REG_BITS == 64 >> +#if (TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_ext32s_i64) >> case INDEX_op_ext32s_i64: >> +#endif >> +#if (TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_ext32u_i64) >> case INDEX_op_ext32u_i64: >> #endif >> if (temps[args[1]].state == TCG_TEMP_CONST) { >> > > Or (not even compile tested): Sure, but then the next target would pop up with optionals not implemented. The real alternative would be to make these ops non-optional :) Or to have automatic generation of the optionals based on the primitives, if the optionals are not present. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] TCG: Add preprocessor guards for optional tcg ops
On 11.08.2011, at 14:28, Avi Kivity wrote: > On 08/11/2011 02:40 PM, Alexander Graf wrote: >> On 11.08.2011, at 13:24, malc wrote: >> >> > On Thu, 11 Aug 2011, Alexander Graf wrote: >> > >> >> While compiling current HEAD on a ppc64 box, I was confronted with the >> >> following compile errors: >> >> >> >> tcg/optimize.c: In function ?tcg_constant_folding?: >> >> tcg/optimize.c:546: error: ?INDEX_op_not_i32? undeclared (first use in >> >> this function) >> >> tcg/optimize.c:546: error: (Each undeclared identifier is reported only >> >> once >> >> tcg/optimize.c:546: error: for each function it appears in.) >> >> tcg/optimize.c:546: error: ?INDEX_op_not_i64? undeclared (first use in >> >> this function) >> >> tcg/optimize.c:573: error: ?INDEX_op_ext32u_i64? undeclared (first use >> >> in this function) >> >> make[1]: *** [tcg/optimize.o] Error 1 >> >> >> >> Obviously, the optimize.c tries to use TCG opcode constants that are >> >> optional >> >> and thus not defined in some targets, such as ppc64. >> >> >> >> This patch guards them with the proper #ifdefs, so compilation works >> >> again. >> >> >> >> Signed-off-by: Alexander Graf >> >> --- >> >> tcg/optimize.c |8 +++- >> >> 1 files changed, 7 insertions(+), 1 deletions(-) >> >> >> >> diff --git a/tcg/optimize.c b/tcg/optimize.c >> >> index 7eb5eb1..7b4954c 100644 >> >> --- a/tcg/optimize.c >> >> +++ b/tcg/optimize.c >> >> @@ -543,7 +543,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s, >> >> uint16_t *tcg_opc_ptr, >> >> gen_args += 2; >> >> args += 2; >> >> break; >> >> +#if ((TCG_TARGET_REG_BITS == 32)&& defined(TCG_TARGET_HAS_not_i32)) || >> >> \ >> >> +((TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_not_i32)&& \ >> >> + defined(TCG_TARGET_HAS_not_i64)) >> >> CASE_OP_32_64(not): >> >> +#endif >> >> #ifdef TCG_TARGET_HAS_ext8s_i32 >> >> case INDEX_op_ext8s_i32: >> >> #endif >> >> @@ -568,8 +572,10 @@ static TCGArg *tcg_constant_folding(TCGContext *s, >> >> uint16_t *tcg_opc_ptr, >> >> #ifdef TCG_TARGET_HAS_ext16u_i64 >> >> case INDEX_op_ext16u_i64: >> >> #endif >> >> -#if TCG_TARGET_REG_BITS == 64 >> >> +#if (TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_ext32s_i64) >> >> case INDEX_op_ext32s_i64: >> >> +#endif >> >> +#if (TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_ext32u_i64) >> >> case INDEX_op_ext32u_i64: >> >> #endif >> >> if (temps[args[1]].state == TCG_TEMP_CONST) { >> >> >> > >> > Or (not even compile tested): >> >> Sure, but then the next target would pop up with optionals not implemented. >> The real alternative would be to make these ops non-optional :) >> > > Or to have automatic generation of the optionals based on the primitives, if > the optionals are not present. That's what's happening in the background already, no? The line is towards tcg users though, not it tcg internal code. Alex
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 11 August 2011 13:16, Paolo Bonzini wrote: > On 08/11/2011 01:30 PM, Peter Maydell wrote: >> Can you give more details of what compiler/platform this was >> a problem for? My reading of the C standard is that the compiler >> isn't allowed to trash env across this longjmp, because it's >> a variable of automatic scope which isn't modified between the >> setjmp and the longjmp... > > longjmp can destroy any non-volatile variable (-Wclobbered warns about > this). "All accessible objects have values [...] as of the time the longjmp function was called, except that the values of objects of automatic storage duration that are local to the function containing the invocation of the corresponding setjmp macro that do not have volatile-qualified type and have been changed between the setjmp invocation and longjmp call are indeterminate." -- C99 section 7.13.2.1 para 3. So variables may only be destroyed if they are all of: * local to the function calling setjmp * not volatile * changed between setjmp and longjmp We don't change env between the setjmp and longjmp so the compiler should not trash it. (Indeed according to Jan in http://lists.gnu.org/archive/html/qemu-devel/2011-07/msg00144.html -Wclobbered doesn't complain about this code.) -- PMM
Re: [Qemu-devel] [PATCH] TCG: Add preprocessor guards for optional tcg ops
On 08/11/2011 03:36 PM, Alexander Graf wrote: > > Or to have automatic generation of the optionals based on the primitives, if the optionals are not present. That's what's happening in the background already, no? The line is towards tcg users though, not it tcg internal code. Yes, and it doesn't make sense to optimize through synthetic instructions. -- 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/11/2011 03:03 AM, Shribman, Aidan wrote: From: Anthony Liguori [mailto:anth...@codemonkey.ws] Sent: Wednesday, August 10, 2011 10:28 PM To: Avi Kivity Cc: Blue Swirl; Stefan Hajnoczi; Shribman, Aidan; qemu-devel Developers; libvir-l...@redhat.com Subject: Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps a) A query-migration-caps command that returns a dict with two lists of strings. Something like: { 'execute': 'query-migration-caps' } { 'return' : { 'capabilities': [ 'xbzrle' ], 'current': [] } } b) A set-migration-caps command that takes a list of strings. It simply takes the intersection of the capabilities set with the argument and sets the current set to the result. Something like: { 'execute': 'set-migration-caps', 'arguments': { 'set': [ 'xbzrle' ] }} { 'return' : {} } We may want to further sub-divide capabilities into categories: { 'execute': 'query-migration-caps' } { 'return' : { 'encoding' : { 'current', 'asn.1', 'proto2', 'thrift', etc. } }, { 'delta' : { 'xbzrle', "xdelta", ...} }, { 'compression' : { 'snappy', 'lzo' } } } This would help libvirt/management to select features automatically or manually (via UI) without having to 'understand' the any given capability meaning. I would prefer caps to be mostly transparent to libvirt. In fact, I'd like to see exactly three caps: xbzrle, asn1, and autonegotiate. I'd like to move the caps negotation into the protocol itself. Yes. But that negotiation needs to become part of the "protocol" for migration. In the absence of that negotiation, we need to use the wire protocol we use today. We cannot have ad-hoc feature negotiation for every change we make to the wire protocol. Agreed. Therefore caps plus xbzrle could be added before ASN.1/v1.0 without breaking anything as long as when 'set-migration-caps' is not issued Qemu uses the current protocol. Exactly. Regards, Anthony Liguori Aidan
[Qemu-devel] [Bug 521994] Re: Windows 98 doesn't detect mouse on qemu and SeaBIOS.
** Changed in: qemu-kvm (Debian) Status: Fix Committed => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/521994 Title: Windows 98 doesn't detect mouse on qemu and SeaBIOS. Status in QEMU: Fix Committed Status in “qemu-kvm” package in Debian: Fix Released Bug description: A windows 98 guest doesn't detect mouse on recent qemu. I bisected and the result is fd646122418ecefcde228d43821d07da79dd99bb is the first bad commit commit fd646122418ecefcde228d43821d07da79dd99bb Author: Anthony Liguori Date: Fri Oct 30 09:06:09 2009 -0500 Switch pc bios from pc-bios to seabios SeaBIOS is a port of pc-bios to GCC. Besides using a more modern tool chain, SeaBIOS introduces a number of new features including PMM support, better BEV and BCV support, and better PnP support. Signed-off-by: Anthony Liguori I got following messages with DEBUG_BIOS Start bios (version 0.5.1-20100111_132716-squirrel.codemonkey.ws) Ram Size=0x0800 (0x high) CPU Mhz=2271 Found 1 cpu(s) max supported 1 cpu(s) PIIX3/PIIX4 init: elcr=00 0c PCI: bus=0 devfn=0x00: vendor_id=0x8086 device_id=0x1237 PCI: bus=0 devfn=0x08: vendor_id=0x8086 device_id=0x7000 PCI: bus=0 devfn=0x09: vendor_id=0x8086 device_id=0x7010 region 4: 0xc000 PCI: bus=0 devfn=0x0b: vendor_id=0x8086 device_id=0x7113 PCI: bus=0 devfn=0x10: vendor_id=0x1013 device_id=0x00b8 region 0: 0xe000 region 1: 0xe200 region 6: 0xe201 MP table addr=0x000f89b0 MPC table addr=0x000f89c0 size=224 SMBIOS ptr=0x000f8990 table=0x07fffef0 ACPI tables: RSDP=0x000f8960 RSDT=0x07ffde30 Scan for VGA option rom Running option rom at c000:0003 VGABios $Id$ Turning on vga console Starting SeaBIOS (version 0.5.1-20100111_132716-squirrel.codemonkey.ws) Found 0 lpt ports Found 0 serial ports ATA controller 0 at 1f0/3f4/c000 (irq 14 dev 9) ATA controller 1 at 170/374/c008 (irq 15 dev 9) ps2 irq but no data. ata0-0: PCHS=812/16/63 translation=none LCHS=812/16/63 ata0-1: PCHS=1152/16/56 translation=none LCHS=1024/16/56 ps2_recvbyte timeout keyboard initialized Scan for option roms Returned 53248 bytes of ZoneHigh e820 map has 6 items: 0: - 0009f400 = 1 1: 0009f400 - 000a = 2 2: 000f - 0010 = 2 3: 0010 - 07ffd000 = 1 4: 07ffd000 - 0800 = 2 5: fffc - 0001 = 2 enter handle_19: NULL Booting from Hard Disk... Booting from :7c00 pnp call arg1=5 pnp call arg1=0 ps2_recvbyte timeout ps2_recvbyte timeout ps2_recvbyte timeout ps2_recvbyte timeout To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/521994/+subscriptions
Re: [Qemu-devel] [PATCH v4] XBZRLE delta for live migration of large memory apps
On 08/11/2011 03:17 AM, Avi Kivity wrote: 3) a management tool should be able to query the source and destination, and then enable xzbrle if both sides support it. You can argue that (3) could be static. A command could be added to toggle it dynamically through the monitor. But no matter what, someone has to touch libvirt and any other tool that works with QEMU to make this thing work. But this is a general problem. Any optional change to the migration protocol has exactly the same characteristics whether it's XZBRLE, XZBRLE v2 (if there is a v2), ASN.1, or any other form of compression that rolls around. If we have two-way communication we can do this transparently in the protocol itself. Yes. This should be one of the initial caps to introduce. Instead of teaching management tools how to deal with all of these things, let's just fix this problem once. It just takes: a) A query-migration-caps command that returns a dict with two lists of strings. Something like: { 'execute': 'query-migration-caps' } { 'return' : { 'capabilities': [ 'xbzrle' ], 'current': [] } } b) A set-migration-caps command that takes a list of strings. It simply takes the intersection of the capabilities set with the argument and sets the current set to the result. Something like: { 'execute': 'set-migration-caps', 'arguments': { 'set': [ 'xbzrle' ] }} { 'return' : {} } c) An internal interface to register a capability and an internal interface to check if a capability is currently enabled. The xzbrle code just needs to disable itself if the capability isn't set. Then we teach libvirt (and other tools) to query the caps list on the source, set the destination, query the current set on the destination, and then set that set on the source. This is only if the capability has no side effect. Right, it can't change the output of any monitor commands or anything like that. It's strictly about the encoding of the wire protocol which ought to be transparent to libvirt. As we introduce new things, like the next great compression protocol, or ASN.1, we don't need to touch libvirt again. libvirt can still know about the caps and selectively override QEMU if it's so inclined but it prevents us from reinventing the same mechanisms over and over again. Right. Yes. But that negotiation needs to become part of the "protocol" for migration. In the absence of that negotiation, we need to use the wire protocol we use today. We cannot have ad-hoc feature negotiation for every change we make to the wire protocol. Okay, as long as we have someone willing to implement it. Sounds like a good hackathon project :-) Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 08/11/2011 02:40 PM, Peter Maydell wrote: "All accessible objects have values [...] as of the time the longjmp function was called, except that the values of objects of automatic storage duration that are local to the function containing the invocation of the corresponding setjmp macro that do not have volatile-qualified type and have been changed between the setjmp invocation and longjmp call are indeterminate." -- C99 section 7.13.2.1 para 3. So variables may only be destroyed if they are all of: * local to the function calling setjmp * not volatile * changed between setjmp and longjmp I didn't remember this third part. Thanks for bringing up facts. :) We don't change env between the setjmp and longjmp so the compiler should not trash it. (Indeed according to Jan in http://lists.gnu.org/archive/html/qemu-devel/2011-07/msg00144.html -Wclobbered doesn't complain about this code.) Then it's a compiler bug, not smartness. Making env volatile (or making a volatile copy if there is a performance impact) should still be enough to work around it. Paolo
Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write
On Thu, Aug 11, 2011 at 1:10 PM, Paolo Bonzini wrote: > On 08/11/2011 09:58 AM, Stefan Hajnoczi wrote: >> >> On Thu, Aug 04, 2011 at 07:14:43PM +0200, Paolo Bonzini wrote: >>> >>> These helpers do a full transfer from an in-memory buffer to >>> target memory, with full support for MMIO areas. It will be used to >>> store >>> the reply of an emulated command into a QEMUSGList provided by the >>> adapter. >>> >>> Signed-off-by: Paolo Bonzini >>> --- >>> cutils.c | 8 +++--- >>> dma-helpers.c | 63 >>> + >>> dma.h | 5 >>> 3 files changed, 72 insertions(+), 4 deletions(-) >> >> I don't understand this patch. If we have a memory buffer that needs to >> be transferred to target memory, then there is no need for bounce >> buffers or cpu_physical_memory_map(). >> >> Can we use cpu_physical_memory_rw() on each sglist element instead? No >> -EAGAIN necessary because the memory buffer already acts as the local >> bounce buffer. > > Doh, you're obviously right. I don't know what I was thinking. :) > > What do you think about passing the residual bytes for short transfers? > Should I look into updating BlockDriverCompletionFunc, or is the approach > of patch 2 okay? If I have an excuse to learn more about Coccinelle, that > can be fun. :) The bdrv_aio_readv() and bdrv_aio_writev() functions don't have the concept of residual bytes. They only work on fully completed I/O operations. If there is an error they pass -errno. Therefore I don't think BlockDriverCompletionFunc is the right type to add residual bytes to. It seems that residual bytes are a SCSI layer concept that the block layer does not deal with. Stefan
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 11 August 2011 14:13, Paolo Bonzini wrote: > On 08/11/2011 02:40 PM, Peter Maydell wrote: >> We don't change env between the setjmp and longjmp so the compiler >> should not trash it. (Indeed according to Jan in >> http://lists.gnu.org/archive/html/qemu-devel/2011-07/msg00144.html >> -Wclobbered doesn't complain about this code.) > > Then it's a compiler bug, not smartness. Making env volatile (or making a > volatile copy if there is a performance impact) should still be enough to > work around it. Yes. (It would have to be a volatile copy, I think, env is a function parameter and I don't think you can make those volatile.) https://bugs.launchpad.net/qemu/+bug/823902 includes some discussion of the effects on the test of adding the volatile copy. -- PMM
[Qemu-devel] [RFC] Planning for 1.0 (and freezing the master branch)
Hi, I've posted an initial proposal for the 1.0 release on the wiki[1]. The release would be targeted for December 1st. Unlike previous releases, I'm proposing that instead of forking master into a stable branch and releasing from there, we stop taking features into master and all work on stablizing master. Historically, we forked stable for the releases simply because it was the least intrusive way to get us on a somewhat reasonable release schedule. I think especially since we're targeting a 1.0, we're ready to get a bit more serious about releases. I see the following advantages in using master for releases: 1) All maintainers are participating in the process which means releases are much less likely to be delayed due to lack of time from a single person. 2) Everyone (contributors) are forced to focus on stability which should improve the release's quality. 3) Feature development can still happen in maintainers trees. I think this is an important step to moving to a merge-window based development model which I think will be our best way to scale long term. Obviously, this will only work well if all the maintainers participate so I'd really like to hear what everyone thinks about this. [1] http://wiki.qemu.org/Planning/1.0 Regards, Anthony Liguori
Re: [Qemu-devel] RFCv2: virDomainSnapshotCreateXML enhancements
[ CCed qemu-devel, just in case someone's interested ] Am 11.08.2011 15:23, schrieb Eric Blake: >> [ Okay, some of it is handled later in this document, but I think it's >> still useful to leave this summary in my mail. External VM state is >> something that you don't seem to have covered yet - can't we do this >> already with live migration to a file? ] > > Yes, external VM state is already covered with live migration to file, > and I will not be touching it while implementing this RFC, but future > extensions may be able to further unify the two concepts. Thanks for your explanation regarding the multiple dimensions of snapshot options, it all makes sense. I also agree with your incremental approach. I just wanted to make sure that we keep the possible extension in mind so that we won't end up in a design that makes assumptions that don't hold true in the long run. And in the long run I think that looking into unifying these features is something that should be done, because they are really similar. >>> libvirt can be taught to honor persistent=no for qemu by creating a >>> qcow2 wrapper file prior to starting qemu, then tearing down that >>> wrapper after the fact, although I'll probably leave that for later in >>> my patch series. >> >> qemu can already do this with -drive snapshot=on. It must be allowed to >> create a temporary file for this to work, of course. Is that a problem? >> If not, I would just forward the option to qemu. > > Where would that file be created? If the main image is in a directory, > is the temporary would also live in that directory (shared storage > visible to another qemu for migration purposes) or in local storage > (preventing migration)? It uses whatever mkstemp() returns, i.e. usually something in /tmp. > If migration is possible, would libvirt need to > be able to learn the name of the temporary file so as to tell the new > qemu on the destination the same temporary file name it should open? That's a good point that I haven't thought of. Temporary disks isn't something that immediately reminds me of VMs using live migration, but there's really no reason against it. So maybe duplicating this in libvirt could make some sense indeed. > What about if the main image is a block device - there, the temporary > file obviously has to live somewhere else, but how does qemu decide > where, and should that decision be configurable by the user? How will > things interact with SELinux labeling? What about down the road when we > add enhancements to enforce that qemu cannot open() files on NFS, but > must instead receive fds by inheritance? Yeah, that was basically my question, if letting qemu create a file in /tmp would be a problem from a libvirt/SELinux perspective. Of course, you're much more flexible if libvirt does it manually and allows to specify where you want to create the temporary image etc. > This certainly sounds like some fertile ground for design decisions on > how libvirt and qemu should interact; I don't know if -drive snapshot=on > is reliable enough for use by libvirt, or whether libvirt will end up > having to manage things itself. > > Obviously, my implementation of this RFC will start simple, by rejecting > persistent=no for qemu, until we've answered some of those other design > questions; I can get snapshot_blkdev support working before we have to > tackle this enhancement. > >>> The other thing to be aware of is that with internal snapshots, qcow2 >>> maintains a distinction between current state and a snapshot - that is, >>> qcow2 is _always_ tracking a delta, and never modifies a named snapshot, >>> even when you use 'qemu-img snapshot -a' to revert to different snapshot >>> names. But with named files, the original file now becomes a read-only >>> backing file to a new active file; if we revert to the original file, >>> and make any modifications to it, the active file that was using it as >>> backing will be corrupted. Therefore, the safest thing is to reject any >>> attempt to revert to any snapshot (whether checkpoint or disk snapshot) >>> that has an existing child snapshot consisting of an external disk >>> snapshot. The metadata for each of these children can be deleted >>> manually, but that requires quite a few API calls (learn how many >>> children exist, get the list of children, and for each child, get its >>> xml to see if that child has the target snapshot as a parent, and if so >>> delete the snapshot). So as shorthand, virDomainRevertToSnapshot will >>> be taught a new flag, VIR_DOMAIN_SNAPSHOT_REVERT_DELETE_CHILDREN, which >>> first deletes any children of the snapshot about to be deleted prior to >>> reverting to that particular child. >> >> I think the API should make it possible to revert to a given external >> snapshot without deleting all children, but by creating another qcow2 >> file that uses the same backing file. Basically this new qcow2 file >> would be the equivalent to the "current state
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 08/11/2011 03:31 PM, Peter Maydell wrote: Then it's a compiler bug, not smartness. Making env volatile (or making a volatile copy if there is a performance impact) should still be enough to work around it. Yes. (It would have to be a volatile copy, I think, env is a function parameter and I don't think you can make those volatile.) https://bugs.launchpad.net/qemu/+bug/823902 includes some discussion of the effects on the test of adding the volatile copy. I'm not sure about what to read from there: If I make cpu_single_env thread local with __thread and leave 0d101... in, then again it works reliably on 32bit Lucid, and is flaky on 64 bit Oneiric (5/10 2 hangs, 3 segs) I've also tried using a volatile local variable in cpu_exec to hold a copy of env and restore that rather than cpu_single_env. With this it's solid on 32bit lucid and flaky on 64bit Oneirc; these failures on 64bit OO look like it running off the end of the code buffer (all 0 code), jumping to non-existent code addresses and a seg in tb_reset_jump_recursive2. It looks like neither a thread-local cpu_single_env nor a volatile copy fix the bug?!? I cannot think off-hand of a reason why thread-local cpu_single_env should not work for iothread under Unix, BTW. Since cpu_single_env is only set/used by a thread at a time (under the global lock), its users cannot distinguish between a thread-local variable and a global. The only problem would be Windows, which runs cpu_signal in a thread different than the CPU thread. But that can be fixed easily in qemu_cpu_kick_thread. Paolo
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 11 August 2011 15:10, Paolo Bonzini wrote: > I'm not sure about what to read from there: > >> If I make cpu_single_env thread local with __thread and leave >> 0d101... in, then again it works reliably on 32bit Lucid, and is >> flaky on 64 bit Oneiric (5/10 2 hangs, 3 segs) >> >> I've also tried using a volatile local variable in cpu_exec to hold >> a copy of env and restore that rather than cpu_single_env. With this >> it's solid on 32bit lucid and flaky on 64bit Oneirc; these failures >> on 64bit OO look like it running off the end of the code buffer (all >> 0 code), jumping to non-existent code addresses and a seg in >> tb_reset_jump_recursive2. > > It looks like neither a thread-local cpu_single_env nor a volatile copy fix > the bug?!? As I say at the bottom of that bug I'm assuming I'm hitting multiple bugs. Although it's not clear to me why I don't hit them on 32bit lucid. Dave
Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write
On 08/11/2011 03:29 PM, Stefan Hajnoczi wrote: > > What do you think about passing the residual bytes for short transfers? >Should I look into updating BlockDriverCompletionFunc, or is the approach > of patch 2 okay? If I have an excuse to learn more about Coccinelle, that > can be fun.:) The bdrv_aio_readv() and bdrv_aio_writev() functions don't have the concept of residual bytes. They only work on fully completed I/O operations. If there is an error they pass -errno. But if a transfer was split due to failure of cpu_physical_memory_map, and only the second part fails, you can have a short transfer and you need to pass residual bytes back. The only way out of this is to make a bounce buffer as big as all the unmappable parts of the S/G list, which is undesirable of course. So the residual bytes are a general DMA concept, not specific to SCSI. Therefore I don't think BlockDriverCompletionFunc is the right type to add residual bytes to. Right, I would rather update BlockDriverCompletionFunc to pass the AIOCB as a third parameter, and store the residual bytes in the DMAAIOCB (with a getter that the completion function can use). Paolo
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 11 August 2011 15:10, Paolo Bonzini wrote: > On 08/11/2011 03:31 PM, Peter Maydell wrote: Then it's a compiler bug, not smartness. Making env volatile (or making a volatile copy if there is a performance impact) should still be enough to work around it. >> >> Yes. (It would have to be a volatile copy, I think, env is a function >> parameter and I don't think you can make those volatile.) >> https://bugs.launchpad.net/qemu/+bug/823902 includes some discussion >> of the effects on the test of adding the volatile copy. > > I'm not sure about what to read from there: > >> If I make cpu_single_env thread local with __thread and leave >> 0d101... in, then again it works reliably on 32bit Lucid, and is >> flaky on 64 bit Oneiric (5/10 2 hangs, 3 segs) >> >> I've also tried using a volatile local variable in cpu_exec to hold >> a copy of env and restore that rather than cpu_single_env. With this >> it's solid on 32bit lucid and flaky on 64bit Oneirc; these failures >> on 64bit OO look like it running off the end of the code buffer (all >> 0 code), jumping to non-existent code addresses and a seg in >> tb_reset_jump_recursive2. > > It looks like neither a thread-local cpu_single_env nor a volatile copy fix > the bug?!? My guess is that we're running into the various other problems qemu has with significantly multithreaded programs in user-mode, which makes it a bit hard to disentangle this specific regression. (In particular segfaults in tb_reset_jump_recursive2 are almost certainly bug 668799.) > I cannot think off-hand of a reason why thread-local cpu_single_env should > not work for iothread under Unix, BTW. Since cpu_single_env is only > set/used by a thread at a time (under the global lock), its users cannot > distinguish between a thread-local variable and a global. Thanks for the clarification. As you say, as long as we don't ever try to access it from another thread we're fine... > The only problem would be Windows, which runs cpu_signal in a thread > different than the CPU thread. But that can be fixed easily in > qemu_cpu_kick_thread. ...and we just need to fix this. -- PMM
Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp
On 08/11/2011 04:24 PM, Peter Maydell wrote: I cannot think off-hand of a reason why thread-local cpu_single_env should not work for iothread under Unix, BTW. Since cpu_single_env is only set/used by a thread at a time (under the global lock), its users cannot distinguish between a thread-local variable and a global. Thanks for the clarification. As you say, as long as we don't ever try to access it from another thread we're fine... Yes, and the current usage of the lock should be enough of a guarantee. The only problem would be Windows, which runs cpu_signal in a thread different than the CPU thread. But that can be fixed easily in qemu_cpu_kick_thread. ...and we just need to fix this. Untested (uncompiled) patch follows: diff --git a/cpus.c b/cpus.c index 6bf4e3f..04e52fe 100644 --- a/cpus.c +++ b/cpus.c @@ -179,10 +179,10 @@ static void cpu_handle_guest_debug(CPUState *env) } #ifdef CONFIG_IOTHREAD -static void cpu_signal(int sig) +static inline void do_cpu_kick(CPUState *env) { -if (cpu_single_env) { -cpu_exit(cpu_single_env); +if (env) { +cpu_exit(env); } exit_request = 1; } @@ -476,6 +476,13 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) } } +#ifdef CONFIG_IOTHREAD +static void cpu_signal(int sig) +{ +do_cpu_kick(cpu_single_env); +} +#endif + static void qemu_tcg_init_cpu_signals(void) { #ifdef CONFIG_IOTHREAD @@ -858,7 +865,7 @@ static void qemu_cpu_kick_thread(CPUState *env) #else /* _WIN32 */ if (!qemu_cpu_is_self(env)) { SuspendThread(env->thread->thread); -cpu_signal(0); +do_cpu_kick(env); ResumeThread(env->thread->thread); } #endif
Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write
Am 11.08.2011 16:24, schrieb Paolo Bonzini: > On 08/11/2011 03:29 PM, Stefan Hajnoczi wrote: What do you think about passing the residual bytes for short transfers? Should I look into updating BlockDriverCompletionFunc, or is the approach of patch 2 okay? If I have an excuse to learn more about Coccinelle, that can be fun.:) >> The bdrv_aio_readv() and bdrv_aio_writev() functions don't have the >> concept of residual bytes. They only work on fully completed I/O >> operations. If there is an error they pass -errno. > > But if a transfer was split due to failure of cpu_physical_memory_map, > and only the second part fails, you can have a short transfer and you > need to pass residual bytes back. The only way out of this is to make a > bounce buffer as big as all the unmappable parts of the S/G list, which > is undesirable of course. So the residual bytes are a general DMA > concept, not specific to SCSI. > >> Therefore I don't think BlockDriverCompletionFunc is the right type >> to add residual bytes to. > > Right, I would rather update BlockDriverCompletionFunc to pass the AIOCB > as a third parameter, and store the residual bytes in the DMAAIOCB (with > a getter that the completion function can use). Isn't the DMAAIOCB already passed as opaque to the callback? Kevin
Re: [Qemu-devel] [PATCH 00/11] Memory API conversion for ISA
On 08/11/2011 12:48 AM, Avi Kivity wrote: > If we're going to work on this in parallel, we'd better coordinate. > Right now I'm going though all files the contain qemu_ram_alloc > alphabetically and converting them (now in 'O'). If anyone wants a > letter, let me know. I've been looking at files that contain isa_init_ioport. Although I fear all the easy ones are now done and they get more difficult from here... r~
Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write
On 08/11/2011 04:37 PM, Kevin Wolf wrote: > Right, I would rather update BlockDriverCompletionFunc to pass the AIOCB > as a third parameter, and store the residual bytes in the DMAAIOCB (with > a getter that the completion function can use). Isn't the DMAAIOCB already passed as opaque to the callback? It is passed to the dma_bdrv_cb, but not to the caller-provided callback. If the operation completes before dma_bdrv_{read,write} returns, the AIOCB is not stored anywhere and the asynchronous callback does not have access to it. Usually it does not have anything to do with it, but in this case it could get the residual. Another possibility is always completing DMA in a bottom half. This ensures that the callback can access the AIOCB, but it exposes an implementation detail to the caller, so I don't like it. Paolo
Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write
Am 11.08.2011 17:05, schrieb Paolo Bonzini: > On 08/11/2011 04:37 PM, Kevin Wolf wrote: >>> Right, I would rather update BlockDriverCompletionFunc to pass the AIOCB >>> as a third parameter, and store the residual bytes in the DMAAIOCB (with >>> a getter that the completion function can use). >> >> Isn't the DMAAIOCB already passed as opaque to the callback? > > It is passed to the dma_bdrv_cb, but not to the caller-provided > callback. If the operation completes before dma_bdrv_{read,write} > returns, the AIOCB is not stored anywhere and the asynchronous callback > does not have access to it. Usually it does not have anything to do > with it, but in this case it could get the residual. > > Another possibility is always completing DMA in a bottom half. This > ensures that the callback can access the AIOCB, but it exposes an > implementation detail to the caller, so I don't like it. At least in the block layer, AIO callbacks may never be called before the submission function has returned. I think this makes the DMA helpers provide the same behaviour. But I'm not sure if the definition of the AIOCB struct isn't private to the block layer. Kevin
Re: [Qemu-devel] [PATCH 00/11] Memory API conversion for ISA
On 08/11/2011 05:58 PM, Richard Henderson wrote: On 08/11/2011 12:48 AM, Avi Kivity wrote: > If we're going to work on this in parallel, we'd better coordinate. > Right now I'm going though all files the contain qemu_ram_alloc > alphabetically and converting them (now in 'O'). If anyone wants a > letter, let me know. I've been looking at files that contain isa_init_ioport. Although I fear all the easy ones are now done and they get more difficult from here... I think we need a multi-region helper: static const ISAIOPort list = { { 0x3c0, read_3c0, write_3c0 }, ... }; isa_init_ioport_list(dev, list); -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write
On 08/11/2011 05:12 PM, Kevin Wolf wrote: > Another possibility is always completing DMA in a bottom half. This > ensures that the callback can access the AIOCB, but it exposes an > implementation detail to the caller, so I don't like it. At least in the block layer, AIO callbacks may never be called before the submission function has returned. I think this makes the DMA helpers provide the same behaviour. But I'm not sure if the definition of the AIOCB struct isn't private to the block layer. Yes, it is; I would add a getter that is specific to the DMAAIOCB. Paolo
Re: [Qemu-devel] [PATCHSET 2] VirtFS coroutine changes
On Mon, 8 Aug 2011 22:36:27 +0530, "Aneesh Kumar K.V" wrote: > Full patchset is available at > > git://repo.or.cz/qemu/v9fs.git for-upstream-1 > > -aneesh > > Any update on this. I updated the for-upstream-1 branch with changes to fold few patches. But there are no code changes. -aneesh
[Qemu-devel] sparc32_dma: correctly initialize ledma base address
The ledma base address defaults to 0xff00 on reset. This fixes a bug with Solaris and SS-20 OBP when boot net is skipped. Signed-off-by: Bob Breuer --- diff --git a/hw/sparc32_dma.c b/hw/sparc32_dma.c index e75694b..61812fb 100644 --- a/hw/sparc32_dma.c +++ b/hw/sparc32_dma.c @@ -252,6 +252,9 @@ static void dma_reset(DeviceState *d) memset(s->dmaregs, 0, DMA_SIZE); s->dmaregs[0] = DMA_VER; +if (s->is_ledma) { +s->dmaregs[3] = 0xff00; +} } static const VMStateDescription vmstate_dma = {
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
On Thu, 11 Aug 2011, Avi Kivity wrote: On 08/11/2011 12:01 PM, Gerhard Wiesinger wrote: Hello Avi, #0 0x003a060328f5 in raise () from /lib64/libc.so.6 #1 0x003a060340d5 in abort () from /lib64/libc.so.6 #2 0x003a0602b8b5 in __assert_fail () from /lib64/libc.so.6 #3 0x00435339 in memory_region_del_subregion (mr=out>, subregion=)at /root/download/qemu/git/qemu-kvm-test/memory.c:1168 #4 0x0041eb9b in pci_update_mappings (d=0x1a90bc0) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1134 #5 0x00420a9c in pci_default_write_config (d=0x1a90bc0, addr=4, val=, l=) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1213 #6 0x004329a6 in kvm_handle_io (env=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/kvm-all.c:858 #7 kvm_cpu_exec (env=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/kvm-all.c:997 #8 0x0040bd4a in qemu_kvm_cpu_thread_fn (arg=0x1931af0) at /root/download/qemu/git/qemu-kvm-test/cpus.c:806 #9 0x003a06807761 in start_thread () from /lib64/libpthread.so.0 #10 0x003a060e098d in clone () from /lib64/libc.so.6 In frame 4, can you print out i, *r, and d->io_regions[0 through 6]? Some of them may be optimized out unfortunately. See below. Ciao, Gerhard (gdb) frame 4 #4 0x0041eb9b in pci_update_mappings (d=0x1a90bc0) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1134 1134memory_region_del_subregion(r->address_space, r->memory); (gdb) print i $1 = (gdb) print *r $2 = {addr = 22058952032257, size = 32, filtered_size = 171717340864446496, type = 1 '\001', memory = 0x1a9, address_space = 0x200019282f0} (gdb) print d->io_regions[0] $3 = {addr = 22058952032257, size = 32, filtered_size = 171717340864446496, type = 1 '\001', memory = 0x1a9, address_space = 0x200019282f0} (gdb) print d->io_regions[1] $4 = {addr = 17113088, size = 32, filtered_size = 32, type = 0 '\000', memory = 0x1a911c8, address_space = 0x192} (gdb) print d->io_regions[2] $5 = {addr = 0, size = 0, filtered_size = 0, type = 0 '\000', memory = 0x0, address_space = 0x0} (gdb) print d->io_regions[3] $6 = {addr = 0, size = 0, filtered_size = 0, type = 239 '\357', memory = 0x0, address_space = 0x0} (gdb) print d->io_regions[4] $7 = {addr = 0, size = 0, filtered_size = 0, type = 0 '\000', memory = 0x0, address_space = 0x0} (gdb) print d->io_regions[5] $8 = {addr = 0, size = 0, filtered_size = 0, type = 0 '\000', memory = 0x0, address_space = 0x0} (gdb) print d->io_regions[6] $9 = {addr = 0, size = 0, filtered_size = 0, type = 0 '\000', memory = 0x0, address_space = 0x0} -- http://www.wiesinger.com/
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
On Thu, 11 Aug 2011, Avi Kivity wrote: This should be faster today with really new kernels (the problem is not in qemu) but I'm not sure if it's fast enough. What's a "really new" kernel? In which version were performance optimizations done? (Currently I'm using 2.6.34.7, hadn't time yet to update from FC13 to FC15 ...) Ciao, Gerhard
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
On 08/11/2011 07:11 PM, Gerhard Wiesinger wrote: On Thu, 11 Aug 2011, Avi Kivity wrote: This should be faster today with really new kernels (the problem is not in qemu) but I'm not sure if it's fast enough. What's a "really new" kernel? In which version were performance optimizations done? (Currently I'm using 2.6.34.7, hadn't time yet to update from FC13 to FC15 ...) Not sure really... some of the improvements were in kvm itself (rcu_note_context_switch), some in the rcu core. Try out F15, it will give you 3.0 (and gnome-shell). -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PULL] [PATCHSET 1] VirtFS coroutine changes
On Mon, 8 Aug 2011 22:33:48 +0530, "Aneesh Kumar K.V" wrote: > Full patchset is available at > > git://repo.or.cz/qemu/v9fs.git for-upstream-1 > > -aneesh Here is the updated pull request. I folded few patches based on the review. But there are no code 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 (15): 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 Stefan Hajnoczi (1): coroutine: add gthread dependency Venkateswararao Jujjuri (5): hw/9pfs: Update v9fs_readlink to use coroutine hw/9pfs: Add yield support for mkdir coroutine hw/9pfs: Update mkdir to use coroutines hw/9pfs: Add yield support for remove hw/9pfs: Update v9fs_remove to use coroutines Venkateswararao Jujjuri (JV) (3): [virtio-9p] Add infrastructure to support glib threads and coroutines. [virtio-9p] Change all pdu handlers to coroutines. hw/9pfs: Add yeild support for readlink Makefile.objs |3 + configure | 18 +- hw/9pfs/codir.c| 85 +++ hw/9pfs/cofile.c | 32 ++ hw/9pfs/cofs.c | 171 ++ hw/9pfs/coxattr.c | 50 ++ hw/9pfs/virtio-9p-coth.c | 102 hw/9pfs/virtio-9p-coth.h | 79 +++ hw/9pfs/virtio-9p-device.c |7 +- hw/9pfs/virtio-9p.c| 1269 +--- hw/9pfs/virtio-9p.h| 76 +--- 11 files changed, 1031 insertions(+), 861 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] [PATCH 18/24] versatile_pci: convert to memory API
On 8 August 2011 18:07, Avi Kivity wrote: > -static uint32_t pci_vpb_config_readb (void *opaque, target_phys_addr_t addr) > +static uint64_t pci_vpb_config_read(void *opaque, target_phys_addr_t addr, > + unsigned size) > { > uint32_t val; > - val = pci_data_read(opaque, vpb_pci_config_addr (addr), 1); > - return val; > + val = pci_data_read(opaque, vpb_pci_config_addr(addr), size); > + return size; > } This should be 'return val', not 'return size', I think... (otherwise PCI is broken on versatilepb; tested with your memory-region branch rather than with this particular patch.) -- PMM
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
On 08/11/2011 07:08 PM, Gerhard Wiesinger wrote: (gdb) frame 4 #4 0x0041eb9b in pci_update_mappings (d=0x1a90bc0) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1134 1134memory_region_del_subregion(r->address_space, r->memory); (gdb) print i $1 = (gdb) print *r $2 = {addr = 22058952032257, size = 32, filtered_size = 171717340864446496, type = 1 '\001', memory = 0x1a9, address_space = 0x200019282f0} (gdb) print d->io_regions[0] $3 = {addr = 22058952032257, size = 32, filtered_size = 171717340864446496, type = 1 '\001', memory = 0x1a9, address_space = 0x200019282f0} Yikes, this looks like corruption, it the leading 0x2000 in address_space is out of place. Can you step through lsi pci bar registration and place a data breakpoint on address_space, and see where it gets this value? 'addr' looks bad too. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Safely reopening image files by stashing fds
On Thu, Aug 11, 2011 at 7:37 AM, Kevin Wolf wrote: > Am 10.08.2011 19:20, schrieb Blue Swirl: >> On Wed, Aug 10, 2011 at 7:58 AM, Kevin Wolf wrote: >>> Am 09.08.2011 21:39, schrieb Blue Swirl: On Tue, Aug 9, 2011 at 12:24 PM, Kevin Wolf wrote: > Am 09.08.2011 14:00, schrieb Stefan Hajnoczi: >> I liked the idea of doing a generic FDStash type that the monitor and >> bdrv_reopen() can use. Blue's idea to hook at the qemu_open() level >> takes that further. > > Well, having something that works for the raw-posix, the monitor and > maybe some more things is nice. Having something that works for > raw-posix, Sheepdog and rbd is an absolute requirement and I can't see > how FDStash solves that. For Sheepdog also network access functions would need to be hooked. RBD seems to use librados functions for low level I/O, so that needs some RBD specific wrappers. > Even raw-win32 doesn't have an int fd, but a > HANDLE hfile for its backend. Just replace "int fd" with "FDStash fd" everywhere? >>> >>> And how is FDStash defined? The current proposed is this one: >>> >>> /* A stashed file descriptor */ >>> typedef FDEntry { >>> const char *name; >>> int fd; >>> QLIST_ENTRY(FDEntry) next; >>> } FDEntry; >>> >>> /* A container for stashing file descriptors */ >>> typedef struct FDStash { >>> QLIST_HEAD(, FDEntry) fds; >>> } FDStash; >>> >>> So there we have the int fd again. If you want something generic, you >> >> What's the problem: >> typedef struct FDEntry { >> const char *name; >> #ifdef _WIN32 >> HANDLE fd; >> #else >> int fd; >> #endif >> QLIST_ENTRY(FDEntry) next; >> } FDEntry; >> >> Renaming 'fd' to something that does not immediately look like 'int' >> may be useful. > > This isn't any more generic, it merely covers two special cases instead > of only one. > > You have used the fact that raw-posix and raw-win32 are never compiled > in at the same time, so that you can use an #ifdef to conditionally pull > out parts of their internal data structures into a generic struct (which > in itself is a layering violation) > > It doesn't help with the other backend drivers like curl, nbd, rbd, > sheepdog and last but not least our special friend vvfat. Probably vvfat would not need any FDEntry. Curl has the same problem as RBD that it's not possible to inject a file descriptor. >>> would have to start letting block drivers extend FDEntry with their own >>> fields (and how would the first bdrv_open work then?) and basically >>> arrive at something like a BDRVReopenState. >> >> I'd not handle FDEntries at block level at all (or only at raw level), >> then there would not be first mover problems. > > Well, but how does it solve the bdrv_reopen problem if it's not visible > on the block level? > >> Extending the uses of FDEntries to for example network stuff >> (Sheepdog) could need some kind of unified API for both file and net >> operations which is not what we have now (bdrv_read/write etc. vs. >> direct recv/send). Then this new API would use FDEntries instead of >> fds, sockets or HANDLEs. The 'name' field could be either network >> address or file path. Though maybe we are only interested in >> open/connect part so unification would not be needed for other >> operations. > > Now you have handled three special cases, but still haven't got a > generic solution. > > But considering that you don't want to touch the block layer API and > therefore I don't even have an idea of what you would use FDEntries > for... Let's go back one step: What problem are you trying to solve, and > what does the API look like that you're thinking of? It doesn't seem to > be the same as Stefan suggested. Excellent point. I think I was thinking of solving SELinux/NFS and privilege separation problems with this one. From this one step back perspective those seem to be slightly different issues. For this problem, BDRVReopenState could be OK. FDStash may be useful otherwise.
Re: [Qemu-devel] [PATCH 0/2] Fix wide ioport access cracking
On 08/11/2011 07:20 PM, Avi Kivity wrote: On 08/11/2011 07:08 PM, Gerhard Wiesinger wrote: (gdb) frame 4 #4 0x0041eb9b in pci_update_mappings (d=0x1a90bc0) at /root/download/qemu/git/qemu-kvm-test/hw/pci.c:1134 1134memory_region_del_subregion(r->address_space, r->memory); (gdb) print i $1 = (gdb) print *r $2 = {addr = 22058952032257, size = 32, filtered_size = 171717340864446496, type = 1 '\001', memory = 0x1a9, address_space = 0x200019282f0} (gdb) print d->io_regions[0] $3 = {addr = 22058952032257, size = 32, filtered_size = 171717340864446496, type = 1 '\001', memory = 0x1a9, address_space = 0x200019282f0} Yikes, this looks like corruption, it the leading 0x2000 in address_space is out of place. Can you step through lsi pci bar registration and place a data breakpoint on address_space, and see where it gets this value? 'addr' looks bad too. Or maybe it's just -O2 screwing up debug information. Please change ./configure to set -O1 and redo. Please print *r.memory as well. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v3] Add support for fd: protocol
On 07/26/2011 08:51 AM, Corey Bryant wrote: > +static int raw_open_fd(BlockDriverState *bs, const char *filename, int flags) > +{ > +BDRVRawState *s = bs->opaque; > +const char *fd_str; > +int fd; > + > +/* extract the file descriptor - fail if it's not fd: */ > +if (!strstart(filename, "fd:",&fd_str)) { > +return -EINVAL; > +} > + > +if (!qemu_isdigit(fd_str[0])) { > +/* get fd from monitor */ > +fd = qemu_get_fd(fd_str); > +if (fd == -1) { > +return -EBADF; > +} > +} else { > +char *endptr = NULL; > + > +fd = strtol(fd_str,&endptr, 10); > +if (*endptr || (fd == 0&& fd_str == endptr)) { > +return -EBADF; > +} > +} > + > +s->fd = fd; > +s->type = FTYPE_FILE; > + > +return raw_open_common(bs, filename, flags, 0); > +} > + > +static BlockDriver bdrv_file_fd = { > +.format_name = "file", > +.protocol_name = "fd", > +.instance_size = sizeof(BDRVRawState), > +.bdrv_probe = NULL, /* no probe for protocols */ > +.bdrv_file_open = raw_open_fd, > +.bdrv_read = raw_read, > +.bdrv_write = raw_write, > +.bdrv_close = raw_close, > +.bdrv_flush = raw_flush, > +.bdrv_discard = raw_discard, > + > +.bdrv_aio_readv = raw_aio_readv, > +.bdrv_aio_writev = raw_aio_writev, > +.bdrv_aio_flush = raw_aio_flush, > + > +.bdrv_truncate = raw_truncate, > +.bdrv_getlength = raw_getlength, > + > +.create_options = raw_create_options, > +}; > + I'm a bit unsure of how to support CD-ROM with the fd: protocol. I don't think use of bdrv_file_fd is an option. For example, while raw_open_fd may work, there is no eject support. Another approach is to have 2 BlockDriver structs that support the fd protocol. For example, creating a new BlockDriver struct that mirrors bdrv_host_cdrom. So you would have bdrv_host_cdrom_fd with a corresponding cdrom_open_fd function. But that doesn't appear possible as it appears that the protocol must be unique among all BlockDriver structs. Do we need to introduce a similar protocol (maybe "cdfd") to support passing of CD-ROM file descriptors?
Re: [Qemu-devel] [PATCH] TCG: Add preprocessor guards for optional tcg ops
On Thu, Aug 11, 2011 at 12:58 PM, Avi Kivity wrote: > On 08/11/2011 03:36 PM, Alexander Graf wrote: >> >> > >> > Or to have automatic generation of the optionals based on the >> > primitives, if the optionals are not present. >> >> That's what's happening in the background already, no? The line is towards >> tcg users though, not it tcg internal code. >> > > Yes, and it doesn't make sense to optimize through synthetic instructions. I'd just create all INDEX_op_* enums and adjust TCG targets etc. to call tcg_abort() in the default case.
[Qemu-devel] [Bug 824650] [NEW] Latest GIT assert error in arp_table.c
Public bug reported: The latest git version of qemu (commit 8cc7c3952d4d0a681d8d4c3ac89a206a5bfd7f00) crashes after a few minutes. All was fine up to a few days ago. This is wth both x86 and sparc emulation, on an x86_64 host. e.g. qemu-system-sparc -drive file=netbsd5.0.2-sparc,index=0,media=disk,cache=unsafe -m 256 -boot c -nographic -redir tcp:2232::22: qemu-system-sparc: slirp/arp_table.c:75: arp_table_search: Assertion `(ip_addr & (__extension__ ({ register unsigned int __v, __x = (~(0xf << 28)); if (__builtin_constant_p (__x)) __v = __x) & 0xff00) >> 24) | (((__x) & 0x00ff) >> 8) | (((__x) & 0xff00) << 8) | (((__x) & 0x00ff) << 24)); else __asm__ ("bswap %0" : "=r" (__v) : "0" (__x)); __v; }))) != 0' failed. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/824650 Title: Latest GIT assert error in arp_table.c Status in QEMU: New Bug description: The latest git version of qemu (commit 8cc7c3952d4d0a681d8d4c3ac89a206a5bfd7f00) crashes after a few minutes. All was fine up to a few days ago. This is wth both x86 and sparc emulation, on an x86_64 host. e.g. qemu-system-sparc -drive file=netbsd5.0.2-sparc,index=0,media=disk,cache=unsafe -m 256 -boot c -nographic -redir tcp:2232::22: qemu-system-sparc: slirp/arp_table.c:75: arp_table_search: Assertion `(ip_addr & (__extension__ ({ register unsigned int __v, __x = (~(0xf << 28)); if (__builtin_constant_p (__x)) __v = __x) & 0xff00) >> 24) | (((__x) & 0x00ff) >> 8) | (((__x) & 0xff00) << 8) | (((__x) & 0x00ff) << 24)); else __asm__ ("bswap %0" : "=r" (__v) : "0" (__x)); __v; }))) != 0' failed. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/824650/+subscriptions
Re: [Qemu-devel] [RFC] postcopy livemigration proposal
Hello everyone, so basically this is a tradeoff between not having a long latency for the migration to succeed and reducing the total network traffic (and CPU load) in the migration source and destination and reducing the memory footprint a bit, by adding an initial latency to the memory accesses on the destination of the migration (i.e. causing a more significant and noticeable slowdown to the guest). It's more or less like if when the guest starts on the destination node, it will find all its memory swapped out to a network swap device, so it needs to do I/O for the first access (side note: and hopefully it won't run out of memory while the memory is copied to the destination node or the guest will crash). On Thu, Aug 11, 2011 at 11:19:19AM +0900, Isaku Yamahata wrote: > On Wed, Aug 10, 2011 at 04:55:32PM +0300, Avi Kivity wrote: > > I'm not 100% sure, but I think that thp and ksm need the vma to be > > anonymous, not just the page. > > Yes, they seems to check if not only the page is anonymous, but also the vma. > I'd like to hear from Andrea before digging into the code deeply. The vma doesn't need to be anonymous for THP, an mmap on /dev/zero MAP_PRIVATE also is backed by THP. But it must be close to anonymous and not have special VM_IO/PFNMAP flags or khugepaged/ksm will not scan it. ->vm_file itself isn't checked by THP/KSM (sure for THP because of the /dev/zero example which I explicitly fixed as it wasn't fully handled initially). NOTE a chardevice won't work on RHEL6 because I didn't allow /dev/zero to use it there (it wasn't an important enough feature and it was more risky) but upstream it should work already. A chardevice doing this may work, even if it would be simpler/cleaner if this was still an anonymous vma. A chardevice could act similar to /dev/zero MAP_PRIVATE. In theory KSM should work on /dev/zero too, you can test that if you want. But a chardevice will require dealing with permissions when we don't actually need special permissions for this. Another problem is you can't migrate the stuff using hugepages or it'd multiply the latency 512 times (with 2M contiguous access it won't make a difference but if the guest is accessing memory randomly it would make a difference). So you will have to relay on khugepaged to collapse the hugepages later. That should work but initially the guest will run slower even when the migration is already fully completed. > If it is possible to convert the vma into anonymous, swap device or > backed by device/file wouldn't matter in respect to ksm and thp. > Acquiring mmap_sem suffices? A swap device would require root permissions and we don't want qemu to mangle over the swapdevices automatically. It'd be bad to add new admin requirements, few people would use it. Ideally the migration API should remain the same and it should be an internal tweak in qemu to select which migration mode to use beforehand. Even if it was a swap device it'd still require special operations to setup swap entries in the process pagetables before the pages exists. A swap device may give more complication than it solves. If it was only KVM accessing the guest physical memory we could just handle it in KVM, and call get_user_pages_fast, if that fails and it's the first ever invocation we just talk with QEMU to get the page and establish it by hand. But qemu can also write to memory and if it's a partial write and the guest reads the not-written yet part with get_user_pages_fast+spte establishment, it'll go wrong. Maybe qemu is already doing all checks on the pages it's going to write and we could hook there too from qemu side. Another more generic (not KVM centric) that will not require a special chardev or a special daemon way could be a new: sys_set_muserload(unsigned long start, unsigned long len, int signal) sys_muserload(void *from, void *to) When sys_set_muserload is called the region start,start+len gets covered by muserload swap entries that trigger special page faults. When anything touches the memory with an muserload swap entry still set, the thread gets a signal with force_sig_info_fault(si_signo = signal), and the signal handler will get the faulting address in info.si_addr. The signal handler is then responsible to call sys_userload after talking the thread doing the TCP send/recv() talking with the qemu source. The recv(mmap(4096), 4096) should generate a page in the destination node in some random mapping (aligned). Then the muserload(tcp_received_page_address, guest_faulting_physical_address_from_info_si_addr) does get_user_pages on tcp_received_page_address, takes it away from the tcp_received_page_address (clears the pte at that address), adjusts page->index for the new vma, and maps the page zercopy atomically into the "guest_faulting_physical_address_from_info_si_addr" new address, if and only if the pagetable at address is still of muserload type. Then the signal munmap(tcp_received_page_address, 4096) to truncate/free the vma
Re: [Qemu-devel] RFCv2: virDomainSnapshotCreateXML enhancements
On 08/11/2011 08:11 AM, Kevin Wolf wrote: I agree with you. It feels a bit backwards for snapshots, but it's really the only reasonable thing to do if you're using external snapshots. That you can't rename block devices is actually a very point point, too. There's one more point to consider: If creating a snapshot of foo.img just creates a new bar.img, but I keep working on foo.img, I might expect that by deleting bar.img I remove the snapshot, but foo.img keeps working. More ideas on this front: One of the ideas of 'live snapshot' is to grab state that I can copy to an independent backup, taking as much time as needed, with minimal interruption to qemu. Given an original 'file' of any format, then we can consider the sequence: rename file to file.tmp (assuming we figure out how to teach qemu about renames) use snapshot_blkdev to recreate file with file.tmp as backup in parallel: copy file.tmp to file.snapshot block pull the contents of file.tmp back into file when both tasks have completed, remove file.tmp Now, I have created a snapshot file.snap, which can safely be deleted without breaking 'file', and with minimal downtime to the qemu process. It's just that there is a window of time where the the snapshot is still in progress (that is, until both file.snap and the block pull have completed); dealing with the wrinkle that this forces 'file' to now be qcow2, even if it started out raw; and dealing with rename() issues not being usable on block devices. And a non-zero window of time between starting the sequence and reaching a stable completion implies ramifications to whether other commands would be locked out in the meantime, or whether it can be broken into multiple steps with progress checks along the way, whether events need to be exposed to track when pieces complete, and so on. Another idea is that if qemu would ever gain a way to export the contents of an internal snapshot or backing file (aka external snapshot), independently of how that state differs from the current state, then another operation would be: with qcow2 file, create an internal snapshot use new API to copy out the snapshot state into file.snap, while qemu is still actively modifying current state remove the internal snapshot with the net result that appears the same as creating file.snap as an external snapshot of a given state in time, but where the original qcow2 file is not impacted if file.snap is deleted. So working with renames might turn out to be tricky in many ways, and not only technical ones. Hopefully we're leaving enough flexibility to support these additional snapshot modes, even if we don't implement everything in the first round. 2. It is possible to add a new libvirt API, virDomainSnapshotCreateFrom, which takes an existing snapshot as a child of the given snapshot passed in as its parent. This would combine the action of reverting to a disk-snapshot along with the xml argument necessary for naming a new live file, so that you could indeed support branching off the disk-snapshot with a user-specified or libvirt-generated new active file name without having to delete the existing children that were branched off the old active file name, and making the original base file the backing file to both branches. Unfortunately, adding a new API is out of the question for backporting purposes. This API would be completely pointless with internal snapshots, right? On the contrary, it might be useful as a way to convert an internal snapshot into an external one. But yes, we can already do branching children off internal snapshots without needing this new feature, so the new feature's main point is for use in creating a branching child off an external disk snapshot. The ideal result would be an API where the user doesn't really have to deal with internal vs. external snapshots other than setting the right flag/XML option/whatever and libvirt would do the mapping to the low-level functions. Of course, if we want to avoid renames (for which there are good reasons), then maybe we can't really get a unified API for internal and external snapshots. In this case, maybe using completely different functions to signal that we have different semantics might be appropriate. This looks like it still needs a lot of thought. Different functions at the qemu level, at the libvirt level, or both? I agree that the ideal libvirt semantics is a single interface with enough expressivity to properly map to all the underlying qemu options, where libvirt correctly decides between migrate to disk and qemu-img, savevm, snapshot_blkdev, block pull, or any other underlying operations, while still properly rejecting any combinations that are possible in the XML matrix but unsupported by current qemu capabilities. 2a. But thinking about it a bit more, maybe we don't need a new API, but just an XML enhancement to the existing virDomainSnapshotCreateXML! That is, if I specify
Re: [Qemu-devel] [RFC] Planning for 1.0 (and freezing the master branch)
On Thu, Aug 11, 2011 at 1:46 PM, Anthony Liguori wrote: > Hi, > > I've posted an initial proposal for the 1.0 release on the wiki[1]. > > The release would be targeted for December 1st. Unlike previous releases, > I'm proposing that instead of forking master into a stable branch and > releasing from there, we stop taking features into master and all work on > stablizing master. > > Historically, we forked stable for the releases simply because it was the > least intrusive way to get us on a somewhat reasonable release schedule. I > think especially since we're targeting a 1.0, we're ready to get a bit more > serious about releases. Even more historically, with CVS, not forking was the only way. > I see the following advantages in using master for releases: > > 1) All maintainers are participating in the process which means releases are > much less likely to be delayed due to lack of time from a single person. > > 2) Everyone (contributors) are forced to focus on stability which should > improve the release's quality. > > 3) Feature development can still happen in maintainers trees. I think this > is an important step to moving to a merge-window based development model > which I think will be our best way to scale long term. > > Obviously, this will only work well if all the maintainers participate so > I'd really like to hear what everyone thinks about this. > > [1] http://wiki.qemu.org/Planning/1.0 In general the idea is OK. Especially soft freeze could solve problems like those qemu-ga inclusion had. Two weeks for soft freeze would be close to OK but I think a month of hard freeze is too long. With the previous releases, the problems in stable were ironed out within a week or two. How about 2 + 2? I'm not convinced about merge window model at least how it's used with Linux kernel development. There will be a lot of breakage during the merge window. Major changes at the same time would need major rebasing efforts since they would be developed and merged within same time frame. Also pretty strong coordination would be needed. We don't have the luxury of infinite army of monkeys lead by genius who has dedicated his entire life to the project like kernel has. I'd rather try to keep the code base at (sort of) release quality at all times and merge changes every now and then.
Re: [Qemu-devel] [PATCH 1/2] pflash: Support read-only mode
On Thu, Jul 28, 2011 at 14:05, Jordan Justen wrote: > On Thu, Jul 28, 2011 at 11:26, Jan Kiszka wrote: >> On 2011-07-27 17:38, Jordan Justen wrote: >>> On Wed, Jul 27, 2011 at 02:30, Jan Kiszka wrote: On 2011-07-25 23:34, Jordan Justen wrote: > Read-only mode is indicated by bdrv_is_read_only > > When read-only mode is enabled, no changes will be made > to the flash image in memory, and no bdrv_write calls will be > made. > > Signed-off-by: Jordan Justen > Cc: Jan Kiszka > Cc: Aurelien Jarno > Cc: Anthony Liguori > --- > blockdev.c | 3 +- > hw/pflash_cfi01.c | 36 ++- > hw/pflash_cfi02.c | 82 > > 3 files changed, 68 insertions(+), 53 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 0b8d3a4..566a502 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -521,7 +521,8 @@ DriveInfo *drive_init(QemuOpts *opts, int > default_to_scsi) > /* CDROM is fine for any interface, don't check. */ > ro = 1; > } else if (ro == 1) { > - if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && > type != IF_NONE) { > + if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && > + type != IF_NONE && type != IF_PFLASH) { > error_report("readonly not supported by this bus type"); > goto err; > } > diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c > index 90fdc84..11ac490 100644 > --- a/hw/pflash_cfi01.c > +++ b/hw/pflash_cfi01.c > @@ -284,8 +284,10 @@ static void pflash_write(pflash_t *pfl, > target_phys_addr_t offset, > TARGET_FMT_plx "\n", > __func__, offset, pfl->sector_len); > > - memset(p + offset, 0xff, pfl->sector_len); > - pflash_update(pfl, offset, pfl->sector_len); > + if (!pfl->ro) { > + memset(p + offset, 0xff, pfl->sector_len); > + pflash_update(pfl, offset, pfl->sector_len); > + } > pfl->status |= 0x80; /* Ready! */ > break; > case 0x50: /* Clear status bits */ > @@ -324,8 +326,10 @@ static void pflash_write(pflash_t *pfl, > target_phys_addr_t offset, > case 0x10: /* Single Byte Program */ > case 0x40: /* Single Byte Program */ > DPRINTF("%s: Single Byte Program\n", __func__); > - pflash_data_write(pfl, offset, value, width, be); > - pflash_update(pfl, offset, width); > + if (!pfl->ro) { > + pflash_data_write(pfl, offset, value, width, be); > + pflash_update(pfl, offset, width); > + } > pfl->status |= 0x80; /* Ready! */ > pfl->wcycle = 0; > break; > @@ -373,7 +377,9 @@ static void pflash_write(pflash_t *pfl, > target_phys_addr_t offset, > case 2: > switch (pfl->cmd) { > case 0xe8: /* Block write */ > - pflash_data_write(pfl, offset, value, width, be); > + if (!pfl->ro) { > + pflash_data_write(pfl, offset, value, width, be); > + } > > pfl->status |= 0x80; > > @@ -383,8 +389,10 @@ static void pflash_write(pflash_t *pfl, > target_phys_addr_t offset, > > DPRINTF("%s: block write finished\n", __func__); > pfl->wcycle++; > - /* Flush the entire write buffer onto backing storage. > */ > - pflash_update(pfl, offset & mask, pfl->writeblock_size); > + if (!pfl->ro) { > + /* Flush the entire write buffer onto backing > storage. */ > + pflash_update(pfl, offset & mask, > pfl->writeblock_size); > + } > } > > pfl->counter--; > @@ -621,13 +629,13 @@ pflash_t *pflash_cfi01_register(target_phys_addr_t > base, ram_addr_t off, > return NULL; > } > } > -#if 0 /* XXX: there should be a bit to set up read-only, > - * the same way the hardware does (with WP pin). > - */ > - pfl->ro = 1; > -#else > - pfl->ro = 0; > -#endif > + > + if (pfl->bs) { > + pfl->ro = bdrv_is_read_only(pfl->bs); > + } else { > + pfl->ro = 0; > + } > + > pfl->timer = qemu_new_timer_ns(vm_clock, pflash_timer, pfl); > pfl->base = base; > pfl->sector_len = sector_len; > diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c > index 725cd1e..920209d 100644 > --- a/hw/pflash_cfi0
Re: [Qemu-devel] [PATCH 18/24] versatile_pci: convert to memory API
On 08/11/2011 07:20 PM, Peter Maydell wrote: On 8 August 2011 18:07, Avi Kivity wrote: > -static uint32_t pci_vpb_config_readb (void *opaque, target_phys_addr_t addr) > +static uint64_t pci_vpb_config_read(void *opaque, target_phys_addr_t addr, > +unsigned size) >{ > uint32_t val; > -val = pci_data_read(opaque, vpb_pci_config_addr (addr), 1); > -return val; > +val = pci_data_read(opaque, vpb_pci_config_addr(addr), size); > +return size; >} This should be 'return val', not 'return size', I think... Wow, how did that happen? (otherwise PCI is broken on versatilepb; tested with your memory-region branch rather than with this particular patch.) Updated patch to follow, thanks for the report. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v1.1 18/24] versatile_pci: convert to memory API
Signed-off-by: Avi Kivity --- v1.1: fix bogus 'return size', thanks to Peter Maydell hw/versatile_pci.c | 92 --- 1 files changed, 43 insertions(+), 49 deletions(-) diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c index e1d5c0b..98e56f1 100644 --- a/hw/versatile_pci.c +++ b/hw/versatile_pci.c @@ -16,7 +16,9 @@ typedef struct { SysBusDevice busdev; qemu_irq irq[4]; int realview; -int mem_config; +MemoryRegion mem_config; +MemoryRegion mem_config2; +MemoryRegion isa; } PCIVPBState; static inline uint32_t vpb_pci_config_addr(target_phys_addr_t addr) @@ -24,55 +26,24 @@ static inline uint32_t vpb_pci_config_addr(target_phys_addr_t addr) return addr & 0xff; } -static void pci_vpb_config_writeb (void *opaque, target_phys_addr_t addr, - uint32_t val) +static void pci_vpb_config_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned size) { -pci_data_write(opaque, vpb_pci_config_addr (addr), val, 1); +pci_data_write(opaque, vpb_pci_config_addr(addr), val, size); } -static void pci_vpb_config_writew (void *opaque, target_phys_addr_t addr, - uint32_t val) -{ -pci_data_write(opaque, vpb_pci_config_addr (addr), val, 2); -} - -static void pci_vpb_config_writel (void *opaque, target_phys_addr_t addr, - uint32_t val) -{ -pci_data_write(opaque, vpb_pci_config_addr (addr), val, 4); -} - -static uint32_t pci_vpb_config_readb (void *opaque, target_phys_addr_t addr) -{ -uint32_t val; -val = pci_data_read(opaque, vpb_pci_config_addr (addr), 1); -return val; -} - -static uint32_t pci_vpb_config_readw (void *opaque, target_phys_addr_t addr) -{ -uint32_t val; -val = pci_data_read(opaque, vpb_pci_config_addr (addr), 2); -return val; -} - -static uint32_t pci_vpb_config_readl (void *opaque, target_phys_addr_t addr) +static uint64_t pci_vpb_config_read(void *opaque, target_phys_addr_t addr, +unsigned size) { uint32_t val; -val = pci_data_read(opaque, vpb_pci_config_addr (addr), 4); +val = pci_data_read(opaque, vpb_pci_config_addr(addr), size); return val; } -static CPUWriteMemoryFunc * const pci_vpb_config_write[] = { -&pci_vpb_config_writeb, -&pci_vpb_config_writew, -&pci_vpb_config_writel, -}; - -static CPUReadMemoryFunc * const pci_vpb_config_read[] = { -&pci_vpb_config_readb, -&pci_vpb_config_readw, -&pci_vpb_config_readl, +static const MemoryRegionOps pci_vpb_config_ops = { +.read = pci_vpb_config_read, +.write = pci_vpb_config_write, +.endianness = DEVICE_NATIVE_ENDIAN, }; static int pci_vpb_map_irq(PCIDevice *d, int irq_num) @@ -87,17 +58,35 @@ static void pci_vpb_set_irq(void *opaque, int irq_num, int level) qemu_set_irq(pic[irq_num], level); } + static void pci_vpb_map(SysBusDevice *dev, target_phys_addr_t base) { PCIVPBState *s = (PCIVPBState *)dev; /* Selfconfig area. */ -cpu_register_physical_memory(base + 0x0100, 0x100, s->mem_config); +memory_region_add_subregion(get_system_memory(), base + 0x0100, +&s->mem_config); +/* Normal config area. */ +memory_region_add_subregion(get_system_memory(), base + 0x0200, +&s->mem_config2); + +if (s->realview) { +/* IO memory area. */ +memory_region_add_subregion(get_system_memory(), base + 0x0300, +&s->isa); +} +} + +static void pci_vpb_unmap(SysBusDevice *dev, target_phys_addr_t base) +{ +PCIVPBState *s = (PCIVPBState *)dev; +/* Selfconfig area. */ +memory_region_del_subregion(get_system_memory(), &s->mem_config); /* Normal config area. */ -cpu_register_physical_memory(base + 0x0200, 0x100, s->mem_config); +memory_region_del_subregion(get_system_memory(), &s->mem_config2); if (s->realview) { /* IO memory area. */ -isa_mmio_init(base + 0x0300, 0x0010); +memory_region_del_subregion(get_system_memory(), &s->isa); } } @@ -117,10 +106,15 @@ static int pci_vpb_init(SysBusDevice *dev) /* ??? Register memory space. */ -s->mem_config = cpu_register_io_memory(pci_vpb_config_read, - pci_vpb_config_write, bus, - DEVICE_LITTLE_ENDIAN); -sysbus_init_mmio_cb(dev, 0x0400, pci_vpb_map); +memory_region_init_io(&s->mem_config, &pci_vpb_config_ops, bus, + "pci-vpb-selfconfig", 0x100); +memory_region_init_io(&s->mem_config2, &pci_vpb_config_ops, bus, + "pci-vpb-config", 0x100); +if (s->realview) { +isa_mmio_setup(&s->isa, 0x010); +} + +sysbus_init_mmio_cb2(d
Re: [Qemu-devel] [PATCH] TCG: Add preprocessor guards for optional tcg ops
On 08/11/2011 09:38 AM, Blue Swirl wrote: > On Thu, Aug 11, 2011 at 12:58 PM, Avi Kivity wrote: >> On 08/11/2011 03:36 PM, Alexander Graf wrote: >>> Or to have automatic generation of the optionals based on the primitives, if the optionals are not present. >>> >>> That's what's happening in the background already, no? The line is towards >>> tcg users though, not it tcg internal code. >>> >> >> Yes, and it doesn't make sense to optimize through synthetic instructions. > > I'd just create all INDEX_op_* enums and adjust TCG targets etc. to > call tcg_abort() in the default case. > Seconded. We can still expand the optional enums exactly as we do now, but make sure that the enum is always present. That'll clean up a *lot* of ifdefs throughout tcg/*. r~
Re: [Qemu-devel] [RFC] Planning for 1.0 (and freezing the master branch)
On 08/11/2011 12:30 PM, Blue Swirl wrote: On Thu, Aug 11, 2011 at 1:46 PM, Anthony Liguori wrote: Hi, I've posted an initial proposal for the 1.0 release on the wiki[1]. The release would be targeted for December 1st. Unlike previous releases, I'm proposing that instead of forking master into a stable branch and releasing from there, we stop taking features into master and all work on stablizing master. Historically, we forked stable for the releases simply because it was the least intrusive way to get us on a somewhat reasonable release schedule. I think especially since we're targeting a 1.0, we're ready to get a bit more serious about releases. Even more historically, with CVS, not forking was the only way. Indeed :-) 3) Feature development can still happen in maintainers trees. I think this is an important step to moving to a merge-window based development model which I think will be our best way to scale long term. Obviously, this will only work well if all the maintainers participate so I'd really like to hear what everyone thinks about this. [1] http://wiki.qemu.org/Planning/1.0 In general the idea is OK. Especially soft freeze could solve problems like those qemu-ga inclusion had. Two weeks for soft freeze would be close to OK but I think a month of hard freeze is too long. With the previous releases, the problems in stable were ironed out within a week or two. How about 2 + 2? I feel like 2 weeks was too short for this release and releases in the past. Conversely, I feel like more than 2 weeks on a different branch is too long because people move on to shinier things. That's why I'm proposing 4 weeks of hard freeze w/o opening up a new development branch. That gives us more time to iron out any issues and makes sure people still keep focusing on stability. I want to move to 4 weeks primarily because I want to increase the amount of release testing we do. We haven't had a really active set of stable releases in a long time so for the most part, the .0 or .1 release is what people are going to be using. I think that means that we should put a bit more effort into making .0 be as strong as it can be. I'm not convinced about merge window model at least how it's used with Linux kernel development. There will be a lot of breakage during the merge window. Major changes at the same time would need major rebasing efforts since they would be developed and merged within same time frame. Also pretty strong coordination would be needed. We don't have the luxury of infinite army of monkeys lead by genius who has dedicated his entire life to the project like kernel has. I'd rather try to keep the code base at (sort of) release quality at all times and merge changes every now and then. That's why I started with a 4 week freeze proposal :-) It gives us a chance to try it out. If it works well, maybe we try 6-8 weeks for 1.1. If it doesn't, we can drop back down to 2. As long as we're not making massive changes, I think its a good idea to experiment a bit. Regards, Anthony Liguori
[Qemu-devel] [Bug 824716] [NEW] linux-user broken for targets with TARGET_ABI32 (i.e. qemu-sparc32plus)
Public bug reported: I just debugged a problem I had with linux-user for qemu-sparc32plus. Turns out that sparc32plus is defined as a 64bit target with TARGET_ABI32 set. This correctly leads to abi_ulong (and others) being defined as uint32_t. However most of the code (in syscall.c) uses tswapl for these values, which swaps the endianess of a target long (which is 64bit). This doesn't match the uin32_t abi_ulongs and fails! So it appears to me like one would need to define something like an aswapl which swaps abi_ulongs and replace most of the tswapls there... ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/824716 Title: linux-user broken for targets with TARGET_ABI32 (i.e. qemu- sparc32plus) Status in QEMU: New Bug description: I just debugged a problem I had with linux-user for qemu-sparc32plus. Turns out that sparc32plus is defined as a 64bit target with TARGET_ABI32 set. This correctly leads to abi_ulong (and others) being defined as uint32_t. However most of the code (in syscall.c) uses tswapl for these values, which swaps the endianess of a target long (which is 64bit). This doesn't match the uin32_t abi_ulongs and fails! So it appears to me like one would need to define something like an aswapl which swaps abi_ulongs and replace most of the tswapls there... To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/824716/+subscriptions