Hi Eduardo, Planning to make couple of changes after testing and review. Let me know if I missed something. Will wait for your feedback before the next revision. > -----Original Message----- > From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] > On Behalf Of Babu Moger > Sent: Friday, June 8, 2018 5:56 PM > To: mst@xxxxxxxxxx; marcel.apfelbaum@xxxxxxxxx; pbonzini@xxxxxxxxxx; > rth@xxxxxxxxxxx; ehabkost@xxxxxxxxxx; mtosatti@xxxxxxxxxx > Cc: qemu-devel@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Moger, Babu > <Babu.Moger@xxxxxxx>; kash@xxxxxxxxxxxxxx; geoff@xxxxxxxxxxxxxxx > Subject: [PATCH v13 2/5] i386: Introduce auto_topoext bit to manage > topoext > > Introduce the auto_topoext bit to to control topoext feature. > > Also add new field auto_topoext(in X86CPUDefinition). This will > be used to enable topoext on newer CPU models where topoext can > be supported. > > Signed-off-by: Babu Moger <babu.moger@xxxxxxx> > --- > include/hw/i386/pc.h | 4 ++++ > target/i386/cpu.c | 12 ++++++++++++ > target/i386/cpu.h | 5 +++++ > 3 files changed, 21 insertions(+) > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 04d1f8c..cc30ec3 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -303,6 +303,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, > uint64_t *); > .driver = TYPE_X86_CPU,\ > .property = "legacy-cache",\ > .value = "on",\ > + },{\ > + .driver = TYPE_X86_CPU,\ > + .property = "auto-topoext",\ > + .value = "off",\ > }, We don't need this. We are already setting this false in x86_cpu_properties. But we need to set it "on" for EPYC on PC_COMPAT_3_0 whenever we define it. That will be a separate patch. Correct? > > #define PC_COMPAT_2_11 \ > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 86fb1a4..d3411ed 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -1283,6 +1283,8 @@ struct X86CPUDefinition { > FeatureWordArray features; > const char *model_id; > CPUCaches *cache_info; > + /* Set it if topoext can be enabled in CPU models */ > + int auto_topoext; > }; > > static CPUCaches epyc_cache_info = { > @@ -3517,6 +3519,9 @@ static void x86_cpu_load_def(X86CPU *cpu, > X86CPUDefinition *def, Error **errp) > /* legacy-cache defaults to 'off' if CPU model provides cache info */ > cpu->legacy_cache = !def->cache_info; > > + /* Set auto_topoext if both machine property and CPU model supports it > */ > + cpu->auto_topoext = cpu->auto_topoext & def->auto_topoext; This could be more appropriate like this. cpu->auto_topoext = cpu->auto_topoext && def->auto_topoext; > + > /* Special cases not set in the X86CPUDefinition structs: */ > /* TODO: in-kernel irqchip for hvf */ > if (kvm_enabled()) { > @@ -5382,6 +5387,13 @@ static Property x86_cpu_properties[] = { > DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true), > > /* > + * auto-topoext property will be used to enable topoext feature. > + * This will be disabled on all the older CPU models. Will be > + * enabled on newer CPU modeles which can support topology extention. > + */ > + DEFINE_PROP_BOOL("auto-topoext", X86CPU, auto_topoext, false), > + > + /* > * From "Requirements for Implementing the Microsoft > * Hypervisor Interface": > * https://docs.microsoft.com/en-us/virtualization/hyper-v-on- > windows/reference/tlfs > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 89c82be..8783d36 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -1409,6 +1409,11 @@ struct X86CPU { > */ > bool legacy_cache; > > + /* Compatibility bits to enable topoext feature on all newer machines > + * Disabled on older machines. Enabled on newer CPU models > + */ > + bool auto_topoext; > + > /* Compatibility bits for old machine types: */ > bool enable_cpuid_0xb; > > -- > 1.8.3.1
