[SeaBIOS] [Qemu-devel] [PATCH v3] fw/msr_feature_control: add support to set MSR_IA32_FEATURE_CONTROL

[Laszlo Ersek]

On 07/06/16 15:03, Haozhong Zhang wrote:
> On 07/06/16 13:04, Laszlo Ersek wrote:
>> On 07/06/16 08:42, Laszlo Ersek wrote:
>>> On 07/06/16 08:28, Haozhong Zhang wrote:
>>>> Hi Ashok,
>>>>
>>>> On 07/06/16 02:18, Paolo Bonzini wrote:
>>>>>> I forgot to restore MSR_IA32_FEATURE_CONTROL in the resume path, and
>>>>>> MSR_IA32_FEATURE_CONTROL is zero after S3 resume.
>>>>>
>>>>> This is a bug.  Sorry Laszlo. :)
>>>>>
>>>>>> Not restore MSR_IA32_FEATURE_CONTROL during S3 resume does not affect
>>>>>> at least Linux guest (tested 4.5). Current QEMU may advise the guest
>>>>>> firmware to set bit 20 (for LMCE), bit 2 (for VMX) and bit 0 (lock
>>>>>> bit).
>>>>>> - For LMCE, Linux only checks bit 20 and bit 0 at boot time and then
>>>>>>   keeps using the result even after resume.
>>>>>
>>>>> On real hardware, LMCE would not be enabled after resume.  I'm not
>>>>> sure what would happen, but it wouldn't be good.
>>>>
>>>> Could you help to check if the LMCE bit in MSR_IA32_FEATURE_CONTROL is
>>>> set after S3 resume on the real hardware?
>>>
>>> The SDM says that IA32_FEATURE_CONTROL is zeroed on logical processor reset.
>>>
>>>   23.7 ENABLING AND ENTERING VMX OPERATION
>>>
>>>   [...] VMXON is also controlled by the IA32_FEATURE_CONTROL MSR (MSR
>>>   address 3AH). This MSR is cleared to zero when a logical processor is
>>>   reset. [...]
>>
>> Actually, I think there is a bug in KVM at the moment. I ran the
>> following test:
>>
>> - modified OVMF to set the MSR to value 0x5 on just the BSP
>> - booted an i440fx and a Q35 (SMM-enabled) OVMF guest
>> - checked "rdmsr -a 0x3a" in both
>> - ran "pm-suspend" in both guests, woke them
>> - repeated the rdmsr command
>>
>> The result is that the BSP had the 0x5 MSR value both after cold boot
>> and after S3 resume. So, KVM does not seem to implement clearing of the MSR.
>>
> 
> Interesting result, is setting MSR on BSP also called after S3 resume?

Yes. Henceforth my middle name should be "bumblebee", because today I've
been bumbling from error to error.

In short, I messed up my ad-hoc OVMF patch, and added the wrmsr to a
location that is on both the normal boot path and the S3 resume path. I
fixed the patch and now I get the same result as you -- the MSR is
indeed clear after S3 resume.

> I went through your test steps with OVMF replaced by a modified
> SeaBIOS which only sets MSR_IA32_FEATURE_CONTROL on BSP at boot time,
> the result before S3 resume is the value on BSP is 5 and others are 0,
> and the result after S3 resume is values on all CPUs are 0.

Right.

>> I checked kvm/next (currently at
>> 196f20ca52e8c7281932663c348fa54b82d03914), and vmx_vcpu_reset() does not
>> seem to zero vmx->msr_ia32_feature_control.
>>
> 
> The function reset cpu state in QEMU after S3 resume is
> x86_cpu_reset(CPUState *s) in target-i386/cpu.c which is called for
> all vcpus and does
> 
>     memset(env, 0, offsetof(CPUX86State, cpuid_level));
> 
> CPUX86State.msr_ia32_feature_control is before .cpuid_level, so guest
> MSR_IA32_FEATURE_CONTROL on all vcpus should be zero after S3 resume.

Thank you for the analysis and sorry about the noise.

Laszlo