[Qemu-devel] [PATCH 0/8] target/ppc: switch fpr/vsrl registers so all VSX registers are in host endian order

Mark Cave-Ayland posted 8 patches 6 years, 8 months ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190303172343.13406-1-mark.cave-ayland@ilande.co.uk
Maintainers: David Gibson <david@gibson.dropbear.id.au>
There is a newer version of this series
target/ppc/cpu.h                    | 56 +++++++++++++++++++++++++++++++++++--
target/ppc/internal.h               | 27 +++---------------
target/ppc/machine.c                |  8 +++---
target/ppc/translate.c              | 28 ++++++++-----------
target/ppc/translate/vmx-impl.inc.c | 27 ++++++++----------
target/ppc/translate/vsx-impl.inc.c | 39 +++-----------------------
6 files changed, 88 insertions(+), 97 deletions(-)
[Qemu-devel] [PATCH 0/8] target/ppc: switch fpr/vsrl registers so all VSX registers are in host endian order
Posted by Mark Cave-Ayland 6 years, 8 months ago
After some investigation into Andrew's report of corruption in his ppc64le
tests at https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07234.html, I
discovered the underlying cause was that the first 32 VSX registers are not
stored in host endian order.

This is something that Richard and I had discussed before, but missed that with
VSX if you have source registers from different register sets then even logical
operations will give you the wrong result.

Rather than revert 7b8fe477e1 "target/ppc: convert VSX logical operations to
vector operations" let's keep the use of the accelerated vector instructions,
and instead fix the real problem which is to switch the first 32 VSX registers
to host endian order matching the VMX registers.

Patches 1-5 aim to consolidate the offset calculations for both CPUPPCState
and the associated _ptr() functions into one single place.

With this preliminary work complete, patch 6 switches the first 32 registers
into host endian order without too much difficulty.

Finally now that all VSX registers are stored in the same way, the vsr offset
functions and get_cpu_vsrh()/get_cpu_vsrl() can be simplified accordingly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


Mark Cave-Ayland (8):
  target/ppc: introduce single fpr_offset() function
  target/ppc: introduce single vsrl_offset() function
  target/ppc: move Vsr* macros from internal.h to cpu.h
  target/ppc: introduce avrh_offset() and avrl_offset() functions
  target/ppc: introduce avr_offset() function
  target/ppc: switch fpr/vsrl registers so all VSX registers are in host
    endian order
  target/ppc: introduce vsrh_offset() function
  target/ppc: simplify get_cpu_vsrh() and get_cpu_vsrl() functions

 target/ppc/cpu.h                    | 56 +++++++++++++++++++++++++++++++++++--
 target/ppc/internal.h               | 27 +++---------------
 target/ppc/machine.c                |  8 +++---
 target/ppc/translate.c              | 28 ++++++++-----------
 target/ppc/translate/vmx-impl.inc.c | 27 ++++++++----------
 target/ppc/translate/vsx-impl.inc.c | 39 +++-----------------------
 6 files changed, 88 insertions(+), 97 deletions(-)

-- 
2.11.0


Re: [Qemu-devel] [PATCH 0/8] target/ppc: switch fpr/vsrl registers so all VSX registers are in host endian order
Posted by David Gibson 6 years, 8 months ago
On Sun, Mar 03, 2019 at 05:23:35PM +0000, Mark Cave-Ayland wrote:
> After some investigation into Andrew's report of corruption in his ppc64le
> tests at https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07234.html, I
> discovered the underlying cause was that the first 32 VSX registers are not
> stored in host endian order.
> 
> This is something that Richard and I had discussed before, but missed that with
> VSX if you have source registers from different register sets then even logical
> operations will give you the wrong result.
> 
> Rather than revert 7b8fe477e1 "target/ppc: convert VSX logical operations to
> vector operations" let's keep the use of the accelerated vector instructions,
> and instead fix the real problem which is to switch the first 32 VSX registers
> to host endian order matching the VMX registers.
> 
> Patches 1-5 aim to consolidate the offset calculations for both CPUPPCState
> and the associated _ptr() functions into one single place.
> 
> With this preliminary work complete, patch 6 switches the first 32 registers
> into host endian order without too much difficulty.
> 
> Finally now that all VSX registers are stored in the same way, the vsr offset
> functions and get_cpu_vsrh()/get_cpu_vsrl() can be simplified accordingly.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

I've applied the first two patches.  The rest I'll wait on a respin
addressing Richard's comments.

> 
> 
> Mark Cave-Ayland (8):
>   target/ppc: introduce single fpr_offset() function
>   target/ppc: introduce single vsrl_offset() function
>   target/ppc: move Vsr* macros from internal.h to cpu.h
>   target/ppc: introduce avrh_offset() and avrl_offset() functions
>   target/ppc: introduce avr_offset() function
>   target/ppc: switch fpr/vsrl registers so all VSX registers are in host
>     endian order
>   target/ppc: introduce vsrh_offset() function
>   target/ppc: simplify get_cpu_vsrh() and get_cpu_vsrl() functions
> 
>  target/ppc/cpu.h                    | 56 +++++++++++++++++++++++++++++++++++--
>  target/ppc/internal.h               | 27 +++---------------
>  target/ppc/machine.c                |  8 +++---
>  target/ppc/translate.c              | 28 ++++++++-----------
>  target/ppc/translate/vmx-impl.inc.c | 27 ++++++++----------
>  target/ppc/translate/vsx-impl.inc.c | 39 +++-----------------------
>  6 files changed, 88 insertions(+), 97 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson