[kvm-unit-tests,v1,2/4] s390x: fix storing/loading fregs to right address
diff mbox series

Message ID 20180824115059.1517-3-david@redhat.com
State New
Headers show
Series
  • s390x: simple DXC test
Related show

Commit Message

David Hildenbrand Aug. 24, 2018, 11:50 a.m. UTC
LARL is wrong here, as we specify an explicit address. Let's use LA
(we could also drop it and fixup all loads/stores, but this is easier).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 s390x/cstart64.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Huth Aug. 27, 2018, 8:42 a.m. UTC | #1
On 2018-08-24 13:50, David Hildenbrand wrote:
> LARL is wrong here, as we specify an explicit address. Let's use LA
> (we could also drop it and fixup all loads/stores, but this is easier).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  s390x/cstart64.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 2f02d4d..02a4f77 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -58,7 +58,7 @@ init_psw_cont:
>  	/* save grs 0-15 */
>  	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
>  	/* save fprs 0-15 + fpc */
> -	larl	%r1, GEN_LC_SW_INT_FPRS
> +	la	%r1, GEN_LC_SW_INT_FPRS
>  	std	%f0, 0(%r1)
>  	std	%f1, 8(%r1)
>  	std	%f2, 16(%r1)
> @@ -80,7 +80,7 @@ init_psw_cont:
>  
>  	.macro RESTORE_REGS
>  	/* restore fprs 0-15 + fpc */
> -	larl	%r1, GEN_LC_SW_INT_FPRS
> +	la	%r1, GEN_LC_SW_INT_FPRS
>  	ld	%f0, 0(%r1)
>  	ld	%f1, 8(%r1)
>  	ld	%f2, 16(%r1)

Uh, relative addressing was certainly wrong here, I guess this caused
some weird corruption of the code in the memory?

Reviewed-by: Thomas Huth <thuth@redhat.com>
David Hildenbrand Aug. 27, 2018, 9:12 a.m. UTC | #2
On 27.08.2018 10:42, Thomas Huth wrote:
> On 2018-08-24 13:50, David Hildenbrand wrote:
>> LARL is wrong here, as we specify an explicit address. Let's use LA
>> (we could also drop it and fixup all loads/stores, but this is easier).
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  s390x/cstart64.S | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>> index 2f02d4d..02a4f77 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -58,7 +58,7 @@ init_psw_cont:
>>  	/* save grs 0-15 */
>>  	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
>>  	/* save fprs 0-15 + fpc */
>> -	larl	%r1, GEN_LC_SW_INT_FPRS
>> +	la	%r1, GEN_LC_SW_INT_FPRS
>>  	std	%f0, 0(%r1)
>>  	std	%f1, 8(%r1)
>>  	std	%f2, 16(%r1)
>> @@ -80,7 +80,7 @@ init_psw_cont:
>>  
>>  	.macro RESTORE_REGS
>>  	/* restore fprs 0-15 + fpc */
>> -	larl	%r1, GEN_LC_SW_INT_FPRS
>> +	la	%r1, GEN_LC_SW_INT_FPRS
>>  	ld	%f0, 0(%r1)
>>  	ld	%f1, 8(%r1)
>>  	ld	%f2, 16(%r1)
> 
> Uh, relative addressing was certainly wrong here, I guess this caused
> some weird corruption of the code in the memory?

Guess we were lucky, at least I never saw any such thing. But yes, we
were writing/reading to some wrong memory area.

> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 

Thanks!
Janosch Frank Aug. 27, 2018, 2:38 p.m. UTC | #3
On 24.08.2018 13:50, David Hildenbrand wrote:
> LARL is wrong here, as we specify an explicit address. Let's use LA
> (we could also drop it and fixup all loads/stores, but this is easier).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  s390x/cstart64.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 2f02d4d..02a4f77 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -58,7 +58,7 @@ init_psw_cont:
>  	/* save grs 0-15 */
>  	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
>  	/* save fprs 0-15 + fpc */
> -	larl	%r1, GEN_LC_SW_INT_FPRS
> +	la	%r1, GEN_LC_SW_INT_FPRS
>  	std	%f0, 0(%r1)
>  	std	%f1, 8(%r1)
>  	std	%f2, 16(%r1)
> @@ -80,7 +80,7 @@ init_psw_cont:
>  
>  	.macro RESTORE_REGS
>  	/* restore fprs 0-15 + fpc */
> -	larl	%r1, GEN_LC_SW_INT_FPRS
> +	la	%r1, GEN_LC_SW_INT_FPRS
>  	ld	%f0, 0(%r1)
>  	ld	%f1, 8(%r1)
>  	ld	%f2, 16(%r1)
>

Patch
diff mbox series

diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 2f02d4d..02a4f77 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -58,7 +58,7 @@  init_psw_cont:
 	/* save grs 0-15 */
 	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
 	/* save fprs 0-15 + fpc */
-	larl	%r1, GEN_LC_SW_INT_FPRS
+	la	%r1, GEN_LC_SW_INT_FPRS
 	std	%f0, 0(%r1)
 	std	%f1, 8(%r1)
 	std	%f2, 16(%r1)
@@ -80,7 +80,7 @@  init_psw_cont:
 
 	.macro RESTORE_REGS
 	/* restore fprs 0-15 + fpc */
-	larl	%r1, GEN_LC_SW_INT_FPRS
+	la	%r1, GEN_LC_SW_INT_FPRS
 	ld	%f0, 0(%r1)
 	ld	%f1, 8(%r1)
 	ld	%f2, 16(%r1)