On Mon, Apr 14, 2025 at 06:38:47PM +0200, Cornelia Huck wrote:
> From: Eric Auger <[email protected]>
>
> If the interface for writable ID registers is available, expose 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 host,SYSREG_ID_AA64ISAR0_EL1_DP=0x0
> which sets DP field of ID_AA64ISAR0_EL1 to 0.
Functionally this works, but stylewise it is rather too verbose
IMHO. I understand this aims to mtch the arm feature names, but
we can at least drop the SYSREG_ prefix here which IMHO doesn't
add much value. The <REG> component only has a small number of
possible prefixes, so it seems pretty unlikely we would get a
name clash between these and some other QOM property.
Also could we stick with lowercase, rather than uppercase. I
appreciate the spec uses uppercase, but that doesn't concern
itself with end user usage. If we just plain transform everything
to lowercase, there's still a clear mapping to the spec that
people will understand [1].
This example uses '-cpu host', but does this also work
with '-cpu max' ?
Conceptually '-cpu max' is supposed to be functionally identical
to '-cpu host' when KVM is enabled. Obviously you'd ned to
exclude it from '-cpu max' with TCG or other non-KVM accels.
> +/*
> + * 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__);
What scenario triggers this warning ? Is this in relation to QEMU
auto-detecting host CPU features, as opposed to user -cpu input ?
> + 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 */
> +void kvm_arm_expose_idreg_properties(ARMCPU *cpu, ARM64SysReg *regs)
> +{
> + int i, idx;
> + IdRegMap *map = cpu->writable_map;
> + Object *obj = OBJECT(cpu);
> +
> + for (i = 0; i < NR_ID_REGS; i++) {
> + uint64_t mask = map->regs[i];
> +
> + if (mask) {
> + /* reg @i has some writable fields, decode them */
> + idx = kvm_idx_to_idregs_idx(i);
> + if (idx < 0) {
> + /* no matching reg? */
> + warn_report("%s: reg %d writable, but not in list of
> idregs?",
> + __func__, i);
> + } else {
> + decode_idreg_writemap(obj, i, mask, ®s[idx]);
> + }
> + }
> + }
> +}
> +
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 :|
