Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception

[Jan Kiszka <jan.kiszka@web.de>]

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

Marcelo Tosatti wrote:
> On Wed, Nov 11, 2009 at 09:07:08PM +0100, Jan Kiszka wrote:
>> Marcelo Tosatti wrote:
>>> From: Joerg Roedel <joerg.roedel@amd.com>
>>>
>>> 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@web.de>
>> 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?

Right, forgot for the moment that triple fault implies user space.

> 
>> 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).

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:

> @@ -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);
>  }

So far cr2 was not touched on DF, now it is.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]