On Sun, Sep 15, 2013 at 09:40:37PM +0100, Peter Maydell wrote: > On 15 September 2013 21:25, Michael S. Tsirkin <m...@redhat.com> wrote: > > On Sun, Sep 15, 2013 at 06:32:13PM +0100, Peter Maydell wrote: > >> On 15 September 2013 18:30, Michael S. Tsirkin <m...@redhat.com> wrote: > >> > On Sun, Sep 15, 2013 at 07:16:41PM +0300, Marcel Apfelbaum wrote: > >> >> +static const MemoryRegionOps master_abort_mem_ops = { > >> >> + .read = master_abort_mem_read, > >> >> + .write = master_abort_mem_write, > >> >> + .endianness = DEVICE_NATIVE_ENDIAN, > >> >> +}; > >> >> + > >> > > >> > Please make it little endian. > >> > DEVICE_NATIVE_ENDIAN is almost always a bug. > >> > >> ...when dealing with PCI devices. For a random device on the system bus > >> it's often correct. > > > native is really qemu host endian-ness ... what are some > > examples when it's actually correct? > > "native" means "if the device's MMIO callback does 'return 0x12345678;' > for a 32 bit read then the guest CPU should see 0x12345678". That's > almost always what you want for simple devices (which may in fact > only support 32 bit accesses to registers), because it means you don't > have to fill your device with explicit endianness swaps.
But this means that you device behaves differently depending on the endian-ness of the guest system. So it only makes sense if the device is very system specific: anything outside hw/<specific architecture> is at least in theory not a system specific device so it should not do this. > It's also useful > if that kind of simple device might be built into either a little endian > or a bigendian system: as far as the device is concerned its 32 bit > registers still have (say) the TXRDY bit at the low end of the status > register even if the CPU is bigendian. > > That said, our current setup of marking the mmio ops with an endianness > is really conflating what in real hardware is a number of distinct properties > of the CPU, the bus, any bus controllers (like PCI!) in the path between > CPU and device and finally the device itself. So it's inherently both > confusing and confused. > > -- PMM