Re: [PATCH]: Fix MSR_K7_EVNTSEL{0,3} for SVM

Chris Lalancette
2008-04-25T18:43:19Z

Avi Kivity wrote:
>
> Hmm, looking back at the dump:
>
>> 1811: 8d 86 00 00 ff 3f lea 0x3fff0000(%rsi),%eax
>> 1817: 83 f8 03 cmp $0x3,%eax
>> 181a: 0f 87 e2 01 00 00 ja 1a02 <svm_set_msr+0x27f>
>
> So while gcc is using %rsi, it loads the result back into %eax, which
> has the effect of dropping back into 32-bits. So looks like gcc was
> right here. Sorry for spreading confusion and apologies to gcc.
>

Avi,
Arg. I was completely, utterly wrong about the problem here (although
there is definitely still a problem). I'm sorry for making a confusing mess out
of this. Here is what is actually happening:

During startup, the RHEL-4 x86_64 kernel (2.6.9-67.EL, if you care) setups up
the NMI watchdog. It does the following:

for(i = 0; i < 4; ++i) {
/* Simulator may not support it */
if (checking_wrmsrl(MSR_K7_EVNTSEL0+i, 0UL))
return;
wrmsrl(MSR_K7_PERFCTR0+i, 0UL);
}

checking_wrmsrl() just does a "test write" to the msr; because of the code that
is currently in there, this succeeds. However, when it tries to do the
MSR_K7_PERFCTR0 wrmsr, *that* is where it fails, since we don't currently handle
that MSR, and KVM injects a GPF into the guest (which kills it). My previous
patch just happened to fix this because it was making checking_wrmsrl() fail on
the EVNTSEL0, so we just returned out of this loop rather than trying to write
to the PERFCTR0.

Unfortunately, we can't just "fake emulate" MSR_K7_PERFCTR[0-3] like we are
doing for MSR_K7_EVNTSEL[0-3]; if they are there, linux expects to be able to
put values into them. I think the correct solution here is to emulate
MSR_K7_PERFCTR[0-3] and MSR_K7_EVNTSEL[0-3] for real. I'm working on a patch to
do this now.

Chris Lalancette