Re: [Qemu-devel] KVM call minutes for Feb 8
On 02/13/2011 08:57 PM, Anthony Liguori wrote: It shouldn't be able to dead lock if the locking is designed right. As an aside, one advantage of the qemuthread wrappers is that we can add lockdep mechanisms. (It's true that these could be added to glib as well, but getting stuff into glib is a pain). Paolo
[Qemu-devel] [PATCH] target-arm: fix support for vrecpe.
Now use the same algorithm as described in the ARM ARM. Signed-off-by: Christophe Lyon christophe.l...@st.com --- target-arm/helper.c | 72 ++ 1 files changed, 60 insertions(+), 12 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 7f63a28..1ab5ae9 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2687,13 +2687,53 @@ float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env) /* NEON helpers. */ -/* TODO: The architecture specifies the value that the estimate functions - should return. We return the exact reciprocal/root instead. */ -float32 HELPER(recpe_f32)(float32 a, CPUState *env) +/* The algorithm that must be used to calculate the estimate + * is specified by the ARM ARM. + */ +static float64 recip_estimate(float64 a, CPUState *env) { float_status *s = env-vfp.fp_status; -float32 one = int32_to_float32(1, s); -return float32_div(one, a, s); +float64 one = int64_to_float64(1, s); +/* q = (int)(a * 512.0) */ +float64 x512 = int64_to_float64(512, s); +float64 q = float64_mul(x512, a, s); +int64_t q_int = float64_to_int64_round_to_zero(q, s); + +/* r = 1.0 / (((double)q + 0.5) / 512.0) */ +q = int64_to_float64(q_int, s); +float64 half = float64_div(one, int64_to_float64(2, s), s); +q = float64_add(q, half, s); +q = float64_div(q, x512, s); +q = float64_div(one, q, s); + +/* s = (int)(256.0 * r + 0.5) */ +q = float64_mul(q, int64_to_float64(256, s), s); +q = float64_add(q, half, s); +q_int = float64_to_int64_round_to_zero(q, s); + +/* return (double)s / 256.0 */ +return float64_div(int64_to_float64(q_int, s), int64_to_float64(256, s), s); +} + +/* TODO: handle NaNs, zero and infinity as special input values. */ +float32 HELPER(recpe_f32)(float32 a, CPUState *env) +{ +float64 f64; +uint32_t val32; + +int result_exp; + +f64 = make_float64(((int64_t)0x3FE 52) + | ((int64_t)(float32_val(a) 0x7F) 29)); + +result_exp = 253 - ((float32_val(a) 0x7F80) 23); + +f64 = recip_estimate(f64, env); + +val32 = (float32_val(a) 0x8000) +| ((result_exp 0xFF) 23) +| ((float64_val(f64) 29) 0x7F); +return make_float32(val32); } float32 HELPER(rsqrte_f32)(float32 a, CPUState *env) @@ -2705,13 +2745,21 @@ float32 HELPER(rsqrte_f32)(float32 a, CPUState *env) uint32_t HELPER(recpe_u32)(uint32_t a, CPUState *env) { -float_status *s = env-vfp.fp_status; -float32 tmp; -tmp = int32_to_float32(a, s); -tmp = float32_scalbn(tmp, -32, s); -tmp = helper_recpe_f32(tmp, env); -tmp = float32_scalbn(tmp, 31, s); -return float32_to_int32(tmp, s); +union { +int64_t i; +float64 f; +} dp_operand; + +if ((a 0x8000) == 0) { +return 0x; +} + +dp_operand.i = ((int64_t)0x3FE 52) +| ((int64_t)(a 0x7FFF) 21); + +dp_operand.f = recip_estimate (dp_operand.f, env); + +return 0x8000 | ((dp_operand.i 21) 0x7FFF); } uint32_t HELPER(rsqrte_u32)(uint32_t a, CPUState *env) -- 1.7.2.3
[Qemu-devel] Re: [RFC] qapi: events in QMP
Am 13.02.2011 19:08, schrieb Anthony Liguori: Hi, In my QAPI branch[1], I've now got almost every existing QMP command converted with (hopefully) all of the hard problems solved. There is only one remaining thing to attack before posting for inclusion and that's events. Here's my current thinking about what to do. I've had a quick look at your branch and I have one general question: Is there a specific reason why you duplicate existing monitor handlers instead of converting the existing ones? Are the old ones still used? It would probably be much easier to review if you had a real patch that contains the actual changes instead of just one big addition of the duplicated code. Proposal for events in QAPI For QAPI, I'd like to model events on the notion of signals and slots[2]. A client would explicitly connect to a signal through a QMP interface which would result in a slot being added that then generates an event. Internally in QEMU, we could also connect slots to the same signals. Since we don't have an object API in QMP, we'd use a pair of connect/disconnect functions that had enough information to identify the signal. Example: We would define QEVENT_BLOCK_IO_EVENT as: # qmp-schema.json { 'BlockIOEvent': {'device': 'str', 'action': 'str', 'operation': 'str'} } The 'Event' suffix is used to determine that the type is an event and not a structure. This would generate the following structures for QEMU: typedef void (BlockIOEventFunc)(const char *device, const char *action, const char *operation, void *opaque); Why is an event not a structure? For one it makes things inconsistent (you have this 'Event' suffix magic), and it's not even convenient. The parameter list of the BlockIOEventFunc might become very long. At the moment you have three const char* there and I think it's only going to grow - who is supposed to remember the right order of arguments? So why not make the event a struct and have a typedef void (BlockIOEventFunc)(BlockIOEvent *evt)? By the way, we're not that short on disk space. Let's call it 'string' instead of 'str'. typedef struct BlockIOEvent { /* contents are private */ } BlockIOEvent; /* Connect a slot to a BlockIOEvent signal and return a handle to the connection */ int block_io_event_connect(BlockIOEvent *event, BlockIOEventFunc *cb, void *opaque); /* Signal any connect slots of a BlockIOEvent */ void block_io_event_signal(BlockIOEvent *event, const char *device, const char *action, const char *operation); /* Disconnect a slot from a signal based on a connection handle */ void block_io_event_disconnect(BlockIOEvent *event, int handle); In the case of BlockIOEvent, this is a global signal that is not tied to any specific object so there would be a global BlockIOEvent object within QEMU. From a QMP perspective, we would have the following function in the schema: [ 'block-io-event', {}, {}, 'BlockIOEvent' ] Not sure what this list is supposed to tell me... I see that you use the same for command, so I guess the first one is something like an command/event name, the second seems to be the arguments, last could be the type of the return value, the third no idea. So would an event look like a response to a command that was never issued? Another Example Here's an example of BlockDeviceEvent, the successor to BlockIOEvent. It makes use of signal accessor arguments and QAPI enumeration support: So are we going to drop BlockIOEvent (or any other event that will be extended) or will both old and new version continue to exist and we always signal both of them? Kevin
Re: [Qemu-devel] [RFC][PATCH v6 00/23] virtagent: host/guest RPC communication agent
Hi, A brain-dump on how we're planning to do this. I tried to keep it concise, but that only went so far :) We extend qemu-va, the virtagent guest agent, to be able to create a unix socket in the guest that listens for connections. [ ... ] 1) There may be user-level RPCs that are generally desirable...such as being able to execute a script as an unpriveledged user, or access a file in the same way. So it makes sense to invoke user-level agent startup with a login script. But for X-related things like Spice, an .xsession hook makes more sense. I usually login via gdm, login and X session are not separate then. If we do both, the .xsession-spawned daemon will take over, but any existing stateful interactions with that user will be effectively reset. This may be undesirable. One option is to XOpenDisplay() on demand, rather than at startup. We won't know what $env{DISPLAY} is then, however, and there could be multiple displays for that user. I would make the user-agent just run without using X11 in case $DISPLAY is unset and be done with it. I would also try hard to avoid adding any X11-specific stuff into the protocol. The protocol for cut+paste and the other gui stuff should work equally well for non-linux guests (windows, macos?) and (when it hits mainstream some day) wayland. agent to do it and then notify the host, however. The only way I see around this is to only start the user-level daemon via .xsession or similar. Start via .xsession is the way to go IMHO. I'd like to move forward with this approach, but with the goal here being that we have one agent to rule them all, I'd like to know whether those of you involved with the spice vdagent work think this is a reasonable approach. spice allows a single user-level agent today and also does only gui-stuff, so that approach works fine for us. Can spice do anything useful with more than 1 display per user? multihead (one virtual desktop on multiple physical displays) yes. Truely separate displays (aka separate X sessions) no. I doubt running two X xessions on two displays on a single machine is a use case we need to worry about. There where approaches to do that with physical machines, so you can have two people share one computer by hooking two displays, two keyboards and two mice. It wasn't very successful. And with virtual machines this doesn't make sense at all IMHO. cheers, Gerd PS: sorry for the delay, was sick for two weeks.
[Qemu-devel] [RFC] Some more io-thread optimizations
Hi, patch below further reduces the io-thread overhead in tcg mode so that specifically emulating smp boxes gets noticeably faster. Its essence: poll the file descriptors until select returns 0, keeping the global mutex locked. This reduces ping pong with the vcpu threads, most noticeably in tcg mode where we run in lock-step. Split up in two patches, I'm planning to route those changes via the kvm queue (as they collide with other patches there). Jan 8- diff --git a/sysemu.h b/sysemu.h index 23ae17e..0a69464 100644 --- a/sysemu.h +++ b/sysemu.h @@ -73,7 +73,7 @@ void cpu_synchronize_all_post_init(void); void qemu_announce_self(void); -void main_loop_wait(int nonblocking); +int main_loop_wait(int nonblocking); bool qemu_savevm_state_blocked(Monitor *mon); int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, diff --git a/vl.c b/vl.c index ed2cdfa..66b7c6f 100644 --- a/vl.c +++ b/vl.c @@ -1311,7 +1311,7 @@ void qemu_system_powerdown_request(void) qemu_notify_event(); } -void main_loop_wait(int nonblocking) +int main_loop_wait(int nonblocking) { IOHandlerRecord *ioh; fd_set rfds, wfds, xfds; @@ -1356,9 +1356,16 @@ void main_loop_wait(int nonblocking) slirp_select_fill(nfds, rfds, wfds, xfds); -qemu_mutex_unlock_iothread(); +if (timeout 0) { +qemu_mutex_unlock_iothread(); +} + ret = select(nfds + 1, rfds, wfds, xfds, tv); -qemu_mutex_lock_iothread(); + +if (timeout 0) { +qemu_mutex_lock_iothread(); +} + if (ret 0) { IOHandlerRecord *pioh; @@ -1386,6 +1393,7 @@ void main_loop_wait(int nonblocking) them. */ qemu_bh_poll(); +return ret; } static int vm_can_run(void) @@ -1405,6 +1413,7 @@ qemu_irq qemu_system_powerdown; static void main_loop(void) { +int last_io = 0; int r; qemu_main_loop_start(); @@ -1421,7 +1430,12 @@ static void main_loop(void) #ifdef CONFIG_PROFILER ti = profile_getclock(); #endif -main_loop_wait(nonblocking); +#ifdef CONFIG_IOTHREAD +if (last_io 0) { +nonblocking = true; +} +#endif +last_io = main_loop_wait(nonblocking); #ifdef CONFIG_PROFILER dev_time += profile_getclock() - ti; #endif
[Qemu-devel] Ping: [PATCH] Handle icount for powerpc tbl/tbu/decr load and store.
Potential reviewers CC: On Feb 8, 2011, at 10:59 AM, Tristan Gingold wrote: Handle option '-icount X' on powerpc targets. Signed-off-by: Tristan Gingold ging...@adacore.com --- target-ppc/translate_init.c | 36 1 files changed, 36 insertions(+), 0 deletions(-) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 907535e..7ef86ad 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -154,12 +154,24 @@ static void spr_read_ureg (void *opaque, int gprn, int sprn) #if !defined(CONFIG_USER_ONLY) static void spr_read_decr (void *opaque, int gprn, int sprn) { +if (use_icount) +gen_io_start(); gen_helper_load_decr(cpu_gpr[gprn]); +if (use_icount) { +gen_io_end(); + gen_stop_exception(opaque); +} } static void spr_write_decr (void *opaque, int sprn, int gprn) { +if (use_icount) +gen_io_start(); gen_helper_store_decr(cpu_gpr[gprn]); +if (use_icount) { +gen_io_end(); + gen_stop_exception(opaque); +} } #endif @@ -167,12 +179,24 @@ static void spr_write_decr (void *opaque, int sprn, int gprn) /* Time base */ static void spr_read_tbl (void *opaque, int gprn, int sprn) { +if (use_icount) +gen_io_start(); gen_helper_load_tbl(cpu_gpr[gprn]); +if (use_icount) { +gen_io_end(); + gen_stop_exception(opaque); +} } static void spr_read_tbu (void *opaque, int gprn, int sprn) { +if (use_icount) +gen_io_start(); gen_helper_load_tbu(cpu_gpr[gprn]); +if (use_icount) { +gen_io_end(); + gen_stop_exception(opaque); +} } __attribute__ (( unused )) @@ -190,12 +214,24 @@ static void spr_read_atbu (void *opaque, int gprn, int sprn) #if !defined(CONFIG_USER_ONLY) static void spr_write_tbl (void *opaque, int sprn, int gprn) { +if (use_icount) +gen_io_start(); gen_helper_store_tbl(cpu_gpr[gprn]); +if (use_icount) { +gen_io_end(); + gen_stop_exception(opaque); +} } static void spr_write_tbu (void *opaque, int sprn, int gprn) { +if (use_icount) +gen_io_start(); gen_helper_store_tbu(cpu_gpr[gprn]); +if (use_icount) { +gen_io_end(); + gen_stop_exception(opaque); +} } __attribute__ (( unused )) -- 1.7.3.GIT
[Qemu-devel] [PATCH V6 4/4 resend] nmi: report error(QError) when the cpu-index is invalid
When cpu-index is found invalid in runtime, it will report QERR_INVALID_PARAMETER_VALUE. Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- diff --git a/monitor.c b/monitor.c index 1b1c0ba..82935f0 100644 --- a/monitor.c +++ b/monitor.c @@ -2563,6 +2563,7 @@ static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data) return 0; } +qerror_report(QERR_INVALID_PARAMETER_VALUE, cpu-index, a CPU number); return -1; } #endif
[Qemu-devel] [PATCH V6 1/4 resend] nmi: convert cpu_index to cpu-index
cpu-index which uses hyphen is better name. Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- diff --git a/hmp-commands.hx b/hmp-commands.hx index 5d4cb9e..e43ac7c 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -721,7 +721,7 @@ ETEXI #if defined(TARGET_I386) { .name = nmi, -.args_type = cpu_index:i, +.args_type = cpu-index:i, .params = cpu, .help = inject an NMI on the given CPU, .mhandler.cmd = do_inject_nmi, diff --git a/monitor.c b/monitor.c index 27883f8..a916771 100644 --- a/monitor.c +++ b/monitor.c @@ -2545,7 +2545,7 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict) static void do_inject_nmi(Monitor *mon, const QDict *qdict) { CPUState *env; -int cpu_index = qdict_get_int(qdict, cpu_index); +int cpu_index = qdict_get_int(qdict, cpu-index); for (env = first_cpu; env != NULL; env = env-next_cpu) if (env-cpu_index == cpu_index) {
[Qemu-devel] [PATCH V6 2/4 resend] nmi: make cpu-index argument optional
When the argument cpu-index is not given, then nmi command will inject NMI on all CPUs. This simulate the nmi button on physical machine. Note: it will allow non-argument nmi command and change the human monitor behavior. Thanks to Markus Armbruster for correcting the logic detecting cpu-index is given or not. Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- diff --git a/hmp-commands.hx b/hmp-commands.hx index e43ac7c..ec1a4db 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -721,9 +721,10 @@ ETEXI #if defined(TARGET_I386) { .name = nmi, -.args_type = cpu-index:i, -.params = cpu, -.help = inject an NMI on the given CPU, +.args_type = cpu-index:i?, +.params = [cpu], +.help = Inject an NMI on all CPUs if no argument is given, + otherwise inject it on the specified CPU, .mhandler.cmd = do_inject_nmi, }, #endif diff --git a/monitor.c b/monitor.c index a916771..387b020 100644 --- a/monitor.c +++ b/monitor.c @@ -2545,8 +2545,15 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict) static void do_inject_nmi(Monitor *mon, const QDict *qdict) { CPUState *env; -int cpu_index = qdict_get_int(qdict, cpu-index); +int cpu_index; +if (!qdict_haskey(qdict, cpu-index)) { +for (env = first_cpu; env != NULL; env = env-next_cpu) +cpu_interrupt(env, CPU_INTERRUPT_NMI); +return; +} + +cpu_index = qdict_get_int(qdict, cpu-index); for (env = first_cpu; env != NULL; env = env-next_cpu) if (env-cpu_index == cpu_index) { if (kvm_enabled())
[Qemu-devel] [PATCH V6 3/4 resend] qmp, nmi: convert do_inject_nmi() to QObject
Make we can inject NMI via qemu-monitor-protocol. We use inject-nmi for the qmp command name, the meaning is clearer. Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- diff --git a/hmp-commands.hx b/hmp-commands.hx index b2c6cd6..6d3e7d2 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -744,7 +744,8 @@ ETEXI .params = [cpu], .help = Inject an NMI on all CPUs if no argument is given, otherwise inject it on the specified CPU, -.mhandler.cmd = do_inject_nmi, +.user_print = monitor_user_noop, +.mhandler.cmd_new = do_inject_nmi, }, #endif STEXI diff --git a/monitor.c b/monitor.c index c1123ae..3cdb768 100644 --- a/monitor.c +++ b/monitor.c @@ -2555,7 +2555,7 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict) #endif #if defined(TARGET_I386) -static void do_inject_nmi(Monitor *mon, const QDict *qdict) +static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data) { CPUState *env; int cpu_index; @@ -2563,15 +2563,17 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict) if (!qdict_haskey(qdict, cpu-index)) { for (env = first_cpu; env != NULL; env = env-next_cpu) cpu_interrupt(env, CPU_INTERRUPT_NMI); -return; +return 0; } cpu_index = qdict_get_int(qdict, cpu-index); for (env = first_cpu; env != NULL; env = env-next_cpu) if (env-cpu_index == cpu_index) { cpu_interrupt(env, CPU_INTERRUPT_NMI); -break; +return 0; } + +return -1; } #endif diff --git a/qmp-commands.hx b/qmp-commands.hx index df40a3d..40b8c13 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -429,6 +429,34 @@ Example: EQMP +#if defined(TARGET_I386) +{ +.name = inject-nmi, +.args_type = cpu-index:i?, +.params = [cpu], +.help = Inject an NMI on all CPUs if no argument is given, + otherwise inject it on the specified CPU, +.user_print = monitor_user_noop, +.mhandler.cmd_new = do_inject_nmi, +}, +#endif +SQMP +inject-nmi +-- + +Inject an NMI on all CPUs or the given CPU (x86 only). + +Arguments: + +- cpu-index: the index of the CPU to be injected NMI (json-int, optional) + +Example: + +- { execute: inject-nmi, arguments: { cpu-index: 0 } } +- { return: {} } + +EQMP + { .name = migrate, .args_type = detach:-d,blk:-b,inc:-i,uri:s,
[Qemu-devel] Re: [PATCH V6 1/4] nmi: convert cpu_index to cpu-index
On 02/09/2011 07:48 PM, Luiz Capitulino wrote: You should use Anthony's tree: git://git.qemu.org/qemu.git Done, thank you for your concern and patience. see my sent emails: [PATCH V6 1/4 resend] [PATCH V6 2/4 resend] [PATCH V6 3/4 resend] [PATCH V6 4/4 resend] Thanks again. Lai
[Qemu-devel] [PATCH v2 0/2] target-arm: fix Neon VUZP, VZIP instructions
This patch series fixes bugs in the Neon VZIP and VUZP instructions by abandoning the existing inline implementations in favour of calling out to straightforward helper functions. The inline routines could generate 50+ TCG ops each, which is well over the recommended limit in tcg/README for using helpers instead; they also did not give the correct results... I've tested these patches using the usual random instruction generation approach. V2 changes: moved the decoding of register numbers, size and q flag out of the helper functions into translate.c; split the helpers up into one per (size, q) combination. Peter Maydell (2): target-arm: Move Neon VUZP to helper functions target-arm: Move Neon VZIP to helper functions target-arm/helpers.h | 11 +++ target-arm/neon_helper.c | 186 +++ target-arm/translate.c | 215 +++--- 3 files changed, 267 insertions(+), 145 deletions(-)
[Qemu-devel] [PATCH v2 2/2] target-arm: Move Neon VZIP to helper functions
Move the implementation of the Neon VUZP unzip instruction from inline code to helper functions. (At 50+ TCG ops it was well over the recommended limit for coding inline.) The helper implementations also give the correct answers where the inline implementation did not. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/helpers.h |5 ++ target-arm/neon_helper.c | 92 ++ target-arm/translate.c | 109 +++--- 3 files changed, 133 insertions(+), 73 deletions(-) diff --git a/target-arm/helpers.h b/target-arm/helpers.h index 88c2fce..e494b47 100644 --- a/target-arm/helpers.h +++ b/target-arm/helpers.h @@ -465,5 +465,10 @@ DEF_HELPER_3(neon_unzip16, void, env, i32, i32) DEF_HELPER_3(neon_qunzip8, void, env, i32, i32) DEF_HELPER_3(neon_qunzip16, void, env, i32, i32) DEF_HELPER_3(neon_qunzip32, void, env, i32, i32) +DEF_HELPER_3(neon_zip8, void, env, i32, i32) +DEF_HELPER_3(neon_zip16, void, env, i32, i32) +DEF_HELPER_3(neon_qzip8, void, env, i32, i32) +DEF_HELPER_3(neon_qzip16, void, env, i32, i32) +DEF_HELPER_3(neon_qzip32, void, env, i32, i32) #include def-helper.h diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c index 6e2a2fd..cee9ee4 100644 --- a/target-arm/neon_helper.c +++ b/target-arm/neon_helper.c @@ -1757,3 +1757,95 @@ void HELPER(neon_unzip16)(CPUState *env, uint32_t rd, uint32_t rm) env-vfp.regs[rm] = make_float64(m0); env-vfp.regs[rd] = make_float64(d0); } + +void HELPER(neon_qzip8)(CPUState *env, uint32_t rd, uint32_t rm) +{ +uint64_t zm0 = float64_val(env-vfp.regs[rm]); +uint64_t zm1 = float64_val(env-vfp.regs[rm + 1]); +uint64_t zd0 = float64_val(env-vfp.regs[rd]); +uint64_t zd1 = float64_val(env-vfp.regs[rd + 1]); +uint64_t d0 = ELEM(zd0, 0, 8) | (ELEM(zm0, 0, 8) 8) +| (ELEM(zd0, 1, 8) 16) | (ELEM(zm0, 1, 8) 24) +| (ELEM(zd0, 2, 8) 32) | (ELEM(zm0, 2, 8) 40) +| (ELEM(zd0, 3, 8) 48) | (ELEM(zm0, 3, 8) 56); +uint64_t d1 = ELEM(zd0, 4, 8) | (ELEM(zm0, 4, 8) 8) +| (ELEM(zd0, 5, 8) 16) | (ELEM(zm0, 5, 8) 24) +| (ELEM(zd0, 6, 8) 32) | (ELEM(zm0, 6, 8) 40) +| (ELEM(zd0, 7, 8) 48) | (ELEM(zm0, 7, 8) 56); +uint64_t m0 = ELEM(zd1, 0, 8) | (ELEM(zm1, 0, 8) 8) +| (ELEM(zd1, 1, 8) 16) | (ELEM(zm1, 1, 8) 24) +| (ELEM(zd1, 2, 8) 32) | (ELEM(zm1, 2, 8) 40) +| (ELEM(zd1, 3, 8) 48) | (ELEM(zm1, 3, 8) 56); +uint64_t m1 = ELEM(zd1, 4, 8) | (ELEM(zm1, 4, 8) 8) +| (ELEM(zd1, 5, 8) 16) | (ELEM(zm1, 5, 8) 24) +| (ELEM(zd1, 6, 8) 32) | (ELEM(zm1, 6, 8) 40) +| (ELEM(zd1, 7, 8) 48) | (ELEM(zm1, 7, 8) 56); +env-vfp.regs[rm] = make_float64(m0); +env-vfp.regs[rm + 1] = make_float64(m1); +env-vfp.regs[rd] = make_float64(d0); +env-vfp.regs[rd + 1] = make_float64(d1); +} + +void HELPER(neon_qzip16)(CPUState *env, uint32_t rd, uint32_t rm) +{ +uint64_t zm0 = float64_val(env-vfp.regs[rm]); +uint64_t zm1 = float64_val(env-vfp.regs[rm + 1]); +uint64_t zd0 = float64_val(env-vfp.regs[rd]); +uint64_t zd1 = float64_val(env-vfp.regs[rd + 1]); +uint64_t d0 = ELEM(zd0, 0, 16) | (ELEM(zm0, 0, 16) 16) +| (ELEM(zd0, 1, 16) 32) | (ELEM(zm0, 1, 16) 48); +uint64_t d1 = ELEM(zd0, 2, 16) | (ELEM(zm0, 2, 16) 16) +| (ELEM(zd0, 3, 16) 32) | (ELEM(zm0, 3, 16) 48); +uint64_t m0 = ELEM(zd1, 0, 16) | (ELEM(zm1, 0, 16) 16) +| (ELEM(zd1, 1, 16) 32) | (ELEM(zm1, 1, 16) 48); +uint64_t m1 = ELEM(zd1, 2, 16) | (ELEM(zm1, 2, 16) 16) +| (ELEM(zd1, 3, 16) 32) | (ELEM(zm1, 3, 16) 48); +env-vfp.regs[rm] = make_float64(m0); +env-vfp.regs[rm + 1] = make_float64(m1); +env-vfp.regs[rd] = make_float64(d0); +env-vfp.regs[rd + 1] = make_float64(d1); +} + +void HELPER(neon_qzip32)(CPUState *env, uint32_t rd, uint32_t rm) +{ +uint64_t zm0 = float64_val(env-vfp.regs[rm]); +uint64_t zm1 = float64_val(env-vfp.regs[rm + 1]); +uint64_t zd0 = float64_val(env-vfp.regs[rd]); +uint64_t zd1 = float64_val(env-vfp.regs[rd + 1]); +uint64_t d0 = ELEM(zd0, 0, 32) | (ELEM(zm0, 0, 32) 32); +uint64_t d1 = ELEM(zd0, 1, 32) | (ELEM(zm0, 1, 32) 32); +uint64_t m0 = ELEM(zd1, 0, 32) | (ELEM(zm1, 0, 32) 32); +uint64_t m1 = ELEM(zd1, 1, 32) | (ELEM(zm1, 1, 32) 32); +env-vfp.regs[rm] = make_float64(m0); +env-vfp.regs[rm + 1] = make_float64(m1); +env-vfp.regs[rd] = make_float64(d0); +env-vfp.regs[rd + 1] = make_float64(d1); +} + +void HELPER(neon_zip8)(CPUState *env, uint32_t rd, uint32_t rm) +{ +uint64_t zm = float64_val(env-vfp.regs[rm]); +uint64_t zd = float64_val(env-vfp.regs[rd]); +uint64_t d0 = ELEM(zd, 0, 8) | (ELEM(zm, 0, 8) 8) +| (ELEM(zd, 1, 8) 16) | (ELEM(zm, 1, 8) 24) +| (ELEM(zd, 2, 8) 32) | (ELEM(zm, 2, 8) 40) +| (ELEM(zd, 3, 8) 48) | (ELEM(zm, 3, 8) 56); +uint64_t m0 = ELEM(zd, 4, 8) |
[Qemu-devel] [PATCH v2 1/2] target-arm: Move Neon VUZP to helper functions
Move the implementation of the Neon VUZP unzip instruction from inline code to helper functions. (At 50+ TCG ops it was well over the recommended limit for coding inline.) The helper implementations also fix the handling of the quadword version of the instruction. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/helpers.h |6 +++ target-arm/neon_helper.c | 94 ++ target-arm/translate.c | 112 +++--- 3 files changed, 137 insertions(+), 75 deletions(-) diff --git a/target-arm/helpers.h b/target-arm/helpers.h index 77f1635..88c2fce 100644 --- a/target-arm/helpers.h +++ b/target-arm/helpers.h @@ -460,4 +460,10 @@ DEF_HELPER_3(iwmmxt_muladdswl, i64, i64, i32, i32) DEF_HELPER_2(set_teecr, void, env, i32) +DEF_HELPER_3(neon_unzip8, void, env, i32, i32) +DEF_HELPER_3(neon_unzip16, void, env, i32, i32) +DEF_HELPER_3(neon_qunzip8, void, env, i32, i32) +DEF_HELPER_3(neon_qunzip16, void, env, i32, i32) +DEF_HELPER_3(neon_qunzip32, void, env, i32, i32) + #include def-helper.h diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c index dc09968..6e2a2fd 100644 --- a/target-arm/neon_helper.c +++ b/target-arm/neon_helper.c @@ -1663,3 +1663,97 @@ uint32_t HELPER(neon_acgt_f32)(uint32_t a, uint32_t b) float32 f1 = float32_abs(vfp_itos(b)); return (float32_compare_quiet(f0, f1, NFS) 0) ? ~0 : 0; } + +#define ELEM(V, N, SIZE) (((V) ((N) * (SIZE))) ((1ull (SIZE)) - 1)) + +void HELPER(neon_qunzip8)(CPUState *env, uint32_t rd, uint32_t rm) +{ +uint64_t zm0 = float64_val(env-vfp.regs[rm]); +uint64_t zm1 = float64_val(env-vfp.regs[rm + 1]); +uint64_t zd0 = float64_val(env-vfp.regs[rd]); +uint64_t zd1 = float64_val(env-vfp.regs[rd + 1]); +uint64_t d0 = ELEM(zd0, 0, 8) | (ELEM(zd0, 2, 8) 8) +| (ELEM(zd0, 4, 8) 16) | (ELEM(zd0, 6, 8) 24) +| (ELEM(zd1, 0, 8) 32) | (ELEM(zd1, 2, 8) 40) +| (ELEM(zd1, 4, 8) 48) | (ELEM(zd1, 6, 8) 56); +uint64_t d1 = ELEM(zm0, 0, 8) | (ELEM(zm0, 2, 8) 8) +| (ELEM(zm0, 4, 8) 16) | (ELEM(zm0, 6, 8) 24) +| (ELEM(zm1, 0, 8) 32) | (ELEM(zm1, 2, 8) 40) +| (ELEM(zm1, 4, 8) 48) | (ELEM(zm1, 6, 8) 56); +uint64_t m0 = ELEM(zd0, 1, 8) | (ELEM(zd0, 3, 8) 8) +| (ELEM(zd0, 5, 8) 16) | (ELEM(zd0, 7, 8) 24) +| (ELEM(zd1, 1, 8) 32) | (ELEM(zd1, 3, 8) 40) +| (ELEM(zd1, 5, 8) 48) | (ELEM(zd1, 7, 8) 56); +uint64_t m1 = ELEM(zm0, 1, 8) | (ELEM(zm0, 3, 8) 8) +| (ELEM(zm0, 5, 8) 16) | (ELEM(zm0, 7, 8) 24) +| (ELEM(zm1, 1, 8) 32) | (ELEM(zm1, 3, 8) 40) +| (ELEM(zm1, 5, 8) 48) | (ELEM(zm1, 7, 8) 56); +env-vfp.regs[rm] = make_float64(m0); +env-vfp.regs[rm + 1] = make_float64(m1); +env-vfp.regs[rd] = make_float64(d0); +env-vfp.regs[rd + 1] = make_float64(d1); +} + +void HELPER(neon_qunzip16)(CPUState *env, uint32_t rd, uint32_t rm) +{ +uint64_t zm0 = float64_val(env-vfp.regs[rm]); +uint64_t zm1 = float64_val(env-vfp.regs[rm + 1]); +uint64_t zd0 = float64_val(env-vfp.regs[rd]); +uint64_t zd1 = float64_val(env-vfp.regs[rd + 1]); +uint64_t d0 = ELEM(zd0, 0, 16) | (ELEM(zd0, 2, 16) 16) +| (ELEM(zd1, 0, 16) 32) | (ELEM(zd1, 2, 16) 48); +uint64_t d1 = ELEM(zm0, 0, 16) | (ELEM(zm0, 2, 16) 16) +| (ELEM(zm1, 0, 16) 32) | (ELEM(zm1, 2, 16) 48); +uint64_t m0 = ELEM(zd0, 1, 16) | (ELEM(zd0, 3, 16) 16) +| (ELEM(zd1, 1, 16) 32) | (ELEM(zd1, 3, 16) 48); +uint64_t m1 = ELEM(zm0, 1, 16) | (ELEM(zm0, 3, 16) 16) +| (ELEM(zm1, 1, 16) 32) | (ELEM(zm1, 3, 16) 48); +env-vfp.regs[rm] = make_float64(m0); +env-vfp.regs[rm + 1] = make_float64(m1); +env-vfp.regs[rd] = make_float64(d0); +env-vfp.regs[rd + 1] = make_float64(d1); +} + +void HELPER(neon_qunzip32)(CPUState *env, uint32_t rd, uint32_t rm) +{ +uint64_t zm0 = float64_val(env-vfp.regs[rm]); +uint64_t zm1 = float64_val(env-vfp.regs[rm + 1]); +uint64_t zd0 = float64_val(env-vfp.regs[rd]); +uint64_t zd1 = float64_val(env-vfp.regs[rd + 1]); +uint64_t d0 = ELEM(zd0, 0, 32) | (ELEM(zd1, 0, 32) 32); +uint64_t d1 = ELEM(zm0, 0, 32) | (ELEM(zm1, 0, 32) 32); +uint64_t m0 = ELEM(zd0, 1, 32) | (ELEM(zd1, 1, 32) 32); +uint64_t m1 = ELEM(zm0, 1, 32) | (ELEM(zm1, 1, 32) 32); +env-vfp.regs[rm] = make_float64(m0); +env-vfp.regs[rm + 1] = make_float64(m1); +env-vfp.regs[rd] = make_float64(d0); +env-vfp.regs[rd + 1] = make_float64(d1); +} + +void HELPER(neon_unzip8)(CPUState *env, uint32_t rd, uint32_t rm) +{ +uint64_t zm = float64_val(env-vfp.regs[rm]); +uint64_t zd = float64_val(env-vfp.regs[rd]); +uint64_t d0 = ELEM(zd, 0, 8) | (ELEM(zd, 2, 8) 8) +| (ELEM(zd, 4, 8) 16) | (ELEM(zd, 6, 8) 24) +| (ELEM(zm, 0, 8) 32) | (ELEM(zm, 2, 8) 40) +| (ELEM(zm, 4, 8) 48) | (ELEM(zm, 6, 8) 56); +uint64_t m0 = ELEM(zd, 1, 8) |
Re: [Qemu-devel] KVM call minutes for Feb 8
On Sun, Feb 13, 2011 at 01:38:12PM -0600, Anthony Liguori wrote: On 02/13/2011 12:08 PM, Gleb Natapov wrote: On Sun, Feb 13, 2011 at 10:56:30AM -0600, Anthony Liguori wrote: qemu -device i440fx,id=nb -device piix3,id=sb,chipset=nb -device ioapic,id=ioapic,chipset=sb -device cpu,ioapic=ioapic,northbridge=nb Is not all that unreasonable and presents a fully functioning PC. Sure. And -M blah is a shortcut. Exactly. Or better yet, blah is a config file that contains [device nb] driver=i440fx You are trying to model how particular (very ancient) HW looked like, instead of emulating guest visible functionality, but is dead end since things are changing constantly. Northbridge functionality moves onto cpu for instance. What CPU i440fx was designed for? Pentium? What if user runs QEMU with emulated CPU that in real life has internal memory controller? Does you config have sense for such setup? Should we allow to specify only Pentium CPU since this is how real HW worked? Yes, how we structure the device tree will change over time. If users have to specify this stuff, we've already failed to start with. And nevertheless this is what you propose here. User should be able to specify what devices he wants in a stable format (FDT for instance) and qemu should create the machine from it. How it is done internally is different question, but if FDT cannot be reproduced from internal state we are doing something wrong. The important thing is: we should model how different parts of HW work together, not how chips are interconnected. Look at FDTs of real machines. They may contain a lot of devices, but in reality most of them are sitting in the same chip as CPU. The i440fx is definitely guest visible. And yes, a guest could look at freak out that an i440fx is in use with a non-Pentium class CPU. We cheat here, and it works just fine. And not modeling PIIX3 exactly as in real life (by dropping IDE or USB or even adding ACPI from PIIX4) works just fine (we checked it here too). And yes, a guest could look into details and freak out too. But guests don't do that (otherwise they would have to support each chipset in existence). The state that i440fx expose to a guest is of any interest only to firmware and luckily we have our own firmware to take care of the details. PIIX3 is not mush more visible to a guest. This is just marketing name for collection of HW put in one chip. Remove USB and you get PIIX2, add ACPI and you get PIIX4. Why would you want to govern qemu design from such insignificant details? [device sb] driver=piix3 And piix3 refers to piix3.cfg which describe devices that present on the chipset. I disagree in this case that it's the best thing to do, but in general, yeah, we should provide a mechanism to have essentially device tree macros such that one high level device represents a larger tree of devices. This would be a good mechanism to support the concept of machines. But I don't think we should rely on having this tomorrow as doing this well is going to be challenging. That is what we (almost) have now. If going your direction makes it challenging we shouldn't go there. chipset=nb [device ioapic] driver=ioapic chipset=sb Here, for instance, IOAPIC is included in a chipset for a long time now. Why user should care that piix3 didn't have it. How this detail changes qemu functionality? If it doesn't why should we expose it? Yeah, I'd be inclined to push the ioapic into the chipset here except it may be useful to have multiple ioapics at some point down the road. When machine has multiple ioapic I am almost sure all of them are present on a chipset, not as stand alone chips. Since having multiple IOAPIC affects also firmware I am not sure it is useful to support different number of IOAPICs in qemu. Yes, we're exposing gory details of the device model. No, I don't have a problem with that. [device cpu] driver=cpu ioapic=ioapic Why ioapic here? Doesn't cpu talks to ioapic via northbridge? The i440fx is the northbridge. For the i440fx, the I/O apic is a separate chip. But it is connected to i440fx, not CPU like in your theoretical config. Northbridge, southbridge are just names that do not have any architectural significance. Like other said in this thread on other architectures there is no such naming at all. No wonder, since on most embedded archs (that I know of) all functionality was included on the same chip as CPU. On x86 PCs historically motherboard had three main chips (cpu, ram + pic controller, common hw), so they got those names. Now in Intel Sandy Bridge all of the functions of the northbridge reside on the chip itself for instance and I am sure there will be Atoms with all HW on the same chip. -- Gleb.
Re: [Qemu-devel] [PATCH] target-arm: fix support for vrecpe.
On 14.02.2011 10:46, Christophe Lyon wrote: Now use the same algorithm as described in the ARM ARM. oops sorry, I have sent the wrong patch. Please ignore this version. Christophe.
[Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc '
On 01/31/11 21:43, Anthony Liguori wrote: commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a regression in the change vnc password command that changed the behavior of setting the VNC password to an empty string from disabling login to disabling authentication. This commit refactors the code to eliminate this overloaded semantics in vnc_display_password and instead introduces the vnc_display_disable_login. The monitor implementation then determines the behavior of an empty or missing string. Hmm, now about simply never ever changing vs-auth? cheers, Gerd
[Qemu-devel] Re: [RFC] prep: enable irq sharing on ide again
On 02/08/11 00:26, Alexander Graf wrote: The new ISA infrastructure checks for potential irq sharing bugs on interrupt lines, because usually irq lines on isa can't be shared. The PREP spec however mandates that the irq lines for both IDE ports are shared and according to Aurelien this also used to work just fine. So let's add a way to enable this sharing again, so we don't introduce unnecessary regressions over older versions of Qemu. Had a patch for that, got shoot down for reasons I don't remember, attached for reference. It basically allows IRQ sharing in case the two devices sharing the IRQ are of the same kind. In that case you usually have a single guest driver handling both devices and IRQ sharing works most of the time. I don't mind much which approach we take ... cheers, Gerd From 44d7b59ee9cc8a57ea7b6ecdf53798d2c74d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann kra...@redhat.com Date: Fri, 11 Sep 2009 13:43:46 +0200 Subject: [PATCH] isa: refine irq reservations There are a few cases where IRQ sharing on the ISA bus is used and possible. In general only devices of the same kind can do that. A few use cases: * serial lines 1+3 share irq 4 * serial lines 2+4 share irq 3 * parallel ports share irq 7 * ppc/prep: ide ports share irq 13 This patch refines the irq reservation mechanism for the isa bus to handle those cases. It keeps track of the driver which owns the IRQ in question and allows irq sharing for devices handled by the same driver. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/isa-bus.c | 16 +--- 1 files changed, 13 insertions(+), 3 deletions(-) diff --git a/hw/isa-bus.c b/hw/isa-bus.c index 4e306de..a99e793 100644 --- a/hw/isa-bus.c +++ b/hw/isa-bus.c @@ -26,6 +26,7 @@ struct ISABus { BusState qbus; qemu_irq *irqs; uint32_t assigned; +DeviceInfo *irq_owner[16]; }; static ISABus *isabus; target_phys_addr_t isa_mem_base = 0; @@ -72,7 +73,9 @@ qemu_irq isa_reserve_irq(int isairq) exit(1); } if (isabus-assigned (1 isairq)) { -fprintf(stderr, isa irq %d already assigned\n, isairq); +DeviceInfo *owner = isabus-irq_owner[isairq]; +fprintf(stderr, isa irq %d already assigned (%s)\n, +isairq, owner ? owner-name : unknown); exit(1); } isabus-assigned |= (1 isairq); @@ -83,10 +86,17 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq) { assert(dev-nirqs ARRAY_SIZE(dev-isairq)); if (isabus-assigned (1 isairq)) { -fprintf(stderr, isa irq %d already assigned\n, isairq); -exit(1); +DeviceInfo *owner = isabus-irq_owner[isairq]; +if (owner == dev-qdev.info) { +/* irq sharing is ok in case the same driver handles both */; +} else { +fprintf(stderr, isa irq %d already assigned (%s)\n, +isairq, owner ? owner-name : unknown); +exit(1); +} } isabus-assigned |= (1 isairq); +isabus-irq_owner[isairq] = dev-qdev.info; dev-isairq[dev-nirqs] = isairq; *p = isabus-irqs[isairq]; dev-nirqs++; -- 1.7.1
[Qemu-devel] Re: Ping: [PATCH] Handle icount for powerpc tbl/tbu/decr load and store.
Tristan Gingold wrote: Potential reviewers CC: On Feb 8, 2011, at 10:59 AM, Tristan Gingold wrote: Handle option '-icount X' on powerpc targets. Signed-off-by: Tristan Gingold ging...@adacore.com Braces are broken. Edgar knows his way around icount a lot better than me - I've never actually used it. Edgar, any comments? Alex
[Qemu-devel] Re: [RFC] prep: enable irq sharing on ide again
Gerd Hoffmann wrote: On 02/08/11 00:26, Alexander Graf wrote: The new ISA infrastructure checks for potential irq sharing bugs on interrupt lines, because usually irq lines on isa can't be shared. The PREP spec however mandates that the irq lines for both IDE ports are shared and according to Aurelien this also used to work just fine. So let's add a way to enable this sharing again, so we don't introduce unnecessary regressions over older versions of Qemu. Had a patch for that, got shoot down for reasons I don't remember, attached for reference. It basically allows IRQ sharing in case the two devices sharing the IRQ are of the same kind. In that case you usually have a single guest driver handling both devices and IRQ sharing works most of the time. I don't mind much which approach we take ... I'm very indifferent myself :). This is not an issue we should waste too much time/effort on. Alex
[Qemu-devel] Re: [RFC] prep: enable irq sharing on ide again
On 2011-02-14 12:22, Gerd Hoffmann wrote: On 02/08/11 00:26, Alexander Graf wrote: The new ISA infrastructure checks for potential irq sharing bugs on interrupt lines, because usually irq lines on isa can't be shared. The PREP spec however mandates that the irq lines for both IDE ports are shared and according to Aurelien this also used to work just fine. So let's add a way to enable this sharing again, so we don't introduce unnecessary regressions over older versions of Qemu. Had a patch for that, got shoot down for reasons I don't remember, attached for reference. It basically allows IRQ sharing in case the two devices sharing the IRQ are of the same kind. In that case you usually have a single guest driver handling both devices and IRQ sharing works most of the time. Yeah, 3x -serial XXX is also broken for x86 targets as the third device shares its IRQ with the first - like on real HW... Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v2] PS/2 keyboard Scancode Set 3 support
Am 13.02.2011 11:07, schrieb Roy Tam: The following patch adds PS/2 keyboard Scancode Set 3 support. Sign-off-by: Roy Tam roy...@gmail.com -- v2: checkpatch.pl style fixes diff --git a/hw/ps2.c b/hw/ps2.c index 762bb00..6bea0ef 100644 --- a/hw/ps2.c +++ b/hw/ps2.c @@ -143,13 +143,85 @@ static void ps2_put_keycode(void *opaque, int keycode) { PS2KbdState *s = opaque; -/* XXX: add support for scancode sets 1 and 3 */ -if (!s-translate keycode 0xe0 s-scancode_set == 2) - { +/* XXX: add support for scancode sets 1 */ +if (!s-translate keycode 0xe0 s-scancode_set 1) { if (keycode 0x80) ps2_queue(s-common, 0xf0); keycode = ps2_raw_keycode[keycode 0x7f]; - } +if (s-scancode_set == 3) { +switch (keycode) { +case 0x1: +keycode = 0x47; +break; +case 0x3: +keycode = 0x27; +break; +case 0x4: +keycode = 0x17; +break; +case 0x5: +keycode = 0x7; +break; +case 0x6: +keycode = 0xf; +break; +case 0x7: +keycode = 0x5e; +break; +case 0x9: +keycode = 0x4f; +break; +case 0xa: +keycode = 0x3f; +break; +case 0xb: +keycode = 0x2f; +break; +case 0xc: +keycode = 0x1f; +break; +case 0x11: +keycode = 0x19; +break; +case 0x14: +keycode = 0x11; +break; +case 0x58: +keycode = 0x14; +break; +case 0x5d: +keycode = 0x5c; +break; +case 0x76: +keycode = 0x8; +break; +case 0x77: +keycode = 0x76; +break; +case 0x78: +keycode = 0x56; +break; +case 0x79: +keycode = 0x7c; +break; +case 0x7b: +keycode = 0x84; +break; +case 0x7c: +keycode = 0x7e; +break; +case 0x7e: +keycode = 0x5f; +break; +case 0x83: +keycode = 0x37; +break; +case 0x84: +keycode = 0x57; +break; +} +} +} ps2_queue(s-common, keycode); } Wouldn't a second table like ps2_raw_keycode be better than a huge switch block that translates from scancode set 2 to 3? Kevin
[Qemu-devel] Re: Ping: [PATCH] Handle icount for powerpc tbl/tbu/decr load and store.
On Mon, Feb 14, 2011 at 12:34:05PM +0100, Alexander Graf wrote: Tristan Gingold wrote: Potential reviewers CC: On Feb 8, 2011, at 10:59 AM, Tristan Gingold wrote: Handle option '-icount X' on powerpc targets. Signed-off-by: Tristan Gingold ging...@adacore.com Braces are broken. Edgar knows his way around icount a lot better than me - I've never actually used it. Edgar, any comments? AFAICS, the patch looks OK (except for the indentation). Cheers
[Qemu-devel] Re: Ping: [PATCH] Handle icount for powerpc tbl/tbu/decr load and store.
On Feb 14, 2011, at 12:46 PM, Edgar E. Iglesias wrote: On Mon, Feb 14, 2011 at 12:34:05PM +0100, Alexander Graf wrote: Tristan Gingold wrote: Potential reviewers CC: On Feb 8, 2011, at 10:59 AM, Tristan Gingold wrote: Handle option '-icount X' on powerpc targets. Signed-off-by: Tristan Gingold ging...@adacore.com Braces are broken. Edgar knows his way around icount a lot better than me - I've never actually used it. Edgar, any comments? AFAICS, the patch looks OK (except for the indentation). Thanks. New version soon.
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
On 02/14/2011 03:50 AM, Kevin Wolf wrote: Am 13.02.2011 19:08, schrieb Anthony Liguori: Hi, In my QAPI branch[1], I've now got almost every existing QMP command converted with (hopefully) all of the hard problems solved. There is only one remaining thing to attack before posting for inclusion and that's events. Here's my current thinking about what to do. I've had a quick look at your branch and I have one general question: Is there a specific reason why you duplicate existing monitor handlers instead of converting the existing ones? Are the old ones still used? It's just temporary. It makes it very easy to test code side-by-side. By the time I submit, I'll remove the old code. Proposal for events in QAPI For QAPI, I'd like to model events on the notion of signals and slots[2]. A client would explicitly connect to a signal through a QMP interface which would result in a slot being added that then generates an event. Internally in QEMU, we could also connect slots to the same signals. Since we don't have an object API in QMP, we'd use a pair of connect/disconnect functions that had enough information to identify the signal. Example: We would define QEVENT_BLOCK_IO_EVENT as: # qmp-schema.json { 'BlockIOEvent': {'device': 'str', 'action': 'str', 'operation': 'str'} } The 'Event' suffix is used to determine that the type is an event and not a structure. This would generate the following structures for QEMU: typedef void (BlockIOEventFunc)(const char *device, const char *action, const char *operation, void *opaque); Why is an event not a structure? For one it makes things inconsistent (you have this 'Event' suffix magic), and it's not even convenient. The parameter list of the BlockIOEventFunc might become very long. At the moment you have three const char* there and I think it's only going to grow - who is supposed to remember the right order of arguments? So why not make the event a struct and have a typedef void (BlockIOEventFunc)(BlockIOEvent *evt)? A signal is a function call. You can pass a structure as a parameter is you so desire but the natural thing to do is pass position arguments. If you've got a ton of signal arguments, it's probably an indication that you're doing something wrong. typedef struct BlockIOEvent { /* contents are private */ } BlockIOEvent; /* Connect a slot to a BlockIOEvent signal and return a handle to the connection */ int block_io_event_connect(BlockIOEvent *event, BlockIOEventFunc *cb, void *opaque); /* Signal any connect slots of a BlockIOEvent */ void block_io_event_signal(BlockIOEvent *event, const char *device, const char *action, const char *operation); /* Disconnect a slot from a signal based on a connection handle */ void block_io_event_disconnect(BlockIOEvent *event, int handle); In the case of BlockIOEvent, this is a global signal that is not tied to any specific object so there would be a global BlockIOEvent object within QEMU. From a QMP perspective, we would have the following function in the schema: [ 'block-io-event', {}, {}, 'BlockIOEvent' ] Not sure what this list is supposed to tell me... I see that you use the same for command, so I guess the first one is something like an command/event name, the second seems to be the arguments, last could be the type of the return value, the third no idea.' Sorry, first is the name of the function, second is required arguments, third is optional arguments, last is return value. So would an event look like a response to a command that was never issued? From a protocol perspective, events look like this today: { event: BlockIOError, data: { device: ide1-cd0, operation: read, action: report } } The only change to the protocol I'm proposing is adding a tag field which would give us: { event: BlockIOError, tag: event023234, data: { device: ide1-cd0, operation: read, action: report } } Another Example Here's an example of BlockDeviceEvent, the successor to BlockIOEvent. It makes use of signal accessor arguments and QAPI enumeration support: So are we going to drop BlockIOEvent (or any other event that will be extended) or will both old and new version continue to exist and we always signal both of them? Both have to exist. We can't drop old events without breaking backwards compatibility. Regards, Anthony Liguori Kevin
[Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc '
On 02/14/2011 04:57 AM, Gerd Hoffmann wrote: On 01/31/11 21:43, Anthony Liguori wrote: commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a regression in the change vnc password command that changed the behavior of setting the VNC password to an empty string from disabling login to disabling authentication. This commit refactors the code to eliminate this overloaded semantics in vnc_display_password and instead introduces the vnc_display_disable_login. The monitor implementation then determines the behavior of an empty or missing string. Hmm, now about simply never ever changing vs-auth? If auth is none and you do a vnc change password then if we don't set vs-auth to vnc, it won't have the desired effect. I really dislike the semantics of this command but that was a past mistake. Regards, Anthony Liguori cheers, Gerd
[Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc '
On Mon, Feb 14, 2011 at 06:10:04AM -0600, Anthony Liguori wrote: On 02/14/2011 04:57 AM, Gerd Hoffmann wrote: On 01/31/11 21:43, Anthony Liguori wrote: commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a regression in the change vnc password command that changed the behavior of setting the VNC password to an empty string from disabling login to disabling authentication. This commit refactors the code to eliminate this overloaded semantics in vnc_display_password and instead introduces the vnc_display_disable_login. The monitor implementation then determines the behavior of an empty or missing string. Hmm, now about simply never ever changing vs-auth? If auth is none and you do a vnc change password then if we don't set vs-auth to vnc, it won't have the desired effect. I really dislike the semantics of this command but that was a past mistake. Actually blindly setting 'vs-auth' to 'vnc' is also a security flaw. If using the VeNCrypt security method, then 'vs-auth' will be VENCRYPT and the 'vs-subauth' will possibly indicate the 'VNC' sub-auth scheme. So we really do want the change password command to leave 'vs-auth' alone completely - just change the password string, with no side effects on auth methods. If an app intends to use the change password command it will have already launched QEMU with neccessary -vnc flags to set the desired vs-auth and vs-subauth methods. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
Am 14.02.2011 13:03, schrieb Anthony Liguori: On 02/14/2011 03:50 AM, Kevin Wolf wrote: Am 13.02.2011 19:08, schrieb Anthony Liguori: Proposal for events in QAPI For QAPI, I'd like to model events on the notion of signals and slots[2]. A client would explicitly connect to a signal through a QMP interface which would result in a slot being added that then generates an event. Internally in QEMU, we could also connect slots to the same signals. Since we don't have an object API in QMP, we'd use a pair of connect/disconnect functions that had enough information to identify the signal. Example: We would define QEVENT_BLOCK_IO_EVENT as: # qmp-schema.json { 'BlockIOEvent': {'device': 'str', 'action': 'str', 'operation': 'str'} } The 'Event' suffix is used to determine that the type is an event and not a structure. This would generate the following structures for QEMU: typedef void (BlockIOEventFunc)(const char *device, const char *action, const char *operation, void *opaque); Why is an event not a structure? For one it makes things inconsistent (you have this 'Event' suffix magic), and it's not even convenient. The parameter list of the BlockIOEventFunc might become very long. At the moment you have three const char* there and I think it's only going to grow - who is supposed to remember the right order of arguments? So why not make the event a struct and have a typedef void (BlockIOEventFunc)(BlockIOEvent *evt)? A signal is a function call. You can pass a structure as a parameter is you so desire but the natural thing to do is pass position arguments. If you've got a ton of signal arguments, it's probably an indication that you're doing something wrong. Yes. For example, you're taking tons of independent arguments for something that is logically a single entity, namely a block error event. ;-) I'm almost sure that we'll want to add more things to this specific event, for example a more detailed error description (Luiz once suggested using errno here, but seems it hasn't made its way into upstream). And I would be surprised if we never wanted to add more information to other events, too. typedef struct BlockIOEvent { /* contents are private */ } BlockIOEvent; /* Connect a slot to a BlockIOEvent signal and return a handle to the connection */ int block_io_event_connect(BlockIOEvent *event, BlockIOEventFunc *cb, void *opaque); /* Signal any connect slots of a BlockIOEvent */ void block_io_event_signal(BlockIOEvent *event, const char *device, const char *action, const char *operation); /* Disconnect a slot from a signal based on a connection handle */ void block_io_event_disconnect(BlockIOEvent *event, int handle); In the case of BlockIOEvent, this is a global signal that is not tied to any specific object so there would be a global BlockIOEvent object within QEMU. From a QMP perspective, we would have the following function in the schema: [ 'block-io-event', {}, {}, 'BlockIOEvent' ] Not sure what this list is supposed to tell me... I see that you use the same for command, so I guess the first one is something like an command/event name, the second seems to be the arguments, last could be the type of the return value, the third no idea.' Sorry, first is the name of the function, second is required arguments, third is optional arguments, last is return value. So would an event look like a response to a command that was never issued? From a protocol perspective, events look like this today: { event: BlockIOError, data: { device: ide1-cd0, operation: read, action: report } } The only change to the protocol I'm proposing is adding a tag field which would give us: { event: BlockIOError, tag: event023234, data: { device: ide1-cd0, operation: read, action: report } } Right, I was more referring to this schema thing. I didn't quite understand yet if/why functions and events are the same thing from that perspective. They seem to be some kind of function that is called from within qemu, gets its arguments from the event_connect call and returns its return value to the QMP client. Is that about right? Another Example Here's an example of BlockDeviceEvent, the successor to BlockIOEvent. It makes use of signal accessor arguments and QAPI enumeration support: So are we going to drop BlockIOEvent (or any other event that will be extended) or will both old and new version continue to exist and we always signal both of them? Both have to exist. We can't drop old events without breaking backwards compatibility. Right. So a second event doesn't sound like something we'd love to have... Maybe extending the existing ones with optional arguments would be better? Kevin
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
On Mon, 14 Feb 2011 13:32:45 +0100 Kevin Wolf kw...@redhat.com wrote: Am 14.02.2011 13:03, schrieb Anthony Liguori: On 02/14/2011 03:50 AM, Kevin Wolf wrote: Am 13.02.2011 19:08, schrieb Anthony Liguori: Proposal for events in QAPI For QAPI, I'd like to model events on the notion of signals and slots[2]. A client would explicitly connect to a signal through a QMP interface which would result in a slot being added that then generates an event. Internally in QEMU, we could also connect slots to the same signals. Since we don't have an object API in QMP, we'd use a pair of connect/disconnect functions that had enough information to identify the signal. Example: We would define QEVENT_BLOCK_IO_EVENT as: # qmp-schema.json { 'BlockIOEvent': {'device': 'str', 'action': 'str', 'operation': 'str'} } The 'Event' suffix is used to determine that the type is an event and not a structure. This would generate the following structures for QEMU: typedef void (BlockIOEventFunc)(const char *device, const char *action, const char *operation, void *opaque); Why is an event not a structure? For one it makes things inconsistent (you have this 'Event' suffix magic), and it's not even convenient. The parameter list of the BlockIOEventFunc might become very long. At the moment you have three const char* there and I think it's only going to grow - who is supposed to remember the right order of arguments? So why not make the event a struct and have a typedef void (BlockIOEventFunc)(BlockIOEvent *evt)? A signal is a function call. You can pass a structure as a parameter is you so desire but the natural thing to do is pass position arguments. If you've got a ton of signal arguments, it's probably an indication that you're doing something wrong. Yes. For example, you're taking tons of independent arguments for something that is logically a single entity, namely a block error event. ;-) I'm almost sure that we'll want to add more things to this specific event, for example a more detailed error description (Luiz once suggested using errno here, but seems it hasn't made its way into upstream). And I would be surprised if we never wanted to add more information to other events, too. So the question is: how does the schema based design support extending commands or events? Does it require adding new commands/events? While the current code is in really in bad shape currently, I'm not sure that having this disadvantage will pay off the new design.
[Qemu-devel] [Bug 706766] Re: [Qemu-kvm] qemu-img fail to create qcow image
Fixed patch in qemu-kvm: 96df67d1c3928704cd76d0b2e76372ef18658e85, close this bug. ** Changed in: qemu Status: Confirmed = Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/706766 Title: [Qemu-kvm] qemu-img fail to create qcow image Status in QEMU: Fix Committed Bug description: On qemu-kvm tree 6f32e3d09d990fd50008756fcb446b55e0c0af79, complier it. Then try to create a qcow image with a exist raw image, create fail. reproduce steps: 1) build qemu-kvm 2) ./qemu-img create -b ./ia32e_fc10.img -f qcow2 ./qcow.img 3) Fail to create qcow image, it show error qemu-img: Could not open './qcow.img'
Re: [Qemu-devel] [PATCH v2] PS/2 keyboard Scancode Set 3 support
2011/2/14 Kevin Wolf kw...@redhat.com: Am 13.02.2011 11:07, schrieb Roy Tam: The following patch adds PS/2 keyboard Scancode Set 3 support. Sign-off-by: Roy Tam roy...@gmail.com -- v2: checkpatch.pl style fixes diff --git a/hw/ps2.c b/hw/ps2.c index 762bb00..6bea0ef 100644 --- a/hw/ps2.c +++ b/hw/ps2.c @@ -143,13 +143,85 @@ static void ps2_put_keycode(void *opaque, int keycode) { PS2KbdState *s = opaque; - /* XXX: add support for scancode sets 1 and 3 */ - if (!s-translate keycode 0xe0 s-scancode_set == 2) - { + /* XXX: add support for scancode sets 1 */ + if (!s-translate keycode 0xe0 s-scancode_set 1) { if (keycode 0x80) ps2_queue(s-common, 0xf0); keycode = ps2_raw_keycode[keycode 0x7f]; - } + if (s-scancode_set == 3) { + switch (keycode) { + case 0x1: + keycode = 0x47; + break; + case 0x3: + keycode = 0x27; + break; + case 0x4: + keycode = 0x17; + break; + case 0x5: + keycode = 0x7; + break; + case 0x6: + keycode = 0xf; + break; + case 0x7: + keycode = 0x5e; + break; + case 0x9: + keycode = 0x4f; + break; + case 0xa: + keycode = 0x3f; + break; + case 0xb: + keycode = 0x2f; + break; + case 0xc: + keycode = 0x1f; + break; + case 0x11: + keycode = 0x19; + break; + case 0x14: + keycode = 0x11; + break; + case 0x58: + keycode = 0x14; + break; + case 0x5d: + keycode = 0x5c; + break; + case 0x76: + keycode = 0x8; + break; + case 0x77: + keycode = 0x76; + break; + case 0x78: + keycode = 0x56; + break; + case 0x79: + keycode = 0x7c; + break; + case 0x7b: + keycode = 0x84; + break; + case 0x7c: + keycode = 0x7e; + break; + case 0x7e: + keycode = 0x5f; + break; + case 0x83: + keycode = 0x37; + break; + case 0x84: + keycode = 0x57; + break; + } + } + } ps2_queue(s-common, keycode); } Wouldn't a second table like ps2_raw_keycode be better than a huge switch block that translates from scancode set 2 to 3? Yeah, but I hate fixing old coding style to newer one. But still, I will do it for this time. I just wonder why not all sources are converted to new coding style when new coding style was announced. Roy Kevin
[Qemu-devel] [PATCH v3] PS/2 keyboard Scancode Set 3 support
The following patch adds PS/2 keyboard Scancode Set 3 support. Sign-off-by: Roy Tam roy...@gmail.com -- v3: change from switch to array, more style fixes v2: checkpatch.pl style fixes diff --git a/hw/ps2.c b/hw/ps2.c index 762bb00..da4737a 100644 --- a/hw/ps2.c +++ b/hw/ps2.c @@ -110,14 +110,24 @@ typedef struct { /* Table to convert from PC scancodes to raw scancodes. */ static const unsigned char ps2_raw_keycode[128] = { - 0,118, 22, 30, 38, 37, 46, 54, 61, 62, 70, 69, 78, 85,102, 13, - 21, 29, 36, 45, 44, 53, 60, 67, 68, 77, 84, 91, 90, 20, 28, 27, - 35, 43, 52, 51, 59, 66, 75, 76, 82, 14, 18, 93, 26, 34, 33, 42, - 50, 49, 58, 65, 73, 74, 89,124, 17, 41, 88, 5, 6, 4, 12, 3, - 11, 2, 10, 1, 9,119,126,108,117,125,123,107,115,116,121,105, -114,122,112,113,127, 96, 97,120, 7, 15, 23, 31, 39, 47, 55, 63, - 71, 79, 86, 94, 8, 16, 24, 32, 40, 48, 56, 64, 72, 80, 87,111, - 19, 25, 57, 81, 83, 92, 95, 98, 99,100,101,103,104,106,109,110 + 0, 118, 22, 30, 38, 37, 46, 54, 61, 62, 70, 69, 78, 85, 102, 13, + 21, 29, 36, 45, 44, 53, 60, 67, 68, 77, 84, 91, 90, 20, 28, 27, + 35, 43, 52, 51, 59, 66, 75, 76, 82, 14, 18, 93, 26, 34, 33, 42, + 50, 49, 58, 65, 73, 74, 89, 124, 17, 41, 88, 5, 6, 4, 12, 3, + 11, 2, 10, 1, 9, 119, 126, 108, 117, 125, 123, 107, 115, 116, 121, 105, +114, 122, 112, 113, 127, 96, 97, 120, 7, 15, 23, 31, 39, 47, 55, 63, + 71, 79, 86, 94, 8, 16, 24, 32, 40, 48, 56, 64, 72, 80, 87, 111, + 19, 25, 57, 81, 83, 92, 95, 98, 99, 100, 101, 103, 104, 106, 109, 110 +}; +static const unsigned char ps2_raw_keycode_set3[128] = { + 0, 8, 22, 30, 38, 37, 46, 54, 61, 62, 70, 69, 78, 85, 102, 13, + 21, 29, 36, 45, 44, 53, 60, 67, 68, 77, 84, 91, 90, 17, 28, 27, + 35, 43, 52, 51, 59, 66, 75, 76, 82, 14, 18, 92, 26, 34, 33, 42, + 50, 49, 58, 65, 73, 74, 89, 126, 25, 41, 20, 7, 15, 23, 31, 39, + 47, 2, 63, 71, 79, 118, 95, 108, 117, 125, 132, 107, 115, 116, 124, 105, +114, 122, 112, 113, 127, 96, 97, 86, 94, 15, 23, 31, 39, 47, 55, 63, + 71, 79, 86, 94, 8, 16, 24, 32, 40, 48, 56, 64, 72, 80, 87, 111, + 19, 25, 57, 81, 83, 92, 95, 98, 99, 100, 101, 103, 104, 106, 109, 110 }; void ps2_queue(void *opaque, int b) @@ -144,11 +154,15 @@ static void ps2_put_keycode(void *opaque, int keycode) PS2KbdState *s = opaque; /* XXX: add support for scancode sets 1 and 3 */ -if (!s-translate keycode 0xe0 s-scancode_set == 2) - { -if (keycode 0x80) +if (!s-translate keycode 0xe0 s-scancode_set 1) { +if (keycode 0x80) { ps2_queue(s-common, 0xf0); -keycode = ps2_raw_keycode[keycode 0x7f]; +} +if (s-scancode_set == 2) { +keycode = ps2_raw_keycode[keycode 0x7f]; +} else if (s-scancode_set == 3) { +keycode = ps2_raw_keycode_set3[keycode 0x7f]; +} } ps2_queue(s-common, keycode); } ps2_scancode_set3.patch Description: Binary data
Re: [Qemu-devel] [PATCH v2] PS/2 keyboard Scancode Set 3 support
Am 14.02.2011 13:49, schrieb Roy Tam: 2011/2/14 Kevin Wolf kw...@redhat.com: Am 13.02.2011 11:07, schrieb Roy Tam: The following patch adds PS/2 keyboard Scancode Set 3 support. Sign-off-by: Roy Tam roy...@gmail.com -- v2: checkpatch.pl style fixes diff --git a/hw/ps2.c b/hw/ps2.c index 762bb00..6bea0ef 100644 --- a/hw/ps2.c +++ b/hw/ps2.c @@ -143,13 +143,85 @@ static void ps2_put_keycode(void *opaque, int keycode) { PS2KbdState *s = opaque; -/* XXX: add support for scancode sets 1 and 3 */ -if (!s-translate keycode 0xe0 s-scancode_set == 2) - { +/* XXX: add support for scancode sets 1 */ +if (!s-translate keycode 0xe0 s-scancode_set 1) { if (keycode 0x80) ps2_queue(s-common, 0xf0); keycode = ps2_raw_keycode[keycode 0x7f]; - } +if (s-scancode_set == 3) { +switch (keycode) { +case 0x1: +keycode = 0x47; +break; +case 0x3: +keycode = 0x27; +break; +case 0x4: +keycode = 0x17; +break; +case 0x5: +keycode = 0x7; +break; +case 0x6: +keycode = 0xf; +break; +case 0x7: +keycode = 0x5e; +break; +case 0x9: +keycode = 0x4f; +break; +case 0xa: +keycode = 0x3f; +break; +case 0xb: +keycode = 0x2f; +break; +case 0xc: +keycode = 0x1f; +break; +case 0x11: +keycode = 0x19; +break; +case 0x14: +keycode = 0x11; +break; +case 0x58: +keycode = 0x14; +break; +case 0x5d: +keycode = 0x5c; +break; +case 0x76: +keycode = 0x8; +break; +case 0x77: +keycode = 0x76; +break; +case 0x78: +keycode = 0x56; +break; +case 0x79: +keycode = 0x7c; +break; +case 0x7b: +keycode = 0x84; +break; +case 0x7c: +keycode = 0x7e; +break; +case 0x7e: +keycode = 0x5f; +break; +case 0x83: +keycode = 0x37; +break; +case 0x84: +keycode = 0x57; +break; +} +} +} ps2_queue(s-common, keycode); } Wouldn't a second table like ps2_raw_keycode be better than a huge switch block that translates from scancode set 2 to 3? Yeah, but I hate fixing old coding style to newer one. But still, I will do it for this time. I just wonder why not all sources are converted to new coding style when new coding style was announced. Not sure what you're referring to, how is this related to coding style? Kevin
Re: [Qemu-devel] [PATCH v3] PS/2 keyboard Scancode Set 3 support
Am 14.02.2011 13:59, schrieb Roy Tam: The following patch adds PS/2 keyboard Scancode Set 3 support. Sign-off-by: Roy Tam roy...@gmail.com -- v3: change from switch to array, more style fixes v2: checkpatch.pl style fixes Looks much better now. :-) Kevin
Re: [Qemu-devel] [PATCH v2] PS/2 keyboard Scancode Set 3 support
2011/2/14 Kevin Wolf kw...@redhat.com: Am 14.02.2011 13:49, schrieb Roy Tam: 2011/2/14 Kevin Wolf kw...@redhat.com: Am 13.02.2011 11:07, schrieb Roy Tam: The following patch adds PS/2 keyboard Scancode Set 3 support. Sign-off-by: Roy Tam roy...@gmail.com -- v2: checkpatch.pl style fixes diff --git a/hw/ps2.c b/hw/ps2.c index 762bb00..6bea0ef 100644 --- a/hw/ps2.c +++ b/hw/ps2.c @@ -143,13 +143,85 @@ static void ps2_put_keycode(void *opaque, int keycode) { PS2KbdState *s = opaque; - /* XXX: add support for scancode sets 1 and 3 */ - if (!s-translate keycode 0xe0 s-scancode_set == 2) - { + /* XXX: add support for scancode sets 1 */ + if (!s-translate keycode 0xe0 s-scancode_set 1) { if (keycode 0x80) ps2_queue(s-common, 0xf0); keycode = ps2_raw_keycode[keycode 0x7f]; - } + if (s-scancode_set == 3) { + switch (keycode) { + case 0x1: + keycode = 0x47; + break; + case 0x3: + keycode = 0x27; + break; + case 0x4: + keycode = 0x17; + break; + case 0x5: + keycode = 0x7; + break; + case 0x6: + keycode = 0xf; + break; + case 0x7: + keycode = 0x5e; + break; + case 0x9: + keycode = 0x4f; + break; + case 0xa: + keycode = 0x3f; + break; + case 0xb: + keycode = 0x2f; + break; + case 0xc: + keycode = 0x1f; + break; + case 0x11: + keycode = 0x19; + break; + case 0x14: + keycode = 0x11; + break; + case 0x58: + keycode = 0x14; + break; + case 0x5d: + keycode = 0x5c; + break; + case 0x76: + keycode = 0x8; + break; + case 0x77: + keycode = 0x76; + break; + case 0x78: + keycode = 0x56; + break; + case 0x79: + keycode = 0x7c; + break; + case 0x7b: + keycode = 0x84; + break; + case 0x7c: + keycode = 0x7e; + break; + case 0x7e: + keycode = 0x5f; + break; + case 0x83: + keycode = 0x37; + break; + case 0x84: + keycode = 0x57; + break; + } + } + } ps2_queue(s-common, keycode); } Wouldn't a second table like ps2_raw_keycode be better than a huge switch block that translates from scancode set 2 to 3? Yeah, but I hate fixing old coding style to newer one. But still, I will do it for this time. I just wonder why not all sources are converted to new coding style when new coding style was announced. Not sure what you're referring to, how is this related to coding style? Newly submitted patch are necessary to pass scripts/checkpatch.pl test. scripts/checkpatch.pl check the patch if it use new coding style. But not all original source code are converted to new coding style, and I have to do it myself. You can have a look on patch v3, I need to take some more time on making correct amount of spaces in the old array to make whole patch having consistent, new coding style. I highly suggest doing a whole coding style review on existing source codes of QEMU. Kevin
[Qemu-devel] Re: [RFC] qapi: events in QMP
On Sun, 13 Feb 2011 12:08:04 -0600 Anthony Liguori anth...@codemonkey.ws wrote: Hi, In my QAPI branch[1], I've now got almost every existing QMP command converted with (hopefully) all of the hard problems solved. There is only one remaining thing to attack before posting for inclusion and that's events. Here's my current thinking about what to do. Events in QMP Today QMP events are asynchronous messages. They are not tied explicitly to any type of context, do not have a well defined format, and are have no mechanism to mask or filter events. As of right now, we have somewhere around a dozen events. Goals of QAPI 1) Make all interfaces consumable in C such that we can use the interfaces in QEMU 2) Make all interfaces exposed through a library using code generation from static introspection 3) Make all interfaces well specified in a formal schema Proposal for events in QAPI For QAPI, I'd like to model events on the notion of signals and slots[2]. A client would explicitly connect to a signal through a QMP interface which would result in a slot being added that then generates an event. Internally in QEMU, we could also connect slots to the same signals. Since we don't have an object API in QMP, we'd use a pair of connect/disconnect functions that had enough information to identify the signal. This seems to be the right way to do this in C, but I wonder if it will get complex/bloated to require this on the wire protocol. In the initial discussions on QMP events, we decided that it was simpler/easier to just send all events and let the client do the masking if it wants to. Later on, Daniel commented that it would be useful to able to mask events early on.. Now, this proposal will require registration. We don't seem to make our mind.. Daniel, what do you think? Example: We would define QEVENT_BLOCK_IO_EVENT as: I won't comment on the code right now, I want to read it in detail in your branch, so that I can do a better review. Will try to do it in the next few days. My only immediate concern is that, as I commented on the other email, this new model will require us to add new commands/events when extending existing commands/events, right?
[Qemu-devel] Re: [RFC] qapi: events in QMP
On Mon, Feb 14, 2011 at 11:28:52AM -0200, Luiz Capitulino wrote: On Sun, 13 Feb 2011 12:08:04 -0600 Anthony Liguori anth...@codemonkey.ws wrote: Hi, In my QAPI branch[1], I've now got almost every existing QMP command converted with (hopefully) all of the hard problems solved. There is only one remaining thing to attack before posting for inclusion and that's events. Here's my current thinking about what to do. Events in QMP Today QMP events are asynchronous messages. They are not tied explicitly to any type of context, do not have a well defined format, and are have no mechanism to mask or filter events. As of right now, we have somewhere around a dozen events. Goals of QAPI 1) Make all interfaces consumable in C such that we can use the interfaces in QEMU 2) Make all interfaces exposed through a library using code generation from static introspection 3) Make all interfaces well specified in a formal schema Proposal for events in QAPI For QAPI, I'd like to model events on the notion of signals and slots[2]. A client would explicitly connect to a signal through a QMP interface which would result in a slot being added that then generates an event. Internally in QEMU, we could also connect slots to the same signals. Since we don't have an object API in QMP, we'd use a pair of connect/disconnect functions that had enough information to identify the signal. This seems to be the right way to do this in C, but I wonder if it will get complex/bloated to require this on the wire protocol. In the initial discussions on QMP events, we decided that it was simpler/easier to just send all events and let the client do the masking if it wants to. Later on, Daniel commented that it would be useful to able to mask events early on.. Now, this proposal will require registration. We don't seem to make our mind.. Daniel, what do you think? I've always thought it was bad to have all events enabled all the time, because it is not scalable as the number frequency of events increases. Requiring the app register for each event it cares about is more scalable also allows the app to discover if a particular event is actually supported by that QEMU version. eg The initial 'qmp_capabilities' greeting from the client could include the list of all desired event names from the client, and the server could reply with the list of those event it can actually support has enabled. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc '
On 02/14/11 13:10, Anthony Liguori wrote: On 02/14/2011 04:57 AM, Gerd Hoffmann wrote: On 01/31/11 21:43, Anthony Liguori wrote: commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a regression in the change vnc password command that changed the behavior of setting the VNC password to an empty string from disabling login to disabling authentication. This commit refactors the code to eliminate this overloaded semantics in vnc_display_password and instead introduces the vnc_display_disable_login. The monitor implementation then determines the behavior of an empty or missing string. Hmm, now about simply never ever changing vs-auth? If auth is none and you do a vnc change password then if we don't set vs-auth to vnc, it won't have the desired effect. If you want a password-protected vnc session you should better explicitly say so using '-vnc :0,password', otherwise you'll have a window (between qemu start and setting the password) where vnc clients can connect without a password. Going from none to vnc automagically when setting a password encourages this insecure way to enable password protection. IMHO we should stop doing this. There are backward compatibility issues though as qemu did this for quite some time ... Going from vnc to none automagically when setting a empty password is a no-go from a security point of view, especially as older qemu versions did *not* do that. I don't think we'll need a monitor command to switch authentication methods on the fly. YMMV. cheers, Gerd
[Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc '
On 02/14/2011 06:24 AM, Daniel P. Berrange wrote: On Mon, Feb 14, 2011 at 06:10:04AM -0600, Anthony Liguori wrote: On 02/14/2011 04:57 AM, Gerd Hoffmann wrote: On 01/31/11 21:43, Anthony Liguori wrote: commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a regression in the change vnc password command that changed the behavior of setting the VNC password to an empty string from disabling login to disabling authentication. This commit refactors the code to eliminate this overloaded semantics in vnc_display_password and instead introduces the vnc_display_disable_login. The monitor implementation then determines the behavior of an empty or missing string. Hmm, now about simply never ever changing vs-auth? If auth is none and you do a vnc change password then if we don't set vs-auth to vnc, it won't have the desired effect. I really dislike the semantics of this command but that was a past mistake. Actually blindly setting 'vs-auth' to 'vnc' is also a security flaw. But this is the semantics of the command. I agree it's stupid but a security flaw is a regression and this is not a regression. This is why the set-password command no longer does any of this nonsense. If using the VeNCrypt security method, then 'vs-auth' will be VENCRYPT and the 'vs-subauth' will possibly indicate the 'VNC' sub-auth scheme. So we really do want the change password command to leave 'vs-auth' alone completely - just change the password string, with no side effects on auth methods. If an app intends to use the change password command it will have already launched QEMU with neccessary -vnc flags to set the desired vs-auth and vs-subauth methods. I think I see how this could work but I'm not sure it's worth doing. I'd rather just leave the (bad) semantics of this command alone and deprecate the interface. Regards, Anthony Liguori Regards, Daniel
[Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc '
On 02/14/2011 07:56 AM, Gerd Hoffmann wrote: On 02/14/11 13:10, Anthony Liguori wrote: On 02/14/2011 04:57 AM, Gerd Hoffmann wrote: On 01/31/11 21:43, Anthony Liguori wrote: commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a regression in the change vnc password command that changed the behavior of setting the VNC password to an empty string from disabling login to disabling authentication. This commit refactors the code to eliminate this overloaded semantics in vnc_display_password and instead introduces the vnc_display_disable_login. The monitor implementation then determines the behavior of an empty or missing string. Hmm, now about simply never ever changing vs-auth? If auth is none and you do a vnc change password then if we don't set vs-auth to vnc, it won't have the desired effect. If you want a password-protected vnc session you should better explicitly say so using '-vnc :0,password', otherwise you'll have a window (between qemu start and setting the password) where vnc clients can connect without a password. Going from none to vnc automagically when setting a password encourages this insecure way to enable password protection. IMHO we should stop doing this. There are backward compatibility issues though as qemu did this for quite some time ... Yeah, change vnc is deprecated, set-password has sane semantics. Going from vnc to none automagically when setting a empty password is a no-go from a security point of view, especially as older qemu versions did *not* do that. ] Yup, this was the reason for the CVE. I don't think we'll need a monitor command to switch authentication methods on the fly. YMMV. Actually, I do, but that's far off on my radar screen. One thing we're noticing is that QEMU is not terribly forgiving in a hosting environment. If you make a mistake configuring something (like use the wrong auth type), you're only recourse is restarting the guest with the new options. Restarting a guest of a paying customer == unhappy customer. The more we can change without restarting a guest the better IMHO. Regards, Anthony Liguori cheers, Gerd
[Qemu-devel] Re: [RFC] qapi: events in QMP
On 02/14/2011 07:33 AM, Daniel P. Berrange wrote: On Mon, Feb 14, 2011 at 11:28:52AM -0200, Luiz Capitulino wrote: On Sun, 13 Feb 2011 12:08:04 -0600 Anthony Liguorianth...@codemonkey.ws wrote: Hi, In my QAPI branch[1], I've now got almost every existing QMP command converted with (hopefully) all of the hard problems solved. There is only one remaining thing to attack before posting for inclusion and that's events. Here's my current thinking about what to do. Events in QMP Today QMP events are asynchronous messages. They are not tied explicitly to any type of context, do not have a well defined format, and are have no mechanism to mask or filter events. As of right now, we have somewhere around a dozen events. Goals of QAPI 1) Make all interfaces consumable in C such that we can use the interfaces in QEMU 2) Make all interfaces exposed through a library using code generation from static introspection 3) Make all interfaces well specified in a formal schema Proposal for events in QAPI For QAPI, I'd like to model events on the notion of signals and slots[2]. A client would explicitly connect to a signal through a QMP interface which would result in a slot being added that then generates an event. Internally in QEMU, we could also connect slots to the same signals. Since we don't have an object API in QMP, we'd use a pair of connect/disconnect functions that had enough information to identify the signal. This seems to be the right way to do this in C, but I wonder if it will get complex/bloated to require this on the wire protocol. In the initial discussions on QMP events, we decided that it was simpler/easier to just send all events and let the client do the masking if it wants to. Later on, Daniel commented that it would be useful to able to mask events early on.. Now, this proposal will require registration. We don't seem to make our mind.. Daniel, what do you think? I've always thought it was bad to have all events enabled all the time, because it is not scalable as the number frequency of events increases. Requiring the app register for each event it cares about is more scalable also allows the app to discover if a particular event is actually supported by that QEMU version. Exactly. eg The initial 'qmp_capabilities' greeting from the client could include the list of all desired event names from the client, and the server could reply with the list of those event it can actually support has enabled. The trouble is that if we want to have events associated with particular objects, then this method doesn't really work well. For instance, if we had some high frequency event for block drivers, I may want to register the event for a specific device only for a short period of time. Having a simple mask/unmask global event doesn't really do that well. I think a key to success in QMP is to make the API usable in QEMU and I think we need that ability for it to be useful in QEMU. Regards, Anthony Liguori Daniel
[Qemu-devel] Re: Migration of WinXP Guest - usb+network failure
On 02/08/11 14:32, Peter Lieven wrote: Hi, is there any known issue when migrating a WinXP SP3 guest with qemu-kvm 0.13.0 or qemu-kvm-0.12.5? If I migrate such a guest with a Realtek rtl8139 Network Device and an USB Mouse Tablet after migration the USB Tablet doesn't work any more and network stalls. I have seen the mouse moving a few seconds after migration and that stop moving. usb devices simply don't have migration support. The 0.14 release will add that for usb hid devices and the usb hub. Whenever the devices work after migration depends on how the guest os handles usb errors. Recent linux kernels just reset+reinitialize hid devices which fail, log a message about it and go on without a hitch. Other guests might be more sensitive, so this could be it, but maybe it isn't. No idea on the rtl issue. cheers, Gerd
[Qemu-devel] Re: [RFC] qapi: events in QMP
On 02/14/2011 07:28 AM, Luiz Capitulino wrote: On Sun, 13 Feb 2011 12:08:04 -0600 Anthony Liguorianth...@codemonkey.ws wrote: Hi, In my QAPI branch[1], I've now got almost every existing QMP command converted with (hopefully) all of the hard problems solved. There is only one remaining thing to attack before posting for inclusion and that's events. Here's my current thinking about what to do. Events in QMP Today QMP events are asynchronous messages. They are not tied explicitly to any type of context, do not have a well defined format, and are have no mechanism to mask or filter events. As of right now, we have somewhere around a dozen events. Goals of QAPI 1) Make all interfaces consumable in C such that we can use the interfaces in QEMU 2) Make all interfaces exposed through a library using code generation from static introspection 3) Make all interfaces well specified in a formal schema Proposal for events in QAPI For QAPI, I'd like to model events on the notion of signals and slots[2]. A client would explicitly connect to a signal through a QMP interface which would result in a slot being added that then generates an event. Internally in QEMU, we could also connect slots to the same signals. Since we don't have an object API in QMP, we'd use a pair of connect/disconnect functions that had enough information to identify the signal. This seems to be the right way to do this in C, but I wonder if it will get complex/bloated to require this on the wire protocol. It adds a bunch of new RPC functions, but I don't see a better way. In the initial discussions on QMP events, we decided that it was simpler/easier to just send all events and let the client do the masking if it wants to. Later on, Daniel commented that it would be useful to able to mask events early on.. Now, this proposal will require registration. We don't seem to make our mind.. Daniel, what do you think? Example: We would define QEVENT_BLOCK_IO_EVENT as: I won't comment on the code right now, I want to read it in detail in your branch, so that I can do a better review. Will try to do it in the next few days. My only immediate concern is that, as I commented on the other email, this new model will require us to add new commands/events when extending existing commands/events, right? No, optional parameters are supported. This changes the structure size and function signature which means that libqmp has to bump it's version number but from a wire protocol perspective, a new and/or libqmp will work just fine with all versions of QEMU that support QMP. The version bump of libqmp is surely undesirable though so we should restrict these type of changes. It's very hard to make this type of change in a compatible way regardless of C though so that's generally true. Regards, Anthony Liguori
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
On 02/14/2011 06:45 AM, Luiz Capitulino wrote: So the question is: how does the schema based design support extending commands or events? Does it require adding new commands/events? Well, let me ask you, how do we do that today? Let's say that I want to add a new parameter to the `change' function so that I can include a salt parameter as part of the password. The way we'd do this today is by checking for the 'salt' parameter in qdict, and if it's not present, use a random salt or something like that. However, if I'm a QMP client, how can I tell whether you're going to ignore my salt parameter or actually use it? Nothing in QMP tells me this today. If I set the salt parameter in the `change' command, I'll just get a success message. Even if we expose a schema, but leave things as-is, having to parse the schema as part of a function call is pretty horrible, particularly if distros do silly things like backport some optional parameters and not others. If those optional parameters are deeply nested in a structure, it's even worse. OTOH, if we introduce a new command to set the password with a salt, it becomes very easy for the client to support. The do something as simple as: if qmp.has_command(vnc-set-password-with-salt): qmp.vnc_set_password_with_salt('foobar', 'X*') else: window.set_weak_security_icon(True) qmp.vnc_set_password('foobar') Now you could answer, hey, we can add capabilities then those capabilities can quickly get out of hand. Regards, Anthony Liguori While the current code is in really in bad shape currently, I'm not sure that having this disadvantage will pay off the new design.
[Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc '
On Mon, Feb 14, 2011 at 08:14:07AM -0600, Anthony Liguori wrote: On 02/14/2011 06:24 AM, Daniel P. Berrange wrote: On Mon, Feb 14, 2011 at 06:10:04AM -0600, Anthony Liguori wrote: On 02/14/2011 04:57 AM, Gerd Hoffmann wrote: On 01/31/11 21:43, Anthony Liguori wrote: commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a regression in the change vnc password command that changed the behavior of setting the VNC password to an empty string from disabling login to disabling authentication. This commit refactors the code to eliminate this overloaded semantics in vnc_display_password and instead introduces the vnc_display_disable_login. The monitor implementation then determines the behavior of an empty or missing string. Hmm, now about simply never ever changing vs-auth? If auth is none and you do a vnc change password then if we don't set vs-auth to vnc, it won't have the desired effect. I really dislike the semantics of this command but that was a past mistake. Actually blindly setting 'vs-auth' to 'vnc' is also a security flaw. But this is the semantics of the command. I agree it's stupid but a security flaw is a regression and this is not a regression. This is why the set-password command no longer does any of this nonsense. If using the VeNCrypt security method, then 'vs-auth' will be VENCRYPT and the 'vs-subauth' will possibly indicate the 'VNC' sub-auth scheme. So we really do want the change password command to leave 'vs-auth' alone completely - just change the password string, with no side effects on auth methods. If an app intends to use the change password command it will have already launched QEMU with neccessary -vnc flags to set the desired vs-auth and vs-subauth methods. I think I see how this could work but I'm not sure it's worth doing. I'd rather just leave the (bad) semantics of this command alone and deprecate the interface. If you make it blindly set 'vs-auth = VNC' then you haven't fully fixed the security flawed, because you will be downgrading from 'vencrypt+vnc' auth to just 'vnc' which loose all your data encryption capabilities. Just reverting the previous commit 52c18be9e99dabe295321153fda7fce9f76647ac fully addresses the problems leaving the command side-effect free as it was originally designed implemented. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] Remote Desktop integration
I was wondering if there is any documentation on what the original source for the VNC/RDP/Spice/etc remote disktop servers and what changes were needed to integrate with Qemu. I'm in the process of setting up a Virtual server and I noticed that my favorite Remote Desktop server is not one of the options[freeNX...or is it called NX server]...so time permitting I'd like to use it instead[though of course for my virtual linux boxes, I CAN use it by simply installing it ON the servers]
[Qemu-devel] Re: Problem with DOS application and 286 DOS Extender application
On Sun, Feb 13, 2011 at 03:06:44PM +0100, Gerhard Wiesinger wrote: Hello, After some fortune I found out that also Turbo Debugger 286 doesn't work under plain DOS 6.22 (without any memory mananger just pressing F5) or with some memory mananagers (HIMEM.SYS, EMM386, QEMM386). Error message is: Error 266 loading D:\DIR\TD286.EXE into extended memory. So it looks like that there is a major issue with extended memory. Any ideas how to fix or how to find the problem and fix it? Version is latest seabios and QEMU from git as of now (own builds). FYI - I took a quick look at this. It does not appear to be SeaBIOS related as SeaBIOS under Bochs works fine. Qemu fails for me as reported above. Kvm (AMD) also fails for me with a slightly different set of error messages (error 269 instead of 266). I noticed that under Bochs I get this message as it runs (not sure if it is meaningful): 00383234030i[CPU0 ] TASK SWITCH: switching to the same TSS ! Under qemu, I see a report of a cpu exception during execution of the command (again, not sure if it is meaningful). I grabbed the qemu execution log around the exception if anyone wishes to take a look at it. -Kevin IN: 0x00107694: pop%cx 0x00107695: mov0x5(%si),%al 0x00107698: and$0xfd,%al 0x0010769a: mov%al,0x5(%si) 0x0010769d: xor%ax,%ax 0x0010769f: mov%ax,-0x8(%bp) 0x001076a2: mov-0x2(%bp),%ax 0x001076a5: mov%ax,-0x6(%bp) 0x001076a8: ltr%ax 0x001076ab: push %cx 0x001076ac: mov%ax,%cx 0x001076ae: call 0x105530 EAX=0158 EBX=8000 ECX=0158 EDX=41b0 ESI=0148 EDI=0278 EBP=0589 ESP=057d EIP=2cf0 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0028 0001b940 2f1f 9300 DPL=0 DS16 [-WA] CS =0020 00102840 9a00 DPL=0 CS16 [-R-] SS =0150 0010b5e0 05ce 9300 DPL=0 DS16 [-WA] DS =0008 0001b440 03ff 9300 DPL=0 DS16 [-WA] FS =0010 0001b940 9300 DPL=0 DS16 [-WA] GS =0010 0001b940 9300 DPL=0 DS16 [-WA] LDT= 8200 DPL=0 LDT TR =0158 0010bbc0 05ce 8100 DPL=0 TSS16-avl GDT= 0001b440 03ff IDT= 00100010 07ff CR0=0013 CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 CCS=00a4 CCD= CCO=LOGICW EFER= EAX=0158 EBX=8000 ECX=0158 EDX=41b0 ESI=0158 EDI=0278 EBP=0589 ESP=057f EIP=4e71 EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0028 0001b940 2f1f 9300 DPL=0 DS16 [-WA] CS =0020 00102840 9a00 DPL=0 CS16 [-R-] SS =0150 0010b5e0 05ce 9300 DPL=0 DS16 [-WA] DS =0008 0001b440 03ff 9300 DPL=0 DS16 [-WA] FS =0010 0001b940 9300 DPL=0 DS16 [-WA] GS =0010 0001b940 9300 DPL=0 DS16 [-WA] LDT= 8200 DPL=0 LDT TR =0158 0010bbc0 05ce 8100 DPL=0 TSS16-avl GDT= 0001b440 03ff IDT= 00100010 07ff CR0=0013 CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 CCS=00ac CCD=0158 CCO=SHLW EFER= IN: 0x001076b1: pop%cx 0x001076b2: mov0x5(%si),%al 0x001076b5: and$0xfd,%al 0x001076b7: mov%al,0x5(%si) 0x001076ba: ljmp *-0x8(%bp) check_exception old: 0x new 0xa 0: v=0a e=0160 i=0 cpl=0 IP=0160: pc= SP=0168:05c4 EAX= EAX= EBX= ECX= EDX= ESI= EDI= EBP= ESP=05c4 EIP= EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES = CS =0160 SS =0168 DS =0170 FS = GS = LDT=0168 0010c1a0 7fff 8200 DPL=0 LDT TR =0158 0010bbc0 05ce 8100 DPL=0 TSS16-avl GDT= 0001b440 03ff IDT= 00100010 07ff CR0=001b CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 CCS= CCD=0081 CCO=LOGICB EFER= EAX= EBX= ECX= EDX= ESI= EDI= EBP= ESP=05bc EIP=3ae8 EFL=0086 [--S--P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES = CS =0020 00102840 9a00 DPL=0 CS16 [-R-] SS =0168 DS =0170 FS = GS = LDT=0168
[Qemu-devel] [PATCH 29/37] kvm: Separate TCG from KVM cpu execution
From: Jan Kiszka jan.kis...@siemens.com Mixing up TCG bits with KVM already led to problems around eflags emulation on x86. Moreover, quite some code that TCG requires on cpu enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and kvm_cpu_exec as early as possible. The core logic of cpu_halted from cpu_exec is added to kvm_arch_process_irqchip_events. Moving away from cpu_exec makes exception_index meaningless for KVM, we can simply pass the exit reason directly (only EXCP_DEBUG vs. rest is relevant). Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpu-exec.c| 19 ++- cpus.c| 10 +- kvm-all.c | 19 +-- target-i386/kvm.c |6 +++--- 4 files changed, 23 insertions(+), 31 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 9c0b10d..b03b3a7 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -226,13 +226,11 @@ int cpu_exec(CPUState *env1) } #if defined(TARGET_I386) -if (!kvm_enabled()) { -/* put eflags in CPU temporary format */ -CC_SRC = env-eflags (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); -DF = 1 - (2 * ((env-eflags 10) 1)); -CC_OP = CC_OP_EFLAGS; -env-eflags = ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); -} +/* put eflags in CPU temporary format */ +CC_SRC = env-eflags (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); +DF = 1 - (2 * ((env-eflags 10) 1)); +CC_OP = CC_OP_EFLAGS; +env-eflags = ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); #elif defined(TARGET_SPARC) #elif defined(TARGET_M68K) env-cc_op = CC_OP_FLAGS; @@ -257,7 +255,7 @@ int cpu_exec(CPUState *env1) if (setjmp(env-jmp_env) == 0) { #if defined(__sparc__) !defined(CONFIG_SOLARIS) #undef env -env = cpu_single_env; +env = cpu_single_env; #define env cpu_single_env #endif /* if an exception is pending, we execute it here */ @@ -316,11 +314,6 @@ int cpu_exec(CPUState *env1) } } -if (kvm_enabled()) { -kvm_cpu_exec(env); -longjmp(env-jmp_env, 1); -} - next_tb = 0; /* force lookup of first TB */ for(;;) { interrupt_request = env-interrupt_request; diff --git a/cpus.c b/cpus.c index c7e86c2..468544c 100644 --- a/cpus.c +++ b/cpus.c @@ -800,8 +800,6 @@ static void qemu_kvm_wait_io_event(CPUState *env) qemu_wait_io_event_common(env); } -static int qemu_cpu_exec(CPUState *env); - static void *qemu_kvm_cpu_thread_fn(void *arg) { CPUState *env = arg; @@ -829,7 +827,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) while (1) { if (cpu_can_run(env)) { -r = qemu_cpu_exec(env); +r = kvm_cpu_exec(env); if (r == EXCP_DEBUG) { cpu_handle_debug_exception(env); } @@ -1040,7 +1038,7 @@ void vm_stop(int reason) #endif -static int qemu_cpu_exec(CPUState *env) +static int tcg_cpu_exec(CPUState *env) { int ret; #ifdef CONFIG_PROFILER @@ -1095,9 +1093,11 @@ bool cpu_exec_all(void) break; } if (cpu_can_run(env)) { -r = qemu_cpu_exec(env); if (kvm_enabled()) { +r = kvm_cpu_exec(env); qemu_kvm_eat_signals(env); +} else { +r = tcg_cpu_exec(env); } if (r == EXCP_DEBUG) { cpu_handle_debug_exception(env); diff --git a/kvm-all.c b/kvm-all.c index 19cf188..802c6b8 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -895,10 +895,11 @@ int kvm_cpu_exec(CPUState *env) if (kvm_arch_process_irqchip_events(env)) { env-exit_request = 0; -env-exception_index = EXCP_HLT; -return 0; +return EXCP_HLT; } +cpu_single_env = env; + do { if (env-kvm_vcpu_dirty) { kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE); @@ -927,7 +928,6 @@ int kvm_cpu_exec(CPUState *env) kvm_flush_coalesced_mmio_buffer(); if (ret == -EINTR || ret == -EAGAIN) { -cpu_exit(env); DPRINTF(io window exit\n); ret = 0; break; @@ -978,8 +978,8 @@ int kvm_cpu_exec(CPUState *env) DPRINTF(kvm_exit_debug\n); #ifdef KVM_CAP_SET_GUEST_DEBUG if (kvm_arch_debug(run-debug.arch)) { -env-exception_index = EXCP_DEBUG; -return 0; +ret = EXCP_DEBUG; +goto out; } /* re-enter, this exception was guest-internal */ ret = 1; @@ -995,13 +995,12 @@ int kvm_cpu_exec(CPUState *env) if (ret 0) { cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); vm_stop(VMSTOP_PANIC); -env-exit_request = 1; -} -if (env-exit_request) { -env-exit_request = 0; -
[Qemu-devel] [PATCH 05/37] Leave inner main_loop faster on pending requests
From: Jan Kiszka jan.kis...@siemens.com If there is any pending request that requires us to leave the inner loop if main_loop, makes sure we do this as soon as possible by enforcing non-blocking IO processing. At this change, move variable definitions out of the inner loop to improve readability. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- vl.c | 11 +++ 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/vl.c b/vl.c index 30263d6..9c628f0 100644 --- a/vl.c +++ b/vl.c @@ -1402,18 +1402,21 @@ qemu_irq qemu_system_powerdown; static void main_loop(void) { +bool nonblocking = false; +#ifdef CONFIG_PROFILER +int64_t ti; +#endif int r; qemu_main_loop_start(); for (;;) { do { -bool nonblocking = false; -#ifdef CONFIG_PROFILER -int64_t ti; -#endif #ifndef CONFIG_IOTHREAD nonblocking = cpu_exec_all(); +if (!vm_can_run()) { +nonblocking = true; +} #endif #ifdef CONFIG_PROFILER ti = profile_getclock(); -- 1.7.4
[Qemu-devel] [PATCH 24/37] Refactor cpu_has_work/any_cpu_has_work in cpus.c
From: Jan Kiszka jan.kis...@siemens.com Avoid duplicate use of the function name cpu_has_work, it's confusing, also their scope. Refactor cpu_has_work to cpu_thread_is_idle and do the same with any_cpu_has_work. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 43 +++ 1 files changed, 23 insertions(+), 20 deletions(-) diff --git a/cpus.c b/cpus.c index d54ec7d..e963208 100644 --- a/cpus.c +++ b/cpus.c @@ -137,29 +137,30 @@ static int cpu_can_run(CPUState *env) return 1; } -static int cpu_has_work(CPUState *env) +static bool cpu_thread_is_idle(CPUState *env) { -if (env-stop) -return 1; -if (env-queued_work_first) -return 1; -if (env-stopped || !vm_running) -return 0; -if (!env-halted) -return 1; -if (qemu_cpu_has_work(env)) -return 1; -return 0; +if (env-stop || env-queued_work_first) { +return false; +} +if (env-stopped || !vm_running) { +return true; +} +if (!env-halted || qemu_cpu_has_work(env)) { +return false; +} +return true; } -static int any_cpu_has_work(void) +static bool all_cpu_threads_idle(void) { CPUState *env; -for (env = first_cpu; env != NULL; env = env-next_cpu) -if (cpu_has_work(env)) -return 1; -return 0; +for (env = first_cpu; env != NULL; env = env-next_cpu) { +if (!cpu_thread_is_idle(env)) { +return false; +} +} +return true; } static void cpu_debug_handler(CPUState *env) @@ -743,8 +744,9 @@ static void qemu_tcg_wait_io_event(void) { CPUState *env; -while (!any_cpu_has_work()) +while (all_cpu_threads_idle()) { qemu_cond_timedwait(tcg_halt_cond, qemu_global_mutex, 1000); +} qemu_mutex_unlock(qemu_global_mutex); @@ -765,8 +767,9 @@ static void qemu_tcg_wait_io_event(void) static void qemu_kvm_wait_io_event(CPUState *env) { -while (!cpu_has_work(env)) +while (cpu_thread_is_idle(env)) { qemu_cond_timedwait(env-halt_cond, qemu_global_mutex, 1000); +} qemu_kvm_eat_signals(env); qemu_wait_io_event_common(env); @@ -1070,7 +1073,7 @@ bool cpu_exec_all(void) } } exit_request = 0; -return any_cpu_has_work(); +return !all_cpu_threads_idle(); } void set_numa_modes(void) -- 1.7.4
[Qemu-devel] [PATCH 25/37] Fix a few coding style violations in cpus.c
From: Jan Kiszka jan.kis...@siemens.com No functional changes. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 71 +++ 1 files changed, 44 insertions(+), 27 deletions(-) diff --git a/cpus.c b/cpus.c index e963208..802d15a 100644 --- a/cpus.c +++ b/cpus.c @@ -130,10 +130,12 @@ static void do_vm_stop(int reason) static int cpu_can_run(CPUState *env) { -if (env-stop) +if (env-stop) { return 0; -if (env-stopped || !vm_running) +} +if (env-stopped || !vm_running) { return 0; +} return 1; } @@ -225,9 +227,9 @@ static void qemu_event_increment(void) static const uint64_t val = 1; ssize_t ret; -if (io_thread_fd == -1) +if (io_thread_fd == -1) { return; - +} do { ret = write(io_thread_fd, val, sizeof(val)); } while (ret 0 errno == EINTR); @@ -258,17 +260,17 @@ static int qemu_event_init(void) int fds[2]; err = qemu_eventfd(fds); -if (err == -1) +if (err == -1) { return -errno; - +} err = fcntl_setfl(fds[0], O_NONBLOCK); -if (err 0) +if (err 0) { goto fail; - +} err = fcntl_setfl(fds[1], O_NONBLOCK); -if (err 0) +if (err 0) { goto fail; - +} qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL, (void *)(unsigned long)fds[0]); @@ -527,7 +529,6 @@ void pause_all_vcpus(void) void qemu_cpu_kick(void *env) { -return; } void qemu_cpu_kick_self(void) @@ -660,13 +661,15 @@ int qemu_init_main_loop(void) blocked_signals = block_io_signals(); ret = qemu_signalfd_init(blocked_signals); -if (ret) +if (ret) { return ret; +} /* Note eventfd must be drained before signalfd handlers run */ ret = qemu_event_init(); -if (ret) +if (ret) { return ret; +} qemu_cond_init(qemu_pause_cond); qemu_cond_init(qemu_system_cond); @@ -696,10 +699,11 @@ void run_on_cpu(CPUState *env, void (*func)(void *data), void *data) wi.func = func; wi.data = data; -if (!env-queued_work_first) +if (!env-queued_work_first) { env-queued_work_first = wi; -else +} else { env-queued_work_last-next = wi; +} env-queued_work_last = wi; wi.next = NULL; wi.done = false; @@ -717,8 +721,9 @@ static void flush_queued_work(CPUState *env) { struct qemu_work_item *wi; -if (!env-queued_work_first) +if (!env-queued_work_first) { return; +} while ((wi = env-queued_work_first)) { env-queued_work_first = wi-next; @@ -798,12 +803,14 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) qemu_cond_signal(qemu_cpu_cond); /* and wait for machine initialization */ -while (!qemu_system_ready) +while (!qemu_system_ready) { qemu_cond_timedwait(qemu_system_cond, qemu_global_mutex, 100); +} while (1) { -if (cpu_can_run(env)) +if (cpu_can_run(env)) { qemu_cpu_exec(env); +} qemu_kvm_wait_io_event(env); } @@ -819,13 +826,15 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) /* signal CPU creation */ qemu_mutex_lock(qemu_global_mutex); -for (env = first_cpu; env != NULL; env = env-next_cpu) +for (env = first_cpu; env != NULL; env = env-next_cpu) { env-created = 1; +} qemu_cond_signal(qemu_cpu_cond); /* and wait for machine initialization */ -while (!qemu_system_ready) +while (!qemu_system_ready) { qemu_cond_timedwait(qemu_system_cond, qemu_global_mutex, 100); +} while (1) { cpu_exec_all(); @@ -838,6 +847,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) void qemu_cpu_kick(void *_env) { CPUState *env = _env; + qemu_cond_broadcast(env-halt_cond); if (!env-thread_kicked) { qemu_thread_signal(env-thread, SIG_IPI); @@ -889,8 +899,9 @@ static int all_vcpus_paused(void) CPUState *penv = first_cpu; while (penv) { -if (!penv-stopped) +if (!penv-stopped) { return 0; +} penv = (CPUState *)penv-next_cpu; } @@ -932,14 +943,16 @@ void resume_all_vcpus(void) static void qemu_tcg_init_vcpu(void *_env) { CPUState *env = _env; + /* share a single thread for all cpus with TCG */ if (!tcg_cpu_thread) { env-thread = qemu_mallocz(sizeof(QemuThread)); env-halt_cond = qemu_mallocz(sizeof(QemuCond)); qemu_cond_init(env-halt_cond); qemu_thread_create(env-thread, qemu_tcg_cpu_thread_fn, env); -while (env-created == 0) +while (env-created == 0) { qemu_cond_timedwait(qemu_cpu_cond, qemu_global_mutex, 100); +} tcg_cpu_thread = env-thread; tcg_halt_cond = env-halt_cond; } else { @@ -954,8 +967,9 @@
[Qemu-devel] [PATCH 01/37] Prevent abortion on multiple VCPU kicks
From: Jan Kiszka jan.kis...@siemens.com If we call qemu_cpu_kick more than once before the target was able to process the signal, pthread_kill will fail, and qemu will abort. Prevent this by avoiding the redundant signal. This logic can be found in qemu-kvm as well. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpu-defs.h |1 + cpus.c |6 +- 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/cpu-defs.h b/cpu-defs.h index 8d4bf86..db809ed 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -205,6 +205,7 @@ typedef struct CPUWatchpoint { uint32_t stopped; /* Artificially stopped */\ struct QemuThread *thread; \ struct QemuCond *halt_cond; \ +int thread_kicked; \ struct qemu_work_item *queued_work_first, *queued_work_last;\ const char *cpu_model_str; \ struct KVMState *kvm_state; \ diff --git a/cpus.c b/cpus.c index 4c9928e..ab6e40e 100644 --- a/cpus.c +++ b/cpus.c @@ -481,6 +481,7 @@ static void qemu_wait_io_event_common(CPUState *env) qemu_cond_signal(qemu_pause_cond); } flush_queued_work(env); +env-thread_kicked = false; } static void qemu_tcg_wait_io_event(void) @@ -648,7 +649,10 @@ void qemu_cpu_kick(void *_env) { CPUState *env = _env; qemu_cond_broadcast(env-halt_cond); -qemu_thread_signal(env-thread, SIG_IPI); +if (!env-thread_kicked) { +qemu_thread_signal(env-thread, SIG_IPI); +env-thread_kicked = true; +} } int qemu_cpu_self(void *_env) -- 1.7.4
[Qemu-devel] [PATCH 00/37] [PULL] qemu-kvm.git uq/master queue
The following changes since commit 8668f61d20eac971d116ebbe8436b4ae963884a8: vmmouse: fix queue_size field initialization (2011-02-12 17:44:11 +) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master Anthony PERARD (1): Introduce log_start/log_stop in CPUPhysMemoryClient Glauber Costa (1): kvm: make tsc stable over migration and machine start Jan Kiszka (35): Prevent abortion on multiple VCPU kicks Stop current VCPU on synchronous reset requests Process vmstop requests in IO thread Trigger exit from cpu_exec_all on pending IO events Leave inner main_loop faster on pending requests Flatten the main loop kvm: Report proper error on GET_VCPU_MMAP_SIZE failures kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn kvm: Handle kvm_init_vcpu errors kvm: Provide sigbus services arch-independently Refactor signal setup functions in cpus.c kvm: Set up signal mask also for !CONFIG_IOTHREAD kvm: Refactor qemu_kvm_eat_signals kvm: Call qemu_kvm_eat_signals also under !CONFIG_IOTHREAD Set up signalfd under !CONFIG_IOTHREAD kvm: Fix race between timer signals and vcpu entry under !IOTHREAD kvm: Add MCE signal support for !CONFIG_IOTHREAD Introduce VCPU self-signaling service kvm: Unconditionally reenter kernel after IO exits kvm: Remove static return code of kvm_handle_io kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN Refactor kvmtcg function names in cpus.c Refactor cpu_has_work/any_cpu_has_work in cpus.c Fix a few coding style violations in cpus.c Improve vm_stop reason declarations Refactor debug and vmstop request interface Move debug exception handling out of cpu_exec kvm: Separate TCG from KVM cpu execution kvm: x86: Prepare VCPU loop for in-kernel irqchip kvm: Drop return values from kvm_arch_pre/post_run kvm: x86: Catch and report failing IRQ and NMI injections kvm: Remove unneeded memory slot reservation cirrus: Remove obsolete kvm.h include kvm: Make kvm_state globally available kvm: x86: Introduce kvmclock device to save/restore its state Makefile.objs |2 +- Makefile.target|4 +- configure |6 + cpu-all.h |6 + cpu-common.h |4 + cpu-defs.h |1 + cpu-exec.c | 43 +--- cpus.c | 725 cpus.h |3 +- exec.c | 30 +++ gdbstub.c | 19 +- hw/cirrus_vga.c|1 - hw/ide/core.c |2 +- hw/kvmclock.c | 125 + hw/kvmclock.h | 14 + hw/pc_piix.c | 32 ++- hw/scsi-disk.c |2 +- hw/vga.c | 31 ++- hw/vhost.c |2 + hw/virtio-blk.c|2 +- hw/watchdog.c |2 +- kvm-all.c | 89 --- kvm-stub.c | 15 +- kvm.h | 16 +- migration.c|2 +- monitor.c |4 +- qemu-common.h |1 + savevm.c |4 +- sysemu.h | 12 + target-i386/cpu.h |1 + target-i386/kvm.c | 112 ++--- target-ppc/kvm.c | 16 +- target-s390x/kvm.c | 16 +- vl.c | 62 +++-- 34 files changed, 921 insertions(+), 485 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.h
[Qemu-devel] [PATCH 21/37] kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN
From: Jan Kiszka jan.kis...@siemens.com The reset we issue on KVM_EXIT_SHUTDOWN implies that we should also leave the VCPU loop. As we now check for exit_request which is set by qemu_system_reset_request, this bug is no longer critical. Still it's an unneeded extra turn. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- kvm-all.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 4729ec5..42dfed8 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -963,7 +963,6 @@ int kvm_cpu_exec(CPUState *env) case KVM_EXIT_SHUTDOWN: DPRINTF(shutdown\n); qemu_system_reset_request(); -ret = 1; break; case KVM_EXIT_UNKNOWN: fprintf(stderr, KVM: unknown exit, hardware reason % PRIx64 \n, -- 1.7.4
[Qemu-devel] [PATCH 32/37] kvm: x86: Catch and report failing IRQ and NMI injections
From: Jan Kiszka jan.kis...@siemens.com We do not need to abort, but the user should be notified that weird things go on. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- target-i386/kvm.c | 16 +--- 1 files changed, 13 insertions(+), 3 deletions(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 8668cb3..0aa0a41 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1442,11 +1442,17 @@ int kvm_arch_get_registers(CPUState *env) void kvm_arch_pre_run(CPUState *env, struct kvm_run *run) { +int ret; + /* Inject NMI */ if (env-interrupt_request CPU_INTERRUPT_NMI) { env-interrupt_request = ~CPU_INTERRUPT_NMI; DPRINTF(injected NMI\n); -kvm_vcpu_ioctl(env, KVM_NMI); +ret = kvm_vcpu_ioctl(env, KVM_NMI); +if (ret 0) { +fprintf(stderr, KVM: injection failed, NMI lost (%s)\n, +strerror(-ret)); +} } if (!kvm_irqchip_in_kernel()) { @@ -1467,9 +1473,13 @@ void kvm_arch_pre_run(CPUState *env, struct kvm_run *run) struct kvm_interrupt intr; intr.irq = irq; -/* FIXME: errors */ DPRINTF(injected interrupt %d\n, irq); -kvm_vcpu_ioctl(env, KVM_INTERRUPT, intr); +ret = kvm_vcpu_ioctl(env, KVM_INTERRUPT, intr); +if (ret 0) { +fprintf(stderr, +KVM: injection failed, interrupt lost (%s)\n, +strerror(-ret)); +} } } -- 1.7.4
[Qemu-devel] [PATCH 27/37] Refactor debug and vmstop request interface
From: Jan Kiszka jan.kis...@siemens.com Instead of fiddling with debug_requested and vmstop_requested directly, introduce qemu_system_debug_request and turn qemu_system_vmstop_request into a public interface. This aligns those services with exiting ones in vl.c. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c |9 + cpus.h |2 -- sysemu.h |2 ++ vl.c | 20 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/cpus.c b/cpus.c index ca1f01d..97a6d4f 100644 --- a/cpus.c +++ b/cpus.c @@ -168,8 +168,7 @@ static bool all_cpu_threads_idle(void) static void cpu_debug_handler(CPUState *env) { gdb_set_stop_cpu(env); -debug_requested = VMSTOP_DEBUG; -vm_stop(VMSTOP_DEBUG); +qemu_system_debug_request(); } #ifdef CONFIG_LINUX @@ -990,12 +989,6 @@ void qemu_notify_event(void) qemu_event_increment(); } -static void qemu_system_vmstop_request(int reason) -{ -vmstop_requested = reason; -qemu_notify_event(); -} - void cpu_stop_current(void) { if (cpu_single_env) { diff --git a/cpus.h b/cpus.h index 4cadb64..e021126 100644 --- a/cpus.h +++ b/cpus.h @@ -11,8 +11,6 @@ void cpu_stop_current(void); /* vl.c */ extern int smp_cores; extern int smp_threads; -extern int debug_requested; -extern int vmstop_requested; void vm_state_notify(int running, int reason); bool cpu_exec_all(void); void set_numa_modes(void); diff --git a/sysemu.h b/sysemu.h index 0628d3d..0a83ab9 100644 --- a/sysemu.h +++ b/sysemu.h @@ -61,6 +61,8 @@ void cpu_disable_ticks(void); void qemu_system_reset_request(void); void qemu_system_shutdown_request(void); void qemu_system_powerdown_request(void); +void qemu_system_debug_request(void); +void qemu_system_vmstop_request(int reason); int qemu_shutdown_requested(void); int qemu_reset_requested(void); int qemu_powerdown_requested(void); diff --git a/vl.c b/vl.c index 6d2d1d3..eebe684 100644 --- a/vl.c +++ b/vl.c @@ -1217,8 +1217,8 @@ static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers = static int reset_requested; static int shutdown_requested; static int powerdown_requested; -int debug_requested; -int vmstop_requested; +static int debug_requested; +static int vmstop_requested; int qemu_shutdown_requested(void) { @@ -1312,6 +1312,18 @@ void qemu_system_powerdown_request(void) qemu_notify_event(); } +void qemu_system_debug_request(void) +{ +debug_requested = 1; +vm_stop(VMSTOP_DEBUG); +} + +void qemu_system_vmstop_request(int reason) +{ +vmstop_requested = reason; +qemu_notify_event(); +} + void main_loop_wait(int nonblocking) { IOHandlerRecord *ioh; @@ -1427,8 +1439,8 @@ static void main_loop(void) dev_time += profile_getclock() - ti; #endif -if ((r = qemu_debug_requested())) { -vm_stop(r); +if (qemu_debug_requested()) { +vm_stop(VMSTOP_DEBUG); } if (qemu_shutdown_requested()) { monitor_protocol_event(QEVENT_SHUTDOWN, NULL); -- 1.7.4
[Qemu-devel] [PATCH 06/37] Flatten the main loop
From: Jan Kiszka jan.kis...@siemens.com First of all, vm_can_run is a misnomer, it actually means no request pending. Moreover, there is no need to check all pending requests twice, the first time via the inner loop check and then again when actually processing the requests. We can simply remove the inner loop and do the checks directly. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- vl.c | 30 +++--- 1 files changed, 15 insertions(+), 15 deletions(-) diff --git a/vl.c b/vl.c index 9c628f0..c9fa266 100644 --- a/vl.c +++ b/vl.c @@ -1389,14 +1389,16 @@ void main_loop_wait(int nonblocking) } -static int vm_can_run(void) +#ifndef CONFIG_IOTHREAD +static int vm_request_pending(void) { -return !(powerdown_requested || - reset_requested || - shutdown_requested || - debug_requested || - vmstop_requested); +return powerdown_requested || + reset_requested || + shutdown_requested || + debug_requested || + vmstop_requested; } +#endif qemu_irq qemu_system_powerdown; @@ -1411,21 +1413,19 @@ static void main_loop(void) qemu_main_loop_start(); for (;;) { -do { #ifndef CONFIG_IOTHREAD -nonblocking = cpu_exec_all(); -if (!vm_can_run()) { -nonblocking = true; -} +nonblocking = cpu_exec_all(); +if (vm_request_pending()) { +nonblocking = true; +} #endif #ifdef CONFIG_PROFILER -ti = profile_getclock(); +ti = profile_getclock(); #endif -main_loop_wait(nonblocking); +main_loop_wait(nonblocking); #ifdef CONFIG_PROFILER -dev_time += profile_getclock() - ti; +dev_time += profile_getclock() - ti; #endif -} while (vm_can_run()); if ((r = qemu_debug_requested())) { vm_stop(r); -- 1.7.4
[Qemu-devel] [PATCH 13/37] kvm: Refactor qemu_kvm_eat_signals
From: Jan Kiszka jan.kis...@siemens.com We do not use the timeout, so drop its logic. As we always poll our signals, we do not need to drop the global lock. Removing those calls allows some further simplifications. Also fix the error processing of sigpending at this chance. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Reviewed-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 23 +++ 1 files changed, 7 insertions(+), 16 deletions(-) diff --git a/cpus.c b/cpus.c index f27dfbd..d9c9111 100644 --- a/cpus.c +++ b/cpus.c @@ -644,31 +644,22 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo, } } -static void qemu_kvm_eat_signal(CPUState *env, int timeout) +static void qemu_kvm_eat_signals(CPUState *env) { -struct timespec ts; -int r, e; +struct timespec ts = { 0, 0 }; siginfo_t siginfo; sigset_t waitset; sigset_t chkset; - -ts.tv_sec = timeout / 1000; -ts.tv_nsec = (timeout % 1000) * 100; +int r; sigemptyset(waitset); sigaddset(waitset, SIG_IPI); sigaddset(waitset, SIGBUS); do { -qemu_mutex_unlock(qemu_global_mutex); - r = sigtimedwait(waitset, siginfo, ts); -e = errno; - -qemu_mutex_lock(qemu_global_mutex); - -if (r == -1 !(e == EAGAIN || e == EINTR)) { -fprintf(stderr, sigtimedwait: %s\n, strerror(e)); +if (r == -1 !(errno == EAGAIN || errno == EINTR)) { +perror(sigtimedwait); exit(1); } @@ -684,7 +675,7 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout) r = sigpending(chkset); if (r == -1) { -fprintf(stderr, sigpending: %s\n, strerror(e)); +perror(sigpending); exit(1); } } while (sigismember(chkset, SIG_IPI) || sigismember(chkset, SIGBUS)); @@ -695,7 +686,7 @@ static void qemu_kvm_wait_io_event(CPUState *env) while (!cpu_has_work(env)) qemu_cond_timedwait(env-halt_cond, qemu_global_mutex, 1000); -qemu_kvm_eat_signal(env, 0); +qemu_kvm_eat_signals(env); qemu_wait_io_event_common(env); } -- 1.7.4
[Qemu-devel] [PATCH 36/37] kvm: Make kvm_state globally available
From: Jan Kiszka jan.kis...@siemens.com KVM-assisted devices need access to it but we have no clean channel to distribute a reference. As a workaround until there is a better solution, export kvm_state for global use, though use should remain restricted to the mentioned scenario. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- kvm-all.c |2 +- kvm.h |1 + 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index ecac0b3..e6a7de4 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -78,7 +78,7 @@ struct KVMState int many_ioeventfds; }; -static KVMState *kvm_state; +KVMState *kvm_state; static const KVMCapabilityInfo kvm_required_capabilites[] = { KVM_CAP_INFO(USER_MEMORY), diff --git a/kvm.h b/kvm.h index 4caa6ec..59b2c29 100644 --- a/kvm.h +++ b/kvm.h @@ -85,6 +85,7 @@ int kvm_on_sigbus(int code, void *addr); struct KVMState; typedef struct KVMState KVMState; +extern KVMState *kvm_state; int kvm_ioctl(KVMState *s, int type, ...); -- 1.7.4
[Qemu-devel] [PATCH 03/37] Process vmstop requests in IO thread
From: Jan Kiszka jan.kis...@siemens.com A pending vmstop request is also a reason to leave the inner main loop. So far we ignored it, and pending stop requests issued over VCPU threads were simply ignored. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- vl.c | 14 +- 1 files changed, 5 insertions(+), 9 deletions(-) diff --git a/vl.c b/vl.c index 5e3f7f2..30263d6 100644 --- a/vl.c +++ b/vl.c @@ -1391,15 +1391,11 @@ void main_loop_wait(int nonblocking) static int vm_can_run(void) { -if (powerdown_requested) -return 0; -if (reset_requested) -return 0; -if (shutdown_requested) -return 0; -if (debug_requested) -return 0; -return 1; +return !(powerdown_requested || + reset_requested || + shutdown_requested || + debug_requested || + vmstop_requested); } qemu_irq qemu_system_powerdown; -- 1.7.4
[Qemu-devel] [PATCH 18/37] Introduce VCPU self-signaling service
From: Jan Kiszka jan.kis...@siemens.com Introduce qemu_cpu_kick_self to send SIG_IPI to the calling VCPU context. First user will be kvm. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c| 21 + qemu-common.h |1 + 2 files changed, 22 insertions(+), 0 deletions(-) diff --git a/cpus.c b/cpus.c index 9a3fc31..6a85dc8 100644 --- a/cpus.c +++ b/cpus.c @@ -529,6 +529,17 @@ void qemu_cpu_kick(void *env) return; } +void qemu_cpu_kick_self(void) +{ +#ifndef _WIN32 +assert(cpu_single_env); + +raise(SIG_IPI); +#else +abort(); +#endif +} + void qemu_notify_event(void) { CPUState *env = cpu_single_env; @@ -831,6 +842,16 @@ void qemu_cpu_kick(void *_env) } } +void qemu_cpu_kick_self(void) +{ +assert(cpu_single_env); + +if (!cpu_single_env-thread_kicked) { +qemu_thread_signal(cpu_single_env-thread, SIG_IPI); +cpu_single_env-thread_kicked = true; +} +} + int qemu_cpu_self(void *_env) { CPUState *env = _env; diff --git a/qemu-common.h b/qemu-common.h index c7ff280..a4d9c21 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -288,6 +288,7 @@ void qemu_notify_event(void); /* Unblock cpu */ void qemu_cpu_kick(void *env); +void qemu_cpu_kick_self(void); int qemu_cpu_self(void *env); /* work queue */ -- 1.7.4
[Qemu-devel] [PATCH 26/37] Improve vm_stop reason declarations
From: Jan Kiszka jan.kis...@siemens.com Define and use dedicated constants for vm_stop reasons, they actually have nothing to do with the EXCP_* defines used so far. At this chance, specify more detailed reasons so that VM state change handlers can evaluate them. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c |4 ++-- gdbstub.c | 19 ++- hw/ide/core.c |2 +- hw/scsi-disk.c |2 +- hw/virtio-blk.c |2 +- hw/watchdog.c |2 +- kvm-all.c |2 +- migration.c |2 +- monitor.c |4 ++-- savevm.c|4 ++-- sysemu.h| 10 ++ vl.c|2 +- 12 files changed, 33 insertions(+), 22 deletions(-) diff --git a/cpus.c b/cpus.c index 802d15a..ca1f01d 100644 --- a/cpus.c +++ b/cpus.c @@ -168,8 +168,8 @@ static bool all_cpu_threads_idle(void) static void cpu_debug_handler(CPUState *env) { gdb_set_stop_cpu(env); -debug_requested = EXCP_DEBUG; -vm_stop(EXCP_DEBUG); +debug_requested = VMSTOP_DEBUG; +vm_stop(VMSTOP_DEBUG); } #ifdef CONFIG_LINUX diff --git a/gdbstub.c b/gdbstub.c index d6556c9..ed51a8a 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2194,14 +2194,14 @@ static void gdb_vm_state_change(void *opaque, int running, int reason) const char *type; int ret; -if (running || (reason != EXCP_DEBUG reason != EXCP_INTERRUPT) || -s-state == RS_INACTIVE || s-state == RS_SYSCALL) +if (running || (reason != VMSTOP_DEBUG reason != VMSTOP_USER) || +s-state == RS_INACTIVE || s-state == RS_SYSCALL) { return; - +} /* disable single step if it was enable */ cpu_single_step(env, 0); -if (reason == EXCP_DEBUG) { +if (reason == VMSTOP_DEBUG) { if (env-watchpoint_hit) { switch (env-watchpoint_hit-flags BP_MEM_ACCESS) { case BP_MEM_READ: @@ -2252,7 +2252,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) gdb_current_syscall_cb = cb; s-state = RS_SYSCALL; #ifndef CONFIG_USER_ONLY -vm_stop(EXCP_DEBUG); +vm_stop(VMSTOP_DEBUG); #endif s-state = RS_IDLE; va_start(va, fmt); @@ -2326,7 +2326,7 @@ static void gdb_read_byte(GDBState *s, int ch) if (vm_running) { /* when the CPU is running, we cannot do anything except stop it when receiving a char */ -vm_stop(EXCP_INTERRUPT); +vm_stop(VMSTOP_USER); } else #endif { @@ -2588,7 +2588,7 @@ static void gdb_chr_event(void *opaque, int event) { switch (event) { case CHR_EVENT_OPENED: -vm_stop(EXCP_INTERRUPT); +vm_stop(VMSTOP_USER); gdb_has_xml = 0; break; default: @@ -2628,8 +2628,9 @@ static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len) #ifndef _WIN32 static void gdb_sigterm_handler(int signal) { -if (vm_running) -vm_stop(EXCP_INTERRUPT); +if (vm_running) { +vm_stop(VMSTOP_USER); +} } #endif diff --git a/hw/ide/core.c b/hw/ide/core.c index dd63664..9c91a49 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -465,7 +465,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) s-bus-dma-ops-set_unit(s-bus-dma, s-unit); s-bus-dma-ops-add_status(s-bus-dma, op); bdrv_mon_event(s-bs, BDRV_ACTION_STOP, is_read); -vm_stop(0); +vm_stop(VMSTOP_DISKFULL); } else { if (op BM_STATUS_DMA_RETRY) { dma_buf_commit(s, 0); diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 488eedd..b05e654 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -239,7 +239,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) r-status |= SCSI_REQ_STATUS_RETRY | type; bdrv_mon_event(s-bs, BDRV_ACTION_STOP, is_read); -vm_stop(0); +vm_stop(VMSTOP_DISKFULL); } else { if (type == SCSI_REQ_STATUS_RETRY_READ) { r-req.bus-complete(r-req.bus, SCSI_REASON_DATA, r-req.tag, 0); diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index ffac5a4..b14fb99 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -78,7 +78,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, req-next = s-rq; s-rq = req; bdrv_mon_event(s-bs, BDRV_ACTION_STOP, is_read); -vm_stop(0); +vm_stop(VMSTOP_DISKFULL); } else { virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); bdrv_mon_event(s-bs, BDRV_ACTION_REPORT, is_read); diff --git a/hw/watchdog.c b/hw/watchdog.c index e9dd56e..1c900a1 100644 --- a/hw/watchdog.c +++ b/hw/watchdog.c @@ -132,7 +132,7 @@ void watchdog_perform_action(void) case WDT_PAUSE: /* same as 'stop' command in monitor */ watchdog_mon_event(pause); -vm_stop(0); +vm_stop(VMSTOP_WATCHDOG); break; case WDT_DEBUG: diff --git a/kvm-all.c
[Qemu-devel] [PATCH 10/37] kvm: Provide sigbus services arch-independently
From: Jan Kiszka jan.kis...@siemens.com Provide arch-independent kvm_on_sigbus* stubs to remove the #ifdef'ery from cpus.c. This patch also fixes --disable-kvm build by providing the missing kvm_on_sigbus_vcpu kvm-stub. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Reviewed-by: Paolo Bonzini pbonz...@redhat.com Acked-by: Alexander Graf ag...@suse.de Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 10 -- kvm-all.c | 10 ++ kvm-stub.c |5 + kvm.h |7 +-- target-i386/kvm.c |4 ++-- target-ppc/kvm.c | 10 ++ target-s390x/kvm.c | 10 ++ 7 files changed, 46 insertions(+), 10 deletions(-) diff --git a/cpus.c b/cpus.c index 3a72d06..4975317 100644 --- a/cpus.c +++ b/cpus.c @@ -539,10 +539,9 @@ static void sigbus_reraise(void) static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo, void *ctx) { -#if defined(TARGET_I386) -if (kvm_on_sigbus(siginfo-ssi_code, (void *)(intptr_t)siginfo-ssi_addr)) -#endif +if (kvm_on_sigbus(siginfo-ssi_code, (void *)(intptr_t)siginfo-ssi_addr)) { sigbus_reraise(); +} } static void qemu_kvm_eat_signal(CPUState *env, int timeout) @@ -575,10 +574,9 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout) switch (r) { case SIGBUS: -#ifdef TARGET_I386 -if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) -#endif +if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) { sigbus_reraise(); +} break; default: break; diff --git a/kvm-all.c b/kvm-all.c index 55f6ac3..a83aff2 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1366,3 +1366,13 @@ int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign) return -ENOSYS; #endif } + +int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr) +{ +return kvm_arch_on_sigbus_vcpu(env, code, addr); +} + +int kvm_on_sigbus(int code, void *addr) +{ +return kvm_arch_on_sigbus(code, addr); +} diff --git a/kvm-stub.c b/kvm-stub.c index 88682f2..d6b6c8e 100644 --- a/kvm-stub.c +++ b/kvm-stub.c @@ -147,6 +147,11 @@ int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool assign) return -ENOSYS; } +int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr) +{ +return 1; +} + int kvm_on_sigbus(int code, void *addr) { return 1; diff --git a/kvm.h b/kvm.h index ca57517..b2fb5c6 100644 --- a/kvm.h +++ b/kvm.h @@ -81,6 +81,9 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset); int kvm_pit_in_kernel(void); int kvm_irqchip_in_kernel(void); +int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr); +int kvm_on_sigbus(int code, void *addr); + /* internal API */ struct KVMState; @@ -121,8 +124,8 @@ int kvm_arch_init_vcpu(CPUState *env); void kvm_arch_reset_vcpu(CPUState *env); -int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr); -int kvm_on_sigbus(int code, void *addr); +int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr); +int kvm_arch_on_sigbus(int code, void *addr); struct kvm_guest_debug; struct kvm_debug_exit_arch; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 05010bb..9df8ff8 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1839,7 +1839,7 @@ static void kvm_mce_inj_srao_memscrub2(CPUState *env, target_phys_addr_t paddr) #endif -int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr) +int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr) { #if defined(KVM_CAP_MCE) void *vaddr; @@ -1889,7 +1889,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr) return 0; } -int kvm_on_sigbus(int code, void *addr) +int kvm_arch_on_sigbus(int code, void *addr) { #if defined(KVM_CAP_MCE) if ((first_cpu-mcg_cap MCG_SER_P) addr code == BUS_MCEERR_AO) { diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 710eca1..93ecc57 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -404,3 +404,13 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env) { return true; } + +int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr) +{ +return 1; +} + +int kvm_arch_on_sigbus(int code, void *addr) +{ +return 1; +} diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 38823f5..1702c46 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -505,3 +505,13 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env) { return true; } + +int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr) +{ +return 1; +} + +int kvm_arch_on_sigbus(int code, void *addr) +{ +return 1; +} -- 1.7.4
[Qemu-devel] [PATCH 12/37] kvm: Set up signal mask also for !CONFIG_IOTHREAD
From: Jan Kiszka jan.kis...@siemens.com Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. It's unused so far, but this infrastructure will be required for self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As Windows doesn't support signal services, we need to provide a stub for the init function. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 29 +++-- 1 files changed, 27 insertions(+), 2 deletions(-) diff --git a/cpus.c b/cpus.c index 62120c4..f27dfbd 100644 --- a/cpus.c +++ b/cpus.c @@ -223,11 +223,9 @@ fail: return err; } -#ifdef CONFIG_IOTHREAD static void dummy_signal(int sig) { } -#endif #else /* _WIN32 */ @@ -259,6 +257,32 @@ static void qemu_event_increment(void) #endif /* _WIN32 */ #ifndef CONFIG_IOTHREAD +static void qemu_kvm_init_cpu_signals(CPUState *env) +{ +#ifndef _WIN32 +int r; +sigset_t set; +struct sigaction sigact; + +memset(sigact, 0, sizeof(sigact)); +sigact.sa_handler = dummy_signal; +sigaction(SIG_IPI, sigact, NULL); + +sigemptyset(set); +sigaddset(set, SIG_IPI); +pthread_sigmask(SIG_BLOCK, set, NULL); + +pthread_sigmask(SIG_BLOCK, NULL, set); +sigdelset(set, SIG_IPI); +sigdelset(set, SIGBUS); +r = kvm_set_signal_mask(env, set); +if (r) { +fprintf(stderr, kvm_set_signal_mask: %s\n, strerror(-r)); +exit(1); +} +#endif +} + int qemu_init_main_loop(void) { cpu_set_debug_excp_handler(cpu_debug_handler); @@ -284,6 +308,7 @@ void qemu_init_vcpu(void *_env) fprintf(stderr, kvm_init_vcpu failed: %s\n, strerror(-r)); exit(1); } +qemu_kvm_init_cpu_signals(env); } } -- 1.7.4
[Qemu-devel] [PATCH 08/37] kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn
From: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpus.c b/cpus.c index 0abc009..8232d44 100644 --- a/cpus.c +++ b/cpus.c @@ -603,8 +603,8 @@ static void *kvm_cpu_thread_fn(void *arg) qemu_mutex_lock(qemu_global_mutex); qemu_thread_self(env-thread); -if (kvm_enabled()) -kvm_init_vcpu(env); + +kvm_init_vcpu(env); kvm_init_ipi(env); -- 1.7.4
[Qemu-devel] [PATCH 37/37] kvm: x86: Introduce kvmclock device to save/restore its state
From: Jan Kiszka jan.kis...@siemens.com If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day. Signed-off-by: Jan Kiszka jan.kis...@siemens.com CC: Glauber Costa glom...@redhat.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- Makefile.target |4 +- hw/kvmclock.c | 125 +++ hw/kvmclock.h | 14 ++ hw/pc_piix.c| 32 +++--- 4 files changed, 166 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.h diff --git a/Makefile.target b/Makefile.target index a6c30dd..5a0fd40 100644 --- a/Makefile.target +++ b/Makefile.target @@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak @@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmport.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objects diff --git a/hw/kvmclock.c b/hw/kvmclock.c new file mode 100644 index 000..b6ceddf --- /dev/null +++ b/hw/kvmclock.c @@ -0,0 +1,125 @@ +/* + * QEMU KVM support, paravirtual clock device + * + * Copyright (C) 2011 Siemens AG + * + * Authors: + * Jan Kiszkajan.kis...@siemens.com + * + * This work is licensed under the terms of the GNU GPL version 2. + * See the COPYING file in the top-level directory. + * + */ + +#include qemu-common.h +#include sysemu.h +#include sysbus.h +#include kvm.h +#include kvmclock.h + +#if defined(CONFIG_KVM_PARA) defined(KVM_CAP_ADJUST_CLOCK) + +#include linux/kvm.h +#include linux/kvm_para.h + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +} KVMClockState; + +static void kvmclock_pre_save(void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s-clock_valid) { +return; +} +ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, data); +if (ret 0) { +fprintf(stderr, KVM_GET_CLOCK failed: %s\n, strerror(ret)); +data.clock = 0; +} +s-clock = data.clock; +/* + * If the VM is stopped, declare the clock state valid to avoid re-reading + * it on next vmsave (which would return a different value). Will be reset + * when the VM is continued. + */ +s-clock_valid = !vm_running; +} + +static int kvmclock_post_load(void *opaque, int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + +data.clock = s-clock; +data.flags = 0; +return kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, data); +} + +static void kvmclock_vm_state_change(void *opaque, int running, int reason) +{ +KVMClockState *s = opaque; + +if (running) { +s-clock_valid = false; +} +} + +static int kvmclock_init(SysBusDevice *dev) +{ +KVMClockState *s = FROM_SYSBUS(KVMClockState, dev); + +qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s); +return 0; +} + +static const VMStateDescription kvmclock_vmsd = { +.name = kvmclock, +.version_id = 1, +.minimum_version_id = 1, +.minimum_version_id_old = 1, +.pre_save = kvmclock_pre_save, +.post_load = kvmclock_post_load, +.fields = (VMStateField[]) { +VMSTATE_UINT64(clock, KVMClockState), +VMSTATE_END_OF_LIST() +} +}; + +static SysBusDeviceInfo kvmclock_info = { +.qdev.name = kvmclock, +.qdev.size = sizeof(KVMClockState), +.qdev.vmsd = kvmclock_vmsd, +.qdev.no_user = 1, +.init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create(void) +{ +if (kvm_enabled() +first_cpu-cpuid_kvm_features (1ULL KVM_FEATURE_CLOCKSOURCE)) { +sysbus_create_simple(kvmclock, -1, NULL); +} +} + +static void kvmclock_register_device(void) +{ +if (kvm_enabled()) { +sysbus_register_withprop(kvmclock_info); +} +} + +device_init(kvmclock_register_device); + +#else /* !(CONFIG_KVM_PARA KVM_CAP_ADJUST_CLOCK) */ + +void kvmclock_create(void) +{ +} +#endif /* !(CONFIG_KVM_PARA KVM_CAP_ADJUST_CLOCK) */ diff --git a/hw/kvmclock.h b/hw/kvmclock.h new file mode 100644 index 000..7a83cbe --- /dev/null +++ b/hw/kvmclock.h @@ -0,0 +1,14 @@ +/* + * QEMU KVM support, paravirtual clock device + * + * Copyright (C) 2011 Siemens AG + * + * Authors: + * Jan Kiszkajan.kis...@siemens.com + * + * This work is licensed under the terms of the GNU GPL version 2. + * See the COPYING
[Qemu-devel] [PATCH 35/37] cirrus: Remove obsolete kvm.h include
From: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- hw/cirrus_vga.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 5f45b5d..2724f7b 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -31,7 +31,6 @@ #include pci.h #include console.h #include vga_int.h -#include kvm.h #include loader.h /* -- 1.7.4
[Qemu-devel] [PATCH 19/37] kvm: Unconditionally reenter kernel after IO exits
From: Jan Kiszka jan.kis...@siemens.com KVM requires to reenter the kernel after IO exits in order to complete instruction emulation. Failing to do so will leave the kernel state inconsistently behind. To ensure that we will get back ASAP, we issue a self-signal that will cause KVM_RUN to return once the pending operations are completed. We can move kvm_arch_process_irqchip_events out of the inner VCPU loop. The only state that mattered at its old place was a pending INIT request. Catch it in kvm_arch_pre_run and also trigger a self-signal to process the request on next kvm_cpu_exec. This patch also fixes the missing exit_request check in kvm_cpu_exec in the CONFIG_IOTHREAD case. Signed-off-by: Jan Kiszka jan.kis...@siemens.com CC: Gleb Natapov g...@redhat.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- kvm-all.c | 31 +-- target-i386/kvm.c |5 + 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index a83aff2..0c20f9e 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -199,7 +199,6 @@ int kvm_pit_in_kernel(void) return kvm_state-pit_in_kernel; } - int kvm_init_vcpu(CPUState *env) { KVMState *s = kvm_state; @@ -896,29 +895,33 @@ int kvm_cpu_exec(CPUState *env) DPRINTF(kvm_cpu_exec()\n); -do { -#ifndef CONFIG_IOTHREAD -if (env-exit_request) { -DPRINTF(interrupt exit requested\n); -ret = 0; -break; -} -#endif - -if (kvm_arch_process_irqchip_events(env)) { -ret = 0; -break; -} +if (kvm_arch_process_irqchip_events(env)) { +env-exit_request = 0; +env-exception_index = EXCP_HLT; +return 0; +} +do { if (env-kvm_vcpu_dirty) { kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE); env-kvm_vcpu_dirty = 0; } kvm_arch_pre_run(env, run); +if (env-exit_request) { +DPRINTF(interrupt exit requested\n); +/* + * KVM requires us to reenter the kernel after IO exits to complete + * instruction emulation. This self-signal will ensure that we + * leave ASAP again. + */ +qemu_cpu_kick_self(); +} cpu_single_env = NULL; qemu_mutex_unlock_iothread(); + ret = kvm_vcpu_ioctl(env, KVM_RUN, 0); + qemu_mutex_lock_iothread(); cpu_single_env = env; kvm_arch_post_run(env, run); diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 9df8ff8..8a87244 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1426,6 +1426,11 @@ int kvm_arch_get_registers(CPUState *env) int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) { +/* Force the VCPU out of its inner loop to process the INIT request */ +if (env-interrupt_request CPU_INTERRUPT_INIT) { +env-exit_request = 1; +} + /* Inject NMI */ if (env-interrupt_request CPU_INTERRUPT_NMI) { env-interrupt_request = ~CPU_INTERRUPT_NMI; -- 1.7.4
[Qemu-devel] [PATCH 34/37] Introduce log_start/log_stop in CPUPhysMemoryClient
From: Anthony PERARD anthony.per...@citrix.com In order to use log_start/log_stop with Xen as well in the vga code, this two operations have been put in CPUPhysMemoryClient. The two new functions cpu_physical_log_start,cpu_physical_log_stop are used in hw/vga.c and replace the kvm_log_start/stop. With this, vga does no longer depends on kvm header. [ Jan: rebasing and style fixlets ] Signed-off-by: Anthony PERARD anthony.per...@citrix.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpu-all.h|6 ++ cpu-common.h |4 exec.c | 30 ++ hw/vga.c | 31 --- hw/vhost.c |2 ++ kvm-all.c|8 ++-- kvm-stub.c | 10 -- kvm.h|3 --- 8 files changed, 64 insertions(+), 30 deletions(-) diff --git a/cpu-all.h b/cpu-all.h index ffbd6a4..87b0f86 100644 --- a/cpu-all.h +++ b/cpu-all.h @@ -959,6 +959,12 @@ int cpu_physical_memory_get_dirty_tracking(void); int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, target_phys_addr_t end_addr); +int cpu_physical_log_start(target_phys_addr_t start_addr, + ram_addr_t size); + +int cpu_physical_log_stop(target_phys_addr_t start_addr, + ram_addr_t size); + void dump_exec_info(FILE *f, fprintf_function cpu_fprintf); #endif /* !CONFIG_USER_ONLY */ diff --git a/cpu-common.h b/cpu-common.h index 6d4a898..54d21d4 100644 --- a/cpu-common.h +++ b/cpu-common.h @@ -96,6 +96,10 @@ struct CPUPhysMemoryClient { target_phys_addr_t end_addr); int (*migration_log)(struct CPUPhysMemoryClient *client, int enable); +int (*log_start)(struct CPUPhysMemoryClient *client, + target_phys_addr_t phys_addr, ram_addr_t size); +int (*log_stop)(struct CPUPhysMemoryClient *client, +target_phys_addr_t phys_addr, ram_addr_t size); QLIST_ENTRY(CPUPhysMemoryClient) list; }; diff --git a/exec.c b/exec.c index 1b70dc1..d611100 100644 --- a/exec.c +++ b/exec.c @@ -2078,6 +2078,36 @@ int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, return ret; } +int cpu_physical_log_start(target_phys_addr_t start_addr, + ram_addr_t size) +{ +CPUPhysMemoryClient *client; +QLIST_FOREACH(client, memory_client_list, list) { +if (client-log_start) { +int r = client-log_start(client, start_addr, size); +if (r 0) { +return r; +} +} +} +return 0; +} + +int cpu_physical_log_stop(target_phys_addr_t start_addr, + ram_addr_t size) +{ +CPUPhysMemoryClient *client; +QLIST_FOREACH(client, memory_client_list, list) { +if (client-log_stop) { +int r = client-log_stop(client, start_addr, size); +if (r 0) { +return r; +} +} +} +return 0; +} + static inline void tlb_update_dirty(CPUTLBEntry *tlb_entry) { ram_addr_t ram_addr; diff --git a/hw/vga.c b/hw/vga.c index e2151a2..c22b8af 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -28,7 +28,6 @@ #include vga_int.h #include pixel_ops.h #include qemu-timer.h -#include kvm.h //#define DEBUG_VGA //#define DEBUG_VGA_MEM @@ -1573,34 +1572,36 @@ static void vga_sync_dirty_bitmap(VGACommonState *s) void vga_dirty_log_start(VGACommonState *s) { -if (kvm_enabled() s-map_addr) -kvm_log_start(s-map_addr, s-map_end - s-map_addr); +if (s-map_addr) { +cpu_physical_log_start(s-map_addr, s-map_end - s-map_addr); +} -if (kvm_enabled() s-lfb_vram_mapped) { -kvm_log_start(isa_mem_base + 0xa, 0x8000); -kvm_log_start(isa_mem_base + 0xa8000, 0x8000); +if (s-lfb_vram_mapped) { +cpu_physical_log_start(isa_mem_base + 0xa, 0x8000); +cpu_physical_log_start(isa_mem_base + 0xa8000, 0x8000); } #ifdef CONFIG_BOCHS_VBE -if (kvm_enabled() s-vbe_mapped) { -kvm_log_start(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s-vram_size); +if (s-vbe_mapped) { +cpu_physical_log_start(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s-vram_size); } #endif } void vga_dirty_log_stop(VGACommonState *s) { -if (kvm_enabled() s-map_addr) - kvm_log_stop(s-map_addr, s-map_end - s-map_addr); +if (s-map_addr) { +cpu_physical_log_stop(s-map_addr, s-map_end - s-map_addr); +} -if (kvm_enabled() s-lfb_vram_mapped) { - kvm_log_stop(isa_mem_base + 0xa, 0x8000); - kvm_log_stop(isa_mem_base + 0xa8000, 0x8000); +if (s-lfb_vram_mapped) { +cpu_physical_log_stop(isa_mem_base + 0xa, 0x8000); +cpu_physical_log_stop(isa_mem_base + 0xa8000, 0x8000); } #ifdef CONFIG_BOCHS_VBE -if (kvm_enabled() s-vbe_mapped) { -
[Qemu-devel] [PATCH 31/37] kvm: Drop return values from kvm_arch_pre/post_run
From: Jan Kiszka jan.kis...@siemens.com We do not check them, and the only arch with non-empty implementations always returns 0 (this is also true for qemu-kvm). Signed-off-by: Jan Kiszka jan.kis...@siemens.com CC: Alexander Graf ag...@suse.de Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- kvm.h |5 ++--- target-i386/kvm.c |8 ++-- target-ppc/kvm.c |6 ++ target-s390x/kvm.c |6 ++ 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/kvm.h b/kvm.h index b2fb5c6..7a4d550 100644 --- a/kvm.h +++ b/kvm.h @@ -99,12 +99,11 @@ int kvm_vcpu_ioctl(CPUState *env, int type, ...); extern const KVMCapabilityInfo kvm_arch_required_capabilities[]; -int kvm_arch_post_run(CPUState *env, struct kvm_run *run); +void kvm_arch_pre_run(CPUState *env, struct kvm_run *run); +void kvm_arch_post_run(CPUState *env, struct kvm_run *run); int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run); -int kvm_arch_pre_run(CPUState *env, struct kvm_run *run); - int kvm_arch_process_irqchip_events(CPUState *env); int kvm_arch_get_registers(CPUState *env); diff --git a/target-i386/kvm.c b/target-i386/kvm.c index f02f478..8668cb3 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1440,7 +1440,7 @@ int kvm_arch_get_registers(CPUState *env) return 0; } -int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) +void kvm_arch_pre_run(CPUState *env, struct kvm_run *run) { /* Inject NMI */ if (env-interrupt_request CPU_INTERRUPT_NMI) { @@ -1486,11 +1486,9 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) DPRINTF(setting tpr\n); run-cr8 = cpu_get_apic_tpr(env-apic_state); } - -return 0; } -int kvm_arch_post_run(CPUState *env, struct kvm_run *run) +void kvm_arch_post_run(CPUState *env, struct kvm_run *run) { if (run-if_flag) { env-eflags |= IF_MASK; @@ -1499,8 +1497,6 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run) } cpu_set_apic_tpr(env-apic_state, run-cr8); cpu_set_apic_base(env-apic_state, run-apic_base); - -return 0; } int kvm_arch_process_irqchip_events(CPUState *env) diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 93ecc57..bd4012a 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) return 0; } -int kvm_arch_post_run(CPUState *env, struct kvm_run *run) +void kvm_arch_post_run(CPUState *env, struct kvm_run *run) { -return 0; } -int kvm_arch_process_irqchip_events(CPUState *env) +void kvm_arch_process_irqchip_events(CPUState *env) { -return 0; } static int kvmppc_handle_halt(CPUState *env) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 1702c46..b349812 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -169,14 +169,12 @@ int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp) return 0; } -int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) +void kvm_arch_pre_run(CPUState *env, struct kvm_run *run) { -return 0; } -int kvm_arch_post_run(CPUState *env, struct kvm_run *run) +void kvm_arch_post_run(CPUState *env, struct kvm_run *run) { -return 0; } int kvm_arch_process_irqchip_events(CPUState *env) -- 1.7.4
[Qemu-devel] [PATCH 07/37] kvm: Report proper error on GET_VCPU_MMAP_SIZE failures
From: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- kvm-all.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 2ec9e09..55f6ac3 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -219,6 +219,7 @@ int kvm_init_vcpu(CPUState *env) mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0); if (mmap_size 0) { +ret = mmap_size; DPRINTF(KVM_GET_VCPU_MMAP_SIZE failed\n); goto err; } -- 1.7.4
[Qemu-devel] [PATCH 02/37] Stop current VCPU on synchronous reset requests
From: Jan Kiszka jan.kis...@siemens.com If some I/O operation ends up calling qemu_system_reset_request in VCPU context, we record this and inform the io-thread, but we do not terminate the VCPU loop. This can lead to fairly unexpected behavior if the triggering reset operation is supposed to work synchronously. Fix this for TCG (when run in deterministic I/O mode) by setting the VCPU on stop and issuing a cpu_exit. KVM requires some more work on its VCPU loop. [ ported from qemu-kvm ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 17 + cpus.h |1 + vl.c |1 + 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cpus.c b/cpus.c index ab6e40e..f8a88e3 100644 --- a/cpus.c +++ b/cpus.c @@ -312,6 +312,10 @@ void qemu_notify_event(void) void qemu_mutex_lock_iothread(void) {} void qemu_mutex_unlock_iothread(void) {} +void cpu_stop_current(void) +{ +} + void vm_stop(int reason) { do_vm_stop(reason); @@ -852,6 +856,14 @@ static void qemu_system_vmstop_request(int reason) qemu_notify_event(); } +void cpu_stop_current(void) +{ +if (cpu_single_env) { +cpu_single_env-stopped = 1; +cpu_exit(cpu_single_env); +} +} + void vm_stop(int reason) { QemuThread me; @@ -863,10 +875,7 @@ void vm_stop(int reason) * FIXME: should not return to device code in case * vm_stop() has been requested. */ -if (cpu_single_env) { -cpu_exit(cpu_single_env); -cpu_single_env-stop = 1; -} +cpu_stop_current(); return; } do_vm_stop(reason); diff --git a/cpus.h b/cpus.h index bf4d9bb..4cadb64 100644 --- a/cpus.h +++ b/cpus.h @@ -6,6 +6,7 @@ int qemu_init_main_loop(void); void qemu_main_loop_start(void); void resume_all_vcpus(void); void pause_all_vcpus(void); +void cpu_stop_current(void); /* vl.c */ extern int smp_cores; diff --git a/vl.c b/vl.c index ed2cdfa..5e3f7f2 100644 --- a/vl.c +++ b/vl.c @@ -1296,6 +1296,7 @@ void qemu_system_reset_request(void) } else { reset_requested = 1; } +cpu_stop_current(); qemu_notify_event(); } -- 1.7.4
[Qemu-devel] [PATCH 14/37] kvm: Call qemu_kvm_eat_signals also under !CONFIG_IOTHREAD
From: Jan Kiszka jan.kis...@siemens.com Move qemu_kvm_eat_signals around and call it also when the IO-thread is not used. Do not yet process SIGBUS, will be armed in a separate step. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 90 +--- 1 files changed, 52 insertions(+), 38 deletions(-) diff --git a/cpus.c b/cpus.c index d9c9111..f2ec74c 100644 --- a/cpus.c +++ b/cpus.c @@ -227,6 +227,47 @@ static void dummy_signal(int sig) { } +static void sigbus_reraise(void); + +static void qemu_kvm_eat_signals(CPUState *env) +{ +struct timespec ts = { 0, 0 }; +siginfo_t siginfo; +sigset_t waitset; +sigset_t chkset; +int r; + +sigemptyset(waitset); +sigaddset(waitset, SIG_IPI); +sigaddset(waitset, SIGBUS); + +do { +r = sigtimedwait(waitset, siginfo, ts); +if (r == -1 !(errno == EAGAIN || errno == EINTR)) { +perror(sigtimedwait); +exit(1); +} + +switch (r) { +#ifdef CONFIG_IOTHREAD +case SIGBUS: +if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) { +sigbus_reraise(); +} +break; +#endif +default: +break; +} + +r = sigpending(chkset); +if (r == -1) { +perror(sigpending); +exit(1); +} +} while (sigismember(chkset, SIG_IPI) || sigismember(chkset, SIGBUS)); +} + #else /* _WIN32 */ HANDLE qemu_event_handle; @@ -254,6 +295,10 @@ static void qemu_event_increment(void) exit (1); } } + +static void qemu_kvm_eat_signals(CPUState *env) +{ +} #endif /* _WIN32 */ #ifndef CONFIG_IOTHREAD @@ -644,43 +689,6 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo, } } -static void qemu_kvm_eat_signals(CPUState *env) -{ -struct timespec ts = { 0, 0 }; -siginfo_t siginfo; -sigset_t waitset; -sigset_t chkset; -int r; - -sigemptyset(waitset); -sigaddset(waitset, SIG_IPI); -sigaddset(waitset, SIGBUS); - -do { -r = sigtimedwait(waitset, siginfo, ts); -if (r == -1 !(errno == EAGAIN || errno == EINTR)) { -perror(sigtimedwait); -exit(1); -} - -switch (r) { -case SIGBUS: -if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) { -sigbus_reraise(); -} -break; -default: -break; -} - -r = sigpending(chkset); -if (r == -1) { -perror(sigpending); -exit(1); -} -} while (sigismember(chkset, SIG_IPI) || sigismember(chkset, SIGBUS)); -} - static void qemu_kvm_wait_io_event(CPUState *env) { while (!cpu_has_work(env)) @@ -953,6 +961,8 @@ static int qemu_cpu_exec(CPUState *env) bool cpu_exec_all(void) { +int r; + if (next_cpu == NULL) next_cpu = first_cpu; for (; next_cpu != NULL !exit_request; next_cpu = next_cpu-next_cpu) { @@ -964,7 +974,11 @@ bool cpu_exec_all(void) if (qemu_alarm_pending()) break; if (cpu_can_run(env)) { -if (qemu_cpu_exec(env) == EXCP_DEBUG) { +r = qemu_cpu_exec(env); +if (kvm_enabled()) { +qemu_kvm_eat_signals(env); +} +if (r == EXCP_DEBUG) { break; } } else if (env-stop) { -- 1.7.4
[Qemu-devel] [PATCH 30/37] kvm: x86: Prepare VCPU loop for in-kernel irqchip
From: Jan Kiszka jan.kis...@siemens.com Effectively no functional change yet as kvm_irqchip_in_kernel still only returns 0, but this patch will allow qemu-kvm to adopt the VCPU loop of upsteam KVM. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- target-i386/kvm.c | 69 +--- 1 files changed, 38 insertions(+), 31 deletions(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 377a0a3..f02f478 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1442,11 +1442,6 @@ int kvm_arch_get_registers(CPUState *env) int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) { -/* Force the VCPU out of its inner loop to process the INIT request */ -if (env-interrupt_request CPU_INTERRUPT_INIT) { -env-exit_request = 1; -} - /* Inject NMI */ if (env-interrupt_request CPU_INTERRUPT_NMI) { env-interrupt_request = ~CPU_INTERRUPT_NMI; @@ -1454,35 +1449,43 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) kvm_vcpu_ioctl(env, KVM_NMI); } -/* Try to inject an interrupt if the guest can accept it */ -if (run-ready_for_interrupt_injection -(env-interrupt_request CPU_INTERRUPT_HARD) -(env-eflags IF_MASK)) { -int irq; - -env-interrupt_request = ~CPU_INTERRUPT_HARD; -irq = cpu_get_pic_interrupt(env); -if (irq = 0) { -struct kvm_interrupt intr; -intr.irq = irq; -/* FIXME: errors */ -DPRINTF(injected interrupt %d\n, irq); -kvm_vcpu_ioctl(env, KVM_INTERRUPT, intr); +if (!kvm_irqchip_in_kernel()) { +/* Force the VCPU out of its inner loop to process the INIT request */ +if (env-interrupt_request CPU_INTERRUPT_INIT) { +env-exit_request = 1; } -} -/* If we have an interrupt but the guest is not ready to receive an - * interrupt, request an interrupt window exit. This will - * cause a return to userspace as soon as the guest is ready to - * receive interrupts. */ -if ((env-interrupt_request CPU_INTERRUPT_HARD)) { -run-request_interrupt_window = 1; -} else { -run-request_interrupt_window = 0; -} +/* Try to inject an interrupt if the guest can accept it */ +if (run-ready_for_interrupt_injection +(env-interrupt_request CPU_INTERRUPT_HARD) +(env-eflags IF_MASK)) { +int irq; + +env-interrupt_request = ~CPU_INTERRUPT_HARD; +irq = cpu_get_pic_interrupt(env); +if (irq = 0) { +struct kvm_interrupt intr; + +intr.irq = irq; +/* FIXME: errors */ +DPRINTF(injected interrupt %d\n, irq); +kvm_vcpu_ioctl(env, KVM_INTERRUPT, intr); +} +} -DPRINTF(setting tpr\n); -run-cr8 = cpu_get_apic_tpr(env-apic_state); +/* If we have an interrupt but the guest is not ready to receive an + * interrupt, request an interrupt window exit. This will + * cause a return to userspace as soon as the guest is ready to + * receive interrupts. */ +if ((env-interrupt_request CPU_INTERRUPT_HARD)) { +run-request_interrupt_window = 1; +} else { +run-request_interrupt_window = 0; +} + +DPRINTF(setting tpr\n); +run-cr8 = cpu_get_apic_tpr(env-apic_state); +} return 0; } @@ -1502,6 +1505,10 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run) int kvm_arch_process_irqchip_events(CPUState *env) { +if (kvm_irqchip_in_kernel()) { +return 0; +} + if (env-interrupt_request (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) { env-halted = 0; } -- 1.7.4
[Qemu-devel] [PATCH 15/37] Set up signalfd under !CONFIG_IOTHREAD
From: Jan Kiszka jan.kis...@siemens.com Will be required for SIGBUS handling. For obvious reasons, this will remain a nop on Windows hosts. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Reviewed-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- Makefile.objs |2 +- cpus.c| 117 +++-- 2 files changed, 65 insertions(+), 54 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index adb5842..b21f9d3 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -141,7 +141,7 @@ common-obj-y += $(addprefix ui/, $(ui-obj-y)) common-obj-y += iov.o acl.o common-obj-$(CONFIG_THREAD) += qemu-thread.o -common-obj-$(CONFIG_IOTHREAD) += compatfd.o +common-obj-$(CONFIG_POSIX) += compatfd.o common-obj-y += notify.o event_notifier.o common-obj-y += qemu-timer.o qemu-timer-common.o diff --git a/cpus.c b/cpus.c index f2ec74c..84792f0 100644 --- a/cpus.c +++ b/cpus.c @@ -227,6 +227,59 @@ static void dummy_signal(int sig) { } +/* If we have signalfd, we mask out the signals we want to handle and then + * use signalfd to listen for them. We rely on whatever the current signal + * handler is to dispatch the signals when we receive them. + */ +static void sigfd_handler(void *opaque) +{ +int fd = (unsigned long) opaque; +struct qemu_signalfd_siginfo info; +struct sigaction action; +ssize_t len; + +while (1) { +do { +len = read(fd, info, sizeof(info)); +} while (len == -1 errno == EINTR); + +if (len == -1 errno == EAGAIN) { +break; +} + +if (len != sizeof(info)) { +printf(read from sigfd returned %zd: %m\n, len); +return; +} + +sigaction(info.ssi_signo, NULL, action); +if ((action.sa_flags SA_SIGINFO) action.sa_sigaction) { +action.sa_sigaction(info.ssi_signo, +(siginfo_t *)info, NULL); +} else if (action.sa_handler) { +action.sa_handler(info.ssi_signo); +} +} +} + +static int qemu_signalfd_init(sigset_t mask) +{ +int sigfd; + +sigfd = qemu_signalfd(mask); +if (sigfd == -1) { +fprintf(stderr, failed to create signalfd\n); +return -errno; +} + +fcntl_setfl(sigfd, O_NONBLOCK); + +qemu_set_fd_handler2(sigfd, NULL, sigfd_handler, NULL, + (void *)(unsigned long) sigfd); + +return 0; +} + static void sigbus_reraise(void); static void qemu_kvm_eat_signals(CPUState *env) @@ -330,6 +383,17 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) int qemu_init_main_loop(void) { +#ifndef _WIN32 +sigset_t blocked_signals; +int ret; + +sigemptyset(blocked_signals); + +ret = qemu_signalfd_init(blocked_signals); +if (ret) { +return ret; +} +#endif cpu_set_debug_excp_handler(cpu_debug_handler); return qemu_event_init(); @@ -426,41 +490,6 @@ static QemuCond qemu_system_cond; static QemuCond qemu_pause_cond; static QemuCond qemu_work_cond; -/* If we have signalfd, we mask out the signals we want to handle and then - * use signalfd to listen for them. We rely on whatever the current signal - * handler is to dispatch the signals when we receive them. - */ -static void sigfd_handler(void *opaque) -{ -int fd = (unsigned long) opaque; -struct qemu_signalfd_siginfo info; -struct sigaction action; -ssize_t len; - -while (1) { -do { -len = read(fd, info, sizeof(info)); -} while (len == -1 errno == EINTR); - -if (len == -1 errno == EAGAIN) { -break; -} - -if (len != sizeof(info)) { -printf(read from sigfd returned %zd: %m\n, len); -return; -} - -sigaction(info.ssi_signo, NULL, action); -if ((action.sa_flags SA_SIGINFO) action.sa_sigaction) { -action.sa_sigaction(info.ssi_signo, -(siginfo_t *)info, NULL); -} else if (action.sa_handler) { -action.sa_handler(info.ssi_signo); -} -} -} - static void cpu_signal(int sig) { if (cpu_single_env) { @@ -532,24 +561,6 @@ static sigset_t block_io_signals(void) return set; } -static int qemu_signalfd_init(sigset_t mask) -{ -int sigfd; - -sigfd = qemu_signalfd(mask); -if (sigfd == -1) { -fprintf(stderr, failed to create signalfd\n); -return -errno; -} - -fcntl_setfl(sigfd, O_NONBLOCK); - -qemu_set_fd_handler2(sigfd, NULL, sigfd_handler, NULL, - (void *)(unsigned long) sigfd); - -return 0; -} - int qemu_init_main_loop(void) { int ret; -- 1.7.4
[Qemu-devel] [PATCH 22/37] kvm: make tsc stable over migration and machine start
From: Glauber Costa glom...@redhat.com If the machine is stopped, we should not record two different tsc values upon a save operation. The same problem happens with kvmclock. But kvmclock is taking a different diretion, being now seen as a separate device. Since this is unlikely to happen with the tsc, I am taking the approach here of simply registering a handler for state change, and using a per-CPUState variable that prevents double updates for the TSC. Signed-off-by: Glauber Costa glom...@redhat.com CC: Jan Kiszka jan.kis...@web.de Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- target-i386/cpu.h |1 + target-i386/kvm.c | 18 +- 2 files changed, 18 insertions(+), 1 deletions(-) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index af701a4..5f1df8b 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -734,6 +734,7 @@ typedef struct CPUX86State { uint32_t sipi_vector; uint32_t cpuid_kvm_features; uint32_t cpuid_svm_features; +bool tsc_valid; /* in order to simplify APIC support, we leave this pointer to the user */ diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 8a87244..ba183c4 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -301,6 +301,15 @@ void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t status, #endif } +static void cpu_update_state(void *opaque, int running, int reason) +{ +CPUState *env = opaque; + +if (running) { +env-tsc_valid = false; +} +} + int kvm_arch_init_vcpu(CPUState *env) { struct { @@ -434,6 +443,8 @@ int kvm_arch_init_vcpu(CPUState *env) } #endif +qemu_add_vm_change_state_handler(cpu_update_state, env); + return kvm_vcpu_ioctl(env, KVM_SET_CPUID2, cpuid_data); } @@ -1061,7 +1072,12 @@ static int kvm_get_msrs(CPUState *env) if (has_msr_hsave_pa) { msrs[n++].index = MSR_VM_HSAVE_PA; } -msrs[n++].index = MSR_IA32_TSC; + +if (!env-tsc_valid) { +msrs[n++].index = MSR_IA32_TSC; +env-tsc_valid = !vm_running; +} + #ifdef TARGET_X86_64 if (lm_capable_kernel) { msrs[n++].index = MSR_CSTAR; -- 1.7.4
Re: [Qemu-devel] Remote Desktop integration
On Mon, Feb 14, 2011 at 3:01 PM, Gary Mort garyam...@gmail.com wrote: I was wondering if there is any documentation on what the original source for the VNC/RDP/Spice/etc remote disktop servers and what changes were needed to integrate with Qemu. I'm in the process of setting up a Virtual server and I noticed that my favorite Remote Desktop server is not one of the options[freeNX...or is it called NX server]...so time permitting I'd like to use it instead[though of course for my virtual linux boxes, I CAN use it by simply installing it ON the servers] FreeNX / NX can only be run from userspace, there is no way to implement that into QEMU. Simply install it on your guests if you want to use it. -- Corentin Chary http://xf.iksaif.net
[Qemu-devel] [PATCH 16/37] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
From: Jan Kiszka jan.kis...@siemens.com Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between checking for exit_request on vcpu entry and timer signals arriving before KVM starts to catch them. Plug it by blocking both timer related signals also on !CONFIG_IOTHREAD and process those via signalfd. As this fix depends on real signalfd support (otherwise the timer signals only kick the compat helper thread, and the main thread hangs), we need to detect the invalid constellation and abort configure. Signed-off-by: Jan Kiszka jan.kis...@siemens.com CC: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- configure |6 ++ cpus.c| 31 ++- 2 files changed, 36 insertions(+), 1 deletions(-) diff --git a/configure b/configure index 598e8e1..a3f5345 100755 --- a/configure +++ b/configure @@ -2057,6 +2057,12 @@ EOF if compile_prog ; then signalfd=yes +elif test $kvm = yes -a $io_thread != yes; then + echo + echo ERROR: Host kernel lacks signalfd() support, + echo but KVM depends on it when the IO thread is disabled. + echo + exit 1 fi # check if eventfd is supported diff --git a/cpus.c b/cpus.c index 84792f0..38dd506 100644 --- a/cpus.c +++ b/cpus.c @@ -319,6 +319,12 @@ static void qemu_kvm_eat_signals(CPUState *env) exit(1); } } while (sigismember(chkset, SIG_IPI) || sigismember(chkset, SIGBUS)); + +#ifndef CONFIG_IOTHREAD +if (sigismember(chkset, SIGIO) || sigismember(chkset, SIGALRM)) { +qemu_notify_event(); +} +#endif } #else /* _WIN32 */ @@ -368,11 +374,15 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) sigemptyset(set); sigaddset(set, SIG_IPI); +sigaddset(set, SIGIO); +sigaddset(set, SIGALRM); pthread_sigmask(SIG_BLOCK, set, NULL); pthread_sigmask(SIG_BLOCK, NULL, set); sigdelset(set, SIG_IPI); sigdelset(set, SIGBUS); +sigdelset(set, SIGIO); +sigdelset(set, SIGALRM); r = kvm_set_signal_mask(env, set); if (r) { fprintf(stderr, kvm_set_signal_mask: %s\n, strerror(-r)); @@ -381,13 +391,32 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) #endif } +#ifndef _WIN32 +static sigset_t block_synchronous_signals(void) +{ +sigset_t set; + +sigemptyset(set); +if (kvm_enabled()) { +/* + * We need to process timer signals synchronously to avoid a race + * between exit_request check and KVM vcpu entry. + */ +sigaddset(set, SIGIO); +sigaddset(set, SIGALRM); +} + +return set; +} +#endif + int qemu_init_main_loop(void) { #ifndef _WIN32 sigset_t blocked_signals; int ret; -sigemptyset(blocked_signals); +blocked_signals = block_synchronous_signals(); ret = qemu_signalfd_init(blocked_signals); if (ret) { -- 1.7.4
[Qemu-devel] [PATCH 23/37] Refactor kvmtcg function names in cpus.c
From: Jan Kiszka jan.kis...@siemens.com Pure interface cosmetics: Ensure that only kvm core services (as declared in kvm.h) start with kvm_. Prepend qemu_ to those that violate this rule in cpus.c. Also rename the corresponding tcg functions for the sake of consistency. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 16 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cpus.c b/cpus.c index 6a85dc8..d54ec7d 100644 --- a/cpus.c +++ b/cpus.c @@ -774,7 +774,7 @@ static void qemu_kvm_wait_io_event(CPUState *env) static int qemu_cpu_exec(CPUState *env); -static void *kvm_cpu_thread_fn(void *arg) +static void *qemu_kvm_cpu_thread_fn(void *arg) { CPUState *env = arg; int r; @@ -807,7 +807,7 @@ static void *kvm_cpu_thread_fn(void *arg) return NULL; } -static void *tcg_cpu_thread_fn(void *arg) +static void *qemu_tcg_cpu_thread_fn(void *arg) { CPUState *env = arg; @@ -926,7 +926,7 @@ void resume_all_vcpus(void) } } -static void tcg_init_vcpu(void *_env) +static void qemu_tcg_init_vcpu(void *_env) { CPUState *env = _env; /* share a single thread for all cpus with TCG */ @@ -934,7 +934,7 @@ static void tcg_init_vcpu(void *_env) env-thread = qemu_mallocz(sizeof(QemuThread)); env-halt_cond = qemu_mallocz(sizeof(QemuCond)); qemu_cond_init(env-halt_cond); -qemu_thread_create(env-thread, tcg_cpu_thread_fn, env); +qemu_thread_create(env-thread, qemu_tcg_cpu_thread_fn, env); while (env-created == 0) qemu_cond_timedwait(qemu_cpu_cond, qemu_global_mutex, 100); tcg_cpu_thread = env-thread; @@ -945,12 +945,12 @@ static void tcg_init_vcpu(void *_env) } } -static void kvm_start_vcpu(CPUState *env) +static void qemu_kvm_start_vcpu(CPUState *env) { env-thread = qemu_mallocz(sizeof(QemuThread)); env-halt_cond = qemu_mallocz(sizeof(QemuCond)); qemu_cond_init(env-halt_cond); -qemu_thread_create(env-thread, kvm_cpu_thread_fn, env); +qemu_thread_create(env-thread, qemu_kvm_cpu_thread_fn, env); while (env-created == 0) qemu_cond_timedwait(qemu_cpu_cond, qemu_global_mutex, 100); } @@ -962,9 +962,9 @@ void qemu_init_vcpu(void *_env) env-nr_cores = smp_cores; env-nr_threads = smp_threads; if (kvm_enabled()) -kvm_start_vcpu(env); +qemu_kvm_start_vcpu(env); else -tcg_init_vcpu(env); +qemu_tcg_init_vcpu(env); } void qemu_notify_event(void) -- 1.7.4
[Qemu-devel] [PATCH 09/37] kvm: Handle kvm_init_vcpu errors
From: Jan Kiszka jan.kis...@siemens.com Do not ignore errors of kvm_init_vcpu, they are fatal. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 19 +++ 1 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cpus.c b/cpus.c index 8232d44..3a72d06 100644 --- a/cpus.c +++ b/cpus.c @@ -265,12 +265,18 @@ void qemu_main_loop_start(void) void qemu_init_vcpu(void *_env) { CPUState *env = _env; +int r; env-nr_cores = smp_cores; env-nr_threads = smp_threads; -if (kvm_enabled()) -kvm_init_vcpu(env); -return; + +if (kvm_enabled()) { +r = kvm_init_vcpu(env); +if (r 0) { +fprintf(stderr, kvm_init_vcpu failed: %s\n, strerror(-r)); +exit(1); +} +} } int qemu_cpu_self(void *env) @@ -600,11 +606,16 @@ static int qemu_cpu_exec(CPUState *env); static void *kvm_cpu_thread_fn(void *arg) { CPUState *env = arg; +int r; qemu_mutex_lock(qemu_global_mutex); qemu_thread_self(env-thread); -kvm_init_vcpu(env); +r = kvm_init_vcpu(env); +if (r 0) { +fprintf(stderr, kvm_init_vcpu failed: %s\n, strerror(-r)); +exit(1); +} kvm_init_ipi(env); -- 1.7.4
[Qemu-devel] [PATCH 11/37] Refactor signal setup functions in cpus.c
From: Jan Kiszka jan.kis...@siemens.com Move {tcg,kvm}_init_ipi and block_io_signals to avoid prototypes, rename the former two to clarify that they deal with more than SIG_IPI. No functional changes - except for the tiny fixup of strerror usage. The forward declaration of sigbus_handler is just temporarily, it will be moved in a succeeding patch. dummy_signal is moved into the !_WIN32 block as we will soon need it also for !CONFIG_IOTHREAD. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 162 +--- 1 files changed, 83 insertions(+), 79 deletions(-) diff --git a/cpus.c b/cpus.c index 4975317..62120c4 100644 --- a/cpus.c +++ b/cpus.c @@ -222,7 +222,15 @@ fail: close(fds[1]); return err; } -#else + +#ifdef CONFIG_IOTHREAD +static void dummy_signal(int sig) +{ +} +#endif + +#else /* _WIN32 */ + HANDLE qemu_event_handle; static void dummy_event_handler(void *opaque) @@ -248,7 +256,7 @@ static void qemu_event_increment(void) exit (1); } } -#endif +#endif /* _WIN32 */ #ifndef CONFIG_IOTHREAD int qemu_init_main_loop(void) @@ -348,10 +356,6 @@ static QemuCond qemu_system_cond; static QemuCond qemu_pause_cond; static QemuCond qemu_work_cond; -static void tcg_init_ipi(void); -static void kvm_init_ipi(CPUState *env); -static sigset_t block_io_signals(void); - /* If we have signalfd, we mask out the signals we want to handle and then * use signalfd to listen for them. We rely on whatever the current signal * handler is to dispatch the signals when we receive them. @@ -387,6 +391,77 @@ static void sigfd_handler(void *opaque) } } +static void cpu_signal(int sig) +{ +if (cpu_single_env) { +cpu_exit(cpu_single_env); +} +exit_request = 1; +} + +static void qemu_kvm_init_cpu_signals(CPUState *env) +{ +int r; +sigset_t set; +struct sigaction sigact; + +memset(sigact, 0, sizeof(sigact)); +sigact.sa_handler = dummy_signal; +sigaction(SIG_IPI, sigact, NULL); + +pthread_sigmask(SIG_BLOCK, NULL, set); +sigdelset(set, SIG_IPI); +sigdelset(set, SIGBUS); +r = kvm_set_signal_mask(env, set); +if (r) { +fprintf(stderr, kvm_set_signal_mask: %s\n, strerror(-r)); +exit(1); +} +} + +static void qemu_tcg_init_cpu_signals(void) +{ +sigset_t set; +struct sigaction sigact; + +memset(sigact, 0, sizeof(sigact)); +sigact.sa_handler = cpu_signal; +sigaction(SIG_IPI, sigact, NULL); + +sigemptyset(set); +sigaddset(set, SIG_IPI); +pthread_sigmask(SIG_UNBLOCK, set, NULL); +} + +static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo, + void *ctx); + +static sigset_t block_io_signals(void) +{ +sigset_t set; +struct sigaction action; + +/* SIGUSR2 used by posix-aio-compat.c */ +sigemptyset(set); +sigaddset(set, SIGUSR2); +pthread_sigmask(SIG_UNBLOCK, set, NULL); + +sigemptyset(set); +sigaddset(set, SIGIO); +sigaddset(set, SIGALRM); +sigaddset(set, SIG_IPI); +sigaddset(set, SIGBUS); +pthread_sigmask(SIG_BLOCK, set, NULL); + +memset(action, 0, sizeof(action)); +action.sa_flags = SA_SIGINFO; +action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler; +sigaction(SIGBUS, action, NULL); +prctl(PR_MCE_KILL, 1, 1, 0, 0); + +return set; +} + static int qemu_signalfd_init(sigset_t mask) { int sigfd; @@ -615,7 +690,7 @@ static void *kvm_cpu_thread_fn(void *arg) exit(1); } -kvm_init_ipi(env); +qemu_kvm_init_cpu_signals(env); /* signal CPU creation */ env-created = 1; @@ -638,7 +713,7 @@ static void *tcg_cpu_thread_fn(void *arg) { CPUState *env = arg; -tcg_init_ipi(); +qemu_tcg_init_cpu_signals(); qemu_thread_self(env-thread); /* signal CPU creation */ @@ -679,77 +754,6 @@ int qemu_cpu_self(void *_env) return qemu_thread_equal(this, env-thread); } -static void cpu_signal(int sig) -{ -if (cpu_single_env) -cpu_exit(cpu_single_env); -exit_request = 1; -} - -static void tcg_init_ipi(void) -{ -sigset_t set; -struct sigaction sigact; - -memset(sigact, 0, sizeof(sigact)); -sigact.sa_handler = cpu_signal; -sigaction(SIG_IPI, sigact, NULL); - -sigemptyset(set); -sigaddset(set, SIG_IPI); -pthread_sigmask(SIG_UNBLOCK, set, NULL); -} - -static void dummy_signal(int sig) -{ -} - -static void kvm_init_ipi(CPUState *env) -{ -int r; -sigset_t set; -struct sigaction sigact; - -memset(sigact, 0, sizeof(sigact)); -sigact.sa_handler = dummy_signal; -sigaction(SIG_IPI, sigact, NULL); - -pthread_sigmask(SIG_BLOCK, NULL, set); -sigdelset(set, SIG_IPI); -sigdelset(set, SIGBUS); -r = kvm_set_signal_mask(env, set); -if (r) { -fprintf(stderr, kvm_set_signal_mask: %s\n, strerror(r)); -
[Qemu-devel] [PATCH 28/37] Move debug exception handling out of cpu_exec
From: Jan Kiszka jan.kis...@siemens.com To prepare splitting up KVM and TCG CPU entry/exit, move the debug exception into cpus.c and invoke cpu_handle_debug_exception on return from qemu_cpu_exec. This also allows to clean up the debug request signaling: We can assign the job of informing main-loop to qemu_system_debug_request and stop the calling cpu directly in cpu_handle_debug_exception. That means a debug stop will now only be signaled via debug_requested and not additionally via vmstop_requested. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpu-exec.c | 24 cpus.c | 35 ++- vl.c |2 +- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 8c9fb8b..9c0b10d 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -196,28 +196,6 @@ static inline TranslationBlock *tb_find_fast(void) return tb; } -static CPUDebugExcpHandler *debug_excp_handler; - -CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler) -{ -CPUDebugExcpHandler *old_handler = debug_excp_handler; - -debug_excp_handler = handler; -return old_handler; -} - -static void cpu_handle_debug_exception(CPUState *env) -{ -CPUWatchpoint *wp; - -if (!env-watchpoint_hit) -QTAILQ_FOREACH(wp, env-watchpoints, entry) -wp-flags = ~BP_WATCHPOINT_HIT; - -if (debug_excp_handler) -debug_excp_handler(env); -} - /* main execution loop */ volatile sig_atomic_t exit_request; @@ -287,8 +265,6 @@ int cpu_exec(CPUState *env1) if (env-exception_index = EXCP_INTERRUPT) { /* exit request from the cpu execution loop */ ret = env-exception_index; -if (ret == EXCP_DEBUG) -cpu_handle_debug_exception(env); break; } else { #if defined(CONFIG_USER_ONLY) diff --git a/cpus.c b/cpus.c index 97a6d4f..c7e86c2 100644 --- a/cpus.c +++ b/cpus.c @@ -165,10 +165,34 @@ static bool all_cpu_threads_idle(void) return true; } -static void cpu_debug_handler(CPUState *env) +static CPUDebugExcpHandler *debug_excp_handler; + +CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler) +{ +CPUDebugExcpHandler *old_handler = debug_excp_handler; + +debug_excp_handler = handler; +return old_handler; +} + +static void cpu_handle_debug_exception(CPUState *env) { +CPUWatchpoint *wp; + +if (!env-watchpoint_hit) { +QTAILQ_FOREACH(wp, env-watchpoints, entry) { +wp-flags = ~BP_WATCHPOINT_HIT; +} +} +if (debug_excp_handler) { +debug_excp_handler(env); +} + gdb_set_stop_cpu(env); qemu_system_debug_request(); +#ifdef CONFIG_IOTHREAD +env-stopped = 1; +#endif } #ifdef CONFIG_LINUX @@ -479,7 +503,6 @@ int qemu_init_main_loop(void) return ret; } #endif -cpu_set_debug_excp_handler(cpu_debug_handler); qemu_init_sigbus(); @@ -653,8 +676,6 @@ int qemu_init_main_loop(void) int ret; sigset_t blocked_signals; -cpu_set_debug_excp_handler(cpu_debug_handler); - qemu_init_sigbus(); blocked_signals = block_io_signals(); @@ -808,7 +829,10 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) while (1) { if (cpu_can_run(env)) { -qemu_cpu_exec(env); +r = qemu_cpu_exec(env); +if (r == EXCP_DEBUG) { +cpu_handle_debug_exception(env); +} } qemu_kvm_wait_io_event(env); } @@ -1076,6 +1100,7 @@ bool cpu_exec_all(void) qemu_kvm_eat_signals(env); } if (r == EXCP_DEBUG) { +cpu_handle_debug_exception(env); break; } } else if (env-stop) { diff --git a/vl.c b/vl.c index eebe684..b436952 100644 --- a/vl.c +++ b/vl.c @@ -1315,7 +1315,7 @@ void qemu_system_powerdown_request(void) void qemu_system_debug_request(void) { debug_requested = 1; -vm_stop(VMSTOP_DEBUG); +qemu_notify_event(); } void qemu_system_vmstop_request(int reason) -- 1.7.4
[Qemu-devel] [PATCH 33/37] kvm: Remove unneeded memory slot reservation
From: Jan Kiszka jan.kis...@siemens.com The number of slots and the location of private ones changed several times in KVM's early days. However, it's stable since 2.6.29 (our required baseline), and slots 8..11 are no longer reserved since then. So remove this unneeded restriction. Signed-off-by: Jan Kiszka jan.kis...@siemens.com CC: Alex Williamson alex.william...@redhat.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- kvm-all.c |4 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 802c6b8..14b6c1e 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -91,10 +91,6 @@ static KVMSlot *kvm_alloc_slot(KVMState *s) int i; for (i = 0; i ARRAY_SIZE(s-slots); i++) { -/* KVM private memory slots */ -if (i = 8 i 12) { -continue; -} if (s-slots[i].memory_size == 0) { return s-slots[i]; } -- 1.7.4
[Qemu-devel] [PATCH 20/37] kvm: Remove static return code of kvm_handle_io
From: Jan Kiszka jan.kis...@siemens.com Improve the readability of the exit dispatcher by moving the static return value of kvm_handle_io to its caller. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- kvm-all.c | 17 - 1 files changed, 8 insertions(+), 9 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 0c20f9e..4729ec5 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -774,8 +774,8 @@ err: return ret; } -static int kvm_handle_io(uint16_t port, void *data, int direction, int size, - uint32_t count) +static void kvm_handle_io(uint16_t port, void *data, int direction, int size, + uint32_t count) { int i; uint8_t *ptr = data; @@ -809,8 +809,6 @@ static int kvm_handle_io(uint16_t port, void *data, int direction, int size, ptr += size; } - -return 1; } #ifdef KVM_CAP_INTERNAL_ERROR_DATA @@ -944,11 +942,12 @@ int kvm_cpu_exec(CPUState *env) switch (run-exit_reason) { case KVM_EXIT_IO: DPRINTF(handle_io\n); -ret = kvm_handle_io(run-io.port, -(uint8_t *)run + run-io.data_offset, -run-io.direction, -run-io.size, -run-io.count); +kvm_handle_io(run-io.port, + (uint8_t *)run + run-io.data_offset, + run-io.direction, + run-io.size, + run-io.count); +ret = 1; break; case KVM_EXIT_MMIO: DPRINTF(handle_mmio\n); -- 1.7.4
[Qemu-devel] [PATCH 17/37] kvm: Add MCE signal support for !CONFIG_IOTHREAD
From: Jan Kiszka jan.kis...@siemens.com Currently, we only configure and process MCE-related SIGBUS events if CONFIG_IOTHREAD is enabled. The groundwork is laid, we just need to factor out the required handler registration and system configuration. Signed-off-by: Jan Kiszka jan.kis...@siemens.com CC: Huang Ying ying.hu...@intel.com CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com CC: Jin Dongming jin.dongm...@np.css.fujitsu.com Signed-off-by: Marcelo Tosatti mtosa...@redhat.com --- cpus.c | 107 +++- 1 files changed, 65 insertions(+), 42 deletions(-) diff --git a/cpus.c b/cpus.c index 38dd506..9a3fc31 100644 --- a/cpus.c +++ b/cpus.c @@ -34,9 +34,6 @@ #include cpus.h #include compatfd.h -#ifdef CONFIG_LINUX -#include sys/prctl.h -#endif #ifdef SIGRTMIN #define SIG_IPI (SIGRTMIN+4) @@ -44,10 +41,24 @@ #define SIG_IPI SIGUSR1 #endif +#ifdef CONFIG_LINUX + +#include sys/prctl.h + #ifndef PR_MCE_KILL #define PR_MCE_KILL 33 #endif +#ifndef PR_MCE_KILL_SET +#define PR_MCE_KILL_SET 1 +#endif + +#ifndef PR_MCE_KILL_EARLY +#define PR_MCE_KILL_EARLY 1 +#endif + +#endif /* CONFIG_LINUX */ + static CPUState *next_cpu; /***/ @@ -158,6 +169,52 @@ static void cpu_debug_handler(CPUState *env) vm_stop(EXCP_DEBUG); } +#ifdef CONFIG_LINUX +static void sigbus_reraise(void) +{ +sigset_t set; +struct sigaction action; + +memset(action, 0, sizeof(action)); +action.sa_handler = SIG_DFL; +if (!sigaction(SIGBUS, action, NULL)) { +raise(SIGBUS); +sigemptyset(set); +sigaddset(set, SIGBUS); +sigprocmask(SIG_UNBLOCK, set, NULL); +} +perror(Failed to re-raise SIGBUS!\n); +abort(); +} + +static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo, + void *ctx) +{ +if (kvm_on_sigbus(siginfo-ssi_code, + (void *)(intptr_t)siginfo-ssi_addr)) { +sigbus_reraise(); +} +} + +static void qemu_init_sigbus(void) +{ +struct sigaction action; + +memset(action, 0, sizeof(action)); +action.sa_flags = SA_SIGINFO; +action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler; +sigaction(SIGBUS, action, NULL); + +prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0); +} + +#else /* !CONFIG_LINUX */ + +static void qemu_init_sigbus(void) +{ +} +#endif /* !CONFIG_LINUX */ + #ifndef _WIN32 static int io_thread_fd = -1; @@ -280,8 +337,6 @@ static int qemu_signalfd_init(sigset_t mask) return 0; } -static void sigbus_reraise(void); - static void qemu_kvm_eat_signals(CPUState *env) { struct timespec ts = { 0, 0 }; @@ -302,13 +357,11 @@ static void qemu_kvm_eat_signals(CPUState *env) } switch (r) { -#ifdef CONFIG_IOTHREAD case SIGBUS: if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) { sigbus_reraise(); } break; -#endif default: break; } @@ -397,6 +450,7 @@ static sigset_t block_synchronous_signals(void) sigset_t set; sigemptyset(set); +sigaddset(set, SIGBUS); if (kvm_enabled()) { /* * We need to process timer signals synchronously to avoid a race @@ -425,6 +479,8 @@ int qemu_init_main_loop(void) #endif cpu_set_debug_excp_handler(cpu_debug_handler); +qemu_init_sigbus(); + return qemu_event_init(); } @@ -561,13 +617,9 @@ static void qemu_tcg_init_cpu_signals(void) pthread_sigmask(SIG_UNBLOCK, set, NULL); } -static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo, - void *ctx); - static sigset_t block_io_signals(void) { sigset_t set; -struct sigaction action; /* SIGUSR2 used by posix-aio-compat.c */ sigemptyset(set); @@ -581,12 +633,6 @@ static sigset_t block_io_signals(void) sigaddset(set, SIGBUS); pthread_sigmask(SIG_BLOCK, set, NULL); -memset(action, 0, sizeof(action)); -action.sa_flags = SA_SIGINFO; -action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler; -sigaction(SIGBUS, action, NULL); -prctl(PR_MCE_KILL, 1, 1, 0, 0); - return set; } @@ -597,6 +643,8 @@ int qemu_init_main_loop(void) cpu_set_debug_excp_handler(cpu_debug_handler); +qemu_init_sigbus(); + blocked_signals = block_io_signals(); ret = qemu_signalfd_init(blocked_signals); @@ -704,31 +752,6 @@ static void qemu_tcg_wait_io_event(void) } } -static void sigbus_reraise(void) -{ -sigset_t set; -struct sigaction action; - -memset(action, 0, sizeof(action)); -action.sa_handler = SIG_DFL; -if (!sigaction(SIGBUS, action, NULL)) { -raise(SIGBUS); -sigemptyset(set); -sigaddset(set, SIGBUS); -sigprocmask(SIG_UNBLOCK, set, NULL); -} -perror(Failed to re-raise
[Qemu-devel] [PATCH] softfloat: export float32_nan and float32_infinity.
These two special values are needed to implement some helper functions, which return these values in some cases. Signed-off-by: Christophe Lyon christophe.l...@st.com --- fpu/softfloat-specialize.h |9 + fpu/softfloat.h|2 ++ 2 files changed, 11 insertions(+), 0 deletions(-) diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h index 2d025bf..d5d8c63 100644 --- a/fpu/softfloat-specialize.h +++ b/fpu/softfloat-specialize.h @@ -201,6 +201,15 @@ int float32_is_signaling_nan( float32 a_ ) } /* +| Returns the default NaN. +**/ + +float32 float32_nan(void) +{ +return float32_default_nan; +} + +/* | Returns a quiet NaN if the single-precision floating point value `a' is a | signaling NaN; otherwise returns `a'. **/ diff --git a/fpu/softfloat.h b/fpu/softfloat.h index e57ee1e..610f245 100644 --- a/fpu/softfloat.h +++ b/fpu/softfloat.h @@ -319,6 +319,7 @@ int float32_compare_quiet( float32, float32 STATUS_PARAM ); int float32_is_quiet_nan( float32 ); int float32_is_signaling_nan( float32 ); float32 float32_maybe_silence_nan( float32 ); +float32 float32_nan(void); float32 float32_scalbn( float32, int STATUS_PARAM ); INLINE float32 float32_abs(float32 a) @@ -365,6 +366,7 @@ INLINE int float32_is_zero_or_denormal(float32 a) #define float32_zero make_float32(0) #define float32_one make_float32(0x3f80) #define float32_ln2 make_float32(0x3f317218) +#define float32_infinity make_float32(0x7f80) /* | Software IEC/IEEE double-precision conversion routines. -- 1.7.2.3
[Qemu-devel] [PATCH] Fix obvious mistake in pxa2xx i2s driver
RST bit is (1 3) bit, not (1 2), fix condition that enables i2s if ENB is set and RST is not set. Signed-off-by: Vasily Khoruzhick anars...@gmail.com --- hw/pxa2xx.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c index d966846..68b67ae 100644 --- a/hw/pxa2xx.c +++ b/hw/pxa2xx.c @@ -1631,7 +1631,7 @@ static void pxa2xx_i2s_write(void *opaque, target_phys_addr_t addr, } if (value (1 4)) /* EFWR */ printf(%s: Attempt to use special function\n, __FUNCTION__); -s-enable = ((value ^ 4) 5) == 5;/* ENB !RST*/ +s-enable = ((value ^ (1 3)) 9) == 9; /* ENB !RST*/ pxa2xx_i2s_update(s); break; case SACR1: -- 1.7.4
[Qemu-devel] Re: RFC: New API for PPC for vcpu mmu access
On Sun, 13 Feb 2011 23:43:40 +0100 Alexander Graf ag...@suse.de wrote: struct kvmppc_book3e_tlb_entry { union { __u64 mas8_1; struct { __u32 mas8; __u32 mas1; }; }; __u64 mas2; union { __u64 mas7_3 struct { __u32 mas7; __u32 mas3; }; }; }; Looks good to me, except for the anonymous unions and structs of course. Avi dislikes those :). :-( Is there any obvious reason we need to have unions in the first place? The compiler should be clever enough to pick the right size accessors when writing/reading masked __u64 entries, no? Yes, the intent was just to make it easier to access individual mas registers, and reuse existing bit declarations that are defined relative to individual registers. Why clutter up the source code when the compiler can deal with it? Same applies to the anonymous unions/structs -- it's done that way to present the fields in the most straightforward manner (equivalent to how the SPRs themselves are named and aliased). If it's a firm NACK on the existing structure, I think I'd rather just drop the paired versions altogether (but still order them this way so it can be changed back if there's any desire to in the future, without breaking compatibility). The struct name should also have a version indicator - it's the data descriptor only a single specific mmu_type, right? It handles both KVM_MMU_PPC_BOOK3E_NOHV and KVM_MMU_PPC_BOOK3E_HV. For an MMU type of KVM_MMU_PPC_BOOK3E_NOHV, the mas8 in kvmppc_book3e_tlb_entry is present but not supported. struct kvmppc_book3e_tlb_params { /* * book3e defines 4 TLBs. Individual implementations may have * fewer. TLBs that do not already exist on the target must be * configured with a size of zero. A tlb_ways value of zero means * the array is fully associative. Only TLBs that are already * set-associative on the target may be configured with a different * associativity. A set-associative TLB may not exceed 255 ways. * * KVM will adjust TLBnCFG based on the sizes configured here, * though arrays greater than 2048 entries will have TLBnCFG[NENTRY] * set to zero. * * The size of any TLB that is set-associative must be a multiple of * the number of ways, and the number of sets must be a power of two. * * The page sizes supported by a TLB shall be determined by reading * the TLB configuration registers. This is not adjustable by userspace. * [Note: need sregs] */ __u32 tlb_sizes[4]; __u8 tlb_ways[4]; __u32 reserved[11]; }; KVM_CONFIG_TLB -- Capability: KVM_CAP_SW_TLB Type: vcpu ioctl Parameters: struct kvm_config_tlb (in) Returns: 0 on success -1 on error struct kvm_config_tlb { __u64 params; __u64 array; __u32 mmu_type; __u32 array_len; Some reserved bits please. IIRC Avi also really likes it when in addition to reserved fields there's also a features int that indicates which reserved fields are actually usable. Shouldn't hurt here either, right? params itself is a versioned struct, with reserved bits. Do we really need it here as well? - The hash for determining set number is: (MAS2[EPN] / page_size) (num_sets - 1) where page_size is the smallest page size supported by the TLB, and num_sets is the tlb_sizes[] value divided by the tlb_ways[] value. If a book3e chip is made for which a different hash is needed, a new MMU type must be used, to ensure that userspace and KVM agree on layout. Please state the size explicitly then. It's 1k, right? It's 4K on Freescale chips. We should probably implement sregs first, in which case qemu can read the MMU config registers to find out the minimum supported page size. If we specify 4K here, we should probably just go ahead and stick FSL in the MMU type name. Specifying the hash itself already makes me nervous about claiming the more generic name. -Scott
Re: [Qemu-devel] KVM call minutes for Feb 8
On Mon, Feb 14, 2011 at 12:42 AM, Anthony Liguori anth...@codemonkey.ws wrote: On 02/13/2011 03:00 PM, Blue Swirl wrote: On Sun, Feb 13, 2011 at 9:57 PM, Anthony Liguorianth...@codemonkey.ws wrote: On 02/13/2011 01:37 PM, Blue Swirl wrote: On Sun, Feb 13, 2011 at 5:31 PM, Anthony Liguorianth...@codemonkey.ws wrote: qdev doesn't expose any state today. qdev properties are construction-only properties that happen to be stored in each device state. What we really need is a full property framework that includes properties with hookable getters and setters along with the ability to mark properties as construct-only, read-only, or read-write. But I think it's reasonable to expose construct-only properties as just an initfn argument. Sounds OK. About read-write properties, what happens if we one day have extensive threading, and locks are pushed to device level? I can imagine a deadlock involving one thread running in IO thread for a device and another trying to access that device's properties. Maybe that is not different from function call version. You need hookable setters/getters that can acquire a lock and do the right thing. It shouldn't be able to dead lock if the locking is designed right. Yes, but qemu_irq is very restricted as it only models a signal bit of information and doesn't really have a mechanism to attach/detach in any generic way. Basic signals are already very useful for many purposes, since they match digital logic signals in real HW. In theory, whole machines could be constructed with just qemu_irq and NAND gate emulator. ;-) It's not just in theory. In the C++ port of QEMU that I wrote, I implemented an AND, OR, and XOR gate and implemented a full 32-bit adder by just using a device config file. If done correctly, using referencing can be extremely powerful. A full adder is a good example. The gates really don't have any concept of bus and the relationship between gates is definitely not a tree. In the message passing IRQ discussion earlier, it was IIRC decided that the one bit version would not be changed but a separate message passing version would be created if ever needed. C already has a message passing interface that supports type safety called function pointers :-) An object that implements multiple interfaces where the interface becomes the message passing interface is exactly what I've been saying we need. It's flexible and the compiler helps us enforce typing. Any interfaces of a base class should make sense even for derived classes. That means if the base class is going to expose essentially a pin-out interface, that if I have a PCIDevice and cast it to Device, I should be able to interact with the GPIO interface to interact with the PCI device. Presumably, that means interfacing at the PCI signalling level. That's insane to model in QEMU :-) This would be doable, if we built buses from a bunch of signals, like in VHDL or Verilog. It would simplify aliased MMIO addresses nicely, the undecoded address pins would be ignored. I don't think it would be useful, but a separate interface could be added for connecting to PCIBus with just qemu_irqs. Yeah, it's possible, but I don't want to spend my time doing this. In reality, GPIO only makes sense for a small class of simple devices where modelling the pin-out interface makes sense (like a 7-segment LCD). That suggests that GPIO should not be in the DeviceState interface but instead should be in a SimpleDevice subclass or something like that. Could you point to examples of SystemBus overuse? anthony@titi:~/git/qemu/hw$ grep qdev_create *.c | wc -l 73 anthony@titi:~/git/qemu/hw$ grep 'qdev_create(NULL' *.c | wc -l 56 SystemBus has become a catch-all for shallow qdev conversions. We've got Northbridges, RAM, and network devices sitting on the same bus... On Sparc32 I have not bothered to create a SBus bus. Now it would be useful to get bootindex corrected. Most devices (even on-board IO) should use SBus. The only other bus (MBus) would exist between CPU, IOMMU and memory. But SysBus fitted the need until recently. A good way to judge where a device is using a bus interface correct: does all of it's interactions with any other part of the guest state involve method calls to the bus? Right now, the answer is no for just about every device in QEMU. This is the problem that qdev really was meant to solve and we're not really any further along solving it unfortunately. I'm not arguing against a generic factory interface, I'm arguing that it should be separate. IOW: SerialState *serial_create(int iobase, int irq, ...); static DeviceState *qdev_serial_create(QemuOpts *opts); static void serial_init(void) { qdev_register(serial, qdev_serial_create); } The key point is that when we create devices internally, we should have a C-friendly, type-safe interface
Re: [Qemu-devel] [PATCH 3/6] target-arm: fix unsigned 64 bit right shifts.
On 11 February 2011 15:10, christophe.l...@st.com wrote: From: Christophe Lyon christophe.l...@st.com Fix range of shift amounts which always give 0 as result. Signed-off-by: Christophe Lyon christophe.l...@st.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org
Re: [Qemu-devel] [PATCH 5/6] target-arm: fix Neon VQSHRN and VSHRN.
On 11 February 2011 15:11, christophe.l...@st.com wrote: From: Christophe Lyon christophe.l...@st.com Call the normal shift helpers instead of the rounding ones. Signed-off-by: Christophe Lyon christophe.l...@st.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org
Re: [Qemu-devel] [PATCH 4/6] target-arm: fix saturated values for Neon right shifts.
On 11 February 2011 15:11, christophe.l...@st.com wrote: --- a/target-arm/neon_helper.c +++ b/target-arm/neon_helper.c @@ -903,7 +903,7 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop) dest = src1 tmp; \ if ((dest tmp) != src1) { \ SET_QC(); \ - dest = src1 31; \ + dest = (uint32_t)(1 (sizeof(src1) * 8 - 1)) - (src1 0 ? 1 : 0); \ } \ }} while (0) This gives the right values but is a pretty long expression (among other things it is more than 80 chars and breaks the QEMU coding style). I'd rather do the same as the existing qshl_s* helper: dest = (uint32_t)(1 (sizeof(src1) * 8 - 1)); \ if (src1 0) { \ dest--; \ } \ NEON_VOP_ENV(qrshl_s8, neon_s8, 4) @@ -924,7 +924,11 @@ uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint32_t valop, uint32_t shiftop) dest = val shift; if ((dest shift) != val) { SET_QC(); - dest = (uint32_t)(1 (sizeof(val) * 8 - 1)) - (val 0 ? 1 : 0); + if (val 0) { + dest = INT32_MIN; + } else { + dest = INT32_MAX; + } Again, right answers but the way most of the rest of the code forces a 32 bit value to signed saturation is dest = (val 31) ^ ~SIGNBIT; -- PMM
Re: [Qemu-devel] [PATCH 4/6] target-arm: fix saturated values for Neon right shifts.
On 14 February 2011 17:46, Peter Maydell peter.mayd...@linaro.org wrote: On 11 February 2011 15:11, christophe.l...@st.com wrote: NEON_VOP_ENV(qrshl_s8, neon_s8, 4) @@ -924,7 +924,11 @@ uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint32_t valop, uint32_t shiftop) dest = val shift; if ((dest shift) != val) { SET_QC(); - dest = (uint32_t)(1 (sizeof(val) * 8 - 1)) - (val 0 ? 1 : 0); + if (val 0) { + dest = INT32_MIN; + } else { + dest = INT32_MAX; + } Again, right answers but the way most of the rest of the code forces a 32 bit value to signed saturation is dest = (val 31) ^ ~SIGNBIT; ...and also this is patching a function newly introduced in patch 1/6 -- better to just have 1/6 have the correct code. -- PMM
Re: [Qemu-devel] [PATCH 6/6] target-arm: fix decoding of Neon 64 bit shifts.
On 11 February 2011 15:11, christophe.l...@st.com wrote: From: Christophe Lyon christophe.l...@st.com Fix decoding of 64 bits variants of VSHRN, VRSHRN, VQSHRN, VQSHRUN, VQRSHRN, VQRSHRUN, taking into account whether inputs are unsigned or not. Signed-off-by: Christophe Lyon christophe.l...@st.com Mostly OK (gives correct answers). Style issues: tmp = neon_load_reg(rm + pass, 0); - gen_neon_shift_narrow(size, tmp, tmp2, q, u); + gen_neon_shift_narrow(size, tmp, tmp2, q, input_unsigned); tmp3 = neon_load_reg(rm + pass, 1); - gen_neon_shift_narrow(size, tmp3, tmp2, q, u); + gen_neon_shift_narrow(size, tmp3, tmp2, q, input_unsigned); These lines are 80 chars now. } else { - if (op == 8) + if (u) { /* VQSHRN / VQRSHRN */ + gen_neon_narrow_satu(size - 1, tmp, cpu_V0); + } else { /* VQSHRN / VQRSHRN */ Missing indentation. The other problem with this area of the code is that it is not correctly handling the case where pass 1 wants to read a register which is the target for pass 2. I have a patch to fix this which I'll post in a moment. -- PMM
[Qemu-devel] KVM call agenda for Feb 15
Please send in any agenda items you are interested in covering. thanks, -chris
Re: [Qemu-devel] [PATCH 1/6] target-arm: Fix rounding constant addition for Neon shift instructions.
On 11 February 2011 15:10, christophe.l...@st.com wrote: +uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop) +{ + int32_t dest; + int32_t val = (int32_t)valop; + int8_t shift = (int8_t)shiftop; + if (shift = 32) { + dest = 0; + } else if (shift -32) { + dest = val 31; This is the wrong answer: large rounding right shifts give zero. + } else if (shift == -32) { + dest = val 31; + dest++; + dest = 1; These three lines will always result in dest becoming 0 regardless of the input value. I'm going to post a patch which fixes these as part of getting the answers right for VRSHL by large shift counts in general. -- PMM
Re: [Qemu-devel] [PULL 0.14] linux-user fixes
On Thu, Feb 10, 2011 at 11:53:10AM +0200, Riku Voipio wrote: The following changes since commit 343c1de916b1841cd5fd5f813add9c87590d72e8: x86: Fix MCA broadcast parameters for TCG case (2011-02-08 12:37:30 +0100) are available in the git repository at: git://gitorious.org/qemu-maemo/qemu.git linux-user-for-0.14 Martin Mohring (1): linux-user: fix for loopmount ioctl Stefan Weil (1): linux-user: Fix possible realloc memory leak linux-user/elfload.c |8 +--- linux-user/ioctls.h |2 -- 2 files changed, 5 insertions(+), 5 deletions(-) Applied. Thanks, Justin
Re: [Qemu-devel] [PATCH 2/6] target-arm: fix Neon right shifts with shift amount == input width.
On 11 February 2011 15:10, christophe.l...@st.com wrote: From: Christophe Lyon christophe.l...@st.com Fix rshl helpers (s8, s16, s64, u8, u16) Signed-off-by: Christophe Lyon christophe.l...@st.com --- target-arm/neon_helper.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c index 3f1f3d4..1ac362f 100644 --- a/target-arm/neon_helper.c +++ b/target-arm/neon_helper.c @@ -548,7 +548,7 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop) } else if (tmp -(ssize_t)sizeof(src1) * 8) { \ dest = src1 (sizeof(src1) * 8 - 1); \ } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \ - dest = src1 (tmp - 1); \ + dest = src1 (-tmp - 1); \ dest++; \ dest = 1; \ Again, these three lines have the same effect as dest = 0, so we can fold into the previous if(). } else if (tmp 0) { \ @@ -594,7 +594,7 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop) val = 0; } else if (shift -64) { val = 63; You didn't change this case, but it is the wrong answer: should be 0. - } else if (shift == -63) { + } else if (shift == -64) { val = 63; val++; val = 1; Always results in 0. -- PMM
Re: [Qemu-devel] [PATCH v3 0/6] target-arm: Fix Neon shift instructions.
On 11 February 2011 15:10, christophe.l...@st.com wrote: From: Christophe Lyon christophe.l...@st.com This patch series provides fixes such that ARM Neon instructions VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN, VQSHRN, VSHRN, VQSHRUN now pass all my tests. I have reworked all these patches and I hope they are now easier to review. Thanks; this was indeed a lot easier to review. Mostly this is OK, there are a few things: * minor style issues * handling of very large shift counts is not right (both in code you wrote and existing routines) * the shift-and-narrow loop doesn't handle the case where pass 1 reads registers pass 0 writes I have patches which (sitting on top of your 6) fix all these and give a clean pass on the random instruction set testing for the shift instructions. Unless you object, I think the simplest thing will be for me to just fix the minor nits I identified in your patches and then post a combined series of your patches and mine. -- PMM
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
On Mon, 14 Feb 2011 08:39:11 -0600 Anthony Liguori anth...@codemonkey.ws wrote: On 02/14/2011 06:45 AM, Luiz Capitulino wrote: So the question is: how does the schema based design support extending commands or events? Does it require adding new commands/events? Well, let me ask you, how do we do that today? Let's say that I want to add a new parameter to the `change' function so that I can include a salt parameter as part of the password. The way we'd do this today is by checking for the 'salt' parameter in qdict, and if it's not present, use a random salt or something like that. You likely want to do what you did before. Of course that you have to consider if what you're doing is extending an existing command or badly overloading it (like change is today), in this case you'll want to add a new command instead. But yes, the use-case here is extending an existing command. However, if I'm a QMP client, how can I tell whether you're going to ignore my salt parameter or actually use it? Nothing in QMP tells me this today. If I set the salt parameter in the `change' command, I'll just get a success message. I'm sorry? { execute: change, arguments: { device: vnc, target: password, arg: 1234, salt: r1 } } {error: {class: InvalidParameter, desc: Invalid parameter 'salt', data: {name: salt}}} Even if we expose a schema, but leave things as-is, having to parse the schema as part of a function call is pretty horrible, That's a client implementation detail, they are not required to do it as part of a function call. But let me ask, if we don't expose a schema, how will clients be able to query available commands/events and their parameters? particularly if distros do silly things like backport some optional parameters and not others. If those optional parameters are deeply nested in a structure, it's even worse. Why would they do this? I mean, if distros (or anyone else shipping qemu) goes that deep on changing the wire protocol they are on their own, why would we want to solve this problem? Note that this is different from having a modular design where we can offer the possibility of disabling features, say, in configure. That should be possible, but it's different from choosing random parameters in commands. OTOH, if we introduce a new command to set the password with a salt, it becomes very easy for the client to support. The do something as simple as: if qmp.has_command(vnc-set-password-with-salt): qmp.vnc_set_password_with_salt('foobar', 'X*') else: window.set_weak_security_icon(True) qmp.vnc_set_password('foobar') Now you could answer, hey, we can add capabilities then those capabilities can quickly get out of hand. Adding one command per new argument has its problems too and it's even worse with events, as clients will have to be changed to handle a new event just because of a parameter addition. Look, although I did _not_ check any code yet, your description of the QAPI looks really exciting. I'm not against it, what bothers me though is this number of small limitations we're imposing to the wire protocol. Why don't we make libqmp internal only? This way we're free to change it whatever we want.