From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLV75-0005zO-2r for qemu-devel@nongnu.org; Tue, 19 Jan 2016 07:10:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLV6y-0000Go-QK for qemu-devel@nongnu.org; Tue, 19 Jan 2016 07:10:18 -0500 Received: from e06smtp12.uk.ibm.com ([195.75.94.108]:60464) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLV6y-0000GH-IV for qemu-devel@nongnu.org; Tue, 19 Jan 2016 07:10:12 -0500 Received: from localhost by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 19 Jan 2016 12:10:11 -0000 Date: Tue, 19 Jan 2016 13:10:04 +0100 From: Greg Kurz Message-ID: <20160119131004.75cd3b76@bahia.huguette.org> In-Reply-To: <20160119005510.GU9301@voom.fritz.box> References: <20160115150005.17358.43294.stgit@bahia.huguette.org> <20160115150012.17358.95155.stgit@bahia.huguette.org> <20160118021644.GG9301@voom.fritz.box> <20160118095156.31bcbdc4@bahia.huguette.org> <20160119005510.GU9301@voom.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/7] target-ppc: kvm: fix floating point registers sync on little-endian hosts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Paolo Bonzini , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Anton Blanchard , Alexander Graf On Tue, 19 Jan 2016 11:55:10 +1100 David Gibson 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 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 > > > > > > 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