On Fri, 02 Aug 2024 22:57:54 +0100, Mark Brown <[email protected]> wrote: > > Currently the get-reg-list test uses directly specified numeric values to > define system registers to validate. Since we already have a macro which > allows us to use the generated system register definitions from the main > kernel easily let's update all the registers where we have specified the > name in a comment to just use that macro. This reduces the number of > places where we need to validate the name to number mapping. > > This conversion was done with the sed command: > > sed -i -E 's-ARM64_SYS_REG.*/\* (.*) \*/-KVM_ARM64_SYS_REG(SYS_\1),-' > tools/testing/selftests/kvm/aarch64/get-reg-list.c >
[Eyes rolling] What I asked about scripting the whole thing, it never occurred to me that you would use the *comments* as a reliable source of information. Do we have anything less reliable than comments in the kernel? The matching must be done from the arch/arm64/tools/sysreg file, because that's the (admittedly dubious) source of truth. We actually trust the encodings because they are reported by the kernel itself. The comment is hand-written, and likely wrong. Also, this hides the horrible truth about existing ABI bugs, see below. > We still have a number of numerically specified registers, some of these > are reserved registers without defined names (eg, unallocated ID registers) > and others don't have kernel macro definitions yet. > > No change in the generated output. > > Suggested-by: Marc Zyngier <[email protected]> > Signed-off-by: Mark Brown <[email protected]> > --- > tools/testing/selftests/kvm/aarch64/get-reg-list.c | 208 > ++++++++++----------- > 1 file changed, 104 insertions(+), 104 deletions(-) > > diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c > b/tools/testing/selftests/kvm/aarch64/get-reg-list.c > index a00322970578..4d786c4ab28a 100644 > --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c > +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c > @@ -313,14 +313,14 @@ static __u64 base_regs[] = { > KVM_REG_ARM_FW_FEAT_BMAP_REG(0), /* KVM_REG_ARM_STD_BMAP */ > KVM_REG_ARM_FW_FEAT_BMAP_REG(1), /* KVM_REG_ARM_STD_HYP_BMAP */ > KVM_REG_ARM_FW_FEAT_BMAP_REG(2), /* KVM_REG_ARM_VENDOR_HYP_BMAP > */ > - ARM64_SYS_REG(3, 3, 14, 3, 1), /* CNTV_CTL_EL0 */ > - ARM64_SYS_REG(3, 3, 14, 3, 2), /* CNTV_CVAL_EL0 */ > + KVM_ARM64_SYS_REG(SYS_CNTV_CTL_EL0), > + KVM_ARM64_SYS_REG(SYS_CNTV_CVAL_EL0), > ARM64_SYS_REG(3, 3, 14, 0, 2), Great. So not only you fail convert a register, but you also ignore the nugget described in arch/arm64/invlude/uapi/asm/kvm.h:267. Sure, having both described hides the crap, as we don't attach any significance to the registers themselves. But that shows how untrustworthy the comments are. > - ARM64_SYS_REG(3, 0, 0, 0, 0), /* MIDR_EL1 */ > - ARM64_SYS_REG(3, 0, 0, 0, 6), /* REVIDR_EL1 */ > - ARM64_SYS_REG(3, 1, 0, 0, 1), /* CLIDR_EL1 */ > - ARM64_SYS_REG(3, 1, 0, 0, 7), /* AIDR_EL1 */ > - ARM64_SYS_REG(3, 3, 0, 0, 1), /* CTR_EL0 */ > + KVM_ARM64_SYS_REG(SYS_MIDR_EL1), > + KVM_ARM64_SYS_REG(SYS_REVIDR_EL1), > + KVM_ARM64_SYS_REG(SYS_CLIDR_EL1), > + KVM_ARM64_SYS_REG(SYS_AIDR_EL1), > + KVM_ARM64_SYS_REG(SYS_CTR_EL0), > ARM64_SYS_REG(2, 0, 0, 0, 4), > ARM64_SYS_REG(2, 0, 0, 0, 5), > ARM64_SYS_REG(2, 0, 0, 0, 6), As far as I can tell, these registers are not unallocated, and they should be named. Thanks, M. -- Without deviation from the norm, progress is not possible.
