| Message ID | 20200212081328.7385-3-tao3.xu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add extra information to versioned CPU models | expand |
On Wed, Feb 12, 2020 at 04:13:26PM +0800, Tao Xu wrote: > Add new version of Snowridge, Denverton, Opteron_G3, EPYC, and Dhyana > CPU model to remove MONITOR/MWAIT feature. > > After QEMU/KVM use "-overcommit cpu-pm=on" to expose MONITOR/MWAIT > (commit id 6f131f13e68d648a8e4f083c667ab1acd88ce4cd), the MONITOR/MWAIT > feature in these CPU model is unused. > > Signed-off-by: Tao Xu <tao3.xu@intel.com> What exactly is the problem you are trying to fix? No CPU model will ever have monitor=on set by default with KVM, because kvm_default_props has a monitor=off element. > --- > target/i386/cpu.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 848c992cd3..6905e4eabd 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -3731,6 +3731,14 @@ static X86CPUDefinition builtin_x86_defs[] = { > { /* end of list */ }, > }, > }, > + { > + .version = 3, > + .props = (PropValue[]) { > + /* mpx was already removed by -v2 above */ > + { "monitor", "off" }, > + { /* end of list */ }, > + }, > + }, > { /* end of list */ }, > }, > }, > @@ -3842,6 +3850,17 @@ static X86CPUDefinition builtin_x86_defs[] = { > CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM, > .xlevel = 0x80000008, > .model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)", > + .versions = (X86CPUVersionDefinition[]) { > + { .version = 1 }, > + { > + .version = 2, > + .props = (PropValue[]) { > + { "monitor", "off" }, > + { /* end of list */ }, > + }, > + }, > + { /* end of list */ }, > + }, > }, > { > .name = "Opteron_G4", > @@ -3966,6 +3985,14 @@ static X86CPUDefinition builtin_x86_defs[] = { > { /* end of list */ } > } > }, > + { > + .version = 3, > + .props = (PropValue[]) { > + /* ibpb was already enabled by -v2 above */ > + { "monitor", "off" }, > + { /* end of list */ }, > + }, > + }, > { /* end of list */ } > } > }, > @@ -4018,6 +4045,17 @@ static X86CPUDefinition builtin_x86_defs[] = { > .xlevel = 0x8000001E, > .model_id = "Hygon Dhyana Processor", > .cache_info = &epyc_cache_info, > + .versions = (X86CPUVersionDefinition[]) { > + { .version = 1 }, > + { > + .version = 2, > + .props = (PropValue[]) { > + { "monitor", "off" }, > + { /* end of list */ }, > + }, > + }, > + { /* end of list */ }, > + }, > }, > }; > > -- > 2.20.1 > >
On 2/29/2020 5:39 AM, Eduardo Habkost wrote: > On Wed, Feb 12, 2020 at 04:13:26PM +0800, Tao Xu wrote: >> Add new version of Snowridge, Denverton, Opteron_G3, EPYC, and Dhyana >> CPU model to uremove MONITOR/MWAIT featre. >> >> After QEMU/KVM use "-overcommit cpu-pm=on" to expose MONITOR/MWAIT >> (commit id 6f131f13e68d648a8e4f083c667ab1acd88ce4cd), the MONITOR/MWAIT >> feature in these CPU model is unused. >> >> Signed-off-by: Tao Xu <tao3.xu@intel.com> > > What exactly is the problem you are trying to fix? > > No CPU model will ever have monitor=on set by default with KVM, > because kvm_default_props has a monitor=off element. > Maybe it is not a fix. For example, when we boot a guest with Denverton cpu model, guest cannot detect MONITOR/MWAIT and boot with no warning, because of "monitor=off" by default. The MONITOR/MWAIT feature in these CPU model is unused,but no harm. I am wondering if we should remove it from existing CPU models.
On Mon, Mar 02, 2020 at 07:47:28PM +0800, Tao Xu wrote: > On 2/29/2020 5:39 AM, Eduardo Habkost wrote: > > On Wed, Feb 12, 2020 at 04:13:26PM +0800, Tao Xu wrote: > > > Add new version of Snowridge, Denverton, Opteron_G3, EPYC, and Dhyana > > > CPU model to uremove MONITOR/MWAIT featre. > > > > > > After QEMU/KVM use "-overcommit cpu-pm=on" to expose MONITOR/MWAIT > > > (commit id 6f131f13e68d648a8e4f083c667ab1acd88ce4cd), the MONITOR/MWAIT > > > feature in these CPU model is unused. > > > > > > Signed-off-by: Tao Xu <tao3.xu@intel.com> > > > > What exactly is the problem you are trying to fix? > > > > No CPU model will ever have monitor=on set by default with KVM, > > because kvm_default_props has a monitor=off element. > > > > Maybe it is not a fix. For example, when we boot a guest with Denverton > cpu model, guest cannot detect MONITOR/MWAIT and boot with no warning, > because of "monitor=off" by default. The MONITOR/MWAIT feature in these CPU > model is unused,but no harm. I am wondering if we should remove it from > existing CPU models. As monitor=off is on kvm_default_props, changing the CPU model table will only affect other accelerators (e.g. TCG, where MONITOR/MWAIT support is advertised as supported). We shouldn't be dictating policy for other accelerators just because KVM doesn't support it. Removing the feature on kvm_default_props is sufficient.
On 3/3/2020 1:19 AM, Eduardo Habkost wrote: > On Mon, Mar 02, 2020 at 07:47:28PM +0800, Tao Xu wrote: >> On 2/29/2020 5:39 AM, Eduardo Habkost wrote: >>> On Wed, Feb 12, 2020 at 04:13:26PM +0800, Tao Xu wrote: >>>> Add new version of Snowridge, Denverton, Opteron_G3, EPYC, and Dhyana >>>> CPU model to uremove MONITOR/MWAIT featre. >>>> >>>> After QEMU/KVM use "-overcommit cpu-pm=on" to expose MONITOR/MWAIT >>>> (commit id 6f131f13e68d648a8e4f083c667ab1acd88ce4cd), the MONITOR/MWAIT >>>> feature in these CPU model is unused. >>>> >>>> Signed-off-by: Tao Xu <tao3.xu@intel.com> >>> >>> What exactly is the problem you are trying to fix? >>> >>> No CPU model will ever have monitor=on set by default with KVM, >>> because kvm_default_props has a monitor=off element. >>> >> >> Maybe it is not a fix. For example, when we boot a guest with Denverton >> cpu model, guest cannot detect MONITOR/MWAIT and boot with no warning, >> because of "monitor=off" by default. The MONITOR/MWAIT feature in these CPU >> model is unused,but no harm. I am wondering if we should remove it from >> existing CPU models. > > As monitor=off is on kvm_default_props, changing the CPU model > table will only affect other accelerators (e.g. TCG, where > MONITOR/MWAIT support is advertised as supported). > > We shouldn't be dictating policy for other accelerators just > because KVM doesn't support it. Removing the feature on > kvm_default_props is sufficient. > I understand, thanks.
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 848c992cd3..6905e4eabd 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -3731,6 +3731,14 @@ static X86CPUDefinition builtin_x86_defs[] = { { /* end of list */ }, }, }, + { + .version = 3, + .props = (PropValue[]) { + /* mpx was already removed by -v2 above */ + { "monitor", "off" }, + { /* end of list */ }, + }, + }, { /* end of list */ }, }, }, @@ -3842,6 +3850,17 @@ static X86CPUDefinition builtin_x86_defs[] = { CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM, .xlevel = 0x80000008, .model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)", + .versions = (X86CPUVersionDefinition[]) { + { .version = 1 }, + { + .version = 2, + .props = (PropValue[]) { + { "monitor", "off" }, + { /* end of list */ }, + }, + }, + { /* end of list */ }, + }, }, { .name = "Opteron_G4", @@ -3966,6 +3985,14 @@ static X86CPUDefinition builtin_x86_defs[] = { { /* end of list */ } } }, + { + .version = 3, + .props = (PropValue[]) { + /* ibpb was already enabled by -v2 above */ + { "monitor", "off" }, + { /* end of list */ }, + }, + }, { /* end of list */ } } }, @@ -4018,6 +4045,17 @@ static X86CPUDefinition builtin_x86_defs[] = { .xlevel = 0x8000001E, .model_id = "Hygon Dhyana Processor", .cache_info = &epyc_cache_info, + .versions = (X86CPUVersionDefinition[]) { + { .version = 1 }, + { + .version = 2, + .props = (PropValue[]) { + { "monitor", "off" }, + { /* end of list */ }, + }, + }, + { /* end of list */ }, + }, }, };
Add new version of Snowridge, Denverton, Opteron_G3, EPYC, and Dhyana CPU model to remove MONITOR/MWAIT feature. After QEMU/KVM use "-overcommit cpu-pm=on" to expose MONITOR/MWAIT (commit id 6f131f13e68d648a8e4f083c667ab1acd88ce4cd), the MONITOR/MWAIT feature in these CPU model is unused. Signed-off-by: Tao Xu <tao3.xu@intel.com> --- target/i386/cpu.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)