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

[Andrew Jones]

On Mon, Dec 16, 2019 at 03:44:05PM +0000, Peter Maydell wrote:
> On Mon, 16 Dec 2019 at 15:33, Peter Maydell <address@hidden> wrote:
> > 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.
> 
> ...perhaps we should work around this kernel bug in the
> kvm_to_cpreg_id() and cpreg_to_kvm_id() functions. (Need
> to think through/test whether that would break migration.)
>

I just did some grepping for this too and, while it's easy to get
lost, I think I've confirmed what you state, that the only visible
effect is in gdb. I'll try to test this, but I think the workaround
in kvm_to_cpreg_id and cpreg_to_kvm_id is a probably a good idea,
because

   1) new qemu will be corrected

   2) migrate old qemu to new qemu (with same machine type)

      gdb cnt and cval swapped until first kvmstate sync

      (maybe too small a window of brokenness to notice/care?)

   3) migrate new qemu to old qemu (with same machine type)

      gdb cnt and cval correct until first kvmstate sync

      (old machine type keeps its old bug - except for same small
       window as for (2))

Thanks,
drew