Re: [PATCH] svm: implement NEXTRIPsave SVM feature

[Andre Przywara <andre.przywara@amd.com>]

Alexander Graf wrote:
> On 11.04.2010, at 23:40, Alexander Graf wrote:
> 
>> /* Either adds offset n to the instruction counter or takes the next
>>    instruction pointer from the vmcb if the CPU supports it */
>>
>> static u64 svm_next_rip(struct kvm_vcpu *vcpu, int add)
>> {
>>        if (svm->vmcb->control.next_rip != 0)
> 
> In fact, that if should probably be:
> 
>         if (svm_has(SVM_FEATURE_NRIP))
This is not sufficient. The next RIP is only provided for some 
intercepts (namely instruction intercepts), so one would need to check 
the validity of this field anyway. By definition reserved VMCB fields 
are 0, and as 0 is never a valid _next_ RIP, this is a quick and correct 
check.

Regards,
Andre.

P.S. I don't have a strong opinion about your proposed refactoring, if 
Avi agrees I will rework it. I only found the current fix small and 
easy, and the mentioned patch for older CPUs removed the add line 
anyway, so the concerns you rose did not apply to the original version 
of the patch.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12