Re: [PATCH 4/4] Fix task switching.
Gleb Natapov wrote: On Tue, Mar 31, 2009 at 05:21:16PM +0200, Kohl, Bernhard (NSN - DE/Munich) wrote: Bernhard Kohl wrote: Jan Kiszka wrote: Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Does this series fix all issues Bernhard, Thomas and Julian stumbled over? Jan I will try this today. Thanks. Yes, it works for us (Thomas + Bernhard). Great. Thanks for testing. Same here: No obvious regressions found while running various NMI/IRQ tests. Jan signature.asc Description: OpenPGP digital signature
RE: [PATCH 4/4] Fix task switching.
Jan Kiszka wrote: Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Does this series fix all issues Bernhard, Thomas and Julian stumbled over? Jan I will try this today. Thanks. Bernhard -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/4] Fix task switching.
Bernhard Kohl wrote: Jan Kiszka wrote: Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Does this series fix all issues Bernhard, Thomas and Julian stumbled over? Jan I will try this today. Thanks. Yes, it works for us (Thomas + Bernhard). Bernhard -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Fix task switching.
On Tue, Mar 31, 2009 at 05:21:16PM +0200, Kohl, Bernhard (NSN - DE/Munich) wrote: Bernhard Kohl wrote: Jan Kiszka wrote: Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Does this series fix all issues Bernhard, Thomas and Julian stumbled over? Jan I will try this today. Thanks. Yes, it works for us (Thomas + Bernhard). Great. Thanks for testing. -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Fix task switching.
Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Looks good, but please split into (at least) two patches. Also please provide a test case so we don't regress again. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Fix task switching.
On Mon, Mar 30, 2009 at 10:39:21AM +0300, Avi Kivity wrote: Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Looks good, but please split into (at least) two patches. Also please provide a test case so we don't regress again. This what I am using for testing. After running make you should get kernel.bin that can be booted from grub. Runs on real HW too. I am planing to add more test. Signed-off-by: Gleb Natapov g...@redhat.com diff --git a/user/test/x86/kvmtest/Makefile b/user/test/x86/kvmtest/Makefile new file mode 100644 index 000..b93935f --- /dev/null +++ b/user/test/x86/kvmtest/Makefile @@ -0,0 +1,33 @@ +CC=gcc +AS=gcc +CFLAGS=-m32 -I. -O2 -Wall +ASFLAGS=-m32 -I. +OBJS=kernel.o lib.o boot.o memory.o gdt.o idt.o isrs.o tss.o uart.o +ALLOBJS=$(OBJS) tests/tests.o + +PHONY := all +all: kernel.bin + $(MAKE) -C tests + +kernel.bin: $(ALLOBJS) kernel.ld + ld -T kernel.ld $(ALLOBJS) -o $@ + +install: kernel.bin + cp $ /boot/ + +tests/tests.o: + $(MAKE) -C tests + +-include $(OBJS:.o=.d) + +# compile and generate dependency info +%.o: %.c + gcc -c $(CFLAGS) $*.c -o $*.o + gcc -MM $(CFLAGS) $*.c $*.d + +PHONY += clean +clean: + $(MAKE) -C tests + -rm *.o *~ *.d kernel.bin + +.PHONY: $(PHONY) diff --git a/user/test/x86/kvmtest/boot.S b/user/test/x86/kvmtest/boot.S new file mode 100644 index 000..f74015c --- /dev/null +++ b/user/test/x86/kvmtest/boot.S @@ -0,0 +1,357 @@ +/* boot.S - bootstrap the kernel */ +/* Copyright (C) 1999, 2001 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ + +#define ASM 1 +#include multiboot.h +#include kernel.h + +.text + +.globl start, _start +start: +_start: +jmp multiboot_entry + +/* Align 32 bits boundary. */ +.align 4 + +/* Multiboot header. */ +multiboot_header: +/* magic */ +.long MULTIBOOT_HEADER_MAGIC +/* flags */ +.long MULTIBOOT_HEADER_FLAGS +/* checksum */ +.long -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS) +#ifndef __ELF__ + /* header_addr */ + .long multiboot_header + /* load_addr */ + .long _start + /* load_end_addr */ + .long _edata + /* bss_end_addr */ + .long _end + /* entry_addr */ + .long multiboot_entry +#endif /* ! __ELF__ */ + + multiboot_entry: + /* Initialize the stack pointer. */ + movl$(STACK_START), %esp + + /* Reset EFLAGS. */ + pushl $0 + popf + + /* Push the pointer to the Multiboot information structure. */ + pushl %ebx + /* Push the magic value. */ + pushl %eax + + /* Now enter the C main function... */ + callcmain + + /* Halt. */ + pushl $halt_message + pushl $0 + callprintk + + loop: hlt + jmp loop + +.globl isr0 +.globl isr1 +.globl isr2 +.globl isr3 +.globl isr4 +.globl isr5 +.globl isr6 +.globl isr7 +.globl isr8 +.globl isr9 +.globl isr10 +.globl isr11 +.globl isr12 +.globl isr13 +.globl isr14 +.globl isr15 +.globl isr16 +.globl isr17 +.globl isr18 +.globl isr19 +.globl isr20 +.globl isr21 +.globl isr22 +.globl isr23 +.globl isr24 +.globl isr25 +.globl isr26 +.globl isr27 +.globl isr28 +.globl isr29 +.globl isr30 +.globl isr31 + +/* 0: Divide By Zero Exception */ +isr0: + cli + pushl $0 + pushl $0 + jmp isr_common_stub + +/* 1: Debug Exception */ +isr1: + cli + pushl $0 + pushl $1 + jmp isr_common_stub + +/* 2: Non Maskable Interrupt Exception */ +isr2: + cli + pushl $0 + pushl $2 + jmp isr_common_stub + +/* 3: Int 3 Exception */ +isr3: + cli + pushl $0 + pushl $3 + jmp isr_common_stub + +/* 4: INTO Exception */ +isr4: + cli + pushl $0 + pushl $4 + jmp isr_common_stub + +/* 5: Out of Bounds Exception */ +isr5: + cli + pushl $0 + pushl $5 + jmp isr_common_stub + +/* 6: Invalid Opcode Exception
Re: [PATCH 4/4] Fix task switching.
Gleb Natapov wrote:
The patch fixes two problems with task switching.
1. Back link is written to a wrong TSS.
2. Instruction emulation is not needed if the reason for task switch
is a task gate in IDT and access to it is caused by an external even.
2 is currently solved only for VMX since there is not reliable way to
skip an instruction in SVM. We should emulate it instead.
Does this series fix all issues Bernhard, Thomas and Julian stumbled over?
Jan
Signed-off-by: Gleb Natapov g...@redhat.com
---
arch/x86/include/asm/svm.h |1 +
arch/x86/kvm/svm.c | 25 ++---
arch/x86/kvm/vmx.c | 40 +---
arch/x86/kvm/x86.c | 40 +++-
4 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 82ada75..85574b7 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -225,6 +225,7 @@ struct __attribute__ ((__packed__)) vmcb {
#define SVM_EVTINJ_VALID_ERR (1 11)
#define SVM_EXITINTINFO_VEC_MASK SVM_EVTINJ_VEC_MASK
+#define SVM_EXITINTINFO_TYPE_MASK SVM_EVTINJ_TYPE_MASK
#define SVM_EXITINTINFO_TYPE_INTR SVM_EVTINJ_TYPE_INTR
#define SVM_EXITINTINFO_TYPE_NMI SVM_EVTINJ_TYPE_NMI
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1fcbc17..3ffb695 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1823,17 +1823,28 @@ static int task_switch_interception(struct vcpu_svm
*svm,
struct kvm_run *kvm_run)
{
u16 tss_selector;
+ int reason;
+ int int_type = svm-vmcb-control.exit_int_info
+ SVM_EXITINTINFO_TYPE_MASK;
tss_selector = (u16)svm-vmcb-control.exit_info_1;
+
if (svm-vmcb-control.exit_info_2
(1ULL SVM_EXITINFOSHIFT_TS_REASON_IRET))
- return kvm_task_switch(svm-vcpu, tss_selector,
-TASK_SWITCH_IRET);
- if (svm-vmcb-control.exit_info_2
- (1ULL SVM_EXITINFOSHIFT_TS_REASON_JMP))
- return kvm_task_switch(svm-vcpu, tss_selector,
-TASK_SWITCH_JMP);
- return kvm_task_switch(svm-vcpu, tss_selector, TASK_SWITCH_CALL);
+ reason = TASK_SWITCH_IRET;
+ else if (svm-vmcb-control.exit_info_2
+ (1ULL SVM_EXITINFOSHIFT_TS_REASON_JMP))
+ reason = TASK_SWITCH_JMP;
+ else if (svm-vmcb-control.exit_int_info SVM_EXITINTINFO_VALID)
+ reason = TASK_SWITCH_GATE;
+ else
+ reason = TASK_SWITCH_CALL;
+
+
+ if (reason != TASK_SWITCH_GATE || int_type == SVM_EXITINTINFO_TYPE_SOFT)
+ skip_emulated_instruction(svm-vcpu);
+
+ return kvm_task_switch(svm-vcpu, tss_selector, reason);
}
static int cpuid_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 0da7a9e..01db958 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3025,22 +3025,40 @@ static int handle_task_switch(struct kvm_vcpu *vcpu,
struct kvm_run *kvm_run)
struct vcpu_vmx *vmx = to_vmx(vcpu);
unsigned long exit_qualification;
u16 tss_selector;
- int reason;
+ int reason, type, idt_v;
+
+ idt_v = (vmx-idt_vectoring_info VECTORING_INFO_VALID_MASK);
+ type = (vmx-idt_vectoring_info VECTORING_INFO_TYPE_MASK);
exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
reason = (u32)exit_qualification 30;
- if (reason == TASK_SWITCH_GATE vmx-vcpu.arch.nmi_injected
- (vmx-idt_vectoring_info VECTORING_INFO_VALID_MASK)
- (vmx-idt_vectoring_info VECTORING_INFO_TYPE_MASK)
- == INTR_TYPE_NMI_INTR) {
- vcpu-arch.nmi_injected = false;
- if (cpu_has_virtual_nmis())
- vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
- GUEST_INTR_STATE_NMI);
+ if (reason == TASK_SWITCH_GATE idt_v) {
+ switch (type) {
+ case INTR_TYPE_NMI_INTR:
+ vcpu-arch.nmi_injected = false;
+ if (cpu_has_virtual_nmis())
+ vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
+ GUEST_INTR_STATE_NMI);
+ break;
+ case INTR_TYPE_EXT_INTR:
+ kvm_clear_interrupt_queue(vcpu);
+ break;
+ case INTR_TYPE_HARD_EXCEPTION:
+ case INTR_TYPE_SOFT_EXCEPTION:
+ kvm_clear_exception_queue(vcpu);
+ break;
+ default:
+ break;
+ }
}
tss_selector = exit_qualification;
+ if (!idt_v || (type != INTR_TYPE_HARD_EXCEPTION
+type !=
Re: [PATCH 4/4] Fix task switching.
On Mon, Mar 30, 2009 at 06:04:45PM +0200, Jan Kiszka wrote: Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Does this series fix all issues Bernhard, Thomas and Julian stumbled over? Haven't tried. I wrote my own tests for task switching. How can I check it? -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Fix task switching.
Gleb Natapov wrote: On Mon, Mar 30, 2009 at 06:04:45PM +0200, Jan Kiszka wrote: Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Does this series fix all issues Bernhard, Thomas and Julian stumbled over? Haven't tried. I wrote my own tests for task switching. How can I check it? There is a test case attached to Julian's sourceforge-reported bug: https://sourceforge.net/tracker/?func=detailatid=893831aid=2681442group_id=180599 And I guess Thomas or Bernhard will be happy to give it a try, too... :) There was one issue, the IRQ injection bug [1] which was related to IRQ tasks IIRC. Thomas and I finally suspected after a private chat that there is actually a different reason behind it, something like interrupt.pending should be cleared when the injection took place via an (emulated) task switch. Any news on this, Thomas? Jan [1] http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/29288 signature.asc Description: OpenPGP digital signature
Re: [PATCH 4/4] Fix task switching.
On Mon, Mar 30, 2009 at 06:35:05PM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Mon, Mar 30, 2009 at 06:04:45PM +0200, Jan Kiszka wrote: Gleb Natapov wrote: The patch fixes two problems with task switching. 1. Back link is written to a wrong TSS. 2. Instruction emulation is not needed if the reason for task switch is a task gate in IDT and access to it is caused by an external even. 2 is currently solved only for VMX since there is not reliable way to skip an instruction in SVM. We should emulate it instead. Does this series fix all issues Bernhard, Thomas and Julian stumbled over? Haven't tried. I wrote my own tests for task switching. How can I check it? There is a test case attached to Julian's sourceforge-reported bug: https://sourceforge.net/tracker/?func=detailatid=893831aid=2681442group_id=180599 I'll try that. And I guess Thomas or Bernhard will be happy to give it a try, too... :) There was one issue, the IRQ injection bug [1] which was related to IRQ tasks IIRC. Thomas and I finally suspected after a private chat that there is actually a different reason behind it, something like interrupt.pending should be cleared when the injection took place via an (emulated) task switch. Any news on this, Thomas? If this is the case then the patch series should fix it. Jan [1] http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/29288 -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Fix task switching.
On Mon, Mar 30, 2009 at 06:35:05PM +0200, Jan Kiszka wrote: Haven't tried. I wrote my own tests for task switching. How can I check it? There is a test case attached to Julian's sourceforge-reported bug: https://sourceforge.net/tracker/?func=detailatid=893831aid=2681442group_id=180599 Works for me. -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] Fix task switching.
Gleb Natapov g...@redhat.com writes: On Mon, Mar 30, 2009 at 06:35:05PM +0200, Jan Kiszka wrote: Haven't tried. I wrote my own tests for task switching. How can I check it? There is a test case attached to Julian's sourceforge-reported bug: https://sourceforge.net/tracker/?func=detailatid=893831aid=2681442group_id=180599 Works for me. Then the patches should be fine (at least for me *g*). Regards, -- Julian Stecklina The day Microsoft makes something that doesn't suck is probably the day they start making vacuum cleaners - Ernst Jan Plugge -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
