Re: [RFC PATCH v2 0/5] target/arm/kvm: Adjust virtual time

[Andrew Jones <drjones@redhat.com>]

On Mon, Dec 16, 2019 at 04:18:23PM +0000, Marc Zyngier wrote:
> On 2019-12-16 15:33, Peter Maydell wrote:
> > On Thu, 12 Dec 2019 at 17:33, Andrew Jones <drjones@redhat.com> 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)?
>

Will do. I'll try to get to it this week.

Thanks,
drew