[PATCH 0/3] hw/ppc: Replace tswap32() by stl_endian_p()

[PATCH 0/3] hw/ppc: Replace tswap32() by stl_endian_p()

[Philippe Mathieu-Daudé]

Expose ppc_cpu_is_big_endian(), then use it calling the target
agnostic stl_endian_p() method, removing the use of the target
specific tswap32() method.

Philippe Mathieu-Daudé (3):
  target/ppc: Expose ppc_cpu_is_big_endian() prototype in 'cpu.h'
  hw/ppc/sam460ex: Replace tswap32() by stl_endian_p()
  hw/ppc/virtex_ml507: Replace tswap32() by stl_endian_p()

 target/ppc/cpu.h      | 2 ++
 hw/ppc/sam460ex.c     | 5 +++--
 hw/ppc/virtex_ml507.c | 2 +-
 target/ppc/cpu_init.c | 2 +-
 4 files changed, 7 insertions(+), 4 deletions(-)

-- 
2.45.2

[PATCH 1/3] target/ppc: Expose ppc_cpu_is_big_endian() prototype in 'cpu.h'

[Philippe Mathieu-Daudé]

Expose ppc_cpu_is_big_endian() so it can be used by hw/ code.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/cpu.h      | 2 ++
 target/ppc/cpu_init.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 321ed2da75..4a4911502d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -3007,6 +3007,8 @@ static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv)
 
     return ile;
 }
+
+bool ppc_cpu_is_big_endian(CPUState *cs);
 #endif
 
 void dump_mmu(CPUPPCState *env);
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 23881d09e9..00e0a8bf2c 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7343,7 +7343,7 @@ static void ppc_cpu_reset_hold(Object *obj, ResetType type)
 
 #ifndef CONFIG_USER_ONLY
 
-static bool ppc_cpu_is_big_endian(CPUState *cs)
+bool ppc_cpu_is_big_endian(CPUState *cs)
 {
     cpu_synchronize_state(cs);
 
-- 
2.45.2

[PATCH 2/3] hw/ppc/sam460ex: Replace tswap32() by stl_endian_p()

[Philippe Mathieu-Daudé]

Replace the target-specific tswap32() call by stl_endian_p()
which does the same but takes the endianness as argument, thus
is target-agnostic.
Get the vCPU endianness calling ppc_cpu_is_big_endian().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/sam460ex.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 8dc75fb9f0..6257ddbec6 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -248,10 +248,11 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
 static void main_cpu_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
+    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     struct boot_info *bi = env->load_info;
 
-    cpu_reset(CPU(cpu));
+    cpu_reset(cs);
 
     /* either we have a kernel to boot or we jump to U-Boot */
     if (bi->entry != UBOOT_ENTRY) {
@@ -261,7 +262,7 @@ static void main_cpu_reset(void *opaque)
 
         /* Create a mapping for the kernel.  */
         mmubooke_create_initial_mapping(env, 0, 0);
-        env->gpr[6] = tswap32(EPAPR_MAGIC);
+        stl_endian_p(ppc_cpu_is_big_endian(cs), &env->gpr[6], EPAPR_MAGIC);
         env->gpr[7] = (16 * MiB) - 8; /* bi->ima_size; */
 
     } else {
-- 
2.45.2

Re: [PATCH 2/3] hw/ppc/sam460ex: Replace tswap32() by stl_endian_p()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2028 bytes --]

On Mon, 30 Sep 2024, Philippe Mathieu-Daudé wrote:
> Replace the target-specific tswap32() call by stl_endian_p()
> which does the same but takes the endianness as argument, thus
> is target-agnostic.
> Get the vCPU endianness calling ppc_cpu_is_big_endian().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/sam460ex.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 8dc75fb9f0..6257ddbec6 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -248,10 +248,11 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
> static void main_cpu_reset(void *opaque)
> {
>     PowerPCCPU *cpu = opaque;
> +    CPUState *cs = CPU(cpu);
>     CPUPPCState *env = &cpu->env;
>     struct boot_info *bi = env->load_info;
>
> -    cpu_reset(CPU(cpu));
> +    cpu_reset(cs);
>
>     /* either we have a kernel to boot or we jump to U-Boot */
>     if (bi->entry != UBOOT_ENTRY) {
> @@ -261,7 +262,7 @@ static void main_cpu_reset(void *opaque)
>
>         /* Create a mapping for the kernel.  */
>         mmubooke_create_initial_mapping(env, 0, 0);
> -        env->gpr[6] = tswap32(EPAPR_MAGIC);
> +        stl_endian_p(ppc_cpu_is_big_endian(cs), &env->gpr[6], EPAPR_MAGIC);

I think this slightly changes behaviour even if getting the same result. 
The ppc_cpu_is_big_endian() checks the CPU bit for current mode while 
previously tswap32 only checks for host endianness vs. big endian which is 
the default mode for PPC which is what the CPU is in during boot where 
this data is used. So even with checking the bit it'd be the same but the 
check is not needed. I think you could/should just have hard coded big 
endian here to preserve the current behaviour.

(There were some discussion a while back if this EPAPR_MAGIC is correct or 
needed at all but preserving current behaviour is a good enough for now.)

Regards,
BALATON Zoltan

>         env->gpr[7] = (16 * MiB) - 8; /* bi->ima_size; */
>
>     } else {
>

[PATCH 3/3] hw/ppc/virtex_ml507: Replace tswap32() by stl_endian_p()

[Philippe Mathieu-Daudé]

Replace the target-specific tswap32() call by stl_endian_p()
which does the same but takes the endianness as argument, thus
is target-agnostic.
Get the vCPU endianness calling ppc_cpu_is_big_endian().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/virtex_ml507.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index c49da1f46f..135dcbde06 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -141,7 +141,7 @@ static void main_cpu_reset(void *opaque)
 
     /* Create a mapping for the kernel.  */
     mmubooke_create_initial_mapping(env, 0, 0);
-    env->gpr[6] = tswap32(EPAPR_MAGIC);
+    stl_endian_p(ppc_cpu_is_big_endian(CPU(cpu)), &env->gpr[6], EPAPR_MAGIC);
     env->gpr[7] = bi->ima_size;
 }
 
-- 
2.45.2

Re: [PATCH 3/3] hw/ppc/virtex_ml507: Replace tswap32() by stl_endian_p()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]


On Mon, 30 Sep 2024, Philippe Mathieu-Daudé wrote:
> Replace the target-specific tswap32() call by stl_endian_p()
> which does the same but takes the endianness as argument, thus
> is target-agnostic.
> Get the vCPU endianness calling ppc_cpu_is_big_endian().

I don't know about virtex but maybe the same comment applies as for 
sam460ex.

Regards,
BALATON Zoltan

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/virtex_ml507.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index c49da1f46f..135dcbde06 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -141,7 +141,7 @@ static void main_cpu_reset(void *opaque)
>
>     /* Create a mapping for the kernel.  */
>     mmubooke_create_initial_mapping(env, 0, 0);
> -    env->gpr[6] = tswap32(EPAPR_MAGIC);
> +    stl_endian_p(ppc_cpu_is_big_endian(CPU(cpu)), &env->gpr[6], EPAPR_MAGIC);
>     env->gpr[7] = bi->ima_size;
> }
>
>