[Qemu-devel] Re: [PULL (resend, rebase) 2/5] virtio-serial: Disallow generic ports at id 0

2011-03-10 Thread Amit Shah
On (Thu) 10 Mar 2011 [11:39:16], Amit Shah wrote:
 Port 0 is reserved for virtconsole devices for backward compatibility
 with the old -virtioconsole (from qemu 0.12) device type.
 
 libvirt prior to commit 8e28c5d40200b4c5d483bd585d237b9d870372e5 used
 port 0 for generic ports.  libvirt will no longer do that, but disallow
 instantiating generic ports at id 0 from qemu as well.
 
 Signed-off-by: Amit Shah amit.s...@redhat.com

Updated patch below, fixes a build break after rebase.  The git tree
in the pull request has been updated with this fix.

From 78f1d849a8d739fa7377b8a790a60ffc293aa786 Mon Sep 17 00:00:00 2001
Message-Id: 
78f1d849a8d739fa7377b8a790a60ffc293aa786.1299745288.git.amit.s...@redhat.com
In-Reply-To: cover.1299745288.git.amit.s...@redhat.com
References: cover.1299745288.git.amit.s...@redhat.com
From: Amit Shah amit.s...@redhat.com
Date: Thu, 3 Feb 2011 13:05:07 +0530
Subject: [PULL (resend, rebase) 2/5] virtio-serial: Disallow generic ports at 
id 0

Port 0 is reserved for virtconsole devices for backward compatibility
with the old -virtioconsole (from qemu 0.12) device type.

libvirt prior to commit 8e28c5d40200b4c5d483bd585d237b9d870372e5 used
port 0 for generic ports.  libvirt will no longer do that, but disallow
instantiating generic ports at id 0 from qemu as well.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio-console.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index c235b27..4440784 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -11,6 +11,7 @@
  */
 
 #include qemu-char.h
+#include qemu-error.h
 #include virtio-serial.h
 
 typedef struct VirtConsole {
@@ -113,6 +114,14 @@ static int virtserialport_initfn(VirtIOSerialPort *port)
 {
 VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
 
+if (port-id == 0) {
+/*
+ * Disallow a generic port at id 0, that's reserved for
+ * console ports.
+ */
+error_report(Port number 0 on virtio-serial devices reserved for 
virtconsole devices for backward compatibility.);
+return -1;
+}
 return generic_port_init(vcon, port);
 }
 
-- 
1.7.4




Amit



[Qemu-devel] [PATCH 07/32] vmstate: port arm_timer

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/arm_timer.c |   37 ++---
 1 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/hw/arm_timer.c b/hw/arm_timer.c
index cfd1ebe..dac9e70 100644
--- a/hw/arm_timer.c
+++ b/hw/arm_timer.c
@@ -140,28 +140,19 @@ static void arm_timer_tick(void *opaque)
 arm_timer_update(s);
 }

-static void arm_timer_save(QEMUFile *f, void *opaque)
-{
-arm_timer_state *s = (arm_timer_state *)opaque;
-qemu_put_be32(f, s-control);
-qemu_put_be32(f, s-limit);
-qemu_put_be32(f, s-int_level);
-qemu_put_ptimer(f, s-timer);
-}
-
-static int arm_timer_load(QEMUFile *f, void *opaque, int version_id)
-{
-arm_timer_state *s = (arm_timer_state *)opaque;
-
-if (version_id != 1)
-return -EINVAL;
-
-s-control = qemu_get_be32(f);
-s-limit = qemu_get_be32(f);
-s-int_level = qemu_get_be32(f);
-qemu_get_ptimer(f, s-timer);
-return 0;
-}
+static const VMStateDescription vmstate_arm_timer = {
+.name = arm_timer,
+.version_id = 1,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT32(control, arm_timer_state),
+VMSTATE_UINT32(limit, arm_timer_state),
+VMSTATE_INT32(int_level, arm_timer_state),
+VMSTATE_PTIMER(timer, arm_timer_state),
+VMSTATE_END_OF_LIST()
+}
+};

 static arm_timer_state *arm_timer_init(uint32_t freq)
 {
@@ -174,7 +165,7 @@ static arm_timer_state *arm_timer_init(uint32_t freq)

 bh = qemu_bh_new(arm_timer_tick, s);
 s-timer = ptimer_init(bh);
-register_savevm(NULL, arm_timer, -1, 1, arm_timer_save, arm_timer_load, 
s);
+vmstate_register(NULL, -1, vmstate_arm_timer, s);
 return s;
 }

-- 
1.7.4




[Qemu-devel] [PATCH 07/13] vmstate: port nand

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/nand.c |   73 
 1 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/hw/nand.c b/hw/nand.c
index 9f978d8..37e51d7 100644
--- a/hw/nand.c
+++ b/hw/nand.c
@@ -66,6 +66,8 @@ struct NANDFlashState {
 void (*blk_write)(NANDFlashState *s);
 void (*blk_erase)(NANDFlashState *s);
 void (*blk_load)(NANDFlashState *s, uint32_t addr, int offset);
+
+uint32_t ioaddr_vmstate;
 };

 # define NAND_NO_AUTOINCR  0x0001
@@ -281,48 +283,51 @@ static void nand_command(NANDFlashState *s)
 }
 }

-static void nand_save(QEMUFile *f, void *opaque)
+static void nand_pre_save(void *opaque)
 {
-NANDFlashState *s = (NANDFlashState *) opaque;
-qemu_put_byte(f, s-cle);
-qemu_put_byte(f, s-ale);
-qemu_put_byte(f, s-ce);
-qemu_put_byte(f, s-wp);
-qemu_put_byte(f, s-gnd);
-qemu_put_buffer(f, s-io, sizeof(s-io));
-qemu_put_be32(f, s-ioaddr - s-io);
-qemu_put_be32(f, s-iolen);
-
-qemu_put_be32s(f, s-cmd);
-qemu_put_be32s(f, s-addr);
-qemu_put_be32(f, s-addrlen);
-qemu_put_be32(f, s-status);
-qemu_put_be32(f, s-offset);
-/* XXX: do we want to save s-storage too? */
+NANDFlashState *s = opaque;
+
+s-ioaddr_vmstate = s-ioaddr - s-io;
 }

-static int nand_load(QEMUFile *f, void *opaque, int version_id)
+static int nand_post_load(void *opaque, int version_id)
 {
-NANDFlashState *s = (NANDFlashState *) opaque;
-s-cle = qemu_get_byte(f);
-s-ale = qemu_get_byte(f);
-s-ce = qemu_get_byte(f);
-s-wp = qemu_get_byte(f);
-s-gnd = qemu_get_byte(f);
-qemu_get_buffer(f, s-io, sizeof(s-io));
-s-ioaddr = s-io + qemu_get_be32(f);
-s-iolen = qemu_get_be32(f);
-if (s-ioaddr = s-io + sizeof(s-io) || s-ioaddr  s-io)
+NANDFlashState *s = opaque;
+
+if (s-ioaddr_vmstate  sizeof(s-io)) {
 return -EINVAL;
+}
+s-ioaddr = s-io + s-ioaddr_vmstate;

-qemu_get_be32s(f, s-cmd);
-qemu_get_be32s(f, s-addr);
-s-addrlen = qemu_get_be32(f);
-s-status = qemu_get_be32(f);
-s-offset = qemu_get_be32(f);
 return 0;
 }

+static const VMStateDescription vmstate_nand = {
+.name = nand,
+.version_id = 0,
+.minimum_version_id = 0,
+.minimum_version_id_old = 0,
+.pre_save = nand_pre_save,
+.post_load = nand_post_load,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT8(cle, NANDFlashState),
+VMSTATE_UINT8(ale, NANDFlashState),
+VMSTATE_UINT8(ce, NANDFlashState),
+VMSTATE_UINT8(wp, NANDFlashState),
+VMSTATE_UINT8(gnd, NANDFlashState),
+VMSTATE_BUFFER(io, NANDFlashState),
+VMSTATE_UINT32(ioaddr_vmstate, NANDFlashState),
+VMSTATE_INT32(iolen, NANDFlashState),
+VMSTATE_UINT32(cmd, NANDFlashState),
+VMSTATE_UINT32(addr, NANDFlashState),
+VMSTATE_INT32(addrlen, NANDFlashState),
+VMSTATE_INT32(status, NANDFlashState),
+VMSTATE_INT32(offset, NANDFlashState),
+/* XXX: do we want to save s-storage too? */
+VMSTATE_END_OF_LIST()
+}
+};
+
 /*
  * Chip inputs are CLE, ALE, CE, WP, GND and eight I/O pins.  Chip
  * outputs are R/B and eight I/O pins.
@@ -502,7 +507,7 @@ NANDFlashState *nand_init(int manf_id, int chip_id)
is used.  */
 s-ioaddr = s-io;

-register_savevm(NULL, nand, -1, 0, nand_save, nand_load, s);
+vmstate_register(NULL, -1, vmstate_nand, s);

 return s;
 }
-- 
1.7.4




[Qemu-devel] [PATCH v2 3/3] qemu_next_deadline should not consider host-time timers

2011-03-10 Thread Paolo Bonzini
It is purely for icount-based virtual timers.  And now that we got the
code right, rename the function to clarify the intended scope.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 cpus.c   |4 ++--
 qemu-timer.c |   11 +++
 qemu-timer.h |2 +-
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/cpus.c b/cpus.c
index a953bac..d410f63 100644
--- a/cpus.c
+++ b/cpus.c
@@ -861,7 +861,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 
 while (1) {
 cpu_exec_all();
-if (use_icount  qemu_next_deadline() = 0) {
+if (use_icount  qemu_next_icount_deadline() = 0) {
 qemu_notify_event();
 }
 qemu_tcg_wait_io_event();
@@ -1059,7 +1059,7 @@ static int tcg_cpu_exec(CPUState *env)
 qemu_icount -= (env-icount_decr.u16.low + env-icount_extra);
 env-icount_decr.u16.low = 0;
 env-icount_extra = 0;
-count = qemu_icount_round (qemu_next_deadline());
+count = qemu_icount_round (qemu_next_icount_deadline());
 qemu_icount += count;
 decr = (count  0x) ? 0x : count;
 count -= decr;
diff --git a/qemu-timer.c b/qemu-timer.c
index 62dd504..c527844 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -688,21 +688,16 @@ static void host_alarm_handler(int host_signum)
 }
 }
 
-int64_t qemu_next_deadline(void)
+int64_t qemu_next_icount_deadline(void)
 {
 /* To avoid problems with overflow limit this to 2^32.  */
 int64_t delta = INT32_MAX;
 
+assert(use_icount);
 if (active_timers[QEMU_CLOCK_VIRTUAL]) {
 delta = active_timers[QEMU_CLOCK_VIRTUAL]-expire_time -
  qemu_get_clock_ns(vm_clock);
 }
-if (active_timers[QEMU_CLOCK_HOST]) {
-int64_t hdelta = active_timers[QEMU_CLOCK_HOST]-expire_time -
- qemu_get_clock_ns(host_clock);
-if (hdelta  delta)
-delta = hdelta;
-}
 
 if (delta  0)
 delta = 0;
@@ -1096,7 +1091,7 @@ int qemu_calculate_timeout(void)
 } else {
 /* Wait for either IO to occur or the next
timer event.  */
-add = qemu_next_deadline();
+add = qemu_next_icount_deadline();
 /* We advance the timer before checking for IO.
Limit the amount we advance so that early IO
activity won't get the guest too far ahead.  */
diff --git a/qemu-timer.h b/qemu-timer.h
index 8cd8f83..9c3e52f 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -46,7 +46,7 @@ int qemu_timer_expired(QEMUTimer *timer_head, int64_t 
current_time);
 
 void qemu_run_all_timers(void);
 int qemu_alarm_pending(void);
-int64_t qemu_next_deadline(void);
+int64_t qemu_next_icount_deadline(void);
 void configure_alarms(char const *opt);
 void configure_icount(const char *option);
 int qemu_calculate_timeout(void);
-- 
1.7.4




[Qemu-devel] [PATCH 10/13] piix4: create PIIX4State

2011-03-10 Thread Juan Quintela
It only contains a PCIDevice by know, but it makes easy to use migration code

Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/piix4.c |   29 +
 1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/hw/piix4.c b/hw/piix4.c
index 72073cd..40cd91a 100644
--- a/hw/piix4.c
+++ b/hw/piix4.c
@@ -30,10 +30,14 @@

 PCIDevice *piix4_dev;

+typedef struct PIIX4State {
+PCIDevice dev;
+} PIIX4State;
+
 static void piix4_reset(void *opaque)
 {
-PCIDevice *d = opaque;
-uint8_t *pci_conf = d-config;
+PIIX4State *d = opaque;
+uint8_t *pci_conf = d-dev.config;

 pci_conf[0x04] = 0x07; // master, memory and I/O
 pci_conf[0x05] = 0x00;
@@ -70,31 +74,32 @@ static void piix4_reset(void *opaque)

 static void piix_save(QEMUFile* f, void *opaque)
 {
-PCIDevice *d = opaque;
-pci_device_save(d, f);
+PIIX4State *d = opaque;
+pci_device_save(d-dev, f);
 }

 static int piix_load(QEMUFile* f, void *opaque, int version_id)
 {
-PCIDevice *d = opaque;
+PIIX4State *d = opaque;
 if (version_id != 2)
 return -EINVAL;
-return pci_device_load(d, f);
+return pci_device_load(d-dev, f);
 }

-static int piix4_initfn(PCIDevice *d)
+static int piix4_initfn(PCIDevice *dev)
 {
+PIIX4State *d = DO_UPCAST(PIIX4State, dev, dev);
 uint8_t *pci_conf;

-isa_bus_new(d-qdev);
-register_savevm(d-qdev, PIIX4, 0, 2, piix_save, piix_load, d);
+isa_bus_new(d-dev.qdev);
+register_savevm(d-dev.qdev, PIIX4, 0, 2, piix_save, piix_load, d);

-pci_conf = d-config;
+pci_conf = d-dev.config;
 pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
 pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371AB_0); // 
82371AB/EB/MB PIIX4 PCI-to-ISA bridge
 pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_ISA);

-piix4_dev = d;
+piix4_dev = d-dev;
 qemu_register_reset(piix4_reset, d);
 return 0;
 }
@@ -111,7 +116,7 @@ static PCIDeviceInfo piix4_info[] = {
 {
 .qdev.name= PIIX4,
 .qdev.desc= ISA bridge,
-.qdev.size= sizeof(PCIDevice),
+.qdev.size= sizeof(PIIX4State),
 .qdev.no_user = 1,
 .no_hotplug   = 1,
 .init = piix4_initfn,
-- 
1.7.4




[Qemu-devel] [PATCH 15/32] vmstate: port stellaris ssi bus

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/stellaris.c |   31 +++
 1 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/hw/stellaris.c b/hw/stellaris.c
index 715e48c..9b83fb4 100644
--- a/hw/stellaris.c
+++ b/hw/stellaris.c
@@ -1219,24 +1219,16 @@ static uint32_t stellaris_ssi_bus_transfer(SSISlave 
*dev, uint32_t val)
 return ssi_transfer(s-bus[s-current_dev], val);
 }

-static void stellaris_ssi_bus_save(QEMUFile *f, void *opaque)
-{
-stellaris_ssi_bus_state *s = (stellaris_ssi_bus_state *)opaque;
-
-qemu_put_be32(f, s-current_dev);
-}
-
-static int stellaris_ssi_bus_load(QEMUFile *f, void *opaque, int version_id)
-{
-stellaris_ssi_bus_state *s = (stellaris_ssi_bus_state *)opaque;
-
-if (version_id != 1)
-return -EINVAL;
-
-s-current_dev = qemu_get_be32(f);
-
-return 0;
-}
+static const VMStateDescription vmstate_stellaris_ssi_bus = {
+.name = stellaris_ssi_bus,
+.version_id = 1,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_INT32(current_dev, stellaris_ssi_bus_state),
+VMSTATE_END_OF_LIST()
+}
+};

 static int stellaris_ssi_bus_init(SSISlave *dev)
 {
@@ -1246,8 +1238,7 @@ static int stellaris_ssi_bus_init(SSISlave *dev)
 s-bus[1] = ssi_create_bus(dev-qdev, ssi1);
 qdev_init_gpio_in(dev-qdev, stellaris_ssi_bus_select, 1);

-register_savevm(dev-qdev, stellaris_ssi_bus, -1, 1,
-stellaris_ssi_bus_save, stellaris_ssi_bus_load, s);
+vmstate_register(dev-qdev, -1, vmstate_stellaris_ssi_bus, s);
 return 0;
 }

-- 
1.7.4




Re: [Qemu-devel] [PATCH 14/22] qapi: add query-version QMP command

2011-03-10 Thread Avi Kivity

On 03/10/2011 02:41 PM, Avi Kivity wrote:
I don't think I want to make this sort of change just yet.  Also note 
that the schema that will be exposed over the wire is not directly 
related to the schema we use for code generation.


Right, we have to nail down the format for the former, though.



btw.  Back in the day when json was proposed vs. a custom text-line 
oriented protocol, it beat established RPCs because they were all so 
cumbersome with IDLs and code generation and general heavyweightness.  
And now we're making our json-rpc exactly like that.  There's a moral in 
there somewhere.


--
error compiling committee.c: too many arguments to function




[Qemu-devel] [PATCH 1/2] sockets: add qemu_socketpair()

2011-03-10 Thread Corentin Chary
Signed-off-by: Corentin Chary corentin.ch...@gmail.com
---
 osdep.c   |   83 +
 qemu_socket.h |1 +
 2 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/osdep.c b/osdep.c
index 327583b..93bfbe0 100644
--- a/osdep.c
+++ b/osdep.c
@@ -147,6 +147,89 @@ int qemu_socket(int domain, int type, int protocol)
 return ret;
 }
 
+#ifdef _WIN32
+int qemu_socketpair(int domain, int type, int protocol, int socks[2])
+{
+union {
+   struct sockaddr_in inaddr;
+   struct sockaddr addr;
+} a;
+int listener;
+socklen_t addrlen = sizeof(a.inaddr);
+int reuse = 1;
+
+if (domain == AF_UNIX) {
+domain = AF_INET;
+}
+
+if (socks == 0) {
+return EINVAL;
+}
+
+listener = qemu_socket(domain, type, protocol);
+if (listener  0) {
+return listener;
+}
+
+memset(a, 0, sizeof(a));
+a.inaddr.sin_family = AF_INET;
+a.inaddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+a.inaddr.sin_port = 0;
+
+socks[0] = socks[1] = -1;
+
+if (setsockopt(listener, SOL_SOCKET, SO_REUSEADDR,
+   (char*) reuse, (socklen_t) sizeof(reuse)) == -1) {
+goto error;
+}
+if  (bind(listener, a.addr, sizeof(a.inaddr))  0) {
+goto error;
+}
+
+memset(a, 0, sizeof(a));
+if  (getsockname(listener, a.addr, addrlen)  0) {
+goto error;
+}
+
+a.inaddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+a.inaddr.sin_family = AF_INET;
+
+if (listen(listener, 1)  0) {
+goto error;
+}
+
+socks[0] = qemu_socket(AF_INET, SOCK_STREAM, 0);
+if (socks[0]  0) {
+goto error;
+}
+if (connect(socks[0], a.addr, sizeof(a.inaddr))  0) {
+goto error;
+}
+
+socks[1] = qemu_accept(listener, NULL, NULL);
+if (socks[1]  0) {
+goto error;
+}
+
+closesocket(listener);
+return 0;
+
+error:
+if (listener != -1)
+closesocket(listener);
+if (socks[0] != -1)
+closesocket(socks[0]);
+if (socks[1] != -1)
+closesocket(socks[1]);
+return -1;
+}
+#else
+int qemu_socketpair(int domain, int type, int protocol, int socks[2])
+{
+return socketpair(domain, type, protocol, socks);
+}
+#endif
+
 /*
  * Accept a connection and set FD_CLOEXEC
  */
diff --git a/qemu_socket.h b/qemu_socket.h
index 180e4db..d7eb9a5 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -34,6 +34,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
 
 /* misc helpers */
 int qemu_socket(int domain, int type, int protocol);
+int qemu_socketpair(int domain, int type, int protocol, int socks[2]);
 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
 void socket_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
-- 
1.7.3.4




[Qemu-devel] [PATCH v2 2/3] Revert wrong fixes for -icount in the iothread case

2011-03-10 Thread Paolo Bonzini
This reverts commits 225d02cd and c9f7383c.  While some parts of
the latter could be saved, I preferred a smooth, complete revert.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 qemu-timer.c |   66 +++--
 1 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 88c7b28..62dd504 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -110,9 +110,12 @@ static int64_t cpu_get_clock(void)
 }
 }
 
+#ifndef CONFIG_IOTHREAD
 static int64_t qemu_icount_delta(void)
 {
-if (use_icount == 1) {
+if (!use_icount) {
+return 5000 * (int64_t) 100;
+} else if (use_icount == 1) {
 /* When not using an adaptive execution frequency
we tend to get badly out of sync with real time,
so just delay for a reasonable amount of time.  */
@@ -121,6 +124,7 @@ static int64_t qemu_icount_delta(void)
 return cpu_get_icount() - cpu_get_clock();
 }
 }
+#endif
 
 /* enable cpu_get_ticks() */
 void cpu_enable_ticks(void)
@@ -1074,39 +1078,41 @@ void quit_timers(void)
 
 int qemu_calculate_timeout(void)
 {
+#ifndef CONFIG_IOTHREAD
 int timeout;
-int64_t add;
-int64_t delta;
 
-/* When using icount, making forward progress with qemu_icount when the
-   guest CPU is idle is critical. We only use the static io-thread timeout
-   for non icount runs.  */
-if (!use_icount || !vm_running) {
-return 5000;
-}
-
-/* Advance virtual time to the next event.  */
-delta = qemu_icount_delta();
-if (delta  0) {
-/* If virtual time is ahead of real time then just
-   wait for IO.  */
-timeout = (delta + 99) / 100;
-} else {
-/* Wait for either IO to occur or the next
-   timer event.  */
-add = qemu_next_deadline();
-/* We advance the timer before checking for IO.
-   Limit the amount we advance so that early IO
-   activity won't get the guest too far ahead.  */
-if (add  1000)
-add = 1000;
-delta += add;
-qemu_icount += qemu_icount_round (add);
-timeout = delta / 100;
-if (timeout  0)
-timeout = 0;
+if (!vm_running)
+timeout = 5000;
+else {
+ /* XXX: use timeout computed from timers */
+int64_t add;
+int64_t delta;
+/* Advance virtual time to the next event.  */
+   delta = qemu_icount_delta();
+if (delta  0) {
+/* If virtual time is ahead of real time then just
+   wait for IO.  */
+timeout = (delta + 99) / 100;
+} else {
+/* Wait for either IO to occur or the next
+   timer event.  */
+add = qemu_next_deadline();
+/* We advance the timer before checking for IO.
+   Limit the amount we advance so that early IO
+   activity won't get the guest too far ahead.  */
+if (add  1000)
+add = 1000;
+delta += add;
+qemu_icount += qemu_icount_round (add);
+timeout = delta / 100;
+if (timeout  0)
+timeout = 0;
+}
 }
 
 return timeout;
+#else /* CONFIG_IOTHREAD */
+return 1000;
+#endif
 }
 
-- 
1.7.4





[Qemu-devel] [PATCH 13/13] vmstate: port mac_dbdma

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/mac_dbdma.c |   46 ++
 1 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/hw/mac_dbdma.c b/hw/mac_dbdma.c
index c108aee..ed4458e 100644
--- a/hw/mac_dbdma.c
+++ b/hw/mac_dbdma.c
@@ -810,30 +810,28 @@ static CPUReadMemoryFunc * const dbdma_read[] = {
 dbdma_readl,
 };

-static void dbdma_save(QEMUFile *f, void *opaque)
-{
-DBDMAState *s = opaque;
-unsigned int i, j;
-
-for (i = 0; i  DBDMA_CHANNELS; i++)
-for (j = 0; j  DBDMA_REGS; j++)
-qemu_put_be32s(f, s-channels[i].regs[j]);
-}
-
-static int dbdma_load(QEMUFile *f, void *opaque, int version_id)
-{
-DBDMAState *s = opaque;
-unsigned int i, j;
-
-if (version_id != 2)
-return -EINVAL;
-
-for (i = 0; i  DBDMA_CHANNELS; i++)
-for (j = 0; j  DBDMA_REGS; j++)
-qemu_get_be32s(f, s-channels[i].regs[j]);
+static const VMStateDescription vmstate_dbdma_channel = {
+.name = dbdma_channel,
+.version_id = 0,
+.minimum_version_id = 0,
+.minimum_version_id_old = 0,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT32_ARRAY(regs, struct DBDMA_channel, DBDMA_REGS),
+VMSTATE_END_OF_LIST()
+}
+};

-return 0;
-}
+static const VMStateDescription vmstate_dbdma = {
+.name = dbdma,
+.version_id = 2,
+.minimum_version_id = 2,
+.minimum_version_id_old = 2,
+.fields  = (VMStateField[]) {
+VMSTATE_STRUCT_ARRAY(channels, DBDMAState, DBDMA_CHANNELS, 1,
+ vmstate_dbdma_channel, DBDMA_channel),
+VMSTATE_END_OF_LIST()
+}
+};

 static void dbdma_reset(void *opaque)
 {
@@ -852,7 +850,7 @@ void* DBDMA_init (int *dbdma_mem_index)

 *dbdma_mem_index = cpu_register_io_memory(dbdma_read, dbdma_write, s,
   DEVICE_LITTLE_ENDIAN);
-register_savevm(NULL, dbdma, -1, 1, dbdma_save, dbdma_load, s);
+vmstate_register(NULL, -1, vmstate_dbdma, s);
 qemu_register_reset(dbdma_reset, s);

 dbdma_bh = qemu_bh_new(DBDMA_run_bh, s);
-- 
1.7.4




[Qemu-devel] [PATCH 2/2] vnc: don't mess up with iohandlers in the vnc thread

2011-03-10 Thread Corentin Chary
The threaded VNC servers messed up with QEMU fd handlers without
any kind of locking, and that can cause some nasty race conditions.

Using qemu_mutex_lock_iothread() won't work because vnc_dpy_cpy(),
which will wait for the current job queue to finish, can be called with
the iothread lock held.

Instead, we now store the data in a temporary buffer, and use a socket
pair to notify the main thread that new data is available. The data
is not directly send through this socket because it would make the code
more complex without any performance benefit.

vnc_[un]lock_ouput() is still needed to access VncState members like
abort, csock or jobs_buffer.

Thanks to Jan Kiszka for helping me solve this issue.

Signed-off-by: Corentin Chary corentin.ch...@gmail.com
---
 ui/vnc-jobs-async.c |   50 --
 ui/vnc-jobs-sync.c  |4 
 ui/vnc-jobs.h   |1 +
 ui/vnc.c|   16 
 ui/vnc.h|2 ++
 5 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c
index f596247..543b5a9 100644
--- a/ui/vnc-jobs-async.c
+++ b/ui/vnc-jobs-async.c
@@ -28,6 +28,7 @@
 
 #include vnc.h
 #include vnc-jobs.h
+#include qemu_socket.h
 
 /*
  * Locking:
@@ -155,6 +156,24 @@ void vnc_jobs_join(VncState *vs)
 qemu_cond_wait(queue-cond, queue-mutex);
 }
 vnc_unlock_queue(queue);
+vnc_jobs_consume_buffer(vs);
+}
+
+void vnc_jobs_consume_buffer(VncState *vs)
+{
+bool flush;
+
+vnc_lock_output(vs);
+if (vs-jobs_buffer.offset) {
+vnc_write(vs, vs-jobs_buffer.buffer, vs-jobs_buffer.offset);
+buffer_reset(vs-jobs_buffer);
+}
+flush = vs-csock != -1  vs-abort != true;
+vnc_unlock_output(vs);
+
+if (flush) {
+  vnc_flush(vs);
+}
 }
 
 /*
@@ -197,7 +216,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 VncState vs;
 int n_rectangles;
 int saved_offset;
-bool flush;
 
 vnc_lock_queue(queue);
 while (QTAILQ_EMPTY(queue-jobs)  !queue-exit) {
@@ -213,6 +231,7 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 
 vnc_lock_output(job-vs);
 if (job-vs-csock == -1 || job-vs-abort == true) {
+vnc_unlock_output(job-vs);
 goto disconnected;
 }
 vnc_unlock_output(job-vs);
@@ -233,10 +252,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 
 if (job-vs-csock == -1) {
 vnc_unlock_display(job-vs-vd);
-/* output mutex must be locked before going to
- * disconnected:
- */
-vnc_lock_output(job-vs);
 goto disconnected;
 }
 
@@ -254,24 +269,23 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 vs.output.buffer[saved_offset] = (n_rectangles  8)  0xFF;
 vs.output.buffer[saved_offset + 1] = n_rectangles  0xFF;
 
-/* Switch back buffers */
 vnc_lock_output(job-vs);
-if (job-vs-csock == -1) {
-goto disconnected;
-}
-
-vnc_write(job-vs, vs.output.buffer, vs.output.offset);
+if (job-vs-csock != -1) {
+int notify = !job-vs-jobs_buffer.offset;
 
-disconnected:
-/* Copy persistent encoding data */
-vnc_async_encoding_end(job-vs, vs);
-flush = (job-vs-csock != -1  job-vs-abort != true);
-vnc_unlock_output(job-vs);
+buffer_reserve(job-vs-jobs_buffer, vs.output.offset);
+buffer_append(job-vs-jobs_buffer, vs.output.buffer,
+  vs.output.offset);
+/* Copy persistent encoding data */
+vnc_async_encoding_end(job-vs, vs);
 
-if (flush) {
-vnc_flush(job-vs);
+if (notify) {
+send(job-vs-jobs_socks[1], (char []){1}, 1, 0);
+}
 }
+vnc_unlock_output(job-vs);
 
+disconnected:
 vnc_lock_queue(queue);
 QTAILQ_REMOVE(queue-jobs, job, next);
 vnc_unlock_queue(queue);
diff --git a/ui/vnc-jobs-sync.c b/ui/vnc-jobs-sync.c
index 49b77af..3a4d7df 100644
--- a/ui/vnc-jobs-sync.c
+++ b/ui/vnc-jobs-sync.c
@@ -36,6 +36,10 @@ void vnc_jobs_join(VncState *vs)
 {
 }
 
+void vnc_jobs_consume_buffer(VncState *vs)
+{
+}
+
 VncJob *vnc_job_new(VncState *vs)
 {
 vs-job.vs = vs;
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index b8dab81..7c529b7 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -37,6 +37,7 @@ void vnc_job_push(VncJob *job);
 bool vnc_has_job(VncState *vs);
 void vnc_jobs_clear(VncState *vs);
 void vnc_jobs_join(VncState *vs);
+void vnc_jobs_consume_buffer(VncState *vs);
 
 #ifdef CONFIG_VNC_THREAD
 
diff --git a/ui/vnc.c b/ui/vnc.c
index 610f884..48a81a2 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1011,6 +1011,10 @@ static void vnc_disconnect_finish(VncState *vs)
 
 #ifdef CONFIG_VNC_THREAD
 qemu_mutex_destroy(vs-output_mutex);
+qemu_set_fd_handler2(vs-jobs_socks[0], NULL, NULL, NULL, vs);
+close(vs-jobs_socks[0]);
+close(vs-jobs_socks[1]);
+buffer_free(vs-jobs_buffer);
 #endif
 for (i = 0; i  VNC_STAT_ROWS; ++i) {
 

Re: [Qemu-devel] virtio block device and sysfs

2011-03-10 Thread Marc Haber
On Tue, Sep 14, 2010 at 09:43:22AM +0200, Marc Haber wrote:
 On Mon, Sep 13, 2010 at 09:34:24AM -0500, Ryan Harper wrote:
  It'll only be available to guests launched with newer qemu (0.13) as
  virtio-blk serial support is a new feature.
 
 Thanks for the information, I'll wait for the next release (or the
 time to locally build 0.13-rc for testing, whatever happens first).

I now have qemu-kvm 0.14, and the serial number is imported to the
guest just fine, and there is also already a udev rule present in
Debian which will make the disk show up in /dev/disk/by-id/virtio-foo.

Thank you very much for helping, I really appreciate that.

Greetings
Marc

-- 
-
Marc Haber | I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things.Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190



[Qemu-devel] [PATCH 11/13] vmstate: port piix4

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/piix4.c |   25 +++--
 1 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/hw/piix4.c b/hw/piix4.c
index 40cd91a..71f1f84 100644
--- a/hw/piix4.c
+++ b/hw/piix4.c
@@ -72,19 +72,16 @@ static void piix4_reset(void *opaque)
 pci_conf[0xae] = 0x00;
 }

-static void piix_save(QEMUFile* f, void *opaque)
-{
-PIIX4State *d = opaque;
-pci_device_save(d-dev, f);
-}
-
-static int piix_load(QEMUFile* f, void *opaque, int version_id)
-{
-PIIX4State *d = opaque;
-if (version_id != 2)
-return -EINVAL;
-return pci_device_load(d-dev, f);
-}
+static const VMStateDescription vmstate_piix4 = {
+.name = PIIX4,
+.version_id = 2,
+.minimum_version_id = 2,
+.minimum_version_id_old = 2,
+.fields  = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(dev, PIIX4State),
+VMSTATE_END_OF_LIST()
+}
+};

 static int piix4_initfn(PCIDevice *dev)
 {
@@ -92,7 +89,6 @@ static int piix4_initfn(PCIDevice *dev)
 uint8_t *pci_conf;

 isa_bus_new(d-dev.qdev);
-register_savevm(d-dev.qdev, PIIX4, 0, 2, piix_save, piix_load, d);

 pci_conf = d-dev.config;
 pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
@@ -117,6 +113,7 @@ static PCIDeviceInfo piix4_info[] = {
 .qdev.name= PIIX4,
 .qdev.desc= ISA bridge,
 .qdev.size= sizeof(PIIX4State),
+.qdev.vmsd= vmstate_piix4,
 .qdev.no_user = 1,
 .no_hotplug   = 1,
 .init = piix4_initfn,
-- 
1.7.4




[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jan Kiszka
On 2011-03-10 12:48, Gleb Natapov wrote:
 On Thu, Mar 10, 2011 at 12:27:55PM +0100, Jan Kiszka wrote:
 On 2011-03-10 10:47, Gleb Natapov wrote:
 On Wed, Mar 09, 2011 at 08:51:23PM -0800, Jordan Justen wrote:
 Hi all,

 I have documented a simple flash-like device which I think could be
 useful for qemu/kvm in some cases.  (Particularly for allowing
 persistent UEFI non-volatile variables.)

 http://wiki.qemu.org/Features/System_Flash

 Let me know if you have any suggestions or concerns.


 Two things. First You suggest to replace -bios with -flash. This will
 make firmware upgrade painful process that will have to be performed
 from inside the guest since the same flash image will contain both
 firmware and whatever data was stored on a flash which presumably you
 want to reuse after upgrading a firmware. My suggestion is to extend
 -bios option like this:

 -bios bios.bin,flash=flash.bin,flash_base=addr

 flash.bin will be mapped at address flash_base, or, if flash_base is not
 present, just below bios.bin.

 ...or define -flash in a way that allows mapping the bios image as an
 overlay to the otherwise guest-managed flash image.

 It is not much different from what I proposed. The result will be the
 same. Even option syntax will probably be the same :)

-bios is PC-centric, the new command should be generic.

 

 Second. I asked how flash is programmed because interfaces like CFI
 where you write into flash memory address range to issue commands cannot
 be emulated efficiently in KVM. KVM supports either regular memory slots
 or IO memory, but in your proposal the same memory behaves as IO on
 write and regular memory on read. Better idea would be to present
 non-volatile flash as ISA virtio device. Should be simple to implement.

 Why not enhancing KVM memory slots to support direct read access while
 writes are trapped and forwarded to a user space device model?
 Yes we can make memory slot that will be treated as memory on read and
 IO on write, but first relying on that will prevent using flash interface
 on older kernels and second it is not enough to implement the proposal.
 When magic value is written into an address, the address become IO for
 reading too, but KVM slot granularity is page, not byte, so KVM will
 have to remove the slot to make it IO, but KVM can't execute code from
 IO region (yet), so we will not be able to run firmware from flash and
 simultaneously write into the flash. 

Yeah, right. I remember that this was also hairy over TCG if you tried
to optimize flash emulation so that writing doesn't take orders of
magnitude longer than on real HW.

BTW, the programming granularity is not bytes but chips with common CFI.
But that's still tricky if you want to run code from the same chip while
updating parts of it. The easiest workaround would be handling the
overlay regions as ROM all the time. Not accurate but realizable without
kernel changes.

 
 Virtio
 means that you have to patch the guest (which might be something else
 than flexible Linux...).

 This intended to be used by firmware only and we control that.

I'm thinking beyond this use case, beyond firmware flashes, beyond x86.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH v2 1/3] really fix -icount in the iothread case

2011-03-10 Thread Paolo Bonzini
The correct fix for -icount is to consider the biggest difference
between iothread and non-iothread modes.  In the traditional model,
CPUs run _before_ the iothread calls select (or WaitForMultipleObjects
for Win32).  In the iothread model, CPUs run while the iothread
isn't holding the mutex, i.e. _during_ those same calls.

So, the iothread should always block as long as possible to let
the CPUs run smoothly---the timeout might as well be infinite---and
either the OS or the CPU thread itself will let the iothread know
when something happens.  At this point, the iothread wakes up and
interrupts the CPU.

This is exactly the approach that this patch takes: when cpu_exec_all
returns in -icount mode, and it is because a vm_clock deadline has
been met, it wakes up the iothread to process the timers.  This fixes
icount better than any other patch that was posted.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 cpus.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index 0f33945..a953bac 100644
--- a/cpus.c
+++ b/cpus.c
@@ -861,6 +861,9 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 
 while (1) {
 cpu_exec_all();
+if (use_icount  qemu_next_deadline() = 0) {
+qemu_notify_event();
+}
 qemu_tcg_wait_io_event();
 }
 
-- 
1.7.4





[Qemu-devel] Re: [PATCH 2/2] vnc: don't mess up with iohandlers in the vnc thread

2011-03-10 Thread Paolo Bonzini

On 03/10/2011 01:59 PM, Corentin Chary wrote:

Instead, we now store the data in a temporary buffer, and use a socket
pair to notify the main thread that new data is available.


You can use a bottom half for this instead of a special socket. 
Signaling a bottom half is async-signal- and thread-safe.


Paolo



[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Gleb Natapov
On Thu, Mar 10, 2011 at 01:06:14PM +0100, Jan Kiszka wrote:
 On 2011-03-10 12:48, Gleb Natapov wrote:
  On Thu, Mar 10, 2011 at 12:27:55PM +0100, Jan Kiszka wrote:
  On 2011-03-10 10:47, Gleb Natapov wrote:
  On Wed, Mar 09, 2011 at 08:51:23PM -0800, Jordan Justen wrote:
  Hi all,
 
  I have documented a simple flash-like device which I think could be
  useful for qemu/kvm in some cases.  (Particularly for allowing
  persistent UEFI non-volatile variables.)
 
  http://wiki.qemu.org/Features/System_Flash
 
  Let me know if you have any suggestions or concerns.
 
 
  Two things. First You suggest to replace -bios with -flash. This will
  make firmware upgrade painful process that will have to be performed
  from inside the guest since the same flash image will contain both
  firmware and whatever data was stored on a flash which presumably you
  want to reuse after upgrading a firmware. My suggestion is to extend
  -bios option like this:
 
  -bios bios.bin,flash=flash.bin,flash_base=addr
 
  flash.bin will be mapped at address flash_base, or, if flash_base is not
  present, just below bios.bin.
 
  ...or define -flash in a way that allows mapping the bios image as an
  overlay to the otherwise guest-managed flash image.
 
  It is not much different from what I proposed. The result will be the
  same. Even option syntax will probably be the same :)
 
 -bios is PC-centric, the new command should be generic.
 
Well, I tried to reuse the option we already have instead of introducing
another one. -bios can be extended beyond PC and represent general
firmware specification. But I like the option you proposed in other
email too, so I am not going to defend this one.


  
 
  Second. I asked how flash is programmed because interfaces like CFI
  where you write into flash memory address range to issue commands cannot
  be emulated efficiently in KVM. KVM supports either regular memory slots
  or IO memory, but in your proposal the same memory behaves as IO on
  write and regular memory on read. Better idea would be to present
  non-volatile flash as ISA virtio device. Should be simple to implement.
 
  Why not enhancing KVM memory slots to support direct read access while
  writes are trapped and forwarded to a user space device model?
  Yes we can make memory slot that will be treated as memory on read and
  IO on write, but first relying on that will prevent using flash interface
  on older kernels and second it is not enough to implement the proposal.
  When magic value is written into an address, the address become IO for
  reading too, but KVM slot granularity is page, not byte, so KVM will
  have to remove the slot to make it IO, but KVM can't execute code from
  IO region (yet), so we will not be able to run firmware from flash and
  simultaneously write into the flash. 
 
 Yeah, right. I remember that this was also hairy over TCG if you tried
 to optimize flash emulation so that writing doesn't take orders of
 magnitude longer than on real HW.
 
 BTW, the programming granularity is not bytes but chips with common CFI.
 But that's still tricky if you want to run code from the same chip while
 updating parts of it. The easiest workaround would be handling the
 overlay regions as ROM all the time. Not accurate but realizable without
 kernel changes.
 
So flash will be always IO and overlay will be always ROM. This will
work, except BIOS upgrade from inside the guest will not be possible,
but since we do not support this today too it doesn't bother me to much.

  
  Virtio
  means that you have to patch the guest (which might be something else
  than flexible Linux...).
 
  This intended to be used by firmware only and we control that.
 
 I'm thinking beyond this use case, beyond firmware flashes, beyond x86.
 
OK, but since both interfaces (virtio and one proposed in the wiki) are PV
I fail to see the difference between them for any use case. If we
implement CFI then it will be another story.

--
Gleb.



[Qemu-devel] [PATCH 32/32] vmstate: stellaris use unused for placeholder entries

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/stellaris.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/stellaris.c b/hw/stellaris.c
index 6e31d89..74815ad 100644
--- a/hw/stellaris.c
+++ b/hw/stellaris.c
@@ -291,8 +291,7 @@ static const VMStateDescription vmstate_stellaris_gptm = {
 VMSTATE_UINT32(control, gptm_state),
 VMSTATE_UINT32(state, gptm_state),
 VMSTATE_UINT32(mask, gptm_state),
-VMSTATE_UINT32(mode[0], gptm_state),
-VMSTATE_UINT32(mode[0], gptm_state),
+VMSTATE_UNUSED(8),
 VMSTATE_UINT32_ARRAY(load, gptm_state, 2),
 VMSTATE_UINT32_ARRAY(match, gptm_state, 2),
 VMSTATE_UINT32_ARRAY(prescale, gptm_state, 2),
-- 
1.7.4




[Qemu-devel] [PATCH 05/13] vmstate: port max111x

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/max111x.c |   49 +
 1 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/hw/max111x.c b/hw/max111x.c
index 3adc3e4..70cd1af 100644
--- a/hw/max111x.c
+++ b/hw/max111x.c
@@ -94,36 +94,22 @@ static uint32_t max111x_transfer(SSISlave *dev, uint32_t 
value)
 return max111x_read(s);
 }

-static void max111x_save(QEMUFile *f, void *opaque)
-{
-MAX111xState *s = (MAX111xState *) opaque;
-int i;
-
-qemu_put_8s(f, s-tb1);
-qemu_put_8s(f, s-rb2);
-qemu_put_8s(f, s-rb3);
-qemu_put_be32(f, s-inputs);
-qemu_put_be32(f, s-com);
-for (i = 0; i  s-inputs; i ++)
-qemu_put_byte(f, s-input[i]);
-}
-
-static int max111x_load(QEMUFile *f, void *opaque, int version_id)
-{
-MAX111xState *s = (MAX111xState *) opaque;
-int i;
-
-qemu_get_8s(f, s-tb1);
-qemu_get_8s(f, s-rb2);
-qemu_get_8s(f, s-rb3);
-if (s-inputs != qemu_get_be32(f))
-return -EINVAL;
-s-com = qemu_get_be32(f);
-for (i = 0; i  s-inputs; i ++)
-s-input[i] = qemu_get_byte(f);
-
-return 0;
-}
+static const VMStateDescription vmstate_max111x = {
+.name = max111x,
+.version_id = 0,
+.minimum_version_id = 0,
+.minimum_version_id_old = 0,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT8(tb1, MAX111xState),
+VMSTATE_UINT8(rb2, MAX111xState),
+VMSTATE_UINT8(rb3, MAX111xState),
+VMSTATE_INT32_EQUAL(inputs, MAX111xState),
+VMSTATE_INT32(com, MAX111xState),
+VMSTATE_ARRAY_INT32_UNSAFE(input, MAX111xState, inputs,
+   vmstate_info_uint8, uint8_t),
+VMSTATE_END_OF_LIST()
+}
+};

 static int max111x_init(SSISlave *dev, int inputs)
 {
@@ -143,8 +129,7 @@ static int max111x_init(SSISlave *dev, int inputs)
 s-input[7] = 0x80;
 s-com = 0;

-register_savevm(dev-qdev, max111x, -1, 0,
-max111x_save, max111x_load, s);
+vmstate_register(dev-qdev, -1, vmstate_max111x, s);
 return 0;
 }

-- 
1.7.4




Re: [Qemu-devel] [PATCH 14/22] qapi: add query-version QMP command

2011-03-10 Thread Avi Kivity

On 03/09/2011 04:47 PM, Anthony Liguori wrote:

[

{ 'type': 'MyType', fields: [['a', 'str'], ['b', 'int'], ['c', 
'AnotherType']] }

{ 'event': 'MY_EVENT', 'arguments': [ ... ] }
{ 'command': 'my-command', 'arguments': [ ... ], 'return': ... }

]

which leaves us room for additional metainformation.

The concern is more about non-qemu consumers of the schema.



Yeah, I dislike it too and I've been leaning towards changing it.

My preference would be:

{ 'type': 'MyType', 'fields': { 'a': 'str', 'b': 'int', 'c': 
'AnotherType' } }

{ 'event': 'MY_EVENT', 'arguments': {...} }
{ 'command': 'my-command', 'arguments': {...}, 'returns': 'int' }

I do prefer the dictionary syntax for arguments over a list because a 
list implies order.  Plus I think the syntax is just awkward and a 
whole lot easier to get wrong (too many/few elements in list).


Yeah.  We can rationalize it by saying that most dynamic consumers of 
the schema will not care about argument order, and that if they do, they 
can implement a custom parser.


I don't think I want to make this sort of change just yet.  Also note 
that the schema that will be exposed over the wire is not directly 
related to the schema we use for code generation.


Right, we have to nail down the format for the former, though.

--
error compiling committee.c: too many arguments to function




[Qemu-devel] Re: [PATCH 2/2] vnc: don't mess up with iohandlers in the vnc thread

2011-03-10 Thread Anthony Liguori

On 03/10/2011 07:06 AM, Paolo Bonzini wrote:

On 03/10/2011 01:59 PM, Corentin Chary wrote:

Instead, we now store the data in a temporary buffer, and use a socket
pair to notify the main thread that new data is available.


You can use a bottom half for this instead of a special socket. 
Signaling a bottom half is async-signal- and thread-safe.


Bottom halves are thread safe?

I don't think so.

They probably should be but they aren't today.

Regards,

Anthony Liguori



Paolo





[Qemu-devel] Re: [PATCH 2/2] vnc: don't mess up with iohandlers in the vnc thread

2011-03-10 Thread Peter Lieven

On 10.03.2011 13:59, Corentin Chary wrote:

The threaded VNC servers messed up with QEMU fd handlers without
any kind of locking, and that can cause some nasty race conditions.

Using qemu_mutex_lock_iothread() won't work because vnc_dpy_cpy(),
which will wait for the current job queue to finish, can be called with
the iothread lock held.

Instead, we now store the data in a temporary buffer, and use a socket
pair to notify the main thread that new data is available. The data
is not directly send through this socket because it would make the code
more complex without any performance benefit.

vnc_[un]lock_ouput() is still needed to access VncState members like
abort, csock or jobs_buffer.



is this compatible with qemu-kvm?

thanks,
peter



Thanks to Jan Kiszka for helping me solve this issue.

Signed-off-by: Corentin Charycorentin.ch...@gmail.com
---
  ui/vnc-jobs-async.c |   50 --
  ui/vnc-jobs-sync.c  |4 
  ui/vnc-jobs.h   |1 +
  ui/vnc.c|   16 
  ui/vnc.h|2 ++
  5 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c
index f596247..543b5a9 100644
--- a/ui/vnc-jobs-async.c
+++ b/ui/vnc-jobs-async.c
@@ -28,6 +28,7 @@

  #include vnc.h
  #include vnc-jobs.h
+#include qemu_socket.h

  /*
   * Locking:
@@ -155,6 +156,24 @@ void vnc_jobs_join(VncState *vs)
  qemu_cond_wait(queue-cond,queue-mutex);
  }
  vnc_unlock_queue(queue);
+vnc_jobs_consume_buffer(vs);
+}
+
+void vnc_jobs_consume_buffer(VncState *vs)
+{
+bool flush;
+
+vnc_lock_output(vs);
+if (vs-jobs_buffer.offset) {
+vnc_write(vs, vs-jobs_buffer.buffer, vs-jobs_buffer.offset);
+buffer_reset(vs-jobs_buffer);
+}
+flush = vs-csock != -1  vs-abort != true;
+vnc_unlock_output(vs);
+
+if (flush) {
+  vnc_flush(vs);
+}
  }

  /*
@@ -197,7 +216,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
  VncState vs;
  int n_rectangles;
  int saved_offset;
-bool flush;

  vnc_lock_queue(queue);
  while (QTAILQ_EMPTY(queue-jobs)  !queue-exit) {
@@ -213,6 +231,7 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)

  vnc_lock_output(job-vs);
  if (job-vs-csock == -1 || job-vs-abort == true) {
+vnc_unlock_output(job-vs);
  goto disconnected;
  }
  vnc_unlock_output(job-vs);
@@ -233,10 +252,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)

  if (job-vs-csock == -1) {
  vnc_unlock_display(job-vs-vd);
-/* output mutex must be locked before going to
- * disconnected:
- */
-vnc_lock_output(job-vs);
  goto disconnected;
  }

@@ -254,24 +269,23 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
  vs.output.buffer[saved_offset] = (n_rectangles  8)  0xFF;
  vs.output.buffer[saved_offset + 1] = n_rectangles  0xFF;

-/* Switch back buffers */
  vnc_lock_output(job-vs);
-if (job-vs-csock == -1) {
-goto disconnected;
-}
-
-vnc_write(job-vs, vs.output.buffer, vs.output.offset);
+if (job-vs-csock != -1) {
+int notify = !job-vs-jobs_buffer.offset;

-disconnected:
-/* Copy persistent encoding data */
-vnc_async_encoding_end(job-vs,vs);
-flush = (job-vs-csock != -1  job-vs-abort != true);
-vnc_unlock_output(job-vs);
+buffer_reserve(job-vs-jobs_buffer, vs.output.offset);
+buffer_append(job-vs-jobs_buffer, vs.output.buffer,
+  vs.output.offset);
+/* Copy persistent encoding data */
+vnc_async_encoding_end(job-vs,vs);

-if (flush) {
-vnc_flush(job-vs);
+if (notify) {
+send(job-vs-jobs_socks[1], (char []){1}, 1, 0);
+}
  }
+vnc_unlock_output(job-vs);

+disconnected:
  vnc_lock_queue(queue);
  QTAILQ_REMOVE(queue-jobs, job, next);
  vnc_unlock_queue(queue);
diff --git a/ui/vnc-jobs-sync.c b/ui/vnc-jobs-sync.c
index 49b77af..3a4d7df 100644
--- a/ui/vnc-jobs-sync.c
+++ b/ui/vnc-jobs-sync.c
@@ -36,6 +36,10 @@ void vnc_jobs_join(VncState *vs)
  {
  }

+void vnc_jobs_consume_buffer(VncState *vs)
+{
+}
+
  VncJob *vnc_job_new(VncState *vs)
  {
  vs-job.vs = vs;
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index b8dab81..7c529b7 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -37,6 +37,7 @@ void vnc_job_push(VncJob *job);
  bool vnc_has_job(VncState *vs);
  void vnc_jobs_clear(VncState *vs);
  void vnc_jobs_join(VncState *vs);
+void vnc_jobs_consume_buffer(VncState *vs);

  #ifdef CONFIG_VNC_THREAD

diff --git a/ui/vnc.c b/ui/vnc.c
index 610f884..48a81a2 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1011,6 +1011,10 @@ static void vnc_disconnect_finish(VncState *vs)

  #ifdef CONFIG_VNC_THREAD
  qemu_mutex_destroy(vs-output_mutex);
+qemu_set_fd_handler2(vs-jobs_socks[0], NULL, NULL, NULL, vs);
+

[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jan Kiszka
On 2011-03-10 13:17, Gleb Natapov wrote:
 On Thu, Mar 10, 2011 at 01:06:14PM +0100, Jan Kiszka wrote:
 On 2011-03-10 12:48, Gleb Natapov wrote:
 On Thu, Mar 10, 2011 at 12:27:55PM +0100, Jan Kiszka wrote:
 On 2011-03-10 10:47, Gleb Natapov wrote:
 On Wed, Mar 09, 2011 at 08:51:23PM -0800, Jordan Justen wrote:
 Hi all,

 I have documented a simple flash-like device which I think could be
 useful for qemu/kvm in some cases.  (Particularly for allowing
 persistent UEFI non-volatile variables.)

 http://wiki.qemu.org/Features/System_Flash

 Let me know if you have any suggestions or concerns.


 Two things. First You suggest to replace -bios with -flash. This will
 make firmware upgrade painful process that will have to be performed
 from inside the guest since the same flash image will contain both
 firmware and whatever data was stored on a flash which presumably you
 want to reuse after upgrading a firmware. My suggestion is to extend
 -bios option like this:

 -bios bios.bin,flash=flash.bin,flash_base=addr

 flash.bin will be mapped at address flash_base, or, if flash_base is not
 present, just below bios.bin.

 ...or define -flash in a way that allows mapping the bios image as an
 overlay to the otherwise guest-managed flash image.

 It is not much different from what I proposed. The result will be the
 same. Even option syntax will probably be the same :)

 -bios is PC-centric, the new command should be generic.

 Well, I tried to reuse the option we already have instead of introducing
 another one. -bios can be extended beyond PC and represent general
 firmware specification. But I like the option you proposed in other
 email too, so I am not going to defend this one.
 
 


 Second. I asked how flash is programmed because interfaces like CFI
 where you write into flash memory address range to issue commands cannot
 be emulated efficiently in KVM. KVM supports either regular memory slots
 or IO memory, but in your proposal the same memory behaves as IO on
 write and regular memory on read. Better idea would be to present
 non-volatile flash as ISA virtio device. Should be simple to implement.

 Why not enhancing KVM memory slots to support direct read access while
 writes are trapped and forwarded to a user space device model?
 Yes we can make memory slot that will be treated as memory on read and
 IO on write, but first relying on that will prevent using flash interface
 on older kernels and second it is not enough to implement the proposal.
 When magic value is written into an address, the address become IO for
 reading too, but KVM slot granularity is page, not byte, so KVM will
 have to remove the slot to make it IO, but KVM can't execute code from
 IO region (yet), so we will not be able to run firmware from flash and
 simultaneously write into the flash. 

 Yeah, right. I remember that this was also hairy over TCG if you tried
 to optimize flash emulation so that writing doesn't take orders of
 magnitude longer than on real HW.

 BTW, the programming granularity is not bytes but chips with common CFI.
 But that's still tricky if you want to run code from the same chip while
 updating parts of it. The easiest workaround would be handling the
 overlay regions as ROM all the time. Not accurate but realizable without
 kernel changes.

 So flash will be always IO and overlay will be always ROM. This will

Yes, and once we have KVM support for read-RAM/write-IO slots, flash
will be able to switch between ROM and IO mode just like it already does
under TCG.

 work, except BIOS upgrade from inside the guest will not be possible,
 but since we do not support this today too it doesn't bother me to much.
 

 Virtio
 means that you have to patch the guest (which might be something else
 than flexible Linux...).

 This intended to be used by firmware only and we control that.

 I'm thinking beyond this use case, beyond firmware flashes, beyond x86.

 OK, but since both interfaces (virtio and one proposed in the wiki) are PV
 I fail to see the difference between them for any use case. If we
 implement CFI then it will be another story.

I'm proposing CFI (which already exists) with BIOS exception to avoid PV
as far as possible.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] Re: [PATCH 2/2] vnc: don't mess up with iohandlers in the vnc thread

2011-03-10 Thread Paolo Bonzini

On 03/10/2011 02:45 PM, Anthony Liguori wrote:

On 03/10/2011 07:06 AM, Paolo Bonzini wrote:

On 03/10/2011 01:59 PM, Corentin Chary wrote:

Instead, we now store the data in a temporary buffer, and use a socket
pair to notify the main thread that new data is available.


You can use a bottom half for this instead of a special socket.
Signaling a bottom half is async-signal- and thread-safe.


Bottom halves are thread safe?

I don't think so.

They probably should be but they aren't today.


Creating a new bottom half is not thread-safe, but scheduling one is. 
Assuming that you never use qemu_bh_schedule_idle, qemu_bh_schedule 
boils down to:


if (bh-scheduled)
return;
bh-scheduled = 1;
/* stop the currently executing CPU to execute the BH ASAP */
qemu_notify_event();

You may have a spurious wakeup if two threads race on the same bottom 
half (including the signaling thread racing with the IO thread), but 
overall you can safely treat them as thread-safe.


Paolo



[Qemu-devel] Re: [PATCH 2/2] vnc: don't mess up with iohandlers in the vnc thread

2011-03-10 Thread Corentin Chary
On Thu, Mar 10, 2011 at 1:45 PM, Anthony Liguori aligu...@us.ibm.com wrote:
 On 03/10/2011 07:06 AM, Paolo Bonzini wrote:

 On 03/10/2011 01:59 PM, Corentin Chary wrote:

 Instead, we now store the data in a temporary buffer, and use a socket
 pair to notify the main thread that new data is available.

 You can use a bottom half for this instead of a special socket. Signaling
 a bottom half is async-signal- and thread-safe.

 Bottom halves are thread safe?

 I don't think so.

The bottom halves API is not thread safe, but calling
qemu_bh_schedule_idle() in another thread *seems* to be safe (here, it
would be protected from qemu_bh_delete() by vnc_lock_output()).

-- 
Corentin Chary
http://xf.iksaif.net



[Qemu-devel] [PATCH 11/32] vmstate: port pxa2xx_keypad

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/pxa2xx_keypad.c |   53 ---
 1 files changed, 17 insertions(+), 36 deletions(-)

diff --git a/hw/pxa2xx_keypad.c b/hw/pxa2xx_keypad.c
index d77dbf1..10ef154 100644
--- a/hw/pxa2xx_keypad.c
+++ b/hw/pxa2xx_keypad.c
@@ -289,40 +289,22 @@ static CPUWriteMemoryFunc * const pxa2xx_keypad_writefn[] 
= {
 pxa2xx_keypad_write
 };

-static void pxa2xx_keypad_save(QEMUFile *f, void *opaque)
-{
-PXA2xxKeyPadState *s = (PXA2xxKeyPadState *) opaque;
-
-qemu_put_be32s(f, s-kpc);
-qemu_put_be32s(f, s-kpdk);
-qemu_put_be32s(f, s-kprec);
-qemu_put_be32s(f, s-kpmk);
-qemu_put_be32s(f, s-kpas);
-qemu_put_be32s(f, s-kpasmkp[0]);
-qemu_put_be32s(f, s-kpasmkp[1]);
-qemu_put_be32s(f, s-kpasmkp[2]);
-qemu_put_be32s(f, s-kpasmkp[3]);
-qemu_put_be32s(f, s-kpkdi);
-
-}
-
-static int pxa2xx_keypad_load(QEMUFile *f, void *opaque, int version_id)
-{
-PXA2xxKeyPadState *s = (PXA2xxKeyPadState *) opaque;
-
-qemu_get_be32s(f, s-kpc);
-qemu_get_be32s(f, s-kpdk);
-qemu_get_be32s(f, s-kprec);
-qemu_get_be32s(f, s-kpmk);
-qemu_get_be32s(f, s-kpas);
-qemu_get_be32s(f, s-kpasmkp[0]);
-qemu_get_be32s(f, s-kpasmkp[1]);
-qemu_get_be32s(f, s-kpasmkp[2]);
-qemu_get_be32s(f, s-kpasmkp[3]);
-qemu_get_be32s(f, s-kpkdi);
-
-return 0;
-}
+static const VMStateDescription vmstate_pxa2xx_keypad = {
+.name = pxa2xx_keypad,
+.version_id = 0,
+.minimum_version_id = 0,
+.minimum_version_id_old = 0,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT32(kpc, PXA2xxKeyPadState),
+VMSTATE_UINT32(kpdk, PXA2xxKeyPadState),
+VMSTATE_UINT32(kprec, PXA2xxKeyPadState),
+VMSTATE_UINT32(kpmk, PXA2xxKeyPadState),
+VMSTATE_UINT32(kpas, PXA2xxKeyPadState),
+VMSTATE_UINT32_ARRAY(kpasmkp, PXA2xxKeyPadState, 4),
+VMSTATE_UINT32(kpkdi, PXA2xxKeyPadState),
+VMSTATE_END_OF_LIST()
+}
+};

 PXA2xxKeyPadState *pxa27x_keypad_init(target_phys_addr_t base,
 qemu_irq irq)
@@ -337,8 +319,7 @@ PXA2xxKeyPadState *pxa27x_keypad_init(target_phys_addr_t 
base,
 pxa2xx_keypad_writefn, s, DEVICE_NATIVE_ENDIAN);
 cpu_register_physical_memory(base, 0x0010, iomemtype);

-register_savevm(NULL, pxa2xx_keypad, 0, 0,
-pxa2xx_keypad_save, pxa2xx_keypad_load, s);
+vmstate_register(NULL, 0, vmstate_pxa2xx_keypad, s);

 return s;
 }
-- 
1.7.4




[Qemu-devel] [PATCH 1/3] build: Create tools-obj-y variable

2011-03-10 Thread Juan Quintela
All our tools have to have exactly all this objects, just share them.

Signed-off-by: Juan Quintela quint...@redhat.com
---
 Makefile |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index eca4c76..9e090cb 100644
--- a/Makefile
+++ b/Makefile
@@ -150,14 +150,18 @@ version.o: $(SRC_PATH)/version.rc config-host.mak
 version-obj-$(CONFIG_WIN32) += version.o
 ##

+tools-obj-y=qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y)
+tools-obj-y+=$(block-obj-y) $(qobject-obj-y) $(version-obj-y)
+tools-obj-y+=qemu-timer-common.o
+
 qemu-img.o: qemu-img-cmds.h
 qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o: $(GENERATED_HEADERS)

-qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o
+qemu-img$(EXESUF): qemu-img.o $(tools-obj-y)

-qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o
+qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y)

-qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o
+qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y)

 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h  $  $@,  GEN  
 $@)
-- 
1.7.4




Re: [Qemu-devel] [PATCH 0/9] VMState infrastructure

2011-03-10 Thread Anthony Liguori

On 03/10/2011 05:33 AM, Juan Quintela wrote:

Hi

This is the infrastructure that I pushed on my previous series.
Anthony don't like 58 patches series (why? O:-) And then split the
series in three.


Yeah, my intention was that you not send all series at once though :-)

At any rate this series looks good.

Regards,

Anthony Liguori


This are the infrastructure patches needed for the other two series.

Anthony, please apply.

Later, Juan.

Juan Quintela (9):
   vmstate: add VMSTATE_UINT32_EQUAL
   vmstate: Fix varrays with uint8 indexes
   vmstate: add UINT32 VARRAYS
   vmstate: add VMSTATE_STRUCT_VARRAY_INT32
   vmstate: add VMSTATE_INT64_ARRAY
   vmstate: add VMSTATE_STRUCT_VARRAY_UINT32
   vmstate: Add a way to send a partial array
   vmstate: be able to store/save a pci device from a pointer
   vmstate: move timers to use test instead of version

  hw/hw.h  |   78 ++
  savevm.c |   25 
  2 files changed, 98 insertions(+), 5 deletions(-)






[Qemu-devel] [PATCH v2 0/3] build: make sharing of objects explicit

2011-03-10 Thread Juan Quintela
Hi

Another week, another version.

v2:
- rename common-obj-y to softmmu-obj-y, so we can use common-obj-y
  for objects shared between tools and softmmu.


v1:

- all tools shared the same list of object files, create a variable instead
  or repeating them (tools-obj-y).
- tools and softmmu targets share lots of objects, just make that explicit
  with shared-obj-y.

Please review, Juan.

Juan Quintela (3):
  build: Create tools-obj-y variable
  build: Rename common-obj-y to softmmu-obj-y
  build: Create common-obj-y for objects shared between tools and
softmmu

 Makefile|   12 +++--
 Makefile.objs   |  120 --
 Makefile.target |2 +-
 3 files changed, 70 insertions(+), 64 deletions(-)

-- 
1.7.4




[Qemu-devel] Re: [PATCH 2/2] vnc: don't mess up with iohandlers in the vnc thread

2011-03-10 Thread Paolo Bonzini

On 03/10/2011 02:54 PM, Corentin Chary wrote:

   You can use a bottom half for this instead of a special socket. Signaling
   a bottom half is async-signal- and thread-safe.

  Bottom halves are thread safe?

  I don't think so.

The bottom halves API is not thread safe, but calling
qemu_bh_schedule_idle()


Not _idle please.


in another thread *seems* to be safe (here, it
would be protected from qemu_bh_delete() by vnc_lock_output()).


If it weren't protected against qemu_bh_delete, you would have already 
the same race between writing to the socket and closing it in another 
thread.


Paolo



[Qemu-devel] [PATCH 3/3] build: Create common-obj-y for objects shared between tools and softmmu

2011-03-10 Thread Juan Quintela
This way we don't have to repeat them in two places.

Signed-off-by: Juan Quintela quint...@redhat.com
---
 Makefile  |4 +---
 Makefile.objs |   14 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 7811d74..42d2cab 100644
--- a/Makefile
+++ b/Makefile
@@ -150,9 +150,7 @@ version.o: $(SRC_PATH)/version.rc config-host.mak
 version-obj-$(CONFIG_WIN32) += version.o
 ##

-tools-obj-y=qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y)
-tools-obj-y+=$(block-obj-y) $(qobject-obj-y) $(version-obj-y)
-tools-obj-y+=qemu-timer-common.o
+tools-obj-y = qemu-tool.o $(common-obj-y) $(trace-obj-y) $(version-obj-y)

 qemu-img.o: qemu-img-cmds.h
 qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o: $(GENERATED_HEADERS)
diff --git a/Makefile.objs b/Makefile.objs
index b525e81..7e54ae3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -54,17 +54,21 @@ fsdev-nested-$(CONFIG_VIRTFS) = qemu-fsdev.o
 fsdev-obj-$(CONFIG_VIRTFS) += $(addprefix fsdev/, $(fsdev-nested-y))

 ##
+# common-obj-y has the object that are shared by qemu softmmu and tools
+
+common-obj-y  = qemu-error.o $(block-obj-y) $(qobject-obj-y) $(oslib-obj-y)
+common-obj-y += qemu-timer-common.o
+
+##
 # libqemu_common.a: Target independent part of system emulation. The
 # long term path is to suppress *all* target specific code in case of
 # system emulation, i.e. a single QEMU executable should support all
 # CPUs and machines.

-softmmu-obj-y = $(block-obj-y) blockdev.o
+softmmu-obj-y = $(common-obj-y) blockdev.o
 softmmu-obj-y += $(net-obj-y)
-softmmu-obj-y += $(qobject-obj-y)
 softmmu-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX))
-softmmu-obj-y += readline.o console.o cursor.o async.o qemu-error.o
-softmmu-obj-y += $(oslib-obj-y)
+softmmu-obj-y += readline.o console.o cursor.o async.o
 softmmu-obj-$(CONFIG_WIN32) += os-win32.o
 softmmu-obj-$(CONFIG_POSIX) += os-posix.o

@@ -145,7 +149,7 @@ softmmu-obj-y += iov.o acl.o
 softmmu-obj-$(CONFIG_THREAD) += qemu-thread.o
 softmmu-obj-$(CONFIG_POSIX) += compatfd.o
 softmmu-obj-y += notify.o event_notifier.o
-softmmu-obj-y += qemu-timer.o qemu-timer-common.o
+softmmu-obj-y += qemu-timer.o

 slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
 slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o
-- 
1.7.4




[Qemu-devel] [PATCH 2/3] build: Rename common-obj-y to softmmu-obj-y

2011-03-10 Thread Juan Quintela
It really represent object files shared between all softmmu targets.
We will use common-obj-y for objects shared between softmmu and
tools on next commit

Signed-off-by: Juan Quintela quint...@redhat.com
---
 Makefile|4 +-
 Makefile.objs   |  116 +++---
 Makefile.target |2 +-
 3 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/Makefile b/Makefile
index 9e090cb..7811d74 100644
--- a/Makefile
+++ b/Makefile
@@ -87,8 +87,8 @@ ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/Makefile.objs
 endif

-$(common-obj-y): $(GENERATED_HEADERS)
-$(filter %-softmmu,$(SUBDIR_RULES)): $(trace-obj-y) $(common-obj-y) 
subdir-libdis
+$(softmmu-obj-y): $(GENERATED_HEADERS)
+$(filter %-softmmu,$(SUBDIR_RULES)): $(trace-obj-y) $(softmmu-obj-y) 
subdir-libdis

 $(filter %-user,$(SUBDIR_RULES)): $(GENERATED_HEADERS) $(trace-obj-y) 
subdir-libdis-user subdir-libuser

diff --git a/Makefile.objs b/Makefile.objs
index 9e98a66..b525e81 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -59,54 +59,54 @@ fsdev-obj-$(CONFIG_VIRTFS) += $(addprefix fsdev/, 
$(fsdev-nested-y))
 # system emulation, i.e. a single QEMU executable should support all
 # CPUs and machines.

-common-obj-y = $(block-obj-y) blockdev.o
-common-obj-y += $(net-obj-y)
-common-obj-y += $(qobject-obj-y)
-common-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX))
-common-obj-y += readline.o console.o cursor.o async.o qemu-error.o
-common-obj-y += $(oslib-obj-y)
-common-obj-$(CONFIG_WIN32) += os-win32.o
-common-obj-$(CONFIG_POSIX) += os-posix.o
-
-common-obj-y += tcg-runtime.o host-utils.o
-common-obj-y += irq.o ioport.o input.o
-common-obj-$(CONFIG_PTIMER) += ptimer.o
-common-obj-$(CONFIG_MAX7310) += max7310.o
-common-obj-$(CONFIG_WM8750) += wm8750.o
-common-obj-$(CONFIG_TWL92230) += twl92230.o
-common-obj-$(CONFIG_TSC2005) += tsc2005.o
-common-obj-$(CONFIG_LM832X) += lm832x.o
-common-obj-$(CONFIG_TMP105) += tmp105.o
-common-obj-$(CONFIG_STELLARIS_INPUT) += stellaris_input.o
-common-obj-$(CONFIG_SSD0303) += ssd0303.o
-common-obj-$(CONFIG_SSD0323) += ssd0323.o
-common-obj-$(CONFIG_ADS7846) += ads7846.o
-common-obj-$(CONFIG_MAX111X) += max111x.o
-common-obj-$(CONFIG_DS1338) += ds1338.o
-common-obj-y += i2c.o smbus.o smbus_eeprom.o
-common-obj-y += eeprom93xx.o
-common-obj-y += scsi-disk.o cdrom.o
-common-obj-y += scsi-generic.o scsi-bus.o
-common-obj-y += usb.o usb-hub.o usb-$(HOST_USB).o usb-hid.o usb-msd.o 
usb-wacom.o
-common-obj-y += usb-serial.o usb-net.o usb-bus.o usb-desc.o
-common-obj-$(CONFIG_SSI) += ssi.o
-common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
-common-obj-$(CONFIG_SD) += sd.o
-common-obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o 
usb-bt.o
-common-obj-y += bt-hci-csr.o
-common-obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o
-common-obj-y += qemu-char.o savevm.o #aio.o
-common-obj-y += msmouse.o ps2.o
-common-obj-y += qdev.o qdev-properties.o
-common-obj-y += block-migration.o
-common-obj-y += pflib.o
-common-obj-y += bitmap.o bitops.o
-
-common-obj-$(CONFIG_BRLAPI) += baum.o
-common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
-common-obj-$(CONFIG_WIN32) += version.o
-
-common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o 
ui/spice-display.o spice-qemu-char.o
+softmmu-obj-y = $(block-obj-y) blockdev.o
+softmmu-obj-y += $(net-obj-y)
+softmmu-obj-y += $(qobject-obj-y)
+softmmu-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX))
+softmmu-obj-y += readline.o console.o cursor.o async.o qemu-error.o
+softmmu-obj-y += $(oslib-obj-y)
+softmmu-obj-$(CONFIG_WIN32) += os-win32.o
+softmmu-obj-$(CONFIG_POSIX) += os-posix.o
+
+softmmu-obj-y += tcg-runtime.o host-utils.o
+softmmu-obj-y += irq.o ioport.o input.o
+softmmu-obj-$(CONFIG_PTIMER) += ptimer.o
+softmmu-obj-$(CONFIG_MAX7310) += max7310.o
+softmmu-obj-$(CONFIG_WM8750) += wm8750.o
+softmmu-obj-$(CONFIG_TWL92230) += twl92230.o
+softmmu-obj-$(CONFIG_TSC2005) += tsc2005.o
+softmmu-obj-$(CONFIG_LM832X) += lm832x.o
+softmmu-obj-$(CONFIG_TMP105) += tmp105.o
+softmmu-obj-$(CONFIG_STELLARIS_INPUT) += stellaris_input.o
+softmmu-obj-$(CONFIG_SSD0303) += ssd0303.o
+softmmu-obj-$(CONFIG_SSD0323) += ssd0323.o
+softmmu-obj-$(CONFIG_ADS7846) += ads7846.o
+softmmu-obj-$(CONFIG_MAX111X) += max111x.o
+softmmu-obj-$(CONFIG_DS1338) += ds1338.o
+softmmu-obj-y += i2c.o smbus.o smbus_eeprom.o
+softmmu-obj-y += eeprom93xx.o
+softmmu-obj-y += scsi-disk.o cdrom.o
+softmmu-obj-y += scsi-generic.o scsi-bus.o
+softmmu-obj-y += usb.o usb-hub.o usb-$(HOST_USB).o usb-hid.o usb-msd.o 
usb-wacom.o
+softmmu-obj-y += usb-serial.o usb-net.o usb-bus.o usb-desc.o
+softmmu-obj-$(CONFIG_SSI) += ssi.o
+softmmu-obj-$(CONFIG_SSI_SD) += ssi-sd.o
+softmmu-obj-$(CONFIG_SD) += sd.o
+softmmu-obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o bt-hci.o 
bt-hid.o usb-bt.o
+softmmu-obj-y += bt-hci-csr.o
+softmmu-obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o
+softmmu-obj-y += 

Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Anthony Liguori

On 03/10/2011 06:39 AM, Avi Kivity wrote:
What I mean is that the client should specify the handle, like it does 
for everything else it gives a name (netdevs, blockdevs, SCM_RIGHT 
fds, etc).


  { execute: listen-event, arguments: { event: blah, id: blah1 } }
  { execute: unlisten-event arguments: { id: blah1 } }


Yeah, I understand, it just doesn't fit the model quite as well of 
signal accessors.


Normally, in a signal/slot event model, you'd have some notion of an 
object model and/or hierarchy.  For instance, with dbus, you'd do 
something like:


bus = dbus.SystemBus()
hal = # magic to get a hal object
device = hal.FindDeviceByCapability('media.storage')

device.connect_to_signal('media-changed', fn)

We don't have objects and I don't mean to introduce them, but I like the 
idea of treating signals as objects and returning them via an accessor 
function.


So the idea is that the handle is a marshalled form of the signal object.

{ 'BLOCK_IO_ERROR': { 'device': 'str', 'action': 'str', 'operation': 
'str' } }

[ 'get-block-io-error-event': {}, 'BLOCK_IO_ERROR' }

The way we marshal a 'BLOCK_IO_ERROR' type is by generating a unique 
handle and returning that.


I don't follow at all.  Where's the handle here?  Why don't we return 
the BLOCK_IO_ERROR as an object, on the wire?


How we marshal an object depends on the RPC layer.

We marshal events on the wire as an integer handle.  That is only a 
concept within the wire protocol.


We could just as easily return an object but without diving into JSON 
class hinting, it'd be pretty meaningless because we'd just return { 
'handle': 32} instead of 32.


While this looks like an int on the wire, at both the server and 
libqmp level, it looks like a BlockIoErrorEvent object.  So in QEMU:


BlockIoErrorEvent *qmp_get_block_io_error_event(Error **errp)
{
}

And in libqmp:

BlockIoErrorEvent *libqmp_get_block_io_error_event(QmpSession *sess, 
Error **errp)

{
}


What would the wire exchange look like?


 { 'execute': 'get-block-io-error-event' }
 { 'return' : 32 }
...
 { 'event': 'BLOCK_IO_ERROR', 'data': { 'action': 'stop', 'device': 
'ide0-hd0', 'operation': 'read' }, 'tag': 32 }

...
 { 'execute': 'put-event', 'arguments': { 'tag': 32 } }

Ignoring default events, you'll never see an event until you execute 
a signal accessor function.  When you execute this function, you will 
start receiving the events and those events will carry a tag 
containing the handle returned by the signal accessor.


A signal accessor is a command to start listening to a signal?


Yes, it basically enables the signal for the session.

So why not have the signal accessor provide the tag?  Like execute: 
blah provides a tag?


How would this map to a C API?  You'd either have to completely drop the 
notion of signal objects and use a separate mechanism to register 
callbacks against a tag (and lose type safety) or do some major munging 
to have the C API take a radically different signature than the wire 
protocol.




Within libqmp, any time you execute a signal accessor, a new signal 
object is created of the appropriate type.  When that object is 
destroyed, you send a put-event to stop receiving the signal.


When you connect to a signal object (via libqmp), you don't execute 
the signal accessor because the object is already receiving the signal.


Default events (which exist to preserve compatibility) are a set of 
events that are automatically connected to after qmp_capabilities is 
executed.  Because these connections are implicit, they arrive 
without a handle in the event object.


At this point, libqmp just ignores default events.  In the future, 
I'd like to add a command that can be executed before 
qmp_capabilities that will avoid connecting to default events.


I'm really confused.  Part of that is because the conversation mixes 
libqmp, server API, and wire protocol.  I'd like to understand the 
wire protocol first, everything else follows from that.


No, it's the opposite for me.  We design a good C API and then figure 
out how to make it work well as a wire protocol.  The whole point of 
this effort is to build an API that we can consume within QEMU such that 
we can start breaking large chunks of code out of the main executable.


Regards,

Anthony Liguori



[Qemu-devel] [PATCH] get rid of private bitmap functions in block/sheepdog.c, use generic ones

2011-03-10 Thread Michael Tokarev
qemu now has generic bitmap functions,
so don't redefine them in sheepdog.c,
use common header instead.  A small cleanup.

Here's only one function which is actually
used in sheepdog and gets replaced with
a generic one (simplified):

- static inline int test_bit(int nr, const volatile unsigned long *addr)
+ static inline int test_bit(int nr, const unsigned long *addr)
 {
-  return ((1UL  (nr % BITS_PER_LONG))
 ((unsigned long*)addr)[nr / BITS_PER_LONG])) != 0;
+  return 1UL  (addr[nr / BITS_PER_LONG]  (nr  (BITS_PER_LONG-1)));
 }

The body is equivalent, but the argument is not: there's
volatile in there.  Why it is used for - I'm not sure.

Signed-off-by: Michael Tokarev m...@tls.msk.ru

diff --git a/block/sheepdog.c b/block/sheepdog.c
index a54e0de..98946d7 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -13,6 +13,7 @@
 #include qemu-error.h
 #include qemu_socket.h
 #include block_int.h
+#include bitops.h
 
 #define SD_PROTO_VER 0x01
 
@@ -1829,20 +1830,6 @@ static int sd_snapshot_delete(BlockDriverState *bs, 
const char *snapshot_id)
 return 0;
 }
 
-#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
-#define BITS_PER_BYTE8
-#define BITS_TO_LONGS(nr)DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
-#define DECLARE_BITMAP(name,bits)   \
-unsigned long name[BITS_TO_LONGS(bits)]
-
-#define BITS_PER_LONG (BITS_PER_BYTE * sizeof(long))
-
-static inline int test_bit(unsigned int nr, const unsigned long *addr)
-{
-return ((1UL  (nr % BITS_PER_LONG)) 
-(((unsigned long *)addr)[nr / BITS_PER_LONG])) != 0;
-}
-
 static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
 {
 BDRVSheepdogState *s = bs-opaque;



[Qemu-devel] [PATCH] Consolidate DisplaySurface allocation in qemu_alloc_display()

2011-03-10 Thread Jes . Sorensen
From: Jes Sorensen jes.soren...@redhat.com

This removes various code duplication from console.e and sdl.c

Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
 console.c |   45 +
 console.h |3 +++
 ui/sdl.c  |   21 -
 3 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/console.c b/console.c
index 57d6eb5..4939a72 100644
--- a/console.c
+++ b/console.c
@@ -1278,35 +1278,40 @@ static DisplaySurface* 
defaultallocator_create_displaysurface(int width, int hei
 {
 DisplaySurface *surface = (DisplaySurface*) 
qemu_mallocz(sizeof(DisplaySurface));
 
-surface-width = width;
-surface-height = height;
-surface-linesize = width * 4;
-surface-pf = qemu_default_pixelformat(32);
-#ifdef HOST_WORDS_BIGENDIAN
-surface-flags = QEMU_ALLOCATED_FLAG | QEMU_BIG_ENDIAN_FLAG;
-#else
-surface-flags = QEMU_ALLOCATED_FLAG;
-#endif
-surface-data = (uint8_t*) qemu_mallocz(surface-linesize * 
surface-height);
-
+int linesize = width * 4;
+surface = qemu_alloc_display(surface, width, height, linesize,
+ qemu_default_pixelformat(32), 0);
 return surface;
 }
 
 static DisplaySurface* defaultallocator_resize_displaysurface(DisplaySurface 
*surface,
   int width, int height)
 {
+int linesize = width * 4;
+surface = qemu_alloc_display(surface, width, height, linesize,
+ qemu_default_pixelformat(32), 0);
+return surface;
+}
+
+DisplaySurface*
+qemu_alloc_display(DisplaySurface *surface, int width, int height,
+   int linesize, PixelFormat pf, int newflags)
+{
+void *data;
 surface-width = width;
 surface-height = height;
-surface-linesize = width * 4;
-surface-pf = qemu_default_pixelformat(32);
-if (surface-flags  QEMU_ALLOCATED_FLAG)
-surface-data = (uint8_t*) qemu_realloc(surface-data, 
surface-linesize * surface-height);
-else
-surface-data = (uint8_t*) qemu_malloc(surface-linesize * 
surface-height);
+surface-linesize = linesize;
+surface-pf = pf;
+if (surface-flags  QEMU_ALLOCATED_FLAG) {
+data = qemu_realloc(surface-data,
+surface-linesize * surface-height);
+} else {
+data = qemu_malloc(surface-linesize * surface-height);
+}
+surface-data = (uint8_t *)data;
+surface-flags = newflags | QEMU_ALLOCATED_FLAG;
 #ifdef HOST_WORDS_BIGENDIAN
-surface-flags = QEMU_ALLOCATED_FLAG | QEMU_BIG_ENDIAN_FLAG;
-#else
-surface-flags = QEMU_ALLOCATED_FLAG;
+surface-flags |= QEMU_BIG_ENDIAN_FLAG;
 #endif
 
 return surface;
diff --git a/console.h b/console.h
index f4e4741..dec9a76 100644
--- a/console.h
+++ b/console.h
@@ -189,6 +189,9 @@ void register_displaystate(DisplayState *ds);
 DisplayState *get_displaystate(void);
 DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp,
 int linesize, uint8_t *data);
+DisplaySurface* qemu_alloc_display(DisplaySurface *surface, int width,
+   int height, int linesize,
+   PixelFormat pf, int newflags);
 PixelFormat qemu_different_endianness_pixelformat(int bpp);
 PixelFormat qemu_default_pixelformat(int bpp);
 
diff --git a/ui/sdl.c b/ui/sdl.c
index 47ac49c..6c10ea6 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -176,23 +176,18 @@ static DisplaySurface* sdl_create_displaysurface(int 
width, int height)
 
 surface-width = width;
 surface-height = height;
-
+
 if (scaling_active) {
+int linesize;
+PixelFormat pf;
 if (host_format.BytesPerPixel != 2  host_format.BytesPerPixel != 4) {
-surface-linesize = width * 4;
-surface-pf = qemu_default_pixelformat(32);
+linesize = width * 4;
+pf = qemu_default_pixelformat(32);
 } else {
-surface-linesize = width * host_format.BytesPerPixel;
-surface-pf = sdl_to_qemu_pixelformat(host_format);
+linesize = width * host_format.BytesPerPixel;
+pf = sdl_to_qemu_pixelformat(host_format);
 }
-#ifdef HOST_WORDS_BIGENDIAN
-surface-flags = QEMU_ALLOCATED_FLAG | QEMU_BIG_ENDIAN_FLAG;
-#else
-surface-flags = QEMU_ALLOCATED_FLAG;
-#endif
-surface-data = (uint8_t*) qemu_mallocz(surface-linesize * 
surface-height);
-
-return surface;
+return qemu_alloc_display(surface, width, height, linesize, pf, 0);
 }
 
 if (host_format.BitsPerPixel == 16)
-- 
1.7.4




Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Avi Kivity

On 03/10/2011 04:12 PM, Anthony Liguori wrote:

On 03/10/2011 06:39 AM, Avi Kivity wrote:
What I mean is that the client should specify the handle, like it 
does for everything else it gives a name (netdevs, blockdevs, 
SCM_RIGHT fds, etc).


  { execute: listen-event, arguments: { event: blah, id: blah1 } }
  { execute: unlisten-event arguments: { id: blah1 } }


Yeah, I understand, it just doesn't fit the model quite as well of 
signal accessors.


Normally, in a signal/slot event model, you'd have some notion of an 
object model and/or hierarchy.  For instance, with dbus, you'd do 
something like:


bus = dbus.SystemBus()
hal = # magic to get a hal object
device = hal.FindDeviceByCapability('media.storage')

device.connect_to_signal('media-changed', fn)

We don't have objects and I don't mean to introduce them, but I like 
the idea of treating signals as objects and returning them via an 
accessor function.


So the idea is that the handle is a marshalled form of the signal object.


It's not a marshalled form, it's an opaque handle.  A marshalled form 
doesn't destroy information.


Anyway it would work with a client-provided tag just as well.  
connect_to_signal() could manufacture one and provide it to the server.




{ 'BLOCK_IO_ERROR': { 'device': 'str', 'action': 'str', 'operation': 
'str' } }

[ 'get-block-io-error-event': {}, 'BLOCK_IO_ERROR' }

The way we marshal a 'BLOCK_IO_ERROR' type is by generating a unique 
handle and returning that.


I don't follow at all.  Where's the handle here?  Why don't we return 
the BLOCK_IO_ERROR as an object, on the wire?


How we marshal an object depends on the RPC layer.

We marshal events on the wire as an integer handle.  That is only a 
concept within the wire protocol.


I don't think it's an accurate description.  We marshall an event as a 
json object according to the schema above (BLOCK_IO_ERROR).  How we 
marshall a subscription to an event, or an unsubscription to an event, 
depends on how we specify the protocol.  It has nothing to do with 
client or server internal rpc stubs.




We could just as easily return an object but without diving into JSON 
class hinting, it'd be pretty meaningless because we'd just return { 
'handle': 32} instead of 32.


Right, I suggest we return nothing at all.  Instead the client provides 
the handle.




While this looks like an int on the wire, at both the server and 
libqmp level, it looks like a BlockIoErrorEvent object.  So in QEMU:


BlockIoErrorEvent *qmp_get_block_io_error_event(Error **errp)
{
}

And in libqmp:

BlockIoErrorEvent *libqmp_get_block_io_error_event(QmpSession *sess, 
Error **errp)

{
}


What would the wire exchange look like?


 { 'execute': 'get-block-io-error-event' }
 { 'return' : 32 }
...
 { 'event': 'BLOCK_IO_ERROR', 'data': { 'action': 'stop', 'device': 
'ide0-hd0', 'operation': 'read' }, 'tag': 32 }

...
 { 'execute': 'put-event', 'arguments': { 'tag': 32 } }


Well, I may be biased, but I prefer my variant.

btw, it's good to decree that a subscription is immediately followed by 
an event with the current state (which means events have to provide 
state and be idempotent); so the subscribe-and-query pattern is provided 
automatically.


btw2, I now nominate subscribe and unsubscribe as replacements for get 
and put.




Ignoring default events, you'll never see an event until you execute 
a signal accessor function.  When you execute this function, you 
will start receiving the events and those events will carry a tag 
containing the handle returned by the signal accessor.


A signal accessor is a command to start listening to a signal?


Yes, it basically enables the signal for the session.


Okay, the subscription command.



So why not have the signal accessor provide the tag?  Like execute: 
blah provides a tag?


How would this map to a C API?  You'd either have to completely drop 
the notion of signal objects and use a separate mechanism to register 
callbacks against a tag (and lose type safety) or do some major 
munging to have the C API take a radically different signature than 
the wire protocol.


A C API could create and maintain tags under the covers (an int that 
keeps increasing would do).






Within libqmp, any time you execute a signal accessor, a new signal 
object is created of the appropriate type.  When that object is 
destroyed, you send a put-event to stop receiving the signal.


When you connect to a signal object (via libqmp), you don't execute 
the signal accessor because the object is already receiving the signal.


Default events (which exist to preserve compatibility) are a set of 
events that are automatically connected to after qmp_capabilities is 
executed.  Because these connections are implicit, they arrive 
without a handle in the event object.


At this point, libqmp just ignores default events.  In the future, 
I'd like to add a command that can be executed before 
qmp_capabilities that will avoid connecting to default events.


I'm 

[Qemu-devel] [PATCH 30/32] vmstate: port syborg_keyboard

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/syborg_keyboard.c |   57 +++---
 1 files changed, 17 insertions(+), 40 deletions(-)

diff --git a/hw/syborg_keyboard.c b/hw/syborg_keyboard.c
index d295e99..706a039 100644
--- a/hw/syborg_keyboard.c
+++ b/hw/syborg_keyboard.c
@@ -51,11 +51,11 @@ enum {

 typedef struct {
 SysBusDevice busdev;
-int int_enabled;
+uint32_t int_enabled;
 int extension_bit;
 uint32_t fifo_size;
 uint32_t *key_fifo;
-int read_pos, read_count;
+uint32_t read_pos, read_count;
 qemu_irq irq;
 } SyborgKeyboardState;

@@ -165,43 +165,21 @@ static void syborg_keyboard_event(void *opaque, int 
keycode)
 syborg_keyboard_update(s);
 }

-static void syborg_keyboard_save(QEMUFile *f, void *opaque)
-{
-SyborgKeyboardState *s = (SyborgKeyboardState *)opaque;
-int i;
-
-qemu_put_be32(f, s-fifo_size);
-qemu_put_be32(f, s-int_enabled);
-qemu_put_be32(f, s-extension_bit);
-qemu_put_be32(f, s-read_pos);
-qemu_put_be32(f, s-read_count);
-for (i = 0; i  s-fifo_size; i++) {
-qemu_put_be32(f, s-key_fifo[i]);
-}
-}
-
-static int syborg_keyboard_load(QEMUFile *f, void *opaque, int version_id)
-{
-SyborgKeyboardState *s = (SyborgKeyboardState *)opaque;
-uint32_t val;
-int i;
-
-if (version_id != 1)
-return -EINVAL;
-
-val = qemu_get_be32(f);
-if (val != s-fifo_size)
-return -EINVAL;
-
-s-int_enabled = qemu_get_be32(f);
-s-extension_bit = qemu_get_be32(f);
-s-read_pos = qemu_get_be32(f);
-s-read_count = qemu_get_be32(f);
-for (i = 0; i  s-fifo_size; i++) {
-s-key_fifo[i] = qemu_get_be32(f);
+static const VMStateDescription vmstate_syborg_keyboard = {
+.name = syborg_keyboard,
+.version_id = 1,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT32_EQUAL(fifo_size, SyborgKeyboardState),
+VMSTATE_UINT32(int_enabled, SyborgKeyboardState),
+VMSTATE_UINT32(read_pos, SyborgKeyboardState),
+VMSTATE_UINT32(read_count, SyborgKeyboardState),
+VMSTATE_VARRAY_UINT32(key_fifo, SyborgKeyboardState, fifo_size, 1,
+  vmstate_info_uint32, uint32),
+VMSTATE_END_OF_LIST()
 }
-return 0;
-}
+};

 static int syborg_keyboard_init(SysBusDevice *dev)
 {
@@ -221,8 +199,7 @@ static int syborg_keyboard_init(SysBusDevice *dev)

 qemu_add_kbd_event_handler(syborg_keyboard_event, s);

-register_savevm(dev-qdev, syborg_keyboard, -1, 1,
-syborg_keyboard_save, syborg_keyboard_load, s);
+vmstate_register(dev-qdev, -1, vmstate_syborg_keyboard, s);
 return 0;
 }

-- 
1.7.4




[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jan Kiszka
On 2011-03-10 12:53, Paolo Bonzini wrote:
 On 03/10/2011 12:46 PM, Jan Kiszka wrote:
 Better define flash chips as qdev devices and make the attributes qdev
 properties:

  -device flash,image=...,base=...,overlay=...,overlay_start=...

 Images should be addressed by block device IDs and created via '-drive'
 (likely requires a new interface type 'flash').
 
 if=none will do.

Yeah, of course (already used for other host interfaces, if!=none is for
legacy use only).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH 06/13] nand: pin values are uint8_t

2011-03-10 Thread Juan Quintela
Signed-off-by: Juan Quintela quint...@redhat.com
---
 hw/flash.h |4 ++--
 hw/nand.c  |6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/flash.h b/hw/flash.h
index d7d103e..c22e1a9 100644
--- a/hw/flash.h
+++ b/hw/flash.h
@@ -21,8 +21,8 @@ pflash_t *pflash_cfi02_register(target_phys_addr_t base, 
ram_addr_t off,
 typedef struct NANDFlashState NANDFlashState;
 NANDFlashState *nand_init(int manf_id, int chip_id);
 void nand_done(NANDFlashState *s);
-void nand_setpins(NANDFlashState *s,
-int cle, int ale, int ce, int wp, int gnd);
+void nand_setpins(NANDFlashState *s, uint8_t cle, uint8_t ale,
+  uint8_t ce, uint8_t wp, uint8_t gnd);
 void nand_getpins(NANDFlashState *s, int *rb);
 void nand_setio(NANDFlashState *s, uint8_t value);
 uint8_t nand_getio(NANDFlashState *s);
diff --git a/hw/nand.c b/hw/nand.c
index f414aa1..9f978d8 100644
--- a/hw/nand.c
+++ b/hw/nand.c
@@ -52,7 +52,7 @@ struct NANDFlashState {
 BlockDriverState *bdrv;
 int mem_oob;

-int cle, ale, ce, wp, gnd;
+uint8_t cle, ale, ce, wp, gnd;

 uint8_t io[MAX_PAGE + MAX_OOB + 0x400];
 uint8_t *ioaddr;
@@ -329,8 +329,8 @@ static int nand_load(QEMUFile *f, void *opaque, int 
version_id)
  *
  * CE, WP and R/B are active low.
  */
-void nand_setpins(NANDFlashState *s,
-int cle, int ale, int ce, int wp, int gnd)
+void nand_setpins(NANDFlashState *s, uint8_t cle, uint8_t ale,
+  uint8_t ce, uint8_t wp, uint8_t gnd)
 {
 s-cle = cle;
 s-ale = ale;
-- 
1.7.4




[Qemu-devel] [PATCH v5] vnc: don't mess up with iohandlers in the vnc thread

2011-03-10 Thread Corentin Chary
The threaded VNC servers messed up with QEMU fd handlers without
any kind of locking, and that can cause some nasty race conditions.

Using qemu_mutex_lock_iothread() won't work because vnc_dpy_cpy(),
which will wait for the current job queue to finish, can be called with
the iothread lock held.

Instead, we now store the data in a temporary buffer, and use a bottom
half to notify the main thread that new data is available.

vnc_[un]lock_ouput() is still needed to access VncState members like
abort, csock or jobs_buffer.

Signed-off-by: Corentin Chary corentin.ch...@gmail.com
---
 ui/vnc-jobs-async.c |   48 +---
 ui/vnc-jobs.h   |1 +
 ui/vnc.c|   12 
 ui/vnc.h|2 ++
 4 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c
index f596247..4438776 100644
--- a/ui/vnc-jobs-async.c
+++ b/ui/vnc-jobs-async.c
@@ -28,6 +28,7 @@
 
 #include vnc.h
 #include vnc-jobs.h
+#include qemu_socket.h
 
 /*
  * Locking:
@@ -155,6 +156,24 @@ void vnc_jobs_join(VncState *vs)
 qemu_cond_wait(queue-cond, queue-mutex);
 }
 vnc_unlock_queue(queue);
+vnc_jobs_consume_buffer(vs);
+}
+
+void vnc_jobs_consume_buffer(VncState *vs)
+{
+bool flush;
+
+vnc_lock_output(vs);
+if (vs-jobs_buffer.offset) {
+vnc_write(vs, vs-jobs_buffer.buffer, vs-jobs_buffer.offset);
+buffer_reset(vs-jobs_buffer);
+}
+flush = vs-csock != -1  vs-abort != true;
+vnc_unlock_output(vs);
+
+if (flush) {
+  vnc_flush(vs);
+}
 }
 
 /*
@@ -197,7 +216,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 VncState vs;
 int n_rectangles;
 int saved_offset;
-bool flush;
 
 vnc_lock_queue(queue);
 while (QTAILQ_EMPTY(queue-jobs)  !queue-exit) {
@@ -213,6 +231,7 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 
 vnc_lock_output(job-vs);
 if (job-vs-csock == -1 || job-vs-abort == true) {
+vnc_unlock_output(job-vs);
 goto disconnected;
 }
 vnc_unlock_output(job-vs);
@@ -233,10 +252,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 
 if (job-vs-csock == -1) {
 vnc_unlock_display(job-vs-vd);
-/* output mutex must be locked before going to
- * disconnected:
- */
-vnc_lock_output(job-vs);
 goto disconnected;
 }
 
@@ -254,24 +269,19 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 vs.output.buffer[saved_offset] = (n_rectangles  8)  0xFF;
 vs.output.buffer[saved_offset + 1] = n_rectangles  0xFF;
 
-/* Switch back buffers */
 vnc_lock_output(job-vs);
-if (job-vs-csock == -1) {
-goto disconnected;
+if (job-vs-csock != -1) {
+buffer_reserve(job-vs-jobs_buffer, vs.output.offset);
+buffer_append(job-vs-jobs_buffer, vs.output.buffer,
+  vs.output.offset);
+/* Copy persistent encoding data */
+vnc_async_encoding_end(job-vs, vs);
+
+   qemu_bh_schedule(job-vs-bh);
 }
-
-vnc_write(job-vs, vs.output.buffer, vs.output.offset);
-
-disconnected:
-/* Copy persistent encoding data */
-vnc_async_encoding_end(job-vs, vs);
-flush = (job-vs-csock != -1  job-vs-abort != true);
 vnc_unlock_output(job-vs);
 
-if (flush) {
-vnc_flush(job-vs);
-}
-
+disconnected:
 vnc_lock_queue(queue);
 QTAILQ_REMOVE(queue-jobs, job, next);
 vnc_unlock_queue(queue);
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index b8dab81..4c661f9 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -40,6 +40,7 @@ void vnc_jobs_join(VncState *vs);
 
 #ifdef CONFIG_VNC_THREAD
 
+void vnc_jobs_consume_buffer(VncState *vs);
 void vnc_start_worker_thread(void);
 bool vnc_worker_thread_running(void);
 void vnc_stop_worker_thread(void);
diff --git a/ui/vnc.c b/ui/vnc.c
index 610f884..f28873b 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1011,7 +1011,10 @@ static void vnc_disconnect_finish(VncState *vs)
 
 #ifdef CONFIG_VNC_THREAD
 qemu_mutex_destroy(vs-output_mutex);
+qemu_bh_delete(vs-bh);
+buffer_free(vs-jobs_buffer);
 #endif
+
 for (i = 0; i  VNC_STAT_ROWS; ++i) {
 qemu_free(vs-lossy_rect[i]);
 }
@@ -1226,6 +1229,14 @@ static long vnc_client_read_plain(VncState *vs)
 return ret;
 }
 
+#ifdef CONFIG_VNC_THREAD
+static void vnc_jobs_bh(void *opaque)
+{
+VncState *vs = opaque;
+
+vnc_jobs_consume_buffer(vs);
+}
+#endif
 
 /*
  * First function called whenever there is more data to be read from
@@ -2525,6 +2536,7 @@ static void vnc_connect(VncDisplay *vd, int csock)
 
 #ifdef CONFIG_VNC_THREAD
 qemu_mutex_init(vs-output_mutex);
+vs-bh = qemu_bh_new(vnc_jobs_bh, vs);
 #endif
 
 QTAILQ_INSERT_HEAD(vd-clients, vs, next);
diff --git a/ui/vnc.h b/ui/vnc.h
index 8a1e7b9..bca5f87 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -283,6 +283,8 @@ struct VncState
 VncJob job;
 #else
 

Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Avi Kivity

On 03/10/2011 04:24 PM, Avi Kivity wrote:

What would the wire exchange look like?


 { 'execute': 'get-block-io-error-event' }
 { 'return' : 32 }
...
 { 'event': 'BLOCK_IO_ERROR', 'data': { 'action': 'stop', 'device': 
'ide0-hd0', 'operation': 'read' }, 'tag': 32 }

...
 { 'execute': 'put-event', 'arguments': { 'tag': 32 } }



Well, I may be biased, but I prefer my variant.

btw, it's good to decree that a subscription is immediately followed 
by an event with the current state (which means events have to provide 
state and be idempotent); so the subscribe-and-query pattern is 
provided automatically.


btw2, I now nominate subscribe and unsubscribe as replacements for get 
and put.


I also think it should be at the protocol layer:

 { execute: some-command, id: foo, arguments: { ... } }
 { result: { ... }, id: foo }
 { subscribe: block-io-error, id: bar, arguments: { ... } }
 { result: { ... } id: bar }
 { event: block-io-error, id: bar, data : { ... } }
 { unsubscribe: block-io-error, id: bar }
 { result: { ... } id: bar }

So events are now protocol-level pieces like commands, and the use of 
tags is uniform.


--
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Anthony Liguori

On 03/10/2011 08:24 AM, Avi Kivity wrote:

On 03/10/2011 04:12 PM, Anthony Liguori wrote:

On 03/10/2011 06:39 AM, Avi Kivity wrote:
What I mean is that the client should specify the handle, like it 
does for everything else it gives a name (netdevs, blockdevs, 
SCM_RIGHT fds, etc).


  { execute: listen-event, arguments: { event: blah, id: blah1 } }
  { execute: unlisten-event arguments: { id: blah1 } }


Yeah, I understand, it just doesn't fit the model quite as well of 
signal accessors.


Normally, in a signal/slot event model, you'd have some notion of an 
object model and/or hierarchy.  For instance, with dbus, you'd do 
something like:


bus = dbus.SystemBus()
hal = # magic to get a hal object
device = hal.FindDeviceByCapability('media.storage')

device.connect_to_signal('media-changed', fn)

We don't have objects and I don't mean to introduce them, but I like 
the idea of treating signals as objects and returning them via an 
accessor function.


So the idea is that the handle is a marshalled form of the signal 
object.


It's not a marshalled form, it's an opaque handle.  A marshalled form 
doesn't destroy information.


Anyway it would work with a client-provided tag just as well.  
connect_to_signal() could manufacture one and provide it to the server.


It's all just bits, right? :-)

We pretty much need to keep the QEMU signature the same.  That would 
mean an internal signature of:


BlockIoErrorEvent *qmp_connect_block_io_error_event(Error **errp)
{
}

So the marshal function would then need to do something like:

void qmp_marshal_connect_block_io_error_event(QmpState *state, QDict 
*args, QObject **ret_data, Error **errp)

{
  BlockIoErrorEvent *ev;
  QObject *tag = qdict_get_obj(args, tag);

  ev = qmp_connect_block_io_error_event(errp);
  qmp_state_add_connection(state, ev-signal, tag);
}

That's not too bad, but would the schema be:

[ 'connect-block-io-error-event', {}, 'BLOCK_IO_ERROR' ]

Or would it be:

[ 'connect-block-io-error-event', { 'tag': 'variant' }, 'none' ]

I'm really opposed to adding a variant type to the schema.  I'm also not 
a big fan of there not being a 1-1 relationship to the wire protocol and 
the C API.


I think it's easy to rationalize 'this is how events are marshalled' vs. 
'for events, a totally different declaration is generated'.


{ 'BLOCK_IO_ERROR': { 'device': 'str', 'action': 'str', 
'operation': 'str' } }

[ 'get-block-io-error-event': {}, 'BLOCK_IO_ERROR' }

The way we marshal a 'BLOCK_IO_ERROR' type is by generating a 
unique handle and returning that.


I don't follow at all.  Where's the handle here?  Why don't we 
return the BLOCK_IO_ERROR as an object, on the wire?


How we marshal an object depends on the RPC layer.

We marshal events on the wire as an integer handle.  That is only a 
concept within the wire protocol.


I don't think it's an accurate description.  We marshall an event as a 
json object according to the schema above (BLOCK_IO_ERROR).  How we 
marshall a subscription to an event, or an unsubscription to an event, 
depends on how we specify the protocol.


It's not really a subscription to an event.  It is an event.

Maybe signal accessor is the wrong word.  Maybe signal factory conveys a 
better notion of what it is.




 { 'execute': 'get-block-io-error-event' }
 { 'return' : 32 }
...
 { 'event': 'BLOCK_IO_ERROR', 'data': { 'action': 'stop', 'device': 
'ide0-hd0', 'operation': 'read' }, 'tag': 32 }

...
 { 'execute': 'put-event', 'arguments': { 'tag': 32 } }


Well, I may be biased, but I prefer my variant.

btw, it's good to decree that a subscription is immediately followed 
by an event with the current state (which means events have to provide 
state and be idempotent); so the subscribe-and-query pattern is 
provided automatically.


Not all events are updates of data.  BLOCK_IO_ERROR is a great example 
of this.  There is no useful information that can be sent after a 
connection.


btw2, I now nominate subscribe and unsubscribe as replacements for get 
and put.


Subscribe implies sub/pub in my mind and we're not publishing events so 
I don't think it fits the model.


A pub/sub event model would be interesting to think through but without 
a global namespace and object model, I don't think we can make it fit well.


That's why I'm using signal/slots.  It's much more conducive to a 
procedural model.
I'm really confused.  Part of that is because the conversation mixes 
libqmp, server API, and wire protocol.  I'd like to understand the wire 
protocol first, everything else follows from that.


No, it's the opposite for me.  We design a good C API and then figure 
out how to make it work well as a wire protocol.  The whole point of 
this effort is to build an API that we can consume within QEMU such 
that we can start breaking large chunks of code out of the main 
executable.


That makes a C centric wire protocol.  Clients don't have to be C.


But a C client is by far the most important of all 

Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Anthony Liguori

On 03/10/2011 09:30 AM, Avi Kivity wrote:

On 03/10/2011 04:24 PM, Avi Kivity wrote:

What would the wire exchange look like?


 { 'execute': 'get-block-io-error-event' }
 { 'return' : 32 }
...
 { 'event': 'BLOCK_IO_ERROR', 'data': { 'action': 'stop', 'device': 
'ide0-hd0', 'operation': 'read' }, 'tag': 32 }

...
 { 'execute': 'put-event', 'arguments': { 'tag': 32 } }



Well, I may be biased, but I prefer my variant.

btw, it's good to decree that a subscription is immediately followed 
by an event with the current state (which means events have to 
provide state and be idempotent); so the subscribe-and-query pattern 
is provided automatically.


btw2, I now nominate subscribe and unsubscribe as replacements for 
get and put.


I also think it should be at the protocol layer:

 { execute: some-command, id: foo, arguments: { ... } }
 { result: { ... }, id: foo }
 { subscribe: block-io-error, id: bar, arguments: { ... } }
 { result: { ... } id: bar }
 { event: block-io-error, id: bar, data : { ... } }
 { unsubscribe: block-io-error, id: bar }
 { result: { ... } id: bar }

So events are now protocol-level pieces like commands, and the use of 
tags is uniform.


Maybe for QMPv2, but for QMPv1, this is going to introduce an extremely 
incompatible change.


Actually, we missed the json-rpc boat in designing QMP.  It should look 
like:


 { method: some-command, id: foo, params: { ... } }
 { result: { ... }, id: foo, params: { ... }, error: null }
 { method: connect-block-io-error, id: bar, params: { ... } }
 { result: { ... }, id: bar, error: null }
 { method: block-io-error, id: null, params: { ... } }

Keys are different and null is passed instead of not including a tag.  
Events are sent exactly like methods but id is null.  A result is never 
sent to the server for an event.


One of the good things about using a code generator and IDL though is 
that we can add a -qmp dev,protocol=json-rpc that encodes 
appropriately.  We can probably do this translation at the QObject level 
really.  But in the future, we can also add -qmp dev,protocol=xml-rpc, 
-qmp dev,protocol=rest, or -qmp dev,protocol=asn1-rpc


Regards,

Anthony Liguori





Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Avi Kivity

On 03/10/2011 05:33 PM, Anthony Liguori wrote:


We pretty much need to keep the QEMU signature the same.  That would 
mean an internal signature of:


BlockIoErrorEvent *qmp_connect_block_io_error_event(Error **errp)
{
}

So the marshal function would then need to do something like:

void qmp_marshal_connect_block_io_error_event(QmpState *state, QDict 
*args, QObject **ret_data, Error **errp)

{
  BlockIoErrorEvent *ev;
  QObject *tag = qdict_get_obj(args, tag);

  ev = qmp_connect_block_io_error_event(errp);
  qmp_state_add_connection(state, ev-signal, tag);
}


That can be mashed around however we like.



That's not too bad, but would the schema be:

[ 'connect-block-io-error-event', {}, 'BLOCK_IO_ERROR' ]

Or would it be:

[ 'connect-block-io-error-event', { 'tag': 'variant' }, 'none' ]

I'm really opposed to adding a variant type to the schema.  I'm also 
not a big fan of there not being a 1-1 relationship to the wire 
protocol and the C API.


I think it's easy to rationalize 'this is how events are marshalled' 
vs. 'for events, a totally different declaration is generated'.


Under my latest proposal it wouldn't be in the schema at all (like 
command tags are not in the schema) since it's a protocol-level entity.


I don't think it's an accurate description.  We marshall an event as 
a json object according to the schema above (BLOCK_IO_ERROR).  How we 
marshall a subscription to an event, or an unsubscription to an 
event, depends on how we specify the protocol.


It's not really a subscription to an event.  It is an event.


No, the event happens (potentially) multiple times.  Or zero.  You don't 
get the event, you ask qemu to notify you.




Maybe signal accessor is the wrong word.  Maybe signal factory conveys 
a better notion of what it is.


It's even more confusing to me.





 { 'execute': 'get-block-io-error-event' }
 { 'return' : 32 }
...
 { 'event': 'BLOCK_IO_ERROR', 'data': { 'action': 'stop', 'device': 
'ide0-hd0', 'operation': 'read' }, 'tag': 32 }

...
 { 'execute': 'put-event', 'arguments': { 'tag': 32 } }


Well, I may be biased, but I prefer my variant.

btw, it's good to decree that a subscription is immediately followed 
by an event with the current state (which means events have to 
provide state and be idempotent); so the subscribe-and-query pattern 
is provided automatically.


Not all events are updates of data.  BLOCK_IO_ERROR is a great example 
of this.  There is no useful information that can be sent after a 
connection.


You could say the blockdev's state is fine, or it has encountered an error.

How do you detect if there's an error if you've crashed (you=client in 
this case)?




btw2, I now nominate subscribe and unsubscribe as replacements for 
get and put.


Subscribe implies sub/pub in my mind and we're not publishing events 
so I don't think it fits the model.


A pub/sub event model would be interesting to think through but 
without a global namespace and object model, I don't think we can make 
it fit well.


I feel we're still not communicating.  What does 'get-*-event' mean?

I think you're using some nomenclature that is unfamiliar to me.

That's why I'm using signal/slots.  It's much more conducive to a 
procedural model.


I still don't follow.  We have a connection, over which we ask the other 
side to let us know when something happens, then that other side lets us 
know when it happens, then we ask it to stop, then it stops.  There are 
no signals or slots anywhere.  If there are in the code, let's not mix 
it up with the protocol.



That makes a C centric wire protocol.  Clients don't have to be C.


But a C client is by far the most important of all clients--QEMU.  If 
we use QMP extensively internally, then we guarantee that the API is 
expressive and robust.


No, internally we have the most scope to fix mistakes.



If we build the API only for third-party clients, we end up with 
pretty much what we have today.  An API with good intentions but 
that's more or less impossible to use in practice.


Or we have something that's nice for C but hard to use or inconsistent 
with whatever language a management client is written in.


--
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Avi Kivity

On 03/10/2011 05:41 PM, Anthony Liguori wrote:

I also think it should be at the protocol layer:

 { execute: some-command, id: foo, arguments: { ... } }
 { result: { ... }, id: foo }
 { subscribe: block-io-error, id: bar, arguments: { ... } }
 { result: { ... } id: bar }
 { event: block-io-error, id: bar, data : { ... } }
 { unsubscribe: block-io-error, id: bar }
 { result: { ... } id: bar }

So events are now protocol-level pieces like commands, and the use of 
tags is uniform.



Maybe for QMPv2, but for QMPv1, this is going to introduce an 
extremely incompatible change.


Why?  It's 100% backwards compatible.



Actually, we missed the json-rpc boat in designing QMP.  It should 
look like:




IIRC I looked at it and it didn't impress me.


 { method: some-command, id: foo, params: { ... } }
 { result: { ... }, id: foo, params: { ... }, error: null }
 { method: connect-block-io-error, id: bar, params: { ... } }
 { result: { ... }, id: bar, error: null }
 { method: block-io-error, id: null, params: { ... } }





Keys are different and null is passed instead of not including a tag.  
Events are sent exactly like methods but id is null.  A result is 
never sent to the server for an event.


That means that we need a per-event command, instead of making it 
protocol-level.




One of the good things about using a code generator and IDL though is 
that we can add a -qmp dev,protocol=json-rpc that encodes 
appropriately.  We can probably do this translation at the QObject 
level really.  But in the future, we can also add -qmp 
dev,protocol=xml-rpc, -qmp dev,protocol=rest, or -qmp 
dev,protocol=asn1-rpc




Let's get one protocol that works first.

--
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Anthony Liguori

On 03/10/2011 09:45 AM, Avi Kivity wrote:


btw2, I now nominate subscribe and unsubscribe as replacements for 
get and put.


Subscribe implies sub/pub in my mind and we're not publishing events 
so I don't think it fits the model.


A pub/sub event model would be interesting to think through but 
without a global namespace and object model, I don't think we can 
make it fit well.


I feel we're still not communicating.  What does 'get-*-event' mean?

I think you're using some nomenclature that is unfamiliar to me.


No, I'm just defending something that I think fundamentally sucks.

I very purposefully am trying to avoid heavy protocol visible changes at 
this stage.  The only reason I added signal accessors is that the 
current event model is unusable from a C API.


I am in full agreement that the current signal model needs to be 
rethought and should be changed at the protocol level.  I just don't 
want to do that right now because there are a ton of internal 
improvements that can be made by without doing that.


The signal accessors are ugly but they're just a handful of commands 
that can be deprecated over time.  We should revisit events but we 
should take the time to design it and plan for a protocol addition for 0.16.




That's why I'm using signal/slots.  It's much more conducive to a 
procedural model.


I still don't follow.  We have a connection, over which we ask the 
other side to let us know when something happens, then that other side 
lets us know when it happens, then we ask it to stop, then it stops.  
There are no signals or slots anywhere.  If there are in the code, 
let's not mix it up with the protocol.


Dropping my legacy baggage, here's what I'd like to see us do from a 
protocol perspective.  I'd like to first introduce class hinting and 
switch all of the string handles that we use today to class handles.   So:


{ execute: query-block }
{ return: [ { __jsonclass__: 'BlockDevice', id=ide0-hd0 }, { 
__jsonclass__: 'BlockDevice', id=ide1-cd0 } ] }


{ execute: connect, arguments: { 'obj': { __jsonclass__: BlockDevice, 
id=ide0-hd0 }, 'signal': 'io-error' } }

{ return: { __jsonclass__: Connection, id=1 } }

{ signal: 'io-error', connection: { __jsonclass__: Connection, id=1 }, 
arguments: { action='stop', ... } }


{ execute: disconnect, arguments: { 'connection': { __jsonclass__: 
Connection, id=1 } } }

{ return: null }

The advantages here are many.  You get much stronger typing in C.  If 
the schema is done right, it trivially maps to an object model in Python.


Regards,

Anthony Liguori


That makes a C centric wire protocol.  Clients don't have to be C.


But a C client is by far the most important of all clients--QEMU.  If 
we use QMP extensively internally, then we guarantee that the API is 
expressive and robust.


No, internally we have the most scope to fix mistakes.



If we build the API only for third-party clients, we end up with 
pretty much what we have today.  An API with good intentions but 
that's more or less impossible to use in practice.


Or we have something that's nice for C but hard to use or inconsistent 
with whatever language a management client is written in.







Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

2011-03-10 Thread Anthony Liguori

On 03/10/2011 09:49 AM, Avi Kivity wrote:

On 03/10/2011 05:41 PM, Anthony Liguori wrote:

I also think it should be at the protocol layer:

 { execute: some-command, id: foo, arguments: { ... } }
 { result: { ... }, id: foo }
 { subscribe: block-io-error, id: bar, arguments: { ... } }
 { result: { ... } id: bar }
 { event: block-io-error, id: bar, data : { ... } }
 { unsubscribe: block-io-error, id: bar }
 { result: { ... } id: bar }

So events are now protocol-level pieces like commands, and the use 
of tags is uniform.



Maybe for QMPv2, but for QMPv1, this is going to introduce an 
extremely incompatible change.


Why?  It's 100% backwards compatible.


It's a very significant change for clients.  While technical compatible, 
it would require a change to the client infrastructure to support the 
new feature.


I'm not saying we shouldn't make a change like this, but we should 
minimize these type of changes.


Regards,

Anthony Liguori




[Qemu-devel] [PATCH] target-arm: Fix UNDEF cases in Thumb load/store

2011-03-10 Thread Peter Maydell
Decode of Thumb load/store was merging together the cases of 'bit 11==0'
(reg+reg LSL imm) and 'bit 11==1' (reg+imm). This happens to work for
valid instruction patterns but meant that we would not UNDEF for the
cases the architecture mandates that we must. Make the decode actually
look at bit 11 as well as [10..8] so that we UNDEF in the right places.

This change also removes what was a spurious unreachable 'case 8',
and correctly frees TCG temporaries on the illegal-insn codepaths.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
This patch was mostly prompted by that dodgy 'case 8' which I noted
when doing the preload/hint space patches a month or so ago; I have
finally added support for testing loads and stores to risu, so I
can confirm that this patch doesn't break the non-UNDEF cases.

 target-arm/translate.c |   29 ++---
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 062de5e..0afdbfb 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8378,39 +8378,42 @@ static int disas_thumb2_insn(CPUState *env, 
DisasContext *s, uint16_t insn_hw1)
 tcg_gen_addi_i32(addr, addr, imm);
 } else {
 imm = insn  0xff;
-switch ((insn  8)  7) {
-case 0: case 8: /* Shifted Register.  */
+switch ((insn  8)  0xf) {
+case 0x0: /* Shifted Register.  */
 shift = (insn  4)  0xf;
-if (shift  3)
+if (shift  3) {
+tcg_temp_free_i32(addr);
 goto illegal_op;
+}
 tmp = load_reg(s, rm);
 if (shift)
 tcg_gen_shli_i32(tmp, tmp, shift);
 tcg_gen_add_i32(addr, addr, tmp);
 tcg_temp_free_i32(tmp);
 break;
-case 4: /* Negative offset.  */
+case 0xc: /* Negative offset.  */
 tcg_gen_addi_i32(addr, addr, -imm);
 break;
-case 6: /* User privilege.  */
+case 0xe: /* User privilege.  */
 tcg_gen_addi_i32(addr, addr, imm);
 user = 1;
 break;
-case 1: /* Post-decrement.  */
+case 0x9: /* Post-decrement.  */
 imm = -imm;
 /* Fall through.  */
-case 3: /* Post-increment.  */
+case 0xb: /* Post-increment.  */
 postinc = 1;
 writeback = 1;
 break;
-case 5: /* Pre-decrement.  */
+case 0xd: /* Pre-decrement.  */
 imm = -imm;
 /* Fall through.  */
-case 7: /* Pre-increment.  */
+case 0xf: /* Pre-increment.  */
 tcg_gen_addi_i32(addr, addr, imm);
 writeback = 1;
 break;
 default:
+tcg_temp_free_i32(addr);
 goto illegal_op;
 }
 }
@@ -8423,7 +8426,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext 
*s, uint16_t insn_hw1)
 case 1: tmp = gen_ld16u(addr, user); break;
 case 5: tmp = gen_ld16s(addr, user); break;
 case 2: tmp = gen_ld32(addr, user); break;
-default: goto illegal_op;
+default:
+tcg_temp_free_i32(addr);
+goto illegal_op;
 }
 if (rs == 15) {
 gen_bx(s, tmp);
@@ -8437,7 +8442,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext 
*s, uint16_t insn_hw1)
 case 0: gen_st8(tmp, addr, user); break;
 case 1: gen_st16(tmp, addr, user); break;
 case 2: gen_st32(tmp, addr, user); break;
-default: goto illegal_op;
+default:
+tcg_temp_free_i32(addr);
+goto illegal_op;
 }
 }
 if (postinc)
-- 
1.7.1




[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 01:10, Avi Kivity a...@redhat.com wrote:
 On 03/10/2011 06:51 AM, Jordan Justen wrote:

 http://wiki.qemu.org/Features/System_Flash


 - make the programming interface the same as an existing device

How strongly do you feel about this?

For one thing, real devices are not as flexible as QEMU for flash
sizes.  QEMU allows for any 64kb multiple bios size.  Real world
devices generally only support powers of 2 sizes.

Firmware hub devices are somewhat simplistic to emulate, but I think
they use 16MB of address space, while only providing = 1MB of flash
storage.

SPI devices are available in many sizes, so it might be possible to
choose a 16MB device to emulate.  But, it would be a lot more complex
to emulate as it would it involve emulating an SPI contoller + the
device.

I thought this might be a case where deviation from real hardware
emulation could better serve the VM's needs.

-Jordan



[Qemu-devel] [PATCH] target-arm: Fix GE bits for v6media signed modulo arithmetic

2011-03-10 Thread Peter Maydell
Fix the signed modulo arithmetic helpers for the v6media
instructions (SADD8, SSUB8, SADD16, SSUB16, SASX, SSAX) to set
the GE bits correctly (based on the result of the add or subtract
before it is truncated to 16 bits, not after).

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
 target-arm/helper.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index d360121..4f2b440 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2171,7 +2171,7 @@ static inline uint8_t sub8_usat(uint8_t a, uint8_t b)
 /* Signed modulo arithmetic.  */
 #define SARITH16(a, b, n, op) do { \
 int32_t sum; \
-sum = (int16_t)((uint16_t)(a) op (uint16_t)(b)); \
+sum = (int32_t)(int16_t)(a) op (int32_t)(int16_t)(b); \
 RESULT(sum, n, 16); \
 if (sum = 0) \
 ge |= 3  (n * 2); \
@@ -2179,7 +2179,7 @@ static inline uint8_t sub8_usat(uint8_t a, uint8_t b)
 
 #define SARITH8(a, b, n, op) do { \
 int32_t sum; \
-sum = (int8_t)((uint8_t)(a) op (uint8_t)(b)); \
+sum = (int32_t)(int8_t)(a) op (int32_t)(int8_t)(b); \
 RESULT(sum, n, 8); \
 if (sum = 0) \
 ge |= 1  n; \
-- 
1.7.1




[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 01:47, Gleb Natapov g...@redhat.com wrote:
 Two things. First You suggest to replace -bios with -flash. This will
 make firmware upgrade painful process that will have to be performed
 from inside the guest since the same flash image will contain both
 firmware and whatever data was stored on a flash which presumably you
 want to reuse after upgrading a firmware.

Yes, this definitely could add firmware upgrade issues, but I thought
this could be the responsibility of the firmware itself.  For example,
OVMF could have an outside of VM tool to merge new releases, or it
could have an inside of VM firmware update process.

 My suggestion is to extend
 -bios option like this:

 -bios bios.bin,flash=flash.bin,flash_base=addr

 flash.bin will be mapped at address flash_base, or, if flash_base is not
 present, just below bios.bin.

I did not intend to replace -bios.  I intended to override -bios
usage.  So, if -flash is not used, then it would operate as today.  If
-flash is used, then it would override the -bios file.

So, for the firmware update issues mentioned above, it would not
impact, say SeaBIOS...

 Second. I asked how flash is programmed because interfaces like CFI
 where you write into flash memory address range to issue commands cannot
 be emulated efficiently in KVM. KVM supports either regular memory slots
 or IO memory, but in your proposal the same memory behaves as IO on
 write and regular memory on read. Better idea would be to present
 non-volatile flash as ISA virtio device. Should be simple to implement.

I would be concerned about performance for KVM.  In my proposal, all
reads would probably have to be treated as MMIO, since reads are
involved in the programming sequences.

If the flash device was 1MB, and it was read entirely via MMIO
trapping would there be a significant performance hit on KVM?  If so,
I think I will have to consider a hybrid approach like you mentioned
above, where most of the firmware is mapped as memory (copied from
bios.bin), while a small amount of memory below this is available as
flash.

But, in real systems, accessing the flash memory is significantly
slower than RAM, so the real question is, how bad would the
performance be?

Thanks,

-Jordan



[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 03:46, Jan Kiszka jan.kis...@siemens.com wrote:
 On 2011-03-10 12:27, Jan Kiszka wrote:
 On 2011-03-10 10:47, Gleb Natapov wrote:
 My suggestion is to extend
 -bios option like this:

 -bios bios.bin,flash=flash.bin,flash_base=addr

 flash.bin will be mapped at address flash_base, or, if flash_base is not
 present, just below bios.bin.

 ...or define -flash in a way that allows mapping the bios image as an
 overlay to the otherwise guest-managed flash image.

 Better define flash chips as qdev devices and make the attributes qdev
 properties:

    -device flash,image=...,base=...,overlay=...,overlay_start=...

I was hoping it would not necessarily require a script to run OVMF. :)

The original proposal would have allowed for:

qemu -L . -flash ovmf.fd

-Jordan



[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 04:27, Jan Kiszka jan.kis...@siemens.com wrote:
 On 2011-03-10 13:17, Gleb Natapov wrote:
 So flash will be always IO and overlay will be always ROM. This will

 Yes, and once we have KVM support for read-RAM/write-IO slots, flash
 will be able to switch between ROM and IO mode just like it already does
 under TCG.

Interesting.  Will this allow the read-RAM to be converted to read-IO
if a write-IO triggers it?  And, then reverted to read-RAM again...

Thanks,

-Jordan



[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Gleb Natapov
On Thu, Mar 10, 2011 at 10:59:07AM -0800, Jordan Justen wrote:
 On Thu, Mar 10, 2011 at 01:47, Gleb Natapov g...@redhat.com wrote:
  Two things. First You suggest to replace -bios with -flash. This will
  make firmware upgrade painful process that will have to be performed
  from inside the guest since the same flash image will contain both
  firmware and whatever data was stored on a flash which presumably you
  want to reuse after upgrading a firmware.
 
 Yes, this definitely could add firmware upgrade issues, but I thought
 this could be the responsibility of the firmware itself.  For example,
 OVMF could have an outside of VM tool to merge new releases, or it
 could have an inside of VM firmware update process.
Why require another tool if can do without? I don't see any advantages
in storing firmware code and its non-volatile storage in the same image,
but I do see disadvantages.

 
  My suggestion is to extend
  -bios option like this:
 
  -bios bios.bin,flash=flash.bin,flash_base=addr
 
  flash.bin will be mapped at address flash_base, or, if flash_base is not
  present, just below bios.bin.
 
 I did not intend to replace -bios.  I intended to override -bios
 usage.  So, if -flash is not used, then it would operate as today.  If
 -flash is used, then it would override the -bios file.
 
 So, for the firmware update issues mentioned above, it would not
 impact, say SeaBIOS...
 
OVMF is not different from SeaBIOS as far as KVM/qemu is concerned. SeaBIOS
want to have non-volatile storage too.

  Second. I asked how flash is programmed because interfaces like CFI
  where you write into flash memory address range to issue commands cannot
  be emulated efficiently in KVM. KVM supports either regular memory slots
  or IO memory, but in your proposal the same memory behaves as IO on
  write and regular memory on read. Better idea would be to present
  non-volatile flash as ISA virtio device. Should be simple to implement.
 
 I would be concerned about performance for KVM.  In my proposal, all
 reads would probably have to be treated as MMIO, since reads are
 involved in the programming sequences.
 
 If the flash device was 1MB, and it was read entirely via MMIO
 trapping would there be a significant performance hit on KVM?  If so,
 I think I will have to consider a hybrid approach like you mentioned
 above, where most of the firmware is mapped as memory (copied from
 bios.bin), while a small amount of memory below this is available as
 flash.
 
It is not even about performance (which will be very bad for 1MB). KVM
can't run code from MMIO region, so the part that contains firmware
has to be memory.

 But, in real systems, accessing the flash memory is significantly
 slower than RAM, so the real question is, how bad would the
 performance be?
 
 Thanks,
 
 -Jordan

--
Gleb.



[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Gleb Natapov
On Thu, Mar 10, 2011 at 11:08:32AM -0800, Jordan Justen wrote:
 On Thu, Mar 10, 2011 at 04:27, Jan Kiszka jan.kis...@siemens.com wrote:
  On 2011-03-10 13:17, Gleb Natapov wrote:
  So flash will be always IO and overlay will be always ROM. This will
 
  Yes, and once we have KVM support for read-RAM/write-IO slots, flash
  will be able to switch between ROM and IO mode just like it already does
  under TCG.
 
 Interesting.  Will this allow the read-RAM to be converted to read-IO
 if a write-IO triggers it?  And, then reverted to read-RAM again...
 
Yes.

--
Gleb.



Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Anthony Liguori

On 03/10/2011 01:03 PM, Jordan Justen wrote:

On Thu, Mar 10, 2011 at 03:46, Jan Kiszkajan.kis...@siemens.com  wrote:

On 2011-03-10 12:27, Jan Kiszka wrote:

On 2011-03-10 10:47, Gleb Natapov wrote:

My suggestion is to extend
-bios option like this:

-bios bios.bin,flash=flash.bin,flash_base=addr

flash.bin will be mapped at address flash_base, or, if flash_base is not
present, just below bios.bin.

...or define -flash in a way that allows mapping the bios image as an
overlay to the otherwise guest-managed flash image.

Better define flash chips as qdev devices and make the attributes qdev
properties:

-device flash,image=...,base=...,overlay=...,overlay_start=...

I was hoping it would not necessarily require a script to run OVMF. :)

The original proposal would have allowed for:

qemu -L . -flash ovmf.fd


If you implement a CSM for Tiano Core, then you won't need to use any 
special parameters because we can just use OVMF by default ;-)


Regards,

Anthony Liguori


-Jordan






[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 11:12, Gleb Natapov g...@redhat.com wrote:
 On Thu, Mar 10, 2011 at 10:59:07AM -0800, Jordan Justen wrote:
 Yes, this definitely could add firmware upgrade issues, but I thought
 this could be the responsibility of the firmware itself.  For example,
 OVMF could have an outside of VM tool to merge new releases, or it
 could have an inside of VM firmware update process.
 Why require another tool if can do without? I don't see any advantages
 in storing firmware code and its non-volatile storage in the same image,
 but I do see disadvantages.

I agree.  The implications of a firmware image getting out of sync
with qemu were a bit of a concern to me.  But, having both -bios +
-flash just below -bios was starting to seem a bit complicated.

And, I guess as a firmware developer, I thought it might be
interesting to consider enabling a firmware update process within the
VM. :)

How about?
1) Pure rom:
-bios bios.bin
2) Rom + flash below rom:
-bios bios.bin,flash=flash.bin
3) Pure flash:
-bios flash=flash.bin

Or, with a separate new -flash option:
1) Pure rom:
-bios bios.bin
or no -bios or -flash parameters
2) Rom + flash below rom:
-bios bios.bin -flash flash.bin
3) Pure flash:
-flash flash.bin

 It is not even about performance (which will be very bad for 1MB). KVM
 can't run code from MMIO region, so the part that contains firmware
 has to be memory.

Hmm.  That's good to know. :)

So, perhaps this feature should build upon the other feature you and
Jan are discussing.  When will it become available?

Thanks,

-Jordan



Re: [Qemu-devel] Issue with snapshot outside qcow2 disk - qemu 0.14.0

2011-03-10 Thread SAURAV LAHIRI
Thank you Stefan and Jes for providing further inputs.

Details on use case:

The high level use case is that of being able to backup user specified disks of 
a VM without having to bring down the VM. 

That was the reason that I had started of with running the qemu-img snapshot 
-c snap1... on a running vm. So when I have a snapshot i can back this up. 
Without a frozen and consistent snapshot, backing up the qcow2 disk image would 
cause serious issues on restore. But after your inputs shelved the plans of 
creating snapshot on running vm.

So next approach planned did take into account that  I have to shutdown the VM 
for creating the snapshot. It would appear that, since the internal snapshot 
creation is an instantaneous process I could bring down the vm, create an 
internal snapshot, bring up the VM. and this entire thing would take a minimal 
time. 

Once the VM is up I can continue with converting the internal snapshot to an 
external snapshot and then back up the external snapshot to my backup location. 
The duration of conversion from internal to external obviously would depend on 
the size of the original qcow2 disk. But since the VM is up the duration would 
not concern me much as long as it completes within some practical time limits.


snapshot_blkdev: Regarding this  I do have a couple of questions.

1. If the snapshot cannot be merged then it could mean that there are several 
snapshot files. One readonly  for each of the previous snapshots and the last 
one being the active one, which handles all the current writes. Post backup If 
do have to restore to a particular snapshot then i would probably have to copy 
all the files in the chain and maintain the entire chain. But would it not 
affect read performance if several snapshot files are maintained, particularly 
if the VM is hosting a database like mysql ? Could you please clarify.

2. I have seen that at times the qemu monitor command is not able to connnect 
to  the monitor socket as libvirt it seems controls the monitor socket. If I 
shutdown libvirt then commands like socat is able to connect. But since my 
current environment does use libvirt, shutting down libvirt is not an option. 
Is there any way around this ?

Appreciate your help.

Regards
sl










--- On Thu, 10/3/11, Jes Sorensen jes.soren...@redhat.com wrote:

From: Jes Sorensen jes.soren...@redhat.com
Subject: Re: [Qemu-devel] Issue with snapshot outside qcow2 disk - qemu 0.14.0
To: Stefan Hajnoczi stefa...@gmail.com
Cc: SAURAV LAHIRI saurav_lah...@yahoo.com, qemu-devel@nongnu.org
Date: Thursday, 10 March, 2011, 5:59

On 03/10/11 10:58, Stefan Hajnoczi wrote:
 On Thu, Mar 10, 2011 at 9:32 AM, Jes Sorensen jes.soren...@redhat.com wrote:
 On 03/10/11 10:27, Stefan Hajnoczi wrote:
 I have CCed Jes who has been working on a live snapshot mechanism.  He
 recently added the snapshot_blkdev monitor command that takes a
 snapshot of a block device while the VM is running.  A new image file
 is created based off the original image file (which will no longer be
 modified), all new disk writes go to the new image file.  It is safe
 to perform read-only access to the original image file.  There
 currently is no support to merge the snapshot changes back into the
 original image while the VM is running, but I think that is the next
 planned step.

 Yes, keep in mind that the live snapshot is only for external snapshot
 files, it doesn't deal with internal snapshots.
 
 Yep, that's why I'm interested in Saurav's use case.  Many use cases
 work with either internal or external snapshot but it depends on the
 details.

Actually I think there's very little reason to keep internal snapshot
support. It doesn't buy us much, but it adds unnecessary complexity.

Cheers,
Jes




  

Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 11:23, Anthony Liguori anth...@codemonkey.ws wrote:
 If you implement a CSM for Tiano Core, then you won't need to use any
 special parameters because we can just use OVMF by default ;-)

Sorry, but I can't do this.  This is unlikely to change anytime soon.

But, if someone seeks to put forth a serious effort around a CSM (or
most anything UEFI or edk2 related), then they ought to be able to
expect support from our tianocore community (which includes me).

If our tianocore community had an open source CSM available, I should
be able to include it in OVMF.  A BSD licensed CSM would be much
easier for OVMF to integrate directly, but any open-source CSM would
allow for the possibility of an OVMF fork with the CSM included.

-Jordan



Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Антон Кочков
As I'm working on bootrom loading support for omap/arm platform, I'm
have suggestion about something more universal than -bios (and even
-flash) option. Because Flash can be NOR, can be NAND, but on-chip
memory is not flash memory. So may be something like -rom option?

Best regards,
Anton Kochkov.




On Thu, Mar 10, 2011 at 22:50, Jordan Justen jljus...@gmail.com wrote:
 On Thu, Mar 10, 2011 at 11:12, Gleb Natapov g...@redhat.com wrote:
 On Thu, Mar 10, 2011 at 10:59:07AM -0800, Jordan Justen wrote:
 Yes, this definitely could add firmware upgrade issues, but I thought
 this could be the responsibility of the firmware itself.  For example,
 OVMF could have an outside of VM tool to merge new releases, or it
 could have an inside of VM firmware update process.
 Why require another tool if can do without? I don't see any advantages
 in storing firmware code and its non-volatile storage in the same image,
 but I do see disadvantages.

 I agree.  The implications of a firmware image getting out of sync
 with qemu were a bit of a concern to me.  But, having both -bios +
 -flash just below -bios was starting to seem a bit complicated.

 And, I guess as a firmware developer, I thought it might be
 interesting to consider enabling a firmware update process within the
 VM. :)

 How about?
 1) Pure rom:
 -bios bios.bin
 2) Rom + flash below rom:
 -bios bios.bin,flash=flash.bin
 3) Pure flash:
 -bios flash=flash.bin

 Or, with a separate new -flash option:
 1) Pure rom:
 -bios bios.bin
 or no -bios or -flash parameters
 2) Rom + flash below rom:
 -bios bios.bin -flash flash.bin
 3) Pure flash:
 -flash flash.bin

 It is not even about performance (which will be very bad for 1MB). KVM
 can't run code from MMIO region, so the part that contains firmware
 has to be memory.

 Hmm.  That's good to know. :)

 So, perhaps this feature should build upon the other feature you and
 Jan are discussing.  When will it become available?

 Thanks,

 -Jordan





[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Gleb Natapov
On Thu, Mar 10, 2011 at 11:50:42AM -0800, Jordan Justen wrote:
  It is not even about performance (which will be very bad for 1MB). KVM
  can't run code from MMIO region, so the part that contains firmware
  has to be memory.
 
 Hmm.  That's good to know. :)
 
 So, perhaps this feature should build upon the other feature you and
 Jan are discussing.  When will it become available?
 
When somebody will be motivated enough to send patches. If this was the
only thing that stands in a way of QEMU having EFI firmware with CSM
support it would have happened much faster though.

--
Gleb.



[Qemu-devel] [PATCH] Fix performance regression in qemu_get_ram_ptr

2011-03-10 Thread Vincent Palatin
When the commit f471a17e9d869df3c6573f7ec02c4725676d6f3a converted the
ram_blocks structure to QLIST, it also removed the conditional check before
switching the current block at the beginning of the list.

In the common use case where ram_blocks has a few blocks with only one
frequently accessed (the main RAM), this has a performance impact as it
performs the useless list operations on each call (which are on a really
hot path).

On my machine emulation (ARM on amd64), this patch reduces the
percentage of CPU time spent in qemu_get_ram_ptr from 6.3% to 2.1% in the
profiling of a full boot.

Signed-off-by: Vincent Palatin vpala...@chromium.org
---
 exec.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index d611100..81f08b7 100644
--- a/exec.c
+++ b/exec.c
@@ -2957,8 +2957,11 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
 
 QLIST_FOREACH(block, ram_list.blocks, next) {
 if (addr - block-offset  block-length) {
-QLIST_REMOVE(block, next);
-QLIST_INSERT_HEAD(ram_list.blocks, block, next);
+/* Move this entry to to start of the list.  */
+if (block != QLIST_FIRST(ram_list.blocks)) {
+QLIST_REMOVE(block, next);
+QLIST_INSERT_HEAD(ram_list.blocks, block, next);
+}
 return block-host + (addr - block-offset);
 }
 }
-- 
1.7.3.1




Re: [Qemu-devel] Issue with snapshot outside qcow2 disk - qemu 0.14.0

2011-03-10 Thread Stefan Hajnoczi
On Thu, Mar 10, 2011 at 7:57 PM, SAURAV LAHIRI saurav_lah...@yahoo.com wrote:
 The high level use case is that of being able to backup user specified disks 
 of a VM without having to bring down the VM.

Excellent, that sounds exactly like Jes is addressing so future
QEMU/KVM releases will hopefully have the live snapshot/merge
capability.

 snapshot_blkdev: Regarding this  I do have a couple of questions.

 1. If the snapshot cannot be merged then it could mean that there are several 
 snapshot files. One readonly  for each of the previous snapshots and the last 
 one being the active one, which handles all the current writes. Post backup 
 If do have to restore to a particular snapshot then i would probably have to 
 copy all the files in the chain and maintain the entire chain. But would it 
 not affect read performance if several snapshot files are maintained, 
 particularly if the VM is hosting a database like mysql ? Could you please 
 clarify.

If the VM is not running you can use the qemu-img commit command to
merge the snapshot back down into the base image.  After that you only
have one image file again and can restart the VM.  Hopefully the
deltas are small enough that this process is quick.

In the future a live merge command will take care of this and avoid
the downtime.

 2. I have seen that at times the qemu monitor command is not able to connnect 
 to  the monitor socket as libvirt it seems controls the monitor socket. If I 
 shutdown libvirt then commands like socat is able to connect. But since my 
 current environment does use libvirt, shutting down libvirt is not an option. 
 Is there any way around this ?

New versions of libvirt have a virsh qemu-monitor-command command
you can use to send a QEMU monitor command.

Stefan



[Qemu-devel] Re: [PATCH] Fix performance regression in qemu_get_ram_ptr

2011-03-10 Thread Alex Williamson
On Thu, 2011-03-10 at 15:47 -0500, Vincent Palatin wrote:
 When the commit f471a17e9d869df3c6573f7ec02c4725676d6f3a converted the
 ram_blocks structure to QLIST, it also removed the conditional check before
 switching the current block at the beginning of the list.
 
 In the common use case where ram_blocks has a few blocks with only one
 frequently accessed (the main RAM), this has a performance impact as it
 performs the useless list operations on each call (which are on a really
 hot path).
 
 On my machine emulation (ARM on amd64), this patch reduces the
 percentage of CPU time spent in qemu_get_ram_ptr from 6.3% to 2.1% in the
 profiling of a full boot.
 
 Signed-off-by: Vincent Palatin vpala...@chromium.org
 ---
  exec.c |7 +--
  1 files changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/exec.c b/exec.c
 index d611100..81f08b7 100644
 --- a/exec.c
 +++ b/exec.c
 @@ -2957,8 +2957,11 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
  
  QLIST_FOREACH(block, ram_list.blocks, next) {
  if (addr - block-offset  block-length) {
 -QLIST_REMOVE(block, next);
 -QLIST_INSERT_HEAD(ram_list.blocks, block, next);
 +/* Move this entry to to start of the list.  */
 +if (block != QLIST_FIRST(ram_list.blocks)) {
 +QLIST_REMOVE(block, next);
 +QLIST_INSERT_HEAD(ram_list.blocks, block, next);
 +}
  return block-host + (addr - block-offset);
  }
  }

Looks good

Acked-by: Alex Williamson alex.william...@redhat.com




Re: [Qemu-devel] RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Hi,

as the lead developer of the open source flashrom utility
http://www.flashrom.org/ I have to say that it would be nice to have
Qemu emulate a flash chip. Right now flashrom is using its own flash
chip emulator for testing, and being able to use flashrom in Qemu would
be a nice addition.

Auf 10.03.2011 05:51, Jordan Justen schrieb:
 I have documented a simple flash-like device which I think could be
 useful for qemu/kvm in some cases.  (Particularly for allowing
 persistent UEFI non-volatile variables.)

 http://wiki.qemu.org/Features/System_Flash

 Let me know if you have any suggestions or concerns.
   

Is there any reason why you chose to invent an interface for the flash
chip which is more complicated than the interface used by common flash
chips out there?
Maybe some EFI requirement?

Regards,
Carl-Daniel



Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Auf 10.03.2011 12:48, Gleb Natapov schrieb:
 On Thu, Mar 10, 2011 at 12:27:55PM +0100, Jan Kiszka wrote:
   
 On 2011-03-10 10:47, Gleb Natapov wrote:
 
 Second. I asked how flash is programmed because interfaces like CFI
 where you write into flash memory address range to issue commands cannot
 be emulated efficiently in KVM. KVM supports either regular memory slots
 or IO memory, but in your proposal the same memory behaves as IO on
 write and regular memory on read. Better idea would be to present
 non-volatile flash as ISA virtio device. Should be simple to implement.
   
 Why not enhancing KVM memory slots to support direct read access while
 writes are trapped and forwarded to a user space device model?
 
 Yes we can make memory slot that will be treated as memory on read and
 IO on write, but first relying on that will prevent using flash interface
 on older kernels and second it is not enough to implement the proposal.
 When magic value is written into an address, the address become IO for
 reading too, but KVM slot granularity is page, not byte, so KVM will
 have to remove the slot to make it IO, but KVM can't execute code from
 IO region (yet), so we will not be able to run firmware from flash and
 simultaneously write into the flash.
   

If you have a Parallel/LPC/FWH flash chip in your mainboard, it is
technically impossible to execute code from flash while you are writing
to _any_ part of the flash chip because every single read from the flash
chip during an active write will toggle one bit of the read data.
So if your code already runs on real x86, you will never hit that problem.

(SPI flash is an exception, but it uses out-of-band access anyway,
usually via some southbridge interface, and that means the IO vs.
execution conflict won't happen there either.)

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/




Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Auf 10.03.2011 13:06, Jan Kiszka schrieb:
 BTW, the programming granularity is not bytes but chips with common CFI.
 But that's still tricky if you want to run code from the same chip while
 updating parts of it. The easiest workaround would be handling the
 overlay regions as ROM all the time. Not accurate but realizable without
 kernel changes.
   

I've yet to see CFI chips on x86.


 On Thu, Mar 10, 2011 at 12:27:55PM +0100, Jan Kiszka wrote:
 
 Virtio
 means that you have to patch the guest (which might be something else
 than flexible Linux...).

   
 This intended to be used by firmware only and we control that.
 
 I'm thinking beyond this use case, beyond firmware flashes, beyond x86.
   

If you're thinking beyond x86, most flash is probably using SPI nowadays
because the reduced PCB footprint can save lots of money. And for SPI
you only need OOB access for write and the memory region itself is readonly.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/




Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Auf 10.03.2011 19:43, Jordan Justen schrieb:
 On Thu, Mar 10, 2011 at 01:10, Avi Kivity a...@redhat.com wrote:
   
 On 03/10/2011 06:51 AM, Jordan Justen wrote:
 
 http://wiki.qemu.org/Features/System_Flash

   
 - make the programming interface the same as an existing device
 
 How strongly do you feel about this?

 For one thing, real devices are not as flexible as QEMU for flash
 sizes.  QEMU allows for any 64kb multiple bios size.  Real world
 devices generally only support powers of 2 sizes.

 Firmware hub devices are somewhat simplistic to emulate, but I think
 they use 16MB of address space, while only providing = 1MB of flash
 storage.
   

Up to 4 MB on real hardware, and if you use Parallel flash devices,
there is no limit at all (except cost). The software interface is
identical for read/write/erase/probe.


 SPI devices are available in many sizes, so it might be possible to
 choose a 16MB device to emulate.  But, it would be a lot more complex
 to emulate as it would it involve emulating an SPI contoller + the
 device.
   

I have written a SPI flash chip emulator (it emulates 3 different
real-world SPI flash chips) and am willing to contribute it to Qemu if
there is interest. The code is pretty small, and adding a SPI host
controller emulator should be a few lines of code extra. Not a big problem.


 I thought this might be a case where deviation from real hardware
 emulation could better serve the VM's needs.
   

If we have to write the code anyway, and if it can work just fine with
current KVM/Qemu, is there a reason not to use the same interface as
real hardware?

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/




[Qemu-devel] Re: [PATCH] Fix performance regression in qemu_get_ram_ptr

2011-03-10 Thread Chris Wright
* Vincent Palatin (vpala...@chromium.org) wrote:
 When the commit f471a17e9d869df3c6573f7ec02c4725676d6f3a converted the
 ram_blocks structure to QLIST, it also removed the conditional check before
 switching the current block at the beginning of the list.

Nice catch.

 In the common use case where ram_blocks has a few blocks with only one
 frequently accessed (the main RAM), this has a performance impact as it
 performs the useless list operations on each call (which are on a really
 hot path).
 
 On my machine emulation (ARM on amd64), this patch reduces the
 percentage of CPU time spent in qemu_get_ram_ptr from 6.3% to 2.1% in the
 profiling of a full boot.

Hopefully this is back on par with before the QLIST switchover.

 Signed-off-by: Vincent Palatin vpala...@chromium.org

Acked-by: Chris Wright chr...@redhat.com

 ---
  exec.c |7 +--
  1 files changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/exec.c b/exec.c
 index d611100..81f08b7 100644
 --- a/exec.c
 +++ b/exec.c
 @@ -2957,8 +2957,11 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
  
  QLIST_FOREACH(block, ram_list.blocks, next) {
  if (addr - block-offset  block-length) {
 -QLIST_REMOVE(block, next);
 -QLIST_INSERT_HEAD(ram_list.blocks, block, next);
 +/* Move this entry to to start of the list.  */
 +if (block != QLIST_FIRST(ram_list.blocks)) {
 +QLIST_REMOVE(block, next);
 +QLIST_INSERT_HEAD(ram_list.blocks, block, next);
 +}

Pretty close to self-documenting code now.  Not sure if it's subtle enough
to warrant change to the comment like:
 
  /* Move block to head of list if it's not there already */

thanks,
-chris



Re: [Qemu-devel] RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 13:37, Carl-Daniel Hailfinger
c-d.hailfinger.devel.2...@gmx.net wrote:
 Auf 10.03.2011 05:51, Jordan Justen schrieb:
 I have documented a simple flash-like device which I think could be
 useful for qemu/kvm in some cases.  (Particularly for allowing
 persistent UEFI non-volatile variables.)

 http://wiki.qemu.org/Features/System_Flash

 Let me know if you have any suggestions or concerns.


 Is there any reason why you chose to invent an interface for the flash
 chip which is more complicated than the interface used by common flash
 chips out there?
 Maybe some EFI requirement?

This is a simpler version than the flash devices I'm used to dealing
with for x86/x86-64 UEFI systems.  Can you suggest an alternative real
interface that is simpler?

Thanks,

-Jordan



Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 13:41, Carl-Daniel Hailfinger
c-d.hailfinger.devel.2...@gmx.net wrote:
 Auf 10.03.2011 12:48, Gleb Natapov schrieb:
 Yes we can make memory slot that will be treated as memory on read and
 IO on write, but first relying on that will prevent using flash interface
 on older kernels and second it is not enough to implement the proposal.
 When magic value is written into an address, the address become IO for
 reading too, but KVM slot granularity is page, not byte, so KVM will
 have to remove the slot to make it IO, but KVM can't execute code from
 IO region (yet), so we will not be able to run firmware from flash and
 simultaneously write into the flash.


 If you have a Parallel/LPC/FWH flash chip in your mainboard, it is
 technically impossible to execute code from flash while you are writing
 to _any_ part of the flash chip because every single read from the flash
 chip during an active write will toggle one bit of the read data.
 So if your code already runs on real x86, you will never hit that problem.

 (SPI flash is an exception, but it uses out-of-band access anyway,
 usually via some southbridge interface, and that means the IO vs.
 execution conflict won't happen there either.)

I've not seen a firmware that attempts to update flash while also
executing from flash.  For OVMF or UEFI, I don't would not think this
should be a requirement.  (Although, my proposal would support this.)

-Jordan



Re: [Qemu-devel] RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Auf 10.03.2011 22:55, Jordan Justen schrieb:
 On Thu, Mar 10, 2011 at 13:37, Carl-Daniel Hailfinger
 c-d.hailfinger.devel.2...@gmx.net wrote:
   
 Auf 10.03.2011 05:51, Jordan Justen schrieb:
 
 I have documented a simple flash-like device which I think could be
 useful for qemu/kvm in some cases.  (Particularly for allowing
 persistent UEFI non-volatile variables.)

 http://wiki.qemu.org/Features/System_Flash

 Let me know if you have any suggestions or concerns.

   
 Is there any reason why you chose to invent an interface for the flash
 chip which is more complicated than the interface used by common flash
 chips out there?
 Maybe some EFI requirement?
 
 This is a simpler version than the flash devices I'm used to dealing
 with for x86/x86-64 UEFI systems.  Can you suggest an alternative real
 interface that is simpler?
   

Pseudocode for the real interface most common on x86 before SPI came along:

Write a page (256 bytes) or a few bytes:
chip_writeb(0xAA, bios_base + 0x);
chip_writeb(0x55, bios_base + 0x2AAA);
chip_writeb(0xA0, bios_base + 0x);
chip_writeb(databyte0, bios_base + addr);
chip_writeb(databyte1, bios_base + addr + 1);
chip_writeb(databyte2, bios_base + addr + 2);
chip_writeb(databyte3, bios_base + addr + 3);
... up to 256 databytes.
chip_readb(bios_base);
The last chip_readb(bios_base) is repeated until the result does not
change anymore.

For me, that looks pretty simple and straightforward, especially
compared to other more exotic flash chip interfaces.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/




Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Scott Wood
On Thu, 10 Mar 2011 22:46:34 +0100
Carl-Daniel Hailfinger c-d.hailfinger.devel.2...@gmx.net wrote:

 Auf 10.03.2011 13:06, Jan Kiszka schrieb:
  I'm thinking beyond this use case, beyond firmware flashes, beyond x86.

 
 If you're thinking beyond x86, most flash is probably using SPI nowadays
 because the reduced PCB footprint can save lots of money. And for SPI
 you only need OOB access for write and the memory region itself is readonly.

CFI still seems pretty dominant, at least in evaluation boards, which seem
more likely to be a qemu target than custom hardware.

-Scott




Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 13:52, Carl-Daniel Hailfinger
c-d.hailfinger.devel.2...@gmx.net wrote:
 Auf 10.03.2011 19:43, Jordan Justen schrieb:
 I thought this might be a case where deviation from real hardware
 emulation could better serve the VM's needs.


 If we have to write the code anyway, and if it can work just fine with
 current KVM/Qemu, is there a reason not to use the same interface as
 real hardware?

If there was a real device emulated by qemu, I would gladly make use
of it in OVMF.

I just thought this proposal would be much easier to implement, and
not be restricted to any particular size.  (Since -bios currently
supports any size that is a multiple of 64kb.)  A real device is going
to imply a certain size.

-Jordan



Re: [Qemu-devel] RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 14:10, Carl-Daniel Hailfinger
c-d.hailfinger.devel.2...@gmx.net wrote:
 Auf 10.03.2011 22:55, Jordan Justen schrieb:
 On Thu, Mar 10, 2011 at 13:37, Carl-Daniel Hailfinger
 c-d.hailfinger.devel.2...@gmx.net wrote:
 Is there any reason why you chose to invent an interface for the flash
 chip which is more complicated than the interface used by common flash
 chips out there?
 Maybe some EFI requirement?

 This is a simpler version than the flash devices I'm used to dealing
 with for x86/x86-64 UEFI systems.  Can you suggest an alternative real
 interface that is simpler?


 Pseudocode for the real interface most common on x86 before SPI came along:

 Write a page (256 bytes) or a few bytes:
 chip_writeb(0xAA, bios_base + 0x);
 chip_writeb(0x55, bios_base + 0x2AAA);
 chip_writeb(0xA0, bios_base + 0x);
 chip_writeb(databyte0, bios_base + addr);
 chip_writeb(databyte1, bios_base + addr + 1);
 chip_writeb(databyte2, bios_base + addr + 2);
 chip_writeb(databyte3, bios_base + addr + 3);
 ... up to 256 databytes.
 chip_readb(bios_base);
 The last chip_readb(bios_base) is repeated until the result does not
 change anymore.

 For me, that looks pretty simple and straightforward, especially
 compared to other more exotic flash chip interfaces.

I disagree that you think my proposal is more complicated than this example.

But, I would agree, that all other things being equal, emulating a
real device would be preferable.

I would like to know what people's thoughts are regarding supporting
various devices sizes.  I think that right now -bios should support
files sizes that are multiples of 64kb up to ~16MB.  (Perhaps even
larger.)

A large -bios file can be interesting for embedding an OS image into
the rom/flash device...

Carl-Daniel, do you think we can address this while still supporting
real hardware emulation?

-Jordan



Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Auf 10.03.2011 23:14, Jordan Justen schrieb:
 On Thu, Mar 10, 2011 at 13:52, Carl-Daniel Hailfinger
 c-d.hailfinger.devel.2...@gmx.net wrote:
   
 Auf 10.03.2011 19:43, Jordan Justen schrieb:
 
 I thought this might be a case where deviation from real hardware
 emulation could better serve the VM's needs.
   
 If we have to write the code anyway, and if it can work just fine with
 current KVM/Qemu, is there a reason not to use the same interface as
 real hardware?
 
 If there was a real device emulated by qemu, I would gladly make use
 of it in OVMF.
   

Nice. So should I dig up my flash emulator code and check which chips
are supported? Porting the code to Qemu shouldn't be too hard.


 I just thought this proposal would be much easier to implement, and
 not be restricted to any particular size.  (Since -bios currently
 supports any size that is a multiple of 64kb.)  A real device is going
 to imply a certain size.
   

Right, the constant size argument is definitely a point we need to talk
about.

We could sidestep the issue by always using a 16 MByte flash device
which gets filled from the top with the firmware image, but I'm not sure
if that has other side effects.
Another way to solve this would be to emulate multiple flash chips, one
per power-of-two size between 128 kB and 16 MB. Some SPI flash chip
families encode the size into their ID, so determining the size
algorithmically is as simple as calculating
1  idbyte3
and emulating this is equally simple.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/




Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 14:31, Carl-Daniel Hailfinger
c-d.hailfinger.devel.2...@gmx.net wrote:
 Right, the constant size argument is definitely a point we need to talk
 about.

 We could sidestep the issue by always using a 16 MByte flash device
 which gets filled from the top with the firmware image, but I'm not sure
 if that has other side effects.
 Another way to solve this would be to emulate multiple flash chips, one
 per power-of-two size between 128 kB and 16 MB. Some SPI flash chip
 families encode the size into their ID, so determining the size
 algorithmically is as simple as calculating
 1  idbyte3
 and emulating this is equally simple.

Power of 2 from 128kb to 16MB sounds reasonable to me.

How would the SPI host controller be discovered?  Would the firmware
be able to depend on having control of the device at OS runtime?  This
would be needed for UEFI non-volatile variables to make sure they can
always be written.

Thanks,

-Jordan



[Qemu-devel] Re: [PATCH] Fix performance regression in qemu_get_ram_ptr

2011-03-10 Thread Anthony Liguori

On 03/10/2011 02:47 PM, Vincent Palatin wrote:

When the commit f471a17e9d869df3c6573f7ec02c4725676d6f3a converted the
ram_blocks structure to QLIST, it also removed the conditional check before
switching the current block at the beginning of the list.

In the common use case where ram_blocks has a few blocks with only one
frequently accessed (the main RAM), this has a performance impact as it
performs the useless list operations on each call (which are on a really
hot path).

On my machine emulation (ARM on amd64), this patch reduces the
percentage of CPU time spent in qemu_get_ram_ptr from 6.3% to 2.1% in the
profiling of a full boot.

Signed-off-by: Vincent Palatinvpala...@chromium.org


Applied.  Thanks.

Regards,

Anthony Liguori


---
  exec.c |7 +--
  1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index d611100..81f08b7 100644
--- a/exec.c
+++ b/exec.c
@@ -2957,8 +2957,11 @@ void *qemu_get_ram_ptr(ram_addr_t addr)

  QLIST_FOREACH(block,ram_list.blocks, next) {
  if (addr - block-offset  block-length) {
-QLIST_REMOVE(block, next);
-QLIST_INSERT_HEAD(ram_list.blocks, block, next);
+/* Move this entry to to start of the list.  */
+if (block != QLIST_FIRST(ram_list.blocks)) {
+QLIST_REMOVE(block, next);
+QLIST_INSERT_HEAD(ram_list.blocks, block, next);
+}
  return block-host + (addr - block-offset);
  }
  }





Re: [Qemu-devel] [PATCH 0/9] VMState infrastructure

2011-03-10 Thread Anthony Liguori

On 03/10/2011 05:33 AM, Juan Quintela wrote:

Hi

This is the infrastructure that I pushed on my previous series.
Anthony don't like 58 patches series (why? O:-) And then split the
series in three.

This are the infrastructure patches needed for the other two series.

Anthony, please apply.


Applied.  Thanks.

Regards,

Anthony Liguori


Later, Juan.

Juan Quintela (9):
   vmstate: add VMSTATE_UINT32_EQUAL
   vmstate: Fix varrays with uint8 indexes
   vmstate: add UINT32 VARRAYS
   vmstate: add VMSTATE_STRUCT_VARRAY_INT32
   vmstate: add VMSTATE_INT64_ARRAY
   vmstate: add VMSTATE_STRUCT_VARRAY_UINT32
   vmstate: Add a way to send a partial array
   vmstate: be able to store/save a pci device from a pointer
   vmstate: move timers to use test instead of version

  hw/hw.h  |   78 ++
  savevm.c |   25 
  2 files changed, 98 insertions(+), 5 deletions(-)






[Qemu-devel] Re: [PATCH] hmp-commands.hx: fix badly merged client_migrate_info command

2011-03-10 Thread Anthony Liguori

On 03/09/2011 09:54 AM, jes.soren...@redhat.com wrote:

From: Jes Sorensenjes.soren...@redhat.com

client_migrate_info was merged badly,


It wasn't merged badly, it was implemented badly.  The initial 
description confused me because it sounded like a bad merge conflict 
resolution but it just was wrong from the start.



  placing it between the command
and the documentation for another command. In addition it did not
respect the general rule of hmp-commands.hx, of having command
definition before the documentation.

Signed-off-by: Jes Sorensenjes.soren...@redhat.com


Applied.  Thanks.

Regards,

Anthony Liguori


---
  hmp-commands.hx |   32 
  1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 372bef4..834e6a8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -821,16 +821,12 @@ Set maximum tolerated downtime (in seconds) for migration.
  ETEXI

  {
-.name   = snapshot_blkdev,
-.args_type  = device:B,snapshot_file:s?,format:s?,
-.params = device [new-image-file] [format],
-.help   = initiates a live snapshot\n\t\t\t
-  of device. If a new image file is specified, 
the\n\t\t\t
-  new image file will become the new root image.\n\t\t\t
-  If format is specified, the snapshot file will\n\t\t\t
-  be created in that format. Otherwise the\n\t\t\t
-  snapshot will be internal! (currently unsupported),
-.mhandler.cmd_new = do_snapshot_blkdev,
+.name   = client_migrate_info,
+.args_type  = 
protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?,
+.params = protocol hostname port tls-port cert-subject,
+.help   = send migration info to spice/vnc client,
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = client_migrate_info,
  },

  STEXI
@@ -842,12 +838,16 @@ new parameters (if specified) once the vm migration 
finished successfully.
  ETEXI

  {
-.name   = client_migrate_info,
-.args_type  = 
protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?,
-.params = protocol hostname port tls-port cert-subject,
-.help   = send migration info to spice/vnc client,
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = client_migrate_info,
+.name   = snapshot_blkdev,
+.args_type  = device:B,snapshot_file:s?,format:s?,
+.params = device [new-image-file] [format],
+.help   = initiates a live snapshot\n\t\t\t
+  of device. If a new image file is specified, 
the\n\t\t\t
+  new image file will become the new root image.\n\t\t\t
+  If format is specified, the snapshot file will\n\t\t\t
+  be created in that format. Otherwise the\n\t\t\t
+  snapshot will be internal! (currently unsupported),
+.mhandler.cmd_new = do_snapshot_blkdev,
  },

  STEXI





Re: [Qemu-devel] [PATCH] vnc: Fix stack corruption and other bitmap related bugs

2011-03-10 Thread Anthony Liguori

On 03/03/2011 02:37 PM, Stefan Weil wrote:

Commit bc2429b9174ac2d3c56b7fd35884b0d89ec7fb02 introduced
a severe bug (stack corruption).

bitmap_clear was called with a wrong argument
which caused out-of-bound writes to the local variable width_mask.

This bug was detected with QEMU running on windows.
It also occurs with wine:

*** stack smashing detected ***:  terminated
wine: Unhandled illegal instruction at address 0x6115c7 (thread 0009), starting 
debugger...

The bug is not windows specific!

Instead of fixing the wrong parameter value, bitmap_clear(), bitmap_set
and width_mask were removed, and bitmap_intersect() was replaced by
!bitmap_empty(). The new operation is much shorter and equivalent to
the old operations.

The declarations of the dirty bitmaps in vnc.h were also wrong for 64 bit
hosts because of a rounding effect: for these hosts, VNC_MAX_WIDTH is no
longer a multiple of (16 * BITS_PER_LONG), so the rounded value of
VNC_DIRTY_WORDS was too small.

Fix both declarations by using the macro which is designed for this
purpose.

Cc: Corentin Charycorenti...@iksaif.net
Cc: Wen Congyangwe...@cn.fujitsu.com
Cc: Gerhard Wiesingerli...@wiesinger.com
Cc: Anthony Liguorialigu...@us.ibm.com
Signed-off-by: Stefan Weilw...@mail.berlios.de


Applied.  Thanks.

Regards,

Anthony Liguori


---
  ui/vnc.c |6 +-
  ui/vnc.h |9 ++---
  2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 610f884..34dc0cd 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2383,7 +2383,6 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
  uint8_t *guest_row;
  uint8_t *server_row;
  int cmp_bytes;
-unsigned long width_mask[VNC_DIRTY_WORDS];
  VncState *vs;
  int has_dirty = 0;

@@ -2399,14 +2398,11 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
   * Check and copy modified bits from guest to server surface.
   * Update server dirty map.
   */
-bitmap_set(width_mask, 0, (ds_get_width(vd-ds) / 16));
-bitmap_clear(width_mask, (ds_get_width(vd-ds) / 16),
- VNC_DIRTY_WORDS * BITS_PER_LONG);
  cmp_bytes = 16 * ds_get_bytes_per_pixel(vd-ds);
  guest_row  = vd-guest.ds-data;
  server_row = vd-server-data;
  for (y = 0; y  vd-guest.ds-height; y++) {
-if (bitmap_intersects(vd-guest.dirty[y], width_mask, 
VNC_DIRTY_WORDS)) {
+if (!bitmap_empty(vd-guest.dirty[y], VNC_DIRTY_BITS)) {
  int x;
  uint8_t *guest_ptr;
  uint8_t *server_ptr;
diff --git a/ui/vnc.h b/ui/vnc.h
index 8a1e7b9..f10c5dc 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -79,9 +79,12 @@ typedef void VncSendHextileTile(VncState *vs,
  void *last_fg,
  int *has_bg, int *has_fg);

+/* VNC_MAX_WIDTH must be a multiple of 16. */
  #define VNC_MAX_WIDTH 2560
  #define VNC_MAX_HEIGHT 2048
-#define VNC_DIRTY_WORDS (VNC_MAX_WIDTH / (16 * BITS_PER_LONG))
+
+/* VNC_DIRTY_BITS is the number of bits in the dirty bitmap. */
+#define VNC_DIRTY_BITS (VNC_MAX_WIDTH / 16)

  #define VNC_STAT_RECT  64
  #define VNC_STAT_COLS (VNC_MAX_WIDTH / VNC_STAT_RECT)
@@ -114,7 +117,7 @@ typedef struct VncRectStat VncRectStat;
  struct VncSurface
  {
  struct timeval last_freq_check;
-unsigned long dirty[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS];
+DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT], VNC_MAX_WIDTH / 16);
  VncRectStat stats[VNC_STAT_ROWS][VNC_STAT_COLS];
  DisplaySurface *ds;
  };
@@ -234,7 +237,7 @@ struct VncState
  int csock;

  DisplayState *ds;
-unsigned long dirty[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS];
+DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT], VNC_DIRTY_BITS);
  uint8_t **lossy_rect; /* Not an Array to avoid costly memcpy in
 * vnc-jobs-async.c */






Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Auf 10.03.2011 23:58, Jordan Justen schrieb:
 On Thu, Mar 10, 2011 at 14:31, Carl-Daniel Hailfinger
 c-d.hailfinger.devel.2...@gmx.net wrote:
   
 Right, the constant size argument is definitely a point we need to talk
 about.

 We could sidestep the issue by always using a 16 MByte flash device
 which gets filled from the top with the firmware image, but I'm not sure
 if that has other side effects.
 Another way to solve this would be to emulate multiple flash chips, one
 per power-of-two size between 128 kB and 16 MB. Some SPI flash chip
 families encode the size into their ID, so determining the size
 algorithmically is as simple as calculating
 1  idbyte3
 and emulating this is equally simple.
 
 Power of 2 from 128kb to 16MB sounds reasonable to me.

 How would the SPI host controller be discovered?

PCI ID of the ISA bridge of the chipset. AFAIK most flashing programs
out there use that criterion. There is one drawback, though: We should
use an interface which works well for all emulated chipsets in Qemu, and
that may a bit harder.
Is there a way to get PCI IDs of all emulated chipsets in Qemu so I can
take a look if there is a chance to to this correctly?



 Would the firmware
 be able to depend on having control of the device at OS runtime?  This
 would be needed for UEFI non-volatile variables to make sure they can
 always be written.
   

UEFI _should not_ have control of the device at OS runtime on real
hardware for security reasons, unless UEFI slipped a rootkit into the
OS. Not sure about Windows, but I'm pretty sure Linux will not run any
UEFI code (except maybe during early init).

Think flash update. If some flash update software runs under your OS of
choice, and UEFI is allowed to perform read/write accesses to flash at
the same time, you will get random corruption. You could do it like some
AMD chipsets, and provide some sort of semaphore for flash access
coordination between a flash updater and the BIOS/EFI, but I don't think
any Intel chipset can do that. Newer Intel chipsets allow locking out
flash accesses not coming from the management engine, but UEFI does not
run in the management engine, so that feature won't help us here.

That said, if any OS out there indeed runs UEFI code regularly during OS
runtime, and that UEFI code wants to access flash, it has to hope that
nobody else is trying to access flash at the same time. An easy way out
would be to use the ACPI NVS region while the machine is running an OS,
but changes would not automatically be persistent without help from the
OS or some ACPI handler on shutdown.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/




Re: [Qemu-devel] RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Hi Jordan,

thanks for your insights.

Auf 10.03.2011 23:29, Jordan Justen schrieb:
 On Thu, Mar 10, 2011 at 14:10, Carl-Daniel Hailfinger
 c-d.hailfinger.devel.2...@gmx.net wrote:
   
 Auf 10.03.2011 22:55, Jordan Justen schrieb:
 
 On Thu, Mar 10, 2011 at 13:37, Carl-Daniel Hailfinger
 c-d.hailfinger.devel.2...@gmx.net wrote:
   
 Is there any reason why you chose to invent an interface for the flash
 chip which is more complicated than the interface used by common flash
 chips out there?
 Maybe some EFI requirement?
 
 This is a simpler version than the flash devices I'm used to dealing
 with for x86/x86-64 UEFI systems.  Can you suggest an alternative real
 interface that is simpler?
   
 Pseudocode for the real interface most common on x86 before SPI came along:

 Write a page (256 bytes) or a few bytes:
 chip_writeb(0xAA, bios_base + 0x);
 chip_writeb(0x55, bios_base + 0x2AAA);
 chip_writeb(0xA0, bios_base + 0x);
 chip_writeb(databyte0, bios_base + addr);
 chip_writeb(databyte1, bios_base + addr + 1);
 chip_writeb(databyte2, bios_base + addr + 2);
 chip_writeb(databyte3, bios_base + addr + 3);
 ... up to 256 databytes.
 chip_readb(bios_base);
 The last chip_readb(bios_base) is repeated until the result does not
 change anymore.

 For me, that looks pretty simple and straightforward, especially
 compared to other more exotic flash chip interfaces.
 
 I disagree that you think my proposal is more complicated than this example.
   

Upon rereading your proposal, I think the unfamiliarity of the proposed
interface and the index/data design triggered my complicated reflex.


 But, I would agree, that all other things being equal, emulating a
 real device would be preferable.

 I would like to know what people's thoughts are regarding supporting
 various devices sizes.  I think that right now -bios should support
 files sizes that are multiples of 64kb up to ~16MB.  (Perhaps even
 larger.)
   

On recent Intel chipsets you are limited to 16 MB mapped to the top of
the 32bit address space. The same limitation exists for most other x86
chipsets as well. That said, some people may want bigger images, but for
x86 this may cause conflicts with the HPET region.


 A large -bios file can be interesting for embedding an OS image into
 the rom/flash device...
   

I've seen people embed coreboot+Linux+X.org into a 4 MB image on x86, so
I think 16 MB are plenty (flash bigger than that is either NAND or
expensive or slow, and would require significant effort to emulate
correctly (NAND) or simply not exist in consumer equipment for
speed/money reasons).


 Carl-Daniel, do you think we can address this while still supporting
 real hardware emulation?
   

If we restrict ourselves to the 128kB-16MB range, I think I can find a
flash chip family which has the criteria we want. 64 kByte flash chips
still exist, but usually not as part of a family which reaches 16 MByte.

We should decide first if we want SPI flash or Parallel flash, and then
I can try to find a matching flash chip family.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/




[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jan Kiszka
On 2011-03-10 23:10, Carl-Daniel Hailfinger wrote:
 Auf 10.03.2011 22:55, Jordan Justen schrieb:
 On Thu, Mar 10, 2011 at 13:37, Carl-Daniel Hailfinger
 c-d.hailfinger.devel.2...@gmx.net wrote:
   
 Auf 10.03.2011 05:51, Jordan Justen schrieb:
 
 I have documented a simple flash-like device which I think could be
 useful for qemu/kvm in some cases.  (Particularly for allowing
 persistent UEFI non-volatile variables.)

 http://wiki.qemu.org/Features/System_Flash

 Let me know if you have any suggestions or concerns.

   
 Is there any reason why you chose to invent an interface for the flash
 chip which is more complicated than the interface used by common flash
 chips out there?
 Maybe some EFI requirement?
 
 This is a simpler version than the flash devices I'm used to dealing
 with for x86/x86-64 UEFI systems.  Can you suggest an alternative real
 interface that is simpler?
   
 
 Pseudocode for the real interface most common on x86 before SPI came along:
 
 Write a page (256 bytes) or a few bytes:
 chip_writeb(0xAA, bios_base + 0x);
 chip_writeb(0x55, bios_base + 0x2AAA);
 chip_writeb(0xA0, bios_base + 0x);
 chip_writeb(databyte0, bios_base + addr);
 chip_writeb(databyte1, bios_base + addr + 1);
 chip_writeb(databyte2, bios_base + addr + 2);
 chip_writeb(databyte3, bios_base + addr + 3);
 ... up to 256 databytes.
 chip_readb(bios_base);
 The last chip_readb(bios_base) is repeated until the result does not
 change anymore.

Hmm, I may oversee some subtle difference, but this looks pretty much
like CFI to me (see hw/pflash_cfi02.c).

At least it's an in-band interface, which is the better choice as we
currently only have a PIIX3 southbridge for x86, predating even FWHs.

Jan



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Carl-Daniel Hailfinger
Auf 11.03.2011 01:19, Jan Kiszka schrieb:
 On 2011-03-10 23:10, Carl-Daniel Hailfinger wrote:
   
 Auf 10.03.2011 22:55, Jordan Justen schrieb:
 
 On Thu, Mar 10, 2011 at 13:37, Carl-Daniel Hailfinger
 c-d.hailfinger.devel.2...@gmx.net wrote:
   
   
 Auf 10.03.2011 05:51, Jordan Justen schrieb:
 
 
 I have documented a simple flash-like device which I think could be
 useful for qemu/kvm in some cases.  (Particularly for allowing
 persistent UEFI non-volatile variables.)

 http://wiki.qemu.org/Features/System_Flash

 Let me know if you have any suggestions or concerns.

   
   
 Is there any reason why you chose to invent an interface for the flash
 chip which is more complicated than the interface used by common flash
 chips out there?
 Maybe some EFI requirement?
 
 
 This is a simpler version than the flash devices I'm used to dealing
 with for x86/x86-64 UEFI systems.  Can you suggest an alternative real
 interface that is simpler?
   
   
 Pseudocode for the real interface most common on x86 before SPI came along:

 Write a page (256 bytes) or a few bytes:
 chip_writeb(0xAA, bios_base + 0x);
 chip_writeb(0x55, bios_base + 0x2AAA);
 chip_writeb(0xA0, bios_base + 0x);
 chip_writeb(databyte0, bios_base + addr);
 chip_writeb(databyte1, bios_base + addr + 1);
 chip_writeb(databyte2, bios_base + addr + 2);
 chip_writeb(databyte3, bios_base + addr + 3);
 ... up to 256 databytes.
 chip_readb(bios_base);
 The last chip_readb(bios_base) is repeated until the result does not
 change anymore.
 
 Hmm, I may oversee some subtle difference, but this looks pretty much
 like CFI to me (see hw/pflash_cfi02.c).
   

I thought CFI also implements a way to retrieve device size/geometry
with the Query (0x98) command, but that part is rarely implemented on
x86 flash (I didn't see any yet).
That said, the non-query commands are identical AFAICS.


 At least it's an in-band interface, which is the better choice as we
 currently only have a PIIX3 southbridge for x86, predating even FWHs.
   

Right, that pretty much kills the option of using SPI unless someone
wants to emulate a flash translation controller (e.g. the ITE IT8716F
Super I/O). Can be done, would work, but the IT8716F has some quirks
(max 1 MB SPI flash chips) which make it a less desirable option for
emulation.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/




[Qemu-devel] [PATCH v6 1/3] rtl8139: cleanup FCS calculation

2011-03-10 Thread Benjamin Poirier
clean out ifdef's around ethernet checksum calculation

Signed-off-by: Benjamin Poirier benjamin.poir...@gmail.com
Cc: Igor V. Kovalenko igor.v.kovale...@gmail.com
Cc: Jason Wang jasow...@redhat.com
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Blue Swirl blauwir...@gmail.com
---
 hw/rtl8139.c |   20 +++-
 1 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index a22530c..3772ac1 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -47,6 +47,9 @@
  *  Darwin)
  */
 
+/* For crc32 */
+#include zlib.h
+
 #include hw.h
 #include pci.h
 #include qemu-timer.h
@@ -62,14 +65,6 @@
 /* debug RTL8139 card C+ mode only */
 //#define DEBUG_RTL8139CP 1
 
-/* Calculate CRCs properly on Rx packets */
-#define RTL8139_CALCULATE_RXCRC 1
-
-#if defined(RTL8139_CALCULATE_RXCRC)
-/* For crc32 */
-#include zlib.h
-#endif
-
 #define SET_MASKED(input, mask, curr) \
 ( ( (input)  ~(mask) ) | ( (curr)  (mask) ) )
 
@@ -1030,11 +1025,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, 
const uint8_t *buf, size_
 }
 
 /* write checksum */
-#if defined (RTL8139_CALCULATE_RXCRC)
 val = cpu_to_le32(crc32(0, buf, size));
-#else
-val = 0;
-#endif
 cpu_physical_memory_write( rx_addr+size, (uint8_t *)val, 4);
 
 /* first segment of received packet flag */
@@ -1136,12 +1127,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, 
const uint8_t *buf, size_
 rtl8139_write_buffer(s, buf, size);
 
 /* write checksum */
-#if defined (RTL8139_CALCULATE_RXCRC)
 val = cpu_to_le32(crc32(0, buf, size));
-#else
-val = 0;
-#endif
-
 rtl8139_write_buffer(s, (uint8_t *)val, 4);
 
 /* correct buffer write pointer */
-- 
1.7.2.3




[Qemu-devel] [PATCH v6] rtl8139: add vlan support

2011-03-10 Thread Benjamin Poirier
Here is version 6 of my patchset to add vlan support to the emulated rtl8139
nic.

Changes since v5:
* moved all receive changes to add vlan tag extraction
* fixed checkpatch.pl style issues
* fixed bugs in receive case related to small buffers and loopback
  mode. Moved too small buffer code back where it used to be, though
  it is changed in content.

Changes since v4:
* removed alloca(), for real. Thanks to the reviewers for their
  patience. This patchset now has more versions than the vlan header
  has bytes!
* corrected the unlikely, debug printf and long lines, as per comments
* cleaned out ifdef's pertaining to ethernet checksum calculation.
  According to a comment since removed they were related to an
  optimization:
   RTL8139 provides frame CRC with received packet, this feature
   seems to be ignored by most drivers, disabled by default
  see commit ccf1d14

I've tested v5 using x86_64 host/guest with the usual procedure. I've also ran
the clang analyzer on the qemu code base, just for fun.

Changes since v3:
* removed alloca() and #include net/ethernet.h as per comments
* reordered patches to put extraction before insertion. Extraction
  touches only the receive path but insertion touches both. The two
  patches are now needed to have vlan functionnality.

I've tested v4 with x86_64 host/guest. I used the same testing procedure as
before. I've tested a plain configuration as well as one with tso + vlan
offload, successfully.

I had to hack around the Linux 8139cp driver to be able to enable tso on vlan
which leads me to wonder, can someone with access to the C+ spec or a real
card confirm that it can do tso and vlan offload at the same time? The patch
I used for the kernel is at https://gist.github.com/851895.

Changes since v2:
insertion:
* moved insertion later in the process, to handle tso
* use qemu_sendv_packet() to insert the tag for us
* added dot1q_buf parameter to rtl8139_do_receive() to avoid some
  memcpy() in loopback mode. Note that the code path through that
  function is unchanged when dot1q_buf is NULL.

extraction:
* reduced the amount of copying by moving the frame too short logic
  after the removal of the vlan tag (as is done in e1000.c for
  example). Unfortunately, that logic can no longer be shared betwen
  C+ and C mode.

I've posted v2 of these patches back in November
http://article.gmane.org/gmane.comp.emulators.qemu/84252

I've tested v3 on the following combinations of guest and hosts:
host: x86_64, guest: x86_64
host: x86_64, guest: ppc32
host: ppc32, guest: ppc32

Testing on the x86_64 host used '-net tap' and consisted of:
* making an http transfert on the untagged interface.
* ping -s 0-1472 to another host on a vlan.
* making an scp upload to another host on a vlan.

Testing on the ppc32 host used '-net socket' connected to an x86_64 qemu-kvm
running the virtio nic and consisted of:
* establishing an ssh connection between the two using an untagged interface.
* ping -s 0-1472 between the two using a vlan.
* making an scp transfer in both directions using a vlan.

All that was successful. Nevertheless, it doesn't exercise all code paths so
care is in order.

Please note that the lack of vlan support in rtl8139 has taken a few people
aback:
https://bugzilla.redhat.com/show_bug.cgi?id=516587
http://article.gmane.org/gmane.linux.network.general/14266

Thanks,
-Ben



[Qemu-devel] [PATCH v6 3/3] rtl8139: add vlan tag insertion

2011-03-10 Thread Benjamin Poirier
Add support to the emulated hardware to insert vlan tags in packets
going from the guest to the network.

Signed-off-by: Benjamin Poirier benjamin.poir...@gmail.com
Cc: Igor V. Kovalenko igor.v.kovale...@gmail.com
Cc: Jason Wang jasow...@redhat.com
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Blue Swirl blauwir...@gmail.com
---
 hw/rtl8139.c |   57 +
 1 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 312d362..11034fb 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -1821,7 +1821,8 @@ static uint32_t rtl8139_RxConfig_read(RTL8139State *s)
 return ret;
 }
 
-static void rtl8139_transfer_frame(RTL8139State *s, const uint8_t *buf, int 
size, int do_interrupt)
+static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
+int do_interrupt, const uint8_t *dot1q_buf)
 {
 if (!size)
 {
@@ -1832,11 +1833,22 @@ static void rtl8139_transfer_frame(RTL8139State *s, 
const uint8_t *buf, int size
 if (TxLoopBack == (s-TxConfig  TxLoopBack))
 {
 DEBUG_PRINT((RTL8139: +++ transmit loopback mode\n));
-rtl8139_do_receive(s-nic-nc, buf, size, do_interrupt, NULL);
+rtl8139_do_receive(s-nic-nc, buf, size, do_interrupt, dot1q_buf);
 }
 else
 {
-qemu_send_packet(s-nic-nc, buf, size);
+if (dot1q_buf) {
+struct iovec iov[] = {
+{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 },
+{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN },
+{ .iov_base = buf + ETHER_ADDR_LEN * 2,
+.iov_len = size - ETHER_ADDR_LEN * 2 },
+};
+
+qemu_sendv_packet(s-nic-nc, iov, ARRAY_SIZE(iov));
+} else {
+qemu_send_packet(s-nic-nc, buf, size);
+}
 }
 }
 
@@ -1870,7 +1882,7 @@ static int rtl8139_transmit_one(RTL8139State *s, int 
descriptor)
 s-TxStatus[descriptor] |= TxHostOwns;
 s-TxStatus[descriptor] |= TxStatOK;
 
-rtl8139_transfer_frame(s, txbuffer, txsize, 0);
+rtl8139_transfer_frame(s, txbuffer, txsize, 0, NULL);
 
 DEBUG_PRINT((RTL8139: +++ transmitted %d bytes from descriptor %d\n, 
txsize, descriptor));
 
@@ -1997,7 +2009,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 
 cpu_physical_memory_read(cplus_tx_ring_desc,(uint8_t *)val, 4);
 txdw0 = le32_to_cpu(val);
-/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
 cpu_physical_memory_read(cplus_tx_ring_desc+4,  (uint8_t *)val, 4);
 txdw1 = le32_to_cpu(val);
 cpu_physical_memory_read(cplus_tx_ring_desc+8,  (uint8_t *)val, 4);
@@ -2009,9 +2020,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
descriptor,
txdw0, txdw1, txbufLO, txbufHI));
 
-/* TODO: the following discard cast should clean clang analyzer output */
-(void)txdw1;
-
 /* w0 ownership flag */
 #define CP_TX_OWN (131)
 /* w0 end of ring flag */
@@ -2035,9 +2043,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 /* w0 bits 0...15 : buffer size */
 #define CP_TX_BUFFER_SIZE (116)
 #define CP_TX_BUFFER_SIZE_MASK (CP_TX_BUFFER_SIZE - 1)
-/* w1 tag available flag */
-#define CP_RX_TAGC (117)
-/* w1 bits 0...15 : VLAN tag */
+/* w1 add tag flag */
+#define CP_TX_TAGC (117)
+/* w1 bits 0...15 : VLAN tag (big endian) */
 #define CP_TX_VLAN_TAG_MASK ((116) - 1)
 /* w2 low  32bit of Rx buffer ptr */
 /* w3 high 32bit of Rx buffer ptr */
@@ -2137,13 +2145,13 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 /* update ring data */
 val = cpu_to_le32(txdw0);
 cpu_physical_memory_write(cplus_tx_ring_desc,(uint8_t *)val, 4);
-/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
-//val = cpu_to_le32(txdw1);
-//cpu_physical_memory_write(cplus_tx_ring_desc+4,  val, 4);
 
 /* Now decide if descriptor being processed is holding the last segment of 
packet */
 if (txdw0  CP_TX_LS)
 {
+uint8_t dot1q_buffer_space[VLAN_HLEN];
+uint16_t *dot1q_buffer;
+
 DEBUG_PRINT((RTL8139: +++ C+ Tx mode : descriptor %d is last segment 
descriptor\n, descriptor));
 
 /* can transfer fully assembled packet */
@@ -2152,6 +2160,21 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 int  saved_size= s-cplus_txbuffer_offset;
 int  saved_buffer_len = s-cplus_txbuffer_len;
 
+/* create vlan tag */
+if (txdw1  CP_TX_TAGC) {
+/* the vlan tag is in BE byte order in the descriptor
+ * BE + le_to_cpu() + ~swap()~ = cpu */
+DEBUG_PRINT((RTL8139: +++ C+ Tx mode : inserting vlan tag with 
+tci: %u\n, bswap16(txdw1  CP_TX_VLAN_TAG_MASK)));
+
+dot1q_buffer = (uint16_t *) dot1q_buffer_space;
+dot1q_buffer[0] = cpu_to_be16(ETH_P_8021Q);
+/* BE + le_to_cpu() + ~cpu_to_le()~ = BE */
+

[Qemu-devel] [PATCH v6 2/3] rtl8139: add vlan tag extraction

2011-03-10 Thread Benjamin Poirier
Add support to the emulated hardware to extract vlan tags in packets
going from the network to the guest.

Signed-off-by: Benjamin Poirier benjamin.poir...@gmail.com
Cc: Igor V. Kovalenko igor.v.kovale...@gmail.com
Cc: Jason Wang jasow...@redhat.com
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Blue Swirl blauwir...@gmail.com

--

AFAIK, extraction is optional to get vlans working. The driver
requests rx detagging but should not assume that it was done. Under
Linux, the mac layer will catch the vlan ethertype. I only added this
part for completeness (to emulate the hardware more truthfully...)
---
 hw/rtl8139.c |  125 ++
 1 files changed, 108 insertions(+), 17 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 3772ac1..312d362 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -72,6 +72,22 @@
 #define MOD2(input, size) \
 ( ( input )  ( size - 1 )  )
 
+#define min(x, y) ({\
+typeof(x) _min1 = (x);  \
+typeof(y) _min2 = (y);  \
+(void) (_min1 == _min2);  \
+_min1  _min2 ? _min1 : _min2; })
+
+#define ETHER_ADDR_LEN 6
+#define ETHER_TYPE_LEN 2
+#define ETH_HLEN (ETHER_ADDR_LEN * 2 + ETHER_TYPE_LEN)
+#define ETH_P_IP0x0800  /* Internet Protocol packet */
+#define ETH_P_8021Q 0x8100  /* 802.1Q VLAN Extended Header  */
+#define ETH_MTU 1500
+
+#define VLAN_TCI_LEN 2
+#define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN)
+
 #if defined (DEBUG_RTL8139)
 #  define DEBUG_PRINT(x) do { printf x ; } while (0)
 #else
@@ -777,6 +793,14 @@ static void rtl8139_write_buffer(RTL8139State *s, const 
void *buf, int size)
 s-RxBufAddr += size;
 }
 
+static unsigned long rtl8139_write_buffer_crc(RTL8139State *s, const void
+*buf, int size, unsigned long crc)
+{
+rtl8139_write_buffer(s, buf, size);
+return crc32(crc, buf, size);
+}
+
+
 #define MIN_BUF_SIZE 60
 static inline target_phys_addr_t rtl8139_addr64(uint32_t low, uint32_t high)
 {
@@ -809,14 +833,20 @@ static int rtl8139_can_receive(VLANClientState *nc)
 }
 }
 
-static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, 
size_t size_, int do_interrupt)
+static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
+size_t buf_size, int do_interrupt, const uint8_t *dot1q_buf)
 {
 RTL8139State *s = DO_UPCAST(NICState, nc, nc)-opaque;
+/* size_ is the total length of argument buffers */
+const int size_ = buf_size + (dot1q_buf ? VLAN_HLEN : 0);
+/* size is the length of the buffer passed to the driver */
 int size = size_;
+const uint8_t *next_part;
+size_t next_part_size;
 
 uint32_t packet_header = 0;
 
-uint8_t buf1[60];
+uint8_t buf1[MIN_BUF_SIZE];
 static const uint8_t broadcast_macaddr[6] =
 { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -930,10 +960,28 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, 
const uint8_t *buf, size_
 
 /* if too small buffer, then expand it */
 if (size  MIN_BUF_SIZE) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE - size);
+size_t copied_size;
+
+copied_size = min((size_t) ETHER_ADDR_LEN * 2, buf_size);
+memcpy(buf1, buf, copied_size);
+buf_size -= copied_size;
+
+if (dot1q_buf) {
+if (copied_size = ETHER_ADDR_LEN * 2) {
+memcpy(buf1 + copied_size, dot1q_buf, VLAN_HLEN);
+copied_size += VLAN_HLEN;
+}
+dot1q_buf = NULL;
+}
+
+if (buf_size  0) {
+memcpy(buf1 + copied_size, buf[ETHER_ADDR_LEN * 2], buf_size);
+copied_size += buf_size;
+}
+memset(buf1 + copied_size, 0, MIN_BUF_SIZE - copied_size);
 buf = buf1;
 size = MIN_BUF_SIZE;
+buf_size = MIN_BUF_SIZE;
 }
 
 if (rtl8139_cp_receiver_enabled(s))
@@ -996,6 +1044,37 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, 
const uint8_t *buf, size_
 
 uint32_t rx_space = rxdw0  CP_RX_BUFFER_SIZE_MASK;
 
+/* write VLAN info to descriptor variables.
+ * The logical packet is formed of:
+ * * buf[0..12[,
+ * * dot1q_buf (if offloading is enabled and tag is present or
+ *   dot1q_buf is passed in argument),
+ * * next_part[0..next_part_size[.
+ */
+next_part = buf[ETHER_ADDR_LEN * 2];
+if (s-CpCmd  CPlusRxVLAN  (dot1q_buf || be16_to_cpup((uint16_t *)
+next_part) == ETH_P_8021Q)) {
+if (!dot1q_buf) {
+/* the tag is in the buffer */
+dot1q_buf = next_part;
+next_part += VLAN_HLEN;
+}
+size -= VLAN_HLEN;
+
+rxdw1 = ~CP_RX_VLAN_TAG_MASK;
+/* BE + ~le_to_cpu()~ + cpu_to_le() = BE */
+rxdw1 |= CP_RX_TAVA | le16_to_cpup((uint16_t *)
+dot1q_buf[ETHER_TYPE_LEN]);
+
+DEBUG_PRINT((RTL8139: 

Re: [Qemu-devel] Re: RFC: emulation of system flash

2011-03-10 Thread Jordan Justen
On Thu, Mar 10, 2011 at 15:41, Carl-Daniel Hailfinger
c-d.hailfinger.devel.2...@gmx.net wrote:
 Auf 10.03.2011 23:58, Jordan Justen schrieb:
 Would the firmware
 be able to depend on having control of the device at OS runtime?  This
 would be needed for UEFI non-volatile variables to make sure they can
 always be written.


 UEFI _should not_ have control of the device at OS runtime on real
 hardware for security reasons, unless UEFI slipped a rootkit into the
 OS. Not sure about Windows, but I'm pretty sure Linux will not run any
 UEFI code (except maybe during early init).

UEFI non-volatile variables are a runtime service, meaning the OS
should be able to utilize them at any time.

It is up to the OS whether it wants to actually make use of the
runtime services, of course.  Both Windows and Linux do have
interfaces available to modify UEFI variables at runtime.

 Think flash update. If some flash update software runs under your OS of
 choice, and UEFI is allowed to perform read/write accesses to flash at
 the same time, you will get random corruption. You could do it like some
 AMD chipsets, and provide some sort of semaphore for flash access
 coordination between a flash updater and the BIOS/EFI, but I don't think
 any Intel chipset can do that. Newer Intel chipsets allow locking out
 flash accesses not coming from the management engine, but UEFI does not
 run in the management engine, so that feature won't help us here.

The UEFI systems (meaning motherboard+firmware) that I have worked on
generally do not allow the flash (code) to be modified while the OS is
running.  Instead, UEFI has a 'capsule' concept where firmware update
data is transfered to the firmware from the OS during a 'reboot' of
sorts.  The firmware validates the capsule data, and then flashes it
on the boot following the reset.

But, the sections of the flash which non-volatile variables are stored
in can be updated by the UEFI firmware, and there are mechanisms which
can restrict this access as well to prevent corruption of the NV
variables.

Unfortunately, I assume these security mechanisms often come into
conflict with useful tools like flashrom.  (At least during OS
runtime.)

 That said, if any OS out there indeed runs UEFI code regularly during OS
 runtime, and that UEFI code wants to access flash, it has to hope that
 nobody else is trying to access flash at the same time. An easy way out
 would be to use the ACPI NVS region while the machine is running an OS,
 but changes would not automatically be persistent without help from the
 OS or some ACPI handler on shutdown.

To be UEFI compatible, the non-volatile variable write should become
persistent immediately after the call returns successfully.  This has
been the case on most UEFI systems that I have worked on.

-Jordan



[Qemu-devel] [Bug 688085] Re: Guest kernel hang during boot when KVM is active on i386 host

2011-03-10 Thread Bug Watch Updater
** Changed in: meego
   Status: In Progress = 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/688085

Title:
  Guest kernel hang during boot when KVM is active on i386 host

Status in meego project:
  Fix Released
Status in QEMU:
  Fix Released
Status in qemu-kvm:
  Fix Released
Status in “kvm” package in Ubuntu:
  Invalid
Status in “linux” package in Ubuntu:
  Fix Released
Status in “qemu” package in Ubuntu:
  Invalid
Status in “qemu-kvm” package in Ubuntu:
  Invalid
Status in “kvm” source package in Maverick:
  New
Status in “linux” source package in Maverick:
  New
Status in “qemu” source package in Maverick:
  New
Status in “qemu-kvm” source package in Maverick:
  New

Bug description:
  Binary package hint: qemu

  Guest kernel hang during boot when KVM is active on i386 host

  See the patch.
  http://www.spinics.net/lists/kvm/msg40800.html

  How to reproduce:
  1. install Maversick x86 (not amd64)
  2. ensure you have  kvm support in processor
  3. kvm -kernel /boot/initrd.img-2.6.35-24-generic-pae
  4. kvm -no-kvm -kernel /boot/initrd.img-2.6.35-24-generic-pae works OK.

  SRU Justification:
  Impact: Users cannot boot KVM guests on i386 hosts
  2. How bug addressed:  The upstream commit at 
http://www.spinics.net/lists/kvm/msg40800.html fixed it
  3. Patch:  A kernel patch is attached to this bug.
  4. Reproduce: boot an i386 kernel on a kvm-capable host.  Try to boot a kvm 
guest.
  5. Regression potential: since this is cherrypicking a commit from a future 
upstream which had already been changed, regression is possible.  However if 
there is a regression, it should only affect users of KVM on i386 hosts, which 
currently fail anyway.



[Qemu-devel] Re: [PATCH v6 1/3] rtl8139: cleanup FCS calculation

2011-03-10 Thread Igor Kovalenko
On Fri, Mar 11, 2011 at 3:35 AM, Benjamin Poirier
benjamin.poir...@gmail.com wrote:
 clean out ifdef's around ethernet checksum calculation

 Signed-off-by: Benjamin Poirier benjamin.poir...@gmail.com
 Cc: Igor V. Kovalenko igor.v.kovale...@gmail.com
 Cc: Jason Wang jasow...@redhat.com
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Blue Swirl blauwir...@gmail.com
 ---
  hw/rtl8139.c |   20 +++-
  1 files changed, 3 insertions(+), 17 deletions(-)

Signed-off-by: Igor V. Kovalenko igor.v.kovale...@gmail.com

-- 
Kind regards,
Igor V. Kovalenko



Re: [Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions

2011-03-10 Thread Venkateswararao Jujjuri (JV)
On 3/10/2011 4:29 AM, Stefan Hajnoczi wrote:
 On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar mo...@in.ibm.com wrote:
 Add chroot functionality for systemcalls that can operate on a file
 using relative directory file descriptor.
 
 I suspect the relative directory approach is broken and escapes the
 chroot.  Here's why:
 
 The request is local_chmod(fs_ctx, /.., credp).  dirname(/..) is
 / and basename(..) is ...

We should never receive protocol operations with relative path.
Client should always resolve to full path and send the request.
If the client is malicious this scenario can be be possible.. but in that case
it is fine to fail the operation.

Thanks,
JV

 I'm not 100% sure of the semantics but I suspect that chmodat(dir_fd,
 .., ...) does not honor the chroot since your current task is not
 inside the chroot.  If so, then you can manipulate the parent
 directory of the chroot using some of the operations added in this
 patch.
 
 The safe solution is to perform all operations inside the chroot.
 This will require extending the chroot socket protocol.
 
 Stefan
 





[Qemu-devel] Re: [PATCH] kvm: ppc: Fix breakage of kvm_arch_pre_run/process_irqchip_events

2011-03-10 Thread Alexander Graf

On 17.02.2011, at 22:01, Jan Kiszka wrote:

 On 2011-02-07 12:19, Jan Kiszka wrote:
 We do not check them, and the only arch with non-empty implementations
 always returns 0 (this is also true for qemu-kvm).
 
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 CC: Alexander Graf ag...@suse.de
 ---
 kvm.h  |5 ++---
 target-i386/kvm.c  |8 ++--
 target-ppc/kvm.c   |6 ++
 target-s390x/kvm.c |6 ++
 4 files changed, 8 insertions(+), 17 deletions(-)
 
 
 ...
 
 diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
 index 93ecc57..bd4012a 100644
 --- a/target-ppc/kvm.c
 +++ b/target-ppc/kvm.c
 @@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run 
 *run)
 return 0;
 }
 
 -int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 +void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 {
 -return 0;
 }
 
 -int kvm_arch_process_irqchip_events(CPUState *env)
 +void kvm_arch_process_irqchip_events(CPUState *env)
 {
 -return 0;
 }
 
 Oops. Do we already have a built-bot for KVM-enabled PPC (and s390)
 targets somewhere?

Just before leaving for vacation I prepared a machine for each and gave stefan 
access to them. Looks like they're not officially running though - will try to 
look at this asap.


Alex




Re: [Qemu-devel] [PATCH 02/17] lm32: translation routines

2011-03-10 Thread Alexander Graf

On 17.02.2011, at 23:51, Michael Walle wrote:

 Am Samstag 12 Februar 2011, 07:49:52 schrieb Blue Swirl:
 That said, IMHO the best handling of unknown opcodes would be to kill the
 VM.
 
 In this case it should be OK. Alternatively the VM could be halted, so
 that instead of restarting QEMU, only system_reset needs to be issued.
 This may be more useful for developers, since for example registers
 and memory can be examined after the error.
 
 Good idea! May I call vm_stop() in a tcg helper? Like in the following 
 example:
 
 void helper_vm_stop(uint32_t msg_id)
 {
if (qemu_log_enabled()) {
qemu_log(VM stopped: %s, err_msg_str[msg_id]);
} else {
fprintf(stderr, VM stopped: %s, err_msg_str[msg_id]);
}
 #ifndef CONFIG_USER_ONLY
vm_stop(0);
 #endif
env-exception_index = EXCP_HALTED;
cpu_loop_exit();
 }
 
 If not, what is the proper way to stop/pause the VM from within the executed 
 code?

Since I haven't seen any reply yet: Can't you just do the same as hlt and 
disable interrupts?


Alex




Re: [Xen-devel] Re: [Qemu-devel] [PATCH V10 02/15] xen: Make xen build only on x86 target.

2011-03-10 Thread Alexander Graf

On 24.02.2011, at 18:59, Anthony Liguori wrote:

 On 02/24/2011 11:46 AM, Jan Kiszka wrote:
 On 2011-02-24 18:27, Anthony Liguori wrote:
   
 On 02/24/2011 10:25 AM, Anthony PERARD wrote:
 
 On Thu, Feb 24, 2011 at 16:11, Anthony Liguorianth...@codemonkey.ws   
 wrote:
 
   
 Is this really necessary?  The advantage to building globally is that it
 keeps the code from getting unnecessary i386-isms.
 
 
 Nop, is not necessary, I add this patch after this mail:
 http://lists.nongnu.org/archive/html/qemu-devel/2010-12/msg00044.html
 
   
 Alex, do you feel strongly here?
 
 I'm not Alex, but I brought this issue up:
 
 Either build xen bits once for all archs or restrict it to the only
 foreseeable arch with support in qemu. But please don't built it for
 each and every target separately.
   
 
 Oh yes, I misunderstood.  I thought we had previously built it for all 
 architectures.  Yes, we should only build it once.

There's really no point in building it for anything but x86. Xen has never had 
been broadly used in PV mode on non-x86. Not sure ports even exist.


Alex




Re: [Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions

2011-03-10 Thread Stefan Hajnoczi
On Fri, Mar 11, 2011 at 5:54 AM, Venkateswararao Jujjuri (JV)
jv...@linux.vnet.ibm.com wrote:
 On 3/10/2011 4:29 AM, Stefan Hajnoczi wrote:
 On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar mo...@in.ibm.com wrote:
 Add chroot functionality for systemcalls that can operate on a file
 using relative directory file descriptor.

 I suspect the relative directory approach is broken and escapes the
 chroot.  Here's why:

 The request is local_chmod(fs_ctx, /.., credp).  dirname(/..) is
 / and basename(..) is ...

 We should never receive protocol operations with relative path.
 Client should always resolve to full path and send the request.
 If the client is malicious this scenario can be be possible.. but in that case
 it is fine to fail the operation.

What I haven't audited yet is whether symlinks can be abused in any of
these *at(2) operations.

The *at(2) approach seems like a shortcut to avoid implementing
individual chroot protocol requests/responses for stat(2) and friends.
 But it carries the risk that if we don't use NOFOLLOW then we can be
tricked into escaping the chroot because we're performing the
operation outside the chroot.

I'll take a look later today to make sure all operations safe traverse
paths outside the chroot.

Stefan



Re: [Qemu-devel] Issue with snapshot outside qcow2 disk - qemu 0.14.0

2011-03-10 Thread Jes Sorensen
On 03/10/11 22:04, Stefan Hajnoczi wrote:
 On Thu, Mar 10, 2011 at 7:57 PM, SAURAV LAHIRI saurav_lah...@yahoo.com 
 wrote:
 The high level use case is that of being able to backup user specified disks 
 of a VM without having to bring down the VM.
 
 Excellent, that sounds exactly like Jes is addressing so future
 QEMU/KVM releases will hopefully have the live snapshot/merge
 capability.
 
 snapshot_blkdev: Regarding this  I do have a couple of questions.

 1. If the snapshot cannot be merged then it could mean that there are 
 several snapshot files. One readonly  for each of the previous snapshots and 
 the last one being the active one, which handles all the current writes. 
 Post backup If do have to restore to a particular snapshot then i would 
 probably have to copy all the files in the chain and maintain the entire 
 chain. But would it not affect read performance if several snapshot files 
 are maintained, particularly if the VM is hosting a database like mysql ? 
 Could you please clarify.
 
 If the VM is not running you can use the qemu-img commit command to
 merge the snapshot back down into the base image.  After that you only
 have one image file again and can restart the VM.  Hopefully the
 deltas are small enough that this process is quick.
 
 In the future a live merge command will take care of this and avoid
 the downtime.

Yep, qemu-img convert should be able to copy it into a single image so
you can delete the chain.

Cheers,
Jes



[Qemu-devel] Re: [PATCH] kvm: ppc: Fix breakage of kvm_arch_pre_run/process_irqchip_events

2011-03-10 Thread Stefan Hajnoczi
On Fri, Mar 11, 2011 at 5:55 AM, Alexander Graf ag...@suse.de wrote:

 On 17.02.2011, at 22:01, Jan Kiszka wrote:

 On 2011-02-07 12:19, Jan Kiszka wrote:
 We do not check them, and the only arch with non-empty implementations
 always returns 0 (this is also true for qemu-kvm).

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 CC: Alexander Graf ag...@suse.de
 ---
 kvm.h              |    5 ++---
 target-i386/kvm.c  |    8 ++--
 target-ppc/kvm.c   |    6 ++
 target-s390x/kvm.c |    6 ++
 4 files changed, 8 insertions(+), 17 deletions(-)


 ...

 diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
 index 93ecc57..bd4012a 100644
 --- a/target-ppc/kvm.c
 +++ b/target-ppc/kvm.c
 @@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run 
 *run)
     return 0;
 }

 -int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 +void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 {
 -    return 0;
 }

 -int kvm_arch_process_irqchip_events(CPUState *env)
 +void kvm_arch_process_irqchip_events(CPUState *env)
 {
 -    return 0;
 }

 Oops. Do we already have a built-bot for KVM-enabled PPC (and s390)
 targets somewhere?

 Just before leaving for vacation I prepared a machine for each and gave 
 stefan access to them. Looks like they're not officially running though - 
 will try to look at this asap.

They are in the process of being added to the buildbot by Daniel
Gollub.  However, the ppc box is unable to build qemu.git because it
hits ENOMEM while compiling.  I doubled swap size but that didn't fix
the issue so I need to investigate more.  At least s390 should be good
to go soon and I will send an update when it is up and running.

Stefan



[Qemu-devel] Re: [PATCH] hmp-commands.hx: fix badly merged client_migrate_info command

2011-03-10 Thread Jes Sorensen
On 03/11/11 00:21, Anthony Liguori wrote:
 On 03/09/2011 09:54 AM, jes.soren...@redhat.com wrote:
 From: Jes Sorensenjes.soren...@redhat.com

 client_migrate_info was merged badly,
 
 It wasn't merged badly, it was implemented badly.  The initial
 description confused me because it sounded like a bad merge conflict
 resolution but it just was wrong from the start.

I wasn't quite sure where the badness happened, it basically looked like
a bad cleanup after a git pull. Sorry if I gave the impression that the
merge at your end was to blame.

Cheers,
Jes



  1   2   >