Re: [kvm-devel] [PATCH] x86: handle double and triple faults for every exception

[Avi K. <av...@qu...>]

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 <joe...@am...>
>> Cc: Jan Kiszka <jan...@si...>
>> ---
>>  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...@si...>
>
> ---
>  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.