On 2019-12-16 15:33, Peter Maydell wrote:
> On Thu, 12 Dec 2019 at 17:33, Andrew Jones <address@hidden>
wrote:
>
> > Userspace that wants to set KVM_REG_ARM_TIMER_CNT should beware
that
> > the KVM register ID is not correct. This cannot be fixed
because
> > it's
> > UAPI and if the UAPI headers are used then it can't be a
problem.
> > However, if a userspace attempts to create the ID themselves
from
> > the
> > register's specification, then they will get
KVM_REG_ARM_TIMER_CVAL
> > instead, as the _CNT and _CVAL definitions have their register
> > parameters swapped.
>
> So, to be clear, you mean that:
>
> (1) the kernel headers say:
>
> /* EL0 Virtual Timer Registers */
> #define KVM_REG_ARM_TIMER_CTL ARM64_SYS_REG(3, 3, 14, 3,
1)
> #define KVM_REG_ARM_TIMER_CNT ARM64_SYS_REG(3, 3, 14, 3,
2)
> #define KVM_REG_ARM_TIMER_CVAL ARM64_SYS_REG(3, 3, 14, 0,
2)
>
> (2) some of the RHSes of these are wrong
>
> (3) but the kernel internally is using the same 'wrong' value, so
> userspace also needs to use that value, ie trust the #defined name
> rather than manufacturing one ?
>
> That's awkward. I think it would be worth at least having a kernel
> patch to add a comment clearly documenting this bug.
>
> (This error seems to only be in the 64-bit ABI, not 32-bit.)
Yeah, this is pretty bad. I wonder how we managed not to notice
this for so long... :-(.
Andrew, could you please write a patch documenting this (both in
the UAPI headers and in the documentation)?