On Fri, 02 Aug 2024 22:57:54 +0100, Mark Brown broonie@kernel.org 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 maz@kernel.org Signed-off-by: Mark Brown broonie@kernel.org
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.