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