Re: [PATCH v3 hci-8.0/hci-8.1 6/6] i386: keep SVM leaf features without svm on vz types up to vz9.2.0 #PSBM-144630

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] i386: x86_cpu_compat_kept_features() keeps SVM leaf bits after svm is host-filtered
--

commit cc8f40cd79cdf926fae1c7d33e11aa26bbf6654b
Author: Denis V. Lunev <den@openvz.org>

i386: keep SVM leaf features without svm on vz types up to vz9.2.0 #PSBM-144630

This adds an x-svm-features-compat CPU property and enables it in
pc_vz_9_2_compat. Under KVM, requested SVM leaf features are kept when svm
is off, both against the feature dependency on svm and against the host
feature filter.

> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c

[ ... ]

> @@ -9138,6 +9145,10 @@ static uint64_t x86_cpu_compat_kept_features(X86CPU *cpu, FeatureWord w)
>          if (cpu->v_vmsave_vmload_compat && IS_AMD_CPU(env)) {
>              kept = CPUID_SVM_V_VMSAVE_VMLOAD;
>          }
> +        if (cpu->svm_features_compat &&
> +            !(env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM)) {
> +            kept = ~0ULL;
> +        }
>          break;
>      default:
>          break;

[Severity: Low]
Can this condition tell "svm=off was requested" apart from "svm=on was
requested but the host filtered it"?

x86_cpu_compat_kept_features() reads env->features[FEAT_8000_0001_ECX] when
it is called from x86_cpu_filter_features():

    uint64_t unavailable_features = requested_features & ~host_feat &
        ~x86_cpu_compat_kept_features(cpu, w);
    mark_unavailable_features(cpu, w, unavailable_features, prefix);

In the FeatureWord enum, FEAT_8000_0001_ECX comes before FEAT_SVM, so the
loop filters FEAT_8000_0001_ECX first. If the user requested svm=on, for
example through a host-model CPU, and the host lacks nested SVM, for
example kvm_amd nested=0, mark_unavailable_features() clears
CPUID_EXT3_SVM first.

When the loop then reaches FEAT_SVM, the check above sees the svm bit
already cleared and sets kept = ~0ULL. Every requested FEAT_SVM bit that
the host doesn't support is then neither cleared nor reported. This goes
beyond the stated "svm off" scope in the commit message.

The guest-visible CPUID looks unchanged, because cpu_x86_cpuid() returns
zeros for leaf 0x8000000a without svm. The svm bit is still reported as
unavailable, so enforce still fails. The xlevel adjustment happens in
x86_cpu_expand_features() before filtering. What does change is that npt,
nrip-save and the other SVM leaf features are no longer listed as filtered,
and env->features[FEAT_SVM] keeps unsupported bits.

Would it be better to record whether svm was requested off before
x86_cpu_filter_features() runs, or to check env->user_features, instead of
reading the already-filtered env->features value?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261029122527.365104-1-den@openvz.org?part=6