Handle multiple exceptions (fixes Win2003 reboot by triple fault)
cover Handle multiple exceptions (fixes Win2003 reboot by triple fault)
I suppose a complete fix would be to follow the "Conditions for Generating a Double Fault" with support for handling exceptions serially.
But this works for me.
PATCH 1/2 KVM: x86: handle double and triple faults for every exception
From: Joerg Roedel <[email protected]>
The current KVM x86 exception code handles double and triple faults only for page fault exceptions. This patch extends this detection for every exception that gets queued for the guest.
arch/x86/kvm/x86.c +17 -20
@@ -170,9 +170,21 @@ void kvm_set_apic_base(struct kvm_vcpu * }EXPORT_SYMBOL_GPL (kvm_set_apic_base); +static void handle_multiple_faults (struct kvm_vcpu *vcpu) +{ +if (vcpu->arch.exception.nr !=DF_VECTOR ) { + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ; + }else +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +} +void kvm_queue_exception (struct kvm_vcpu *vcpu,unsigned nr) { -WARN_ON (vcpu->arch.exception.pending); +if (vcpu->arch.exception.pending) { +handle_multiple_faults (vcpu); +return ; + } vcpu->arch.exception.pending = true; vcpu->arch.exception.has_error_code = false; vcpu->arch.exception.nr = nr;
@@ -184,24 +196,6 @@ void kvm_inject_page_fault(struct kvm_vc { ++vcpu->stat.pf_guest; -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }
@@ -214,7 +208,10 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);void kvm_queue_exception_e (struct kvm_vcpu *vcpu,unsigned nr,u32 error_code) { -WARN_ON (vcpu->arch.exception.pending); +if (vcpu->arch.exception.pending) { +handle_multiple_faults (vcpu); +return ; + } vcpu->arch.exception.pending = true; vcpu->arch.exception.has_error_code = true; vcpu->arch.exception.nr = nr;
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 8 lines · Marcelo Tosatti
From: Joerg Roedel <[email protected]>
The current KVM x86 exception code handles double and triple faults only for page fault exceptions. This patch extends this detection for every exception that gets queued for the guest.
Signed-off-by Joerg Roedel <[email protected]>CC Jan Kiszka <[email protected]>
For a moment I felt like I was time traveling - back in '08. :)
Reading the archive I noticed that someone posted a fix-up for this patch:
http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/16931
Why don't we need this anymore?
Moreover, are we sure to not regress /wrt to the cases that shall be handled serially? So far they should have triggered the WARN_ON, right? But maybe that went through unnoticed, and the guest was simply happy to not have triggered a triple fault...
Jan
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 10 lines · Jan Kiszka
Quoted 8 lines · Marcelo Tosatti
From: Joerg Roedel <[email protected]>
The current KVM x86 exception code handles double and triple faults only for page fault exceptions. This patch extends this detection for every exception that gets queued for the guest.
Signed-off-by Joerg Roedel <[email protected]>CC Jan Kiszka <[email protected]>
Quoted 7 lines
For a moment I felt like I was time traveling - back in '08. :)
Reading the archive I noticed that someone posted a fix-up for this patch:
http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/16931
Why don't we need this anymore?
I suppose qemu-kvm's call to set_sregs (via system_reset) will end up clearing pending exception?
Quoted 2 lines
Moreover, are we sure to not regress /wrt to the cases that shall be handled serially? So far they should have triggered the
WARN_ON, right?
Right.
How can it regress though, given that serially handled exceptions are not supported at the moment (you get a WARN_ON and lose the previously queued anyway).
Quoted 2 lines
But maybe that went through unnoticed, and the guest was simply happy to not have triggered a triple fault...
Can't parse. What went through unnoticed, and when/which guest was happy to not trigger a triple fault?
BTW, from my understanding of the documentation, the triple fault should happen only on an exception during execution of the double fault handler, which is not what the pagefault injection code does (or Joerg's patch and the TSS exceptions in the task switch code).
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 12 lines · Marcelo Tosatti
Quoted 11 lines · Jan Kiszka
Quoted 8 lines · Marcelo Tosatti
From: Joerg Roedel <[email protected]>
The current KVM x86 exception code handles double and triple faults only for page fault exceptions. This patch extends this detection for every exception that gets queued for the guest.
Signed-off-by Joerg Roedel <[email protected]>CC Jan Kiszka <[email protected]>For a moment I felt like I was time traveling - back in '08. :)
Quoted 8 lines
Quoted 5 lines
Reading the archive I noticed that someone posted a fix-up for this patch:
http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/16931
Why don't we need this anymore?
I suppose qemu-kvm's call to
set_sregs(viasystem_reset) will end up clearing pending exception?
Right, forgot for the moment that triple fault implies user space.
Quoted 9 lines
Quoted 3 lines
Moreover, are we sure to not regress /wrt to the cases that shall be handled serially? So far they should have triggered the
WARN_ON, right?Right.
How can it regress though, given that serially handled exceptions are not supported at the moment (you get a
WARN_ONand lose the previously queued anyway).
The guest so far sees the second exception as the result, now it sees DF. So the behavior changes from broken to broken, but I wondered if the current state is already so broken that this change doesn't matter.
Another micro difference is this:
Quoted 15 lines
arch/x86/kvm/x86.c +0
-11@@ -184,24 +196,6 @@ void kvm_inject_page_fault(struct kvm_vc { ++vcpu->stat.pf_guest; -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default :
arch/x86/kvm/x86.c void kvm_inject_page_fault(struct kvm_vc · line 201
Quoted 10 lines
arch/x86/kvm/x86.c +0
-7-/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }
So far cr2 was not touched on DF, now it is.
Jan
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 30 lines · Jan Kiszka
Quoted 12 lines · Marcelo Tosatti
Quoted 11 lines · Jan Kiszka
Quoted 8 lines · Marcelo Tosatti
From: Joerg Roedel <[email protected]>
The current KVM x86 exception code handles double and triple faults only for page fault exceptions. This patch extends this detection for every exception that gets queued for the guest.
Signed-off-by Joerg Roedel <[email protected]>CC Jan Kiszka <[email protected]>For a moment I felt like I was time traveling - back in '08. :)
Quoted 8 lines
Quoted 5 lines
Reading the archive I noticed that someone posted a fix-up for this patch:
http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/16931
Why don't we need this anymore?
I suppose qemu-kvm's call to
set_sregs(viasystem_reset) will end up clearing pending exception?Right, forgot for the moment that triple fault implies user space.
Quoted 6 lines
Quoted 3 lines
Moreover, are we sure to not regress /wrt to the cases that shall be handled serially? So far they should have triggered the
WARN_ON, right?Right.
Quoted 7 lines
Quoted 3 lines
How can it regress though, given that serially handled exceptions are not supported at the moment (you get a
WARN_ONand lose the previously queued anyway).The guest so far sees the second exception as the result, now it sees DF. So the behavior changes from broken to broken, but I wondered if the current state is already so broken that this change doesn't matter.
I see your point. I suppose the WARN_ON is there to catch any code paths that could trigger (unsupported) multiple exceptions, and apparently no path does that now (other than pagefault which is handled separately) ?
Quoted 21 lines
Another micro difference is this:
Quoted 19 lines
arch/x86/kvm/x86.c +0
-15@@ -184,24 +196,6 @@ void kvm_inject_page_fault(struct kvm_vc { ++vcpu->stat.pf_guest; -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false;
Quoted 8 lines
arch/x86/kvm/x86.c void kvm_inject_page_fault(struct kvm_vc · line 201
Quoted 6 lines
arch/x86/kvm/x86.c +0
-3-break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }So far cr2 was not touched on DF, now it is.
Yep. The PF was overwritten with DF, which means the cr2 value will not be interpreted by the guest?
Quoted 3 lines
Jan
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 32 lines
Quoted 23 lines
Another micro difference is this:
Quoted 19 lines
arch/x86/kvm/x86.c +0
-15@@ -184,24 +196,6 @@ void kvm_inject_page_fault(struct kvm_vc { ++vcpu->stat.pf_guest; -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT ,&vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false;Quoted 9 lines
arch/x86/kvm/x86.c void kvm_inject_page_fault(struct kvm_vc · line 201
Quoted 7 lines
arch/x86/kvm/x86.c +0
-3-break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }So far cr2 was not touched on DF, now it is.
Quoted 3 lines
Yep. The PF was overwritten with DF, which means the cr2 value will not be interpreted by the guest?
The note under interrupt 14 documentation in 5.15 indicates that cr2 is updated when the page fault is detected, not delivered, so it is correct to update cr2 immediately.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
In reply to Marcelo TosattiQuoted 16 lines · Marcelo Tosatti
From: Joerg Roedel <[email protected]>
The current KVM x86 exception code handles double and triple faults only for page fault exceptions. This patch extends this detection for every exception that gets queued for the guest.
Signed-off-by Joerg Roedel <[email protected]>CC Jan Kiszka <[email protected]>Index:
kvm/arch/x86/kvm/x86.c
===================================================================arch/x86/kvm/x86.c +0
-0@@ -170,9 +170,21 @@ void kvm_set_apic_base(struct kvm_vcpu * }EXPORT_SYMBOL_GPL (kvm_set_apic_base);
arch/x86/kvm/x86.c void kvm_set_apic_base(struct kvm_vcpu * · line 181
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+static void handle_multiple_faults (struct kvm_vcpu *vcpu) +{ +if (vcpu->arch.exception.nr !=DF_VECTOR ) { + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ; + }else +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +} +
Making #DF from two bening exceptions is very wrong.
Quoted 20 lines
arch/x86/kvm/x86.c +4
-7void kvm_queue_exception (struct kvm_vcpu *vcpu,unsigned nr) { -WARN_ON (vcpu->arch.exception.pending); +if (vcpu->arch.exception.pending) { +handle_multiple_faults (vcpu); +return ; + } vcpu->arch.exception.pending = true; vcpu->arch.exception.has_error_code = false; vcpu->arch.exception.nr = nr;@@ -184,24 +196,6 @@ void kvm_inject_page_fault(struct kvm_vc { ++vcpu->stat.pf_guest; -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ;
arch/x86/kvm/x86.c void kvm_inject_page_fault(struct kvm_vc · line 202
Quoted 10 lines
arch/x86/kvm/x86.c +0
-10-case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ;
When exceptions are handled serially previous exception have to be replaced by new one. Think about #PF during #DE. #PF should be handled first before #DE can proceed.
Quoted 19 lines
arch/x86/kvm/x86.c +4
-3- } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }@@ -214,7 +208,10 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);void kvm_queue_exception_e (struct kvm_vcpu *vcpu,unsigned nr,u32 error_code) { -WARN_ON (vcpu->arch.exception.pending); +if (vcpu->arch.exception.pending) { +handle_multiple_faults (vcpu); +return ; + } vcpu->arch.exception.pending = true; vcpu->arch.exception.has_error_code = true; vcpu->arch.exception.nr = nr;Signature · 1 line
--
--
Gleb.
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 13 lines · Gleb Natapov
Quoted 13 lines
arch/x86/kvm/x86.c +0
-12-if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT ,&vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope
Quoted 9 lines
arch/x86/kvm/x86.c void kvm_inject_page_fault(struct kvm_vc · line 202
Quoted 5 lines
arch/x86/kvm/x86.c +0
-4-that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ;When exceptions are handled serially previous exception have to be replaced by new one. Think about #PF during #DE. #PF should be handled first before #DE can proceed.
"replacing" exceptions is dangerous in the case of debug exceptions and machine checks, since restarting execution won't recover them.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 22 lines · Avi Kivity
Quoted 12 lines · Gleb Natapov
Quoted 12 lines
arch/x86/kvm/x86.c +0
-11-if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT ,&vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default :Quoted 8 lines
arch/x86/kvm/x86.c void kvm_inject_page_fault(struct kvm_vc · line 202
Quoted 5 lines
arch/x86/kvm/x86.c +0
-5-/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ;When exceptions are handled serially previous exception have to be replaced by new one. Think about #PF during #DE. #PF should be handled first before #DE can proceed.
Quoted 3 lines
"replacing" exceptions is dangerous in the case of debug exceptions and machine checks, since restarting execution won't recover them.
But not replacing them is not better. No point in re-injecting exception that causes another exception.
--
Gleb.
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 8 lines · Gleb Natapov
Quoted 5 lines
"replacing" exceptions is dangerous in the case of debug exceptions and machine checks, since restarting execution won't recover them.
But not replacing them is not better. No point in re-injecting exception that causes another exception.
Right, just pointing out there's still a small hole left. Replacing is much better ignoring.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 6 lines · Avi Kivity
Quoted 5 lines · Gleb Natapov
Quoted 5 lines
"replacing" exceptions is dangerous in the case of debug exceptions and machine checks, since restarting execution won't recover them.
Quoted 6 lines
Quoted 3 lines
But not replacing them is not better. No point in re-injecting exception that causes another exception.
Right, just pointing out there's still a small hole left. Replacing is much better ignoring.
Is the hardware doing some queuing in this case? Would it be overly complicated to adopt the real behavior here?
Jan
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 10 lines · Jan Kiszka
Quoted 8 lines · Avi Kivity
Quoted 6 lines · Gleb Natapov
Quoted 6 lines
"replacing" exceptions is dangerous in the case of debug exceptions and machine checks, since restarting execution won't recover them.
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
But not replacing them is not better. No point in re-injecting exception that causes another exception.
Right, just pointing out there's still a small hole left. Replacing is much better ignoring.
Is the hardware doing some queuing in this case?
Gleb has verified by testing, and Intel has confirmed, that the hardware does not queue, and will in fact lose traps and NMIs in such a case. So the small hole is actually present in the processor and we're better off not queueing! I've forgotten about this but Gleb reminded me.
Quoted 3 lines
Would it be overly
complicated to adopt the real behavior here?
Not only would it be difficult, it would also be incorrect.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception
Quoted 25 lines · Avi Kivity
Quoted 10 lines · Jan Kiszka
Quoted 8 lines · Avi Kivity
Quoted 6 lines · Gleb Natapov
Quoted 6 lines
"replacing" exceptions is dangerous in the case of debug exceptions and machine checks, since restarting execution won't recover them.
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
But not replacing them is not better. No point in re-injecting exception that causes another exception.
Right, just pointing out there's still a small hole left. Replacing is much better ignoring.
Is the hardware doing some queuing in this case?
Gleb has verified by testing, and Intel has confirmed, that the hardware does not queue, and will in fact lose traps and NMIs in such a case. So the small hole is actually present in the processor and we're better off not queueing! I've forgotten about this but Gleb reminded me.
Quoted 6 lines
Quoted 3 lines
Would it be overly
complicated to adopt the real behavior here?Not only would it be difficult, it would also be incorrect.
I'm all for correctness - so this correctly drop events. :)
Jan
PATCH 2/2 KVM: x86: raise TSS exception for NULL CS and SS segments
In reply to Marcelo TosattiWindows 2003 uses task switch to triple fault and reboot (the other exception being reserved pdptrs bits).
arch/x86/kvm/x86.c +11 -0
@@ -4091,6 +4091,15 @@ static int is_vm86_segment(struct kvm_vc (kvm_x86_ops->get_rflags (vcpu) &X86_EFLAGS_VM ); } +static void kvm_check_segment_descriptor (struct kvm_vcpu *vcpu,int seg, +u16 selector) +{ +/* NULL selector is not valid for CS and SS */ +if (seg ==VCPU_SREG_CS || seg ==VCPU_SREG_SS ) +if (!selector) +kvm_queue_exception_e (vcpu,TS_VECTOR , selector >>3 ); +} +int kvm_load_segment_descriptor (struct kvm_vcpu *vcpu,u16 selector,int type_bits,int seg) {
@@ -4100,6 +4109,8 @@ int kvm_load_segment_descriptor(struct kreturn kvm_load_realmode_segment (vcpu, selector, seg);if (load_segment_descriptor_to_kvm_desct (vcpu, selector, &kvm_seg))return 1 ; + +kvm_check_segment_descriptor (vcpu, seg, selector); kvm_seg.type |= type_bits; if (seg != VCPU_SREG_SS && seg != VCPU_SREG_CS &&
Quoted 6 lines · Marcelo Tosatti
I suppose a complete fix would be to follow the "Conditions for Generating a Double Fault" with support for handling exceptions serially.
But this works for me.
I prefer proper solution. Like one attached (this is combination of ths patch by Eddie Dong and my fix):
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
arch/x86/kvm/x86.c +54 -28
@@ -248,12 +248,61 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) }EXPORT_SYMBOL_GPL (kvm_set_apic_base); +#define EXCPT_BENIGN 0 +#define EXCPT_CONTRIBUTORY 1 +#define EXCPT_PF 2 + +static int exception_class (int vector) +{ +if (vector ==14 ) +return EXCPT_PF ; +else if (vector ==0 || (vector >=10 && vector <=13 )) +return EXCPT_CONTRIBUTORY ; +else +return EXCPT_BENIGN ; +} + +static void kvm_multiple_exception (struct kvm_vcpu *vcpu, +unsigned nr,bool has_error,u32 error_code) +{ +u32 prev_nr; +int class1, class2; + +if (!vcpu->arch.exception.pending) { + queue: + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = has_error; + vcpu->arch.exception.nr = nr; + vcpu->arch.exception.error_code = error_code; +return ; + } + +/* to check exception */ + prev_nr = vcpu->arch.exception.nr; +if (prev_nr ==DF_VECTOR ) { +/* triple fault -> shutdown */ +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +return ; + } + class1 =exception_class (prev_nr); + class2 =exception_class (nr); +if ((class1 ==EXCPT_CONTRIBUTORY && class2 ==EXCPT_CONTRIBUTORY ) + || (class1 ==EXCPT_PF && class2 !=EXCPT_BENIGN )) { +/* generate double fault per SDM Table 5-5 */ + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = true; + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ; + }else +/* replace previous exception with a new one in a hope +that instruction re-execution will regenerate lost +exception */ + goto queue; +} +void kvm_queue_exception (struct kvm_vcpu *vcpu,unsigned nr) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = false; - vcpu->arch.exception.nr = nr; +kvm_multiple_exception (vcpu, nr, false,0 ); }EXPORT_SYMBOL_GPL (kvm_queue_exception);
@@ -261,25 +310,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, u32 error_code) { ++vcpu->stat.pf_guest; - -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }
@@ -292,11 +322,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);void kvm_queue_exception_e (struct kvm_vcpu *vcpu,unsigned nr,u32 error_code) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = true; - vcpu->arch.exception.nr = nr; - vcpu->arch.exception.error_code = error_code; +kvm_multiple_exception (vcpu, nr, true, error_code); }EXPORT_SYMBOL_GPL (kvm_queue_exception_e);
--
Gleb.
Quoted 9 lines · Gleb Natapov
Quoted 6 lines · Marcelo Tosatti
I suppose a complete fix would be to follow the "Conditions for Generating a Double Fault" with support for handling exceptions serially.
But this works for me.
I prefer proper solution. Like one attached (this is combination of ths patch by Eddie Dong and my fix):
Nice, preferred here as well. I only have a minor comment below.
Quoted 13 lines
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
Signed-off-by Eddie Dong <[email protected]>Signed-off-by Gleb Natapov <[email protected]>arch/x86/kvm/x86.c +0
-0@@ -248,12 +248,61 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) }EXPORT_SYMBOL_GPL (kvm_set_apic_base);
arch/x86/kvm/x86.c void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) · line 259
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+#define EXCPT_BENIGN 0 +#define EXCPT_CONTRIBUTORY 1 +#define EXCPT_PF 2 + +static int exception_class (int vector) +{ +if (vector ==14 ) +return EXCPT_PF ; +else if (vector ==0 || (vector >=10 && vector <=13 ))
xx_VECTOR?
Quoted 82 lines
arch/x86/kvm/x86.c +44
-23+return EXCPT_CONTRIBUTORY ; + else + return EXCPT_BENIGN; +} + +static void kvm_multiple_exception (struct kvm_vcpu *vcpu, +unsigned nr,bool has_error,u32 error_code) +{ +u32 prev_nr; +int class1, class2; + +if (!vcpu->arch.exception.pending) { + queue: + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = has_error; + vcpu->arch.exception.nr = nr; + vcpu->arch.exception.error_code = error_code; +return ; + } + +/* to check exception */ + prev_nr = vcpu->arch.exception.nr; +if (prev_nr ==DF_VECTOR ) { +/* triple fault -> shutdown */ +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +return ; + } + class1 =exception_class (prev_nr); + class2 =exception_class (nr); +if ((class1 ==EXCPT_CONTRIBUTORY && class2 ==EXCPT_CONTRIBUTORY ) + || (class1 ==EXCPT_PF && class2 !=EXCPT_BENIGN )) { +/* generate double fault per SDM Table 5-5 */ + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = true; + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ; + }else +/* replace previous exception with a new one in a hope +that instruction re-execution will regenerate lost +exception */ + goto queue; +} +void kvm_queue_exception (struct kvm_vcpu *vcpu,unsigned nr) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = false; - vcpu->arch.exception.nr = nr; +kvm_multiple_exception (vcpu, nr, false,0 ); }EXPORT_SYMBOL_GPL (kvm_queue_exception);@@ -261,25 +310,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, u32 error_code) { ++vcpu->stat.pf_guest; - -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }@@ -292,11 +322,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);void kvm_queue_exception_e (struct kvm_vcpu *vcpu,unsigned nr,u32 error_code)
arch/x86/kvm/x86.c EXPORT_SYMBOL_GPL(kvm_inject_nmi); · line 328
Quoted 10 lines
arch/x86/kvm/x86.c +1
-5{ -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = true; - vcpu->arch.exception.nr = nr; - vcpu->arch.exception.error_code = error_code; +kvm_multiple_exception (vcpu, nr, true, error_code); }EXPORT_SYMBOL_GPL (kvm_queue_exception_e);
Jan
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Gleb Natapov
Quoted 4 lines · Marcelo Tosatti
I suppose a complete fix would be to follow the "Conditions for Generating a Double Fault" with support for handling exceptions serially.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
But this works for me.
I prefer proper solution. Like one attached (this is combination of ths patch by Eddie Dong and my fix):
Nice, preferred here as well. I only have a minor comment below.
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
arch/x86/kvm/x86.c +54 -28
@@ -248,12 +248,61 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) }EXPORT_SYMBOL_GPL (kvm_set_apic_base); +#define EXCPT_BENIGN 0 +#define EXCPT_CONTRIBUTORY 1 +#define EXCPT_PF 2 + +static int exception_class (int vector) +{ +if (vector ==14 ) +return EXCPT_PF ; +else if (vector ==DE_VECTOR || (vector >=TS_VECTOR && vector <=GP_VECTOR )) +return EXCPT_CONTRIBUTORY ; +else +return EXCPT_BENIGN ; +} + +static void kvm_multiple_exception (struct kvm_vcpu *vcpu, +unsigned nr,bool has_error,u32 error_code) +{ +u32 prev_nr; +int class1, class2; + +if (!vcpu->arch.exception.pending) { + queue: + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = has_error; + vcpu->arch.exception.nr = nr; + vcpu->arch.exception.error_code = error_code; +return ; + } + +/* to check exception */ + prev_nr = vcpu->arch.exception.nr; +if (prev_nr ==DF_VECTOR ) { +/* triple fault -> shutdown */ +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +return ; + } + class1 =exception_class (prev_nr); + class2 =exception_class (nr); +if ((class1 ==EXCPT_CONTRIBUTORY && class2 ==EXCPT_CONTRIBUTORY ) + || (class1 ==EXCPT_PF && class2 !=EXCPT_BENIGN )) { +/* generate double fault per SDM Table 5-5 */ + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = true; + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ; + }else +/* replace previous exception with a new one in a hope +that instruction re-execution will regenerate lost +exception */ + goto queue; +} +void kvm_queue_exception (struct kvm_vcpu *vcpu,unsigned nr) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = false; - vcpu->arch.exception.nr = nr; +kvm_multiple_exception (vcpu, nr, false,0 ); }EXPORT_SYMBOL_GPL (kvm_queue_exception);
@@ -261,25 +310,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, u32 error_code) { ++vcpu->stat.pf_guest; - -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }
@@ -292,11 +322,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);void kvm_queue_exception_e (struct kvm_vcpu *vcpu,unsigned nr,u32 error_code) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = true; - vcpu->arch.exception.nr = nr; - vcpu->arch.exception.error_code = error_code; +kvm_multiple_exception (vcpu, nr, true, error_code); }EXPORT_SYMBOL_GPL (kvm_queue_exception_e);
--
Gleb.
Quoted 36 lines · Gleb Natapov
Quoted 8 lines · Jan Kiszka
Quoted 6 lines · Gleb Natapov
Quoted 4 lines · Marcelo Tosatti
I suppose a complete fix would be to follow the "Conditions for Generating a Double Fault" with support for handling exceptions serially.
Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
But this works for me.
I prefer proper solution. Like one attached (this is combination of ths patch by Eddie Dong and my fix):
Nice, preferred here as well. I only have a minor comment below.
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
Signed-off-by Eddie Dong<[email protected]>Signed-off-by Gleb Natapov<[email protected]>arch/x86/kvm/x86.c +4
-0@@ -248,12 +248,61 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) }EXPORT_SYMBOL_GPL (kvm_set_apic_base); +#define EXCPT_BENIGN 0 +#define EXCPT_CONTRIBUTORY 1 +#define EXCPT_PF 2 +
arch/x86/kvm/x86.c void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) · line 264
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+static int exception_class (int vector) +{ +if (vector ==14 ) +return EXCPT_PF ; +else if (vector ==DE_VECTOR || (vector>=TS_VECTOR && vector<=GP_VECTOR )) +return EXCPT_CONTRIBUTORY ; +else +return EXCPT_BENIGN ; +}
It's actually less readable. I know 11 is between 10 and 13, but is NP_VECTOR between TS_VECTOR and GP_VECTOR?
This is better as a switch, or even:
u8 exception_class[] = {
[PF_VECTOR] EXPT_PF,
etc.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 9 lines
It's actually less readable. I know 11 is between 10 and 13, but is
NP_VECTORbetweenTS_VECTORandGP_VECTOR?This is better as a switch, or even:
u8 exception_class[] = {
[PF_VECTOR]EXPT_PF,etc.
OK what about this then:
From: Eddie Dong<[email protected]>
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
arch/x86/kvm/x86.c +61 -28
@@ -248,12 +248,68 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) }EXPORT_SYMBOL_GPL (kvm_set_apic_base); +#define EXCPT_BENIGN 0 +#define EXCPT_CONTRIBUTORY 1 +#define EXCPT_PF 2 + +static int exception_class (int vector) +{ +switch (vector) { +case PF_VECTOR : +return EXCPT_PF ; +case DE_VECTOR : +case TS_VECTOR : +case NP_VECTOR : +case SS_VECTOR : +case GP_VECTOR : +return EXCPT_CONTRIBUTORY ; +default : +break ; + } +return EXCPT_BENIGN ; +} + +static void kvm_multiple_exception (struct kvm_vcpu *vcpu, +unsigned nr,bool has_error,u32 error_code) +{ +u32 prev_nr; +int class1, class2; + +if (!vcpu->arch.exception.pending) { + queue: + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = has_error; + vcpu->arch.exception.nr = nr; + vcpu->arch.exception.error_code = error_code; +return ; + } + +/* to check exception */ + prev_nr = vcpu->arch.exception.nr; +if (prev_nr ==DF_VECTOR ) { +/* triple fault -> shutdown */ +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +return ; + } + class1 =exception_class (prev_nr); + class2 =exception_class (nr); +if ((class1 ==EXCPT_CONTRIBUTORY && class2 ==EXCPT_CONTRIBUTORY ) + || (class1 ==EXCPT_PF && class2 !=EXCPT_BENIGN )) { +/* generate double fault per SDM Table 5-5 */ + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = true; + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ; + }else +/* replace previous exception with a new one in a hope +that instruction re-execution will regenerate lost +exception */ + goto queue; +} +void kvm_queue_exception (struct kvm_vcpu *vcpu,unsigned nr) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = false; - vcpu->arch.exception.nr = nr; +kvm_multiple_exception (vcpu, nr, false,0 ); }EXPORT_SYMBOL_GPL (kvm_queue_exception);
@@ -261,25 +317,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, u32 error_code) { ++vcpu->stat.pf_guest; - -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }
@@ -292,11 +329,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);void kvm_queue_exception_e (struct kvm_vcpu *vcpu,unsigned nr,u32 error_code) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = true; - vcpu->arch.exception.nr = nr; - vcpu->arch.exception.error_code = error_code; +kvm_multiple_exception (vcpu, nr, true, error_code); }EXPORT_SYMBOL_GPL (kvm_queue_exception_e);
--
Gleb.
Quoted 10 lines · Gleb Natapov
Quoted 9 lines
It's actually less readable. I know 11 is between 10 and 13, but is
NP_VECTORbetweenTS_VECTORandGP_VECTOR?This is better as a switch, or even:
u8 exception_class[] = {
[PF_VECTOR]EXPT_PF,etc.
OK what about this then:
Is there a consensus on this yet? I'm tracking this issue and didn't see anything get committed yet.
Signature · 4 lines
Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx [email protected]
Quoted 10 lines · Gleb Natapov
Quoted 9 lines
It's actually less readable. I know 11 is between 10 and 13, but is
NP_VECTORbetweenTS_VECTORandGP_VECTOR?This is better as a switch, or even:
u8 exception_class[] = {
[PF_VECTOR]EXPT_PF,etc.
OK what about this then:
Looks good.
Quoted 130 lines
From: Eddie Dong<[email protected]>
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
Signed-off-by Eddie Dong<[email protected]>Signed-off-by Gleb Natapov<[email protected]>arch/x86/kvm/x86.c +61
-29@@ -248,12 +248,68 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) }EXPORT_SYMBOL_GPL (kvm_set_apic_base); +#define EXCPT_BENIGN 0 +#define EXCPT_CONTRIBUTORY 1 +#define EXCPT_PF 2 + +static int exception_class (int vector) +{ +switch (vector) { +case PF_VECTOR : +return EXCPT_PF ; +case DE_VECTOR : +case TS_VECTOR : +case NP_VECTOR : +case SS_VECTOR : +case GP_VECTOR : +return EXCPT_CONTRIBUTORY ; +default : +break ; + } +return EXCPT_BENIGN ; +} + +static void kvm_multiple_exception (struct kvm_vcpu *vcpu, +unsigned nr,bool has_error,u32 error_code) +{ +u32 prev_nr; +int class1, class2; + +if (!vcpu->arch.exception.pending) { + queue: + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = has_error; + vcpu->arch.exception.nr = nr; + vcpu->arch.exception.error_code = error_code; +return ; + } + +/* to check exception */ + prev_nr = vcpu->arch.exception.nr; +if (prev_nr ==DF_VECTOR ) { +/* triple fault -> shutdown */ +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +return ; + } + class1 =exception_class (prev_nr); + class2 =exception_class (nr); +if ((class1 ==EXCPT_CONTRIBUTORY && class2 ==EXCPT_CONTRIBUTORY ) + || (class1 ==EXCPT_PF && class2 !=EXCPT_BENIGN )) { +/* generate double fault per SDM Table 5-5 */ + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = true; + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ; + }else +/* replace previous exception with a new one in a hope +that instruction re-execution will regenerate lost +exception */ + goto queue; +} +void kvm_queue_exception (struct kvm_vcpu *vcpu,unsigned nr) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = false; - vcpu->arch.exception.nr = nr; +kvm_multiple_exception (vcpu, nr, false,0 ); }EXPORT_SYMBOL_GPL (kvm_queue_exception);@@ -261,25 +317,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, u32 error_code) { ++vcpu->stat.pf_guest; - -if (vcpu->arch.exception.pending) { -switch (vcpu->arch.exception.nr) { -case DF_VECTOR : -/* triple fault -> shutdown */ -set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); -return ; -case PF_VECTOR : - vcpu->arch.exception.nr =DF_VECTOR ; - vcpu->arch.exception.error_code =0 ; -return ; -default : -/* replace previous exception with a new one in a hope -that instruction re-execution will regenerate lost -exception */ - vcpu->arch.exception.pending = false; -break ; - } - } vcpu->arch.cr2 = addr;kvm_queue_exception_e (vcpu,PF_VECTOR , error_code); }@@ -292,11 +329,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);void kvm_queue_exception_e (struct kvm_vcpu *vcpu,unsigned nr,u32 error_code) { -WARN_ON (vcpu->arch.exception.pending); - vcpu->arch.exception.pending = true; - vcpu->arch.exception.has_error_code = true; - vcpu->arch.exception.nr = nr; - vcpu->arch.exception.error_code = error_code; +kvm_multiple_exception (vcpu, nr, true, error_code); }EXPORT_SYMBOL_GPL (kvm_queue_exception_e); --Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 8 lines · Gleb Natapov
Quoted 8 lines
It's actually less readable. I know 11 is between 10 and 13, but is
NP_VECTORbetweenTS_VECTORandGP_VECTOR?This is better as a switch, or even:
u8 exception_class[] = {
[PF_VECTOR]EXPT_PF,
Quoted 9 lines
Quoted 2 lines
etc.
OK what about this then:
From: Eddie Dong<[email protected]>
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
Much better.
Signature · 1 line
error compiling committee.c: too many arguments to function
Marcelo Tosatti
AppliedQuoted 11 lines · Gleb Natapov
Quoted 9 lines
It's actually less readable. I know 11 is between 10 and 13, but is
NP_VECTORbetweenTS_VECTORandGP_VECTOR?This is better as a switch, or even:
u8 exception_class[] = {
[PF_VECTOR]EXPT_PF,etc.
OK what about this then:
Quoted 7 lines
From: Eddie Dong<[email protected]>
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
Signed-off-by Eddie Dong<[email protected]>Signed-off-by Gleb Natapov<[email protected]>
Applied this and TSS exceptions, thanks.
Marcelo Tosatti
Tested-byQuoted 7 lines · Gleb Natapov
Quoted 6 lines · Marcelo Tosatti
I suppose a complete fix would be to follow the "Conditions for Generating a Double Fault" with support for handling exceptions serially.
But this works for me.
Quoted 8 lines
I prefer proper solution. Like one attached (this is combination of ths patch by Eddie Dong and my fix):
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
Signed-off-by Eddie Dong <[email protected]>Signed-off-by Gleb Natapov <[email protected]>
Nice.
Quoted 48 lines
arch/x86/kvm/x86.c +40
-0@@ -248,12 +248,61 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) }EXPORT_SYMBOL_GPL (kvm_set_apic_base); +#define EXCPT_BENIGN 0 +#define EXCPT_CONTRIBUTORY 1 +#define EXCPT_PF 2 + +static int exception_class (int vector) +{ +if (vector ==14 ) +return EXCPT_PF ; +else if (vector ==0 || (vector >=10 && vector <=13 )) +return EXCPT_CONTRIBUTORY ; +else +return EXCPT_BENIGN ; +} + +static void kvm_multiple_exception (struct kvm_vcpu *vcpu, +unsigned nr,bool has_error,u32 error_code) +{ +u32 prev_nr; +int class1, class2; + +if (!vcpu->arch.exception.pending) { + queue: + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = has_error; + vcpu->arch.exception.nr = nr; + vcpu->arch.exception.error_code = error_code; +return ; + } + +/* to check exception */ + prev_nr = vcpu->arch.exception.nr; +if (prev_nr ==DF_VECTOR ) { +/* triple fault -> shutdown */ +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +return ; + } + class1 =exception_class (prev_nr); + class2 =exception_class (nr); +if ((class1 ==EXCPT_CONTRIBUTORY && class2 ==EXCPT_CONTRIBUTORY ) + || (class1 ==EXCPT_PF && class2 !=EXCPT_BENIGN )) {
arch/x86/kvm/x86.c void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) · line 300
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+/* generate double fault per SDM Table 5-5 */ + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = true; + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ; + }else +/* replace previous exception with a new one in a hope +that instruction re-execution will regenerate lost +exception */
Out of curiosity, why not an exception queue?
arch/x86/kvm/x86.c void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) · line 300
Quoted 1 line
arch/x86/kvm/x86.c +1
-0+ goto queue;
This goto seems unnecessary.
Quoted 75 lines · Marcelo Tosatti
Quoted 7 lines · Gleb Natapov
Quoted 6 lines · Marcelo Tosatti
I suppose a complete fix would be to follow the "Conditions for Generating a Double Fault" with support for handling exceptions serially.
But this works for me.
Quoted 8 lines
I prefer proper solution. Like one attached (this is combination of ths patch by Eddie Dong and my fix):
Move Double-Fault generation logic out of page fault exception generating function to cover more generic case.
Signed-off-by Eddie Dong <[email protected]>Signed-off-by Gleb Natapov <[email protected]>Nice.
Tested-by Marcelo Tosatti <[email protected]>Quoted 53 lines
arch/x86/kvm/x86.c +45
-0@@ -248,12 +248,61 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) }EXPORT_SYMBOL_GPL (kvm_set_apic_base); +#define EXCPT_BENIGN 0 +#define EXCPT_CONTRIBUTORY 1 +#define EXCPT_PF 2 + +static int exception_class (int vector) +{ +if (vector ==14 ) +return EXCPT_PF ; +else if (vector ==0 || (vector >=10 && vector <=13 )) +return EXCPT_CONTRIBUTORY ; +else +return EXCPT_BENIGN ; +} + +static void kvm_multiple_exception (struct kvm_vcpu *vcpu, +unsigned nr,bool has_error,u32 error_code) +{ +u32 prev_nr; +int class1, class2; + +if (!vcpu->arch.exception.pending) { + queue: + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = has_error; + vcpu->arch.exception.nr = nr; + vcpu->arch.exception.error_code = error_code; +return ; + } + +/* to check exception */ + prev_nr = vcpu->arch.exception.nr; +if (prev_nr ==DF_VECTOR ) { +/* triple fault -> shutdown */ +set_bit (KVM_REQ_TRIPLE_FAULT , &vcpu->requests); +return ; + } + class1 =exception_class (prev_nr); + class2 =exception_class (nr); +if ((class1 ==EXCPT_CONTRIBUTORY && class2 ==EXCPT_CONTRIBUTORY ) + || (class1 ==EXCPT_PF && class2 !=EXCPT_BENIGN )) { +/* generate double fault per SDM Table 5-5 */ + vcpu->arch.exception.pending = true; + vcpu->arch.exception.has_error_code = true; + vcpu->arch.exception.nr =DF_VECTOR ; + vcpu->arch.exception.error_code =0 ;
Quoted 7 lines
arch/x86/kvm/x86.c void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) · line 299
Quoted 4 lines
arch/x86/kvm/x86.c +4
-0+ }else +/* replace previous exception with a new one in a hope +that instruction re-execution will regenerate lost +exception */ Out of curiosity, why not an exception queue?
It seems unnecessary. Lost exception will be regenerated after instruction that caused it will be re-executed.
Quoted 3 lines
arch/x86/kvm/x86.c void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) · line 300
Quoted 1 line
arch/x86/kvm/x86.c +1
-0+ goto queue;This goto seems unnecessary.
It is very important. It replaces previous exception with the new one. This allows CPU to proceed in situation where exception injection causes another exception serially. Think about #DE that causes #PF (because stack is not mapped for instance). If we'll drop #PF and re-inject #DE it will generate #PF once again. If we'll drop #DE and inject #PF then #PF handler with hopefully fix #PF condition and #DE will be regenerated when devision will be retried.
--
Gleb.