On Tue, 19 Jan 2016 11:55:10 +1100 David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Mon, Jan 18, 2016 at 09:51:56AM +0100, Greg Kurz wrote: > > On Mon, 18 Jan 2016 13:16:44 +1100 > > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > > > On Fri, Jan 15, 2016 at 04:00:12PM +0100, Greg Kurz wrote: > > > > On VSX capable CPUs, the 32 FP registers are mapped to the high-bits > > > > of the 32 first VSX registers. So if you have: > > > > > > > > VSR31 = (uint128) 0x0102030405060708090a0b0c0d0e0f00 > > > > > > > > then > > > > > > > > FPR31 = (uint64) 0x0102030405060708 > > > > > > > > The kernel stores the VSX registers in the fp_state struct following the > > > > host endian element ordering. > > > > > > > > On big-endian: > > > > > > > > fp_state.fpr[31][0] = 0x0102030405060708 > > > > fp_state.fpr[31][1] = 0x090a0b0c0d0e0f00 > > > > > > > > On little-endian: > > > > > > > > fp_state.fpr[31][0] = 0x090a0b0c0d0e0f00 > > > > fp_state.fpr[31][1] = 0x0102030405060708 > > > > > > > > The KVM_GET_ONE_REG and KVM_SET_ONE_REG ioctls preserve this ordering, > > > > but > > > > QEMU considers it as big-endian and always copies element [0] to the > > > > fpr[] array and element [1] to the vsr[] array. This does not work with > > > > little-endian hosts, and you will get: > > > > > > > > (qemu) p $f31 > > > > 0x90a0b0c0d0e0f00 > > > > > > > > instead of: > > > > > > > > (qemu) p $f31 > > > > 0x102030405060708 > > > > > > > > This patch fixes the element ordering for little-endian hosts. > > > > > > > > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > > > > > > If I'm understanding correctly, the only reason this bug didn't affect > > > things other than the gdbstub is because the get and put routines had > > > > Well it is not only gdbstub actually... as showed in the changelog, it also > > affects the QEMU monitor which outputs wrong values since it calls > > kvm_get_fpu() > > as well. > > Yes, sorry, I didn't express that well. My point is that the only > reason things aren't going horribly wrong is that qemu is only ever > touching the FP/VSX values for debug, and the get/put into KVM is I fully agree with that QEMU not touching FP/VSX is a key point for not breaking anything. > wrong in such a way that the right values go back again as long as > qemu doesn't try to change them. > I suppose so but I must confess I did not invest time to understand how this KVM bug did not break the guest in some way... > > > mirrored bugs. So although qemu ended up with definitely wrong > > > information in its internal state, it reshuffled it to be right on > > > setting it back into KVM. > > > > > > Is that correct? > > > > > > > My guess is that the bug only affects gdbstub and ppc_cpu_dump_state(), > > because > > these are the only cases where QEMU parses the state of FP registers... this > > is indeed confirmed by the KVM bug you are referring to, that had no visible > > effect for more than a year BTW. > > Ok. > > Still waiting for a reply for my query on 5/7, then I'm happy to apply > these. > Yeah sorry for the delay... I had written a reply but I wasn't happy with my poor English *again* so I spent some more time rewording. I've answered at last ! :) Thanks ! -- Greg