s390x/tcg: always enable AFP for linux-user

PATCH Applied David Hildenbrand 6 participants 7 messages Oct 8, 2018 – Oct 9, 2018

Share & tools

David Hildenbrand

author

2018-10-08 15:32 UTC #0 lore raw

PATCH 1/1 s390x/tcg: always enable AFP for linux-user

linux-user should always enable AFP, otherwise our emulated binary might crash once it tries to make use of additional floating-point registers or instructions.

Cc Peter Maydell <[email protected]>

Cc Alex Bennée <[email protected]>

Fixes db0504154e ("s390x/tcg: check for AFP-register, BFP and DFP data exceptions")

Reported-by Alex Bennée <[email protected]>

Signed-off-by David Hildenbrand <[email protected]>

---
 target/s390x/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

target/s390x/cpu.c +5 -0

index 8ed4823d6e..18ba7f85a5 100644
@@ -145,6 +145,11 @@ static void s390_cpu_full_reset(CPUState *s)
     env->cregs[0] = CR0_RESET;
     env->cregs[14] = CR14_RESET;
 
+#if defined(CONFIG_USER_ONLY)
+    /* user mode should always be allowed to use the full FPU */
+    env->cregs[0] |= CR0_AFP;
+#endif
+
     /* architectured initial value for Breaking-Event-Address register */
     env->gbea = 1;

Signature · 1 line

2.17.1

Thomas Huth

Reviewed-by on v1 1/1

2018-10-08 15:40 UTC #1 lore raw

Quoted 20 lines · David Hildenbrand

linux-user should always enable AFP, otherwise our emulated binary might crash once it tries to make use of additional floating-point registers or instructions.
Cc Peter Maydell <[email protected]>
Cc Alex Bennée <[email protected]>
Fixes db0504154e ("s390x/tcg: check for AFP-register, BFP and DFP data exceptions")
Reported-by Alex Bennée <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)
target/s390x/cpu.c +0 -0
index 8ed4823d6e..18ba7f85a5 100644
@@ -145,6 +145,11 @@ static void s390_cpu_full_reset(CPUState *s)
     env->cregs[0] = CR0_RESET;

target/s390x/cpu.c static void s390_cpu_full_reset(CPUState *s) · line 155

Quoted 10 lines

target/s390x/cpu.c +5 -0

     env->cregs[14] = CR14_RESET;
 
+#if defined(CONFIG_USER_ONLY)
+    /* user mode should always be allowed to use the full FPU */
+    env->cregs[0] |= CR0_AFP;
+#endif
+
     /* architectured initial value for Breaking-Event-Address register */
     env->gbea = 1;

Reviewed-by Thomas Huth <[email protected]>

Cornelia Huck

Applied

2018-10-08 16:05 UTC #2 lore raw

In reply to David Hildenbrand

Quoted 19 lines · On Mon, 8 Oct 2018 17:32:04 +0200 David Hildenbrand <[email protected]> wrote:

linux-user should always enable AFP, otherwise our emulated binary might crash once it tries to make use of additional floating-point registers or instructions.
Cc Peter Maydell <[email protected]>
Cc Alex Bennée <[email protected]>
Fixes db0504154e ("s390x/tcg: check for AFP-register, BFP and DFP data exceptions")
Reported-by Alex Bennée <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)
target/s390x/cpu.c +0 -0
index 8ed4823d6e..18ba7f85a5 100644
@@ -145,6 +145,11 @@ static void s390_cpu_full_reset(CPUState *s)
     env->cregs[0] = CR0_RESET;

target/s390x/cpu.c static void s390_cpu_full_reset(CPUState *s) · line 155

Quoted 10 lines

target/s390x/cpu.c +5 -0

     env->cregs[14] = CR14_RESET;
 
+#if defined(CONFIG_USER_ONLY)
+    /* user mode should always be allowed to use the full FPU */
+    env->cregs[0] |= CR0_AFP;
+#endif
+
     /* architectured initial value for Breaking-Event-Address register */
     env->gbea = 1;

I've reproduced the crash via make check-tcg, and this patch fixes it.

Thanks, applied.

Richard Henderson

Reviewed-by on v1 1/1

2018-10-08 16:05 UTC #3 lore raw

In reply to David Hildenbrand

Quoted 4 lines · David Hildenbrand

linux-user should always enable AFP, otherwise our emulated binary might crash once it tries to make use of additional floating-point registers or instructions.

Quoted 8 lines

Cc Peter Maydell <[email protected]>
Cc Alex Bennée <[email protected]>
Fixes db0504154e ("s390x/tcg: check for AFP-register, BFP and DFP data exceptions")
Reported-by Alex Bennée <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

Reviewed-by Richard Henderson <[email protected]>

Alex Bennée

Reviewed-by Tested-by on v1 1/1

2018-10-08 16:11 UTC #4 lore raw

In reply to David Hildenbrand

Quoted 18 lines · David Hildenbrand

linux-user should always enable AFP, otherwise our emulated binary might crash once it tries to make use of additional floating-point registers or instructions.
Cc Peter Maydell <[email protected]>
Cc Alex Bennée <[email protected]>
Fixes db0504154e ("s390x/tcg: check for AFP-register, BFP and DFP data exceptions")
Reported-by Alex Bennée <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)
target/s390x/cpu.c +0 -0
index 8ed4823d6e..18ba7f85a5 100644

target/s390x/cpu.c static void s390_cpu_full_reset(CPUState *s) · line 152

Quoted 8 lines

target/s390x/cpu.c +5 -0

@@ -145,6 +145,11 @@ static void s390_cpu_full_reset(CPUState *s)
     env->cregs[0] = CR0_RESET;
     env->cregs[14] = CR14_RESET;
 
+#if defined(CONFIG_USER_ONLY)
+    /* user mode should always be allowed to use the full FPU */
+    env->cregs[0] |= CR0_AFP;
+#endif
+

Reviewed-by Alex Bennée <[email protected]>

Tested-by Alex Bennée <[email protected]>

target/s390x/cpu.c static void s390_cpu_full_reset(CPUState *s) · line 154

Quoted 2 lines

target/s390x/cpu.c +0 -0

     /* architectured initial value for Breaking-Event-Address register */
     env->gbea = 1;

--
Alex Bennée

Cornelia Huck

Reviewed-by Tested-by on v1 1/1

2018-10-08 16:16 UTC #5 lore raw

In reply to David Hildenbrand

Quoted 19 lines · On Mon, 8 Oct 2018 17:32:04 +0200 David Hildenbrand <[email protected]> wrote:

linux-user should always enable AFP, otherwise our emulated binary might crash once it tries to make use of additional floating-point registers or instructions.
Cc Peter Maydell <[email protected]>
Cc Alex Bennée <[email protected]>
Fixes db0504154e ("s390x/tcg: check for AFP-register, BFP and DFP data exceptions")
Reported-by Alex Bennée <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)
target/s390x/cpu.c +0 -0
index 8ed4823d6e..18ba7f85a5 100644
@@ -145,6 +145,11 @@ static void s390_cpu_full_reset(CPUState *s)
     env->cregs[0] = CR0_RESET;

target/s390x/cpu.c static void s390_cpu_full_reset(CPUState *s) · line 155

Quoted 10 lines

target/s390x/cpu.c +5 -0

     env->cregs[14] = CR14_RESET;
 
+#if defined(CONFIG_USER_ONLY)
+    /* user mode should always be allowed to use the full FPU */
+    env->cregs[0] |= CR0_AFP;
+#endif
+
     /* architectured initial value for Breaking-Event-Address register */
     env->gbea = 1;

In case this is applied directly:

Reviewed-by Cornelia Huck <[email protected]>

Tested-by Cornelia Huck <[email protected]>

Peter Maydell

2018-10-09 10:31 UTC #6 lore raw

Quoted 35 lines · Cornelia Huck

Quoted 19 lines · On Mon, 8 Oct 2018 17:32:04 +0200 David Hildenbrand <[email protected]> wrote:
linux-user should always enable AFP, otherwise our emulated binary might crash once it tries to make use of additional floating-point registers or instructions.
Cc Peter Maydell <[email protected]>
Cc Alex Bennée <[email protected]>
Fixes db0504154e ("s390x/tcg: check for AFP-register, BFP and DFP data exceptions")
Reported-by Alex Bennée <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)
target/s390x/cpu.c +0 -0
index 8ed4823d6e..18ba7f85a5 100644
@@ -145,6 +145,11 @@ static void s390_cpu_full_reset(CPUState *s)
     env->cregs[0] = CR0_RESET;
target/s390x/cpu.c static void s390_cpu_full_reset(CPUState *s) · line 155
Quoted 10 lines
target/s390x/cpu.c +5 -0
     env->cregs[14] = CR14_RESET;
 
+#if defined(CONFIG_USER_ONLY)
+    /* user mode should always be allowed to use the full FPU */
+    env->cregs[0] |= CR0_AFP;
+#endif
+
     /* architectured initial value for Breaking-Event-Address register */
     env->gbea = 1;
 
In case this is applied directly:

Quoted 2 lines

Reviewed-by Cornelia Huck <[email protected]>
Tested-by Cornelia Huck <[email protected]>

Thanks; I've applied it to master, and also added 'make check-tcg' to my set of things I run as part of a merge, which should catch this kind of bug in future.

-- PMM