On 10/3/24 14:31, Pierrick Bouvier wrote:
On 10/3/24 13:48, Philippe Mathieu-Daudé wrote:
On 3/10/24 18:04, Pierrick Bouvier wrote:
On 10/3/24 09:02, Philippe Mathieu-Daudé wrote:
On 30/9/24 16:32, Thomas Huth wrote:
On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
Replace a pair of memcpy() + tswap32() by stl_endian_p(),
which also swap the value using target endianness.
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
hw/xtensa/xtfpga.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 228f00b045..521fe84b01 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc
*board, MachineState *machine)
const size_t boot_sz = TARGET_BIG_ENDIAN ?
sizeof(boot_be)
:
sizeof(boot_le);
uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
- uint32_t entry_pc = tswap32(entry_point);
- uint32_t entry_a2 = tswap32(tagptr);
- memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
- memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
+ stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
+ stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
Why don't you simply use stl_p() here?
We want to remove the tswap32() calls...
I think is point is that you could directly use stl_be_p, instead of
stl_endian_p(TARGET_BIT_ENDIAN, ...).
TARGET_BIG_ENDIAN is defined as 0 on little endian, and 1 on big one.
The following change isn't worth it:
if (TARGET_BIG_ENDIAN) {
stl_be_p(boot + 8, tagptr);
} else {
stl_le_p(boot + 8, tagptr);
}
Maybe I'm missing Thomas point, as the xtfpga machines are available
for both xtensa-softmmu (LE) and xtensaeb-softmmu (BE).
I don't know if your intent is to make be/le variant "private" and
relies only on endian_p though.
My intent is to enforce endian agnostic API uses when possible, and
use LE/BE specific variant when it is known at build time.
Oh ok, it's me who missed your point then.
For some reason, I thought we were always calling big endian variant.
Thus, your implementation makes totally sense.
Let's see if Thomas meant something different.
Else,
Reviewed-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
Looking more closely,
stl_p is already correctly defined when you know at compile time your
target endianness. So Thomas was referring to this.
https://gitlab.com/qemu-project/qemu/-/blame/master/include/exec/cpu-all.h?ref_type=heads#L49
