[PATCH] target/i386: Remove monitor from some CPU model

Tao Xu posted 1 patch 6 years, 2 months ago
Test asan passed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191115083345.22638-1-tao3.xu@intel.com
Maintainers: Richard Henderson <rth@twiddle.net>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>
target/i386/cpu.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
[PATCH] target/i386: Remove monitor from some CPU model
Posted by Tao Xu 6 years, 2 months ago
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 | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a624163ac2..7c5f1e8fe0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2770,6 +2770,19 @@ static X86CPUDefinition builtin_x86_defs[] = {
             MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY,
         .xlevel = 0x80000008,
         .model_id = "Intel Atom Processor (Denverton)",
+        .versions = (X86CPUVersionDefinition[]) {
+            { .version = 1 },
+            {
+                .version = 2,
+                .props = (PropValue[]) {
+                    { "monitor", "off" },
+                    { "model-id",
+                      "Intel Atom Processor (Denverton, no MONITOR)" },
+                    { /* end of list */ },
+                },
+            },
+            { /* end of list */ },
+        },
     },
     {
         .name = "Snowridge",
@@ -2850,6 +2863,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
                     { /* end of list */ },
                 },
             },
+            {
+                .version = 3,
+                .props = (PropValue[]) {
+                    /* mpx was already removed by -v2 above */
+                    { "monitor", "off" },
+                    { "model-id",
+                      "Intel Atom Processor (Snowridge, no MPX, no MONITOR)" },
+                    { /* end of list */ },
+                },
+            },
             { /* end of list */ },
         },
     },
@@ -2961,6 +2984,19 @@ 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" },
+                    { "model-id",
+                      "AMD Opteron 23xx (Gen 3 Class Opteron, no MONITOR)" },
+                    { /* end of list */ },
+                },
+            },
+            { /* end of list */ },
+        },
     },
     {
         .name = "Opteron_G4",
@@ -3085,6 +3121,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
                     { /* end of list */ }
                 }
             },
+            {
+                .version = 3,
+                .props = (PropValue[]) {
+                    /* ibpb was already enabled by -v2 above */
+                    { "monitor", "off" },
+                    { "model-id",
+                      "AMD EPYC Processor (with IBPB, no MONITOR)" },
+                    { /* end of list */ },
+                },
+            },
             { /* end of list */ }
         }
     },
@@ -3137,6 +3183,18 @@ 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" },
+                    { "model-id", "Hygon Dhyana Processor (no MONITOR)" },
+                    { /* end of list */ },
+                },
+            },
+            { /* end of list */ },
+        },
     },
 };
 
-- 
2.20.1


Re: [PATCH] target/i386: Remove monitor from some CPU model
Posted by Eduardo Habkost 6 years, 2 months ago
On Fri, Nov 15, 2019 at 04:33:45PM +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>
> ---
>  target/i386/cpu.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a624163ac2..7c5f1e8fe0 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2770,6 +2770,19 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY,
>          .xlevel = 0x80000008,
>          .model_id = "Intel Atom Processor (Denverton)",
> +        .versions = (X86CPUVersionDefinition[]) {
> +            { .version = 1 },
> +            {
> +                .version = 2,
> +                .props = (PropValue[]) {
> +                    { "monitor", "off" },
> +                    { "model-id",
> +                      "Intel Atom Processor (Denverton, no MONITOR)" },

We never changed model-id when adding/removing features in
machine-type code, and I don't see why we should start doing that
now.  This info might be helpful on "-cpu help", but probably
confusing for people looking at /proc/cpuinfo inside the VM.

If you think it is important to add extra info to "-cpu help", I
suggest you send a patch adding a separate field instead of
changing model ID on CPUID.

> [...]

-- 
Eduardo


Re: [PATCH] target/i386: Remove monitor from some CPU model
Posted by Tao Xu 6 years, 2 months ago
On 11/19/2019 6:10 AM, Eduardo Habkost wrote:
> On Fri, Nov 15, 2019 at 04:33:45PM +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>
>> ---
>>   target/i386/cpu.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 58 insertions(+)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index a624163ac2..7c5f1e8fe0 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -2770,6 +2770,19 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>               MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY,
>>           .xlevel = 0x80000008,
>>           .model_id = "Intel Atom Processor (Denverton)",
>> +        .versions = (X86CPUVersionDefinition[]) {
>> +            { .version = 1 },
>> +            {
>> +                .version = 2,
>> +                .props = (PropValue[]) {
>> +                    { "monitor", "off" },
>> +                    { "model-id",
>> +                      "Intel Atom Processor (Denverton, no MONITOR)" },
> 
> We never changed model-id when adding/removing features in
> machine-type code, and I don't see why we should start doing that
> now.  This info might be helpful on "-cpu help", but probably
> confusing for people looking at /proc/cpuinfo inside the VM.
> 
> If you think it is important to add extra info to "-cpu help", I
> suggest you send a patch adding a separate field instead of
> changing model ID on CPUID.
> 

OK I will add a new info to do this.

Re: [PATCH] target/i386: Remove monitor from some CPU model
Posted by Tao Xu 6 years, 2 months ago
I am so forry for sending this old version patch by mistake.

Please ignore this patch.

On 12/2/2019 2:28 PM, Xu, Tao3 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>
> ---
>   target/i386/cpu.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 58 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a624163ac2..7c5f1e8fe0 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2770,6 +2770,19 @@ static X86CPUDefinition builtin_x86_defs[] = {
>               MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY,
>           .xlevel = 0x80000008,
>           .model_id = "Intel Atom Processor (Denverton)",
> +        .versions = (X86CPUVersionDefinition[]) {
> +            { .version = 1 },
> +            {
> +                .version = 2,
> +                .props = (PropValue[]) {
> +                    { "monitor", "off" },
> +                    { "model-id",
> +                      "Intel Atom Processor (Denverton, no MONITOR)" },
> +                    { /* end of list */ },
> +                },
> +            },
> +            { /* end of list */ },
> +        },
>       },
>       {
>           .name = "Snowridge",
> @@ -2850,6 +2863,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
>                       { /* end of list */ },
>                   },
>               },
> +            {
> +                .version = 3,
> +                .props = (PropValue[]) {
> +                    /* mpx was already removed by -v2 above */
> +                    { "monitor", "off" },
> +                    { "model-id",
> +                      "Intel Atom Processor (Snowridge, no MPX, no MONITOR)" },
> +                    { /* end of list */ },
> +                },
> +            },
>               { /* end of list */ },
>           },
>       },
> @@ -2961,6 +2984,19 @@ 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" },
> +                    { "model-id",
> +                      "AMD Opteron 23xx (Gen 3 Class Opteron, no MONITOR)" },
> +                    { /* end of list */ },
> +                },
> +            },
> +            { /* end of list */ },
> +        },
>       },
>       {
>           .name = "Opteron_G4",
> @@ -3085,6 +3121,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
>                       { /* end of list */ }
>                   }
>               },
> +            {
> +                .version = 3,
> +                .props = (PropValue[]) {
> +                    /* ibpb was already enabled by -v2 above */
> +                    { "monitor", "off" },
> +                    { "model-id",
> +                      "AMD EPYC Processor (with IBPB, no MONITOR)" },
> +                    { /* end of list */ },
> +                },
> +            },
>               { /* end of list */ }
>           }
>       },
> @@ -3137,6 +3183,18 @@ 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" },
> +                    { "model-id", "Hygon Dhyana Processor (no MONITOR)" },
> +                    { /* end of list */ },
> +                },
> +            },
> +            { /* end of list */ },
> +        },
>       },
>   };
>   
>