Skip to contentSkip to conversation

s390x/cpumodel: wire up cpu type + id for TCG

PATCH In review David Hildenbrand 2 participants 5 messages Jun 9, 2017

lore
Share & tools
lore

David Hildenbrand

author
#0 lore raw

v2 cover s390x/cpumodel: wire up cpu type + id for TCG

Once Thomas' interception handler kvm-unit-test is only, I'll add my stidp test.

I played with adding a "wout_m1_64_align", but I am not sure if that is really helpful has we will only have a handful of users.

I am working on some more alignment fixes (spx, stap and friends ) and will send them out soon.

Low address protection checks are missing as for most other instructions as well.

v1 -> v2:
- Don't generate a helper
- Use MO_ALIGN to check alignment

David Hildenbrand (2):
target/s390x: specification exceptions are suppressing s390x/cpumodel: wire up cpu type + id for TCG

 target/s390x/cpu.h         | 2 +-
 target/s390x/cpu_models.c  | 8 ++++++--
 target/s390x/helper.c      | 2 +-
 target/s390x/insn-data.def | 2 +-
 target/s390x/misc_helper.c | 9 ++++++---
 target/s390x/translate.c   | 9 ++-------
 6 files changed, 17 insertions(+), 15 deletions(-)
Signature · 1 line
2.9.3
author
#1 lore raw

PATCH v2 1/2 target/s390x: specification exceptions are suppressing

The PSW in the old PSW has to point at the next instruction, as specification exceptions are defined to be "suppressing or completed", but not nullified.

Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
target/s390x/helper.c +1 -1
index a8d20c5..42a2465 100644
@@ -737,6 +737,6 @@ void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     if (retaddr) {
         cpu_restore_state(cs, retaddr);
     }
-    program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER);
+    program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER_INC);
 }
 #endif /* CONFIG_USER_ONLY */
Signature · 1 line
2.9.3
#2 lore raw

Re: [Qemu-devel] [PATCH v2 1/2] target/s390x: specification exceptions are suppressing

Quoted 14 lines · David Hildenbrand

The PSW in the old PSW has to point at the next instruction, as specification exceptions are defined to be "suppressing or completed", but not nullified.

Signed-off-by David Hildenbrand <[email protected]>
---
  target/s390x/helper.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
target/s390x/helper.c +0 -0
index a8d20c5..42a2465 100644

target/s390x/helper.c void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr, · line 740

Quoted 5 lines
target/s390x/helper.c +1 -1
@@ -737,6 +737,6 @@ void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
      if (retaddr) {
          cpu_restore_state(cs, retaddr);
      }
-    program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER);
+    program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER_INC);

I folded this into your following patch to use ILEN_AUTO.

r~

author
#3 lore raw

PATCH v2 2/2 s390x/cpumodel: wire up cpu type + id for TCG

In reply to David Hildenbrand

Let's properly expose the CPU type (machine-type number) via "STORE CPU ID" and "STORE SUBSYSTEM INFORMATION".

As TCG emulates basic mode, the CPU identification number has the format "Annnnn", whereby A is the CPU address, and n are parts of the CPU serial number (0 for us for now).

A specification exception will be injected if the address is not aligned to a double word. Low address protection will not be checked as we're missing some more general support for that.

Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.h         | 2 +-
 target/s390x/cpu_models.c  | 8 ++++++--
 target/s390x/insn-data.def | 2 +-
 target/s390x/misc_helper.c | 9 ++++++---
 target/s390x/translate.c   | 9 ++-------
 5 files changed, 16 insertions(+), 14 deletions(-)
target/s390x/cpu.h +1 -1
index a4d31df..502d3d7 100644
@@ -149,7 +149,7 @@ typedef struct CPUS390XState {
     CPU_COMMON
 
     uint32_t cpu_num;
-    uint32_t machine_type;
+    uint64_t cpuid;
 
     uint64_t tod_offset;
     uint64_t tod_basetime;
target/s390x/cpu_models.c +6 -2
index b34318f..c508ae1 100644
@@ -737,8 +737,6 @@ static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
 
     if (kvm_enabled()) {
         kvm_s390_apply_cpu_model(model, errp);
-    } else if (model) {
-        /* FIXME TCG - use data for stdip/stfl */
     }
 
     if (!*errp) {

@@ -786,6 +784,12 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
     }
 
     apply_cpu_model(cpu->model, errp);
+
+    cpu->env.cpuid = s390_cpuid_from_cpu_model(cpu->model);
+    if (tcg_enabled()) {
+        /* basic mode, write the cpu address into the first 4 bit of the ID */
+        cpu->env.cpuid |= ((uint64_t)cpu->env.cpu_num & 0xf) << 54;
+    }
 }
 
 static void get_feature(Object *obj, Visitor *v, const char *name,
target/s390x/insn-data.def +1 -1
index 73dd05d..d089707 100644
@@ -960,7 +960,7 @@
 /* STORE CPU ADDRESS */
     C(0xb212, STAP,    S,     Z,   la2, 0, new, m1_16, stap, 0)
 /* STORE CPU ID */
-    C(0xb202, STIDP,   S,     Z,   la2, 0, new, m1_64, stidp, 0)
+    C(0xb202, STIDP,   S,     Z,   la2, 0, new, 0, stidp, 0)
 /* STORE CPU TIMER */
     C(0xb209, STPT,    S,     Z,   la2, 0, new, m1_64, stpt, 0)
 /* STORE FACILITY LIST */
target/s390x/misc_helper.c +6 -3
index edcdf17..75fd13e 100644
@@ -383,6 +383,7 @@ uint64_t HELPER(stpt)(CPUS390XState *env)
 uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
                       uint64_t r0, uint64_t r1)
 {
+    S390CPU *cpu = s390_env_get_cpu(env);
     int cc = 0;
     int sel1, sel2;
 

@@ -402,12 +403,14 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
         if ((sel1 == 1) && (sel2 == 1)) {
             /* Basic Machine Configuration */
             struct sysib_111 sysib;
+            char type[5] = {};
 
             memset(&sysib, 0, sizeof(sysib));
             ebcdic_put(sysib.manuf, "QEMU            ", 16);
-            /* same as machine type number in STORE CPU ID */
-            ebcdic_put(sysib.type, "QEMU", 4);
-            /* same as model number in STORE CPU ID */
+            /* same as machine type number in STORE CPU ID, but in EBCDIC */
+            snprintf(type, ARRAY_SIZE(type), "%X", cpu->model->def->type);
+            ebcdic_put(sysib.type, type, 4);
+            /* model number (not stored in STORE CPU ID for z/Architecure) */
             ebcdic_put(sysib.model, "QEMU            ", 16);
             ebcdic_put(sysib.sequence, "QEMU            ", 16);
             ebcdic_put(sysib.plant, "QEMU", 4);
target/s390x/translate.c +2 -7
index 95f91d4..2f182cc 100644
@@ -3877,14 +3877,9 @@ static ExitStatus op_stctl(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_stidp(DisasContext *s, DisasOps *o)
 {
-    TCGv_i64 t1 = tcg_temp_new_i64();
-
     check_privileged(s);
-    tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, cpu_num));
-    tcg_gen_ld32u_i64(t1, cpu_env, offsetof(CPUS390XState, machine_type));
-    tcg_gen_deposit_i64(o->out, o->out, t1, 32, 32);
-    tcg_temp_free_i64(t1);
-
+    tcg_gen_ld_i64(o->out, cpu_env, offsetof(CPUS390XState, cpuid));
+    tcg_gen_qemu_st_i64(o->out, o->addr1, get_mem_index(s), MO_TEQ | MO_ALIGN);
     return NO_EXIT;
 }
Signature · 1 line
2.9.3
#4 lore raw
Quoted 47 lines · David Hildenbrand

Let's properly expose the CPU type (machine-type number) via "STORE CPU ID" and "STORE SUBSYSTEM INFORMATION".

As TCG emulates basic mode, the CPU identification number has the format "Annnnn", whereby A is the CPU address, and n are parts of the CPU serial number (0 for us for now).

A specification exception will be injected if the address is not aligned to a double word. Low address protection will not be checked as we're missing some more general support for that.

Signed-off-by David Hildenbrand <[email protected]>
---
  target/s390x/cpu.h         | 2 +-
  target/s390x/cpu_models.c  | 8 ++++++--
  target/s390x/insn-data.def | 2 +-
  target/s390x/misc_helper.c | 9 ++++++---
  target/s390x/translate.c   | 9 ++-------
  5 files changed, 16 insertions(+), 14 deletions(-)
target/s390x/cpu.h +1 -1
index a4d31df..502d3d7 100644
@@ -149,7 +149,7 @@ typedef struct CPUS390XState {
      CPU_COMMON
 
      uint32_t cpu_num;
-    uint32_t machine_type;
+    uint64_t cpuid;
 
      uint64_t tod_offset;
      uint64_t tod_basetime;
target/s390x/cpu_models.c +0 -2
index b34318f..c508ae1 100644
@@ -737,8 +737,6 @@ static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
 
      if (kvm_enabled()) {
          kvm_s390_apply_cpu_model(model, errp);
-    } else if (model) {
-        /* FIXME TCG - use data for stdip/stfl */
      }
 
      if (!*errp) {

target/s390x/cpu_models.c void s390_realize_cpu_model(CPUState *cs, Error **errp) · line 791

Quoted 8 lines
target/s390x/cpu_models.c +5 -0
@@ -786,6 +784,12 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
      }
 
      apply_cpu_model(cpu->model, errp);
+
+    cpu->env.cpuid = s390_cpuid_from_cpu_model(cpu->model);
+    if (tcg_enabled()) {
+        /* basic mode, write the cpu address into the first 4 bit of the ID */
+        cpu->env.cpuid |= ((uint64_t)cpu->env.cpu_num & 0xf) << 54;

I changed this to deposit64 and applied the patch to my tree.

r~