Re: [PATCH RFCv2 02/20] arm/cpu: Add sysreg definitions in cpu-sysregs.h

[Richard Henderson]

On 12/6/24 05:21, Cornelia Huck wrote:
> +#define SYS_ID_AA64PFR0_EL1                             sys_reg(3, 0, 0, 4, 0)
...
> +typedef struct ARMSysReg {
> +    int op0;
> +    int op1;
> +    int crn;
> +    int crm;
> +    int op2;
> +} ARMSysReg;
...
> +static inline ARMSysReg sys_reg(int op0, int op1, int crn, int crm, int op2)
> +{
> +        ARMSysReg sr = {op0, op1, crn, crm, op2};
> +
> +        return sr;
> +}

Not a fan.  Why take 20 bytes to represent these?

Our existing ENCODE_CP_REG and ENCODE_AA64_CP_REG macros seem much better, even if the 
argument ordering doesn't match the column ordering in Table D22-2.

> @@ -841,6 +849,51 @@ typedef struct IdRegMap {
>       uint64_t regs[NR_ID_REGS];
>   } IdRegMap;
>   
> +#define ARM_FEATURE_ID_RANGE_IDX(op0, op1, crn, crm, op2)               \
> +        ({                                                              \
> +                __u64 __op1 = (op1) & 3;                                \
> +                __op1 -= (__op1 == 3);                                  \
> +                (__op1 << 6 | ((crm) & 7) << 3 | (op2));                \
> +        })

Ah, well, this answers my question re patch 1.

It seems a shame to use 128 slots to represent all 9 id registers in the 
op1={1,3} space.

Do we really need anything beyond the defined registers, or even the defined registers for 
which qemu knows how to do anything?

I'm certainly happy to replace ARMISARegisters fields with an array, but more 
like

enum ARMIDRegisterIdx {
    ID_AA64ISAR0_IDX,
    etc
    ordering arbitrary, either machine or macro generated,
    but every register has a symbolic index.
    NUM_ID_IDX,
};

enum ARMSysregs {
    SYS_ID_AA64PFR0_EL1 = ENCODE_AA64_CP_REG(...),
    etc
};

const uint32_t id_register_sysreg[NUM_ID_IDX] = {
    [ID_AA64ISAR0_IDX] = SYS_ID_AA64PFR0_EL1,
    etc
};

struct ARMISARegisters {
    uint64_t id[NUM_ID_IDX];
};

This seems trivial to automate, and wastes no space.


r~