| Message ID | 1427725708-52100-12-git-send-email-mimu@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: [...] > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c > index 829945d..1698b52 100644 > --- a/target-s390x/cpu.c > +++ b/target-s390x/cpu.c > @@ -37,6 +37,11 @@ > #define CR0_RESET 0xE0UL > #define CR14_RESET 0xC2000000UL; > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > +{ > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > +} > + > /* generate CPU information for cpu -? */ > void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf) > { > @@ -74,6 +79,30 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) > > return entry; > } > + > +CpuModelInfo *arch_query_cpu_model(Error **errp) > +{ > + CpuModelInfo *info; > + S390CPUClass *cc; > + > + if (!s390_cpu_models_used()) { > + return NULL; > + } First question is: why you don't want to just return { name: "none", accel: ... } when TYPE_S390_CPU ("-cpu none") is used? Now, assuming that you really want to return NULL if -cpu none is used, why don't you just ask for the first CPU (like you already do below) and check if it is a named CPU model, instead of adding a new global? e.g.: static bool s390_cpu_class_is_model(S390CPUClass *cc) { return cpuid(cc->proc) != 0; } CpuModelInfo *arch_query_cpu_model(Error **errp) { S390CPUClass *cc; S390CPUClass *cc; cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0)); if (!s390_cpu_class_is_model(cc)) { return NULL; } [...] } > + info = g_try_new0(CpuModelInfo, 1); > + if (!info) { > + return NULL; > + } > + cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0)); > + info->name = strdup_s390_cpu_name(cc); I don't think the current version of strdup_s390_cpu_name() can ever return NULL. Do you expect it to return NULL in the future? > + if (!info->name) { > + g_free(info); > + return NULL; > + } > + if (kvm_enabled()) { > + info->accel = ACCEL_ID_KVM; This could be a CPUState field, added automatically by cpu_generic_init() using current_machine->accel. > + } > + return info; > +} > #endif > > static void s390_cpu_set_pc(CPUState *cs, vaddr value) > -- > 1.8.3.1 >
On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > This patch implements a new QMP request named 'query-cpu-model'. > It returns the cpu model of cpu 0 and its backing accelerator. > > request: > {"execute" : "query-cpu-model" } > > answer: > {"return" : {"name": "2827-ga2", "accel": "kvm" }} If you are returning information about an existing CPU, why not just extend the output of "query-cpus"? (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be undesired. But in this case we could add an optional parameter to disable the return of data that requires stopping the VCPU). > > Alias names are resolved to their respective machine type and GA names > already during cpu instantiation. Thus, also a cpu model like 'host' > which is implemented as alias will return its normalized cpu model name. > > Furthermore the patch implements the following function: > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> [...]
On 03/30/2015 08:28 AM, Michael Mueller wrote: > This patch implements a new QMP request named 'query-cpu-model'. > It returns the cpu model of cpu 0 and its backing accelerator. > > request: > {"execute" : "query-cpu-model" } > > answer: > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > Alias names are resolved to their respective machine type and GA names > already during cpu instantiation. Thus, also a cpu model like 'host' > which is implemented as alias will return its normalized cpu model name. > > Furthermore the patch implements the following function: > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> > --- > +++ b/qapi-schema.json > @@ -2516,6 +2516,16 @@ > { 'command': 'query-machines', 'returns': ['MachineInfo'] } > > ## > +# @AccelId > +# > +# Defines accelerator ids > +# > +# Since: 2.4 > +## > +{ 'enum': 'AccelId', > + 'data': ['qtest', 'tcg', 'kvm', 'xen' ] } Unusual spacing (0 spaces after '[' but 2 spaces before closing ']'?), but not necessarily wrong. > +## > +# @CpuModelInfo: > +# > +# Virtual CPU model definition. > +# > +# @name: the name of the CPU model definition > +# > +# @accel: AccelId (name) of this cpu models accelerator s/models/model's/
On 03/30/2015 02:17 PM, Eduardo Habkost wrote: > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: >> This patch implements a new QMP request named 'query-cpu-model'. >> It returns the cpu model of cpu 0 and its backing accelerator. >> >> request: >> {"execute" : "query-cpu-model" } >> >> answer: >> {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > If you are returning information about an existing CPU, why not just > extend the output of "query-cpus"? > > (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be > undesired. But in this case we could add an optional parameter to > disable the return of data that requires stopping the VCPU). And how would libvirt learn about the existence of that optional parameter? Without introspection, a new command is easier to query than learning about whether an optional parameter exists (then again, we're hoping to get introspection into 2.4, so it may be a moot question).
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Mon, 30 Mar 2015 14:19:27 -0600 Eric Blake <eblake@redhat.com> wrote: > On 03/30/2015 08:28 AM, Michael Mueller wrote: > > This patch implements a new QMP request named 'query-cpu-model'. > > It returns the cpu model of cpu 0 and its backing accelerator. > > > > request: > > {"execute" : "query-cpu-model" } > > > > answer: > > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > > > Alias names are resolved to their respective machine type and GA names > > already during cpu instantiation. Thus, also a cpu model like 'host' > > which is implemented as alias will return its normalized cpu model name. > > > > Furthermore the patch implements the following function: > > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> > > --- > > > +++ b/qapi-schema.json > > @@ -2516,6 +2516,16 @@ > > { 'command': 'query-machines', 'returns': ['MachineInfo'] } > > > > ## > > +# @AccelId > > +# > > +# Defines accelerator ids > > +# > > +# Since: 2.4 > > +## > > +{ 'enum': 'AccelId', > > + 'data': ['qtest', 'tcg', 'kvm', 'xen' ] } > > Unusual spacing (0 spaces after '[' but 2 spaces before closing ']'?), > but not necessarily wrong. > > > > +## > > +# @CpuModelInfo: > > +# > > +# Virtual CPU model definition. > > +# > > +# @name: the name of the CPU model definition > > +# > > +# @accel: AccelId (name) of this cpu models accelerator > > s/models/model's/ > > Will fix both findings... Thanks, Michael -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVGlM7AAoJELPcPLQSJsKQV1IP/07cv8CB1ggyWFq7pCjo/x4G A6sjzPziW9sZdrcaMsDb2Px62ddyx+4KymnzzdjKTrKGePZGOBQNd0xwCeIDanw9 Z+hVBG8vzqPiwBM25xKUMYl+NDC7SPeqv6YSFelEBAcbsZGtNb9CBqycjbc8AWtj Qy8PiLlbPQLxUiFIvnTYe3umW60CtnfUoAiRXs3wQyBkWsUZ7HI7/Rsm8wRIq0tE HX52/xuD79rQvgqeJi+2zxHyB+WGv0GrvOzIB22d93peMOrdXyQj9GYM6R4Fj7pT XASifUzztLYlUqNh6MRYmrUaLbsV12ERUKitcd1Cw7l8T7tpIh/NpaOhrm8VosVh P1Wm9UWha1MfF/rW/u/3sW/82Pv+eQ54a9XYd4H4tD3PFMMmIbZK/9D69BpyxtSZ 45fe4jiKO87bryMtaYPH9oAc8VOmOR9EI94p4q/GK8swaYQ5DGNAPiFlWlfmgg4B VGXmiHE0VeJOH5dh5+wT/5gjZu3ZmUQNtqhT09rfG0jvMZVUFT0qr5vXQtYXW4Gj DbK3uir15ovHYBErfun11vs15tUoy4PT3OMsmgelgQl1FQG8T7FD9asj72m3vLUh UVvRj1KduWET4b4W5SBgzLlMdRyTAcXRPKuDCcSCtqfxE4+jsAp3+mYbLpB2y5Aw Qjklt025kmr0A7pNrL4d =ZBtF -----END PGP SIGNATURE-----
On Mon, 30 Mar 2015 16:50:20 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: Hello Eduardo, > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > [...] > > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c > > index 829945d..1698b52 100644 > > --- a/target-s390x/cpu.c > > +++ b/target-s390x/cpu.c > > @@ -37,6 +37,11 @@ > > #define CR0_RESET 0xE0UL > > #define CR14_RESET 0xC2000000UL; > > > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > +{ > > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > +} > > + > > /* generate CPU information for cpu -? */ > > void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf) > > { > > @@ -74,6 +79,30 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) > > > > return entry; > > } > > + > > +CpuModelInfo *arch_query_cpu_model(Error **errp) > > +{ > > + CpuModelInfo *info; > > + S390CPUClass *cc; > > + > > + if (!s390_cpu_models_used()) { > > + return NULL; > > + } > > First question is: why you don't want to just return > { name: "none", accel: ... } > when TYPE_S390_CPU ("-cpu none") is used? That's a good idea, indeed! CPU model "none" is used as well in case option -cpu is omitted. > > Now, assuming that you really want to return NULL if -cpu none is used, > why don't you just ask for the first CPU (like you already do below) and > check if it is a named CPU model, instead of adding a new global? e.g.: > > static bool s390_cpu_class_is_model(S390CPUClass *cc) > { > return cpuid(cc->proc) != 0; > } > > CpuModelInfo *arch_query_cpu_model(Error **errp) > { > S390CPUClass *cc; > S390CPUClass *cc; > > cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0)); > if (!s390_cpu_class_is_model(cc)) { > return NULL; > } > [...] > } > > > > + info = g_try_new0(CpuModelInfo, 1); > > + if (!info) { > > + return NULL; > > + } > > + cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0)); > > + info->name = strdup_s390_cpu_name(cc); > > I don't think the current version of strdup_s390_cpu_name() can ever > return NULL. Do you expect it to return NULL in the future? No I do not expect this. I will drop the NULL test. > > > + if (!info->name) { > > + g_free(info); > > + return NULL; > > + } > > + if (kvm_enabled()) { > > + info->accel = ACCEL_ID_KVM; > > This could be a CPUState field, added automatically by > cpu_generic_init() using current_machine->accel. I will add a CPUState field and see how it works out... > > > + } > > + return info; > > +} > > #endif > > > > static void s390_cpu_set_pc(CPUState *cs, vaddr value) > > -- > > 1.8.3.1 > > > Thanks a lot, Michael
On Mon, 30 Mar 2015 17:17:21 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > > This patch implements a new QMP request named 'query-cpu-model'. > > It returns the cpu model of cpu 0 and its backing accelerator. > > > > request: > > {"execute" : "query-cpu-model" } > > > > answer: > > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > If you are returning information about an existing CPU, why not just > extend the output of "query-cpus"? > > (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be > undesired. But in this case we could add an optional parameter to > disable the return of data that requires stopping the VCPU). Will the cpu_cpu_syncronize_state() really hurt in real life? query-cpus will be called only once a while... I will prepare the extension of query-cpus as an option but initially without the optional parameter. > > > > > Alias names are resolved to their respective machine type and GA names > > already during cpu instantiation. Thus, also a cpu model like 'host' > > which is implemented as alias will return its normalized cpu model name. > > > > Furthermore the patch implements the following function: > > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> > [...] > Thanks, Michael
On Mon, Mar 30, 2015 at 02:20:43PM -0600, Eric Blake wrote: > On 03/30/2015 02:17 PM, Eduardo Habkost wrote: > > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > >> This patch implements a new QMP request named 'query-cpu-model'. > >> It returns the cpu model of cpu 0 and its backing accelerator. > >> > >> request: > >> {"execute" : "query-cpu-model" } > >> > >> answer: > >> {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > > > If you are returning information about an existing CPU, why not just > > extend the output of "query-cpus"? > > > > (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be > > undesired. But in this case we could add an optional parameter to > > disable the return of data that requires stopping the VCPU). > > And how would libvirt learn about the existence of that optional > parameter? Without introspection, a new command is easier to query than > learning about whether an optional parameter exists (then again, we're > hoping to get introspection into 2.4, so it may be a moot question). Even if we don't get introspection, a query-cpus-v2 command (with the extra parameter) could be extended in the future to return additional information about CPUs, instead of being specific for CPU model information. We also have the option of simply providing predictable QOM paths for CPU objects, then clients could use qom-get to get what they need through QOM properties. There was a discussion about it some time ago, but I don't remember the conclusion.
On Tue, Mar 31, 2015 at 01:21:05PM +0200, Michael Mueller wrote: > On Mon, 30 Mar 2015 17:17:21 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > > > This patch implements a new QMP request named 'query-cpu-model'. > > > It returns the cpu model of cpu 0 and its backing accelerator. > > > > > > request: > > > {"execute" : "query-cpu-model" } > > > > > > answer: > > > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > > > If you are returning information about an existing CPU, why not just > > extend the output of "query-cpus"? > > > > (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be > > undesired. But in this case we could add an optional parameter to > > disable the return of data that requires stopping the VCPU). > > Will the cpu_cpu_syncronize_state() really hurt in real life? > query-cpus will be called only once a while... > I was just thinking about possible reasons you wouldn't want to reuse query-cpus, and thought cpu_synchronize_state() call could be one of them. > I will prepare the extension of query-cpus as an option but initially > without the optional parameter. I agree we can simply add the new info to query-cpus without any extra parameter, and (if really necessary) we can worry about optimizing it by avoiding the cpu_synchronize_state() call later.
On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > This patch implements a new QMP request named 'query-cpu-model'. > It returns the cpu model of cpu 0 and its backing accelerator. > > request: > {"execute" : "query-cpu-model" } > > answer: > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > Alias names are resolved to their respective machine type and GA names > already during cpu instantiation. Thus, also a cpu model like 'host' > which is implemented as alias will return its normalized cpu model name. > > Furthermore the patch implements the following function: > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> > --- [...] > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > +{ > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > +} How exactly is this information going to be used by clients? If getting the correct type and ga values is important for them, maybe you could add them as integer fields, instead of requiring clients to parse the CPU model name?
On Tue, 31 Mar 2015 15:35:26 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > > This patch implements a new QMP request named 'query-cpu-model'. > > It returns the cpu model of cpu 0 and its backing accelerator. > > > > request: > > {"execute" : "query-cpu-model" } > > > > answer: > > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > > > Alias names are resolved to their respective machine type and GA names > > already during cpu instantiation. Thus, also a cpu model like 'host' > > which is implemented as alias will return its normalized cpu model name. > > > > Furthermore the patch implements the following function: > > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> > > --- > [...] > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > +{ > > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > +} > > How exactly is this information going to be used by clients? If getting > the correct type and ga values is important for them, maybe you could > add them as integer fields, instead of requiring clients to parse the > CPU model name? The consumer don't need to parse the name, it is just important for them to have distinctive names that correlate with the names returned by query-cpu-definitions. Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential migration target can be verified, i.e. its query-cpu-definitions answer for "kvm" has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism also the largest common denominator can be calculated. That model will be used then. I also changed the above mentioned routine to map the cpu model none case: static inline char *strdup_s390_cpu_name(S390CPUClass *cc) { if (cpuid(cc->proc)) { return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); } else { return g_strdup("none"); } } This implicitly will fail a comparison for cpu model ("none", "kvm") as that will never be part of the query-cpu-definitions answer. I actually applied a couple of your suggestions like: - test for NULL skipped after strdup_s390_cpu_name() - strdup_s390_cpu_name() now also handles none cpu model case - omit runnable and is-default field from query-cpu-definitions answer when they are false - global variable cpu_models_used dropped - function s390_cpu_models_used() dropped - routine query-cpu-definitions has a single code path now Only the integration of the ACCEL_ID with the cpu state in cpu_generic_init() and the change for the query-cpus implementation is under construction. I hope to resend the patches by tomorrow evening. Thanks, Michael >
On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote: > On Tue, 31 Mar 2015 15:35:26 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > > > This patch implements a new QMP request named 'query-cpu-model'. > > > It returns the cpu model of cpu 0 and its backing accelerator. > > > > > > request: > > > {"execute" : "query-cpu-model" } > > > > > > answer: > > > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > > > > > Alias names are resolved to their respective machine type and GA names > > > already during cpu instantiation. Thus, also a cpu model like 'host' > > > which is implemented as alias will return its normalized cpu model name. > > > > > > Furthermore the patch implements the following function: > > > > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> > > > --- > > [...] > > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > > +{ > > > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > > +} > > > > How exactly is this information going to be used by clients? If getting > > the correct type and ga values is important for them, maybe you could > > add them as integer fields, instead of requiring clients to parse the > > CPU model name? > > The consumer don't need to parse the name, it is just important for them to have > distinctive names that correlate with the names returned by query-cpu-definitions. > Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential > migration target can be verified, i.e. its query-cpu-definitions answer for "kvm" > has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism > also the largest common denominator can be calculated. That model will be used then. Understood. So the point is to really have a name that can be found at query-cpu-definitions. Makes sense. (BTW, if you reused strdup_s390_cpu_name() inside s390_cpu_compare_class_name() too, you would automatically ensure that query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will always agree with each other). > > I also changed the above mentioned routine to map the cpu model none case: > > static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > { > if (cpuid(cc->proc)) { > return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > } else { > return g_strdup("none"); > } > } What about: static const char *s390_cpu_name(S390CPUClass *cc) { return cc->model_name; } And then you can just set cc->model_name=_name inside S390_PROC_DEF (and set it to "none" inside s390_cpu_class_init()). I wonder if this class->model_name conversion could be made generic inside the CPU class. We already have a CPU::class_by_name() method, so it makes sense to have the opposite function too. (But I wouldn't mind making this s390-specific first, and converted later to generic code if appropriate). > > This implicitly will fail a comparison for cpu model ("none", "kvm") as that will > never be part of the query-cpu-definitions answer. I am not sure I follow. If ("none", "kvm") is never in the list, is "-cpu none -machine accel=kvm" always an invalid use case? (I don't understand completely the meaning of "-cpu none" yet. How does the CPU look like for the guest in this case? Is it possible to live-migrate when using -cpu none?)
On Wed, 1 Apr 2015 10:01:13 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote: > > On Tue, 31 Mar 2015 15:35:26 -0300 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > > > > This patch implements a new QMP request named 'query-cpu-model'. > > > > It returns the cpu model of cpu 0 and its backing accelerator. > > > > > > > > request: > > > > {"execute" : "query-cpu-model" } > > > > > > > > answer: > > > > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > > > > > > > Alias names are resolved to their respective machine type and GA names > > > > already during cpu instantiation. Thus, also a cpu model like 'host' > > > > which is implemented as alias will return its normalized cpu model name. > > > > > > > > Furthermore the patch implements the following function: > > > > > > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > > > > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> > > > > --- > > > [...] > > > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > > > +{ > > > > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > > > +} > > > > > > How exactly is this information going to be used by clients? If getting > > > the correct type and ga values is important for them, maybe you could > > > add them as integer fields, instead of requiring clients to parse the > > > CPU model name? > > > > The consumer don't need to parse the name, it is just important for them to have > > distinctive names that correlate with the names returned by query-cpu-definitions. > > Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential > > migration target can be verified, i.e. its query-cpu-definitions answer for "kvm" > > has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism > > also the largest common denominator can be calculated. That model will be used then. > > Understood. So the point is to really have a name that can be found at > query-cpu-definitions. Makes sense. > > (BTW, if you reused strdup_s390_cpu_name() inside > s390_cpu_compare_class_name() too, you would automatically ensure that > query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will > always agree with each other). I have to verify but it seems to make sense from reading... I will do that if it works. :-) > > > > > I also changed the above mentioned routine to map the cpu model none case: > > > > static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > { > > if (cpuid(cc->proc)) { > > return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > } else { > > return g_strdup("none"); > > } > > } > > What about: > > static const char *s390_cpu_name(S390CPUClass *cc) > { > return cc->model_name; > } > > And then you can just set cc->model_name=_name inside S390_PROC_DEF (and > set it to "none" inside s390_cpu_class_init()). > Wouldn't that store redundant information... but it would at least shift the work into the initialization phase and do the format just once per model. > I wonder if this class->model_name conversion could be made generic > inside the CPU class. We already have a CPU::class_by_name() method, so > it makes sense to have the opposite function too. > > (But I wouldn't mind making this s390-specific first, and converted > later to generic code if appropriate). ok > > > > > This implicitly will fail a comparison for cpu model ("none", "kvm") as that will > > never be part of the query-cpu-definitions answer. > > I am not sure I follow. If ("none", "kvm") is never in the list, is > "-cpu none -machine accel=kvm" always an invalid use case? Not directly invalid as "-cpu none" will be the same as omitting the -cpu option. KVM will setup the vcpu properties withou any QEMU control to whatever the hosting machine and the kvm kernel code offers. That will allow to run QEMU against a KVM version that is not aware of the s390 cpu model ioctls. > > (I don't understand completely the meaning of "-cpu none" yet. How does > the CPU look like for the guest in this case? Is it possible to > live-migrate when using -cpu none?) And yes, that does not make sense in a migration context. The answer on query-cpu-model (or query-cpus) will be ("none", "kvm") and that will never match a runnable model. The guest cpu will be derived from the hosting system and the kvm kernel as it is currently without the cpu model interface. I hope I made it better to understand now... Michael >
(CCing libvir-list and Jiri Denemark for libvirt-related discussion about -cpu host/none, and live-migration safety expectations) On Wed, Apr 01, 2015 at 06:31:23PM +0200, Michael Mueller wrote: > On Wed, 1 Apr 2015 10:01:13 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote: > > > On Tue, 31 Mar 2015 15:35:26 -0300 > > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > > > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > > > > > This patch implements a new QMP request named 'query-cpu-model'. > > > > > It returns the cpu model of cpu 0 and its backing accelerator. > > > > > > > > > > request: > > > > > {"execute" : "query-cpu-model" } > > > > > > > > > > answer: > > > > > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > > > > > > > > > Alias names are resolved to their respective machine type and GA names > > > > > already during cpu instantiation. Thus, also a cpu model like 'host' > > > > > which is implemented as alias will return its normalized cpu model name. > > > > > > > > > > Furthermore the patch implements the following function: > > > > > > > > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > > > > > > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> > > > > > --- > > > > [...] > > > > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > > > > +{ > > > > > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > > > > +} > > > > > > > > How exactly is this information going to be used by clients? If getting > > > > the correct type and ga values is important for them, maybe you could > > > > add them as integer fields, instead of requiring clients to parse the > > > > CPU model name? > > > > > > The consumer don't need to parse the name, it is just important for them to have > > > distinctive names that correlate with the names returned by query-cpu-definitions. > > > Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential > > > migration target can be verified, i.e. its query-cpu-definitions answer for "kvm" > > > has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism > > > also the largest common denominator can be calculated. That model will be used then. > > > > Understood. So the point is to really have a name that can be found at > > query-cpu-definitions. Makes sense. > > > > (BTW, if you reused strdup_s390_cpu_name() inside > > s390_cpu_compare_class_name() too, you would automatically ensure that > > query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will > > always agree with each other). > > I have to verify but it seems to make sense from reading... I will do that if it works. :-) > > > > > > > > > I also changed the above mentioned routine to map the cpu model none case: > > > > > > static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > > { > > > if (cpuid(cc->proc)) { > > > return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > > } else { > > > return g_strdup("none"); > > > } > > > } > > > > What about: > > > > static const char *s390_cpu_name(S390CPUClass *cc) > > { > > return cc->model_name; > > } > > > > And then you can just set cc->model_name=_name inside S390_PROC_DEF (and > > set it to "none" inside s390_cpu_class_init()). > > > > Wouldn't that store redundant information... but it would at least shift the work into the > initialization phase and do the format just once per model. To be honest, calculating the CPU model name on the fly with strdup_s390_cpu_name() like you did above wouldn't be a problem to me. I just wanted to avoid having the same CPU model name logic (and special cases like "none") duplicated inside strdup_s390_cpu_name(), S390_PROC_DEF, s390_cpu_class_by_name(), and maybe other places. Maybe this duplication can be eliminated by simply reusing strdup_s390_cpu_name() inside s390_cpu_compare_class_name(). > > > I wonder if this class->model_name conversion could be made generic > > inside the CPU class. We already have a CPU::class_by_name() method, so > > it makes sense to have the opposite function too. > > > > (But I wouldn't mind making this s390-specific first, and converted > > later to generic code if appropriate). > > ok > > > > > > > > > This implicitly will fail a comparison for cpu model ("none", "kvm") as that will > > > never be part of the query-cpu-definitions answer. > > > > I am not sure I follow. If ("none", "kvm") is never in the list, is > > "-cpu none -machine accel=kvm" always an invalid use case? > > Not directly invalid as "-cpu none" will be the same as omitting the -cpu option. > KVM will setup the vcpu properties withou any QEMU control to whatever the hosting > machine and the kvm kernel code offers. That will allow to run QEMU against a KVM > version that is not aware of the s390 cpu model ioctls. It looks like we have conflicting expectations about query-cpu-definitions, it seems: on the one hand, if "-cpu none" is valid I believe it should appear on the query-cpu-definitions return value; on the other hand, it is not (always?) migration-safe, so just comparing the source query-cpus data with the target query-cpu-definitions data wouldn't be enough to ensure live-migration safety. On x86, we have a similar problem with "-cpu host", that changes depending on the host hardware and host kernel. We solve this problem by making libvirt code aware of the set of valid CPU models, and libvirt has special cases for "-cpu host". If you don't want to encode that knowledge in libvirt or other management software for s390, it looks like you need something like a "stable-abi-safe" field on CpuDefinitionInfo? > > > > > (I don't understand completely the meaning of "-cpu none" yet. How does > > the CPU look like for the guest in this case? Is it possible to > > live-migrate when using -cpu none?) > > And yes, that does not make sense in a migration context. The answer on query-cpu-model > (or query-cpus) will be ("none", "kvm") and that will never match a runnable model. > The guest cpu will be derived from the hosting system and the kvm kernel as it is currently > without the cpu model interface. > > I hope I made it better to understand now... Yes, thanks!
On Wed, 1 Apr 2015 13:59:05 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > > Not directly invalid as "-cpu none" will be the same as omitting the -cpu option. > > KVM will setup the vcpu properties withou any QEMU control to whatever the hosting > > machine and the kvm kernel code offers. That will allow to run QEMU against a KVM > > version that is not aware of the s390 cpu model ioctls. > > It looks like we have conflicting expectations about > query-cpu-definitions, it seems: on the one hand, if "-cpu none" is > valid I believe it should appear on the query-cpu-definitions return > value; on the other hand, it is not (always?) migration-safe, so just > comparing the source query-cpus data with the target > query-cpu-definitions data wouldn't be enough to ensure live-migration > safety. There are other cases as well where the value given with -cpu is *not* part of the cpu definition list and that is aliases: [mimu@p57lp59 (master-cpu-model) qemu]$ ./s390x-softmmu/qemu-system-s390x -machine s390-ccw,accel=kvm -cpu ? s390 2064-ga1 IBM zSeries 900 GA1 s390 2064-ga2 IBM zSeries 900 GA2 s390 2064-ga3 IBM zSeries 900 GA3 s390 2064 (alias for 2064-ga3) s390 z900 (alias for 2064-ga3) ... s390 z10 (alias for 2097-ga3) s390 z10-ec (alias for 2097-ga3) s390 2098-ga1 IBM System z10 BC GA1 s390 2098-ga2 IBM System z10 BC GA2 s390 2098 (alias for 2098-ga2) s390 z10-bc (alias for 2098-ga2) s390 2817-ga1 IBM zEnterprise 196 GA1 s390 2817-ga2 IBM zEnterprise 196 GA2 s390 2817 (alias for 2817-ga2) s390 z196 (alias for 2817-ga2) s390 2818-ga1 IBM zEnterprise 114 GA1 s390 2818 (alias for 2818-ga1) s390 z114 (alias for 2818-ga1) s390 2827-ga1 IBM zEnterprise EC12 GA1 s390 2827-ga2 IBM zEnterprise EC12 GA2 s390 2827 (alias for 2827-ga2) s390 zEC12 (alias for 2827-ga2) s390 host (alias for 2827-ga2) s390 2828-ga1 IBM zEnterprise BC12 GA1 s390 2828 (alias for 2828-ga1) s390 zBC12 (alias for 2828-ga1) As you can see "host" is in s390x case always an alias and also all other aliases are normalized to their real cpu models in the cpu-definitions list. > > On x86, we have a similar problem with "-cpu host", that changes > depending on the host hardware and host kernel. We solve this problem by > making libvirt code aware of the set of valid CPU models, and libvirt > has special cases for "-cpu host". "-cpu host" is not a special case for s390, it will return ("2827-ga2", "kvm") as cpu model or whatever model the hosting system implements. > > If you don't want to encode that knowledge in libvirt or other > management software for s390, it looks like you need something like a > "stable-abi-safe" field on CpuDefinitionInfo? Exactly that fulfills the "name" field for s390 already in my view. And cpu model "none" just means that QEMU does not manage the cpu model. That's also the reason why I initially returned an empty "[]" model and not "none". This somewhat convinces me to go back to this approach... Michael
On Wed, 1 Apr 2015 21:05:31 +0200 Michael Mueller <mimu@linux.vnet.ibm.com> wrote: > And cpu model "none" just means that QEMU does not manage the cpu model. That's also > the reason why I initially returned an empty "[]" model and not "none". This somewhat > convinces me to go back to this approach... And for query-cpus that can be represented as a non-existent model field: [{"current":true,"CPU":0,"halted":false,"thread_id":39110}, ...]
On Wed, Apr 01, 2015 at 09:05:31PM +0200, Michael Mueller wrote: > On Wed, 1 Apr 2015 13:59:05 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > Not directly invalid as "-cpu none" will be the same as omitting the -cpu option. > > > KVM will setup the vcpu properties withou any QEMU control to whatever the hosting > > > machine and the kvm kernel code offers. That will allow to run QEMU against a KVM > > > version that is not aware of the s390 cpu model ioctls. > > > > It looks like we have conflicting expectations about > > query-cpu-definitions, it seems: on the one hand, if "-cpu none" is > > valid I believe it should appear on the query-cpu-definitions return > > value; on the other hand, it is not (always?) migration-safe, so just > > comparing the source query-cpus data with the target > > query-cpu-definitions data wouldn't be enough to ensure live-migration > > safety. > > There are other cases as well where the value given with -cpu is *not* part > of the cpu definition list and that is aliases: > > [mimu@p57lp59 (master-cpu-model) qemu]$ ./s390x-softmmu/qemu-system-s390x -machine > s390-ccw,accel=kvm -cpu ? > s390 2064-ga1 IBM zSeries 900 GA1 > s390 2064-ga2 IBM zSeries 900 GA2 > s390 2064-ga3 IBM zSeries 900 GA3 > s390 2064 (alias for 2064-ga3) > s390 z900 (alias for 2064-ga3) > ... > s390 z10 (alias for 2097-ga3) > s390 z10-ec (alias for 2097-ga3) > s390 2098-ga1 IBM System z10 BC GA1 > s390 2098-ga2 IBM System z10 BC GA2 > s390 2098 (alias for 2098-ga2) > s390 z10-bc (alias for 2098-ga2) > s390 2817-ga1 IBM zEnterprise 196 GA1 > s390 2817-ga2 IBM zEnterprise 196 GA2 > s390 2817 (alias for 2817-ga2) > s390 z196 (alias for 2817-ga2) > s390 2818-ga1 IBM zEnterprise 114 GA1 > s390 2818 (alias for 2818-ga1) > s390 z114 (alias for 2818-ga1) > s390 2827-ga1 IBM zEnterprise EC12 GA1 > s390 2827-ga2 IBM zEnterprise EC12 GA2 > s390 2827 (alias for 2827-ga2) > s390 zEC12 (alias for 2827-ga2) > s390 host (alias for 2827-ga2) > s390 2828-ga1 IBM zEnterprise BC12 GA1 > s390 2828 (alias for 2828-ga1) > s390 zBC12 (alias for 2828-ga1) > > As you can see "host" is in s390x case always an alias and also all other aliases > are normalized to their real cpu models in the cpu-definitions list. > > > > > On x86, we have a similar problem with "-cpu host", that changes > > depending on the host hardware and host kernel. We solve this problem by > > making libvirt code aware of the set of valid CPU models, and libvirt > > has special cases for "-cpu host". > > "-cpu host" is not a special case for s390, it will return ("2827-ga2", "kvm") as > cpu model or whatever model the hosting system implements. > > > > > If you don't want to encode that knowledge in libvirt or other > > management software for s390, it looks like you need something like a > > "stable-abi-safe" field on CpuDefinitionInfo? > > Exactly that fulfills the "name" field for s390 already in my view. > > And cpu model "none" just means that QEMU does not manage the cpu model. That's also > the reason why I initially returned an empty "[]" model and not "none". This somewhat > convinces me to go back to this approach... I understand the reasons for your approach and it seems to work for s390, but the only problem I see is that you are adding an additional (undocumented?) s390-specific constraint to the semantics of query-cpu-models: that the model name will appear on the list only if it can be safely migratable. This may prevent us from unifying CPU model code into generic code later. But if we add a simple stable-abi-safe field to the list (even if s390 set it to to true for all models and omit aliases and "none" in this first version), we will have clearer semantics that can still be honoured by other architectures (and by generic code) later.
On Wed, 1 Apr 2015 20:05:24 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > > If you don't want to encode that knowledge in libvirt or other > > > management software for s390, it looks like you need something like a > > > "stable-abi-safe" field on CpuDefinitionInfo? > > > > Exactly that fulfills the "name" field for s390 already in my view. > > > > And cpu model "none" just means that QEMU does not manage the cpu model. That's also > > the reason why I initially returned an empty "[]" model and not "none". This somewhat > > convinces me to go back to this approach... > > I understand the reasons for your approach and it seems to work for > s390, but the only problem I see is that you are adding an additional > (undocumented?) s390-specific constraint to the semantics of > query-cpu-models: that the model name will appear on the list only if it > can be safely migratable. This may prevent us from unifying CPU model > code into generic code later. I agree that an aliases is something different compared with the CPU model none as there is a CPU class representing it. And thus, when implicitly or explicitly selected, shall be presented in the CPU definition list as well. If I would set "runnable" to false as it now (bad), it would be sorted out by the "considered for migration" test but it would be misleading as it is always runnable. Though an additional field like "migrate-able" could express that characteristic. > > But if we add a simple stable-abi-safe field to the list (even if s390 > set it to to true for all models and omit aliases and "none" in this > first version), we will have clearer semantics that can still be > honoured by other architectures (and by generic code) later. To be honest I currently don't right get the idea that you follow with that stable-abi-save field... But eventually yes (I wrote this before the section above) The stable-abi-save field means: "Take me into account for whatever kind of CPU model related comparison you perform between two running QEMU instances as I represent a well defined aspect. Thus CPU model none will be { "name": "none", "runnable: true, "stable-abi-save": false } and the aliases can be represented as { "name": <alias>, "runnable": <true|false>, "stable-abi-save": false } in the s390 case, right? Michael
On Thu, Apr 02, 2015 at 09:09:07AM +0200, Michael Mueller wrote: > On Wed, 1 Apr 2015 20:05:24 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > > > > > If you don't want to encode that knowledge in libvirt or other > > > > management software for s390, it looks like you need something like a > > > > "stable-abi-safe" field on CpuDefinitionInfo? > > > > > > Exactly that fulfills the "name" field for s390 already in my view. > > > > > > And cpu model "none" just means that QEMU does not manage the cpu model. That's also > > > the reason why I initially returned an empty "[]" model and not "none". This somewhat > > > convinces me to go back to this approach... > > > > I understand the reasons for your approach and it seems to work for > > s390, but the only problem I see is that you are adding an additional > > (undocumented?) s390-specific constraint to the semantics of > > query-cpu-models: that the model name will appear on the list only if it > > can be safely migratable. This may prevent us from unifying CPU model > > code into generic code later. > > I agree that an aliases is something different compared with the CPU model none as > there is a CPU class representing it. And thus, when implicitly or explicitly selected, > shall be presented in the CPU definition list as well. If I would set "runnable" to > false as it now (bad), it would be sorted out by the "considered for migration" test but it > would be misleading as it is always runnable. Though an additional field like "migrate-able" > could express that characteristic. Exactly. > > > > > But if we add a simple stable-abi-safe field to the list (even if s390 > > set it to to true for all models and omit aliases and "none" in this > > first version), we will have clearer semantics that can still be > > honoured by other architectures (and by generic code) later. > > To be honest I currently don't right get the idea that you follow with that > stable-abi-save field... But eventually yes (I wrote this before the section above) > > The stable-abi-save field means: "Take me into account for whatever kind of > CPU model related comparison you perform between two running QEMU instances as I > represent a well defined aspect. Yes. "stable-abi-safe" would mean that nothing guest-visible will change in the CPU model when running a different QEMU version or running in a different host, thus making it safe to live-migrate (as long as you keep the same machine+accelerator and don't change other guest-visible configuration in the QEMU command-line, of course). That's a constraint we already keep in the x86 CPU models, except for "-cpu host". In other words, it means "as long as the name matches the query-cpus output from the source host, it is guaranteed to be safe to live-migrate". Which is the constraint you need, right? (I am not 100% sure about the naming. Maybe we should call it "live-migration-safe"?) > > Thus CPU model none will be { "name": "none", "runnable: true, "stable-abi-save": false } and > the aliases can be represented as { "name": <alias>, "runnable": <true|false>, "stable-abi-save": > false } in the s390 case, right? Exactly. We don't need to return them in the first version if you don't need to (althought I don't see a reason to not return them). It will just allow us to return them in the future.
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h index 54b36c1..86344a2 100644 --- a/include/sysemu/arch_init.h +++ b/include/sysemu/arch_init.h @@ -37,5 +37,6 @@ int kvm_available(void); int xen_available(void); CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp); +CpuModelInfo *arch_query_cpu_model(Error **errp); #endif diff --git a/qapi-schema.json b/qapi-schema.json index ac9594d..14d294f 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2516,6 +2516,16 @@ { 'command': 'query-machines', 'returns': ['MachineInfo'] } ## +# @AccelId +# +# Defines accelerator ids +# +# Since: 2.4 +## +{ 'enum': 'AccelId', + 'data': ['qtest', 'tcg', 'kvm', 'xen' ] } + +## # @CpuDefinitionInfo: # # Virtual CPU definition. @@ -2538,6 +2548,31 @@ ## { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] } +## +# @CpuModelInfo: +# +# Virtual CPU model definition. +# +# @name: the name of the CPU model definition +# +# @accel: AccelId (name) of this cpu models accelerator +# +# Since: 2.4 +## +{ 'type': 'CpuModelInfo', + 'data': { 'name': 'str', 'accel': 'AccelId' } } + +## +# @query-cpu-model: +# +# Return the current virtual CPU model +# +# Returns: CpuModelInfo +# +# Since: 2.4 +## +{ 'command': 'query-cpu-model', 'returns': 'CpuModelInfo' } + # @AddfdInfo: # # Information about a file descriptor that was added to an fd set. diff --git a/qmp-commands.hx b/qmp-commands.hx index 3a42ad0..8fe577f 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3417,6 +3417,12 @@ EQMP }, { + .name = "query-cpu-model", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_cpu_model, + }, + + { .name = "query-target", .args_type = "", .mhandler.cmd_new = qmp_marshal_input_query_target, diff --git a/qmp.c b/qmp.c index c479e77..ad63803 100644 --- a/qmp.c +++ b/qmp.c @@ -572,6 +572,11 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return arch_query_cpu_definitions(errp); } +CpuModelInfo *qmp_query_cpu_model(Error **errp) +{ + return arch_query_cpu_model(errp); +} + void qmp_add_client(const char *protocol, const char *fdname, bool has_skipauth, bool skipauth, bool has_tls, bool tls, Error **errp) diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index dce9cd2..ca70d5d 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -1,4 +1,5 @@ stub-obj-y += arch-query-cpu-def.o +stub-obj-y += arch-query-cpu-mod.o stub-obj-y += bdrv-commit-all.o stub-obj-y += chr-baum-init.o stub-obj-y += chr-msmouse.o diff --git a/stubs/arch-query-cpu-mod.c b/stubs/arch-query-cpu-mod.c new file mode 100644 index 0000000..90ebd08 --- /dev/null +++ b/stubs/arch-query-cpu-mod.c @@ -0,0 +1,9 @@ +#include "qemu-common.h" +#include "sysemu/arch_init.h" +#include "qapi/qmp/qerror.h" + +CpuModelInfo *arch_query_cpu_model(Error **errp) +{ + error_set(errp, QERR_UNSUPPORTED); + return NULL; +} diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c index ba873ea..187d110 100644 --- a/target-s390x/cpu-models.c +++ b/target-s390x/cpu-models.c @@ -707,3 +707,17 @@ void s390_cpu_model_init(S390CPUClass *cc) } } } + +/** + * s390_cpu_models_used: + * + * This function indicates if cpus with model properties are in use. + * + * Returns: a boolean value. + * + * Since: 2.4 + */ +bool s390_cpu_models_used(void) +{ + return cpu_models_used; +} diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h index fe3997f..67f0519 100644 --- a/target-s390x/cpu-models.h +++ b/target-s390x/cpu-models.h @@ -112,6 +112,7 @@ bool s390_cpu_classes_initialized(void); uint64_t *s390_fac_list_mask_by_machine(const char *name); uint64_t *s390_current_fac_list_mask(void); void s390_cpu_model_init(S390CPUClass *cc); +bool s390_cpu_models_used(void); extern uint64_t qemu_s390_fac_list_mask[]; diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c index 829945d..1698b52 100644 --- a/target-s390x/cpu.c +++ b/target-s390x/cpu.c @@ -37,6 +37,11 @@ #define CR0_RESET 0xE0UL #define CR14_RESET 0xC2000000UL; +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) +{ + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); +} + /* generate CPU information for cpu -? */ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf) { @@ -74,6 +79,30 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) return entry; } + +CpuModelInfo *arch_query_cpu_model(Error **errp) +{ + CpuModelInfo *info; + S390CPUClass *cc; + + if (!s390_cpu_models_used()) { + return NULL; + } + info = g_try_new0(CpuModelInfo, 1); + if (!info) { + return NULL; + } + cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0)); + info->name = strdup_s390_cpu_name(cc); + if (!info->name) { + g_free(info); + return NULL; + } + if (kvm_enabled()) { + info->accel = ACCEL_ID_KVM; + } + return info; +} #endif static void s390_cpu_set_pc(CPUState *cs, vaddr value)
This patch implements a new QMP request named 'query-cpu-model'. It returns the cpu model of cpu 0 and its backing accelerator. request: {"execute" : "query-cpu-model" } answer: {"return" : {"name": "2827-ga2", "accel": "kvm" }} Alias names are resolved to their respective machine type and GA names already during cpu instantiation. Thus, also a cpu model like 'host' which is implemented as alias will return its normalized cpu model name. Furthermore the patch implements the following function: - s390_cpu_models_used(), returns true if S390 cpu models are in use Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com> --- include/sysemu/arch_init.h | 1 + qapi-schema.json | 35 +++++++++++++++++++++++++++++++++++ qmp-commands.hx | 6 ++++++ qmp.c | 5 +++++ stubs/Makefile.objs | 1 + stubs/arch-query-cpu-mod.c | 9 +++++++++ target-s390x/cpu-models.c | 14 ++++++++++++++ target-s390x/cpu-models.h | 1 + target-s390x/cpu.c | 29 +++++++++++++++++++++++++++++ 9 files changed, 101 insertions(+) create mode 100644 stubs/arch-query-cpu-mod.c