Re: [PATCH hci-8.0/hci-8.1] i386: hide la57 at fresh start for named CPU models too #VSTOR-153243

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] i386: la57 hidden on fresh start for named models is not carried to migration destination
- [Medium] i386: x-no-la57-on-start now makes named-model CPUID depend on start vs incoming
--

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

i386: hide la57 at fresh start for named CPU models too #VSTOR-153243

This extends x-no-la57-on-start so that la57 is hidden on a fresh start for
every CPU model on the affected Virtuozzo machine types, not only for "-cpu
host". The bit stays visible if la57=on is given explicitly, and on incoming
migration.

> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8d2ce279a725..e22fafeaef09 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -9116,8 +9116,7 @@ static bool x86_cpu_hide_la57(X86CPU *cpu)
>  {
>      CPUX86State *env = &cpu->env;
>  
> -    if (!cpu->no_la57_on_start || !X86_CPU_GET_CLASS(cpu)->max_features ||
> -        !kvm_enabled() ||
> +    if (!cpu->no_la57_on_start || !kvm_enabled() ||
>          !(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_LA57) ||
>          (env->user_features[FEAT_7_0_ECX] & CPUID_7_0_ECX_LA57)) {
>          return false;

[Severity: Medium]
Can the guest-visible CPUID differ between a fresh start and an incoming
migration for named models now?

With the max_features check removed, x86_cpu_hide_la57() applies to every
model that carries la57 (Icelake-Server, SapphireRapids, GraniteRapids,
EPYC-Genoa, EPYC-Turin) when la57=on is not given explicitly.  The result
still ends with:

    return !runstate_check(RUN_STATE_INMIGRATE);

On a fresh start this returns true and x86_cpu_realizefn() strips the bit:

    if (x86_cpu_hide_la57(cpu)) {
        mark_unavailable_features(cpu, FEAT_7_0_ECX, CPUID_7_0_ECX_LA57, NULL);
    }

On the destination, the runstate is INMIGRATE, so the function returns
false and the model's default la57 is kept.  The same command line then
gives a different CPUID depending on how the process was started.

Nothing in QEMU records that la57 was hidden on the source.
x-no-la57-on-start is a plain bool property that pc_vz_10_0_compat sets
statically with { TYPE_X86_CPU, "x-no-la57-on-start", "on" }.  So a VM
started fresh with la57 hidden could see la57 exposed after live migration,
save/restore or snapshot restore, unless the management layer writes
la57=off on the destination command line.

The commit message says "libvirt records the filtered la57 as disabled in
the live definition for check='partial'".  Is the same true for named
models used with check='none' or check='full'?  The commit message does not
say, and the code does not show it.

The hotplug branch compares against first_cpu, so could the same mismatch
appear there too?

Also, existing VMs that deliberately use an la57-capable Intel named model
will change their guest-visible CPUID on cold start after the QEMU update,
without a new machine type.  The commit message acknowledges this for the
Intel models.

Finally, the patch notes say this has not yet been run on an LA57 host.
Has the migration, save and snapshot behaviour for named models been
checked?

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