avoid casts when initializing structures

Due to our kernel heritage we have code in kvmtool that relies on
the (still) implicit -std=gnu89 compiler switch.
It turns out that this just affects some s

avoid casts when initializing structures

Due to our kernel heritage we have code in kvmtool that relies on
the (still) implicit -std=gnu89 compiler switch.
It turns out that this just affects some structure initialization,
where we currently provide a cast to the type, which upsets GCC for
anything beyond -std=gnu89 (for instance gnu99 or gnu11).
We do need the casts when initializing structures that are not
assigned to the same type, so we put it there explicitly.

This allows us to compile with all the three GNU standards GCC
currently supports: gnu89/90, gnu99 and gnu11.
GCC threatens people with moving to gnu11 as the new default standard,
so lets fix this better sooner than later.
(Compiling without GNU extensions still breaks and I don't bother to
fix that without very good reasons.)

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>

show more ...

kvm tools: use mutex abstraction instead of pthread mutex

We already have something to wrap pthread with mutex_[init,lock,unlock]
calls. This patch creates a new struct mutex abstraction and moves
e

kvm tools: use mutex abstraction instead of pthread mutex

We already have something to wrap pthread with mutex_[init,lock,unlock]
calls. This patch creates a new struct mutex abstraction and moves
everything to work with it.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>

show more ...

kvm tools, qcow: Fix locking issues

The virtio_blk_do_io() function can enter the QCOW code through
disk_image__{read,write,flush}() from multiple threads because it uses a thread
pool for I/O reque

kvm tools, qcow: Fix locking issues

The virtio_blk_do_io() function can enter the QCOW code through
disk_image__{read,write,flush}() from multiple threads because it uses a thread
pool for I/O requests. Thus, use locking to make the QCOW2 code thread-safe.

Cc: Asias He <asias.hejun@gmail.com>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Prasad Joshi <prasadjoshi124@gmail.com>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>

show more ...

kvm tools: Fix and improve the CPU register dump debug output code

* Pekka Enberg <penberg@kernel.org> wrote:

> Ingo Molnar reported that 'kill -3' didn't work on his machine:
>
> * Ingo Molnar <

kvm tools: Fix and improve the CPU register dump debug output code

* Pekka Enberg <penberg@kernel.org> wrote:

> Ingo Molnar reported that 'kill -3' didn't work on his machine:
>
> * Ingo Molnar <mingo@elte.hu> wrote:
>
> > This is really cumbersome to debug - is there some good way to get to the RIP
> > that the guest is hanging in? If kvm would print that out to the host console
> > (even if it's just the raw RIP initially) on a kill -3 that would help
> > enormously.
>
> Looks like the code should be doing that already - but the ioctl(KVM_GET_SREGS)
> hangs:
>
> [pid 748] ioctl(6, KVM_GET_SREGS
>
> Avi Kivity pointed out that it's not safe to call KVM_GET_SREGS (or other vcpu
> related ioctls) from other threads:
>
> > is it not OK to call KVM_GET_SREGS from other threads than the one
> > that's doing KVM_RUN?
>
> From Documentation/kvm/api.txt:
>
> - vcpu ioctls: These query and set attributes that control the operation
> of a single virtual cpu.
>
> Only run vcpu ioctls from the same thread that was used to create the
> vcpu.
>
> Fix that up by using pthread_kill() to force the threads that are doing KVM_RUN
> to do the register dumps.
>
> Reported: Ingo Molnar <mingo@elte.hu>
> Cc: Asias He <asias.hejun@gmail.com>
> Cc: Avi Kivity <avi@redhat.com>
> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Prasad Joshi <prasadjoshi124@gmail.com>
> Cc: Sasha Levin <levinsasha928@gmail.com>
> Signed-off-by: Pekka Enberg <penberg@kernel.org>
> ---
> tools/kvm/kvm-run.c | 20 +++++++++++++++++---
> 1 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
> index eb50b6a..58e2977 100644
> --- a/tools/kvm/kvm-run.c
> +++ b/tools/kvm/kvm-run.c
> @@ -127,6 +127,18 @@ static const struct option options[] = {
> OPT_END()
> };
>
> +static void handle_sigusr1(int sig)
> +{
> + struct kvm_cpu *cpu = current_kvm_cpu;
> +
> + if (!cpu)
> + return;
> +
> + kvm_cpu__show_registers(cpu);
> + kvm_cpu__show_code(cpu);
> + kvm_cpu__show_page_tables(cpu);
> +}
> +
> static void handle_sigquit(int sig)
> {
> int i;
> @@ -134,9 +146,10 @@ static void handle_sigquit(int sig)
> for (i = 0; i < nrcpus; i++) {
> struct kvm_cpu *cpu = kvm_cpus[i];
>
> - kvm_cpu__show_registers(cpu);
> - kvm_cpu__show_code(cpu);
> - kvm_cpu__show_page_tables(cpu);
> + if (!cpu)
> + continue;
> +
> + pthread_kill(cpu->thread, SIGUSR1);
> }
>
> serial8250__inject_sysrq(kvm);

i can see a couple of problems with the debug printout code, which currently
produces a stream of such dumps for each vcpu:

Registers:
rip: 0000000000000000 rsp: 00000000000016ca flags: 0000000000010002
rax: 0000000000000000 rbx: 0000000000000000 rcx: 0000000000000000
rdx: 0000000000000000 rsi: 0000000000000000 rdi: 0000000000000000
rbp: 0000000000008000 r8: 0000000000000000 r9: 0000000000000000
r10: 0000000000000000 r11: 0000000000000000 r12: 0000000000000000
r13: 0000000000000000 r14: 0000000000000000 r15: 0000000000000000
cr0: 0000000060000010 cr2: 0000000000000070 cr3: 0000000000000000
cr4: 0000000000000000 cr8: 0000000000000000
Segment registers:
register selector base limit type p dpl db s l g avl
cs f000 00000000000f0000 0000ffff 03 1 3 0 1 0 0 0
ss 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
ds 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
es 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
fs 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
gs 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
tr 0000 0000000000000000 0000ffff 0b 1 0 0 0 0 0 0
ldt 0000 0000000000000000 0000ffff 02 1 0 0 0 0 0 0
gdt 0000000000000000 0000ffff
idt 0000000000000000 0000ffff
[ efer: 0000000000000000 apic base: 00000000fee00900 nmi: enabled ]
Interrupt bitmap:
0000000000000000 0000000000000000 0000000000000000 0000000000000000
Code: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 <cf> eb 0d 90 90 90 90 90 90 90 90 90 90 90 90 90 f6 c4 0e 75 4b
Stack:
0x000016ca: 00 00 00 00 00 00 00 00
0x000016d2: 00 00 00 00 00 00 00 00
0x000016da: 00 00 00 00 00 00 00 00
0x000016e2: 00 00 00 00 00 00 00 00

The problems are:

- This does not work very well on SMP with lots of vcpus, because the printing
is unserialized, resulting in a jumbled mess of an output, all vcpus trying
to print to the console at once, often mixing lines and characters randomly.

- stdout from a signal handler must be flushed, otherwise lines can remain
buffered if someone saves the output via 'tee' for example.

- the dumps from the various CPUs are not distinguishable - they are just
dumped after each other with no identification

- the various printouts are rather hard to parse visually - it's not easy to see
various properties "at a glance" because the dump is visually confusing.

The patch below addresses these concerns, serializes the output, tidies up the
printout, resulting in this new output:

#
# vCPU #0's dump:
#

Registers:
----------
rip: 0000000000000000 rsp: 00000000000008bc flags: 0000000000010002
rax: 0000000000000000 rbx: 0000000000000000 rcx: 0000000000000000
rdx: 0000000000000000 rsi: 0000000000000000 rdi: 0000000000000000
rbp: 0000000000008000 r8: 0000000000000000 r9: 0000000000000000
r10: 0000000000000000 r11: 0000000000000000 r12: 0000000000000000
r13: 0000000000000000 r14: 0000000000000000 r15: 0000000000000000
cr0: 0000000060000010 cr2: 0000000000000070 cr3: 0000000000000000
cr4: 0000000000000000 cr8: 0000000000000000

Segment registers:
------------------
register selector base limit type p dpl db s l g avl
cs f000 00000000000f0000 0000ffff 03 1 3 0 1 0 0 0
ss 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
ds 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
es 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
fs 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
gs 1000 0000000000010000 0000ffff 03 1 3 0 1 0 0 0
tr 0000 0000000000000000 0000ffff 0b 1 0 0 0 0 0 0
ldt 0000 0000000000000000 0000ffff 02 1 0 0 0 0 0 0
gdt 0000000000000000 0000ffff
idt 0000000000000000 0000ffff

APIC:
-----
efer: 0000000000000000 apic base: 00000000fee00900 nmi: enabled

Interrupt bitmap:
-----------------
0000000000000000 0000000000000000 0000000000000000 0000000000000000

Code:
-----
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 <cf> eb 0d 90 90 90 90 90 90 90 90 90 90 90 90 90 f6 c4 0e 75 4b

Stack:
------
0x000008bc: 00 00 00 00 00 00 00 00
0x000008c4: 00 00 00 00 00 00 00 00
0x000008cc: 00 00 00 00 00 00 00 00
0x000008d4: 00 00 00 00 00 00 00 00

Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Pekka Enberg <penberg@kernel.org>

show more ...

kvm tools: Use mutex_lock() and mutex_unlock() wrappers

This patch implements less hostile mutex_lock() and mutex_lock() wrappers on
top of the pthread API equivalents as suggested by Ingo Molnar:

kvm tools: Use mutex_lock() and mutex_unlock() wrappers

This patch implements less hostile mutex_lock() and mutex_lock() wrappers on
top of the pthread API equivalents as suggested by Ingo Molnar:

glibc/pthreads mutex API semantics are pretty silly IMO.

I *think* it would be better to try to match the kernel API here, and provide
trivial wrappers around mutex_lock()/mutex_unlock(). We wont ever bring down
threads in a hostile way, so we wont actually need the error returns. CPU
threads should probably only exit once the kvm process exits, after all
cleanup has been done.

That way usage would be more obvious and more familar to kernel developers :-)

[ It would also open up the possibility, in the far future, to bring lockdep to
user-space ;-) ]

Cc: Asias He <asias.hejun@gmail.com>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Pekka Enberg <penberg@kernel.org>

show more ...