On Thu, Dec 22, 2022 at 10:55:48PM +0100, Philippe Mathieu-Daudé wrote:
> ARM CPUs fetch instructions in little-endian.
>
> smpboot[] encoded instructions are written in little-endian.
>
> We call tswap32() on the array. tswap32 function swap a 32-bit
> value if the target endianness doesn't match the host one.
> Otherwise it is a NOP.
>
> * On a little-endian host, the array is stored as it. tswap32()
> is a NOP, and the vCPU fetches the instructions as it, in
> little-endian.
>
> * On a big-endian host, the array is stored as it. tswap32()
> swap the instructions to little-endian, and the vCPU fetches
> the instructions as it, in little-endian.
>
> Using tswap() on system emulation is a bit odd: while the target
> particularities might change the system emulation, the host ones
> (such its endianness) shouldn't interfere.
>
> We can simplify by using const_le32() to always store the
> instructions in the array in little-endian, removing the need
> for the dubious tswap().
Hi Philippe,
>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> ---
> hw/arm/xilinx_zynq.c | 27 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 3190cc0b8d..4316143b71 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -71,6 +71,11 @@ static const int dma_irqs[8] = {
>
> #define ZYNQ_SDHCI_CAPABILITIES 0x69ec0080 /* Datasheet: UG585 (v1.12.1) */
>
> +struct ZynqMachineState {
> + MachineState parent;
> + Clock *ps_clk;
> +};
> +
> #define ARMV7_IMM16(x) (extract32((x), 0, 12) | \
> extract32((x), 12, 4) << 16)
>
> @@ -79,29 +84,21 @@ static const int dma_irqs[8] = {
> */
>
> #define SLCR_WRITE(addr, val) \
> - 0xe3001000 + ARMV7_IMM16(extract32((val), 0, 16)), /* movw r1 ... */ \
> - 0xe3401000 + ARMV7_IMM16(extract32((val), 16, 16)), /* movt r1 ... */ \
> - 0xe5801000 + (addr)
> -
> -struct ZynqMachineState {
> - MachineState parent;
> - Clock *ps_clk;
> -};
> + cpu_to_le32(0xe3001000 + ARMV7_IMM16(extract32((val), 0, 16))), /* movw
> r1 ... */ \
> + cpu_to_le32(0xe3401000 + ARMV7_IMM16(extract32((val), 16, 16))), /* movt
> r1 ... */ \
Looks like the callers all pass in constants, perhaps const_le32 should be used
everywhere or am I missing something?
> + const_le32(0xe5801000 + (addr))
>
> static void zynq_write_board_setup(ARMCPU *cpu,
> const struct arm_boot_info *info)
> {
> - int n;
> - uint32_t board_setup_blob[] = {
> - 0xe3a004f8, /* mov r0, #0xf8000000 */
> + const uint32_t board_setup_blob[] = {
> + const_le32(0xe3a004f8), /* mov r0, #0xf8000000 */
> SLCR_WRITE(SLCR_UNLOCK_OFFSET, SLCR_XILINX_UNLOCK_KEY),
> SLCR_WRITE(SLCR_ARM_PLL_OFFSET, 0x00014008),
> SLCR_WRITE(SLCR_LOCK_OFFSET, SLCR_XILINX_LOCK_KEY),
> - 0xe12fff1e, /* bx lr */
> + const_le32(0xe12fff1e) /* bx lr */
> };
> - for (n = 0; n < ARRAY_SIZE(board_setup_blob); n++) {
> - board_setup_blob[n] = tswap32(board_setup_blob[n]);
> - }
> +
> rom_add_blob_fixed("board-setup", board_setup_blob,
> sizeof(board_setup_blob), BOARD_SETUP_ADDR);
> }
> --
> 2.38.1
>
