[PATCH] kvm: kvmtrace: kvm_trace in kernel for supporting big_endian
>From 8f1cd69b77054681bbb7e81a1a2d31b887db9fbb Mon Sep 17 00:00:00 2001 From: Tan Li <[EMAIL PROTECTED]> Date: Fri, 23 May 2008 14:41:17 +0800 Subject: [PATCH] kvm: kvmtrace: kvm_trace in kernel for supporting big_endian Currently kvmtrace is not portable. This will prevent from copying a trace file from big-endian target to little-endian workstation for analysis. In the patch, kernel outputs metadata containing a magic number to trace log. Signed-off-by: Tan Li <[EMAIL PROTECTED]> --- include/linux/kvm.h |4 ++-- virt/kvm/kvm_trace.c | 18 -- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/include/linux/kvm.h b/include/linux/kvm.h index a281afe..ca08cb1 100644 --- a/include/linux/kvm.h +++ b/include/linux/kvm.h @@ -294,14 +294,14 @@ struct kvm_trace_rec { __u32 vcpu_id; union { struct { - __u32 cycle_lo, cycle_hi; + __u64 cycle_u64; __u32 extra_u32[KVM_TRC_EXTRA_MAX]; } cycle; struct { __u32 extra_u32[KVM_TRC_EXTRA_MAX]; } nocycle; } u; -}; +} __attribute__((packed)); #define KVMIO 0xAE diff --git a/virt/kvm/kvm_trace.c b/virt/kvm/kvm_trace.c index 0e49547..58141f3 100644 --- a/virt/kvm/kvm_trace.c +++ b/virt/kvm/kvm_trace.c @@ -72,11 +72,7 @@ static void kvm_add_trace(void *probe_private, void *call_data, rec.cycle_in= p->cycle_in; if (rec.cycle_in) { - u64 cycle = 0; - - cycle = get_cycles(); - rec.u.cycle.cycle_lo = (u32)cycle; - rec.u.cycle.cycle_hi = (u32)(cycle >> 32); + rec.u.cycle.cycle_u64 = get_cycles(); for (i = 0; i < rec.extra_u32; i++) rec.u.cycle.extra_u32[i] = va_arg(*args, u32); @@ -114,8 +110,18 @@ static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf, { struct kvm_trace *kt; - if (!relay_buf_full(buf)) + if (!relay_buf_full(buf)) { + if (!prev_subbuf) { + /* +* executed only once when the channel is opened +* save metadata as first record +*/ + subbuf_start_reserve(buf, sizeof(u32)); + *(u32 *)subbuf = 0x12345678; + } + return 1; + } kt = buf->chan->private_data; atomic_inc(&kt->lost_records); -- 1.5.3.4 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] qemu: fix ATAPI read drive structure command
I believe this fixes bug 1967368. Windows Vista hangs if you try to run diskpart.exe with a DVD image loaded. This does not occur on Xen because their version of Qemu doesn't try to emulate the GPCMD_READ_DVD_STRUCTURE command. If I comment out the version in KVM, diskpart.exe works. Digging through the versions of the MMC-6 spec I can find online, I believe the problem is that we're ignoring the allocation length field. This specifies the maximum number of bytes that may be returned by the drive. Instead we're returning the maximum possible table size each time. I also found that we seem to be using the wrong field for the format request. Byte 2 is MSB of the address field, we want byte 7. I also added a few missing comments for the fields as we fill them in. I've only tested this with Vista since it's the only thing that I know using this command. Thanks, Alex qemu: fix ATAPI read drive structure command Make use of the allocation length field in the command and only return the number of bytes requested. Fix location of format byte in command. Add comments for more fields as we fill them in. This fixes bug 1967368 (diskpart.exe in Vista hangs with DVD image loaded). Signed-off-by: Alex Williamson <[EMAIL PROTECTED]> -- diff --git a/qemu/hw/ide.c b/qemu/hw/ide.c index 69363a9..0246fb6 100644 --- a/qemu/hw/ide.c +++ b/qemu/hw/ide.c @@ -1653,7 +1653,8 @@ static void ide_atapi_cmd(IDEState *s) { int media = packet[1]; int layer = packet[6]; -int format = packet[2]; +int format = packet[7]; +int length = ube16_to_cpu(packet + 8); uint64_t total_sectors; if (media != 0 || layer != 0) @@ -1672,20 +1673,26 @@ static void ide_atapi_cmd(IDEState *s) break; } -memset(buf, 0, 2052); +if (length == 0) +length = 2048 + 4; +if (length < 20) +ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST, +ASC_INV_FIELD_IN_CMD_PACKET); + +memset(buf, 0, length); buf[4] = 1; // DVD-ROM, part version 1 buf[5] = 0xf; // 120mm disc, maximum rate unspecified buf[6] = 0; // one layer, embossed data -buf[7] = 0; +buf[7] = 0; // default densities -cpu_to_ube32(buf + 8, 0); -cpu_to_ube32(buf + 12, total_sectors - 1); -cpu_to_ube32(buf + 16, total_sectors - 1); +cpu_to_ube32(buf + 8, 0); // start sector +cpu_to_ube32(buf + 12, total_sectors - 1); // end sector +cpu_to_ube32(buf + 16, total_sectors - 1); // l0 end sector -cpu_to_be16wu((uint16_t *)buf, 2048 + 4); +cpu_to_be16wu((uint16_t *)buf, length); -ide_atapi_cmd_reply(s, 2048 + 3, 2048 + 4); +ide_atapi_cmd_reply(s, length, length); break; default: -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix silly output for virtio devices in /proc/interrupts]
On Friday 23 May 2008 07:58:44 Anthony Liguori wrote: > Rusty Russell wrote: > > On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote: > >> Author: Chris Lalancette <[EMAIL PROTECTED]> > >> Date: Thu May 15 09:04:55 2008 -0400 > >> > >> register_virtio_device was doing something silly, in that it was > >> overwriting what the calling driver stuck into .bus_id" for the name. > >> This caused problems in the output of /proc/interrupts, since when you > >> request_irq(), it doesn't actually copy the devname you pass in but just > >> stores a pointer to the data. The fix is to just not have > >> register_virtio_device do anything with the bus_id, and assume the > >> higher level driver set it up properly. > > > > OK, but only one higher-level driver will set it up properly: kvm. > > Neither lguest nor s/390 do this, and as a result, they fail to register > > *any* devices. > > > > The following patch should fix it for s/390 (it's identical to the lguest > > patch), but would prefer testing (S/390-ers cc'd). > > It may actually be better for virtio to set this up. The problem is > that if you have multiple transports that are registering virtio > devices, it's impossible at the transport level to guarantee uniqueness > while still using the "virtio%d" naming. Except the current scheme is > no good, we'd have to push the dev_index into virtio too. That's true. OK, let's hoist the index counter into common code instead. The alternative is to use different busid namings for each transport, and that seems like too much confusion: sysfs will tell you where it comes from if you really need to know. Patch series coming. Thanks, Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm-qemu: Proper vm_stop on debug events
Anthony Liguori wrote: > Jan Kiszka wrote: >> When a vcpu exits after hitting a debug exception, we have to invoke >> vm_stop(EXCP_DEBUG). But this has to take place over the io-thread. >> >> This patch introduces kvm_debug_stop_requested to signal this event, and >> it takes care that the interrupted vcpu itself goes immediately into >> stop state. >> >> Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]> >> --- >> qemu/qemu-kvm.c |9 - >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> Index: b/qemu/qemu-kvm.c >> === >> --- a/qemu/qemu-kvm.c >> +++ b/qemu/qemu-kvm.c >> @@ -58,6 +58,8 @@ pthread_t io_thread; >> static int io_thread_fd = -1; >> static int io_thread_sigfd = -1; >> >> +static int kvm_debug_stop_requested; >> + >> > > Why use this instead of just keying off of exception_index == EXCP_DEBUG? > >> static inline unsigned long kvm_get_thread_id(void) >> { >> return syscall(SYS_gettid); >> @@ -517,6 +519,10 @@ int kvm_main_loop(void) >> qemu_system_powerdown(); >> else if (qemu_reset_requested()) >> qemu_kvm_system_reset(); >> +else if (kvm_debug_stop_requested) { >> +kvm_debug_stop_requested = 0; >> +vm_stop(EXCP_DEBUG); >> +} >> } >> >> pause_all_threads(); >> @@ -529,7 +535,8 @@ static int kvm_debug(void *opaque, int v >> { >> CPUState *env = cpu_single_env; >> >> -env->exception_index = EXCP_DEBUG; >> +kvm_debug_stop_requested = 1; >> +vcpu_info[vcpu].stopped = 1; >> > > This isn't quite right. In the very least, you need to set stopping = 0 > and signal on the qemu_pause_cond. Thinking it through more though, a > breakpoint should stop all VCPUs, right? It does already, give it a try. > > vm_stop(EXCP_DEBUG) will actually do this. It invokes the > vm_state_notify callbacks and the io-thread registers one. I think you > should probably just issue vm_stop(EXCP_DEBUG) from kvm_debug. As explained, it can't be called over the vcpu contexts (there is even an assert() fence against it). This policy has been recently agreed on while fixing other deadlock issues of qemu-kvm. Jan signature.asc Description: OpenPGP digital signature
[PATCH 3/3] kvm-userspace: Switch to KVM_SET_GUEST_DEBUG
This patch switches both libkvm as well as the qemu pieces over to the new guest debug interface. This means, instead of relying on hardware breakpoints, breakpoint traps are patched into the guest code at the required spots. Breakpoint management is done inside the qemu-kvm, transparent to gdbstub and also avoiding that the gdb frontend takes over. This allows for running debuggers inside the guest while guest debugging it active, because the host can cleanly tell apart host- and guest-originated breakpoint events. Yet improvable are x86 corner cases when using single-step ("forgotten" debug flags on the guest's stack). And, of courese, the yet empty non-x86 helper functions have to be populated... Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]> --- libkvm/kvm-common.h |2 libkvm/libkvm-x86.c | 20 ++ libkvm/libkvm.c | 11 +-- libkvm/libkvm.h | 14 +++- qemu/exec.c | 15 ++-- qemu/qemu-kvm-ia64.c| 17 + qemu/qemu-kvm-powerpc.c | 17 + qemu/qemu-kvm-x86.c | 32 + qemu/qemu-kvm.c | 154 +--- qemu/qemu-kvm.h | 27 +++- 10 files changed, 268 insertions(+), 41 deletions(-) Index: b/libkvm/kvm-common.h === --- a/libkvm/kvm-common.h +++ b/libkvm/kvm-common.h @@ -85,4 +85,6 @@ int handle_io_window(kvm_context_t kvm); int handle_debug(kvm_context_t kvm, int vcpu); int try_push_interrupts(kvm_context_t kvm); +int handle_debug(kvm_context_t kvm, int vcpu); + #endif Index: b/libkvm/libkvm-x86.c === --- a/libkvm/libkvm-x86.c +++ b/libkvm/libkvm-x86.c @@ -661,3 +661,23 @@ int kvm_disable_tpr_access_reporting(kvm } #endif + +int handle_debug(kvm_context_t kvm, int vcpu) +{ +#ifdef KVM_CAP_SET_GUEST_DEBUG + struct kvm_run *run = kvm->run[vcpu]; + unsigned long watchpoint = 0; + int break_type; + + if (run->debug.arch.exception == 1) { + /* TODO: fully process run->debug.arch */ + break_type = KVM_GDB_BREAKPOINT_HW; + } else + break_type = KVM_GDB_BREAKPOINT_SW; + + return kvm->callbacks->debug(kvm->opaque, vcpu, break_type, +run->debug.pc, watchpoint); +#else + return -ENOSYS; +#endif +} Index: b/libkvm/libkvm.c === --- a/libkvm/libkvm.c +++ b/libkvm/libkvm.c @@ -738,11 +738,6 @@ static int handle_io(kvm_context_t kvm, return 0; } -int handle_debug(kvm_context_t kvm, int vcpu) -{ - return kvm->callbacks->debug(kvm->opaque, vcpu); -} - int kvm_get_regs(kvm_context_t kvm, int vcpu, struct kvm_regs *regs) { return ioctl(kvm->vcpu_fd[vcpu], KVM_GET_REGS, regs); @@ -945,10 +940,12 @@ int kvm_inject_irq(kvm_context_t kvm, in return ioctl(kvm->vcpu_fd[vcpu], KVM_INTERRUPT, &intr); } -int kvm_guest_debug(kvm_context_t kvm, int vcpu, struct kvm_debug_guest *dbg) +#ifdef KVM_CAP_SET_GUEST_DEBUG +int kvm_set_guest_debug(kvm_context_t kvm, int vcpu, struct kvm_guest_debug *dbg) { - return ioctl(kvm->vcpu_fd[vcpu], KVM_DEBUG_GUEST, dbg); + return ioctl(kvm->vcpu_fd[vcpu], KVM_SET_GUEST_DEBUG, dbg); } +#endif int kvm_set_signal_mask(kvm_context_t kvm, int vcpu, const sigset_t *sigset) { Index: b/qemu/exec.c === --- a/qemu/exec.c +++ b/qemu/exec.c @@ -1157,6 +1157,9 @@ int cpu_breakpoint_insert(CPUState *env, #if defined(TARGET_HAS_ICE) int i; +if (kvm_enabled()) +return kvm_insert_breakpoint(env, pc, len, type); + for(i = 0; i < env->nb_breakpoints; i++) { if (env->breakpoints[i] == pc) return 0; @@ -1166,9 +1169,6 @@ int cpu_breakpoint_insert(CPUState *env, return -ENOBUFS; env->breakpoints[env->nb_breakpoints++] = pc; -if (kvm_enabled()) - kvm_update_debugger(env); - breakpoint_invalidate(env, pc); return 0; #else @@ -1182,6 +1182,10 @@ int cpu_breakpoint_remove(CPUState *env, { #if defined(TARGET_HAS_ICE) int i; + +if (kvm_enabled()) +return kvm_remove_breakpoint(env, pc, len, type); + for(i = 0; i < env->nb_breakpoints; i++) { if (env->breakpoints[i] == pc) goto found; @@ -1192,9 +1196,6 @@ int cpu_breakpoint_remove(CPUState *env, if (i < env->nb_breakpoints) env->breakpoints[i] = env->breakpoints[env->nb_breakpoints]; -if (kvm_enabled()) - kvm_update_debugger(env); - breakpoint_invalidate(env, pc); return 0; #else @@ -1214,7 +1215,7 @@ void cpu_single_step(CPUState *env, int tb_flush(env); } if (kvm_enabled()) - kvm_update_debugger(env); + kvm_update_guest_debug(env, KVM_GDB_BREAKPOINT_NONE); #endif } Index: b/qemu/qemu-kvm.c ==
[PATCH 2/3] qemu-kvm: Introduce single vcpu pause/resume
Small helpers that allow to pause and resume only a single vcpu thread. Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]> --- qemu/qemu-kvm.c | 29 ++--- 1 file changed, 22 insertions(+), 7 deletions(-) Index: b/qemu/qemu-kvm.c === --- a/qemu/qemu-kvm.c +++ b/qemu/qemu-kvm.c @@ -207,6 +207,17 @@ static int all_threads_paused(void) return 1; } +static void pause_vcpu_thread(int index) +{ +assert(cpu_single_env->cpu_index != index); + +vcpu_info[index].stop = 1; +pthread_kill(vcpu_info[index].thread, SIG_IPI); + +while (!vcpu_info[index].stopped) + qemu_cond_wait(&qemu_pause_cond); +} + static void pause_all_threads(void) { int i; @@ -221,17 +232,21 @@ static void pause_all_threads(void) qemu_cond_wait(&qemu_pause_cond); } +static void resume_vcpu_thread(int index) +{ +assert(!cpu_single_env); + +vcpu_info[index].stop = 0; +vcpu_info[index].stopped = 0; +pthread_kill(vcpu_info[index].thread, SIG_IPI); +} + static void resume_all_threads(void) { int i; -assert(!cpu_single_env); - -for (i = 0; i < smp_cpus; ++i) { - vcpu_info[i].stop = 0; - vcpu_info[i].stopped = 0; - pthread_kill(vcpu_info[i].thread, SIG_IPI); -} +for (i = 0; i < smp_cpus; ++i) + resume_vcpu_thread(i); } static void kvm_vm_state_change_handler(void *context, int running) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] KVM: Rework guest debugging interface
This rips out the support for KVM_DEBUG_GUEST and introduces a new IOCTL instead: KVM_SET_GUEST_DEBUG. The IOCTL payload consists of a generic part, controlling the "main switch" and the single-step feature. The arch specific part adds an x86 interface for intercepting both types of debug exceptions separately and re-injecting them when the host is not interested. Moreover, it lays the ground for full hardware debug register / watchpoint support. To signal breakpoint events properly back to userland, the current PC as well as an arch-specific data block is now returned along KVM_EXIT_DEBUG. For x86, the arch block contains the debug exception and relevant debug registers to tell possible debug events apart. The availability of the new interface is signaled by KVM_CAP_SET_GUEST_DEBUG. Note that both svm and vtx are supported, but only the latter was tested yet. Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]> --- arch/ia64/kvm/kvm-ia64.c |4 +- arch/powerpc/kvm/powerpc.c |4 +- arch/s390/kvm/kvm-s390.c |4 +- arch/x86/kvm/svm.c | 45 +++- arch/x86/kvm/vmx.c | 83 ++--- arch/x86/kvm/x86.c | 17 - include/asm-ia64/kvm.h |7 +++ include/asm-powerpc/kvm.h |7 +++ include/asm-s390/kvm.h |7 +++ include/asm-x86/kvm.h | 17 + include/asm-x86/kvm_host.h | 11 + include/linux/kvm.h| 52 +++- include/linux/kvm_host.h |6 +-- virt/kvm/kvm_main.c|6 +-- 14 files changed, 168 insertions(+), 102 deletions(-) Index: b/arch/x86/kvm/x86.c === --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2730,10 +2730,6 @@ static int __vcpu_run(struct kvm_vcpu *v down_read(&vcpu->kvm->slots_lock); vapic_enter(vcpu); -preempted: - if (vcpu->guest_debug.enabled) - kvm_x86_ops->guest_debug_pre(vcpu); - again: if (vcpu->requests) if (test_and_clear_bit(KVM_REQ_MMU_RELOAD, &vcpu->requests)) @@ -2869,7 +2865,7 @@ out: if (r > 0) { kvm_resched(vcpu); down_read(&vcpu->kvm->slots_lock); - goto preempted; + goto again; } post_kvm_run_save(vcpu, kvm_run); @@ -2973,7 +2969,7 @@ int kvm_arch_vcpu_ioctl_get_regs(struct /* * Don't leak debug flags in case they were set for guest debugging */ - if (vcpu->guest_debug.enabled && vcpu->guest_debug.singlestep) + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) regs->rflags &= ~(X86_EFLAGS_TF | X86_EFLAGS_RF); vcpu_put(vcpu); @@ -3589,8 +3585,8 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct return 0; } -int kvm_arch_vcpu_ioctl_debug_guest(struct kvm_vcpu *vcpu, - struct kvm_debug_guest *dbg) +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, + struct kvm_guest_debug *dbg) { int r; @@ -3598,6 +3594,11 @@ int kvm_arch_vcpu_ioctl_debug_guest(stru r = kvm_x86_ops->set_guest_debug(vcpu, dbg); + if (dbg->control & KVM_GUESTDBG_INJECT_DB) + kvm_queue_exception(vcpu, DB_VECTOR); + else if (dbg->control & KVM_GUESTDBG_INJECT_BP) + kvm_queue_exception(vcpu, BP_VECTOR); + vcpu_put(vcpu); return r; Index: b/include/asm-x86/kvm.h === --- a/include/asm-x86/kvm.h +++ b/include/asm-x86/kvm.h @@ -230,4 +230,21 @@ struct kvm_pit_state { #define KVM_TRC_APIC_ACCESS (KVM_TRC_HANDLER + 0x14) #define KVM_TRC_TDP_FAULT(KVM_TRC_HANDLER + 0x15) +struct kvm_debug_exit_arch { + __u32 exception; + __u32 pad; + __u64 dr6; + __u64 dr7; +}; + +#define KVM_GUESTDBG_USE_SW_BP 0x0001 +#define KVM_GUESTDBG_USE_HW_BP 0x0002 +#define KVM_GUESTDBG_INJECT_DB 0x0004 +#define KVM_GUESTDBG_INJECT_BP 0x0008 + +/* for KVM_SET_GUEST_DEBUG */ +struct kvm_guest_debug_arch { + __u64 debugreg[8]; +}; + #endif Index: b/include/asm-x86/kvm_host.h === --- a/include/asm-x86/kvm_host.h +++ b/include/asm-x86/kvm_host.h @@ -54,6 +54,8 @@ #define KVM_PAGES_PER_HPAGE (KVM_HPAGE_SIZE / PAGE_SIZE) #define DE_VECTOR 0 +#define DB_VECTOR 1 +#define BP_VECTOR 3 #define UD_VECTOR 6 #define NM_VECTOR 7 #define DF_VECTOR 8 @@ -122,12 +124,6 @@ enum { #define KVM_NR_MEM_OBJS 40 -struct kvm_guest_debug { - int enabled; - unsigned long bp[4]; - int singlestep; -}; - /* * We don't want allocation failures within the mmu code, so we preallocate * enough memory for a single page fault in a cache. @@ -383,8 +379,7 @@ struct kvm_x86_ops { void (*vcpu_put)(struct k
[PATCH 0/3] Rework guest debug interface
The existing guest debugging interface of kvm is limited in several ways: - depends on hardware breakpoints, thus suffers from running out of this scarce resource. - assumes that there are only 4 hardware breakpoints (even if some archs may once provide more) - doesn't support watchpoint or additional arch-specific debugging features This patch series reworks the interface, providing usable software-based breakpoint support, but also laying the ground for patches under development that will add full hardware debugging features. Content: Patch 1 - Replace KVM_DEBUG_GUEST with KVM_SET_GUEST_DEBUG interface, implement kernel side support Patch 2 - Tiny helper that allows pause and resume for a single vcpu Patch 3 - Userland changes that adopt KVM_SET_GUEST_DEBUG and switch to software breakpoints Depends on: - "Fix monitor and gdbstub deadlocks v2" [1] - "Proper vm_stop on debug events" [2] It's late here and the series is still fresh, but everything seems to works nicely - even using gdb inside the guest while the host has active breakpoints on the guest as well. Looking forward to feedback and review. Jan [1] http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/17444 [2] http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/17775 signature.asc Description: OpenPGP digital signature
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
On Thursday 22 May 2008 21:35:19 Jens Axboe wrote: > On Thu, May 22 2008, Rusty Russell wrote: > > Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to > > test here, it's my root block dev). Other drivers seem to do > > blk_cleanup_queue() *after* del_gendisk: does it matter? > > > > Jens CC'd: he's gentle with my dumb questions... Rusty. > > del_gendisk() can generate IO, so it would seem safer to do that > _before_ putting the queue reference :-) Thanks Jens. Chris, I've fixed that up here. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems with GRUB and boot=on flag
Alberto Treviño wrote: I am having a really weird problem. I reported about it earlier (see http://article.gmane.org/gmane.comp.emulators.kvm.devel/15884) thinking the problem was with SCSI disks, but that's not the case. Let me try to explain. I have a drive that has GRUB and Linux. If I run the following command, it boots normally, even though the boot flag has been set to off: qemu-system-x86_64 -m 256 -drive file=hd0.qcow2,if=ide,bus=0,index=0,media=disk,boot=off How did you install grub on this drive? Is this a distro image or did you install grub yourself? Regards, Anthony Liguori If I run the following, qemu crashes when GRUB loads: qemu-system-x86_64 -m 256 -drive file=hd0.qcow2,if=ide,bus=0,index=0,media=disk,boot=on Qemu reports the following when it crashes: exception 13 (33) rax rbx 0800 rcx rdx 00e0 rsi 7d98 rdi 0008f788 rsp 2018 rbp 0001 r8 r9 r10 r11 r12 r13 r14 r15 rip 0003 rflags 00033202 cs (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) ds (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) es 0800 (8000/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) ss (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) fs (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) gs (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) tr (fffbd000/2088 p 1 dpl 0 db 0 s 0 type b l 0 g 0 avl 0) ldt (/ p 1 dpl 0 db 0 s 0 type 2 l 0 g 0 avl 0) gdt fb1f2/30 idt 0/3ff cr0 10 cr2 0 cr3 0 cr4 0 cr8 0 efer 0 code: 53 ff 00 --> f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 a5 fe 00 f0 87 e9 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff Aborted If I change the interface to SCSI and set the boot flag to off, Qemu can't find a valid boot drive and does not boot. If I change the interface to SCSI and set the boot flag to on, I get the same crash. According to Anthony Liguori's original extboot post, "boot=on" turns on the extboot code, so my problem is the combination of extboot and GRUB. A response on my original post suggested this might be a real mode issue. However, it seems more of an extboot problem with some of the calls from GRUB. What's the status of the extboot in C project? Perhaps I could try that to see if it fixes the problem. Does anyone have any other suggestions? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Problems with GRUB and boot=on flag
I am having a really weird problem. I reported about it earlier (see http://article.gmane.org/gmane.comp.emulators.kvm.devel/15884) thinking the problem was with SCSI disks, but that's not the case. Let me try to explain. I have a drive that has GRUB and Linux. If I run the following command, it boots normally, even though the boot flag has been set to off: qemu-system-x86_64 -m 256 -drive file=hd0.qcow2,if=ide,bus=0,index=0,media=disk,boot=off If I run the following, qemu crashes when GRUB loads: qemu-system-x86_64 -m 256 -drive file=hd0.qcow2,if=ide,bus=0,index=0,media=disk,boot=on Qemu reports the following when it crashes: exception 13 (33) rax rbx 0800 rcx rdx 00e0 rsi 7d98 rdi 0008f788 rsp 2018 rbp 0001 r8 r9 r10 r11 r12 r13 r14 r15 rip 0003 rflags 00033202 cs (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) ds (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) es 0800 (8000/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) ss (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) fs (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) gs (/ p 1 dpl 3 db 0 s 1 type 3 l 0 g 0 avl 0) tr (fffbd000/2088 p 1 dpl 0 db 0 s 0 type b l 0 g 0 avl 0) ldt (/ p 1 dpl 0 db 0 s 0 type 2 l 0 g 0 avl 0) gdt fb1f2/30 idt 0/3ff cr0 10 cr2 0 cr3 0 cr4 0 cr8 0 efer 0 code: 53 ff 00 --> f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff 00 f0 a5 fe 00 f0 87 e9 00 f0 53 ff 00 f0 53 ff 00 f0 53 ff Aborted If I change the interface to SCSI and set the boot flag to off, Qemu can't find a valid boot drive and does not boot. If I change the interface to SCSI and set the boot flag to on, I get the same crash. According to Anthony Liguori's original extboot post, "boot=on" turns on the extboot code, so my problem is the combination of extboot and GRUB. A response on my original post suggested this might be a real mode issue. However, it seems more of an extboot problem with some of the calls from GRUB. What's the status of the extboot in C project? Perhaps I could try that to see if it fixes the problem. Does anyone have any other suggestions? -- Alberto Treviño [EMAIL PROTECTED] Testing Center Brigham Young University -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm-qemu: Proper vm_stop on debug events
Jan Kiszka wrote: When a vcpu exits after hitting a debug exception, we have to invoke vm_stop(EXCP_DEBUG). But this has to take place over the io-thread. This patch introduces kvm_debug_stop_requested to signal this event, and it takes care that the interrupted vcpu itself goes immediately into stop state. Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]> --- qemu/qemu-kvm.c |9 - 1 file changed, 8 insertions(+), 1 deletion(-) Index: b/qemu/qemu-kvm.c === --- a/qemu/qemu-kvm.c +++ b/qemu/qemu-kvm.c @@ -58,6 +58,8 @@ pthread_t io_thread; static int io_thread_fd = -1; static int io_thread_sigfd = -1; +static int kvm_debug_stop_requested; + Why use this instead of just keying off of exception_index == EXCP_DEBUG? static inline unsigned long kvm_get_thread_id(void) { return syscall(SYS_gettid); @@ -517,6 +519,10 @@ int kvm_main_loop(void) qemu_system_powerdown(); else if (qemu_reset_requested()) qemu_kvm_system_reset(); + else if (kvm_debug_stop_requested) { + kvm_debug_stop_requested = 0; + vm_stop(EXCP_DEBUG); + } } pause_all_threads(); @@ -529,7 +535,8 @@ static int kvm_debug(void *opaque, int v { CPUState *env = cpu_single_env; -env->exception_index = EXCP_DEBUG; +kvm_debug_stop_requested = 1; +vcpu_info[vcpu].stopped = 1; This isn't quite right. In the very least, you need to set stopping = 0 and signal on the qemu_pause_cond. Thinking it through more though, a breakpoint should stop all VCPUs, right? vm_stop(EXCP_DEBUG) will actually do this. It invokes the vm_state_notify callbacks and the io-thread registers one. I think you should probably just issue vm_stop(EXCP_DEBUG) from kvm_debug. Regards, Anthony Liguori return 1; } -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] kvm-qemu: Proper vm_stop on debug events
When a vcpu exits after hitting a debug exception, we have to invoke vm_stop(EXCP_DEBUG). But this has to take place over the io-thread. This patch introduces kvm_debug_stop_requested to signal this event, and it takes care that the interrupted vcpu itself goes immediately into stop state. Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]> --- qemu/qemu-kvm.c |9 - 1 file changed, 8 insertions(+), 1 deletion(-) Index: b/qemu/qemu-kvm.c === --- a/qemu/qemu-kvm.c +++ b/qemu/qemu-kvm.c @@ -58,6 +58,8 @@ pthread_t io_thread; static int io_thread_fd = -1; static int io_thread_sigfd = -1; +static int kvm_debug_stop_requested; + static inline unsigned long kvm_get_thread_id(void) { return syscall(SYS_gettid); @@ -517,6 +519,10 @@ int kvm_main_loop(void) qemu_system_powerdown(); else if (qemu_reset_requested()) qemu_kvm_system_reset(); + else if (kvm_debug_stop_requested) { + kvm_debug_stop_requested = 0; + vm_stop(EXCP_DEBUG); + } } pause_all_threads(); @@ -529,7 +535,8 @@ static int kvm_debug(void *opaque, int v { CPUState *env = cpu_single_env; -env->exception_index = EXCP_DEBUG; +kvm_debug_stop_requested = 1; +vcpu_info[vcpu].stopped = 1; return 1; } -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Patch - Big real mode emulation
On Thu, May 22, 2008 at 2:18 AM, Kamble, Nitin A <[EMAIL PROTECTED]> wrote: > Hi Guillaume, > Good progress so far. FYI, When I was looking at it the hardest part > for the big real mode support was from boot loaders from the open suse > Linux 10.1 & 10.2 install CDs. > In that the boot loader was painting fancy graphics on screen, which > needed above 1MB memory access. So it used the big real mode to do those > memory accesses in the syslinux boot loader. Well, with this patch I got openSUSE 10.2 Live CD working. It crashes at times, but this happens randomly (much like the issue that occured with Ubuntu Live CDs before) so I guess this is not something to do with big real mode. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] performance with guests running 2.4 kernels (specifically RHEL3)
The short answer is that I am still see large system time hiccups in the guests due to kscand in the guest scanning its active lists. I do see better response for a KVM_MAX_PTE_HISTORY of 3 than with 4. (For completeness I also tried a history of 2, but it performed worse than 3 which is no surprise given the meaning of it.) I have been able to scratch out a simplistic program that stimulates kscand activity similar to what is going on in my real guest (see attached). The program requests a memory allocation, initializes it (to get it backed) and then in a loop sweeps through the memory in chunks similar to a program using parts of its memory here and there but eventually accessing all of it. Start the RHEL3/CentOS 3 guest with *2GB* of RAM (or more). The key is using a fair amount of highmem. Start a couple of instances of the attached. For example, I've been using these 2: memuser 768M 120 5 300 memuser 384M 300 10 600 Together these instances take up a 1GB of RAM and once initialized consume very little CPU. On kvm they make kscand and kswapd go nuts every 5-15 minutes. For comparison, I do not see the same behavior for an identical setup running on esx 3.5. david Avi Kivity wrote: > Avi Kivity wrote: >> >> There are (at least) three options available: >> - detect and special-case this scenario >> - change the flood detector to be per page table instead of per vcpu >> - change the flood detector to look at a list of recently used page >> tables instead of the last page table >> >> I'm having a hard time trying to pick between the second and third >> options. >> > > The answer turns out to be "yes", so here's a patch that adds a pte > access history table for each shadowed guest page-table. Let me know if > it helps. Benchmarking a variety of workloads on all guests supported > by kvm is left as an exercise for the reader, but I suspect the patch > will either improve things all around, or can be modified to do so. > /* simple program to malloc memory, inialize it, and * then repetitively use it to keep it active. */ #include #include #include #include #include #include #include /* goal is to sweep memory every T1 sec by accessing a * percentage at a time and sleeping T2 sec in between accesses. * Once all the memory has been accessed, sleep for T3 sec * before starting the cycle over. */ #define T1 180 #define T2 5 #define T3 300 const char *timestamp(void); void usage(const char *prog) { fprintf(stderr, "\nusage: %s memlen{M|K}) [t1 t2 t3]\n", prog); } int main(int argc, char *argv[]) { int len; char *endp; int factor, endp_len; int start, incr; int t1 = T1, t2 = T2, t3 = T3; char *mem; char c = 0; if (argc < 2) { usage(basename(argv[0])); return 1; } /* * determine memory to request */ len = (int) strtol(argv[1], &endp, 0); factor = 1; endp_len = strlen(endp); if ((endp_len == 1) && ((*endp == 'M') || (*endp == 'm'))) factor = 1024 * 1024; else if ((endp_len == 1) && ((*endp == 'K') || (*endp == 'k'))) factor = 1024; else if (endp_len) { fprintf(stderr, "invalid memory len.\n"); return 1; } len *= factor; if (len == 0) { fprintf(stdout, "memory len is 0.\n"); return 1; } /* * convert times if given */ if (argc > 2) { if (argc < 5) { usage(basename(argv[0])); return 1; } t1 = atoi(argv[2]); t2 = atoi(argv[3]); t3 = atoi(argv[4]); } /* * amount of memory to sweep at one time */ if (t1 && t2) incr = len / t1 * t2; else incr = len; mem = (char *) malloc(len); if (mem == NULL) { fprintf(stderr, "malloc failed\n"); return 1; } printf("memory allocated. initializing to 0\n"); memset(mem, 0, len); start = 0; printf("%s starting memory update.\n", timestamp()); while (1) { c++; if (c == 0x7f) c = 0; memset(mem + start, c, incr); start += incr; if ((start >= len) || ((start + incr) >= len)) { printf("%s scan complete. sleeping %d\n", timestamp(), t3); start = 0; sleep(t3); printf("%s starting memory update.\n", timestamp()); } else if (t2) sleep(t2); } return 0; } const char *timestamp(void) { static char date[64]; struct timeval now; struct tm ltime; memset(date, 0, sizeof(date)); if (gettimeofday(&now, NULL) == 0) { if (localtime_r(&now.tv_sec,
Re: [Fwd: [PATCH]: Fix silly output for virtio devices in /proc/interrupts]
Rusty Russell wrote: On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote: Author: Chris Lalancette <[EMAIL PROTECTED]> Date: Thu May 15 09:04:55 2008 -0400 register_virtio_device was doing something silly, in that it was overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of /proc/interrupts, since when you request_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not have register_virtio_device do anything with the bus_id, and assume the higher level driver set it up properly. OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices. The following patch should fix it for s/390 (it's identical to the lguest patch), but would prefer testing (S/390-ers cc'd). It may actually be better for virtio to set this up. The problem is that if you have multiple transports that are registering virtio devices, it's impossible at the transport level to guarantee uniqueness while still using the "virtio%d" naming. Except the current scheme is no good, we'd have to push the dev_index into virtio too. Regards, Anthony Liguori === virtio: S/390 set name of virtio devices directly. Chris has a patch 'Fix silly output for virtio devices in /proc/interrupts' which requires callers to the virtio driver infrastructure to set the bus_ids themselves. This does that for s/390. Signed-off-by: Rusty Russell <[EMAIL PROTECTED]> Cc: Christian Borntraeger <[EMAIL PROTECTED]> Cc: Martin Schwidefsky <[EMAIL PROTECTED]> Cc: Carsten Otte <[EMAIL PROTECTED]> Cc: Heiko Carstens <[EMAIL PROTECTED]> Cc: Chris Lalancette <[EMAIL PROTECTED]> --- drivers/s390/kvm/kvm_virtio.c |2 ++ 1 file changed, 2 insertions(+) diff -r c903ef6b391f drivers/s390/kvm/kvm_virtio.c --- a/drivers/s390/kvm/kvm_virtio.c Thu May 22 22:31:31 2008 +1000 +++ b/drivers/s390/kvm/kvm_virtio.c Thu May 22 22:32:55 2008 +1000 @@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de kdev->vdev.dev.parent = &kvm_root; kdev->vdev.index = dev_index++; + snprintf(kdev->vdev.dev.bus_id, BUS_ID_SIZE, "virtio%d", +kdev->vdev.index); kdev->vdev.id.device = d->type; kdev->vdev.config = &kvm_vq_configspace_ops; kdev->desc = d; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix breakpoints coming into KVM that are corrupted from qemu
1 file changed, 2 insertions(+) qemu/qemu-kvm.c |2 ++ Bad breakpoints are being passed down to KVM due to the fact that breakpoint structure in the kvm_debug_guest structure are not being initialized. This fixes this. This fix can also be found in the RFC patches that Jan Kiszka has sent for adding soft breakpoints. Signed-off-by: Jerone Young <[EMAIL PROTECTED]> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c --- a/qemu/qemu-kvm.c +++ b/qemu/qemu-kvm.c @@ -704,6 +704,8 @@ int kvm_update_debugger(CPUState *env) struct kvm_debug_guest dbg; int i; +memset(dbg.breakpoints, 0, sizeof(dbg.breakpoints)); + dbg.enabled = 0; if (env->nb_breakpoints || env->singlestep_enabled) { dbg.enabled = 1; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] Fix for kvm qemu to return control to gdb stub on breakpoints
Anthony Liguori wrote: > Jan Kiszka wrote: >> Jerone Young wrote: >> >>> 1 file changed, 2 insertions(+) >>> qemu/qemu-kvm.c |2 ++ >>> >>> >>> Currently breakpoints do not fully work for x86 or any other arch >>> with kvm enable qemu. Control is not being returned by to the gdb >>> stub. This patch add back this ability to return control to the gdb >>> stub when a debug interrupt is hit. >>> >>> This is in the io thread so it's best to get comments on this. Is it >>> in the best place? Should more be done here? >>> >>> Signed-off-by: Jerone Young <[EMAIL PROTECTED]> >>> >>> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c >>> --- a/qemu/qemu-kvm.c >>> +++ b/qemu/qemu-kvm.c >>> @@ -352,6 +352,8 @@ static int kvm_main_loop_cpu(CPUState *e >>> update_regs_for_init(env); >>> if (!(env->hflags & HF_HALTED_MASK) && !info->init) >>> kvm_cpu_exec(env); >>> +if (env->exception_index == EXCP_DEBUG) >>> +vm_stop(EXCP_DEBUG); >>> >> >> This isn't enough, please see >> >> http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/17660 >> >> and the rest of that series - just RFC, but already usable. >> > > While I haven't looked through your whole series, the io-thread caused a > regression with gdb and while I don't think that this patch is enough of > a fix, it's certainly the right idea for fixing that regression. > > Soft breakpoints is a nice thing to do, but I think orthogonal to what > this patch is addressing. For sure this or a similar approach is required, and I'm fine if we fix this beforehand. I just pointed to my series to avoid duplicate and lengthy debugging of known and (hopefully) fixed issues. I already went through this. :) Jan signature.asc Description: OpenPGP digital signature
Re: [PATCH] [RFC] Fix for kvm qemu to return control to gdb stub on breakpoints
Jerone Young wrote: > This is mainly to try and fix the current debug state with using > hardware breakpoints (until your software breakpoint work is ready for Well, you may be lucky with hw-breakpoints on Intel, given userland is patched. But if you aren't (IIRC, current code leaks debug register settings outside the guest, e.g.), I wouldn't invest time here. The debug register handling requires a rewrite in order to support proper virtualization + guest debugging. Right now I'm trying the clean up my debug patches and get things out: 1. Reworked guest debug interface 2. Fixed and enhanced userland support (including soft-BPs) 3. debug register virtualization 4. guest debugging with hw-breakpoints/watchpoints I hope step 1 & 2 can be completed tonight, 3 & 4 finally over the weekend. > primetime). I actually do need to submit another patch outside of this > for initialization of some variables (which I see is in your patch too), > as it ends up passing down garbage. Yeah, that was another issue. > > For now this is a patch mainly just to get qemu to break into the gdb > stub when a debug interrupt comes in. Though I have a feeling more may > be needed. If you want to fix userland, you also have to ensure that the vcpu thread that received the breakpoint doesn't continue to run - see my patch. That said, if you have spare cycles left to spent on the debugging thing, I would try to re-schedule my work and push some packages over to your side. Just let me know! :) Jan signature.asc Description: OpenPGP digital signature
[PATCH] kvm-userland: Update .gitignore
Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]> --- .gitignore | 24 1 file changed, 24 insertions(+) Index: b/.gitignore === --- a/.gitignore +++ b/.gitignore @@ -33,3 +33,27 @@ bios/*.txt bios/acpi-dsdt.aml vgabios/*.bin vgabios/*.txt +extboot/extboot.bin +extboot/extboot.img +extboot/signrom +kernel/Module.markers +kernel/i825[49].[ch] +kernel/include-compat/asm +kernel/include-compat/asm-x86/asm-x86 +kernel/include +kernel/ioapic.[ch] +kernel/iodev.h +kernel/irq.[ch] +kernel/kvm_trace.c +kernel/lapic.[ch] +kernel/mmu.h +kernel/modules.order +kernel/tss.h +kernel/x86.c +qemu/pc-bios/extboot.bin +qemu/qemu-doc.html +qemu/*.1 +qemu/*.pod +qemu/qemu-tech.html +user/kvmtrace +user/test/x86/bootstrap -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 3/4] kvm-vmx: KVM_EXIT_DEBUG on #BP exceptions
On Thursday 22 May 2008 09:34:08 Avi Kivity wrote: > Hollis Blanchard wrote: > > In addition to injecting debug interrupts, qemu should also be able to inject > > machine checks, for the case of undecoded MMIO accesses. > > > > As long as qemu can access the complete register state (e.g. including "reason > > for machine check" registers), I don't think the injection interface *needs* > > to be more complicated than "exception number". [snip] > > In the mmio case, I think it makes more sense to have a 'mmio failed' > flag, and kvm can generate and inject the exception. OK, that sounds reasonable. > We mostly try to keep cpu emulation outside userspace. Except for this debug stuff? :) > (of course, that depends on what happens on real hardware. Is there a > machine check pin? or does the cpu generate the exception internally?) There is a machine check pin, FWIW. It's the chipset (potentially several IO bridges away) that discovers no device decoded the MMIO address, and asynchronously notifies the core. I'm happy with machine check as an "MMIO failed" flag, so I guess that leaves us with a separate "inject debug event" ioctl. -- Hollis Blanchard IBM Linux Technology Center -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] Fix for kvm qemu to return control to gdb stub on breakpoints
This is mainly to try and fix the current debug state with using hardware breakpoints (until your software breakpoint work is ready for primetime). I actually do need to submit another patch outside of this for initialization of some variables (which I see is in your patch too), as it ends up passing down garbage. For now this is a patch mainly just to get qemu to break into the gdb stub when a debug interrupt comes in. Though I have a feeling more may be needed. On Thu, 2008-05-22 at 18:43 +0200, Jan Kiszka wrote: > Jerone Young wrote: > > 1 file changed, 2 insertions(+) > > qemu/qemu-kvm.c |2 ++ > > > > > > Currently breakpoints do not fully work for x86 or any other arch with kvm > > enable qemu. Control is not being returned by to the gdb stub. This patch > > add back this ability to return control to the gdb stub when a debug > > interrupt is hit. > > > > This is in the io thread so it's best to get comments on this. Is it in the > > best place? Should more be done here? > > > > Signed-off-by: Jerone Young <[EMAIL PROTECTED]> > > > > diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c > > --- a/qemu/qemu-kvm.c > > +++ b/qemu/qemu-kvm.c > > @@ -352,6 +352,8 @@ static int kvm_main_loop_cpu(CPUState *e > > update_regs_for_init(env); > > if (!(env->hflags & HF_HALTED_MASK) && !info->init) > > kvm_cpu_exec(env); > > + if (env->exception_index == EXCP_DEBUG) > > + vm_stop(EXCP_DEBUG); > > This isn't enough, please see > > http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/17660 > > and the rest of that series - just RFC, but already usable. > > Jan > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] Fix for kvm qemu to return control to gdb stub on breakpoints
Jan Kiszka wrote: Jerone Young wrote: 1 file changed, 2 insertions(+) qemu/qemu-kvm.c |2 ++ Currently breakpoints do not fully work for x86 or any other arch with kvm enable qemu. Control is not being returned by to the gdb stub. This patch add back this ability to return control to the gdb stub when a debug interrupt is hit. This is in the io thread so it's best to get comments on this. Is it in the best place? Should more be done here? Signed-off-by: Jerone Young <[EMAIL PROTECTED]> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c --- a/qemu/qemu-kvm.c +++ b/qemu/qemu-kvm.c @@ -352,6 +352,8 @@ static int kvm_main_loop_cpu(CPUState *e update_regs_for_init(env); if (!(env->hflags & HF_HALTED_MASK) && !info->init) kvm_cpu_exec(env); + if (env->exception_index == EXCP_DEBUG) + vm_stop(EXCP_DEBUG); This isn't enough, please see http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/17660 and the rest of that series - just RFC, but already usable. While I haven't looked through your whole series, the io-thread caused a regression with gdb and while I don't think that this patch is enough of a fix, it's certainly the right idea for fixing that regression. Soft breakpoints is a nice thing to do, but I think orthogonal to what this patch is addressing. Regards, Anthony Liguori Jan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] Fix for kvm qemu to return control to gdb stub on breakpoints
Jerone Young wrote: > 1 file changed, 2 insertions(+) > qemu/qemu-kvm.c |2 ++ > > > Currently breakpoints do not fully work for x86 or any other arch with kvm > enable qemu. Control is not being returned by to the gdb stub. This patch add > back this ability to return control to the gdb stub when a debug interrupt is > hit. > > This is in the io thread so it's best to get comments on this. Is it in the > best place? Should more be done here? > > Signed-off-by: Jerone Young <[EMAIL PROTECTED]> > > diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c > --- a/qemu/qemu-kvm.c > +++ b/qemu/qemu-kvm.c > @@ -352,6 +352,8 @@ static int kvm_main_loop_cpu(CPUState *e > update_regs_for_init(env); > if (!(env->hflags & HF_HALTED_MASK) && !info->init) > kvm_cpu_exec(env); > + if (env->exception_index == EXCP_DEBUG) > + vm_stop(EXCP_DEBUG); This isn't enough, please see http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/17660 and the rest of that series - just RFC, but already usable. Jan signature.asc Description: OpenPGP digital signature
[PATCH] [RFC] Fix for kvm qemu to return control to gdb stub on breakpoints
1 file changed, 2 insertions(+) qemu/qemu-kvm.c |2 ++ Currently breakpoints do not fully work for x86 or any other arch with kvm enable qemu. Control is not being returned by to the gdb stub. This patch add back this ability to return control to the gdb stub when a debug interrupt is hit. This is in the io thread so it's best to get comments on this. Is it in the best place? Should more be done here? Signed-off-by: Jerone Young <[EMAIL PROTECTED]> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c --- a/qemu/qemu-kvm.c +++ b/qemu/qemu-kvm.c @@ -352,6 +352,8 @@ static int kvm_main_loop_cpu(CPUState *e update_regs_for_init(env); if (!(env->hflags & HF_HALTED_MASK) && !info->init) kvm_cpu_exec(env); + if (env->exception_index == EXCP_DEBUG) + vm_stop(EXCP_DEBUG); env->interrupt_request &= ~CPU_INTERRUPT_EXIT; kvm_main_loop_wait(env, 0); if (info->reload_regs) { -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 2/4] kvm: Arch-specifc KVM_EXIT_DEBUG payload
Jan Kiszka wrote: Well, might be useful. But then we would also have to teach this support to the gdb remote protocol and the gdb frontend, because neither of both seems to make use of it yet. And do other archs have it as well? If at all, I guess the way this is realized there varies a lot, and thus abstracting it is the real problem. However, we can keep it in mind for the time the basic feature set works perfectly. :) Right, no need to implement it now. But you can reserve some bytes in the ioctl argument. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 3/4] kvm-vmx: KVM_EXIT_DEBUG on #BP exceptions
Hollis Blanchard wrote: In addition to injecting debug interrupts, qemu should also be able to inject machine checks, for the case of undecoded MMIO accesses. As long as qemu can access the complete register state (e.g. including "reason for machine check" registers), I don't think the injection interface *needs* to be more complicated than "exception number". OTOH, I can see the argument for atomic injection operations, so to support that you'd end up with a sub-structure like kvm_exit, something like: struct kvm_inject_arch { u32 exception_type; union { struct machine_check { u32 mcsr; } mc; struct debug { } debug; }; } This stuff is completely arch-specific, so making a common "inject" ioctl number that simply calls arch function (like GET_SREGS) would be appropriate. In the mmio case, I think it makes more sense to have a 'mmio failed' flag, and kvm can generate and inject the exception. We mostly try to keep cpu emulation outside userspace. (of course, that depends on what happens on real hardware. Is there a machine check pin? or does the cpu generate the exception internally?) -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 2/4] kvm: Arch-specifc KVM_EXIT_DEBUG payload
Avi Kivity wrote: > Jan Kiszka wrote: >> >>> We need to handle branch tracing and last branch recording as well. It >>> doesn't have to be in this patchset, though. >>> >> >> IIRC, both features are fairly vendor specific. Do we have the same >> level of support on both AMD and Intel CPUs? Moreover, I doubt that the >> debug interface would be the right place for them. >> > > Branch tracing is common, and is very much related to debugging (it > modifies singlestep to trap only on branches, not on non-branching > insns). LBR is either common or very similar, and is also very > important for debugging. Well, might be useful. But then we would also have to teach this support to the gdb remote protocol and the gdb frontend, because neither of both seems to make use of it yet. And do other archs have it as well? If at all, I guess the way this is realized there varies a lot, and thus abstracting it is the real problem. However, we can keep it in mind for the time the basic feature set works perfectly. :) Jan signature.asc Description: OpenPGP digital signature
Re: [kvm-devel] [RFC][PATCH 3/4] kvm-vmx: KVM_EXIT_DEBUG on #BP exceptions
Jan Kiszka wrote: /me is still trying to find explicit statements in the Intel docs about what happens to the TF flag when the CPU enters an interrupt or an exception handler. This influences how single stepping guests can be realized, specifically when trying to step into guest's int3 handling... It's cleared. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 3/4] kvm-vmx: KVM_EXIT_DEBUG on #BP exceptions
On Thursday 22 May 2008 08:31:46 Jan Kiszka wrote: > Avi Kivity wrote: > > Jan Kiszka wrote: > >> In order to allow the gdbstub of QEMU to push (soft) breakpoint handling > >> completely into the gdb frontend, this patch enables guest exits also > >> for #BP exceptions - in case guest debugging was turned on. > >> > >> Along this enhancement, this patch also fixes the flag manipulation for > >> the singlestep mode. > >> > > > > Suppose userspace determines the exception is due to a guest > > breakpoint. How does it inject the debug exception? > > Good question. Is there no "inject exception #XX" mechanism in kvm yet? > > Will need this, as my current impression is that we better keep track of > breakpoints at qemu level to tell guest soft-BPs apart from host > injected ones. Would you suggest to add a separate IOCTL for exception > injection then? Or should the new guest debug IOCTL contain a flag that > signals "inject breakpoint trap" (both for guest soft-BP hits as well as > guests already in single step mode)? In addition to injecting debug interrupts, qemu should also be able to inject machine checks, for the case of undecoded MMIO accesses. As long as qemu can access the complete register state (e.g. including "reason for machine check" registers), I don't think the injection interface *needs* to be more complicated than "exception number". OTOH, I can see the argument for atomic injection operations, so to support that you'd end up with a sub-structure like kvm_exit, something like: struct kvm_inject_arch { u32 exception_type; union { struct machine_check { u32 mcsr; } mc; struct debug { } debug; }; } This stuff is completely arch-specific, so making a common "inject" ioctl number that simply calls arch function (like GET_SREGS) would be appropriate. -- Hollis Blanchard IBM Linux Technology Center -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 3/4] kvm-vmx: KVM_EXIT_DEBUG on #BP exceptions
Avi Kivity wrote: > Jan Kiszka wrote: >> Avi Kivity wrote: >> >>> Jan Kiszka wrote: >>> In order to allow the gdbstub of QEMU to push (soft) breakpoint handling completely into the gdb frontend, this patch enables guest exits also for #BP exceptions - in case guest debugging was turned on. Along this enhancement, this patch also fixes the flag manipulation for the singlestep mode. >>> Suppose userspace determines the exception is due to a guest >>> breakpoint. How does it inject the debug exception? >>> >> >> Good question. Is there no "inject exception #XX" mechanism in kvm yet? >> >> > > No userspace interface for it. The kernel injects plenty > (kvm_queue_exception). > >> Will need this, as my current impression is that we better keep track of >> breakpoints at qemu level to tell guest soft-BPs apart from host >> injected ones. Would you suggest to add a separate IOCTL for exception >> injection then? Or should the new guest debug IOCTL contain a flag that >> signals "inject breakpoint trap" (both for guest soft-BP hits as well as >> guests already in single step mode)? > > A debug specific thing may allow us to limit the generality of the > implementation. > > Or maybe, disable int 3 trapping, single step, reenable int 3 trapping > -> no need to inject vectors. /me is still trying to find explicit statements in the Intel docs about what happens to the TF flag when the CPU enters an interrupt or an exception handler. This influences how single stepping guests can be realized, specifically when trying to step into guest's int3 handling... Jan signature.asc Description: OpenPGP digital signature
Re: [PATCH 5 of 5] [kvm powerpc] Report bad GFNs
On Wednesday 21 May 2008 21:47:40 Anthony Liguori wrote: > Hollis Blanchard wrote: > > This code shouldn't be hit anyways, but when it is, it's useful to have a > > little more information about the failure. > > > > Signed-off-by: Hollis Blanchard <[EMAIL PROTECTED]> > > > > diff --git a/arch/powerpc/kvm/44x_tlb.c b/arch/powerpc/kvm/44x_tlb.c > > --- a/arch/powerpc/kvm/44x_tlb.c > > +++ b/arch/powerpc/kvm/44x_tlb.c > > @@ -145,7 +145,7 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcp > > down_read(¤t->mm->mmap_sem); > > new_page = gfn_to_page(vcpu->kvm, gfn); > > if (is_error_page(new_page)) { > > - printk(KERN_ERR "Couldn't get guest page!\n"); > > + printk(KERN_ERR "Couldn't get guest page for gfn %lx!\n", gfn); > > kvm_release_page_clean(new_page); > > up_read(¤t->mm->mmap_sem); > > return; > > > > FWIW, you should probably rate limit this sort of thing if this is what > would get triggered if the guest tried to program the tlb with an > invalid gfn. That was my initial thought as well, but in general, kvmppc_mmu_map() is only called if a TLB entry is "safe" -- meaning kvm_is_visible_gfn(kvm, gfn) was true. So in the absence of bugs elsewhere, this should never happen... but we know how that goes. ;) Anyways, that's why I'm not worried about rate limiting here. -- Hollis Blanchard IBM Linux Technology Center -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 2/4] kvm: Arch-specifc KVM_EXIT_DEBUG payload
Jan Kiszka wrote: We need to handle branch tracing and last branch recording as well. It doesn't have to be in this patchset, though. IIRC, both features are fairly vendor specific. Do we have the same level of support on both AMD and Intel CPUs? Moreover, I doubt that the debug interface would be the right place for them. Branch tracing is common, and is very much related to debugging (it modifies singlestep to trap only on branches, not on non-branching insns). LBR is either common or very similar, and is also very important for debugging. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 3/4] kvm-vmx: KVM_EXIT_DEBUG on #BP exceptions
Jan Kiszka wrote: Avi Kivity wrote: Jan Kiszka wrote: In order to allow the gdbstub of QEMU to push (soft) breakpoint handling completely into the gdb frontend, this patch enables guest exits also for #BP exceptions - in case guest debugging was turned on. Along this enhancement, this patch also fixes the flag manipulation for the singlestep mode. Suppose userspace determines the exception is due to a guest breakpoint. How does it inject the debug exception? Good question. Is there no "inject exception #XX" mechanism in kvm yet? No userspace interface for it. The kernel injects plenty (kvm_queue_exception). Will need this, as my current impression is that we better keep track of breakpoints at qemu level to tell guest soft-BPs apart from host injected ones. Would you suggest to add a separate IOCTL for exception injection then? Or should the new guest debug IOCTL contain a flag that signals "inject breakpoint trap" (both for guest soft-BP hits as well as guests already in single step mode)? A debug specific thing may allow us to limit the generality of the implementation. Or maybe, disable int 3 trapping, single step, reenable int 3 trapping -> no need to inject vectors. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 2/4] kvm: Arch-specifc KVM_EXIT_DEBUG payload
Avi Kivity wrote: > Jan Kiszka wrote: >> This adds an arch field to kvm_run.debug, the payload that is returned >> to user space on KVM_EXIT_DEBUG guest exits. For x86, this field is now >> supposed to report the precise debug exception (#DB or #BP) and the >> current state of the debug registers (the latter is not yet >> implemented). >> >> +struct kvm_debug_exit_arch { >> +__u32 exception; >> > > Need padding here. > >> +__u64 dr[8]; >> +}; >> + >> > > In general I prefer decoding bitfields into something more usable (see > for example the segment registers), but in this case, I guess the raw > registers are better. IMHO, there is no point in parsing those arch-specific bit field in kernel space as long as the kernel has nothing to do with them (except saving and restoring them). BTW, the upcoming version will look like this: struct kvm_debug_exit_arch { __u32 exception; __u32 pad; __u64 dr6; __u64 dr7; }; (No need for reporting unused or unmodified registers here.) > > We need to handle branch tracing and last branch recording as well. It > doesn't have to be in this patchset, though. IIRC, both features are fairly vendor specific. Do we have the same level of support on both AMD and Intel CPUs? Moreover, I doubt that the debug interface would be the right place for them. Jan signature.asc Description: OpenPGP digital signature
Re: [Fwd: [PATCH]: Fix silly output for virtio devices in /proc/interrupts]
Am Donnerstag, 22. Mai 2008 schrieb Rusty Russell: > virtio: S/390 set name of virtio devices directly. > > Chris has a patch 'Fix silly output for virtio devices in /proc/interrupts' > which requires callers to the virtio driver infrastructure to set the bus_ids > themselves. This does that for s/390. > > Signed-off-by: Rusty Russell <[EMAIL PROTECTED]> > Cc: Christian Borntraeger <[EMAIL PROTECTED]> > Cc: Martin Schwidefsky <[EMAIL PROTECTED]> > Cc: Carsten Otte <[EMAIL PROTECTED]> > Cc: Heiko Carstens <[EMAIL PROTECTED]> > Cc: Chris Lalancette <[EMAIL PROTECTED]> > --- > drivers/s390/kvm/kvm_virtio.c |2 ++ > 1 file changed, 2 insertions(+) > > diff -r c903ef6b391f drivers/s390/kvm/kvm_virtio.c > --- a/drivers/s390/kvm/kvm_virtio.c Thu May 22 22:31:31 2008 +1000 > +++ b/drivers/s390/kvm/kvm_virtio.c Thu May 22 22:32:55 2008 +1000 > @@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de > > kdev->vdev.dev.parent = &kvm_root; > kdev->vdev.index = dev_index++; > + snprintf(kdev->vdev.dev.bus_id, BUS_ID_SIZE, "virtio%d", > + kdev->vdev.index); > kdev->vdev.id.device = d->type; > kdev->vdev.config = &kvm_vq_configspace_ops; > kdev->desc = d; > > Tested-by: Christian Borntraeger <[EMAIL PROTECTED]> Acked-by: Christian Borntraeger <[EMAIL PROTECTED]> This will probably create a fuzzy hunk with another patch from Cornelia Huck (CCed) which will be send to Gregs tree soon. This patch converts kvm_root to dynamic allocation, but the fuzz will be easy to fix. Christian -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] [RFC][PATCH 3/4] kvm-vmx: KVM_EXIT_DEBUG on #BP exceptions
Avi Kivity wrote: > Jan Kiszka wrote: >> In order to allow the gdbstub of QEMU to push (soft) breakpoint handling >> completely into the gdb frontend, this patch enables guest exits also >> for #BP exceptions - in case guest debugging was turned on. >> >> Along this enhancement, this patch also fixes the flag manipulation for >> the singlestep mode. >> > > Suppose userspace determines the exception is due to a guest > breakpoint. How does it inject the debug exception? Good question. Is there no "inject exception #XX" mechanism in kvm yet? Will need this, as my current impression is that we better keep track of breakpoints at qemu level to tell guest soft-BPs apart from host injected ones. Would you suggest to add a separate IOCTL for exception injection then? Or should the new guest debug IOCTL contain a flag that signals "inject breakpoint trap" (both for guest soft-BP hits as well as guests already in single step mode)? Jan signature.asc Description: OpenPGP digital signature
Re: [kvm-devel] [RFC][PATCH 2/4] kvm: Arch-specifc KVM_EXIT_DEBUG payload
Avi Kivity wrote: > Jan Kiszka wrote: >> This adds an arch field to kvm_run.debug, the payload that is returned >> to user space on KVM_EXIT_DEBUG guest exits. For x86, this field is now >> supposed to report the precise debug exception (#DB or #BP) and the >> current state of the debug registers (the latter is not yet >> implemented). >> >> Index: b/include/asm-x86/kvm.h >> === >> --- a/include/asm-x86/kvm.h >> +++ b/include/asm-x86/kvm.h >> @@ -230,4 +230,9 @@ struct kvm_pit_state { >> #define KVM_TRC_APIC_ACCESS (KVM_TRC_HANDLER + 0x14) >> #define KVM_TRC_TDP_FAULT(KVM_TRC_HANDLER + 0x15) >> >> +struct kvm_debug_exit_arch { >> +__u32 exception; >> +__u64 dr[8]; >> +}; >> + >> > > Need empty structures for non-x86. > > Need a KVM_CAP_ to indicate presence of this feature. Have all this in patch already, but need time to finish it, test it, and roll it out. There are two usability issues I would like to resolve first (to see if that has impact on the kernel-user interface), see following posts. Jan signature.asc Description: OpenPGP digital signature
Re: [kvm-devel] Vmware ESX under KVM - guest GPF in installer
Avi Kivity wrote: Yes, EFER support was added recently. Are you using the modules from kvm-66 or from your host kernel? Please try kvm-66's modules and report. I was able to boot the esx installer (I had to specify -smp 2 as there seem to be problems with the mptable on uniprocessors). esx 3.5 didn't want to run, though, complaining about the lack of mtrr support. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-devel] Vmware ESX under KVM - guest GPF in installer
Ian Kirk wrote: Hi, I wanted to try running Vmware ESX without the effort of finding a physical machine. Googling around I found that it is possible to run ESX under other hypervisors/emulators, e.g. Vmware Workstation. So I thought i'd give KVM a go. Using kvm-66 on 2.6.24.4-64.fc8PAE, the (mostly text mode) installer crashes after starting with "esx text". (I can't easily test kvm-68 at the mo due to needing to do a reboot of the host) Microsoft Virtual Server R2-SP1 kinda works, however the virtual NIC is unsupported by the ESX installer. Qemu 0.9.1 (for Windows) fails at a similar point. Is this a "fixed in newer kvm" or a generic problem (be it kvm or ESX installer CDROM kernel) ? I hadn't even got as far as installing/running ESX. Regards, Ian host dmesg says: kvm: emulating exchange as write kvm: 9329: cpu0 unhandled rdmsr: 0xc080 kvm: 9350: cpu0 unhandled rdmsr: 0xc080 Yes, EFER support was added recently. Are you using the modules from kvm-66 or from your host kernel? Please try kvm-66's modules and report. I was able to boot the esx installer (I had to specify -smp 2 as there seem to be problems with the mptable on uniprocessors). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix silly output for virtio devices in /proc/interrupts]
Rusty Russell wrote: > On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote: >> Author: Chris Lalancette <[EMAIL PROTECTED]> >> Date: Thu May 15 09:04:55 2008 -0400 >> >> register_virtio_device was doing something silly, in that it was >> overwriting what the calling driver stuck into .bus_id" for the name. This >> caused problems in the output of /proc/interrupts, since when you >> request_irq(), it doesn't actually copy the devname you pass in but just >> stores a pointer to the data. The fix is to just not have >> register_virtio_device do anything with the bus_id, and assume the higher >> level driver set it up properly. > > OK, but only one higher-level driver will set it up properly: kvm. Neither > lguest nor s/390 do this, and as a result, they fail to register *any* > devices. Ah, OK. Alternatively, we could do: snprintf(bus_id, BUS_ID_SIZE, "virtio%d", index) in register_virtio_device(), and just fix the one user who does it themselves (kvm) to not duplicate the work. Either way is fine with me. Chris Lalancette -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix silly output for virtio devices in /proc/interrupts]
On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote: > Author: Chris Lalancette <[EMAIL PROTECTED]> > Date: Thu May 15 09:04:55 2008 -0400 > > register_virtio_device was doing something silly, in that it was > overwriting what the calling driver stuck into .bus_id" for the name. This > caused problems in the output of /proc/interrupts, since when you > request_irq(), it doesn't actually copy the devname you pass in but just > stores a pointer to the data. The fix is to just not have > register_virtio_device do anything with the bus_id, and assume the higher > level driver set it up properly. OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices. The following patch should fix it for s/390 (it's identical to the lguest patch), but would prefer testing (S/390-ers cc'd). === virtio: S/390 set name of virtio devices directly. Chris has a patch 'Fix silly output for virtio devices in /proc/interrupts' which requires callers to the virtio driver infrastructure to set the bus_ids themselves. This does that for s/390. Signed-off-by: Rusty Russell <[EMAIL PROTECTED]> Cc: Christian Borntraeger <[EMAIL PROTECTED]> Cc: Martin Schwidefsky <[EMAIL PROTECTED]> Cc: Carsten Otte <[EMAIL PROTECTED]> Cc: Heiko Carstens <[EMAIL PROTECTED]> Cc: Chris Lalancette <[EMAIL PROTECTED]> --- drivers/s390/kvm/kvm_virtio.c |2 ++ 1 file changed, 2 insertions(+) diff -r c903ef6b391f drivers/s390/kvm/kvm_virtio.c --- a/drivers/s390/kvm/kvm_virtio.c Thu May 22 22:31:31 2008 +1000 +++ b/drivers/s390/kvm/kvm_virtio.c Thu May 22 22:32:55 2008 +1000 @@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de kdev->vdev.dev.parent = &kvm_root; kdev->vdev.index = dev_index++; + snprintf(kdev->vdev.dev.bus_id, BUS_ID_SIZE, "virtio%d", +kdev->vdev.index); kdev->vdev.id.device = d->type; kdev->vdev.config = &kvm_vq_configspace_ops; kdev->desc = d; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
On Thu, May 22 2008, Chris Lalancette wrote: > Jens Axboe wrote: > > On Thu, May 22 2008, Rusty Russell wrote: > >> On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote: > >>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > >>> index 4962e62..c678ac5 100644 > >>> --- a/drivers/block/virtio_blk.c > >>> +++ b/drivers/block/virtio_blk.c > >>> @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev) > >>> vdev->config->reset(vdev); > >>> > >>> blk_cleanup_queue(vblk->disk->queue); > >>> + del_gendisk(vblk->disk); > >>> put_disk(vblk->disk); > >>> unregister_blkdev(major, "virtblk"); > >>> mempool_destroy(vblk->pool); > >> Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to > >> test here, it's my root block dev). Other drivers seem to do > >> blk_cleanup_queue() *after* del_gendisk: does it matter? > >> > >> Jens CC'd: he's gentle with my dumb questions... Rusty. > > > > del_gendisk() can generate IO, so it would seem safer to do that > > _before_ putting the queue reference :-) > > > > Ah, good to know. Out of curiousity, why would del_gendisk() generate > additional I/O? It does invalidate+sync. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
Jens Axboe wrote: > On Thu, May 22 2008, Rusty Russell wrote: >> On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote: >>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c >>> index 4962e62..c678ac5 100644 >>> --- a/drivers/block/virtio_blk.c >>> +++ b/drivers/block/virtio_blk.c >>> @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev) >>> vdev->config->reset(vdev); >>> >>> blk_cleanup_queue(vblk->disk->queue); >>> + del_gendisk(vblk->disk); >>> put_disk(vblk->disk); >>> unregister_blkdev(major, "virtblk"); >>> mempool_destroy(vblk->pool); >> Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to >> test here, it's my root block dev). Other drivers seem to do >> blk_cleanup_queue() *after* del_gendisk: does it matter? >> >> Jens CC'd: he's gentle with my dumb questions... Rusty. > > del_gendisk() can generate IO, so it would seem safer to do that > _before_ putting the queue reference :-) > Ah, good to know. Out of curiousity, why would del_gendisk() generate additional I/O? Chris Lalancette -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
On Thu, May 22 2008, Rusty Russell wrote: > On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote: > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > > index 4962e62..c678ac5 100644 > > --- a/drivers/block/virtio_blk.c > > +++ b/drivers/block/virtio_blk.c > > @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev) > > vdev->config->reset(vdev); > > > > blk_cleanup_queue(vblk->disk->queue); > > + del_gendisk(vblk->disk); > > put_disk(vblk->disk); > > unregister_blkdev(major, "virtblk"); > > mempool_destroy(vblk->pool); > > Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to > test here, it's my root block dev). Other drivers seem to do > blk_cleanup_queue() *after* del_gendisk: does it matter? > > Jens CC'd: he's gentle with my dumb questions... Rusty. del_gendisk() can generate IO, so it would seem safer to do that _before_ putting the queue reference :-) -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote: > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > index 4962e62..c678ac5 100644 > --- a/drivers/block/virtio_blk.c > +++ b/drivers/block/virtio_blk.c > @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev) > vdev->config->reset(vdev); > > blk_cleanup_queue(vblk->disk->queue); > + del_gendisk(vblk->disk); > put_disk(vblk->disk); > unregister_blkdev(major, "virtblk"); > mempool_destroy(vblk->pool); Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to test here, it's my root block dev). Other drivers seem to do blk_cleanup_queue() *after* del_gendisk: does it matter? Jens CC'd: he's gentle with my dumb questions... Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -mm 0/2] x86: per-device dma_mapping_ops
On Monday 19 May 2008 12:01:27 FUJITA Tomonori wrote: > This is an updated version of the patchset to add per-device > dma_mapping_ops support for CONFIG_X86_64 like POWER architecture > does: > > http://lkml.org/lkml/2008/5/13/36 > > This is against 2.6.26-rc2-mm1 (the changes since the v1 are pretty > trivial like dropping the change for v850 arch). > > This enables us to cleanly fix the Calgary IOMMU issue that some > devices are not behind the IOMMU [1]. > > I think that per-device dma_mapping_ops support would be also helpful > for KVM people to support PCI passthrough but Andi thinks that this > makes it difficult to support the PCI passthrough (see the above > thread). So I CC'ed this to KVM camp. Comments are appreciated. I think what's more useful is a chain with a properly defined order or hierarchy (based on what Muli suggested last time we discussed this http://lkml.org/lkml/2007/11/12/44 ) The suggested order was (in calling order): pvdma->hardare->nommu/swiotlb The discussion in the thread pointed to above has details as to why. > A pointer to dma_mapping_ops to struct dev_archdata is added. If the > pointer is non NULL, DMA operations in asm/dma-mapping.h use it. If > it's NULL, the system-wide dma_ops pointer is used as before. > If it's useful for KVM people, I plan to implement a mechanism to > register a hook called when a new pci (or dma capable) device is OK; this sounds helpful. the hook can make a hypercall and confirm with the host kernel if the device in question is an assigned physical device. If yes, we replace the dma_ops. Though, the original intent of having stackable ops is that we might want to go through the swiotlb in the guest even for an assigned device if the guest dma addresses are not in the addressable range of the guest chipset. > created (it works with hot plugging). It enables IOMMUs to set up an > appropriate dma_mapping_ops per device. >From what we've discussed so far, it looks like stackable dma ops will definitely be needed. Does this patchset provide something that stacking won't? > [1] http://lkml.org/lkml/2008/5/8/423 Amit. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUGFIX/PATCH 0/6] kvm-s390 fixes
Carsten Otte wrote: Hi Avi, this series contains a few Bug fixes of issues that we've identified. I consider them all safe for 2.6.26, and in fact some of them fix serious issues in the current upstream code. Applied and queued all. Martin, can you ack patch 2, which is in s390 code, not kvm code (although it is only triggered when kvm is invoked AFAICT). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Patch - Big real mode emulation
On Wed, 21 May 2008 13:18:05 -0300 Marcelo Tosatti <[EMAIL PROTECTED]> wrote: > On Wed, May 21, 2008 at 11:34:10AM +0200, Guillaume Thouvenin wrote: > > Hello, > > > > Opensuse 10.3 is it uses a version of gfxboot that reads SS after > > switching from real to protected mode, where SS contains an invalid > > value, which VMX does not allow. So this patch > > > > > add: /* add */ > > + if ((c->d & ModRM) && c->modrm_mod == 3) { > > + c->dst.bytes = (c->d & ByteOp) ? 1 : c->op_bytes; > > + c->dst.ptr = decode_register(c->modrm_rm, c->regs, > > c->d & ByteOp); > > + } > > emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags); > > break; > > I don't see any difference from the previous patch here (on the issue that > add result was stored in the wrong register) ? > > 6486: 66 64 89 3e 72 01 mov%edi,%fs:0x172 > 648c: 66 be 8d 03 00 00 mov$0x38d,%esi > 6492: 66 c1 e6 04 shl$0x4,%esi > 6496: 66 b8 98 0a 00 00 mov$0xa98,%eax > 649c: 66 03 f0add%eax,%esi > > So "66 03 f0" stores result in eax instead of esi. And of course this > can be fatal (in the FreeDOS case the TSS data was copied to a wrong > location). Better fix that before merging. Hi Marcelo, Oops yes, thank you for pointing this out. I saw your remarks in the previous thread but forgot to fix this issue, sorry. I will add it with the next patches. Thanks for your help, Guillaume -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Patch - Big real mode emulation
On Wed, 21 May 2008 17:10:52 +0300 Avi Kivity <[EMAIL PROTECTED]> wrote: > Avi Kivity wrote: > >> @@ -1342,6 +1346,10 @@ special_insn: > >> switch (c->b) { > >> case 0x00 ... 0x05: > >> add: /* add */ > >> + if ((c->d & ModRM) && c->modrm_mod == 3) { > >> + c->dst.bytes = (c->d & ByteOp) ? 1 : c->op_bytes; > >> + c->dst.ptr = decode_register(c->modrm_rm, c->regs, c->d & ByteOp); > >> + } > > > > This is not specific to the add instruction. We really need to switch > > decode_modrm() to decode into a struct operand. > > > > Btw, I see that the decoder already handles this (see DstMem decoding > for the case where mod == 3). Is this code really needed now? Right. What misses in the decoder is the update of c->dst.bytes. I noticed that without the update, the value of c->dst.bytes was equal to 0 during the emulation and then, emulate_2op_SrcV("add",...) did nothing. So I will move the update of c->dst.bytes in the decoder. Otherwise, thank you for reviewing the patch. I'm working on the patch according to your remarks. Best regards, Guillaume -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html