* [PATCH] x86: handle double and triple faults for every exception @ 2008-04-29 15:02 Joerg Roedel 2008-04-30 8:45 ` Jan Kiszka 2008-05-02 11:07 ` Avi Kivity 0 siblings, 2 replies; 9+ messages in thread From: Joerg Roedel @ 2008-04-29 15:02 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm-devel, Joerg Roedel, Jan Kiszka 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 <joerg.roedel@amd.com> Cc: Jan Kiszka <jan.kiszka@siemens.com> --- arch/x86/kvm/x86.c | 31 +++++++++++++++++-------------- 1 files changed, 17 insertions(+), 14 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 578a0c1..c05aa32 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -144,9 +144,21 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) } 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; @@ -157,25 +169,16 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, u32 error_code) { ++vcpu->stat.pf_guest; - if (vcpu->arch.exception.pending) { - if (vcpu->arch.exception.nr == PF_VECTOR) { - printk(KERN_DEBUG "kvm: inject_page_fault:" - " double fault 0x%lx\n", addr); - vcpu->arch.exception.nr = DF_VECTOR; - vcpu->arch.exception.error_code = 0; - } else if (vcpu->arch.exception.nr == DF_VECTOR) { - /* triple fault -> shutdown */ - set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); - } - return; - } vcpu->arch.cr2 = addr; kvm_queue_exception_e(vcpu, PF_VECTOR, error_code); } 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; -- 1.5.3.7 ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: handle double and triple faults for every exception 2008-04-29 15:02 [PATCH] x86: handle double and triple faults for every exception Joerg Roedel @ 2008-04-30 8:45 ` Jan Kiszka 2008-04-30 9:48 ` Joerg Roedel 2008-04-30 9:51 ` Avi Kivity 2008-05-02 11:07 ` Avi Kivity 1 sibling, 2 replies; 9+ messages in thread From: Jan Kiszka @ 2008-04-30 8:45 UTC (permalink / raw) To: Joerg Roedel; +Cc: kvm-devel, Avi Kivity Joerg Roedel wrote: > 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 <joerg.roedel@amd.com> > Cc: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/x86/kvm/x86.c | 31 +++++++++++++++++-------------- > 1 files changed, 17 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 578a0c1..c05aa32 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -144,9 +144,21 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) > } > 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; > @@ -157,25 +169,16 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, > u32 error_code) > { > ++vcpu->stat.pf_guest; > - if (vcpu->arch.exception.pending) { > - if (vcpu->arch.exception.nr == PF_VECTOR) { > - printk(KERN_DEBUG "kvm: inject_page_fault:" > - " double fault 0x%lx\n", addr); > - vcpu->arch.exception.nr = DF_VECTOR; > - vcpu->arch.exception.error_code = 0; > - } else if (vcpu->arch.exception.nr == DF_VECTOR) { > - /* triple fault -> shutdown */ > - set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); > - } > - return; > - } > vcpu->arch.cr2 = addr; > kvm_queue_exception_e(vcpu, PF_VECTOR, error_code); > } > > 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; And here is an add-on patch to fix reset-on-triple-fault: Clear the pending original exception when raising a triple fault. This allows to re-use the vcpu instance, e.g. after a reset which is typically issued as reaction on the triple fault. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- arch/x86/kvm/x86.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Index: b/arch/x86/kvm/x86.c =================================================================== --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -149,8 +149,10 @@ static void handle_multiple_faults(struc if (vcpu->arch.exception.nr != DF_VECTOR) { vcpu->arch.exception.nr = DF_VECTOR; vcpu->arch.exception.error_code = 0; - } else + } else { set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); + vcpu->arch.exception.pending = false; + } } void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr) ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: handle double and triple faults for every exception 2008-04-30 8:45 ` Jan Kiszka @ 2008-04-30 9:48 ` Joerg Roedel 2008-04-30 9:51 ` Avi Kivity 1 sibling, 0 replies; 9+ messages in thread From: Joerg Roedel @ 2008-04-30 9:48 UTC (permalink / raw) To: Jan Kiszka; +Cc: kvm-devel, Avi Kivity On Wed, Apr 30, 2008 at 10:45:12AM +0200, Jan Kiszka wrote: > Joerg Roedel wrote: > > 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 <joerg.roedel@amd.com> > > Cc: Jan Kiszka <jan.kiszka@siemens.com> > > --- > > arch/x86/kvm/x86.c | 31 +++++++++++++++++-------------- > > 1 files changed, 17 insertions(+), 14 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 578a0c1..c05aa32 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -144,9 +144,21 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) > > } > > 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; > > @@ -157,25 +169,16 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, > > u32 error_code) > > { > > ++vcpu->stat.pf_guest; > > - if (vcpu->arch.exception.pending) { > > - if (vcpu->arch.exception.nr == PF_VECTOR) { > > - printk(KERN_DEBUG "kvm: inject_page_fault:" > > - " double fault 0x%lx\n", addr); > > - vcpu->arch.exception.nr = DF_VECTOR; > > - vcpu->arch.exception.error_code = 0; > > - } else if (vcpu->arch.exception.nr == DF_VECTOR) { > > - /* triple fault -> shutdown */ > > - set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); > > - } > > - return; > > - } > > vcpu->arch.cr2 = addr; > > kvm_queue_exception_e(vcpu, PF_VECTOR, error_code); > > } > > > > 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; > > And here is an add-on patch to fix reset-on-triple-fault: > > > Clear the pending original exception when raising a triple fault. This > allows to re-use the vcpu instance, e.g. after a reset which is > typically issued as reaction on the triple fault. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > --- > arch/x86/kvm/x86.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > Index: b/arch/x86/kvm/x86.c > =================================================================== > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -149,8 +149,10 @@ static void handle_multiple_faults(struc > if (vcpu->arch.exception.nr != DF_VECTOR) { > vcpu->arch.exception.nr = DF_VECTOR; > vcpu->arch.exception.error_code = 0; > - } else > + } else { > set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); > + vcpu->arch.exception.pending = false; > + } > } > > void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr) Ah, indeed. Thanks. -- | AMD Saxony Limited Liability Company & Co. KG Operating | Wilschdorfer Landstr. 101, 01109 Dresden, Germany System | Register Court Dresden: HRA 4896 Research | General Partner authorized to represent: Center | AMD Saxony LLC (Wilmington, Delaware, US) | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: handle double and triple faults for every exception 2008-04-30 8:45 ` Jan Kiszka 2008-04-30 9:48 ` Joerg Roedel @ 2008-04-30 9:51 ` Avi Kivity 2008-04-30 10:42 ` Jan Kiszka 1 sibling, 1 reply; 9+ messages in thread From: Avi Kivity @ 2008-04-30 9:51 UTC (permalink / raw) To: Jan Kiszka; +Cc: kvm-devel Jan Kiszka wrote: > Joerg Roedel wrote: > >> 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 <joerg.roedel@amd.com> >> Cc: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> arch/x86/kvm/x86.c | 31 +++++++++++++++++-------------- >> 1 files changed, 17 insertions(+), 14 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 578a0c1..c05aa32 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -144,9 +144,21 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) >> } >> 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; >> @@ -157,25 +169,16 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, >> u32 error_code) >> { >> ++vcpu->stat.pf_guest; >> - if (vcpu->arch.exception.pending) { >> - if (vcpu->arch.exception.nr == PF_VECTOR) { >> - printk(KERN_DEBUG "kvm: inject_page_fault:" >> - " double fault 0x%lx\n", addr); >> - vcpu->arch.exception.nr = DF_VECTOR; >> - vcpu->arch.exception.error_code = 0; >> - } else if (vcpu->arch.exception.nr == DF_VECTOR) { >> - /* triple fault -> shutdown */ >> - set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); >> - } >> - return; >> - } >> vcpu->arch.cr2 = addr; >> kvm_queue_exception_e(vcpu, PF_VECTOR, error_code); >> } >> >> 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; >> > > And here is an add-on patch to fix reset-on-triple-fault: > > > Clear the pending original exception when raising a triple fault. This > allows to re-use the vcpu instance, e.g. after a reset which is > typically issued as reaction on the triple fault. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > --- > arch/x86/kvm/x86.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > Index: b/arch/x86/kvm/x86.c > =================================================================== > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -149,8 +149,10 @@ static void handle_multiple_faults(struc > if (vcpu->arch.exception.nr != DF_VECTOR) { > vcpu->arch.exception.nr = DF_VECTOR; > vcpu->arch.exception.error_code = 0; > - } else > + } else { > set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); > + vcpu->arch.exception.pending = false; > + } > } > > void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr) > > There's a bigger problem here. The exception queue is hidden state that qemu and load and save. -- Any sufficiently difficult bug is indistinguishable from a feature. ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: handle double and triple faults for every exception 2008-04-30 9:51 ` Avi Kivity @ 2008-04-30 10:42 ` Jan Kiszka 2008-04-30 10:52 ` Avi Kivity 0 siblings, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2008-04-30 10:42 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm-devel Avi Kivity wrote: > Jan Kiszka wrote: >> Joerg Roedel wrote: >> >>> 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 <joerg.roedel@amd.com> >>> Cc: Jan Kiszka <jan.kiszka@siemens.com> >>> --- >>> arch/x86/kvm/x86.c | 31 +++++++++++++++++-------------- >>> 1 files changed, 17 insertions(+), 14 deletions(-) >>> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 578a0c1..c05aa32 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -144,9 +144,21 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, >>> u64 data) >>> } >>> 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; >>> @@ -157,25 +169,16 @@ void kvm_inject_page_fault(struct kvm_vcpu >>> *vcpu, unsigned long addr, >>> u32 error_code) >>> { >>> ++vcpu->stat.pf_guest; >>> - if (vcpu->arch.exception.pending) { >>> - if (vcpu->arch.exception.nr == PF_VECTOR) { >>> - printk(KERN_DEBUG "kvm: inject_page_fault:" >>> - " double fault 0x%lx\n", addr); >>> - vcpu->arch.exception.nr = DF_VECTOR; >>> - vcpu->arch.exception.error_code = 0; >>> - } else if (vcpu->arch.exception.nr == DF_VECTOR) { >>> - /* triple fault -> shutdown */ >>> - set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); >>> - } >>> - return; >>> - } >>> vcpu->arch.cr2 = addr; >>> kvm_queue_exception_e(vcpu, PF_VECTOR, error_code); >>> } >>> >>> 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; >>> >> >> And here is an add-on patch to fix reset-on-triple-fault: >> >> >> Clear the pending original exception when raising a triple fault. This >> allows to re-use the vcpu instance, e.g. after a reset which is >> typically issued as reaction on the triple fault. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> >> --- >> arch/x86/kvm/x86.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> Index: b/arch/x86/kvm/x86.c >> =================================================================== >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -149,8 +149,10 @@ static void handle_multiple_faults(struc >> if (vcpu->arch.exception.nr != DF_VECTOR) { >> vcpu->arch.exception.nr = DF_VECTOR; >> vcpu->arch.exception.error_code = 0; >> - } else >> + } else { >> set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); >> + vcpu->arch.exception.pending = false; >> + } >> } >> >> void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr) >> >> > > There's a bigger problem here. The exception queue is hidden state that > qemu and load and save. Could you elaborate a bit on what the problematic scenario precisely is (that pending triple faults would not be saved/restored while pending exceptions are?), and if I/we can do anything to resolve it? Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: handle double and triple faults for every exception 2008-04-30 10:42 ` Jan Kiszka @ 2008-04-30 10:52 ` Avi Kivity 2008-04-30 15:59 ` Jan Kiszka 0 siblings, 1 reply; 9+ messages in thread From: Avi Kivity @ 2008-04-30 10:52 UTC (permalink / raw) To: Jan Kiszka; +Cc: kvm-devel Jan Kiszka wrote: >>> Clear the pending original exception when raising a triple fault. This >>> allows to re-use the vcpu instance, e.g. after a reset which is >>> typically issued as reaction on the triple fault. >>> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> --- >>> arch/x86/kvm/x86.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> Index: b/arch/x86/kvm/x86.c >>> =================================================================== >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -149,8 +149,10 @@ static void handle_multiple_faults(struc >>> if (vcpu->arch.exception.nr != DF_VECTOR) { >>> vcpu->arch.exception.nr = DF_VECTOR; >>> vcpu->arch.exception.error_code = 0; >>> - } else >>> + } else { >>> set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); >>> + vcpu->arch.exception.pending = false; >>> + } >>> } >>> >>> void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr) >>> >>> >>> >> There's a bigger problem here. The exception queue is hidden state that >> qemu and load and save. >> > > Could you elaborate a bit on what the problematic scenario precisely is > (that pending triple faults would not be saved/restored while pending > exceptions are?), and if I/we can do anything to resolve it? > Two scenarios: savevm (no pending exception) guest runs... loadvm (with a pending exception in the current state) spurious exception injected savevm (pending exception, lost) new qemu instance (or live migration) loadvm (exception not delivered) The second scenario is not too bad, I guess: for fault-like exceptions, the first instruction would fault again and the exception would be regenerated. The first scenario is bad, but I guess very unlikely. One fix would be to expose the exception queue to userspace. I don't like it since this is not x86 architectural state but a kvm artifact. Maybe we should clear the exception queue on kvm_set_sregs() (that should fix the reset case as well). -- Any sufficiently difficult bug is indistinguishable from a feature. ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: handle double and triple faults for every exception 2008-04-30 10:52 ` Avi Kivity @ 2008-04-30 15:59 ` Jan Kiszka 2008-04-30 17:09 ` Avi Kivity 0 siblings, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2008-04-30 15:59 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm-devel Avi Kivity wrote: > Jan Kiszka wrote: >>>> Clear the pending original exception when raising a triple fault. This >>>> allows to re-use the vcpu instance, e.g. after a reset which is >>>> typically issued as reaction on the triple fault. >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> --- >>>> arch/x86/kvm/x86.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> Index: b/arch/x86/kvm/x86.c >>>> =================================================================== >>>> --- a/arch/x86/kvm/x86.c >>>> +++ b/arch/x86/kvm/x86.c >>>> @@ -149,8 +149,10 @@ static void handle_multiple_faults(struc >>>> if (vcpu->arch.exception.nr != DF_VECTOR) { >>>> vcpu->arch.exception.nr = DF_VECTOR; >>>> vcpu->arch.exception.error_code = 0; >>>> - } else >>>> + } else { >>>> set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); >>>> + vcpu->arch.exception.pending = false; >>>> + } >>>> } >>>> >>>> void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr) >>>> >>>> >>> There's a bigger problem here. The exception queue is hidden state that >>> qemu and load and save. >>> >> >> Could you elaborate a bit on what the problematic scenario precisely is >> (that pending triple faults would not be saved/restored while pending >> exceptions are?), and if I/we can do anything to resolve it? >> > > Two scenarios: > > savevm (no pending exception) > guest runs... > loadvm (with a pending exception in the current state) > spurious exception injected > > savevm (pending exception, lost) > new qemu instance (or live migration) > loadvm (exception not delivered) > > The second scenario is not too bad, I guess: for fault-like exceptions, > the first instruction would fault again and the exception would be > regenerated. The first scenario is bad, but I guess very unlikely. > > One fix would be to expose the exception queue to userspace. I don't > like it since this is not x86 architectural state but a kvm artifact. > Maybe we should clear the exception queue on kvm_set_sregs() (that > should fix the reset case as well). Yes, the following still works for me. But I'm not the right person to ask if there are obscure cases where you may not want this clearing while just editing some registers (I'm thinking of debugger scenarios now). --------- Clear pending exceptions when setting new register values. This avoids spurious exceptions after restoring a vcpu state or after reset-on-triple-fault. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- arch/x86/kvm/x86.c | 2 ++ 1 file changed, 2 insertions(+) Index: b/arch/x86/kvm/x86.c =================================================================== --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3025,6 +3025,8 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_x86_ops->decache_regs(vcpu); + vcpu->arch.exception.pending = false; + vcpu_put(vcpu); return 0; ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: handle double and triple faults for every exception 2008-04-30 15:59 ` Jan Kiszka @ 2008-04-30 17:09 ` Avi Kivity 0 siblings, 0 replies; 9+ messages in thread From: Avi Kivity @ 2008-04-30 17:09 UTC (permalink / raw) To: Jan Kiszka; +Cc: kvm-devel Jan Kiszka wrote: > Yes, the following still works for me. But I'm not the right person to > ask if there are obscure cases where you may not want this clearing > while just editing some registers (I'm thinking of debugger scenarios > now). > > Applied, as the bug we're fixing is much more likely than the bug we're creating. Thanks. -- Any sufficiently difficult bug is indistinguishable from a feature. ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: handle double and triple faults for every exception 2008-04-29 15:02 [PATCH] x86: handle double and triple faults for every exception Joerg Roedel 2008-04-30 8:45 ` Jan Kiszka @ 2008-05-02 11:07 ` Avi Kivity 1 sibling, 0 replies; 9+ messages in thread From: Avi Kivity @ 2008-05-02 11:07 UTC (permalink / raw) To: Joerg Roedel; +Cc: kvm-devel, Jan Kiszka Joerg Roedel wrote: > 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. > Some double faults are handled serially rather than generating a #DF. For example #UD followed by #PF. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2008-05-02 11:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-04-29 15:02 [PATCH] x86: handle double and triple faults for every exception Joerg Roedel 2008-04-30 8:45 ` Jan Kiszka 2008-04-30 9:48 ` Joerg Roedel 2008-04-30 9:51 ` Avi Kivity 2008-04-30 10:42 ` Jan Kiszka 2008-04-30 10:52 ` Avi Kivity 2008-04-30 15:59 ` Jan Kiszka 2008-04-30 17:09 ` Avi Kivity 2008-05-02 11:07 ` Avi Kivity
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox