Alexander Graf wrote:
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?
A wrap-around to zero? From kernel space to user space? Come on, that
sounds a bit constructed (A20, someone?). I dimly remember reading in
our internal docs that 0 is a safe indicator for a missing NEXTRIP. I
will do some research tomorrow.
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?
Yes. It removes the "guessed" value lines entirely and triggers a decode
if NEXTRIP is not available.
> I was more concerned about readability here - it's great to
> be able to follow code on what it does :-).
Maybe a comment about the overriding behavior of the NEXTRIP line would
appease you?
Andre.
--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12
--
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
