mbox series

[00/19] Declare device little or big endian

Message ID cover.1566829168.git.tony.nguyen@bt.com
Headers show
Series Declare device little or big endian | expand

Message

Tony Nguyen Aug. 26, 2019, 2:21 p.m. UTC 
This series is an attempt to re-declare devices with DEVICE_NATIVE_ENDIAN as
DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN.

This clean up was split out from an earlier series which collapsed byte swaps
along the I/O path.

On Wed, 7 Aug 2019 at 12:42, Paolo Bonzini wrote:
> On 07/08/19 10:31, tony.nguyen@bt wrote:
>>
>> Device realizing code with MemorRegionOps endianness as
>> DEVICE_NATIVE_ENDIAN is not common code.
>> 
>> Corrected devices were identified by making the declaration of
>> DEVICE_NATIVE_ENDIAN conditional upon NEED_CPU_H and then listing
>> what failed to compile.
>
> The general approach makes sense.  However, most of these should not be
> DEVICE_NATIVE_ENDIAN.  I can help with some of them.

On Fri, 16 Aug 2019 at 13:02, Peter Maydell wrote:
> 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...

Approach this in two steps.

1. Naively. 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
as DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN respectively.

2. Manually. Inspect with heuristics (thanks Paolo):
- if not used, re-declare as DEVICE_HOST_ENDIAN.
- if max/min size=1, re-declare as DEVICE_HOST_ENDIAN.
- if just a bit bucket, re-declare as DEVICE_HOST_ENDIAN
- if PCI, re-declare as DEVICE_LITTLE_ENDIAN.
- if for {ARM|unicore32} only, re-declare as DEVICE_LITTLE_ENDIAN.
- if for SPARC only, re-declare as DEVICE_BIG_ENDIAN.

Likely still not %100 correct, but I think more than before.

Since RFC:
- Applied Paolo's heuristics

Tony Nguyen (19):
  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/core: 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/ipack: 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-*

 hw/audio/cs4231.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         |  4 ++--
 hw/block/fdc.c                 |  4 ++--
 hw/block/onenand.c             |  2 +-
 hw/char/Makefile.objs          |  4 ++--
 hw/char/cadence_uart.c         |  2 +-
 hw/char/escc.c                 |  2 +-
 hw/char/etraxfs_ser.c          |  2 +-
 hw/char/grlib_apbuart.c        |  2 +-
 hw/char/imx_serial.c           |  2 +-
 hw/char/lm32_uart.c            |  2 +-
 hw/char/milkymist-uart.c       |  2 +-
 hw/char/pl011.c                |  2 +-
 hw/core/empty_slot.c           |  2 +-
 hw/display/Makefile.objs       |  2 +-
 hw/display/exynos4210_fimd.c   |  2 +-
 hw/display/jazz_led.c          |  2 +-
 hw/display/pl110.c             |  2 +-
 hw/display/tc6393xb.c          |  2 +-
 hw/dma/Makefile.objs           |  4 ++--
 hw/dma/bcm2835_dma.c           |  4 ++--
 hw/dma/etraxfs_dma.c           | 14 +++++++-------
 hw/dma/i8257.c                 |  4 ++--
 hw/dma/pl080.c                 |  2 +-
 hw/dma/pl330.c                 |  2 +-
 hw/dma/puv3_dma.c              |  2 +-
 hw/dma/sparc32_dma.c           |  2 +-
 hw/gpio/pl061.c                |  2 +-
 hw/gpio/puv3_gpio.c            |  2 +-
 hw/gpio/zaurus.c               |  2 +-
 hw/i2c/exynos4210_i2c.c        |  2 +-
 hw/i2c/imx_i2c.c               |  2 +-
 hw/i2c/mpc_i2c.c               |  2 +-
 hw/i2c/versatile_i2c.c         |  2 +-
 hw/input/Makefile.objs         |  2 +-
 hw/input/pl050.c               |  2 +-
 hw/intc/Makefile.objs          |  2 +-
 hw/intc/arm_gic.c              | 12 ++++++------
 hw/intc/arm_gicv3.c            |  4 ++--
 hw/intc/arm_gicv3_its_common.c |  2 +-
 hw/intc/etraxfs_pic.c          |  2 +-
 hw/intc/imx_avic.c             |  2 +-
 hw/intc/imx_gpcv2.c            |  2 +-
 hw/intc/pl190.c                |  2 +-
 hw/intc/puv3_intc.c            |  2 +-
 hw/intc/slavio_intctl.c        |  4 ++--
 hw/ipack/tpci200.c             | 10 +++++-----
 hw/isa/vt82c686.c              |  2 +-
 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/edu.c                  |  2 +-
 hw/misc/mos6522.c              |  2 +-
 hw/misc/puv3_pm.c              |  2 +-
 hw/misc/unimp.c                |  2 +-
 hw/net/Makefile.objs           |  2 +-
 hw/net/allwinner_emac.c        |  2 +-
 hw/net/imx_fec.c               |  2 +-
 hw/net/lan9118.c               |  4 ++--
 hw/net/lance.c                 |  2 +-
 hw/net/smc91c111.c             |  2 +-
 hw/net/stellaris_enet.c        |  2 +-
 hw/pci-host/bonito.c           |  2 +-
 hw/pci-host/q35.c              |  2 +-
 hw/pci-host/versatile.c        |  4 ++--
 hw/scsi/Makefile.objs          |  2 +-
 hw/sd/pl181.c                  |  2 +-
 hw/ssi/Makefile.objs           |  2 +-
 hw/ssi/mss-spi.c               |  2 +-
 hw/ssi/pl022.c                 |  2 +-
 hw/ssi/stm32f2xx_spi.c         |  2 +-
 hw/ssi/xilinx_spips.c          |  2 +-
 hw/timer/Makefile.objs         |  2 +-
 hw/timer/a9gtimer.c            |  4 ++--
 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/cadence_ttc.c         |  2 +-
 hw/timer/grlib_gptimer.c       |  2 +-
 hw/timer/hpet.c                |  2 +-
 hw/timer/imx_epit.c            |  2 +-
 hw/timer/imx_gpt.c             |  2 +-
 hw/timer/lm32_timer.c          |  2 +-
 hw/timer/milkymist-sysctl.c    |  2 +-
 hw/timer/mss-timer.c           |  2 +-
 hw/timer/pl031.c               |  2 +-
 hw/timer/puv3_ost.c            |  2 +-
 hw/timer/slavio_timer.c        |  2 +-
 hw/timer/stm32f2xx_timer.c     |  2 +-
 hw/timer/sun4v-rtc.c           |  2 +-
 hw/virtio/Makefile.objs        |  2 +-
 98 files changed, 128 insertions(+), 128 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 2, 2019, 11:42 a.m. UTC | #1 
On 8/26/19 4:21 PM, Tony Nguyen wrote:
> This series is an attempt to re-declare devices with DEVICE_NATIVE_ENDIAN as
> DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN.
> 
> This clean up was split out from an earlier series which collapsed byte swaps
> along the I/O path.
> 
> On Wed, 7 Aug 2019 at 12:42, Paolo Bonzini wrote:
>> On 07/08/19 10:31, tony.nguyen@bt wrote:
>>>
>>> Device realizing code with MemorRegionOps endianness as
>>> DEVICE_NATIVE_ENDIAN is not common code.
>>>
>>> Corrected devices were identified by making the declaration of
>>> DEVICE_NATIVE_ENDIAN conditional upon NEED_CPU_H and then listing
>>> what failed to compile.
>>
>> The general approach makes sense.  However, most of these should not be
>> DEVICE_NATIVE_ENDIAN.  I can help with some of them.
> 
> On Fri, 16 Aug 2019 at 13:02, Peter Maydell wrote:
>> 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...
> 
> Approach this in two steps.
> 
> 1. Naively. 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
> as DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN respectively.
> 
> 2. Manually. Inspect with heuristics (thanks Paolo):
> - if not used, re-declare as DEVICE_HOST_ENDIAN.
> - if max/min size=1, re-declare as DEVICE_HOST_ENDIAN.
> - if just a bit bucket, re-declare as DEVICE_HOST_ENDIAN
> - if PCI, re-declare as DEVICE_LITTLE_ENDIAN.
> - if for {ARM|unicore32} only, re-declare as DEVICE_LITTLE_ENDIAN.
> - if for SPARC only, re-declare as DEVICE_BIG_ENDIAN.

Maybe this list is better to split your patches.

Which are the 'not used' ones?

TYPE_TPCI200 (from IndustryPack) can go with PCI.

You can put all the PrimeCell devices together (eventually with the 
other ARM devices):

  hw/audio/pl041.c
  hw/char/pl011.c
  hw/display/pl110.c
  hw/dma/pl080.c
  hw/dma/pl330.c
  hw/gpio/pl061.c
  hw/input/pl050.c
  hw/intc/pl190.c
  hw/sd/pl181.c
  hw/ssi/pl022.c
  hw/timer/pl031.c