On Mon, Sep 25, 2023 at 03:14:53AM -0400, EwanHai wrote:
> Date: Mon, 25 Sep 2023 03:14:53 -0400
> From: EwanHai <ewanhai-oc@xxxxxxxxxxx>
> Subject: [PATCH] target/i386/kvm: Refine VMX controls setting for backward
> compatibility
> X-Mailer: git-send-email 2.34.1
>
> Commit 4a910e1 ("target/i386: do not set unsupported VMX secondary
> execution controls") implemented a workaround for hosts that have
> specific CPUID features but do not support the corresponding VMX
> controls, e.g., hosts support RDSEED but do not support RDSEED-Exiting.
>
> In detail, commit 4a910e1 introduced a flag `has_msr_vmx_procbased_clts2`.
> If KVM has `MSR_IA32_VMX_PROCBASED_CTLS2` in its msr list, QEMU would
> use KVM's settings, avoiding any modifications to this MSR.
>
> However, this commit (4a910e1) didn’t account for cases in older Linux
s/didn’t/didn't/
> kernels(e.g., linux-4.19.90) where `MSR_IA32_VMX_PROCBASED_CTLS2` is
For this old kernel, it's better to add the brief lifecycle note (e.g.,
lts, EOL) to illustrate the value of considering such compatibility
fixes.
> in `kvm_feature_msrs`—obtained by ioctl(KVM_GET_MSR_FEATURE_INDEX_LIST),
s/—obtained/-obtained/
> but not in `kvm_msr_list`—obtained by ioctl(KVM_GET_MSR_INDEX_LIST).
s/—obtained/-obtained/
> As a result,it did not set the `has_msr_vmx_procbased_clts2` flag based
> on `kvm_msr_list` alone, even though KVM maintains the value of this MSR.
>
> This patch supplements the above logic, ensuring that
> `has_msr_vmx_procbased_clts2` is correctly set by checking both MSR
> lists, thus maintaining compatibility with older kernels.
>
> Signed-off-by: EwanHai <ewanhai-oc@xxxxxxxxxxx>
> ---
> target/i386/kvm/kvm.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index af101fcdf6..6299284de4 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -2343,6 +2343,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;
> @@ -2377,6 +2378,11 @@ static int kvm_get_supported_feature_msrs(KVMState *s)
> return ret;
> }
It's worth adding a comment here to indicate that this is a
compatibility fix.
-Zhao
>
> + 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;
> + }
> + }
> return 0;
> }
>
> --
> 2.34.1
>
