RFE: OvmfPkg: Set MSR_IA32_FEATURE_CONTROL by following QEMU fw_cfg file

[ghost]

Hi,

A recent QEMU patch series [1] adds a new fw_cfg file "etc/msr_feature_control" to publish the value that the guest firmware is advised to write to MSR_IA32_FEATURE_CONTROL (MSR 0x3a). This should be supported by OVMF as well [2].

• What is it good for? For some hardware features, e.g. Intel VMX and local MCE, OS usually expects certain bits in MSR_IA32_FEATURE_CONTROL are set by firmware. Using corresponding features (e.g access VMX MSRs) without setting those bits can result in #GP. Instead of adding code to guest firmware everytime a new feature is added to QEMU, we can use a QEMU fw_cfg file to publish the advised value of MSR_IA32_FEATURE_CONTROL.
• Path name and internal format Path: etc/msr_feature_control Format: one unsigned 64-bit integer which is the value that is advised to set to MSR_IA32_FEATURE_CONTROL.
• Is MSR_IA32_FEATURE_CONTROL part of chipset state? Yes, firmware should set MSR_IA32_FEATURE_CONTROL at every boot. Firmware is expected to set MSR_IA32_FEATURE_CONTROL to the value in etc/msr_feature_control at S3 resume.
• How to verify?
  1. Apply this patchset (https://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg06266.html) againts QEMU commit 6f1d2d1 and build QEMU with KVM support (e.g. ./configure --enable-kvm).
  2. Build KVM commit c45dcc7 or newer.
  3. Launch a Linux guest in QEMU on Linux by command qemu-system-x86_64 -enable-kvm -smp 4 -cpu qemu64,+vmx -hda PATH_TO_GUEST_IMAGE -m 512
  4. In Linux guest, use msr-tools (https://01.org/msr-tools) to check MSR_IA32_FEATURE_CONTROL by command rdmsr -a 0x3a It's expected to get 4 lines of '5', which means MSR_IA32_FEATURE_CONTROL on all CPUs are set to 5.

[lersek]

Thanks, the description looks good. A few more questions:

• the uint64_t value is in little endian, right?
• from the SeaBIOS patch set, it seems to me that the MSR has to be written on all APs separately. If that's the case (can you please confirm?), that will significantly complicate the implementation, because then we'll have to modify UefiCpuPkg modules too (CpuDxe minimally), and it could (likely) require changes to the SMM stuff as well (CpuS3DataDxe e.g.).

[ghost]

• Yes, the value is in little endian.
• Yes, MSR_IA32_FEATURE_CONTROL must be written on BSP and all APs.

[lersek]

Design discussion started at http://thread.gmane.org/gmane.comp.bios.edk2.devel/14049

[lersek]

@hzzhan9: can you recommend a simple QEMU command line, and give guest kernel (or userspace) instructions for verifying the feature?

Thanks!

[lersek]

The S3 requirement needs clarification: http://www.seabios.org/pipermail/seabios/2016-July/010761.html

[lersek]

Apparently S3 is required.

I'm also attaching a minimal kernel module that can be used for reading any MSR on any CPU, for testing this feature. Just create any empty git repo with git init, then apply the patch with git am. The commit message contains the build and run instructions.

... Sigh, github is stupid, it doesn't allow me to attach *.patch. I have to zip it just to make github happy. Sorry. tester.zip

[lersek]

Posted series: [PATCH 0/4] OvmfPkg: program MSR_IA32_FEATURE_CONTROL from fw_cfg on all processors http://thread.gmane.org/gmane.comp.bios.edk2.devel/14214

[lersek]

Posted series [PATCH v2 0/5] OvmfPkg: program MSR_IA32_FEATURE_CONTROL from fw_cfg on all processors http://thread.gmane.org/gmane.comp.bios.edk2.devel/14471

[lersek]

Posted series [PATCH v3 0/5] OvmfPkg: program MSR_IA32_FEATURE_CONTROL from fw_cfg on all processors http://thread.gmane.org/gmane.comp.bios.edk2.devel/14529

[lersek]

(Commit range 51773d49de84..dbab994991c7.)

[lersek]

This (closed) item has been manually migrated to https://tianocore.acgmultimedia.com/show_bug.cgi?id=86