Re: [PATCH v3 08/10] arm/cpu: more customization for the kvm host cpu model

[Cornelia Huck <cohuck@redhat.com>]

On Tue, May 13 2025, Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Mon, Apr 14, 2025 at 06:38:47PM +0200, Cornelia Huck wrote:
>> From: Eric Auger <eric.auger@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?