| Message ID | 1591184823-140846-1-git-send-email-robert.hu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] Introduce (x86) CPU model deprecation API | expand |
On 6/3/20 6:47 AM, Robert Hoo wrote: > Complement versioned CPU model framework with the ability of marking some > versions deprecated. When that CPU model is chosen, get some warning. The > warning message is customized, e.g. telling in which future QEMU version will > it be obsoleted. > The deprecation message will also appear by x86_cpu_list_entry(), e.g. '-cpu > help'. > QMP 'query-cpu-definitions' will also return a bool value indicating the > deprecation status. > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > --- > exec.c | 3 +++ > include/hw/core/cpu.h | 1 + > qapi/machine-target.json | 3 ++- > target/i386/cpu.c | 45 +++++++++++++++++++++++++++++++++++++++++++-- > 4 files changed, 49 insertions(+), 3 deletions(-) > +++ b/qapi/machine-target.json > @@ -309,7 +309,8 @@ > 'static': 'bool', > '*unavailable-features': [ 'str' ], > 'typename': 'str', > - '*alias-of' : 'str' }, > + '*alias-of' : 'str', > + 'deprecated' : 'bool' }, Missing documentation of the new member. Should it be optional (present only when true)? > @@ -1638,6 +1639,11 @@ struct X86CPUModel { > * This matters only for "-cpu help" and query-cpu-definitions > */ > bool is_alias; > + /* > + * If true, this is deprecated and obsoleted in the future. > + * Trying to use deprecated CPU model shall be warned. If true, this model is deprecated, and may be removed in the future. Trying to use it now will cause a warning. > + */ > + bool deprecated; > }; >
On Wed, 2020-06-03 at 09:11 -0500, Eric Blake wrote: > On 6/3/20 6:47 AM, Robert Hoo wrote: > > Complement versioned CPU model framework with the ability of > > marking some > > versions deprecated. When that CPU model is chosen, get some > > warning. The > > warning message is customized, e.g. telling in which future QEMU > > version will > > it be obsoleted. > > The deprecation message will also appear by x86_cpu_list_entry(), > > e.g. '-cpu > > help'. > > QMP 'query-cpu-definitions' will also return a bool value > > indicating the > > deprecation status. > > > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > > --- > > exec.c | 3 +++ > > include/hw/core/cpu.h | 1 + > > qapi/machine-target.json | 3 ++- > > target/i386/cpu.c | 45 > > +++++++++++++++++++++++++++++++++++++++++++-- > > 4 files changed, 49 insertions(+), 3 deletions(-) > > +++ b/qapi/machine-target.json > > @@ -309,7 +309,8 @@ > > 'static': 'bool', > > '*unavailable-features': [ 'str' ], > > 'typename': 'str', > > - '*alias-of' : 'str' }, > > + '*alias-of' : 'str', > > + 'deprecated' : 'bool' }, > > Missing documentation of the new member. Should it be optional > (present > only when true)? Which document do you mean? How to make it optional? (Sorry, new to QMP) > > > @@ -1638,6 +1639,11 @@ struct X86CPUModel { > > * This matters only for "-cpu help" and query-cpu- > > definitions > > */ > > bool is_alias; > > + /* > > + * If true, this is deprecated and obsoleted in the future. > > + * Trying to use deprecated CPU model shall be warned. > > If true, this model is deprecated, and may be removed in the future. > Trying to use it now will cause a warning. Thanks Eric:) > > > + */ > > + bool deprecated; > > }; > > > >
On 6/4/20 3:07 AM, Robert Hoo wrote: >>> +++ b/qapi/machine-target.json >>> @@ -309,7 +309,8 @@ >>> 'static': 'bool', >>> '*unavailable-features': [ 'str' ], >>> 'typename': 'str', >>> - '*alias-of' : 'str' }, >>> + '*alias-of' : 'str', >>> + 'deprecated' : 'bool' }, >> >> Missing documentation of the new member. Should it be optional >> (present >> only when true)? > Which document do you mean? A few lines earlier is '@alias-of: ...'; you'll need to add a similar line for '@deprecated', mentioning it is '(since 5.1)'. > How to make it optional? Name it '*deprecated', then deal with 'has_deprecated' in the C code for the cases where the member should be output.
On Thu, 2020-06-04 at 06:59 -0500, Eric Blake wrote: > On 6/4/20 3:07 AM, Robert Hoo wrote: > > > > > +++ b/qapi/machine-target.json > > > > @@ -309,7 +309,8 @@ > > > > 'static': 'bool', > > > > '*unavailable-features': [ 'str' ], > > > > 'typename': 'str', > > > > - '*alias-of' : 'str' }, > > > > + '*alias-of' : 'str', > > > > + 'deprecated' : 'bool' }, > > > > > > Missing documentation of the new member. Should it be optional > > > (present > > > only when true)? > > > > Which document do you mean? Thanks Eric:) > > A few lines earlier is '@alias-of: ...'; you'll need to add a > similar > line for '@deprecated', mentioning it is '(since 5.1)'. > > > How to make it optional? How about not making it optional? refer to Machineinfo::deprecated. > > Name it '*deprecated', then deal with 'has_deprecated' in the C code > for > the cases where the member should be output. >
On 6/4/20 9:47 PM, Robert Hoo wrote: > On Thu, 2020-06-04 at 06:59 -0500, Eric Blake wrote: >> On 6/4/20 3:07 AM, Robert Hoo wrote: >> >>>>> +++ b/qapi/machine-target.json >>>>> @@ -309,7 +309,8 @@ >>>>> 'static': 'bool', >>>>> '*unavailable-features': [ 'str' ], >>>>> 'typename': 'str', >>>>> - '*alias-of' : 'str' }, >>>>> + '*alias-of' : 'str', >>>>> + 'deprecated' : 'bool' }, >>>> >>>> Missing documentation of the new member. Should it be optional >>>> (present >>>> only when true)? >>> >>> Which document do you mean? > > Thanks Eric:) > >> >> A few lines earlier is '@alias-of: ...'; you'll need to add a >> similar >> line for '@deprecated', mentioning it is '(since 5.1)'. >> >>> How to make it optional? > > How about not making it optional? refer to Machineinfo::deprecated. Always providing it doesn't hurt. If there is precedence for not making it optional, mentioning that precedence in the commit message can't hurt.
On Fri, 2020-06-05 at 08:47 -0500, Eric Blake wrote: > On 6/4/20 9:47 PM, Robert Hoo wrote: > > On Thu, 2020-06-04 at 06:59 -0500, Eric Blake wrote: > > > On 6/4/20 3:07 AM, Robert Hoo wrote: > > > > > > > > > +++ b/qapi/machine-target.json > > > > > > @@ -309,7 +309,8 @@ > > > > > > 'static': 'bool', > > > > > > '*unavailable-features': [ 'str' ], > > > > > > 'typename': 'str', > > > > > > - '*alias-of' : 'str' }, > > > > > > + '*alias-of' : 'str', > > > > > > + 'deprecated' : 'bool' }, > > > > > > > > > > Missing documentation of the new member. Should it be > > > > > optional > > > > > (present > > > > > only when true)? > > > > > > > > Which document do you mean? > > > > Thanks Eric:) > > > > > > > > A few lines earlier is '@alias-of: ...'; you'll need to add a > > > similar > > > line for '@deprecated', mentioning it is '(since 5.1)'. > > > > > > > How to make it optional? > > > > How about not making it optional? refer to Machineinfo::deprecated. > > Always providing it doesn't hurt. If there is precedence for not > making > it optional, mentioning that precedence in the commit message can't > hurt. No specific precedence. Just feel a little weird that adding an additional boolean, just for judging another boolean should present or not. esp. given that Machineinfo::deprecated is not optional. >
diff --git a/exec.c b/exec.c index 5162f0d..a403937 100644 --- a/exec.c +++ b/exec.c @@ -981,6 +981,9 @@ const char *parse_cpu_option(const char *cpu_option) cpu_type = object_class_get_name(oc); cc = CPU_CLASS(oc); cc->parse_features(cpu_type, model_pieces[1], &error_fatal); + if (cc->deprecation_check) { + cc->deprecation_check(oc); + } g_strfreev(model_pieces); return cpu_type; } diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 07f7698..b2df186 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -218,6 +218,7 @@ typedef struct CPUClass { void (*disas_set_info)(CPUState *cpu, disassemble_info *info); vaddr (*adjust_watchpoint_address)(CPUState *cpu, vaddr addr, int len); void (*tcg_initialize)(void); + void (*deprecation_check)(ObjectClass *oc); /* Keep non-pointer data at the end to minimize holes. */ int gdb_num_core_regs; diff --git a/qapi/machine-target.json b/qapi/machine-target.json index f2c8294..2e7db97 100644 --- a/qapi/machine-target.json +++ b/qapi/machine-target.json @@ -309,7 +309,8 @@ 'static': 'bool', '*unavailable-features': [ 'str' ], 'typename': 'str', - '*alias-of' : 'str' }, + '*alias-of' : 'str', + 'deprecated' : 'bool' }, 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' } ## diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 3733d9a..d7ac22f 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1599,6 +1599,7 @@ typedef struct X86CPUVersionDefinition { const char *alias; const char *note; PropValue *props; + bool deprecated; } X86CPUVersionDefinition; /* Base definition for a CPU model */ @@ -1638,6 +1639,11 @@ struct X86CPUModel { * This matters only for "-cpu help" and query-cpu-definitions */ bool is_alias; + /* + * If true, this is deprecated and obsoleted in the future. + * Trying to use deprecated CPU model shall be warned. + */ + bool deprecated; }; /* Get full model name for CPU version */ @@ -4128,8 +4134,7 @@ static X86CPUVersion x86_cpu_model_resolve_version(const X86CPUModel *model) X86CPUVersion v = model->version; if (v == CPU_VERSION_AUTO) { v = default_cpu_version; - } - if (v == CPU_VERSION_LATEST) { + } else if (v == CPU_VERSION_LATEST) { return x86_cpu_model_last_version(model); } return v; @@ -4975,6 +4980,7 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data) info->migration_safe = cc->migration_safe; info->has_migration_safe = true; info->q_static = cc->static_model; + info->deprecated = cc->model ? cc->model->deprecated : false; /* * Old machine types won't report aliases, so that alias translation * doesn't break compatibility with previous QEMU versions. @@ -5411,6 +5417,7 @@ static void x86_register_cpudef_types(X86CPUDefinition *def) m->cpudef = def; m->version = vdef->version; m->note = vdef->note; + m->deprecated = vdef->deprecated; x86_register_cpu_model_type(name, m); if (vdef->alias) { @@ -5418,6 +5425,8 @@ static void x86_register_cpudef_types(X86CPUDefinition *def) am->cpudef = def; am->version = vdef->version; am->is_alias = true; + am->note = vdef->note; + am->deprecated = vdef->deprecated; x86_register_cpu_model_type(vdef->alias, am); } } @@ -7229,6 +7238,37 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_END_OF_LIST() }; +static void x86_cpu_deprecation_check(ObjectClass *oc) +{ + X86CPUClass *xcc = X86_CPU_CLASS(oc); + X86CPUVersion effective_version; + const X86CPUVersionDefinition *vdef; + + if (xcc->model == NULL) { + return; + } + + if (xcc->model->version == CPU_VERSION_LEGACY) { + /* Treat legacy version as v1 */ + effective_version = 1; + } else { + effective_version = x86_cpu_model_resolve_version(xcc->model); + } + + vdef = xcc->model->cpudef->versions; + + if (vdef == NULL) { + return; + } else { + if (vdef[effective_version - 1].deprecated) { + warn_report("Effective CPU model '%s' -- %s", + x86_cpu_versioned_model_name(xcc->model->cpudef,\ + effective_version), + vdef[effective_version - 1].note); + } + } +} + static void x86_cpu_common_class_init(ObjectClass *oc, void *data) { X86CPUClass *xcc = X86_CPU_CLASS(oc); @@ -7287,6 +7327,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data) cc->tlb_fill = x86_cpu_tlb_fill; #endif cc->disas_set_info = x86_disas_set_info; + cc->deprecation_check = x86_cpu_deprecation_check; dc->user_creatable = true; }
Complement versioned CPU model framework with the ability of marking some versions deprecated. When that CPU model is chosen, get some warning. The warning message is customized, e.g. telling in which future QEMU version will it be obsoleted. The deprecation message will also appear by x86_cpu_list_entry(), e.g. '-cpu help'. QMP 'query-cpu-definitions' will also return a bool value indicating the deprecation status. Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> --- exec.c | 3 +++ include/hw/core/cpu.h | 1 + qapi/machine-target.json | 3 ++- target/i386/cpu.c | 45 +++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 49 insertions(+), 3 deletions(-)