* [PATCH 0/3] hw/arm: Replace tswap32() by stl_endian_p()
@ 2024-09-30 22:12 Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 1/3] target/arm: Expose arm_cpu_code_is_big_endian() prototype in 'cpu.h' Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-30 22:12 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Peter Maydell, Thomas Huth, Hao Wu,
Laurent Vivier, Joel Stanley, qemu-arm, Andrew Jeffery,
Steven Lee, Tyrone Ting, Edgar E. Iglesias, Igor Mitsyanko,
Philippe Mathieu-Daudé, Cédric Le Goater, Jamin Lin,
Troy Lee, Anton Johansson
- Expose arm_cpu_code_is_big_endian()
- pass ARMCPU argument to arm_write_bootloader() so we can call
arm_cpu_code_is_big_endian() on it,
- Replace target specific tswap32() by target agnostic stl_endian_p()
Tested on little & big endian hosts.
Philippe Mathieu-Daudé (3):
target/arm: Expose arm_cpu_code_is_big_endian() prototype in 'cpu.h'
hw/arm: Have arm_write_bootloader() take a ARMCPU argument
hw/arm: Replace tswap32() calls by target agnostic stl_endian_p()
include/hw/arm/boot.h | 9 ++++++---
target/arm/cpu.h | 7 +++++++
hw/arm/aspeed.c | 3 +--
hw/arm/boot.c | 17 ++++++++++-------
hw/arm/exynos4210.c | 7 +++----
hw/arm/npcm7xx.c | 6 ++++--
hw/arm/raspi.c | 4 ++--
hw/arm/xilinx_zynq.c | 5 +++--
linux-user/aarch64/cpu_loop.c | 4 ++--
linux-user/arm/cpu_loop.c | 4 ++--
target/arm/cpu.c | 6 ++----
11 files changed, 42 insertions(+), 30 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] target/arm: Expose arm_cpu_code_is_big_endian() prototype in 'cpu.h'
2024-09-30 22:12 [PATCH 0/3] hw/arm: Replace tswap32() by stl_endian_p() Philippe Mathieu-Daudé
@ 2024-09-30 22:12 ` Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 2/3] hw/arm: Have arm_write_bootloader() take a ARMCPU argument Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 3/3] hw/arm: Replace tswap32() calls by target agnostic stl_endian_p() Philippe Mathieu-Daudé
2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-30 22:12 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Peter Maydell, Thomas Huth, Hao Wu,
Laurent Vivier, Joel Stanley, qemu-arm, Andrew Jeffery,
Steven Lee, Tyrone Ting, Edgar E. Iglesias, Igor Mitsyanko,
Philippe Mathieu-Daudé, Cédric Le Goater, Jamin Lin,
Troy Lee, Anton Johansson
Expose arm_cpu_code_is_big_endian() so it can be used by hw/ code.
Use it in few places where it was open coded.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
target/arm/cpu.h | 7 +++++++
linux-user/aarch64/cpu_loop.c | 4 ++--
linux-user/arm/cpu_loop.c | 4 ++--
target/arm/cpu.c | 6 ++----
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f065756c5c..da8f2b2ec8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3057,6 +3057,13 @@ static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)
}
}
+static inline bool bswap_code(bool sctlr_b);
+
+static inline bool arm_cpu_code_is_big_endian(CPUARMState *env)
+{
+ return bswap_code(arm_sctlr_b(env));
+}
+
#include "exec/cpu-all.h"
/*
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 71cdc8be50..68ff3c14f8 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -29,7 +29,7 @@
#define get_user_code_u32(x, gaddr, env) \
({ abi_long __r = get_user_u32((x), (gaddr)); \
- if (!__r && bswap_code(arm_sctlr_b(env))) { \
+ if (!__r && arm_cpu_code_is_big_endian(env)) { \
(x) = bswap32(x); \
} \
__r; \
@@ -37,7 +37,7 @@
#define get_user_code_u16(x, gaddr, env) \
({ abi_long __r = get_user_u16((x), (gaddr)); \
- if (!__r && bswap_code(arm_sctlr_b(env))) { \
+ if (!__r && arm_cpu_code_is_big_endian(env)) { \
(x) = bswap16(x); \
} \
__r; \
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index ec665862d9..0cc056be31 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -29,7 +29,7 @@
#define get_user_code_u32(x, gaddr, env) \
({ abi_long __r = get_user_u32((x), (gaddr)); \
- if (!__r && bswap_code(arm_sctlr_b(env))) { \
+ if (!__r && arm_cpu_code_is_big_endian(env)) { \
(x) = bswap32(x); \
} \
__r; \
@@ -37,7 +37,7 @@
#define get_user_code_u16(x, gaddr, env) \
({ abi_long __r = get_user_u16((x), (gaddr)); \
- if (!__r && bswap_code(arm_sctlr_b(env))) { \
+ if (!__r && arm_cpu_code_is_big_endian(env)) { \
(x) = bswap16(x); \
} \
__r; \
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 19191c2391..f3198ee2f2 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1167,7 +1167,6 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
{
ARMCPU *ac = ARM_CPU(cpu);
CPUARMState *env = &ac->env;
- bool sctlr_b;
if (is_a64(env)) {
info->cap_arch = CS_ARCH_ARM64;
@@ -1194,8 +1193,7 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
info->cap_mode = cap_mode;
}
- sctlr_b = arm_sctlr_b(env);
- if (bswap_code(sctlr_b)) {
+ if (arm_cpu_code_is_big_endian(env)) {
#if TARGET_BIG_ENDIAN
info->endian = BFD_ENDIAN_LITTLE;
#else
@@ -1204,7 +1202,7 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
}
info->flags &= ~INSN_ARM_BE32;
#ifndef CONFIG_USER_ONLY
- if (sctlr_b) {
+ if (arm_sctlr_b(env)) {
info->flags |= INSN_ARM_BE32;
}
#endif
--
2.45.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] hw/arm: Have arm_write_bootloader() take a ARMCPU argument
2024-09-30 22:12 [PATCH 0/3] hw/arm: Replace tswap32() by stl_endian_p() Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 1/3] target/arm: Expose arm_cpu_code_is_big_endian() prototype in 'cpu.h' Philippe Mathieu-Daudé
@ 2024-09-30 22:12 ` Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 3/3] hw/arm: Replace tswap32() calls by target agnostic stl_endian_p() Philippe Mathieu-Daudé
2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-30 22:12 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Peter Maydell, Thomas Huth, Hao Wu,
Laurent Vivier, Joel Stanley, qemu-arm, Andrew Jeffery,
Steven Lee, Tyrone Ting, Edgar E. Iglesias, Igor Mitsyanko,
Philippe Mathieu-Daudé, Cédric Le Goater, Jamin Lin,
Troy Lee, Anton Johansson
The next commit will replace tswap32() calls by stl_endian_p()
ones in bootloader.c. In order to do that, we'll need to know
the vCPU endianness. This information is retrievable with
arm_cpu_code_is_big_endian(), but we need to access CPUARMState.
As a first step, pass ARMCPU as argument to arm_write_bootloader()
so it'll be able to access cpu->env.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
include/hw/arm/boot.h | 9 ++++++---
hw/arm/aspeed.c | 3 +--
hw/arm/boot.c | 9 +++++----
hw/arm/raspi.c | 4 ++--
4 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
index 80c492d742..3d1226ab00 100644
--- a/include/hw/arm/boot.h
+++ b/include/hw/arm/boot.h
@@ -206,13 +206,15 @@ typedef struct ARMInsnFixup {
/**
* arm_write_bootloader - write a bootloader to guest memory
* @name: name of the bootloader blob
- * @as: AddressSpace to write the bootloader
+ * @cpu: handle to the first CPU object
+ * @info: handle to the boot info struct
* @addr: guest address to write it
* @insns: the blob to be loaded
* @fixupcontext: context to be used for any fixups in @insns
*
* Write a bootloader to guest memory at address @addr in the address
- * space @as. @name is the name to use for the resulting ROM blob, so
+ * space returned by @arm_boot_address_space().
+ * @name is the name to use for the resulting ROM blob, so
* it should be unique in the system and reasonably identifiable for debugging.
*
* @insns must be an array of ARMInsnFixup structs, each of which has
@@ -228,7 +230,8 @@ typedef struct ARMInsnFixup {
* the entries that @insns refers to.
*/
void arm_write_bootloader(const char *name,
- AddressSpace *as, hwaddr addr,
+ ARMCPU *cpu, const struct arm_boot_info *info,
+ hwaddr addr,
const ARMInsnFixup *insns,
const uint32_t *fixupcontext);
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index cf0c6c580b..cf5fb92238 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -217,7 +217,6 @@ struct AspeedMachineState {
static void aspeed_write_smpboot(ARMCPU *cpu,
const struct arm_boot_info *info)
{
- AddressSpace *as = arm_boot_address_space(cpu, info);
static const ARMInsnFixup poll_mailbox_ready[] = {
/*
* r2 = per-cpu go sign value
@@ -244,7 +243,7 @@ static void aspeed_write_smpboot(ARMCPU *cpu,
};
static const uint32_t fixupcontext[FIXUP_MAX] = { 0 };
- arm_write_bootloader("aspeed.smpboot", as, info->smp_loader_start,
+ arm_write_bootloader("aspeed.smpboot", cpu, info, info->smp_loader_start,
poll_mailbox_ready, fixupcontext);
}
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 5301d8d318..6efd21f9c2 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -131,10 +131,12 @@ static const ARMInsnFixup smpboot[] = {
};
void arm_write_bootloader(const char *name,
- AddressSpace *as, hwaddr addr,
+ ARMCPU *cpu, const struct arm_boot_info *info,
+ hwaddr addr,
const ARMInsnFixup *insns,
const uint32_t *fixupcontext)
{
+ AddressSpace *as = arm_boot_address_space(cpu, info);
/* Fix up the specified bootloader fragment and write it into
* guest memory using rom_add_blob_fixed(). fixupcontext is
* an array giving the values to write in for the fixup types
@@ -185,7 +187,6 @@ static void default_write_secondary(ARMCPU *cpu,
const struct arm_boot_info *info)
{
uint32_t fixupcontext[FIXUP_MAX];
- AddressSpace *as = arm_boot_address_space(cpu, info);
fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
@@ -195,7 +196,7 @@ static void default_write_secondary(ARMCPU *cpu,
fixupcontext[FIXUP_DSB] = CP15_DSB_INSN;
}
- arm_write_bootloader("smpboot", as, info->smp_loader_start,
+ arm_write_bootloader("smpboot", cpu, info, info->smp_loader_start,
smpboot, fixupcontext);
}
@@ -1128,7 +1129,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
fixupcontext[FIXUP_ENTRYPOINT_LO] = entry;
fixupcontext[FIXUP_ENTRYPOINT_HI] = entry >> 32;
- arm_write_bootloader("bootloader", as, info->loader_start,
+ arm_write_bootloader("bootloader", cpu, info, info->loader_start,
primary_loader, fixupcontext);
if (info->write_board_setup) {
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index a7a662f40d..84fffe2a02 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -137,7 +137,7 @@ static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
QEMU_BUILD_BUG_ON((BOARDSETUP_ADDR & 0xf) != 0
|| (BOARDSETUP_ADDR >> 4) >= 0x100);
- arm_write_bootloader("raspi_smpboot", arm_boot_address_space(cpu, info),
+ arm_write_bootloader("raspi_smpboot", cpu, info,
info->smp_loader_start, smpboot, fixupcontext);
}
@@ -172,7 +172,7 @@ static void write_smpboot64(ARMCPU *cpu, const struct arm_boot_info *info)
0, 0, 0, 0
};
- arm_write_bootloader("raspi_smpboot", as, info->smp_loader_start,
+ arm_write_bootloader("raspi_smpboot", cpu, info, info->smp_loader_start,
smpboot, fixupcontext);
rom_add_blob_fixed_as("raspi_spintables", spintables, sizeof(spintables),
SPINTABLE_ADDR, as);
--
2.45.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] hw/arm: Replace tswap32() calls by target agnostic stl_endian_p()
2024-09-30 22:12 [PATCH 0/3] hw/arm: Replace tswap32() by stl_endian_p() Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 1/3] target/arm: Expose arm_cpu_code_is_big_endian() prototype in 'cpu.h' Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 2/3] hw/arm: Have arm_write_bootloader() take a ARMCPU argument Philippe Mathieu-Daudé
@ 2024-09-30 22:12 ` Philippe Mathieu-Daudé
2024-10-01 9:35 ` Peter Maydell
2 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-30 22:12 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Peter Maydell, Thomas Huth, Hao Wu,
Laurent Vivier, Joel Stanley, qemu-arm, Andrew Jeffery,
Steven Lee, Tyrone Ting, Edgar E. Iglesias, Igor Mitsyanko,
Philippe Mathieu-Daudé, Cédric Le Goater, Jamin Lin,
Troy Lee, Anton Johansson
Replace the target-specific tswap32() calls by stl_endian_p()
which does the same but takes the endianness as argument, thus
is target-agnostic.
Get the vCPU endianness calling arm_cpu_code_is_big_endian().
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/boot.c | 8 +++++---
hw/arm/exynos4210.c | 7 +++----
hw/arm/npcm7xx.c | 6 ++++--
hw/arm/xilinx_zynq.c | 5 +++--
4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 6efd21f9c2..6e8dc00e6d 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -137,6 +137,7 @@ void arm_write_bootloader(const char *name,
const uint32_t *fixupcontext)
{
AddressSpace *as = arm_boot_address_space(cpu, info);
+ bool be = arm_cpu_code_is_big_endian(&cpu->env);
/* Fix up the specified bootloader fragment and write it into
* guest memory using rom_add_blob_fixed(). fixupcontext is
* an array giving the values to write in for the fixup types
@@ -173,7 +174,7 @@ void arm_write_bootloader(const char *name,
default:
abort();
}
- code[i] = tswap32(insn);
+ stl_endian_p(be, &code[i], insn);
}
assert((len * sizeof(uint32_t)) < BOOTLOADER_MAX_SIZE);
@@ -205,6 +206,7 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
hwaddr mvbar_addr)
{
AddressSpace *as = arm_boot_address_space(cpu, info);
+ bool be = arm_cpu_code_is_big_endian(&cpu->env);
int n;
uint32_t mvbar_blob[] = {
/* mvbar_addr: secure monitor vectors
@@ -243,13 +245,13 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
|| (info->board_setup_addr + sizeof(board_setup_blob) <= mvbar_addr));
for (n = 0; n < ARRAY_SIZE(mvbar_blob); n++) {
- mvbar_blob[n] = tswap32(mvbar_blob[n]);
+ stl_endian_p(be, &mvbar_blob[n], mvbar_blob[n]);
}
rom_add_blob_fixed_as("board-setup-mvbar", mvbar_blob, sizeof(mvbar_blob),
mvbar_addr, as);
for (n = 0; n < ARRAY_SIZE(board_setup_blob); n++) {
- board_setup_blob[n] = tswap32(board_setup_blob[n]);
+ stl_endian_p(be, &board_setup_blob[n], board_setup_blob[n]);
}
rom_add_blob_fixed_as("board-setup", board_setup_blob,
sizeof(board_setup_blob), info->board_setup_addr, as);
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index e3f1de2631..78e3fae3c1 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -23,7 +23,6 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
-#include "exec/tswap.h"
#include "cpu.h"
#include "hw/cpu/a9mpcore.h"
#include "hw/irq.h"
@@ -473,7 +472,7 @@ static const MemoryRegionOps exynos4210_chipid_and_omr_ops = {
void exynos4210_write_secondary(ARMCPU *cpu,
const struct arm_boot_info *info)
{
- int n;
+ bool be = arm_cpu_code_is_big_endian(&cpu->env);
uint32_t smpboot[] = {
0xe59f3034, /* ldr r3, External gic_cpu_if */
0xe59f2034, /* ldr r2, Internal gic_cpu_if */
@@ -496,8 +495,8 @@ void exynos4210_write_secondary(ARMCPU *cpu,
};
smpboot[ARRAY_SIZE(smpboot) - 1] = info->smp_bootreg_addr;
smpboot[ARRAY_SIZE(smpboot) - 2] = info->gic_cpu_if_addr;
- for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
- smpboot[n] = tswap32(smpboot[n]);
+ for (int n = 0; n < ARRAY_SIZE(smpboot); n++) {
+ stl_endian_p(be, &smpboot[n], smpboot[n]);
}
rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
info->smp_loader_start);
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index cb7791301b..6afdbf1598 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -309,6 +309,7 @@ static const struct {
static void npcm7xx_write_board_setup(ARMCPU *cpu,
const struct arm_boot_info *info)
{
+ bool be = arm_cpu_code_is_big_endian(&cpu->env);
uint32_t board_setup[] = {
0xe59f0010, /* ldr r0, clk_base_addr */
0xe59f1010, /* ldr r1, pllcon1_value */
@@ -323,7 +324,7 @@ static void npcm7xx_write_board_setup(ARMCPU *cpu,
int i;
for (i = 0; i < ARRAY_SIZE(board_setup); i++) {
- board_setup[i] = tswap32(board_setup[i]);
+ stl_endian_p(be, &board_setup[i], board_setup[i]);
}
rom_add_blob_fixed("board-setup", board_setup, sizeof(board_setup),
info->board_setup_addr);
@@ -332,6 +333,7 @@ static void npcm7xx_write_board_setup(ARMCPU *cpu,
static void npcm7xx_write_secondary_boot(ARMCPU *cpu,
const struct arm_boot_info *info)
{
+ bool be = arm_cpu_code_is_big_endian(&cpu->env);
/*
* The default smpboot stub halts the secondary CPU with a 'wfi'
* instruction, but the arch/arm/mach-npcm/platsmp.c in the Linux kernel
@@ -353,7 +355,7 @@ static void npcm7xx_write_secondary_boot(ARMCPU *cpu,
int i;
for (i = 0; i < ARRAY_SIZE(smpboot); i++) {
- smpboot[i] = tswap32(smpboot[i]);
+ stl_endian_p(be, &smpboot[i], smpboot[i]);
}
rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 37c234f5ab..0d6e246543 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -36,9 +36,9 @@
#include "hw/qdev-clock.h"
#include "sysemu/reset.h"
#include "qom/object.h"
-#include "exec/tswap.h"
#include "target/arm/cpu-qom.h"
#include "qapi/visitor.h"
+#include "cpu.h"
#define TYPE_ZYNQ_MACHINE MACHINE_TYPE_NAME("xilinx-zynq-a9")
OBJECT_DECLARE_SIMPLE_TYPE(ZynqMachineState, ZYNQ_MACHINE)
@@ -97,6 +97,7 @@ struct ZynqMachineState {
static void zynq_write_board_setup(ARMCPU *cpu,
const struct arm_boot_info *info)
{
+ bool be = arm_cpu_code_is_big_endian(&cpu->env);
int n;
uint32_t board_setup_blob[] = {
0xe3a004f8, /* mov r0, #0xf8000000 */
@@ -106,7 +107,7 @@ static void zynq_write_board_setup(ARMCPU *cpu,
0xe12fff1e, /* bx lr */
};
for (n = 0; n < ARRAY_SIZE(board_setup_blob); n++) {
- board_setup_blob[n] = tswap32(board_setup_blob[n]);
+ stl_endian_p(be, &board_setup_blob[n], board_setup_blob[n]);
}
rom_add_blob_fixed("board-setup", board_setup_blob,
sizeof(board_setup_blob), BOARD_SETUP_ADDR);
--
2.45.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] hw/arm: Replace tswap32() calls by target agnostic stl_endian_p()
2024-09-30 22:12 ` [PATCH 3/3] hw/arm: Replace tswap32() calls by target agnostic stl_endian_p() Philippe Mathieu-Daudé
@ 2024-10-01 9:35 ` Peter Maydell
0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2024-10-01 9:35 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Alistair Francis, Thomas Huth, Hao Wu, Laurent Vivier,
Joel Stanley, qemu-arm, Andrew Jeffery, Steven Lee, Tyrone Ting,
Edgar E. Iglesias, Igor Mitsyanko, Cédric Le Goater,
Jamin Lin, Troy Lee, Anton Johansson
On Mon, 30 Sept 2024 at 23:12, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Replace the target-specific tswap32() calls by stl_endian_p()
> which does the same but takes the endianness as argument, thus
> is target-agnostic.
> Get the vCPU endianness calling arm_cpu_code_is_big_endian().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/boot.c | 8 +++++---
> hw/arm/exynos4210.c | 7 +++----
> hw/arm/npcm7xx.c | 6 ++++--
> hw/arm/xilinx_zynq.c | 5 +++--
> 4 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 6efd21f9c2..6e8dc00e6d 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -137,6 +137,7 @@ void arm_write_bootloader(const char *name,
> const uint32_t *fixupcontext)
> {
> AddressSpace *as = arm_boot_address_space(cpu, info);
> + bool be = arm_cpu_code_is_big_endian(&cpu->env);
> /* Fix up the specified bootloader fragment and write it into
> * guest memory using rom_add_blob_fixed(). fixupcontext is
> * an array giving the values to write in for the fixup types
> @@ -173,7 +174,7 @@ void arm_write_bootloader(const char *name,
> default:
> abort();
> }
> - code[i] = tswap32(insn);
> + stl_endian_p(be, &code[i], insn);
> }
This is a behaviour change. For Arm, TARGET_BIG_ENDIAN
is always false, so tswap32() is "swap to/from little endian".
But arm_cpu_code_is_big_endian() looks at the state of
the guest vCPU (specifically, its SCTLR.B bit) and so
may swap to either big or little endian.
These functions are also called before the CPU is
reset for the first time, and before do_cpu_reset()
has maybe set SCTLR_B based on the ELF file. So we
can't guarantee SCTLR.B to be set correctly here where
we're trying to use it.
Maybe we do get this wrong for the old ARMv6-and-earlier
BE32 model where SCTLR.B might be non-zero -- they're
such a niche case that I don't suppose gets tested often.
(ARMv7-and-up is BE8 and instructions are always little
endian even when data is big endian.) But we should
separate out bug fixes from refactorings.
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-01 15:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-30 22:12 [PATCH 0/3] hw/arm: Replace tswap32() by stl_endian_p() Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 1/3] target/arm: Expose arm_cpu_code_is_big_endian() prototype in 'cpu.h' Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 2/3] hw/arm: Have arm_write_bootloader() take a ARMCPU argument Philippe Mathieu-Daudé
2024-09-30 22:12 ` [PATCH 3/3] hw/arm: Replace tswap32() calls by target agnostic stl_endian_p() Philippe Mathieu-Daudé
2024-10-01 9:35 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).