Re: [RFC PATCH v7 18/18] arm-qmp-cmds: introspection for ID register props

[Markus Armbruster <armbru@redhat.com>]

Eric Auger <eric.auger@redhat.com> writes:

> Hi Markus,
>
> On 8/20/26 1:50 PM, Markus Armbruster wrote:
>> Eric Auger <eric.auger@redhat.com> writes:
>>
>>> From: Cornelia Huck <cohuck@redhat.com>
>>>
>>> Implement the capability to query available ID register values by
>>> adding SYSREG_* options and values to the cpu model expansion for the
>>> host model, if available.
>>>
>>> Excerpt:
>>> (QEMU) query-cpu-model-expansion type=full model={"name":"host"}
>>
>> Is this qmp-shell?
> yes it is. Is that a problem?

I wouldn't call use of qmp-shell a problem.  Except some reader may not
recognize it from the example input, or may not even know it exists.
Mentioning it takes care of all that.

I prefer to use QMP directly myself, like this:

    $ socat "READLINE,history=$HOME/.qmp_history,prompt=QMP> " UNIX-CONNECT:/path/to/socket

But that's just personal preference.

>>> {"return": {"model": {"name": "host", "props": {"SYSREG_ID_AA64PFR0_EL1_EL3": 1,
>>> "SYSREG_ID_AA64ISAR2_EL1_CLRBHB": 0, "SYSREG_CTR_EL0_L1Ip": 3,
>>> "SYSREG_MIDR_EL1_PartNum": 3407, "SYSREG_CTR_EL0_DminLine": 4,
>>> "SYSREG_ID_AA64MMFR0_EL1_PARange": 5, "SYSREG_ID_AA64MMFR1_EL1_ECBHB": 0
>>> ../..
>>>
>>> This allows the upper stack to detect available writable ID regs and
>>> the "host passthrough model" values.
>>>
>>> It also allows to test some ID reg field values:
>>> (QEMU) query-cpu-model-expansion type=full model={"name":"host","props":{"SYSREG_ID_AA64ISAR0_EL1_DP":0x13}}
>>> {"error": {"class": "GenericError", "desc": "idreg SYSREG_ID_AA64ISAR0_EL1_DP set value (0x13) exceeds length of field (4)!"}}
>>>
>>> (QEMU) query-cpu-model-expansion type=full model={"name":"host","props":{"SYSREG_ID_AA64ISAR0_EL1_DP":0x2}}
>>> {"error": {"class": "GenericError", "desc": "idreg SYSREG_ID_AA64ISAR0_EL1_DP set value (0x2) does not match any arch valid enum value!"}}
>>
>> The examples use CPU "host".  Inconvenient for me, because I run into
>> "The CPU type 'host' requires KVM" when I try to play with them myself.
>> If you don't actually need "host" here, consider using something that
>> works more widely, just to help reviewers.
> It only applies to the host model, that's the reason. Extending this to
> TCG models is theorerically feasible but would induce huge changes
> without much benefits I think. With TCG you already have named vcpu
> models you can migrate as is. The purpose of this series is to allow
> tweaking the host passthrough model to allow migration between
> differents hosts.

Thanks!

>>> The following checks are performed:
>>> - the value does not exceed the field size
>>> - if the field is associated to enum values, the set value must be one
>>>   of those enum values
>>> - the value is applied against a scratch vcpu making sure the setting is
>>>   not rejected for this host
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>>
>>> ---
>>>
>>> v5 -> v6:
>>> - add the write capability
>>> ---
>>>  target/arm/arm-qmp-cmds.c | 98 +++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 98 insertions(+)
>>>
>>> diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
>>> index 83ec95c290..41fefadcea 100644
>>> --- a/target/arm/arm-qmp-cmds.c
>>> +++ b/target/arm/arm-qmp-cmds.c
>>> @@ -21,6 +21,7 @@
>>>   */
>>>  
>>>  #include "qemu/osdep.h"
>>> +#include "qemu/error-report.h"
>>>  #include "qemu/target-info.h"
>>>  #include "hw/core/boards.h"
>>>  #include "kvm_arm.h"
>>> @@ -30,7 +31,10 @@
>>>  #include "qapi/qapi-commands-machine.h"
>>>  #include "qapi/qapi-commands-misc-arm.h"
>>>  #include "qobject/qdict.h"
>>> +#include "qobject/qnum.h"
>>>  #include "qom/qom-qobject.h"
>>> +#include <linux/kvm.h>
>>> +#include "system/kvm.h"
>>>  #include "cpu.h"
>>>  
>>>  static GICCapability *gic_cap_new(int version)
>>> @@ -84,11 +88,14 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>>>                                                       Error **errp)
>>>  {
>>>      CpuModelExpansionInfo *expansion_info;
>>> +    ObjectPropertyIterator iter;
>>>      const QDict *qdict_in;
>>> +    ObjectProperty *idregprop;
>>>      QDict *qdict_out;
>>>      ObjectClass *oc;
>>>      Object *obj;
>>>      const char *name;
>>> +    int fdarray[3];
>>>      int i;
>>>  
>>>      if (type != CPU_MODEL_EXPANSION_TYPE_FULL) {
>>> @@ -133,6 +140,38 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>>>  
>>>      obj = object_new(object_class_get_name(oc));
>>>  
>>> +    if (kvm_enabled()) {
>>> +        bool pmuv3_supported = kvm_check_extension(kvm_state, KVM_CAP_ARM_PMU_V3);
>>> +        bool sve_supported = kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE);
>>> +        struct kvm_vcpu_init init = { .target = -1, };
>>> +        bool el2_supported = kvm_arm_el2_supported();
>>> +        bool pauth_supported;
>>> +        int ret;
>>> +
>>> +        pauth_supported = kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
>>> +                          kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC);
>>> +
>>> +        if (sve_supported) {
>>> +            init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
>>> +        }
>>> +        if (el2_supported) {
>>> +            init.features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>>> +        }
>>> +        if (pauth_supported) {
>>> +            init.features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
>>> +                             1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
>>> +        }
>>> +        if (pmuv3_supported) {
>>> +            init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
>>> +        }
>>> +
>>> +        ret = kvm_arm_create_scratch_host_vcpu(fdarray, &init);
>>> +        if (!ret) {
>>> +            error_setg(errp, "failing creating a scratch vcpu");
>>
>> This error message feels off.  I'd use something like "can't create
>> FOO".  Still bad, because it provides no clue on why.  What are the
>> possible failure modes?
> OK. There are quite a lot of possible failures which are handled in
> kvm_arm_create_scratch_host_vcpu()
> opening of /dev/kvm, KVM_CREATE_VM, KVM_CREATE_VCPU,
> KVM_ARM_PREFERRED_TARGET, KVM_ARM_VCPU_INIT.
> In next version I will add an Error handle to
> kvm_arm_create_scratch_host_vcpu() so that precise error message gets
> returned.

Makes sense.

[...]