[Qemu-devel] Re: [PATCH v3 12/21] kvm: x86: Drop MCE MSRs write back restrictions

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]

Am 05.01.2011 09:33, Huang Ying wrote:
> On Wed, 2011-01-05 at 16:07 +0800, Jan Kiszka wrote:
>> Am 05.01.2011 07:42, Huang Ying wrote:
>>> On Tue, 2011-01-04 at 16:32 +0800, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> There is no need to restrict writing back MCE MSRs to reset or full
>>>> state updates as setting their values has no side effects.
>>>
>>> Sorry for late.
>>
>> Don't worry.
>>
>>>
>>> The MCE MSRs contents is sticky for warm reset except MCG_STATUS, so
>>> their content should be kept.  And the following sequence may set
>>> uncorrected value in MCE registers.
>>>
>>> savevm -> loadvm -> (OS clear MCE registers) -> reset -> (MCE registers
>>> has new (uncorrected) value)
>>
>> Sorry, I can't follow. Unless I miss some subtle detail, the question is
>> not when we transfer the mcg_* CPUState fields to the kernel, but when
>> and how we manipulate them in user space, e.g. on reset. Where are those
>> fields touched incorrectly between get and put msrs so that we cannot
>> write them back?
> 
> If my understanding is correct, MSRs are not saved to user space
> (env->mce_banks) during reset in current code.  So if all MCE MSRs are
> restored to kernel, their user space contents from previous loadvm may
> be put into kernel after reset.

Ah, good point! We need

diff --git a/vl.c b/vl.c
index 1958e01..23d4169 100644
--- a/vl.c
+++ b/vl.c
@@ -1353,6 +1353,7 @@ static void main_loop(void)
         }
         if (qemu_reset_requested()) {
             pause_all_vcpus();
+            cpu_synchronize_all_states();
             qemu_system_reset();
             resume_all_vcpus();
         }

and corresponding (sigh...) qemu-kvm patch. Will post them.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]