[BUG] Oops with KVM-27

[BUG] Oops with KVM-27

[Luca Tettamanti]

Hello,
my kernel just exploded :)

The host is running 2.6-git-current, with KVM modules from KVM-27
package. kernel is 32bit, SMP, with PREEMPT enabled, no HIGHMEM (but I'm
using CONFIG_VMSPLIT_3G_OPT=y). The CPU is a Core2 (hence I'm using
kvm-intel).
Guest was a Fedora7 setup DVD, which died somewhere during the
installation (anaconda was already active at that point). Bad news is
that I cannot reproduce the bug :|

This is the OOPS:

kvm: emulating exchange as write
------------[ cut here ]------------
kernel BUG at /root/kvm-27/kernel/mmu.c:276!
invalid opcode: 0000 [#1]
PREEMPT SMP 
Modules linked in: ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack ip_tables x_tables rtc_core rtc_lib tun kvm_intel kvm radeon drm binfmt_misc nfs button cpufreq_stats cpufreq_userspace cpufreq_powersave cpufreq_conservative cls_u32 cls_route sch_sfq sch_cbq des cbc blkcipher sha1 md5 hmac crypto_hash cryptomgr crypto_algapi nfsd exportfs lockd sunrpc vfat fat nls_base fuse cpufreq_ondemand acpi_cpufreq freq_table i2c_isa ipv6 snd_hda_intel snd_pcm_oss snd_mixer_oss snd_pcm snd_timer ohci1394 snd intel_agp ieee1394 parport_pc atl1 uhci_hcd ehci_hcd i2c_i801 agpgart soundcore snd_page_alloc parport e100 usbcore mii dm_snapshot dm_mod thermal processor fan pata_ali sata_uli reiserfs xfs
CPU:    0
EIP:    0060:[<f24ad9b6>]    Not tainted VLI
EFLAGS: 00010246   (2.6.22-rc3-libata-gf285e3d3-dirty #67)
EIP is at mmu_memory_cache_alloc+0xd/0x36 [kvm]
eax: 00000000   ebx: 00000000   ecx: db19f2f4   edx: 0000002c
esi: 00000322   edi: db19e528   ebp: 00003d1d   esp: ca73fc80
ds: 007b   es: 007b   fs: 00d8  gs: 0000  ss: 0068
Process qemu (pid: 2680, ti=ca73f000 task=cc3160f0 task.ti=ca73f000)
Stack: f24aa0e2 00000000 db19e528 f24ae1eb db19f0b4 00000000 e21458e8 b129b4a0 
       db19f0b4 00003d1d 00000018 c0000000 f24ae3ea 00000002 00000000 00000000 
       00000000 00000003 db19f0b4 00003d1d 00000003 00000003 f24ae4b6 db19f0b4 
Call Trace:
 [<f24aa0e2>] gfn_to_page+0x14/0x27 [kvm]
 [<f24ae1eb>] kvm_mmu_get_page+0x1b2/0x31c [kvm]
 [<f24ae3ea>] mmu_alloc_roots+0x95/0xf0 [kvm]
 [<f24ae4b6>] paging_new_cr3+0x21/0x48 [kvm]
 [<f24aab16>] set_cr3+0x82/0x8c [kvm]
 [<f249b51d>] handle_cr+0xf8/0x253 [kvm_intel]
 [<f249b1b5>] handle_exception+0x120/0x208 [kvm_intel]
 [<f249af6c>] vmx_vcpu_run+0x59d/0x6c6 [kvm_intel]
 [<b02f2bb6>] __mutex_lock_slowpath+0x259/0x261
 [<f2499f34>] vmx_vcpu_load+0x2a/0xcc [kvm_intel]
 [<f24ac7d1>] kvm_vcpu_ioctl+0x29a/0xcff [kvm]
 [<b0294ea5>] sock_common_recvmsg+0x3e/0x54
 [<b029396f>] sock_recvmsg+0xcf/0xe8
 [<b0293a44>] sock_sendmsg+0xbc/0xd4
 [<b0131c35>] autoremove_wake_function+0x0/0x35
 [<b0172133>] core_sys_select+0x234/0x30f
 [<b0103189>] setup_sigcontext+0x105/0x189
 [<b02f41cf>] _spin_unlock_irq+0x20/0x41
 [<b013b1ee>] trace_hardirqs_on+0x11a/0x13d
 [<b0103a56>] do_notify_resume+0x5d1/0x611
 [<b02f41da>] _spin_unlock_irq+0x2b/0x41
 [<b01039b4>] do_notify_resume+0x52f/0x611
 [<b010898b>] convert_fxsr_from_user+0x26/0xe6
 [<f24ac537>] kvm_vcpu_ioctl+0x0/0xcff [kvm]
 [<b0171007>] do_ioctl+0x1f/0x62
 [<b0171281>] vfs_ioctl+0x237/0x249
 [<b01712c6>] sys_ioctl+0x33/0x4d
 [<b0103e78>] syscall_call+0x7/0xb
 =======================
Code: 01 00 00 e8 ce ff ff ff 8d 83 ec 01 00 00 81 c3 40 02 00 00 e8 bd ff ff ff 89 d8 5b eb b8 57 89 c1 53 83 ec 04 8b 00 85 c0 75 04 <0f> 0b eb fe 48 8b 5c 81 04 89 01 89 d1 31 c0 c1 e9 02 89 df f3 
EIP: [<f24ad9b6>] mmu_memory_cache_alloc+0xd/0x36 [kvm] SS:ESP 0068:ca73fc80
note: qemu[2680] exited with preempt_count 2
BUG: sleeping function called from invalid context at /home/kronos/src/linux-2.6.git/kernel/rwsem.c:20
in_atomic():1, irqs_disabled():0
INFO: lockdep is turned off.
 [<b01348ca>] down_read+0x15/0x49
 [<b013e800>] futex_wake+0x19/0xb3
 [<b013e919>] do_futex+0x7f/0xaad
 [<b01caa47>] vsnprintf+0x450/0x48c
 [<b02f4197>] _spin_unlock_irqrestore+0x40/0x58
 [<b01218f0>] release_console_sem+0x1a0/0x1b9
 [<b0121d88>] vprintk+0x2b7/0x305
 [<b011c716>] try_to_wake_up+0x325/0x32f
 [<b013f40f>] sys_futex+0xc8/0xda
 [<b011f578>] mm_release+0x81/0x87
 [<b0122cf7>] exit_mm+0x13/0xd6
 [<b012410a>] do_exit+0x1bc/0x69b
 [<b01053f1>] die+0x1e5/0x211
 [<b0105784>] do_invalid_op+0x0/0x8a
 [<b0105805>] do_invalid_op+0x81/0x8a
 [<f24ad9b6>] mmu_memory_cache_alloc+0xd/0x36 [kvm]
 [<f24ae61f>] page_header_update_slot+0x1e/0x4b [kvm]
 [<f24ae8b5>] paging32_set_pte_common+0x269/0x2a1 [kvm]
 [<b02f4432>] error_code+0x72/0x78
 [<f24ad9b6>] mmu_memory_cache_alloc+0xd/0x36 [kvm]
 [<f24aa0e2>] gfn_to_page+0x14/0x27 [kvm]
 [<f24ae1eb>] kvm_mmu_get_page+0x1b2/0x31c [kvm]
 [<f24ae3ea>] mmu_alloc_roots+0x95/0xf0 [kvm]
 [<f24ae4b6>] paging_new_cr3+0x21/0x48 [kvm]
 [<f24aab16>] set_cr3+0x82/0x8c [kvm]
 [<f249b51d>] handle_cr+0xf8/0x253 [kvm_intel]
 [<f249b1b5>] handle_exception+0x120/0x208 [kvm_intel]
 [<f249af6c>] vmx_vcpu_run+0x59d/0x6c6 [kvm_intel]
 [<b02f2bb6>] __mutex_lock_slowpath+0x259/0x261
 [<f2499f34>] vmx_vcpu_load+0x2a/0xcc [kvm_intel]
 [<f24ac7d1>] kvm_vcpu_ioctl+0x29a/0xcff [kvm]
 [<b0294ea5>] sock_common_recvmsg+0x3e/0x54
 [<b029396f>] sock_recvmsg+0xcf/0xe8
 [<b0293a44>] sock_sendmsg+0xbc/0xd4
 [<b0131c35>] autoremove_wake_function+0x0/0x35
 [<b0172133>] core_sys_select+0x234/0x30f
 [<b0103189>] setup_sigcontext+0x105/0x189
 [<b02f41cf>] _spin_unlock_irq+0x20/0x41
 [<b013b1ee>] trace_hardirqs_on+0x11a/0x13d
 [<b0103a56>] do_notify_resume+0x5d1/0x611
 [<b02f41da>] _spin_unlock_irq+0x2b/0x41
 [<b01039b4>] do_notify_resume+0x52f/0x611
 [<b010898b>] convert_fxsr_from_user+0x26/0xe6
 [<f24ac537>] kvm_vcpu_ioctl+0x0/0xcff [kvm]
 [<b0171007>] do_ioctl+0x1f/0x62
 [<b0171281>] vfs_ioctl+0x237/0x249
 [<b01712c6>] sys_ioctl+0x33/0x4d
 [<b0103e78>] syscall_call+0x7/0xb
 =======================

Luca
-- 
Il coraggio non mi manca.
E` la paura che mi frega...

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

Luca Tettamanti wrote:
> Hello,
> my kernel just exploded :)
>
> The host is running 2.6-git-current, with KVM modules from KVM-27
> package. kernel is 32bit, SMP, with PREEMPT enabled, no HIGHMEM (but I'm
> using CONFIG_VMSPLIT_3G_OPT=y). The CPU is a Core2 (hence I'm using
> kvm-intel).
> Guest was a Fedora7 setup DVD, which died somewhere during the
> installation (anaconda was already active at that point). Bad news is
> that I cannot reproduce the bug :|
>   
Fortunately the trace clearly shows the problem (out of mmu working 
memory on guest context switch).  The attached patch should fix it.  Let 
me know if it works for you.


-- 
error compiling committee.c: too many arguments to function


[-- Attachment #2: kvm-fix-oops-on-guest-context-switch.patch --]
[-- Type: text/x-patch, Size: 4551 bytes --]

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index 0632d0b..0fdd5a6 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -543,6 +543,8 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 		       const u8 *old, const u8 *new, int bytes);
 int kvm_mmu_unprotect_page_virt(struct kvm_vcpu *vcpu, gva_t gva);
 void kvm_mmu_free_some_pages(struct kvm_vcpu *vcpu);
+int kvm_mmu_load(struct kvm_vcpu *vcpu);
+void kvm_mmu_unload(struct kvm_vcpu *vcpu);
 
 int kvm_hypercall(struct kvm_vcpu *vcpu, struct kvm_run *run);
 
@@ -554,6 +556,14 @@ static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
 	return vcpu->mmu.page_fault(vcpu, gva, error_code);
 }
 
+static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
+{
+	if (likely(vcpu->mmu.root_hpa != INVALID_PAGE))
+		return 0;
+
+	return kvm_mmu_load(vcpu);
+}
+
 static inline int is_long_mode(struct kvm_vcpu *vcpu)
 {
 #ifdef CONFIG_X86_64
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
diff --git a/drivers/kvm/mmu.c b/drivers/kvm/mmu.c
index 283df03..5915d7a 100644
--- a/drivers/kvm/mmu.c
+++ b/drivers/kvm/mmu.c
@@ -949,9 +949,7 @@ static int nonpaging_init_context(struct kvm_vcpu *vcpu)
 	context->free = nonpaging_free;
 	context->root_level = 0;
 	context->shadow_root_level = PT32E_ROOT_LEVEL;
-	mmu_alloc_roots(vcpu);
-	ASSERT(VALID_PAGE(context->root_hpa));
-	kvm_arch_ops->set_cr3(vcpu, context->root_hpa);
+	context->root_hpa = INVALID_PAGE;
 	return 0;
 }
 
@@ -965,11 +963,6 @@ static void paging_new_cr3(struct kvm_vcpu *vcpu)
 {
 	pgprintk("%s: cr3 %lx\n", __FUNCTION__, vcpu->cr3);
 	mmu_free_roots(vcpu);
-	if (unlikely(vcpu->kvm->n_free_mmu_pages < KVM_MIN_FREE_MMU_PAGES))
-		kvm_mmu_free_some_pages(vcpu);
-	mmu_alloc_roots(vcpu);
-	kvm_mmu_flush_tlb(vcpu);
-	kvm_arch_ops->set_cr3(vcpu, vcpu->mmu.root_hpa);
 }
 
 static void inject_page_fault(struct kvm_vcpu *vcpu,
@@ -1003,10 +996,7 @@ static int paging64_init_context_common(struct kvm_vcpu *vcpu, int level)
 	context->free = paging_free;
 	context->root_level = level;
 	context->shadow_root_level = level;
-	mmu_alloc_roots(vcpu);
-	ASSERT(VALID_PAGE(context->root_hpa));
-	kvm_arch_ops->set_cr3(vcpu, context->root_hpa |
-		    (vcpu->cr3 & (CR3_PCD_MASK | CR3_WPT_MASK)));
+	context->root_hpa = INVALID_PAGE;
 	return 0;
 }
 
@@ -1025,10 +1015,7 @@ static int paging32_init_context(struct kvm_vcpu *vcpu)
 	context->free = paging_free;
 	context->root_level = PT32_ROOT_LEVEL;
 	context->shadow_root_level = PT32E_ROOT_LEVEL;
-	mmu_alloc_roots(vcpu);
-	ASSERT(VALID_PAGE(context->root_hpa));
-	kvm_arch_ops->set_cr3(vcpu, context->root_hpa |
-		    (vcpu->cr3 & (CR3_PCD_MASK | CR3_WPT_MASK)));
+	context->root_hpa = INVALID_PAGE;
 	return 0;
 }
 
@@ -1042,7 +1029,6 @@ static int init_kvm_mmu(struct kvm_vcpu *vcpu)
 	ASSERT(vcpu);
 	ASSERT(!VALID_PAGE(vcpu->mmu.root_hpa));
 
-	mmu_topup_memory_caches(vcpu);
 	if (!is_paging(vcpu))
 		return nonpaging_init_context(vcpu);
 	else if (is_long_mode(vcpu))
@@ -1064,16 +1050,31 @@ static void destroy_kvm_mmu(struct kvm_vcpu *vcpu)
 
 int kvm_mmu_reset_context(struct kvm_vcpu *vcpu)
 {
+	destroy_kvm_mmu(vcpu);
+	return init_kvm_mmu(vcpu);
+}
+
+int kvm_mmu_load(struct kvm_vcpu *vcpu)
+{
 	int r;
 
-	destroy_kvm_mmu(vcpu);
-	r = init_kvm_mmu(vcpu);
-	if (r < 0)
-		goto out;
+	spin_lock(&vcpu->kvm->lock);
 	r = mmu_topup_memory_caches(vcpu);
+	if (r)
+		goto out;
+	mmu_alloc_roots(vcpu);
+	kvm_arch_ops->set_cr3(vcpu, vcpu->mmu.root_hpa);
+	kvm_mmu_flush_tlb(vcpu);
 out:
+	spin_unlock(&vcpu->kvm->lock);
 	return r;
 }
+EXPORT_SYMBOL_GPL(kvm_mmu_load);
+
+void kvm_mmu_unload(struct kvm_vcpu *vcpu)
+{
+	mmu_free_roots(vcpu);
+}
 
 static void mmu_pte_write_zap_pte(struct kvm_vcpu *vcpu,
 				  struct kvm_mmu_page *page,
diff --git a/drivers/kvm/paging_tmpl.h b/drivers/kvm/paging_tmpl.h
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index b621403..ed33f59 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1482,6 +1482,10 @@ static int svm_vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	int r;
 
 again:
+	r = kvm_mmu_reload(vcpu);
+	if (unlikely(r))
+		return r;
+
 	if (!vcpu->mmio_read_completed)
 		do_interrupt_requests(vcpu, kvm_run);
 
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 09e3609..a499989 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -1987,6 +1987,10 @@ again:
 	vmx_save_host_state(vcpu);
 	kvm_load_guest_fpu(vcpu);
 
+	r = kvm_mmu_reload(vcpu);
+	if (unlikely(r))
+		goto out;
+
 	/*
 	 * Loading guest fpu may have cleared host cr0.ts
 	 */

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

[-- Attachment #1: Type: text/plain, Size: 4735 bytes --]

Hi,

Il Mon, Jun 04, 2007 at 12:35:37PM +0300, Avi Kivity ha scritto: 
> Luca Tettamanti wrote:
> >Hello,
> >my kernel just exploded :)
> >
> >The host is running 2.6-git-current, with KVM modules from KVM-27
> >package. kernel is 32bit, SMP, with PREEMPT enabled, no HIGHMEM (but I'm
> >using CONFIG_VMSPLIT_3G_OPT=y). The CPU is a Core2 (hence I'm using
> >kvm-intel).
> >Guest was a Fedora7 setup DVD, which died somewhere during the
> >installation (anaconda was already active at that point). Bad news is
> >that I cannot reproduce the bug :|
> >  
> Fortunately the trace clearly shows the problem (out of mmu working 
> memory on guest context switch).  The attached patch should fix it.  Let 
> me know if it works for you.

It turned out that it was somewhat reproducible with fedora installer.
With your patch it doesn't oops anymore.

While doing repeated tests with the installer I ran into another
(unrelated) problem. Sometimes the guest kernel hangs at boot at:

NET: Registered protocol family 2

with any kind of networking options (except for -net none, which works).
With -no-kvm it boots with any networking option.

The only difference in dmesg is that when KVM is enable the guest uses
the TSC:

 NetLabel:  unlabeled traffic allowed by default
-Time: tsc clocksource has been installed.
 PCI: Ignore bogus resource 6 [0:0] of 0000:00:02.0

For reference this is the command line that I'm using:

./kvm-27/qemu/i386-softmmu/qemu -hda /home/kronos/tmp/fedora.img
  -cdrom /home/kronos/tmp/boot.iso -boot d -net tap -net nic -m 256

and boot.iso is the fedora7 net install image (you can find it on any
mirror: fedora/linux/releases/7/Fedora/arch/os/images/boot.iso).

The guest kernel doesn't respond to sysrq, so I don't known exactly
where it's hanging. The stack trace on the host seems rather
uninteresting:

qemu          S 00000002  2404 18905   7312 (NOTLB)
       dca4db48 00000086 00000000 00000002 b0478900 eec4a0f0 b02f418b b0478900 
       0000000a 00000000 eec4a0f0 ef31ca70 267db8c3 000008c7 00003ea3 eec4a1fc 
       b1810980 efcc62a0 b0478900 b0129580 00000000 00000292 dca4db58 0023935c 
Call Trace:
 [<b02f418b>] _spin_unlock_irqrestore+0x34/0x58
 [<b0129580>] __mod_timer+0x9d/0xa7
 [<b02f2258>] schedule_timeout+0x70/0x8d
 [<b02f418b>] _spin_unlock_irqrestore+0x34/0x58
 [<b01291e0>] process_timeout+0x0/0x5
 [<b02f2253>] schedule_timeout+0x6b/0x8d
 [<b0171eb1>] do_select+0x399/0x3e7
 [<b0172496>] __pollwait+0x0/0xac
 [<b011c720>] default_wake_function+0x0/0xc
 [<b0171766>] free_poll_entry+0xe/0x16
 [<b0171786>] poll_freewait+0x18/0x4c
 [<b0171abc>] do_sys_poll+0x302/0x327
 [<b0172496>] __pollwait+0x0/0xac
 [<b011c720>] default_wake_function+0x0/0xc
 [<b011b26a>] task_rq_lock+0x36/0x5d
 [<b02f3c59>] _spin_lock+0x33/0x3e
 [<b02f4197>] _spin_unlock_irqrestore+0x40/0x58
 [<b011c716>] try_to_wake_up+0x325/0x32f
 [<b013b017>] mark_held_locks+0x39/0x53
 [<b02f418b>] _spin_unlock_irqrestore+0x34/0x58
 [<b0103ec0>] restore_nocheck+0x12/0x15
 [<b013b1ee>] trace_hardirqs_on+0x11a/0x13d
 [<b010679a>] do_IRQ+0xc4/0xde
 [<b0103ec0>] restore_nocheck+0x12/0x15
 [<b01721ed>] core_sys_select+0x2ee/0x30f
 [<b0103189>] setup_sigcontext+0x105/0x189
 [<b02f41cf>] _spin_unlock_irq+0x20/0x41
 [<b013b1ee>] trace_hardirqs_on+0x11a/0x13d
 [<b0103a56>] do_notify_resume+0x5d1/0x611
 [<b02f41da>] _spin_unlock_irq+0x2b/0x41
 [<b01039b4>] do_notify_resume+0x52f/0x611
 [<b0103ec0>] restore_nocheck+0x12/0x15
 [<b010898b>] convert_fxsr_from_user+0x26/0xe6
 [<b01725e6>] sys_select+0xa4/0x187
 [<b0103ec0>] restore_nocheck+0x12/0x15
 [<b013b1ee>] trace_hardirqs_on+0x11a/0x13d
 [<b0103e78>] syscall_call+0x7/0xb
 =======================
qemu          S CF9E5DC0  2996 18911   7312 (NOTLB)
       cf9e5dd4 00000082 00000002 cf9e5dc0 cf9e5dbc 00000000 b013b1ee cf9e5ea0 
       00000007 00000001 d252b4f0 b194c030 70a8bf29 000008ac 0000a554 d252b5fc 
       b181a980 efcc62a0 00232330 00000003 00000000 00000000 cf9e5ea0 efcc62d4 
Call Trace:
 [<b013b1ee>] trace_hardirqs_on+0x11a/0x13d
 [<b013de28>] futex_wait+0x251/0x3ed
 [<b0134156>] hrtimer_wakeup+0x0/0x18
 [<b013de19>] futex_wait+0x242/0x3ed
 [<b011c720>] default_wake_function+0x0/0xc
 [<b013e906>] do_futex+0x6c/0xaad
 [<b012acf7>] sys_rt_sigqueueinfo+0x44/0x4e
 [<b0135a4e>] getnstimeofday+0x30/0xbe
 [<b0134627>] ktime_get_ts+0x16/0x44
 [<b013f40f>] sys_futex+0xc8/0xda
 [<b0103e78>] syscall_call+0x7/0xb
 =======================

I'm attaching the dmesg for both -kvm and -no-kvm cases.

Luca
-- 
"La teoria e` quando sappiamo come funzionano le cose ma non funzionano.
 La pratica e` quando le cose funzionano ma non sappiamo perche`.
 Abbiamo unito la teoria e la pratica: le cose non funzionano piu` e non
 sappiamo il perche`." -- A. Einstein

[-- Attachment #2: serial-kvm.txt --]
[-- Type: text/plain, Size: 5329 bytes --]

Linux version 2.6.21-1.3194.fc7 (kojibuilder@xenbuilder3.fedora.phx.redhat.com) (gcc version 4.1.2 20070502 (Red Hat 4.1.2-12)) #1 SMP Wed May 23 22:11:19 EDT 2007

BIOS-provided physical RAM map:

sanitize start

sanitize end

copy_e820_map() start: 0000000000000000 size: 000000000009fc00 end: 000000000009fc00 type: 1

copy_e820_map() type is E820_RAM

copy_e820_map() start: 000000000009fc00 size: 0000000000000400 end: 00000000000a0000 type: 2

copy_e820_map() start: 00000000000e8000 size: 0000000000018000 end: 0000000000100000 type: 2

copy_e820_map() start: 0000000000100000 size: 000000000ff00000 end: 0000000010000000 type: 1

copy_e820_map() type is E820_RAM

copy_e820_map() start: 00000000fffc0000 size: 0000000000040000 end: 0000000100000000 type: 2

 BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)

 BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)

 BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)

 BIOS-e820: 0000000000100000 - 0000000010000000 (usable)

 BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)

0MB HIGHMEM available.

256MB LOWMEM available.

Using x86 segment limits to approximate NX protection

Entering add_active_range(0, 0, 65536) 0 entries of 256 used

Zone PFN ranges:

  DMA             0 ->     4096

  Normal       4096 ->    65536

  HighMem     65536 ->    65536

early_node_map[1] active PFN ranges

    0:        0 ->    65536

On node 0 totalpages: 65536

  DMA zone: 32 pages used for memmap

  DMA zone: 0 pages reserved

  DMA zone: 4064 pages, LIFO batch:0

  Normal zone: 480 pages used for memmap

  Normal zone: 60960 pages, LIFO batch:15

  HighMem zone: 0 pages used for memmap

DMI not present or invalid.

Using APIC driver default

ACPI: no DMI BIOS year, acpi=force is required to enable ACPI

ACPI: Disabling ACPI support

Allocating PCI resources starting at 20000000 (gap: 10000000:effc0000)

Built 1 zonelists.  Total pages: 65024

Kernel command line: initrd=initrd.img console=tty0 console=ttyS0 debug BOOT_IMAGE=vmlinuz 

Found and enabled local APIC!

mapped APIC to ffffd000 (fee00000)

Enabling fast FPU save and restore... done.

Enabling unmasked SIMD FPU exception support... done.

Initializing CPU#0

CPU 0 irqstacks, hard=c076e000 soft=c074e000

PID hash table entries: 1024 (order: 10, 4096 bytes)

Detected 2135.363 MHz processor.

Console: colour VGA+ 80x25

Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)

Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)

Memory: 249556k/262144k available (2037k kernel code, 12036k reserved, 1069k data, 236k init, 0k highmem)

virtual kernel memory layout:

    fixmap  : 0xffc55000 - 0xfffff000   (3752 kB)

    pkmap   : 0xff800000 - 0xffc00000   (4096 kB)

    vmalloc : 0xd0800000 - 0xff7fe000   ( 751 MB)

    lowmem  : 0xc0000000 - 0xd0000000   ( 256 MB)

      .init : 0xc070e000 - 0xc0749000   ( 236 kB)

      .data : 0xc05fd722 - 0xc0708cb4   (1069 kB)

      .text : 0xc0400000 - 0xc05fd722   (2037 kB)

Checking if this processor honours the WP bit even in supervisor mode... Ok.

Calibrating delay using timer specific routine.. 17122.84 BogoMIPS (lpj=8561424)

Security Framework v1.0.0 initialized

SELinux:  Initializing.

SELinux:  Starting in permissive mode

selinux_register_security:  Registering secondary module capability

Capability LSM initialized as secondary

Mount-cache hash table entries: 512

CPU: After generic identify, caps: 0781abfd 00000000 00000000 00000000 00000001 00000000 00000000

CPU: L1 I cache: 8K

CPU: L2 cache: 128K

CPU: After all inits, caps: 0781a3fd 00000000 00000000 00000040 00000001 00000000 00000000

Checking 'hlt' instruction... OK.

SMP alternatives: switching to UP code

Freeing SMP alternatives: 13k freed

CPU0: Intel Pentium II (Klamath) stepping 03

SMP motherboard not detected.

Brought up 1 CPUs

sizeof(vma)=84 bytes

sizeof(page)=32 bytes

sizeof(inode)=336 bytes

sizeof(dentry)=132 bytes

sizeof(ext3inode)=488 bytes

sizeof(buffer_head)=56 bytes

sizeof(skbuff)=176 bytes

sizeof(task_struct)=1376 bytes

Time: 19:24:25  Date: 05/04/107

NET: Registered protocol family 16

PCI: PCI BIOS revision 2.10 entry at 0xf9fa0, last bus=0

PCI: Using configuration type 1

Setting up standard PCI resources

ACPI: Interpreter disabled.

Linux Plug and Play Support v0.97 (c) Adam Belay

pnp: PnP ACPI: disabled

usbcore: registered new interface driver usbfs

usbcore: registered new interface driver hub

usbcore: registered new device driver usb

PCI: Probing PCI hardware

PCI: Probing PCI hardware (bus 00)

* Found PM-Timer Bug on the chipset. Due to workarounds for a bug,

* this clock source is slow. Consider trying other clock sources

PCI quirk: region b100-b10f claimed by PIIX4 SMB

Boot video device is 0000:00:02.0

PCI: Using IRQ router PIIX/ICH [8086/7000] at 0000:00:01.0

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

NetLabel: Initializing

NetLabel:  domain hash size = 128

NetLabel:  protocols = UNLABELED CIPSOv4

NetLabel:  unlabeled traffic allowed by default

PCI: Ignore bogus resource 6 [0:0] of 0000:00:02.0

NET: Registered protocol family 2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: serial-nokvm.txt --]
[-- Type: text/plain; charset=UTF8, Size: 8229 bytes --]

Linux version 2.6.21-1.3194.fc7 (kojibuilder@xenbuilder3.fedora.phx.redhat.com) (gcc version 4.1.2 20070502 (Red Hat 4.1.2-12)) #1 SMP Wed May 23 22:11:19 EDT 2007

BIOS-provided physical RAM map:

sanitize start

sanitize end

copy_e820_map() start: 0000000000000000 size: 000000000009fc00 end: 000000000009fc00 type: 1

copy_e820_map() type is E820_RAM

copy_e820_map() start: 000000000009fc00 size: 0000000000000400 end: 00000000000a0000 type: 2

copy_e820_map() start: 00000000000e8000 size: 0000000000018000 end: 0000000000100000 type: 2

copy_e820_map() start: 0000000000100000 size: 000000000ff00000 end: 0000000010000000 type: 1

copy_e820_map() type is E820_RAM

copy_e820_map() start: 00000000fffc0000 size: 0000000000040000 end: 0000000100000000 type: 2

 BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)

 BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)

 BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)

 BIOS-e820: 0000000000100000 - 0000000010000000 (usable)

 BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)

0MB HIGHMEM available.

256MB LOWMEM available.

Using x86 segment limits to approximate NX protection

Entering add_active_range(0, 0, 65536) 0 entries of 256 used

Zone PFN ranges:

  DMA             0 ->     4096

  Normal       4096 ->    65536

  HighMem     65536 ->    65536

early_node_map[1] active PFN ranges

    0:        0 ->    65536

On node 0 totalpages: 65536

  DMA zone: 32 pages used for memmap

  DMA zone: 0 pages reserved

  DMA zone: 4064 pages, LIFO batch:0

  Normal zone: 480 pages used for memmap

  Normal zone: 60960 pages, LIFO batch:15

  HighMem zone: 0 pages used for memmap

DMI not present or invalid.

Using APIC driver default

ACPI: no DMI BIOS year, acpi=force is required to enable ACPI

ACPI: Disabling ACPI support

Allocating PCI resources starting at 20000000 (gap: 10000000:effc0000)

Built 1 zonelists.  Total pages: 65024

Kernel command line: initrd=initrd.img console=tty0 console=ttyS0 debug BOOT_IMAGE=vmlinuz 

Found and enabled local APIC!

mapped APIC to ffffd000 (fee00000)

Enabling fast FPU save and restore... done.

Enabling unmasked SIMD FPU exception support... done.

Initializing CPU#0

CPU 0 irqstacks, hard=c076e000 soft=c074e000

PID hash table entries: 1024 (order: 10, 4096 bytes)

Detected 2135.092 MHz processor.

Console: colour VGA+ 80x25

Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)

Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)

Memory: 249556k/262144k available (2037k kernel code, 12036k reserved, 1069k data, 236k init, 0k highmem)

virtual kernel memory layout:

    fixmap  : 0xffc55000 - 0xfffff000   (3752 kB)

    pkmap   : 0xff800000 - 0xffc00000   (4096 kB)

    vmalloc : 0xd0800000 - 0xff7fe000   ( 751 MB)

    lowmem  : 0xc0000000 - 0xd0000000   ( 256 MB)

      .init : 0xc070e000 - 0xc0749000   ( 236 kB)

      .data : 0xc05fd722 - 0xc0708cb4   (1069 kB)

      .text : 0xc0400000 - 0xc05fd722   (2037 kB)

Checking if this processor honours the WP bit even in supervisor mode... Ok.

Calibrating delay using timer specific routine.. 17104.71 BogoMIPS (lpj=8552357)

Security Framework v1.0.0 initialized

SELinux:  Initializing.

SELinux:  Starting in permissive mode

selinux_register_security:  Registering secondary module capability

Capability LSM initialized as secondary

Mount-cache hash table entries: 512

CPU: After generic identify, caps: 0781abfd 00000000 00000000 00000000 00000001 00000000 00000000

CPU: L1 I cache: 8K

CPU: L2 cache: 128K

CPU: After all inits, caps: 0781a3fd 00000000 00000000 00000040 00000001 00000000 00000000

Checking 'hlt' instruction... OK.

SMP alternatives: switching to UP code

Freeing SMP alternatives: 13k freed

CPU0: Intel Pentium II (Klamath) stepping 03

SMP motherboard not detected.

Brought up 1 CPUs

sizeof(vma)=84 bytes

sizeof(page)=32 bytes

sizeof(inode)=336 bytes

sizeof(dentry)=132 bytes

sizeof(ext3inode)=488 bytes

sizeof(buffer_head)=56 bytes

sizeof(skbuff)=176 bytes

sizeof(task_struct)=1376 bytes

Time: 19:26:14  Date: 05/04/107

NET: Registered protocol family 16

PCI: PCI BIOS revision 2.10 entry at 0xf9fa0, last bus=0

PCI: Using configuration type 1

Setting up standard PCI resources

ACPI: Interpreter disabled.

Linux Plug and Play Support v0.97 (c) Adam Belay

pnp: PnP ACPI: disabled

usbcore: registered new interface driver usbfs

usbcore: registered new interface driver hub

usbcore: registered new device driver usb

PCI: Probing PCI hardware

PCI: Probing PCI hardware (bus 00)

* Found PM-Timer Bug on the chipset. Due to workarounds for a bug,

* this clock source is slow. Consider trying other clock sources

PCI quirk: region b100-b10f claimed by PIIX4 SMB

Boot video device is 0000:00:02.0

PCI: Using IRQ router PIIX/ICH [8086/7000] at 0000:00:01.0

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

PCI: BIOS reporting unknown device 01:00

NetLabel: Initializing

NetLabel:  domain hash size = 128

NetLabel:  protocols = UNLABELED CIPSOv4

NetLabel:  unlabeled traffic allowed by default

Time: tsc clocksource has been installed.

PCI: Ignore bogus resource 6 [0:0] of 0000:00:02.0

NET: Registered protocol family 2

IP route cache hash table entries: 2048 (order: 1, 8192 bytes)

TCP established hash table entries: 8192 (order: 4, 98304 bytes)

TCP bind hash table entries: 8192 (order: 4, 65536 bytes)

TCP: Hash tables configured (established 8192 bind 8192)

TCP reno registered

checking if image is initramfs... it is

Freeing initrd memory: 5541k freed

apm: BIOS version 1.2 Flags 0x03 (Driver version 1.16ac)

audit: initializing netlink socket (disabled)

audit(1180985175.184:1): initialized

Total HugeTLB memory allocated, 0

VFS: Disk quotas dquot_6.5.1

Dquot-cache hash table entries: 1024 (order 0, 4096 bytes)

SELinux:  Registering netfilter hooks

ksign: Installing public key data

Loading keyring

- Added public key C3680E46D35DB7E1

- User ID: Red Hat, Inc. (Kernel Module GPG key)

io scheduler noop registered

io scheduler anticipatory registered

io scheduler deadline registered

io scheduler cfq registered (default)

Limiting direct PCI/PCI transfers.

PCI: PIIX3: Enabling Passive Release on 0000:00:01.0

Activating ISA DMA hang workarounds.

isapnp: Scanning for PnP cards...

isapnp: No Plug & Play device found

Real Time Clock Driver v1.12ac

Non-volatile memory driver v1.2

Linux agpgart interface v0.102 (c) Dave Jones

Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, IRQ sharing enabled

ÿserial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16450

Clocksource tsc unstable (delta = 1700358775 ns)

RAMDISK driver initialized: 16 RAM disks of 16384K size 4096 blocksize

Time: pit clocksource has been installed.

input: Macintosh mouse button emulation as /class/input/input0

usbcore: registered new interface driver libusual

usbcore: registered new interface driver hiddev

usbcore: registered new interface driver usbhid

drivers/usb/input/hid-core.c: v2.6:USB HID core driver

PNP: No PS/2 controller found. Probing ports directly.

serio: i8042 KBD port at 0x60,0x64 irq 1

serio: i8042 AUX port at 0x60,0x64 irq 12

mice: PS/2 mouse device common for all mice

input: AT Translated Set 2 keyboard as /class/input/input1

TCP bic registered

Initializing XFRM netlink socket

NET: Registered protocol family 1

NET: Registered protocol family 17

Using IPI No-Shortcut mode

  Magic number: 11:688:444

drivers/rtc/hctosys.c: unable to open rtc device (rtc0)

Freeing unused kernel memory: 236k freed

Write protecting the kernel read-only data: 803k

Greetings.

anaconda installer init version 11.2.0.66 starting

mounting /proc filesystem... done

creating /dev filesystem... done

mounting /dev/pts (unix98 pty) filesystem... done

mounting /sys filesystem... done

input: ImExPS/2 Generic Explorer Mouse as /class/input/input2

anaconda installer init version 11.2.0.66 using a serial console

trying to remount root filesystem read write... done

mounting /tmp as ramfs... done

running install...

running /sbin/loader

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca Tettamanti wrote:
> It turned out that it was somewhat reproducible with fedora installer.
> With your patch it doesn't oops anymore.
>
>   

Ok, good.

> While doing repeated tests with the installer I ran into another
> (unrelated) problem. Sometimes the guest kernel hangs at boot at:
>
> NET: Registered protocol family 2
>
> with any kind of networking options (except for -net none, which works).
> With -no-kvm it boots with any networking option.
>
>   

Can you try to pin the guest on a single core with taskset:

   taskset 1 qemu ...

?

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

Il Mon, Jun 04, 2007 at 11:51:10PM +0300, Avi Kivity ha scritto: 
> > While doing repeated tests with the installer I ran into another
> > (unrelated) problem. Sometimes the guest kernel hangs at boot at:
> >
> > NET: Registered protocol family 2
> >
> > with any kind of networking options (except for -net none, which works).
> > With -no-kvm it boots with any networking option.
> >
> >   
> 
> Can you try to pin the guest on a single core with taskset:
> 
>    taskset 1 qemu ...

Doesn't help. What works is 'nolapic', i.e. disabling the local APIC on
the guest kernel.
I've also tried disabling TSC (notsc) and forcing PIT as the clocksource
(clocksouce=pit clock=pit); neither of them helped.

Luca
-- 
Il dottore mi ha detto di smettere di fare cene intime per quattro.
A meno che non ci siamo altre tre persone.

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca Tettamanti wrote:
> Il Mon, Jun 04, 2007 at 11:51:10PM +0300, Avi Kivity ha scritto: 
>   
>>> While doing repeated tests with the installer I ran into another
>>> (unrelated) problem. Sometimes the guest kernel hangs at boot at:
>>>
>>> NET: Registered protocol family 2
>>>
>>> with any kind of networking options (except for -net none, which works).
>>> With -no-kvm it boots with any networking option.
>>>
>>>   
>>>       
>> Can you try to pin the guest on a single core with taskset:
>>
>>    taskset 1 qemu ...
>>     
>
> Doesn't help. What works is 'nolapic', i.e. disabling the local APIC on
> the guest kernel.
> I've also tried disabling TSC (notsc) and forcing PIT as the clocksource
> (clocksouce=pit clock=pit); neither of them helped.
>
>   

Is this a regression relative to a previous kvm version?


-- 
error compiling committee.c: too many arguments to function

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca]

On 6/5/07, Avi Kivity <avi@qumranet.com> wrote:
> Luca Tettamanti wrote:
> > Il Mon, Jun 04, 2007 at 11:51:10PM +0300, Avi Kivity ha scritto:
> >
> >>> While doing repeated tests with the installer I ran into another
> >>> (unrelated) problem. Sometimes the guest kernel hangs at boot at:
> >>>
> >>> NET: Registered protocol family 2
> >>>
> >>> with any kind of networking options (except for -net none, which works).
> >>> With -no-kvm it boots with any networking option.
> >>>
> >>>
> >>>
> >> Can you try to pin the guest on a single core with taskset:
> >>
> >>    taskset 1 qemu ...
> >>
> >
> > Doesn't help. What works is 'nolapic', i.e. disabling the local APIC on
> > the guest kernel.
> > I've also tried disabling TSC (notsc) and forcing PIT as the clocksource
> > (clocksouce=pit clock=pit); neither of them helped.
> >
> >
>
> Is this a regression relative to a previous kvm version?

Hello,
sorry for the delay, I was having troubles compiling older KVMs with a
recent kernel...
The last version that works is kvm-21; starting from kvm-22 the VM
hangs during network initialization (now always, but pretty often).
This only occurs when the guest is Fedora7 setup ISO. The regular boot
(i.e. from the hd) seems unaffected.

Luca

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca wrote:
> On 6/5/07, Avi Kivity <avi@qumranet.com> wrote:
>> Luca Tettamanti wrote:
>> > Il Mon, Jun 04, 2007 at 11:51:10PM +0300, Avi Kivity ha scritto:
>> >
>> >>> While doing repeated tests with the installer I ran into another
>> >>> (unrelated) problem. Sometimes the guest kernel hangs at boot at:
>> >>>
>> >>> NET: Registered protocol family 2
>> >>>
>> >>> with any kind of networking options (except for -net none, which 
>> works).
>> >>> With -no-kvm it boots with any networking option.
>> >>>
>> >>>
>> >>>
>> >> Can you try to pin the guest on a single core with taskset:
>> >>
>> >>    taskset 1 qemu ...
>> >>
>> >
>> > Doesn't help. What works is 'nolapic', i.e. disabling the local 
>> APIC on
>> > the guest kernel.
>> > I've also tried disabling TSC (notsc) and forcing PIT as the 
>> clocksource
>> > (clocksouce=pit clock=pit); neither of them helped.
>> >
>> >
>>
>> Is this a regression relative to a previous kvm version?
>
> Hello,
> sorry for the delay, I was having troubles compiling older KVMs with a
> recent kernel...
> The last version that works is kvm-21; starting from kvm-22 the VM
> hangs during network initialization (now always, but pretty often).
> This only occurs when the guest is Fedora7 setup ISO. The regular boot
> (i.e. from the hd) seems unaffected.

I've managed to reproduce this on kvm-21 (it takes many boots for this 
to happen, but it does eventually).

-- 
error compiling committee.c: too many arguments to function

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca]

On 6/10/07, Avi Kivity <avi@qumranet.com> wrote:
> Luca wrote:
> > On 6/5/07, Avi Kivity <avi@qumranet.com> wrote:
> >> Luca Tettamanti wrote:
> >> > Il Mon, Jun 04, 2007 at 11:51:10PM +0300, Avi Kivity ha scritto:
> >> >
> >> >>> While doing repeated tests with the installer I ran into another
> >> >>> (unrelated) problem. Sometimes the guest kernel hangs at boot at:
> >> >>>
> >> >>> NET: Registered protocol family 2
> >> >>>
> >> >>> with any kind of networking options (except for -net none, which
> >> works).
> >> >>> With -no-kvm it boots with any networking option.
> >> >>>
> >> >>>
> >> >>>
> >> >> Can you try to pin the guest on a single core with taskset:
> >> >>
> >> >>    taskset 1 qemu ...
> >> >>
> >> >
> >> > Doesn't help. What works is 'nolapic', i.e. disabling the local
> >> APIC on
> >> > the guest kernel.
> >> > I've also tried disabling TSC (notsc) and forcing PIT as the
> >> clocksource
> >> > (clocksouce=pit clock=pit); neither of them helped.
> >> >
> >> >
> >>
> >> Is this a regression relative to a previous kvm version?
> >
> > Hello,
> > sorry for the delay, I was having troubles compiling older KVMs with a
> > recent kernel...
> > The last version that works is kvm-21; starting from kvm-22 the VM
> > hangs during network initialization (now always, but pretty often).
> > This only occurs when the guest is Fedora7 setup ISO. The regular boot
> > (i.e. from the hd) seems unaffected.
>
> I've managed to reproduce this on kvm-21 (it takes many boots for this
> to happen, but it does eventually).

Hum, any clue on the cause? Should I test older versions?

Luca

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca wrote:
>>
>> I've managed to reproduce this on kvm-21 (it takes many boots for this
>> to happen, but it does eventually).
>
> Hum, any clue on the cause? 

 From what I've seen, it's the new Linux clocksource code.

> Should I test older versions?

They're unlikely to be better.  Instead, it would be best to see what 
the guest is doing.

I suggest downloading the source rpm for the kernel, building it, and 
sprinkling printk()s until we know exactly what source the guest is 
executing at the time of the hang.

Use of the qemu -kernel, -append, and -serial can redirect the console 
to the command line so that it's easy to capture all debug output.

-- 
error compiling committee.c: too many arguments to function

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca]

On 6/11/07, Avi Kivity <avi@qumranet.com> wrote:
> Luca wrote:
> >>
> >> I've managed to reproduce this on kvm-21 (it takes many boots for this
> >> to happen, but it does eventually).
> >
> > Hum, any clue on the cause?
>
>  From what I've seen, it's the new Linux clocksource code.

Actually I tried forcing the PIT (and any other combination of
tsc,acpi_pm,jiffies) as the clocksource, without success.

> > Should I test older versions?
>
> They're unlikely to be better.  Instead, it would be best to see what
> the guest is doing.
>
> I suggest downloading the source rpm for the kernel, building it, and
> sprinkling printk()s until we know exactly what source the guest is
> executing at the time of the hang.

Ok, will do. Meanwhile I discovered that the kernel on the boot cd
(the one that hangs) is compiled for i586, while the one installed on
disk is for i686 (this one works).

i686 has this options enabled:

+CONFIG_X86_GOOD_APIC=y
+CONFIG_X86_USE_PPRO_CHECKSUM=y
+CONFIG_X86_TSC=y

but disabling tsc on the command line doesn't make any difference. Is
it possible that KVM is choking on some instruction not used by the
i686 kernel?

Luca

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca wrote:
> On 6/11/07, Avi Kivity <avi@qumranet.com> wrote:
>> Luca wrote:
>> >>
>> >> I've managed to reproduce this on kvm-21 (it takes many boots for
>> this
>> >> to happen, but it does eventually).
>> >
>> > Hum, any clue on the cause?
>>
>>  From what I've seen, it's the new Linux clocksource code.
>
> Actually I tried forcing the PIT (and any other combination of
> tsc,acpi_pm,jiffies) as the clocksource, without success.
>

Well, there's lots of APIC stuff going on when it hangs.

>> > Should I test older versions?
>>
>> They're unlikely to be better.  Instead, it would be best to see what
>> the guest is doing.
>>
>> I suggest downloading the source rpm for the kernel, building it, and
>> sprinkling printk()s until we know exactly what source the guest is
>> executing at the time of the hang.
>
> Ok, will do. Meanwhile I discovered that the kernel on the boot cd
> (the one that hangs) is compiled for i586, while the one installed on
> disk is for i686 (this one works).

Ah.

>
> i686 has this options enabled:
>
> +CONFIG_X86_GOOD_APIC=y
> +CONFIG_X86_USE_PPRO_CHECKSUM=y
> +CONFIG_X86_TSC=y
>
> but disabling tsc on the command line doesn't make any difference. Is
> it possible that KVM is choking on some instruction not used by the
> i686 kernel?

Unlikely, as then the hang would occur all the time instead of randomly.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

Il Mon, Jun 11, 2007 at 10:44:45AM +0300, Avi Kivity ha scritto: 
> Luca wrote:
> >>
> >>I've managed to reproduce this on kvm-21 (it takes many boots for this
> >>to happen, but it does eventually).
> >
> >Hum, any clue on the cause? 
> 
> From what I've seen, it's the new Linux clocksource code.
> 
> >Should I test older versions?
> 
> They're unlikely to be better.  Instead, it would be best to see what 
> the guest is doing.

RCU is not working. Network initialization hangs because it happens to
be the first RCU user.
The guest is stuck waiting for RCU syncronization:

[    4.992207]  [<c04321c5>] synchronize_rcu+0x4e/0x80
[    4.994379]  [<c0431db5>] wakeme_after_rcu+0x0/0x8
[    4.996521]  [<c0599ad1>] synchronize_net+0x64/0x8c
[    4.998678]  [<c05d70e0>] inet_register_protosw+0xef/0x151
[    5.000984]  [<c072d79e>] inet_init+0x1cd/0x498

wait_for_completion() in synchronize_rcu() calls schedule() and the
completion is never signaled (wakeme_after_rcu is never called).
The completion AFAICS would be signaled via rcu_process_callbacks(),
which is called in tasklet context.
Scheduler and completion are working fine since they're used in other
part of the kernel without problems.

To recap:

i686 F7 kernel: always works.

i586 F7 kernel: sometime hangs due to RCU problems. When it does work
it's because the LAPIC is disabled on boot:

Using local APIC timer interrupts.
calibrating APIC timer ...
... lapic delta = 25745109
... PM timer delta = 0
..... delta 25745109
..... mult: 1105912110
..... calibration result: 4119217
..... CPU clock speed is 8794.0417 MHz.
..... host bus clock speed is 4119.0217 MHz.
... verify APIC timer
... jiffies delta = 103
APIC timer disabled due to verification failure.

When it doesn't work LAPIC passes the test:

[    1.304717] Using local APIC timer interrupts.
[    1.304719] calibrating APIC timer ...
[    1.718823] ... lapic delta = 25251444
[    1.720582] ... PM timer delta = 0
[    1.722219] ..... delta 25251444
[    1.723827] ..... mult: 1084706136
[    1.725470] ..... calibration result: 4040231
[    1.727374] ..... CPU clock speed is 8625.0780 MHz.
[    1.729396] ..... host bus clock speed is 4040.0231 MHz.
[    1.731540] ... verify APIC timer
[    2.158342] ... jiffies delta = 102
[    2.160035] ... jiffies result ok

i586 F7 kernel, with 'nolapic': always works.

Luca
-- 
Sbagliare è umano, ma per incasinare davvero le cose serve un computer.

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca Tettamanti wrote:
> Il Mon, Jun 11, 2007 at 10:44:45AM +0300, Avi Kivity ha scritto: 
>   
>> Luca wrote:
>>     
>>>> I've managed to reproduce this on kvm-21 (it takes many boots for this
>>>> to happen, but it does eventually).
>>>>         
>>> Hum, any clue on the cause? 
>>>       
>> From what I've seen, it's the new Linux clocksource code.
>>
>>     
>>> Should I test older versions?
>>>       
>> They're unlikely to be better.  Instead, it would be best to see what 
>> the guest is doing.
>>     
>
> RCU is not working. Network initialization hangs because it happens to
> be the first RCU user.
> The guest is stuck waiting for RCU syncronization:
>
> [    4.992207]  [<c04321c5>] synchronize_rcu+0x4e/0x80
> [    4.994379]  [<c0431db5>] wakeme_after_rcu+0x0/0x8
> [    4.996521]  [<c0599ad1>] synchronize_net+0x64/0x8c
> [    4.998678]  [<c05d70e0>] inet_register_protosw+0xef/0x151
> [    5.000984]  [<c072d79e>] inet_init+0x1cd/0x498
>
> wait_for_completion() in synchronize_rcu() calls schedule() and the
> completion is never signaled (wakeme_after_rcu is never called).
> The completion AFAICS would be signaled via rcu_process_callbacks(),
> which is called in tasklet context.
> Scheduler and completion are working fine since they're used in other
> part of the kernel without problems.
>
> To recap:
>
> i686 F7 kernel: always works.
>
> i586 F7 kernel: sometime hangs due to RCU problems. When it does work
> it's because the LAPIC is disabled on boot:
>
> Using local APIC timer interrupts.
> calibrating APIC timer ...
> ... lapic delta = 25745109
> ... PM timer delta = 0
> ..... delta 25745109
> ..... mult: 1105912110
> ..... calibration result: 4119217
> ..... CPU clock speed is 8794.0417 MHz.
> ..... host bus clock speed is 4119.0217 MHz.
> ... verify APIC timer
> ... jiffies delta = 103
> APIC timer disabled due to verification failure.
>
> When it doesn't work LAPIC passes the test:
>
> [    1.304717] Using local APIC timer interrupts.
> [    1.304719] calibrating APIC timer ...
> [    1.718823] ... lapic delta = 25251444
> [    1.720582] ... PM timer delta = 0
> [    1.722219] ..... delta 25251444
> [    1.723827] ..... mult: 1084706136
> [    1.725470] ..... calibration result: 4040231
> [    1.727374] ..... CPU clock speed is 8625.0780 MHz.
> [    1.729396] ..... host bus clock speed is 4040.0231 MHz.
> [    1.731540] ... verify APIC timer
> [    2.158342] ... jiffies delta = 102
> [    2.160035] ... jiffies result ok
>
> i586 F7 kernel, with 'nolapic': always works.
>   

Can you check which .config option causes it (a special type of 
bisecting...)?

This looks likely based on your findings:

-CONFIG_X86_ALIGNMENT_16=y
+CONFIG_X86_GOOD_APIC=y
 CONFIG_X86_INTEL_USERCOPY=y
+CONFIG_X86_USE_PPRO_CHECKSUM=y
+CONFIG_X86_TSC=y

I expect it's not directly related to i586 vs i686.


-- 
error compiling committee.c: too many arguments to function

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

Il Wed, Jun 13, 2007 at 11:59:25AM +0300, Avi Kivity ha scritto: 
> Luca Tettamanti wrote:
> >Il Mon, Jun 11, 2007 at 10:44:45AM +0300, Avi Kivity ha scritto: 
> >  
> >>Luca wrote:
> >>    
> >>>>I've managed to reproduce this on kvm-21 (it takes many boots for this
> >>>>to happen, but it does eventually).
> >>>>        
> >>>Hum, any clue on the cause? 
> >>>      
> >>From what I've seen, it's the new Linux clocksource code.
> >>
> >>    
> >>>Should I test older versions?
> >>>      
> >>They're unlikely to be better.  Instead, it would be best to see what 
> >>the guest is doing.
> >>    
> >
> >RCU is not working. Network initialization hangs because it happens to
> >be the first RCU user.
> >The guest is stuck waiting for RCU syncronization:
> >
> >[    4.992207]  [<c04321c5>] synchronize_rcu+0x4e/0x80
> >[    4.994379]  [<c0431db5>] wakeme_after_rcu+0x0/0x8
> >[    4.996521]  [<c0599ad1>] synchronize_net+0x64/0x8c
> >[    4.998678]  [<c05d70e0>] inet_register_protosw+0xef/0x151
> >[    5.000984]  [<c072d79e>] inet_init+0x1cd/0x498
> >
> >wait_for_completion() in synchronize_rcu() calls schedule() and the
> >completion is never signaled (wakeme_after_rcu is never called).
> >The completion AFAICS would be signaled via rcu_process_callbacks(),
> >which is called in tasklet context.
> >Scheduler and completion are working fine since they're used in other
> >part of the kernel without problems.
> >
> >To recap:
> >
> >i686 F7 kernel: always works.
> >
> >i586 F7 kernel: sometime hangs due to RCU problems. When it does work
> >it's because the LAPIC is disabled on boot:
> >
> >Using local APIC timer interrupts.
> >calibrating APIC timer ...
> >... lapic delta = 25745109
> >... PM timer delta = 0
> >..... delta 25745109
> >..... mult: 1105912110
> >..... calibration result: 4119217
> >..... CPU clock speed is 8794.0417 MHz.
> >..... host bus clock speed is 4119.0217 MHz.
> >... verify APIC timer
> >... jiffies delta = 103
> >APIC timer disabled due to verification failure.
> >
> >When it doesn't work LAPIC passes the test:
> >
> >[    1.304717] Using local APIC timer interrupts.
> >[    1.304719] calibrating APIC timer ...
> >[    1.718823] ... lapic delta = 25251444
> >[    1.720582] ... PM timer delta = 0
> >[    1.722219] ..... delta 25251444
> >[    1.723827] ..... mult: 1084706136
> >[    1.725470] ..... calibration result: 4040231
> >[    1.727374] ..... CPU clock speed is 8625.0780 MHz.
> >[    1.729396] ..... host bus clock speed is 4040.0231 MHz.
> >[    1.731540] ... verify APIC timer
> >[    2.158342] ... jiffies delta = 102
> >[    2.160035] ... jiffies result ok
> >
> >i586 F7 kernel, with 'nolapic': always works.
> >  
> 
> Can you check which .config option causes it (a special type of 
> bisecting...)?
> 
> This looks likely based on your findings:
> 
> -CONFIG_X86_ALIGNMENT_16=y
> +CONFIG_X86_GOOD_APIC=y
> CONFIG_X86_INTEL_USERCOPY=y
> +CONFIG_X86_USE_PPRO_CHECKSUM=y
> +CONFIG_X86_TSC=y
> 
> I expect it's not directly related to i586 vs i686.

And the winner is... CONFIG_X86_GOOD_APIC ;-)

I think that !GOOD_APIC is a workaround for "11AP erratum in Pentium
Processor Specification Update" aka read-before-write bug.

The config symbol is used in include/asm-i386/apic.h:

#ifdef CONFIG_X86_GOOD_APIC
# define FORCE_READ_AROUND_WRITE 0
# define apic_read_around(x)
# define apic_write_around(x,y) apic_write((x),(y))
#else
# define FORCE_READ_AROUND_WRITE 1
# define apic_read_around(x) apic_read(x)
# define apic_write_around(x,y) apic_write_atomic((x),(y))
#endif

With GOOD_APIC apic_read_around is a nop, while apic_write_around is a
normal write. With !GOOD_APIC apic_write_around writes to the APIC reg
using xchg. With !GOOD_APIC and this patch:

--- include/asm-i386/apic.h~	2007-04-26 05:08:32.000000000 +0200
+++ include/asm-i386/apic.h	2007-06-13 22:35:00.000000000 +0200
@@ -56,7 +56,8 @@
 static __inline fastcall void native_apic_write_atomic(unsigned long reg,
 						       unsigned long v)
 {
-	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
+//	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
+	*((volatile unsigned long *)(APIC_BASE+reg)) = v;
 }
 
 static __inline fastcall unsigned long native_apic_read(unsigned long reg)

The kernel boots fine. 

Luca
-- 
"Sei l'unica donna della mia vita".
(Adamo)

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca Tettamanti wrote:
> With GOOD_APIC apic_read_around is a nop, while apic_write_around is a
> normal write. With !GOOD_APIC apic_write_around writes to the APIC reg
> using xchg. With !GOOD_APIC and this patch:
>
> --- include/asm-i386/apic.h~	2007-04-26 05:08:32.000000000 +0200
> +++ include/asm-i386/apic.h	2007-06-13 22:35:00.000000000 +0200
> @@ -56,7 +56,8 @@
>  static __inline fastcall void native_apic_write_atomic(unsigned long reg,
>  						       unsigned long v)
>  {
> -	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> +//	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> +	*((volatile unsigned long *)(APIC_BASE+reg)) = v;
>  }
>  
>  static __inline fastcall unsigned long native_apic_read(unsigned long reg)
>
> The kernel boots fine. 
>   

Looking at the xchg emulation code, it seems fine, but clearly it 
isn't.  Can you add logging to the kernel apic driver and to the qemu 
device emulation (qemu/hw/apic.c, apic_mem_readl()/apic_mem_writel()) 
and compare the results?

-- 
error compiling committee.c: too many arguments to function

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]

Il Thu, Jun 14, 2007 at 11:26:29AM +0300, Avi Kivity ha scritto: 
> Luca Tettamanti wrote:
> >With GOOD_APIC apic_read_around is a nop, while apic_write_around is a
> >normal write. With !GOOD_APIC apic_write_around writes to the APIC reg
> >using xchg. With !GOOD_APIC and this patch:
> >
> >--- include/asm-i386/apic.h~	2007-04-26 05:08:32.000000000 +0200
> >+++ include/asm-i386/apic.h	2007-06-13 22:35:00.000000000 +0200
> >@@ -56,7 +56,8 @@
> > static __inline fastcall void native_apic_write_atomic(unsigned long reg,
> > 						       unsigned long v)
> > {
> >-	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> >+//	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> >+	*((volatile unsigned long *)(APIC_BASE+reg)) = v;
> > }
> > 
> > static __inline fastcall unsigned long native_apic_read(unsigned long reg)
> >
> >The kernel boots fine. 
> >  
> 
> Looking at the xchg emulation code, it seems fine, but clearly it 
> isn't.  Can you add logging to the kernel apic driver and to the qemu 
> device emulation (qemu/hw/apic.c, apic_mem_readl()/apic_mem_writel()) 
> and compare the results?

Done, but at this point I don't know what I'm looking at ;)

I'm attaching logs for working and non working kernels. I've made
apic_read_around() a nop in both cases since it doesn't influence the
outcome; the only difference is that working kernel writes directly to
the mapped APIC registers while the non-working one uses xchg.

As expected kernel side logs show zero differences (except for timer
calibration).

QEMU is a different story. The most obvious difference is that in
non-working case each write is preceded by a read at the same register
(due to xchg).

What is strange is that almost all the writes done using
apic_write_atomic are not hitting QEMU (see qemu.diff). In the log
there's only the read generated by the xchg.

For example, during timer calibration the kernel is doing:

write to 0x0b0
read from 0x390

but - in the non-working case - QEMU sees:

read from 0x0b0 (probably generated by xchg)
read from 0x390

but it doesn't see the write!

The write is not always lost:

apic_read: 0xffffd030 = 0x00050011
apic_write_atomic: 0xffffd370 = 0x000100fe
...
apic_read: fee00030 = 00050011
apic_read: fee00370 = 00010000
apic_write: fee00370 = 000100fe

Luca
-- 
> While we're on all of this, are we going to change "tained" to some
> other less alarmist word?
"screwed" -- Alexander Viro

[-- Attachment #2: apic-logs.tar.bz2 --]
[-- Type: application/octet-stream, Size: 24424 bytes --]

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

Il Thu, Jun 14, 2007 at 11:26:29AM +0300, Avi Kivity ha scritto: 
> Luca Tettamanti wrote:
> >With GOOD_APIC apic_read_around is a nop, while apic_write_around is a
> >normal write. With !GOOD_APIC apic_write_around writes to the APIC reg
> >using xchg. With !GOOD_APIC and this patch:
> >
> >--- include/asm-i386/apic.h~	2007-04-26 05:08:32.000000000 +0200
> >+++ include/asm-i386/apic.h	2007-06-13 22:35:00.000000000 +0200
> >@@ -56,7 +56,8 @@
> > static __inline fastcall void native_apic_write_atomic(unsigned long reg,
> > 						       unsigned long v)
> > {
> >-	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> >+//	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> >+	*((volatile unsigned long *)(APIC_BASE+reg)) = v;
> > }
> > 
> > static __inline fastcall unsigned long native_apic_read(unsigned long reg)
> >
> >The kernel boots fine. 
> >  
> 
> Looking at the xchg emulation code, it seems fine, but clearly it 
> isn't.

Btw, I've put a printk in x86_emulate.c, where it prepares the operands
for the xchg operations: all the write_atomic are hitting this point,
so the write is lost somewhere in cmpxchg_emulated->write_emulated.

Luca
-- 
Il dottore mi ha detto di smettere di fare cene intime per quattro.
A meno che non ci siamo altre tre persone.

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

Il Fri, Jun 15, 2007 at 12:53:24AM +0200, Luca Tettamanti ha scritto: 
> Il Thu, Jun 14, 2007 at 11:26:29AM +0300, Avi Kivity ha scritto: 
> > Luca Tettamanti wrote:
> > >With GOOD_APIC apic_read_around is a nop, while apic_write_around is a
> > >normal write. With !GOOD_APIC apic_write_around writes to the APIC reg
> > >using xchg. With !GOOD_APIC and this patch:
> > >
> > >--- include/asm-i386/apic.h~	2007-04-26 05:08:32.000000000 +0200
> > >+++ include/asm-i386/apic.h	2007-06-13 22:35:00.000000000 +0200
> > >@@ -56,7 +56,8 @@
> > > static __inline fastcall void native_apic_write_atomic(unsigned long reg,
> > > 						       unsigned long v)
> > > {
> > >-	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> > >+//	xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> > >+	*((volatile unsigned long *)(APIC_BASE+reg)) = v;
> > > }
> > > 
> > > static __inline fastcall unsigned long native_apic_read(unsigned long reg)
> > >
> > >The kernel boots fine. 
> > >  
> > 
> > Looking at the xchg emulation code, it seems fine, but clearly it 
> > isn't.
> 
> Btw, I've put a printk in x86_emulate.c, where it prepares the operands
> for the xchg operations: all the write_atomic are hitting this point,
> so the write is lost somewhere in cmpxchg_emulated->write_emulated.

Got it!
The emulator skips the writeback if the old value is unchanged, so the
apic doesn't see the write.

Forcing the writeback:

-      if ((d & Mov) || (dst.orig_val != dst.val)) {
-      if ((d & Mov) || (dst.orig_val != dst.val) || isxchg) {

seems to fix the issue :D I'm not sure that fix is correct though. This
is the patch I'm using:


--- a/kernel/x86_emulate.c	2007-06-03 10:31:15.000000000 +0200
+++ b/kernel/x86_emulate.c	2007-06-15 01:12:12.000000000 +0200
@@ -486,6 +486,7 @@
 	unsigned long _regs[NR_VCPU_REGS];
 	unsigned long _eip = ctxt->vcpu->rip, _eflags = ctxt->eflags;
 	unsigned long modrm_val = 0;
+	int isxchg = 0;
 
 	memcpy(_regs, ctxt->vcpu->regs, sizeof _regs);
 
@@ -912,6 +913,7 @@
 		break;
 	case 0x86 ... 0x87:	/* xchg */
 		/* Write back the register source. */
+		isxchg = 1;
 		switch (dst.bytes) {
 		case 1:
 			*(u8 *) src.ptr = (u8) dst.val;
@@ -1056,7 +1058,7 @@
 	}
 
 writeback:
-	if ((d & Mov) || (dst.orig_val != dst.val)) {
+	if ((d & Mov) || (dst.orig_val != dst.val) || isxchg) {
 		switch (dst.type) {
 		case OP_REG:
 			/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */

Luca
-- 
"Su cio` di cui non si puo` parlare e` bene tacere".
 Ludwig Wittgenstein

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca]

On 6/15/07, Luca Tettamanti <kronos.it@gmail.com> wrote:
> Il Fri, Jun 15, 2007 at 12:53:24AM +0200, Luca Tettamanti ha scritto:
> > Il Thu, Jun 14, 2007 at 11:26:29AM +0300, Avi Kivity ha scritto:
> > > Luca Tettamanti wrote:
> > > >With GOOD_APIC apic_read_around is a nop, while apic_write_around is a
> > > >normal write. With !GOOD_APIC apic_write_around writes to the APIC reg
> > > >using xchg. With !GOOD_APIC and this patch:
> > > >
> > > >--- include/asm-i386/apic.h~       2007-04-26 05:08:32.000000000 +0200
> > > >+++ include/asm-i386/apic.h        2007-06-13 22:35:00.000000000 +0200
> > > >@@ -56,7 +56,8 @@
> > > > static __inline fastcall void native_apic_write_atomic(unsigned long reg,
> > > >                                                  unsigned long v)
> > > > {
> > > >-  xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> > > >+//        xchg((volatile unsigned long *)(APIC_BASE+reg), v);
> > > >+  *((volatile unsigned long *)(APIC_BASE+reg)) = v;
> > > > }
> > > >
> > > > static __inline fastcall unsigned long native_apic_read(unsigned long reg)
> > > >
> > > >The kernel boots fine.
> > > >
> > >
> > > Looking at the xchg emulation code, it seems fine, but clearly it
> > > isn't.
> >
> > Btw, I've put a printk in x86_emulate.c, where it prepares the operands
> > for the xchg operations: all the write_atomic are hitting this point,
> > so the write is lost somewhere in cmpxchg_emulated->write_emulated.
>
> Got it!
> The emulator skips the writeback if the old value is unchanged, so the
> apic doesn't see the write.
>
> Forcing the writeback:
>
> -      if ((d & Mov) || (dst.orig_val != dst.val)) {
> -      if ((d & Mov) || (dst.orig_val != dst.val) || isxchg) {
>
> seems to fix the issue :D I'm not sure that fix is correct though.

After a bit of thinking: it's correct but removes an optimization;
furthermore it may miss other instructions that write to memory mapped
areas.
A more proper fix should be "force the writeback if dst.ptr is in some
kind of mmio area".

Ok, enough of reply-to-self. I'll go to sleep...
Luca

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca wrote:
>>
>> Got it!
>> The emulator skips the writeback if the old value is unchanged, so the
>> apic doesn't see the write.
>>
>> Forcing the writeback:
>>
>> -      if ((d & Mov) || (dst.orig_val != dst.val)) {
>> -      if ((d & Mov) || (dst.orig_val != dst.val) || isxchg) {
>>
>> seems to fix the issue :D I'm not sure that fix is correct though.
>

Good detective work!


> After a bit of thinking: it's correct but removes an optimization;
> furthermore it may miss other instructions that write to memory mapped
> areas.
> A more proper fix should be "force the writeback if dst.ptr is in some
> kind of mmio area".
>

I think we can just disable the optimization, and (in a separate patch)
add it back in emulator_write_phys(), where we know we're modifying
memory and not hitting mmio.

Incidentally, in Xen, where this code originated from, this emulator is
used only for page table updates (which are always to memory).  A
separate emulator is used for mmio.  I guess the optimization targets
the vm scanner doing test_and_clear_bit() type of operations against the
page tables' dirty bits.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

Il Fri, Jun 15, 2007 at 12:06:50PM +0300, Avi Kivity ha scritto: 
> > After a bit of thinking: it's correct but removes an optimization;
> > furthermore it may miss other instructions that write to memory mapped
> > areas.
> > A more proper fix should be "force the writeback if dst.ptr is in some
> > kind of mmio area".
> >
> 
> I think we can just disable the optimization, and (in a separate patch)
> add it back in emulator_write_phys(), where we know we're modifying
> memory and not hitting mmio.

Problem is that in emulator_write_phys() we've already lost track of the
previous (dst.orig_val) value. I moved up the decision in
cmpxchg_emulated; unfortunately this means that the non-locked write
path (write_emulated) can't do this optimization, unless I change its
signature to include the old value.

The first patch makes the writeback step uncoditional whenever we have a
destination operand (the mov check (d & Mov) may be superfluous, yes?).
The write-to-registry path still has the optimization that skips the
write if possible.


--- a/kernel/x86_emulate.c	2007-06-15 21:13:51.000000000 +0200
+++ b/kernel/x86_emulate.c	2007-06-15 22:12:15.000000000 +0200
@@ -1057,9 +1057,8 @@
 	}
 
 writeback:
-	if ((d & Mov) || (dst.orig_val != dst.val)) {
-		switch (dst.type) {
-		case OP_REG:
+	if ((d & Mov) || (d & DstMask) == DstMem || (d & DstMask) == DstReg ) {
+		if (dst.type == OP_REG && dst.orig_val != dst.val) {
 			/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
 			switch (dst.bytes) {
 			case 1:
@@ -1075,8 +1074,10 @@
 				*dst.ptr = dst.val;
 				break;
 			}
-			break;
-		case OP_MEM:
+		} else if (dst.type == OP_MEM) {
+			/* Always dispatch the write, since it may hit a
+			 * MMIO area.
+			 */
 			if (lock_prefix)
 				rc = ops->cmpxchg_emulated((unsigned long)dst.
 							   ptr, &dst.orig_val,
@@ -1088,8 +1089,6 @@
 							 ctxt);
 			if (rc != 0)
 				goto done;
-		default:
-			break;
 		}
 	}
 

Next one: I've splitted emulator_write_phys into emulator_write_phys_mem
(for normal RAM) and emulator_write_phys_mmio (for the rest). The
cmpxchg code path is roughly:

if (!mapped)
        page_fault();

page = gfn_to_page(...)
if (page) {
        if (!memcmp(old, new))
                return X86EMUL_CONTINUE;

        write_mem();
        retrun X86EMUL_CONTINUE;
}
/* for mmio always do the write */
write_mmio();

I'm a bit confused about this test, found in emulator_write_phys
(original code):

if (((gpa + bytes - 1) >> PAGE_SHIFT) != (gpa >> PAGE_SHIFT))
        return 0;

AFAICT is makes the emulator skip the write if the modified area spans
across two different (physical) pages. When this happens write_emulated
does a MMIO write. I'd expect the function to load the 2 pages and do the
memory write on both instead.
I've preserved this logic in the following patch, but I don't understand
why it's correct :|

--- a/kernel/kvm_main.c	2007-06-15 21:18:08.000000000 +0200
+++ b/kernel/kvm_main.c	2007-06-15 23:34:13.000000000 +0200
@@ -1125,26 +1125,39 @@
 	}
 }
 
-static int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
-			       const void *val, int bytes)
+static int emulator_write_phys_mem(struct kvm_vcpu *vcpu, gpa_t gpa,
+				   struct page *page, const void *val,
+				   int bytes)
 {
-	struct page *page;
 	void *virt;
-	unsigned offset = offset_in_page(gpa);
+	unsigned int offset;
+
+	offset = offset_in_page(gpa);
 
 	if (((gpa + bytes - 1) >> PAGE_SHIFT) != (gpa >> PAGE_SHIFT))
 		return 0;
-	page = gfn_to_page(vcpu->kvm, gpa >> PAGE_SHIFT);
-	if (!page)
-		return 0;
+
 	mark_page_dirty(vcpu->kvm, gpa >> PAGE_SHIFT);
 	virt = kmap_atomic(page, KM_USER0);
 	kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
 	memcpy(virt + offset_in_page(gpa), val, bytes);
 	kunmap_atomic(virt, KM_USER0);
+
 	return 1;
 }
 
+static int emulator_write_phys_mmio(struct kvm_vcpu *vcpu, gpa_t gpa,
+				    const void *val, int bytes)
+{
+	vcpu->mmio_needed = 1;
+	vcpu->mmio_phys_addr = gpa;
+	vcpu->mmio_size = bytes;
+	vcpu->mmio_is_write = 1;
+	memcpy(vcpu->mmio_data, val, bytes);
+
+	return 0;
+}
+
 static int emulator_write_emulated(unsigned long addr,
 				   const void *val,
 				   unsigned int bytes,
@@ -1152,20 +1165,18 @@
 {
 	struct kvm_vcpu *vcpu = ctxt->vcpu;
 	gpa_t gpa = vcpu->mmu.gva_to_gpa(vcpu, addr);
+	struct page *page;
 
 	if (gpa == UNMAPPED_GVA) {
 		kvm_arch_ops->inject_page_fault(vcpu, addr, 2);
 		return X86EMUL_PROPAGATE_FAULT;
 	}
 
-	if (emulator_write_phys(vcpu, gpa, val, bytes))
+	page = gfn_to_page(vcpu->kvm, gpa >> PAGE_SHIFT);
+	if (page && emulator_write_phys_mem(vcpu, gpa, page, val, bytes))
 		return X86EMUL_CONTINUE;
 
-	vcpu->mmio_needed = 1;
-	vcpu->mmio_phys_addr = gpa;
-	vcpu->mmio_size = bytes;
-	vcpu->mmio_is_write = 1;
-	memcpy(vcpu->mmio_data, val, bytes);
+	emulator_write_phys_mmio(vcpu, gpa, val, bytes);
 
 	return X86EMUL_CONTINUE;
 }
@@ -1177,12 +1188,37 @@
 				     struct x86_emulate_ctxt *ctxt)
 {
 	static int reported;
+	struct kvm_vcpu *vcpu;
+	gpa_t gpa;
+	struct page *page;
 
 	if (!reported) {
 		reported = 1;
 		printk(KERN_WARNING "kvm: emulating exchange as write\n");
 	}
-	return emulator_write_emulated(addr, new, bytes, ctxt);
+
+	vcpu = ctxt->vcpu;
+	gpa = vcpu->mmu.gva_to_gpa(vcpu, addr);
+
+	if (gpa == UNMAPPED_GVA) {
+		kvm_arch_ops->inject_page_fault(vcpu, addr, 2);
+		return X86EMUL_PROPAGATE_FAULT;
+	}
+
+	page = gfn_to_page(vcpu->kvm, gpa >> PAGE_SHIFT);
+	if (page) {
+		/* Regular memory */
+		if (!memcmp(old, new, bytes))
+			return X86EMUL_CONTINUE;
+
+		if (emulator_write_phys_mem(vcpu, gpa, page, new, bytes))
+			return X86EMUL_CONTINUE;
+	}
+
+	/* MMIO */
+	emulator_write_phys_mmio(vcpu, gpa, new, bytes);
+
+	return X86EMUL_CONTINUE;
 }
 
 static unsigned long get_segment_base(struct kvm_vcpu *vcpu, int seg)


Both patches apply to kvm-28 and have been run-time tested with 32bit
guest on 32bit host, with a VMX processor.

If patches look good I'll resubmit with proper changelog and
signed-off.

Luca
-- 
The trouble with computers is that they do what you tell them, 
not what you want.
D. Cohen

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca Tettamanti wrote:
> Il Fri, Jun 15, 2007 at 12:06:50PM +0300, Avi Kivity ha scritto: 
>   
>>> After a bit of thinking: it's correct but removes an optimization;
>>> furthermore it may miss other instructions that write to memory mapped
>>> areas.
>>> A more proper fix should be "force the writeback if dst.ptr is in some
>>> kind of mmio area".
>>>
>>>       
>> I think we can just disable the optimization, and (in a separate patch)
>> add it back in emulator_write_phys(), where we know we're modifying
>> memory and not hitting mmio.
>>     
>
> Problem is that in emulator_write_phys() we've already lost track of the
> previous (dst.orig_val) value. I moved up the decision in
>   

Actually we haven't; just before the memcpy(), we can put a memcmp() to
guard the kvm_mmu_pte_write(), which is the really expensive operation,
especially with guest smp.

> cmpxchg_emulated; unfortunately this means that the non-locked write
> path (write_emulated) can't do this optimization, unless I change its
> signature to include the old value.
>
> The first patch makes the writeback step uncoditional whenever we have a
> destination operand (the mov check (d & Mov) may be superfluous, yes?).
> The write-to-registry path still has the optimization that skips the
> write if possible.
>   

The mov check is in done since the destination is not read for moves; so
the check for change would read uninitialized memory.

I think we can simply remove the if ().  For the register case, the
check is more expensive that the write; for mmio, we don't want it; and
for memory writes, we can put it in emulator_write_phys().

> Next one: I've splitted emulator_write_phys into emulator_write_phys_mem
> (for normal RAM) and emulator_write_phys_mmio (for the rest). The
>   

This is in order to properly emulate cmpxchg(), right?  I don't think
it's necessary since all that memory is write protected and the
modifications happen under kvm->lock.

>
> I'm a bit confused about this test, found in emulator_write_phys
> (original code):
>
> if (((gpa + bytes - 1) >> PAGE_SHIFT) != (gpa >> PAGE_SHIFT))
>         return 0;
>
> AFAICT is makes the emulator skip the write if the modified area spans
> across two different (physical) pages. When this happens write_emulated
> does a MMIO write. I'd expect the function to load the 2 pages and do the
> memory write on both instead.
>   

This isn't skipping the write; instead it uses the mmio path to update
memory instead of the direct path, as I was too lazy to code split page
writes.  It's also wrong in case someone uses nonaligned writes to
update page tables, but no-one does that.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [kvm-devel] [BUG] Oops with KVM-27

[Luca Tettamanti]

Il Sat, Jun 16, 2007 at 10:43:23AM +0300, Avi Kivity ha scritto: 
> Luca Tettamanti wrote:
> > Il Fri, Jun 15, 2007 at 12:06:50PM +0300, Avi Kivity ha scritto: 
> >   
> >>> After a bit of thinking: it's correct but removes an optimization;
> >>> furthermore it may miss other instructions that write to memory mapped
> >>> areas.
> >>> A more proper fix should be "force the writeback if dst.ptr is in some
> >>> kind of mmio area".
> >>>
> >>>       
> >> I think we can just disable the optimization, and (in a separate patch)
> >> add it back in emulator_write_phys(), where we know we're modifying
> >> memory and not hitting mmio.
> >>     
> >
> > Problem is that in emulator_write_phys() we've already lost track of the
> > previous (dst.orig_val) value. I moved up the decision in
> >   
> 
> Actually we haven't; just before the memcpy(), we can put a memcmp() to
> guard the kvm_mmu_pte_write(), which is the really expensive operation,
> especially with guest smp.

Yup, but it seemed wasteful to map (at least when highmem is in use) a
page just to check for something that we already knew. That was a
preemptive optmization though, I haven't actually benchmarked the cost
of setting up the mapping ;-)

[...]
> I think we can simply remove the if ().  For the register case, the
> check is more expensive that the write; for mmio, we don't want it; and
> for memory writes, we can put it in emulator_write_phys().

Ok, this way it's simpler. How does this look:

--- a/kernel/x86_emulate.c	2007-06-15 21:13:51.000000000 +0200
+++ b/kernel/x86_emulate.c	2007-06-17 16:57:50.000000000 +0200
@@ -1057,40 +1057,38 @@
 	}
 
 writeback:
-	if ((d & Mov) || (dst.orig_val != dst.val)) {
-		switch (dst.type) {
-		case OP_REG:
-			/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
-			switch (dst.bytes) {
-			case 1:
-				*(u8 *)dst.ptr = (u8)dst.val;
-				break;
-			case 2:
-				*(u16 *)dst.ptr = (u16)dst.val;
-				break;
-			case 4:
-				*dst.ptr = (u32)dst.val;
-				break;	/* 64b: zero-ext */
-			case 8:
-				*dst.ptr = dst.val;
-				break;
-			}
+	switch (dst.type) {
+	case OP_REG:
+		/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
+		switch (dst.bytes) {
+		case 1:
+			*(u8 *)dst.ptr = (u8)dst.val;
 			break;
-		case OP_MEM:
-			if (lock_prefix)
-				rc = ops->cmpxchg_emulated((unsigned long)dst.
-							   ptr, &dst.orig_val,
-							   &dst.val, dst.bytes,
-							   ctxt);
-			else
-				rc = ops->write_emulated((unsigned long)dst.ptr,
-							 &dst.val, dst.bytes,
-							 ctxt);
-			if (rc != 0)
-				goto done;
-		default:
+		case 2:
+			*(u16 *)dst.ptr = (u16)dst.val;
+			break;
+		case 4:
+			*dst.ptr = (u32)dst.val;
+			break;	/* 64b: zero-ext */
+		case 8:
+			*dst.ptr = dst.val;
 			break;
 		}
+		break;
+	case OP_MEM:
+		if (lock_prefix)
+			rc = ops->cmpxchg_emulated((unsigned long)dst.
+						   ptr, &dst.orig_val,
+						   &dst.val, dst.bytes,
+						   ctxt);
+		else
+			rc = ops->write_emulated((unsigned long)dst.ptr,
+						 &dst.val, dst.bytes,
+						 ctxt);
+		if (rc != 0)
+			goto done;
+	default:
+		break;
 	}
 
 	/* Commit shadow register state. */

--- a/kernel/kvm_main.c	2007-06-15 21:18:08.000000000 +0200
+++ b/kernel/kvm_main.c	2007-06-17 16:59:33.000000000 +0200
@@ -1139,8 +1139,10 @@
 		return 0;
 	mark_page_dirty(vcpu->kvm, gpa >> PAGE_SHIFT);
 	virt = kmap_atomic(page, KM_USER0);
-	kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
-	memcpy(virt + offset_in_page(gpa), val, bytes);
+	if (memcmp(virt + offset_in_page(gpa), val, bytes)) {
+		kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
+		memcpy(virt + offset_in_page(gpa), val, bytes);
+	}
 	kunmap_atomic(virt, KM_USER0);
 	return 1;
 }



Luca
-- 
Runtime error 6D at f000:a12f : user incompetente

Re: [kvm-devel] [BUG] Oops with KVM-27

[Avi Kivity]

Luca Tettamanti wrote:
>> Actually we haven't; just before the memcpy(), we can put a memcmp() to
>> guard the kvm_mmu_pte_write(), which is the really expensive operation,
>> especially with guest smp.
>>     
>
> Yup, but it seemed wasteful to map (at least when highmem is in use) a
> page just to check for something that we already knew. That was a
> preemptive optmization though, I haven't actually benchmarked the cost
> of setting up the mapping ;-)
>
>   

It's negligible compared to the vmexit cost and to the emulation  (which 
does a kmap_atomic() for every byte of the instruction; this can be 
easily optimized away).

In any case, I expect that performance sensitive uses will use x86_64, 
whereas i386 is mostly for desktops.


>> I think we can simply remove the if ().  For the register case, the
>> check is more expensive that the write; for mmio, we don't want it; and
>> for memory writes, we can put it in emulator_write_phys().
>>     
>
> Ok, this way it's simpler. How does this look:
>
> --- a/kernel/x86_emulate.c	2007-06-15 21:13:51.000000000 +0200
> +++ b/kernel/x86_emulate.c	2007-06-17 16:57:50.000000000 +0200
> @@ -1057,40 +1057,38 @@
>  	}
>  
>  writeback:
> -	if ((d & Mov) || (dst.orig_val != dst.val)) {
> -		switch (dst.type) {
> -		case OP_REG:
> -			/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
> -			switch (dst.bytes) {
> -			case 1:
> -				*(u8 *)dst.ptr = (u8)dst.val;
> -				break;
> -			case 2:
> -				*(u16 *)dst.ptr = (u16)dst.val;
> -				break;
> -			case 4:
> -				*dst.ptr = (u32)dst.val;
> -				break;	/* 64b: zero-ext */
> -			case 8:
> -				*dst.ptr = dst.val;
> -				break;
> -			}
> +	switch (dst.type) {
> +	case OP_REG:
> +		/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
> +		switch (dst.bytes) {
> +		case 1:
> +			*(u8 *)dst.ptr = (u8)dst.val;
>  			break;
> -		case OP_MEM:
> -			if (lock_prefix)
> -				rc = ops->cmpxchg_emulated((unsigned long)dst.
> -							   ptr, &dst.orig_val,
> -							   &dst.val, dst.bytes,
> -							   ctxt);
> -			else
> -				rc = ops->write_emulated((unsigned long)dst.ptr,
> -							 &dst.val, dst.bytes,
> -							 ctxt);
> -			if (rc != 0)
> -				goto done;
> -		default:
> +		case 2:
> +			*(u16 *)dst.ptr = (u16)dst.val;
> +			break;
> +		case 4:
> +			*dst.ptr = (u32)dst.val;
> +			break;	/* 64b: zero-ext */
> +		case 8:
> +			*dst.ptr = dst.val;
>  			break;
>  		}
> +		break;
> +	case OP_MEM:
> +		if (lock_prefix)
> +			rc = ops->cmpxchg_emulated((unsigned long)dst.
> +						   ptr, &dst.orig_val,
> +						   &dst.val, dst.bytes,
> +						   ctxt);
> +		else
> +			rc = ops->write_emulated((unsigned long)dst.ptr,
> +						 &dst.val, dst.bytes,
> +						 ctxt);
> +		if (rc != 0)
> +			goto done;
> +	default:
> +		break;
>  	}
>  
>  	/* Commit shadow register state. */
>
> --- a/kernel/kvm_main.c	2007-06-15 21:18:08.000000000 +0200
> +++ b/kernel/kvm_main.c	2007-06-17 16:59:33.000000000 +0200
> @@ -1139,8 +1139,10 @@
>  		return 0;
>  	mark_page_dirty(vcpu->kvm, gpa >> PAGE_SHIFT);
>  	virt = kmap_atomic(page, KM_USER0);
> -	kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
> -	memcpy(virt + offset_in_page(gpa), val, bytes);
> +	if (memcmp(virt + offset_in_page(gpa), val, bytes)) {
> +		kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
> +		memcpy(virt + offset_in_page(gpa), val, bytes);
> +	}
>  	kunmap_atomic(virt, KM_USER0);
>  	return 1;
>  }
>
>
>   

Excellent.  We win back a precious indentation level and fix a bug at 
the same time.  Please test, send me a changelog and a signoff and I'll 
commit it.


-- 
error compiling committee.c: too many arguments to function

[PATCH 1/2] kvm: Fix x86 emulator writeback

[Luca Tettamanti]

When the old value and new one are the same the emulator skips the
write; this is undesiderable when the destination is a MMIO area and the
write shall be performed regardless of the previous value. This
optimization breaks e.g. a Linux guest APIC compiled without
X86_GOOD_APIC.

Remove the check and always perform the writeback stage in the
emulation.

Signed-Off-By: Luca Tettamanti <kronos.it@gmail.com>

---
 drivers/kvm/x86_emulate.c |   60 +++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
index a4a8481..cea9f3a 100644
--- a/drivers/kvm/x86_emulate.c
+++ b/drivers/kvm/x86_emulate.c
@@ -1057,40 +1057,38 @@ done_prefixes:
 	}
 
 writeback:
-	if ((d & Mov) || (dst.orig_val != dst.val)) {
-		switch (dst.type) {
-		case OP_REG:
-			/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
-			switch (dst.bytes) {
-			case 1:
-				*(u8 *)dst.ptr = (u8)dst.val;
-				break;
-			case 2:
-				*(u16 *)dst.ptr = (u16)dst.val;
-				break;
-			case 4:
-				*dst.ptr = (u32)dst.val;
-				break;	/* 64b: zero-ext */
-			case 8:
-				*dst.ptr = dst.val;
-				break;
-			}
+	switch (dst.type) {
+	case OP_REG:
+		/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
+		switch (dst.bytes) {
+		case 1:
+			*(u8 *)dst.ptr = (u8)dst.val;
 			break;
-		case OP_MEM:
-			if (lock_prefix)
-				rc = ops->cmpxchg_emulated((unsigned long)dst.
-							   ptr, &dst.orig_val,
-							   &dst.val, dst.bytes,
-							   ctxt);
-			else
-				rc = ops->write_emulated((unsigned long)dst.ptr,
-							 &dst.val, dst.bytes,
-							 ctxt);
-			if (rc != 0)
-				goto done;
-		default:
+		case 2:
+			*(u16 *)dst.ptr = (u16)dst.val;
+			break;
+		case 4:
+			*dst.ptr = (u32)dst.val;
+			break;	/* 64b: zero-ext */
+		case 8:
+			*dst.ptr = dst.val;
 			break;
 		}
+		break;
+	case OP_MEM:
+		if (lock_prefix)
+			rc = ops->cmpxchg_emulated((unsigned long)dst.
+						   ptr, &dst.orig_val,
+						   &dst.val, dst.bytes,
+						   ctxt);
+		else
+			rc = ops->write_emulated((unsigned long)dst.ptr,
+						 &dst.val, dst.bytes,
+						 ctxt);
+		if (rc != 0)
+			goto done;
+	default:
+		break;
 	}
 
 	/* Commit shadow register state. */

Luca
-- 
Software is like sex; it's better when it's free.
Linus Torvalds

[PATCH 2/2] kvm: avoid useless memory write when possible

[Luca Tettamanti]

When writing to normal memory and the memory area is unchanged the write
can be safely skipped, avoiding the costly kvm_mmu_pte_write.

Signed-Off-By: Luca Tettamanti <kronos.it@gmail.com>

---

 drivers/kvm/kvm_main.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 633c2ed..9b7b0b9 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -1139,8 +1139,10 @@ static int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 		return 0;
 	mark_page_dirty(vcpu->kvm, gpa >> PAGE_SHIFT);
 	virt = kmap_atomic(page, KM_USER0);
-	kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
-	memcpy(virt + offset_in_page(gpa), val, bytes);
+	if (memcmp(virt + offset_in_page(gpa), val, bytes)) {
+		kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
+		memcpy(virt + offset_in_page(gpa), val, bytes);
+	}
 	kunmap_atomic(virt, KM_USER0);
 	return 1;
 }


Luca
-- 
Se non puoi convincerli, confondili.

Re: [PATCH 1/2] kvm: Fix x86 emulator writeback

[Avi Kivity]

Luca Tettamanti wrote:
> When the old value and new one are the same the emulator skips the
> write; this is undesiderable when the destination is a MMIO area and the
> write shall be performed regardless of the previous value. This
> optimization breaks e.g. a Linux guest APIC compiled without
> X86_GOOD_APIC.
>
> Remove the check and always perform the writeback stage in the
> emulation.
>
>   

Applied this and the next patch.  Thanks!

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/2] kvm: Fix x86 emulator writeback

[Avi Kivity]

Luca Tettamanti wrote:
> When the old value and new one are the same the emulator skips the
> write; this is undesiderable when the destination is a MMIO area and the
> write shall be performed regardless of the previous value. This
> optimization breaks e.g. a Linux guest APIC compiled without
> X86_GOOD_APIC.
>
> Remove the check and always perform the writeback stage in the
> emulation.
>
>   

Unfortunately, this kills Windows XP (first run with a guest crash, 
second with a host oops), so I reverted it.  I'd guess some operation  
which doesn't need writeback ends up in the modified code.  Previously, 
the check caused it to skip writeback, but now it writes back random 
memory, causing a crash.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/2] kvm: Fix x86 emulator writeback

[Avi Kivity]

Avi Kivity wrote:
> Luca Tettamanti wrote:
>> When the old value and new one are the same the emulator skips the
>> write; this is undesiderable when the destination is a MMIO area and the
>> write shall be performed regardless of the previous value. This
>> optimization breaks e.g. a Linux guest APIC compiled without
>> X86_GOOD_APIC.
>>
>> Remove the check and always perform the writeback stage in the
>> emulation.
>>
>>   
>
> Unfortunately, this kills Windows XP (first run with a guest crash, 
> second with a host oops), so I reverted it.  I'd guess some operation  
> which doesn't need writeback ends up in the modified code.  
> Previously, the check caused it to skip writeback, but now it writes 
> back random memory, causing a crash.
>

There are comments around like

>     /* Disable writeback. */
>     dst.orig_val = dst.val;

Best option is probably to add an explicit disable_writeback flag and 
set it there.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/2] kvm: Fix x86 emulator writeback

[Luca Tettamanti]

Il Mon, Jun 18, 2007 at 02:32:39PM +0300, Avi Kivity ha scritto: 
> >Unfortunately, this kills Windows XP (first run with a guest crash, 
> >second with a host oops), so I reverted it.  I'd guess some operation  
> >which doesn't need writeback ends up in the modified code.  
> >Previously, the check caused it to skip writeback, but now it writes 
> >back random memory, causing a crash.
> >
> 
> There are comments around like
> 
> >    /* Disable writeback. */
> >    dst.orig_val = dst.val;
> 
> Best option is probably to add an explicit disable_writeback flag and 
> set it there.

I've tested this patch with linux, solaris and winxp, on a 32 bit guest.
So far everything is fine ;)
David, this patch should fix the lockup you're seeing.

diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 633c2ed..9b7b0b9 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -1139,8 +1139,10 @@ static int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 		return 0;
 	mark_page_dirty(vcpu->kvm, gpa >> PAGE_SHIFT);
 	virt = kmap_atomic(page, KM_USER0);
-	kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
-	memcpy(virt + offset_in_page(gpa), val, bytes);
+	if (memcmp(virt + offset_in_page(gpa), val, bytes)) {
+		kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
+		memcpy(virt + offset_in_page(gpa), val, bytes);
+	}
 	kunmap_atomic(virt, KM_USER0);
 	return 1;
 }
diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
index a4a8481..eb10448 100644
--- a/drivers/kvm/x86_emulate.c
+++ b/drivers/kvm/x86_emulate.c
@@ -482,6 +482,7 @@ x86_emulate_memop(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
 	int mode = ctxt->mode;
 	unsigned long modrm_ea;
 	int use_modrm_ea, index_reg = 0, base_reg = 0, scale, rip_relative = 0;
+	int no_wb = 0;
 
 	/* Shadow copy of register state. Committed on successful emulation. */
 	unsigned long _regs[NR_VCPU_REGS];
@@ -1048,7 +1049,7 @@ done_prefixes:
 						      _regs[VCPU_REGS_RSP]),
 				     &dst.val, dst.bytes, ctxt)) != 0)
 				goto done;
-			dst.val = dst.orig_val;	/* skanky: disable writeback */
+			no_wb = 1;	/* skanky: disable writeback */
 			break;
 		default:
 			goto cannot_emulate;
@@ -1057,7 +1058,7 @@ done_prefixes:
 	}
 
 writeback:
-	if ((d & Mov) || (dst.orig_val != dst.val)) {
+	if (!no_wb) {
 		switch (dst.type) {
 		case OP_REG:
 			/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
@@ -1306,7 +1307,7 @@ twobyte_insn:
 
 twobyte_special_insn:
 	/* Disable writeback. */
-	dst.orig_val = dst.val;
+	no_wb = 1;
 	switch (b) {
 	case 0x09:		/* wbinvd */
 		break;


Luca
-- 
"Ci sono le balle, le megaballe e le statistiche".
 Mark Twain

[PATCH 1/2] kvm: Fix x86 emulator writeback

[Luca Tettamanti]

When the old value and new one are the same the emulator skips the
write; this is undesiderable when the destination is a MMIO area and the
write shall be performed regardless of the previous value. This
optimization breaks e.g. a Linux guest APIC compiled without
X86_GOOD_APIC.

Remove the check and perform the writeback stage in the emulation unless
it's explicitly disabled (currently push and some 2 bytes instructions
may disable the writeback).

Signed-Off-By: Luca Tettamanti <kronos.it@gmail.com>

---

 Tested with Fedora7, Solaris10 and WinXP on a 32 bit host with Intel
 CPU.

 drivers/kvm/x86_emulate.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
index a4a8481..eb10448 100644
--- a/drivers/kvm/x86_emulate.c
+++ b/drivers/kvm/x86_emulate.c
@@ -482,6 +482,7 @@ x86_emulate_memop(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
 	int mode = ctxt->mode;
 	unsigned long modrm_ea;
 	int use_modrm_ea, index_reg = 0, base_reg = 0, scale, rip_relative = 0;
+	int no_wb = 0;
 
 	/* Shadow copy of register state. Committed on successful emulation. */
 	unsigned long _regs[NR_VCPU_REGS];
@@ -1048,7 +1049,7 @@ done_prefixes:
 						      _regs[VCPU_REGS_RSP]),
 				     &dst.val, dst.bytes, ctxt)) != 0)
 				goto done;
-			dst.val = dst.orig_val;	/* skanky: disable writeback */
+			no_wb = 1;	/* skanky: disable writeback */
 			break;
 		default:
 			goto cannot_emulate;
@@ -1057,7 +1058,7 @@ done_prefixes:
 	}
 
 writeback:
-	if ((d & Mov) || (dst.orig_val != dst.val)) {
+	if (!no_wb) {
 		switch (dst.type) {
 		case OP_REG:
 			/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
@@ -1306,7 +1307,7 @@ twobyte_insn:
 
 twobyte_special_insn:
 	/* Disable writeback. */
-	dst.orig_val = dst.val;
+	no_wb = 1;
 	switch (b) {
 	case 0x09:		/* wbinvd */
 		break;


Luca
-- 
Colui che sorride quando le cose vanno male ha pensato a qualcuno a cui
dare la colpa.

[PATCH 2/2] kvm: avoid useless memory write when possible

[Luca Tettamanti]

When writing to normal memory and the memory area is unchanged the write
can be safely skipped, avoiding the costly kvm_mmu_pte_write.

Signed-Off-By: Luca Tettamanti <kronos.it@gmail.com>

---
 Tested with Fedora7, Solaris10 and WinXP on a 32 bit host with Intel
 CPU.

 drivers/kvm/kvm_main.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 633c2ed..9b7b0b9 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -1139,8 +1139,10 @@ static int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 		return 0;
 	mark_page_dirty(vcpu->kvm, gpa >> PAGE_SHIFT);
 	virt = kmap_atomic(page, KM_USER0);
-	kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
-	memcpy(virt + offset_in_page(gpa), val, bytes);
+	if (memcmp(virt + offset_in_page(gpa), val, bytes)) {
+		kvm_mmu_pte_write(vcpu, gpa, virt + offset, val, bytes);
+		memcpy(virt + offset_in_page(gpa), val, bytes);
+	}
 	kunmap_atomic(virt, KM_USER0);
 	return 1;
 }

Luca
-- 
Regole per la felicit?:
1. Sii soddisfatto di quello che hai.
2. Assicurati di avere tutto.

Re: [PATCH 1/2] kvm: Fix x86 emulator writeback

[Avi Kivity]

Luca Tettamanti wrote:
> When the old value and new one are the same the emulator skips the
> write; this is undesiderable when the destination is a MMIO area and the
> write shall be performed regardless of the previous value. This
> optimization breaks e.g. a Linux guest APIC compiled without
> X86_GOOD_APIC.
>
> Remove the check and perform the writeback stage in the emulation unless
> it's explicitly disabled (currently push and some 2 bytes instructions
> may disable the writeback).
>
>   

Applied both, thanks.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.