Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)
On 04/21/11 22:58, Michael Roth wrote: On 04/21/2011 09:10 AM, Jes Sorensen wrote: On 04/18/11 17:02, Michael Roth wrote: One thing I cannot seem to figure out with this tree - the agent commands do not seem to show up in the monitor? What am I missing? Hmm, for some reason this email never hit my inbox. You mean with the human monitor? Currently, with these new patches, we're QMP only. And most of the command names/semantics have changed as well. The qapi-schema.json changes in patch 16 have the new prototypes, and the 0 patch has some usage examples. Hi Michael, Yeah it was the conclusion I came to on Thursday when I was working on porting the freeze patches over. After fighting the json %#$%#$%#$ I ended up with something I couldn't test in the end :( Any plans to add human monitor support in the near future? Cheers, Jes
Re: [Qemu-devel] [PATCH v2 0/3] io-thread optimizations
On 2011-04-25 20:35, Aurelien Jarno wrote: On Thu, Apr 14, 2011 at 09:14:35AM +0200, Jan Kiszka wrote: On 2011-04-13 22:16, Aurelien Jarno wrote: On Mon, Apr 11, 2011 at 10:27:41PM +0200, Jan Kiszka wrote: These patches were posted before. They bring down the overhead of the io-thread mode for TCG here, specifically when emulating SMP. The major change in this version, besides rebasing, is the exclusion of KVM from the main loop polling optimization. Jan Kiszka (3): Do not drop global mutex for polled main loop runs Poll main loop after I/O events were received Do not kick vcpus in TCG mode cpus.c |2 +- sysemu.h |2 +- vl.c | 22 -- 3 files changed, 18 insertions(+), 8 deletions(-) Thanks for working on improving the io-thread with TCG. Your patches make sense, but they don't seems to fix the slowdown observed when enabling the io-thread. Well maybe they were not supposed to. This is for example the results of netperf between guest and host using virtio: no io-thread122 MB/s io-thread97 MB/s io-thread + patches 98 MB/s Can you capture ftraces of io-thread enabled disabled runs? They just need to cover a hand full of frames. From what I have been able to get from the ftraces documentation, it's possible multiple tracers. Which tracers would you like to use there? The best would be a set of command lines to run. Sorry, of course: Just download, build install trace-cmd [1], then execute trace-cmd record -b 16000 -e all while qemu is running. The result is written to trace.dat in the current directory. Visualize it via trace-cmd report (or kernelshark if you built that as well). Jan [1] git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/trace-cmd.git signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PULL] linux-user pending patches
The following changes since commit b0b36e5d2e4c8a96c2f6dbc0981a9fd0cde111d8: doc: fix slirp description (2011-04-25 23:10:04 +0200) are available in the git repository at: git://gitorious.org/qemu-maemo/qemu.git linux-user-for-upstream Alexander Graf (1): linux-user: add s390x to llseek list Laurent Vivier (3): linux-user: improve traces linux-user: convert ioctl(SIOCGIFCONF, ...) result. linux-user: add ioctl(SIOCGIWNAME, ...) support. Riku Voipio (2): [v2] linux-user: bigger default stack linux-user: untie syscalls from UID16 linux-user/alpha/syscall_nr.h |7 -- linux-user/ioctls.h |4 +- linux-user/strace.c | 161 + linux-user/strace.list| 12 ++-- linux-user/syscall.c | 154 +++ linux-user/syscall_defs.h |8 ++- 6 files changed, 315 insertions(+), 31 deletions(-)
Re: [Qemu-devel] [PATCH] pflash: Restore fix lazy ROMD switching
On 2011-04-10 12:53, Jan Kiszka wrote:
On 2011-04-10 10:38, Jan Kiszka wrote:
On 2011-04-03 22:16, Jordan Justen wrote:
When checking pfl-rom_mode for when to lazily reenter ROMD mode,
the value was check was the opposite of what it should have been.
This prevent the part from returning to ROMD mode after a write
was made to the CFI rom region.
Signed-off-by: Jordan Justen jordan.l.jus...@intel.com
---
hw/pflash_cfi02.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
index 30c8aa4..370c5ee 100644
--- a/hw/pflash_cfi02.c
+++ b/hw/pflash_cfi02.c
@@ -112,7 +112,7 @@ static uint32_t pflash_read (pflash_t *pfl,
target_phys_addr_t offset,
DPRINTF(%s: offset TARGET_FMT_plx \n, __func__, offset);
ret = -1;
-if (pfl-rom_mode) {
+if (!pfl-rom_mode) {
/* Lazy reset of to ROMD mode */
if (pfl-wcycle == 0)
pflash_register_memory(pfl, 1);
Indeed, that block looks weird to its author today as well. But
inverting the logic completely defeats the purpose of lazy mode
switching (will likely file a patch to remove the block). We should
instead switch back using the timer.
That was wishful thinking. We were actually lacking a switch-back on
read, something that the old twisted logic was papering over. Patch
below should resolve that more gracefully, at least it does so here.
Jan
--8--
Commit 5145b3d1cc revealed a bug in the lazy ROMD switch-back logic, but
resolved it by breaking that feature. This approach addresses the issue
by switching back to ROMD after a certain amount of read accesses
without further unlock sequences.
Signed-off-by: Jan Kiszka jan.kis...@web.de
---
hw/pflash_cfi02.c | 12
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
index 370c5ee..14bbc34 100644
--- a/hw/pflash_cfi02.c
+++ b/hw/pflash_cfi02.c
@@ -50,6 +50,8 @@ do { \
#define DPRINTF(fmt, ...) do { } while (0)
#endif
+#define PFLASH_LAZY_ROMD_THRESHOLD 42
+
struct pflash_t {
BlockDriverState *bs;
target_phys_addr_t base;
@@ -70,6 +72,7 @@ struct pflash_t {
ram_addr_t off;
int fl_mem;
int rom_mode;
+int read_counter; /* used for lazy switch-back to rom mode */
void *storage;
};
@@ -112,10 +115,10 @@ static uint32_t pflash_read (pflash_t *pfl,
target_phys_addr_t offset,
DPRINTF(%s: offset TARGET_FMT_plx \n, __func__, offset);
ret = -1;
-if (!pfl-rom_mode) {
-/* Lazy reset of to ROMD mode */
-if (pfl-wcycle == 0)
-pflash_register_memory(pfl, 1);
+/* Lazy reset to ROMD mode after a certain amount of read accesses */
+if (!pfl-rom_mode pfl-wcycle == 0
+++pfl-read_counter PFLASH_LAZY_ROMD_THRESHOLD) {
+pflash_register_memory(pfl, 1);
}
offset = pfl-chip_len - 1;
boff = offset 0xFF;
@@ -254,6 +257,7 @@ static void pflash_write (pflash_t *pfl,
target_phys_addr_t offset,
/* Set the device in I/O access mode if required */
if (pfl-rom_mode)
pflash_register_memory(pfl, 0);
+pfl-read_counter = 0;
/* We're in read mode */
check_unlock0:
if (boff == 0x55 cmd == 0x98) {
Please apply unless concerns remain.
Jan
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [Bug 697197] Re: Empty password allows access to VNC in libvirt
** Bug watch added: Debian Bug tracker #611134 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=611134 ** Also affects: qemu-kvm (Debian) via http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=611134 Importance: Unknown Status: Unknown -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/697197 Title: Empty password allows access to VNC in libvirt Status in libvirt virtualization API: Unknown Status in QEMU: Confirmed Status in qemu-kvm: Unknown Status in “libvirt” package in Ubuntu: Invalid Status in “qemu-kvm” package in Ubuntu: Fix Released Status in “libvirt” source package in Lucid: Invalid Status in “qemu-kvm” source package in Lucid: Fix Released Status in “libvirt” source package in Maverick: Invalid Status in “qemu-kvm” source package in Maverick: Fix Released Status in “libvirt” source package in Natty: Invalid Status in “qemu-kvm” source package in Natty: Fix Released Status in “libvirt” source package in Karmic: Invalid Status in “qemu-kvm” source package in Karmic: Fix Released Status in “qemu-kvm” package in Debian: Unknown Bug description: The help in the /etc/libvirt/qemu.conf states To allow access without passwords, leave this commented out. An empty string will still enable passwords, but be rejected by QEMU effectively preventing any use of VNC. yet setting: vnc_password= allows access to the vnc console without any password prompt just as if it is hashed out completely. ProblemType: Bug DistroRelease: Ubuntu 10.10 Package: libvirt-bin 0.8.3-1ubuntu14 ProcVersionSignature: Ubuntu 2.6.35-24.42-server 2.6.35.8 Uname: Linux 2.6.35-24-server x86_64 Architecture: amd64 Date: Tue Jan 4 12:18:35 2011 InstallationMedia: Ubuntu-Server 10.04.1 LTS Lucid Lynx - Release amd64 (20100816.2) ProcEnviron: LANG=en_GB.UTF-8 SHELL=/bin/bash SourcePackage: libvirt
[Qemu-devel] [PATCH] rtl8139: Fix compilation for w32/w64
Compilation for Windows needs a different declaration for the
printf format attribute, so use the macro which was defined for
this purpose.
Cc: Benjamin Poirier benjamin.poir...@gmail.com
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
hw/rtl8139.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 623fb0c..b997fe7 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -88,8 +88,7 @@
# define DPRINTF(fmt, ...) \
do { fprintf(stderr, RTL8139: fmt, ## __VA_ARGS__); } while (0)
#else
-static inline __attribute__ ((format (printf, 1, 2)))
-int DPRINTF(const char *fmt, ...)
+static inline GCC_FMT_ATTR(1, 2) int DPRINTF(const char *fmt, ...)
{
return 0;
}
--
1.7.2.5
[Qemu-devel] [PATCH] Fix typo in code and comments
Replace writeable - writable
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
block.c |2 +-
block/sheepdog.c |4 +-
hw/eepro100.c|2 +-
hw/eeprom93xx.c | 10
hw/msi.c |2 +-
hw/msix.c|2 +-
hw/pci.c |6 ++--
hw/pci.h |2 +-
hw/rtl8139.c | 44 ++---
hw/sun4m_iommu.c |2 +-
target-mips/translate_init.c |2 +-
11 files changed, 41 insertions(+), 37 deletions(-)
diff --git a/block.c b/block.c
index f731c7a..8767d31 100644
--- a/block.c
+++ b/block.c
@@ -455,7 +455,7 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
open_flags = flags ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
/*
- * Snapshots should be writeable.
+ * Snapshots should be writable.
*/
if (bs-is_temporary) {
open_flags |= BDRV_O_RDWR;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 98946d7..0392ca8 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -196,7 +196,7 @@ static inline uint64_t fnv_64a_buf(void *buf, size_t len,
uint64_t hval)
return hval;
}
-static inline int is_data_obj_writeable(SheepdogInode *inode, unsigned int idx)
+static inline int is_data_obj_writable(SheepdogInode *inode, unsigned int idx)
{
return inode-vdi_id == inode-data_vdi_id[idx];
}
@@ -1577,7 +1577,7 @@ static void sd_readv_writev_bh_cb(void *p)
create = 1;
} else if (acb-aiocb_type == AIOCB_WRITE_UDATA
-!is_data_obj_writeable(inode, idx)) {
+!is_data_obj_writable(inode, idx)) {
/* Copy-On-Write */
create = 1;
old_oid = oid;
diff --git a/hw/eepro100.c b/hw/eepro100.c
index ffcbc3d..dabb64a 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -1125,7 +1125,7 @@ static void eepro100_write_eeprom(eeprom_t * eeprom,
uint8_t val)
{
TRACE(EEPROM, logout(val=0x%02x\n, val));
-/* mask unwriteable bits */
+/* mask unwritable bits */
#if 0
val = SET_MASKED(val, 0x31, eeprom-value);
#endif
diff --git a/hw/eeprom93xx.c b/hw/eeprom93xx.c
index 660b28f..7b21f98 100644
--- a/hw/eeprom93xx.c
+++ b/hw/eeprom93xx.c
@@ -75,7 +75,7 @@ struct _eeprom_t {
uint8_t tick;
uint8_t address;
uint8_t command;
-uint8_t writeable;
+uint8_t writable;
uint8_t eecs;
uint8_t eesk;
@@ -130,7 +130,7 @@ static const VMStateDescription vmstate_eeprom = {
VMSTATE_UINT8(tick, eeprom_t),
VMSTATE_UINT8(address, eeprom_t),
VMSTATE_UINT8(command, eeprom_t),
-VMSTATE_UINT8(writeable, eeprom_t),
+VMSTATE_UINT8(writable, eeprom_t),
VMSTATE_UINT8(eecs, eeprom_t),
VMSTATE_UINT8(eesk, eeprom_t),
@@ -165,7 +165,7 @@ void eeprom93xx_write(eeprom_t *eeprom, int eecs, int eesk,
int eedi)
address = 0x0;
} else if (eeprom-eecs ! eecs) {
/* End chip select cycle. This triggers write / erase. */
-if (eeprom-writeable) {
+if (eeprom-writable) {
uint8_t subcommand = address (eeprom-addrbits - 2);
if (command == 0 subcommand == 2) {
/* Erase all. */
@@ -232,7 +232,7 @@ void eeprom93xx_write(eeprom_t *eeprom, int eecs, int eesk,
int eedi)
switch (address (eeprom-addrbits - 2)) {
case 0:
logout(write disable command\n);
-eeprom-writeable = 0;
+eeprom-writable = 0;
break;
case 1:
logout(write all command\n);
@@ -242,7 +242,7 @@ void eeprom93xx_write(eeprom_t *eeprom, int eecs, int eesk,
int eedi)
break;
case 3:
logout(write enable command\n);
-eeprom-writeable = 1;
+eeprom-writable = 1;
break;
}
} else {
diff --git a/hw/msi.c b/hw/msi.c
index 3dc3a24..b81ac79 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -155,7 +155,7 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
pci_set_word(dev-wmask + msi_data_off(dev, msi64bit), 0x);
if (msi_per_vector_mask) {
-/* Make mask bits 0 to nr_vectors - 1 writeable. */
+/* Make mask bits 0 to nr_vectors - 1 writable. */
pci_set_long(dev-wmask + msi_mask_off(dev, msi64bit),
0x (PCI_MSI_VECTORS_MAX - nr_vectors));
}
diff --git a/hw/msix.c b/hw/msix.c
index daaf9b7..af40e26 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -87,7 +87,7 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned
short nentries,
pci_set_long(config + MSIX_PBA_OFFSET,
[Qemu-devel] [Bug 697197] Re: Empty password allows access to VNC in libvirt
** Changed in: qemu-kvm (Debian) Status: Unknown = New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/697197 Title: Empty password allows access to VNC in libvirt Status in libvirt virtualization API: Unknown Status in QEMU: Confirmed Status in qemu-kvm: Unknown Status in “libvirt” package in Ubuntu: Invalid Status in “qemu-kvm” package in Ubuntu: Fix Released Status in “libvirt” source package in Lucid: Invalid Status in “qemu-kvm” source package in Lucid: Fix Released Status in “libvirt” source package in Maverick: Invalid Status in “qemu-kvm” source package in Maverick: Fix Released Status in “libvirt” source package in Natty: Invalid Status in “qemu-kvm” source package in Natty: Fix Released Status in “libvirt” source package in Karmic: Invalid Status in “qemu-kvm” source package in Karmic: Fix Released Status in “qemu-kvm” package in Debian: New Bug description: The help in the /etc/libvirt/qemu.conf states To allow access without passwords, leave this commented out. An empty string will still enable passwords, but be rejected by QEMU effectively preventing any use of VNC. yet setting: vnc_password= allows access to the vnc console without any password prompt just as if it is hashed out completely. ProblemType: Bug DistroRelease: Ubuntu 10.10 Package: libvirt-bin 0.8.3-1ubuntu14 ProcVersionSignature: Ubuntu 2.6.35-24.42-server 2.6.35.8 Uname: Linux 2.6.35-24-server x86_64 Architecture: amd64 Date: Tue Jan 4 12:18:35 2011 InstallationMedia: Ubuntu-Server 10.04.1 LTS Lucid Lynx - Release amd64 (20100816.2) ProcEnviron: LANG=en_GB.UTF-8 SHELL=/bin/bash SourcePackage: libvirt
Re: [Qemu-devel] [PATCH] qed: Fix consistency check on 32-bit hosts
Am 24.04.2011 19:38, schrieb Stefan Hajnoczi: The qed_bytes_to_clusters() function is normally used with size_t lengths. Consistency check used it with file size length and therefore failed on 32-bit hosts when the image file is 4 GB or more. Make qed_bytes_to_clusters() explicitly 64-bit and update consistency check to keep 64-bit cluster counts. Reported-by: Michael Tokarev m...@tls.msk.ru Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Thanks, applied to the block branch. Kevin
[Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu_reset
Instead of having an extra reset function at machine level and special
code for processing INIT, move the initialization of halted into the
cpu reset handler.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
hw/pc.c | 12 ++--
target-i386/helper.c |5 -
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 6939c04..8ef86db 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -913,14 +913,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int
level)
}
}
-static void pc_cpu_reset(void *opaque)
-{
-CPUState *env = opaque;
-
-cpu_reset(env);
-env-halted = !cpu_is_bsp(env);
-}
-
static CPUState *pc_new_cpu(const char *cpu_model)
{
CPUState *env;
@@ -934,8 +926,8 @@ static CPUState *pc_new_cpu(const char *cpu_model)
env-cpuid_apic_id = env-cpu_index;
env-apic_state = apic_init(env, env-cpuid_apic_id);
}
-qemu_register_reset(pc_cpu_reset, env);
-pc_cpu_reset(env);
+qemu_register_reset((QEMUResetHandler *)cpu_reset, env);
+cpu_reset(env);
return env;
}
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 89df997..56cca96 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -106,6 +106,10 @@ void cpu_reset(CPUX86State *env)
env-dr[7] = DR7_FIXED_1;
cpu_breakpoint_remove_all(env, BP_CPU);
cpu_watchpoint_remove_all(env, BP_CPU);
+
+#if !defined(CONFIG_USER_ONLY)
+env-halted = !cpu_is_bsp(env);
+#endif
}
void cpu_x86_close(CPUX86State *env)
@@ -1282,7 +1286,6 @@ void do_cpu_init(CPUState *env)
env-interrupt_request = sipi;
env-pat = pat;
apic_init_reset(env-apic_state);
-env-halted = !cpu_is_bsp(env);
}
void do_cpu_sipi(CPUState *env)
Re: [Qemu-devel] kvm crashes with spice while loading qxl
Hi, [ ... back online now ... ] /var/tmp/portage/app-emulation/qemu-kvm-0.14.0/work/qemu-kvm-0.14.0/qemu-kvm.c:1724: kvm_mutex_unlock: Assertion `!cpu_single_env' failed. That's a spice bug. In fact, there are a lot of qemu_mutex_lock/unlock_iothread in that subsystem. I bet at least a few of them can cause even more subtle problems. Two general issues with dropping the global mutex like this: - The caller of mutex_unlock is responsible for maintaining cpu_single_env across the unlocked phase (that's related to the abort above). This is true for qemu-kvm only, right? qemu-kvm specific patches which add the cpu_single_env tracking (not polished yet) are here: http://cgit.freedesktop.org/spice/qemu/log/?h=spice.kvm.v28 - Dropping the lock in the middle of a callback is risky. That may enable re-entrances of code sections that weren't designed for this Hmm, indeed. Spice requires a careful review regarding such issues. Or it should pioneer with introducing its own lock so that we can handle at least related I/O activities over the VCPUs without holding the global mutex (but I bet it's not the simplest candidate for such a new scheme). spice/qxl used to have its own locking scheme. That didn't work out though. spice server is threaded and calls back into qxl from spice thread context, and some of these callbacks need access to qemu data structures (display surface) and thus lock protection which covers more than just the spice subsystem. I'll look hard again whenever I can find a way out of this (preferably drop the need for the global lock somehow). For now I'm pretty busy with the email backlog though ... cheers, Gerd
Re: [Qemu-devel] kvm crashes with spice while loading qxl
On 2011-04-26 10:53, Gerd Hoffmann wrote: Hi, [ ... back online now ... ] /var/tmp/portage/app-emulation/qemu-kvm-0.14.0/work/qemu-kvm-0.14.0/qemu-kvm.c:1724: kvm_mutex_unlock: Assertion `!cpu_single_env' failed. That's a spice bug. In fact, there are a lot of qemu_mutex_lock/unlock_iothread in that subsystem. I bet at least a few of them can cause even more subtle problems. Two general issues with dropping the global mutex like this: - The caller of mutex_unlock is responsible for maintaining cpu_single_env across the unlocked phase (that's related to the abort above). This is true for qemu-kvm only, right? Nope, this applies to both implementations. qemu-kvm specific patches which add the cpu_single_env tracking (not polished yet) are here: http://cgit.freedesktop.org/spice/qemu/log/?h=spice.kvm.v28 Cannot spot that quickly: In which way are they specific to qemu-kvm? If they are, try to focus on upstream first. The qemu-kvm differences are virtually deprecated, and I hope we can remove them really soon now (my patches are all ready). - Dropping the lock in the middle of a callback is risky. That may enable re-entrances of code sections that weren't designed for this Hmm, indeed. Spice requires a careful review regarding such issues. Or it should pioneer with introducing its own lock so that we can handle at least related I/O activities over the VCPUs without holding the global mutex (but I bet it's not the simplest candidate for such a new scheme). spice/qxl used to have its own locking scheme. That didn't work out though. spice server is threaded and calls back into qxl from spice thread context, and some of these callbacks need access to qemu data structures (display surface) and thus lock protection which covers more than just the spice subsystem. I'll look hard again whenever I can find a way out of this (preferably drop the need for the global lock somehow). For now I'm pretty busy with the email backlog though ... Yeah, I can imagine... Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] QEMU: Discussion of separating core functionality vs supportive features
Hi, I think that would work well for spice. Spice uses shared memory from the pci device for both the framebuffer and surfaces/commands, but this is Is that the only DMA do you do? That's good for this model. Yes. Spice does both reads and writes though, so a way to tag pages as dirty is needed. cheers, Gerd
Re: [Qemu-devel] [PATCH] Fix bug with virtio-9p fsync
On Mon, Apr 25, 2011 at 6:54 PM, Sassan Panahinejad sas...@sassan.me.uk wrote:
Thanks for finding and fixing this. Please see this wiki page on
contributing patches to QEMU:
http://wiki.qemu.org/Contribute/SubmitAPatch
v9fs_fsync and possibly others break when asked to operate on a directory.
It does not check fid_type to see if it is operating on a directory and
therefore accesses the wrong element of the fs union.
This error can result in guest applications failing (in my case it was dpkg).
This patch fixes the issue, although there may be other, similar bugs in
virtio-9p.
---
hw/virtio-9p.c | 5 -
1 files changed, 4 insertions(+), 1 deletions(-)
Missing Signed-off-by:.
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..09fb5da 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1875,7 +1875,10 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
v9fs_post_do_fsync(s, pdu, err);
return;
}
- err = v9fs_do_fsync(s, fidp-fs.fd, datasync);
+ if (fidp-fid_type == P9_FID_DIR)
+ err = v9fs_do_fsync(s, dirfd(fidp-fs.dir), datasync);
+ else
+ err = v9fs_do_fsync(s, fidp-fs.fd, datasync);
Please follow QEMU coding style and always use {} with if ... else.
Stefan
Re: [Qemu-devel] [PATCH] net: add drop_packets parameter to -net nic
On Mon, Apr 25, 2011 at 3:06 PM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote: 2011/4/25 Stefan Hajnoczi stefa...@gmail.com: 2011/4/25 Nguyễn Thái Ngọc Duy pclo...@gmail.com: Dropping packets is sometimes perferred behavior. Add drop_packets parameter to NICConf struct and let nic simulation decide how to use it. Only e1000 supports this for now. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation is missing, but I'm not even sure if there's any other user who finds this useful. Can you explain why you are adding this? You are trying to bypass the send queue and drop packets instead? Yes. I have a driver that does connection hand shaking at ethernet level. If anything goes wrong, it disables rx and after a while a new session will be started from higher level. The other end has a timer and keeps sending data until it times out. If this end does not respond properly until the timer is timed out, the other end starts sending connection request packets periodically for a new session. When this end enables rx again, in real world it would receive a fresh req packet and send ack. Because of queuing, it receives packets from old session and sends out a series of nack because it expects req packet. Depends on how long rx is disabled until a new session is started, the driver will have to process all queued (invalid) packets and delay session establishment some more. I think dropping packets will improve this situation. But again, this driver is peculiar. I don't think there are many drivers that do dialog-style like this. The behavior you are describing sounds like a bug in QEMU's network layer. If RX is disabled we should not queue incoming packets. Have you looked into fixing QEMU so that the queue is disabled when RX is disabled? Stefan
[Qemu-devel] KVM call agenda for April 26th
Please, send in any agenda items you are interested in covering. From last week: Tools for resource accounting the virtual machines. Luis Antonio Galindo Castro (FunkyM0nk3y) funkymons...@gmail.com Later, Juan.
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix typo in code and comments
On Tue, Apr 26, 2011 at 9:29 AM, Stefan Weil w...@mail.berlios.de wrote: Replace writeable - writable Why make this change? writeable and writable are both commonly used spellings. Stefan
Re: [Qemu-devel] kvm crashes with spice while loading qxl
On Tue, Apr 26, 2011 at 10:53:04AM +0200, Gerd Hoffmann wrote:
Hi,
[ ... back online now ... ]
/var/tmp/portage/app-emulation/qemu-kvm-0.14.0/work/qemu-kvm-0.14.0/qemu-kvm.c:1724:
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
That's a spice bug. In fact, there are a lot of
qemu_mutex_lock/unlock_iothread in that subsystem. I bet at least a few
of them can cause even more subtle problems.
Two general issues with dropping the global mutex like this:
- The caller of mutex_unlock is responsible for maintaining
cpu_single_env across the unlocked phase (that's related to the
abort above).
This is true for qemu-kvm only, right?
qemu-kvm specific patches which add the cpu_single_env tracking (not
polished yet) are here:
http://cgit.freedesktop.org/spice/qemu/log/?h=spice.kvm.v28
- Dropping the lock in the middle of a callback is risky. That may
enable re-entrances of code sections that weren't designed for this
Hmm, indeed.
Spice requires a careful review regarding such issues. Or it should
pioneer with introducing its own lock so that we can handle at least
related I/O activities over the VCPUs without holding the global mutex
(but I bet it's not the simplest candidate for such a new scheme).
spice/qxl used to have its own locking scheme. That didn't work out
though. spice server is threaded and calls back into qxl from spice
thread context, and some of these callbacks need access to qemu data
structures (display surface) and thus lock protection which covers
more than just the spice subsystem.
I'll look hard again whenever I can find a way out of this
(preferably drop the need for the global lock somehow). For now I'm
pretty busy with the email backlog though ...
We (Hans, Uri, and Me) have already sent a fix for this, it seems to have
passed everyone's testing, and it basically does just that - drops the
use of the mutex. It's in
http://cgit.freedesktop.org/spice/qemu/log/?h=spice.v32.kvm,
see the patches:
qxl/spice-display: move pipe to ssd
qxl: implement get_command in vga mode without locks
qxl/spice: remove qemu_mutex_{un,}lock_iothread around dispatcher
hw/qxl-render: drop cursor locks, replace with pipe
And specifically the comments too.
Alon
cheers,
Gerd
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] kvm crashes with spice while loading qxl
On 04/26/11 11:06, Jan Kiszka wrote: On 2011-04-26 10:53, Gerd Hoffmann wrote: Two general issues with dropping the global mutex like this: - The caller of mutex_unlock is responsible for maintaining cpu_single_env across the unlocked phase (that's related to the abort above). This is true for qemu-kvm only, right? Nope, this applies to both implementations. Oops. qemu-kvm specific patches which add the cpu_single_env tracking (not polished yet) are here: http://cgit.freedesktop.org/spice/qemu/log/?h=spice.kvm.v28 Cannot spot that quickly: In which way are they specific to qemu-kvm? cpu_single_env bookeeping. But if upstream needs that too having specific patches is pretty pointless. I'll go fix it up upstream then. cheers, Gerd
Re: [Qemu-devel] [PATCH] qed: Fix consistency check on 32-bit hosts
On Tue, Apr 26, 2011 at 9:44 AM, Kevin Wolf kw...@redhat.com wrote: Am 24.04.2011 19:38, schrieb Stefan Hajnoczi: The qed_bytes_to_clusters() function is normally used with size_t lengths. Consistency check used it with file size length and therefore failed on 32-bit hosts when the image file is 4 GB or more. Make qed_bytes_to_clusters() explicitly 64-bit and update consistency check to keep 64-bit cluster counts. Reported-by: Michael Tokarev m...@tls.msk.ru Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Thanks, applied to the block branch. Please apply to stable. This fixes a segfault when checking an image on 32-bit hosts. Stefan
[Qemu-devel] [PATCH] handle bind(), listen() race
Hello, we've seen a very occasional failure in the startup of qemu where the
call to inet_listen() for the VNC port fails with EADDRINUSE.
I believe there is a race condition when two qemu processes both bind to the
same port, in one the subsequent call to listen() will succeed and the other
fail. Below is a patch which appears to deal with this scenario but we would
appreciate any comments on it.
Thanks
Simon
diff -ur qemu-0.14.0-org/qemu-sockets.c qemu-0.14.0/qemu-sockets.c
--- qemu-0.14.0-org/qemu-sockets.c 2011-02-16 14:44:05.0 +
+++ qemu-0.14.0/qemu-sockets.c 2011-04-26 10:41:14.472067346 +0100
@@ -158,6 +158,7 @@
/* create socket + bind */
for (e = res; e != NULL; e = e-ai_next) {
+rebind:
getnameinfo((struct sockaddr*)e-ai_addr,e-ai_addrlen,
uaddr,INET6_ADDRSTRLEN,uport,32,
NI_NUMERICHOST | NI_NUMERICSERV);
@@ -182,7 +183,19 @@
if (sockets_debug)
fprintf(stderr,%s: bind(%s,%s,%d): OK\n, __FUNCTION__,
inet_strfamily(e-ai_family), uaddr,
inet_getport(e));
-goto listen;
+if (listen(slisten,1) == 0) {
+ goto listened;
+ }
+ else {
+ int err = errno;
+
+ perror(listen);
+ closesocket(slisten);
+ if (err == EADDRINUSE)
+ goto rebind;
+ freeaddrinfo(res);
+ return -1;
+ }
}
try_next = to (inet_getport(e) = to + port_offset);
if (!try_next || sockets_debug)
@@ -201,13 +214,7 @@
freeaddrinfo(res);
return -1;
-listen:
-if (listen(slisten,1) != 0) {
-perror(listen);
-closesocket(slisten);
-freeaddrinfo(res);
-return -1;
-}
+listened:
snprintf(uport, sizeof(uport), %d, inet_getport(e) - port_offset);
qemu_opt_set(opts, host, uaddr);
qemu_opt_set(opts, port, uport);
Re: [Qemu-devel] [PATCH v2 01/11] minor whitespace/indentation fixes
Stefan Hajnoczi stefa...@gmail.com writes: On Wed, Apr 6, 2011 at 7:33 PM, Lluís xscr...@gmx.net wrote: Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu --- configure | 30 +- hmp-commands.hx | 28 monitor.c | 14 ++-- vl.c | 64 --- 4 files changed, 68 insertions(+), 68 deletions(-) Not tracing related. Whitespace changes make it harder for forks/downstream and clutter git-blame(1). Please drop this patch. I'm not arguing for or against this patch, just want to point to git-blame -w.
Re: [Qemu-devel] [PATCH] net: add drop_packets parameter to -net nic
On Tue, Apr 26, 2011 at 4:14 PM, Stefan Hajnoczi stefa...@gmail.com wrote: The behavior you are describing sounds like a bug in QEMU's network layer. If RX is disabled we should not queue incoming packets. Have you looked into fixing QEMU so that the queue is disabled when RX is disabled? it's in e1000_can_receive(): it can receive if rx is enabled (E1000_RCTL_EN) and has enough buffer, which means if the driver disables rx, packets queue up. Isn't that correct behavior? Sorry I'm new in this area. -- Duy
[Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev.
This fixes regression caused by commit
2d6c1ef40f3678ab47a4d14fb5dadaa486bfcda6
(char: Prevent multiple devices opening same chardev).
-nodefaults -nographic -chardev stdio,id=stdio,mux=on,signal=off -mon stdio
-device virtio-serial-pci -device virtconsole,chardev=stdio -device
isa-serial,chardev=stdio
fails with
qemu-system-x86_64: -device isa-serial,chardev=stdio: Property
'isa-serial.chardev' can't take value 'stdio', it's in use
Signed-off-by: Kusanagi Kouichi sl...@ac.auone-net.jp
---
hw/qdev-properties.c |4 ++--
qemu-char.c |5 -
qemu-char.h |2 +-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 1088a26..eff2d24 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -354,10 +354,10 @@ static int parse_chr(DeviceState *dev, Property *prop,
const char *str)
if (*ptr == NULL) {
return -ENOENT;
}
-if ((*ptr)-assigned) {
+if ((*ptr)-avail_connections 1) {
return -EEXIST;
}
-(*ptr)-assigned = 1;
+--(*ptr)-avail_connections;
return 0;
}
diff --git a/qemu-char.c b/qemu-char.c
index 03858d4..f5c2dbc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -199,7 +199,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
{
if (!opaque) {
/* chr driver being released. */
-s-assigned = 0;
+++s-avail_connections;
}
s-chr_can_read = fd_can_read;
s-chr_read = fd_read;
@@ -2544,7 +2544,10 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
snprintf(base-label, len, %s-base, qemu_opts_id(opts));
chr = qemu_chr_open_mux(base);
chr-filename = base-filename;
+chr-avail_connections = MAX_MUX;
QTAILQ_INSERT_TAIL(chardevs, chr, next);
+} else {
+chr-avail_connections = 1;
}
chr-label = qemu_strdup(qemu_opts_id(opts));
return chr;
diff --git a/qemu-char.h b/qemu-char.h
index fb96eef..f669827 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -70,7 +70,7 @@ struct CharDriverState {
char *label;
char *filename;
int opened;
-int assigned; /* chardev assigned to a device */
+int avail_connections;
QTAILQ_ENTRY(CharDriverState) next;
};
--
1.7.4.4
Re: [Qemu-devel] [PATCH] target-arm: Minimal implementation of performance counters
On Mon, Apr 25, 2011 at 11:35:54PM +0100, Peter Maydell wrote: On 25 April 2011 23:31, Aurelien Jarno aurel...@aurel32.net wrote: On Mon, Apr 25, 2011 at 10:59:52PM +0100, Peter Maydell wrote: On 25 April 2011 22:09, Aurelien Jarno aurel...@aurel32.net wrote: Instead of having this complex test for all cp15 access, but only for catching a few access to performance registers, wouldn't it make more sense to have this test and an exception triggering directly in helper.c? That was what my first design did, but in discussions on IRC with Paul Brook he basically said that you can't generate an exception in the helper routine, you have to either generate runtime code to do the test or throw away the TBs. Unfortunately I forget the exact rationale, so I've cc'd Paul to remind me :-) This is something strange, plenty of targets are raising exceptions from helpers without any problem. You'd at minimum need to move the cp15 helper functions to a different file, they're currently in helper.c which doesn't get compiled with access to the global 'env' register. But I got the impression there was something more significant than that. I agree, but it's something that has to be done sooner or later anyway. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] pSeries build breakage
On Mon, Apr 25, 2011 at 06:42:25PM +0200, Andreas Färber wrote: Hello, Building QEMU HEAD (347ac8e35661eff1c2b5ec74d11ee152f2a61856 target- i386: switch to softfloat) on OSX/ppc64 results in: [...] LINK arm-softmmu/qemu-system-arm make: *** pc-bios/spapr-rtas: No such file or directory. Stop. make: *** [romsubdir-spapr-rtas] Error 2 Could this be a VPATH issue? The source pc-bios dir does have such a directory but not the build dir. I think it probably is. I hit something else when fiddling with some debian packaging, seemed to go away when I built in the source directory. The makefile rules for this copied from the ones for optionrom/ and I don't really understand what's going wrong :/ -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
[Qemu-devel] [PATCH] virtio: Move virtio-pci to hw library
This module has no target dependencies (except for target_phys_addr_t size) and can thus be built as part of libhw. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- Makefile.objs |1 + Makefile.target |1 - 2 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index fad1dce..3192bf5 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -171,6 +171,7 @@ user-obj-y += cutils.o cache-utils.o hw-obj-y = hw-obj-y += vl.o loader.o hw-obj-$(CONFIG_VIRTIO) += virtio.o virtio-console.o +hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o hw-obj-y += fw_cfg.o hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o hw-obj-$(CONFIG_PCI) += msix.o msi.o diff --git a/Makefile.target b/Makefile.target index cfc7091..5da9e59 100644 --- a/Makefile.target +++ b/Makefile.target @@ -191,7 +191,6 @@ obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o # need to fix this properly obj-$(CONFIG_NO_PCI) += pci-stub.o obj-$(CONFIG_VIRTIO) += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o -obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o obj-y += vhost_net.o obj-$(CONFIG_VHOST_NET) += vhost.o obj-$(CONFIG_REALLY_VIRTFS) += virtio-9p.o -- 1.7.1
Re: [Qemu-devel] KVM call agenda for April 26th
On 04/26/11 11:24, Juan Quintela wrote: Please, send in any agenda items you are interested in covering. From last week: Tools for resource accounting the virtual machines. Luis Antonio Galindo Castro (FunkyM0nk3y) funkymons...@gmail.com - Status of glib tree - next steps? Jes
[Qemu-devel] [PATCH] Fix bug with virtio-9p fsync
v9fs_fsync and possibly others break when asked to operate on a directory.
It does not check fid_type to see if it is operating on a directory and
therefore accesses the wrong element of the fs union.
This error can result in guest applications failing (in my case it was dpkg).
This patch fixes the issue, although there may be other, similar bugs in
virtio-9p.
Signed-off-by: Sassan Panahinejad sas...@sassan.me.uk
---
hw/virtio-9p.c |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..cc4fdc8 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1875,7 +1875,11 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
v9fs_post_do_fsync(s, pdu, err);
return;
}
-err = v9fs_do_fsync(s, fidp-fs.fd, datasync);
+if (fidp-fid_type == P9_FID_DIR) {
+err = v9fs_do_fsync(s, dirfd(fidp-fs.dir), datasync);
+} else {
+err = v9fs_do_fsync(s, fidp-fs.fd, datasync);
+}
v9fs_post_do_fsync(s, pdu, err);
}
--
1.7.0.4
[Qemu-devel] [PATCH 6/6] trace: [trace-events] fix print formats in some events
From: Lluís xscr...@gmx.net Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- trace-events |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/trace-events b/trace-events index 8272c86..77c96a5 100644 --- a/trace-events +++ b/trace-events @@ -254,8 +254,8 @@ disable leon3_set_irq(int intno) Set CPU IRQ %d disable leon3_reset_irq(int intno) Reset CPU IRQ %d # spice-qemu-char.c -disable spice_vmc_write(ssize_t out, int len) spice wrottn %lu of requested %zd -disable spice_vmc_read(int bytes, int len) spice read %lu of requested %zd +disable spice_vmc_write(ssize_t out, int len) spice wrottn %zd of requested %d +disable spice_vmc_read(int bytes, int len) spice read %d of requested %d disable spice_vmc_register_interface(void *scd) spice vmc registered interface %p disable spice_vmc_unregister_interface(void *scd) spice vmc unregistered interface %p -- 1.7.4.1
[Qemu-devel] [PATCH 1/6] tracetool: allow ) in trace output string
From: Paolo Bonzini pbonz...@redhat.com
Be greedy in matching the trailing \)* pattern. Otherwise, all the
text in the trace string up to the last closed parenthesis is taken as
part of the prototype.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
scripts/tracetool |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/tracetool b/scripts/tracetool
index 412f695..9912f36 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -51,7 +51,7 @@ get_args()
{
local args
args=${1#*\(}
-args=${args%\)*}
+args=${args%%\)*}
echo $args
}
--
1.7.4.1
[Qemu-devel] [PATCH 4/6] docs/tracing.txt: minor documentation fixes
From: Lluís xscr...@gmx.net Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- docs/tracing.txt | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/tracing.txt b/docs/tracing.txt index 905a083..c99a0f2 100644 --- a/docs/tracing.txt +++ b/docs/tracing.txt @@ -26,14 +26,14 @@ for debugging, profiling, and observing execution. == Trace events == -There is a set of static trace events declared in the trace-events source +There is a set of static trace events declared in the trace-events source file. Each trace event declaration names the event, its arguments, and the format string which can be used for pretty-printing: qemu_malloc(size_t size, void *ptr) size %zu ptr %p qemu_free(void *ptr) ptr %p -The trace-events file is processed by the tracetool script during build to +The trace-events file is processed by the tracetool script during build to generate code for the trace events. Trace events are invoked directly from source code like this: @@ -52,10 +52,10 @@ source code like this: === Declaring trace events === -The tracetool script produces the trace.h header file which is included by +The tracetool script produces the trace.h header file which is included by every source file that uses trace events. Since many source files include -trace.h, it uses a minimum of types and other header files included to keep -the namespace clean and compile times and dependencies down. +trace.h, it uses a minimum of types and other header files included to keep the +namespace clean and compile times and dependencies down. Trace events should use types as follows: @@ -110,10 +110,10 @@ portability macros, ensure they are preceded and followed by double quotes: == Trace backends == -The tracetool script automates tedious trace event code generation and also +The tracetool script automates tedious trace event code generation and also keeps the trace event declarations independent of the trace backend. The trace events are not tightly coupled to a specific trace backend, such as LTTng or -SystemTap. Support for trace backends can be added by extending the tracetool +SystemTap. Support for trace backends can be added by extending the tracetool script. The trace backend is chosen at configure time and only one trace backend can @@ -181,12 +181,12 @@ events at runtime inside QEMU: Analyzing trace files The simple backend produces binary trace files that can be formatted with the -simpletrace.py script. The script takes the trace-events file and the binary +simpletrace.py script. The script takes the trace-events file and the binary trace: ./simpletrace.py trace-events trace-12345 -You must ensure that the same trace-events file was used to build QEMU, +You must ensure that the same trace-events file was used to build QEMU, otherwise trace event declarations may have changed and output will not be consistent. -- 1.7.4.1
[Qemu-devel] [PULL 0/6] Tracing patches
The following changes since commit b0b36e5d2e4c8a96c2f6dbc0981a9fd0cde111d8: doc: fix slirp description (2011-04-25 23:10:04 +0200) are available in the git repository at: git://repo.or.cz/qemu/stefanha.git tracing Lluís (3): docs/tracing.txt: minor documentation fixes trace: [ust] fix generation of 'trace.c' on events without args trace: [trace-events] fix print formats in some events Paolo Bonzini (1): tracetool: allow ) in trace output string Stefan Hajnoczi (2): trace: Remove %s in grlib trace events docs: Trace events must not expect pointer dereferencing docs/tracing.txt | 23 ++- hw/grlib_apbuart.c |2 +- hw/grlib_gptimer.c | 29 ++--- hw/grlib_irqmp.c |4 ++-- scripts/tracetool |9 + trace-events | 14 +++--- 6 files changed, 43 insertions(+), 38 deletions(-)
[Qemu-devel] [PATCH 5/6] trace: [ust] fix generation of 'trace.c' on events without args
From: Lluís xscr...@gmx.net
Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
scripts/tracetool |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/scripts/tracetool b/scripts/tracetool
index 9912f36..2155a57 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -338,6 +338,7 @@ linetoc_ust()
name=$(get_name $1)
args=$(get_args $1)
argnames=$(get_argnames $1, ,)
+[ -z $argnames ] || argnames=, $argnames
fmt=$(get_fmt $1)
cat EOF
@@ -345,7 +346,7 @@ DEFINE_TRACE(ust_$name);
static void ust_${name}_probe($args)
{
-trace_mark(ust, $name, $fmt, $argnames);
+trace_mark(ust, $name, $fmt$argnames);
}
EOF
@@ -488,7 +489,7 @@ EOF
cat EOF
$arg = \$arg$i;
EOF
- i=$((i+1))
+i=$((i+1))
done
cat EOF
@@ -585,7 +586,7 @@ tracetostap()
exit 1
fi
if [ -z $probeprefix ]; then
- probeprefix=qemu.$targettype.$targetarch;
+probeprefix=qemu.$targettype.$targetarch;
fi
echo /* This file is autogenerated by tracetool, do not edit. */
convert stap
--
1.7.4.1
[Qemu-devel] [PATCH 2/6] trace: Remove %s in grlib trace events
Trace events cannot use %s in their format strings because trace
backends vary in how they can deference pointers (if at all). Recording
const char * values is not meaningful if their contents are not recorded
too.
Change grlib trace events that rely on strings so that they communicate
similar information without using strings.
A follow-up patch explains this limitation and updates docs/tracing.txt.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
hw/grlib_apbuart.c |2 +-
hw/grlib_gptimer.c | 29 ++---
hw/grlib_irqmp.c |4 ++--
trace-events | 10 +-
4 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
index 101b150..169a56e 100644
--- a/hw/grlib_apbuart.c
+++ b/hw/grlib_apbuart.c
@@ -133,7 +133,7 @@ grlib_apbuart_writel(void *opaque, target_phys_addr_t addr,
uint32_t value)
break;
}
-trace_grlib_apbuart_unknown_register(write, addr);
+trace_grlib_apbuart_writel_unknown(addr, value);
}
static CPUReadMemoryFunc * const grlib_apbuart_read[] = {
diff --git a/hw/grlib_gptimer.c b/hw/grlib_gptimer.c
index 596a900..99e9033 100644
--- a/hw/grlib_gptimer.c
+++ b/hw/grlib_gptimer.c
@@ -165,15 +165,15 @@ static uint32_t grlib_gptimer_readl(void *opaque,
target_phys_addr_t addr)
/* Unit registers */
switch (addr) {
case SCALER_OFFSET:
-trace_grlib_gptimer_readl(-1, scaler:, unit-scaler);
+trace_grlib_gptimer_readl(-1, addr, unit-scaler);
return unit-scaler;
case SCALER_RELOAD_OFFSET:
-trace_grlib_gptimer_readl(-1, reload:, unit-reload);
+trace_grlib_gptimer_readl(-1, addr, unit-reload);
return unit-reload;
case CONFIG_OFFSET:
-trace_grlib_gptimer_readl(-1, config:, unit-config);
+trace_grlib_gptimer_readl(-1, addr, unit-config);
return unit-config;
default:
@@ -189,17 +189,16 @@ static uint32_t grlib_gptimer_readl(void *opaque,
target_phys_addr_t addr)
switch (timer_addr) {
case COUNTER_OFFSET:
value = ptimer_get_count(unit-timers[id].ptimer);
-trace_grlib_gptimer_readl(id, counter value:, value);
+trace_grlib_gptimer_readl(id, addr, value);
return value;
case COUNTER_RELOAD_OFFSET:
value = unit-timers[id].reload;
-trace_grlib_gptimer_readl(id, reload value:, value);
+trace_grlib_gptimer_readl(id, addr, value);
return value;
case CONFIG_OFFSET:
-trace_grlib_gptimer_readl(id, scaler value:,
- unit-timers[id].config);
+trace_grlib_gptimer_readl(id, addr, unit-timers[id].config);
return unit-timers[id].config;
default:
@@ -208,7 +207,7 @@ static uint32_t grlib_gptimer_readl(void *opaque,
target_phys_addr_t addr)
}
-trace_grlib_gptimer_unknown_register(read, addr);
+trace_grlib_gptimer_readl(-1, addr, 0);
return 0;
}
@@ -226,19 +225,19 @@ grlib_gptimer_writel(void *opaque, target_phys_addr_t
addr, uint32_t value)
case SCALER_OFFSET:
value = 0x; /* clean up the value */
unit-scaler = value;
-trace_grlib_gptimer_writel(-1, scaler:, unit-scaler);
+trace_grlib_gptimer_writel(-1, addr, unit-scaler);
return;
case SCALER_RELOAD_OFFSET:
value = 0x; /* clean up the value */
unit-reload = value;
-trace_grlib_gptimer_writel(-1, reload:, unit-reload);
+trace_grlib_gptimer_writel(-1, addr, unit-reload);
grlib_gptimer_set_scaler(unit, value);
return;
case CONFIG_OFFSET:
/* Read Only (disable timer freeze not supported) */
-trace_grlib_gptimer_writel(-1, config (Read Only):, 0);
+trace_grlib_gptimer_writel(-1, addr, 0);
return;
default:
@@ -253,18 +252,18 @@ grlib_gptimer_writel(void *opaque, target_phys_addr_t
addr, uint32_t value)
/* GPTimer registers */
switch (timer_addr) {
case COUNTER_OFFSET:
-trace_grlib_gptimer_writel(id, counter:, value);
+trace_grlib_gptimer_writel(id, addr, value);
unit-timers[id].counter = value;
grlib_gptimer_enable(unit-timers[id]);
return;
case COUNTER_RELOAD_OFFSET:
-trace_grlib_gptimer_writel(id, reload:, value);
+trace_grlib_gptimer_writel(id, addr, value);
unit-timers[id].reload = value;
return;
case CONFIG_OFFSET:
-trace_grlib_gptimer_writel(id, config:, value);
+trace_grlib_gptimer_writel(id, addr, value);
if (value GPTIMER_INT_PENDING) {
/* clear pending bit */
@@ -297,7 +296,7 @@ grlib_gptimer_writel(void *opaque, target_phys_addr_t addr,
uint32_t value)
}
-
[Qemu-devel] [PATCH 3/6] docs: Trace events must not expect pointer dereferencing
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- docs/tracing.txt |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/docs/tracing.txt b/docs/tracing.txt index f15069c..905a083 100644 --- a/docs/tracing.txt +++ b/docs/tracing.txt @@ -69,6 +69,11 @@ Trace events should use types as follows: cannot include all user-defined struct declarations and it is therefore necessary to use void * for pointers to structs. + Pointers (including char *) cannot be dereferenced easily (or at all) in + some trace backends. If pointers are used, ensure they are meaningful by + themselves and do not assume the data they point to will be traced. Do + not pass in string arguments. + * For everything else, use primitive scalar types (char, int, long) with the appropriate signedness. -- 1.7.4.1
[Qemu-devel] [PULL] Trivial patches
Only one patch but I want to keep them flowing regularly. The following changes since commit b0b36e5d2e4c8a96c2f6dbc0981a9fd0cde111d8: doc: fix slirp description (2011-04-25 23:10:04 +0200) are available in the git repository at: git://repo.or.cz/qemu/stefanha.git trivial-patches Brad Hards (1): vl: trivial spelling fix vl.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
[Qemu-devel] [PATCH] vl: trivial spelling fix
From: Brad Hards br...@frogmouth.net Signed-off-by: Brad Hards br...@frogmouth.net Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- vl.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/vl.c b/vl.c index 68c3b53..b46ee66 100644 --- a/vl.c +++ b/vl.c @@ -756,7 +756,7 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev, /* * This function returns null terminated string that consist of new line - * separated device pathes. + * separated device paths. * * memory pointed by size is assigned total length of the array in bytes * -- 1.7.4.1
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On 04/25/2011 08:10 PM, Paolo Bonzini wrote: On 04/25/2011 12:27 PM, Lluís wrote: But in any case, I'm still not sure if stderr should have programatic tracing state controls. Yes, please, stderr is even more useful than simple when you're using it under gdb. Agreed, trace control seems really useful with stderr, and we should be able to do this in a generic way (shared by simple and stderr backends). -- Fabien Chouteau
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On Tue, Apr 26, 2011 at 1:30 PM, Fabien Chouteau chout...@adacore.com wrote: On 04/25/2011 08:10 PM, Paolo Bonzini wrote: On 04/25/2011 12:27 PM, Lluís wrote: But in any case, I'm still not sure if stderr should have programatic tracing state controls. Yes, please, stderr is even more useful than simple when you're using it under gdb. Agreed, trace control seems really useful with stderr, and we should be able to do this in a generic way (shared by simple and stderr backends). The commonality between stderr and simple is having a set of trace events with on/off states. Generating trace.h/trace.c is mostly common. Toggling trace event states from the monitor as well as -trace events=file are common. The simple backend additionally allows setting and flushing the output file. It also supports dumping the trace buffer. Stefan
Re: [Qemu-devel] KVM call agenda for April 26th
On 2011-04-26 11:24, Juan Quintela wrote: Please, send in any agenda items you are interested in covering. From last week: Tools for resource accounting the virtual machines. Luis Antonio Galindo Castro (FunkyM0nk3y) funkymons...@gmail.com - status of QCFG (would be nice-to-have for building machine options on top) - qemu-kvm merge - status - next steps, specifically: - upstreaming in-kernel irqchip support Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Qemu 0.14.1 last call for stable patches
It has come time to do the 0.14.1 stable release. If there are any patches which should make this release, please send them along. Remember, to be included, they need to be in the development tree already, and bug fixes only. I will cherry pick as appropriate. Thanks, Justin
Re: [Qemu-devel] [PATCH] Fix bug with virtio-9p fsync
On Tue, Apr 26, 2011 at 1:14 PM, Sassan Panahinejad sas...@sassan.me.uk wrote:
v9fs_fsync and possibly others break when asked to operate on a directory.
It does not check fid_type to see if it is operating on a directory and
therefore accesses the wrong element of the fs union.
This error can result in guest applications failing (in my case it was dpkg).
This patch fixes the issue, although there may be other, similar bugs in
virtio-9p.
Signed-off-by: Sassan Panahinejad sas...@sassan.me.uk
---
hw/virtio-9p.c | 6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..cc4fdc8 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1875,7 +1875,11 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
v9fs_post_do_fsync(s, pdu, err);
return;
}
- err = v9fs_do_fsync(s, fidp-fs.fd, datasync);
+ if (fidp-fid_type == P9_FID_DIR) {
+ err = v9fs_do_fsync(s, dirfd(fidp-fs.dir), datasync);
+ } else {
+ err = v9fs_do_fsync(s, fidp-fs.fd, datasync);
+ }
What about P9_FID_XATTR, seems like we have the same issue there too?
wstat, lock, and getlock need closer auditing and perhaps fixing.
Stefan
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On 04/26/2011 02:38 PM, Stefan Hajnoczi wrote: The simple backend additionally allows setting and flushing the output file. It also supports dumping the trace buffer. I agree that neither of these would be a particularly interesting addition to the stderr backend. Paolo
Re: [Qemu-devel] [PATCH] ioapic: Do not set irr for masked edge IRQs
On 2011-04-09 13:18, Jan Kiszka wrote:
From: Jan Kiszka jan.kis...@siemens.com
So far we set IRR for edge IRQs even if the pin is masked. If the guest
later on unmasks and switches the pin to level-triggered mode, irr will
remain set, causing an IRQ storm. The point is that setting IRR is not
correct in this case according to the spec, and avoiding this resolves
the issue.
Reported-and-tested-by: Isaku Yamahata yamah...@valinux.co.jp
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
hw/ioapic.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 569327d..6c26e82 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -160,8 +160,9 @@ static void ioapic_set_irq(void *opaque, int vector, int
level)
s-irr = ~mask;
}
} else {
-/* edge triggered */
-if (level) {
+/* According to the 82093AA manual, we must ignore edge requests
+ * if the input pin is masked. */
+if (level !(entry IOAPIC_LVT_MASKED)) {
s-irr |= mask;
ioapic_service(s);
}
Ping?
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] KVM call agenda for April 26th
On 04/26/2011 06:47 AM, Jes Sorensen wrote: On 04/26/11 11:24, Juan Quintela wrote: Please, send in any agenda items you are interested in covering. From last week: Tools for resource accounting the virtual machines. Luis Antonio Galindo Castro (FunkyM0nk3y)funkymons...@gmail.com - Status of glib tree - next steps? I actually decided to just rewriting all of QEMU in Python and C++.. ;-) Regards, Anthony Liguori Jes
Re: [Qemu-devel] KVM call agenda for April 26th
On 04/26/2011 07:55 AM, Jan Kiszka wrote: On 2011-04-26 11:24, Juan Quintela wrote: Please, send in any agenda items you are interested in covering. From last week: Tools for resource accounting the virtual machines. Luis Antonio Galindo Castro (FunkyM0nk3y)funkymons...@gmail.com - status of QCFG (would be nice-to-have for building machine options on top) I have a new code generator that makes QCFG much less intrusive. I've had some urgent things come up over the past couple weeks but hope to get some more time to spend on the patches very soon. I think the best merge strategy is probably going to be very incremental given my recent slowness so I'll see about trying to get a first mergable series in the next couple days. Regards, Anthony Liguori - qemu-kvm merge - status - next steps, specifically: - upstreaming in-kernel irqchip support Jan
Re: [Qemu-devel] QEMU: Discussion of separating core functionality vs supportive features
On 04/26/2011 04:14 AM, Gerd Hoffmann wrote: Hi, I think that would work well for spice. Spice uses shared memory from the pci device for both the framebuffer and surfaces/commands, but this is Is that the only DMA do you do? That's good for this model. Yes. Spice does both reads and writes though, so a way to tag pages as dirty is needed. Just implementing Spice as it currently is in a separate process isn't going to be useful IMHO. I would think that the best approach would be to parse all of the ring requests in QEMU itself, and issue higher level commands to a separate process. You can still have the video memory segment mapped in a separate process but QEMU should know enough about what's going on to take care of dirtying the memory. Sort of like how we deal with SCSI passthrough. We interpret enough of the command to hand it off to something else and then handle the return logic. Having QEMU as an intermediary is important to preserve our current security model. We shouldn't be passing unsanitized guest input to an external process. Regards, Anthony Liguori cheers, Gerd
[Qemu-devel] [PATCH] [trivial] fix -virtfs ? help to match reality
The correct option is mount_tag, while helpt text says mnt_tag.
Addresses Debian #623858.
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
vl.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/vl.c b/vl.c
index 68c3b53..d141a16 100644
--- a/vl.c
+++ b/vl.c
@@ -2468,7 +2468,7 @@ int main(int argc, char **argv, char **envp)
qemu_opt_get(opts, security_model) == NULL) {
fprintf(stderr, Usage: -virtfs fstype,path=/share_path/,
security_model=[mapped|passthrough|none],
-mnt_tag=tag.\n);
+mount_tag=tag.\n);
exit(1);
}
--
1.7.2.5
Re: [Qemu-devel] [PULL] Trivial patches
On 04/26/2011 07:29 AM, Stefan Hajnoczi wrote: Only one patch but I want to keep them flowing regularly. The following changes since commit b0b36e5d2e4c8a96c2f6dbc0981a9fd0cde111d8: doc: fix slirp description (2011-04-25 23:10:04 +0200) are available in the git repository at: git://repo.or.cz/qemu/stefanha.git trivial-patches Pulled. Thanks. Regards, Anthony Liguori Brad Hards (1): vl: trivial spelling fix vl.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Re: [Qemu-devel] [PULL 0/6] Tracing patches
On 04/26/2011 07:25 AM, Stefan Hajnoczi wrote: The following changes since commit b0b36e5d2e4c8a96c2f6dbc0981a9fd0cde111d8: doc: fix slirp description (2011-04-25 23:10:04 +0200) are available in the git repository at: git://repo.or.cz/qemu/stefanha.git tracing Pulled. Thanks. Regards, Anthony Liguori Lluís (3): docs/tracing.txt: minor documentation fixes trace: [ust] fix generation of 'trace.c' on events without args trace: [trace-events] fix print formats in some events Paolo Bonzini (1): tracetool: allow ) in trace output string Stefan Hajnoczi (2): trace: Remove %s in grlib trace events docs: Trace events must not expect pointer dereferencing docs/tracing.txt | 23 ++- hw/grlib_apbuart.c |2 +- hw/grlib_gptimer.c | 29 ++--- hw/grlib_irqmp.c |4 ++-- scripts/tracetool |9 + trace-events | 14 +++--- 6 files changed, 43 insertions(+), 38 deletions(-)
Re: [Qemu-devel] [RFC][PATCH v2 09/17] qmp proxy: core code for proxying qmp requests to guest
On Mon, Apr 18, 2011 at 4:02 PM, Michael Roth mdr...@linux.vnet.ibm.com wrote:
+static int qmp_proxy_cancel_request(QmpProxy *p, QmpProxyRequest *r)
+{
+ if (r r-cb) {
+ r-cb(r-opaque, NULL, NULL);
+ }
+
+ return 0;
+}
+
+static int qmp_proxy_cancel_all(QmpProxy *p)
+{
+ QmpProxyRequest *r, *tmp;
+ QTAILQ_FOREACH_SAFE(r, p-requests, entry, tmp) {
+ qmp_proxy_cancel_request(p, r);
+ QTAILQ_REMOVE(p-requests, r, entry);
+ }
+
+ return 0;
+}
qmp_proxy_cancel_all() will remove requests from the list.
qmp_proxy_cancel_request() will not remove it from the list. This
could cause confusion in the future if someone adds a call to
qmp_proxy_cancel_request() without realizing that it will not remove
the request from the list. The two function's names are similar, it
would be nice if they acted the same way.
+static void qmp_proxy_process_event(JSONMessageParser *parser, QList *tokens)
+{
+ QmpProxy *p = container_of(parser, QmpProxy, parser);
+ QmpProxyRequest *r;
+ QObject *obj;
+ QDict *qdict;
+ Error *err = NULL;
+
+ fprintf(stderr, qmp proxy: called\n);
+ obj = json_parser_parse_err(tokens, NULL, err);
+ if (!obj) {
+ fprintf(stderr, qmp proxy: failed to parse\n);
+ return;
+ } else {
+ fprintf(stderr, qmp proxy: parse successful\n);
+ qdict = qobject_to_qdict(obj);
+ }
+
+ if (qdict_haskey(qdict, _control_event)) {
+ /* handle transport-level control event */
+ qmp_proxy_process_control_event(p, qdict);
+ } else if (qdict_haskey(qdict, return)) {
+ /* handle proxied qmp command response */
+ fprintf(stderr, received return\n);
+ r = QTAILQ_FIRST(p-requests);
+ if (!r) {
+ fprintf(stderr, received return, but no request queued\n);
QDECREF(qdict)?
+ return;
+ }
+ /* XXX: can't assume type here */
+ fprintf(stderr, recieved response for cmd: %s\nreturn: %s\n,
+ r-name, qstring_get_str(qobject_to_json(QOBJECT(qdict;
+ r-cb(r-opaque, qdict_get(qdict, return), NULL);
+ QTAILQ_REMOVE(p-requests, r, entry);
+ qemu_free(r);
+ fprintf(stderr, done handling response\n);
+ } else {
+ fprintf(stderr, received invalid payload format\n);
+ }
+
+ QDECREF(qdict);
+}
+void qmp_proxy_send_request(QmpProxy *p, const char *name,
+ const QDict *args, Error **errp,
+ QmpGuestCompletionFunc *cb, void *opaque)
+{
+ QmpProxyRequest *r = qemu_mallocz(sizeof(QmpProxyRequest));
+ QDict *payload = qdict_new();
+ QString *json;
+
+ /* TODO: don't really need to hold on to name/args after encoding */
+ r-name = name;
+ r-args = args;
+ r-cb = cb;
+ r-opaque = opaque;
+
+ qdict_put_obj(payload, execute, QOBJECT(qstring_from_str(r-name)));
+ /* TODO: casting a const so we can add it to our dictionary. bad. */
+ qdict_put_obj(payload, arguments, QOBJECT((QDict *)args));
+
+ json = qobject_to_json(QOBJECT((QDict *)payload));
+ if (!json) {
+ goto out_bad;
+ }
+
+ QTAILQ_INSERT_TAIL(p-requests, r, entry);
+ g_string_append(p-tx, qstring_get_str(json));
+ QDECREF(json);
+ qmp_proxy_write(p);
+ return;
+
+out_bad:
+ cb(opaque, NULL, NULL);
+ qemu_free(r);
Need to free payload?
+}
+
+QmpProxy *qmp_proxy_new(CharDriverState *chr)
+{
+ QmpProxy *p = qemu_mallocz(sizeof(QmpProxy));
+
+ signal_init(guest_agent_up_event);
+ signal_init(guest_agent_reset_event);
+
+ /* there's a reason for this madness */
Helpful comment :)
+ p-tx_timer = qemu_new_timer(rt_clock, qmp_proxy_write_handler, p);
+ p-tx_timer_interval = 10;
+ p-tx = g_string_new();
+ p-chr = chr;
+ json_message_parser_init(p-parser, qmp_proxy_process_event);
+ QTAILQ_INIT(p-requests);
+
+ return p;
+}
+
+void qmp_proxy_close(QmpProxy *p)
+{
+ qmp_proxy_cancel_all(p);
+ g_string_free(p-tx, TRUE);
Free tx_timer?
+ qemu_free(p);
+}
[Qemu-devel] Question on qemu build environment.
I have noticed that qemu does not fully function on FreeBSD sparc64. Besides n...@freebsd.org and myself, has anyone tried building and running qemu under FreeBSD sparc64?
Re: [Qemu-devel] [RFC PATCH 0/3 V8] QAPI: add inject-nmi qmp command
On Thu, 21 Apr 2011 11:23:54 +0800 Lai Jiangshan la...@cn.fujitsu.com wrote: Hi, Anthony Liguori Any suggestion? Although all command line interfaces will be converted to to use QMP interfaces in 0.16, I hope inject-nmi come into QAPI earlier, 0.15. I don't know what Anthony thinks about adding new commands like this one that early to the new QMP interface, but adding them to current QMP will certainly cause less code churn on your side. That's what I'd recommend for now.
Re: [Qemu-devel] [PATCH] Fix bug with virtio-9p fsync
On 26 April 2011 13:58, Stefan Hajnoczi stefa...@gmail.com wrote: What about P9_FID_XATTR, seems like we have the same issue there too? wstat, lock, and getlock need closer auditing and perhaps fixing. Stefan Sorry, forgot to hit reply-to-all. Yes, it is probable that those functions will suffer from the same bug. I will have to study XATTR and see how that will be affected. I don't know whether it is possible for these functions to be called for XATTR, and if it is then I do not know the proper way to handle it. Perhaps we should have some function or macro to obtain the correct FD from an fidp structure, which could be used for fsync, wstat, lock and getlock? Sassan
Re: [Qemu-devel] [RFC PATCH 0/3 V8] QAPI: add inject-nmi qmp command
On 04/26/2011 08:26 AM, Luiz Capitulino wrote: On Thu, 21 Apr 2011 11:23:54 +0800 Lai Jiangshanla...@cn.fujitsu.com wrote: Hi, Anthony Liguori Any suggestion? Although all command line interfaces will be converted to to use QMP interfaces in 0.16, I hope inject-nmi come into QAPI earlier, 0.15. I don't know what Anthony thinks about adding new commands like this one that early to the new QMP interface, but adding them to current QMP will certainly cause less code churn on your side. That's what I'd recommend for now. Yeah, sorry, this whole series has been confused in the QAPI discussion. I did not intend for QAPI to be disruptive to current development. As far as I can tell, the last series that was posted (before the QAPI post) still had checkpatch.pl issues (scripts/checkpatch.pl btw) and we had agreed that once that was resolved, it would come in through Luiz's tree. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH v2] vl.c: Replace -virtfs string manipulation with QemuOpts
On Wed, Mar 16, 2011 at 8:31 AM, Stefan Hajnoczi stefa...@linux.vnet.ibm.com wrote: The -virtfs option creates an fsdev representing the pass-through file system and a guest-visible virtio-9p-pci device that can access this file system. This patch replaces the string manipulation used to build and reparse option lists with direct QemuOpts calls. Removing the string manipulation code makes it easier to maintain and less error prone. An error message is also updated to use mount_tag instead of mnt_tag. Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- v2: * Updated error message according to JV's suggestion vl.c | 56 +++- 1 files changed, 19 insertions(+), 37 deletions(-) Please help kill the string manipulation and review + merge this! :) Stefan
Re: [Qemu-devel] [RFC][PATCH v2 15/17] guest agent: qemu-ga daemon
On 04/25/11 14:27, Ian Molton wrote: On Fri, 2011-04-22 at 13:51 +0200, Jes Sorensen wrote: Hiding things you miss when reading the code, it's a classic for people to do if(foo) bleh(); on the same line, and whoever reads the code will expect the action on the next line, especially if foo is a long complex statement. It's one of these 'just don't do it, it bites you in the end' things. Meh. I dont see it that way... Sure, if it was one line out of 20 written that way, it would be weird, but as is, its just part of a block of identical lines. It is a matter of consistency, we allow it in one place, we suddenly have it all over. The moment someone wants to add a slightly more complex case to such a switch statement it is all down the drain. It is way better to stay consistent across the board. I dont really see a parallel with the if() statement either since the condition in the switch() case isnt on the same line as such. I must admit that I only write one-liner if statements if the condition is short though. Writing one-liner if() statements is inherently broken, or you could call it the Perl syndrome. Write-once, read-never. Jes
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
Stefan Hajnoczi writes: On Tue, Apr 26, 2011 at 1:30 PM, Fabien Chouteau chout...@adacore.com wrote: On 04/25/2011 08:10 PM, Paolo Bonzini wrote: On 04/25/2011 12:27 PM, Lluís wrote: But in any case, I'm still not sure if stderr should have programatic tracing state controls. Yes, please, stderr is even more useful than simple when you're using it under gdb. Agreed, trace control seems really useful with stderr, and we should be able to do this in a generic way (shared by simple and stderr backends). The commonality between stderr and simple is having a set of trace events with on/off states. Generating trace.h/trace.c is mostly common. Toggling trace event states from the monitor as well as -trace events=file are common. The simple backend additionally allows setting and flushing the output file. It also supports dumping the trace buffer. Ok, what I was thinking about long time ago is providing some generic trace_* interface for the common cmdline and monitor commands to use (basically list event names and query or change their dynamic state, which is the only common part on all backends). With this, every backend is responsible of providing a suitable implementation. If no implementation is provided, a default one will be used that simply results in qemu erroring out when any of these functionalities is invoked. I think this was already discussed, and the idea was rejected because it seemed too verbose for qemu to implement tracepoint controls when other external tools already exist for such backends (e.g., ust). Maybe providing the default implementation on the previous paragraph would solve the issues. Lluis -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] KVM call agenda for April 26th
On 04/26/11 15:09, Anthony Liguori wrote: On 04/26/2011 06:47 AM, Jes Sorensen wrote: On 04/26/11 11:24, Juan Quintela wrote: Please, send in any agenda items you are interested in covering. From last week: Tools for resource accounting the virtual machines. Luis Antonio Galindo Castro (FunkyM0nk3y)funkymons...@gmail.com - Status of glib tree - next steps? I actually decided to just rewriting all of QEMU in Python and C++.. You'll have to compete with my Java port!
Re: [Qemu-devel] [PATCH] Fix bug with virtio-9p fsync
On 04/26/2011 06:29 AM, Sassan Panahinejad wrote: I will have to study XATTR and see how that will be affected. I don't know whether it is possible for these functions to be called for XATTR, and if it is then I do not know the proper way to handle it. Perhaps we should have some function or macro to obtain the correct FD from an fidp structure, which could be used for fsync, wstat, lock and getlock? I agree; we need some level of macro for this. How about doing that as part of this patch itself? Thanks, JV
Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)
On 04/26/2011 01:57 AM, Jes Sorensen wrote: On 04/21/11 22:58, Michael Roth wrote: On 04/21/2011 09:10 AM, Jes Sorensen wrote: On 04/18/11 17:02, Michael Roth wrote: One thing I cannot seem to figure out with this tree - the agent commands do not seem to show up in the monitor? What am I missing? Hmm, for some reason this email never hit my inbox. You mean with the human monitor? Currently, with these new patches, we're QMP only. And most of the command names/semantics have changed as well. The qapi-schema.json changes in patch 16 have the new prototypes, and the 0 patch has some usage examples. Hi Michael, Yeah it was the conclusion I came to on Thursday when I was working on porting the freeze patches over. After fighting the json %#$%#$%#$ I ended up with something I couldn't test in the end :( I actually worked on getting most of the fsfreeze ported over yesterday, were they any major changes from the last set of patches other than the porting work? If so, feel free to send the patches my way and I'll hack on those a bit, otherwise I plan to have the fsfreeze stuff included in the next re-spin later this week. Any plans to add human monitor support in the near future? The main target will be QMP for the initial patches. But ideally the HMP commands we add will have a pretty close correspondence with QMP. Cheers, Jes
Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)
On 04/26/11 16:27, Michael Roth wrote: On 04/26/2011 01:57 AM, Jes Sorensen wrote: Yeah it was the conclusion I came to on Thursday when I was working on porting the freeze patches over. After fighting the json %#$%#$%#$ I ended up with something I couldn't test in the end :( I actually worked on getting most of the fsfreeze ported over yesterday, were they any major changes from the last set of patches other than the porting work? If so, feel free to send the patches my way and I'll hack on those a bit, otherwise I plan to have the fsfreeze stuff included in the next re-spin later this week. I'll try and post them later today. Any plans to add human monitor support in the near future? The main target will be QMP for the initial patches. But ideally the HMP commands we add will have a pretty close correspondence with QMP. That is unfortunate, QMP is really user unfriendly :( Cheers, Jes
Re: [Qemu-devel] [PATCH] [trivial] fix -virtfs ? help to match reality
On 04/26/2011 06:15 AM, Michael Tokarev wrote:
The correct option is mount_tag, while helpt text says mnt_tag.
Addresses Debian #623858.
Signed-off-by: Michael Tokarevm...@tls.msk.ru
Reviewed-by: Venkateswararao Jujjuri(JV) jv...@linux.vnet.ibm.com
---
vl.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/vl.c b/vl.c
index 68c3b53..d141a16 100644
--- a/vl.c
+++ b/vl.c
@@ -2468,7 +2468,7 @@ int main(int argc, char **argv, char **envp)
qemu_opt_get(opts, security_model) == NULL) {
fprintf(stderr, Usage: -virtfs fstype,path=/share_path/,
security_model=[mapped|passthrough|none],
-mnt_tag=tag.\n);
+mount_tag=tag.\n);
exit(1);
}
Re: [Qemu-devel] [RFC][PATCH v2 09/17] qmp proxy: core code for proxying qmp requests to guest
On 04/26/2011 08:21 AM, Stefan Hajnoczi wrote:
On Mon, Apr 18, 2011 at 4:02 PM, Michael Rothmdr...@linux.vnet.ibm.com wrote:
+static int qmp_proxy_cancel_request(QmpProxy *p, QmpProxyRequest *r)
+{
+if (r r-cb) {
+r-cb(r-opaque, NULL, NULL);
+}
+
+return 0;
+}
+
+static int qmp_proxy_cancel_all(QmpProxy *p)
+{
+QmpProxyRequest *r, *tmp;
+QTAILQ_FOREACH_SAFE(r,p-requests, entry, tmp) {
+qmp_proxy_cancel_request(p, r);
+QTAILQ_REMOVE(p-requests, r, entry);
+}
+
+return 0;
+}
qmp_proxy_cancel_all() will remove requests from the list.
qmp_proxy_cancel_request() will not remove it from the list. This
could cause confusion in the future if someone adds a call to
qmp_proxy_cancel_request() without realizing that it will not remove
the request from the list. The two function's names are similar, it
would be nice if they acted the same way.
+static void qmp_proxy_process_event(JSONMessageParser *parser, QList *tokens)
+{
+QmpProxy *p = container_of(parser, QmpProxy, parser);
+QmpProxyRequest *r;
+QObject *obj;
+QDict *qdict;
+Error *err = NULL;
+
+fprintf(stderr, qmp proxy: called\n);
+obj = json_parser_parse_err(tokens, NULL,err);
+if (!obj) {
+fprintf(stderr, qmp proxy: failed to parse\n);
+return;
+} else {
+fprintf(stderr, qmp proxy: parse successful\n);
+qdict = qobject_to_qdict(obj);
+}
+
+if (qdict_haskey(qdict, _control_event)) {
+/* handle transport-level control event */
+qmp_proxy_process_control_event(p, qdict);
+} else if (qdict_haskey(qdict, return)) {
+/* handle proxied qmp command response */
+fprintf(stderr, received return\n);
+r = QTAILQ_FIRST(p-requests);
+if (!r) {
+fprintf(stderr, received return, but no request queued\n);
QDECREF(qdict)?
+return;
+}
+/* XXX: can't assume type here */
+fprintf(stderr, recieved response for cmd: %s\nreturn: %s\n,
+r-name, qstring_get_str(qobject_to_json(QOBJECT(qdict;
+r-cb(r-opaque, qdict_get(qdict, return), NULL);
+QTAILQ_REMOVE(p-requests, r, entry);
+qemu_free(r);
+fprintf(stderr, done handling response\n);
+} else {
+fprintf(stderr, received invalid payload format\n);
+}
+
+QDECREF(qdict);
+}
+void qmp_proxy_send_request(QmpProxy *p, const char *name,
+const QDict *args, Error **errp,
+QmpGuestCompletionFunc *cb, void *opaque)
+{
+QmpProxyRequest *r = qemu_mallocz(sizeof(QmpProxyRequest));
+QDict *payload = qdict_new();
+QString *json;
+
+/* TODO: don't really need to hold on to name/args after encoding */
+r-name = name;
+r-args = args;
+r-cb = cb;
+r-opaque = opaque;
+
+qdict_put_obj(payload, execute, QOBJECT(qstring_from_str(r-name)));
+/* TODO: casting a const so we can add it to our dictionary. bad. */
+qdict_put_obj(payload, arguments, QOBJECT((QDict *)args));
+
+json = qobject_to_json(QOBJECT((QDict *)payload));
+if (!json) {
+goto out_bad;
+}
+
+QTAILQ_INSERT_TAIL(p-requests, r, entry);
+g_string_append(p-tx, qstring_get_str(json));
+QDECREF(json);
+qmp_proxy_write(p);
+return;
+
+out_bad:
+cb(opaque, NULL, NULL);
+qemu_free(r);
Need to free payload?
+}
+
+QmpProxy *qmp_proxy_new(CharDriverState *chr)
+{
+QmpProxy *p = qemu_mallocz(sizeof(QmpProxy));
+
+signal_init(guest_agent_up_event);
+signal_init(guest_agent_reset_event);
+
+/* there's a reason for this madness */
Helpful comment :)
+p-tx_timer = qemu_new_timer(rt_clock, qmp_proxy_write_handler, p);
+p-tx_timer_interval = 10;
+p-tx = g_string_new();
+p-chr = chr;
+json_message_parser_init(p-parser, qmp_proxy_process_event);
+QTAILQ_INIT(p-requests);
+
+return p;
+}
+
+void qmp_proxy_close(QmpProxy *p)
+{
+qmp_proxy_cancel_all(p);
+g_string_free(p-tx, TRUE);
Free tx_timer?
+qemu_free(p);
+}
All good catches/suggestions, thanks.
[Qemu-devel] KVM call minutes for Apr 26
Tools for resource accounting the virtual machines. - Luis Castro was not on the call Status of glib tree - next steps? - full conversion done in tree - still targeting 0.15 status of QCFG - code generator rewritten to be more generic and useful - merge core infrastructure first - to not block other work waiting on full conversion - still need to complete full conversion qemu-kvm merge - status - review and merge/feedback pending from Avi on current outstanding patches - still have some 60 patches - break them into a few smaller series - next steps, specifically: - upstreaming in-kernel irqchip support - MSI/MSI-X (cleanup and make mergable) - this is a decent amount of work, Jan is solo...anyone want to help? - need to be careful of regressions - add tests to avi's autotest run (e.g., cpu hotplug) - cpu hotplug test initiated from host side - online needs some cooperation in linux - still unclear on what's supported, windows apparently only supports online autotest - had autotest test day, feedback coming on list - some issues with getting set up - having basic common config could be useful KVM Forum reminder - send in your proposals
Re: [Qemu-devel] [PATCH] lsi53c895a: add support for ABORT messages
On 09.03.2011 10:25, Bernhard Kohl wrote: Am 09.03.2011 09:47, schrieb ext Kevin Wolf: Am 09.03.2011 00:04, schrieb Peter Lieven: Am 07.10.2010 um 13:27 schrieb Kevin Wolf: Am 06.09.2010 16:42, schrieb Bernhard Kohl: If these messages are not handled correctly the guest driver may hang. Always mandatory: - ABORT - BUS DEVICE RESET Mandatory if tagged queuing is implemented (which disks usually do): - ABORT TAG - CLEAR QUEUE Signed-off-by: Bernhard Kohlbernhard.k...@nsn.com Nicholas, as you seem to have touched the lsi code recently, care to review this one? Assuming that you are reasonably familiar with both the hardware and the code, you should be quicker than me with this. Is there a reason why this patch was never added to the stable qemu-kvm release? At least int qemu-kvm-0.14.0 I still can't find it. The reason is that it still didn't get a review. I still depend on this patch. It's needed for our legacy guest OS. It's heavily in use here with STGT iSCSI disks via scsi-generic. i recently saw some qemu-kvm 0.12.5 guests with scsi and this patch applies crashing when we updated our backend iscsi storages. (short interrupt in traffic flow, iscsi disconnect + reconnect) i always see: lsi_scsi: error: ORDERED queue not implemented and then either the maschine just hangs or it even aborts due to this assertion: qemu-kvm-0.12.5: /usr/src/qemu-kvm-0.12.5/hw/lsi53c895a.c:596: lsi_reselect: Assertion `s-current == ((void *)0)' failed. any ideas? peter Bernhard
[Qemu-devel] [Bug 741887] Re: virsh snapshot-create too slow (kvm, qcow2, savevm)
@edison, if you want to push such a patch, please do it through upstream, since it is actually a new feature. I'm going to mark this 'wontfix' (as I thought I had done before), rather than invalid, though the latter still sounds accurate as well. ** Changed in: qemu-kvm (Ubuntu) Status: Confirmed = Won't Fix -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/741887 Title: virsh snapshot-create too slow (kvm, qcow2, savevm) Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Won't Fix Bug description: Action == # time virsh snapshot-create 1 * Taking snapshot of a running KVM virtual machine Result == Domain snapshot 1300983161 created real4m46.994s user0m0.000s sys 0m0.010s Expected result === * Snapshot taken after few seconds instead of minutes. Environment === * Ubuntu Natty Narwhal upgraded from Lucid and Meerkat, fully updated. * Stock natty packages of libvirt and qemu installed (libvirt-bin 0.8.8-1ubuntu5; libvirt0 0.8.8-1ubuntu5; qemu-common 0.14.0+noroms- 0ubuntu3; qemu-kvm 0.14.0+noroms-0ubuntu3). * Virtual machine disk format is qcow2 (debian 5 installed) image: /storage/debian.qcow2 file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 1.2G cluster_size: 65536 Snapshot list: IDTAG VM SIZEDATE VM CLOCK 1 snap01 48M 2011-03-24 09:46:33 00:00:58.899 2 1300979368 58M 2011-03-24 11:09:28 00:01:03.589 3 1300983161 57M 2011-03-24 12:12:41 00:00:51.905 * qcow2 disk is stored on ext4 filesystem, without RAID or LVM or any special setup. * running guest VM takes about 40M RAM from inside, from outside 576M are given to that machine * host has fast dual-core pentium cpu with virtualization support, around 8G of RAM and 7200rpm harddrive (dd from urandom to file gives about 20M/s) * running processes: sshd, atd (empty), crond (empty), libvirtd, tmux, bash, rsyslogd, upstart-socket-bridge, udevd, dnsmasq, iotop (python) * networking is done by bridging and bonding Detail description == * Under root, command 'virsh create-snapshot 1' is issued on booted and running KVM machine with debian inside. * After about four minutes, the process is done. * 'iotop' shows two 'kvm' processes reading/writing to disk. First one has IO around 1500 K/s, second one has around 400 K/s. That takes about three minutes. Then first process grabs about 3 M/s of IO and suddenly dissapears (1-2 sec). Then second process does about 7.5 M/s of IO for around a 1-2 minutes. * Snapshot is successfuly created and is usable for reverting or extracting. * Pretty much the same behaviour occurs when command 'savevm' is issued directly from qemu monitor, without using libvirt at all (actually, virsh snapshot-create just calls 'savevm' to the monitor socket). * This behaviour was observed on lucid, meerkat, natty and even with git version of libvirt (f44bfb7fb978c9313ce050a1c4149bf04aa0a670). Also slowsave packages from https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/524447 gave this issue. Thank you for helping to solve this issue! ProblemType: Bug DistroRelease: Ubuntu 11.04 Package: libvirt-bin 0.8.8-1ubuntu5 ProcVersionSignature: Ubuntu 2.6.38-7.38-server 2.6.38 Uname: Linux 2.6.38-7-server x86_64 Architecture: amd64 Date: Thu Mar 24 12:19:41 2011 InstallationMedia: Ubuntu-Server 10.04.2 LTS Lucid Lynx - Release amd64 (20110211.1) ProcEnviron: LANG=en_US.UTF-8 SHELL=/bin/bash SourcePackage: libvirt UpgradeStatus: No upgrade log present (probably fresh install)
[Qemu-devel] [PATCH] Add QMP fsfreeze support
From: Jes Sorensen jes.soren...@redhat.com
This patch adds the following QMP commands:
qga-guest-fsfreeze:
- Freezes all local file systems in the guest. Command will return
the number of file systems that were frozen.
qga-guest-fsthaw:
- Thaws all local file systems in the guest. Command will return
the number of file systems that were thawed.
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
qapi-schema.json | 24 ++
qga/guest-agent-commands.c | 177
2 files changed, 201 insertions(+), 0 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index e2f209d..2c86cec 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1562,6 +1562,30 @@
{ 'option': 'blockdev', 'data': 'BlockdevConfig', 'implicit': 'id' }
##
+# @guest-fsfreeze:
+#
+# json gibberish for guest-fsfreeze command
+#
+# Returns: Number of file systems frozen
+#
+# Since: 0.15.0
+##
+{ 'command': 'guest-fsfreeze',
+ 'returns': 'int' }
+
+##
+# @guest-fsthaw:
+#
+# json gibberish for guest-fsthaw command
+#
+# Returns: Number of file systems thawed
+#
+# Since: 0.15.0
+##
+{ 'command': 'guest-fsthaw',
+ 'returns': 'int' }
+
+##
# @VncConfig:
#
# Configuration options for the built-in VNC server.
diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
index 843ef36..4bc2c57 100644
--- a/qga/guest-agent-commands.c
+++ b/qga/guest-agent-commands.c
@@ -11,6 +11,10 @@
*/
#include glib.h
+#include mntent.h
+#include sys/types.h
+#include sys/ioctl.h
+#include linux/fs.h
#include guest-agent.h
static bool enable_syslog = true;
@@ -262,6 +266,179 @@ struct GuestFileSeek *qga_guest_file_seek(int64_t
filehandle, int64_t offset,
return seek_data;
}
+/*
+ * Walk the mount table and build a list of local file systems
+ */
+struct direntry {
+char *dirname;
+char *devtype;
+struct direntry *next;
+};
+
+struct va_freezestate {
+struct direntry *mount_list;
+int status;
+};
+
+/*
+ * This should be in a header file, but we have none :(
+ */
+enum va_fsfreeze_status {
+FREEZE_ERROR = -1,
+FREEZE_THAWED = 0,
+FREEZE_INPROGRESS = 1,
+FREEZE_FROZEN = 2,
+FREEZE_THAWINPROGRESS = 3,
+};
+
+static struct va_freezestate freezestate;
+
+static int build_mount_list(void)
+{
+struct mntent *mnt;
+struct direntry *entry;
+struct direntry *next;
+char const *mtab = MOUNTED;
+FILE *fp;
+
+fp = setmntent(mtab, r);
+if (!fp) {
+fprintf(stderr, unable to read mtab\n);
+goto fail;
+}
+
+while ((mnt = getmntent(fp))) {
+/*
+ * An entry which device name doesn't start with a '/' is
+ * either a dummy file system or a network file system.
+ * Add special handling for smbfs and cifs as is done by
+ * coreutils as well.
+ */
+if ((mnt-mnt_fsname[0] != '/') ||
+(strcmp(mnt-mnt_type, smbfs) == 0) ||
+(strcmp(mnt-mnt_type, cifs) == 0)) {
+continue;
+}
+
+entry = qemu_malloc(sizeof(struct direntry));
+entry-dirname = qemu_strdup(mnt-mnt_dir);
+entry-devtype = qemu_strdup(mnt-mnt_type);
+entry-next = freezestate.mount_list;
+
+freezestate.mount_list = entry;
+}
+
+endmntent(fp);
+
+return 0;
+
+fail:
+while(freezestate.mount_list) {
+next = freezestate.mount_list-next;
+qemu_free(freezestate.mount_list-dirname);
+qemu_free(freezestate.mount_list-devtype);
+qemu_free(freezestate.mount_list);
+freezestate.mount_list = next;
+}
+
+return -1;
+}
+
+/*
+ * qga_guest_fsfreeze(): Walk list of mounted file systems in the
+ * guest, and freeze the ones which are real local file systems.
+ * return values: Number of file systems frozen, -1 on error.
+ */
+int64_t qga_guest_fsfreeze(Error **err)
+{
+struct direntry *entry;
+int fd, i = 0;
+int64_t ret;
+SLOG(va_fsfreeze());
+
+if (freezestate.status != FREEZE_THAWED) {
+ret = 0;
+goto out;
+}
+
+ret = build_mount_list();
+if (ret 0) {
+goto out;
+}
+
+freezestate.status = FREEZE_INPROGRESS;
+
+entry = freezestate.mount_list;
+while(entry) {
+fd = qemu_open(entry-dirname, O_RDONLY);
+if (fd == -1) {
+ret = errno;
+goto error;
+}
+ret = ioctl(fd, FIFREEZE);
+close(fd);
+if (ret 0 ret != EOPNOTSUPP) {
+goto error;
+}
+
+entry = entry-next;
+i++;
+}
+
+freezestate.status = FREEZE_FROZEN;
+ret = i;
+out:
+return ret;
+error:
+if (i 0) {
+freezestate.status = FREEZE_ERROR;
+}
+goto out;
+}
+
+/*
+ * qga_guest_fsthaw(): Walk list of frozen file systems in the guest, and
+ * thaw them.
+ * return values: Number of file systems thawed on success, -1 on error.
+ */
+int64_t qga_guest_fsthaw(Error **err)
+{
+struct
Re: [Qemu-devel] [PATCH 12/18] Insert event_tap_mmio() to cpu_physical_memory_rw() in exec.c.
On 2011-04-26 16:24, 大村 圭 wrote: 2011/4/25 Jan Kiszka jan.kis...@web.de: On 2011-04-25 13:00, OHMURA Kei wrote: From: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp Record mmio write event to replay it upon failover. Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp Signed-off-by: OHMURA Kei ohmura@lab.ntt.co.jp --- exec.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/exec.c b/exec.c index c3dc68a..3c3cece 100644 --- a/exec.c +++ b/exec.c @@ -33,6 +33,7 @@ #include osdep.h #include kvm.h #include qemu-timer.h +#include event-tap.h #if defined(CONFIG_USER_ONLY) #include qemu.h #include signal.h @@ -3736,6 +3737,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf, io_index = (pd IO_MEM_SHIFT) (IO_MEM_NB_ENTRIES - 1); if (p) addr1 = (addr ~TARGET_PAGE_MASK) + p-region_offset; + +event_tap_mmio(addr, buf, len); + You know that this is incomplete? A few devices are calling st*_phys directly, specifically virtio. What kind of mmio should be traced here, device or CPU originated? Or both? Jan To let Kemari replay outputs upon failover, tracing CPU originated mmio (specifically write requests) should be enough. IIUC, we can reproduce device originated mmio as a result of cpu originated mmio. OK, I see. But this tap will only work for KVM. I think you either have to catch the other paths that TCG could take as well or maybe better move the hook into kvm-all - then it's absolutely clear that this is no generic feature. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] KVM call minutes for Apr 26
On 04/26/2011 05:41 PM, Chris Wright wrote: - having basic common config could be useful My config is: --- include tests_base.cfg include cdkeys.cfg image_name(_.*)? ?= images/ cdrom(_.*)? ?= isos/ drive_cache=unsafe extra_params = -enable-kvm variants: - @avi: only no_pci_assignable only qcow2 only ide only smp2 only Fedora.9.32 Fedora.9.64 WinVista.64sp1 WinXP only install setup boot reboot migrate shutdown only rtl8139 only smallpages abort_on_error = yes kill_vm.* ?= no kill_unresponsive_vms.* ?= no WinXP.64|Win2003.64: no shutdown no reboot # Choose your test list from the testsets defined only avi pci_assignable = no serial_console = no --- In addition I run the kvm-unit-tests tests. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v2] configure: Make epoll_create1 test work around SPARC glibc bug
Work around a SPARC glibc bug which caused the epoll_create1 configure
test to wrongly claim that the function was present. Some versions of
SPARC glibc provided the function in the library but didn't declare
it in the include file; the result is that gcc warns about an implicit
declaration but a link succeeds. So we reference the function as a
value rather than a function call to induce a compile time error
if the declaration was not present.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
v1-v2: instead of compiling the test with -Werror, use the approach
suggested by Blue Swirl to force a compile-time failure if the
declaration is missing from the header file.
configure | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/configure b/configure
index de44bac..2f5 100755
--- a/configure
+++ b/configure
@@ -2225,7 +2225,15 @@ cat $TMPC EOF
int main(void)
{
-epoll_create1(0);
+/* Note that we use epoll_create1 as a value, not as
+ * a function being called. This is necessary so that on
+ * old SPARC glibc versions where the function was present in
+ * the library but not declared in the header file we will
+ * fail the configure check. (Otherwise we will get a compiler
+ * warning but not an error, and will proceed to fail the
+ * qemu compile where we compile with -Werror.)
+ */
+epoll_create1;
return 0;
}
EOF
--
1.7.1
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Tue, Apr 26, 2011 at 5:34 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Tue, Apr 26, 2011 at 12:29 AM, Aurelien Jarno aurel...@aurel32.net wrote:
On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko atar4q...@gmail.com
wrote:
On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
Do you have public test case?
It is possible to code this delay slot write test but real issue may
be corruption elsewhere.
The test case is trivial: it's just the two instructions, branch and
wrpr.
In theory there could be multiple issues including compiler induced
ones.
I'd prefer to see some kind of reproducible testcase.
Ok, attached a 40 byte long test (the first 32 bytes are not used and
needed only because the bios entry point is 0x20).
$ git pull make sparc64-softmmu/qemu-system-sparc64 -bios
test-wrpr.bin -nographic
Already up-to-date.
make[1]: Nothing to be done for `all'.
/mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
Aborted
The problem seems to be that wrpr is using a non-local
TCG tmp (cpu_tmp0).
Just tried the test case with write to %pil - seems like write itself is
OK.
The issue appears to be with save_state() call since adding save_state
to %pil case provokes the same tcg abort.
The problem is that cpu_tmp0, not being a local tmp, doesn't
need to be saved across helper calls. This results in the
TCG optimizer getting rid of it even though it's later used.
Look at the log and you'll see what I mean :-)
I'm not very comfortable with tcg yet. Would it be possible to teach
optimizer working with delay slots? Or do I look in the wrong place.
The problem is not on the TCG side, but on the target-sparc/translate.c
side:
| case 0x32: /* wrwim, V9 wrpr */
| {
| if (!supervisor(dc))
| goto priv_insn;
| tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2);
| #ifdef TARGET_SPARC64
Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
saved across TCG branches.
[...]
| case 6: // pstate
| save_state(dc, cpu_cond);
| gen_helper_wrpstate(cpu_tmp0);
| dc-npc = DYNAMIC_PC;
| break;
save_state() calls save_npc(), which in turns might call
gen_generic_branch():
| static inline void gen_generic_branch(target_ulong npc1, target_ulong npc2,
| TCGv r_cond)
| {
| int l1, l2;
|
| l1 = gen_new_label();
| l2 = gen_new_label();
|
| tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
|
| tcg_gen_movi_tl(cpu_npc, npc1);
| tcg_gen_br(l2);
|
| gen_set_label(l1);
| tcg_gen_movi_tl(cpu_npc, npc2);
| gen_set_label(l2);
| }
And here is the TCG branch, which drop the TCG temp cpu_temp0.
The solution is either to rewrite gen_generic_branch() without TCG
branches, or to use a TCG temp local instead of a TCG temp.
Thanks!
I think the issue is more clear now, and loading to local temporary
works in this case.
Does not explain why unmodified qemu works with wrpr pstate not in delay slot.
Because the TCG branch is not generated in save_npc()?
I looked at my linux kernel builds and do not see any wrpr pstate in delay
slot.
Meaning you are not going to fix the bug? ;-)
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
Re: [Qemu-devel] Supporting emulation of IOMMUs
Hello David, On Thu, Apr 21, 2011 at 03:03:47AM -0400, David Gibson wrote: A few months ago, Eduard - Gabriel Munteanu posted a series of patches implementing support for emulating the AMD PCI IOMMU (http://lists.nongnu.org/archive/html/qemu-devel/2011-01/msg03196.html). In fact, this series implemented a general DMA/IOMMU layer which can be used by any device model, and one translation backend for this implementing the AMD specific PCI IOMMU. the patches for AMD IOMMU emulation are not yet upstream has you have already noticed. Eduard is busy with his studies at the moment so any help is greatly appreciated :) Feel free to pick up hist patches and re-submit them (keeping the author correct of course). If you submit your code, it would be cool if you could put me on Cc so I can easily follow the discussion and jump in if required. Regards, Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Re: [Qemu-devel] [PATCH 12/18] Insert event_tap_mmio() to cpu_physical_memory_rw() in exec.c.
2011/4/25 Jan Kiszka jan.kis...@web.de: On 2011-04-25 13:00, OHMURA Kei wrote: From: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp Record mmio write event to replay it upon failover. Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp Signed-off-by: OHMURA Kei ohmura@lab.ntt.co.jp --- exec.c | 4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/exec.c b/exec.c index c3dc68a..3c3cece 100644 --- a/exec.c +++ b/exec.c @@ -33,6 +33,7 @@ #include osdep.h #include kvm.h #include qemu-timer.h +#include event-tap.h #if defined(CONFIG_USER_ONLY) #include qemu.h #include signal.h @@ -3736,6 +3737,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf, io_index = (pd IO_MEM_SHIFT) (IO_MEM_NB_ENTRIES - 1); if (p) addr1 = (addr ~TARGET_PAGE_MASK) + p-region_offset; + + event_tap_mmio(addr, buf, len); + You know that this is incomplete? A few devices are calling st*_phys directly, specifically virtio. What kind of mmio should be traced here, device or CPU originated? Or both? Jan To let Kemari replay outputs upon failover, tracing CPU originated mmio (specifically write requests) should be enough. IIUC, we can reproduce device originated mmio as a result of cpu originated mmio. Thanks, Kei
[Qemu-devel] [PATCH] Fix bug with virtio-9p fsync
v9fs_fsync and possibly others break when asked to operate on a directory.
It does not check fid_type to see if it is operating on a directory and
therefore accesses the wrong element of the fs union.
This error can result in guest applications failing (in my case it was dpkg).
This patch fixes the issue, although there may be other, similar bugs in
virtio-9p.
Signed-off-by: Sassan Panahinejad sas...@sassan.me.uk
---
Here I've implemented it as a function. If you'd prefer a macro or inline
function then let me know.
Thanks
Sassan
hw/virtio-9p.c | 43 +--
1 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..26be0fc 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1860,6 +1860,18 @@ static void v9fs_post_do_fsync(V9fsState *s, V9fsPDU
*pdu, int err)
complete_pdu(s, pdu, err);
}
+static int v9fs_get_fd(V9fsFidState *fidp)
+{
+switch (fidp-fid_type) {
+case P9_FID_FILE:
+return fidp-fs.fd;
+case P9_FID_DIR:
+return dirfd(fidp-fs.dir);
+default:
+return -1;
+}
+}
+
static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
{
int32_t fid;
@@ -1867,6 +1879,7 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
V9fsFidState *fidp;
int datasync;
int err;
+int fd;
pdu_unmarshal(pdu, offset, dd, fid, datasync);
fidp = lookup_fid(s, fid);
@@ -1875,7 +1888,11 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
v9fs_post_do_fsync(s, pdu, err);
return;
}
-err = v9fs_do_fsync(s, fidp-fs.fd, datasync);
+if ((fd = v9fs_get_fd(fidp)) = 0) {
+err = v9fs_do_fsync(s, fd, datasync);
+} else {
+ err = -EINVAL;
+}
v9fs_post_do_fsync(s, pdu, err);
}
@@ -2984,6 +3001,7 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu)
int32_t fid;
V9fsWstatState *vs;
int err = 0;
+int fd;
vs = qemu_malloc(sizeof(*vs));
vs-pdu = pdu;
@@ -2999,7 +3017,10 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu)
/* do we need to sync the file? */
if (donttouch_stat(vs-v9stat)) {
-err = v9fs_do_fsync(s, vs-fidp-fs.fd, 0);
+if ((fd = v9fs_get_fd(vs-fidp)) = 0) {
+err = v9fs_do_fsync(s, fd, 0);
+} else
+err = -EINVAL;
v9fs_wstat_post_fsync(s, vs, err);
return;
}
@@ -3176,6 +3197,7 @@ static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
{
int32_t fid, err = 0;
V9fsLockState *vs;
+int fd;
vs = qemu_mallocz(sizeof(*vs));
vs-pdu = pdu;
@@ -3198,8 +3220,12 @@ static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
err = -ENOENT;
goto out;
}
-
-err = v9fs_do_fstat(s, vs-fidp-fs.fd, vs-stbuf);
+if ((fd = v9fs_get_fd(vs-fidp)) = 0) {
+err = v9fs_do_fstat(s, fd, vs-stbuf);
+} else {
+err = -EINVAL;
+goto out;
+}
if (err 0) {
err = -errno;
goto out;
@@ -3221,6 +3247,7 @@ static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
{
int32_t fid, err = 0;
V9fsGetlockState *vs;
+int fd;
vs = qemu_mallocz(sizeof(*vs));
vs-pdu = pdu;
@@ -3236,8 +3263,12 @@ static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
err = -ENOENT;
goto out;
}
-
-err = v9fs_do_fstat(s, vs-fidp-fs.fd, vs-stbuf);
+if ((fd = v9fs_get_fd(vs-fidp)) = 0) {
+err = v9fs_do_fstat(s, fd, vs-stbuf);
+} else {
+err = -EINVAL;
+goto out;
+}
if (err 0) {
err = -errno;
goto out;
--
1.7.0.4
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno aurel...@aurel32.net wrote:
On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko atar4q...@gmail.com
wrote:
On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
Do you have public test case?
It is possible to code this delay slot write test but real issue may
be corruption elsewhere.
The test case is trivial: it's just the two instructions, branch and
wrpr.
In theory there could be multiple issues including compiler induced
ones.
I'd prefer to see some kind of reproducible testcase.
Ok, attached a 40 byte long test (the first 32 bytes are not used and
needed only because the bios entry point is 0x20).
$ git pull make sparc64-softmmu/qemu-system-sparc64 -bios
test-wrpr.bin -nographic
Already up-to-date.
make[1]: Nothing to be done for `all'.
/mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
Aborted
The problem seems to be that wrpr is using a non-local
TCG tmp (cpu_tmp0).
Just tried the test case with write to %pil - seems like write itself is
OK.
The issue appears to be with save_state() call since adding save_state
to %pil case provokes the same tcg abort.
The problem is that cpu_tmp0, not being a local tmp, doesn't
need to be saved across helper calls. This results in the
TCG optimizer getting rid of it even though it's later used.
Look at the log and you'll see what I mean :-)
I'm not very comfortable with tcg yet. Would it be possible to teach
optimizer working with delay slots? Or do I look in the wrong place.
The problem is not on the TCG side, but on the target-sparc/translate.c
side:
| case 0x32: /* wrwim, V9 wrpr */
| {
| if (!supervisor(dc))
| goto priv_insn;
| tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2);
| #ifdef TARGET_SPARC64
Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
saved across TCG branches.
[...]
| case 6: // pstate
| save_state(dc, cpu_cond);
| gen_helper_wrpstate(cpu_tmp0);
| dc-npc = DYNAMIC_PC;
| break;
save_state() calls save_npc(), which in turns might call
gen_generic_branch():
Hmm. This is not the only instruction using save_state() and cpu_tmp0.
At least ldd is another example.
| static inline void gen_generic_branch(target_ulong npc1, target_ulong npc2,
| TCGv r_cond)
| {
| int l1, l2;
|
| l1 = gen_new_label();
| l2 = gen_new_label();
|
| tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
|
| tcg_gen_movi_tl(cpu_npc, npc1);
| tcg_gen_br(l2);
|
| gen_set_label(l1);
| tcg_gen_movi_tl(cpu_npc, npc2);
| gen_set_label(l2);
| }
And here is the TCG branch, which drop the TCG temp cpu_temp0.
The solution is either to rewrite gen_generic_branch() without TCG
branches, or to use a TCG temp local instead of a TCG temp.
You mean something like
case 6: // pstate
{
TCGv r_temp;
r_temp = tcg_temp_new();
tcg_gen_mov_tl(r_temp, cpu_tmp0);
save_state(dc, cpu_cond);
gen_helper_wrpstate(r_temp);
tcg_temp_free(r_temp);
dc-npc = DYNAMIC_PC;
}
break;
?
This fails with the same error message.
Artyom
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix typo in code and comments
Am Dienstag 26 April 2011 11:25:58 schrieb Stefan Hajnoczi: On Tue, Apr 26, 2011 at 9:29 AM, Stefan Weil w...@mail.berlios.de wrote: Replace writeable - writable Why make this change? writeable and writable are both commonly used spellings. It seems like writeable is the commonly used term in computer sciences and writable is the normal english adjective formed from to write + -able in general English.[1] As we are refering to computer science related writeable it should be left as is imho. But as I'm no native speaker, you might feel different on this. On a side note: Samba offers both spellings as valid for thier configuration files. [2] [1] http://www.thefreedictionary.com/writeable [2] http://forums.contribs.org/index.php?topic=22258.0 With regards, Dipl. Phys. Jan M. Simons Institute of Crystallography RWTH Aachen University
Re: [Qemu-devel] [PATCH] lsi53c895a: add support for ABORT messages
26.04.2011 18:46, Peter Lieven wrote: [] i recently saw some qemu-kvm 0.12.5 guests with scsi and this patch applies crashing when we updated our backend iscsi storages. (short interrupt in traffic flow, iscsi disconnect + reconnect) i always see: lsi_scsi: error: ORDERED queue not implemented and then either the maschine just hangs or it even aborts due to this assertion: qemu-kvm-0.12.5: /usr/src/qemu-kvm-0.12.5/hw/lsi53c895a.c:596: lsi_reselect: Assertion `s-current == ((void *)0)' failed. http://bugs.debian.org/613413 talks about this very issue too ;) any ideas? Unfortunately, no, except that it looks like scsi support is not of production quality still. /mjt
Re: [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary
On 25 April 2011 02:23, YuYeon Oh yuyeon...@samsung.com wrote: target-arm: fix LDMIA bug on page boundary (You don't need to repeat the Subject summary line in the body, it makes the git changelog look a bit odd when the patch is applied with 'git am'). When consecutive memory locations are on page boundary, a base register may be loaded before page fault occurs. After page fault handling, it losts the memory location information. To solve this problem, loading a base register has to put back. Signed-off-by: Yuyeon Oh yuyeon...@samsung.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org I've tested this and confirmed that it fixes this bug for the Thumb T2 encoding. However, the same problem still exists for the T1 (16 bit) encoding; I'll send a patch for that in a moment. (The ARM encoding did not have this bug.) -- PMM
[Qemu-devel] [PATCH] target-arm: Don't update base register on abort in Thumb T1 LDM
Make sure the base register isn't updated if it is in the load list
for a Thumb LDM (T1 encoding) which aborts partway through the load.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/translate.c | 17 ++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index d8da514..a1af436 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9454,7 +9454,10 @@ static void disas_thumb_insn(CPUState *env, DisasContext
*s)
break;
case 12:
+{
/* load/store multiple */
+TCGv loaded_var;
+TCGV_UNUSED(loaded_var);
rn = (insn 8) 0x7;
addr = load_reg(s, rn);
for (i = 0; i 8; i++) {
@@ -9462,7 +9465,11 @@ static void disas_thumb_insn(CPUState *env, DisasContext
*s)
if (insn (1 11)) {
/* load */
tmp = gen_ld32(addr, IS_USER(s));
-store_reg(s, i, tmp);
+if (i == rn) {
+loaded_var = tmp;
+} else {
+store_reg(s, i, tmp);
+}
} else {
/* store */
tmp = load_reg(s, i);
@@ -9472,14 +9479,18 @@ static void disas_thumb_insn(CPUState *env,
DisasContext *s)
tcg_gen_addi_i32(addr, addr, 4);
}
}
-/* Base register writeback. */
if ((insn (1 rn)) == 0) {
+/* base reg not in list: base register writeback */
store_reg(s, rn, addr);
} else {
+/* base reg in list: if load, complete it now */
+if (insn (1 11)) {
+store_reg(s, rn, loaded_var);
+}
tcg_temp_free_i32(addr);
}
break;
-
+}
case 13:
/* conditional branch or swi */
cond = (insn 8) 0xf;
--
1.7.1
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix typo in code and comments
Am 26.04.2011 19:04, schrieb Jan Marten Simons: Am Dienstag 26 April 2011 11:25:58 schrieb Stefan Hajnoczi: On Tue, Apr 26, 2011 at 9:29 AM, Stefan Weil w...@mail.berlios.de wrote: Replace writeable - writable Why make this change? writeable and writable are both commonly used spellings. It seems like writeable is the commonly used term in computer sciences and writable is the normal english adjective formed from to write + -able in general English.[1] As we are refering to computer science related writeable it should be left as is imho. But as I'm no native speaker, you might feel different on this. On a side note: Samba offers both spellings as valid for thier configuration files. [2] [1] http://www.thefreedictionary.com/writeable [2] http://forums.contribs.org/index.php?topic=22258.0 With regards, Dipl. Phys. Jan M. Simons Institute of Crystallography RWTH Aachen University Commonly used is not necessarily correct. The Oxford dictionary only accepts writable (even when I select american english). Same result with Merriam-Webster. Google suggests writable instead of writeable. I see writeable only in computer programs and related contexts, therefore I assume that it is simply a very common spelling error contributed by non-native speakers (like myself). Interesting detail: The spelling checker of my mailing client (Icedove) marks writeable as correct and writable as wrong (which is obviously wrong). Maybe I should send a bug report to Debian. The ratio of writeable:writable in unpatched qemu is 37:47. Even if both writings were correct, I might argue that a uniform spelling is better and choose the form which is more commonly used. Let's improve the spelling for computer programs a little bit!
Re: [Qemu-devel] KVM call minutes for Apr 26
On Tue, 2011-04-26 at 17:58 +0300, Avi Kivity wrote: On 04/26/2011 05:41 PM, Chris Wright wrote: - having basic common config could be useful My config is: --- include tests_base.cfg include cdkeys.cfg image_name(_.*)? ?= images/ cdrom(_.*)? ?= isos/ drive_cache=unsafe extra_params = -enable-kvm variants: - @avi: only no_pci_assignable only qcow2 only ide only smp2 only Fedora.9.32 Fedora.9.64 WinVista.64sp1 WinXP ^ Maybe Fedora could be bumped to F14, and WinVista, replaced with Win7 only install setup boot reboot migrate shutdown ^ Instead of the install setup unattended install could be used here... only rtl8139 only smallpages abort_on_error = yes kill_vm.* ?= no kill_unresponsive_vms.* ?= no WinXP.64|Win2003.64: no shutdown no reboot # Choose your test list from the testsets defined only avi pci_assignable = no serial_console = no --- In addition I run the kvm-unit-tests tests.
Re: [Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu_reset
On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka jan.kis...@siemens.com wrote: Instead of having an extra reset function at machine level and special code for processing INIT, move the initialization of halted into the cpu reset handler. Nack. A CPU is designated as a BSP at board level. CPUs do not need to know about this at all.
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Tue, Apr 26, 2011 at 8:26 PM, Artyom Tarasenko atar4q...@gmail.com wrote:
On Tue, Apr 26, 2011 at 5:34 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Tue, Apr 26, 2011 at 12:29 AM, Aurelien Jarno aurel...@aurel32.net
wrote:
On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko
atar4q...@gmail.com wrote:
On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
Do you have public test case?
It is possible to code this delay slot write test but real issue
may
be corruption elsewhere.
The test case is trivial: it's just the two instructions, branch and
wrpr.
In theory there could be multiple issues including compiler induced
ones.
I'd prefer to see some kind of reproducible testcase.
Ok, attached a 40 byte long test (the first 32 bytes are not used and
needed only because the bios entry point is 0x20).
$ git pull make sparc64-softmmu/qemu-system-sparc64 -bios
test-wrpr.bin -nographic
Already up-to-date.
make[1]: Nothing to be done for `all'.
/mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
Aborted
The problem seems to be that wrpr is using a non-local
TCG tmp (cpu_tmp0).
Just tried the test case with write to %pil - seems like write itself
is OK.
The issue appears to be with save_state() call since adding save_state
to %pil case provokes the same tcg abort.
The problem is that cpu_tmp0, not being a local tmp, doesn't
need to be saved across helper calls. This results in the
TCG optimizer getting rid of it even though it's later used.
Look at the log and you'll see what I mean :-)
I'm not very comfortable with tcg yet. Would it be possible to teach
optimizer working with delay slots? Or do I look in the wrong place.
The problem is not on the TCG side, but on the target-sparc/translate.c
side:
| case 0x32: /* wrwim, V9 wrpr */
| {
| if (!supervisor(dc))
| goto priv_insn;
| tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2);
| #ifdef TARGET_SPARC64
Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
saved across TCG branches.
[...]
| case 6: // pstate
| save_state(dc, cpu_cond);
| gen_helper_wrpstate(cpu_tmp0);
| dc-npc = DYNAMIC_PC;
| break;
save_state() calls save_npc(), which in turns might call
gen_generic_branch():
| static inline void gen_generic_branch(target_ulong npc1, target_ulong
npc2,
| TCGv r_cond)
| {
| int l1, l2;
|
| l1 = gen_new_label();
| l2 = gen_new_label();
|
| tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
|
| tcg_gen_movi_tl(cpu_npc, npc1);
| tcg_gen_br(l2);
|
| gen_set_label(l1);
| tcg_gen_movi_tl(cpu_npc, npc2);
| gen_set_label(l2);
| }
And here is the TCG branch, which drop the TCG temp cpu_temp0.
The solution is either to rewrite gen_generic_branch() without TCG
branches, or to use a TCG temp local instead of a TCG temp.
Thanks!
I think the issue is more clear now, and loading to local temporary
works in this case.
Does not explain why unmodified qemu works with wrpr pstate not in delay
slot.
Because the TCG branch is not generated in save_npc()?
I looked at my linux kernel builds and do not see any wrpr pstate in delay
slot.
Meaning you are not going to fix the bug? ;-)
More like I need to know where the bug is
because there is no issue running without wrpr in delay slot.
--
Kind regards,
Igor V. Kovalenko
Re: [Qemu-devel] Question on qemu build environment.
On Tue, Apr 26, 2011 at 4:23 PM, Super Bisquit superbisq...@gmail.com wrote: I have noticed that qemu does not fully function on FreeBSD sparc64. Besides n...@freebsd.org and myself, has anyone tried building and running qemu under FreeBSD sparc64? I think you are the first to report. On OpenBSD/Sparc64 I could run qemu-system-sparc with the test image and get a command prompt (it seems to be broken now), but i386 emulator (or Sparc64 TCG target) has problems with unaligned accesses.
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Tue, Apr 26, 2011 at 8:02 PM, Artyom Tarasenko atar4q...@gmail.com wrote:
On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno aurel...@aurel32.net wrote:
On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko atar4q...@gmail.com
wrote:
On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
Do you have public test case?
It is possible to code this delay slot write test but real issue may
be corruption elsewhere.
The test case is trivial: it's just the two instructions, branch and
wrpr.
In theory there could be multiple issues including compiler induced
ones.
I'd prefer to see some kind of reproducible testcase.
Ok, attached a 40 byte long test (the first 32 bytes are not used and
needed only because the bios entry point is 0x20).
$ git pull make sparc64-softmmu/qemu-system-sparc64 -bios
test-wrpr.bin -nographic
Already up-to-date.
make[1]: Nothing to be done for `all'.
/mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
Aborted
The problem seems to be that wrpr is using a non-local
TCG tmp (cpu_tmp0).
Just tried the test case with write to %pil - seems like write itself is
OK.
The issue appears to be with save_state() call since adding save_state
to %pil case provokes the same tcg abort.
The problem is that cpu_tmp0, not being a local tmp, doesn't
need to be saved across helper calls. This results in the
TCG optimizer getting rid of it even though it's later used.
Look at the log and you'll see what I mean :-)
I'm not very comfortable with tcg yet. Would it be possible to teach
optimizer working with delay slots? Or do I look in the wrong place.
The problem is not on the TCG side, but on the target-sparc/translate.c
side:
| case 0x32: /* wrwim, V9 wrpr */
| {
| if (!supervisor(dc))
| goto priv_insn;
| tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2);
| #ifdef TARGET_SPARC64
Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
saved across TCG branches.
[...]
| case 6: // pstate
| save_state(dc, cpu_cond);
| gen_helper_wrpstate(cpu_tmp0);
| dc-npc = DYNAMIC_PC;
| break;
save_state() calls save_npc(), which in turns might call
gen_generic_branch():
Hmm. This is not the only instruction using save_state() and cpu_tmp0.
At least ldd is another example.
| static inline void gen_generic_branch(target_ulong npc1, target_ulong npc2,
| TCGv r_cond)
| {
| int l1, l2;
|
| l1 = gen_new_label();
| l2 = gen_new_label();
|
| tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
|
| tcg_gen_movi_tl(cpu_npc, npc1);
| tcg_gen_br(l2);
|
| gen_set_label(l1);
| tcg_gen_movi_tl(cpu_npc, npc2);
| gen_set_label(l2);
| }
And here is the TCG branch, which drop the TCG temp cpu_temp0.
The solution is either to rewrite gen_generic_branch() without TCG
branches, or to use a TCG temp local instead of a TCG temp.
You mean something like
case 6: // pstate
{
TCGv r_temp;
r_temp = tcg_temp_new();
tcg_gen_mov_tl(r_temp, cpu_tmp0);
save_state(dc, cpu_cond);
gen_helper_wrpstate(r_temp);
tcg_temp_free(r_temp);
dc-npc = DYNAMIC_PC;
}
break;
?
This fails with the same error message.
Close, but you need to use tcg_temp_local_new(). Does this work?
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 3c958b2..52fa2f1 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -3505,9 +3505,15 @@ static void disas_sparc_insn(DisasContext * dc)
tcg_gen_mov_tl(cpu_tbr, cpu_tmp0);
break;
case 6: // pstate
-save_state(dc, cpu_cond);
-gen_helper_wrpstate(cpu_tmp0);
-dc-npc = DYNAMIC_PC;
+{
+TCGv r_tmp = tcg_temp_local_new();
+
+
Re: [Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu_reset
On 2011-04-26 20:00, Blue Swirl wrote: On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka jan.kis...@siemens.com wrote: Instead of having an extra reset function at machine level and special code for processing INIT, move the initialization of halted into the cpu reset handler. Nack. A CPU is designated as a BSP at board level. CPUs do not need to know about this at all. That's why we have cpu_is_bsp() in pc.c. Obviously, every CPU (which includes the APIC) must know if it is supposed to be BP or AP. It would be unable to enter the right state after reset otherwise (e.g. Intel SDM 9.1). cpu_is_bsp() is basically reporting the result of the MP init protocol in condensed from. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Tue, Apr 26, 2011 at 10:36 PM, Blue Swirl blauwir...@gmail.com wrote:
On Tue, Apr 26, 2011 at 8:02 PM, Artyom Tarasenko atar4q...@gmail.com wrote:
On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno aurel...@aurel32.net
wrote:
On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko
atar4q...@gmail.com wrote:
On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
Do you have public test case?
It is possible to code this delay slot write test but real issue
may
be corruption elsewhere.
The test case is trivial: it's just the two instructions, branch and
wrpr.
In theory there could be multiple issues including compiler induced
ones.
I'd prefer to see some kind of reproducible testcase.
Ok, attached a 40 byte long test (the first 32 bytes are not used and
needed only because the bios entry point is 0x20).
$ git pull make sparc64-softmmu/qemu-system-sparc64 -bios
test-wrpr.bin -nographic
Already up-to-date.
make[1]: Nothing to be done for `all'.
/mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
Aborted
The problem seems to be that wrpr is using a non-local
TCG tmp (cpu_tmp0).
Just tried the test case with write to %pil - seems like write itself
is OK.
The issue appears to be with save_state() call since adding save_state
to %pil case provokes the same tcg abort.
The problem is that cpu_tmp0, not being a local tmp, doesn't
need to be saved across helper calls. This results in the
TCG optimizer getting rid of it even though it's later used.
Look at the log and you'll see what I mean :-)
I'm not very comfortable with tcg yet. Would it be possible to teach
optimizer working with delay slots? Or do I look in the wrong place.
The problem is not on the TCG side, but on the target-sparc/translate.c
side:
| case 0x32: /* wrwim, V9 wrpr */
| {
| if (!supervisor(dc))
| goto priv_insn;
| tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2);
| #ifdef TARGET_SPARC64
Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
saved across TCG branches.
[...]
| case 6: // pstate
| save_state(dc, cpu_cond);
| gen_helper_wrpstate(cpu_tmp0);
| dc-npc = DYNAMIC_PC;
| break;
save_state() calls save_npc(), which in turns might call
gen_generic_branch():
Hmm. This is not the only instruction using save_state() and cpu_tmp0.
At least ldd is another example.
| static inline void gen_generic_branch(target_ulong npc1, target_ulong
npc2,
| TCGv r_cond)
| {
| int l1, l2;
|
| l1 = gen_new_label();
| l2 = gen_new_label();
|
| tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
|
| tcg_gen_movi_tl(cpu_npc, npc1);
| tcg_gen_br(l2);
|
| gen_set_label(l1);
| tcg_gen_movi_tl(cpu_npc, npc2);
| gen_set_label(l2);
| }
And here is the TCG branch, which drop the TCG temp cpu_temp0.
The solution is either to rewrite gen_generic_branch() without TCG
branches, or to use a TCG temp local instead of a TCG temp.
You mean something like
case 6: // pstate
{
TCGv r_temp;
r_temp = tcg_temp_new();
tcg_gen_mov_tl(r_temp, cpu_tmp0);
save_state(dc, cpu_cond);
gen_helper_wrpstate(r_temp);
tcg_temp_free(r_temp);
dc-npc = DYNAMIC_PC;
}
break;
?
This fails with the same error message.
Close, but you need to use tcg_temp_local_new(). Does this work?
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 3c958b2..52fa2f1 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -3505,9 +3505,15 @@ static void disas_sparc_insn(DisasContext * dc)
tcg_gen_mov_tl(cpu_tbr, cpu_tmp0);
break;
case 6: // pstate
- save_state(dc, cpu_cond);
- gen_helper_wrpstate(cpu_tmp0);
- dc-npc = DYNAMIC_PC;
+
Re: [Qemu-devel] is it just me or is ne2k broken in qemu?
On Sun, 17 Apr 2011 00:37:35 +0400 Michael Tokarev m...@tls.msk.ru wrote: 15.04.2011 18:17, Alex Williamson wrote: On Thu, 2011-04-14 at 12:31 +0400, Michael Tokarev wrote: The NIC works for a while, but after a few packets, or a few 1000s of packets, it stalls. In tcpdump on the host I see many ARP requests coming from the guest and each has corresponding ARP reply, but nothing is actually reaching the guest. For testing the iPXE ROMs I booted up each NIC, including ne2k_pci, to a network loaded kernel (~4M) and installation initrd (~8M). I stopped the test at the point where the installer kernel was able to successfully DHCP with the boot NIC. ne2k_pci was definitely the slowest of the cards at loading the images, but I didn't notice any functionality issues. Maybe I didn't let it run long enough, but the boot ROM seems ok with it. I'm doing exactly the same here, -- testing iPXE booting, so booting linux kernel over network. I haven't been able to boot linux on ne2k so far, it fails somewhere down the road after loading initrd+kernel - either during initrd run or about after switching to new init (still running off network ofcourse) after mounting nfs root. So when I encountered this issue I tried non-network boot and various versions of (linux) guest and qemu-kvm - and for me, ne2k always fail (stalls) after some time. And Mulyadi Santosa mentioned it's apparently a known issue due to some timer-related problem in the code. I've hit this issue today and I can reproduce it by downloading a 80M file with wget. My network configuration is: -net nic,model=ne2k_pci -net tap,ifname=vnet0,script=./qemu-ifup-switch It also happens with qemu 0.12.5. I've also tried to enable the device's debug and the last lines I see before it stalls *usually* are: E2000: read=0x07 val=0 or E2000: asic readl val=0xe283fad3 I'll try to reproduce with an older kernel version, but debugging tips are welcome.
Re: [Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu_reset
On Tue, Apr 26, 2011 at 9:55 PM, Jan Kiszka jan.kis...@web.de wrote: On 2011-04-26 20:00, Blue Swirl wrote: On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka jan.kis...@siemens.com wrote: Instead of having an extra reset function at machine level and special code for processing INIT, move the initialization of halted into the cpu reset handler. Nack. A CPU is designated as a BSP at board level. CPUs do not need to know about this at all. That's why we have cpu_is_bsp() in pc.c. Obviously, every CPU (which includes the APIC) must know if it is supposed to be BP or AP. It would be unable to enter the right state after reset otherwise (e.g. Intel SDM 9.1). cpu_is_bsp() is basically reporting the result of the MP init protocol in condensed from. Intel 64 and IA-32 Architectures Software Developer’s Manual vol 3A, 7.5.1 says that the protocol result is stored in APIC MSR. I think we should be using that instead. For example, the board could call cpu_designate_bsp() to set the BSP flag in MSR. Then cpu_is_bsp() would only check the MSR, which naturally belongs to the CPU/APIC domain.
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Tue, Apr 26, 2011 at 10:07 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Tue, Apr 26, 2011 at 10:36 PM, Blue Swirl blauwir...@gmail.com wrote:
On Tue, Apr 26, 2011 at 8:02 PM, Artyom Tarasenko atar4q...@gmail.com
wrote:
On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno aurel...@aurel32.net
wrote:
On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko
atar4q...@gmail.com wrote:
On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
Do you have public test case?
It is possible to code this delay slot write test but real issue
may
be corruption elsewhere.
The test case is trivial: it's just the two instructions, branch and
wrpr.
In theory there could be multiple issues including compiler induced
ones.
I'd prefer to see some kind of reproducible testcase.
Ok, attached a 40 byte long test (the first 32 bytes are not used and
needed only because the bios entry point is 0x20).
$ git pull make sparc64-softmmu/qemu-system-sparc64 -bios
test-wrpr.bin -nographic
Already up-to-date.
make[1]: Nothing to be done for `all'.
/mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
Aborted
The problem seems to be that wrpr is using a non-local
TCG tmp (cpu_tmp0).
Just tried the test case with write to %pil - seems like write itself
is OK.
The issue appears to be with save_state() call since adding save_state
to %pil case provokes the same tcg abort.
The problem is that cpu_tmp0, not being a local tmp, doesn't
need to be saved across helper calls. This results in the
TCG optimizer getting rid of it even though it's later used.
Look at the log and you'll see what I mean :-)
I'm not very comfortable with tcg yet. Would it be possible to teach
optimizer working with delay slots? Or do I look in the wrong place.
The problem is not on the TCG side, but on the target-sparc/translate.c
side:
| case 0x32: /* wrwim, V9 wrpr */
| {
| if (!supervisor(dc))
| goto priv_insn;
| tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2);
| #ifdef TARGET_SPARC64
Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
saved across TCG branches.
[...]
| case 6: // pstate
| save_state(dc, cpu_cond);
| gen_helper_wrpstate(cpu_tmp0);
| dc-npc = DYNAMIC_PC;
| break;
save_state() calls save_npc(), which in turns might call
gen_generic_branch():
Hmm. This is not the only instruction using save_state() and cpu_tmp0.
At least ldd is another example.
| static inline void gen_generic_branch(target_ulong npc1, target_ulong
npc2,
| TCGv r_cond)
| {
| int l1, l2;
|
| l1 = gen_new_label();
| l2 = gen_new_label();
|
| tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
|
| tcg_gen_movi_tl(cpu_npc, npc1);
| tcg_gen_br(l2);
|
| gen_set_label(l1);
| tcg_gen_movi_tl(cpu_npc, npc2);
| gen_set_label(l2);
| }
And here is the TCG branch, which drop the TCG temp cpu_temp0.
The solution is either to rewrite gen_generic_branch() without TCG
branches, or to use a TCG temp local instead of a TCG temp.
You mean something like
case 6: // pstate
{
TCGv r_temp;
r_temp = tcg_temp_new();
tcg_gen_mov_tl(r_temp, cpu_tmp0);
save_state(dc, cpu_cond);
gen_helper_wrpstate(r_temp);
tcg_temp_free(r_temp);
dc-npc = DYNAMIC_PC;
}
break;
?
This fails with the same error message.
Close, but you need to use tcg_temp_local_new(). Does this work?
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 3c958b2..52fa2f1 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -3505,9 +3505,15 @@ static void disas_sparc_insn(DisasContext * dc)
tcg_gen_mov_tl(cpu_tbr, cpu_tmp0);
break;
case 6: // pstate
- save_state(dc, cpu_cond);
- gen_helper_wrpstate(cpu_tmp0);
Re: [Qemu-devel] [PATCH v2] configure: Make epoll_create1 test work around SPARC glibc bug
Thanks, applied.
On Tue, Apr 26, 2011 at 6:56 PM, Peter Maydell peter.mayd...@linaro.org wrote:
Work around a SPARC glibc bug which caused the epoll_create1 configure
test to wrongly claim that the function was present. Some versions of
SPARC glibc provided the function in the library but didn't declare
it in the include file; the result is that gcc warns about an implicit
declaration but a link succeeds. So we reference the function as a
value rather than a function call to induce a compile time error
if the declaration was not present.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
v1-v2: instead of compiling the test with -Werror, use the approach
suggested by Blue Swirl to force a compile-time failure if the
declaration is missing from the header file.
configure | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/configure b/configure
index de44bac..2f5 100755
--- a/configure
+++ b/configure
@@ -2225,7 +2225,15 @@ cat $TMPC EOF
int main(void)
{
- epoll_create1(0);
+ /* Note that we use epoll_create1 as a value, not as
+ * a function being called. This is necessary so that on
+ * old SPARC glibc versions where the function was present in
+ * the library but not declared in the header file we will
+ * fail the configure check. (Otherwise we will get a compiler
+ * warning but not an error, and will proceed to fail the
+ * qemu compile where we compile with -Werror.)
+ */
+ epoll_create1;
return 0;
}
EOF
--
1.7.1
Re: [Qemu-devel] [PATCH 4/6] docs/tracing.txt: minor documentation fixes
On Tue, 26 Apr 2011 10:26:01 pm Stefan Hajnoczi wrote: -There is a set of static trace events declared in the trace-events source +There is a set of static trace events declared in the trace-events source Would it read better if it said There are a set... (i.e. are instead of is)? Brad
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix typo in code and comments
Am 26.04.2011 19:26, schrieb Stefan Weil: Am 26.04.2011 19:04, schrieb Jan Marten Simons: Am Dienstag 26 April 2011 11:25:58 schrieb Stefan Hajnoczi: On Tue, Apr 26, 2011 at 9:29 AM, Stefan Weil w...@mail.berlios.de wrote: Replace writeable - writable Why make this change? writeable and writable are both commonly used spellings. It seems like writeable is the commonly used term in computer sciences and writable is the normal english adjective formed from to write + -able in general English.[1] As we are refering to computer science related writeable it should be left as is imho. But as I'm no native speaker, you might feel different on this. On a side note: Samba offers both spellings as valid for thier configuration files. [2] [1] http://www.thefreedictionary.com/writeable [2] http://forums.contribs.org/index.php?topic=22258.0 With regards, Dipl. Phys. Jan M. Simons Institute of Crystallography RWTH Aachen University Commonly used is not necessarily correct. The Oxford dictionary only accepts writable (even when I select american english). Same result with Merriam-Webster. Google suggests writable instead of writeable. I see writeable only in computer programs and related contexts, therefore I assume that it is simply a very common spelling error contributed by non-native speakers (like myself). Interesting detail: The spelling checker of my mailing client (Icedove) marks writeable as correct and writable as wrong (which is obviously wrong). Maybe I should send a bug report to Debian. The ratio of writeable:writable in unpatched qemu is 37:47. Even if both writings were correct, I might argue that a uniform spelling is better and choose the form which is more commonly used. Let's improve the spelling for computer programs a little bit! Debian's myspell-en-gb is wrong, so I just sent a bug report: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=624249 Several other spelling lists included in Debian are correct, e.g. hunspell-en-us, iamerican, ibritish, aspell-en. Here is a list from a native (american) speaker with common spelling bugs: http://marvin.cs.uidaho.edu/misspell.html It also includes writeable. Cheers, Stefan W.
Re: [Qemu-devel] docs/tracing.txt: minor documentation fixes
On 27/04/11 06:46 +1000, Brad Hards wrote: On Tue, 26 Apr 2011 10:26:01 pm Stefan Hajnoczi wrote: -There is a set of static trace events declared in the trace-events source +There is a set of static trace events declared in the trace-events source Would it read better if it said There are a set... (i.e. are instead of is)? I belive it's correct: there is a set. The alternative would be there are static trace events. Mike -- Mike Day | ul...@ncultra.org | Endurance is a Virtue
Re: [Qemu-devel] [PATCH] Fix bug with virtio-9p fsync
On 04/26/2011 09:51 AM, Sassan Panahinejad wrote:
v9fs_fsync and possibly others break when asked to operate on a directory.
It does not check fid_type to see if it is operating on a directory and
therefore accesses the wrong element of the fs union.
This error can result in guest applications failing (in my case it was dpkg).
This patch fixes the issue, although there may be other, similar bugs in
virtio-9p.
Signed-off-by: Sassan Panahinejadsas...@sassan.me.uk
---
Here I've implemented it as a function. If you'd prefer a macro or inline
function then let me know.
Thanks
Sassan
Please put your commentary on the top. After --- you should only
expect code diff.
hw/virtio-9p.c | 43 +--
1 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..26be0fc 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1860,6 +1860,18 @@ static void v9fs_post_do_fsync(V9fsState *s, V9fsPDU
*pdu, int err)
complete_pdu(s, pdu, err);
}
+static int v9fs_get_fd(V9fsFidState *fidp)
+{
+switch (fidp-fid_type) {
+case P9_FID_FILE:
+return fidp-fs.fd;
+case P9_FID_DIR:
+return dirfd(fidp-fs.dir);
+default:
+return -1;
+}
+}
+
static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
{
int32_t fid;
@@ -1867,6 +1879,7 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
V9fsFidState *fidp;
int datasync;
int err;
+int fd;
pdu_unmarshal(pdu, offset, dd,fid,datasync);
fidp = lookup_fid(s, fid);
@@ -1875,7 +1888,11 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
v9fs_post_do_fsync(s, pdu, err);
return;
}
-err = v9fs_do_fsync(s, fidp-fs.fd, datasync);
+if ((fd = v9fs_get_fd(fidp))= 0) {
+err = v9fs_do_fsync(s, fd, datasync);
+} else {
+ err = -EINVAL;
+}
v9fs_post_do_fsync(s, pdu, err);
}
@@ -2984,6 +3001,7 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu)
int32_t fid;
V9fsWstatState *vs;
int err = 0;
+int fd;
vs = qemu_malloc(sizeof(*vs));
vs-pdu = pdu;
@@ -2999,7 +3017,10 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu)
/* do we need to sync the file? */
if (donttouch_stat(vs-v9stat)) {
-err = v9fs_do_fsync(s, vs-fidp-fs.fd, 0);
+if ((fd = v9fs_get_fd(vs-fidp))= 0) {
+err = v9fs_do_fsync(s, fd, 0);
+} else
+err = -EINVAL;
v9fs_wstat_post_fsync(s, vs, err);
return;
}
@@ -3176,6 +3197,7 @@ static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
{
int32_t fid, err = 0;
V9fsLockState *vs;
+int fd;
vs = qemu_mallocz(sizeof(*vs));
vs-pdu = pdu;
@@ -3198,8 +3220,12 @@ static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
err = -ENOENT;
goto out;
}
-
-err = v9fs_do_fstat(s, vs-fidp-fs.fd,vs-stbuf);
+if ((fd = v9fs_get_fd(vs-fidp))= 0) {
+err = v9fs_do_fstat(s, fd,vs-stbuf);
+} else {
+err = -EINVAL;
+goto out;
+}
I think we need a different handle for lock and getlock.
If the operation is on a dir we need to return
EISDIR
The /cmd/ parameter is F_GETLK, F_GETLK64, F_SETLK, F_SETLK64,
F_SETLKW, or F_SETLKW64, and /fildes/ refers to a directory.
Since the handling is different, I am not sure if the separate function
makes any sense now.
May be go back to your initial check for sync and for locks, if the
fid is a dir type then EISDIR
otherwise EINVAL.
Thanks,
JV
if (err 0) {
err = -errno;
goto out;
@@ -3221,6 +3247,7 @@ static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
{
int32_t fid, err = 0;
V9fsGetlockState *vs;
+int fd;
vs = qemu_mallocz(sizeof(*vs));
vs-pdu = pdu;
@@ -3236,8 +3263,12 @@ static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
err = -ENOENT;
goto out;
}
-
-err = v9fs_do_fstat(s, vs-fidp-fs.fd,vs-stbuf);
+if ((fd = v9fs_get_fd(vs-fidp))= 0) {
+err = v9fs_do_fstat(s, fd,vs-stbuf);
+} else {
+err = -EINVAL;
+goto out;
+}
if (err 0) {
err = -errno;
goto out;
Re: [Qemu-devel] [PULL] linux-user pending patches
On Tue, Apr 26, 2011 at 10:41:07AM +0300, Riku Voipio wrote: The following changes since commit b0b36e5d2e4c8a96c2f6dbc0981a9fd0cde111d8: doc: fix slirp description (2011-04-25 23:10:04 +0200) are available in the git repository at: git://gitorious.org/qemu-maemo/qemu.git linux-user-for-upstream Alexander Graf (1): linux-user: add s390x to llseek list Laurent Vivier (3): linux-user: improve traces linux-user: convert ioctl(SIOCGIFCONF, ...) result. linux-user: add ioctl(SIOCGIWNAME, ...) support. Riku Voipio (2): [v2] linux-user: bigger default stack linux-user: untie syscalls from UID16 linux-user/alpha/syscall_nr.h |7 -- linux-user/ioctls.h |4 +- linux-user/strace.c | 161 + linux-user/strace.list| 12 ++-- linux-user/syscall.c | 154 +++ linux-user/syscall_defs.h |8 ++- 6 files changed, 315 insertions(+), 31 deletions(-) Thanks, pulled. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] configure: support target dependent linking
On Tue, Apr 26, 2011 at 12:24:07AM +0200, Michael Walle wrote:
This patch is the first attempt to make configure more intelligent with
regard to how it links to libraries. It divides the softmmu libraries into
two lists, a general one and a list which depends on the target
architecture.
Signed-off-by: Michael Walle mich...@walle.cc
Reviewed-by: Aurelien Jarno aurel...@aurel32.net
---
configure | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
Thanks, both applied.
diff --git a/configure b/configure
index da2da04..ca675f6 100755
--- a/configure
+++ b/configure
@@ -1946,11 +1946,11 @@ int main(void) { return 0; }
EOF
if compile_prog $fdt_libs ; then
fdt=yes
-libs_softmmu=$fdt_libs $libs_softmmu
else
if test $fdt = yes ; then
feature_not_found fdt
fi
+fdt_libs=
fdt=no
fi
fi
@@ -1967,11 +1967,11 @@ int main(void) { GL_VERSION; return 0; }
EOF
if compile_prog -lGL ; then
opengl=yes
- libs_softmmu=$opengl_libs $libs_softmmu
else
if test $opengl = yes ; then
feature_not_found opengl
fi
+opengl_libs=
opengl=no
fi
fi
@@ -3071,6 +3071,7 @@ target_short_alignment=2
target_int_alignment=4
target_long_alignment=4
target_llong_alignment=8
+target_libs_softmmu=
TARGET_ARCH=$target_arch2
TARGET_BASE_ARCH=
@@ -3104,6 +3105,7 @@ case $target_arch2 in
;;
lm32)
target_phys_bits=32
+target_libs_softmmu=$opengl_libs
;;
m68k)
bflt=yes
@@ -3118,6 +3120,7 @@ case $target_arch2 in
bflt=yes
target_nptl=yes
target_phys_bits=32
+target_libs_softmmu=$fdt_libs
;;
mips|mipsel)
TARGET_ARCH=mips
@@ -3142,6 +3145,7 @@ case $target_arch2 in
gdb_xml_files=power-core.xml power-fpu.xml power-altivec.xml
power-spe.xml
target_phys_bits=32
target_nptl=yes
+target_libs_softmmu=$fdt_libs
;;
ppcemb)
TARGET_BASE_ARCH=ppc
@@ -3149,6 +3153,7 @@ case $target_arch2 in
gdb_xml_files=power-core.xml power-fpu.xml power-altivec.xml
power-spe.xml
target_phys_bits=64
target_nptl=yes
+target_libs_softmmu=$fdt_libs
;;
ppc64)
TARGET_BASE_ARCH=ppc
@@ -3156,6 +3161,7 @@ case $target_arch2 in
gdb_xml_files=power64-core.xml power-fpu.xml power-altivec.xml
power-spe.xml
target_phys_bits=64
target_long_alignment=8
+target_libs_softmmu=$fdt_libs
;;
ppc64abi32)
TARGET_ARCH=ppc64
@@ -3164,6 +3170,7 @@ case $target_arch2 in
echo TARGET_ABI32=y $config_target_mak
gdb_xml_files=power64-core.xml power-fpu.xml power-altivec.xml
power-spe.xml
target_phys_bits=64
+target_libs_softmmu=$fdt_libs
;;
sh4|sh4eb)
TARGET_ARCH=sh4
@@ -3249,7 +3256,7 @@ fi
if test $target_softmmu = yes ; then
echo TARGET_PHYS_ADDR_BITS=$target_phys_bits $config_target_mak
echo CONFIG_SOFTMMU=y $config_target_mak
- echo LIBS+=$libs_softmmu $config_target_mak
+ echo LIBS+=$libs_softmmu $target_libs_softmmu $config_target_mak
echo HWDIR=../libhw$target_phys_bits $config_target_mak
echo subdir-$target: subdir-libhw$target_phys_bits $config_host_mak
fi
--
1.7.2.3
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Wed, Apr 27, 2011 at 12:07 AM, Blue Swirl blauwir...@gmail.com wrote:
On Tue, Apr 26, 2011 at 10:07 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Tue, Apr 26, 2011 at 10:36 PM, Blue Swirl blauwir...@gmail.com wrote:
On Tue, Apr 26, 2011 at 8:02 PM, Artyom Tarasenko atar4q...@gmail.com
wrote:
On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno aurel...@aurel32.net
wrote:
On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
laurent.desnog...@gmail.com wrote:
On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko
atar4q...@gmail.com wrote:
On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
Do you have public test case?
It is possible to code this delay slot write test but real issue
may
be corruption elsewhere.
The test case is trivial: it's just the two instructions, branch
and wrpr.
In theory there could be multiple issues including compiler
induced ones.
I'd prefer to see some kind of reproducible testcase.
Ok, attached a 40 byte long test (the first 32 bytes are not used
and
needed only because the bios entry point is 0x20).
$ git pull make sparc64-softmmu/qemu-system-sparc64 -bios
test-wrpr.bin -nographic
Already up-to-date.
make[1]: Nothing to be done for `all'.
/mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
Aborted
The problem seems to be that wrpr is using a non-local
TCG tmp (cpu_tmp0).
Just tried the test case with write to %pil - seems like write itself
is OK.
The issue appears to be with save_state() call since adding save_state
to %pil case provokes the same tcg abort.
The problem is that cpu_tmp0, not being a local tmp, doesn't
need to be saved across helper calls. This results in the
TCG optimizer getting rid of it even though it's later used.
Look at the log and you'll see what I mean :-)
I'm not very comfortable with tcg yet. Would it be possible to teach
optimizer working with delay slots? Or do I look in the wrong place.
The problem is not on the TCG side, but on the target-sparc/translate.c
side:
| case 0x32: /* wrwim, V9 wrpr */
| {
| if (!supervisor(dc))
| goto priv_insn;
| tcg_gen_xor_tl(cpu_tmp0, cpu_src1,
cpu_src2);
| #ifdef TARGET_SPARC64
Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
saved across TCG branches.
[...]
| case 6: // pstate
| save_state(dc, cpu_cond);
| gen_helper_wrpstate(cpu_tmp0);
| dc-npc = DYNAMIC_PC;
| break;
save_state() calls save_npc(), which in turns might call
gen_generic_branch():
Hmm. This is not the only instruction using save_state() and cpu_tmp0.
At least ldd is another example.
| static inline void gen_generic_branch(target_ulong npc1, target_ulong
npc2,
| TCGv r_cond)
| {
| int l1, l2;
|
| l1 = gen_new_label();
| l2 = gen_new_label();
|
| tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
|
| tcg_gen_movi_tl(cpu_npc, npc1);
| tcg_gen_br(l2);
|
| gen_set_label(l1);
| tcg_gen_movi_tl(cpu_npc, npc2);
| gen_set_label(l2);
| }
And here is the TCG branch, which drop the TCG temp cpu_temp0.
The solution is either to rewrite gen_generic_branch() without TCG
branches, or to use a TCG temp local instead of a TCG temp.
You mean something like
case 6: // pstate
{
TCGv r_temp;
r_temp = tcg_temp_new();
tcg_gen_mov_tl(r_temp, cpu_tmp0);
save_state(dc, cpu_cond);
gen_helper_wrpstate(r_temp);
tcg_temp_free(r_temp);
dc-npc = DYNAMIC_PC;
}
break;
?
This fails with the same error message.
Close, but you need to use tcg_temp_local_new(). Does this work?
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 3c958b2..52fa2f1 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -3505,9 +3505,15 @@ static void disas_sparc_insn(DisasContext * dc)
tcg_gen_mov_tl(cpu_tbr, cpu_tmp0);
break;
case 6: // pstate
-
