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

[Laszlo Ersek <lersek@redhat.com>]

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.

I checked kvm/next (currently at
196f20ca52e8c7281932663c348fa54b82d03914), and vmx_vcpu_reset() does not
seem to zero vmx->msr_ia32_feature_control.

Now, what I absolutely can't tell you is whether this zeroing should
happen regardless of "init_event", or just for a specific value of
"init_event". Whenever I look at "init_event", I have to track it down
to the commit that added it, then locate all the commits that fixed it,
then guess whether the SDM language "logical processor reset" implies a
specific "init_event" value or not. So, I have no idea about "init_event".

Either way, why it is important to me: the edk2 modules that are built
into OVMF use loads of INIT-SIPI-SIPI sequences, for implementing
multiprocessing in the firmware (regardless of the feature at hand). For
example, EFI_MP_SERVICES_PROTOCOL.StartupAllAps() boots all APs with a
programmer-specified routine, after which the APs are HLTed. (Until the
next such call.) INIT-SIPI-SIPI is used by the implementation for
booting the APs.

So I need to know whether those INIT-SIPI-SIPI sequences are supposed to
clear the MSR -- in other words, whether I have to write patches that
explicitly sustain the MSR across these IPIs.

If the answer is "yes", then I would like to ask for a KVM patch that
zeroes vmx->msr_ia32_feature_control "under the right circumstances".

I can only attempt to re-program the MSR in all the necessary places in
edk2 if I can trust that KVM will clear the MSR (visibly to the "rdmsr"
command in the guest) as long as I miss at least one location in edk2.

Thanks!
Laszlo