| Message ID | 20250213091626.3650603-1-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/loongarch: Add full type support for query-cpu-model-expansion | expand |
please discard this patch :( libvirt uses static type already on LoongArch platform, there is compatibility issue. Static type cannot be removed, although I do not know the difference between static type and full type. Regards Bibo Mao On 2025/2/13 下午5:16, Bibo Mao wrote: > With full type for query-cpu-model-expansion qmp command, it shows that > it is not supported. For instance, > query-cpu-model-expansion type=full model={"name":"max"} > > Here is is output, > {"error": {"class": "GenericError", "desc": "The requested expansion type is not supported"}} > > Since full type is not supported and only static type is supported, Here > replace static type with full type for command query-cpu-model-expansion. > > And there is result with this patch, > {"return": {"model": {"name": "max", "props": {"lbt": true, "lasx": true, "pmu": true, "lsx": true}}}} > > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > --- > target/loongarch/loongarch-qmp-cmds.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c > index 3fde5a5a20..429c6d35fd 100644 > --- a/target/loongarch/loongarch-qmp-cmds.c > +++ b/target/loongarch/loongarch-qmp-cmds.c > @@ -56,7 +56,7 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, > const char *name; > int i; > > - if (type != CPU_MODEL_EXPANSION_TYPE_STATIC) { > + if (type != CPU_MODEL_EXPANSION_TYPE_FULL) { > error_setg(errp, "The requested expansion type is not supported"); > return NULL; > } > > base-commit: de278e54aefed143526174335f8286f7437d20be >
Cc: Markus hi, Markus What is the difference between CPU_MODEL_EXPANSION_TYPE_STATIC and CPU_MODEL_EXPANSION_TYPE_FULL? thanks. Song Gao. 在 2025/2/13 下午5:16, Bibo Mao 写道: > With full type for query-cpu-model-expansion qmp command, it shows that > it is not supported. For instance, > query-cpu-model-expansion type=full model={"name":"max"} > > Here is is output, > {"error": {"class": "GenericError", "desc": "The requested expansion type is not supported"}} > > Since full type is not supported and only static type is supported, Here > replace static type with full type for command query-cpu-model-expansion. > > And there is result with this patch, > {"return": {"model": {"name": "max", "props": {"lbt": true, "lasx": true, "pmu": true, "lsx": true}}}} > > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > --- > target/loongarch/loongarch-qmp-cmds.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c > index 3fde5a5a20..429c6d35fd 100644 > --- a/target/loongarch/loongarch-qmp-cmds.c > +++ b/target/loongarch/loongarch-qmp-cmds.c > @@ -56,7 +56,7 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, > const char *name; > int i; > > - if (type != CPU_MODEL_EXPANSION_TYPE_STATIC) { > + if (type != CPU_MODEL_EXPANSION_TYPE_FULL) { > error_setg(errp, "The requested expansion type is not supported"); > return NULL; > } > > base-commit: de278e54aefed143526174335f8286f7437d20be
gaosong <gaosong@loongson.cn> writes: > Cc: Markus > > hi, Markus > > What is the difference between CPU_MODEL_EXPANSION_TYPE_STATIC and > CPU_MODEL_EXPANSION_TYPE_FULL? I don't know :) Here's the documentation: ## # @CpuModelExpansionType: # # An enumeration of CPU model expansion types. # # @static: Expand to a static CPU model, a combination of a static # base model name and property delta changes. As the static base # model will never change, the expanded CPU model will be the # same, independent of QEMU version, machine type, machine # options, and accelerator options. Therefore, the resulting # model can be used by tooling without having to specify a # compatibility machine - e.g. when displaying the "host" model. # The @static CPU models are migration-safe. # # @full: Expand all properties. The produced model is not guaranteed # to be migration-safe, but allows tooling to get an insight and # work with model details. # # .. note:: When a non-migration-safe CPU model is expanded in static # mode, some features enabled by the CPU model may be omitted, # because they can't be implemented by a static CPU model # definition (e.g. cache info passthrough and PMU passthrough in # x86). If you need an accurate representation of the features # enabled by a non-migration-safe CPU model, use @full. If you # need a static representation that will keep ABI compatibility # even when changing QEMU version or machine-type, use @static (but # keep in mind that some features may be omitted). # # Since: 2.8 ## If you have further questions, David Hildenbrand or Eduardo Habkost (cc'ed) might be able to help.
On Thu, 13 Feb 2025 12:48:30 +0100 Markus Armbruster <armbru@redhat.com> wrote: > gaosong <gaosong@loongson.cn> writes: > > > Cc: Markus > > > > hi, Markus > > > > What is the difference between CPU_MODEL_EXPANSION_TYPE_STATIC and > > CPU_MODEL_EXPANSION_TYPE_FULL? the only difference is that 'static' expansion will not report properties not mentioned in hard-codded CPU model definition see: builtin_x86_defs while 'full' will iterate over/report all rw properties of CPU object created from provided model name. > I don't know :) > > Here's the documentation: > > ## > # @CpuModelExpansionType: > # > # An enumeration of CPU model expansion types. > # > # @static: Expand to a static CPU model, a combination of a static > # base model name and property delta changes. As the static base > # model will never change, the expanded CPU model will be the > # same, independent of QEMU version, machine type, machine > # options, and accelerator options. Therefore, the resulting > # model can be used by tooling without having to specify a > # compatibility machine - e.g. when displaying the "host" model. > # The @static CPU models are migration-safe. Looking at related x86 code above description sounds like a fiction. Both 'static' and 'full' do use qmp_query_cpu_model_expansion() x86_cpu_from_model() cpu = object_new(foo_cpu_class) x86_cpu_expand_features(cpu) and the later 2 calls are subjects to being influenced by: "QEMU version, machine type (compat machinery), machine options, and accelerator options" and -cpu option > # > # @full: Expand all properties. The produced model is not guaranteed > # to be migration-safe, but allows tooling to get an insight and > # work with model details. > # > # .. note:: When a non-migration-safe CPU model is expanded in static > # mode, some features enabled by the CPU model may be omitted, > # because they can't be implemented by a static CPU model > # definition (e.g. cache info passthrough and PMU passthrough in > # x86). If you need an accurate representation of the features > # enabled by a non-migration-safe CPU model, use @full. If you > # need a static representation that will keep ABI compatibility > # even when changing QEMU version or machine-type, use @static (but > # keep in mind that some features may be omitted). > # > # Since: 2.8 > ## > > If you have further questions, David Hildenbrand or Eduardo Habkost > (cc'ed) might be able to help. > >
On 13.02.25 17:17, Igor Mammedov wrote: > On Thu, 13 Feb 2025 12:48:30 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> gaosong <gaosong@loongson.cn> writes: >> >>> Cc: Markus >>> >>> hi, Markus >>> >>> What is the difference between CPU_MODEL_EXPANSION_TYPE_STATIC and >>> CPU_MODEL_EXPANSION_TYPE_FULL? > > the only difference is that 'static' expansion will not report properties > not mentioned in hard-codded CPU model definition see: builtin_x86_defs > > while 'full' will iterate over/report all rw properties of CPU object > created from provided model name. > >> I don't know :) >> >> Here's the documentation: >> >> ## >> # @CpuModelExpansionType: >> # >> # An enumeration of CPU model expansion types. >> # >> # @static: Expand to a static CPU model, a combination of a static >> # base model name and property delta changes. As the static base >> # model will never change, the expanded CPU model will be the >> # same, independent of QEMU version, machine type, machine >> # options, and accelerator options. Therefore, the resulting >> # model can be used by tooling without having to specify a >> # compatibility machine - e.g. when displaying the "host" model. >> # The @static CPU models are migration-safe. > > Looking at related x86 code above description sounds like a fiction. On s390x, which added that interface, that is how it's work. It resolves to "-base" models that are fixed for all eternity. x86-64 probably didn't adhere to the description because they were not interested in adding stable base models.
On Thu, 13 Feb 2025 17:40:56 +0800 bibo mao <maobibo@loongson.cn> wrote: > please discard this patch :( > > libvirt uses static type already on LoongArch platform, there is > compatibility issue. Static type cannot be removed, although I do > not know the difference between static type and full type. looking at loongarch-qmp-cmds.c:qmp_query_cpu_model_expansion(), you do filter reported features by global cpu_model_advertised_features[] list. Also cpu 'definition' is mainly governed by its initfn() and properties mentioned in cpu_model_advertised_features[] are created only by loongarch_cpu_post_init() , which is part of la464 and max models. The later call however mutates depending on accelerator, which sort of contradicts 'static' description and feets more to 'full' category. What I'd suggest doing is to support both i.e. do keep 'static' for compat reasons and run the same code for 'full' In nutshell drop if(type) check or explicitly allow both types. > > Regards > Bibo Mao > > On 2025/2/13 下午5:16, Bibo Mao wrote: > > With full type for query-cpu-model-expansion qmp command, it shows that > > it is not supported. For instance, > > query-cpu-model-expansion type=full model={"name":"max"} > > > > Here is is output, > > {"error": {"class": "GenericError", "desc": "The requested expansion type is not supported"}} > > > > Since full type is not supported and only static type is supported, Here > > replace static type with full type for command query-cpu-model-expansion. > > > > And there is result with this patch, > > {"return": {"model": {"name": "max", "props": {"lbt": true, "lasx": true, "pmu": true, "lsx": true}}}} > > > > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > > --- > > target/loongarch/loongarch-qmp-cmds.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c > > index 3fde5a5a20..429c6d35fd 100644 > > --- a/target/loongarch/loongarch-qmp-cmds.c > > +++ b/target/loongarch/loongarch-qmp-cmds.c > > @@ -56,7 +56,7 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, > > const char *name; > > int i; > > > > - if (type != CPU_MODEL_EXPANSION_TYPE_STATIC) { > > + if (type != CPU_MODEL_EXPANSION_TYPE_FULL) { > > error_setg(errp, "The requested expansion type is not supported"); > > return NULL; > > } > > > > base-commit: de278e54aefed143526174335f8286f7437d20be > > > >
On 2025/2/14 上午12:52, Igor Mammedov wrote: > On Thu, 13 Feb 2025 17:40:56 +0800 > bibo mao <maobibo@loongson.cn> wrote: > >> please discard this patch :( >> >> libvirt uses static type already on LoongArch platform, there is >> compatibility issue. Static type cannot be removed, although I do >> not know the difference between static type and full type. > > looking at loongarch-qmp-cmds.c:qmp_query_cpu_model_expansion(), > you do filter reported features by global cpu_model_advertised_features[] list. > Also cpu 'definition' is mainly governed by its initfn() and > properties mentioned in cpu_model_advertised_features[] are created > only by loongarch_cpu_post_init() , which is part of la464 and max models. > The later call however mutates depending on accelerator, which > sort of contradicts 'static' description and feets more to 'full' category. > > What I'd suggest doing is to support both > i.e. do keep 'static' for compat reasons and run the same code for 'full' That is a good idea. will keep static type the same with full type for the present, and implement real static type iff all basic features are implemented and compatible machine requirement is added in future. Regards Bibo Mao > > In nutshell drop if(type) check or explicitly allow both types. > >> >> Regards >> Bibo Mao >> >> On 2025/2/13 下午5:16, Bibo Mao wrote: >>> With full type for query-cpu-model-expansion qmp command, it shows that >>> it is not supported. For instance, >>> query-cpu-model-expansion type=full model={"name":"max"} >>> >>> Here is is output, >>> {"error": {"class": "GenericError", "desc": "The requested expansion type is not supported"}} >>> >>> Since full type is not supported and only static type is supported, Here >>> replace static type with full type for command query-cpu-model-expansion. >>> >>> And there is result with this patch, >>> {"return": {"model": {"name": "max", "props": {"lbt": true, "lasx": true, "pmu": true, "lsx": true}}}} >>> >>> Signed-off-by: Bibo Mao <maobibo@loongson.cn> >>> --- >>> target/loongarch/loongarch-qmp-cmds.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c >>> index 3fde5a5a20..429c6d35fd 100644 >>> --- a/target/loongarch/loongarch-qmp-cmds.c >>> +++ b/target/loongarch/loongarch-qmp-cmds.c >>> @@ -56,7 +56,7 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, >>> const char *name; >>> int i; >>> >>> - if (type != CPU_MODEL_EXPANSION_TYPE_STATIC) { > > >>> + if (type != CPU_MODEL_EXPANSION_TYPE_FULL) { >>> error_setg(errp, "The requested expansion type is not supported"); >>> return NULL; >>> } >>> >>> base-commit: de278e54aefed143526174335f8286f7437d20be >>> >> >>
David Hildenbrand <david@redhat.com> writes: > On 13.02.25 17:17, Igor Mammedov wrote: >> On Thu, 13 Feb 2025 12:48:30 +0100 >> Markus Armbruster <armbru@redhat.com> wrote: >> >>> gaosong <gaosong@loongson.cn> writes: >>> >>>> Cc: Markus >>>> >>>> hi, Markus >>>> >>>> What is the difference between CPU_MODEL_EXPANSION_TYPE_STATIC and >>>> CPU_MODEL_EXPANSION_TYPE_FULL? >> >> the only difference is that 'static' expansion will not report properties >> not mentioned in hard-codded CPU model definition see: builtin_x86_defs >> while 'full' will iterate over/report all rw properties of CPU object >> created from provided model name. >> >>> I don't know :) >>> >>> Here's the documentation: >>> >>> ## >>> # @CpuModelExpansionType: >>> # >>> # An enumeration of CPU model expansion types. >>> # >>> # @static: Expand to a static CPU model, a combination of a static >>> # base model name and property delta changes. As the static base >>> # model will never change, the expanded CPU model will be the >>> # same, independent of QEMU version, machine type, machine >>> # options, and accelerator options. Therefore, the resulting >>> # model can be used by tooling without having to specify a >>> # compatibility machine - e.g. when displaying the "host" model. >>> # The @static CPU models are migration-safe. >> >> Looking at related x86 code above description sounds like a fiction. > > On s390x, which added that interface, that is how it's work. > > It resolves to "-base" models that are fixed for all eternity. > > x86-64 probably didn't adhere to the description because they were not interested in adding stable base models. Looks like we made a mess. Having the same command work differently on different targets is less than ideal. I'm sure there were reasons both for the initial s390x design, and for x86-64 deviating from it later. I'm not qualified to debate this. Regardless, actual behavior should match documented behavior. If actual behavior depends on the target, then documentation needs to explain that in sufficient detail. Could we get this fixed, please?
diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c index 3fde5a5a20..429c6d35fd 100644 --- a/target/loongarch/loongarch-qmp-cmds.c +++ b/target/loongarch/loongarch-qmp-cmds.c @@ -56,7 +56,7 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, const char *name; int i; - if (type != CPU_MODEL_EXPANSION_TYPE_STATIC) { + if (type != CPU_MODEL_EXPANSION_TYPE_FULL) { error_setg(errp, "The requested expansion type is not supported"); return NULL; }
With full type for query-cpu-model-expansion qmp command, it shows that it is not supported. For instance, query-cpu-model-expansion type=full model={"name":"max"} Here is is output, {"error": {"class": "GenericError", "desc": "The requested expansion type is not supported"}} Since full type is not supported and only static type is supported, Here replace static type with full type for command query-cpu-model-expansion. And there is result with this patch, {"return": {"model": {"name": "max", "props": {"lbt": true, "lasx": true, "pmu": true, "lsx": true}}}} Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- target/loongarch/loongarch-qmp-cmds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: de278e54aefed143526174335f8286f7437d20be