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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
