From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NN4Bo-00019c-6X for qemu-devel@nongnu.org; Tue, 22 Dec 2009 07:50:12 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NN4Bj-00018V-U2 for qemu-devel@nongnu.org; Tue, 22 Dec 2009 07:50:11 -0500 Received: from [199.232.76.173] (port=36537 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NN4Bj-00018S-O5 for qemu-devel@nongnu.org; Tue, 22 Dec 2009 07:50:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:3039) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NN4Bj-0001FC-BO for qemu-devel@nongnu.org; Tue, 22 Dec 2009 07:50:07 -0500 Date: Tue, 22 Dec 2009 14:47:08 +0200 From: "Michael S. Tsirkin" Message-ID: <20091222124708.GE16165@redhat.com> References: <1261477458-26222-1-git-send-email-agraf@suse.de> <20091222113626.GB16053@redhat.com> <4B30B678.1060902@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B30B678.1060902@suse.de> Subject: [Qemu-devel] Re: [PATCH] Always swap endianness in DBDMA List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: qemu-devel@nongnu.org, Aurelien Jarno , Laurent Vivier On Tue, Dec 22, 2009 at 01:07:20PM +0100, Alexander Graf wrote: > Michael S. Tsirkin wrote: > > On Tue, Dec 22, 2009 at 11:24:18AM +0100, Alexander Graf wrote: > > > >> When we get an MMIO request, we always get variables in host endianness. The > >> only time we need to actually reverse byte order is when we read bytes from > >> guest memory. > >> > >> Apparently the DBDMA implementation is different there. A lot of the logic > >> in there depends on values being big endian. Now, qemu does all the conversion > >> in the MMIO handlers for us already though, so it turns out that we're in > >> the same byte order from a C point of view, but cpu_to_be32 and be32_to_cpu > >> end up being nops. > >> > >> This makes the code work differently on x86 (little endian) than on ppc (big > >> endian). On x86 it works, on ppc it doesn't. > >> > >> This patch (while being seriously hacky and ugly) makes dbdma emulation work > >> on ppc hosts. I'll leave the real fixing to someone else. > >> > > > > Come on, > > > > #define cpu_to_dbdma32 bswap32 > > #define dbdma_to_cpu32 bswap32 > > > > and then > > > > s/cpu_to_be32/cpu_to_dbdma32/g > > s/be32_to_cpu/dbdma32_to_cpu/g > > > > is not too hard, is it? > > > > Well, if I'd want to do a sed'ish approach I'd just > > s/cpu_to_be32/bswap32/g > s/be32_to_cpu/bswap32/g This would throw away some information: it is better to know whether you are converting from or to cpu mode. Hopefully at some point we will add sparce annotations which will make this even more important. > The problem is that the whole define is just plain wrong which tells me > that the code is using the bswap functions incorrectly. This really > needs to be fixed by someone who knows the dbdma device. I don't see how > calling incorrect calls even more incorrect makes any difference. > > Alex At least build will not break in strange ways e.g. when someone changes cpu_to_be32 to a macro. I don't really know about this hardware either, but why make it more fragile than it already is? -- MST