> -----Original Message----- > From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] > On Behalf Of Eduardo Habkost > Sent: Monday, June 11, 2018 4:10 PM > To: Moger, Babu <Babu.Moger@xxxxxxx> > Cc: mst@xxxxxxxxxx; marcel.apfelbaum@xxxxxxxxx; pbonzini@xxxxxxxxxx; > rth@xxxxxxxxxxx; mtosatti@xxxxxxxxxx; qemu-devel@xxxxxxxxxx; > kvm@xxxxxxxxxxxxxxx; kash@xxxxxxxxxxxxxx; geoff@xxxxxxxxxxxxxxx; Jiri > Denemark <jdenemar@xxxxxxxxxx> > 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
