Re: [PATCH][SVM] Only save/restore MSRs when needed

Avi Kivity
2007-04-28T06:19:44Z

Anthony Liguori wrote:
> Anthony Liguori wrote:
>> I've tested this with 32-bit and 64-bit guests on a 64-bit host and
>> with 32-bit guests on a 32-bit host.
>>
>> I *think* it's doing the right thing wrt to DEBUGCTL but an extra set
>> of eyes would be helpful.
>
> I think the follow patch preserves the current behavior but I'm
> starting to believe that the current behavior is wrong.
>
> The current code assumes that the guest DEBUGCTL gets loaded/saved
> during vmrun/vmexit. By my reading of the spec (I finally printed out
> the new one :-)), DEBUGCTL only gets saved if LBR virtualization is
> enabled. LBR virtualization also triggers the save/restore of the
> other 4 debug registers. The spec seems to suggest that the host
> version is saved before the guest is loaded too.
>
> I think the right behavior is to do nothing if LBR is available and
> enabled; hw will do it all for us. If LBR isn't available, I think we
> should be saving/restoring not just DEBUGCTL but the other 4. We can
> further optimize that path though by doing the same sort of tricks I
> have below.
>
> If that all seems sane, I'll submit another patch to fix that up.
>

Some observations from grepping the sources:

- The host never uses the DEBUGCTL msr. So we only need to restore it
(to zero) if the guest changes it.
- The guest never uses the DEBUGCTL msr, as kvm doesn't handle wrmsr to
DEBUGCTL.

So, doing nothing (in hardware and software) would probably work well
and be quite fast. If we ever emulate DEBUGCTL, we can conditionally
enable hardware or software save/restore (this is fairly similar to lazy
fpu). If Linux ever gets DEBUGCTL support, we'd just ask for a hook so
we can conditionally save and restore on the host side.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.