Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Anthony Liguori <aliguori@us.ibm.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> This series fixes the I/O port endianness problems that came when
> port I/O was made to go through the normal dispatch path.  Targets
> that used isa_mmio to invoke cpu_inl and friends now tried to do
> byte swapping once more than they previously did, causing garbage
> to be written.
>
> This series drops isa_mmio and replace it with an alias of the
> root I/O memory region.  After applying back the LITTLE_ENDIAN
> mark for PortioLists, everything works as expected: the port memory
> regions appear directly in the FlatView with the right endianness,
> and MMIO is dispatched straight to them.
>
> PReP cannot use this due to the non-contiguous map, so we just
> make sure the prep_io_ops are marked DEVICE_NATIVE_ENDIAN; the swap
> will happen when prep_io_ops redispatch to the device.
>
> So we had a change that broke multiple ports, most of them
> under-maintained.  What better occasion to add a unit test?  This
> series does that, using the pc-testdev device as the harness.
> The test was run before and after the I/O changes (in particular
> on top of commit a014ed0, memory: accept mismatching sizes in
> memory_region_access_valid, 2013-05-24), and passes there too.
>
> This also found some unrelated problems caused by the new QOM cast in
> commit 7588e2b (pci: Fold host_buses list into PCIHostState functionality,
> 2013-06-06).
>
> I also smoke-tested prep, with the kernel that Herve provided, and
> pseries.  VGA works for both, which tests the PortioList path.
> Alexey tested Linux (with virtio) on pseries.
>
> Patches 1 to 15 fix all bugs and remove isa_mmio.
>
> Patches 16 to 23 prepare the boards so that the testcase runs
> successfully.  This includes adding missing devices and unbreaking
> other breakage.
>
> Patch 24 adds the test case, patches 25-27 fix big-endian split/combine,
> patch 28 adds a test case for that too.
>
> If preferred, I can prepare a pull request that merges the tests
> from an earlier branchpoint.  This makes it easy to run the tests
> on both the old and the new code.
>
> Paolo

Really nice series.  I'd prefer we simply got rid of the endianness flag
entirely but this is a good step.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

>
> Alexey Kardashevskiy (1):
>   spapr_pci: remove indirection for I/O port access
>
> Paolo Bonzini (27):
>   sh4: do not use isa_mmio
>   ppc_oldworld: do not use isa_mmio
>   ppc_newworld: do not use isa_mmio
>   prep: fix I/O port endianness
>   mips_jazz: do not use isa_mmio
>   mips_r4k: do not use isa_mmio
>   mips_malta: do not use isa_mmio
>   ppc440_bamboo: do not use isa_mmio
>   mipssim: do not use isa_mmio
>   mips_fulong2e: do not use isa_mmio
>   sparc64: remove indirection for I/O port access
>   ebus: do not use isa_mmio
>   isa_mmio: delete
>   Revert "ioport: remove LITTLE_ENDIAN mark for portio"
>   pc-testdev: support 8 and 16-bit accesses to 0xe0
>   pc-testdev: remove useless cpu_to_le64/le64_to_cpu
>   mips: degrade BIOS error to warning
>   sh4: unbreak r2d
>   sparc64: unbreak
>   default-configs: add test device to all machines supporting ISA
>   default-configs: add SuperIO to SH4
>   default-configs/ppc64: add all components of i82378 SuperIO chip used
>     by prep
>   qtest: add test for ISA I/O space endianness
>   memory: move functions around
>   memory: pass MemoryRegion to access_with_adjusted_size
>   memory: check memory region endianness, not target's
>   pc-testdev: add I/O port to test memory.c auto split/combine
>
>  default-configs/alpha-softmmu.mak    |   1 +
>  default-configs/mips-softmmu.mak     |   2 +-
>  default-configs/mips64-softmmu.mak   |   2 +-
>  default-configs/mips64el-softmmu.mak |   2 +-
>  default-configs/mipsel-softmmu.mak   |   2 +-
>  default-configs/ppc-softmmu.mak      |   1 +
>  default-configs/ppc64-softmmu.mak    |   7 +
>  default-configs/ppcemb-softmmu.mak   |   1 +
>  default-configs/sh4-softmmu.mak      |   9 +-
>  default-configs/sh4eb-softmmu.mak    |   9 +-
>  default-configs/sparc64-softmmu.mak  |   1 +
>  hw/isa/Makefile.objs                 |   1 -
>  hw/isa/isa_mmio.c                    |  81 ---------
>  hw/mips/gt64xxx_pci.c                |   3 +-
>  hw/mips/mips_fulong2e.c              |   3 +-
>  hw/mips/mips_jazz.c                  |   8 +-
>  hw/mips/mips_malta.c                 |   3 +-
>  hw/mips/mips_mipssim.c               |   8 +-
>  hw/mips/mips_r4k.c                   |   6 +-
>  hw/misc/pc-testdev.c                 |  28 +++-
>  hw/pci-host/apb.c                    | 101 ++++-------
>  hw/pci-host/bonito.c                 |  25 ++-
>  hw/ppc/mac_newworld.c                |   5 +-
>  hw/ppc/mac_oldworld.c                |   5 +-
>  hw/ppc/ppc440_bamboo.c               |   5 +-
>  hw/ppc/prep.c                        |   2 +-
>  hw/ppc/spapr_pci.c                   |  41 +----
>  hw/sh4/sh_pci.c                      |  42 +++--
>  hw/sparc64/sun4u.c                   |   6 +-
>  include/hw/isa/isa.h                 |   3 -
>  ioport.c                             |   1 +
>  memory.c                             | 101 +++++------
>  tests/Makefile                       |  14 +-
>  tests/endianness-test.c              | 316 +++++++++++++++++++++++++++++++++++
>  34 files changed, 543 insertions(+), 302 deletions(-)
>  delete mode 100644 hw/isa/isa_mmio.c
>  create mode 100644 tests/endianness-test.c
>
> -- 
> 1.8.1.4