Re: [Qemu-devel] [PATCH] x86: Reset MTRR on vCPU reset

[Laszlo Ersek <lersek@redhat.com>]

On 08/14/14 01:17, Laszlo Ersek wrote:

> - With KVM, the lack of loading MTRR state from KVM, combined with the
>   (partial) storing of MTRR state to KVM, has two consequences:
>   - migration invalidates (loses) MTRR state,

I'll concede that migration *already* loses MTRR state (on KVM), even
before your patch. On the incoming host, the difference is that
pre-patch, the guest continues running (after migration) with MTRRs in
the "initial" KVM state, while post-patch, the guest continues running
after an explicit zeroing of the variable MTRR masks and the deftype.

I admit that it wouldn't be right to say that the patch "causes" MTRR
state loss.

With that, I think I've actually convinced myself that your patch is
correct:

The x86_cpu_reset() hunk is correct in any case, independently of KVM
vs. TCG. (On TCG it even improves MTRR conformance.) Splitting that hunk
into a separate patch might be worthwhile, but not overly important.

The kvm_put_msrs() hunk forces a zero write to the variable MTRR
PhysMasks and the DefType, on both reset and on incoming migration. For
reset, this is correct behavior. For incoming migration, it is not, but
it certainly shouldn't qualify as a regression, relative to the current
status (where MTRR state is simply lost and replaced with initial MTRR
state on the incoming host).

I think the above "end results" could be expressed more clearly in the
code, but I'm already wondering if you'll ever talk to me again, so I'm
willing to give my R-b if you think that's useful... :)

(Again, I might be wrong, easily.)

Thanks
Laszlo