[Qemu-devel] [PATCH 00/10] target-s390x: TCG fixes and improvements

[Qemu-devel] [PATCH 00/10] target-s390x: TCG fixes and improvements

[Aurelien Jarno]

This patchset fixes a few issues with the s390x emulation and improves
it a bit by a emulating a few more instructions.

With this patchset and the ones posted a few days ago, I have been able
to build the GNU libc in both a 64-bit guest with 64-bit userland and a
64-bit guest with a 31-bit userland and pass the testsuite in both
cases.

Aurelien Jarno (10):
  target-s390x: fix PSW value on dynamical exception from helpers
  target-s390x: fix MMU index computation
  target-s390x: define default NaN values
  target-s390x: silence NaNs for LOAD LENGTHENED and LOAD ROUNDED
  target-s390x: detect tininess before rounding for FP operations
  target-s390x: improve facilities list
  target-s390x: enable fully implemented facilities
  target-s390x: implement STFLE instruction
  target-s390x: move a few instructions to the correct facility
  target-s390x: implement LAY and LAEY instructions

 fpu/softfloat-specialize.h |  8 +++--
 target-s390x/cpu.c         |  8 +++++
 target-s390x/cpu.h         | 81 +++++++++++++++++++++++++++++++++++++++++++---
 target-s390x/fpu_helper.c  | 12 +++----
 target-s390x/helper.h      |  1 +
 target-s390x/insn-data.def | 13 +++++---
 target-s390x/misc_helper.c | 21 +++++++++++-
 target-s390x/translate.c   | 46 ++++++++++++++++++++++++--
 8 files changed, 170 insertions(+), 20 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH 01/10] target-s390x: fix PSW value on dynamical exception from helpers

[Aurelien Jarno]

runtime_exception computes the psw.addr value using the actual exception
address and the instruction length computed by calling the get_ilen
function. However as explained above the get_ilen code, it returns the
actual instruction length, and not the ILC. Therefore there is no need to
multiply the value by 2.

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/misc_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 3ec7268..b375ab7 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -61,7 +61,7 @@ void QEMU_NORETURN runtime_exception(CPUS390XState *env, int excp,
     /* Advance past the insn.  */
     t = cpu_ldub_code(env, env->psw.addr);
     env->int_pgm_ilen = t = get_ilen(t);
-    env->psw.addr += 2 * t;
+    env->psw.addr += t;
 
     cpu_loop_exit(cs);
 }
-- 
2.1.4

[Qemu-devel] [PATCH 02/10] target-s390x: fix MMU index computation

[Aurelien Jarno]

The cpu_mmu_index function wrongly looks at PSW P bit to determine the
MMU index, while this bit actually only control the use of priviledge
instructions. The addressing mode is detected by looking at the PSW ASC
bits instead.

This used to work more or less correctly up to kernel 3.6 as the kernel
was running in primary space and userland in secondary space. Since
kernel 3.7 the default is to run the kernel in home space and userland
in primary space. While the current QEMU code seems to work it open some
security issues, like accessing the lowcore memory in R/W mode from a
userspace process once it has been accessed by the kernel (it is then
cached by the QEMU TLB).

At the same time change the MMU_USER_IDX value so that it matches the
value used in recent kernels.

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/cpu.h | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 71ef847..99773e0 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -48,7 +48,7 @@
 #define MMU_MODE1_SUFFIX _secondary
 #define MMU_MODE2_SUFFIX _home
 
-#define MMU_USER_IDX 1
+#define MMU_USER_IDX 0
 
 #define MAX_EXT_QUEUE 16
 #define MAX_IO_QUEUE 16
@@ -295,11 +295,18 @@ typedef struct CPUS390XState {
 
 static inline int cpu_mmu_index (CPUS390XState *env)
 {
-    if (env->psw.mask & PSW_MASK_PSTATE) {
+    switch (env->psw.mask & PSW_MASK_ASC) {
+    case PSW_ASC_PRIMARY:
+        return 0;
+    case PSW_ASC_SECONDARY:
         return 1;
+    case PSW_ASC_HOME:
+        return 2;
+    case PSW_ASC_ACCREG:
+        /* Fallthrough: access register mode is not yet supported */
+    default:
+        abort();
     }
-
-    return 0;
 }
 
 static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
-- 
2.1.4

[Qemu-devel] [PATCH 03/10] target-s390x: define default NaN values

[Aurelien Jarno]

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 fpu/softfloat-specialize.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index fa1214a..6dd41d8 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -113,7 +113,7 @@ const float16 float16_default_nan = const_float16(0xFE00);
 #if defined(TARGET_SPARC)
 const float32 float32_default_nan = const_float32(0x7FFFFFFF);
 #elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA) || \
-      defined(TARGET_XTENSA)
+      defined(TARGET_XTENSA) || defined(TARGET_S390X)
 const float32 float32_default_nan = const_float32(0x7FC00000);
 #elif SNAN_BIT_IS_ONE
 const float32 float32_default_nan = const_float32(0x7FBFFFFF);
@@ -126,7 +126,8 @@ const float32 float32_default_nan = const_float32(0xFFC00000);
 *----------------------------------------------------------------------------*/
 #if defined(TARGET_SPARC)
 const float64 float64_default_nan = const_float64(LIT64( 0x7FFFFFFFFFFFFFFF ));
-#elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA)
+#elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA) || \
+      defined(TARGET_S390X)
 const float64 float64_default_nan = const_float64(LIT64( 0x7FF8000000000000 ));
 #elif SNAN_BIT_IS_ONE
 const float64 float64_default_nan = const_float64(LIT64(0x7FF7FFFFFFFFFFFF));
@@ -155,6 +156,9 @@ const floatx80 floatx80_default_nan
 #if SNAN_BIT_IS_ONE
 #define float128_default_nan_high LIT64(0x7FFF7FFFFFFFFFFF)
 #define float128_default_nan_low  LIT64(0xFFFFFFFFFFFFFFFF)
+#elif defined(TARGET_S390X)
+#define float128_default_nan_high LIT64( 0x7FFF800000000000 )
+#define float128_default_nan_low  LIT64( 0x0000000000000000 )
 #else
 #define float128_default_nan_high LIT64( 0xFFFF800000000000 )
 #define float128_default_nan_low  LIT64( 0x0000000000000000 )
-- 
2.1.4

[Qemu-devel] [PATCH 04/10] target-s390x: silence NaNs for LOAD LENGTHENED and LOAD ROUNDED

[Aurelien Jarno]

LOAD LENGTHENED and LOAD ROUNDED are considered as FP operations and
thus need to convert input sNaN into corresponding qNaN.

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/fpu_helper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target-s390x/fpu_helper.c b/target-s390x/fpu_helper.c
index b946ec1..96eabb6 100644
--- a/target-s390x/fpu_helper.c
+++ b/target-s390x/fpu_helper.c
@@ -265,7 +265,7 @@ uint64_t HELPER(ldeb)(CPUS390XState *env, uint64_t f2)
 {
     float64 ret = float32_to_float64(f2, &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return ret;
+    return float64_maybe_silence_nan(ret);
 }
 
 /* convert 128-bit float to 64-bit float */
@@ -273,7 +273,7 @@ uint64_t HELPER(ldxb)(CPUS390XState *env, uint64_t ah, uint64_t al)
 {
     float64 ret = float128_to_float64(make_float128(ah, al), &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return ret;
+    return float64_maybe_silence_nan(ret);
 }
 
 /* convert 64-bit float to 128-bit float */
@@ -281,7 +281,7 @@ uint64_t HELPER(lxdb)(CPUS390XState *env, uint64_t f2)
 {
     float128 ret = float64_to_float128(f2, &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return RET128(ret);
+    return RET128(float128_maybe_silence_nan(ret));
 }
 
 /* convert 32-bit float to 128-bit float */
@@ -289,7 +289,7 @@ uint64_t HELPER(lxeb)(CPUS390XState *env, uint64_t f2)
 {
     float128 ret = float32_to_float128(f2, &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return RET128(ret);
+    return RET128(float128_maybe_silence_nan(ret));
 }
 
 /* convert 64-bit float to 32-bit float */
@@ -297,7 +297,7 @@ uint64_t HELPER(ledb)(CPUS390XState *env, uint64_t f2)
 {
     float32 ret = float64_to_float32(f2, &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return ret;
+    return float32_maybe_silence_nan(ret);
 }
 
 /* convert 128-bit float to 32-bit float */
@@ -305,7 +305,7 @@ uint64_t HELPER(lexb)(CPUS390XState *env, uint64_t ah, uint64_t al)
 {
     float32 ret = float128_to_float32(make_float128(ah, al), &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return ret;
+    return float32_maybe_silence_nan(ret);
 }
 
 /* 32-bit FP compare */
-- 
2.1.4

[Qemu-devel] [PATCH 05/10] target-s390x: detect tininess before rounding for FP operations

[Aurelien Jarno]

The s390x floating point unit detects tininess before rounding, so set
the softfloat fp_status up appropriately.

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/cpu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index d2f9836..7f17823 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -118,6 +118,10 @@ static void s390_cpu_initial_reset(CPUState *s)
 
     env->pfault_token = -1UL;
 
+    /* tininess for underflow is detected before rounding */
+    set_float_detect_tininess(float_tininess_before_rounding,
+                              &env->fpu_status);
+
     /* Reset state inside the kernel that we cannot access yet from QEMU. */
     if (kvm_enabled()) {
         kvm_s390_reset_vcpu(cpu);
@@ -143,6 +147,10 @@ static void s390_cpu_full_reset(CPUState *s)
 
     env->pfault_token = -1UL;
 
+    /* tininess for underflow is detected before rounding */
+    set_float_detect_tininess(float_tininess_before_rounding,
+                              &env->fpu_status);
+
     /* Reset state inside the kernel that we cannot access yet from QEMU. */
     if (kvm_enabled()) {
         kvm_s390_reset_vcpu(cpu);
-- 
2.1.4

[Qemu-devel] [PATCH 06/10] target-s390x: improve facilities list

[Aurelien Jarno]

We currently use an hardcoded value for the STFL instruction. Move that
to a still hardcoded value but computed from bit values. This is more
maintainable and can be reused for the STFLE instruction.

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/cpu.h       | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/translate.c |  4 +--
 2 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 99773e0..8bda2e0 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -165,6 +165,72 @@ typedef struct CPUS390XState {
 #include "cpu-qom.h"
 #include <sysemu/kvm.h>
 
+/* Facilities list */
+static const uint64_t facilities_dw[] = {
+    (1ULL << 63) | /* b 0: z/Architecture new instructions added to ESA/390 */
+    (1ULL << 62) | /* b 1: z/Architecture architectural */
+    (0ULL << 61) | /* b 2: z/Architecture architectural active */
+    (0ULL << 60) | /* b 3: IDTE */
+    (0ULL << 59) | /* b 4: IDTE selective clearing when segtab invalidated */
+    (0ULL << 58) | /* b 5: IDTE selective clearing when regtab invalidated */
+    (0ULL << 57) | /* b 6: ASN-and-LX-reuse facility */
+    (0ULL << 56) | /* b 7: Store-facility-list-extended facility */
+    (0ULL << 55) | /* b 8: Enhanced-DAT facility */
+    (0ULL << 54) | /* b 9: Sense-running-status facility */
+    (0ULL << 53) | /* b10: Conditional-SSKE facility */
+    (0ULL << 52) | /* b11: Configuration-topology facility */
+    (0ULL << 51) | /* b12: IBM internal use */
+    (0ULL << 50) | /* b13: IPTE-Range facility */
+    (0ULL << 49) | /* b14: Nonquiescing key-setting facility */
+    (0ULL << 48) | /* b15: IBM internal use */
+    (0ULL << 47) | /* b16: Extended-translation facility 2 */
+    (0ULL << 46) | /* b17: Message-security assist */
+    (0ULL << 45) | /* b18: Long-displacement facility */
+    (0ULL << 44) | /* b19: High performance long-displacement facility */
+    (0ULL << 43) | /* b20: HFP-multiply-and-add/subtract facility */
+    (0ULL << 42) | /* b21: Extended-immediate facility */
+    (0ULL << 41) | /* b22: Extended-translation facility 3 */
+    (0ULL << 40) | /* b23: HFP-unnormalized-extension facility */
+    (0ULL << 39) | /* b24: ETF2-enhancement facility */
+    (0ULL << 38) | /* b25: Store-clock-fast facility */
+    (0ULL << 37) | /* b26: Parsing-enhancement facility */
+    (0ULL << 36) | /* b27: Move-with-optional-specifications facility */
+    (0ULL << 35) | /* b28: TOD-clock-steering facility */
+    (0ULL << 33) | /* b30: ETF3-enhancement facility */
+    (0ULL << 32) | /* b31: Extract-CPU-time facility */
+    (0ULL << 31) | /* b32: Compare-and-swap-and-store facility */
+    (0ULL << 30) | /* b33: Compare-and-swap-and-store facility 2 */
+    (0ULL << 29) | /* b34: General-instructions-extension facility */
+    (0ULL << 28) | /* b35: Execute-extensions facility  */
+    (0ULL << 27) | /* b36: Enhanced-monitor facility */
+    (0ULL << 26) | /* b37: Floating-point extension facility */
+    (0ULL << 24) | /* b39: IBM internal use */
+    (0ULL << 23) | /* b40: Set-program-parameters facility */
+    (0ULL << 22) | /* b41: Floating-point-support-enhancement facilities */
+    (0ULL << 21) | /* b42: DFP facility */
+    (0ULL << 20) | /* b43: High performance DFP facility */
+    (0ULL << 19) | /* b44: PFPO instruction */
+    (0ULL << 18) | /* b45: Fast-BCR-serialization facility */
+    (0ULL << 17) | /* b46: IBM internal use */
+    (0ULL << 16) | /* b47: CMPSC-enhancement facility */
+    (0ULL << 15) | /* b48: DFP zoned-conversion facility */
+    (0ULL << 14) | /* b49: Execution-hint, load-and-trap facility */
+    (0ULL << 13) | /* b50: Transactional-execution facility */
+    (0ULL << 12) | /* b51: Local-TLB-clearing facility */
+    (0ULL << 11) | /* b52: interlocked-access facility 2 */
+    (0ULL <<  1) | /* b62: IBM internal use */
+    (0ULL <<  0)   /* b63: IBM internal use */
+,
+    (0ULL << 61) | /* b66: Reset-reference-bits-multiple facility */
+    (0ULL << 60) | /* b67: CPU-measurement counter facility */
+    (0ULL << 59) | /* b68: CPU-measurement sampling facility */
+    (0ULL << 54) | /* b73: Transactional-execution facility in zArch */
+    (0ULL << 52) | /* b75: Access-exception-fetch/store-indication facility */
+    (0ULL << 51) | /* b76: Message-security-assist-extension-3 facility */
+    (0ULL << 50) | /* b77: Message-security-assist-extension-4 facility */
+    (0ULL << 49)   /* b78: Enhanced-DAT facility 2 */
+};
+
 /* distinguish between 24 bit and 31 bit addressing */
 #define HIGH_ORDER_BIT 0x80000000
 
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 745195f..542da53 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -3373,10 +3373,8 @@ static ExitStatus op_spt(DisasContext *s, DisasOps *o)
 static ExitStatus op_stfl(DisasContext *s, DisasOps *o)
 {
     TCGv_i64 f, a;
-    /* We really ought to have more complete indication of facilities
-       that we implement.  Address this when STFLE is implemented.  */
     check_privileged(s);
-    f = tcg_const_i64(0xc0000000);
+    f = tcg_const_i64(facilities_dw[0] >> 32);
     a = tcg_const_i64(200);
     tcg_gen_qemu_st32(f, a, get_mem_index(s));
     tcg_temp_free_i64(f);
-- 
2.1.4

[Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Aurelien Jarno]

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/cpu.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 8bda2e0..35bfdec 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -169,7 +169,7 @@ typedef struct CPUS390XState {
 static const uint64_t facilities_dw[] = {
     (1ULL << 63) | /* b 0: z/Architecture new instructions added to ESA/390 */
     (1ULL << 62) | /* b 1: z/Architecture architectural */
-    (0ULL << 61) | /* b 2: z/Architecture architectural active */
+    (1ULL << 61) | /* b 2: z/Architecture architectural active */
     (0ULL << 60) | /* b 3: IDTE */
     (0ULL << 59) | /* b 4: IDTE selective clearing when segtab invalidated */
     (0ULL << 58) | /* b 5: IDTE selective clearing when regtab invalidated */
@@ -188,7 +188,7 @@ static const uint64_t facilities_dw[] = {
     (0ULL << 45) | /* b18: Long-displacement facility */
     (0ULL << 44) | /* b19: High performance long-displacement facility */
     (0ULL << 43) | /* b20: HFP-multiply-and-add/subtract facility */
-    (0ULL << 42) | /* b21: Extended-immediate facility */
+    (1ULL << 42) | /* b21: Extended-immediate facility */
     (0ULL << 41) | /* b22: Extended-translation facility 3 */
     (0ULL << 40) | /* b23: HFP-unnormalized-extension facility */
     (0ULL << 39) | /* b24: ETF2-enhancement facility */
@@ -201,7 +201,7 @@ static const uint64_t facilities_dw[] = {
     (0ULL << 31) | /* b32: Compare-and-swap-and-store facility */
     (0ULL << 30) | /* b33: Compare-and-swap-and-store facility 2 */
     (0ULL << 29) | /* b34: General-instructions-extension facility */
-    (0ULL << 28) | /* b35: Execute-extensions facility  */
+    (1ULL << 28) | /* b35: Execute-extensions facility  */
     (0ULL << 27) | /* b36: Enhanced-monitor facility */
     (0ULL << 26) | /* b37: Floating-point extension facility */
     (0ULL << 24) | /* b39: IBM internal use */
-- 
2.1.4

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Alexander Graf]

On 25.05.15 01:47, Aurelien Jarno wrote:
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Shouldn't this get populated based on the selected -cpu type?


Alex

> ---
>  target-s390x/cpu.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 8bda2e0..35bfdec 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -169,7 +169,7 @@ typedef struct CPUS390XState {
>  static const uint64_t facilities_dw[] = {
>      (1ULL << 63) | /* b 0: z/Architecture new instructions added to ESA/390 */
>      (1ULL << 62) | /* b 1: z/Architecture architectural */
> -    (0ULL << 61) | /* b 2: z/Architecture architectural active */
> +    (1ULL << 61) | /* b 2: z/Architecture architectural active */
>      (0ULL << 60) | /* b 3: IDTE */
>      (0ULL << 59) | /* b 4: IDTE selective clearing when segtab invalidated */
>      (0ULL << 58) | /* b 5: IDTE selective clearing when regtab invalidated */
> @@ -188,7 +188,7 @@ static const uint64_t facilities_dw[] = {
>      (0ULL << 45) | /* b18: Long-displacement facility */
>      (0ULL << 44) | /* b19: High performance long-displacement facility */
>      (0ULL << 43) | /* b20: HFP-multiply-and-add/subtract facility */
> -    (0ULL << 42) | /* b21: Extended-immediate facility */
> +    (1ULL << 42) | /* b21: Extended-immediate facility */
>      (0ULL << 41) | /* b22: Extended-translation facility 3 */
>      (0ULL << 40) | /* b23: HFP-unnormalized-extension facility */
>      (0ULL << 39) | /* b24: ETF2-enhancement facility */
> @@ -201,7 +201,7 @@ static const uint64_t facilities_dw[] = {
>      (0ULL << 31) | /* b32: Compare-and-swap-and-store facility */
>      (0ULL << 30) | /* b33: Compare-and-swap-and-store facility 2 */
>      (0ULL << 29) | /* b34: General-instructions-extension facility */
> -    (0ULL << 28) | /* b35: Execute-extensions facility  */
> +    (1ULL << 28) | /* b35: Execute-extensions facility  */
>      (0ULL << 27) | /* b36: Enhanced-monitor facility */
>      (0ULL << 26) | /* b37: Floating-point extension facility */
>      (0ULL << 24) | /* b39: IBM internal use */
> 

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Aurelien Jarno]

On 2015-05-25 22:39, Alexander Graf wrote:
> 
> 
> On 25.05.15 01:47, Aurelien Jarno wrote:
> > Cc: Alexander Graf <agraf@suse.de>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> 
> Shouldn't this get populated based on the selected -cpu type?

In the long term yes, but given we only implement one CPU type (or
rather none) in TCG mode, we can consider that's already the case.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Alexander Graf]

On 25.05.15 23:02, Aurelien Jarno wrote:
> On 2015-05-25 22:39, Alexander Graf wrote:
>>
>>
>> On 25.05.15 01:47, Aurelien Jarno wrote:
>>> Cc: Alexander Graf <agraf@suse.de>
>>> Cc: Richard Henderson <rth@twiddle.net>
>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>
>> Shouldn't this get populated based on the selected -cpu type?
> 
> In the long term yes, but given we only implement one CPU type (or
> rather none) in TCG mode, we can consider that's already the case.

There are patches coming from IBM to at least add a list of a good
number of s390x cpu types. I'd really like to make use of that and have
actual CPU types selectable.

At least let's move towards that model. So the code in question should
take the facility capabilities from the first cpu object (or the class?)
for example and we bump it to the currently supported feature set in there.


Alex

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Aurelien Jarno]

On 2015-05-25 23:04, Alexander Graf wrote:
> 
> 
> On 25.05.15 23:02, Aurelien Jarno wrote:
> > On 2015-05-25 22:39, Alexander Graf wrote:
> >>
> >>
> >> On 25.05.15 01:47, Aurelien Jarno wrote:
> >>> Cc: Alexander Graf <agraf@suse.de>
> >>> Cc: Richard Henderson <rth@twiddle.net>
> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>
> >> Shouldn't this get populated based on the selected -cpu type?
> > 
> > In the long term yes, but given we only implement one CPU type (or
> > rather none) in TCG mode, we can consider that's already the case.
> 
> There are patches coming from IBM to at least add a list of a good
> number of s390x cpu types. I'd really like to make use of that and have
> actual CPU types selectable.

I guess they are for the KVM mode. Do they provide the corresponding 
facilities list? Probably otherwise that doesn't really differentiate
various CPUs. Please make sure of that when reviewing these patches.

> At least let's move towards that model. So the code in question should
> take the facility capabilities from the first cpu object (or the class?)
> for example and we bump it to the currently supported feature set in there.

Yes, that would work for STFL/STFLE, though we should have a list of
facilities implemented by TCG so we can mask out the non-implemented
facilities. This basically corresponds to the informations provided by
the current patch.

That said that won't work for actually disabling the corresponding
instructions as we don't have a 1 to 1 mapping between the facilities
and the group of instructions. Anyway we don't even check that right
now.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Alexander Graf]

On 25.05.15 23:13, Aurelien Jarno wrote:
> On 2015-05-25 23:04, Alexander Graf wrote:
>>
>>
>> On 25.05.15 23:02, Aurelien Jarno wrote:
>>> On 2015-05-25 22:39, Alexander Graf wrote:
>>>>
>>>>
>>>> On 25.05.15 01:47, Aurelien Jarno wrote:
>>>>> Cc: Alexander Graf <agraf@suse.de>
>>>>> Cc: Richard Henderson <rth@twiddle.net>
>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>>
>>>> Shouldn't this get populated based on the selected -cpu type?
>>>
>>> In the long term yes, but given we only implement one CPU type (or
>>> rather none) in TCG mode, we can consider that's already the case.
>>
>> There are patches coming from IBM to at least add a list of a good
>> number of s390x cpu types. I'd really like to make use of that and have
>> actual CPU types selectable.
> 
> I guess they are for the KVM mode. Do they provide the corresponding 
> facilities list? Probably otherwise that doesn't really differentiate
> various CPUs. Please make sure of that when reviewing these patches.

I could definitely use help on review - it's probably my weakest point ;).

>> At least let's move towards that model. So the code in question should
>> take the facility capabilities from the first cpu object (or the class?)
>> for example and we bump it to the currently supported feature set in there.
> 
> Yes, that would work for STFL/STFLE, though we should have a list of
> facilities implemented by TCG so we can mask out the non-implemented
> facilities. This basically corresponds to the informations provided by
> the current patch.

Ah, so you consider the current list the "these are the features TCG
knows about" list?

> That said that won't work for actually disabling the corresponding
> instructions as we don't have a 1 to 1 mapping between the facilities
> and the group of instructions. Anyway we don't even check that right
> now.

I agree, but the TCG code annotates which facility each opcode belongs
to which means actually limiting it should become trivial. That's really
all I'm asking for - I want to see the light at the end of the tunnel ;).


Alex

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Aurelien Jarno]

On 2015-05-25 23:47, Alexander Graf wrote:
> 
> 
> On 25.05.15 23:13, Aurelien Jarno wrote:
> > On 2015-05-25 23:04, Alexander Graf wrote:
> >>
> >>
> >> On 25.05.15 23:02, Aurelien Jarno wrote:
> >>> On 2015-05-25 22:39, Alexander Graf wrote:
> >>>>
> >>>>
> >>>> On 25.05.15 01:47, Aurelien Jarno wrote:
> >>>>> Cc: Alexander Graf <agraf@suse.de>
> >>>>> Cc: Richard Henderson <rth@twiddle.net>
> >>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>>>
> >>>> Shouldn't this get populated based on the selected -cpu type?
> >>>
> >>> In the long term yes, but given we only implement one CPU type (or
> >>> rather none) in TCG mode, we can consider that's already the case.
> >>
> >> There are patches coming from IBM to at least add a list of a good
> >> number of s390x cpu types. I'd really like to make use of that and have
> >> actual CPU types selectable.
> > 
> > I guess they are for the KVM mode. Do they provide the corresponding 
> > facilities list? Probably otherwise that doesn't really differentiate
> > various CPUs. Please make sure of that when reviewing these patches.
> 
> I could definitely use help on review - it's probably my weakest point ;).
> 
> >> At least let's move towards that model. So the code in question should
> >> take the facility capabilities from the first cpu object (or the class?)
> >> for example and we bump it to the currently supported feature set in there.
> > 
> > Yes, that would work for STFL/STFLE, though we should have a list of
> > facilities implemented by TCG so we can mask out the non-implemented
> > facilities. This basically corresponds to the informations provided by
> > the current patch.
> 
> Ah, so you consider the current list the "these are the features TCG
> knows about" list?
> 
> > That said that won't work for actually disabling the corresponding
> > instructions as we don't have a 1 to 1 mapping between the facilities
> > and the group of instructions. Anyway we don't even check that right
> > now.
> 
> I agree, but the TCG code annotates which facility each opcode belongs
> to which means actually limiting it should become trivial. That's really
> all I'm asking for - I want to see the light at the end of the tunnel ;).

It's trivial doing so for the facilities annotated in TCG. Just that
they don't match one to one with the facilities bits in STFL/STFLE. Some
bits enable multiple facilities and QEMU has also grouped some
facilities together. Also some bits do not actually concern instructions
but rather MMU features. Some other gives additional properties to a
facility: some facilities might be present by disabled, some other might 
have a slow or fast implementation.

We therefore need a conversion function before being able to do that,
and we need to know which format IBM is going to provide in their
patches: list of facilities or STFL/STFLE bits?

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Alexander Graf]

On 26.05.15 08:02, Aurelien Jarno wrote:
> On 2015-05-25 23:47, Alexander Graf wrote:
>>
>>
>> On 25.05.15 23:13, Aurelien Jarno wrote:
>>> On 2015-05-25 23:04, Alexander Graf wrote:
>>>>
>>>>
>>>> On 25.05.15 23:02, Aurelien Jarno wrote:
>>>>> On 2015-05-25 22:39, Alexander Graf wrote:
>>>>>>
>>>>>>
>>>>>> On 25.05.15 01:47, Aurelien Jarno wrote:
>>>>>>> Cc: Alexander Graf <agraf@suse.de>
>>>>>>> Cc: Richard Henderson <rth@twiddle.net>
>>>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>>>>
>>>>>> Shouldn't this get populated based on the selected -cpu type?
>>>>>
>>>>> In the long term yes, but given we only implement one CPU type (or
>>>>> rather none) in TCG mode, we can consider that's already the case.
>>>>
>>>> There are patches coming from IBM to at least add a list of a good
>>>> number of s390x cpu types. I'd really like to make use of that and have
>>>> actual CPU types selectable.
>>>
>>> I guess they are for the KVM mode. Do they provide the corresponding 
>>> facilities list? Probably otherwise that doesn't really differentiate
>>> various CPUs. Please make sure of that when reviewing these patches.
>>
>> I could definitely use help on review - it's probably my weakest point ;).
>>
>>>> At least let's move towards that model. So the code in question should
>>>> take the facility capabilities from the first cpu object (or the class?)
>>>> for example and we bump it to the currently supported feature set in there.
>>>
>>> Yes, that would work for STFL/STFLE, though we should have a list of
>>> facilities implemented by TCG so we can mask out the non-implemented
>>> facilities. This basically corresponds to the informations provided by
>>> the current patch.
>>
>> Ah, so you consider the current list the "these are the features TCG
>> knows about" list?
>>
>>> That said that won't work for actually disabling the corresponding
>>> instructions as we don't have a 1 to 1 mapping between the facilities
>>> and the group of instructions. Anyway we don't even check that right
>>> now.
>>
>> I agree, but the TCG code annotates which facility each opcode belongs
>> to which means actually limiting it should become trivial. That's really
>> all I'm asking for - I want to see the light at the end of the tunnel ;).
> 
> It's trivial doing so for the facilities annotated in TCG. Just that
> they don't match one to one with the facilities bits in STFL/STFLE. Some
> bits enable multiple facilities and QEMU has also grouped some
> facilities together. Also some bits do not actually concern instructions
> but rather MMU features. Some other gives additional properties to a
> facility: some facilities might be present by disabled, some other might 
> have a slow or fast implementation.
> 
> We therefore need a conversion function before being able to do that,
> and we need to know which format IBM is going to provide in their
> patches: list of facilities or STFL/STFLE bits?

Please check out this patch set:

  https://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg03436.html


Alex

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Aurelien Jarno]

On 2015-05-26 10:29, Alexander Graf wrote:
> 
> 
> On 26.05.15 08:02, Aurelien Jarno wrote:
> > On 2015-05-25 23:47, Alexander Graf wrote:
> >>
> >>
> >> On 25.05.15 23:13, Aurelien Jarno wrote:
> >>> On 2015-05-25 23:04, Alexander Graf wrote:
> >>>>
> >>>>
> >>>> On 25.05.15 23:02, Aurelien Jarno wrote:
> >>>>> On 2015-05-25 22:39, Alexander Graf wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 25.05.15 01:47, Aurelien Jarno wrote:
> >>>>>>> Cc: Alexander Graf <agraf@suse.de>
> >>>>>>> Cc: Richard Henderson <rth@twiddle.net>
> >>>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>>>>>
> >>>>>> Shouldn't this get populated based on the selected -cpu type?
> >>>>>
> >>>>> In the long term yes, but given we only implement one CPU type (or
> >>>>> rather none) in TCG mode, we can consider that's already the case.
> >>>>
> >>>> There are patches coming from IBM to at least add a list of a good
> >>>> number of s390x cpu types. I'd really like to make use of that and have
> >>>> actual CPU types selectable.
> >>>
> >>> I guess they are for the KVM mode. Do they provide the corresponding 
> >>> facilities list? Probably otherwise that doesn't really differentiate
> >>> various CPUs. Please make sure of that when reviewing these patches.
> >>
> >> I could definitely use help on review - it's probably my weakest point ;).
> >>
> >>>> At least let's move towards that model. So the code in question should
> >>>> take the facility capabilities from the first cpu object (or the class?)
> >>>> for example and we bump it to the currently supported feature set in there.
> >>>
> >>> Yes, that would work for STFL/STFLE, though we should have a list of
> >>> facilities implemented by TCG so we can mask out the non-implemented
> >>> facilities. This basically corresponds to the informations provided by
> >>> the current patch.
> >>
> >> Ah, so you consider the current list the "these are the features TCG
> >> knows about" list?
> >>
> >>> That said that won't work for actually disabling the corresponding
> >>> instructions as we don't have a 1 to 1 mapping between the facilities
> >>> and the group of instructions. Anyway we don't even check that right
> >>> now.
> >>
> >> I agree, but the TCG code annotates which facility each opcode belongs
> >> to which means actually limiting it should become trivial. That's really
> >> all I'm asking for - I want to see the light at the end of the tunnel ;).
> > 
> > It's trivial doing so for the facilities annotated in TCG. Just that
> > they don't match one to one with the facilities bits in STFL/STFLE. Some
> > bits enable multiple facilities and QEMU has also grouped some
> > facilities together. Also some bits do not actually concern instructions
> > but rather MMU features. Some other gives additional properties to a
> > facility: some facilities might be present by disabled, some other might 
> > have a slow or fast implementation.
> > 
> > We therefore need a conversion function before being able to do that,
> > and we need to know which format IBM is going to provide in their
> > patches: list of facilities or STFL/STFLE bits?
> 
> Please check out this patch set:
> 
>   https://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg03436.html

Thanks, at a first glance it seems we have what we need. I'll try 
to base my STFL/STFLE patches on this patchset in the next days.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 07/10] target-s390x: enable fully implemented facilities

[Alexander Graf]

On 05/26/2015 11:05 AM, Aurelien Jarno wrote:
> On 2015-05-26 10:29, Alexander Graf wrote:
>>
>> On 26.05.15 08:02, Aurelien Jarno wrote:
>>> On 2015-05-25 23:47, Alexander Graf wrote:
>>>>
>>>> On 25.05.15 23:13, Aurelien Jarno wrote:
>>>>> On 2015-05-25 23:04, Alexander Graf wrote:
>>>>>>
>>>>>> On 25.05.15 23:02, Aurelien Jarno wrote:
>>>>>>> On 2015-05-25 22:39, Alexander Graf wrote:
>>>>>>>>
>>>>>>>> On 25.05.15 01:47, Aurelien Jarno wrote:
>>>>>>>>> Cc: Alexander Graf <agraf@suse.de>
>>>>>>>>> Cc: Richard Henderson <rth@twiddle.net>
>>>>>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>>>>>> Shouldn't this get populated based on the selected -cpu type?
>>>>>>> In the long term yes, but given we only implement one CPU type (or
>>>>>>> rather none) in TCG mode, we can consider that's already the case.
>>>>>> There are patches coming from IBM to at least add a list of a good
>>>>>> number of s390x cpu types. I'd really like to make use of that and have
>>>>>> actual CPU types selectable.
>>>>> I guess they are for the KVM mode. Do they provide the corresponding
>>>>> facilities list? Probably otherwise that doesn't really differentiate
>>>>> various CPUs. Please make sure of that when reviewing these patches.
>>>> I could definitely use help on review - it's probably my weakest point ;).
>>>>
>>>>>> At least let's move towards that model. So the code in question should
>>>>>> take the facility capabilities from the first cpu object (or the class?)
>>>>>> for example and we bump it to the currently supported feature set in there.
>>>>> Yes, that would work for STFL/STFLE, though we should have a list of
>>>>> facilities implemented by TCG so we can mask out the non-implemented
>>>>> facilities. This basically corresponds to the informations provided by
>>>>> the current patch.
>>>> Ah, so you consider the current list the "these are the features TCG
>>>> knows about" list?
>>>>
>>>>> That said that won't work for actually disabling the corresponding
>>>>> instructions as we don't have a 1 to 1 mapping between the facilities
>>>>> and the group of instructions. Anyway we don't even check that right
>>>>> now.
>>>> I agree, but the TCG code annotates which facility each opcode belongs
>>>> to which means actually limiting it should become trivial. That's really
>>>> all I'm asking for - I want to see the light at the end of the tunnel ;).
>>> It's trivial doing so for the facilities annotated in TCG. Just that
>>> they don't match one to one with the facilities bits in STFL/STFLE. Some
>>> bits enable multiple facilities and QEMU has also grouped some
>>> facilities together. Also some bits do not actually concern instructions
>>> but rather MMU features. Some other gives additional properties to a
>>> facility: some facilities might be present by disabled, some other might
>>> have a slow or fast implementation.
>>>
>>> We therefore need a conversion function before being able to do that,
>>> and we need to know which format IBM is going to provide in their
>>> patches: list of facilities or STFL/STFLE bits?
>> Please check out this patch set:
>>
>>    https://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg03436.html
> Thanks, at a first glance it seems we have what we need. I'll try
> to base my STFL/STFLE patches on this patchset in the next days.

Awesome, thanks a lot!


Alex

[Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Aurelien Jarno]

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/cpu.h         |  2 +-
 target-s390x/helper.h      |  1 +
 target-s390x/insn-data.def |  2 ++
 target-s390x/misc_helper.c | 19 +++++++++++++++++++
 target-s390x/translate.c   |  7 +++++++
 5 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 35bfdec..3110c1f 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -174,7 +174,7 @@ static const uint64_t facilities_dw[] = {
     (0ULL << 59) | /* b 4: IDTE selective clearing when segtab invalidated */
     (0ULL << 58) | /* b 5: IDTE selective clearing when regtab invalidated */
     (0ULL << 57) | /* b 6: ASN-and-LX-reuse facility */
-    (0ULL << 56) | /* b 7: Store-facility-list-extended facility */
+    (1ULL << 56) | /* b 7: Store-facility-list-extended facility */
     (0ULL << 55) | /* b 8: Enhanced-DAT facility */
     (0ULL << 54) | /* b 9: Sense-running-status facility */
     (0ULL << 53) | /* b10: Conditional-SSKE facility */
diff --git a/target-s390x/helper.h b/target-s390x/helper.h
index e6f2afb..2dc01a0 100644
--- a/target-s390x/helper.h
+++ b/target-s390x/helper.h
@@ -79,6 +79,7 @@ DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
+DEF_HELPER_2(stfle, i32, env, i64)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index 72c3a2e..fd45730 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -827,6 +827,8 @@
 /* STORE CPU TIMER */
     C(0xb209, STPT,    S,     Z,   la2, 0, new, m1_64, stpt, 0)
 /* STORE FACILITY LIST */
+    C(0xb2b0, STFLE,   S,     SFLE, 0, a2, 0, 0, stfle, 0)
+/* STORE FACILITY LIST */
     C(0xb2b1, STFL,    S,     Z,   0, 0, 0, 0, stfl, 0)
 /* STORE PREFIX */
     C(0xb211, STPX,    S,     Z,   la2, 0, new, m1_32, stpx, 0)
diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index b375ab7..711f365 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -76,6 +76,25 @@ void HELPER(exception)(CPUS390XState *env, uint32_t excp)
     cpu_loop_exit(cs);
 }
 
+/* Store faciliy list extended */
+uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t dest)
+{
+    int nf = sizeof(facilities_dw) / sizeof(facilities_dw[0]);
+    int rf = (env->regs[0] & 0xff) + 1;
+    int i;
+
+    for (i = 0; i < MIN(nf, rf); i++) {
+        cpu_stq_data(env, dest, facilities_dw[i]);
+        dest += 8;
+    }
+
+    if (rf > nf) {
+        env->regs[0] = (env->regs[0] & ~0xff) | (nf - 1);
+    }
+
+    return (rf < nf) ? 3 : 0;
+}
+
 #ifndef CONFIG_USER_ONLY
 
 void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 542da53..78b8cdc 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -3382,6 +3382,13 @@ static ExitStatus op_stfl(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_stfle(DisasContext *s, DisasOps *o)
+{
+    gen_helper_stfle(cc_op, cpu_env, o->in2);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_stpt(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-- 
2.1.4

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Richard Henderson]

On 05/24/2015 04:47 PM, Aurelien Jarno wrote:
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Sadly, implementing this breaks current kernels.
I did this about two years ago and havn't figured
out what to do about it.

The silly code in head.S doesn't check for just the
facilities that it needs, it checks for all facilities
that specific processors provide.

If we don't e.g. pretend that we have HFP, despite the
fact that no one uses it, the kernel won't boot.



r~

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Alexander Graf]

On 26.05.15 01:08, Richard Henderson wrote:
> On 05/24/2015 04:47 PM, Aurelien Jarno wrote:
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> 
> Sadly, implementing this breaks current kernels.
> I did this about two years ago and havn't figured
> out what to do about it.
> 
> The silly code in head.S doesn't check for just the
> facilities that it needs, it checks for all facilities
> that specific processors provide.
> 
> If we don't e.g. pretend that we have HFP, despite the
> fact that no one uses it, the kernel won't boot.

It depends on how the kernel is configured too. You can select the
minimum CPU type it can run on. But in most distros it's set to
something way beyond our current emulation scope.

But again, the real solution to this would be to have -cpu available and
have TCG available feature masks possible to get outed-out.


Alex

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Aurelien Jarno]

On 2015-05-25 16:08, Richard Henderson wrote:
> On 05/24/2015 04:47 PM, Aurelien Jarno wrote:
> >Cc: Alexander Graf <agraf@suse.de>
> >Cc: Richard Henderson <rth@twiddle.net>
> >Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> 
> Sadly, implementing this breaks current kernels.
> I did this about two years ago and havn't figured
> out what to do about it.
> 
> The silly code in head.S doesn't check for just the
> facilities that it needs, it checks for all facilities
> that specific processors provide.
> 
> If we don't e.g. pretend that we have HFP, despite the
> fact that no one uses it, the kernel won't boot.

How does it breaks? This patch only enable bits corresponding to 
facility that we fully implement (that's a reason why the patchset
doesn't enable LD facility for example). So it should not be a problem,
at least I have been able to boot kernels successfully.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Richard Henderson]

On 05/25/2015 11:03 PM, Aurelien Jarno wrote:
> On 2015-05-25 16:08, Richard Henderson wrote:
>> On 05/24/2015 04:47 PM, Aurelien Jarno wrote:
>>> Cc: Alexander Graf <agraf@suse.de>
>>> Cc: Richard Henderson <rth@twiddle.net>
>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>
>> Sadly, implementing this breaks current kernels.
>> I did this about two years ago and havn't figured
>> out what to do about it.
>>
>> The silly code in head.S doesn't check for just the
>> facilities that it needs, it checks for all facilities
>> that specific processors provide.
>>
>> If we don't e.g. pretend that we have HFP, despite the
>> fact that no one uses it, the kernel won't boot.
> 
> How does it breaks? This patch only enable bits corresponding to 
> facility that we fully implement (that's a reason why the patchset
> doesn't enable LD facility for example). So it should not be a problem,
> at least I have been able to boot kernels successfully.
> 

Unless the facilities provided are a strict superset of those required by the
kernel, it aborts the boot.  Extremely early in the process.  As Alex said,
it's possible to compile a kernel without this, but none of the vendor-built
kernels do so.  Which raises the bar to what a user needs to do to install.

See linux/arch/s390/kernel/head.S,

# List of facilities that are required. If not all facilities are present
# the kernel will crash. Format is number of facility words with bits set,
# followed by the facility words.

#if defined(CONFIG_MARCH_Z13)
        .long 3, 0xc100eff2, 0xf46ce800, 0x00400000
#elif defined(CONFIG_MARCH_ZEC12)
        .long 3, 0xc100eff2, 0xf46ce800, 0x00400000
#elif defined(CONFIG_MARCH_Z196)
        .long 2, 0xc100eff2, 0xf46c0000
#elif defined(CONFIG_MARCH_Z10)
        .long 2, 0xc100eff2, 0xf0680000
#elif defined(CONFIG_MARCH_Z9_109)
        .long 1, 0xc100efc2
#elif defined(CONFIG_MARCH_Z990)
        .long 1, 0xc0002000
#elif defined(CONFIG_MARCH_Z900)
        .long 1, 0xc0000000
#endif


r~

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Aurelien Jarno]

On 2015-05-26 09:00, Richard Henderson wrote:
> On 05/25/2015 11:03 PM, Aurelien Jarno wrote:
> > On 2015-05-25 16:08, Richard Henderson wrote:
> >> On 05/24/2015 04:47 PM, Aurelien Jarno wrote:
> >>> Cc: Alexander Graf <agraf@suse.de>
> >>> Cc: Richard Henderson <rth@twiddle.net>
> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>
> >> Sadly, implementing this breaks current kernels.
> >> I did this about two years ago and havn't figured
> >> out what to do about it.
> >>
> >> The silly code in head.S doesn't check for just the
> >> facilities that it needs, it checks for all facilities
> >> that specific processors provide.
> >>
> >> If we don't e.g. pretend that we have HFP, despite the
> >> fact that no one uses it, the kernel won't boot.
> > 
> > How does it breaks? This patch only enable bits corresponding to 
> > facility that we fully implement (that's a reason why the patchset
> > doesn't enable LD facility for example). So it should not be a problem,
> > at least I have been able to boot kernels successfully.
> > 
> 
> Unless the facilities provided are a strict superset of those required by the
> kernel, it aborts the boot.  Extremely early in the process.  As Alex said,
> it's possible to compile a kernel without this, but none of the vendor-built
> kernels do so.  Which raises the bar to what a user needs to do to install.
> 
> See linux/arch/s390/kernel/head.S,

Ok, I understand now. That said I don't see how implementing STFLE will
break that. I think it will actually improve things by enabling more
facilities and thus making the kernel happier (but maybe not enough).

> # List of facilities that are required. If not all facilities are present
> # the kernel will crash. Format is number of facility words with bits set,
> # followed by the facility words.
> 
> #if defined(CONFIG_MARCH_Z13)
>         .long 3, 0xc100eff2, 0xf46ce800, 0x00400000
> #elif defined(CONFIG_MARCH_ZEC12)
>         .long 3, 0xc100eff2, 0xf46ce800, 0x00400000
> #elif defined(CONFIG_MARCH_Z196)
>         .long 2, 0xc100eff2, 0xf46c0000
> #elif defined(CONFIG_MARCH_Z10)
>         .long 2, 0xc100eff2, 0xf0680000
> #elif defined(CONFIG_MARCH_Z9_109)
>         .long 1, 0xc100efc2

This one looks a bit complicated to reach. Basically we need to
implement STFLE, MSA, HFP and ETF2, 3 and 3e. But I guess that's
something doable on the medium term

> #elif defined(CONFIG_MARCH_Z990)
>         .long 1, 0xc0002000

For this one we only miss one instruction in the LD facility. I have it
on my TODO list, though it might take a few weeks until it goes to the
top.

> #elif defined(CONFIG_MARCH_Z900)
>         .long 1, 0xc0000000

This corresponds to the current value we provide. CONFIG_MARCH_Z900 also
correspond to the configuration of the Debian kernel, that's why I am
able to boot kernels.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Richard Henderson]

On 05/26/2015 03:03 PM, Aurelien Jarno wrote:
> Ok, I understand now. That said I don't see how implementing STFLE will
> break that. I think it will actually improve things by enabling more
> facilities and thus making the kernel happier (but maybe not enough).

Somewhat amusingly, by not implementing STFLE we bypass this check.


>> #elif defined(CONFIG_MARCH_Z990)
>>         .long 1, 0xc0002000
> 
> For this one we only miss one instruction in the LD facility. I have it
> on my TODO list, though it might take a few weeks until it goes to the
> top.
> 

Which one?

>> #elif defined(CONFIG_MARCH_Z900)
>>         .long 1, 0xc0000000
> 
> This corresponds to the current value we provide. CONFIG_MARCH_Z900 also
> correspond to the configuration of the Debian kernel, that's why I am
> able to boot kernels.

Ah, well that's something.  Fedora defaults to z9-109, and I think SuSE does
the same.


r~

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Alexander Graf]

> Am 27.05.2015 um 16:31 schrieb Richard Henderson <rth@twiddle.net>:
> 
>> On 05/26/2015 03:03 PM, Aurelien Jarno wrote:
>> Ok, I understand now. That said I don't see how implementing STFLE will
>> break that. I think it will actually improve things by enabling more
>> facilities and thus making the kernel happier (but maybe not enough).
> 
> Somewhat amusingly, by not implementing STFLE we bypass this check.
> 
> 
>>> #elif defined(CONFIG_MARCH_Z990)
>>>        .long 1, 0xc0002000
>> 
>> For this one we only miss one instruction in the LD facility. I have it
>> on my TODO list, though it might take a few weeks until it goes to the
>> top.
> 
> Which one?
> 
>>> #elif defined(CONFIG_MARCH_Z900)
>>>        .long 1, 0xc0000000
>> 
>> This corresponds to the current value we provide. CONFIG_MARCH_Z900 also
>> correspond to the configuration of the Debian kernel, that's why I am
>> able to boot kernels.
> 
> Ah, well that's something.  Fedora defaults to z9-109, and I think SuSE does
> the same.

Unfortunately SLE12 has its ALS on z196, so there's quite a bit more work necessary ;)

Alex

> 
> 
> r~

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Aurelien Jarno]

On 2015-05-27 07:31, Richard Henderson wrote:
> On 05/26/2015 03:03 PM, Aurelien Jarno wrote:
> > Ok, I understand now. That said I don't see how implementing STFLE will
> > break that. I think it will actually improve things by enabling more
> > facilities and thus making the kernel happier (but maybe not enough).
> 
> Somewhat amusingly, by not implementing STFLE we bypass this check.

Oh, I see the whole picture now.

> >> #elif defined(CONFIG_MARCH_Z990)
> >>         .long 1, 0xc0002000
> > 
> > For this one we only miss one instruction in the LD facility. I have it
> > on my TODO list, though it might take a few weeks until it goes to the
> > top.
> > 
> 
> Which one?

CVBY, but anyway we don't implement CVB and CVBG either...


> >> #elif defined(CONFIG_MARCH_Z900)
> >>         .long 1, 0xc0000000
> > 
> > This corresponds to the current value we provide. CONFIG_MARCH_Z900 also
> > correspond to the configuration of the Debian kernel, that's why I am
> > able to boot kernels.
> 
> Ah, well that's something.  Fedora defaults to z9-109, and I think SuSE does
> the same.

One strategy would be to enable the bit in STFLE whether the feature is
fully implemented by TCG or not, using the values provided by the CPU
model (IBM patches). After all we have plenty of non-implemented basic
instructions (e.g. all the HFP ones). The risk is that the userland
starts to use some optimized libraries due to that. On the other hand
if the kernel is compiled with this option, chances are that the
userland is built with the same instruction set.

Having a quick look at big sets of missing instructions, it looks like
we are mostly missing HFP (most are in the basic instructions), DFP,
high-word, extended translations, and message-security-assist. high-word
facility should be relatively easy to add, extended translation a bit
more complex. We might want to use libdecnumber like on PPC fro DFP. It
looks like the most problematic are therefore HFP (but that's already
the case anyway) and message-security-assist.

Then there are plenty of facilities with only a few instructions
missing. They might be tricky to implement though (i.e. transactional
memory).

One other strategy would be to create a "any" CPU for linux-user and
also offer it to the softmmu mode.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 08/10] target-s390x: implement STFLE instruction

[Alexander Graf]

> Am 27.05.2015 um 17:57 schrieb Aurelien Jarno <aurelien@aurel32.net>:
> 
>> On 2015-05-27 07:31, Richard Henderson wrote:
>>> On 05/26/2015 03:03 PM, Aurelien Jarno wrote:
>>> Ok, I understand now. That said I don't see how implementing STFLE will
>>> break that. I think it will actually improve things by enabling more
>>> facilities and thus making the kernel happier (but maybe not enough).
>> 
>> Somewhat amusingly, by not implementing STFLE we bypass this check.
> 
> Oh, I see the whole picture now.
> 
>>>> #elif defined(CONFIG_MARCH_Z990)
>>>>        .long 1, 0xc0002000
>>> 
>>> For this one we only miss one instruction in the LD facility. I have it
>>> on my TODO list, though it might take a few weeks until it goes to the
>>> top.
>> 
>> Which one?
> 
> CVBY, but anyway we don't implement CVB and CVBG either...
> 
> 
>>>> #elif defined(CONFIG_MARCH_Z900)
>>>>        .long 1, 0xc0000000
>>> 
>>> This corresponds to the current value we provide. CONFIG_MARCH_Z900 also
>>> correspond to the configuration of the Debian kernel, that's why I am
>>> able to boot kernels.
>> 
>> Ah, well that's something.  Fedora defaults to z9-109, and I think SuSE does
>> the same.
> 
> One strategy would be to enable the bit in STFLE whether the feature is
> fully implemented by TCG or not, using the values provided by the CPU
> model (IBM patches).

So how about we add a "force" parameter to the -cpu command line option to suppress masking of the facility bits with what tcg implements?

> After all we have plenty of non-implemented basic
> instructions (e.g. all the HFP ones). The risk is that the userland
> starts to use some optimized libraries due to that. On the other hand
> if the kernel is compiled with this option, chances are that the
> userland is built with the same instruction set.
> 
> Having a quick look at big sets of missing instructions, it looks like
> we are mostly missing HFP (most are in the basic instructions), DFP,
> high-word, extended translations, and message-security-assist. high-word
> facility should be relatively easy to add, extended translation a bit
> more complex. We might want to use libdecnumber like on PPC fro DFP. It
> looks like the most problematic are therefore HFP (but that's already
> the case anyway) and message-security-assist.
> 
> Then there are plenty of facilities with only a few instructions
> missing. They might be tricky to implement though (i.e. transactional
> memory).

On ppc we just fail every transaction which seems to do the trick. Can we do the same for s390?

> 
> One other strategy would be to create a "any" CPU for linux-user and
> also offer it to the softmmu mode.

I think for softmmu we should stick with real world models, either masked with tcg's implemented facilities or not.

In fact, how about instead of masking, we just make -cpu fail on creation if it wants a facility that tcg doesn't implement yet? Then we can hint the user to -cpu ec12,force to make it work nevertheless


Alex

> 
> -- 
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH 09/10] target-s390x: move a few instructions to the correct facility

[Aurelien Jarno]

LY is part of the long-displacement facility.
RISBHG and RISBLG are part of the high-word facility.
STCMH is part of the z/Architecture.

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/insn-data.def | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index fd45730..d57ce32 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -336,7 +336,7 @@
 /* LOAD */
     C(0x1800, LR,      RR_a,  Z,   0, r2_o, 0, cond_r1r2_32, mov2, 0)
     C(0x5800, L,       RX_a,  Z,   0, a2, new, r1_32, ld32s, 0)
-    C(0xe358, LY,      RXY_a, Z,   0, a2, new, r1_32, ld32s, 0)
+    C(0xe358, LY,      RXY_a, LD,  0, a2, new, r1_32, ld32s, 0)
     C(0xb904, LGR,     RRE,   Z,   0, r2_o, 0, r1, mov2, 0)
     C(0xb914, LGFR,    RRE,   Z,   0, r2_32s, 0, r1, mov2, 0)
     C(0xe304, LG,      RXY_a, Z,   0, a2, r1, 0, ld64, 0)
@@ -595,8 +595,8 @@
 
 /* ROTATE THEN INSERT SELECTED BITS */
     C(0xec55, RISBG,   RIE_f, GIE, 0, r2, r1, 0, risbg, s64)
-    C(0xec5d, RISBHG,  RIE_f, GIE, 0, r2, r1, 0, risbg, 0)
-    C(0xec51, RISBLG,  RIE_f, GIE, 0, r2, r1, 0, risbg, 0)
+    C(0xec5d, RISBHG,  RIE_f, HW,  0, r2, r1, 0, risbg, 0)
+    C(0xec51, RISBLG,  RIE_f, HW,  0, r2, r1, 0, risbg, 0)
 /* ROTATE_THEN <OP> SELECTED BITS */
     C(0xec54, RNSBG,   RIE_f, GIE, 0, r2, r1, 0, rosbg, 0)
     C(0xec56, ROSBG,   RIE_f, GIE, 0, r2, r1, 0, rosbg, 0)
@@ -670,7 +670,7 @@
 /* STORE CHARACTERS UNDER MASK */
     D(0xbe00, STCM,    RS_b,  Z,   r1_o, a2, 0, 0, stcm, 0, 0)
     D(0xeb2d, STCMY,   RSY_b, LD,  r1_o, a2, 0, 0, stcm, 0, 0)
-    D(0xeb2c, STCMH,   RSY_b, LD,  r1_o, a2, 0, 0, stcm, 0, 32)
+    D(0xeb2c, STCMH,   RSY_b, Z,   r1_o, a2, 0, 0, stcm, 0, 32)
 /* STORE HALFWORD */
     C(0x4000, STH,     RX_a,  Z,   r1_o, a2, 0, 0, st16, 0)
     C(0xe370, STHY,    RXY_a, LD,  r1_o, a2, 0, 0, st16, 0)
-- 
2.1.4

[Qemu-devel] [PATCH 10/10] target-s390x: implement LAY and LAEY instructions

[Aurelien Jarno]

This complete the general-instructions-extension facility, enable it.

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-s390x/cpu.h         |  2 +-
 target-s390x/insn-data.def |  3 +++
 target-s390x/translate.c   | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 3110c1f..43a1c70 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -200,7 +200,7 @@ static const uint64_t facilities_dw[] = {
     (0ULL << 32) | /* b31: Extract-CPU-time facility */
     (0ULL << 31) | /* b32: Compare-and-swap-and-store facility */
     (0ULL << 30) | /* b33: Compare-and-swap-and-store facility 2 */
-    (0ULL << 29) | /* b34: General-instructions-extension facility */
+    (1ULL << 29) | /* b34: General-instructions-extension facility */
     (1ULL << 28) | /* b35: Execute-extensions facility  */
     (0ULL << 27) | /* b36: Enhanced-monitor facility */
     (0ULL << 26) | /* b37: Floating-point extension facility */
diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index d57ce32..c44838d 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -357,6 +357,9 @@
 /* LOAD ADDRESS */
     C(0x4100, LA,      RX_a,  Z,   0, a2, 0, r1, mov2, 0)
     C(0xe371, LAY,     RXY_a, LD,  0, a2, 0, r1, mov2, 0)
+/* LOAD ADDRESS EXTENDED */
+    C(0x5100, LAE,     RX_a,  Z,   0, a2, 0, r1, mov2e, 0)
+    C(0xe375, LAEY,    RXY_a, GIE, 0, a2, 0, r1, mov2e, 0)
 /* LOAD ADDRESS RELATIVE LONG */
     C(0xc000, LARL,    RIL_b, Z,   0, ri2, 0, r1, mov2, 0)
 /* LOAD AND ADD */
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 78b8cdc..da7cc42 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2566,6 +2566,41 @@ static ExitStatus op_mov2(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mov2e(DisasContext *s, DisasOps *o)
+{
+    int b2 = get_field(s->fields, b2);
+    TCGv ar1 = tcg_temp_new_i64();
+
+    o->out = o->in2;
+    o->g_out = o->g_in2;
+    TCGV_UNUSED_I64(o->in2);
+    o->g_in2 = false;
+
+    switch (s->tb->flags & FLAG_MASK_ASC) {
+    case PSW_ASC_PRIMARY >> 32:
+        tcg_gen_movi_i64(ar1, 0);
+        break;
+    case PSW_ASC_ACCREG >> 32:
+        tcg_gen_movi_i64(ar1, 1);
+        break;
+    case PSW_ASC_SECONDARY >> 32:
+        if (b2) {
+            tcg_gen_ld32u_i64(ar1, cpu_env, offsetof(CPUS390XState, aregs[b2]));
+        } else {
+            tcg_gen_movi_i64(ar1, 0);
+        }
+        break;
+    case PSW_ASC_HOME >> 32:
+        tcg_gen_movi_i64(ar1, 2);
+        break;
+    }
+
+    tcg_gen_st32_i64(ar1, cpu_env, offsetof(CPUS390XState, aregs[1]));
+    tcg_temp_free_i64(ar1);
+
+    return NO_EXIT;
+}
+
 static ExitStatus op_movx(DisasContext *s, DisasOps *o)
 {
     o->out = o->in1;
-- 
2.1.4

Re: [Qemu-devel] [PATCH 00/10] target-s390x: TCG fixes and improvements

[Alexander Graf]

On 25.05.15 01:47, Aurelien Jarno wrote:
> This patchset fixes a few issues with the s390x emulation and improves
> it a bit by a emulating a few more instructions.
> 
> With this patchset and the ones posted a few days ago, I have been able
> to build the GNU libc in both a 64-bit guest with 64-bit userland and a
> 64-bit guest with a 31-bit userland and pass the testsuite in both
> cases.

Thanks, applied 01-05, 09-10 to s390-next. I had to drop the facility
bit set hunk in patch 10/10.


Alex