Re: [Qemu-devel] [PATCH v4 3/3] hw/pci: handle downstream pci master abort

[Michael S. Tsirkin]

On Sun, Sep 15, 2013 at 09:40:37PM +0100, Peter Maydell wrote:
> On 15 September 2013 21:25, Michael S. Tsirkin <address@hidden> wrote:
> > On Sun, Sep 15, 2013 at 06:32:13PM +0100, Peter Maydell wrote:
> >> On 15 September 2013 18:30, Michael S. Tsirkin <address@hidden> 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