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

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

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