Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough
On Wed, Aug 31, 2011 at 6:00 PM, Dhaval Giani dhaval.gi...@gmail.com wrote: On Wed, Aug 31, 2011 at 10:58 AM, Blue Swirl blauwir...@gmail.com wrote: On Wed, Aug 31, 2011 at 8:38 AM, Avi Kivity a...@redhat.com wrote: On 08/26/2011 10:06 PM, Blue Swirl wrote: Let guests inject tracepoint data via fw_cfg device. At least on x86, fw_cfg is pretty slow, involving multiple exits. IMO, for kvm, even one exit per tracepoint is too high. We need to use a shared memory transport with a way to order guest/host events later on (by using a clock). This could be an easy way, if the guest always had access to an accurate clock, but that may not be the case. From what I understand, kvmclock should be good enoguh for this purpose. That is what I am using. It's only available for KVM on x86, that is most certainly not enough.
Re: [Qemu-devel] [PATCH v2] pc: Fix and clean up PIC-to-APIC IRQ path
On Thu, Sep 1, 2011 at 9:08 AM, Jan Kiszka jan.kis...@siemens.com wrote:
The master PIC is connected to the LINTIN0 of the APICs. As the APIC
currently does not track the state of that line, we have to ask the PIC
to reinject its IRQ after the CPU picked up an event from the APIC.
This introduces pic_get_output to read the master PIC IRQ line state
without changing it. The APIC uses this function to decide if a PIC IRQ
should be reinjected on apic_update_irq. This reflects better how the
real hardware works.
The patch fixes some failures of the kvm unit tests apic and eventinj by
allowing to enable the proper CPU IRQ deassertion when the guest masks
some pending IRQs at PIC level.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
Changes in v2:
- Avoid adding pic_level to the APIC state, obtain PIC output via
pic_get_level instead
- Do not reassert PIC interrupt if APIC is not accepting it
- Use apic_deliver_pic_intr for reassertion to ensure correct
processing
This is not as nice as the previous version /wrt the interaction of PIC
and APIC. But it avoids breaking the APIC vmstate for the sake of
internal changes, also keeping it compatible with the upcoming KVM
in-kernel APIC (that allows no easy pic_level state extraction). The
interconnection between PIC and APIC may look nicer in the future with
QOM. And in the end this just reflects the beauty of the x86
architecture.
hw/apic.c | 4
hw/i8259.c | 15 +++
hw/pc.c | 3 ---
hw/pc.h | 2 +-
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index d8f56c8..8289eef 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -23,6 +23,7 @@
#include host-utils.h
#include sysbus.h
#include trace.h
+#include pc.h
/* APIC Local Vector Table */
#define APIC_LVT_TIMER 0
@@ -399,6 +400,9 @@ static void apic_update_irq(APICState *s)
}
if (apic_irq_pending(s) 0) {
cpu_interrupt(s-cpu_env, CPU_INTERRUPT_HARD);
+ } else if (apic_accept_pic_intr(s-busdev.qdev)
+ pic_get_output(isa_pic)) {
This is indeed ugly. Why doesn't APIC track PIC output?
+ apic_deliver_pic_intr(s-busdev.qdev, 1);
}
}
diff --git a/hw/i8259.c b/hw/i8259.c
index c0b96ab..5498e5b 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -144,8 +144,7 @@ static int pic_get_irq(PicState *s)
/* raise irq to CPU if necessary. must be called every time the active
irq may change */
-/* XXX: should not export it, but it is needed for an APIC kludge */
-void pic_update_irq(PicState2 *s)
+static void pic_update_irq(PicState2 *s)
{
int irq2, irq;
@@ -172,14 +171,9 @@ void pic_update_irq(PicState2 *s)
printf(pic: cpu_interrupt\n);
#endif
qemu_irq_raise(s-parent_irq);
- }
-
-/* all targets should do this rather than acking the IRQ in the cpu */
-#if defined(TARGET_MIPS) || defined(TARGET_PPC) || defined(TARGET_ALPHA)
- else {
+ } else {
qemu_irq_lower(s-parent_irq);
}
-#endif
}
#ifdef DEBUG_IRQ_LATENCY
@@ -436,6 +430,11 @@ uint32_t pic_intack_read(PicState2 *s)
return ret;
}
+int pic_get_output(PicState2 *s)
+{
+ return (pic_get_irq(s-pics[0]) = 0);
+}
+
static void elcr_ioport_write(void *opaque, uint32_t addr, uint32_t val)
{
PicState *s = opaque;
diff --git a/hw/pc.c b/hw/pc.c
index 263fb1a..b7b5d6f 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -156,9 +156,6 @@ int cpu_get_pic_interrupt(CPUState *env)
intno = apic_get_interrupt(env-apic_state);
if (intno = 0) {
- /* set irq request if a PIC irq is still pending */
- /* XXX: improve that */
- pic_update_irq(isa_pic);
return intno;
}
/* read the irq from the PIC */
diff --git a/hw/pc.h b/hw/pc.h
index dae736e..958c77d 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -65,7 +65,7 @@ void pic_set_irq(int irq, int level);
void pic_set_irq_new(void *opaque, int irq, int level);
qemu_irq *i8259_init(qemu_irq parent_irq);
int pic_read_irq(PicState2 *s);
-void pic_update_irq(PicState2 *s);
+int pic_get_output(PicState2 *s);
uint32_t pic_intack_read(PicState2 *s);
void pic_info(Monitor *mon);
void irq_info(Monitor *mon);
Re: [Qemu-devel] [PATCH] [SPARC] Gdbstub: Fix back-trace on SPARC32
On Thu, Sep 1, 2011 at 2:17 PM, Fabien Chouteau chout...@adacore.com wrote:
Gdb expects all registers windows to be flushed in ram, which is not the case
in Qemu. Therefore the back-trace generation doesn't work. This patch adds a
function to handle reads/writes in stack frames as if windows were flushed.
Signed-off-by: Fabien Chouteau chout...@adacore.com
---
gdbstub.c | 10 --
target-sparc/cpu.h | 7
target-sparc/helper.c | 85
+
3 files changed, 99 insertions(+), 3 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 3b87c27..85d5ad7 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -41,6 +41,9 @@
#include qemu_socket.h
#include kvm.h
+#ifndef TARGET_CPU_MEMORY_RW_DEBUG
+#define TARGET_CPU_MEMORY_RW_DEBUG cpu_memory_rw_debug
These days, inline functions are preferred over macros.
+#endif
enum {
GDB_SIGNAL_0 = 0,
@@ -2013,7 +2016,7 @@ static int gdb_handle_packet(GDBState *s, const char
*line_buf)
if (*p == ',')
p++;
len = strtoull(p, NULL, 16);
- if (cpu_memory_rw_debug(s-g_cpu, addr, mem_buf, len, 0) != 0) {
+ if (TARGET_CPU_MEMORY_RW_DEBUG(s-g_cpu, addr, mem_buf, len, 0) !=
0) {
cpu_memory_rw_debug() could remain unwrapped with a generic function
like cpu_gdb_sync_memory() which gdbstub should explicitly call.
Maybe the lazy condition codes etc. could be handled in similar way,
cpu_gdb_sync_registers().
put_packet (s, E14);
} else {
memtohex(buf, mem_buf, len);
@@ -2028,10 +2031,11 @@ static int gdb_handle_packet(GDBState *s, const char
*line_buf)
if (*p == ':')
p++;
hextomem(mem_buf, p, len);
- if (cpu_memory_rw_debug(s-g_cpu, addr, mem_buf, len, 1) != 0)
+ if (TARGET_CPU_MEMORY_RW_DEBUG(s-g_cpu, addr, mem_buf, len, 1) !=
0) {
put_packet(s, E14);
- else
+ } else {
put_packet(s, OK);
+ }
break;
case 'p':
/* Older gdb are really dumb, and don't use 'g' if 'p' is avaialable.
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 8654f26..3f76eaf 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -495,6 +495,13 @@ int cpu_sparc_handle_mmu_fault(CPUSPARCState *env1,
target_ulong address, int rw
target_ulong mmu_probe(CPUSPARCState *env, target_ulong address, int mmulev);
void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env);
+#if !defined(TARGET_SPARC64)
+int sparc_cpu_memory_rw_debug(CPUState *env, target_ulong addr,
+ uint8_t *buf, int len, int is_write);
+#define TARGET_CPU_MEMORY_RW_DEBUG sparc_cpu_memory_rw_debug
+#endif
+
+
/* translate.c */
void gen_intermediate_code_init(CPUSPARCState *env);
diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 1fe1f07..2cf4e8b 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -358,6 +358,91 @@ void dump_mmu(FILE *f, fprintf_function cpu_fprintf,
CPUState *env)
}
}
+
+/* Gdb expects all registers windows to be flushed in ram. This function
handles
+ * reads/writes in stack frames as if windows were flushed. We assume that
the
+ * sparc ABI is followed.
+ */
We can't assume that, it depends on what we are executing (BIOS, OS,
even application). On Sparc64 there are two ABIs (32 bit and 64 bit
with offset of -2047), though calling flushw instruction could handle
that. If the flush happens to trigger a fault, we're in big trouble.
Overall, I think this is too hackish. Maybe this is a bug in GDB
instead, information from backtrace is not reliable if ABI is not
known.
+int sparc_cpu_memory_rw_debug(CPUState *env, target_ulong addr,
+ uint8_t *buf, int len, int is_write)
+{
+ int i;
+ int len1;
+ int cwp = env-cwp;
+
+ for (i = 0; i env-nwindows; i++) {
+ int off;
+ target_ulong fp = env-regbase[cwp * 16 + 22];
+
+ /* Assume fp == 0 means end of frame. */
+ if (fp == 0) {
+ break;
+ }
+
+ cwp = cpu_cwp_inc(env, cwp + 1);
+
+ /* Invalid window ? */
+ if (env-wim (1 cwp)) {
+ break;
+ }
+
+ /* According to the ABI, the stack is growing downward. */
+ if (addr + len fp) {
+ break;
+ }
+
+ /* Not in this frame. */
+ if (addr fp + 64) {
+ continue;
+ }
+
+ /* Handle access before this window. */
+ if (addr fp) {
+ len1 = fp - addr;
+ if (cpu_memory_rw_debug(env, addr, buf, len1, is_write) != 0) {
+ return -1;
+ }
+ addr += len1;
+ len -= len1;
+ buf += len1;
+ }
+
+ /* Access byte per byte to registers. Not very efficient but speed is
+ * not critical.
+ */
+
Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough
On Sat, Sep 3, 2011 at 1:53 AM, Blue Swirl blauwir...@gmail.com wrote: On Wed, Aug 31, 2011 at 6:00 PM, Dhaval Giani dhaval.gi...@gmail.com wrote: On Wed, Aug 31, 2011 at 10:58 AM, Blue Swirl blauwir...@gmail.com wrote: On Wed, Aug 31, 2011 at 8:38 AM, Avi Kivity a...@redhat.com wrote: On 08/26/2011 10:06 PM, Blue Swirl wrote: Let guests inject tracepoint data via fw_cfg device. At least on x86, fw_cfg is pretty slow, involving multiple exits. IMO, for kvm, even one exit per tracepoint is too high. We need to use a shared memory transport with a way to order guest/host events later on (by using a clock). This could be an easy way, if the guest always had access to an accurate clock, but that may not be the case. From what I understand, kvmclock should be good enoguh for this purpose. That is what I am using. It's only available for KVM on x86, that is most certainly not enough. From what I understood it is available on other architectures as well, but let us just confirm with glommer. Thanks! Dhaval
Re: [Qemu-devel] [PATCH] Add support for r6040 NIC
On Thu, Sep 1, 2011 at 9:49 PM, Anthony Liguori anth...@codemonkey.ws wrote: On 09/01/2011 02:39 AM, Markus Armbruster wrote: Blue Swirlblauwir...@gmail.com writes: On Wed, Aug 31, 2011 at 4:06 PM, Anthony Liguorianth...@codemonkey.ws wrote: On 08/31/2011 09:35 AM, malc wrote: On Wed, 31 Aug 2011, Anthony Liguori wrote: Upper case field names are not okay. If you think coding style isn't clear, that's a bug in coding style. Sez hu? Coding style is garbage that should be thrown out of the window. As for looking, yeah, i'm looking at usb with it's lovely hungarian fields, should we stampede to fix it? If the one who's going to maintain the code is fine with whatever naming is used so be it. No. That's how we got into the coding style mess we're in in the first place. There's no benefit to going through and changing existing code but new code needs to be consistent with the vast majority of code in the rest of the tree. It's about overall code base consistency and maintainability. I agree about importance of consistency, though I'd even go further and reformat globally. New code gets introduced based on copying old code so the pain goes on. If we reformat globally (big if), I'm very strongly opposed to doing a global reformat. It makes it harder to use things like git blame which makes reviewing code difficult. Only for one commit, the commits before and after would still be accurate. There is a tradeoff between benefit of consistent, better quality code and usefulness of git blame. I'd value consistency higher. Inaccuracy is also relative, the reformatting commit is still a commit. Following a reasonable policy of using a consistent coding style and only fixing style issues when you touch code for other reasons is well established (this is the kernel policy) and over time will result in a reasonably consistent code base. This assumes that all code gets touched every now and then. But in reality some things may never need any changes and then they remain inconsistent. Then bad practices spread from this unchanged base if it's large enough, just like now. By the way, if we assume the opposite (all code is under change), then some time after global reformat git blame would also be accurate since the reformatting commit would be overwritten by the newer changes.
Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough
On Sat, Sep 3, 2011 at 9:26 AM, Dhaval Giani dhaval.gi...@gmail.com wrote: On Sat, Sep 3, 2011 at 1:53 AM, Blue Swirl blauwir...@gmail.com wrote: On Wed, Aug 31, 2011 at 6:00 PM, Dhaval Giani dhaval.gi...@gmail.com wrote: On Wed, Aug 31, 2011 at 10:58 AM, Blue Swirl blauwir...@gmail.com wrote: On Wed, Aug 31, 2011 at 8:38 AM, Avi Kivity a...@redhat.com wrote: On 08/26/2011 10:06 PM, Blue Swirl wrote: Let guests inject tracepoint data via fw_cfg device. At least on x86, fw_cfg is pretty slow, involving multiple exits. IMO, for kvm, even one exit per tracepoint is too high. We need to use a shared memory transport with a way to order guest/host events later on (by using a clock). This could be an easy way, if the guest always had access to an accurate clock, but that may not be the case. From what I understand, kvmclock should be good enoguh for this purpose. That is what I am using. It's only available for KVM on x86, that is most certainly not enough. From what I understood it is available on other architectures as well, but let us just confirm with glommer. This line in Makefile.target limits the device to KVM on x86: obj-i386-$(CONFIG_KVM) += kvmclock.o Moreover, the code assumes that CPUState structure contains a field cpuid_kvm_features but that is only available on x86. Perhaps the device can be made generic but I don't see how it should work with TCG. Actually I don't understand it at all, I think key functionality must be inside KVM module.
Re: [Qemu-devel] [PATCH v2] pc: Fix and clean up PIC-to-APIC IRQ path
On 2011-09-03 10:58, Blue Swirl wrote:
On Thu, Sep 1, 2011 at 9:08 AM, Jan Kiszka jan.kis...@siemens.com wrote:
The master PIC is connected to the LINTIN0 of the APICs. As the APIC
currently does not track the state of that line, we have to ask the PIC
to reinject its IRQ after the CPU picked up an event from the APIC.
This introduces pic_get_output to read the master PIC IRQ line state
without changing it. The APIC uses this function to decide if a PIC IRQ
should be reinjected on apic_update_irq. This reflects better how the
real hardware works.
The patch fixes some failures of the kvm unit tests apic and eventinj by
allowing to enable the proper CPU IRQ deassertion when the guest masks
some pending IRQs at PIC level.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
Changes in v2:
- Avoid adding pic_level to the APIC state, obtain PIC output via
pic_get_level instead
- Do not reassert PIC interrupt if APIC is not accepting it
- Use apic_deliver_pic_intr for reassertion to ensure correct
processing
This is not as nice as the previous version /wrt the interaction of PIC
and APIC. But it avoids breaking the APIC vmstate for the sake of
internal changes, also keeping it compatible with the upcoming KVM
in-kernel APIC (that allows no easy pic_level state extraction). The
interconnection between PIC and APIC may look nicer in the future with
QOM. And in the end this just reflects the beauty of the x86
architecture.
hw/apic.c |4
hw/i8259.c | 15 +++
hw/pc.c|3 ---
hw/pc.h|2 +-
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index d8f56c8..8289eef 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -23,6 +23,7 @@
#include host-utils.h
#include sysbus.h
#include trace.h
+#include pc.h
/* APIC Local Vector Table */
#define APIC_LVT_TIMER 0
@@ -399,6 +400,9 @@ static void apic_update_irq(APICState *s)
}
if (apic_irq_pending(s) 0) {
cpu_interrupt(s-cpu_env, CPU_INTERRUPT_HARD);
+} else if (apic_accept_pic_intr(s-busdev.qdev)
+ pic_get_output(isa_pic)) {
This is indeed ugly. Why doesn't APIC track PIC output?
For the reasons explained above.
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] target_phys_addr_t vs ram_addr_t
On Fri, Sep 2, 2011 at 6:08 AM, Sinha, Ani ani.si...@tellabs.com wrote: Hi Folks : I am trying to write a virtio driver and towards this end I am looking at the qemu code. I am a little confused about a few things. Unfortunately, the few comments in the code does not make it clear for me. So I am wondering if any kind person on this mailing list would be able to help. First off, what is the difference between target_phys_addr_t and ram_addr_t? I believe the former is a virtual address within the guest but what is the later? The comment says address in ram (different from physical address) but is this the virtual address or the physical address? Is this for guest or for host? target_phys_addr_t is used for guest physical addresses. For example, i386 guest with PAE would need 36 bits which makes this 64 bit type (regardless of host address sizes). Devices should usually only use this type. ram_addr_t (uintptr_t) is linked to the size of host virtual address space. Because of how RAM is implemented, we can't give 64 bit guests more than 4 GB of RAM on a 32 bit host, so in that case it would be 32 bits. On a 64 bit host this can be 64 bits. It can be considered as a subset of target_phys_addr_t. Normally devices need it only if there are RAM areas, like frame buffers. target_ulong is used for guest CPU registers and guest virtual addresses. In the i386 guest with PAE case, this would remain 32 bits. For a 64 bit guest, this is a 64 bit type. Normal devices may not use this since it breaks the layering. In your virtio case things are a bit more complicated since guest and host share the same architecture. Paravirtualized devices may break these rules with caution. Secondly, in function cpu_physical_memory_map(), why is the length parameter an address? If I look at the function virtqueue_map_sg(), the sg.iov_len is defined as type size_t, which sounds like right. However, this value is assigned to variable len which is of type target_phys_addr_t. Is len an address or just a scalar value? size_t depends on host address size. Devices can map larger than 4G physical areas even on a 32 bit host, so target_phys_addr_t must be used. Lastly, in qemu_ram_ptr_length(), what is the length value? What does it signify? The length of the RAM area. RAM can be non-contiguous and base address may be nonzero. One more thing. It would really help guys like me if someone can add comments regarding the various apis, what they do and what the parameters mean in the code. I thought I'd suggest. Patches are welcome :) I am not in the mailing list. So please do a reply-all when responding. For many reasons it is a poor idea to develop code in-house without constant communication to upstream developers, so please keep us informed of your development (RFC patches etc.) if you intend to submit it one day.
Re: [Qemu-devel] [PATCH v2] pc: Fix and clean up PIC-to-APIC IRQ path
On Sat, Sep 3, 2011 at 11:17 AM, Jan Kiszka jan.kis...@web.de wrote:
On 2011-09-03 10:58, Blue Swirl wrote:
On Thu, Sep 1, 2011 at 9:08 AM, Jan Kiszka jan.kis...@siemens.com wrote:
The master PIC is connected to the LINTIN0 of the APICs. As the APIC
currently does not track the state of that line, we have to ask the PIC
to reinject its IRQ after the CPU picked up an event from the APIC.
This introduces pic_get_output to read the master PIC IRQ line state
without changing it. The APIC uses this function to decide if a PIC IRQ
should be reinjected on apic_update_irq. This reflects better how the
real hardware works.
The patch fixes some failures of the kvm unit tests apic and eventinj by
allowing to enable the proper CPU IRQ deassertion when the guest masks
some pending IRQs at PIC level.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
Changes in v2:
- Avoid adding pic_level to the APIC state, obtain PIC output via
pic_get_level instead
- Do not reassert PIC interrupt if APIC is not accepting it
- Use apic_deliver_pic_intr for reassertion to ensure correct
processing
This is not as nice as the previous version /wrt the interaction of PIC
and APIC. But it avoids breaking the APIC vmstate for the sake of
internal changes, also keeping it compatible with the upcoming KVM
in-kernel APIC (that allows no easy pic_level state extraction). The
interconnection between PIC and APIC may look nicer in the future with
QOM. And in the end this just reflects the beauty of the x86
architecture.
hw/apic.c | 4
hw/i8259.c | 15 +++
hw/pc.c | 3 ---
hw/pc.h | 2 +-
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index d8f56c8..8289eef 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -23,6 +23,7 @@
#include host-utils.h
#include sysbus.h
#include trace.h
+#include pc.h
/* APIC Local Vector Table */
#define APIC_LVT_TIMER 0
@@ -399,6 +400,9 @@ static void apic_update_irq(APICState *s)
}
if (apic_irq_pending(s) 0) {
cpu_interrupt(s-cpu_env, CPU_INTERRUPT_HARD);
+ } else if (apic_accept_pic_intr(s-busdev.qdev)
+ pic_get_output(isa_pic)) {
This is indeed ugly. Why doesn't APIC track PIC output?
For the reasons explained above.
Breaking vmstate compatibility? Don't we have all kinds of
compatibility stuff for this?
I'm not opposed to the patch as is, it's still a cleanup, but I wish
this ugliness could be avoided.
Re: [Qemu-devel] [PATCH] Rename qemu - qemu-system-i386
On Fri, Sep 2, 2011 at 3:39 PM, Anthony Liguori aligu...@us.ibm.com wrote: On 08/30/2011 02:24 PM, Blue Swirl wrote: On Mon, Aug 29, 2011 at 2:55 PM, Anthony Liguorialigu...@us.ibm.com wrote: This has been discussed before in the past. The special casing really makes no sense anymore. This seems like a good change to make for 1.0. Signed-off-by: Anthony Liguorialigu...@us.ibm.com --- Makefile | 5 ++--- Makefile.target | 4 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 8606849..51ecdb5 100644 --- a/Makefile +++ b/Makefile @@ -365,9 +365,8 @@ tar: rm -rf /tmp/$(FILE) SYSTEM_TARGETS=$(filter %-softmmu,$(TARGET_DIRS)) -SYSTEM_PROGS=$(patsubst qemu-system-i386,qemu, \ - $(patsubst %-softmmu,qemu-system-%, \ - $(SYSTEM_TARGETS))) +SYSTEM_PROGS=$(patsubst %-softmmu,qemu-system-%, \ + $(SYSTEM_TARGETS)) Maybe the patsubst could be avoided, just rename the build directory from *-softmmu to qemu-system-* while at it? USER_TARGETS=$(filter %-user,$(TARGET_DIRS)) USER_PROGS=$(patsubst %-bsd-user,qemu-%, \ Also here the directory and executable names could be made to match. I thought that historically the phrase softmmu was for the variant of libcpu that had an emulated TLB. I know Peter's been talking about a linux-user mode that uses softmmu since certain combinations of architectures today are impossible with linux-user (ia64 on x86_64 for instance). That could be qemu-linux-softmmu-xxx. If we had a similar mode for system emulation which used host MMU, those system emulators could be named qemu-system-hostmmu-xxx. So I think the current naming conventions probably are reasonable although I'm not opposed to changing them. I think that should be a separate patch though. OK. The names of the build directories do not bring users any major benefits in any case. Regards, Anthony Liguori diff --git a/Makefile.target b/Makefile.target index 07af4d4..29287ed 100644 --- a/Makefile.target +++ b/Makefile.target @@ -27,12 +27,8 @@ ifdef CONFIG_USER_ONLY QEMU_PROG=qemu-$(TARGET_ARCH2) else # system emulator name -ifeq ($(TARGET_ARCH), i386) -QEMU_PROG=qemu$(EXESUF) -else QEMU_PROG=qemu-system-$(TARGET_ARCH2)$(EXESUF) endif -endif PROGS=$(QEMU_PROG) STPFILES= -- 1.7.4.1
Re: [Qemu-devel] [PATCH] virtio: Make memory barriers be memory barriers
On Fri, Sep 2, 2011 at 6:49 AM, Paolo Bonzini pbonz...@redhat.com wrote:
On 09/02/2011 02:08 AM, David Gibson wrote:
Signed-off-by: Alexey Kardashevskiya...@ozlabs.ru
Signed-off-by: David Gibsonda...@gibson.dropbear.id.au
It will most definitely break OpenBSD, but anyway:
Uh, why?
They use an ancient compiler because they do not want to use GPLv3. I
thought it was 4.1.something but actually it is 4.2.1, so it should work.
It works:
$ cat sync_synch.c
void f(void)
{
__sync_synchronize();
}
$ gcc -c sync_synch.c -Wall
$ objdump -d sync_synch.o
sync_synch.o: file format elf64-sparc
Disassembly of section .text:
f:
0: 9d e3 bf 40 save %sp, -192, %sp
4: 81 43 e0 0f membar #StoreStore|#LoadStore|#StoreLoad|#LoadLoad
8: 81 cf e0 08 rett %i7 + 8
c: 01 00 00 00 nop
$ gcc -v
Reading specs from
/usr/bin/../lib/gcc-lib/sparc64-unknown-openbsd4.9/4.2.1/specs
Target: sparc64-unknown-openbsd4.9
Configured with: OpenBSD/sparc64 system compiler
Thread model: posix
gcc version 4.2.1 20070719
Re: [Qemu-devel] [RFC PATCH 0/5] Add configure flag to disable TCG
On Fri, Sep 2, 2011 at 9:47 PM, Anthony Liguori aligu...@us.ibm.com wrote:
Hi,
There have been a few attempts in the past to allow TCG to be disabled
at build time. Recently, Alex made the suggestion that we could do it by
using
the same trick that we used to introduce kvm support. That involves
introducing
a tcg_enabled() macro that will be (0) if TCG is disabled in the build.
GCC is smart enough to do dead code elimination if it sees an if (0) and the
result is that if you can do:
Is this also true for gcc optimization -O0? Didn't we have breakages
because of similar issues recently?
if (tcg_enabled()) {
foo();
}
and it's more or less equivalent to:
#ifdef CONFIG_TCG
foo();
#endif
Without the ugliness that comes from using the preprocessor. I think this
ended
up being pretty straight forward. exec.c could use a fair bit of cleanup but
other than that, this pretty much eliminates all of the TCG code from the
build.
This absolutely is going to break non-x86 KVM builds if they use the
--disable-tcg flag as I haven't tested those yet. The normal TCG build
shouldn't be affected at all though.
In principle, the code assumes that you need KVM if you don't have TCG. Of
course, some extra logic could be added to allow for Xen if TCG isn't present.
Re: [Qemu-devel] [PATCH 5/5] tcg: don't build cpu-exec.o, op_helper.o, or fpu/softloat.o when TCG disabled
On Fri, Sep 2, 2011 at 9:48 PM, Anthony Liguori aligu...@us.ibm.com wrote:
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
---
Makefile.target | 8 +---
cpu-exec.c | 19 ---
cpus.c | 11 ++-
exec.c | 29 ++---
softmmu_template.h | 14 ++
target-i386/helper.c | 18 ++
target-i386/op_helper.c | 18 --
7 files changed, 73 insertions(+), 44 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index 0a786b4..00d3039 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -67,9 +67,11 @@ all: $(PROGS) stap
#
# cpu emulator library
-libobj-y = exec.o cpu-exec.o
-libobj-y += fpu/softfloat.o
-libobj-y += op_helper.o helper.o
+libobj-y = exec.o
+libobj-y += helper.o
+libobj-$(CONFIG_TCG) += cpu-exec.o
+libobj-$(CONFIG_TCG) += op_helper.o
+libobj-$(CONFIG_TCG) += fpu/softfloat.o
libobj-$(CONFIG_TCG) += translate.o translate-all.o
libobj-$(CONFIG_TCG) += tcg/tcg.o tcg/optimize.o
ifeq ($(TARGET_BASE_ARCH), i386)
diff --git a/cpu-exec.c b/cpu-exec.c
index de0d716..c5e4e62 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -26,11 +26,6 @@ int tb_invalidated_flag;
//#define CONFIG_DEBUG_EXEC
-bool qemu_cpu_has_work(CPUState *env)
-{
- return cpu_has_work(env);
-}
-
void cpu_loop_exit(CPUState *env)
{
env-current_tb = NULL;
@@ -178,8 +173,6 @@ static void cpu_handle_debug_exception(CPUState *env)
/* main execution loop */
-volatile sig_atomic_t exit_request;
-
int cpu_exec(CPUState *env)
{
int ret, interrupt_request;
@@ -506,8 +499,10 @@ int cpu_exec(CPUState *env)
if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) {
/* restore flags in standard format */
#if defined(TARGET_I386)
- env-eflags = env-eflags | cpu_cc_compute_all(env,
CC_OP)
- | (DF DF_MASK);
+ env-eflags = env-eflags | (DF DF_MASK);
+ if (tcg_enabled()) {
This and the next change are probably not needed if cpu-exec.c is only
compiled with CONFIG_TCG?
+ env-eflags |= cpu_cc_compute_all(env, CC_OP);
+ }
log_cpu_state(env, X86_DUMP_CCOP);
env-eflags = ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A |
CC_P | CC_C);
#elif defined(TARGET_M68K)
@@ -597,8 +592,10 @@ int cpu_exec(CPUState *env)
#if defined(TARGET_I386)
/* restore flags in standard format */
- env-eflags = env-eflags | cpu_cc_compute_all(env, CC_OP)
- | (DF DF_MASK);
+ env-eflags = env-eflags | (DF DF_MASK);
+ if (tcg_enabled()) {
+ env-eflags |= cpu_cc_compute_all(env, CC_OP);
+ }
#elif defined(TARGET_ARM)
/* XXX: Save/restore host fpu exception state?. */
#elif defined(TARGET_UNICORE32)
diff --git a/cpus.c b/cpus.c
index 54c188c..0d7a8ee 100644
--- a/cpus.c
+++ b/cpus.c
@@ -892,6 +892,7 @@ void vm_stop(int reason)
do_vm_stop(reason);
}
+#ifdef CONFIG_TCG
static int tcg_cpu_exec(CPUState *env)
{
int ret;
@@ -929,6 +930,12 @@ static int tcg_cpu_exec(CPUState *env)
}
return ret;
}
+#else
+static int tcg_cpu_exec(CPUState *env)
+{
+ return 0;
+}
+#endif
bool cpu_exec_all(void)
{
@@ -950,8 +957,10 @@ bool cpu_exec_all(void)
if (kvm_enabled()) {
r = kvm_cpu_exec(env);
qemu_kvm_eat_signals(env);
- } else {
+ } else if (tcg_enabled()) {
r = tcg_cpu_exec(env);
+ } else {
+ r = 0;
}
if (r == EXCP_DEBUG) {
cpu_handle_guest_debug(env);
diff --git a/exec.c b/exec.c
index c7decb9..731b7dc 100644
--- a/exec.c
+++ b/exec.c
@@ -230,6 +230,13 @@ static int tlb_flush_count;
static int tb_flush_count;
static int tb_phys_invalidate_count;
+volatile sig_atomic_t exit_request;
+
+bool qemu_cpu_has_work(CPUState *env)
+{
+ return cpu_has_work(env);
+}
+
#ifdef _WIN32
static void map_exec(void *addr, long size)
{
@@ -901,7 +908,9 @@ void tb_phys_invalidate(TranslationBlock *tb,
tb_page_addr_t page_addr)
invalidate_page_bitmap(p);
}
- tb_invalidated_flag = 1;
+ if (tcg_enabled()) {
+ tb_invalidated_flag = 1;
+ }
/* remove the TB from the hash list */
h = tb_jmp_cache_hash_func(tb-pc);
@@ -1002,8 +1011,10 @@ TranslationBlock *tb_gen_code(CPUState *env,
tb_flush(env);
/* cannot fail at this point */
tb = tb_alloc(pc);
- /* Don't forget to invalidate previous TB info. */
- tb_invalidated_flag = 1;
+ if (tcg_enabled()) {
+ /* Don't forget to invalidate previous TB info. */
+ tb_invalidated_flag = 1;
+ }
}
Re: [Qemu-devel] [PATCH 4/5] tcg: don't build tcg/tcg.o, tcg/optimize.o, or translate-all.o when TCG disabled
On Fri, Sep 2, 2011 at 9:48 PM, Anthony Liguori aligu...@us.ibm.com wrote:
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
---
Makefile.target | 6 +++---
exec.c | 44 +---
target-i386/op_helper.c | 2 +-
translate-all.c | 12 +++-
4 files changed, 36 insertions(+), 28 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index 70f52cc..0a786b4 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -67,11 +67,11 @@ all: $(PROGS) stap
#
# cpu emulator library
-libobj-y = exec.o translate-all.o cpu-exec.o
-libobj-y += tcg/tcg.o tcg/optimize.o
+libobj-y = exec.o cpu-exec.o
libobj-y += fpu/softfloat.o
libobj-y += op_helper.o helper.o
-libobj-$(CONFIG_TCG) += translate.o
+libobj-$(CONFIG_TCG) += translate.o translate-all.o
+libobj-$(CONFIG_TCG) += tcg/tcg.o tcg/optimize.o
ifeq ($(TARGET_BASE_ARCH), i386)
libobj-y += cpuid.o
endif
diff --git a/exec.c b/exec.c
index 578da0e..c7decb9 100644
--- a/exec.c
+++ b/exec.c
@@ -574,15 +574,17 @@ static void code_gen_alloc(unsigned long tb_size)
size. */
void tcg_exec_init(unsigned long tb_size)
{
- cpu_gen_init();
- code_gen_alloc(tb_size);
- code_gen_ptr = code_gen_buffer;
- page_init();
+ if (tcg_enabled()) {
+ cpu_gen_init();
+ code_gen_alloc(tb_size);
+ code_gen_ptr = code_gen_buffer;
+ page_init();
#if !defined(CONFIG_USER_ONLY) || !defined(CONFIG_USE_GUEST_BASE)
/* There's no guest base to take into account, so go ahead and
initialize the prologue now. */
- tcg_prologue_init(tcg_ctx);
+ tcg_prologue_init(tcg_ctx);
#endif
+ }
}
bool tcg_in_use(void)
@@ -992,7 +994,6 @@ TranslationBlock *tb_gen_code(CPUState *env,
uint8_t *tc_ptr;
tb_page_addr_t phys_pc, phys_page2;
target_ulong virt_page2;
- int code_gen_size;
phys_pc = get_page_addr_code(env, pc);
tb = tb_alloc(pc);
@@ -1009,8 +1010,11 @@ TranslationBlock *tb_gen_code(CPUState *env,
tb-cs_base = cs_base;
tb-flags = flags;
tb-cflags = cflags;
- cpu_gen_code(env, tb, code_gen_size);
- code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + code_gen_size +
CODE_GEN_ALIGN - 1) ~(CODE_GEN_ALIGN - 1));
+ if (tcg_enabled()) {
+ int code_gen_size;
+ cpu_gen_code(env, tb, code_gen_size);
+ code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + code_gen_size
+ CODE_GEN_ALIGN - 1) ~(CODE_GEN_ALIGN - 1));
This line is (in original already) longer than 80 chars, please break it.
+ }
/* check next page if needed */
virt_page2 = (pc + tb-size - 1) TARGET_PAGE_MASK;
@@ -1090,7 +1094,9 @@ void tb_invalidate_phys_page_range(tb_page_addr_t
start, tb_page_addr_t end,
restore the CPU state */
current_tb_modified = 1;
- cpu_restore_state(current_tb, env, env-mem_io_pc);
+ if (tcg_enabled()) {
+ cpu_restore_state(current_tb, env, env-mem_io_pc);
+ }
cpu_get_tb_cpu_state(env, current_pc, current_cs_base,
current_flags);
}
@@ -1198,9 +1204,11 @@ static void tb_invalidate_phys_page(tb_page_addr_t
addr,
restore the CPU state */
current_tb_modified = 1;
- cpu_restore_state(current_tb, env, pc);
- cpu_get_tb_cpu_state(env, current_pc, current_cs_base,
- current_flags);
+ if (tcg_enabled()) {
+ cpu_restore_state(current_tb, env, pc);
+ cpu_get_tb_cpu_state(env, current_pc, current_cs_base,
+ current_flags);
+ }
}
#endif /* TARGET_HAS_PRECISE_SMC */
tb_phys_invalidate(tb, addr);
@@ -3427,7 +3435,9 @@ static void check_watchpoint(int offset, int len_mask,
int flags)
cpu_abort(env, check_watchpoint: could not find TB for
pc=%p, (void *)env-mem_io_pc);
}
- cpu_restore_state(tb, env, env-mem_io_pc);
+ if (tcg_enabled()) {
+ cpu_restore_state(tb, env, env-mem_io_pc);
+ }
tb_phys_invalidate(tb, -1);
if (wp-flags BP_STOP_BEFORE_ACCESS) {
env-exception_index = EXCP_DEBUG;
@@ -4668,7 +4678,9 @@ void cpu_io_recompile(CPUState *env, void *retaddr)
retaddr);
}
n = env-icount_decr.u16.low + tb-icount;
- cpu_restore_state(tb, env, (unsigned long)retaddr);
+ if (tcg_enabled()) {
+ cpu_restore_state(tb, env, (unsigned long)retaddr);
+ }
/* Calculate how many instructions had been executed before the fault
occurred. */
n = n -
Re: [Qemu-devel] [PATCH 01/10] Add stub functions for PCI device models to do PCI DMA
On Fri, Sep 2, 2011 at 8:35 AM, Avi Kivity a...@redhat.com wrote: On 09/01/2011 07:32 PM, Anthony Liguori wrote: True. But I still think it's the right thing. We can't really pass a MemoryRegion as the source address, since there is no per-device MemoryRegion. Couldn't the PCI bus expose 255 MemoryRegions though? What would those mean? A MemoryRegion is something that can respond to reads and writes. If I understand Anthony's idea, the MemoryRegion (one of 255 in this case) would identify the device in bus agnostic (even PIO vs. MMIO) way. In the numerous previously failed attempts to manage generic DMA, the upstream handle was either an opaque pointer, bus specific structure or IOMMU handle, but this is much better. From the device MemoryRegion, the bus should be discoverable and then the bus can resolve the chain back to host RAM with similar logic to perform the DMA via IOMMUs or whatever strange buses or devices are involved in between. Awesome! It could still use the pci_address_space I think since that should include RAM too, right? No. In fact, initially, you could have a pci_bus_get_device_memory_region(bus, dev) that just returns pci_address_space(). You just need the memory_st[bwl] functions I think. Maybe we need a different type of object here - MemoryClient or something. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [RFC PATCH 0/5] Add configure flag to disable TCG
On 03.09.2011, at 13:58, Blue Swirl wrote: On Fri, Sep 2, 2011 at 9:47 PM, Anthony Liguori aligu...@us.ibm.com wrote: Hi, There have been a few attempts in the past to allow TCG to be disabled at build time. Recently, Alex made the suggestion that we could do it by using the same trick that we used to introduce kvm support. That involves introducing a tcg_enabled() macro that will be (0) if TCG is disabled in the build. GCC is smart enough to do dead code elimination if it sees an if (0) and the result is that if you can do: Is this also true for gcc optimization -O0? Didn't we have breakages because of similar issues recently? We obviously need to provide stubs for code that doesn't get compiled with --disable-tcg, but we do the same for KVM and Xen today. I agree though that we should have a buildbot running a build with -O0 (--enable-debug) to make sure nothing slips through. Alex
Re: [Qemu-devel] target_phys_addr_t vs ram_addr_t
On 3 September 2011 12:26, Blue Swirl blauwir...@gmail.com wrote: On Fri, Sep 2, 2011 at 6:08 AM, Sinha, Ani ani.si...@tellabs.com wrote: First off, what is the difference between target_phys_addr_t and ram_addr_t? I believe the former is a virtual address within the guest but what is the later? The comment says address in ram (different from physical address) but is this the virtual address or the physical address? Is this for guest or for host? target_phys_addr_t is used for guest physical addresses. For example, i386 guest with PAE would need 36 bits which makes this 64 bit type (regardless of host address sizes). Devices should usually only use this type. ram_addr_t (uintptr_t) is linked to the size of host virtual address space. Because of how RAM is implemented, we can't give 64 bit guests more than 4 GB of RAM on a 32 bit host, so in that case it would be 32 bits. On a 64 bit host this can be 64 bits. It can be considered as a subset of target_phys_addr_t. Normally devices need it only if there are RAM areas, like frame buffers. To add to this and point out some particular wrinkles: Even if on the guest machine RAM doesn't start at physical address 0, the first bit of RAM will generally be at a zero ram_addr_t. If the guest machine has some RAM that is mapped at two physical addresses, then both those target_phys_addr_t values will map to the same ram_addr_t. This is why you can't just cast a ram_addr_t to a target_phys_addr_t or vice-versa. (This kind of situation doesn't happen on the PC but does on some of the embedded boards qemu models.) I think of ram_addr_t as being offset into a big lump of host memory which we have parcelled out to use as guest RAM. -- PMM
Re: [Qemu-devel] [PATCH] Rename qemu - qemu-system-i386
Am 02.09.2011 um 17:40 schrieb Anthony Liguori: On 08/29/2011 09:55 AM, Anthony Liguori wrote: This has been discussed before in the past. The special casing really makes no sense anymore. This seems like a good change to make for 1.0. Signed-off-by: Anthony Liguorialigu...@us.ibm.com Applied. Regards, Anthony Liguori --- Makefile|5 ++--- Makefile.target |4 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 8606849..51ecdb5 100644 --- a/Makefile +++ b/Makefile @@ -365,9 +365,8 @@ tar: rm -rf /tmp/$(FILE) SYSTEM_TARGETS=$(filter %-softmmu,$(TARGET_DIRS)) -SYSTEM_PROGS=$(patsubst qemu-system-i386,qemu, \ - $(patsubst %-softmmu,qemu-system-%, \ - $(SYSTEM_TARGETS))) +SYSTEM_PROGS=$(patsubst %-softmmu,qemu-system-%, \ + $(SYSTEM_TARGETS)) USER_TARGETS=$(filter %-user,$(TARGET_DIRS)) USER_PROGS=$(patsubst %-bsd-user,qemu-%, \ diff --git a/Makefile.target b/Makefile.target index 07af4d4..29287ed 100644 --- a/Makefile.target +++ b/Makefile.target @@ -27,12 +27,8 @@ ifdef CONFIG_USER_ONLY QEMU_PROG=qemu-$(TARGET_ARCH2) else # system emulator name -ifeq ($(TARGET_ARCH), i386) -QEMU_PROG=qemu$(EXESUF) -else QEMU_PROG=qemu-system-$(TARGET_ARCH2)$(EXESUF) endif -endif PROGS=$(QEMU_PROG) STPFILES= This will leave an old qemu executable from a previous `make install` behind. We should check for it and, unless it's a symlink to qemu-system-i386, remove it in the install target. Andreas
[Qemu-devel] [PATCH] hw/qxl: Fix format string errors
Fix format string errors causing compile failure on 32 bit hosts
when spice is enabled.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
This fixes the easy parts of the 32 bit compile failures with
spice enabled. It leaves the following two warnings:
hw/qxl.c: In function 'interface_release_resource':
hw/qxl.c:627:46: error: cast to pointer from integer of different size
hw/qxl.c: In function 'qxl_phys2virt':
hw/qxl.c:1003:16: error: cast to pointer from integer of different size
which appear to be making deeper pointers are 64 bits assumptions
that I don't know enough about the spice code to suggest fixes for.
(Throwing in (intptr_t) casts suppresses the warning but seems a
bit icky...)
Could somebody else take a look at those, please?
hw/qxl-logger.c |2 +-
hw/qxl.c|8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/qxl-logger.c b/hw/qxl-logger.c
index 74cadba..367aad1 100644
--- a/hw/qxl-logger.c
+++ b/hw/qxl-logger.c
@@ -224,7 +224,7 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring,
QXLCommandExt *ext)
if (!qxl-cmdlog) {
return;
}
-fprintf(stderr, %ld qxl-%d/%s:, qemu_get_clock_ns(vm_clock),
+fprintf(stderr, % PRId64 qxl-%d/%s:, qemu_get_clock_ns(vm_clock),
qxl-id, ring);
fprintf(stderr, cmd @ 0x% PRIx64 %s%s, ext-cmd.data,
qxl_name(qxl_type, ext-cmd.type),
diff --git a/hw/qxl.c b/hw/qxl.c
index 45e2401..1fe0b53 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -959,7 +959,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t
slot_id, uint64_t delta,
memslot.generation = d-rom-slot_generation = 0;
qxl_rom_set_dirty(d);
-dprint(d, 1, %s: slot %d: host virt 0x% PRIx64 - 0x% PRIx64 \n,
+dprint(d, 1, %s: slot %d: host virt 0x%lx - 0x%lx\n,
__FUNCTION__, memslot.slot_id,
memslot.virt_start, memslot.virt_end);
@@ -1090,8 +1090,8 @@ static void qxl_set_mode(PCIQXLDevice *d, int modenr, int
loadvm)
.mem= devmem + d-shadow_rom.draw_area_offset,
};
-dprint(d, 1, %s: mode %d [ %d x %d @ %d bpp devmem 0x%lx ]\n,
__FUNCTION__,
- modenr, mode-x_res, mode-y_res, mode-bits, devmem);
+dprint(d, 1, %s: mode %d [ %d x %d @ %d bpp devmem 0x% PRIx64 ]\n,
+ __func__, modenr, mode-x_res, mode-y_res, mode-bits, devmem);
if (!loadvm) {
qxl_hard_reset(d, 0);
}
@@ -1229,7 +1229,7 @@ async_common:
break;
case QXL_IO_LOG:
if (d-guestdebug) {
-fprintf(stderr, qxl/guest-%d: %ld: %s, d-id,
+fprintf(stderr, qxl/guest-%d: % PRId64 : %s, d-id,
qemu_get_clock_ns(vm_clock), d-ram-log_buf);
}
break;
--
1.7.4.1
Re: [Qemu-devel] [PATCH v2] Display logical disk size in 'info block' output
On Fri, Sep 2, 2011 at 5:38 PM, Daniel P. Berrange d...@berrange.com wrote: From: Daniel P. Berrange d...@berrange.com To aid in knowing whether a 'block_resize' was succesful, display the logical disk size in bytes, in the 'info block' output In v2: - Replace sectors with bytes Signed-off-by: Daniel P. Berrange d...@berrange.com --- block.c | 6 -- qmp-commands.hx | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Re: [Qemu-devel] [PATCH] Allow overriding the location of Samba's smbd.
On Fri, Sep 2, 2011 at 9:53 PM, Brad b...@comstyle.com wrote: Allow overriding the location of Samba's smbd. Pretty much every OS I look at has some means of changing this path (patching) so lets just make it easier for OS developers creating packages and/or end users to override the location. Signed-off-by: Brad Smith b...@comstyle.com --- configure | 9 + net.h | 5 - net/slirp.c | 2 +- qemu-options.hx | 6 +++--- 4 files changed, 13 insertions(+), 9 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
[Qemu-devel] Where to log xen_platform_log data
Hi Steven, The Xen platform PCI device has a logging feature that is currently implemented using trace_xen_platform_log(s-log_buffer). String arguments may not be supported by all trace backends so they should be avoided. For example, the simple trace backend logs 8-byte arguments and therefore cannot log strings - it will simply log the char * pointer value, not the actual log message. Here is what docs/tracing.txt says: 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. Is there a better place to send the log output? Stefan
Re: [Qemu-devel] [PATCH 1/2] softfloat: Use uint16 consistently
On 28 August 2011 19:24, Andreas Färber andreas.faer...@web.de wrote: Prepares for uint16 replacement. Signed-off-by: Andreas Färber andreas.faer...@web.de Reviewed-by: Peter Maydell peter.mayd...@linaro.org
Re: [Qemu-devel] [PATCH 2/2] softfloat: Use uint32 consistently
On 28 August 2011 19:24, Andreas Färber andreas.faer...@web.de wrote: Prepares for uint32 replacement. Signed-off-by: Andreas Färber andreas.faer...@web.de Reviewed-by: Peter Maydell peter.mayd...@linaro.org
Re: [Qemu-devel] softfloat breaks cocoa.m
On 2 September 2011 19:18, Andreas Färber andreas.faer...@web.de wrote: What about my preparatory patches? Can you ack them? Sorry, yeah, I'd missed those; that cleanup is worth doing whatever we decide to do here. Now reviewed. -- PMM
[Qemu-devel] [PATCH] ehci: avoid string arguments in trace events
String arguments are not supported by all trace backends. This patch
replaces existing string arguments in hw/usb-ehci.c either with
individual trace events that remain human-friendly or by printing raw
addresses when there is no alternative or downside to that.
States and usbsts bits remain human-friendly since it is hard to
remember all of them. MMIO addresses are printed raw because they would
create many individual trace events and the addresses are usually easy
to remember when debugging. Queue actions remain human-friendly while
device attach only prints the USBDevice pointer.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
hw/usb-ehci.c | 138 +++-
trace-events | 40 ++---
2 files changed, 100 insertions(+), 78 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 47a7fb9..e101fc7 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -435,93 +435,89 @@ struct EHCIState {
*data = val; \
} while(0)
-static const char *ehci_state_names[] = {
-[ EST_INACTIVE ] = INACTIVE,
-[ EST_ACTIVE ] = ACTIVE,
-[ EST_EXECUTING ]= EXECUTING,
-[ EST_SLEEPING ] = SLEEPING,
-[ EST_WAITLISTHEAD ] = WAITLISTHEAD,
-[ EST_FETCHENTRY ] = FETCH ENTRY,
-[ EST_FETCHQH ] = FETCH QH,
-[ EST_FETCHITD ] = FETCH ITD,
-[ EST_ADVANCEQUEUE ] = ADVANCEQUEUE,
-[ EST_FETCHQTD ] = FETCH QTD,
-[ EST_EXECUTE ] = EXECUTE,
-[ EST_WRITEBACK ]= WRITEBACK,
-[ EST_HORIZONTALQH ] = HORIZONTALQH,
-};
-
-static const char *ehci_mmio_names[] = {
-[ CAPLENGTH ]= CAPLENGTH,
-[ HCIVERSION ] = HCIVERSION,
-[ HCSPARAMS ]= HCSPARAMS,
-[ HCCPARAMS ]= HCCPARAMS,
-[ USBCMD ] = USBCMD,
-[ USBSTS ] = USBSTS,
-[ USBINTR ] = USBINTR,
-[ FRINDEX ] = FRINDEX,
-[ PERIODICLISTBASE ] = P-LIST BASE,
-[ ASYNCLISTADDR ]= A-LIST ADDR,
-[ PORTSC_BEGIN ] = PORTSC #0,
-[ PORTSC_BEGIN + 4] = PORTSC #1,
-[ PORTSC_BEGIN + 8] = PORTSC #2,
-[ PORTSC_BEGIN + 12] = PORTSC #3,
-[ CONFIGFLAG ] = CONFIGFLAG,
-};
-
-static const char *nr2str(const char **n, size_t len, uint32_t nr)
+/* Individual human-friendly trace events for states */
+static void ehci_trace_set_state(int async, uint32_t state)
{
-if (nr len n[nr] != NULL) {
-return n[nr];
-} else {
-return unknown;
+switch (state) {
+case EST_INACTIVE:
+trace_usb_ehci_state_inactive(async);
+break;
+case EST_ACTIVE:
+trace_usb_ehci_state_active(async);
+break;
+case EST_EXECUTING:
+trace_usb_ehci_state_executing(async);
+break;
+case EST_SLEEPING:
+trace_usb_ehci_state_sleeping(async);
+break;
+case EST_WAITLISTHEAD:
+trace_usb_ehci_state_waitlisthead(async);
+break;
+case EST_FETCHENTRY:
+trace_usb_ehci_state_fetchentry(async);
+break;
+case EST_FETCHQH:
+trace_usb_ehci_state_fetchqh(async);
+break;
+case EST_FETCHITD:
+trace_usb_ehci_state_fetchitd(async);
+break;
+case EST_ADVANCEQUEUE:
+trace_usb_ehci_state_advancequeue(async);
+break;
+case EST_FETCHQTD:
+trace_usb_ehci_state_fetchqtd(async);
+break;
+case EST_EXECUTE:
+trace_usb_ehci_state_execute(async);
+break;
+case EST_WRITEBACK:
+trace_usb_ehci_state_writeback(async);
+break;
+case EST_HORIZONTALQH:
+trace_usb_ehci_state_horizontalqh(async);
+break;
+default:
+trace_usb_ehci_state_unknown(async, state);
+break;
}
}
-static const char *state2str(uint32_t state)
-{
-return nr2str(ehci_state_names, ARRAY_SIZE(ehci_state_names), state);
-}
-
-static const char *addr2str(target_phys_addr_t addr)
-{
-return nr2str(ehci_mmio_names, ARRAY_SIZE(ehci_mmio_names), addr);
-}
-
static void ehci_trace_usbsts(uint32_t mask, int state)
{
/* interrupts */
if (mask USBSTS_INT) {
-trace_usb_ehci_usbsts(INT, state);
+trace_usb_ehci_usbsts_int(state);
}
if (mask USBSTS_ERRINT) {
-trace_usb_ehci_usbsts(ERRINT, state);
+trace_usb_ehci_usbsts_errint(state);
}
if (mask USBSTS_PCD) {
-trace_usb_ehci_usbsts(PCD, state);
+trace_usb_ehci_usbsts_pcd(state);
}
if (mask USBSTS_FLR) {
-trace_usb_ehci_usbsts(FLR, state);
+trace_usb_ehci_usbsts_flr(state);
}
if (mask USBSTS_HSE) {
-trace_usb_ehci_usbsts(HSE, state);
+trace_usb_ehci_usbsts_hse(state);
}
if (mask USBSTS_IAA) {
-trace_usb_ehci_usbsts(IAA, state);
+trace_usb_ehci_usbsts_iaa(state);
}
/* status */
if (mask USBSTS_HALT) {
-trace_usb_ehci_usbsts(HALT, state);
+
Re: [Qemu-devel] [PATCH] Rename qemu - qemu-system-i386
On 09/03/2011 03:44 PM, Andreas Färber wrote: This will leave an old qemu executable from a previous `make install` behind. We should check for it and, unless it's a symlink to qemu-system-i386, remove it in the install target. No, make install should never remove anything. Paolo
Re: [Qemu-devel] [PATCH] virtio: Make memory barriers be memory barriers
On 09/02/2011 05:45 PM, Michael S. Tsirkin wrote: Well, can you describe an issue in virtio that lfence/sfence help solve in terms of a memory model please? Pls note that guest uses smp_ variants for barriers. /* Make sure buffer is written before we update index. */ wmb(); Without it, a guest could see a partially updated buffer, because the buffer and index writes are unlocked stores to different locations. Even if the guest uses barriers, with ioeventfd it will only order the CPU that is running the guest, not the one that is running the iothread. In fact I'm surprised that it works at all under x86 with ioeventfd. Paolo
Re: [Qemu-devel] [PATCH v2] pc: Fix and clean up PIC-to-APIC IRQ path
On 2011-09-03 13:37, Blue Swirl wrote:
On Sat, Sep 3, 2011 at 11:17 AM, Jan Kiszka jan.kis...@web.de wrote:
On 2011-09-03 10:58, Blue Swirl wrote:
On Thu, Sep 1, 2011 at 9:08 AM, Jan Kiszka jan.kis...@siemens.com wrote:
The master PIC is connected to the LINTIN0 of the APICs. As the APIC
currently does not track the state of that line, we have to ask the PIC
to reinject its IRQ after the CPU picked up an event from the APIC.
This introduces pic_get_output to read the master PIC IRQ line state
without changing it. The APIC uses this function to decide if a PIC IRQ
should be reinjected on apic_update_irq. This reflects better how the
real hardware works.
The patch fixes some failures of the kvm unit tests apic and eventinj by
allowing to enable the proper CPU IRQ deassertion when the guest masks
some pending IRQs at PIC level.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
Changes in v2:
- Avoid adding pic_level to the APIC state, obtain PIC output via
pic_get_level instead
- Do not reassert PIC interrupt if APIC is not accepting it
- Use apic_deliver_pic_intr for reassertion to ensure correct
processing
This is not as nice as the previous version /wrt the interaction of PIC
and APIC. But it avoids breaking the APIC vmstate for the sake of
internal changes, also keeping it compatible with the upcoming KVM
in-kernel APIC (that allows no easy pic_level state extraction). The
interconnection between PIC and APIC may look nicer in the future with
QOM. And in the end this just reflects the beauty of the x86
architecture.
hw/apic.c |4
hw/i8259.c | 15 +++
hw/pc.c|3 ---
hw/pc.h|2 +-
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index d8f56c8..8289eef 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -23,6 +23,7 @@
#include host-utils.h
#include sysbus.h
#include trace.h
+#include pc.h
/* APIC Local Vector Table */
#define APIC_LVT_TIMER 0
@@ -399,6 +400,9 @@ static void apic_update_irq(APICState *s)
}
if (apic_irq_pending(s) 0) {
cpu_interrupt(s-cpu_env, CPU_INTERRUPT_HARD);
+} else if (apic_accept_pic_intr(s-busdev.qdev)
+ pic_get_output(isa_pic)) {
This is indeed ugly. Why doesn't APIC track PIC output?
For the reasons explained above.
Breaking vmstate compatibility? Don't we have all kinds of
compatibility stuff for this?
As I said, we do not have access to some pic_level equivalent with the
in-kernel APIC model. And I don't want to break vmstate compatibility
between user and kernel space models needlessly.
I'm not opposed to the patch as is, it's still a cleanup, but I wish
this ugliness could be avoided.
It's primarily a bug fix now. And the ugliness level should drop again
when IRQ management will be reworked.
Jan
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH] pc_init: Fail on bad kernel
When providing QEMU with a bad '-kernel' parameter, such as a file which
is not really a kernel, QEMU will attempt to allocate a huge amount of
memory and fail either with Failed to allocate memory: Cannot allocate
memory or a GLib error: GLib-ERROR **: gmem.c:170: failed to allocate
18446744073709529965 bytes
This patch handles the case where the magic sig wasn't located in the
provided kernel, and loading it as multiboot failed as well.
Cc: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Sasha Levin levinsasha...@gmail.com
---
hw/pc.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 6b3662e..428440b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -691,8 +691,14 @@ static void load_linux(void *fw_cfg,
/* This looks like a multiboot kernel. If it is, let's stop
treating it like a Linux kernel. */
if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
- kernel_cmdline, kernel_size, header))
+ kernel_cmdline, kernel_size, header)) {
return;
+} else {
+fprintf(stderr, qemu: could not load kernel '%s': %s\n,
+ kernel_filename, strerror(errno));
+ exit(1);
+}
+
protocol = 0;
}
--
1.7.6.1
Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path
On 08/31/2011 11:59 AM, Blue Swirl wrote: On Wed, Aug 31, 2011 at 8:28 AM, Avi Kivitya...@redhat.com wrote: On 08/30/2011 10:19 PM, Blue Swirl wrote: We need some kind of two phase restore. In the first phase all state is restored; since some of that state drivers outputs that are input to other devices, they may experience an edge, and we need to supress that. In the second phase edge detection is unsupressed and the device goes live. No. Devices may not perform any externally visible activities (like toggle a qemu_irq) during or after load because 1) qemu_irq is stateless and 2) since the receiving end is also freshly loaded, both states are already in synch without any calls or toggling. That makes it impossible to migrate level-triggered irq lines. Or at least, the receiver has to remember the state, instead of (or in addition to) the sender. Both ends probably need to remember the state. That should work without any multiphase restores and transient suppressors. It might be also possible to introduce stateful signal lines which save and restore their state, then the receiving end could check what is the current level. However, if you consider that the devices may be restored in random order, if the IRQ line device happens to be restored later, the receiver would still get wrong information. Adding priorities could solve this, but I think stateless IRQs are the only sane way. We shouldn't really use the term IRQ as it's confusing. I like the term pin better because that describes what we're really talking about. qemu_irq is designed oddly today because is represents something that is intrinsically state (whether a pin is high or low) with an edge notification with the assumption that the state is held somewhere else (which is usually true). Modelling stateful pins is useful though for doing something like introspecting pin levels, supporting live migration, etc. The way this works in QOM right now is that the Pin object has a level state that can be queried but it also has the ability to register for notifications on level change. The edge change signal isn't registered until realize. This means that you can connect all of the device models, restore all of the pin states, and then realize the device model all at once. At the point of realize, all of the devices have the right pin levels so each device can add their edge change signals and read the incoming pins and respond accordingly. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path
On 08/31/2011 05:53 AM, Jan Kiszka wrote: On 2011-08-31 10:25, Peter Maydell wrote: On 30 August 2011 20:28, Jan Kiszkajan.kis...@web.de wrote: Yes, that's the current state. Once we have bidirectional IRQ links in place (pushing downward, querying upward - required to skip IRQ routers for fast, lockless deliveries), that should change again. Can you elaborate a bit more on this? I don't think anybody has proposed links with their own internal state before in the qdev/qom discussions... That basic idea is to allow a) a discovery of the currently active IRQ path from source to sink (that would be possible via QOM just using forward links) b) skip updating the states of IRQ routers in the common case, just signaling directly the sink from the source (to allow in-kernel IRQ delivery or to skip taking some device locks). Whenever some router is queried for its current IRQ line state, it would have to ask the preceding IRQ source for its state. So we need a backward link. Can you provide some concrete use-cases of this? I'm not convinced this is really all that important and it seems like tremendous amounts of ugliness would be needed to support it. Regards, Anthony Liguori We haven't thought about how this could be implemented in details yet though. Among other things, it heavily depends on the final QOM design. Jan
Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path
On 09/01/2011 12:58 AM, Avi Kivity wrote:
On 08/31/2011 07:59 PM, Blue Swirl wrote:
That makes it impossible to migrate level-triggered irq lines. Or at
least,
the receiver has to remember the state, instead of (or in addition
to) the
sender.
Both ends probably need to remember the state. That should work
without any multiphase restores and transient suppressors.
State should always correspond to real hardware state - a flip flop or
capacitor. Input is not state, it is input.
It might be also possible to introduce stateful signal lines which
save and restore their state, then the receiving end could check what
is the current level. However, if you consider that the devices may be
restored in random order, if the IRQ line device happens to be
restored later, the receiver would still get wrong information. Adding
priorities could solve this, but I think stateless IRQs are the only
sane way.
I agree that irqs should be stateless, since they don't have any memory
associated.
In Real Life, you can tie a single bit multiple registers together with
boolean logic to form an output pin. This is essentially computed
state. If we wanted to model a stateless pin, we would need to do
something like:
struct Serial {
uint8_t thr;
uint8_t lsr;
};
static bool serial_get_irq(Serial *s) {
return (s-thr THRE) | (s-lsr LSRE);
}
static void serial_write(Serial *s, uint64_t addr, uint8_t value)
{
switch (addr) {
case THR:
bool old_irq = serial_get_irq(s);
s-thr = value;
if (!old_irq serial_get_irq(s)) {
notify_edge_change(s);
}
...
}
static void serial_init(Serial *s)
{
register_pin(s, serial_get_irq);
}
Obviously, this is pretty sucky. This is what we do today but we don't
have a way to query irq value which is wrong. You could fix that by
adding the get function but that's not terribly fun. A better way:
struct Serial {
Pin irq;
uint8_t thr;
uint8_t lsr;
};
static void serial_update_irq(Serial *s)
{
pin_set_level(s-irq, (s-thr THRE) | (s-lsr LSRE));
}
static void serial_write(Serial *s) {
switch (addr) {
case THR:
s-thr = value;
serial_update_irq(s);
...
}
This results in much nicer code. The edge handling can be done in
generic code which will make things more robust overall.
Regards,
Anthony Liguori
Re: [Qemu-devel] [PATCH] Rename qemu - qemu-system-i386
On 09/03/2011 08:44 AM, Andreas Färber wrote: Am 02.09.2011 um 17:40 schrieb Anthony Liguori: On 08/29/2011 09:55 AM, Anthony Liguori wrote: This has been discussed before in the past. The special casing really makes no sense anymore. This seems like a good change to make for 1.0. Signed-off-by: Anthony Liguorialigu...@us.ibm.com Applied. Regards, Anthony Liguori --- Makefile | 5 ++--- Makefile.target | 4 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 8606849..51ecdb5 100644 --- a/Makefile +++ b/Makefile @@ -365,9 +365,8 @@ tar: rm -rf /tmp/$(FILE) SYSTEM_TARGETS=$(filter %-softmmu,$(TARGET_DIRS)) -SYSTEM_PROGS=$(patsubst qemu-system-i386,qemu, \ - $(patsubst %-softmmu,qemu-system-%, \ - $(SYSTEM_TARGETS))) +SYSTEM_PROGS=$(patsubst %-softmmu,qemu-system-%, \ + $(SYSTEM_TARGETS)) USER_TARGETS=$(filter %-user,$(TARGET_DIRS)) USER_PROGS=$(patsubst %-bsd-user,qemu-%, \ diff --git a/Makefile.target b/Makefile.target index 07af4d4..29287ed 100644 --- a/Makefile.target +++ b/Makefile.target @@ -27,12 +27,8 @@ ifdef CONFIG_USER_ONLY QEMU_PROG=qemu-$(TARGET_ARCH2) else # system emulator name -ifeq ($(TARGET_ARCH), i386) -QEMU_PROG=qemu$(EXESUF) -else QEMU_PROG=qemu-system-$(TARGET_ARCH2)$(EXESUF) endif -endif PROGS=$(QEMU_PROG) STPFILES= This will leave an old qemu executable from a previous `make install` behind. You're not supposed to do a make install on top of another install. You're supposed to first do a make uninstall in the old tree than a make install in the new tree. Semantically, this is how a distro package upgrade works. We should check for it and, unless it's a symlink to qemu-system-i386, remove it in the install target. Once we're no longer generating an executable, we should be removing it from the system. It's up to the user to remove old files from the system. Regards, Anthony Liguori Andreas
Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path
On Sat, Sep 3, 2011 at 7:53 PM, Anthony Liguori anth...@codemonkey.ws wrote: On 08/31/2011 11:59 AM, Blue Swirl wrote: On Wed, Aug 31, 2011 at 8:28 AM, Avi Kivitya...@redhat.com wrote: On 08/30/2011 10:19 PM, Blue Swirl wrote: We need some kind of two phase restore. In the first phase all state is restored; since some of that state drivers outputs that are input to other devices, they may experience an edge, and we need to supress that. In the second phase edge detection is unsupressed and the device goes live. No. Devices may not perform any externally visible activities (like toggle a qemu_irq) during or after load because 1) qemu_irq is stateless and 2) since the receiving end is also freshly loaded, both states are already in synch without any calls or toggling. That makes it impossible to migrate level-triggered irq lines. Or at least, the receiver has to remember the state, instead of (or in addition to) the sender. Both ends probably need to remember the state. That should work without any multiphase restores and transient suppressors. It might be also possible to introduce stateful signal lines which save and restore their state, then the receiving end could check what is the current level. However, if you consider that the devices may be restored in random order, if the IRQ line device happens to be restored later, the receiver would still get wrong information. Adding priorities could solve this, but I think stateless IRQs are the only sane way. We shouldn't really use the term IRQ as it's confusing. I like the term pin better because that describes what we're really talking about. qemu_irq is designed oddly today because is represents something that is intrinsically state (whether a pin is high or low) with an edge notification with the assumption that the state is held somewhere else (which is usually true). Modelling stateful pins is useful though for doing something like introspecting pin levels, supporting live migration, etc. The way this works in QOM right now is that the Pin object has a level state that can be queried but it also has the ability to register for notifications on level change. The edge change signal isn't registered until realize. This means that you can connect all of the device models, restore all of the pin states, and then realize the device model all at once. At the point of realize, all of the devices have the right pin levels so each device can add their edge change signals and read the incoming pins and respond accordingly. Even if the devices read the input pins on restore, they shouldn't make any changes to their output pins because that would propagate to other devices. To handle this in non-chaotic way would need hacks to each device, multiphase stuff, priorities or transient suppressors. From the device point of view, restoring is not a state change and no edges should be seen at that moment.
Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path
On Sat, Sep 3, 2011 at 8:07 PM, Anthony Liguori anth...@codemonkey.ws wrote:
On 09/01/2011 12:58 AM, Avi Kivity wrote:
On 08/31/2011 07:59 PM, Blue Swirl wrote:
That makes it impossible to migrate level-triggered irq lines. Or at
least,
the receiver has to remember the state, instead of (or in addition
to) the
sender.
Both ends probably need to remember the state. That should work
without any multiphase restores and transient suppressors.
State should always correspond to real hardware state - a flip flop or
capacitor. Input is not state, it is input.
It might be also possible to introduce stateful signal lines which
save and restore their state, then the receiving end could check what
is the current level. However, if you consider that the devices may be
restored in random order, if the IRQ line device happens to be
restored later, the receiver would still get wrong information. Adding
priorities could solve this, but I think stateless IRQs are the only
sane way.
I agree that irqs should be stateless, since they don't have any memory
associated.
In Real Life, you can tie a single bit multiple registers together with
boolean logic to form an output pin. This is essentially computed state.
If we wanted to model a stateless pin, we would need to do something like:
struct Serial {
uint8_t thr;
uint8_t lsr;
};
static bool serial_get_irq(Serial *s) {
return (s-thr THRE) | (s-lsr LSRE);
}
static void serial_write(Serial *s, uint64_t addr, uint8_t value)
{
switch (addr) {
case THR:
bool old_irq = serial_get_irq(s);
s-thr = value;
if (!old_irq serial_get_irq(s)) {
notify_edge_change(s);
}
...
}
static void serial_init(Serial *s)
{
register_pin(s, serial_get_irq);
}
Obviously, this is pretty sucky. This is what we do today but we don't have
a way to query irq value which is wrong. You could fix that by adding the
get function but that's not terribly fun. A better way:
struct Serial {
Pin irq;
uint8_t thr;
uint8_t lsr;
};
static void serial_update_irq(Serial *s)
{
pin_set_level(s-irq, (s-thr THRE) | (s-lsr LSRE));
}
static void serial_write(Serial *s) {
switch (addr) {
case THR:
s-thr = value;
serial_update_irq(s);
...
}
This results in much nicer code. The edge handling can be done in generic
code which will make things more robust overall.
I'm sorry but I don't see a huge difference, could you elaborate?
Maybe if the internal state of Pin is magically shared between the
endpoint devices (like typedef bool *Pin) and the devices somehow
still save Pin state as part of their own despite the duplication,
this could work. Restoring Pins first and then devices is a sort of
priority scheme.
Re: [Qemu-devel] [PATCH 0/2] Fix packing for MinGW with new macro QEMU_PACKED
On Wed, Aug 31, 2011 at 10:37 AM, Stefan Weil w...@mail.berlios.de wrote: The new series adds macro QEMU_PACKED and converts most code locations mechanically (script) to use this macro. See log message of patch 2/2 for the few exceptions. [PATCH 1/2] Add new macro QEMU_PACKED for packed C structures [PATCH 2/2] Use new macro QEMU_PACKED for packed structures Thanks, applied both.
Re: [Qemu-devel] [PATCH] Allow overriding the location of Samba's smbd.
Thanks, applied.
On Fri, Sep 2, 2011 at 8:53 PM, Brad b...@comstyle.com wrote:
Allow overriding the location of Samba's smbd.
Pretty much every OS I look at has some means of
changing this path (patching) so lets just make
it easier for OS developers creating packages
and/or end users to override the location.
Signed-off-by: Brad Smith b...@comstyle.com
---
configure | 9 +
net.h | 5 -
net/slirp.c | 2 +-
qemu-options.hx | 6 +++---
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index ad60ea0..c669d4e 100755
--- a/configure
+++ b/configure
@@ -413,6 +413,7 @@ SunOS)
make=${MAKE-gmake}
install=${INSTALL-ginstall}
ld=gld
+ smbd=${SMBD-/usr/sfw/sbin/smbd}
needs_libsunmath=no
solarisrev=`uname -r | cut -f2 -d.`
# have to select again, because `uname -m` returns i86pc
@@ -481,6 +482,7 @@ fi
: ${make=${MAKE-make}}
: ${install=${INSTALL-install}}
: ${python=${PYTHON-python}}
+: ${smbd=${SMBD-/usr/sbin/smbd}}
if test $mingw32 = yes ; then
EXESUF=.exe
@@ -525,6 +527,8 @@ for opt do
;;
--python=*) python=$optarg
;;
+ --smbd=*) smbd=$optarg
+ ;;
--extra-cflags=*)
;;
--extra-ldflags=*)
@@ -941,6 +945,7 @@ echo --extra-ldflags=LDFLAGS append extra linker
flags LDFLAGS
echo --make=MAKE use specified make [$make]
echo --install=INSTALL use specified install [$install]
echo --python=PYTHON use specified python [$python]
+echo --smbd=SMBD use specified smbd [$smbd]
echo --static enable static build [$static]
echo --mandir=PATH install man pages in PATH
echo --datadir=PATH install firmware in PATH
@@ -2666,6 +2671,9 @@ echo LDFLAGS $LDFLAGS
echo make $make
echo install $install
echo python $python
+if test $slirp = yes ; then
+ echo smbd $smbd
+fi
echo host CPU $cpu
echo host big endian $bigendian
echo target list $target_list
@@ -2825,6 +2833,7 @@ if test $profiler = yes ; then
fi
if test $slirp = yes ; then
echo CONFIG_SLIRP=y $config_host_mak
+ echo CONFIG_SMBD_COMMAND=\$smbd\ $config_host_mak
QEMU_INCLUDES=-I\$(SRC_PATH)/slirp $QEMU_INCLUDES
fi
if test $vde = yes ; then
diff --git a/net.h b/net.h
index 5a7881c..9f633f8 100644
--- a/net.h
+++ b/net.h
@@ -174,11 +174,6 @@ int do_netdev_del(Monitor *mon, const QDict *qdict,
QObject **ret_data);
#define DEFAULT_NETWORK_SCRIPT /etc/qemu-ifup
#define DEFAULT_NETWORK_DOWN_SCRIPT /etc/qemu-ifdown
-#ifdef __sun__
-#define SMBD_COMMAND /usr/sfw/sbin/smbd
-#else
-#define SMBD_COMMAND /usr/sbin/smbd
-#endif
void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
diff --git a/net/slirp.c b/net/slirp.c
index 3b39d21..c6cda5d 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -529,7 +529,7 @@ static int slirp_smb(SlirpState* s, const char
*exported_dir,
fclose(f);
snprintf(smb_cmdline, sizeof(smb_cmdline), %s -s %s,
- SMBD_COMMAND, smb_conf);
+ CONFIG_SMBD_COMMAND, smb_conf);
if (slirp_add_exec(s-slirp, 0, smb_cmdline, vserver_addr, 139) 0) {
slirp_smb_cleanup(s);
diff --git a/qemu-options.hx b/qemu-options.hx
index 35d95d1..e24a740 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1277,9 +1277,9 @@ or @file{C:\WINNT\SYSTEM32\DRIVERS\ETC\LMHOSTS}
(Windows NT/2000).
Then @file{@var{dir}} can be accessed in @file{\\smbserver\qemu}.
-Note that a SAMBA server must be installed on the host OS in
-@file{/usr/sbin/smbd}. QEMU was tested successfully with smbd versions from
-Red Hat 9, Fedora Core 3 and OpenSUSE 11.x.
+Note that a SAMBA server must be installed on the host OS.
+QEMU was tested successfully with smbd versions from Red Hat 9,
+Fedora Core 3 and OpenSUSE 11.x.
@item
hostfwd=[tcp|udp]:[@var{hostaddr}]:@var{hostport}-[@var{guestaddr}]:@var{guestport}
Redirect incoming TCP or UDP connections to the host port @var{hostport} to
--
1.7.6
Re: [Qemu-devel] [PATCH 1/2] softfloat: Use uint16 consistently
Thanks, applied.
On Sun, Aug 28, 2011 at 6:24 PM, Andreas Färber andreas.faer...@web.de wrote:
Prepares for uint16 replacement.
Signed-off-by: Andreas Färber andreas.faer...@web.de
---
fpu/softfloat.c | 8
fpu/softfloat.h | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7951a0e..be1206d 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -6011,10 +6011,10 @@ unsigned int float32_to_uint32_round_to_zero( float32
a STATUS_PARAM )
return res;
}
-unsigned int float32_to_uint16_round_to_zero( float32 a STATUS_PARAM )
+uint16 float32_to_uint16_round_to_zero( float32 a STATUS_PARAM )
{
int64_t v;
- unsigned int res;
+ uint16 res;
v = float32_to_int64_round_to_zero(a STATUS_VAR);
if (v 0) {
@@ -6065,10 +6065,10 @@ unsigned int float64_to_uint32_round_to_zero( float64
a STATUS_PARAM )
return res;
}
-unsigned int float64_to_uint16_round_to_zero( float64 a STATUS_PARAM )
+uint16 float64_to_uint16_round_to_zero( float64 a STATUS_PARAM )
{
int64_t v;
- unsigned int res;
+ uint16 res;
v = float64_to_int64_round_to_zero(a STATUS_VAR);
if (v 0) {
diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index 3bb7d8f..e1bbe01 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -249,7 +249,7 @@ extern const float16 float16_default_nan;
| Software IEC/IEEE single-precision conversion routines.
**/
int16 float32_to_int16_round_to_zero( float32 STATUS_PARAM );
-unsigned int float32_to_uint16_round_to_zero( float32 STATUS_PARAM );
+uint16 float32_to_uint16_round_to_zero( float32 STATUS_PARAM );
int32 float32_to_int32( float32 STATUS_PARAM );
int32 float32_to_int32_round_to_zero( float32 STATUS_PARAM );
uint32 float32_to_uint32( float32 STATUS_PARAM );
@@ -352,7 +352,7 @@ extern const float32 float32_default_nan;
| Software IEC/IEEE double-precision conversion routines.
**/
int16 float64_to_int16_round_to_zero( float64 STATUS_PARAM );
-unsigned int float64_to_uint16_round_to_zero( float64 STATUS_PARAM );
+uint16 float64_to_uint16_round_to_zero( float64 STATUS_PARAM );
int32 float64_to_int32( float64 STATUS_PARAM );
int32 float64_to_int32_round_to_zero( float64 STATUS_PARAM );
uint32 float64_to_uint32( float64 STATUS_PARAM );
--
1.7.5.3
Re: [Qemu-devel] [PATCH 2/2] softfloat: Use uint32 consistently
Thanks, applied.
On Sun, Aug 28, 2011 at 6:24 PM, Andreas Färber andreas.faer...@web.de wrote:
Prepares for uint32 replacement.
Signed-off-by: Andreas Färber andreas.faer...@web.de
---
fpu/softfloat.c | 20 ++--
fpu/softfloat.h | 4 ++--
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index be1206d..2b20085 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5965,20 +5965,20 @@ int float128_unordered_quiet( float128 a, float128 b
STATUS_PARAM )
}
/* misc functions */
-float32 uint32_to_float32( unsigned int a STATUS_PARAM )
+float32 uint32_to_float32( uint32 a STATUS_PARAM )
{
return int64_to_float32(a STATUS_VAR);
}
-float64 uint32_to_float64( unsigned int a STATUS_PARAM )
+float64 uint32_to_float64( uint32 a STATUS_PARAM )
{
return int64_to_float64(a STATUS_VAR);
}
-unsigned int float32_to_uint32( float32 a STATUS_PARAM )
+uint32 float32_to_uint32( float32 a STATUS_PARAM )
{
int64_t v;
- unsigned int res;
+ uint32 res;
v = float32_to_int64(a STATUS_VAR);
if (v 0) {
@@ -5993,10 +5993,10 @@ unsigned int float32_to_uint32( float32 a
STATUS_PARAM )
return res;
}
-unsigned int float32_to_uint32_round_to_zero( float32 a STATUS_PARAM )
+uint32 float32_to_uint32_round_to_zero( float32 a STATUS_PARAM )
{
int64_t v;
- unsigned int res;
+ uint32 res;
v = float32_to_int64_round_to_zero(a STATUS_VAR);
if (v 0) {
@@ -6029,10 +6029,10 @@ uint16 float32_to_uint16_round_to_zero( float32 a
STATUS_PARAM )
return res;
}
-unsigned int float64_to_uint32( float64 a STATUS_PARAM )
+uint32 float64_to_uint32( float64 a STATUS_PARAM )
{
int64_t v;
- unsigned int res;
+ uint32 res;
v = float64_to_int64(a STATUS_VAR);
if (v 0) {
@@ -6047,10 +6047,10 @@ unsigned int float64_to_uint32( float64 a
STATUS_PARAM )
return res;
}
-unsigned int float64_to_uint32_round_to_zero( float64 a STATUS_PARAM )
+uint32 float64_to_uint32_round_to_zero( float64 a STATUS_PARAM )
{
int64_t v;
- unsigned int res;
+ uint32 res;
v = float64_to_int64_round_to_zero(a STATUS_VAR);
if (v 0) {
diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index e1bbe01..618ddee 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -216,8 +216,8 @@ void float_raise( int8 flags STATUS_PARAM);
**/
float32 int32_to_float32( int32 STATUS_PARAM );
float64 int32_to_float64( int32 STATUS_PARAM );
-float32 uint32_to_float32( unsigned int STATUS_PARAM );
-float64 uint32_to_float64( unsigned int STATUS_PARAM );
+float32 uint32_to_float32( uint32 STATUS_PARAM );
+float64 uint32_to_float64( uint32 STATUS_PARAM );
floatx80 int32_to_floatx80( int32 STATUS_PARAM );
float128 int32_to_float128( int32 STATUS_PARAM );
float32 int64_to_float32( int64 STATUS_PARAM );
--
1.7.5.3
[Qemu-devel] [PATCH 1/2] apb_pci: convert PCI space to memory API
Add a new memory space for PCI instead of using system memory.
This also fixes a bug where VGA region vga.chain4 is
accidentally mapped to 0xa instead of 0x1ff000a.
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
hw/apb_pci.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 6ee2068..c232946 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -71,6 +71,7 @@ typedef struct APBState {
PCIBus *bus;
MemoryRegion apb_config;
MemoryRegion pci_config;
+MemoryRegion pci_mmio;
MemoryRegion pci_ioport;
uint32_t iommu[4];
uint32_t pci_control[16];
@@ -336,12 +337,14 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
sysbus_mmio_map(s, 2, special_base + 0x200ULL);
d = FROM_SYSBUS(APBState, s);
+memory_region_init(d-pci_mmio, pci-mmio, 0x1ULL);
+memory_region_add_subregion(get_system_memory(), mem_base, d-pci_mmio);
+
d-bus = pci_register_bus(d-busdev.qdev, pci,
- pci_apb_set_irq, pci_pbm_map_irq, d,
- get_system_memory(),
- get_system_io(),
- 0, 32);
-pci_bus_set_mem_base(d-bus, mem_base);
+ pci_apb_set_irq, pci_pbm_map_irq, d,
+ d-pci_mmio,
+ get_system_io(),
+ 0, 32);
for (i = 0; i 32; i++) {
sysbus_connect_irq(s, i, pic[i]);
--
1.6.2.4
From 88d72b5a06081d44c03a30c3495bbc49e5ba8d9d Mon Sep 17 00:00:00 2001
Message-Id: 88d72b5a06081d44c03a30c3495bbc49e5ba8d9d.1315084536.git.blauwir...@gmail.com
From: Blue Swirl blauwir...@gmail.com
Date: Sat, 3 Sep 2011 16:38:02 +
Subject: [PATCH 1/2] apb_pci: convert PCI space to memory API
Add a new memory space for PCI instead of using system memory.
This also fixes a bug where VGA region vga.chain4 is
accidentally mapped to 0xa instead of 0x1ff000a.
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
hw/apb_pci.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 6ee2068..c232946 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -71,6 +71,7 @@ typedef struct APBState {
PCIBus *bus;
MemoryRegion apb_config;
MemoryRegion pci_config;
+MemoryRegion pci_mmio;
MemoryRegion pci_ioport;
uint32_t iommu[4];
uint32_t pci_control[16];
@@ -336,12 +337,14 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
sysbus_mmio_map(s, 2, special_base + 0x200ULL);
d = FROM_SYSBUS(APBState, s);
+memory_region_init(d-pci_mmio, pci-mmio, 0x1ULL);
+memory_region_add_subregion(get_system_memory(), mem_base, d-pci_mmio);
+
d-bus = pci_register_bus(d-busdev.qdev, pci,
- pci_apb_set_irq, pci_pbm_map_irq, d,
- get_system_memory(),
- get_system_io(),
- 0, 32);
-pci_bus_set_mem_base(d-bus, mem_base);
+ pci_apb_set_irq, pci_pbm_map_irq, d,
+ d-pci_mmio,
+ get_system_io(),
+ 0, 32);
for (i = 0; i 32; i++) {
sysbus_connect_irq(s, i, pic[i]);
--
1.7.2.5
[Qemu-devel] [PATCH 2/2] PCI: delete unused mem_base and pci_to_cpu_addr
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
hw/pci.c | 14 +-
hw/pci.h |2 --
2 files changed, 1 insertions(+), 15 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 57ff7b1..af74003 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -312,11 +312,6 @@ void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn
hotplug, DeviceState *qdev)
bus-hotplug_qdev = qdev;
}
-void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base)
-{
-bus-mem_base = base;
-}
-
PCIBus *pci_register_bus(DeviceState *parent, const char *name,
pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque,
@@ -833,12 +828,6 @@ PCIDevice *pci_register_device(PCIBus *bus, const
char *name,
return pci_dev;
}
-static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
- target_phys_addr_t addr)
-{
-return addr + bus-mem_base;
-}
-
static void pci_unregister_io_regions(PCIDevice *pci_dev)
{
PCIIORegion *r;
@@ -1066,8 +1055,7 @@ static void pci_update_mappings(PCIDevice *d)
1);
} else {
memory_region_add_subregion_overlap(r-address_space,
-pci_to_cpu_addr(d-bus,
-r-addr),
+r-addr,
r-memory,
1);
}
diff --git a/hw/pci.h b/hw/pci.h
index 391217e..c04b169 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -255,8 +255,6 @@ PCIBus *pci_register_bus(DeviceState *parent,
const char *name,
void pci_device_reset(PCIDevice *dev);
void pci_bus_reset(PCIBus *bus);
-void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base);
-
PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
const char *default_devaddr);
PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model,
--
1.6.2.4
From a2a3612044738ab3ba69c20057308d5e707efe71 Mon Sep 17 00:00:00 2001
Message-Id: a2a3612044738ab3ba69c20057308d5e707efe71.1315084536.git.blauwir...@gmail.com
In-Reply-To: 88d72b5a06081d44c03a30c3495bbc49e5ba8d9d.1315084536.git.blauwir...@gmail.com
References: 88d72b5a06081d44c03a30c3495bbc49e5ba8d9d.1315084536.git.blauwir...@gmail.com
From: Blue Swirl blauwir...@gmail.com
Date: Sat, 3 Sep 2011 16:41:21 +
Subject: [PATCH 2/2] PCI: delete unused mem_base and pci_to_cpu_addr
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
hw/pci.c | 14 +-
hw/pci.h |2 --
2 files changed, 1 insertions(+), 15 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 57ff7b1..af74003 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -312,11 +312,6 @@ void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *qdev)
bus-hotplug_qdev = qdev;
}
-void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base)
-{
-bus-mem_base = base;
-}
-
PCIBus *pci_register_bus(DeviceState *parent, const char *name,
pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque,
@@ -833,12 +828,6 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
return pci_dev;
}
-static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
- target_phys_addr_t addr)
-{
-return addr + bus-mem_base;
-}
-
static void pci_unregister_io_regions(PCIDevice *pci_dev)
{
PCIIORegion *r;
@@ -1066,8 +1055,7 @@ static void pci_update_mappings(PCIDevice *d)
1);
} else {
memory_region_add_subregion_overlap(r-address_space,
-pci_to_cpu_addr(d-bus,
-r-addr),
+r-addr,
r-memory,
1);
}
diff --git a/hw/pci.h b/hw/pci.h
index 391217e..c04b169 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -255,8 +255,6 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
void pci_device_reset(PCIDevice *dev);
void pci_bus_reset(PCIBus *bus);
-void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base);
-
PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
const char *default_devaddr);
PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model,
--
1.7.2.5
Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path
On 09/03/2011 04:10 PM, Blue Swirl wrote:
On Sat, Sep 3, 2011 at 8:07 PM, Anthony Liguorianth...@codemonkey.ws wrote:
On 09/01/2011 12:58 AM, Avi Kivity wrote:
On 08/31/2011 07:59 PM, Blue Swirl wrote:
That makes it impossible to migrate level-triggered irq lines. Or at
least,
the receiver has to remember the state, instead of (or in addition
to) the
sender.
Both ends probably need to remember the state. That should work
without any multiphase restores and transient suppressors.
State should always correspond to real hardware state - a flip flop or
capacitor. Input is not state, it is input.
It might be also possible to introduce stateful signal lines which
save and restore their state, then the receiving end could check what
is the current level. However, if you consider that the devices may be
restored in random order, if the IRQ line device happens to be
restored later, the receiver would still get wrong information. Adding
priorities could solve this, but I think stateless IRQs are the only
sane way.
I agree that irqs should be stateless, since they don't have any memory
associated.
In Real Life, you can tie a single bit multiple registers together with
boolean logic to form an output pin. This is essentially computed state.
If we wanted to model a stateless pin, we would need to do something like:
struct Serial {
uint8_t thr;
uint8_t lsr;
};
static bool serial_get_irq(Serial *s) {
return (s-thr THRE) | (s-lsr LSRE);
}
static void serial_write(Serial *s, uint64_t addr, uint8_t value)
{
switch (addr) {
case THR:
bool old_irq = serial_get_irq(s);
s-thr = value;
if (!old_irq serial_get_irq(s)) {
notify_edge_change(s);
}
...
}
static void serial_init(Serial *s)
{
register_pin(s, serial_get_irq);
}
Obviously, this is pretty sucky. This is what we do today but we don't have
a way to query irq value which is wrong. You could fix that by adding the
get function but that's not terribly fun. A better way:
struct Serial {
Pin irq;
uint8_t thr;
uint8_t lsr;
};
static void serial_update_irq(Serial *s)
{
pin_set_level(s-irq, (s-thr THRE) | (s-lsr LSRE));
}
static void serial_write(Serial *s) {
switch (addr) {
case THR:
s-thr = value;
serial_update_irq(s);
...
}
This results in much nicer code. The edge handling can be done in generic
code which will make things more robust overall.
I'm sorry but I don't see a huge difference, could you elaborate?
The main difference is whether the Pin is capable of determine if there
was a level change on its own. It can only do this is if knows the
current level which implies that its holding state.
Maybe if the internal state of Pin is magically shared between the
endpoint devices (like typedef bool *Pin) and the devices somehow
still save Pin state as part of their own despite the duplication,
I'm somewhat confused by what you mean here.
If you have two devices that have a connection, one has an output pin
and one has an input pin. This would look like this:
struct Serial {
Pin irq; // output pin
};
struct PIIX3 {
Pin *in[16]; // input pins
};
As part of connecting devices, you'd basically do:
PIIX3 piix3;
Serial serial;
piix3.in[4] = serial.irq;
serial.irq setting it pin level doesn't do anything to piix3. piix3 has
to explicitly read the pin state for its behavior to influence anything.
Here's the flow with taking migration into account:
1) PIIX3 maintains some type of state, performs action (A) whenever
in[3] changes its state based on an edge change notifier.
2) During migration, PIIX3 has its state saved as does Serial. Pin is
part of Serial so it also has its state saved.
3) During restore, PIIX3 has its state restored, as does Serial, and
Pin. Action (A) is not invoked because notifiers are not fired when a
device is not realized. Restore happens before a device is realized.
So the scenario you're concerned about doesn't happen and it doesn't
require anything funky.
this could work. Restoring Pins first and then devices is a sort of
priority scheme.
There is no priority. But devices have an explicit realize event and in
general, shouldn't react to other devices until realize happens. You
need this behavior to support construction properly.
Regards,
Anthony Liguori
[Qemu-devel] [PATCH v2] rbd: fix leak in qemu_rbd_open failure paths
Fix leak of s-snap in failure path. Simplify error paths for the whole
function.
Reported-by: Stefan Hajnoczi stefa...@gmail.com
Signed-off-by: Sage Weil s...@newdream.net
---
v2: fixes all error paths, not just the first one.
block/rbd.c | 28 +---
1 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index eaf7912..2f0733e 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -427,10 +427,6 @@ static int qemu_rbd_open(BlockDriverState *bs, const char
*filename, int flags)
conf, sizeof(conf)) 0) {
return -EINVAL;
}
-s-snap = NULL;
-if (snap_buf[0] != '\0') {
-s-snap = qemu_strdup(snap_buf);
-}
clientname = qemu_rbd_parse_clientname(conf, clientname_buf);
r = rados_create(s-cluster, clientname);
@@ -439,12 +435,16 @@ static int qemu_rbd_open(BlockDriverState *bs, const char
*filename, int flags)
return r;
}
+s-snap = NULL;
+if (snap_buf[0] != '\0') {
+s-snap = qemu_strdup(snap_buf);
+}
+
if (strstr(conf, conf=) == NULL) {
r = rados_conf_read_file(s-cluster, NULL);
if (r 0) {
error_report(error reading config file);
-rados_shutdown(s-cluster);
-return r;
+ goto failed_shutdown;
}
}
@@ -452,31 +452,26 @@ static int qemu_rbd_open(BlockDriverState *bs, const char
*filename, int flags)
r = qemu_rbd_set_conf(s-cluster, conf);
if (r 0) {
error_report(error setting config options);
-rados_shutdown(s-cluster);
-return r;
+ goto failed_shutdown;
}
}
r = rados_connect(s-cluster);
if (r 0) {
error_report(error connecting);
-rados_shutdown(s-cluster);
-return r;
+ goto failed_shutdown;
}
r = rados_ioctx_create(s-cluster, pool, s-io_ctx);
if (r 0) {
error_report(error opening pool %s, pool);
-rados_shutdown(s-cluster);
-return r;
+ goto failed_shutdown;
}
r = rbd_open(s-io_ctx, s-name, s-image, s-snap);
if (r 0) {
error_report(error reading header from %s, s-name);
-rados_ioctx_destroy(s-io_ctx);
-rados_shutdown(s-cluster);
-return r;
+ goto failed_open;
}
bs-read_only = (s-snap != NULL);
@@ -497,8 +492,11 @@ static int qemu_rbd_open(BlockDriverState *bs, const char
*filename, int flags)
failed:
rbd_close(s-image);
+failed_open:
rados_ioctx_destroy(s-io_ctx);
+failed_shutdown:
rados_shutdown(s-cluster);
+qemu_free(s-snap);
return r;
}
--
1.7.2.5
Re: [Qemu-devel] Single 64bit memory transaction instead of two 32bit memory transaction
Is there a patch for the new MemoryRegion API? At least for now, I need to be able to issue a single 64, 128 or 256bit transaction to a particular IO device. Is there a way I can do this now as a temp solution? I don't mind coalescing the bytes as they are issued by Qemu but I have no way to know that they're from the same instruction. Any pointers? Thanks, AK _ From: Richard Henderson [mailto:r...@twiddle.net] To: ad...@khaleel.us Cc: qemu-devel@nongnu.org, blauwir...@gmail.com Sent: Fri, 02 Sep 2011 20:53:30 -0500 Subject: Re: [Qemu-devel] Single 64bit memory transaction instead of two 32bit memory transaction On 09/02/2011 12:46 AM, Adnan Khaleel wrote: Is there anyway we can prevent Qemu breaking up 64,128 and 256bit XMM or YMM instructions into smaller chunks and have them issue as a single transaction of the original width? Not yet. The new MemoryRegion API will allow this, but we need to convert all the devices first, then rip out the old API. It's a work in progress... r~
[Qemu-devel] [Bug 840686] [NEW] No such list qemu-users
Public bug reported: The mailing list qemu-users, described on the project page: http://savannah.nongnu.org/mail/?group=qemu as QEMU users mailing list, is not available. The link: http://lists.nongnu.org/mailman/listinfo/qemu-users results in error: No such list qemu-users. Further references: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02316.html ** Affects: qemu Importance: Undecided Status: New ** Tags: wishlist -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/840686 Title: No such list qemu-users Status in QEMU: New Bug description: The mailing list qemu-users, described on the project page: http://savannah.nongnu.org/mail/?group=qemu as QEMU users mailing list, is not available. The link: http://lists.nongnu.org/mailman/listinfo/qemu-users results in error: No such list qemu-users. Further references: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02316.html To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/840686/+subscriptions
Re: [Qemu-devel] TCG sar UB (fwd)
On 09/03/2011 03:47 PM, malc wrote:
Doesn't make much sense to me, guest clearly asked for 0 and not -1,
besides -1 violates TCG's sar constraints and PPC obliges by emiting
illegal instruction in this case.
The shift that the guest asked for was completely folded away.
The -1 comes from gen_shift_rm_T1 in the computation of the new
flags value. This could instead be moved inside the test for != 0,
which is the only place that value is actually used anyway.
Try this. Lightly tested.
r~
diff --git a/target-i386/translate.c b/target-i386/translate.c
index ccef381..b966762 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -1406,70 +1406,84 @@ static void gen_shift_rm_T1(DisasContext *s, int ot,
int op1,
{
target_ulong mask;
int shift_label;
-TCGv t0, t1;
+TCGv t0, t1, t2;
-if (ot == OT_QUAD)
+if (ot == OT_QUAD) {
mask = 0x3f;
-else
+} else {
mask = 0x1f;
+}
/* load */
-if (op1 == OR_TMP0)
+if (op1 == OR_TMP0) {
gen_op_ld_T0_A0(ot + s-mem_index);
-else
+} else {
gen_op_mov_TN_reg(ot, 0, op1);
+}
-tcg_gen_andi_tl(cpu_T[1], cpu_T[1], mask);
+t0 = tcg_temp_local_new();
+t1 = tcg_temp_local_new();
+t2 = tcg_temp_local_new();
-tcg_gen_addi_tl(cpu_tmp5, cpu_T[1], -1);
+tcg_gen_andi_tl(t2, cpu_T[1], mask);
if (is_right) {
if (is_arith) {
gen_exts(ot, cpu_T[0]);
-tcg_gen_sar_tl(cpu_T3, cpu_T[0], cpu_tmp5);
-tcg_gen_sar_tl(cpu_T[0], cpu_T[0], cpu_T[1]);
+tcg_gen_mov_tl(t0, cpu_T[0]);
+tcg_gen_sar_tl(cpu_T[0], cpu_T[0], t2);
} else {
gen_extu(ot, cpu_T[0]);
-tcg_gen_shr_tl(cpu_T3, cpu_T[0], cpu_tmp5);
-tcg_gen_shr_tl(cpu_T[0], cpu_T[0], cpu_T[1]);
+tcg_gen_mov_tl(t0, cpu_T[0]);
+tcg_gen_shr_tl(cpu_T[0], cpu_T[0], t2);
}
} else {
-tcg_gen_shl_tl(cpu_T3, cpu_T[0], cpu_tmp5);
-tcg_gen_shl_tl(cpu_T[0], cpu_T[0], cpu_T[1]);
+tcg_gen_mov_tl(t0, cpu_T[0]);
+tcg_gen_shl_tl(cpu_T[0], cpu_T[0], t2);
}
/* store */
-if (op1 == OR_TMP0)
+if (op1 == OR_TMP0) {
gen_op_st_T0_A0(ot + s-mem_index);
-else
+} else {
gen_op_mov_reg_T0(ot, op1);
-
+}
+
/* update eflags if non zero shift */
-if (s-cc_op != CC_OP_DYNAMIC)
+if (s-cc_op != CC_OP_DYNAMIC) {
gen_op_set_cc_op(s-cc_op);
+}
-/* XXX: inefficient */
-t0 = tcg_temp_local_new();
-t1 = tcg_temp_local_new();
-
-tcg_gen_mov_tl(t0, cpu_T[0]);
-tcg_gen_mov_tl(t1, cpu_T3);
+tcg_gen_mov_tl(t1, cpu_T[0]);
shift_label = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_T[1], 0, shift_label);
+tcg_gen_brcondi_tl(TCG_COND_EQ, t2, 0, shift_label);
-tcg_gen_mov_tl(cpu_cc_src, t1);
-tcg_gen_mov_tl(cpu_cc_dst, t0);
-if (is_right)
+tcg_gen_addi_tl(t2, t2, -1);
+tcg_gen_mov_tl(cpu_cc_dst, t1);
+
+if (is_right) {
+if (is_arith) {
+tcg_gen_sar_tl(cpu_cc_src, t0, t2);
+} else {
+tcg_gen_shr_tl(cpu_cc_src, t0, t2);
+}
+} else {
+tcg_gen_shl_tl(cpu_cc_src, t0, t2);
+}
+
+if (is_right) {
tcg_gen_movi_i32(cpu_cc_op, CC_OP_SARB + ot);
-else
+} else {
tcg_gen_movi_i32(cpu_cc_op, CC_OP_SHLB + ot);
-
+}
+
gen_set_label(shift_label);
s-cc_op = CC_OP_DYNAMIC; /* cannot predict flags after */
tcg_temp_free(t0);
tcg_temp_free(t1);
+tcg_temp_free(t2);
}
static void gen_shift_rm_im(DisasContext *s, int ot, int op1, int op2,
Re: [Qemu-devel] [PATCH] virtio: Make memory barriers be memory barriers
On Fri, Sep 02, 2011 at 06:45:50PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 01, 2011 at 04:31:09PM -0400, Paolo Bonzini wrote: Why not limit the change to ppc then? Because the bug is masked by the x86 memory model, but it is still there even there conceptually. It is not really true that x86 does not need memory barriers, though it doesn't in this case: http://bartoszmilewski.wordpress.com/2008/11/05/who-ordered-memory-fences-on-an-x86/ Paolo Right. To summarize, on x86 we probably want wmb and rmb to be compiler barrier only. Only mb might in theory need to be an mfence. No, wmb needs to be sfence and rmb needs to be lfence. GCC does not provide those, so they should become __sync_synchronize() too, or you should use inline assembly. But there might be reasons why that is not an issue either if we look closely enough. Since the ring buffers are not using locked instructions (no xchg or cmpxchg) the barriers simply must be there, even on x86. Whether it works in practice is not interesting, only the formal model is interesting. Paolo Well, can you describe an issue in virtio that lfence/sfence help solve in terms of a memory model please? Pls note that guest uses smp_ variants for barriers. Ok, so, I'm having a bit of trouble with the fact that I'm having to argue the case that things the protocol requiress to be memory barriers actually *be* memory barriers on all platforms. I mean argue for a richer set of barriers, with per-arch minimal implementations instead of the large but portable hammer of sync_synchronize, if you will. But just leaving them out on x86!? Seriously, wtf? Do you enjoy having software that works chiefly by accident? -- 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
Re: [Qemu-devel] TCG sar UB (fwd)
On Sun, 4 Sep 2011, Richard Henderson wrote:
On 09/03/2011 03:47 PM, malc wrote:
Doesn't make much sense to me, guest clearly asked for 0 and not -1,
besides -1 violates TCG's sar constraints and PPC obliges by emiting
illegal instruction in this case.
The shift that the guest asked for was completely folded away.
The -1 comes from gen_shift_rm_T1 in the computation of the new
flags value. This could instead be moved inside the test for != 0,
which is the only place that value is actually used anyway.
Try this. Lightly tested.
Now i either get hosts illegal instruction or (with logging enabled) a
guest kenrnel panic.
r~
diff --git a/target-i386/translate.c b/target-i386/translate.c
index ccef381..b966762 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -1406,70 +1406,84 @@ static void gen_shift_rm_T1(DisasContext *s, int ot,
int op1,
{
target_ulong mask;
int shift_label;
-TCGv t0, t1;
+TCGv t0, t1, t2;
-if (ot == OT_QUAD)
+if (ot == OT_QUAD) {
mask = 0x3f;
-else
+} else {
mask = 0x1f;
+}
/* load */
-if (op1 == OR_TMP0)
+if (op1 == OR_TMP0) {
gen_op_ld_T0_A0(ot + s-mem_index);
-else
+} else {
gen_op_mov_TN_reg(ot, 0, op1);
+}
-tcg_gen_andi_tl(cpu_T[1], cpu_T[1], mask);
+t0 = tcg_temp_local_new();
+t1 = tcg_temp_local_new();
+t2 = tcg_temp_local_new();
-tcg_gen_addi_tl(cpu_tmp5, cpu_T[1], -1);
+tcg_gen_andi_tl(t2, cpu_T[1], mask);
if (is_right) {
if (is_arith) {
gen_exts(ot, cpu_T[0]);
-tcg_gen_sar_tl(cpu_T3, cpu_T[0], cpu_tmp5);
-tcg_gen_sar_tl(cpu_T[0], cpu_T[0], cpu_T[1]);
+tcg_gen_mov_tl(t0, cpu_T[0]);
+tcg_gen_sar_tl(cpu_T[0], cpu_T[0], t2);
} else {
gen_extu(ot, cpu_T[0]);
-tcg_gen_shr_tl(cpu_T3, cpu_T[0], cpu_tmp5);
-tcg_gen_shr_tl(cpu_T[0], cpu_T[0], cpu_T[1]);
+tcg_gen_mov_tl(t0, cpu_T[0]);
+tcg_gen_shr_tl(cpu_T[0], cpu_T[0], t2);
}
} else {
-tcg_gen_shl_tl(cpu_T3, cpu_T[0], cpu_tmp5);
-tcg_gen_shl_tl(cpu_T[0], cpu_T[0], cpu_T[1]);
+tcg_gen_mov_tl(t0, cpu_T[0]);
+tcg_gen_shl_tl(cpu_T[0], cpu_T[0], t2);
}
/* store */
-if (op1 == OR_TMP0)
+if (op1 == OR_TMP0) {
gen_op_st_T0_A0(ot + s-mem_index);
-else
+} else {
gen_op_mov_reg_T0(ot, op1);
-
+}
+
/* update eflags if non zero shift */
-if (s-cc_op != CC_OP_DYNAMIC)
+if (s-cc_op != CC_OP_DYNAMIC) {
gen_op_set_cc_op(s-cc_op);
+}
-/* XXX: inefficient */
-t0 = tcg_temp_local_new();
-t1 = tcg_temp_local_new();
-
-tcg_gen_mov_tl(t0, cpu_T[0]);
-tcg_gen_mov_tl(t1, cpu_T3);
+tcg_gen_mov_tl(t1, cpu_T[0]);
shift_label = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_T[1], 0, shift_label);
+tcg_gen_brcondi_tl(TCG_COND_EQ, t2, 0, shift_label);
-tcg_gen_mov_tl(cpu_cc_src, t1);
-tcg_gen_mov_tl(cpu_cc_dst, t0);
-if (is_right)
+tcg_gen_addi_tl(t2, t2, -1);
+tcg_gen_mov_tl(cpu_cc_dst, t1);
+
+if (is_right) {
+if (is_arith) {
+tcg_gen_sar_tl(cpu_cc_src, t0, t2);
+} else {
+tcg_gen_shr_tl(cpu_cc_src, t0, t2);
+}
+} else {
+tcg_gen_shl_tl(cpu_cc_src, t0, t2);
+}
+
+if (is_right) {
tcg_gen_movi_i32(cpu_cc_op, CC_OP_SARB + ot);
-else
+} else {
tcg_gen_movi_i32(cpu_cc_op, CC_OP_SHLB + ot);
-
+}
+
gen_set_label(shift_label);
s-cc_op = CC_OP_DYNAMIC; /* cannot predict flags after */
tcg_temp_free(t0);
tcg_temp_free(t1);
+tcg_temp_free(t2);
}
static void gen_shift_rm_im(DisasContext *s, int ot, int op1, int op2,
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH v2 1/2] qxl: send interrupt after migration in case ram-int_pending != 0, RHBZ #732949
On 09/01/2011 10:36 PM, Michael S. Tsirkin wrote:
On Wed, Aug 31, 2011 at 03:37:33PM +0300, Yonit Halperin wrote:
if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt.
This is a general issue with interrupt migration, and PCI core has code
to handle this, migrating interrupts. So rather than work around this
in qxl I'd like us to first understand whether there really exists such
a problem, since if yes it would affect other devices.
Could you help with that please?
I think this issue is spice-specific: the problem is that when a
spice_server thread issues a request for interrupt, the request is
passed to the qemu thread through a pipe. This pipe status is not saved
during migration. Thus, any pending interrupt request are purged when
migration completes.
In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram-int_pending was set.
Maybe this is the only issue?
A way to check would be to call
uint32_t pending = le32_to_cpu(d-ram-int_pending);
uint32_t mask= le32_to_cpu(d-ram-int_mask);
int level = !!(pending mask);
qxl_ring_set_dirty(d);
instead of qxl_set_irq, and see if that is enough.
I was talking about the check in qxl_send_events
Note: I don't object to reusing qxl_set_irq in
production, just let us make sure we don't hide bugs.
As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
You need to sign off :)
---
hw/qxl.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index b34bccf..c7edc60 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1362,7 +1362,6 @@ static void pipe_read(void *opaque)
qxl_set_irq(d);
}
-/* called from spice server thread context only */
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
{
uint32_t old_pending;
@@ -1463,7 +1462,13 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
PCIQXLDevice *qxl = opaque;
qemu_spice_vm_change_state_handler(qxl-ssd, running, reason);
-if (!running qxl-mode == QXL_MODE_NATIVE) {
+if (running) {
+/*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have been
called
+ */
+ qxl_set_irq(qxl);
+} else if (qxl-mode == QXL_MODE_NATIVE) {
/* dirty all vram (which holds surfaces) and devram (primary surface)
* to make sure they are saved */
/* FIXME #1: should go out during live stage */
--
1.7.4.4
