> -----Original Message-----
> From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org]
> On Behalf Of Eduardo Habkost
> Sent: Monday, June 11, 2018 4:10 PM
> To: Moger, Babu <babu.mo...@amd.com>
> Cc: m...@redhat.com; marcel.apfelb...@gmail.com; pbonz...@redhat.com;
> r...@twiddle.net; mtosa...@redhat.com; qemu-devel@nongnu.org;
> k...@vger.kernel.org; k...@tripleback.net; ge...@hostfission.com; Jiri
> Denemark <jdene...@redhat.com>
> Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC
> CPU
>
> On Mon, Jun 11, 2018 at 05:50:30PM -0300, Eduardo Habkost wrote:
> [...]
> > > + /* TOPOEXT feature requires 0x8000001E */
> > > + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT)
> {
> > > + x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel,
> 0x8000001E);
> > > + }
> >
> > I suggest moving this hunk to a separate patch. I'm not 100%
> > sure yet if this will require compat_props code to disable
> > auto-xlevel-increase on older machine-types.
>
> The problem here is that:
> $QEMU -machine pc-i440fx-1.3 -cpu Opteron_G4,+topoext
> currently results in xlevel=0x8000001A, since QEMU 1.3.
>
> (The same applies to all machine-types between 1.3 and 2.12)
>
> I was hoping that we could declare topoext as non-migration-safe,
> but I believe libvirt will already include "topoext" when using
> "host-model" if the host CPU supports TOPOEXT. Jiri, can you
> confirm that?
>
> We can address that with a "x-topoext-auto-xlevel" property, set
> to true on all CPU models by default, and disabled by
> PC_COMPAT_2_12.
>
> The code would become:
>
> if (cpu->topoext_auto_xlevel && env->features[FEAT_8000_0001_ECX] &
> CPUID_EXT3_TOPOEXT) {
> x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001E);
> }
>
> Or, we could simply declare that "-cpu Opteron_G4,+topoext" will
> never increase xlevel automatically (on any machine-type), and
> change the code above to:
>
> if (cpu->auto_topoext && env->features[FEAT_8000_0001_ECX] &
> CPUID_EXT3_TOPOEXT) {
> x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001E);
> }
I was going to do this. But there is one problem. We don't set the
CPUID_EXT3_TOPOEXT in CPU model table. So this won't work.
One more thing I noticed that feature setting should happen much before
x86_cpu_realizefn.
Couple of options.
First option.
1. Set both feature and xlevel here(in x86_cpu_expand_features).
if (cpu->x_auto_topoext {
env->features[FEAT_8000_0001_ECX] |= CPUID_EXT3_TOPOEXT;
x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001E);
}
2. And remove feature setting in x86_cpu_realizefn.
Or
Second option
1.Set the feature bit in CPU model table.
2. Set xlevel in x86_cpu_expand_features using cpu->x_auto_topoext
3. And remove feature setting in x86_cpu_realizefn.
I prefer the second option.
>
> --
> Eduardo
