On Tue, May 13 2025, Daniel P. Berrangé <berra...@redhat.com> wrote:
> On Mon, Apr 14, 2025 at 06:38:47PM +0200, Cornelia Huck wrote: >> From: Eric Auger <eric.au...@redhat.com> >> >> 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. The main reason for the SYSREG_ prefix was to make implementation of the query interface easier; I'll see if we can implement it differently. (We might want to think about how we handle it differently anyway.) > > 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]. Did you forget to include [1]? :) I agree that a lowercase conversion wouldn't make things too hard to find in the docuementation. > > 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. For KVM, this works with -cpu max as well, as it falls through to -cpu host implicitly. aarch64_host_initfn() is basically split into KVM and HVF parts, so HVF will already do the right thing (i.e. not support it), as will tcg (which uses a separate init function.) > > >> +/* >> + * 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 ? The kernel making something writable, but we haven't updated cpu-sysreg-properties.c via the script. As I'd expect regs/fields to be added to sysreg much earlier than KVM making them writable, we'll probably not see much of this warning in practice (only if you run a really old QEMU version with a cutting-edge kernel.) This has mostly triggered for me when I had messed up something while writing the code :) This is not user-input triggered, so maybe we should log it differently, as the user can't really fix this themself?