Re: [PATCH] svm: implement NEXTRIPsave SVM feature

Andre Przywara
2010-04-11T21:52:06Z

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