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