[Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks
Rationale: The following code is difficult to read, but allowed by the
current coding style.
if (a == 5) printf(a was 5.\n);
else if (a == 6) printf(a was 6.\n);
else printf(a was something else entirely.\n);
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
CODING_STYLE | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/CODING_STYLE b/CODING_STYLE
index a579cb1..3ffbc3d 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -51,11 +51,11 @@ QEMU coding style.
4. Block structure
-Every indented statement is braced; even if the block contains just one
-statement. The opening brace is on the line that contains the control
-flow statement that introduces the new block; the closing brace is on the
-same line as the else keyword, or on a line by itself if there is no else
-keyword. Example:
+Every control flow statement is followed by a new indented and braced
+block; even if the block contains just one statement. The opening brace
+is on the line that contains the control flow statement that introduces
+the new block; the closing brace is on the same line as the else keyword,
+or on a line by itself if there is no else keyword. Example:
if (a == 5) {
printf(a was 5.\n);
--
1.6.1.3
[Qemu-devel] Re: [ANNOUNCE] Sheepdog: Distributed Storage System for KVM
On 2009/10/25 17:51, Dietmar Maurer wrote: Do you support multiple guests accessing the same image? A VM image can be attached to any VMs but one VM at a time; multiple running VMs cannot access to the same VM image. I guess this is a problem when you want to do live migrations? Yes, because Sheepdog locks a VM image when it is opened. To avoid this problem, locking must be delayed until migration has done. This is also a TODO item. -- MORITA Kazutaka
[Qemu-devel] [PATCH v3] target-arm: fix neon shift helper functions
From: Juha Riihimäki juha.riihim...@nokia.com
Current code is broken at least on recent compilers, comparison
between signed and unsigned types yield incorrect code and render
the neon shift helper functions defunct. This is the third revision
of this patch, casting all comparisons with the sizeof operator to
signed ssize_t type to force comparisons to be between signed integral
types.
Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
---
target-arm/neon_helper.c | 26 ++
1 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index f32ecd6..5e6452b 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -392,7 +392,8 @@ NEON_VOP(abd_u32, neon_u32, 1)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp = sizeof(src1) * 8 || tmp = -sizeof(src1) * 8) { \
+if (tmp = (ssize_t)sizeof(src1) * 8 || \
+tmp = -(ssize_t)sizeof(src1) * 8) { \
dest = 0; \
} else if (tmp 0) { \
dest = src1 -tmp; \
@@ -420,9 +421,9 @@ uint64_t HELPER(neon_shl_u64)(uint64_t val, uint64_t
shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp = sizeof(src1) * 8) { \
+if (tmp = (ssize_t)sizeof(src1) * 8) { \
dest = 0; \
-} else if (tmp = -sizeof(src1) * 8) { \
+} else if (tmp = -(ssize_t)sizeof(src1) * 8) { \
dest = src1 (sizeof(src1) * 8 - 1); \
} else if (tmp 0) { \
dest = src1 -tmp; \
@@ -453,11 +454,11 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t
shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp = sizeof(src1) * 8) { \
+if (tmp = (ssize_t)sizeof(src1) * 8) { \
dest = 0; \
-} else if (tmp -sizeof(src1) * 8) { \
+} else if (tmp -(ssize_t)sizeof(src1) * 8) { \
dest = src1 (sizeof(src1) * 8 - 1); \
-} else if (tmp == -sizeof(src1) * 8) { \
+} else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
dest = src1 (tmp - 1); \
dest++; \
dest = 1; \
@@ -494,9 +495,10 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t
shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp = sizeof(src1) * 8 || tmp -sizeof(src1) * 8) { \
+if (tmp = (ssize_t)sizeof(src1) * 8 || \
+tmp -(ssize_t)sizeof(src1) * 8) { \
dest = 0; \
-} else if (tmp == -sizeof(src1) * 8) { \
+} else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
dest = src1 (tmp - 1); \
} else if (tmp 0) { \
dest = (src1 + (1 (-1 - tmp))) -tmp; \
@@ -528,14 +530,14 @@ uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t
shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp = sizeof(src1) * 8) { \
+if (tmp = (ssize_t)sizeof(src1) * 8) { \
if (src1) { \
SET_QC(); \
dest = ~0; \
} else { \
dest = 0; \
} \
-} else if (tmp = -sizeof(src1) * 8) { \
+} else if (tmp = -(ssize_t)sizeof(src1) * 8) { \
dest = 0; \
} else if (tmp 0) { \
dest = src1 -tmp; \
@@ -579,11 +581,11 @@ uint64_t HELPER(neon_qshl_u64)(CPUState *env, uint64_t
val, uint64_t shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp = sizeof(src1) * 8) { \
+if (tmp = (ssize_t)sizeof(src1) * 8) { \
if (src1) \
SET_QC(); \
dest = src1 31; \
-} else if (tmp = -sizeof(src1) * 8) { \
+} else if (tmp = -(ssize_t)sizeof(src1) * 8) { \
dest = src1 31; \
} else if (tmp 0) { \
dest = src1 -tmp; \
--
1.6.5
[Qemu-devel] guest OS crash during shutdown/reboot
Hi, I have compiled qemu 0.11.0 for x86 64 bit. I run Fedora Core 10 64 bit as my guest OS. Sometimes, when I shutdown the guest OS gracefully (using halt command), I see a kernel crash with a stack trace on guest OS window at * native_machine_shutdown()*. The RIP points to *native_smp_send_stop()*. This doesn't happen on every shutdown/reboot though. I did see this issue in older versions of qemu too(0.10.0 and 0.10.6). This could be a minor issue for most of you but I need to fix this because I run some automated scripts on the guest OS which reboots it. Does anybody has a fix or any other idea to get around this issue? Any bit of help on this would be really helpful. Thanks, PurnaChandar M
Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened
Amit Shah wrote: On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote: Amit Shah wrote: The OPENED event gets sent also when qemu resets its state initially. The consumers of the event aren't interested in receiving this event on reset. The monitor was. Now its initial prompt on activation is broken. The patch in Anthony's queue, titled 'console: call qemu_chr_reset() in text_console_init' You may also want to rename qemu_chr_reset - unless there is still a need for real reset. fixed that. However, with the qcow2 synchronous patch, the monitor prompt doesn't come up again -- which shows there is a problem with the way the bhs work and also the initial resets. Then the qcow2 patch is already in? At least applying your patch doesn't change the picture. I think the initial resets are a hack to work around something from my reading of it; do you have a better idea of why it's there and how it's all supposed to work? From the monitor's POV, it's not a hack, it's simply the requirement to receive an indication that the console was opened. Does this patch fix/improve something for a different user? If not, please let us revert it. There's another question too: is a separate 'reset' event needed in addition to an 'opened' event? Not for the monitor, but I cannot speak for other users. I think it would be good to check them in details before changing the reset/open semantic. I have a few apps (that are coming as part of the virtio-console work) that need just an 'opened' event and are not interested in the 'reset' event. Amit Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 09/10] target-arm: optimize neon vld/vst ops
On Oct 25, 2009, at 16:01, ext Laurent Desnogues wrote: I don't really like the idea of having tcg_qemu_ld/st not factored in some place, as it makes memory access tracing extensions more intrusive. This brings us back to the problem having function freeing tmps. In that case, you could perhaps create a gen_st16_dont_free function as a temporary workaround? I could, but it is getting ugly =/ How about if I create another patch that moves the temporary variable (de)allocation outside the gen_ldx/ gen_stx functions and then refactor this patch on top of that? Regards, Juha
[Qemu-devel] Re: [PATCH] raw/linux-aio: Also initialize POSIX AIO
Am 25.10.2009 08:19, schrieb Christoph Hellwig: On Thu, Oct 22, 2009 at 11:05:55AM +0200, Kevin Wolf wrote: Yes, it might look like overkill to introduce a abstraction for exactly two backends. I felt the same way. But then, the current implementation just feels totally wrong. It absolutely intransparent when we fall back to paio, and before debugging the bdrv_read/write emulation I didn't even know that we're doing it. And, like I said, why should a block format driver know what AIO method works which way? Because the aio method is part of the block driver. Despite our code organization linux-aio.c and compat-posix-aio.c aren't generic abstractions but sub-modules of raw-posix. Well, my question was not if they are sub-modules of raw-posix - they clearly are - but rather if they should be. But ok, I'll just submit a patch drop the context parameter in paio_* then. Kevin
Re: [Qemu-devel] [PATCH] isa: configure serial+parallel by index.
Markus Armbruster arm...@redhat.com writes: Gerd Hoffmann kra...@redhat.com writes: This patch adds a 'index' property to the isa-parallel and isa-serial devices. This can be used to create devices with the default isa irqs and ioports by simply specifying the index, i.e. -device isa-serial,index=1 instead of -device isa-serial,iobase=0x2f8,irq=3 for ttyS1 aka com2. Likewise for parallel ports. Not mentioned here, only in the code: * index defaults to 0 for the first device to initialize, 1 for the second, and so forth. * It is okay to overwrite the defaults provided by index, e.g. -device isa-serial,index=1,irq=4 Looks fine to me. A similar solution could do for default mac address. One little thing I missed on first reading: making an index property hexadecimal is weird. I'd really expect decimal there.
Re: [Qemu-devel] [PATCH v2 03/10] target-arm: allow modifying vfp fpexc en bit only
On Mon, Oct 26, 2009 at 8:32 AM, juha.riihim...@nokia.com wrote: [...] Shouldn't writes to FPEXC from gdb be protected in the same way? Except for that I agree with your patch. Please correct me if I'm wrong but it seems to me that the code in gdbstub.c never writes anything to the VFP registers, at least it seems to me the code in cpu_gdb_write_register and cpu_gdb_read_register ignore floating point registers. There's code in helper.c that implements writes to coprocessor registers. Look for vfp_gdb_set_reg which is registered by cpu_arm_init. Laurent
Re: [Qemu-devel] [PATCH v2 09/10] target-arm: optimize neon vld/vst ops
On Mon, Oct 26, 2009 at 8:46 AM, juha.riihim...@nokia.com wrote: On Oct 25, 2009, at 16:01, ext Laurent Desnogues wrote: I don't really like the idea of having tcg_qemu_ld/st not factored in some place, as it makes memory access tracing extensions more intrusive. This brings us back to the problem having function freeing tmps. In that case, you could perhaps create a gen_st16_dont_free function as a temporary workaround? I could, but it is getting ugly =/ How about if I create another patch that moves the temporary variable (de)allocation outside the gen_ldx/ gen_stx functions and then refactor this patch on top of that? I'd personally like this, but I guess you'd better wait for some inputs from other QEMU dev's before doing it. Laurent
Re: [Qemu-devel] [PATCH] isa: configure serial+parallel by index.
Looks fine to me. A similar solution could do for default mac address. One little thing I missed on first reading: making an index property hexadecimal is weird. I'd really expect decimal there. Indeed. Wasn't intentional, cut+paste bug. Will fix. cheers, Gerd
Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened
On (Mon) Oct 26 2009 [08:40:12], Jan Kiszka wrote: Amit Shah wrote: On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote: Amit Shah wrote: The OPENED event gets sent also when qemu resets its state initially. The consumers of the event aren't interested in receiving this event on reset. The monitor was. Now its initial prompt on activation is broken. The patch in Anthony's queue, titled 'console: call qemu_chr_reset() in text_console_init' You may also want to rename qemu_chr_reset - unless there is still a need for real reset. Yeah; there isn't a need after my patches -- I've been slowing working towards renaming it all. fixed that. However, with the qcow2 synchronous patch, the monitor prompt doesn't come up again -- which shows there is a problem with the way the bhs work and also the initial resets. Then the qcow2 patch is already in? At least applying your patch doesn't change the picture. Yeah; that patch went in just before my patches. Can you try reverting ef845c3bf421290153154635dc18eaa677cecb43 I think the initial resets are a hack to work around something from my reading of it; do you have a better idea of why it's there and how it's all supposed to work? From the monitor's POV, it's not a hack, it's simply the requirement to receive an indication that the console was opened. Just an indication that the monitor was opened -- agreed. But git history shows you added that as 'reset', so I'm wondering if maybe you wanted it to do something else as well (or you did it that way just because of the way qemu's bhs are handled?). Does this patch fix/improve something for a different user? If not, please let us revert it. There's another question too: is a separate 'reset' event needed in addition to an 'opened' event? Not for the monitor, but I cannot speak for other users. I think it would be good to check them in details before changing the reset/open semantic. As far as I could see in the git history, the 'reset' was added for the monitor. And the others could live with the double 'reset' events they were getting -- one for the reset and one when the device was opened. Amit
[Qemu-devel] [PATCH v3] target-arm: allow modifying vfp fpexc en bit only
From: Juha Riihimäki juha.riihim...@nokia.com
All other bits except for the EN in the VFP FPEXC register are defined
as subarchitecture specific and real functionality for any of the
other bits has not been implemented in QEMU. However, current code
allows modifying all bits in the VFP FPEXC register leading to
problems when guest code is writing 1's to the subarchitecture
specific bits and checking whether the bits stay up to verify the
existence of functionality which in fact does not exist in QEMU.
This patch has been revised to include the same behavior change in
the gdb register write function.
Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
---
target-arm/helper.c|2 +-
target-arm/translate.c |3 +++
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 701629a..ee5df59 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -233,7 +233,7 @@ static int vfp_gdb_set_reg(CPUState *env, uint8_t *buf, int
reg)
switch (reg - nregs) {
case 0: env-vfp.xregs[ARM_VFP_FPSID] = ldl_p(buf); return 4;
case 1: env-vfp.xregs[ARM_VFP_FPSCR] = ldl_p(buf); return 4;
-case 2: env-vfp.xregs[ARM_VFP_FPEXC] = ldl_p(buf); return 4;
+case 2: env-vfp.xregs[ARM_VFP_FPEXC] = ldl_p(buf) (1 30); return 4;
}
return 0;
}
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 8503b92..d19ac7f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2804,6 +2804,9 @@ static int disas_vfp_insn(CPUState * env, DisasContext
*s, uint32_t insn)
case ARM_VFP_FPEXC:
if (IS_USER(s))
return 1;
+/* TODO: VFP subarchitecture support.
+ * For now, keep the EN bit only */
+tcg_gen_andi_i32(tmp, tmp, 1 30);
store_cpu_field(tmp, vfp.xregs[rn]);
gen_lookup_tb(s);
break;
--
1.6.5
Re: [Qemu-devel] [PATCH v3] target-arm: allow modifying vfp fpexc en bit only
On Mon, Oct 26, 2009 at 10:46 AM, juha.riihim...@nokia.com wrote:
From: Juha Riihimäki juha.riihim...@nokia.com
All other bits except for the EN in the VFP FPEXC register are defined
as subarchitecture specific and real functionality for any of the
other bits has not been implemented in QEMU. However, current code
allows modifying all bits in the VFP FPEXC register leading to
problems when guest code is writing 1's to the subarchitecture
specific bits and checking whether the bits stay up to verify the
existence of functionality which in fact does not exist in QEMU.
This patch has been revised to include the same behavior change in
the gdb register write function.
Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
Acked-by: Laurent Desnogues laurent.desnog...@gmail.com
Laurent
---
target-arm/helper.c | 2 +-
target-arm/translate.c | 3 +++
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 701629a..ee5df59 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -233,7 +233,7 @@ static int vfp_gdb_set_reg(CPUState *env, uint8_t *buf,
int reg)
switch (reg - nregs) {
case 0: env-vfp.xregs[ARM_VFP_FPSID] = ldl_p(buf); return 4;
case 1: env-vfp.xregs[ARM_VFP_FPSCR] = ldl_p(buf); return 4;
- case 2: env-vfp.xregs[ARM_VFP_FPEXC] = ldl_p(buf); return 4;
+ case 2: env-vfp.xregs[ARM_VFP_FPEXC] = ldl_p(buf) (1 30); return 4;
}
return 0;
}
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 8503b92..d19ac7f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2804,6 +2804,9 @@ static int disas_vfp_insn(CPUState * env, DisasContext
*s, uint32_t insn)
case ARM_VFP_FPEXC:
if (IS_USER(s))
return 1;
+ /* TODO: VFP subarchitecture support.
+ * For now, keep the EN bit only */
+ tcg_gen_andi_i32(tmp, tmp, 1 30);
store_cpu_field(tmp, vfp.xregs[rn]);
gen_lookup_tb(s);
break;
--
1.6.5
Re: [Qemu-devel] [PATCH v2][RESEND] target-arm: cleanup internal resource leaks
On Thu, Oct 22, 2009 at 1:17 PM, juha.riihim...@nokia.com wrote:
From: Juha Riihimäki juha.riihim...@nokia.com
Revised patch for getting rid of tcg temporary variable leaks in
target-arm/translate.c. This version also includes the leak patch for
gen_set_cpsr macro, now converted as a static inline function, which I
sent earlier as a separate patch on top of this patch.
Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
---
target-arm/translate.c | 116 ---
1 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index e56082b..813f661 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -184,7 +184,12 @@ static void store_reg(DisasContext *s, int reg, TCGv var)
#define gen_uxtb16(var) gen_helper_uxtb16(var, var)
-#define gen_set_cpsr(var, mask) gen_helper_cpsr_write(var,
tcg_const_i32(mask))
+static inline void gen_set_cpsr(TCGv var, uint32_t mask)
+{
+ TCGv tmp_mask = tcg_const_i32(mask);
+ gen_helper_cpsr_write(var, tmp_mask);
+ tcg_temp_free_i32(tmp_mask);
+}
/* Set NZCV flags from the high 4 bits of var. */
#define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
@@ -287,6 +292,7 @@ static TCGv_i64 gen_mulu_i64_i32(TCGv a, TCGv b)
tcg_gen_extu_i32_i64(tmp2, b);
dead_tmp(b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
return tmp1;
}
@@ -300,6 +306,7 @@ static TCGv_i64 gen_muls_i64_i32(TCGv a, TCGv b)
tcg_gen_ext_i32_i64(tmp2, b);
dead_tmp(b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
return tmp1;
}
@@ -312,9 +319,11 @@ static void gen_mull(TCGv a, TCGv b)
tcg_gen_extu_i32_i64(tmp1, a);
tcg_gen_extu_i32_i64(tmp2, b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
tcg_gen_trunc_i64_i32(a, tmp1);
tcg_gen_shri_i64(tmp1, tmp1, 32);
tcg_gen_trunc_i64_i32(b, tmp1);
+ tcg_temp_free_i64(tmp1);
}
/* Signed 32x32-64 multiply. */
@@ -326,9 +335,11 @@ static void gen_imull(TCGv a, TCGv b)
tcg_gen_ext_i32_i64(tmp1, a);
tcg_gen_ext_i32_i64(tmp2, b);
tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+ tcg_temp_free_i64(tmp2);
tcg_gen_trunc_i64_i32(a, tmp1);
tcg_gen_shri_i64(tmp1, tmp1, 32);
tcg_gen_trunc_i64_i32(b, tmp1);
+ tcg_temp_free_i64(tmp1);
}
/* Swap low and high halfwords. */
@@ -542,11 +553,13 @@ static void gen_arm_parallel_addsub(int op1, int op2,
TCGv a, TCGv b)
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(s)
+ tcg_temp_free_ptr(tmp);
break;
case 5:
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(u)
+ tcg_temp_free_ptr(tmp);
break;
#undef gen_pas_helper
#define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
@@ -587,11 +600,13 @@ static void gen_thumb2_parallel_addsub(int op1, int
op2, TCGv a, TCGv b)
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(s)
+ tcg_temp_free_ptr(tmp);
break;
case 4:
tmp = tcg_temp_new_ptr();
tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
PAS_OP(u)
+ tcg_temp_free_ptr(tmp);
break;
#undef gen_pas_helper
#define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
@@ -995,10 +1010,12 @@ static inline void gen_vfp_tosiz(int dp)
#define VFP_GEN_FIX(name) \
static inline void gen_vfp_##name(int dp, int shift) \
{ \
+ TCGv tmp_shift = tcg_const_i32(shift); \
if (dp) \
- gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tcg_const_i32(shift),
cpu_env);\
+ gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tmp_shift, cpu_env);\
else \
- gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tcg_const_i32(shift),
cpu_env);\
+ gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tmp_shift, cpu_env);\
+ tcg_temp_free_i32(tmp_shift); \
}
VFP_GEN_FIX(tosh)
VFP_GEN_FIX(tosl)
@@ -2399,7 +2416,7 @@ static int disas_dsp_insn(CPUState *env, DisasContext
*s, uint32_t insn)
instruction is not defined. */
static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
{
- TCGv tmp;
+ TCGv tmp, tmp2;
uint32_t rd = (insn 12) 0xf;
uint32_t cp = (insn 8) 0xf;
if (IS_USER(s)) {
@@ -2411,14 +2428,18 @@ static int disas_cp_insn(CPUState *env, DisasContext
*s, uint32_t insn)
return 1;
gen_set_pc_im(s-pc);
tmp = new_tmp();
- gen_helper_get_cp(tmp, cpu_env, tcg_const_i32(insn));
+ tmp2 = tcg_const_i32(insn);
+ gen_helper_get_cp(tmp, cpu_env, tmp2);
+ tcg_temp_free(tmp2);
store_reg(s, rd, tmp);
} else {
if (!env-cp[cp].cp_write)
return 1;
Re: [Qemu-devel] [PATCH v2][RESEND] target-arm: cleanup internal resource leaks
On Oct 26, 2009, at 12:46, ext Laurent Desnogues wrote:
@@ -5511,8 +5539,9 @@ static int disas_neon_data_insn(CPUState *
env, DisasContext *s, uint32_t insn)
tcg_gen_movi_i32(tmp, 0);
}
tmp2 = neon_load_reg(rm, 0);
-gen_helper_neon_tbl(tmp2, tmp2, tmp, tcg_const_i32
(rn),
-tcg_const_i32(n));
+tmp4 = tcg_const_i32(rn);
+tmp5 = tcg_const_i32(n);
+gen_helper_neon_tbl(tmp2, tmp2, tmp, tmp4, tmp5);
dead_tmp(tmp);
if (insn (1 6)) {
tmp = neon_load_reg(rd, 1);
@@ -5521,8 +5550,9 @@ static int disas_neon_data_insn(CPUState *
env, DisasContext *s, uint32_t insn)
tcg_gen_movi_i32(tmp, 0);
}
tmp3 = neon_load_reg(rm, 1);
-gen_helper_neon_tbl(tmp3, tmp3, tmp, tcg_const_i32
(rn),
-tcg_const_i32(n));
+gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5);
+dead_tmp(tmp5);
+dead_tmp(tmp4);
I missed this mistake when I acked the patch: you're using
dead_tmp instead of tcg_temp_free... One more reason
to drop dead_tmp :-)
Indeed. I'll fix it, didn't notice it either because I'm using other
new_xxx/dead_xxx helpers myself to catch resource leaks. I should work
around that to get more similar code and eliminate this kind of issues.
The bug will cause false resource leak reports, the actual freeing of
the temporary is done correctly. Since this patch seems to have
already been applied, I'll just provide a patch that fixes this
specific error in the mainline instead of reposting a fixed version of
the original patch.
Regards,
Juha
[Qemu-devel] [PATCH] target-arm: fix incorrect temporary variable freeing
From: Juha Riihimäki juha.riihim...@nokia.com tmp4 and tmp5 temporary variables are allocated using tcg_const_i32 but incorrectly released using dead_tmp which will cause resource leak tracking to report false leaks. Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com --- target-arm/translate.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 9d13d42..1988cc6 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -5535,8 +5535,8 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } tmp3 = neon_load_reg(rm, 1); gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5); -dead_tmp(tmp5); -dead_tmp(tmp4); +tcg_temp_free_i32(tmp5); +tcg_temp_free_i32(tmp4); neon_store_reg(rd, 0, tmp2); neon_store_reg(rd, 1, tmp3); dead_tmp(tmp); -- 1.6.5
[Qemu-devel] [PATCH v4 1/4] rom loader: use qemu_strdup.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/loader.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/loader.c b/hw/loader.c
index 7aa1a67..6baafa8 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -559,7 +559,7 @@ int rom_add_file(const char *file,
rom-name = qemu_strdup(file);
rom-path = qemu_find_file(QEMU_FILE_TYPE_BIOS, rom-name);
if (rom-path == NULL) {
-rom-path = strdup(file);
+rom-path = qemu_strdup(file);
}
fd = open(rom-path, O_RDONLY | O_BINARY);
--
1.6.2.5
[Qemu-devel] [PATCH v4 4/4] use rom loader for pc bios.
The pc bios shows up in 'info roms' now.
Note that the BIOS is mapped to two places: The complete rom at the top
of the memory, and the first 128k at 0xe. Only the first place is
listed in 'info roms'.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/pc.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 9e34104..6b4e008 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1115,8 +1115,8 @@ static void pc_init1(ram_addr_t ram_size,
goto bios_error;
}
bios_offset = qemu_ram_alloc(bios_size);
-ret = load_image(filename, qemu_get_ram_ptr(bios_offset));
-if (ret != bios_size) {
+ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size));
+if (ret != 0) {
bios_error:
fprintf(stderr, qemu: could not load PC BIOS '%s'\n, bios_name);
exit(1);
--
1.6.2.5
[Qemu-devel] [PATCH v4 3/4] vga roms: move loading from pc.c to vga drivers.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/cirrus_vga.c |6 +-
hw/pc.c | 12
hw/vga-isa.c|3 +++
hw/vga-pci.c|4
hw/vga_int.h|2 ++
5 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 9dfe76a..f4c9cdf 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -32,6 +32,7 @@
#include console.h
#include vga_int.h
#include kvm.h
+#include loader.h
/*
* TODO:
@@ -3162,6 +3163,7 @@ void isa_cirrus_vga_init(void)
s-vga.screen_dump, s-vga.text_update,
s-vga);
vmstate_register(0, vmstate_cirrus_vga, s);
+rom_add_vga(VGABIOS_CIRRUS_FILENAME);
/* XXX ISA-LFB support */
}
@@ -3245,7 +3247,9 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
PCI_ADDRESS_SPACE_MEM, cirrus_pci_mmio_map);
}
vmstate_register(0, vmstate_pci_cirrus_vga, d);
- /* XXX: ROM BIOS */
+
+ /* ROM BIOS */
+ rom_add_vga(VGABIOS_CIRRUS_FILENAME);
return 0;
}
diff --git a/hw/pc.c b/hw/pc.c
index 09d60ed..9e34104 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -47,8 +47,6 @@
//#define DEBUG_MULTIBOOT
#define BIOS_FILENAME bios.bin
-#define VGABIOS_FILENAME vgabios.bin
-#define VGABIOS_CIRRUS_FILENAME vgabios-cirrus.bin
#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
@@ -1050,7 +1048,6 @@ static void pc_init1(ram_addr_t ram_size,
IsaIrqState *isa_irq_state;
DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
DriveInfo *fd[MAX_FD];
-int using_vga = cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled;
void *fw_cfg;
if (ram_size = 0xe000 ) {
@@ -1141,15 +1138,6 @@ static void pc_init1(ram_addr_t ram_size,
option_rom_offset = qemu_ram_alloc(PC_ROM_SIZE);
cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE,
option_rom_offset);
-if (using_vga) {
-/* VGA BIOS load */
-if (cirrus_vga_enabled) {
-rom_add_vga(VGABIOS_CIRRUS_FILENAME);
-} else {
-rom_add_vga(VGABIOS_FILENAME);
-}
-}
-
/* map all the bios at the top of memory */
cpu_register_physical_memory((uint32_t)(-bios_size),
bios_size, bios_offset | IO_MEM_ROM);
diff --git a/hw/vga-isa.c b/hw/vga-isa.c
index 7fa31d3..6b387af 100644
--- a/hw/vga-isa.c
+++ b/hw/vga-isa.c
@@ -27,6 +27,7 @@
#include vga_int.h
#include pixel_ops.h
#include qemu-timer.h
+#include loader.h
int isa_vga_init(void)
{
@@ -46,5 +47,7 @@ int isa_vga_init(void)
cpu_register_physical_memory(VBE_DISPI_LFB_PHYSICAL_ADDRESS,
VGA_RAM_SIZE, s-vram_offset);
#endif
+/* ROM BIOS */
+rom_add_vga(VGABIOS_FILENAME);
return 0;
}
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 5e75938..b5fd666 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -28,6 +28,7 @@
#include vga_int.h
#include pixel_ops.h
#include qemu-timer.h
+#include loader.h
typedef struct PCIVGAState {
PCIDevice dev;
@@ -117,6 +118,9 @@ static int pci_vga_initfn(PCIDevice *dev)
pci_register_bar(d-dev, PCI_ROM_SLOT, bios_total_size,
PCI_ADDRESS_SPACE_MEM_PREFETCH, vga_map);
}
+
+ /* ROM BIOS */
+ rom_add_vga(VGABIOS_FILENAME);
return 0;
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index c162c07..b843e52 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -220,6 +220,8 @@ extern const uint8_t sr_mask[8];
extern const uint8_t gr_mask[16];
#define VGA_RAM_SIZE (8192 * 1024)
+#define VGABIOS_FILENAME vgabios.bin
+#define VGABIOS_CIRRUS_FILENAME vgabios-cirrus.bin
extern CPUReadMemoryFunc * const vga_mem_read[3];
extern CPUWriteMemoryFunc * const vga_mem_write[3];
--
1.6.2.5
[Qemu-devel] [PATCH] isa: configure serial+parallel by index.
This patch adds a 'index' property to the isa-parallel and isa-serial
devices. This can be used to create devices with the default isa irqs
and ioports by simply specifying the index, i.e.
-device isa-serial,index=1
instead of
-device isa-serial,iobase=0x2f8,irq=3
for ttyS1 aka com2. Likewise for parallel ports.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/parallel.c | 23 +--
hw/serial.c | 26 +++---
2 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/hw/parallel.c b/hw/parallel.c
index 92eecb1..263b5bf 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -80,6 +80,7 @@ struct ParallelState {
typedef struct ISAParallelState {
ISADevice dev;
+uint32_t index;
uint32_t iobase;
uint32_t isairq;
ParallelState state;
@@ -445,11 +446,14 @@ static void parallel_reset(void *opaque)
s-last_read_offset = ~0U;
}
+static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
+
static int parallel_isa_initfn(ISADevice *dev)
{
+static int index;
ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
ParallelState *s = isa-state;
-int base = isa-iobase;
+int base;
uint8_t dummy;
if (!s-chr) {
@@ -457,6 +461,15 @@ static int parallel_isa_initfn(ISADevice *dev)
exit(1);
}
+if (isa-index == -1)
+isa-index = index;
+if (isa-index = MAX_PARALLEL_PORTS)
+return -1;
+if (isa-iobase == -1)
+isa-iobase = isa_parallel_io[isa-index];
+index++;
+
+base = isa-iobase;
isa_init_irq(dev, s-irq, isa-isairq);
parallel_reset(s);
qemu_register_reset(parallel_reset, s);
@@ -483,15 +496,12 @@ static int parallel_isa_initfn(ISADevice *dev)
return 0;
}
-static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
-
ParallelState *parallel_init(int index, CharDriverState *chr)
{
ISADevice *dev;
dev = isa_create(isa-parallel);
-qdev_prop_set_uint32(dev-qdev, iobase, isa_parallel_io[index]);
-qdev_prop_set_uint32(dev-qdev, irq, 7);
+qdev_prop_set_uint32(dev-qdev, index, index);
qdev_prop_set_chr(dev-qdev, chardev, chr);
if (qdev_init(dev-qdev) 0)
return NULL;
@@ -579,7 +589,8 @@ static ISADeviceInfo parallel_isa_info = {
.qdev.size = sizeof(ISAParallelState),
.init = parallel_isa_initfn,
.qdev.props = (Property[]) {
-DEFINE_PROP_HEX32(iobase, ISAParallelState, iobase, 0x378),
+DEFINE_PROP_UINT32(index, ISAParallelState, index, -1),
+DEFINE_PROP_HEX32(iobase, ISAParallelState, iobase, -1),
DEFINE_PROP_UINT32(irq, ISAParallelState, isairq, 7),
DEFINE_PROP_CHR(chardev, ISAParallelState, state.chr),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/serial.c b/hw/serial.c
index eb14f11..381b027 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -148,6 +148,7 @@ struct SerialState {
typedef struct ISASerialState {
ISADevice dev;
+uint32_t index;
uint32_t iobase;
uint32_t isairq;
SerialState state;
@@ -733,11 +734,25 @@ static void serial_init_core(SerialState *s)
serial_event, s);
}
+static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8,
0x2e8 };
+static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
+
static int serial_isa_initfn(ISADevice *dev)
{
+static int index;
ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
SerialState *s = isa-state;
+if (isa-index == -1)
+isa-index = index;
+if (isa-index = MAX_SERIAL_PORTS)
+return -1;
+if (isa-iobase == -1)
+isa-iobase = isa_serial_io[isa-index];
+if (isa-isairq == -1)
+isa-isairq = isa_serial_irq[isa-index];
+index++;
+
s-baudbase = 115200;
isa_init_irq(dev, s-irq, isa-isairq);
serial_init_core(s);
@@ -748,16 +763,12 @@ static int serial_isa_initfn(ISADevice *dev)
return 0;
}
-static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8,
0x2e8 };
-static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
-
SerialState *serial_isa_init(int index, CharDriverState *chr)
{
ISADevice *dev;
dev = isa_create(isa-serial);
-qdev_prop_set_uint32(dev-qdev, iobase, isa_serial_io[index]);
-qdev_prop_set_uint32(dev-qdev, irq, isa_serial_irq[index]);
+qdev_prop_set_uint32(dev-qdev, index, index);
qdev_prop_set_chr(dev-qdev, chardev, chr);
if (qdev_init(dev-qdev) 0)
return NULL;
@@ -886,8 +897,9 @@ static ISADeviceInfo serial_isa_info = {
.qdev.size = sizeof(ISASerialState),
.init = serial_isa_initfn,
.qdev.props = (Property[]) {
-DEFINE_PROP_HEX32(iobase, ISASerialState, iobase, 0x3f8),
-DEFINE_PROP_UINT32(irq, ISASerialState, isairq, 4),
+DEFINE_PROP_UINT32(index, ISASerialState, index, -1),
+DEFINE_PROP_HEX32(iobase,
Re: [Qemu-devel] [PATCH] target-arm: fix incorrect temporary variable freeing
On Mon, Oct 26, 2009 at 12:02 PM, juha.riihim...@nokia.com wrote: From: Juha Riihimäki juha.riihim...@nokia.com tmp4 and tmp5 temporary variables are allocated using tcg_const_i32 but incorrectly released using dead_tmp which will cause resource leak tracking to report false leaks. Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com Acked-by: Laurent Desnogues laurent.desnog...@gmail.com Laurent --- target-arm/translate.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 9d13d42..1988cc6 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -5535,8 +5535,8 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } tmp3 = neon_load_reg(rm, 1); gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5); - dead_tmp(tmp5); - dead_tmp(tmp4); + tcg_temp_free_i32(tmp5); + tcg_temp_free_i32(tmp4); neon_store_reg(rd, 0, tmp2); neon_store_reg(rd, 1, tmp3); dead_tmp(tmp); -- 1.6.5
Re: [Qemu-devel] Re: [Patch] Make usermode stacksize (-s) configurable at compile-time
Am Sonntag 25 Oktober 2009 22:09:10 schrieb Juan Quintela: If we are changing the code in configure anyways, why don't add the default there? Something like this (I tested that it compiled, but didn't tested usermode). I am assuming that you only want it for linux-user. I like this version. Signed-off-by: Jan-Simon Möller dl...@gmx.de From 0dc910f5d1b2e1dd7120abf32d25db190cd394e5 Mon Sep 17 00:00:00 2001 From: Juan Quintela quint...@redhat.com Date: Sun, 25 Oct 2009 22:06:52 +0100 Subject: [PATCH] User mode stacksize implementation Best, Jan-Simon
[Qemu-devel] [PATCH v4 2/4] rom loader: make vga+rom loading configurable.
The rom_add_vga() and rom_add_option() macros are transformed into
functions. They look at the new rom_enable_driver_roms variable
and only do something if it is set to non-zero, making vga+option rom
loading runtime option. pc_init() sets rom_enable_driver_roms to 1.
With this in place we can move the rom loading calls from pc.c to the
individual drivers.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/loader.c | 15 +++
hw/loader.h |7 +++
hw/pc.c |1 +
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/hw/loader.c b/hw/loader.c
index 6baafa8..a08585b 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -534,6 +534,7 @@ struct Rom {
};
static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms);
+int rom_enable_driver_roms;
static void rom_insert(Rom *rom)
{
@@ -612,6 +613,20 @@ int rom_add_blob(const char *name, const void *blob,
size_t len,
return 0;
}
+int rom_add_vga(const char *file)
+{
+if (!rom_enable_driver_roms)
+return 0;
+return rom_add_file(file, PC_ROM_MIN_VGA, PC_ROM_MAX, PC_ROM_ALIGN);
+}
+
+int rom_add_option(const char *file)
+{
+if (!rom_enable_driver_roms)
+return 0;
+return rom_add_file(file, PC_ROM_MIN_OPTION, PC_ROM_MAX, PC_ROM_ALIGN);
+}
+
static void rom_reset(void *unused)
{
Rom *rom;
diff --git a/hw/loader.h b/hw/loader.h
index 945c662..67dae57 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -38,9 +38,8 @@ void do_info_roms(Monitor *mon);
#define PC_ROM_ALIGN 0x800
#define PC_ROM_SIZE(PC_ROM_MAX - PC_ROM_MIN_VGA)
-#define rom_add_vga(_f) \
-rom_add_file(_f, PC_ROM_MIN_VGA,PC_ROM_MAX, PC_ROM_ALIGN)
-#define rom_add_option(_f) \
-rom_add_file(_f, PC_ROM_MIN_OPTION, PC_ROM_MAX, PC_ROM_ALIGN)
+extern int rom_enable_driver_roms;
+int rom_add_vga(const char *file);
+int rom_add_option(const char *file);
#endif
diff --git a/hw/pc.c b/hw/pc.c
index 408d6d6..09d60ed 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1137,6 +1137,7 @@ static void pc_init1(ram_addr_t ram_size,
+rom_enable_driver_roms = 1;
option_rom_offset = qemu_ram_alloc(PC_ROM_SIZE);
cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE,
option_rom_offset);
--
1.6.2.5
[Qemu-devel] [PATCH v4 0/4] more rom loader patches.
Hi,
A collection of rom loader bits, check individual patches for details.
One more change for rom_add_{vga,option}. We are simply using a
variable now to enable rom loading, so different TARGET_I386 machine
types can have different behavior here. pc98 support needs this.
cheers,
Gerd
[Qemu-devel] [PATCH v10 0/2] virtio-console: Add support for multiple ports for generic guest-host communication
Hello, This iteration of the series fixes a style bug pointed out by Gerd and a typo noticed by Rich. In addition to that, it also introduces a few debug prints in the 'info qtree' output that shows some per-port data that could be helpful for debugging. I've been testing all the features that are presented here from the testsuite at http://fedorapeople.org/gitweb?p=amitshah/public_git/test-virtserial.git The testsuite tests for host-guest interaction as well as qemu chardev-virtioserial interaction. Amit Shah (2): virtio-console: Add a virtio-serial bus, support for multiple ports virtio-console: Add a new virtserialport device for generic serial port support Makefile.target|2 +- hw/pc.c|9 - hw/ppc440_bamboo.c |7 - hw/qdev.c |8 +- hw/virtio-console.c| 206 +++-- hw/virtio-console.h| 19 -- hw/virtio-pci.c|8 +- hw/virtio-serial-bus.c | 785 hw/virtio-serial.h | 227 ++ hw/virtio.h|2 +- qemu-options.hx|6 +- sysemu.h |6 - vl.c | 65 +++-- 13 files changed, 1171 insertions(+), 179 deletions(-) delete mode 100644 hw/virtio-console.h create mode 100644 hw/virtio-serial-bus.c create mode 100644 hw/virtio-serial.h
[Qemu-devel] [PATCH v10 2/2] virtio-console: Add a new virtserialport device for generic serial port support
This patch adds generic serial ports over the virtio serial bus.
These ports have a few more options that are not relevant for
virtio console ports: the ability to cache buffers that are
received for a port while it's disconnected, setting of limits
to the bytes that are cached so as to prevent OOM conditions,
etc.
Sample uses for such a device can be obtaining info from the
guest like the file systems used, apps installed, etc. for
offline usage and logged-in users, clipboard copy-paste, etc.
for online usage.
For requirements, use-cases, test cases and some history see
http://www.linux-kvm.org/page/VMchannel_Requirements
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-console.c | 38 ++
1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index a14133b..c27e556 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -107,3 +107,41 @@ static void virtcon_register(void)
virtio_serial_port_qdev_register(virtcon_info);
}
device_init(virtcon_register)
+
+
+/* Generic Virtio Serial Ports */
+static int virtserial_port_initfn(VirtIOSerialDevice *dev)
+{
+VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, dev-qdev);
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+port-info = dev-info;
+
+if (vcon-chr) {
+qemu_chr_add_handlers(vcon-chr, chr_can_read, chr_read, chr_event,
+ vcon);
+}
+return 0;
+}
+
+static VirtIOSerialPortInfo virtserial_port_info = {
+.qdev.name = virtserialport,
+.qdev.size = sizeof(VirtConsole),
+.init = virtserial_port_initfn,
+.have_data = flush_buf,
+.qdev.props = (Property[]) {
+DEFINE_PROP_CHR(chardev, VirtConsole, chr),
+DEFINE_PROP_STRING(name, VirtConsole, port.name),
+DEFINE_PROP_INT32(cache_buffers, VirtConsole, port.cache_buffers, 1),
+DEFINE_PROP_UINT64(byte_limit, VirtConsole, port.byte_limit, 0),
+DEFINE_PROP_UINT64(guest_byte_limit, VirtConsole,
+ port.guest_byte_limit, 0),
+DEFINE_PROP_END_OF_LIST(),
+},
+};
+
+static void virtserial_port_register(void)
+{
+virtio_serial_port_qdev_register(virtserial_port_info);
+}
+device_init(virtserial_port_register)
--
1.6.2.5
[Qemu-devel] [PATCH] Remove aio_ctx from paio_* interface
The context parameter in paio_submit isn't used anyway, so there is no reason
why block drivers should need to remember it. This also avoids passing a Linux
AIO context to paio_submit (which doesn't do any harm as long as the parameter
is unused, but it is highly confusing).
Signed-off-by: Kevin Wolf kw...@redhat.com
---
To be applied on top of the bdrv_read/write emulation fix.
block/raw-posix-aio.h |4 ++--
block/raw-posix.c | 10 --
posix-aio-compat.c| 11 +--
3 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/block/raw-posix-aio.h b/block/raw-posix-aio.h
index a2d4348..dfc63b8 100644
--- a/block/raw-posix-aio.h
+++ b/block/raw-posix-aio.h
@@ -26,8 +26,8 @@
/* posix-aio-compat.c - thread pool based implementation */
-void *paio_init(void);
-BlockDriverAIOCB *paio_submit(BlockDriverState *bs, void *aio_ctx, int fd,
+int paio_init(void);
+BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
BlockDriverCompletionFunc *cb, void *opaque, int type);
BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 20b37a7..f396247 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -107,7 +107,6 @@ typedef struct BDRVRawState {
int type;
unsigned int lseek_err_cnt;
int open_flags;
-void *aio_ctx;
#if defined(__linux__)
/* linux floppy specific */
int64_t fd_open_time;
@@ -117,6 +116,7 @@ typedef struct BDRVRawState {
#endif
#ifdef CONFIG_LINUX_AIO
int use_aio;
+void *aio_ctx;
#endif
uint8_t* aligned_buf;
} BDRVRawState;
@@ -181,8 +181,7 @@ static int raw_open_common(BlockDriverState *bs, const char
*filename,
} else
#endif
{
-s-aio_ctx = paio_init();
-if (!s-aio_ctx) {
+if (paio_init() 0) {
goto out_free_buf;
}
#ifdef CONFIG_LINUX_AIO
@@ -554,7 +553,7 @@ static BlockDriverAIOCB *raw_aio_submit(BlockDriverState
*bs,
}
}
-return paio_submit(bs, s-aio_ctx, s-fd, sector_num, qiov, nb_sectors,
+return paio_submit(bs, s-fd, sector_num, qiov, nb_sectors,
cb, opaque, type);
}
@@ -582,8 +581,7 @@ static BlockDriverAIOCB *raw_aio_flush(BlockDriverState *bs,
if (fd_open(bs) 0)
return NULL;
-return paio_submit(bs, s-aio_ctx, s-fd, 0, NULL, 0,
- cb, opaque, QEMU_AIO_FLUSH);
+return paio_submit(bs, s-fd, 0, NULL, 0, cb, opaque, QEMU_AIO_FLUSH);
}
static void raw_close(BlockDriverState *bs)
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index ec58288..7f391c9 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -556,7 +556,7 @@ static AIOPool raw_aio_pool = {
.cancel = paio_cancel,
};
-BlockDriverAIOCB *paio_submit(BlockDriverState *bs, void *aio_ctx, int fd,
+BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
BlockDriverCompletionFunc *cb, void *opaque, int type)
{
@@ -607,7 +607,7 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd,
return acb-common;
}
-void *paio_init(void)
+int paio_init(void)
{
struct sigaction act;
PosixAioState *s;
@@ -615,7 +615,7 @@ void *paio_init(void)
int ret;
if (posix_aio_state)
-return posix_aio_state;
+return 0;
s = qemu_malloc(sizeof(PosixAioState));
@@ -627,7 +627,7 @@ void *paio_init(void)
s-first_aio = NULL;
if (pipe(fds) == -1) {
fprintf(stderr, failed to create pipe\n);
-return NULL;
+return -1;
}
s-rfd = fds[0];
@@ -650,6 +650,5 @@ void *paio_init(void)
QTAILQ_INIT(request_list);
posix_aio_state = s;
-
-return posix_aio_state;
+return 0;
}
--
1.6.2.5
[Qemu-devel] [PATCH v10 1/2] virtio-console: Add a virtio-serial bus, support for multiple ports
This patch migrates virtio-console to the qdev infrastructure and
creates a new virtio-serial bus on which multiple ports are exposed as
devices. The bulk of the code now resides in a new file with
virtio-console.c being just a simple qdev device.
This interface enables spawning of multiple virtio consoles as well as generic
serial ports.
The older -virtconsole argument still works, but when using the new
functionality, it is recommended to use
-device virtio-serial-pci -device virtconsole,...
The virtconsole device type accepts a chardev as an argument and a 'name'
argument to identify the corresponding consoles on the host as well as the
guest. The name, if given, is exposed via the 'name' sysfs attribute.
Care has been taken to ensure compatibility with kernels that do not
support multiple ports as well as accepting incoming migrations from older
qemu versions.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
Makefile.target|2 +-
hw/pc.c|9 -
hw/ppc440_bamboo.c |7 -
hw/qdev.c |8 +-
hw/virtio-console.c| 180 +---
hw/virtio-console.h| 19 --
hw/virtio-pci.c|8 +-
hw/virtio-serial-bus.c | 785
hw/virtio-serial.h | 227 ++
hw/virtio.h|2 +-
qemu-options.hx|6 +-
sysemu.h |6 -
vl.c | 65 +++--
13 files changed, 1139 insertions(+), 185 deletions(-)
delete mode 100644 hw/virtio-console.h
create mode 100644 hw/virtio-serial-bus.c
create mode 100644 hw/virtio-serial.h
diff --git a/Makefile.target b/Makefile.target
index 8d146c5..fd86eeb 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -157,7 +157,7 @@ ifdef CONFIG_SOFTMMU
obj-y = vl.o monitor.o pci.o machine.o gdbstub.o
# virtio has to be here due to weird dependency between PCI and virtio-net.
# need to fix this properly
-obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
virtio-pci.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o
virtio-console.o virtio-pci.o
obj-$(CONFIG_KVM) += kvm.o kvm-all.o
obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
LIBS+=-lz
diff --git a/hw/pc.c b/hw/pc.c
index 408d6d6..3dbe718 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1319,15 +1319,6 @@ static void pc_init1(ram_addr_t ram_size,
pci_create_simple(pci_bus, -1, lsi53c895a);
}
}
-
-/* Add virtio console devices */
-if (pci_enabled) {
-for(i = 0; i MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pci_bus, -1, virtio-console-pci);
-}
-}
-}
}
static void pc_init_pci(ram_addr_t ram_size,
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index a488240..1ab9872 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -108,13 +108,6 @@ static void bamboo_init(ram_addr_t ram_size,
env = ppc440ep_init(ram_size, pcibus, pci_irq_nrs, 1, cpu_model);
if (pcibus) {
-/* Add virtio console devices */
-for(i = 0; i MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pcibus, -1, virtio-console-pci);
-}
-}
-
/* Register network interfaces. */
for (i = 0; i nb_nics; i++) {
/* There are no PCI NICs on the Bamboo board, but there are
diff --git a/hw/qdev.c b/hw/qdev.c
index 20f931c..374d2d0 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -321,13 +321,9 @@ void qdev_machine_creation_done(void)
CharDriverState *qdev_init_chardev(DeviceState *dev)
{
static int next_serial;
-static int next_virtconsole;
+
/* FIXME: This is a nasty hack that needs to go away. */
-if (strncmp(dev-info-name, virtio, 6) == 0) {
-return virtcon_hds[next_virtconsole++];
-} else {
-return serial_hds[next_serial++];
-}
+return serial_hds[next_serial++];
}
BusState *qdev_get_parent_bus(DeviceState *dev)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 57f8f89..a14133b 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -1,143 +1,109 @@
/*
- * Virtio Console Device
+ * Virtio Console and Generic Port Devices
*
- * Copyright IBM, Corp. 2008
+ * Copyright Red Hat, Inc. 2009
*
* Authors:
- * Christian Ehrhardt ehrha...@linux.vnet.ibm.com
+ * Amit Shah amit.s...@redhat.com
*
* This work is licensed under the terms of the GNU GPL, version 2. See
* the COPYING file in the top-level directory.
- *
*/
-#include hw.h
#include qemu-char.h
-#include virtio.h
-#include virtio-console.h
-
+#include virtio-serial.h
-typedef struct VirtIOConsole
-{
-VirtIODevice vdev;
-VirtQueue *ivq, *ovq;
+typedef struct VirtConsole {
+VirtIOSerialPort port;
CharDriverState *chr;
-} VirtIOConsole;
+} VirtConsole;
-static VirtIOConsole *to_virtio_console(VirtIODevice *vdev)
-{
-return (VirtIOConsole *)vdev;
-}
-static void
Re: [Qemu-devel] [Patch] Make usermode stacksize (-s) configurable at compile-time
On Sun, Oct 25, 2009 at 06:49:44PM +0100, Jan-Simon Möller wrote:
We encontered problems with the low default value for the stacksize in
usermode (ok, whats low).
For environments like scratchbox, its hard to use -s as qemu is called by
binfmt mechanism.
The attached patch makes this configurable at compile-time.
fwiw in scratchbox we have a wrapper in between binfmt and qemu (called
misc_runner) that sets any
necessary command line parameters for qemu. You might want to consider a
similar approach so we
don't need to add a configure option (and thus hardcode the setting) to qemu
for every command
line option that might need changing.
Signed-off-by: Jan-Simon Möller dl...@gmx.de
diff --git a/configure b/configure
index 43d87c5..dad9175 100755
--- a/configure
+++ b/configure
@@ -220,6 +220,7 @@ uname_release=
io_thread=no
mixemu=no
kerneldir=
+user_mode_stacksize=
aix=no
blobs=yes
pkgversion=
@@ -557,6 +558,8 @@ for opt do
;;
--kerneldir=*) kerneldir=$optarg
;;
+ --user-mode-stacksize=*) user_mode_stacksize=$optarg
+ ;;
--with-pkgversion=*) pkgversion= ($optarg)
;;
--disable-docs) docs=no
@@ -713,6 +716,7 @@ echo --enable-linux-aio enable Linux AIO support
echo --enable-io-thread enable IO thread
echo --disable-blobs disable installing provided firmware blobs
echo --kerneldir=PATH look for kernel includes in PATH
+echo --user-mode-stacksize= set default stack size in bytes (as -s,
only in usermode)
echo
echo NOTE: The object files are built at the place where configure is
launched
exit 1
@@ -2481,6 +2485,16 @@ if test $target_user_only = yes -a $static =
no -a \
ldflags=-pie $ldflags
fi
+# change default stacksize in usermode
+#
+if test $target_user_only = yes -a $user_mode_stacksize != ; then
+ cflags=-DUSER_MODE_STACKSIZE=$user_mode_stacksize $cflags
+ echo user_mode_stack $user_mode_stacksize
+else
+ echo user_mode_stack default
+fi
+
+
if test $target_softmmu = yes -a \( \
$TARGET_ARCH = microblaze -o \
$TARGET_ARCH = cris \) ; then
diff --git a/linux-user/main.c b/linux-user/main.c
index 81a1ada..9ac2421 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -51,7 +51,10 @@ const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
/* XXX: on x86 MAP_GROWSDOWN only works if ESP = address + 32, so
we allocate a bigger stack. Need a better solution, for example
by remapping the process stack directly at the right place */
-unsigned long x86_stack_size = 512 * 1024;
+#ifndef USER_MODE_STACKSIZE
+#define USER_MODE_STACKSIZE (512 * 1024)
+#endif
+unsigned long x86_stack_size = USER_MODE_STACKSIZE;
void gemu_log(const char *fmt, ...)
{
diff --git a/configure b/configure
index 43d87c5..dad9175 100755
--- a/configure
+++ b/configure
@@ -220,6 +220,7 @@ uname_release=
io_thread=no
mixemu=no
kerneldir=
+user_mode_stacksize=
aix=no
blobs=yes
pkgversion=
@@ -557,6 +558,8 @@ for opt do
;;
--kerneldir=*) kerneldir=$optarg
;;
+ --user-mode-stacksize=*) user_mode_stacksize=$optarg
+ ;;
--with-pkgversion=*) pkgversion= ($optarg)
;;
--disable-docs) docs=no
@@ -713,6 +716,7 @@ echo --enable-linux-aio enable Linux AIO support
echo --enable-io-thread enable IO thread
echo --disable-blobs disable installing provided firmware blobs
echo --kerneldir=PATH look for kernel includes in PATH
+echo --user-mode-stacksize= set default stack size in bytes (as -s,
only in usermode)
echo
echo NOTE: The object files are built at the place where configure is
launched
exit 1
@@ -2481,6 +2485,16 @@ if test $target_user_only = yes -a $static =
no -a \
ldflags=-pie $ldflags
fi
+# change default stacksize in usermode
+#
+if test $target_user_only = yes -a $user_mode_stacksize != ; then
+
[Qemu-devel] [PATCH] qemu/virtio: make wmb compiler barrier + comments
wmb must be at least a compiler barrier, even without SMP.
Further, we likely need some rmb()/mb() as well:
I have not audited the code but lguest has mb(),
add a comment for now.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
hw/virtio.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/hw/virtio.c b/hw/virtio.c
index 337ff27..1f92171 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -23,8 +23,11 @@
/* QEMU doesn't strictly need write barriers since everything runs in
* lock-step. We'll leave the calls to wmb() in though to make it obvious for
* KVM or if kqemu gets SMP support.
+ * In any case, we must prevent the compiler from reordering the code.
+ * TODO: we likely need some rmb()/mb() as well.
*/
-#define wmb() do { } while (0)
+
+#define wmb() __asm__ __volatile__(: : :memory)
typedef struct VRingDesc
{
--
1.6.5.rc2
[Qemu-devel] Re: qemu: async sending in tap causes NFS not responding error
Scott Tsai scottt...@gmail.com writes: I recently found that this chageset: http://git.savannah.gnu.org/cgit/qemu.git/commit/?id=e19eb22486f258a421108ac22b8380a4e2f16b97 net: make use of async packet sending API in tap client causes NFS root Linux guest setups using TAP networking to fail with error messages like: nfs: server 172.20.0.1 not responding, still trying nfs: server 172.20.0.1 OK repeat infinitely ... This happens on both the master and stable-0.11 branches on qemu. The attached '0001-net-revert-e19eb22486f258a421108ac22b8380a4e2f16b97.patch' makes NFS root on qemu emulated arm-integrator-cp boards work for me again. After finding: http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg01173.html through Google and reading through the potentially bad commits that Sven found through bisection, I patched tap_send() to not run in a loop (drain the tap send queue in one go?) and the error goes away. Thank you for digging into this. I tested your patch in my environment and can confirm that it solves the problem. Sven
[Qemu-devel] [PATCH] scsi-disk: Inquiry with allocation length of CDB 36
According to SCSI-2 specification,
http://ldkelley.com/SCSI2/SCSI2/SCSI2/SCSI2-08.html#8.2.5 ,
if the allocation length of the command descriptor block (CDB) is too
small to transfer
all of the parameters, the additional length shall not be adjusted to
reflect the truncation.
The 36 mandatory bytes of response are written to outbuf, and then
only the length requested
in CDB is transferred.
Signed-off-by: Artyom Tarasenko atar4q...@gmail.com
---
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2a9268a..1a7487e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -5,6 +5,12 @@
* Based on code by Fabrice Bellard
*
* Written by Paul Brook
+ * Modifications:
+ * 2009-Oct-26 Artyom Tarasenko : implemented stamdard inquiry for the case
+ * when the allocation length of CDB is smaller
+ * than 36.
+ * 2009-Oct-13 Artyom Tarasenko : implemented the block descriptor in the
+ * MODE SENSE response.
*
* This code is licenced under the LGPL.
*
@@ -576,11 +582,6 @@ static int32_t scsi_send_command(SCSIDevice *d,
uint32_t tag,
is less than 5\n, len);
goto fail;
}
-
-if (len 36) {
-BADF(Error: Inquiry (STANDARD) buffer size %d
- is less than 36 (TODO: only 5 required)\n, len);
-}
}
if(len SCSI_MAX_INQUIRY_LEN)
@@ -604,7 +605,13 @@ static int32_t scsi_send_command(SCSIDevice *d,
uint32_t tag,
Some later commands are also implemented. */
outbuf[2] = 3;
outbuf[3] = 2; /* Format 2 */
- outbuf[4] = len - 5; /* Additional Length = (Len - 1) - 4 */
+ if (len 36) {
+ outbuf[4] = len - 5; /* Additional Length = (Len - 1) - 4 */
+ } else {
+ /* If the allocation length of CDB is too small, the
additional length
+ is not adjusted */
+ outbuf[4] = 36 - 5;
+ }
/* Sync data transfer and TCQ. */
outbuf[7] = 0x10 | (r-bus-tcq ? 0x02 : 0);
r-iov.iov_len = len;
Signed-off-by: Artyom Tarasenko atar4q...@gmail.com
---
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2a9268a..1a7487e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -5,6 +5,12 @@
* Based on code by Fabrice Bellard
*
* Written by Paul Brook
+ * Modifications:
+ * 2009-Oct-26 Artyom Tarasenko : implemented stamdard inquiry for the case
+ * when the allocation length of CDB is smaller
+ * than 36.
+ * 2009-Oct-13 Artyom Tarasenko : implemented the block descriptor in the
+ * MODE SENSE response.
*
* This code is licenced under the LGPL.
*
@@ -576,11 +582,6 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t
tag,
is less than 5\n, len);
goto fail;
}
-
-if (len 36) {
-BADF(Error: Inquiry (STANDARD) buffer size %d
- is less than 36 (TODO: only 5 required)\n, len);
-}
}
if(len SCSI_MAX_INQUIRY_LEN)
@@ -604,7 +605,13 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t
tag,
Some later commands are also implemented. */
outbuf[2] = 3;
outbuf[3] = 2; /* Format 2 */
- outbuf[4] = len - 5; /* Additional Length = (Len - 1) - 4 */
+ if (len 36) {
+ outbuf[4] = len - 5; /* Additional Length = (Len - 1) - 4 */
+ } else {
+ /* If the allocation length of CDB is too small, the additional
length
+ is not adjusted */
+ outbuf[4] = 36 - 5;
+ }
/* Sync data transfer and TCQ. */
outbuf[7] = 0x10 | (r-bus-tcq ? 0x02 : 0);
r-iov.iov_len = len;
RE: [Qemu-devel] net packet storms with multiple NICs
-Original Message- From: qemu-devel-bounces+chris.krumme=windriver@nongnu.org [mailto:qemu-devel-bounces+chris.krumme=windriver@nongnu.o rg] On Behalf Of Avi Kivity Sent: Sunday, October 25, 2009 9:23 AM To: Mark McLoughlin Cc: Michael Tokarev; qemu-devel@nongnu.org; KVM list Subject: Re: [Qemu-devel] net packet storms with multiple NICs On 10/23/2009 06:43 PM, Mark McLoughlin wrote: On Fri, 2009-10-23 at 20:25 +0400, Michael Tokarev wrote: I've two questions: o what's the intended usage of all-vlan-equal case, when kvm (or qemu) reflects packets from one interface to another? It's what bridge in linux is for, I think. I don't think it's necessarily an intended use-case for the vlan feature Well, it is. vlan=x really means the ethernet segment named x. If you connect all your guest nics to one vlan, you are connecting them all to one ethernet segment, so any packet transmitted on one will be reflected on others. Whether this is a useful feature is another matter, but the code is functioning as expected. Hello, We had one environment where the NIC understood by u-boot and the NIC understood by the kernel where different. We just attached both to the same VLAN. During u-boot one was used for downloading the kernel, then once the kernel booted the other was used. Not ideal, and maybe not important enough to keep the feature around, but it does get used now and again. Thanks Chris -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] net packet storms with multiple NICs
On 10/26/2009 03:40 PM, Krumme, Chris wrote: Well, it is. vlan=x really means the ethernet segment named x. If you connect all your guest nics to one vlan, you are connecting them all to one ethernet segment, so any packet transmitted on one will be reflected on others. Whether this is a useful feature is another matter, but the code is functioning as expected. Hello, We had one environment where the NIC understood by u-boot and the NIC understood by the kernel where different. We just attached both to the same VLAN. During u-boot one was used for downloading the kernel, then once the kernel booted the other was used. Not ideal, and maybe not important enough to keep the feature around, but it does get used now and again. You could get the same behaviour by using two different vlans connected to the same bridge. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH] new SDL keyboard shortcuts to start and stop VM
On 10/26/2009 03:45 PM, Anthony Liguori wrote: Avi Kivity wrote: On 10/23/2009 03:59 PM, Anthony Liguori wrote: Your answer may be, this is for a developer and they'll be aware of all the short comings/gotchas but this ends up being a rather user-hostile interface. People are never as aware of short comings/gotchas as we'd like them to be. If there was no other way for a developer to do this, I'd be more inclined to find a way to support this but it's just a matter of writing a script or if you really need a short cut, you can do it with standard gnome short cuts or write a very simple vnc client based on gvncviewer (we're talking a dozen lines of added code) to do this for you. vncviewer based displays may work now, but they are inefficient and will likely fall apart if/when we have 3D support. If it's chromium based (which I suspect it will be), you could certainly tunnel it via vnc. But you wouldn't want to. You'd probably get decent throughput at the expense of greater cpu consumption. Much better to have qemu talk to drm. I'd much rather see a real GUI client, perhaps implemented by scripting QObjects or QMP. I'm with you 100% here. I'd rather see our focus put into a proper gui based on QMP than to tack on features to SDL. Maybe slightly less than 100%. I meant a native GUI in the same process as qemu, but talking to QObjects through a scripting language. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH] qemu/msix: fix table access issues
Fixes a couple of issues with msix table access:
- With misbehaving guests, misaligned 4 byte access could overflow
msix table and cause qemu to segfault. Since PCI spec requires
host to only issue dword-aligned accesses, as a fix,
it's enough to mask the address low bits.
- Tables use pci format, not native format, and so
we must use pci_[sg]et_long on read/write.
Reported-by: Juan Quintela quint...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
Tested with x86 guest.
hw/msix.c | 11 ---
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/hw/msix.c b/hw/msix.c
index b0dea91..3d67c4e 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -128,13 +128,10 @@ void msix_write_config(PCIDevice *dev, uint32_t addr,
static uint32_t msix_mmio_readl(void *opaque, target_phys_addr_t addr)
{
PCIDevice *dev = opaque;
-unsigned int offset = addr (MSIX_PAGE_SIZE - 1);
+unsigned int offset = addr (MSIX_PAGE_SIZE - 1) ~0x3;
void *page = dev-msix_table_page;
-uint32_t val = 0;
-memcpy(val, (void *)((char *)page + offset), 4);
-
-return val;
+return pci_get_long(page + offset);
}
static uint32_t msix_mmio_read_unallowed(void *opaque, target_phys_addr_t addr)
@@ -178,9 +175,9 @@ static void msix_mmio_writel(void *opaque,
target_phys_addr_t addr,
uint32_t val)
{
PCIDevice *dev = opaque;
-unsigned int offset = addr (MSIX_PAGE_SIZE - 1);
+unsigned int offset = addr (MSIX_PAGE_SIZE - 1) ~0x3;
int vector = offset / MSIX_ENTRY_SIZE;
-memcpy(dev-msix_table_page + offset, val, 4);
+pci_set_long(dev-msix_table_page + offset, val);
if (!msix_is_masked(dev, vector) msix_is_pending(dev, vector)) {
msix_clr_pending(dev, vector);
msix_notify(dev, vector);
--
1.6.5.rc2
[Qemu-devel] [PATCH v2] Added readonly flag to -drive command
This is a slightly revised patch for adding readonly flag to the -drive command.
Even though this patch is stand-alone, it assumes a previous related patch
(in Anthony staging tree), that passes
the readonly attribute of the drive to the guest OS, applied first.
This enables sharing same image between guests, with readonly access.
Implementaion mark the drive as read_only and changes the flags when actually
opening the file.
The readonly attribute of a qcow also passed to it's base file.
For ide that cannot pass the readonly attribute to the guest OS, disallow the
readonly flag.
Also, return error code from bdrv_truncate for readonly drive.
Signed-off-by: Naphtali Sprei nsp...@redhat.com
---
block.c | 19 +++
block.h |1 +
qemu-config.c |3 +++
vl.c | 10 ++
4 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 33f3d65..cffd95e 100644
--- a/block.c
+++ b/block.c
@@ -331,11 +331,10 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags)
int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
BlockDriver *drv)
{
-int ret, open_flags;
+int ret, open_flags, try_rw;
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];
-bs-read_only = 0;
bs-is_temporary = 0;
bs-encrypted = 0;
bs-valid_key = 0;
@@ -422,9 +421,10 @@ int bdrv_open2(BlockDriverState *bs, const char *filename,
int flags,
/* Note: for compatibility, we open disk image files as RDWR, and
RDONLY as fallback */
+try_rw = !bs-read_only || bs-is_temporary;
if (!(flags BDRV_O_FILE))
-open_flags = BDRV_O_RDWR |
- (flags (BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
+open_flags = (try_rw ? BDRV_O_RDWR : 0) |
+(flags (BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
else
open_flags = flags ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
ret = drv-bdrv_open(bs, filename, open_flags);
@@ -453,6 +453,8 @@ int bdrv_open2(BlockDriverState *bs, const char *filename,
int flags,
/* if there is a backing file, use it */
BlockDriver *back_drv = NULL;
bs-backing_hd = bdrv_new();
+/* pass on read_only property to the backing_hd */
+bs-backing_hd-read_only = bs-read_only;
path_combine(backing_filename, sizeof(backing_filename),
filename, bs-backing_file);
if (bs-backing_format[0] != '\0')
@@ -731,6 +733,8 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
return -ENOMEDIUM;
if (!drv-bdrv_truncate)
return -ENOTSUP;
+if (bs-read_only)
+return -EACCES;
return drv-bdrv_truncate(bs, offset);
}
@@ -925,6 +929,13 @@ int bdrv_is_read_only(BlockDriverState *bs)
return bs-read_only;
}
+int bdrv_set_read_only(BlockDriverState *bs, int read_only)
+{
+int ret = bs-read_only;
+bs-read_only = read_only;
+return ret;
+}
+
int bdrv_is_sg(BlockDriverState *bs)
{
return bs-sg;
diff --git a/block.h b/block.h
index a966afb..302010d 100644
--- a/block.h
+++ b/block.h
@@ -136,6 +136,7 @@ int bdrv_get_type_hint(BlockDriverState *bs);
int bdrv_get_translation_hint(BlockDriverState *bs);
int bdrv_is_removable(BlockDriverState *bs);
int bdrv_is_read_only(BlockDriverState *bs);
+int bdrv_set_read_only(BlockDriverState *bs, int read_only);
int bdrv_is_sg(BlockDriverState *bs);
int bdrv_enable_write_cache(BlockDriverState *bs);
int bdrv_is_inserted(BlockDriverState *bs);
diff --git a/qemu-config.c b/qemu-config.c
index cae92f7..4ac8fa0 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -71,6 +71,9 @@ QemuOptsList qemu_drive_opts = {
.name = addr,
.type = QEMU_OPT_STRING,
.help = pci address (virtio only),
+},{
+.name = readonly,
+.type = QEMU_OPT_BOOL,
},
{ /* end if list */ }
},
diff --git a/vl.c b/vl.c
index eb2744e..97a940e 100644
--- a/vl.c
+++ b/vl.c
@@ -2007,6 +2007,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
int index;
int cache;
int aio = 0;
+int ro = 0;
int bdrv_flags, onerror;
const char *devaddr;
DriveInfo *dinfo;
@@ -2038,6 +2039,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
secs = qemu_opt_get_number(opts, secs, 0);
snapshot = qemu_opt_get_bool(opts, snapshot, 0);
+ro = qemu_opt_get_bool(opts, readonly, 0);
file = qemu_opt_get(opts, file);
serial = qemu_opt_get(opts, serial);
@@ -2329,6 +2331,14 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
bdrv_flags = ~BDRV_O_NATIVE_AIO;
}
+if (ro == 1) {
+if (type == IF_IDE) {
+fprintf(stderr, qemu: readonly flag not supported for drive with
ide interface\n);
+return NULL;
+}
+(void)bdrv_set_read_only(dinfo-bdrv, 1);
+}
+
if (bdrv_open2(dinfo-bdrv, file, bdrv_flags, drv) 0)
[Qemu-devel] [PATCH 4/7] usb: make attach optional.
Add a auto_attach field to USBDevice, which is enabled by default.
USB drivers can clear this field in case they do *not* want the device
being attached (i.e. plugged into a usb port) automatically after
successfull init().
Use cases (see next patches):
* attaching encrypted mass storage devices.
* -usbdevice host:...
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb-bus.c |3 ++-
hw/usb.h |1 +
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 28b517f..87dcc7f 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -45,8 +45,9 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base)
pstrcpy(dev-devname, sizeof(dev-devname), qdev-info-name);
dev-info = info;
+dev-auto_attach = 1;
rc = dev-info-init(dev);
-if (rc == 0)
+if (rc == 0 dev-auto_attach)
usb_device_attach(dev);
return rc;
}
diff --git a/hw/usb.h b/hw/usb.h
index a875d5b..a01f334 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -133,6 +133,7 @@ struct USBDevice {
int speed;
uint8_t addr;
char devname[32];
+int auto_attach;
int attached;
int state;
--
1.6.2.5
[Qemu-devel] [PATCH v2 0/7] use qdev for -usbdevice
Hi, This patch series changes the way the -usbdevice switch (and the usb_add monitor command) is handled. Instead of hard-coding stuff in vl.c it is integrated with qdev by adding new fields to USBDeviceInfo. First patch adds the infrastructure. Follwing patches switch over the usb drivers to the new way of handling things. Not converted (yet) are: * bt: bluetooth is not qdev-ified at all, needs investigation. * net: too many net patches in flight right now. New in v2: - also convert host usb pass through. - bugfix in auto attach opt-out patch. cheers, Gerd
[Qemu-devel] [PATCH 2/7] usb-hid: use qdev for -usbdevice
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb-hid.c |3 +++
hw/usb-wacom.c |1 +
vl.c | 29 -
3 files changed, 4 insertions(+), 29 deletions(-)
diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index d1cc45e..f4a2a48 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -882,6 +882,7 @@ static struct USBDeviceInfo hid_info[] = {
{
.qdev.name = QEMU USB Tablet,
.qdev.alias = usb-tablet,
+.usbdevice_name = tablet,
.qdev.size = sizeof(USBHIDState),
.init = usb_tablet_initfn,
.handle_packet = usb_generic_handle_packet,
@@ -892,6 +893,7 @@ static struct USBDeviceInfo hid_info[] = {
},{
.qdev.name = QEMU USB Mouse,
.qdev.alias = usb-mouse,
+.usbdevice_name = mouse,
.qdev.size = sizeof(USBHIDState),
.init = usb_mouse_initfn,
.handle_packet = usb_generic_handle_packet,
@@ -902,6 +904,7 @@ static struct USBDeviceInfo hid_info[] = {
},{
.qdev.name = QEMU USB Keyboard,
.qdev.alias = usb-kbd,
+.usbdevice_name = keyboard,
.qdev.size = sizeof(USBHIDState),
.init = usb_keyboard_initfn,
.handle_packet = usb_generic_handle_packet,
diff --git a/hw/usb-wacom.c b/hw/usb-wacom.c
index fa97db2..ef61376 100644
--- a/hw/usb-wacom.c
+++ b/hw/usb-wacom.c
@@ -411,6 +411,7 @@ static int usb_wacom_initfn(USBDevice *dev)
static struct USBDeviceInfo wacom_info = {
.qdev.name = QEMU PenPartner Tablet,
.qdev.alias = wacom-tablet,
+.usbdevice_name = wacom-tablet,
.qdev.size = sizeof(USBWacomState),
.init = usb_wacom_initfn,
.handle_packet = usb_generic_handle_packet,
diff --git a/vl.c b/vl.c
index 30b5306..52ab14f 100644
--- a/vl.c
+++ b/vl.c
@@ -2533,31 +2533,10 @@ static void usb_msd_password_cb(void *opaque, int err)
dev-info-handle_destroy(dev);
}
-static struct {
-const char *name;
-const char *qdev;
-} usbdevs[] = {
-{
-.name = mouse,
-.qdev = QEMU USB Mouse,
-},{
-.name = tablet,
-.qdev = QEMU USB Tablet,
-},{
-.name = keyboard,
-.qdev = QEMU USB Keyboard,
-},{
-.name = wacom-tablet,
-.qdev = QEMU PenPartner Tablet,
-}
-};
-
static int usb_device_add(const char *devname, int is_hotplug)
{
const char *p;
-USBBus *bus = usb_bus_find(-1 /* any */);
USBDevice *dev = NULL;
-int i;
if (!usb_enabled)
return -1;
@@ -2567,14 +2546,6 @@ static int usb_device_add(const char *devname, int
is_hotplug)
if (dev)
goto done;
-/* simple devices which don't need extra care */
-for (i = 0; i ARRAY_SIZE(usbdevs); i++) {
-if (strcmp(devname, usbdevs[i].name) != 0)
-continue;
-dev = usb_create_simple(bus, usbdevs[i].qdev);
-goto done;
-}
-
/* the other ones */
if (strstart(devname, host:, p)) {
dev = usb_host_device_open(p);
--
1.6.2.5
[Qemu-devel] [PATCH 1/7] usb core: use qdev for -usbdevice
This patchs adds infrastructure to handle -usbdevice via qdev callbacks.
USBDeviceInfo gets a name field (for the -usbdevice driver name) and a
callback for -usbdevice parameter parsing.
The new usbdevice_create() function walks the qdev driver list and looks
for a usb driver with a matching name. When a parameter parsing
callback is present it is called, otherwise the device is created via
usb_create_simple().
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/qdev.c|2 +-
hw/qdev.h|1 +
hw/usb-bus.c | 47 +++
hw/usb.h |5 +
vl.c |5 +
5 files changed, 59 insertions(+), 1 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 20f931c..b0d92d2 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -35,7 +35,7 @@ static int qdev_hotplug = 0;
/* This is a nasty hack to allow passing a NULL bus to qdev_create. */
static BusState *main_system_bus;
-static DeviceInfo *device_info_list;
+DeviceInfo *device_info_list;
static BusState *qbus_find_recursive(BusState *bus, const char *name,
const BusInfo *info);
diff --git a/hw/qdev.h b/hw/qdev.h
index b79f3e3..738470b 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -140,6 +140,7 @@ struct DeviceInfo {
BusInfo *bus_info;
struct DeviceInfo *next;
};
+extern DeviceInfo *device_info_list;
void qdev_register(DeviceInfo *info);
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 98987a1..28b517f 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -252,3 +252,50 @@ void usb_info(Monitor *mon)
}
}
+/* handle legacy -usbdevice cmd line option */
+USBDevice *usbdevice_create(const char *cmdline)
+{
+USBBus *bus = usb_bus_find(-1 /* any */);
+DeviceInfo *info;
+USBDeviceInfo *usb;
+char driver[32], *params;
+int len;
+
+params = strchr(cmdline,':');
+if (params) {
+params++;
+len = params - cmdline;
+if (len sizeof(driver))
+len = sizeof(driver);
+pstrcpy(driver, len, cmdline);
+} else {
+pstrcpy(driver, sizeof(driver), cmdline);
+}
+
+for (info = device_info_list; info != NULL; info = info-next) {
+if (info-bus_info != usb_bus_info)
+continue;
+usb = DO_UPCAST(USBDeviceInfo, qdev, info);
+if (usb-usbdevice_name == NULL)
+continue;
+if (strcmp(usb-usbdevice_name, driver) != 0)
+continue;
+break;
+}
+if (info == NULL) {
+#if 0
+/* no error because some drivers are not converted (yet) */
+qemu_error(usbdevice %s not found\n, driver);
+#endif
+return NULL;
+}
+
+if (!usb-usbdevice_init) {
+if (params) {
+qemu_error(usbdevice %s accepts no params\n, driver);
+return NULL;
+}
+return usb_create_simple(bus, usb-qdev.name);
+}
+return usb-usbdevice_init(params);
+}
diff --git a/hw/usb.h b/hw/usb.h
index be4fcf6..62362a7 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -183,6 +183,10 @@ struct USBDeviceInfo {
* Returns length or one of the USB_RET_ codes.
*/
int (*handle_data)(USBDevice *dev, USBPacket *p);
+
+/* handle legacy -usbdevice command line options */
+const char *usbdevice_name;
+USBDevice *(*usbdevice_init)(const char *params);
};
typedef void (*usb_attachfn)(USBPort *port, USBDevice *dev);
@@ -309,6 +313,7 @@ void usb_qdev_register(USBDeviceInfo *info);
void usb_qdev_register_many(USBDeviceInfo *info);
USBDevice *usb_create(USBBus *bus, const char *name);
USBDevice *usb_create_simple(USBBus *bus, const char *name);
+USBDevice *usbdevice_create(const char *cmdline);
void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
usb_attachfn attach);
void usb_unregister_port(USBBus *bus, USBPort *port);
diff --git a/vl.c b/vl.c
index eb2744e..30b5306 100644
--- a/vl.c
+++ b/vl.c
@@ -2562,6 +2562,11 @@ static int usb_device_add(const char *devname, int
is_hotplug)
if (!usb_enabled)
return -1;
+/* drivers with .usbdevice_name entry in USBDeviceInfo */
+dev = usbdevice_create(devname);
+if (dev)
+goto done;
+
/* simple devices which don't need extra care */
for (i = 0; i ARRAY_SIZE(usbdevs); i++) {
if (strcmp(devname, usbdevs[i].name) != 0)
--
1.6.2.5
[Qemu-devel] [PATCH 3/7] usb-serial and braille: use qdev for -usbdevice
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/baum.c |6
hw/baum.h |3 --
hw/usb-serial.c | 85 ++-
hw/usb.h|3 --
vl.c|6
5 files changed, 65 insertions(+), 38 deletions(-)
diff --git a/hw/baum.c b/hw/baum.c
index c66e737..8a12985 100644
--- a/hw/baum.c
+++ b/hw/baum.c
@@ -627,9 +627,3 @@ fail_handle:
free(baum);
return NULL;
}
-
-USBDevice *usb_baum_init(void)
-{
-/* USB Product ID of Super Vario 40 */
-return usb_serial_init(productid=FE72:braille);
-}
diff --git a/hw/baum.h b/hw/baum.h
index 39ca4b1..8af710f 100644
--- a/hw/baum.h
+++ b/hw/baum.h
@@ -22,8 +22,5 @@
* THE SOFTWARE.
*/
-/* usb device */
-USBDevice *usb_baum_init(void);
-
/* char device */
CharDriverState *chr_baum_init(QemuOpts *opts);
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index d02f6b2..223d4c3 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -90,8 +90,8 @@ do { printf(usb-serial: fmt , ## __VA_ARGS__); } while (0)
typedef struct {
USBDevice dev;
-uint16_t vendorid;
-uint16_t productid;
+uint32_t vendorid;
+uint32_t productid;
uint8_t recv_buf[RECV_BUF];
uint16_t recv_ptr;
uint16_t recv_used;
@@ -527,15 +527,18 @@ static int usb_serial_initfn(USBDevice *dev)
{
USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
s-dev.speed = USB_SPEED_FULL;
+
+qemu_chr_add_handlers(s-cs, usb_serial_can_read, usb_serial_read,
+ usb_serial_event, s);
+usb_serial_handle_reset(dev);
return 0;
}
-USBDevice *usb_serial_init(const char *filename)
+static USBDevice *usb_serial_init(const char *filename)
{
USBDevice *dev;
-USBSerialState *s;
CharDriverState *cdrv;
-unsigned short vendorid = 0x0403, productid = 0x6001;
+uint32_t vendorid = 0, productid = 0;
char label[32];
static int index;
@@ -545,26 +548,26 @@ USBDevice *usb_serial_init(const char *filename)
if (strstart(filename, vendorid=, p)) {
vendorid = strtol(p, e, 16);
if (e == p || (*e *e != ',' *e != ':')) {
-printf(bogus vendor ID %s\n, p);
+qemu_error(bogus vendor ID %s\n, p);
return NULL;
}
filename = e;
} else if (strstart(filename, productid=, p)) {
productid = strtol(p, e, 16);
if (e == p || (*e *e != ',' *e != ':')) {
-printf(bogus product ID %s\n, p);
+qemu_error(bogus product ID %s\n, p);
return NULL;
}
filename = e;
} else {
-printf(unrecognized serial USB option %s\n, filename);
+qemu_error(unrecognized serial USB option %s\n, filename);
return NULL;
}
while(*filename == ',')
filename++;
}
if (!*filename) {
-printf(character device specification needed\n);
+qemu_error(character device specification needed\n);
return NULL;
}
filename++;
@@ -574,23 +577,56 @@ USBDevice *usb_serial_init(const char *filename)
if (!cdrv)
return NULL;
-dev = usb_create_simple(NULL /* FIXME */, QEMU USB Serial);
-s = DO_UPCAST(USBSerialState, dev, dev);
-s-cs = cdrv;
-s-vendorid = vendorid;
-s-productid = productid;
-snprintf(s-dev.devname, sizeof(s-dev.devname), QEMU USB Serial(%.16s),
- filename);
+dev = usb_create(NULL /* FIXME */, QEMU USB Serial);
+qdev_prop_set_chr(dev-qdev, chardev, cdrv);
+if (vendorid)
+qdev_prop_set_uint16(dev-qdev, vendorid, vendorid);
+if (productid)
+qdev_prop_set_uint16(dev-qdev, productid, productid);
+qdev_init(dev-qdev);
-qemu_chr_add_handlers(cdrv, usb_serial_can_read, usb_serial_read,
- usb_serial_event, s);
+return dev;
+}
+
+static USBDevice *usb_braille_init(const char *unused)
+{
+USBDevice *dev;
+CharDriverState *cdrv;
+
+cdrv = qemu_chr_open(braille, braille, NULL);
+if (!cdrv)
+return NULL;
-usb_serial_handle_reset((USBDevice *)s);
-return (USBDevice *)s;
+dev = usb_create(NULL /* FIXME */, QEMU USB Braille);
+qdev_prop_set_chr(dev-qdev, chardev, cdrv);
+qdev_init(dev-qdev);
+
+return dev;
}
static struct USBDeviceInfo serial_info = {
.qdev.name = QEMU USB Serial,
+.qdev.alias = usb-serial,
+.qdev.size = sizeof(USBSerialState),
+.init = usb_serial_initfn,
+.handle_packet = usb_generic_handle_packet,
+.handle_reset = usb_serial_handle_reset,
+.handle_control = usb_serial_handle_control,
+.handle_data= usb_serial_handle_data,
+.handle_destroy = usb_serial_handle_destroy,
+.usbdevice_name = serial,
+.usbdevice_init = usb_serial_init,
+.qdev.props = (Property[]) {
+
[Qemu-devel] [PATCH 7/7] usb: print attached status in info qtree
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/usb-bus.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/usb-bus.c b/hw/usb-bus.c index 87dcc7f..99d185e 100644 --- a/hw/usb-bus.c +++ b/hw/usb-bus.c @@ -226,9 +226,10 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent) USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev); USBBus *bus = usb_bus_from_device(dev); -monitor_printf(mon, %*saddr %d.%d, speed %s, name %s\n, indent, , - bus-busnr, dev-addr, - usb_speed(dev-speed), dev-devname); +monitor_printf(mon, %*saddr %d.%d, speed %s, name %s%s\n, + indent, , bus-busnr, dev-addr, + usb_speed(dev-speed), dev-devname, + dev-attached ? , attached : ); } void usb_info(Monitor *mon) -- 1.6.2.5
[Qemu-devel] [PATCH 5/7] usb-storage: use qdev for -usbdevice
Hook up usb_msd_init.
Also rework handling of encrypted block devices,
move the code out vl.c.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb-msd.c | 33 +
hw/usb.h |4
vl.c | 25 -
3 files changed, 25 insertions(+), 37 deletions(-)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index dd3010e..8dc494f 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -14,6 +14,7 @@
#include block.h
#include scsi-disk.h
#include console.h
+#include monitor.h
//#define DEBUG_MSD
@@ -508,6 +509,16 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket
*p)
return ret;
}
+static void usb_msd_password_cb(void *opaque, int err)
+{
+MSDState *s = opaque;
+
+if (!err)
+usb_device_attach(s-dev);
+else
+qdev_unplug(s-dev.qdev);
+}
+
static int usb_msd_initfn(USBDevice *dev)
{
MSDState *s = DO_UPCAST(MSDState, dev, dev);
@@ -522,10 +533,21 @@ static int usb_msd_initfn(USBDevice *dev)
s-scsi_dev = scsi_bus_legacy_add_drive(s-bus, s-dinfo, 0);
s-bus.qbus.allow_hotplug = 0;
usb_msd_handle_reset(dev);
+
+if (bdrv_key_required(s-dinfo-bdrv)) {
+if (s-dev.qdev.hotplugged) {
+monitor_read_bdrv_key_start(cur_mon, s-dinfo-bdrv,
+usb_msd_password_cb, s);
+s-dev.auto_attach = 0;
+} else {
+autostart = 0;
+}
+}
+
return 0;
}
-USBDevice *usb_msd_init(const char *filename)
+static USBDevice *usb_msd_init(const char *filename)
{
static int nr=0;
char id[8];
@@ -577,13 +599,6 @@ USBDevice *usb_msd_init(const char *filename)
return dev;
}
-BlockDriverState *usb_msd_get_bdrv(USBDevice *dev)
-{
-MSDState *s = (MSDState *)dev;
-
-return s-dinfo-bdrv;
-}
-
static struct USBDeviceInfo msd_info = {
.qdev.name = QEMU USB MSD,
.qdev.alias = usb-storage,
@@ -593,6 +608,8 @@ static struct USBDeviceInfo msd_info = {
.handle_reset = usb_msd_handle_reset,
.handle_control = usb_msd_handle_control,
.handle_data= usb_msd_handle_data,
+.usbdevice_name = disk,
+.usbdevice_init = usb_msd_init,
.qdev.props = (Property[]) {
DEFINE_PROP_DRIVE(drive, MSDState, dinfo),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/usb.h b/hw/usb.h
index a01f334..351c466 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -256,10 +256,6 @@ void usb_host_info(Monitor *mon);
/* usb-hid.c */
void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *));
-/* usb-msd.c */
-USBDevice *usb_msd_init(const char *filename);
-BlockDriverState *usb_msd_get_bdrv(USBDevice *dev);
-
/* usb-net.c */
USBDevice *usb_net_init(NICInfo *nd);
diff --git a/vl.c b/vl.c
index 64761cf..994065c 100644
--- a/vl.c
+++ b/vl.c
@@ -2523,16 +2523,6 @@ static void smp_parse(const char *optarg)
/***/
/* USB devices */
-static void usb_msd_password_cb(void *opaque, int err)
-{
-USBDevice *dev = opaque;
-
-if (!err)
-usb_device_attach(dev);
-else
-dev-info-handle_destroy(dev);
-}
-
static int usb_device_add(const char *devname, int is_hotplug)
{
const char *p;
@@ -2549,21 +2539,6 @@ static int usb_device_add(const char *devname, int
is_hotplug)
/* the other ones */
if (strstart(devname, host:, p)) {
dev = usb_host_device_open(p);
-} else if (strstart(devname, disk:, p)) {
-BlockDriverState *bs;
-
-dev = usb_msd_init(p);
-if (!dev)
-return -1;
-bs = usb_msd_get_bdrv(dev);
-if (bdrv_key_required(bs)) {
-autostart = 0;
-if (is_hotplug) {
-monitor_read_bdrv_key_start(cur_mon, bs, usb_msd_password_cb,
-dev);
-return 0;
-}
-}
} else if (strstart(devname, net:, p)) {
QemuOpts *opts;
int idx;
--
1.6.2.5
[Qemu-devel] [PATCH 6/7] usb-host: use qdev for -usbdevice + rework.
Changes:
* We don't create/delete devices, we attach/detach them instead.
* The separate autofilter list is gone, we simply walk the list
of devices directly instead.
* Autofiltering is done unconditionally now. Non-auto device scan
code got dropped.
* Autofiltering turns off the timer if there is nothing to do, it
runs only in case there are unattached host devices.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
usb-linux.c | 410 +--
1 files changed, 144 insertions(+), 266 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 9e5d9c4..c1706a9 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -64,10 +64,8 @@ struct usb_ctrlrequest {
typedef int USBScanFunc(void *opaque, int bus_num, int addr, int class_id,
int vendor_id, int product_id,
const char *product_name, int speed);
-static int usb_host_find_device(int *pbus_num, int *paddr,
-char *product_name, int product_name_size,
-const char *devname);
-//#define DEBUG
+
+#define DEBUG
#ifdef DEBUG
#define dprintf printf
@@ -118,6 +116,13 @@ struct ctrl_struct {
uint8_t buffer[2048];
};
+struct USBAutoFilter {
+uint32_t bus_num;
+uint32_t addr;
+uint32_t vendor_id;
+uint32_t product_id;
+};
+
typedef struct USBHostDevice {
USBDevice dev;
int fd;
@@ -134,10 +139,17 @@ typedef struct USBHostDevice {
/* Host side address */
int bus_num;
int addr;
+struct USBAutoFilter match;
-struct USBHostDevice *next;
+QTAILQ_ENTRY(USBHostDevice) next;
} USBHostDevice;
+static QTAILQ_HEAD(, USBHostDevice) hostdevs =
QTAILQ_HEAD_INITIALIZER(hostdevs);
+
+static int usb_host_close(USBHostDevice *dev);
+static int parse_filter(const char *spec, struct USBAutoFilter *f);
+static void usb_host_auto_check(void *unused);
+
static int is_isoc(USBHostDevice *s, int ep)
{
return s-endp_table[ep - 1].type == USBDEVFS_URB_TYPE_ISO;
@@ -158,41 +170,6 @@ static void set_halt(USBHostDevice *s, int ep)
s-endp_table[ep - 1].halted = 1;
}
-static USBHostDevice *hostdev_list;
-
-static void hostdev_link(USBHostDevice *dev)
-{
-dev-next = hostdev_list;
-hostdev_list = dev;
-}
-
-static void hostdev_unlink(USBHostDevice *dev)
-{
-USBHostDevice *pdev = hostdev_list;
-USBHostDevice **prev = hostdev_list;
-
-while (pdev) {
- if (pdev == dev) {
-*prev = dev-next;
-return;
-}
-
-prev = pdev-next;
-pdev = pdev-next;
-}
-}
-
-static USBHostDevice *hostdev_find(int bus_num, int addr)
-{
-USBHostDevice *s = hostdev_list;
-while (s) {
-if (s-bus_num == bus_num s-addr == addr)
-return s;
-s = s-next;
-}
-return NULL;
-}
-
/*
* Async URB state.
* We always allocate one isoc descriptor even for bulk transfers
@@ -252,7 +229,8 @@ static void async_complete(void *opaque)
if (errno == ENODEV !s-closing) {
printf(husb: device %d.%d disconnected\n, s-bus_num,
s-addr);
- usb_device_delete_addr(s-bus_num, s-dev.addr);
+usb_host_close(s);
+usb_host_auto_check(NULL);
return;
}
@@ -405,7 +383,7 @@ static int usb_host_release_interfaces(USBHostDevice *s)
static void usb_host_handle_reset(USBDevice *dev)
{
-USBHostDevice *s = (USBHostDevice *) dev;
+USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev);
dprintf(husb: reset device %u.%u\n, s-bus_num, s-addr);
@@ -418,18 +396,8 @@ static void usb_host_handle_destroy(USBDevice *dev)
{
USBHostDevice *s = (USBHostDevice *)dev;
-s-closing = 1;
-
-qemu_set_fd_handler(s-fd, NULL, NULL, NULL);
-
-hostdev_unlink(s);
-
-async_complete(s);
-
-if (s-fd = 0)
-close(s-fd);
-
-qemu_free(s);
+usb_host_close(s);
+QTAILQ_REMOVE(hostdevs, s, next);
}
static int usb_linux_update_endp_table(USBHostDevice *s);
@@ -889,19 +857,16 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
return 0;
}
-static int usb_host_initfn(USBDevice *dev)
-{
-return 0;
-}
-
-static USBDevice *usb_host_device_open_addr(int bus_num, int addr, const char
*prod_name)
+static int usb_host_open(USBHostDevice *dev, int bus_num,
+ int addr, const char *prod_name)
{
int fd = -1, ret;
-USBDevice *d = NULL;
-USBHostDevice *dev;
struct usbdevfs_connectinfo ci;
char buf[1024];
+if (dev-fd != -1)
+goto fail;
+
printf(husb: open device %d.%d\n, bus_num, addr);
if (!usb_host_device_path) {
@@ -917,9 +882,6 @@ static USBDevice *usb_host_device_open_addr(int bus_num,
int addr, const char *p
}
dprintf(husb: opened %s\n, buf);
-d = usb_create(NULL /* FIXME */, USB Host Device);
-dev = DO_UPCAST(USBHostDevice, dev, d);
-
Re: [Qemu-devel] Re: [PATCH] new SDL keyboard shortcuts to start and stop VM
Avi Kivity wrote: I'd much rather see a real GUI client, perhaps implemented by scripting QObjects or QMP. I'm with you 100% here. I'd rather see our focus put into a proper gui based on QMP than to tack on features to SDL. Maybe slightly less than 100%. I meant a native GUI in the same process as qemu, but talking to QObjects through a scripting language. The trouble here is that if you want to support being able to close the gui and open it again without killing the guest, you need the guest to live in a separate process. The way VMware does it is by having a VNC extension that allows a shared memory transport which limits the CPU overhead. Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH] new SDL keyboard shortcuts to start and stop VM
On 10/26/2009 05:04 PM, Anthony Liguori wrote: Maybe slightly less than 100%. I meant a native GUI in the same process as qemu, but talking to QObjects through a scripting language. The trouble here is that if you want to support being able to close the gui and open it again without killing the guest, you need the guest to live in a separate process. Many applications minimize to the system tray without needing two processes. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH] serial: Support additional serial speed values
* Allow any speed value which is defined for Linux
(and possibly other systems).
* Compare int values instead of double values.
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
qemu-char.c | 87 +-
1 files changed, 61 insertions(+), 26 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 0fd402c..cb65469 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1018,33 +1018,68 @@ static void tty_serial_init(int fd, int speed,
#endif
tcgetattr (fd, tty);
-#define MARGIN 1.1
-if (speed = 50 * MARGIN)
-spd = B50;
-else if (speed = 75 * MARGIN)
-spd = B75;
-else if (speed = 300 * MARGIN)
-spd = B300;
-else if (speed = 600 * MARGIN)
-spd = B600;
-else if (speed = 1200 * MARGIN)
-spd = B1200;
-else if (speed = 2400 * MARGIN)
-spd = B2400;
-else if (speed = 4800 * MARGIN)
-spd = B4800;
-else if (speed = 9600 * MARGIN)
-spd = B9600;
-else if (speed = 19200 * MARGIN)
-spd = B19200;
-else if (speed = 38400 * MARGIN)
-spd = B38400;
-else if (speed = 57600 * MARGIN)
-spd = B57600;
-else if (speed = 115200 * MARGIN)
-spd = B115200;
-else
+#define check_speed(val) if (speed = val) { spd = B##val; break; }
+speed = speed * 10 / 11;
+do {
+check_speed(50);
+check_speed(75);
+check_speed(110);
+check_speed(134);
+check_speed(150);
+check_speed(200);
+check_speed(300);
+check_speed(600);
+check_speed(1200);
+check_speed(1800);
+check_speed(2400);
+check_speed(4800);
+check_speed(9600);
+check_speed(19200);
+check_speed(38400);
+/* Non-Posix values follow. They may be unsupported on some systems. */
+check_speed(57600);
+check_speed(115200);
+#ifdef B230400
+check_speed(230400);
+#endif
+#ifdef B460800
+check_speed(460800);
+#endif
+#ifdef B50
+check_speed(50);
+#endif
+#ifdef B576000
+check_speed(576000);
+#endif
+#ifdef B921600
+check_speed(921600);
+#endif
+#ifdef B100
+check_speed(100);
+#endif
+#ifdef B1152000
+check_speed(1152000);
+#endif
+#ifdef B150
+check_speed(150);
+#endif
+#ifdef B200
+check_speed(200);
+#endif
+#ifdef B250
+check_speed(250);
+#endif
+#ifdef B300
+check_speed(300);
+#endif
+#ifdef B350
+check_speed(350);
+#endif
+#ifdef B400
+check_speed(400);
+#endif
spd = B115200;
+} while (0);
cfsetispeed(tty, spd);
cfsetospeed(tty, spd);
--
1.5.6.5
[Qemu-devel] FAUmachine
Hi list, there's been a lot of discussion about abstraction of hardware devices and machine description. Last Saturday I've been running into some developers from the University of Erlangen (FAU) who work on an x86 emulator, used to simulate machine failures (memory bit flips, hard drive failures) [1]. The most interesting part of their project appears to be the hardware abstraction layer. So in the general discussion of creating a generic machine description, it might be a good idea to get them to contribute their experience to Qemu :-). Maybe we can learn about dead ends and unexplored alternatives from them. FAUmachine guys, it would be awesome if you could either point us to a thorough documentation of the device abstraction layers or give some overview in a reply to this email. Alex [1] http://www.faumachine.org
Re: [Qemu-devel] [PATCH] tcg, tci: Add TCG and interpreter for bytecode (virtual machine)
malc schrieb: On Sun, 11 Oct 2009, Stefan Weil wrote: Stuart Brady schrieb: On Mon, Sep 28, 2009 at 06:50:21PM +0200, Stefan Weil wrote: Please send patches / enhancements based on my latest version from git://repo.or.cz/qemu/ar7.git. Just bug reports for now, unfortunately... [..snip..] git://repo.or.cz/qemu/ar7.git contains the latest changes. If the maintainers want to integrate tci in the official qemu, I can prepare the patches needed to add bytecode generator and interpreter and tcg host support for any host. I've looked at the code, and the problem with helpers is essentially unsolved, if only things were as simple as casting things to `helper_function' and expecting it will work, not that it's _that_ much more complex, but still.. Hello Malc, I read your comments on the possible ABI problems. Nevertheless, as far as I could see all existing TCG hosts simply use fixed registers when calling helper functions. So the same approach should work for TCI, too. At least it is possible to run complete operating systems using this approach. What do you think would be needed to get a first stage of TCI integrated in QEMU master? Regards, Stefan
Re: [Qemu-devel] Re: [PATCH] new SDL keyboard shortcuts to start and stop VM
Avi Kivity wrote: On 10/26/2009 05:04 PM, Anthony Liguori wrote: Maybe slightly less than 100%. I meant a native GUI in the same process as qemu, but talking to QObjects through a scripting language. The trouble here is that if you want to support being able to close the gui and open it again without killing the guest, you need the guest to live in a separate process. Many applications minimize to the system tray without needing two processes. Minimizing or hiding the window are different use cases. Now, I'm not 100% convinced this use-case is absolutely required but historically, it's always come up in discussions of improving the qemu gui. Imagine the following: A user starts a VM at a physical box. Everythings fine but he wants to return to his workstation so he closes the window. He goes back to his workstation and connects to a VNC server (on a different X server). He wants to now bring up the guest's display. This cannot be achieved with a gui in the same process as qemu. Is it necessary to support? I don't know. I'd love to just replace the SDL display with GTK + Cairo. I'm even somewhat inclined to suggest linking to python so that the gui can be written in python... Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] tcg, tci: Add TCG and interpreter for bytecode (virtual machine)
On Mon, 26 Oct 2009, Stefan Weil wrote: malc schrieb: On Sun, 11 Oct 2009, Stefan Weil wrote: Stuart Brady schrieb: On Mon, Sep 28, 2009 at 06:50:21PM +0200, Stefan Weil wrote: Please send patches / enhancements based on my latest version from git://repo.or.cz/qemu/ar7.git. Just bug reports for now, unfortunately... [..snip..] git://repo.or.cz/qemu/ar7.git contains the latest changes. If the maintainers want to integrate tci in the official qemu, I can prepare the patches needed to add bytecode generator and interpreter and tcg host support for any host. I've looked at the code, and the problem with helpers is essentially unsolved, if only things were as simple as casting things to `helper_function' and expecting it will work, not that it's _that_ much more complex, but still.. Hello Malc, I read your comments on the possible ABI problems. Nevertheless, as far as I could see all existing TCG hosts simply use fixed registers when calling helper functions. The problem, as i explained earlier, is that TCI doesn't take types of the parameters into considerations and this will break SVR4 PPC ABI for instance, since long long arguments are aligned. Please read this for details: http://marc.info/?l=qemu-develm=125535217403861w=2 So the same approach should work for TCI, too. At least it is possible to run complete operating systems using this approach. What do you think would be needed to get a first stage of TCI integrated in QEMU master? Fixing the aforementioned problem first, then convincing someone to commit it. I'd guess that having qemu/tci run on something we do not have TCG support for would imrpove the odds. -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks
On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno aurel...@aurel32.net wrote:
Rationale: The following code is difficult to read, but allowed by the
current coding style.
Fully agree.
+Every control flow statement is followed by a new indented and braced
+block; even if the block contains just one statement. The opening brace
+is on the line that contains the control flow statement that introduces
+the new block; the closing brace is on the same line as the else keyword,
+or on a line by itself if there is no else keyword. Example:
I think an exception should be granted for else if case, otherwise
the style would require braces around if, like:
if (a == 5) {
printf(a was 5.\n);
} else {
if (a == 6) {
printf(a was 6.\n);
}
} else {
printf(a was something else entirely.\n);
}
Picking nits: while is a control flow statement, even in do {}
while statement and then it would illegal to require a braced block
after the while statement.
Re: [Qemu-devel] Re: [PATCH] new SDL keyboard shortcuts to start and stop VM
On 10/26/2009 05:49 PM, Anthony Liguori wrote: Many applications minimize to the system tray without needing two processes. Minimizing or hiding the window are different use cases. Now, I'm not 100% convinced this use-case is absolutely required but historically, it's always come up in discussions of improving the qemu gui. Imagine the following: A user starts a VM at a physical box. Everythings fine but he wants to return to his workstation so he closes the window. He goes back to his workstation and connects to a VNC server (on a different X server). He wants to now bring up the guest's display. Users don't have boxes. They have computers. They don't want to open VNC clients and type in meaningless numerical addresses. They do want GUIs which fit with the OSes theme, cut'n'paste, printing, and shared storage, all easily configurable. (and before someone tells me I don't know what users want - users don't read qemu-devel, either). This cannot be achieved with a gui in the same process as qemu. Is it necessary to support? I don't know. In the priority list this is about 3000 places below having nice buttons to eject and insert a CDROM. A user with a box would probably want to run the guest on a server (and use vnc, etc.). I'd love to just replace the SDL display with GTK + Cairo. I'm even somewhat inclined to suggest linking to python so that the gui can be written in python... Best would be to just export a QObject binding to scripting languages, which could then be used to implement GUIs outside the qemu source base. The only tricky part is how to deal with the display. Can we expose the display as a special QDict? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [RFC] in-kernel irqchip : split devices
On Sun, Oct 25, 2009 at 12:26:51PM +0200, Avi Kivity wrote: On 10/14/2009 04:30 PM, Glauber Costa wrote: Hello people, As I promised, I am sending a very brief PoC wrt split devices and in-kernel irqchip. In this mail, I am including only the ioapic version for apreciation. I also have i8259, and apic will take me a little bit more. This is just to try to bind the discussion to real code. I still can't say I like it. The reset function is duplicated, the state representation (which is an ABI) is gratuitously forked. You can't save/restore in-kernel irqchip and userspace irqchip, even though where the code is located is an implementation detail. While we may not care much for the ioapic, it sets a bad precedent for vhost-net, where we'd like to migrate from non-vhost-net hosts to vhost-net hosts without the user noticing anything. Note that we end up with a very slim representation of the device, and the code is much less confusing, IMHO. You can always remove if statements by duplicating the code and pushing the if one level upwards. In total, there is more code, and it is more confusing (since you need to deal with implementation details at a higher level). It pretty much depends on your definition of confusing. Larger? yes, probably. It has a separate file, and doesn't matter how hard we fight duplicates, some will persist. But just the other day, I was on IRC with anthony, trying to draw some conclusions about the behaviour of our ioapic. We went through it, and it took us quite a while to determine what pieces of code were being used, and what were not. This is pretty much what I mean by confusing. With the approach we are proposing, things get much more straightforward -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [RFC] in-kernel irqchip : split devices
Avi Kivity wrote: On 10/14/2009 04:30 PM, Glauber Costa wrote: Hello people, As I promised, I am sending a very brief PoC wrt split devices and in-kernel irqchip. In this mail, I am including only the ioapic version for apreciation. I also have i8259, and apic will take me a little bit more. This is just to try to bind the discussion to real code. I still can't say I like it. The reset function is duplicated, the state representation (which is an ABI) is gratuitously forked. You can't save/restore in-kernel irqchip and userspace irqchip, even though where the code is located is an implementation detail. While we may not care much for the ioapic, it sets a bad precedent for vhost-net, where we'd like to migrate from non-vhost-net hosts to vhost-net hosts without the user noticing anything. Note that we end up with a very slim representation of the device, and the code is much less confusing, IMHO. You can always remove if statements by duplicating the code and pushing the if one level upwards. In total, there is more code, and it is more confusing (since you need to deal with implementation details at a higher level). I'm surprised you feel this way. Maybe this is an issue of having the model in your head vs. not having it because the current in-kernel code is extremely confusing IMHO. When you look at ioapic.c in qemu-kvm, the first question I ask is, what parts of this code is used when using in-kernel apic?. The answer is not at all obvious. To understand it, you have to first search for kvm_enabled() and you'll see that during save/restore the state is synced with the in-kernel state. However, it's not clear whether pio/mmio operations still get processed and certainly not clear whether ioapic_set_irq() is not called anymore. In fact, I think you to start with the assumption that it is which leads you to wonder why it doesn't do kvm_set_irq(). The answers are all subtle and have to do with weird things about how the isa irqs are allocated. It's extremely confusing to someone who doesn't know exactly what's going on. OTOH, the split model makes this all very obvious. Sure there's some duplication but at the end of the day, you don't have to understand very much to see what's going on. We just use userspace for device save/restore and reset support. Code readability wins in my mind over reducing a couple dozen lines of code. Regards, Anthony Liguori
[Qemu-devel] [PATCH] Use msr list to load and save msrs
Since there is an ioctl that tells us which msrs are available,
use it. This saves us from the need of functions like has_star(),
lm_capable(), etc.
Signed-off-by: Glauber Costa glom...@redhat.com
CC: Marcelo Tosatti mtosa...@redhat.com
CC: Avi Kivity a...@redhat.com
---
target-i386/kvm.c | 200 ++---
1 files changed, 98 insertions(+), 102 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7010999..488a3f5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -221,49 +221,41 @@ int kvm_arch_init_vcpu(CPUState *env)
return kvm_vcpu_ioctl(env, KVM_SET_CPUID2, cpuid_data);
}
-static int kvm_has_msr_star(CPUState *env)
+static struct kvm_msr_list *kvm_get_msr_list(CPUState *env)
{
-static int has_msr_star;
+struct kvm_msr_list msr_list;
+static struct kvm_msr_list *kvm_msr_list;
int ret;
-/* first time */
-if (has_msr_star == 0) {
-struct kvm_msr_list msr_list, *kvm_msr_list;
+if (kvm_msr_list)
+return kvm_msr_list;
-has_msr_star = -1;
+/* Obtain MSR list from KVM. These are the MSRs that we must
+ * save/restore */
+msr_list.nmsrs = 0;
+ret = kvm_ioctl(env-kvm_state, KVM_GET_MSR_INDEX_LIST, msr_list);
+if ((ret 0) (ret != -E2BIG)) {
+printf(FAIL1 %d\n, ret);
+return NULL;
+}
-/* Obtain MSR list from KVM. These are the MSRs that we must
- * save/restore */
-msr_list.nmsrs = 0;
-ret = kvm_ioctl(env-kvm_state, KVM_GET_MSR_INDEX_LIST, msr_list);
-if (ret 0)
-return 0;
-
-/* Old kernel modules had a bug and could write beyond the provided
- memory. Allocate at least a safe amount of 1K. */
-kvm_msr_list = qemu_mallocz(MAX(1024, sizeof(msr_list) +
- msr_list.nmsrs *
- sizeof(msr_list.indices[0])));
-
-kvm_msr_list-nmsrs = msr_list.nmsrs;
-ret = kvm_ioctl(env-kvm_state, KVM_GET_MSR_INDEX_LIST, kvm_msr_list);
-if (ret = 0) {
-int i;
-
-for (i = 0; i kvm_msr_list-nmsrs; i++) {
-if (kvm_msr_list-indices[i] == MSR_STAR) {
-has_msr_star = 1;
-break;
-}
-}
-}
+/* Old kernel modules had a bug and could write beyond the provided
+ memory. Allocate at least a safe amount of 1K. */
+kvm_msr_list = qemu_mallocz(MAX(1024, sizeof(msr_list) +
+ msr_list.nmsrs *
+ sizeof(msr_list.indices[0])));
+
+kvm_msr_list-nmsrs = msr_list.nmsrs;
+ret = kvm_ioctl(env-kvm_state, KVM_GET_MSR_INDEX_LIST, kvm_msr_list);
-free(kvm_msr_list);
+if (ret 0) {
+printf(FAIL2\n);
+qemu_free(kvm_msr_list);
+return NULL;
}
-if (has_msr_star == 1)
-return 1;
-return 0;
+return kvm_msr_list;
+
}
int kvm_arch_init(KVMState *s, int smp_cpus)
@@ -455,13 +447,47 @@ static int kvm_put_sregs(CPUState *env)
return kvm_vcpu_ioctl(env, KVM_SET_SREGS, sregs);
}
-static void kvm_msr_entry_set(struct kvm_msr_entry *entry,
- uint32_t index, uint64_t value)
+static uint64_t *kvm_get_msr_data_addr(CPUState *env, int index)
{
-entry-index = index;
-entry-data = value;
+uint64_t *addr;
+
+switch (index) {
+case MSR_IA32_SYSENTER_CS:
+addr = (uint64_t *)env-sysenter_cs;
+break;
+case MSR_IA32_SYSENTER_ESP:
+addr = (uint64_t *)env-sysenter_esp;
+break;
+case MSR_IA32_SYSENTER_EIP:
+addr = (uint64_t *)env-sysenter_eip;
+break;
+case MSR_STAR:
+addr = env-star;
+break;
+#ifdef TARGET_X86_64
+case MSR_CSTAR:
+addr = env-cstar;
+break;
+case MSR_KERNELGSBASE:
+addr = env-kernelgsbase;
+break;
+case MSR_FMASK:
+addr = env-fmask;
+break;
+case MSR_LSTAR:
+addr = env-lstar;
+break;
+#endif
+case MSR_IA32_TSC:
+addr = env-tsc;
+break;
+default:
+addr = NULL;
+}
+return addr;
}
+
static int kvm_put_msrs(CPUState *env)
{
struct {
@@ -469,22 +495,24 @@ static int kvm_put_msrs(CPUState *env)
struct kvm_msr_entry entries[100];
} msr_data;
struct kvm_msr_entry *msrs = msr_data.entries;
-int n = 0;
-
-kvm_msr_entry_set(msrs[n++], MSR_IA32_SYSENTER_CS, env-sysenter_cs);
-kvm_msr_entry_set(msrs[n++], MSR_IA32_SYSENTER_ESP, env-sysenter_esp);
-kvm_msr_entry_set(msrs[n++], MSR_IA32_SYSENTER_EIP, env-sysenter_eip);
-if (kvm_has_msr_star(env))
- kvm_msr_entry_set(msrs[n++], MSR_STAR, env-star);
-kvm_msr_entry_set(msrs[n++], MSR_IA32_TSC, env-tsc);
-#ifdef TARGET_X86_64
-/* FIXME if lm capable */
-
[Qemu-devel] Re: qemu-kvm: sigsegv at exit
On Thu, Oct 22, 2009 at 06:57:27PM -0200, Marcelo Tosatti wrote:
On Thu, Oct 22, 2009 at 02:00:15PM +0200, Michael S. Tsirkin wrote:
Hi!
I'm sometimes getting segfaults when I kill qemu.
This time I caught it when qemu was under gdb:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x411d0940 (LWP 14446)]
0x0040afb4 in qemu_mod_timer (ts=0x19f0fd0,
expire_time=62275467335)
at /home/mst/scm/qemu-kvm/vl.c:1009
1009if ((alarm_timer-flags ALARM_FLAG_EXPIRED) == 0) {
(gdb) l
1004ts-next = *pt;
1005*pt = ts;
1006
1007/* Rearm if necessary */
1008if (pt == active_timers[ts-clock-type]) {
1009if ((alarm_timer-flags ALARM_FLAG_EXPIRED) == 0) {
1010qemu_rearm_alarm_timer(alarm_timer);
1011}
1012/* Interrupt execution to force deadline recalculation.
*/
1013if (use_icount)
(gdb) p alarm_timer
$1 = (struct qemu_alarm_timer *) 0x0
(gdb) where
#0 0x0040afb4 in qemu_mod_timer (ts=0x19f0fd0,
expire_time=62275467335)
at /home/mst/scm/qemu-kvm/vl.c:1009
#1 0x0041aadf in virtio_net_handle_tx (vdev=value optimized
out, vq=0x19f5af0)
at /home/mst/scm/qemu-kvm/hw/virtio-net.c:696
#2 0x00421669 in kvm_run (vcpu=0x19d46a0, env=0x19c2250) at
/home/mst/scm/qemu-kvm/qemu-kvm.c:797
#3 0x004216d6 in kvm_cpu_exec (env=0x83d0f8) at
/home/mst/scm/qemu-kvm/qemu-kvm.c:1714
#4 0x00422981 in ap_main_loop (_env=value optimized out) at
/home/mst/scm/qemu-kvm/qemu-kvm.c:1969
#5 0x00377dc06367 in start_thread () from /lib64/libpthread.so.0
#6 0x00377d0d30ad in clone () from /lib64/libc.so.6
(gdb)
So this probably means that we have already run quit_timers:
static void quit_timers(void)
{
alarm_timer-stop(alarm_timer);
alarm_timer = NULL;
}
but kvm vcpu thread is still running.
Not sure what the right fix is here: should we stop
kvm after main loop has exited?
kvm_main_loop_wait(env, 0) can process the stop request (signalling
iothread that vcpu is stopped, so its OK to exit) and continue to
kvm_cpu_exec.
Can you please try this:
I applied this, and have not yet see any segfaults at exit.
Not sure whether this is means anything as the crash is not
100% reproducable. Push it out to Anthony and we'll see, long term?
Based on the knowledge of how to fix this,
how would you go about reproducing it?
Add code to trigger the race manually, but i'm pretty sure thats it.
Thanks for testing.
[Qemu-devel] Re: qemu-kvm: sigsegv at exit
On Mon, Oct 26, 2009 at 04:43:11PM -0200, Marcelo Tosatti wrote:
On Thu, Oct 22, 2009 at 06:57:27PM -0200, Marcelo Tosatti wrote:
On Thu, Oct 22, 2009 at 02:00:15PM +0200, Michael S. Tsirkin wrote:
Hi!
I'm sometimes getting segfaults when I kill qemu.
This time I caught it when qemu was under gdb:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x411d0940 (LWP 14446)]
0x0040afb4 in qemu_mod_timer (ts=0x19f0fd0,
expire_time=62275467335)
at /home/mst/scm/qemu-kvm/vl.c:1009
1009if ((alarm_timer-flags ALARM_FLAG_EXPIRED) == 0) {
(gdb) l
1004ts-next = *pt;
1005*pt = ts;
1006
1007/* Rearm if necessary */
1008if (pt == active_timers[ts-clock-type]) {
1009if ((alarm_timer-flags ALARM_FLAG_EXPIRED) == 0) {
1010qemu_rearm_alarm_timer(alarm_timer);
1011}
1012/* Interrupt execution to force deadline recalculation.
*/
1013if (use_icount)
(gdb) p alarm_timer
$1 = (struct qemu_alarm_timer *) 0x0
(gdb) where
#0 0x0040afb4 in qemu_mod_timer (ts=0x19f0fd0,
expire_time=62275467335)
at /home/mst/scm/qemu-kvm/vl.c:1009
#1 0x0041aadf in virtio_net_handle_tx (vdev=value optimized
out, vq=0x19f5af0)
at /home/mst/scm/qemu-kvm/hw/virtio-net.c:696
#2 0x00421669 in kvm_run (vcpu=0x19d46a0, env=0x19c2250) at
/home/mst/scm/qemu-kvm/qemu-kvm.c:797
#3 0x004216d6 in kvm_cpu_exec (env=0x83d0f8) at
/home/mst/scm/qemu-kvm/qemu-kvm.c:1714
#4 0x00422981 in ap_main_loop (_env=value optimized out) at
/home/mst/scm/qemu-kvm/qemu-kvm.c:1969
#5 0x00377dc06367 in start_thread () from /lib64/libpthread.so.0
#6 0x00377d0d30ad in clone () from /lib64/libc.so.6
(gdb)
So this probably means that we have already run quit_timers:
static void quit_timers(void)
{
alarm_timer-stop(alarm_timer);
alarm_timer = NULL;
}
but kvm vcpu thread is still running.
Not sure what the right fix is here: should we stop
kvm after main loop has exited?
kvm_main_loop_wait(env, 0) can process the stop request (signalling
iothread that vcpu is stopped, so its OK to exit) and continue to
kvm_cpu_exec.
Can you please try this:
I applied this, and have not yet see any segfaults at exit.
Not sure whether this is means anything as the crash is not
100% reproducable. Push it out to Anthony and we'll see, long term?
Based on the knowledge of how to fix this,
how would you go about reproducing it?
Add code to trigger the race manually,
If you like, send a patch adding such code, I will test.
but i'm pretty sure thats it.
Thanks for testing.
Re: [Qemu-devel] Re: [PATCH] tcg, tci: Add TCG and interpreter for bytecode (virtual machine)
On Sat, Oct 24, 2009 at 11:23:43AM +0800, TeLeMan wrote: On Sat, Oct 24, 2009 at 02:58, Stefan Weil w...@mail.berlios.de wrote: Is patch 4 (call handling) needed, or is it an optimization? If it is needed, the tcg disassembler has to be extended as well. In fact tci has no stack and robber registers and doesn't need simulate the CPU work. I am trying to remove tcg_reg_alloc() in tcg_reg_alloc_op() tcg_reg_alloc_call() and access the temporary variables directly in tci. 'Doesn't need' doesn't necessarily mean 'is better without', though. Perhaps it's best for TCI to reflect the behaviour of other TCG targets where possible? (You can then compare the code that is generated with different numbers of registers, and different constraints, etc.) Cheers, -- Stuart Brady
Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks
On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno aurel...@aurel32.net wrote:
Rationale: The following code is difficult to read, but allowed by the
current coding style.
Fully agree.
+Every control flow statement is followed by a new indented and braced
+block; even if the block contains just one statement. The opening brace
+is on the line that contains the control flow statement that introduces
+the new block; the closing brace is on the same line as the else keyword,
+or on a line by itself if there is no else keyword. Example:
I think an exception should be granted for else if case, otherwise
the style would require braces around if, like:
if (a == 5) {
printf(a was 5.\n);
} else {
if (a == 6) {
printf(a was 6.\n);
}
} else {
printf(a was something else entirely.\n);
}
Picking nits: while is a control flow statement, even in do {}
while statement and then it would illegal to require a braced block
after the while statement.
Good point. Please find another try below:
From: Aurelien Jarno aurel...@aurel32.net
Rationale: The following code is difficult to read:
if (a == 5) printf(a was 5.\n);
else if (a == 6) printf(a was 6.\n);
else printf(a was something else entirely.\n);
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
CODING_STYLE | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/CODING_STYLE b/CODING_STYLE
index a579cb1..c17c3f3 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -51,11 +51,13 @@ QEMU coding style.
4. Block structure
-Every indented statement is braced; even if the block contains just one
-statement. The opening brace is on the line that contains the control
-flow statement that introduces the new block; the closing brace is on the
-same line as the else keyword, or on a line by itself if there is no else
-keyword. Example:
+Every control flow statement is followed by a new indented and braced
+block, except if it is followed by another control flow statement (else
+if) or by a condition (do {} while ()); even if the block contains just
+one statement. The opening brace is on the line that contains the
+control flow statement that introduces the new block; the closing
+brace is on the same line as the else keyword, or on a line by itself
+if there is no else keyword. Example:
if (a == 5) {
printf(a was 5.\n);
--
1.6.1.3
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks
On Mon, Oct 26, 2009 at 10:03 PM, Aurelien Jarno aurel...@aurel32.net wrote:
On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno aurel...@aurel32.net wrote:
Rationale: The following code is difficult to read, but allowed by the
current coding style.
Fully agree.
+Every control flow statement is followed by a new indented and braced
+block; even if the block contains just one statement. The opening brace
+is on the line that contains the control flow statement that introduces
+the new block; the closing brace is on the same line as the else keyword,
+or on a line by itself if there is no else keyword. Example:
I think an exception should be granted for else if case, otherwise
the style would require braces around if, like:
if (a == 5) {
printf(a was 5.\n);
} else {
if (a == 6) {
printf(a was 6.\n);
}
} else {
printf(a was something else entirely.\n);
}
Picking nits: while is a control flow statement, even in do {}
while statement and then it would illegal to require a braced block
after the while statement.
Good point. Please find another try below:
From: Aurelien Jarno aurel...@aurel32.net
Rationale: The following code is difficult to read:
if (a == 5) printf(a was 5.\n);
else if (a == 6) printf(a was 6.\n);
else printf(a was something else entirely.\n);
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
CODING_STYLE | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/CODING_STYLE b/CODING_STYLE
index a579cb1..c17c3f3 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -51,11 +51,13 @@ QEMU coding style.
4. Block structure
-Every indented statement is braced; even if the block contains just one
-statement. The opening brace is on the line that contains the control
-flow statement that introduces the new block; the closing brace is on the
-same line as the else keyword, or on a line by itself if there is no else
-keyword. Example:
+Every control flow statement is followed by a new indented and braced
+block, except if it is followed by another control flow statement (else
+if) or by a condition (do {} while ()); even if the block contains just
+one statement. The opening brace is on the line that contains the
+control flow statement that introduces the new block; the closing
+brace is on the same line as the else keyword, or on a line by itself
+if there is no else keyword. Example:
Nice try, but does it prevent this:
if (x) for (;;) do {
} while (0);
?
Maybe also break and goto can be considered control flow statements.
How about something like wherever C syntax allows potentially
ambiguous sequence of statements, braces must be used, with the
exception of 'else' followed by 'if'? Now the problem becomes
defining ambiguous sequences of statements.
Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened
Amit Shah wrote: On (Mon) Oct 26 2009 [08:40:12], Jan Kiszka wrote: Amit Shah wrote: On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote: Amit Shah wrote: The OPENED event gets sent also when qemu resets its state initially. The consumers of the event aren't interested in receiving this event on reset. The monitor was. Now its initial prompt on activation is broken. The patch in Anthony's queue, titled 'console: call qemu_chr_reset() in text_console_init' You may also want to rename qemu_chr_reset - unless there is still a need for real reset. Yeah; there isn't a need after my patches -- I've been slowing working towards renaming it all. fixed that. However, with the qcow2 synchronous patch, the monitor prompt doesn't come up again -- which shows there is a problem with the way the bhs work and also the initial resets. Then the qcow2 patch is already in? At least applying your patch doesn't change the picture. Yeah; that patch went in just before my patches. Can you try reverting ef845c3bf421290153154635dc18eaa677cecb43 Makes no difference either - but it also does not touch any code that code impact the open/reset signaling. I think the initial resets are a hack to work around something from my reading of it; do you have a better idea of why it's there and how it's all supposed to work? From the monitor's POV, it's not a hack, it's simply the requirement to receive an indication that the console was opened. Just an indication that the monitor was opened -- agreed. But git history shows you added that as 'reset', so I'm wondering if maybe you wanted it to do something else as well (or you did it that way just because of the way qemu's bhs are handled?). I didn't add the reset hook for the monitor (it was Anthony), I just made some improvements. Does this patch fix/improve something for a different user? If not, please let us revert it. There's another question too: is a separate 'reset' event needed in addition to an 'opened' event? Not for the monitor, but I cannot speak for other users. I think it would be good to check them in details before changing the reset/open semantic. As far as I could see in the git history, the 'reset' was added for the monitor. And the others could live with the double 'reset' events they were getting -- one for the reset and one when the device was opened. OK. However, the problems of your approach to avoid potential double resets on startup is that the supposed two events for the monitor (first one from qemu_chr_open_fs, second one via qemu_chr_initial_reset) actually coalesce into one, thus are never delivered. So whatever may happen later on, during startup the skipping of the first reset/open event is bogus. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks
On Mon, Oct 26, 2009 at 10:20:34PM +0200, Blue Swirl wrote:
On Mon, Oct 26, 2009 at 10:03 PM, Aurelien Jarno aurel...@aurel32.net wrote:
On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno aurel...@aurel32.net
wrote:
Rationale: The following code is difficult to read, but allowed by the
current coding style.
Fully agree.
+Every control flow statement is followed by a new indented and braced
+block; even if the block contains just one statement. The opening brace
+is on the line that contains the control flow statement that introduces
+the new block; the closing brace is on the same line as the else
keyword,
+or on a line by itself if there is no else keyword. Example:
I think an exception should be granted for else if case, otherwise
the style would require braces around if, like:
if (a == 5) {
printf(a was 5.\n);
} else {
if (a == 6) {
printf(a was 6.\n);
}
} else {
printf(a was something else entirely.\n);
}
Picking nits: while is a control flow statement, even in do {}
while statement and then it would illegal to require a braced block
after the while statement.
Good point. Please find another try below:
From: Aurelien Jarno aurel...@aurel32.net
Rationale: The following code is difficult to read:
if (a == 5) printf(a was 5.\n);
else if (a == 6) printf(a was 6.\n);
else printf(a was something else entirely.\n);
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
CODING_STYLE | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/CODING_STYLE b/CODING_STYLE
index a579cb1..c17c3f3 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -51,11 +51,13 @@ QEMU coding style.
4. Block structure
-Every indented statement is braced; even if the block contains just one
-statement. The opening brace is on the line that contains the control
-flow statement that introduces the new block; the closing brace is on the
-same line as the else keyword, or on a line by itself if there is no else
-keyword. Example:
+Every control flow statement is followed by a new indented and braced
+block, except if it is followed by another control flow statement (else
+if) or by a condition (do {} while ()); even if the block contains just
+one statement. The opening brace is on the line that contains the
+control flow statement that introduces the new block; the closing
+brace is on the same line as the else keyword, or on a line by itself
+if there is no else keyword. Example:
Nice try, but does it prevent this:
if (x) for (;;) do {
} while (0);
?
We should find a way to define for (;;) as a single statement.
Maybe also break and goto can be considered control flow statements.
Correct, they should be excluded from there as they don't need to be
followed by braces.
How about something like wherever C syntax allows potentially
ambiguous sequence of statements, braces must be used, with the
exception of 'else' followed by 'if'? Now the problem becomes
defining ambiguous sequences of statements.
That's the problem. We have seen that people already take advantage of
the ambiguities, as I would have never have imagined someone writing the
code in the rationale of this patch to avoid putting braces.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] [PATCH] serial: Add missing bit
Serial frames always start with a start bit.
This bit was missing in frame size calculation.
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
hw/serial.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/serial.c b/hw/serial.c
index eb14f11..7552fee 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -231,15 +231,17 @@ static void serial_update_parameters(SerialState *s)
if (s-divider == 0)
return;
+/* Start bit. */
frame_size = 1;
if (s-lcr 0x08) {
+/* Parity bit. */
+frame_size++;
if (s-lcr 0x10)
parity = 'E';
else
parity = 'O';
} else {
parity = 'N';
-frame_size = 0;
}
if (s-lcr 0x04)
stop_bits = 2;
@@ -696,12 +698,12 @@ static void serial_reset(void *opaque)
s-lcr = 0;
s-lsr = UART_LSR_TEMT | UART_LSR_THRE;
s-msr = UART_MSR_DCD | UART_MSR_DSR | UART_MSR_CTS;
-/* Default to 9600 baud, no parity, one stop bit */
+/* Default to 9600 baud, 1 start bit, 8 data bits, 1 stop bit, no parity.
*/
s-divider = 0x0C;
s-mcr = UART_MCR_OUT2;
s-scr = 0;
s-tsr_retry = 0;
-s-char_transmit_time = (get_ticks_per_sec() / 9600) * 9;
+s-char_transmit_time = (get_ticks_per_sec() / 9600) * 10;
s-poll_msl = 0;
fifo_clear(s,RECV_FIFO);
--
1.5.6.5
Re: [Qemu-devel] [PATCH v2 09/10] target-arm: optimize neon vld/vst ops
On Mon, Oct 26, 2009 at 10:11:07AM +0100, Laurent Desnogues wrote: On Mon, Oct 26, 2009 at 8:46 AM, juha.riihim...@nokia.com wrote: On Oct 25, 2009, at 16:01, ext Laurent Desnogues wrote: I don't really like the idea of having tcg_qemu_ld/st not factored in some place, as it makes memory access tracing extensions more intrusive. This brings us back to the problem having function freeing tmps. In that case, you could perhaps create a gen_st16_dont_free function as a temporary workaround? I could, but it is getting ugly =/ How about if I create another patch that moves the temporary variable (de)allocation outside the gen_ldx/ gen_stx functions and then refactor this patch on top of that? I'd personally like this, but I guess you'd better wait for some inputs from other QEMU dev's before doing it. Looks like the best option to me. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks
Aurelien Jarno wrote: That's the problem. We have seen that people already take advantage of the ambiguities, as I would have never have imagined someone writing the code in the rationale of this patch to avoid putting braces. I appreciate the desire to be precise, but we aren't writing a compiler or an automated syntax checker. We're writing a set of guidelines for reasonable people to follow. If someone sets out to defeat CODING_STYLE by introducing some crazy syntax, the solution is simple--just don't take the patch. Good sense must prevail regardless of what CODING_STYLE says. Regards, Anthony Liguori
