[Qemu-devel] [PULL] usb patch queue

2011-08-11 Thread Gerd Hoffmann
  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

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Gerd Hoffmann
  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

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Harsh Bora

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

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Kevin Wolf
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

2011-08-11 Thread Avi Kivity
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

2011-08-11 Thread Avi Kivity
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

2011-08-11 Thread Avi Kivity
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)

2011-08-11 Thread Vlad Glagolev
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

2011-08-11 Thread Avi Kivity

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()

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Michael S. Tsirkin
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

2011-08-11 Thread Michael S. Tsirkin
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

2011-08-11 Thread Stefan Hajnoczi
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

2011-08-11 Thread Stefan Hajnoczi
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

2011-08-11 Thread Gerd Hoffmann
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

2011-08-11 Thread Shribman, Aidan
> 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

2011-08-11 Thread Edgar E. Iglesias
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Gerhard Wiesinger

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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Gerhard Wiesinger

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

2011-08-11 Thread Daniel P. Berrange
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Daniel P. Berrange
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Edgar E. Iglesias
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

2011-08-11 Thread Edgar E. Iglesias
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

2011-08-11 Thread Alexander Graf
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread malc
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

2011-08-11 Thread Peter Maydell
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Alexander Graf

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

2011-08-11 Thread Edgar E. Iglesias
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

2011-08-11 Thread Edgar E. Iglesias
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

2011-08-11 Thread Edgar E. Iglesias
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

2011-08-11 Thread Edgar E. Iglesias
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

2011-08-11 Thread Edgar E. Iglesias
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Paolo Bonzini

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

2011-08-11 Thread Paolo Bonzini

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

2011-08-11 Thread Hans de Goede
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

2011-08-11 Thread Hans de Goede
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

2011-08-11 Thread Hans de Goede
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

2011-08-11 Thread Hans de Goede
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

2011-08-11 Thread Hans de Goede
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Alexander Graf

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

2011-08-11 Thread Peter Maydell
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Anthony Liguori

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.

2011-08-11 Thread Bug Watch Updater
** 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

2011-08-11 Thread Anthony Liguori

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

2011-08-11 Thread Paolo Bonzini

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

2011-08-11 Thread Stefan Hajnoczi
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

2011-08-11 Thread Peter Maydell
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)

2011-08-11 Thread Anthony Liguori

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

2011-08-11 Thread Kevin Wolf
[ 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

2011-08-11 Thread Paolo Bonzini

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

2011-08-11 Thread David Gilbert
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

2011-08-11 Thread 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).


Paolo



Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return from longjmp

2011-08-11 Thread Peter Maydell
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

2011-08-11 Thread Paolo Bonzini

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

2011-08-11 Thread Kevin Wolf
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

2011-08-11 Thread Richard Henderson
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

2011-08-11 Thread 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.


Paolo



Re: [Qemu-devel] [PATCH 05/10] dma-helpers: add dma_buf_read and dma_buf_write

2011-08-11 Thread Kevin Wolf
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Paolo Bonzini

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

2011-08-11 Thread Aneesh Kumar K.V
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

2011-08-11 Thread Bob Breuer
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

2011-08-11 Thread Gerhard Wiesinger

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

2011-08-11 Thread Gerhard Wiesinger

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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Aneesh Kumar K.V
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

2011-08-11 Thread Peter Maydell
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Blue Swirl
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Corey Bryant



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

2011-08-11 Thread Blue Swirl
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

2011-08-11 Thread Nigel Horne
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

2011-08-11 Thread Andrea Arcangeli
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

2011-08-11 Thread Eric Blake

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)

2011-08-11 Thread Blue Swirl
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

2011-08-11 Thread Jordan Justen
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

2011-08-11 Thread Avi Kivity

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

2011-08-11 Thread Avi Kivity
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

2011-08-11 Thread Richard Henderson
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)

2011-08-11 Thread Anthony Liguori

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)

2011-08-11 Thread Matthias Braun
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



  1   2   >