This new cpu model takes by default the host cpu values.
However it exposes uint64 SYSREG properties for writable ID reg
fields exposed by the host kernel. Properties are named
SYSREG_<REG>_<FIELD> with REG and FIELD being those used
in linux arch/arm64/tools/sysreg. This done by matching the
writable fields retrieved from the host kernel against the
generated description of sysregs.

An example of invocation is:
-cpu custom,SYSREG_ID_AA64ISAR0_EL1_DP=0x0
which sets DP field of ID_AA64ISAR0_EL1 to 0.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>

---

At the moment, the custom model does not support legacy options
of the host cpu model. We need to understand what we do with those
latter (SVE, ...). This means that related KVM ioctl are
not called yet.
---
 target/arm/cpu.c        |  15 ++++
 target/arm/cpu64.c      | 153 ++++++++++++++++++++++++++++++++++++++++
 target/arm/trace-events |   6 ++
 3 files changed, 174 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 454d546feb..e5ac3c3e75 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1990,6 +1990,21 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
+    /*
+     * If we failed to retrieve the set of writable ID registers for a "custom"
+     * CPU model, report it here.
+     * In case we did get the set of writable ID registers, set the features to
+     * the configured values here and perform some sanity checks.
+     */
+    if (cpu->writable_id_regs == WRITABLE_ID_REGS_NOT_DISCOVERABLE) {
+        error_setg(errp, "Host kernel does not support discovering "
+                         "writable id registers");
+        return;
+    } else if (cpu->writable_id_regs == WRITABLE_ID_REGS_FAILED) {
+        error_setg(errp, "Failed to discover writable id registers");
+        return;
+    }
+
     if (!cpu->gt_cntfrq_hz) {
         /*
          * 0 means "the board didn't set a value, use the default". (We also
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 86b0797d4b..f10cc4ef8f 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "cpu.h"
 #include "cpregs.h"
 #include "qemu/module.h"
@@ -35,6 +36,8 @@
 #include "cpu-features.h"
 #include "cpregs.h"
 #include "cpu-custom.h"
+#include "cpu-sysregs.h"
+#include "trace.h"
 
 void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
 {
@@ -742,6 +745,153 @@ static void aarch64_max_initfn(Object *obj)
     }
 }
 
+#ifdef CONFIG_KVM
+
+static ARM64SysRegField *get_field(int i, ARM64SysReg *reg)
+{
+    GList *l;
+
+    for (l = reg->fields; l; l = l->next) {
+        ARM64SysRegField *field = (ARM64SysRegField *)l->data;
+
+        if (i >= field->lower && i <= field->upper) {
+            return field;
+        }
+    }
+    return NULL;
+}
+
+static void set_sysreg_prop(Object *obj, Visitor *v,
+                            const char *name, void *opaque,
+                            Error **errp)
+{
+    ARM64SysRegField *field = (ARM64SysRegField *)opaque;
+    ARMCPU *cpu = ARM_CPU(obj);
+    IdRegMap *idregs = &cpu->isar.idregs;
+    uint64_t old, value, mask;
+    int lower = field->lower;
+    int upper = field->upper;
+    int length = upper - lower + 1;
+    int index = field->index;
+
+    if (!visit_type_uint64(v, name, &value, errp)) {
+        return;
+    }
+
+    if (length < 64 && value > ((1 << length) - 1)) {
+        error_setg(errp,
+                   "idreg %s set value (0x%lx) exceeds length of field (%d)!",
+                   name, value, length);
+        return;
+    }
+
+    mask = MAKE_64BIT_MASK(lower, length);
+    value = value << lower;
+    old = idregs->regs[index];
+    idregs->regs[index] = old & ~mask;
+    idregs->regs[index] |= value;
+    trace_set_sysreg_prop(name, old, mask, value, idregs->regs[index]);
+}
+
+static void get_sysreg_prop(Object *obj, Visitor *v,
+                            const char *name, void *opaque,
+                            Error **errp)
+{
+    ARM64SysRegField *field = (ARM64SysRegField *)opaque;
+    ARMCPU *cpu = ARM_CPU(obj);
+    IdRegMap *idregs = &cpu->isar.idregs;
+    int index = field->index;
+
+    error_report("%s %s", __func__, name);
+    visit_type_uint64(v, name, &idregs->regs[index], errp);
+    trace_get_sysreg_prop(name, idregs->regs[index]);
+}
+
+/*
+ * decode_idreg_writemap: Generate props for writable fields
+ *
+ * @obj: CPU object
+ * @index: index of the sysreg
+ * @map: writable map for the sysreg
+ * @reg: description of the sysreg
+ */
+static int
+decode_idreg_writemap(Object *obj, int index, uint64_t map, ARM64SysReg *reg)
+{
+    int i = ctz64(map);
+    int nb_sysreg_props = 0;
+
+    while (map) {
+        ARM64SysRegField *field = get_field(i, reg);
+        int lower, upper;
+        uint64_t mask;
+        char *prop_name;
+
+        if (!field) {
+            /* the field cannot be matched to any know id named field */
+            warn_report("%s bit %d of %s is writable but cannot be matched",
+                        __func__, i, reg->name);
+            warn_report("%s is cpu-sysreg-properties.c up to date?", __func__);
+            map =  map & ~BIT_ULL(i);
+            i = ctz64(map);
+            continue;
+        }
+        lower = field->lower;
+        upper = field->upper;
+        prop_name = g_strdup_printf("SYSREG_%s_%s", reg->name, field->name);
+        trace_decode_idreg_writemap(field->name, lower, upper, prop_name);
+        object_property_add(obj, prop_name, "uint64",
+                            get_sysreg_prop, set_sysreg_prop, NULL, field);
+        nb_sysreg_props++;
+
+        mask = MAKE_64BIT_MASK(lower, upper - lower + 1);
+        map = map & ~mask;
+        i = ctz64(map);
+    }
+    trace_nb_sysreg_props(reg->name, nb_sysreg_props);
+    return 0;
+}
+
+/* analyze the writable mask and generate properties for writable fields */
+static int expose_idreg_properties(Object *obj, IdRegMap *map,
+                                   ARM64SysReg *regs)
+{
+    int i;
+
+    for (i = 0; i < NR_ID_REGS; i++) {
+        uint64_t mask = map->regs[i];
+
+        if (mask) {
+            /* reg @i has some writable fields, decode them */
+            decode_idreg_writemap(obj, i, mask, &regs[i]);
+        }
+    }
+    return 0;
+}
+
+static void aarch64_customcpu_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    int ret;
+
+    cpu->writable_map = g_malloc(sizeof(IdRegMap));
+
+    /* discover via KVM_ARM_GET_REG_WRITABLE_MASKS */
+    ret = kvm_arm_get_writable_id_regs(cpu, cpu->writable_map);
+    if (ret) {
+        /* function will have marked an error */
+        return;
+    }
+
+    /* populate from the host (exhaustive) , validate during realize */
+    kvm_arm_set_cpu_features_from_host(cpu, true);
+
+    /* generate SYSREG properties according to writable masks */
+    expose_idreg_properties(obj, cpu->writable_map, arm64_id_regs);
+}
+
+#endif
+
 static const ARMCPUInfo aarch64_cpus[] = {
     { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
     { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
@@ -749,6 +899,9 @@ static const ARMCPUInfo aarch64_cpus[] = {
 #if defined(CONFIG_KVM) || defined(CONFIG_HVF)
     { .name = "host",               .initfn = aarch64_host_initfn },
 #endif
+#ifdef CONFIG_KVM
+    { .name = "custom",             .initfn = aarch64_customcpu_initfn },
+#endif
 };
 
 static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp)
diff --git a/target/arm/trace-events b/target/arm/trace-events
index 668acf94ab..1b4bd5ab14 100644
--- a/target/arm/trace-events
+++ b/target/arm/trace-events
@@ -15,3 +15,9 @@ arm_gt_update_irq(int timer, int irqstate) "gt_update_irq: timer %d irqstate %d"
 kvm_arm_fixup_msi_route(uint64_t iova, uint64_t gpa) "MSI iova = 0x%"PRIx64" is translated into 0x%"PRIx64
 get_host_cpu_idregs(const char *name, uint64_t value) "scratch vcpu gost value for %s is 0x%"PRIx64
 kvm_arm_writable_idregs_to_cpreg_list(const char *name, uint64_t previous, uint64_t new) "%s overwrite default 0x%"PRIx64" with 0x%"PRIx64
+
+# cpu64.c
+decode_idreg_writemap(const char* name, int lower, int upper, char *prop_name) "%s [%d:%d] is writable (prop %s)"
+get_sysreg_prop(const char *name, uint64_t value) "%s 0x%"PRIx64
+set_sysreg_prop(const char *name, uint64_t old, uint64_t mask, uint64_t field_value, uint64_t new) "%s old reg value=0x%"PRIx64" mask=0x%"PRIx64" new field value=0x%"PRIx64" new reg value=0x%"PRIx64
+nb_sysreg_props(const char *name, int count) "%s: %d SYSREG properties"
-- 
2.41.0

On Fri, Oct 25, 2024 at 12:17:37PM +0200, Eric Auger wrote:
> This new cpu model takes by default the host cpu values.
> However it exposes uint64 SYSREG properties for writable ID reg
> fields exposed by the host kernel. Properties are named
> SYSREG_<REG>_<FIELD> with REG and FIELD being those used
> in linux arch/arm64/tools/sysreg. This done by matching the
> writable fields retrieved from the host kernel against the
> generated description of sysregs.
> 
> An example of invocation is:
> -cpu custom,SYSREG_ID_AA64ISAR0_EL1_DP=0x0
> which sets DP field of ID_AA64ISAR0_EL1 to 0.

"SYSREG_" feels kinda redundant to repeat on every single
feature. 

Also, is this naming convention really the same one that users
will see when they look at /proc/cpuinfo to view features ? It
feels pretty low level to me ?  Naming after the registers &
fields, would be like configuring x86 CPU features by asking
for "SYSREG_EAX_1_ECX_20" instead of saying "vmx" which is the
human friendly name.


> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> 
> ---
> 
> At the moment, the custom model does not support legacy options
> of the host cpu model. We need to understand what we do with those
> latter (SVE, ...). This means that related KVM ioctl are
> not called yet.

It will be pretty painful to have to use different feature
terminology for different CPU models. Everything in libvirt
assuming feature terminology varies per-arch, not per-CPU
model.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Hi Daniel,

On 10/25/24 15:06, Daniel P. Berrangé wrote:
> On Fri, Oct 25, 2024 at 12:17:37PM +0200, Eric Auger wrote:
>> This new cpu model takes by default the host cpu values.
>> However it exposes uint64 SYSREG properties for writable ID reg
>> fields exposed by the host kernel. Properties are named
>> SYSREG_<REG>_<FIELD> with REG and FIELD being those used
>> in linux arch/arm64/tools/sysreg. This done by matching the
>> writable fields retrieved from the host kernel against the
>> generated description of sysregs.
>>
>> An example of invocation is:
>> -cpu custom,SYSREG_ID_AA64ISAR0_EL1_DP=0x0
>> which sets DP field of ID_AA64ISAR0_EL1 to 0.
> "SYSREG_" feels kinda redundant to repeat on every single
> feature. 

I do agree. To be honest this was mostly driven my implementation need
for cpu model expansion. Given the high number of props which are
getting exposed, I iterate on all props and having a prefix let me
return only those SYSREG props. Most probably we can get rid of the
prefix by using some generated code as well.
>
> Also, is this naming convention really the same one that users
> will see when they look at /proc/cpuinfo to view features ? It
No it is not. I do agree that the custom cpu model is very low level. It
is very well suited to test all series turning ID regs as writable but
this would require an extra layer that adapts /proc/cpuinfo feature
level to this regid/field abstraction.

In /cpu/proc you will see somethink like:
 Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
asimdhp cpuid asimdrdm lrcpc dcpop asimddp
> feels pretty low level to me ?  Naming after the registers &
> fields, would be like configuring x86 CPU features by asking
> for "SYSREG_EAX_1_ECX_20" instead of saying "vmx" which is the
> human friendly name.
agreed.
>
>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>
>> ---
>>
>> At the moment, the custom model does not support legacy options
>> of the host cpu model. We need to understand what we do with those
>> latter (SVE, ...). This means that related KVM ioctl are
>> not called yet.
> It will be pretty painful to have to use different feature
> terminology for different CPU models. Everything in libvirt
> assuming feature terminology varies per-arch, not per-CPU
> model.
Actually as far as I understand those regids/fields would fit all kind
of aarch64 Cortex-A CPUs. So they wouldn't vary per-CPU (I mean their
terminology. Their availability will).

Thanks

Eric
>
>
> With regards,
> Daniel

On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
> Hi Daniel,
> 
> On 10/25/24 15:06, Daniel P. Berrangé wrote:
> > On Fri, Oct 25, 2024 at 12:17:37PM +0200, Eric Auger wrote:
> >> This new cpu model takes by default the host cpu values.
> >> However it exposes uint64 SYSREG properties for writable ID reg
> >> fields exposed by the host kernel. Properties are named
> >> SYSREG_<REG>_<FIELD> with REG and FIELD being those used
> >> in linux arch/arm64/tools/sysreg. This done by matching the
> >> writable fields retrieved from the host kernel against the
> >> generated description of sysregs.
> >>
> >> An example of invocation is:
> >> -cpu custom,SYSREG_ID_AA64ISAR0_EL1_DP=0x0
> >> which sets DP field of ID_AA64ISAR0_EL1 to 0.
> > "SYSREG_" feels kinda redundant to repeat on every single
> > feature. 
> 
> I do agree. To be honest this was mostly driven my implementation need
> for cpu model expansion. Given the high number of props which are
> getting exposed, I iterate on all props and having a prefix let me
> return only those SYSREG props. Most probably we can get rid of the
> prefix by using some generated code as well.
> >
> > Also, is this naming convention really the same one that users
> > will see when they look at /proc/cpuinfo to view features ? It
> No it is not. I do agree that the custom cpu model is very low level. It
> is very well suited to test all series turning ID regs as writable but
> this would require an extra layer that adapts /proc/cpuinfo feature
> level to this regid/field abstraction.
> 
> In /cpu/proc you will see somethink like:
>  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
> asimdhp cpuid asimdrdm lrcpc dcpop asimddp

Right, IMHO, this is the terminology that QEMU must use in user
facing APIs.

On x86 we have a conversion tables for named features to register
bits

  https://gitlab.com/qemu-project/qemu/-/blob/master/target/i386/cpu.c#L914

and libvirt does similar

  https://gitlab.com/libvirt/libvirt/-/blob/master/src/cpu_map/x86_features.xml

Yep, it is more work, but it is also much better for humans IMHO.

> > feels pretty low level to me ?  Naming after the registers &
> > fields, would be like configuring x86 CPU features by asking
> > for "SYSREG_EAX_1_ECX_20" instead of saying "vmx" which is the
> > human friendly name.
> agreed.

> >> At the moment, the custom model does not support legacy options
> >> of the host cpu model. We need to understand what we do with those
> >> latter (SVE, ...). This means that related KVM ioctl are
> >> not called yet.
> > It will be pretty painful to have to use different feature
> > terminology for different CPU models. Everything in libvirt
> > assuming feature terminology varies per-arch, not per-CPU
> > model.
> Actually as far as I understand those regids/fields would fit all kind
> of aarch64 Cortex-A CPUs. So they wouldn't vary per-CPU (I mean their
> terminology. Their availability will).

What I mean is can we define  named models for various different
vendor's Cortex-A silicon and just use that without needing to
toggle features, except in rare cases.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
> > On 10/25/24 15:06, Daniel P. Berrangé wrote:
> > > Also, is this naming convention really the same one that users
> > > will see when they look at /proc/cpuinfo to view features ? It
> > No it is not. I do agree that the custom cpu model is very low level. It
> > is very well suited to test all series turning ID regs as writable but
> > this would require an extra layer that adapts /proc/cpuinfo feature
> > level to this regid/field abstraction.
> >
> > In /cpu/proc you will see somethink like:
> >  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
> > asimdhp cpuid asimdrdm lrcpc dcpop asimddp
>
> Right, IMHO, this is the terminology that QEMU must use in user
> facing APIs.

/proc/cpuinfo's naming is rather weird for historical
reasons (for instance there is only one FEAT_FP16 feature
but cpuinfo lists "fphp" and "asimdhp" separately).
Currently QEMU only has to care about cpuinfo name tags
in linux-user/elfload.c where there's a bunch of data
structures for "what hwcaps do we need to advertise
given what the CPU has?". I would definitely prefer it if
we could use architectural feature names...

On other architectures do we do anything to forbid
invalid combinations? For Arm there are architectural
rules about feature X requiring features Y and Z.
Are we going to just let the user create a CPU that
the guest OS will barf on if they want to? (The
user-experience for that is potentially not very nice,
because something like "-cpu cortex-a57,+sve" seems like
something you might want to do but isn't actually valid;
even listing the direct required dependency of FEAT_SVE
like "-cpu cortex-a57,+sve,+fp16" isn't sufficient
because SVE is optional-from-v8.2 and so a guest could
in theory assume the presence of anything mandatory in
v8.1 and v8.2. But even merely diagnosing invalid
combinations is a huge pain, and automagically pulling
in every mandatory implicit or explicit dependency
seems like it might be surprising to users, as well as
being annoying to implement. Currently we sidestep
all of these problems by (a) only allowing the command
line to disable a feature, not to enable it where it
didn't previously exist and (b) mostly not providing
command line flags for individual features...)

thanks
-- PMM

On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
> On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
> > > On 10/25/24 15:06, Daniel P. Berrangé wrote:
> > > > Also, is this naming convention really the same one that users
> > > > will see when they look at /proc/cpuinfo to view features ? It
> > > No it is not. I do agree that the custom cpu model is very low level. It
> > > is very well suited to test all series turning ID regs as writable but
> > > this would require an extra layer that adapts /proc/cpuinfo feature
> > > level to this regid/field abstraction.
> > >
> > > In /cpu/proc you will see somethink like:
> > >  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
> > > asimdhp cpuid asimdrdm lrcpc dcpop asimddp
> >
> > Right, IMHO, this is the terminology that QEMU must use in user
> > facing APIs.
> 
> /proc/cpuinfo's naming is rather weird for historical
> reasons (for instance there is only one FEAT_FP16 feature
> but cpuinfo lists "fphp" and "asimdhp" separately).

There's plenty of wierd history in x86 too. In this
case I might suggest just picking one of the two
common names, and ignoring the other.

If we really wanted to, we could alias the 2nd name
to the first, but its likely not worth the bother.

> Currently QEMU only has to care about cpuinfo name tags
> in linux-user/elfload.c where there's a bunch of data
> structures for "what hwcaps do we need to advertise
> given what the CPU has?". I would definitely prefer it if
> we could use architectural feature names...
> 
> On other architectures do we do anything to forbid
> invalid combinations? For Arm there are architectural
> rules about feature X requiring features Y and Z.
> Are we going to just let the user create a CPU that
> the guest OS will barf on if they want to? (The
> user-experience for that is potentially not very nice,
> because something like "-cpu cortex-a57,+sve" seems like
> something you might want to do but isn't actually valid;
> even listing the direct required dependency of FEAT_SVE
> like "-cpu cortex-a57,+sve,+fp16" isn't sufficient
> because SVE is optional-from-v8.2 and so a guest could
> in theory assume the presence of anything mandatory in
> v8.1 and v8.2. But even merely diagnosing invalid
> combinations is a huge pain, and automagically pulling
> in every mandatory implicit or explicit dependency
> seems like it might be surprising to users, as well as
> being annoying to implement. Currently we sidestep
> all of these problems by (a) only allowing the command
> line to disable a feature, not to enable it where it
> didn't previously exist and (b) mostly not providing
> command line flags for individual features...)

AFAICT, in x86 world, there's no generally documented dependancies
between features. I imagine there might be deps mentioned in an
adhoc basis in docs/specs.  There are certainly implicit deps though
where apps check for X, and blindly assume it means Y+Z also exist.

In the QEMU  & libvirt world though, there's no deps. This is generally
why we discourage apps/users from trying to enable arbitrarily many
features. People should stick to the named CPU models, and just use
feature enablement as an exception.

Enabling individual features *is* certainly a critically important
benefit though, even with the risks from implicit undocumented
dependencies.

Most notably, it has been used to turn on fixes for hardware CVEs,
where microcode introduces a new named feature. Your named CPU
model may not be updated, but as long as QEMU knows the feature
name, you can turn it on.

Overall, while it would be desirable to have a way to express
feature dependencies, I wouldn't call that a blocker for enabling
the ability to turn features on & off.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
> > On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
> > > > On 10/25/24 15:06, Daniel P. Berrangé wrote:
> > > > > Also, is this naming convention really the same one that users
> > > > > will see when they look at /proc/cpuinfo to view features ? It
> > > > No it is not. I do agree that the custom cpu model is very low level. It
> > > > is very well suited to test all series turning ID regs as writable but
> > > > this would require an extra layer that adapts /proc/cpuinfo feature
> > > > level to this regid/field abstraction.
> > > >
> > > > In /cpu/proc you will see somethink like:
> > > >  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
> > > > asimdhp cpuid asimdrdm lrcpc dcpop asimddp
> > >
> > > Right, IMHO, this is the terminology that QEMU must use in user
> > > facing APIs.
> >
> > /proc/cpuinfo's naming is rather weird for historical
> > reasons (for instance there is only one FEAT_FP16 feature
> > but cpuinfo lists "fphp" and "asimdhp" separately).
>
> There's plenty of wierd history in x86 too. In this
> case I might suggest just picking one of the two
> common names, and ignoring the other.
>
> If we really wanted to, we could alias the 2nd name
> to the first, but its likely not worth the bother.

Or we could use the standard set of architectural
feature names, and not have the problem at all, and not
have to document what we mean by our nonstandard names.
(cpuinfo names do actually mostly line up with the
standard names, just not 100%. Similarly gcc/clang command
line options are mostly the architectural feature name.)

thanks
-- PMM

On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
> > > On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
> > > > > On 10/25/24 15:06, Daniel P. Berrangé wrote:
> > > > > > Also, is this naming convention really the same one that users
> > > > > > will see when they look at /proc/cpuinfo to view features ? It
> > > > > No it is not. I do agree that the custom cpu model is very low level. It
> > > > > is very well suited to test all series turning ID regs as writable but
> > > > > this would require an extra layer that adapts /proc/cpuinfo feature
> > > > > level to this regid/field abstraction.
> > > > >
> > > > > In /cpu/proc you will see somethink like:
> > > > >  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
> > > > > asimdhp cpuid asimdrdm lrcpc dcpop asimddp
> > > >
> > > > Right, IMHO, this is the terminology that QEMU must use in user
> > > > facing APIs.
> > >
> > > /proc/cpuinfo's naming is rather weird for historical
> > > reasons (for instance there is only one FEAT_FP16 feature
> > > but cpuinfo lists "fphp" and "asimdhp" separately).
> >
> > There's plenty of wierd history in x86 too. In this
> > case I might suggest just picking one of the two
> > common names, and ignoring the other.
> >
> > If we really wanted to, we could alias the 2nd name
> > to the first, but its likely not worth the bother.
> 
> Or we could use the standard set of architectural
> feature names, and not have the problem at all, and not
> have to document what we mean by our nonstandard names.

+1

There's existing documentation [*] for the standard feature names, which
provides:

 - A short description of what the feature does
 - Any dependencies a particular feature has (e.g.FEAT_VHE implies
   FEAT_LSE, FEAT_Debugv8p1, and FEAT_AA64EL2)
 - The register fields/values that are used to discover the feature.

This seems like the most user-friendly option...

[*]: https://developer.arm.com/documentation/109697/2024_09

-- 
Thanks,
Oliver

Hi Oliver,

On 10/28/24 17:56, Oliver Upton wrote:
> On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
>> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>> On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
>>>> On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>> On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
>>>>>> On 10/25/24 15:06, Daniel P. Berrangé wrote:
>>>>>>> Also, is this naming convention really the same one that users
>>>>>>> will see when they look at /proc/cpuinfo to view features ? It
>>>>>> No it is not. I do agree that the custom cpu model is very low level. It
>>>>>> is very well suited to test all series turning ID regs as writable but
>>>>>> this would require an extra layer that adapts /proc/cpuinfo feature
>>>>>> level to this regid/field abstraction.
>>>>>>
>>>>>> In /cpu/proc you will see somethink like:
>>>>>>  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
>>>>>> asimdhp cpuid asimdrdm lrcpc dcpop asimddp
>>>>> Right, IMHO, this is the terminology that QEMU must use in user
>>>>> facing APIs.
>>>> /proc/cpuinfo's naming is rather weird for historical
>>>> reasons (for instance there is only one FEAT_FP16 feature
>>>> but cpuinfo lists "fphp" and "asimdhp" separately).
>>> There's plenty of wierd history in x86 too. In this
>>> case I might suggest just picking one of the two
>>> common names, and ignoring the other.
>>>
>>> If we really wanted to, we could alias the 2nd name
>>> to the first, but its likely not worth the bother.
>> Or we could use the standard set of architectural
>> feature names, and not have the problem at all, and not
>> have to document what we mean by our nonstandard names.
> +1
>
> There's existing documentation [*] for the standard feature names, which
> provides:
>
>  - A short description of what the feature does
>  - Any dependencies a particular feature has (e.g.FEAT_VHE implies
>    FEAT_LSE, FEAT_Debugv8p1, and FEAT_AA64EL2)
>  - The register fields/values that are used to discover the feature.
>
> This seems like the most user-friendly option...
>
> [*]: https://developer.arm.com/documentation/109697/2024_09

Was just wondering if all the writable ID reg fields were associated to
an official FEAT_ listed in this feature list.
Let's take for instance CTR_EL0.DIC. Is it associated to a given
extension or is it just implementation defined?

Thanks

Eric
>

On Mon, 4 Nov 2024 at 17:09, Eric Auger <eric.auger@redhat.com> wrote:
> Was just wondering if all the writable ID reg fields were associated to
> an official FEAT_ listed in this feature list.
> Let's take for instance CTR_EL0.DIC. Is it associated to a given
> extension or is it just implementation defined?

CTR_EL0.DIC is IMPDEF choice of 0 or 1. There's no associated
FEAT_* -- as an implementation, you are effectively just
reporting here what your implementation's properties are.

(CTR_EL0 is not an ID register in the architectural sense.)

thanks
-- PMM

Hi Peter,

On 11/4/24 18:16, Peter Maydell wrote:
> On Mon, 4 Nov 2024 at 17:09, Eric Auger <eric.auger@redhat.com> wrote:
>> Was just wondering if all the writable ID reg fields were associated to
>> an official FEAT_ listed in this feature list.
>> Let's take for instance CTR_EL0.DIC. Is it associated to a given
>> extension or is it just implementation defined?
> CTR_EL0.DIC is IMPDEF choice of 0 or 1. There's no associated
> FEAT_* -- as an implementation, you are effectively just
> reporting here what your implementation's properties are.
>
> (CTR_EL0 is not an ID register in the architectural sense.)

thank you for the confirmation. So this means we also need to be able to
tune such bit although it does not have any fellow FEAT_*.

Eric
>
> thanks
> -- PMM
>

On Mon, Oct 28 2024, Oliver Upton <oliver.upton@linux.dev> wrote:

> On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
>> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> >
>> > On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
>> > > On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> > > > On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
>> > > > > On 10/25/24 15:06, Daniel P. Berrangé wrote:
>> > > > > > Also, is this naming convention really the same one that users
>> > > > > > will see when they look at /proc/cpuinfo to view features ? It
>> > > > > No it is not. I do agree that the custom cpu model is very low level. It
>> > > > > is very well suited to test all series turning ID regs as writable but
>> > > > > this would require an extra layer that adapts /proc/cpuinfo feature
>> > > > > level to this regid/field abstraction.
>> > > > >
>> > > > > In /cpu/proc you will see somethink like:
>> > > > >  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
>> > > > > asimdhp cpuid asimdrdm lrcpc dcpop asimddp
>> > > >
>> > > > Right, IMHO, this is the terminology that QEMU must use in user
>> > > > facing APIs.
>> > >
>> > > /proc/cpuinfo's naming is rather weird for historical
>> > > reasons (for instance there is only one FEAT_FP16 feature
>> > > but cpuinfo lists "fphp" and "asimdhp" separately).
>> >
>> > There's plenty of wierd history in x86 too. In this
>> > case I might suggest just picking one of the two
>> > common names, and ignoring the other.
>> >
>> > If we really wanted to, we could alias the 2nd name
>> > to the first, but its likely not worth the bother.
>> 
>> Or we could use the standard set of architectural
>> feature names, and not have the problem at all, and not
>> have to document what we mean by our nonstandard names.
>
> +1
>
> There's existing documentation [*] for the standard feature names, which
> provides:
>
>  - A short description of what the feature does
>  - Any dependencies a particular feature has (e.g.FEAT_VHE implies
>    FEAT_LSE, FEAT_Debugv8p1, and FEAT_AA64EL2)
>  - The register fields/values that are used to discover the feature.
>
> This seems like the most user-friendly option...
>
> [*]: https://developer.arm.com/documentation/109697/2024_09

FEAT_XXX sounds good, would be a different approach than this series
obviously, since the user resp. upper software layers would operate on a
per-feature basis, and QEMU would translate to and from registers.

I'm wondering about the amount of translation that would be needed, and
what information would be best exposed via QEMU, given that a feature
may or may not be toggable not only because of what the Arm revision
specifies, but what registers the host kernel allows to be written.

I.e. if we have two cpus that differ in whether FEAT_FOO is provided,
would it make sense to have an extra QMP command so that you can find
out whether FEAT_FOO can be switched off, with QEMU translating from the
set of writable id registers to the set of features that can be changed?

On Wed, Oct 30, 2024 at 05:15:05PM +0100, Cornelia Huck wrote:
> On Mon, Oct 28 2024, Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> > On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
> >> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >> >
> >> > On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
> >> > > On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >> > > > On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
> >> > > > > On 10/25/24 15:06, Daniel P. Berrangé wrote:
> >> > > > > > Also, is this naming convention really the same one that users
> >> > > > > > will see when they look at /proc/cpuinfo to view features ? It
> >> > > > > No it is not. I do agree that the custom cpu model is very low level. It
> >> > > > > is very well suited to test all series turning ID regs as writable but
> >> > > > > this would require an extra layer that adapts /proc/cpuinfo feature
> >> > > > > level to this regid/field abstraction.
> >> > > > >
> >> > > > > In /cpu/proc you will see somethink like:
> >> > > > >  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
> >> > > > > asimdhp cpuid asimdrdm lrcpc dcpop asimddp
> >> > > >
> >> > > > Right, IMHO, this is the terminology that QEMU must use in user
> >> > > > facing APIs.
> >> > >
> >> > > /proc/cpuinfo's naming is rather weird for historical
> >> > > reasons (for instance there is only one FEAT_FP16 feature
> >> > > but cpuinfo lists "fphp" and "asimdhp" separately).
> >> >
> >> > There's plenty of wierd history in x86 too. In this
> >> > case I might suggest just picking one of the two
> >> > common names, and ignoring the other.
> >> >
> >> > If we really wanted to, we could alias the 2nd name
> >> > to the first, but its likely not worth the bother.
> >> 
> >> Or we could use the standard set of architectural
> >> feature names, and not have the problem at all, and not
> >> have to document what we mean by our nonstandard names.
> >
> > +1
> >
> > There's existing documentation [*] for the standard feature names, which
> > provides:
> >
> >  - A short description of what the feature does
> >  - Any dependencies a particular feature has (e.g.FEAT_VHE implies
> >    FEAT_LSE, FEAT_Debugv8p1, and FEAT_AA64EL2)
> >  - The register fields/values that are used to discover the feature.
> >
> > This seems like the most user-friendly option...
> >
> > [*]: https://developer.arm.com/documentation/109697/2024_09
> 
> FEAT_XXX sounds good, would be a different approach than this series
> obviously, since the user resp. upper software layers would operate on a
> per-feature basis, and QEMU would translate to and from registers.
> 
> I'm wondering about the amount of translation that would be needed, and
> what information would be best exposed via QEMU, given that a feature
> may or may not be toggable not only because of what the Arm revision
> specifies, but what registers the host kernel allows to be written.
> 
> I.e. if we have two cpus that differ in whether FEAT_FOO is provided,
> would it make sense to have an extra QMP command so that you can find
> out whether FEAT_FOO can be switched off, with QEMU translating from the
> set of writable id registers to the set of features that can be changed?

If there are restrictions on what features can be turned off, then yes,
we should add that in QMP, as x86 doesn't have such a restriction
currently.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
> > > On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
> > > > > On 10/25/24 15:06, Daniel P. Berrangé wrote:
> > > > > > Also, is this naming convention really the same one that users
> > > > > > will see when they look at /proc/cpuinfo to view features ? It
> > > > > No it is not. I do agree that the custom cpu model is very low level. It
> > > > > is very well suited to test all series turning ID regs as writable but
> > > > > this would require an extra layer that adapts /proc/cpuinfo feature
> > > > > level to this regid/field abstraction.
> > > > >
> > > > > In /cpu/proc you will see somethink like:
> > > > >  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
> > > > > asimdhp cpuid asimdrdm lrcpc dcpop asimddp
> > > >
> > > > Right, IMHO, this is the terminology that QEMU must use in user
> > > > facing APIs.
> > >
> > > /proc/cpuinfo's naming is rather weird for historical
> > > reasons (for instance there is only one FEAT_FP16 feature
> > > but cpuinfo lists "fphp" and "asimdhp" separately).
> >
> > There's plenty of wierd history in x86 too. In this
> > case I might suggest just picking one of the two
> > common names, and ignoring the other.
> >
> > If we really wanted to, we could alias the 2nd name
> > to the first, but its likely not worth the bother.
> 
> Or we could use the standard set of architectural
> feature names, and not have the problem at all, and not
> have to document what we mean by our nonstandard names.
> (cpuinfo names do actually mostly line up with the
> standard names, just not 100%. Similarly gcc/clang command
> line options are mostly the architectural feature name.)

Ah, right, yes. Sorry I mis-understood you originally to be suggesting
the same low level names as this patch.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Hi Daniel,

On 10/28/24 18:04, Daniel P. Berrangé wrote:
> On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
>> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>> On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
>>>> On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>> On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
>>>>>> On 10/25/24 15:06, Daniel P. Berrangé wrote:
>>>>>>> Also, is this naming convention really the same one that users
>>>>>>> will see when they look at /proc/cpuinfo to view features ? It
>>>>>> No it is not. I do agree that the custom cpu model is very low level. It
>>>>>> is very well suited to test all series turning ID regs as writable but
>>>>>> this would require an extra layer that adapts /proc/cpuinfo feature
>>>>>> level to this regid/field abstraction.
>>>>>>
>>>>>> In /cpu/proc you will see somethink like:
>>>>>>  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
>>>>>> asimdhp cpuid asimdrdm lrcpc dcpop asimddp
>>>>> Right, IMHO, this is the terminology that QEMU must use in user
>>>>> facing APIs.
>>>> /proc/cpuinfo's naming is rather weird for historical
>>>> reasons (for instance there is only one FEAT_FP16 feature
>>>> but cpuinfo lists "fphp" and "asimdhp" separately).
>>> There's plenty of wierd history in x86 too. In this
>>> case I might suggest just picking one of the two
>>> common names, and ignoring the other.
>>>
>>> If we really wanted to, we could alias the 2nd name
>>> to the first, but its likely not worth the bother.
>> Or we could use the standard set of architectural
>> feature names, and not have the problem at all, and not
>> have to document what we mean by our nonstandard names.
>> (cpuinfo names do actually mostly line up with the
>> standard names, just not 100%. Similarly gcc/clang command
>> line options are mostly the architectural feature name.)
> Ah, right, yes. Sorry I mis-understood you originally to be suggesting
> the same low level names as this patch.
If my understanding is correct, Peter suggested to rely on the
terminology used in

https://developer.arm.com/documentation/109697/2024_09

the doc pointed to by Oliver.

So I think the next step is to understand how those "high level" features do map onto low level ID register field values. I think a high level feature can map onto separate fields in separate ID regs. This may not be the most common case though. 

Thanks

Eric

>
> With regards,
> Daniel

On Mon, Nov 04 2024, Eric Auger <eric.auger@redhat.com> wrote:

> Hi Daniel,
>
> On 10/28/24 18:04, Daniel P. Berrangé wrote:
>> On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
>>> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>> On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
>>>>> On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>>> On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
>>>>>>> On 10/25/24 15:06, Daniel P. Berrangé wrote:
>>>>>>>> Also, is this naming convention really the same one that users
>>>>>>>> will see when they look at /proc/cpuinfo to view features ? It
>>>>>>> No it is not. I do agree that the custom cpu model is very low level. It
>>>>>>> is very well suited to test all series turning ID regs as writable but
>>>>>>> this would require an extra layer that adapts /proc/cpuinfo feature
>>>>>>> level to this regid/field abstraction.
>>>>>>>
>>>>>>> In /cpu/proc you will see somethink like:
>>>>>>>  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
>>>>>>> asimdhp cpuid asimdrdm lrcpc dcpop asimddp
>>>>>> Right, IMHO, this is the terminology that QEMU must use in user
>>>>>> facing APIs.
>>>>> /proc/cpuinfo's naming is rather weird for historical
>>>>> reasons (for instance there is only one FEAT_FP16 feature
>>>>> but cpuinfo lists "fphp" and "asimdhp" separately).
>>>> There's plenty of wierd history in x86 too. In this
>>>> case I might suggest just picking one of the two
>>>> common names, and ignoring the other.
>>>>
>>>> If we really wanted to, we could alias the 2nd name
>>>> to the first, but its likely not worth the bother.
>>> Or we could use the standard set of architectural
>>> feature names, and not have the problem at all, and not
>>> have to document what we mean by our nonstandard names.
>>> (cpuinfo names do actually mostly line up with the
>>> standard names, just not 100%. Similarly gcc/clang command
>>> line options are mostly the architectural feature name.)
>> Ah, right, yes. Sorry I mis-understood you originally to be suggesting
>> the same low level names as this patch.
> If my understanding is correct, Peter suggested to rely on the
> terminology used in
>
> https://developer.arm.com/documentation/109697/2024_09
>
> the doc pointed to by Oliver.
>
> So I think the next step is to understand how those "high level" features do map onto low level ID register field values. I think a high level feature can map onto separate fields in separate ID regs. This may not be the most common case though. 

I went through all the FEAT_xxx features defined so far and tried to
categorize them (probably with some errors here and there, but the
general trend should be correct.)

There's 335 features defined at the moment.

Of these, the majority (295 by my count) map to one or more values in
one or more id registers. These are what I'd consider the "easy" ones
(added complexity if we deal with serveral values, but in general, it is
clear how to handle them, and most of them actually map to a single
value.) Of course, dependencies may be on top of that.

Then, we have some features (~25 or so) that are actually defined by
dependencies (i.e. FEAT_FOO and FEAT_BAR mean that we have FEAT_BAZ,
sometimes with an architecture extension dependency thrown in as well.)
These features are not really relevant when we compare two cpus since
they do not map to registers directly, but they are relevant if we allow
them to be specified (and use them as a kind of shorthand.) IOW, we'd
need to think about how we'd handle them for comparisons and baselining.

Next, let's talk about architecture extensions. All features have a
level where they have been introduced as optional, some have an upper
limit (e.g. FEAT_AA32EL1 is not allowed from v9.0 onwards), and quite a
number of them (~65 or so) become mandatory with a certain architecture
extension. Sometimes, FEAT_FOO + arch ext also implies FEAT_BAR. If we
introduce Armvx.y named models, we'd need to enforce that some features
are (not) set for a certain model. Complex, but not a showstopper. (We'd
also need to deal with that if we worked on the register level.)

We also have some registers like MIDR/REVIDR that do not correlate with
any FEAT_xxx, but that we still need to handle; I would suggest to deal
with them via separate cpu properties (e.g. specify a list of possible
MIDR/REVIDR pairs.) I hope that there are not too many of them, although
we do have some impdef registers.

That leaves some headscratchers (at least for me.) E.g. FEAT_UINJ, which
is optional from v9.5, and mandatory from v9.6, but where I'm unsure how
we'd discover it, especially as we do not have a way to discover the
architecture extensions. Maybe this will work for named actual cpus
only? I'm also not sure if I understand FEAT_CHK, which is listed as
optional from v8.0 and mandatory from v9.4, but every aarch64 cpu is
supposed to be compliant, because CHKFEAT might be a NOP (and this is
only supposed to check for FEAT_GCS? Yes, I'm confused.)

So tl;dr, we cover a lot of the ID register space via FEAT_xxx (with
varying complexity), but we already know about some exceptions. For some
FEAT_xxx, I'm not sure if we want to handle them at all.

(I also seem to remember that there some things like perf counters that
don't map to any on/off features, but no idea about the details here.)

On Mon, 11 Nov 2024 at 14:29, Cornelia Huck <cohuck@redhat.com> wrote:
> That leaves some headscratchers (at least for me.) E.g. FEAT_UINJ, which
> is optional from v9.5, and mandatory from v9.6, but where I'm unsure how
> we'd discover it, especially as we do not have a way to discover the
> architecture extensions.

I think that's just an accidental omission from the FEAT_UINJ
documentation. There's a UINJ field in ID_AA64PFR2_EL1 which
tells you whether FEAT_UINJ is implemented:

https://developer.arm.com/documentation/109697/2024_09/Feature-descriptions/The-Armv9-6-architecture-extension

> I'm also not sure if I understand FEAT_CHK, which is listed as
> optional from v8.0 and mandatory from v9.4, but every aarch64 cpu is
> supposed to be compliant, because CHKFEAT might be a NOP (and this is
> only supposed to check for FEAT_GCS? Yes, I'm confused.)

CHKFEAT is there for cases where software (especially EL0 software)
might need a fast cross-OS-portable way to detect whether a feature
is present and enabled -- for instance if you want the compiler
to emit a function prologue sequence that includes "do XYZ if
GCS is on". CHKFEAT is in the hint area of the encoding space,
so for any CPU that pre-dates FEAT_CHK it will do nothing (which is
how "no features detectable with CHKFEAT are present" is indicated:
if you look at what CHKFEAT actually does it has a definition that
isn't how you'd naturally implement a feature-detect unless you
needed to make NOP mean no-features-present.)

Currently FEAT_GCS is the only feature where it has been deemed
necessary to provide this kind of is-it-enabled detection, but some
future features might also use the CHKFEAT mechanism.

If you're a piece of system software wanting to see if FEAT_GCS
is present you would use ID_AA64PFR1_EL1.GCS. (In particular, if
you're an OS then until you have enabled GCS for yourself via
GCSCR_EL1, CHKFEAT will tell you "not enabled", so you first need
to look at the ID register to see if the GCSCR_EL1 register is
there at all...)

thanks
-- PMM

On Mon, Nov 11 2024, Cornelia Huck <cohuck@redhat.com> wrote:

> On Mon, Nov 04 2024, Eric Auger <eric.auger@redhat.com> wrote:
>
>> Hi Daniel,
>>
>> On 10/28/24 18:04, Daniel P. Berrangé wrote:
>>> On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
>>>> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>> On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
>>>>>> On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>>>> On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
>>>>>>>> On 10/25/24 15:06, Daniel P. Berrangé wrote:
>>>>>>>>> Also, is this naming convention really the same one that users
>>>>>>>>> will see when they look at /proc/cpuinfo to view features ? It
>>>>>>>> No it is not. I do agree that the custom cpu model is very low level. It
>>>>>>>> is very well suited to test all series turning ID regs as writable but
>>>>>>>> this would require an extra layer that adapts /proc/cpuinfo feature
>>>>>>>> level to this regid/field abstraction.
>>>>>>>>
>>>>>>>> In /cpu/proc you will see somethink like:
>>>>>>>>  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
>>>>>>>> asimdhp cpuid asimdrdm lrcpc dcpop asimddp
>>>>>>> Right, IMHO, this is the terminology that QEMU must use in user
>>>>>>> facing APIs.
>>>>>> /proc/cpuinfo's naming is rather weird for historical
>>>>>> reasons (for instance there is only one FEAT_FP16 feature
>>>>>> but cpuinfo lists "fphp" and "asimdhp" separately).
>>>>> There's plenty of wierd history in x86 too. In this
>>>>> case I might suggest just picking one of the two
>>>>> common names, and ignoring the other.
>>>>>
>>>>> If we really wanted to, we could alias the 2nd name
>>>>> to the first, but its likely not worth the bother.
>>>> Or we could use the standard set of architectural
>>>> feature names, and not have the problem at all, and not
>>>> have to document what we mean by our nonstandard names.
>>>> (cpuinfo names do actually mostly line up with the
>>>> standard names, just not 100%. Similarly gcc/clang command
>>>> line options are mostly the architectural feature name.)
>>> Ah, right, yes. Sorry I mis-understood you originally to be suggesting
>>> the same low level names as this patch.
>> If my understanding is correct, Peter suggested to rely on the
>> terminology used in
>>
>> https://developer.arm.com/documentation/109697/2024_09
>>
>> the doc pointed to by Oliver.
>>
>> So I think the next step is to understand how those "high level" features do map onto low level ID register field values. I think a high level feature can map onto separate fields in separate ID regs. This may not be the most common case though. 
>
> I went through all the FEAT_xxx features defined so far and tried to
> categorize them (probably with some errors here and there, but the
> general trend should be correct.)
>
> There's 335 features defined at the moment.
>
> Of these, the majority (295 by my count) map to one or more values in
> one or more id registers. These are what I'd consider the "easy" ones
> (added complexity if we deal with serveral values, but in general, it is
> clear how to handle them, and most of them actually map to a single
> value.) Of course, dependencies may be on top of that.
>
> Then, we have some features (~25 or so) that are actually defined by
> dependencies (i.e. FEAT_FOO and FEAT_BAR mean that we have FEAT_BAZ,
> sometimes with an architecture extension dependency thrown in as well.)
> These features are not really relevant when we compare two cpus since
> they do not map to registers directly, but they are relevant if we allow
> them to be specified (and use them as a kind of shorthand.) IOW, we'd
> need to think about how we'd handle them for comparisons and baselining.
>
> Next, let's talk about architecture extensions. All features have a
> level where they have been introduced as optional, some have an upper
> limit (e.g. FEAT_AA32EL1 is not allowed from v9.0 onwards), and quite a
> number of them (~65 or so) become mandatory with a certain architecture
> extension. Sometimes, FEAT_FOO + arch ext also implies FEAT_BAR. If we
> introduce Armvx.y named models, we'd need to enforce that some features
> are (not) set for a certain model. Complex, but not a showstopper. (We'd
> also need to deal with that if we worked on the register level.)
>
> We also have some registers like MIDR/REVIDR that do not correlate with
> any FEAT_xxx, but that we still need to handle; I would suggest to deal
> with them via separate cpu properties (e.g. specify a list of possible
> MIDR/REVIDR pairs.) I hope that there are not too many of them, although
> we do have some impdef registers.
>
> That leaves some headscratchers (at least for me.) E.g. FEAT_UINJ, which
> is optional from v9.5, and mandatory from v9.6, but where I'm unsure how
> we'd discover it, especially as we do not have a way to discover the
> architecture extensions. Maybe this will work for named actual cpus
> only? I'm also not sure if I understand FEAT_CHK, which is listed as
> optional from v8.0 and mandatory from v9.4, but every aarch64 cpu is
> supposed to be compliant, because CHKFEAT might be a NOP (and this is
> only supposed to check for FEAT_GCS? Yes, I'm confused.)
>
> So tl;dr, we cover a lot of the ID register space via FEAT_xxx (with
> varying complexity), but we already know about some exceptions. For some
> FEAT_xxx, I'm not sure if we want to handle them at all.
>
> (I also seem to remember that there some things like perf counters that
> don't map to any on/off features, but no idea about the details here.)

Following up on this, some ideas/questions on how a command line could
look like and behave.

Let's say we introduce Armv80, Armv81, ... models for the architecture
extensions and optionally named models for individual cpus we care about
(e.g. My85BasedCpu which would be Armv85 + some stuff on top). The
Armv<xy> models would come with all mandatory features turned on and
would allow all optional features to be set or unset, with
(accelerator-specific) verification on top.

Let's assume that we have

-cpu Armv80,FEAT_AdvSIMD=on,FEAT_FP=on

Since the two features provide each other, I'd assume

-cpu Armv80,FEAT_AdvSIMD=on

or

-cpu Armv80,FEAT_FP=on

to be equivalent, and

-cpu Armv80,FEAT_AdvSIMD=on,FEAT_FP=off

to generate an error. Question: at which level? s390x does dependency
validation in QEMU, we could do it as well, but I think we have way more
dependencies on Arm.

Also, let's take FEAT_BTI, which is optional for 8.4 and mandatory from
8.5 onwards. I'd expect

-cpu Armv84,FEAT_BTI=on   <-- ok
-cpu Armv84  same as -cpu Armv84,FEAT_BTI=off
-cpu Armv85 same as -cpu Arm84,FEAT_BTI=on
-cpu Armv85,FEAT_BTI=off  <-- Error!

When doing baselining while looking at id registers, we'd get

ID_AA64PFR1_EL1.BT == 1 -> must use Armv84 or later
ID_AA64PFR1_EL1.BT == 0 -> must use Armv84 or earlier

(unfortunately, some other features are more complex...)

Would

-cpu Armv84,<all 8.5 mandatory bits>

generate the same cpu as

-cpu Armv85

(and therefore, the two be migratable between each other?)

If we create named cpu models for individual cpus, e.g.

-cpu My85BasedCpu

as equivalent to

-cpu Armv85,FEAT_FOO=on,FEAT_BAR=on

Should we allow

-cpu My85BasedCpu,FEAT_BAR=off

or would the command line need to be

-cpu Armv85,FEAT_FOO=on

(e.g. to deal with not all id regs being writable?)

Regardless of what we end up doing for features and cpu models, we'll
need to be able to specify registers that do not correlate to any
features; especially for things like MIDR/REVIDR, where we'll want to
provide a list of possible target values as pairs;

-cpu Armv85,midr_revidr=(<value>,<value>)

is ugly, and figuring the actual values is not user friendly at all, I
fear :(

And now, at the end of this long email, the most important questions:
What colour should the features have?

Matching the architecture: FEAT_AdvSIMD, FEAT_FP, FEAT_BTI

Dropping "FEAT_": AdvSIMD, FP, BTI

Less shouty: feat_advsimd, feat_fp, feat_bti

Less shouty and shorter: advsimd, fp, bti  -- in this case we'd need to
be careful because we already have sve/sme props, and I'm not sure if we
could do some sugaring without conflicts.

Thoughts?

Hi Conny,

On 11/12/24 17:30, Cornelia Huck wrote:
> On Mon, Nov 11 2024, Cornelia Huck <cohuck@redhat.com> wrote:
>
>> On Mon, Nov 04 2024, Eric Auger <eric.auger@redhat.com> wrote:
>>
>>> Hi Daniel,
>>>
>>> On 10/28/24 18:04, Daniel P. Berrangé wrote:
>>>> On Mon, Oct 28, 2024 at 04:48:18PM +0000, Peter Maydell wrote:
>>>>> On Mon, 28 Oct 2024 at 16:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>>> On Mon, Oct 28, 2024 at 04:16:31PM +0000, Peter Maydell wrote:
>>>>>>> On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>>>>> On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
>>>>>>>>> On 10/25/24 15:06, Daniel P. Berrangé wrote:
>>>>>>>>>> Also, is this naming convention really the same one that users
>>>>>>>>>> will see when they look at /proc/cpuinfo to view features ? It
>>>>>>>>> No it is not. I do agree that the custom cpu model is very low level. It
>>>>>>>>> is very well suited to test all series turning ID regs as writable but
>>>>>>>>> this would require an extra layer that adapts /proc/cpuinfo feature
>>>>>>>>> level to this regid/field abstraction.
>>>>>>>>>
>>>>>>>>> In /cpu/proc you will see somethink like:
>>>>>>>>>  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
>>>>>>>>> asimdhp cpuid asimdrdm lrcpc dcpop asimddp
>>>>>>>> Right, IMHO, this is the terminology that QEMU must use in user
>>>>>>>> facing APIs.
>>>>>>> /proc/cpuinfo's naming is rather weird for historical
>>>>>>> reasons (for instance there is only one FEAT_FP16 feature
>>>>>>> but cpuinfo lists "fphp" and "asimdhp" separately).
>>>>>> There's plenty of wierd history in x86 too. In this
>>>>>> case I might suggest just picking one of the two
>>>>>> common names, and ignoring the other.
>>>>>>
>>>>>> If we really wanted to, we could alias the 2nd name
>>>>>> to the first, but its likely not worth the bother.
>>>>> Or we could use the standard set of architectural
>>>>> feature names, and not have the problem at all, and not
>>>>> have to document what we mean by our nonstandard names.
>>>>> (cpuinfo names do actually mostly line up with the
>>>>> standard names, just not 100%. Similarly gcc/clang command
>>>>> line options are mostly the architectural feature name.)
>>>> Ah, right, yes. Sorry I mis-understood you originally to be suggesting
>>>> the same low level names as this patch.
>>> If my understanding is correct, Peter suggested to rely on the
>>> terminology used in
>>>
>>> https://developer.arm.com/documentation/109697/2024_09
>>>
>>> the doc pointed to by Oliver.
>>>
>>> So I think the next step is to understand how those "high level" features do map onto low level ID register field values. I think a high level feature can map onto separate fields in separate ID regs. This may not be the most common case though. 
>> I went through all the FEAT_xxx features defined so far and tried to
>> categorize them (probably with some errors here and there, but the
>> general trend should be correct.)
>>
>> There's 335 features defined at the moment.
>>
>> Of these, the majority (295 by my count) map to one or more values in
>> one or more id registers. These are what I'd consider the "easy" ones
>> (added complexity if we deal with serveral values, but in general, it is
>> clear how to handle them, and most of them actually map to a single
>> value.) Of course, dependencies may be on top of that.
>>
>> Then, we have some features (~25 or so) that are actually defined by
>> dependencies (i.e. FEAT_FOO and FEAT_BAR mean that we have FEAT_BAZ,
>> sometimes with an architecture extension dependency thrown in as well.)
>> These features are not really relevant when we compare two cpus since
>> they do not map to registers directly, but they are relevant if we allow
>> them to be specified (and use them as a kind of shorthand.) IOW, we'd
>> need to think about how we'd handle them for comparisons and baselining.
>>
>> Next, let's talk about architecture extensions. All features have a
>> level where they have been introduced as optional, some have an upper
>> limit (e.g. FEAT_AA32EL1 is not allowed from v9.0 onwards), and quite a
>> number of them (~65 or so) become mandatory with a certain architecture
>> extension. Sometimes, FEAT_FOO + arch ext also implies FEAT_BAR. If we
>> introduce Armvx.y named models, we'd need to enforce that some features
>> are (not) set for a certain model. Complex, but not a showstopper. (We'd
>> also need to deal with that if we worked on the register level.)
>>
>> We also have some registers like MIDR/REVIDR that do not correlate with
>> any FEAT_xxx, but that we still need to handle; I would suggest to deal
>> with them via separate cpu properties (e.g. specify a list of possible
>> MIDR/REVIDR pairs.) I hope that there are not too many of them, although
>> we do have some impdef registers.
>>
>> That leaves some headscratchers (at least for me.) E.g. FEAT_UINJ, which
>> is optional from v9.5, and mandatory from v9.6, but where I'm unsure how
>> we'd discover it, especially as we do not have a way to discover the
>> architecture extensions. Maybe this will work for named actual cpus
>> only? I'm also not sure if I understand FEAT_CHK, which is listed as
>> optional from v8.0 and mandatory from v9.4, but every aarch64 cpu is
>> supposed to be compliant, because CHKFEAT might be a NOP (and this is
>> only supposed to check for FEAT_GCS? Yes, I'm confused.)
>>
>> So tl;dr, we cover a lot of the ID register space via FEAT_xxx (with
>> varying complexity), but we already know about some exceptions. For some
>> FEAT_xxx, I'm not sure if we want to handle them at all.
>>
>> (I also seem to remember that there some things like perf counters that
>> don't map to any on/off features, but no idea about the details here.)
> Following up on this, some ideas/questions on how a command line could
> look like and behave.
>
> Let's say we introduce Armv80, Armv81, ... models for the architecture
> extensions and optionally named models for individual cpus we care about
> (e.g. My85BasedCpu which would be Armv85 + some stuff on top). The
> Armv<xy> models would come with all mandatory features turned on and
> would allow all optional features to be set or unset, with
> (accelerator-specific) verification on top.
>
> Let's assume that we have
>
> -cpu Armv80,FEAT_AdvSIMD=on,FEAT_FP=on
>
> Since the two features provide each other, I'd assume
>
> -cpu Armv80,FEAT_AdvSIMD=on
>
> or
>
> -cpu Armv80,FEAT_FP=on
>
> to be equivalent, and
>
> -cpu Armv80,FEAT_AdvSIMD=on,FEAT_FP=off
>
> to generate an error. Question: at which level? s390x does dependency
> validation in QEMU, we could do it as well, but I think we have way more
> dependencies on Arm.
>
> Also, let's take FEAT_BTI, which is optional for 8.4 and mandatory from
> 8.5 onwards. I'd expect
>
> -cpu Armv84,FEAT_BTI=on   <-- ok
> -cpu Armv84  same as -cpu Armv84,FEAT_BTI=off
> -cpu Armv85 same as -cpu Arm84,FEAT_BTI=on
except we may have plenty of other 8.5 mandatory features I guess
> -cpu Armv85,FEAT_BTI=off  <-- Error!
>
> When doing baselining while looking at id registers, we'd get
>
> ID_AA64PFR1_EL1.BT == 1 -> must use Armv84 or later
> ID_AA64PFR1_EL1.BT == 0 -> must use Armv84 or earlier
>
> (unfortunately, some other features are more complex...)
>
> Would
>
> -cpu Armv84,<all 8.5 mandatory bits>
>
> generate the same cpu as
>
> -cpu Armv85
>
> (and therefore, the two be migratable between each other?)
I don't think we want this capability. I guess we would support
migration between exact same cpu <type>,<feat list>
>
> If we create named cpu models for individual cpus, e.g.
>
> -cpu My85BasedCpu
>
> as equivalent to
>
> -cpu Armv85,FEAT_FOO=on,FEAT_BAR=on
>
> Should we allow
>
> -cpu My85BasedCpu,FEAT_BAR=off
> or would the command line need to be
>
> -cpu Armv85,FEAT_FOO=on
I am scared about the overall implementation/test matrix. Can we afford
supporting that many setups, revision based models, named models with
all kinds of features optim-in/opt-out. We should also keep in mind what
is our current goal, ie. supporting migration between different machines
but still with sufficient commonalities.

Shouldn't we try to collect community needs in that regard. I don't know
if people will/can tell us what they want to migrate from/to. At least
do the machines comply with the same spec rev or how large is the gap.
How many ID regs/features do mismatch in general. Are those ID regs
currently writable? What would be the best grouping model for them?

Also many ID regs are not writable, because we have not turned them yet
as writable or because they are not that much a problem at the moment.
Maybe some features are not fully turned as writable. I mean some ID
regs they depend on are but not all of them.

So maybe we should reduce the complexity by reducing the scope to the 16
writable regs we have atm (~ 126 writable ID reg fields).
>
> (e.g. to deal with not all id regs being writable?)
>
> Regardless of what we end up doing for features and cpu models, we'll
> need to be able to specify registers that do not correlate to any
> features; especially for things like MIDR/REVIDR, where we'll want to
> provide a list of possible target values as pairs;
>
> -cpu Armv85,midr_revidr=(<value>,<value>)
>
> is ugly, and figuring the actual values is not user friendly at all, I
> fear :(
>
> And now, at the end of this long email, the most important questions:
> What colour should the features have?
>
> Matching the architecture: FEAT_AdvSIMD, FEAT_FP, FEAT_BTI
>
> Dropping "FEAT_": AdvSIMD, FP, BTI
>
> Less shouty: feat_advsimd, feat_fp, feat_bti
>
> Less shouty and shorter: advsimd, fp, bti  -- in this case we'd need to
> be careful because we already have sve/sme props, and I'm not sure if we
> could do some sugaring without conflicts.

Another big question I think is how the upper layer will detect the
relevance of a named model and associated feature. We saw that

/proc/cpuinfo was not that much consistent with FEAT_xxx terminology. Many other features can't be queried I guess. I think it is also part of the problematic. Is it worth exposing stuff that cannot be simply introspected?

Thanks

Eric

>
> Thoughts?
>

On Mon, Oct 28 2024, Peter Maydell <peter.maydell@linaro.org> wrote:

> On Fri, 25 Oct 2024 at 14:24, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
>> > On 10/25/24 15:06, Daniel P. Berrangé wrote:
>> > > Also, is this naming convention really the same one that users
>> > > will see when they look at /proc/cpuinfo to view features ? It
>> > No it is not. I do agree that the custom cpu model is very low level. It
>> > is very well suited to test all series turning ID regs as writable but
>> > this would require an extra layer that adapts /proc/cpuinfo feature
>> > level to this regid/field abstraction.
>> >
>> > In /cpu/proc you will see somethink like:
>> >  Features    : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp
>> > asimdhp cpuid asimdrdm lrcpc dcpop asimddp
>>
>> Right, IMHO, this is the terminology that QEMU must use in user
>> facing APIs.
>
> /proc/cpuinfo's naming is rather weird for historical
> reasons (for instance there is only one FEAT_FP16 feature
> but cpuinfo lists "fphp" and "asimdhp" separately).
> Currently QEMU only has to care about cpuinfo name tags
> in linux-user/elfload.c where there's a bunch of data
> structures for "what hwcaps do we need to advertise
> given what the CPU has?". I would definitely prefer it if
> we could use architectural feature names...
>
> On other architectures do we do anything to forbid
> invalid combinations? For Arm there are architectural
> rules about feature X requiring features Y and Z.
> Are we going to just let the user create a CPU that
> the guest OS will barf on if they want to? (The
> user-experience for that is potentially not very nice,
> because something like "-cpu cortex-a57,+sve" seems like
> something you might want to do but isn't actually valid;
> even listing the direct required dependency of FEAT_SVE
> like "-cpu cortex-a57,+sve,+fp16" isn't sufficient
> because SVE is optional-from-v8.2 and so a guest could
> in theory assume the presence of anything mandatory in
> v8.1 and v8.2. But even merely diagnosing invalid
> combinations is a huge pain, and automagically pulling
> in every mandatory implicit or explicit dependency
> seems like it might be surprising to users, as well as
> being annoying to implement. Currently we sidestep
> all of these problems by (a) only allowing the command
> line to disable a feature, not to enable it where it
> didn't previously exist and (b) mostly not providing
> command line flags for individual features...)

I think s390 does some dependency checking on features, and also rejects
features that are "too new".

On Fri, Oct 25 2024, Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
>> Hi Daniel,
>> 
>> On 10/25/24 15:06, Daniel P. Berrangé wrote:
>> > On Fri, Oct 25, 2024 at 12:17:37PM +0200, Eric Auger wrote:
>> >> At the moment, the custom model does not support legacy options
>> >> of the host cpu model. We need to understand what we do with those
>> >> latter (SVE, ...). This means that related KVM ioctl are
>> >> not called yet.
>> > It will be pretty painful to have to use different feature
>> > terminology for different CPU models. Everything in libvirt
>> > assuming feature terminology varies per-arch, not per-CPU
>> > model.
>> Actually as far as I understand those regids/fields would fit all kind
>> of aarch64 Cortex-A CPUs. So they wouldn't vary per-CPU (I mean their
>> terminology. Their availability will).
>
> What I mean is can we define  named models for various different
> vendor's Cortex-A silicon and just use that without needing to
> toggle features, except in rare cases.

I'm not sure whether creating named models for various cpus would
actually scale; what we probably could do is come up with some Armv8.6,
Armv8.7, ... models and have additional models for some well-known cpus.
That would also give us some basic sanity checks for feature
combinations, if we do not want to support completely custom
frankencpus.

On Mon, Oct 28, 2024 at 05:00:46PM +0100, Cornelia Huck wrote:
> On Fri, Oct 25 2024, Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Fri, Oct 25, 2024 at 03:18:25PM +0200, Eric Auger wrote:
> >> Hi Daniel,
> >> 
> >> On 10/25/24 15:06, Daniel P. Berrangé wrote:
> >> > On Fri, Oct 25, 2024 at 12:17:37PM +0200, Eric Auger wrote:
> >> >> At the moment, the custom model does not support legacy options
> >> >> of the host cpu model. We need to understand what we do with those
> >> >> latter (SVE, ...). This means that related KVM ioctl are
> >> >> not called yet.
> >> > It will be pretty painful to have to use different feature
> >> > terminology for different CPU models. Everything in libvirt
> >> > assuming feature terminology varies per-arch, not per-CPU
> >> > model.
> >> Actually as far as I understand those regids/fields would fit all kind
> >> of aarch64 Cortex-A CPUs. So they wouldn't vary per-CPU (I mean their
> >> terminology. Their availability will).
> >
> > What I mean is can we define  named models for various different
> > vendor's Cortex-A silicon and just use that without needing to
> > toggle features, except in rare cases.
> 
> I'm not sure whether creating named models for various cpus would
> actually scale; what we probably could do is come up with some Armv8.6,
> Armv8.7, ... models and have additional models for some well-known cpus.
> That would also give us some basic sanity checks for feature
> combinations, if we do not want to support completely custom
> frankencpus.

Yep, even on x86 we don't add named models for every conceivable
CPU SKU variant, just one so called "typical" variant for a given
microarchiteture level [1].

The vendor independent Arm CPU spec versions are a good enough
match for typical real world silicon, then it would make sense
to focus mostly on such named CPU models, not try to make an
exhaustive definition of every vendor's impl. I could see there
might be some exceptions where adding a vendor specific named
model would be worthwhile, but hopefully that'd be relatively
uncommon and   -cpu Armv8.6,+feat1,+feat2 would be sufficient.

eg toggling 1/2/3 extra features to precise match a vendor's
named mode is ok, but adding 20-30 extra features get a match
is painful. That's the kind of rule of thumb that could be
used when deciding whether its worth adding a vendor specific
named model, vs relying on the generic spec based models.

With regards,
Daniel

[1] we do have CPU model versions, as a way to "bugfix" our
    CPU definitions. 
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|