[Qemu-devel] Re: [PULL (resend, rebase) 2/5] virtio-serial: Disallow generic ports at id 0
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
* 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
** 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
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
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
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
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.
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
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
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
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
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