Dear Xiaoyao and Maintainers,
Are there any new comments regarding this patch?
On 2/22/24 22:13, Ewan Hai wrote:
On 2/20/24 06:07, Ewan Hai wrote:
On 2/20/24 03:32, Xiaoyao Li wrote:
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 11b8177eff..c8f6c0b531 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2296,6 +2296,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;
@@ -2330,6 +2331,19 @@ static int
kvm_get_supported_feature_msrs(KVMState *s)
return ret;
}
+ /*
+ * Compatibility fix:
+ * Older Linux kernels(<5.3) include the
MSR_IA32_VMX_PROCBASED_CTLS2
we can be more accurate, that kernel version 4.17 to 5.2, reports
MSR_IA32_VMX_PROCBASED_CTLS2 in KVM_GET_MSR_FEATURE_INDEX_LIST but not
KVM_GET_MSR_INDEX_LIST.
Yeah, I'll add this more precise comment to the next patch.
+ * only in feature msr list, but not in regular msr list. This
lead to
+ * an issue in older kernel versions where QEMU, through the
regular
+ * MSR list check, assumes the kernel doesn't maintain this msr,
+ * resulting in incorrect settings by QEMU for this msr.
+ */
+ for (i = 0; i < kvm_feature_msrs->nmsrs; i++) {
+ if (kvm_feature_msrs->indices[i] ==
MSR_IA32_VMX_PROCBASED_CTLS2) {
+ has_msr_vmx_procbased_ctls2 = true;
+ }
+ }
I'm wondering should we move all the initialization of has_msr_*, that
associated with feature MSRs, to here. e.g., has_msr_arch_capabs,
has_msr_vmx_vmfunc,...
I believe this is a more elegant way to fix the issue, which will be
reflected in my next patch.
When attempting to move the detection logic for feature MSRs (currently
including VMX_VMFUNC, UCODE_REV, ARCH_CAPABILITIES,
PROCBASED_CTLS2) from kvm_get_supported_msrs to
kvm_get_supported_feature_msrs in the current QEMU,
I encountered an "error: failed to set MSR 0x491 to 0x***" on kernel
4.19.67.
This issue is due to 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 proactively tries 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)).
Therefore, even if we were to move all feature MSRs to
kvm_get_supported_feature_msrs,VMX_VMFUNC could not be moved due to
the need to maintain compatibility with different kernel versions. This
exception makes our move less elegant. Hence, I am wondering whether we
need to move all feature MSRs to kvm_get_supported_feature_msrs. Perhaps
we just need to simply move PROCBASED_CTLS2 to fix the "failed set
0x48b ..."
type of bugs, and add a comment about it?
