qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 5/6] hw/arm/xilinx_zynq: Remove tswap32() calls and constify


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH 5/6] hw/arm/xilinx_zynq: Remove tswap32() calls and constify smpboot[]
Date: Fri, 23 Dec 2022 11:05:53 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.6.0

On 23/12/22 11:01, Philippe Mathieu-Daudé wrote:
On 23/12/22 04:54, Edgar E. Iglesias wrote:
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é <philmd@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?

extract32() is a function. I agree we can rewrite this macro to remove
it, I was simply lazy ;) I'll do for v2 so the array will be const.

Well it is already runtime const, I meant 'static const' so it becomes
build-time const.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]