mbox series

[Qemu-devel,v7,00/42] Invert Endian bit in SPARCv9 MMU TTE

Message ID 43bc5e07ac614d0e8e740bf6007ff77b@tpw09926dag18e.domain1.systemhost.net (mailing list archive)
Headers show
Series Invert Endian bit in SPARCv9 MMU TTE | expand

Message

Tony Nguyen Aug. 16, 2019, 6:28 a.m. UTC
This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.

It is an attempt of the instructions outlined by Richard Henderson to Mark
Cave-Ayland.

Tested with OpenBSD on sun4u. Solaris 10 is my actual goal, but unfortunately a
separate keyboard issue remains in the way.

On 01/11/17 19:15, Mark Cave-Ayland wrote:

>On 15/08/17 19:10, Richard Henderson wrote:
>
>> [CC Peter re MemTxAttrs below]
>>
>> On 08/15/2017 09:38 AM, Mark Cave-Ayland wrote:
>>> Working through an incorrect endian issue on qemu-system-sparc64, it has
>>> become apparent that at least one OS makes use of the IE (Invert Endian)
>>> bit in the SPARCv9 MMU TTE to map PCI memory space without the
>>> programmer having to manually endian-swap accesses.
>>>
>>> In other words, to quote the UltraSPARC specification: "if this bit is
>>> set, accesses to the associated page are processed with inverse
>>> endianness from what is specified by the instruction (big-for-little and
>>> little-for-big)".

A good explanation by Mark why the IE bit is required.

>>>
>>> Looking through various bits of code, I'm trying to get a feel for the
>>> best way to implement this in an efficient manner. From what I can see
>>> this could be solved using an additional MMU index, however I'm not
>>> overly familiar with the memory and softmmu subsystems.
>>
>> No, it can't be solved with an MMU index.
>>
>>> Can anyone point me in the right direction as to what would be the best
>>> way to implement this feature within QEMU?
>>
>> It's definitely tricky.
>>
>> We definitely need some TLB_FLAGS_MASK bit set so that we're forced through
>> the
>> memory slow path.  There is no other way to bypass the endianness that we've
>> already encoded from the target instruction.
>>
>> Given the tlb_set_page_with_attrs interface, I would think that we need a new
>> bit in MemTxAttrs, so that the target/sparc tlb_fill (and subroutines) can
>> pass
>> along the TTE bit for the given page.
>>
>> We have an existing problem in softmmu_template.h,
>>
>>     /* ??? Note that the io helpers always read data in the target
>>        byte ordering.  We should push the LE/BE request down into io.  */
>>     res = glue(io_read, SUFFIX)(env, mmu_idx, index, addr, retaddr);
>>     res = TGT_BE(res);
>>
>> We do not want to add a third(!) byte swap along the i/o path.  We need to
>> collapse the two that we have already before considering this one.
>>
>> This probably takes the form of:
>>
>> (1) Replacing the "int size" argument with "TCGMemOp memop" for
>>       a) io_{read,write}x in accel/tcg/cputlb.c,
>>       b) memory_region_dispatch_{read,write} in memory.c,
>>       c) adjust_endianness in memory.c.
>>     This carries size+sign+endianness down to the next level.
>>
>> (2) In memory.c, adjust_endianness,
>>
>>      if (memory_region_wrong_endianness(mr)) {
>> -        switch (size) {
>> +        memop ^= MO_BSWAP;
>> +    }
>> +    if (memop & MO_BSWAP) {
>>
>>     For extra credit, re-arrange memory_region_wrong_endianness
>>     to something more explicit -- "wrong" isn't helpful.
>
>Finally I've had a bit of spare time to experiment with this approach,
>and from what I can see there are currently 2 issues:
>
>
>1) Using TCGMemOp in memory.c means it is no longer accelerator agnostic
>
>For the moment I've defined a separate MemOp in memory.h and provided a
>mapping function in io_{read,write}x to map from TCGMemOp to MemOp and
>then pass that into memory_region_dispatch_{read,write}.
>
>Other than not referencing TCGMemOp in the memory API, another reason
>for doing this was that I wasn't convinced that all the MO_ attributes
>were valid outside of TCG. I do, of course, strongly defer to other
>people's knowledge in this area though.
>
>
>2) The above changes to adjust_endianness() fail when
>memory_region_dispatch_{read,write} are called recursively
>
>Whilst booting qemu-system-sparc64 I see that
>memory_region_dispatch_{read,write} get called recursively - once via
>io_{read,write}x and then again via flatview_read_continue() in exec.c.
>
>The net effect of this is that we perform the bswap correctly at the
>tail of the recursion, but then as we travel back up the stack we hit
>memory_region_dispatch_{read,write} once again causing a second bswap
>which means the value is returned with the incorrect endian again.
>
>
>My understanding from your softmmu_template.h comment above is that the
>memory API should do the endian swapping internally allowing the removal
>of the final TGT_BE/TGT_LE applied to the result, or did I get this wrong?
>
>> (3) In tlb_set_page_with_attrs, notice attrs.byte_swap and set
>>     a new TLB_FORCE_SLOW bit within TLB_FLAGS_MASK.
>>
>> (4) In io_{read,write}x, if iotlbentry->attrs.byte_swap is set,
>>     then memop ^= MO_BSWAP.

Thanks all for the feedback. Learnt a lot =)

v2:
- Moved size+sign+endianness attributes from TCGMemOp into MemOp.
  In v1 TCGMemOp was re-purposed entirely into MemOp.
- Replaced MemOp MO_{8|16|32|64} with TCGMemOp MO_{UB|UW|UL|UQ} alias.
  This is to avoid warnings on comparing and coercing different enums.
- Renamed get_memop to get_tcgmemop for clarity.
- MEMOP is now SIZE_MEMOP, which is just ctzl(size).
- Split patch 3/4 so one memory_region_dispatch_{read|write} interface
  is converted per patch.
- Do not reuse TLB_RECHECK, use new TLB_FORCE_SLOW instead.
- Split patch 4/4 so adding the MemTxAddrs parameters and converting
  tlb_set_page() to tlb_set_page_with_attrs() is separate from usage.
- CC'd maintainers.

v3:
- Like v1, the entire TCGMemOp enum is now MemOp.
- MemOp target dependant attributes are conditional upon NEED_CPU_H

v4:
- Added Paolo Bonzini as include/exec/memop.h maintainer

v5:
- Improved commit messages to clarify how interface to access
  MemoryRegion will be converted from "unsigned size" to "MemOp op".
- Moved cpu_transaction_failed() MemOp conversion from patch #11 to #9
  to make review easier.

v6:
- Improved commit messages.
- Include as patch #1 an earlier posted TARGET_ALIGNED_ONLY configure patch.
- Typeless macro SIZE_MEMOP is now inline.
- size_memop now includes CONFIG_DEBUG_TCG code.
- size_memop now also encodes endianness via MO_TE.
- Reverted size_memop operand "unsigned long" back to "unsigned".
- Second pass of size_memop to replace no-op place holder with MO_{8|16|32|64}.
- Delay memory_region_dispatch_{read,write} operand conversion until no-op
  size_memop is implemented so we have proper typing at all points in between.
- Fixed bug where not all memory_region_dispatch_{read,write} callers where
  encoding endianness into the MemOp operand, see patch #20.
- Fixed bug where not all memory_region_dispatch_{read,write} callers were
  collapsing their byte swap into adjust_endianness, see patch #20 and #22.
- Split byte swap collapsing patch (v5 #11) into #21 and #22.
- Corrected non-common *-common-obj to *-obj.
- Replaced enum device_endian with MemOp to simplify endianness checks. A
  straight forward sed but touched *alot* of files. See patch #16 and #17.
- Deleted enum device_endian.
- Deleted DEVICE_HOST_ENDIAN definition.
- Generalized the description of introduced MemTxAttrs attribute byte_swap.

v7:
- Fixed bug where size_memop was implicitly encoding MO_TE. Endianness,
  {MO_TE|MO_BE|MO_LE}, is now explicitly encoded by MemoryRegion accessors.
- While a no-op, size_memop return type remains an unsigned.
- Use '= 0' short hand instead of macro logic to declare host endianness.
- With a new set of constant arguments, sanity checked the compiler is still
  folding away tests in cputlb.c
- Re-declared many native endian devices as little or big endian. This is why
  v7 has +16 patches.

Tony Nguyen (42):
  configure: Define TARGET_ALIGNED_ONLY in configure
  tcg: TCGMemOp is now accelerator independent MemOp
  memory: Introduce size_memop
  target/mips: Access MemoryRegion with MemOp
  hw/s390x: Access MemoryRegion with MemOp
  hw/intc/armv7m_nic: Access MemoryRegion with MemOp
  hw/virtio: Access MemoryRegion with MemOp
  hw/vfio: Access MemoryRegion with MemOp
  exec: Access MemoryRegion with MemOp
  cputlb: Access MemoryRegion with MemOp
  memory: Access MemoryRegion with MemOp
  hw/s390x: Hard code size with MO_{8|16|32|64}
  target/mips: Hard code size with MO_{8|16|32|64}
  exec: Hard code size with MO_{8|16|32|64}
  hw/audio: Declare device little or big endian
  hw/block: Declare device little or big endian
  hw/char: Declare device little or big endian
  hw/display: Declare device little or big endian
  hw/dma: Declare device little or big endian
  hw/gpio: Declare device little or big endian
  hw/i2c: Declare device little or big endian
  hw/input: Declare device little or big endian
  hw/intc: Declare device little or big endian
  hw/isa: Declare device little or big endian
  hw/misc: Declare device little or big endian
  hw/net: Declare device little or big endian
  hw/pci-host: Declare device little or big endian
  hw/sd: Declare device little or big endian
  hw/ssi: Declare device little or big endian
  hw/timer: Declare device little or big endian
  build: Correct non-common common-obj-* to obj-*
  exec: Map device_endian onto MemOp
  exec: Replace device_endian with MemOp
  exec: Delete device_endian
  exec: Delete DEVICE_HOST_ENDIAN
  memory: Access MemoryRegion with endianness
  cputlb: Replace size and endian operands for MemOp
  memory: Single byte swap along the I/O path
  cpu: TLB_FLAGS_MASK bit to force memory slow path
  cputlb: Byte swap memory transaction attribute
  target/sparc: Add TLB entry with attributes
  target/sparc: sun4u Invert Endian TTE bit

 MAINTAINERS                             |   1 +
 accel/tcg/cputlb.c                      | 197 ++++++++++++++------------------
 configure                               |  10 +-
 exec.c                                  |  15 ++-
 hw/acpi/core.c                          |   6 +-
 hw/acpi/cpu.c                           |   2 +-
 hw/acpi/cpu_hotplug.c                   |   2 +-
 hw/acpi/ich9.c                          |   4 +-
 hw/acpi/memory_hotplug.c                |   2 +-
 hw/acpi/nvdimm.c                        |   2 +-
 hw/acpi/pcihp.c                         |   2 +-
 hw/acpi/piix4.c                         |   2 +-
 hw/acpi/tco.c                           |   2 +-
 hw/adc/stm32f2xx_adc.c                  |   2 +-
 hw/alpha/pci.c                          |   6 +-
 hw/alpha/typhoon.c                      |   6 +-
 hw/arm/allwinner-a10.c                  |   2 +-
 hw/arm/armv7m.c                         |   2 +-
 hw/arm/aspeed.c                         |   2 +-
 hw/arm/aspeed_soc.c                     |   2 +-
 hw/arm/exynos4210.c                     |   2 +-
 hw/arm/highbank.c                       |   2 +-
 hw/arm/integratorcp.c                   |   6 +-
 hw/arm/kzm.c                            |   2 +-
 hw/arm/msf2-soc.c                       |   2 +-
 hw/arm/musicpal.c                       |  20 ++--
 hw/arm/omap1.c                          |  40 +++----
 hw/arm/omap2.c                          |  10 +-
 hw/arm/omap_sx1.c                       |   2 +-
 hw/arm/palm.c                           |   2 +-
 hw/arm/pxa2xx.c                         |  20 ++--
 hw/arm/pxa2xx_gpio.c                    |   2 +-
 hw/arm/pxa2xx_pic.c                     |   2 +-
 hw/arm/smmuv3.c                         |   2 +-
 hw/arm/spitz.c                          |   2 +-
 hw/arm/stellaris.c                      |   8 +-
 hw/arm/strongarm.c                      |  12 +-
 hw/arm/versatilepb.c                    |   2 +-
 hw/audio/Makefile.objs                  |   3 +-
 hw/audio/ac97.c                         |   4 +-
 hw/audio/cs4231.c                       |   2 +-
 hw/audio/es1370.c                       |   2 +-
 hw/audio/intel-hda.c                    |   2 +-
 hw/audio/marvell_88w8618.c              |   2 +-
 hw/audio/milkymist-ac97.c               |   2 +-
 hw/audio/pl041.c                        |   2 +-
 hw/block/Makefile.objs                  |   6 +-
 hw/block/fdc.c                          |   4 +-
 hw/block/nvme.c                         |   4 +-
 hw/block/onenand.c                      |   2 +-
 hw/block/pflash_cfi01.c                 |   2 +-
 hw/block/pflash_cfi02.c                 |   2 +-
 hw/char/Makefile.objs                   |   4 +-
 hw/char/bcm2835_aux.c                   |   2 +-
 hw/char/cadence_uart.c                  |   2 +-
 hw/char/cmsdk-apb-uart.c                |   2 +-
 hw/char/debugcon.c                      |   2 +-
 hw/char/digic-uart.c                    |   2 +-
 hw/char/escc.c                          |   2 +-
 hw/char/etraxfs_ser.c                   |   2 +-
 hw/char/exynos4210_uart.c               |   2 +-
 hw/char/grlib_apbuart.c                 |   2 +-
 hw/char/imx_serial.c                    |   2 +-
 hw/char/lm32_uart.c                     |   2 +-
 hw/char/mcf_uart.c                      |   2 +-
 hw/char/milkymist-uart.c                |   2 +-
 hw/char/nrf51_uart.c                    |   2 +-
 hw/char/omap_uart.c                     |   6 +-
 hw/char/parallel.c                      |   2 +-
 hw/char/pl011.c                         |   2 +-
 hw/char/serial.c                        |  26 ++---
 hw/char/sh_serial.c                     |   2 +-
 hw/char/stm32f2xx_usart.c               |   2 +-
 hw/char/xilinx_uartlite.c               |   2 +-
 hw/core/Makefile.objs                   |   2 +-
 hw/core/empty_slot.c                    |   2 +-
 hw/cris/axis_dev88.c                    |   4 +-
 hw/display/Makefile.objs                |   6 +-
 hw/display/ati.c                        |   2 +-
 hw/display/bcm2835_fb.c                 |   2 +-
 hw/display/bochs-display.c              |   4 +-
 hw/display/cg3.c                        |   2 +-
 hw/display/cirrus_vga.c                 |  10 +-
 hw/display/edid-region.c                |   2 +-
 hw/display/exynos4210_fimd.c            |   2 +-
 hw/display/g364fb.c                     |   2 +-
 hw/display/jazz_led.c                   |   2 +-
 hw/display/milkymist-tmu2.c             |   2 +-
 hw/display/milkymist-vgafb.c            |   2 +-
 hw/display/omap_dss.c                   |  10 +-
 hw/display/omap_lcdc.c                  |   2 +-
 hw/display/pl110.c                      |   2 +-
 hw/display/pxa2xx_lcd.c                 |   2 +-
 hw/display/sm501.c                      |  10 +-
 hw/display/tc6393xb.c                   |   2 +-
 hw/display/tcx.c                        |  14 +--
 hw/display/vga-isa-mm.c                 |   2 +-
 hw/display/vga-pci.c                    |   6 +-
 hw/display/vga.c                        |   2 +-
 hw/display/vmware_vga.c                 |   2 +-
 hw/display/xlnx_dp.c                    |   8 +-
 hw/dma/Makefile.objs                    |   6 +-
 hw/dma/bcm2835_dma.c                    |   4 +-
 hw/dma/etraxfs_dma.c                    |   2 +-
 hw/dma/i8257.c                          |   4 +-
 hw/dma/omap_dma.c                       |   4 +-
 hw/dma/pl080.c                          |   2 +-
 hw/dma/pl330.c                          |   2 +-
 hw/dma/puv3_dma.c                       |   2 +-
 hw/dma/pxa2xx_dma.c                     |   2 +-
 hw/dma/rc4030.c                         |   4 +-
 hw/dma/sparc32_dma.c                    |   2 +-
 hw/dma/xilinx_axidma.c                  |   2 +-
 hw/dma/xlnx-zdma.c                      |   2 +-
 hw/dma/xlnx-zynq-devcfg.c               |   2 +-
 hw/dma/xlnx_dpdma.c                     |   2 +-
 hw/gpio/Makefile.objs                   |   2 +-
 hw/gpio/bcm2835_gpio.c                  |   2 +-
 hw/gpio/imx_gpio.c                      |   2 +-
 hw/gpio/mpc8xxx.c                       |   2 +-
 hw/gpio/nrf51_gpio.c                    |   2 +-
 hw/gpio/omap_gpio.c                     |   6 +-
 hw/gpio/pl061.c                         |   2 +-
 hw/gpio/puv3_gpio.c                     |   2 +-
 hw/gpio/zaurus.c                        |   2 +-
 hw/hppa/dino.c                          |   6 +-
 hw/hppa/machine.c                       |   2 +-
 hw/hppa/pci.c                           |   6 +-
 hw/hyperv/hyperv_testdev.c              |   2 +-
 hw/i2c/Makefile.objs                    |   2 +-
 hw/i2c/aspeed_i2c.c                     |   4 +-
 hw/i2c/exynos4210_i2c.c                 |   2 +-
 hw/i2c/imx_i2c.c                        |   2 +-
 hw/i2c/microbit_i2c.c                   |   2 +-
 hw/i2c/mpc_i2c.c                        |   2 +-
 hw/i2c/omap_i2c.c                       |   2 +-
 hw/i2c/pm_smbus.c                       |   2 +-
 hw/i2c/ppc4xx_i2c.c                     |   2 +-
 hw/i2c/versatile_i2c.c                  |   2 +-
 hw/i386/amd_iommu.c                     |   4 +-
 hw/i386/intel_iommu.c                   |   4 +-
 hw/i386/kvm/apic.c                      |   2 +-
 hw/i386/kvmvapic.c                      |   2 +-
 hw/i386/pc.c                            |   6 +-
 hw/i386/vmport.c                        |   2 +-
 hw/i386/xen/xen_apic.c                  |   2 +-
 hw/i386/xen/xen_platform.c              |   4 +-
 hw/i386/xen/xen_pvdevice.c              |   2 +-
 hw/ide/ahci-allwinner.c                 |   2 +-
 hw/ide/ahci.c                           |   4 +-
 hw/ide/macio.c                          |   2 +-
 hw/ide/mmio.c                           |   4 +-
 hw/ide/pci.c                            |   6 +-
 hw/ide/sii3112.c                        |   2 +-
 hw/input/Makefile.objs                  |   2 +-
 hw/input/milkymist-softusb.c            |   2 +-
 hw/input/pckbd.c                        |   6 +-
 hw/input/pl050.c                        |   2 +-
 hw/input/pxa2xx_keypad.c                |   2 +-
 hw/intc/Makefile.objs                   |   6 +-
 hw/intc/allwinner-a10-pic.c             |   2 +-
 hw/intc/apic.c                          |   2 +-
 hw/intc/arm_gic.c                       |  12 +-
 hw/intc/arm_gicv2m.c                    |   2 +-
 hw/intc/arm_gicv3.c                     |   4 +-
 hw/intc/arm_gicv3_its_common.c          |   2 +-
 hw/intc/armv7m_nvic.c                   |  19 +--
 hw/intc/aspeed_vic.c                    |   2 +-
 hw/intc/bcm2835_ic.c                    |   2 +-
 hw/intc/bcm2836_control.c               |   2 +-
 hw/intc/etraxfs_pic.c                   |   2 +-
 hw/intc/exynos4210_combiner.c           |   2 +-
 hw/intc/grlib_irqmp.c                   |   2 +-
 hw/intc/heathrow_pic.c                  |   2 +-
 hw/intc/imx_avic.c                      |   2 +-
 hw/intc/imx_gpcv2.c                     |   2 +-
 hw/intc/ioapic.c                        |   2 +-
 hw/intc/mips_gic.c                      |   2 +-
 hw/intc/omap_intc.c                     |   4 +-
 hw/intc/ompic.c                         |   2 +-
 hw/intc/openpic.c                       |  20 ++--
 hw/intc/openpic_kvm.c                   |   2 +-
 hw/intc/pl190.c                         |   2 +-
 hw/intc/pnv_xive.c                      |  14 +--
 hw/intc/puv3_intc.c                     |   2 +-
 hw/intc/sh_intc.c                       |   2 +-
 hw/intc/slavio_intctl.c                 |   4 +-
 hw/intc/xics_pnv.c                      |   2 +-
 hw/intc/xilinx_intc.c                   |   2 +-
 hw/intc/xive.c                          |   6 +-
 hw/intc/xlnx-pmu-iomod-intc.c           |   2 +-
 hw/intc/xlnx-zynqmp-ipi.c               |   2 +-
 hw/ipack/Makefile.objs                  |   2 +-
 hw/ipack/tpci200.c                      |  10 +-
 hw/ipmi/isa_ipmi_bt.c                   |   2 +-
 hw/ipmi/isa_ipmi_kcs.c                  |   2 +-
 hw/isa/lpc_ich9.c                       |   4 +-
 hw/isa/pc87312.c                        |   2 +-
 hw/isa/vt82c686.c                       |   2 +-
 hw/m68k/mcf5206.c                       |   2 +-
 hw/m68k/mcf5208.c                       |   4 +-
 hw/m68k/mcf_intc.c                      |   2 +-
 hw/microblaze/petalogix_ml605_mmu.c     |   2 +-
 hw/mips/boston.c                        |   6 +-
 hw/mips/gt64xxx_pci.c                   |   2 +-
 hw/mips/mips_jazz.c                     |   8 +-
 hw/mips/mips_malta.c                    |   4 +-
 hw/mips/mips_r4k.c                      |   2 +-
 hw/misc/Makefile.objs                   |  10 +-
 hw/misc/a9scu.c                         |   2 +-
 hw/misc/applesmc.c                      |   6 +-
 hw/misc/arm11scu.c                      |   2 +-
 hw/misc/arm_integrator_debug.c          |   2 +-
 hw/misc/arm_l2x0.c                      |   2 +-
 hw/misc/arm_sysctl.c                    |   2 +-
 hw/misc/armsse-cpuid.c                  |   2 +-
 hw/misc/armsse-mhu.c                    |   2 +-
 hw/misc/aspeed_scu.c                    |   2 +-
 hw/misc/aspeed_sdmc.c                   |   2 +-
 hw/misc/aspeed_xdma.c                   |   2 +-
 hw/misc/bcm2835_mbox.c                  |   2 +-
 hw/misc/bcm2835_property.c              |   2 +-
 hw/misc/bcm2835_rng.c                   |   2 +-
 hw/misc/debugexit.c                     |   2 +-
 hw/misc/eccmemctl.c                     |   4 +-
 hw/misc/edu.c                           |   2 +-
 hw/misc/exynos4210_clk.c                |   2 +-
 hw/misc/exynos4210_pmu.c                |   2 +-
 hw/misc/exynos4210_rng.c                |   2 +-
 hw/misc/grlib_ahb_apb_pnp.c             |   4 +-
 hw/misc/imx25_ccm.c                     |   2 +-
 hw/misc/imx2_wdt.c                      |   2 +-
 hw/misc/imx31_ccm.c                     |   2 +-
 hw/misc/imx6_ccm.c                      |   4 +-
 hw/misc/imx6_src.c                      |   2 +-
 hw/misc/imx6ul_ccm.c                    |   4 +-
 hw/misc/imx7_ccm.c                      |   4 +-
 hw/misc/imx7_gpr.c                      |   2 +-
 hw/misc/imx7_snvs.c                     |   2 +-
 hw/misc/iotkit-secctl.c                 |   4 +-
 hw/misc/iotkit-sysctl.c                 |   2 +-
 hw/misc/iotkit-sysinfo.c                |   2 +-
 hw/misc/ivshmem.c                       |   2 +-
 hw/misc/macio/cuda.c                    |   2 +-
 hw/misc/macio/gpio.c                    |   2 +-
 hw/misc/macio/mac_dbdma.c               |   2 +-
 hw/misc/macio/macio.c                   |   2 +-
 hw/misc/macio/pmu.c                     |   2 +-
 hw/misc/milkymist-hpdmc.c               |   2 +-
 hw/misc/milkymist-pfpu.c                |   2 +-
 hw/misc/mips_cmgcr.c                    |   2 +-
 hw/misc/mips_cpc.c                      |   2 +-
 hw/misc/mips_itu.c                      |   4 +-
 hw/misc/mos6522.c                       |   2 +-
 hw/misc/mps2-fpgaio.c                   |   2 +-
 hw/misc/mps2-scc.c                      |   2 +-
 hw/misc/msf2-sysreg.c                   |   2 +-
 hw/misc/mst_fpga.c                      |   2 +-
 hw/misc/nrf51_rng.c                     |   2 +-
 hw/misc/omap_gpmc.c                     |   6 +-
 hw/misc/omap_l4.c                       |   2 +-
 hw/misc/omap_sdrc.c                     |   2 +-
 hw/misc/omap_tap.c                      |   2 +-
 hw/misc/pc-testdev.c                    |  10 +-
 hw/misc/pci-testdev.c                   |   4 +-
 hw/misc/puv3_pm.c                       |   2 +-
 hw/misc/slavio_misc.c                   |  16 +--
 hw/misc/stm32f2xx_syscfg.c              |   2 +-
 hw/misc/tz-mpc.c                        |   4 +-
 hw/misc/tz-msc.c                        |   2 +-
 hw/misc/tz-ppc.c                        |   2 +-
 hw/misc/unimp.c                         |   2 +-
 hw/misc/zynq-xadc.c                     |   2 +-
 hw/misc/zynq_slcr.c                     |   2 +-
 hw/moxie/moxiesim.c                     |   2 +-
 hw/net/Makefile.objs                    |   2 +-
 hw/net/allwinner_emac.c                 |   2 +-
 hw/net/cadence_gem.c                    |   2 +-
 hw/net/can/can_kvaser_pci.c             |   6 +-
 hw/net/can/can_mioe3680_pci.c           |   4 +-
 hw/net/can/can_pcm3680_pci.c            |   4 +-
 hw/net/dp8393x.c                        |   2 +-
 hw/net/e1000.c                          |   4 +-
 hw/net/e1000e.c                         |   4 +-
 hw/net/eepro100.c                       |   2 +-
 hw/net/etraxfs_eth.c                    |   2 +-
 hw/net/fsl_etsec/etsec.c                |   2 +-
 hw/net/ftgmac100.c                      |   2 +-
 hw/net/imx_fec.c                        |   2 +-
 hw/net/lan9118.c                        |   4 +-
 hw/net/lance.c                          |   2 +-
 hw/net/mcf_fec.c                        |   2 +-
 hw/net/milkymist-minimac2.c             |   2 +-
 hw/net/ne2000.c                         |   2 +-
 hw/net/pcnet-pci.c                      |   4 +-
 hw/net/rocker/rocker.c                  |   2 +-
 hw/net/rtl8139.c                        |   2 +-
 hw/net/smc91c111.c                      |   2 +-
 hw/net/stellaris_enet.c                 |   2 +-
 hw/net/sungem.c                         |  12 +-
 hw/net/sunhme.c                         |  10 +-
 hw/net/vmxnet3.c                        |   4 +-
 hw/net/xgmac.c                          |   2 +-
 hw/net/xilinx_axienet.c                 |   2 +-
 hw/net/xilinx_ethlite.c                 |   2 +-
 hw/nios2/10m50_devboard.c               |   2 +-
 hw/nvram/ds1225y.c                      |   2 +-
 hw/nvram/fw_cfg.c                       |   8 +-
 hw/nvram/mac_nvram.c                    |   2 +-
 hw/nvram/nrf51_nvm.c                    |   8 +-
 hw/openrisc/openrisc_sim.c              |   2 +-
 hw/pci-host/Makefile.objs               |   2 +-
 hw/pci-host/bonito.c                    |  10 +-
 hw/pci-host/designware.c                |   6 +-
 hw/pci-host/piix.c                      |   2 +-
 hw/pci-host/ppce500.c                   |   2 +-
 hw/pci-host/prep.c                      |   4 +-
 hw/pci-host/q35.c                       |   4 +-
 hw/pci-host/sabre.c                     |   4 +-
 hw/pci-host/uninorth.c                  |   4 +-
 hw/pci-host/versatile.c                 |   4 +-
 hw/pci/msix.c                           |   4 +-
 hw/pci/pci_host.c                       |   8 +-
 hw/pci/pcie_host.c                      |   2 +-
 hw/pci/shpc.c                           |   2 +-
 hw/pcmcia/pxa2xx.c                      |   6 +-
 hw/ppc/e500.c                           |   4 +-
 hw/ppc/mpc8544_guts.c                   |   2 +-
 hw/ppc/pnv_core.c                       |   6 +-
 hw/ppc/pnv_lpc.c                        |   8 +-
 hw/ppc/pnv_occ.c                        |   4 +-
 hw/ppc/pnv_psi.c                        |   8 +-
 hw/ppc/pnv_xscom.c                      |   2 +-
 hw/ppc/ppc405_boards.c                  |   4 +-
 hw/ppc/ppc405_uc.c                      |  14 +--
 hw/ppc/ppc440_bamboo.c                  |   4 +-
 hw/ppc/ppc440_pcix.c                    |   4 +-
 hw/ppc/ppc4xx_pci.c                     |   2 +-
 hw/ppc/ppce500_spin.c                   |   2 +-
 hw/ppc/sam460ex.c                       |   4 +-
 hw/ppc/spapr_pci.c                      |   2 +-
 hw/ppc/virtex_ml507.c                   |   2 +-
 hw/rdma/vmw/pvrdma_main.c               |   4 +-
 hw/riscv/sifive_clint.c                 |   2 +-
 hw/riscv/sifive_gpio.c                  |   2 +-
 hw/riscv/sifive_plic.c                  |   2 +-
 hw/riscv/sifive_prci.c                  |   2 +-
 hw/riscv/sifive_test.c                  |   2 +-
 hw/riscv/sifive_uart.c                  |   2 +-
 hw/riscv/virt.c                         |   2 +-
 hw/s390x/s390-pci-bus.c                 |   2 +-
 hw/s390x/s390-pci-inst.c                |  11 +-
 hw/scsi/Makefile.objs                   |   2 +-
 hw/scsi/esp-pci.c                       |   2 +-
 hw/scsi/esp.c                           |   2 +-
 hw/scsi/lsi53c895a.c                    |   6 +-
 hw/scsi/megasas.c                       |   6 +-
 hw/scsi/mptsas.c                        |   6 +-
 hw/scsi/vmw_pvscsi.c                    |   2 +-
 hw/sd/bcm2835_sdhost.c                  |   2 +-
 hw/sd/milkymist-memcard.c               |   2 +-
 hw/sd/omap_mmc.c                        |   2 +-
 hw/sd/pl181.c                           |   2 +-
 hw/sd/pxa2xx_mmci.c                     |   2 +-
 hw/sd/sdhci.c                           |   4 +-
 hw/sh4/r2d.c                            |   2 +-
 hw/sh4/sh7750.c                         |   4 +-
 hw/sh4/sh_pci.c                         |   2 +-
 hw/sparc/sun4m_iommu.c                  |   2 +-
 hw/sparc64/niagara.c                    |   2 +-
 hw/sparc64/sun4u.c                      |   4 +-
 hw/sparc64/sun4u_iommu.c                |   2 +-
 hw/ssi/Makefile.objs                    |   2 +-
 hw/ssi/aspeed_smc.c                     |   6 +-
 hw/ssi/imx_spi.c                        |   2 +-
 hw/ssi/mss-spi.c                        |   2 +-
 hw/ssi/omap_spi.c                       |   2 +-
 hw/ssi/pl022.c                          |   2 +-
 hw/ssi/stm32f2xx_spi.c                  |   2 +-
 hw/ssi/xilinx_spi.c                     |   2 +-
 hw/ssi/xilinx_spips.c                   |   8 +-
 hw/timer/Makefile.objs                  |   6 +-
 hw/timer/a9gtimer.c                     |   4 +-
 hw/timer/allwinner-a10-pit.c            |   2 +-
 hw/timer/altera_timer.c                 |   2 +-
 hw/timer/arm_mptimer.c                  |   4 +-
 hw/timer/arm_timer.c                    |   4 +-
 hw/timer/armv7m_systick.c               |   2 +-
 hw/timer/aspeed_rtc.c                   |   2 +-
 hw/timer/aspeed_timer.c                 |   2 +-
 hw/timer/cadence_ttc.c                  |   2 +-
 hw/timer/cmsdk-apb-dualtimer.c          |   2 +-
 hw/timer/cmsdk-apb-timer.c              |   2 +-
 hw/timer/digic-timer.c                  |   2 +-
 hw/timer/etraxfs_timer.c                |   2 +-
 hw/timer/exynos4210_mct.c               |   2 +-
 hw/timer/exynos4210_pwm.c               |   2 +-
 hw/timer/exynos4210_rtc.c               |   2 +-
 hw/timer/grlib_gptimer.c                |   2 +-
 hw/timer/hpet.c                         |   2 +-
 hw/timer/i8254.c                        |   2 +-
 hw/timer/imx_epit.c                     |   2 +-
 hw/timer/imx_gpt.c                      |   2 +-
 hw/timer/lm32_timer.c                   |   2 +-
 hw/timer/m48t59.c                       |   4 +-
 hw/timer/mc146818rtc.c                  |   2 +-
 hw/timer/milkymist-sysctl.c             |   2 +-
 hw/timer/mss-timer.c                    |   2 +-
 hw/timer/nrf51_timer.c                  |   2 +-
 hw/timer/omap_gptimer.c                 |   2 +-
 hw/timer/omap_synctimer.c               |   2 +-
 hw/timer/pl031.c                        |   2 +-
 hw/timer/puv3_ost.c                     |   2 +-
 hw/timer/pxa2xx_timer.c                 |   2 +-
 hw/timer/sh_timer.c                     |   2 +-
 hw/timer/slavio_timer.c                 |   2 +-
 hw/timer/stm32f2xx_timer.c              |   2 +-
 hw/timer/sun4v-rtc.c                    |   2 +-
 hw/timer/xilinx_timer.c                 |   2 +-
 hw/timer/xlnx-zynqmp-rtc.c              |   2 +-
 hw/tpm/tpm_crb.c                        |   2 +-
 hw/tpm/tpm_tis.c                        |   2 +-
 hw/usb/chipidea.c                       |   4 +-
 hw/usb/hcd-ehci-sysbus.c                |   2 +-
 hw/usb/hcd-ehci.c                       |   6 +-
 hw/usb/hcd-ohci.c                       |   2 +-
 hw/usb/hcd-uhci.c                       |   2 +-
 hw/usb/hcd-xhci.c                       |  10 +-
 hw/usb/tusb6010.c                       |   2 +-
 hw/vfio/common.c                        |   2 +-
 hw/vfio/pci-quirks.c                    |  33 +++---
 hw/vfio/pci.c                           |   4 +-
 hw/virtio/Makefile.objs                 |   2 +-
 hw/virtio/virtio-mmio.c                 |   2 +-
 hw/virtio/virtio-pci.c                  |  27 +++--
 hw/watchdog/cmsdk-apb-watchdog.c        |   2 +-
 hw/watchdog/wdt_aspeed.c                |   2 +-
 hw/watchdog/wdt_i6300esb.c              |   2 +-
 hw/xen/xen_pt.c                         |   2 +-
 hw/xen/xen_pt_msi.c                     |   2 +-
 hw/xtensa/mx_pic.c                      |   2 +-
 hw/xtensa/xtfpga.c                      |   6 +-
 include/exec/cpu-all.h                  |  10 +-
 include/exec/cpu-common.h               |  12 --
 include/exec/memattrs.h                 |   2 +
 include/exec/memop.h                    | 134 ++++++++++++++++++++++
 include/exec/memory.h                   |  11 +-
 include/exec/poison.h                   |   1 +
 include/hw/char/serial.h                |   2 +-
 include/qom/cpu.h                       |   2 +-
 ioport.c                                |   4 +-
 memory.c                                |  55 ++++-----
 memory_ldst.inc.c                       | 153 ++++++++-----------------
 target/alpha/cpu.h                      |   2 -
 target/alpha/translate.c                |   2 +-
 target/arm/translate-a64.c              |  48 ++++----
 target/arm/translate-a64.h              |   2 +-
 target/arm/translate-sve.c              |   2 +-
 target/arm/translate.c                  |  32 +++---
 target/arm/translate.h                  |   2 +-
 target/hppa/cpu.h                       |   1 -
 target/hppa/translate.c                 |  14 +--
 target/i386/translate.c                 | 132 ++++++++++-----------
 target/m68k/translate.c                 |   2 +-
 target/microblaze/translate.c           |   4 +-
 target/mips/cpu.h                       |   2 -
 target/mips/op_helper.c                 |   5 +-
 target/mips/translate.c                 |   8 +-
 target/openrisc/translate.c             |   4 +-
 target/ppc/translate.c                  |  12 +-
 target/riscv/insn_trans/trans_rva.inc.c |   8 +-
 target/riscv/insn_trans/trans_rvi.inc.c |   4 +-
 target/s390x/translate.c                |   6 +-
 target/s390x/translate_vx.inc.c         |  10 +-
 target/sh4/cpu.h                        |   2 -
 target/sparc/cpu.h                      |   4 +-
 target/sparc/mmu_helper.c               |  40 ++++---
 target/sparc/translate.c                |  14 +--
 target/tilegx/translate.c               |  10 +-
 target/tricore/translate.c              |   8 +-
 target/xtensa/cpu.h                     |   2 -
 tcg/README                              |   2 +-
 tcg/aarch64/tcg-target.inc.c            |  26 ++---
 tcg/arm/tcg-target.inc.c                |  26 ++---
 tcg/i386/tcg-target.inc.c               |  24 ++--
 tcg/mips/tcg-target.inc.c               |  16 +--
 tcg/optimize.c                          |   2 +-
 tcg/ppc/tcg-target.inc.c                |  12 +-
 tcg/riscv/tcg-target.inc.c              |  20 ++--
 tcg/s390/tcg-target.inc.c               |  14 +--
 tcg/sparc/tcg-target.inc.c              |   6 +-
 tcg/tcg-op.c                            |  38 +++---
 tcg/tcg-op.h                            |  86 +++++++-------
 tcg/tcg.c                               |   4 +-
 tcg/tcg.h                               |  99 +---------------
 trace/mem-internal.h                    |   4 +-
 trace/mem.h                             |   4 +-
 497 files changed, 1436 insertions(+), 1473 deletions(-)
 create mode 100644 include/exec/memop.h

Comments

Philippe Mathieu-Daudé Aug. 16, 2019, 9:58 a.m. UTC | #1
Hi Tony,

On 8/16/19 8:28 AM, tony.nguyen@bt.com wrote:
> This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.
> 
> v7:
[...]
> - Re-declared many native endian devices as little or big endian. This is why
>   v7 has +16 patches.

Why are you doing that? What is the rational?

Anyhow if this not required by your series, you should split it out of
it, and send it on your principal changes are merged.
I'm worried because this these new patches involve many subsystems (thus
maintainers) and reviewing them will now take a fair amount of time.

> For each device declared with DEVICE_NATIVE_ENDIAN, find the set of
> targets from the set of target/hw/*/device.o.
>
> If the set of targets are all little or all big endian, re-declare
> the device endianness as DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN
> respectively.

If only little endian targets use a device, that doesn't mean the device
is designed in little endian...

Then if a big endian target plan to use this device, it will require
more work and you might have introduced regressions...

I'm not sure this is a safe move.

> This *naive* deduction may result in genuinely native endian devices
> being incorrectly declared as little or big endian, but should not
> introduce regressions for current targets.

Regards,

Phil.
Tony Nguyen Aug. 16, 2019, 11:37 a.m. UTC | #2
Hi Phillippe,

On 8/16/19 7:58 PM, Philippe Mathieu-Daudé wrote:
>On 8/16/19 8:28 AM, tony.nguyen@bt.com wrote:
>> This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.
>>
>> v7:
>[...]
>> - Re-declared many native endian devices as little or big endian. This is why
>>   v7 has +16 patches.
>
>Why are you doing that? What is the rational?

While collapsing the byte swaps, it was suggested in patch #11 of v5 that
consistent use of MemOp simplified endian comparisons. This lead to the
deprecation of enum device_endian by MemOp.

As MO_TE is conditional upon NEED_CPU_H, the s/DEVICE_NATIVE_ENDIAN/MO_TE/
required changing some device object files from common-obj-* to obj-*. In patch
#15 of v6 Paolo noted that most devices should not of been DEVICE_NATIVE_ENDIAN
and hinted at a clean up.

The +16 patches in v7 is the clean up effort.

>Anyhow if this not required by your series, you should split it out of
>it, and send it on your principal changes are merged.
>I'm worried because this these new patches involve many subsystems (thus
>maintainers) and reviewing them will now take a fair amount of time.

Yes, lets split these patches out. They are very much a tangent to the series
purpose.

>> For each device declared with DEVICE_NATIVE_ENDIAN, find the set of
>> targets from the set of target/hw/*/device.o.
>>
>> If the set of targets are all little or all big endian, re-declare
>> the device endianness as DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN
>> respectively.
>
>If only little endian targets use a device, that doesn't mean the device
>is designed in little endian...
>
>Then if a big endian target plan to use this device, it will require
>more work and you might have introduced regressions...
>
>I'm not sure this is a safe move.
>
>> This *naive* deduction may result in genuinely native endian devices
>> being incorrectly declared as little or big endian, but should not
>> introduce regressions for current targets.
>

Roger. Evidently too naive. TBH, most devices I've never heard of...

Regards,
Tony
David Gibson Aug. 16, 2019, 11:43 a.m. UTC | #3
On Fri, Aug 16, 2019 at 11:58:05AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Tony,
> 
> On 8/16/19 8:28 AM, tony.nguyen@bt.com wrote:
> > This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.
> > 
> > v7:
> [...]
> > - Re-declared many native endian devices as little or big endian. This is why
> >   v7 has +16 patches.
> 
> Why are you doing that? What is the rational?
> 
> Anyhow if this not required by your series, you should split it out of
> it, and send it on your principal changes are merged.
> I'm worried because this these new patches involve many subsystems (thus
> maintainers) and reviewing them will now take a fair amount of time.
> 
> > For each device declared with DEVICE_NATIVE_ENDIAN, find the set of
> > targets from the set of target/hw/*/device.o.
> >
> > If the set of targets are all little or all big endian, re-declare
> > the device endianness as DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN
> > respectively.
> 
> If only little endian targets use a device, that doesn't mean the device
> is designed in little endian...
> 
> Then if a big endian target plan to use this device, it will require
> more work and you might have introduced regressions...

Uh.. only if they make the version of the device on a big endian
target big endian.  Which is a terrible idea - if you know a hardware
designer planning to do this, please slap them.
Peter Maydell Aug. 16, 2019, 12:02 p.m. UTC | #4
On Fri, 16 Aug 2019 at 12:37, <tony.nguyen@bt.com> wrote:
>
> Hi Phillippe,
>
> On 8/16/19 7:58 PM, Philippe Mathieu-Daudé wrote:
> >On 8/16/19 8:28 AM, tony.nguyen@bt.com wrote:
> >> For each device declared with DEVICE_NATIVE_ENDIAN, find the set of
> >> targets from the set of target/hw/*/device.o.
> >>
> >> If the set of targets are all little or all big endian, re-declare
> >> the device endianness as DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN
> >> respectively.
> >
> >If only little endian targets use a device, that doesn't mean the device
> >is designed in little endian...
> >
> >Then if a big endian target plan to use this device, it will require
> >more work and you might have introduced regressions...
> >
> >I'm not sure this is a safe move.
> >
> >> This *naive* deduction may result in genuinely native endian devices
> >> being incorrectly declared as little or big endian, but should not
> >> introduce regressions for current targets.
> >
>
> Roger. Evidently too naive. TBH, most devices I've never heard of...

OTOH it's worth noting that it's quite likely that most of
the implementations of these DEVICE_NATIVE_ENDIAN devices
picked it in an equally naive way, by just copying some other
device's code...

thanks
-- PMM
Richard Henderson Aug. 18, 2019, 9:13 a.m. UTC | #5
On 8/16/19 7:28 AM, tony.nguyen@bt.com wrote:
> Tony Nguyen (42):
>   configure: Define TARGET_ALIGNED_ONLY in configure
>   tcg: TCGMemOp is now accelerator independent MemOp
>   memory: Introduce size_memop
>   target/mips: Access MemoryRegion with MemOp
>   hw/s390x: Access MemoryRegion with MemOp
>   hw/intc/armv7m_nic: Access MemoryRegion with MemOp
>   hw/virtio: Access MemoryRegion with MemOp
>   hw/vfio: Access MemoryRegion with MemOp
>   exec: Access MemoryRegion with MemOp
>   cputlb: Access MemoryRegion with MemOp
>   memory: Access MemoryRegion with MemOp
>   hw/s390x: Hard code size with MO_{8|16|32|64}
>   target/mips: Hard code size with MO_{8|16|32|64}
>   exec: Hard code size with MO_{8|16|32|64}

I have queued these 14 patches to tcg-next:

  https://github.com/rth7680/qemu/tree/tcg-next

I agree with the downthread conversation with Phil that the middle device
patches should be split out to a different series.

I have some questions on some of the last few patches, and I don't know how
they would interact cherry-picking from those, so I've left them for now.

I had some trouble applying your patches, as they're encoded quoted-printable,
and "git am" doesn't like that.  If possible, please use "git send-email" to
post your next patch set.


r~