Re: Nested paging in nested SVM setup

[Paolo Bonzini <pbonzini@redhat.com>]

Il 20/08/2014 09:37, Valentine Sinitsyn ha scritto:
> Hi Paolo,
> 
> On 20.08.2014 12:55, Paolo Bonzini wrote:
>> Is the 0x23c always the same?
> No, it's just a garbage - I've seen other values as well (0x80 last time).
> 
>>  Can you try this patch?
> Sure. It does print a warning:
> 
> [ 2176.722098] ------------[ cut here ]------------
> [ 2176.722118] WARNING: CPU: 0 PID: 1488 at
> /home/val/kvm-kmod/x86/x86.c:368 kvm_multiple_exception+0x121/0x130 [kvm]()
> [ 2176.722121] Modules linked in: kvm_amd(O) kvm(O) amd_freq_sensitivity
> snd_hda_codec_realtek snd_hda_codec_hdmi snd_hda_codec_generic
> crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel
> snd_hda_intel aesni_intel snd_hda_controller radeon snd_hda_codec
> ipmi_si aes_x86_64 ipmi_msghandler snd_hwdep ttm r8169 ppdev mii lrw
> gf128mul snd_pcm glue_helper drm_kms_helper snd_timer fam15h_power evdev
> drm shpchp snd ablk_helper cryptd microcode mac_hid soundcore serio_raw
> pcspkr i2c_algo_bit k10temp i2c_piix4 i2c_core parport_pc parport hwmon
> edac_core tpm_tis edac_mce_amd tpm video button acpi_cpufreq processor
> ext4 crc16 mbcache jbd2 sd_mod crc_t10dif crct10dif_common atkbd libps2
> ahci libahci ohci_pci ohci_hcd ehci_pci xhci_hcd libata ehci_hcd usbcore
> scsi_mod usb_common i8042 serio [last unloaded: kvm]
> 
> [ 2176.722217] CPU: 0 PID: 1488 Comm: qemu-system-x86 Tainted: G W  O 
> 3.16.1-1-ARCH #1
> [ 2176.722221] Hardware name: To Be Filled By O.E.M. To Be Filled By
> O.E.M./IMB-A180, BIOS L0.17 05/24/2013
> [ 2176.722224]  0000000000000000 0000000025350f51 ffff8800919fbbc0
> ffffffff8152ae6c
> [ 2176.722229]  0000000000000000 ffff8800919fbbf8 ffffffff8106e45d
> ffff880037f68000
> [ 2176.722234]  0000000000000080 0000000000000001 00000000000081a4
> 0000000000000000
> [ 2176.722239] Call Trace:
> [ 2176.722250]  [<ffffffff8152ae6c>] dump_stack+0x4d/0x6f
> [ 2176.722257]  [<ffffffff8106e45d>] warn_slowpath_common+0x7d/0xa0
> [ 2176.722262]  [<ffffffff8106e58a>] warn_slowpath_null+0x1a/0x20
> [ 2176.722275]  [<ffffffffa0651e41>] kvm_multiple_exception+0x121/0x130
> [kvm]
> [ 2176.722288]  [<ffffffffa06594f8>] x86_emulate_instruction+0x548/0x640
> [kvm]
> [ 2176.722303]  [<ffffffffa06653e1>] kvm_mmu_page_fault+0x91/0xf0 [kvm]
> [ 2176.722310]  [<ffffffffa04eb6a7>] pf_interception+0xd7/0x180 [kvm_amd]
> [ 2176.722317]  [<ffffffff8104e876>] ? native_apic_mem_write+0x6/0x10
> [ 2176.722323]  [<ffffffffa04ef261>] handle_exit+0x141/0x9d0 [kvm_amd]
> [ 2176.722335]  [<ffffffffa065512c>] ? kvm_set_cr8+0x1c/0x20 [kvm]
> [ 2176.722341]  [<ffffffffa04ea3e0>] ? nested_svm_get_tdp_cr3+0x20/0x20
> [kvm_amd]
> [ 2176.722355]  [<ffffffffa065adc7>]
> kvm_arch_vcpu_ioctl_run+0x597/0x1210 [kvm]
> [ 2176.722368]  [<ffffffffa065705b>] ? kvm_arch_vcpu_load+0xbb/0x200 [kvm]
> [ 2176.722378]  [<ffffffffa064a152>] kvm_vcpu_ioctl+0x2b2/0x5c0 [kvm]
> [ 2176.722384]  [<ffffffff810b66b4>] ? __wake_up+0x44/0x50
> [ 2176.722390]  [<ffffffff81200dcc>] ? fsnotify+0x28c/0x370
> [ 2176.722397]  [<ffffffff811d4a70>] do_vfs_ioctl+0x2d0/0x4b0
> [ 2176.722403]  [<ffffffff811df18e>] ? __fget+0x6e/0xb0
> [ 2176.722408]  [<ffffffff811d4cd1>] SyS_ioctl+0x81/0xa0
> [ 2176.722414]  [<ffffffff81530be9>] system_call_fastpath+0x16/0x1b
> [ 2176.722418] ---[ end trace b0f81744c5a5ea4a ]---
> 
> Thanks,
> Valentine
> -- 
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

I audited the various places that return X86EMUl_PROPAGATE_FAULT and
I think the culprit is this code in paging_tmpl.h.

 	real_gpa = mmu->translate_gpa(vcpu, gfn_to_gpa(gfn), access);
	if (real_gpa == UNMAPPED_GVA)
 		return 0;

It returns zero without setting fault.vector.

Another patch...  I will post parts of it separately, if I am right
you should get 0xfe as the vector and a WARN from the gva_to_gpa function.

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index ef297919a691..e5bf13003cd2 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -527,6 +527,7 @@ static unsigned long seg_base(struct x86_emulate_ctxt *ctxt, int seg)
 static int emulate_exception(struct x86_emulate_ctxt *ctxt, int vec,
 			     u32 error, bool valid)
 {
+	WARN_ON(vec > 0x1f);
 	ctxt->exception.vector = vec;
 	ctxt->exception.error_code = error;
 	ctxt->exception.error_code_valid = valid;
@@ -3016,7 +3015,7 @@ static int em_movbe(struct x86_emulate_ctxt *ctxt)
 		ctxt->dst.val = swab64(ctxt->src.val);
 		break;
 	default:
-		return X86EMUL_PROPAGATE_FAULT;
+		BUG();
 	}
 	return X86EMUL_CONTINUE;
 }
@@ -4829,8 +4828,10 @@ writeback:
 	ctxt->eip = ctxt->_eip;
 
 done:
-	if (rc == X86EMUL_PROPAGATE_FAULT)
+	if (rc == X86EMUL_PROPAGATE_FAULT) {
+		WARN_ON(ctxt->exception.vector > 0x1f);
 		ctxt->have_exception = true;
+	}
 	if (rc == X86EMUL_INTERCEPTED)
 		return EMULATION_INTERCEPTED;
 
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 410776528265..cd91d03c9320 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -365,8 +365,10 @@ retry_walk:
 		gfn += pse36_gfn_delta(pte);
 
 	real_gpa = mmu->translate_gpa(vcpu, gfn_to_gpa(gfn), access);
-	if (real_gpa == UNMAPPED_GVA)
+	if (real_gpa == UNMAPPED_GVA) {
+		walker->fault.vector = 0xfe;
 		return 0;
+	}
 
 	walker->gfn = real_gpa >> PAGE_SHIFT;
 
@@ -875,8 +877,10 @@ static gpa_t FNAME(gva_to_gpa)(struct kvm_vcpu *vcpu, gva_t vaddr, u32 access,
 	if (r) {
 		gpa = gfn_to_gpa(walker.gfn);
 		gpa |= vaddr & ~PAGE_MASK;
-	} else if (exception)
+	} else if (exception) {
+		WARN_ON(walker.fault.vector > 0x1f);
 		*exception = walker.fault;
+	}
 
 	return gpa;
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 737b4bdac41c..71f05585894e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5248,6 +5249,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
 
 		ctxt->interruptibility = 0;
 		ctxt->have_exception = false;
+		ctxt->exception.vector = 0xff;
 		ctxt->perm_ok = false;
 
 		ctxt->ud = emulation_type & EMULTYPE_TRAP_UD;