[Qemu-devel] Re: [PATCH 00/15] MMIO endianness cleanup v2

[Blue Swirl]

Thanks, applied.

On Wed, Dec 8, 2010 at 11:05 AM, Alexander Graf <address@hidden> wrote:
> The way mmio endianness is currently implemented is horrifying.
>
> In the real world, CPUs have an endianness and write out data
> to the memory bus. Instead of RAM, a receiving side here can be
> a device. This device gets a byte stream again and needs to
> make sense of it.
>
> Since big endian systems write big endian numbers into memory
> while little endian systems write little endian numbers there,
> the device and software on the CPU need to be aware of this.
>
> In practice, most devices these days (ISA, PCI) assume that
> the data is little endian. So to communicate with such a device
> from the CPU's side, the OS byte swaps all MMIO.
>
> In qemu however, we simply pass the register value we find on
> to the device. So any byte mangling the guest does to compensate
> for the transfer screw us up by exposing byte swapped MMIO
> on the device's side.
>
> The way this has been fixed historically is by constructs like
> this one:
>
> #ifdef TARGET_WORDS_BIGENDIAN
>    val = bswap32(val);
> #endif
>
> With the move to get device code only compiled once, this has
> become harder and harder to justify though, since we don't know
> the target endianness during compile time.
>
> It's especially bad since it doesn't make any sense at all to
> clutter all the device code with endianness workarounds, aside
> from the fact that about 80% of the device code currently does
> the wrong thing :).
>
> So my solution to the issue is to make every device define if
> it's a little, big or native (target) endianness device. This
> basically tells the layers below what endianness the device
> expects mmio to occur in. Little endian devices on little endian
> hosts don't swap. On big endian hosts they do. Same the other
> way around.
>
> The only reason I added "native" endianness is that we have some
> PV devices like the fw_cfg that expect qemu's broken behavior.
> These devices are the minority though. In the long run I'd expect
> to see most code be committed with either of the two endianness
> choices.
>
> The patch set also includes a bunch of conversions for devices
> that were already aware of endianness.
>
> For easy testing or pulling, please use this repo:
>
>  git://repo.or.cz/qemu/agraf.git qemu-endian-fix-v2
>
> v0->v1:
>
>  - make LE targets compile
>  - add one missing conversion
>  - make endian choice be an enum
>
> v1 -> v2:
>
>  - rebase (this thing bitrots _fast_!)
>
> Alexander Graf (15):
>  exec: introduce endianness swapped mmio
>  Add endianness as io mem parameter
>  Make simple io mem handler endian aware
>  dbdma: Make little endian
>  pci-host: Delegate bswap to mmio layer
>  uninorth: Get rid of bswap
>  e1000: Make little endian
>  prep: Declare as little endian
>  versatile_pci: Declare as little endian
>  ppc4xx_pci: Declare as little endian
>  openpic: Replace explicit byte swap with endian hints
>  rtl8139: Declare as little endian
>  heathrow_pic: Declare as little endian
>  isa_mmio: Always use little endian
>  usb_ohci: Always use little endian
>
>  Makefile.objs              |    3 +
>  Makefile.target            |    7 --
>  cpu-common.h               |    8 ++-
>  exec.c                     |  142 
> +++++++++++++++++++++++++++++++++++++++++---
>  hw/apb_pci.c               |    9 ++-
>  hw/apic.c                  |    3 +-
>  hw/arm_gic.c               |    3 +-
>  hw/arm_sysctl.c            |    3 +-
>  hw/arm_timer.c             |    5 +-
>  hw/armv7m.c                |    2 +-
>  hw/axis_dev88.c            |    6 +-
>  hw/bonito.c                |   19 ++++--
>  hw/cirrus_vga.c            |   12 +++-
>  hw/cs4231.c                |    3 +-
>  hw/cuda.c                  |    3 +-
>  hw/dec_pci.c               |    6 +-
>  hw/dp8393x.c               |    3 +-
>  hw/ds1225y.c               |    6 +-
>  hw/e1000.c                 |   11 +---
>  hw/eccmemctl.c             |    6 +-
>  hw/eepro100.c              |    3 +-
>  hw/empty_slot.c            |    3 +-
>  hw/escc.c                  |    3 +-
>  hw/esp.c                   |    3 +-
>  hw/etraxfs_dma.c           |    2 +-
>  hw/etraxfs_eth.c           |    3 +-
>  hw/etraxfs_pic.c           |    3 +-
>  hw/etraxfs_ser.c           |    3 +-
>  hw/etraxfs_timer.c         |    3 +-
>  hw/fdc.c                   |    6 +-
>  hw/fw_cfg.c                |    6 +-
>  hw/g364fb.c                |    3 +-
>  hw/grackle_pci.c           |    6 +-
>  hw/gt64xxx.c               |    9 +--
>  hw/heathrow_pic.c          |    5 +-
>  hw/hpet.c                  |    3 +-
>  hw/ide/macio.c             |    3 +-
>  hw/ide/mmio.c              |    6 +-
>  hw/integratorcp.c          |    9 ++-
>  hw/intel-hda.c             |    3 +-
>  hw/ioapic.c                |    3 +-
>  hw/isa.h                   |    2 +-
>  hw/isa_mmio.c              |  100 ++++++-------------------------
>  hw/ivshmem.c               |    2 +-
>  hw/jazz_led.c              |    3 +-
>  hw/lan9118.c               |    3 +-
>  hw/lance.c                 |    3 +-
>  hw/lsi53c895a.c            |    6 +-
>  hw/m48t59.c                |    3 +-
>  hw/mac_dbdma.c             |    6 +-
>  hw/mac_nvram.c             |    3 +-
>  hw/marvell_88w8618_audio.c |    3 +-
>  hw/mcf5206.c               |    3 +-
>  hw/mcf5208.c               |    6 +-
>  hw/mcf_fec.c               |    3 +-
>  hw/mcf_intc.c              |    3 +-
>  hw/mcf_uart.c              |    3 +-
>  hw/mips_jazz.c             |   13 ++---
>  hw/mips_malta.c            |    3 +-
>  hw/mips_mipssim.c          |    6 +--
>  hw/mips_r4k.c              |    9 +--
>  hw/mpcore.c                |    3 +-
>  hw/msix.c                  |    3 +-
>  hw/mst_fpga.c              |    2 +-
>  hw/musicpal.c              |   24 +++++---
>  hw/omap.h                  |    3 +-
>  hw/omap1.c                 |   42 +++++++------
>  hw/omap2.c                 |    4 +-
>  hw/omap_dma.c              |    4 +-
>  hw/omap_dss.c              |    2 +-
>  hw/omap_gpio.c             |    2 +-
>  hw/omap_gpmc.c             |    2 +-
>  hw/omap_i2c.c              |    2 +-
>  hw/omap_intc.c             |    4 +-
>  hw/omap_l4.c               |    5 +-
>  hw/omap_lcdc.c             |    2 +-
>  hw/omap_mmc.c              |    2 +-
>  hw/omap_sdrc.c             |    2 +-
>  hw/omap_sx1.c              |   15 +++--
>  hw/omap_uart.c             |    2 +-
>  hw/onenand.c               |    2 +-
>  hw/openpic.c               |   29 ++-------
>  hw/palm.c                  |   12 +++-
>  hw/parallel.c              |    3 +-
>  hw/pci_host.c              |  101 ++++++-------------------------
>  hw/pci_host.h              |    6 +-
>  hw/pcie_host.c             |    3 +-
>  hw/pckbd.c                 |    3 +-
>  hw/pcnet-pci.c             |    3 +-
>  hw/pflash_cfi01.c          |    6 +-
>  hw/pflash_cfi02.c          |    4 +-
>  hw/pl011.c                 |    3 +-
>  hw/pl022.c                 |    3 +-
>  hw/pl031.c                 |    3 +-
>  hw/pl050.c                 |    3 +-
>  hw/pl061.c                 |    3 +-
>  hw/pl080.c                 |    3 +-
>  hw/pl110.c                 |    3 +-
>  hw/pl181.c                 |    4 +-
>  hw/pl190.c                 |    3 +-
>  hw/ppc405_boards.c         |    6 +-
>  hw/ppc405_uc.c             |   11 ++-
>  hw/ppc440.c                |    2 +-
>  hw/ppc4xx_pci.c            |   19 +-----
>  hw/ppc_newworld.c          |    5 +-
>  hw/ppc_oldworld.c          |    2 +-
>  hw/ppc_prep.c              |   40 ++-----------
>  hw/ppce500_mpc8544ds.c     |    2 +-
>  hw/ppce500_pci.c           |    9 ++-
>  hw/prep_pci.c              |    3 +-
>  hw/pxa2xx.c                |   25 ++++----
>  hw/pxa2xx_dma.c            |    2 +-
>  hw/pxa2xx_gpio.c           |    2 +-
>  hw/pxa2xx_keypad.c         |    2 +-
>  hw/pxa2xx_lcd.c            |    2 +-
>  hw/pxa2xx_mmci.c           |    2 +-
>  hw/pxa2xx_pcmcia.c         |    6 +-
>  hw/pxa2xx_pic.c            |    2 +-
>  hw/pxa2xx_timer.c          |    2 +-
>  hw/r2d.c                   |    3 +-
>  hw/rc4030.c                |    6 +-
>  hw/realview.c              |    3 +-
>  hw/realview_gic.c          |    3 +-
>  hw/rtl8139.c               |   15 +----
>  hw/sbi.c                   |    3 +-
>  hw/serial.c                |    6 +-
>  hw/sh7750.c                |    6 +-
>  hw/sh_intc.c               |    3 +-
>  hw/sh_pci.c                |    7 +-
>  hw/sh_serial.c             |    3 +-
>  hw/sh_timer.c              |    3 +-
>  hw/slavio_intctl.c         |    6 +-
>  hw/slavio_misc.c           |   24 +++++---
>  hw/slavio_timer.c          |    3 +-
>  hw/sm501.c                 |    9 ++-
>  hw/smc91c111.c             |    3 +-
>  hw/sparc32_dma.c           |    3 +-
>  hw/spitz.c                 |    2 +-
>  hw/stellaris.c             |   12 +++-
>  hw/stellaris_enet.c        |    3 +-
>  hw/sun4c_intctl.c          |    3 +-
>  hw/sun4m_iommu.c           |    3 +-
>  hw/sun4u.c                 |    4 +-
>  hw/syborg_fb.c             |    3 +-
>  hw/syborg_interrupt.c      |    3 +-
>  hw/syborg_keyboard.c       |    3 +-
>  hw/syborg_pointer.c        |    3 +-
>  hw/syborg_rtc.c            |    3 +-
>  hw/syborg_serial.c         |    3 +-
>  hw/syborg_timer.c          |    3 +-
>  hw/syborg_virtio.c         |    3 +-
>  hw/tc6393xb.c              |    2 +-
>  hw/tcx.c                   |    5 +-
>  hw/tusb6010.c              |    2 +-
>  hw/unin_pci.c              |   26 +++++---
>  hw/usb-ohci.c              |   10 +---
>  hw/versatile_pci.c         |   21 +------
>  hw/versatilepb.c           |    3 +-
>  hw/vga-isa-mm.c            |    6 +-
>  hw/vga.c                   |    3 +-
>  hw/vmware_vga.c            |    2 +-
>  hw/wdt_i6300esb.c          |    3 +-
>  hw/xilinx_ethlite.c        |    2 +-
>  hw/xilinx_intc.c           |    2 +-
>  hw/xilinx_timer.c          |    3 +-
>  hw/xilinx_uartlite.c       |    3 +-
>  hw/zaurus.c                |    2 +-
>  rwhandler.c                |    4 +-
>  rwhandler.h                |    2 +-
>  169 files changed, 663 insertions(+), 598 deletions(-)
>
>