On 6/25/24 05:49, Zhao Liu wrote:
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 7ad8072748..a7c6c5b2d0 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2386,6 +2386,7 @@ void kvm_arch_do_init_vcpu(X86CPU *cpu)
  static int kvm_get_supported_feature_msrs(KVMState *s)
  {
      int ret = 0;
+    int i;

      if (kvm_feature_msrs != NULL) {
          return 0;
@@ -2420,6 +2421,20 @@ static int kvm_get_supported_feature_msrs(KVMState *s)
          return ret;
      }

+   /*
+    * Compatibility fix:
+    * Older Linux kernels (4.17~5.2) report MSR_IA32_VMX_PROCBASED_CTLS2
+    * in KVM_GET_MSR_FEATURE_INDEX_LIST but not in KVM_GET_MSR_INDEX_LIST.
+    * This leads to an issue in older kernel versions where QEMU,
+    * through the KVM_GET_MSR_INDEX_LIST check, assumes the kernel
+    * doesn't maintain MSR_IA32_VMX_PROCBASED_CTLS2, resulting in
+    * incorrect settings by QEMU for this MSR.
+    */
+    for (i = 0; i < kvm_feature_msrs->nmsrs; i++) {
nit: `i` could be declared here,

for (int i = 0; i < kvm_feature_msrs->nmsrs; i++) {
do I need to send a v4 version patch,to do this fix?
+        if (kvm_feature_msrs->indices[i] == MSR_IA32_VMX_PROCBASED_CTLS2) {
+            has_msr_vmx_procbased_ctls2 = true;
+        }
+    }
      return 0;
  }

--
2.34.1

Since the minimum KVM version supported for i386 is v4.5 (docs/system/
target-i386.rst), this fix makes sense, so for this patch,

Reviewed-by: Zhao Liu <zhao1....@intel.com>

Additionally, has_msr_vmx_vmfunc has the similar compat issue. I think
it deserves a fix, too.

-Zhao
Thanks for your reply. In fact, I've tried to process has_msr_vmx_vmfunc in the same way as has_msr_vmx_procbased_ctls in this patch, but when I tested on Linux kernel
4.19.67, I encountered an "error: failed to set MSR 0x491 to 0x***".

This issue is due to Linux kernel commit 27c42a1bb ("KVM: nVMX: Enable VMFUNC for the L1 hypervisor", 2017-08-03) exposing VMFUNC to the QEMU guest without corresponding VMFUNC MSR modification code, leading to an error when QEMU attempts to set the VMFUNC MSR. This bug affects kernels from 4.14 to 5.2, with a fix introduced in 5.3 by Paolo (e8a70bd4e "KVM: nVMX: allow setting the VMFUNC controls MSR", 2019-07-02).

So the fix for has_msr_vmx_vmfunc is clearly different from has_msr_vmx_procbased_ctls2. However, due to the different kernel support situations, I have not yet come up with a suitable way to handle the compatibility of has_msr_vmx_procbased_ctls2 across different kernel versions.

Therefore, should we consider only fixing has_msr_vmx_procbased_ctls2 this time and addressing
has_msr_vmx_vmfunc in a future patch when the timing is more appropriate?


Reply via email to