Re: [PATCH] svm: implement NEXTRIPsave SVM feature

[Avi Kivity <avi@redhat.com>]

On 04/12/2010 01:29 PM, Alexander Graf wrote:
> On 12.04.2010, at 12:20, Avi Kivity wrote:
>
>    
>> On 04/12/2010 12:07 AM, Andre Przywara wrote:
>>      
>>> On SVM we set the instruction length of skipped instructions
>>> to hard-coded, well known values, which could be wrong when (bogus,
>>> but valid) prefixes (REX, segment override) are used.
>>> Newer AMD processors (Fam10h 45nm and better, aka. PhenomII or
>>> AthlonII) have an explicit NEXTRIP field in the VMCB containing the
>>> desired information.
>>> Since it is cheap to do so, we use this field to override the guessed
>>> value on newer processors.
>>> A fix for older CPUs would be rather expensive, as it would require
>>> to fetch and partially decode the instruction. As the problem is not
>>> a security issue and needs special, handcrafted code to trigger
>>> (no compiler will ever generate such code), I omit a fix for older
>>> CPUs.
>>> If someone is interested, I have both a patch for these CPUs as well as
>>> demo code triggering this issue: It segfaults under KVM, but runs
>>> perfectly on native Linux.
>>>
>>>
>>> @@ -319,6 +319,9 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu)
>>>   {
>>>   	struct vcpu_svm *svm = to_svm(vcpu);
>>>
>>> +	if (svm->vmcb->control.next_rip != 0)
>>> +		svm->next_rip = svm->vmcb->control.next_rip;
>>> +
>>>   	if (!svm->next_rip) {
>>>   		if (emulate_instruction(vcpu, 0, 0, EMULTYPE_SKIP) !=
>>>   				EMULATE_DONE)
>>>
>>>        
>> How does this interact with nested svm?  Suppose we exit a nested guest, then emulate a #VMEXIT, we'll have next_rip from a previous exit?
>>      
> Since we don't call skip_emulated_instruction on #VMEXIT injection I think we're safe here. The instruction skip is done at the vmrun intercept.
>
>    

I think you're right.  If the guest intercepts, we don't 
skip_emulated_instruction().  If the guest doesn't intercept, we keep 
the nested vmcb, so next_rip is right.

Neat.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.