| Message ID | 20190802122540.26385-4-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm/kvm: enable SVE in guests | expand |
On 8/2/19 5:25 AM, Andrew Jones wrote: > Note, certainly more features may be added to the list of > advertised features, e.g. 'vfp' and 'neon'. The only requirement > is that their property set accessors fail when invalid > configurations are detected. For vfp we would need something like > > set_vfp() > { > if (arm_feature(env, ARM_FEATURE_AARCH64) && > cpu->has_vfp != cpu->has_neon) > error("AArch64 CPUs must have both VFP and Neon or neither") > > in its set accessor, and the same for neon, rather than doing that > check at realize time, which isn't executed at qmp query time. How could this succeed? Either set_vfp or set_neon would be called first, at which point the two features are temporarily out of sync, but the error would trigger anyway. This would seem to require some separate "qmp validate" step that is processed after a collection of properties are set. I was about to say something about this being moot until someone actually wants to be able to disable vfp+neon on aarch64, but then... > +A note about CPU feature dependencies > +------------------------------------- > + > +It's possible for features to have dependencies on other features. I.e. > +it may be possible to change one feature at a time without error, but > +when attempting to change all features at once an error could occur > +depending on the order they are processed. It's also possible changing > +all at once doesn't generate an error, because a feature's dependencies > +are satisfied with other features, but the same feature cannot be changed > +independently without error. For these reasons callers should always > +attempt to make their desired changes all at once in order to ensure the > +collection is valid. ... this language makes me think that you've already encountered an ordering problem that might be better solved with a separate validate step? The actual code looks good. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 8/2/19 9:27 AM, Richard Henderson wrote: > On 8/2/19 5:25 AM, Andrew Jones wrote: >> Note, certainly more features may be added to the list of >> advertised features, e.g. 'vfp' and 'neon'. The only requirement >> is that their property set accessors fail when invalid >> configurations are detected. For vfp we would need something like >> >> set_vfp() >> { >> if (arm_feature(env, ARM_FEATURE_AARCH64) && >> cpu->has_vfp != cpu->has_neon) >> error("AArch64 CPUs must have both VFP and Neon or neither") >> >> in its set accessor, and the same for neon, rather than doing that >> check at realize time, which isn't executed at qmp query time. > > How could this succeed? Either set_vfp or set_neon would be called first, at > which point the two features are temporarily out of sync, but the error would > trigger anyway. > > This would seem to require some separate "qmp validate" step that is processed > after a collection of properties are set. > > I was about to say something about this being moot until someone actually wants > to be able to disable vfp+neon on aarch64, but then... > >> +A note about CPU feature dependencies >> +------------------------------------- >> + >> +It's possible for features to have dependencies on other features. I.e. >> +it may be possible to change one feature at a time without error, but >> +when attempting to change all features at once an error could occur >> +depending on the order they are processed. It's also possible changing >> +all at once doesn't generate an error, because a feature's dependencies >> +are satisfied with other features, but the same feature cannot be changed >> +independently without error. For these reasons callers should always >> +attempt to make their desired changes all at once in order to ensure the >> +collection is valid. > > ... this language makes me think that you've already encountered an ordering > problem that might be better solved with a separate validate step? It appears to me that we can handle both use cases with a single function to handle validation of the cross-dependent properties. It would need to be called at the beginning of arm_cpu_realizefn, for the case in which we are building a cpu that we wish to instantiate, and > + if (!err) { > + visit_check_struct(visitor, &err); > + } here, inside qmp_query_cpu_model_expansion for the query case. Looking at the validation code scattered across multiple functions, across 4 patches, convinces me that the code will be smaller and more readable if we consolidate them into a single validation function. r~
On Fri, Aug 02, 2019 at 06:28:51PM -0700, Richard Henderson wrote: > On 8/2/19 9:27 AM, Richard Henderson wrote: > > On 8/2/19 5:25 AM, Andrew Jones wrote: > >> Note, certainly more features may be added to the list of > >> advertised features, e.g. 'vfp' and 'neon'. The only requirement > >> is that their property set accessors fail when invalid > >> configurations are detected. For vfp we would need something like > >> > >> set_vfp() > >> { > >> if (arm_feature(env, ARM_FEATURE_AARCH64) && > >> cpu->has_vfp != cpu->has_neon) > >> error("AArch64 CPUs must have both VFP and Neon or neither") > >> > >> in its set accessor, and the same for neon, rather than doing that > >> check at realize time, which isn't executed at qmp query time. > > > > How could this succeed? Either set_vfp or set_neon would be called first, at > > which point the two features are temporarily out of sync, but the error would > > trigger anyway. > > > > This would seem to require some separate "qmp validate" step that is processed > > after a collection of properties are set. > > > > I was about to say something about this being moot until someone actually wants > > to be able to disable vfp+neon on aarch64, but then... > > > >> +A note about CPU feature dependencies > >> +------------------------------------- > >> + > >> +It's possible for features to have dependencies on other features. I.e. > >> +it may be possible to change one feature at a time without error, but > >> +when attempting to change all features at once an error could occur > >> +depending on the order they are processed. It's also possible changing > >> +all at once doesn't generate an error, because a feature's dependencies > >> +are satisfied with other features, but the same feature cannot be changed > >> +independently without error. For these reasons callers should always > >> +attempt to make their desired changes all at once in order to ensure the > >> +collection is valid. > > > > ... this language makes me think that you've already encountered an ordering > > problem that might be better solved with a separate validate step? > > It appears to me that we can handle both use cases with a single function to > handle validation of the cross-dependent properties. > > It would need to be called at the beginning of arm_cpu_realizefn, for the case > in which we are building a cpu that we wish to instantiate, and > > > + if (!err) { > > + visit_check_struct(visitor, &err); > > + } > > here, inside qmp_query_cpu_model_expansion for the query case. > > Looking at the validation code scattered across multiple functions, across 4 > patches, convinces me that the code will be smaller and more readable if we > consolidate them into a single validation function. > That's a reasonable suggestion. I do like having self-contained validation, self-contained, but when cross-dependencies arise, then it does make sense to have a master validation function, rather than interconnecting too much. That said, for this series we only enable the qmp query for aarch64, pmu, and sve* properties. aarch64 and pmu are independent, and thus self-contained, and I consider all sve* properties one big entity, so their validation is also self-contained. If we add vfp and neon, then indeed I was wrong with my suggestion in the commit message. They would need a later validation check. Should we just cross that bridge when we get there though? Or would you like me to see how that would work within this series? Thanks, drew
On 8/6/19 5:21 AM, Andrew Jones wrote: > That's a reasonable suggestion. I do like having self-contained > validation, self-contained, but when cross-dependencies arise, then > it does make sense to have a master validation function, rather > than interconnecting too much. That said, for this series we only > enable the qmp query for aarch64, pmu, and sve* properties. aarch64 > and pmu are independent, and thus self-contained... Agreed. > and I consider > all sve* properties one big entity, so their validation is also > self-contained. If we add vfp and neon, then indeed I was wrong > with my suggestion in the commit message. They would need a later > validation check. Should we just cross that bridge when we get there > though? Or would you like me to see how that would work within this > series? While the sve* properties are handled by one function, they are not handled as "one big entity". You examine then apply or diagnose the effects of sve384=on before you separately examine the effects of sve512=on. I think it would be easiest to merely record facts while processing sve<N> and sve-max-vq, with no side effects. Then in the common validation function see the required side effects and diagnose errors all at once. r~
On Wed, Aug 07, 2019 at 08:22:07AM -0700, Richard Henderson wrote: > On 8/6/19 5:21 AM, Andrew Jones wrote: > > That's a reasonable suggestion. I do like having self-contained > > validation, self-contained, but when cross-dependencies arise, then > > it does make sense to have a master validation function, rather > > than interconnecting too much. That said, for this series we only > > enable the qmp query for aarch64, pmu, and sve* properties. aarch64 > > and pmu are independent, and thus self-contained... > > Agreed. > > > and I consider > > all sve* properties one big entity, so their validation is also > > self-contained. If we add vfp and neon, then indeed I was wrong > > with my suggestion in the commit message. They would need a later > > validation check. Should we just cross that bridge when we get there > > though? Or would you like me to see how that would work within this > > series? > > While the sve* properties are handled by one function, they are not handled as > "one big entity". You examine then apply or diagnose the effects of sve384=on > before you separately examine the effects of sve512=on. > > I think it would be easiest to merely record facts while processing sve<N> and > sve-max-vq, with no side effects. Then in the common validation function see > the required side effects and diagnose errors all at once. > I'm not sure. Of course I'd need to experiment with it to be sure, but I'm reluctant to go through that exercise, because I believe that a deferred validation will result in less specific errors messages. For example, how would the validator know in which order the sve<N> properties were provided? Which is necessary to complain that one length is not allowed when another has already been disabled, or vice versa. Also with deferred validation I think I'd need more vq states, at least for when KVM is enabled. For example, if 384-bit vector lengths are not supported on the KVM host, but the user does sve384=on, and all we do is record that, then we've lost the KVM unsupported information and won't find out until we attempt to enable it later at kvm vcpu init time. We'd need a kvm-unsupported-user-enabled state to track that, which also means we're not blindly recording user input in cpu_arm_set_sve_vq() anymore, but are instead applying logic to decide which state we transition to. Thanks, drew
On 8/8/19 1:50 AM, Andrew Jones wrote: > I'm not sure. Of course I'd need to experiment with it to be sure, but > I'm reluctant to go through that exercise, because I believe that a > deferred validation will result in less specific errors messages. For > example, how would the validator know in which order the sve<N> properties > were provided? Which is necessary to complain that one length is not > allowed when another has already been disabled, or vice versa. My point is that we would not *need* to know in which order the properties are provided, and do not care. Indeed, removing this ordering malarkey is a feature not a bug. The error message would simply state, e.g., that sve256 may not be disabled while sve512 is enabled. The error message does not need to reference the order in which they appeared. > Also with deferred validation I think I'd need more vq states, at least > for when KVM is enabled. For example, if 384-bit vector lengths are not > supported on the KVM host, but the user does sve384=on, and all we do > is record that, then we've lost the KVM unsupported information and won't > find out until we attempt to enable it later at kvm vcpu init time. We'd > need a kvm-unsupported-user-enabled state to track that, which also means > we're not blindly recording user input in cpu_arm_set_sve_vq() anymore, > but are instead applying logic to decide which state we transition to. Or perhaps, less vq states. You do not need to compute kvm states early. You can wait to collect those until validation time and keep them in their own local bitmap. bool validate_sve_properties(...) { // Populate uninitialized bits in sve_vq_map from sve_max_vq. // Populate uninitialized bits in sve_vq_map from on bits in sve_vq_map. // All remaining uninitialized bits are set to off. // Reset sve_max_vq to the maximum bit set in sve_vq_map, plus 1. // Diagnose off bits in sve_vq_map from on bits in sve_vq_map. if (kvm_enabled()) { DECLARE_BITMAP(test, ARM_MAX_VQ); kvm_arm_sve_get_vls(CPU(cpu), test); if (!bitmap_equal(test, s->sve_vq_map, s->sve_max_vq)) { bitmap_xor(test, test, s->sve_vq_map, s->sve_max_vq); // Diagnose the differences in TEST: // test[i] & s->sve_vq_map[i] -> i is unsupported by kvm // test[i] & !s->sve_vq_map[i] -> i is required by kvm } } } If you prefer not to experiment with this, then I will. r~
On Thu, Aug 08, 2019 at 11:37:01AM -0700, Richard Henderson wrote: > On 8/8/19 1:50 AM, Andrew Jones wrote: > > I'm not sure. Of course I'd need to experiment with it to be sure, but > > I'm reluctant to go through that exercise, because I believe that a > > deferred validation will result in less specific errors messages. For > > example, how would the validator know in which order the sve<N> properties > > were provided? Which is necessary to complain that one length is not > > allowed when another has already been disabled, or vice versa. > > My point is that we would not *need* to know in which order the properties are > provided, and do not care. Indeed, removing this ordering malarkey is a > feature not a bug. > > The error message would simply state, e.g., that sve256 may not be disabled > while sve512 is enabled. The error message does not need to reference the > order in which they appeared. OK, I agree it doesn't make much difference to the user whether the error hint is the generic "sve256 required with sve512" verse sve256=off,sve512=on "cannot enable sve512 with sve256 disabled" or sve512=on,sve256=off "cannot disable sve256 with sve512 enabled" > > > Also with deferred validation I think I'd need more vq states, at least > > for when KVM is enabled. For example, if 384-bit vector lengths are not > > supported on the KVM host, but the user does sve384=on, and all we do > > is record that, then we've lost the KVM unsupported information and won't > > find out until we attempt to enable it later at kvm vcpu init time. We'd > > need a kvm-unsupported-user-enabled state to track that, which also means > > we're not blindly recording user input in cpu_arm_set_sve_vq() anymore, > > but are instead applying logic to decide which state we transition to. > > Or perhaps, less vq states. You do not need to compute kvm states early. You > can wait to collect those until validation time and keep them in their own > local bitmap. > > bool validate_sve_properties(...) > { > // Populate uninitialized bits in sve_vq_map from sve_max_vq. > // Populate uninitialized bits in sve_vq_map from on bits in sve_vq_map. And disable uninitialized bits in sve_vq_map from OFF bits: auto-disabling Also we can't do these populate uninitialized bits from on/off steps when KVM is enabled without first getting the kvm-supported map. > // All remaining uninitialized bits are set to off. > // Reset sve_max_vq to the maximum bit set in sve_vq_map, plus 1. I wouldn't always do this. If the user explicitly sets a maximum with sve-max-vq, then we should generate errors when other inputs would change that maximum. I would only assert they're the same when both input types are provided. Of course we do the above when only map input is provided though. > // Diagnose off bits in sve_vq_map from on bits in sve_vq_map. > > if (kvm_enabled()) { > DECLARE_BITMAP(test, ARM_MAX_VQ); > kvm_arm_sve_get_vls(CPU(cpu), test); > if (!bitmap_equal(test, s->sve_vq_map, s->sve_max_vq)) { > bitmap_xor(test, test, s->sve_vq_map, s->sve_max_vq); > // Diagnose the differences in TEST: > // test[i] & s->sve_vq_map[i] -> i is unsupported by kvm > // test[i] & !s->sve_vq_map[i] -> i is required by kvm > } > } > } > > If you prefer not to experiment with this, then I will. > Ah, the ol' I'll do it if you don't motivator... I do see some potential for a reduction in complexity with this approach, but I'm not sure we'll save many lines of code. I could do a quick hack on top of this series, just to see how well the validator function looks and works, but I can't get to it until midweek next week. I won't complain if you beat me to it :-) Thanks, drew
diff --git a/docs/arm-cpu-features.rst b/docs/arm-cpu-features.rst new file mode 100644 index 000000000000..c79dcffb5556 --- /dev/null +++ b/docs/arm-cpu-features.rst @@ -0,0 +1,137 @@ +================ +ARM CPU Features +================ + +Examples of probing and using ARM CPU features + +Introduction +============ + +CPU features are optional features that a CPU of supporting type may +choose to implement or not. In QEMU, optional CPU features have +corresponding boolean CPU proprieties that, when enabled, indicate +that the feature is implemented, and, conversely, when disabled, +indicate that it is not implemented. An example of an ARM CPU feature +is the Performance Monitoring Unit (PMU). CPU types such as the +Cortex-A15 and the Cortex-A57, which respectively implement ARM +architecture reference manuals ARMv7-A and ARMv8-A, may both optionally +implement PMUs. For example, if a user wants to use a Cortex-A15 without +a PMU, then the `-cpu` parameter should contain `pmu=off` on the QEMU +command line, i.e. `-cpu cortex-a15,pmu=off`. + +As not all CPU types support all optional CPU features, then whether or +not a CPU property exists depends on the CPU type. For example, CPUs +that implement the ARMv8-A architecture reference manual may optionally +support the AArch32 CPU feature, which may be enabled by disabling the +`aarch64` CPU property. A CPU type such as the Cortex-A15, which does +not implement ARMv8-A, will not have the `aarch64` CPU property. + +QEMU's support may be limited for some CPU features, only partially +supporting the feature or only supporting the feature under certain +configurations. For example, the `aarch64` CPU feature, which, when +disabled, enables the optional AArch32 CPU feature, is only supported +when using the KVM accelerator and when running on a host CPU type that +supports the feature. + +CPU Feature Probing +=================== + +Determining which CPU features are available and functional for a given +CPU type is possible with the `query-cpu-model-expansion` QMP command. +Below are some examples where `scripts/qmp/qmp-shell` (see the top comment +block in the script for usage) is used to issue the QMP commands. + +(1) Determine which CPU features are available for the `max` CPU type + (Note, we started QEMU with qemu-system-aarch64, so `max` is + implementing the ARMv8-A reference manual in this case):: + + (QEMU) query-cpu-model-expansion type=full model={"name":"max"} + { "return": { + "model": { "name": "max", "props": { + "pmu": true, "aarch64": true + }}}} + +We see that the `max` CPU type has the `pmu` and `aarch64` CPU features. +We also see that the CPU features are enabled, as they are all `true`. + +(2) Let's try to disable the PMU:: + + (QEMU) query-cpu-model-expansion type=full model={"name":"max","props":{"pmu":false}} + { "return": { + "model": { "name": "max", "props": { + "pmu": false, "aarch64": true + }}}} + +We see it worked, as `pmu` is now `false`. + +(3) Let's try to disable `aarch64`, which enables the AArch32 CPU feature:: + + (QEMU) query-cpu-model-expansion type=full model={"name":"max","props":{"aarch64":false}} + {"error": { + "class": "GenericError", "desc": + "'aarch64' feature cannot be disabled unless KVM is enabled and 32-bit EL1 is supported" + }} + +It looks like this feature is limited to a configuration we do not +currently have. + +(4) Let's try probing CPU features for the Cortex-A15 CPU type:: + + (QEMU) query-cpu-model-expansion type=full model={"name":"cortex-a15"} + {"return": {"model": {"name": "cortex-a15", "props": {"pmu": true}}}} + +Only the `pmu` CPU feature is available. + +A note about CPU feature dependencies +------------------------------------- + +It's possible for features to have dependencies on other features. I.e. +it may be possible to change one feature at a time without error, but +when attempting to change all features at once an error could occur +depending on the order they are processed. It's also possible changing +all at once doesn't generate an error, because a feature's dependencies +are satisfied with other features, but the same feature cannot be changed +independently without error. For these reasons callers should always +attempt to make their desired changes all at once in order to ensure the +collection is valid. + +A note about CPU models and KVM +------------------------------- + +Named CPU models generally do not work with KVM. There are a few cases +that do work, e.g. using the named CPU model `cortex-a57` with KVM on a +seattle host, but mostly if KVM is enabled the `host` CPU type must be +used. This means the guest is provided all the same CPU features as the +host CPU type has. And, for this reason, the `host` CPU type should +enable all CPU features that the host has by default. Indeed it's even +a bit strange to allow disabling CPU features that the host has when using +the `host` CPU type, but in the absence of CPU models it's the best we can +do if we want to launch guests without all the host's CPU features enabled. + +Enabling KVM also affects the `query-cpu-model-expansion` QMP command. The +affect is not only limited to specific features, as pointed out in example +(3) of "CPU Feature Probing", but also to which CPU types may be expanded. +When KVM is enabled, only the `max`, `host`, and current CPU type may be +expanded. This restriction is necessary as it's not possible to know all +CPU types that may work with KVM, but it does impose a small risk of users +experiencing unexpected errors. For example on a seattle, as mentioned +above, the `cortex-a57` CPU type is also valid when KVM is enabled. +Therefore a user could use the `host` CPU type for the current type, but +then attempt to query `cortex-a57`, however that query will fail with our +restrictions. This shouldn't be an issue though as management layers and +users have been preferring the `host` CPU type for use with KVM for quite +some time. Additionally, if the KVM-enabled QEMU instance running on a +seattle host is using the `cortex-a57` CPU type, then querying `cortex-a57` +will work. + +Using CPU Features +================== + +After determining which CPU features are available and supported for a +given CPU type, then they may be selectively enabled or disabled on the +QEMU command line with that CPU type:: + + $ qemu-system-aarch64 -M virt -cpu max,pmu=off + +The example above disables the PMU for the `max` CPU type. + diff --git a/qapi/machine-target.json b/qapi/machine-target.json index 55310a6aa226..04623224720d 100644 --- a/qapi/machine-target.json +++ b/qapi/machine-target.json @@ -212,7 +212,7 @@ ## { 'struct': 'CpuModelExpansionInfo', 'data': { 'model': 'CpuModelInfo' }, - 'if': 'defined(TARGET_S390X) || defined(TARGET_I386)' } + 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) || defined(TARGET_ARM)' } ## # @query-cpu-model-expansion: @@ -237,7 +237,7 @@ # query-cpu-model-expansion while using these is not advised. # # Some architectures may not support all expansion types. s390x supports -# "full" and "static". +# "full" and "static". Arm only supports "full". # # Returns: a CpuModelExpansionInfo. Returns an error if expanding CPU models is # not supported, if the model cannot be expanded, if the model contains @@ -251,7 +251,7 @@ 'data': { 'type': 'CpuModelExpansionType', 'model': 'CpuModelInfo' }, 'returns': 'CpuModelExpansionInfo', - 'if': 'defined(TARGET_S390X) || defined(TARGET_I386)' } + 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) || defined(TARGET_ARM)' } ## # @CpuDefinitionInfo: diff --git a/target/arm/monitor.c b/target/arm/monitor.c index 6ec6dd04ac2e..96ada22d9cc7 100644 --- a/target/arm/monitor.c +++ b/target/arm/monitor.c @@ -23,7 +23,14 @@ #include "qemu/osdep.h" #include "hw/boards.h" #include "kvm_arm.h" +#include "qapi/error.h" +#include "qapi/visitor.h" +#include "qapi/qobject-input-visitor.h" +#include "qapi/qapi-commands-machine-target.h" #include "qapi/qapi-commands-misc-target.h" +#include "qapi/qmp/qerror.h" +#include "qapi/qmp/qdict.h" +#include "qom/qom-qobject.h" static GICCapability *gic_cap_new(int version) { @@ -82,3 +89,134 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp) return head; } + +static const char *cpu_model_advertised_features[] = { + "aarch64", "pmu", + NULL +}; + +CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, + CpuModelInfo *model, + Error **errp) +{ + CpuModelExpansionInfo *expansion_info; + const QDict *qdict_in = NULL; + QDict *qdict_out; + ObjectClass *oc; + Object *obj; + const char *name; + int i; + + if (type != CPU_MODEL_EXPANSION_TYPE_FULL) { + error_setg(errp, "The requested expansion type is not supported"); + return NULL; + } + + if (!kvm_enabled() && !strcmp(model->name, "host")) { + error_setg(errp, "The CPU type '%s' requires KVM", model->name); + return NULL; + } + + oc = cpu_class_by_name(TYPE_ARM_CPU, model->name); + if (!oc) { + error_setg(errp, "The CPU type '%s' is not a recognized ARM CPU type", + model->name); + return NULL; + } + + if (kvm_enabled()) { + const char *cpu_type = current_machine->cpu_type; + int len = strlen(cpu_type) - strlen(ARM_CPU_TYPE_SUFFIX); + bool supported = false; + + if (!strcmp(model->name, "host") || !strcmp(model->name, "max")) { + /* These are kvmarm's recommended cpu types */ + supported = true; + } else if (strlen(model->name) == len && + !strncmp(model->name, cpu_type, len)) { + /* KVM is enabled and we're using this type, so it works. */ + supported = true; + } + if (!supported) { + error_setg(errp, "We cannot guarantee the CPU type '%s' works " + "with KVM on this host", model->name); + return NULL; + } + } + + if (model->props) { + qdict_in = qobject_to(QDict, model->props); + if (!qdict_in) { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict"); + return NULL; + } + } + + obj = object_new(object_class_get_name(oc)); + + if (qdict_in) { + Visitor *visitor; + Error *err = NULL; + + visitor = qobject_input_visitor_new(model->props); + visit_start_struct(visitor, NULL, NULL, 0, &err); + if (err) { + object_unref(obj); + error_propagate(errp, err); + return NULL; + } + + i = 0; + while ((name = cpu_model_advertised_features[i++]) != NULL) { + if (qdict_get(qdict_in, name)) { + object_property_set(obj, visitor, name, &err); + if (err) { + break; + } + } + } + + if (!err) { + visit_check_struct(visitor, &err); + } + visit_end_struct(visitor, NULL); + visit_free(visitor); + if (err) { + object_unref(obj); + error_propagate(errp, err); + return NULL; + } + } + + expansion_info = g_new0(CpuModelExpansionInfo, 1); + expansion_info->model = g_malloc0(sizeof(*expansion_info->model)); + expansion_info->model->name = g_strdup(model->name); + + qdict_out = qdict_new(); + + i = 0; + while ((name = cpu_model_advertised_features[i++]) != NULL) { + ObjectProperty *prop = object_property_find(obj, name, NULL); + if (prop) { + Error *err = NULL; + QObject *value; + + assert(prop->get); + value = object_property_get_qobject(obj, name, &err); + assert(!err); + + qdict_put_obj(qdict_out, name, value); + } + } + + if (!qdict_size(qdict_out)) { + qobject_unref(qdict_out); + } else { + expansion_info->model->props = QOBJECT(qdict_out); + expansion_info->model->has_props = true; + } + + object_unref(obj); + + return expansion_info; +}
Add support for the query-cpu-model-expansion QMP command to Arm. We do this selectively, only exposing CPU properties which represent optional CPU features which the user may want to enable/disable. Additionally we restrict the list of queryable cpu models to 'max', 'host', or the current type when KVM is in use. And, finally, we only implement expansion type 'full', as Arm does not yet have a "base" CPU type. More details and example queries are described in a new document (docs/arm-cpu-features.rst). Note, certainly more features may be added to the list of advertised features, e.g. 'vfp' and 'neon'. The only requirement is that their property set accessors fail when invalid configurations are detected. For vfp we would need something like set_vfp() { if (arm_feature(env, ARM_FEATURE_AARCH64) && cpu->has_vfp != cpu->has_neon) error("AArch64 CPUs must have both VFP and Neon or neither") in its set accessor, and the same for neon, rather than doing that check at realize time, which isn't executed at qmp query time. Signed-off-by: Andrew Jones <drjones@redhat.com> --- docs/arm-cpu-features.rst | 137 +++++++++++++++++++++++++++++++++++++ qapi/machine-target.json | 6 +- target/arm/monitor.c | 138 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 3 deletions(-) create mode 100644 docs/arm-cpu-features.rst