On 11.04.2010, at 23:51, Andre Przywara wrote:
> 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.
It's not? If you're at -1 which is hlt in our imaginary case. What would the next instruction be?
>
> 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.
What patch for older CPUs? The one that'd be expensive? I was more concerned about readability here - it's great to be able to follow code on what it does :-).
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
