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

[Peter Maydell]

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.)

QEMU does assume that the kernel's ID register values match
the hardware for sysregs in some ways -- we use this when we
construct our mapping from KVM register IDs as returned by
KVM_GET_REG_LIST to QEMU cpreg definitions and thus CPUState
struct fields. I *think* that in this case the only visible
effect will be that gdbstub will show you the CNT value
if you ask it to print the value of the CVAL sysreg.

thanks
-- PMM