Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RFE: OvmfPkg: Set MSR_IA32_FEATURE_CONTROL by following QEMU fw_cfg file #97

Closed
ghost opened this issue Jun 22, 2016 · 11 comments
Closed

RFE: OvmfPkg: Set MSR_IA32_FEATURE_CONTROL by following QEMU fw_cfg file #97

ghost opened this issue Jun 22, 2016 · 11 comments

Comments

@ghost
Copy link

ghost commented Jun 22, 2016 •
edited by ghost
Loading

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
Copy link
Member

lersek commented Jun 22, 2016

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
Copy link
Author

ghost commented Jun 23, 2016

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

@lersek
Copy link
Member

lersek commented Jul 5, 2016

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

@lersek
Copy link
Member

lersek commented Jul 5, 2016

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

Thanks!

@lersek
Copy link
Member

lersek commented Jul 5, 2016

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

@lersek
Copy link
Member

lersek commented Jul 6, 2016

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 added a commit to lersek/edk2 that referenced this issue Jul 7, 2016
@lersek
OvmfPkg/PlatformPei: program MSR_IA32_FEATURE_CONTROL from fw_cfg
a7960ad 
Under certain circumstances, QEMU exposes the "etc/msr_feature_control"
fw_cfg file, with a 64-bit little endian value. The firmware is supposed
to write this value to MSR_IA32_FEATURE_CONTROL (0x3a), on all processors,
on the normal and the S3 resume boot paths.

Utilize EFI_PEI_MPSERVICES_PPI to implement this feature.

Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Fixes: tianocore#97
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
@lersek
Copy link
Member

lersek commented Jul 7, 2016

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 added a commit to lersek/edk2 that referenced this issue Jul 12, 2016
@lersek
OvmfPkg/PlatformPei: program MSR_IA32_FEATURE_CONTROL from fw_cfg
c2badec 
Under certain circumstances, QEMU exposes the "etc/msr_feature_control"
fw_cfg file, with a 64-bit little endian value. The firmware is supposed
to write this value to MSR_IA32_FEATURE_CONTROL (0x3a), on all processors,
on the normal and the S3 resume boot paths.

Utilize EFI_PEI_MPSERVICES_PPI to implement this feature.

Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Fixes: tianocore#97
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
@lersek
Copy link
Member

lersek commented Jul 13, 2016

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 added a commit to lersek/edk2 that referenced this issue Jul 13, 2016
@lersek
OvmfPkg/PlatformPei: program MSR_IA32_FEATURE_CONTROL from fw_cfg
2175ab8 
Under certain circumstances, QEMU exposes the "etc/msr_feature_control"
fw_cfg file, with a 64-bit little endian value. The firmware is supposed
to write this value to MSR_IA32_FEATURE_CONTROL (0x3a), on all processors,
on the normal and the S3 resume boot paths.

Utilize EFI_PEI_MPSERVICES_PPI to implement this feature.

Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Fixes: tianocore#97
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
@lersek
Copy link
Member

lersek commented Jul 13, 2016

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 lersek closed this as completed in dbab994 Jul 15, 2016
@lersek
Copy link
Member

lersek commented Jul 15, 2016

(Commit range 51773d4..dbab994.)

@lersek
Copy link
Member

lersek commented Jul 21, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@lersek