diff mbox series

[1/7] target/ppc: Fix vmul[eo]* instructions marked 2.07

Message ID 20220304175156.2012315-2-matheus.ferst@eldorado.org.br (mailing list archive)
State New, archived
Headers show
Series target/ppc: Vector/VSX instruction batch fixes | expand

Commit Message

Matheus K. Ferst March 4, 2022, 5:51 p.m. UTC
From: "Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br>

Some ISA v2.03 Vector Multiply instructions marked to be ISA v2.07 only.
This patch fixes it.

Fixes: 80eca687c851 ("target/ppc: moved vector even and odd multiplication to decodetree")
Reported-by: Howard Spoelstra <hsp.cat7@gmail.com>
Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 target/ppc/translate/vmx-impl.c.inc | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Richard Henderson March 4, 2022, 10:35 p.m. UTC | #1
On 3/4/22 07:51, matheus.ferst@eldorado.org.br wrote:
> From: "Lucas Mateus Castro (alqotel)"<lucas.araujo@eldorado.org.br>
> 
> Some ISA v2.03 Vector Multiply instructions marked to be ISA v2.07 only.
> This patch fixes it.
> 
> Fixes: 80eca687c851 ("target/ppc: moved vector even and odd multiplication to decodetree")
> Reported-by: Howard Spoelstra<hsp.cat7@gmail.com>
> Suggested-by: Fabiano Rosas<farosas@linux.ibm.com>
> Signed-off-by: Lucas Mateus Castro (alqotel)<lucas.araujo@eldorado.org.br>
> Signed-off-by: Matheus Ferst<matheus.ferst@eldorado.org.br>
> ---
>   target/ppc/translate/vmx-impl.c.inc | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Cédric Le Goater March 5, 2022, 6:36 a.m. UTC | #2
On 3/4/22 18:51, matheus.ferst@eldorado.org.br wrote:
> From: "Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br>
> 
> Some ISA v2.03 Vector Multiply instructions marked to be ISA v2.07 only.
> This patch fixes it.

and MacOSX 10 is also fixed.

There are of lot invalid writes when openbios is loaded :

   ...
   Invalid write at addr 0xB70A8, size 4, region 'ppc_core99.bios', reason: rejected
   Invalid write at addr 0xB70AC, size 4, region 'ppc_core99.bios', reason: rejected
   Invalid write at addr 0xB70B0, size 4, region 'ppc_core99.bios', reason: rejected
   Invalid write at addr 0xB70B4, size 4, region 'ppc_core99.bios', reason: rejected
   ...

Mark,

shouldn't we model the FW region with RAM instead ?

@@ -162,7 +162,7 @@ static void ppc_core99_init(MachineState
      memory_region_add_subregion(get_system_memory(), 0, machine->ram);
  
      /* allocate and load firmware ROM */
-    memory_region_init_rom(bios, NULL, "ppc_core99.bios", PROM_SIZE,
+    memory_region_init_ram(bios, NULL, "ppc_core99.bios", PROM_SIZE,
                             &error_fatal);
      memory_region_add_subregion(get_system_memory(), PROM_BASE, bios);
  

Thanks,

C.


> 
> Fixes: 80eca687c851 ("target/ppc: moved vector even and odd multiplication to decodetree")
> Reported-by: Howard Spoelstra <hsp.cat7@gmail.com>
> Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
>   target/ppc/translate/vmx-impl.c.inc | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target/ppc/translate/vmx-impl.c.inc b/target/ppc/translate/vmx-impl.c.inc
> index f91bee839d..c5d02d13fe 100644
> --- a/target/ppc/translate/vmx-impl.c.inc
> +++ b/target/ppc/translate/vmx-impl.c.inc
> @@ -3141,14 +3141,14 @@ static bool trans_VMULLD(DisasContext *ctx, arg_VX *a)
>       return true;
>   }
>   
> -TRANS_FLAGS2(ALTIVEC_207, VMULESB, do_vx_helper, gen_helper_VMULESB)
> -TRANS_FLAGS2(ALTIVEC_207, VMULOSB, do_vx_helper, gen_helper_VMULOSB)
> -TRANS_FLAGS2(ALTIVEC_207, VMULEUB, do_vx_helper, gen_helper_VMULEUB)
> -TRANS_FLAGS2(ALTIVEC_207, VMULOUB, do_vx_helper, gen_helper_VMULOUB)
> -TRANS_FLAGS2(ALTIVEC_207, VMULESH, do_vx_helper, gen_helper_VMULESH)
> -TRANS_FLAGS2(ALTIVEC_207, VMULOSH, do_vx_helper, gen_helper_VMULOSH)
> -TRANS_FLAGS2(ALTIVEC_207, VMULEUH, do_vx_helper, gen_helper_VMULEUH)
> -TRANS_FLAGS2(ALTIVEC_207, VMULOUH, do_vx_helper, gen_helper_VMULOUH)
> +TRANS_FLAGS(ALTIVEC, VMULESB, do_vx_helper, gen_helper_VMULESB)
> +TRANS_FLAGS(ALTIVEC, VMULOSB, do_vx_helper, gen_helper_VMULOSB)
> +TRANS_FLAGS(ALTIVEC, VMULEUB, do_vx_helper, gen_helper_VMULEUB)
> +TRANS_FLAGS(ALTIVEC, VMULOUB, do_vx_helper, gen_helper_VMULOUB)
> +TRANS_FLAGS(ALTIVEC, VMULESH, do_vx_helper, gen_helper_VMULESH)
> +TRANS_FLAGS(ALTIVEC, VMULOSH, do_vx_helper, gen_helper_VMULOSH)
> +TRANS_FLAGS(ALTIVEC, VMULEUH, do_vx_helper, gen_helper_VMULEUH)
> +TRANS_FLAGS(ALTIVEC, VMULOUH, do_vx_helper, gen_helper_VMULOUH)
>   TRANS_FLAGS2(ALTIVEC_207, VMULESW, do_vx_helper, gen_helper_VMULESW)
>   TRANS_FLAGS2(ALTIVEC_207, VMULOSW, do_vx_helper, gen_helper_VMULOSW)
>   TRANS_FLAGS2(ALTIVEC_207, VMULEUW, do_vx_helper, gen_helper_VMULEUW)
Mark Cave-Ayland March 7, 2022, 8:53 a.m. UTC | #3
On 05/03/2022 06:36, Cédric Le Goater wrote:

> On 3/4/22 18:51, matheus.ferst@eldorado.org.br wrote:
>> From: "Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br>
>>
>> Some ISA v2.03 Vector Multiply instructions marked to be ISA v2.07 only.
>> This patch fixes it.
> 
> and MacOSX 10 is also fixed.
> 
> There are of lot invalid writes when openbios is loaded :
> 
>    ...
>    Invalid write at addr 0xB70A8, size 4, region 'ppc_core99.bios', reason: rejected
>    Invalid write at addr 0xB70AC, size 4, region 'ppc_core99.bios', reason: rejected
>    Invalid write at addr 0xB70B0, size 4, region 'ppc_core99.bios', reason: rejected
>    Invalid write at addr 0xB70B4, size 4, region 'ppc_core99.bios', reason: rejected
>    ...
> 
> Mark,
> 
> shouldn't we model the FW region with RAM instead ?
> 
> @@ -162,7 +162,7 @@ static void ppc_core99_init(MachineState
>       memory_region_add_subregion(get_system_memory(), 0, machine->ram);
> 
>       /* allocate and load firmware ROM */
> -    memory_region_init_rom(bios, NULL, "ppc_core99.bios", PROM_SIZE,
> +    memory_region_init_ram(bios, NULL, "ppc_core99.bios", PROM_SIZE,
>                              &error_fatal);
>       memory_region_add_subregion(get_system_memory(), PROM_BASE, bios);
> 
> 
> Thanks,
> 
> C.

I don't believe so. The original aim of OpenBIOS was to run as a free replacement 
firmware on real hardware, so what should happen is that OpenBIOS should copy at 
least the bss section into RAM early in the startup process. There could well be 
errors in those calculations as I have tried to adjust them over time though.


ATB,

Mark.
diff mbox series

Patch

diff --git a/target/ppc/translate/vmx-impl.c.inc b/target/ppc/translate/vmx-impl.c.inc
index f91bee839d..c5d02d13fe 100644
--- a/target/ppc/translate/vmx-impl.c.inc
+++ b/target/ppc/translate/vmx-impl.c.inc
@@ -3141,14 +3141,14 @@  static bool trans_VMULLD(DisasContext *ctx, arg_VX *a)
     return true;
 }
 
-TRANS_FLAGS2(ALTIVEC_207, VMULESB, do_vx_helper, gen_helper_VMULESB)
-TRANS_FLAGS2(ALTIVEC_207, VMULOSB, do_vx_helper, gen_helper_VMULOSB)
-TRANS_FLAGS2(ALTIVEC_207, VMULEUB, do_vx_helper, gen_helper_VMULEUB)
-TRANS_FLAGS2(ALTIVEC_207, VMULOUB, do_vx_helper, gen_helper_VMULOUB)
-TRANS_FLAGS2(ALTIVEC_207, VMULESH, do_vx_helper, gen_helper_VMULESH)
-TRANS_FLAGS2(ALTIVEC_207, VMULOSH, do_vx_helper, gen_helper_VMULOSH)
-TRANS_FLAGS2(ALTIVEC_207, VMULEUH, do_vx_helper, gen_helper_VMULEUH)
-TRANS_FLAGS2(ALTIVEC_207, VMULOUH, do_vx_helper, gen_helper_VMULOUH)
+TRANS_FLAGS(ALTIVEC, VMULESB, do_vx_helper, gen_helper_VMULESB)
+TRANS_FLAGS(ALTIVEC, VMULOSB, do_vx_helper, gen_helper_VMULOSB)
+TRANS_FLAGS(ALTIVEC, VMULEUB, do_vx_helper, gen_helper_VMULEUB)
+TRANS_FLAGS(ALTIVEC, VMULOUB, do_vx_helper, gen_helper_VMULOUB)
+TRANS_FLAGS(ALTIVEC, VMULESH, do_vx_helper, gen_helper_VMULESH)
+TRANS_FLAGS(ALTIVEC, VMULOSH, do_vx_helper, gen_helper_VMULOSH)
+TRANS_FLAGS(ALTIVEC, VMULEUH, do_vx_helper, gen_helper_VMULEUH)
+TRANS_FLAGS(ALTIVEC, VMULOUH, do_vx_helper, gen_helper_VMULOUH)
 TRANS_FLAGS2(ALTIVEC_207, VMULESW, do_vx_helper, gen_helper_VMULESW)
 TRANS_FLAGS2(ALTIVEC_207, VMULOSW, do_vx_helper, gen_helper_VMULOSW)
 TRANS_FLAGS2(ALTIVEC_207, VMULEUW, do_vx_helper, gen_helper_VMULEUW)