Skip to contentSkip to conversation

s390x/tcg: CCW hotplug support

PATCH Superseded David Hildenbrand 4 participants 14 messages Dec 7, 2017 – Dec 8, 2017

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

David Hildenbrand

author
#0 lore raw

v2 cover s390x/tcg: CCW hotplug support

Hotplugging a ccw device currently fails due to different reasons.

1. The stored machine check information is partially wrong.
2. The TOD programmable field cannot be restored.
3. STCRW cannot get executed.

With these patches, I am able to sucessfully hotplug e.g. virtio-rng by issuing "device_add virtio-rng-ccw,id=rng0" to a Linux guest. Unplug also seems to work.

v1 -> v2:
- avoid accessing unmapped lowcore
- minor style issues, more/better comments and description fixes

David Hildenbrand (5):
s390x/kvm: factor out build_channel_report_mcic() into cpu.h s390x/tcg: fix and cleanup mcck injection s390x/tcg: implement SET CLOCK PROGRAMMABLE FIELD s390x/tcg: indicate value of TODPR in STCKE s390x/tcg: wire up STORE CHANNEL REPORT WORD

 target/s390x/cpu.h         | 22 ++++++++++++++++++++++
 target/s390x/excp_helper.c | 12 ++++++------
 target/s390x/helper.h      |  2 ++
 target/s390x/insn-data.def |  3 +++
 target/s390x/internal.h    |  6 +++---
 target/s390x/kvm.c         | 25 ++-----------------------
 target/s390x/misc_helper.c | 20 ++++++++++++++++++++
 target/s390x/translate.c   | 20 ++++++++++++++++++++
 8 files changed, 78 insertions(+), 32 deletions(-)
Signature · 1 line
2.14.3
author
#1 lore raw

PATCH v2 1/5 s390x/kvm: factor out build_channel_report_mcic() into cpu.h

We'll need it later on in two places. Refactor it to just indicate the validity bits. While at it, introduce a define for the used CR14 bit (we'll also need later on).

Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.h | 22 ++++++++++++++++++++++
 target/s390x/kvm.c | 25 ++-----------------------
 2 files changed, 24 insertions(+), 23 deletions(-)
target/s390x/cpu.h +22 -0
index 9cfbbbac04..f29f4ccd8b 100644
@@ -351,6 +351,9 @@ extern const struct VMStateDescription vmstate_s390_cpu;
 #define CR0_CPU_TIMER_SC        0x0000000000000400ULL
 #define CR0_SERVICE_SC          0x0000000000000200ULL
 
+/* Control register 14 bits */
+#define CR14_CHANNEL_REPORT_SC  0x0000000010000000ULL
+
 /* MMU */
 #define MMU_PRIMARY_IDX         0
 #define MMU_SECONDARY_IDX       1

@@ -674,6 +677,25 @@ struct sysib_322 {
 #define MCIC_VB_CT 0x0000000000020000ULL
 #define MCIC_VB_CC 0x0000000000010000ULL
 
+static inline uint64_t s390_build_validity_mcic(void)
+{
+    uint64_t mcic;
+
+    /* Indicate all validity bits (no damage) only. Other bits have to be
+     * added by the caller. (storage errors, subclasses and subclass modifiers)
+     */
+    mcic = MCIC_VB_WP | MCIC_VB_MS | MCIC_VB_PM | MCIC_VB_IA | MCIC_VB_FP |
+           MCIC_VB_GR | MCIC_VB_CR | MCIC_VB_ST | MCIC_VB_AR | MCIC_VB_PR |
+           MCIC_VB_FC | MCIC_VB_CT | MCIC_VB_CC;
+    if (s390_has_feat(S390_FEAT_VECTOR)) {
+        mcic |= MCIC_VB_VR;
+    }
+    if (s390_has_feat(S390_FEAT_GUARDED_STORAGE)) {
+        mcic |= MCIC_VB_GS;
+    }
+    return mcic;
+}
+
 
 /* cpu.c */
 int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
target/s390x/kvm.c +2 -23
index 97c45d5537..9b8b59f2a2 100644
@@ -1852,33 +1852,12 @@ void kvm_s390_io_interrupt(uint16_t subchannel_id,
     kvm_s390_floating_interrupt(&irq);
 }
 
-static uint64_t build_channel_report_mcic(void)
-{
-    uint64_t mcic;
-
-    /* subclass: indicate channel report pending */
-    mcic = MCIC_SC_CP |
-    /* subclass modifiers: none */
-    /* storage errors: none */
-    /* validity bits: no damage */
-        MCIC_VB_WP | MCIC_VB_MS | MCIC_VB_PM | MCIC_VB_IA | MCIC_VB_FP |
-        MCIC_VB_GR | MCIC_VB_CR | MCIC_VB_ST | MCIC_VB_AR | MCIC_VB_PR |
-        MCIC_VB_FC | MCIC_VB_CT | MCIC_VB_CC;
-    if (s390_has_feat(S390_FEAT_VECTOR)) {
-        mcic |= MCIC_VB_VR;
-    }
-    if (s390_has_feat(S390_FEAT_GUARDED_STORAGE)) {
-        mcic |= MCIC_VB_GS;
-    }
-    return mcic;
-}
-
 void kvm_s390_crw_mchk(void)
 {
     struct kvm_s390_irq irq = {
         .type = KVM_S390_MCHK,
-        .u.mchk.cr14 = 1 << 28,
-        .u.mchk.mcic = build_channel_report_mcic(),
+        .u.mchk.cr14 = CR14_CHANNEL_REPORT_SC,
+        .u.mchk.mcic = s390_build_validity_mcic() | MCIC_SC_CP,
     };
     kvm_s390_floating_interrupt(&irq);
 }
Signature · 1 line
2.14.3
#2 lore raw

Re: [Qemu-devel] [PATCH v2 for-2.12 1/5] s390x/kvm: factor out build_channel_report_mcic() into cpu.h

Quoted 25 lines · On Thu, 7 Dec 2017 16:31:37 +0100 David Hildenbrand <[email protected]> wrote:

We'll need it later on in two places. Refactor it to just indicate the validity bits. While at it, introduce a define for the used CR14 bit (we'll also need later on).

Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/cpu.h | 22 ++++++++++++++++++++++
 target/s390x/kvm.c | 25 ++-----------------------
 2 files changed, 24 insertions(+), 23 deletions(-)
target/s390x/cpu.h +3 -0
index 9cfbbbac04..f29f4ccd8b 100644
@@ -351,6 +351,9 @@ extern const struct VMStateDescription vmstate_s390_cpu;
 #define CR0_CPU_TIMER_SC        0x0000000000000400ULL
 #define CR0_SERVICE_SC          0x0000000000000200ULL
 
+/* Control register 14 bits */
+#define CR14_CHANNEL_REPORT_SC  0x0000000010000000ULL
+
 /* MMU */
 #define MMU_PRIMARY_IDX         0
 #define MMU_SECONDARY_IDX       1

target/s390x/cpu.h struct sysib_322 { · line 686

Quoted 10 lines
target/s390x/cpu.h +7 -0
@@ -674,6 +677,25 @@ struct sysib_322 {
 #define MCIC_VB_CT 0x0000000000020000ULL
 #define MCIC_VB_CC 0x0000000000010000ULL
 
+static inline uint64_t s390_build_validity_mcic(void)
+{
+    uint64_t mcic;
+
+    /* Indicate all validity bits (no damage) only. Other bits have to be
+     * added by the caller. (storage errors, subclasses and subclass modifiers)
+     */

Looks good. I prefer the classic comment style (empty first line), though.

Quoted 55 lines
target/s390x/cpu.h +12 -0
+    mcic = MCIC_VB_WP | MCIC_VB_MS | MCIC_VB_PM | MCIC_VB_IA | MCIC_VB_FP |
+           MCIC_VB_GR | MCIC_VB_CR | MCIC_VB_ST | MCIC_VB_AR | MCIC_VB_PR |
+           MCIC_VB_FC | MCIC_VB_CT | MCIC_VB_CC;
+    if (s390_has_feat(S390_FEAT_VECTOR)) {
+        mcic |= MCIC_VB_VR;
+    }
+    if (s390_has_feat(S390_FEAT_GUARDED_STORAGE)) {
+        mcic |= MCIC_VB_GS;
+    }
+    return mcic;
+}
+
 
 /* cpu.c */
 int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
target/s390x/kvm.c +2 -23
index 97c45d5537..9b8b59f2a2 100644
@@ -1852,33 +1852,12 @@ void kvm_s390_io_interrupt(uint16_t subchannel_id,
     kvm_s390_floating_interrupt(&irq);
 }
 
-static uint64_t build_channel_report_mcic(void)
-{
-    uint64_t mcic;
-
-    /* subclass: indicate channel report pending */
-    mcic = MCIC_SC_CP |
-    /* subclass modifiers: none */
-    /* storage errors: none */
-    /* validity bits: no damage */
-        MCIC_VB_WP | MCIC_VB_MS | MCIC_VB_PM | MCIC_VB_IA | MCIC_VB_FP |
-        MCIC_VB_GR | MCIC_VB_CR | MCIC_VB_ST | MCIC_VB_AR | MCIC_VB_PR |
-        MCIC_VB_FC | MCIC_VB_CT | MCIC_VB_CC;
-    if (s390_has_feat(S390_FEAT_VECTOR)) {
-        mcic |= MCIC_VB_VR;
-    }
-    if (s390_has_feat(S390_FEAT_GUARDED_STORAGE)) {
-        mcic |= MCIC_VB_GS;
-    }
-    return mcic;
-}
-
 void kvm_s390_crw_mchk(void)
 {
     struct kvm_s390_irq irq = {
         .type = KVM_S390_MCHK,
-        .u.mchk.cr14 = 1 << 28,
-        .u.mchk.mcic = build_channel_report_mcic(),
+        .u.mchk.cr14 = CR14_CHANNEL_REPORT_SC,
+        .u.mchk.mcic = s390_build_validity_mcic() | MCIC_SC_CP,
     };
     kvm_s390_floating_interrupt(&irq);
 }
author
#3 lore raw

PATCH v2 2/5 s390x/tcg: fix and cleanup mcck injection

In reply to David Hildenbrand

The architecture mode indication wasn't stored. The split of certain 64bit fields was unnecessary. Also, the complete clock comparator, not just bit 0-55 (starting at byte 1) was stored.

We now generate a proper MCIC via the same helper we use for KVM.

There is be more to clean up, but we will change the other parts later on either way.

Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/excp_helper.c | 12 ++++++------
 target/s390x/internal.h    |  6 +++---
 2 files changed, 9 insertions(+), 9 deletions(-)
target/s390x/excp_helper.c +6 -6
index d831537544..f4697a884d 100644
@@ -395,6 +395,9 @@ static void do_mchk_interrupt(CPUS390XState *env)
 
     lowcore = cpu_map_lowcore(env);
 
+    /* we are always in z/Architecture mode */
+    lowcore->ar_access_id = 1;
+
     for (i = 0; i < 16; i++) {
         lowcore->floating_pt_save_area[i] = cpu_to_be64(get_freg(env, i)->ll);
         lowcore->gpregs_save_area[i] = cpu_to_be64(env->regs[i]);

@@ -404,13 +407,10 @@ static void do_mchk_interrupt(CPUS390XState *env)
     lowcore->prefixreg_save_area = cpu_to_be32(env->psa);
     lowcore->fpt_creg_save_area = cpu_to_be32(env->fpc);
     lowcore->tod_progreg_save_area = cpu_to_be32(env->todpr);
-    lowcore->cpu_timer_save_area[0] = cpu_to_be32(env->cputm >> 32);
-    lowcore->cpu_timer_save_area[1] = cpu_to_be32((uint32_t)env->cputm);
-    lowcore->clock_comp_save_area[0] = cpu_to_be32(env->ckc >> 32);
-    lowcore->clock_comp_save_area[1] = cpu_to_be32((uint32_t)env->ckc);
+    lowcore->cpu_timer_save_area = cpu_to_be64(env->cputm);
+    lowcore->clock_comp_save_area = cpu_to_be64(env->ckc >> 8);
 
-    lowcore->mcck_interruption_code[0] = cpu_to_be32(0x00400f1d);
-    lowcore->mcck_interruption_code[1] = cpu_to_be32(0x40330000);
+    lowcore->mcic = cpu_to_be64(s390_build_validity_mcic() | MCIC_SC_CP);
     lowcore->mcck_old_psw.mask = cpu_to_be64(get_psw_mask(env));
     lowcore->mcck_old_psw.addr = cpu_to_be64(env->psw.addr);
     mask = be64_to_cpu(lowcore->mcck_new_psw.mask);
target/s390x/internal.h +3 -3
index 6817b2c432..1a88e4beb4 100644
@@ -43,7 +43,7 @@ typedef struct LowCore {
     uint8_t         pad3[0xc8 - 0xc4];        /* 0x0c4 */
     uint32_t        stfl_fac_list;            /* 0x0c8 */
     uint8_t         pad4[0xe8 - 0xcc];        /* 0x0cc */
-    uint32_t        mcck_interruption_code[2]; /* 0x0e8 */
+    uint64_t        mcic;                     /* 0x0e8 */
     uint8_t         pad5[0xf4 - 0xf0];        /* 0x0f0 */
     uint32_t        external_damage_code;     /* 0x0f4 */
     uint64_t        failing_storage_address;  /* 0x0f8 */

@@ -118,8 +118,8 @@ typedef struct LowCore {
     uint32_t        fpt_creg_save_area;        /* 0x131c */
     uint8_t         pad16[0x1324 - 0x1320];    /* 0x1320 */
     uint32_t        tod_progreg_save_area;     /* 0x1324 */
-    uint32_t        cpu_timer_save_area[2];    /* 0x1328 */
-    uint32_t        clock_comp_save_area[2];   /* 0x1330 */
+    uint64_t        cpu_timer_save_area;       /* 0x1328 */
+    uint64_t        clock_comp_save_area;      /* 0x1330 */
     uint8_t         pad17[0x1340 - 0x1338];    /* 0x1338 */
     uint32_t        access_regs_save_area[16]; /* 0x1340 */
     uint64_t        cregs_save_area[16];       /* 0x1380 */
Signature · 1 line
2.14.3
#4 lore raw

Re: [Qemu-devel] [PATCH v2 for-2.12 2/5] s390x/tcg: fix and cleanup mcck injection

Quoted 7 lines · On Thu, 7 Dec 2017 16:31:38 +0100 David Hildenbrand <[email protected]> wrote:

The architecture mode indication wasn't stored. The split of certain 64bit fields was unnecessary. Also, the complete clock comparator, not just bit 0-55 (starting at byte 1) was stored.

We now generate a proper MCIC via the same helper we use for KVM.

There is be more to clean up, but we will change the other parts later on

s/is be/is/

Quoted 1 line

either way.

No surprise as I had no idea what I was doing when I initially wrote this :)

Quoted 6 lines
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/excp_helper.c | 12 ++++++------
 target/s390x/internal.h    |  6 +++---
 2 files changed, 9 insertions(+), 9 deletions(-)
author
#5 lore raw

PATCH v2 3/5 s390x/tcg: implement SET CLOCK PROGRAMMABLE FIELD

In reply to David Hildenbrand

Needed for machine check handling inside Linux (when restoring registers).

Except for SIGP and machine checks, we don't make use of the register yet. Sufficient for now.

Reviewed-by Thomas Huth <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/misc_helper.c | 11 +++++++++++
 target/s390x/translate.c   |  7 +++++++
 4 files changed, 21 insertions(+)
target/s390x/helper.h +1 -0
index 9459b73c73..0281c286b8 100644
@@ -127,6 +127,7 @@ DEF_HELPER_3(load_psw, noreturn, env, i64, i64)
 DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env)
 DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64)
+DEF_HELPER_FLAGS_1(sckpf, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(stckc, TCG_CALL_NO_RWG, i64, env)
 DEF_HELPER_FLAGS_2(spt, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env)
target/s390x/insn-data.def +2 -0
index 16e27c8a35..8c2541f545 100644
@@ -999,6 +999,8 @@
     C(0xb204, SCK,     S,     Z,   0, 0, 0, 0, 0, 0)
 /* SET CLOCK COMPARATOR */
     C(0xb206, SCKC,    S,     Z,   0, m2_64, 0, 0, sckc, 0)
+/* SET CLOCK PROGRAMMABLE FIELD */
+    C(0x0107, SCKPF,   E,     Z,   0, 0, 0, 0, sckpf, 0)
 /* SET CPU TIMER */
     C(0xb208, SPT,     S,     Z,   0, m2_64, 0, 0, spt, 0)
 /* SET PREFIX */
target/s390x/misc_helper.c +11 -0
index 6d766ce1e7..25b71f08ee 100644
@@ -146,6 +146,17 @@ void HELPER(sckc)(CPUS390XState *env, uint64_t time)
     timer_mod(env->tod_timer, env->tod_basetime + time);
 }
 
+/* Set Tod Programmable Field */
+void HELPER(sckpf)(CPUS390XState *env)
+{
+    uint32_t val = env->regs[0];
+
+    if (val & 0xffff0000) {
+        s390_program_interrupt(env, PGM_SPECIFICATION, 2, GETPC());
+    }
+    env->todpr = val;
+}
+
 /* Store Clock Comparator */
 uint64_t HELPER(stckc)(CPUS390XState *env)
 {
target/s390x/translate.c +7 -0
index 26cf993405..48b031894a 100644
@@ -3922,6 +3922,13 @@ static ExitStatus op_sckc(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_sckpf(DisasContext *s, DisasOps *o)
+{
+    check_privileged(s);
+    gen_helper_sckpf(cpu_env);
+    return NO_EXIT;
+}
+
 static ExitStatus op_stckc(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
Signature · 1 line
2.14.3
#6 lore raw

Re: [Qemu-devel] [PATCH v2 for-2.12 3/5] s390x/tcg: implement SET CLOCK PROGRAMMABLE FIELD

Quoted 1 line · David Hildenbrand

+DEF_HELPER_FLAGS_1(sckpf, TCG_CALL_NO_RWG, void, env)

...

target/s390x/misc_helper.c void HELPER(sckc)(CPUS390XState *env, uint64_t time) · line 158

Quoted 10 lines
target/s390x/misc_helper.c +10 -0
+/* Set Tod Programmable Field */
+void HELPER(sckpf)(CPUS390XState *env)
+{
+    uint32_t val = env->regs[0];
+
+    if (val & 0xffff0000) {
+        s390_program_interrupt(env, PGM_SPECIFICATION, 2, GETPC());
+    }
+    env->todpr = val;
+}

You do read a tcg global -- regs[0]. Either pass in r0 as a parameter or use TCG_CALL_NO_WG.

r~

author
#7 lore raw

Re: [Qemu-devel] [PATCH v2 for-2.12 3/5] s390x/tcg: implement SET CLOCK PROGRAMMABLE FIELD

Quoted 18 lines · Richard Henderson
Quoted 1 line · David Hildenbrand

+DEF_HELPER_FLAGS_1(sckpf, TCG_CALL_NO_RWG, void, env)

...

target/s390x/misc_helper.c void HELPER(sckc)(CPUS390XState *env, uint64_t time) · line 158

Quoted 10 lines
target/s390x/misc_helper.c +10 -0
+/* Set Tod Programmable Field */
+void HELPER(sckpf)(CPUS390XState *env)
+{
+    uint32_t val = env->regs[0];
+
+    if (val & 0xffff0000) {
+        s390_program_interrupt(env, PGM_SPECIFICATION, 2, GETPC());
+    }
+    env->todpr = val;
+}

You do read a tcg global -- regs[0]. Either pass in r0 as a parameter or use TCG_CALL_NO_WG.

Indeed, I finally understood what the term "global" in this context means. (global tcg vals defined in translate.c)

Quoted 2 lines

r~

Signature · 3 lines
Thanks,

David / dhildenb
author
#8 lore raw

PATCH v2 4/5 s390x/tcg: indicate value of TODPR in STCKE

In reply to David Hildenbrand

We were not yet using the value of the TOD Programmable Register.

Reviewed-by Thomas Huth <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/translate.c | 5 +++++
 1 file changed, 5 insertions(+)
target/s390x/translate.c +5 -0
index 48b031894a..8da8610839 100644
@@ -3897,7 +3897,10 @@ static ExitStatus op_stcke(DisasContext *s, DisasOps *o)
 {
     TCGv_i64 c1 = tcg_temp_new_i64();
     TCGv_i64 c2 = tcg_temp_new_i64();
+    TCGv_i64 todpr = tcg_temp_new_i64();
     gen_helper_stck(c1, cpu_env);
+    /* 16 bit value store in an uint32_t (only valid bits set) */
+    tcg_gen_ld32u_i64(todpr, cpu_env, offsetof(CPUS390XState, todpr));
     /* Shift the 64-bit value into its place as a zero-extended
        104-bit value.  Note that "bit positions 64-103 are always
        non-zero so that they compare differently to STCK"; we set

@@ -3905,11 +3908,13 @@ static ExitStatus op_stcke(DisasContext *s, DisasOps *o)
     tcg_gen_shli_i64(c2, c1, 56);
     tcg_gen_shri_i64(c1, c1, 8);
     tcg_gen_ori_i64(c2, c2, 0x10000);
+    tcg_gen_or_i64(c2, c2, todpr);
     tcg_gen_qemu_st64(c1, o->in2, get_mem_index(s));
     tcg_gen_addi_i64(o->in2, o->in2, 8);
     tcg_gen_qemu_st64(c2, o->in2, get_mem_index(s));
     tcg_temp_free_i64(c1);
     tcg_temp_free_i64(c2);
+    tcg_temp_free_i64(todpr);
     /* ??? We don't implement clock states.  */
     gen_op_movi_cc(s, 0);
     return NO_EXIT;
Signature · 1 line
2.14.3
#9 lore raw

Re: [Qemu-devel] [PATCH v2 for-2.12 4/5] s390x/tcg: indicate value of TODPR in STCKE

target/s390x/translate.c static ExitStatus op_stcke(DisasContext *s, DisasOps *o) · line 3903

Quoted 2 lines · David Hildenbrand
target/s390x/translate.c +2 -0
+    /* 16 bit value store in an uint32_t (only valid bits set) */
+    tcg_gen_ld32u_i64(todpr, cpu_env, offsetof(CPUS390XState, todpr));

Any reason not to use a uint16_t and use tcg_gen_ld16u_i64 here?

r~

author
#10 lore raw

Re: [Qemu-devel] [PATCH v2 for-2.12 4/5] s390x/tcg: indicate value of TODPR in STCKE

Quoted 6 lines · Richard Henderson

target/s390x/translate.c static ExitStatus op_stcke(DisasContext *s, DisasOps *o) · line 3903

Quoted 2 lines · David Hildenbrand
target/s390x/translate.c +2 -0
+    /* 16 bit value store in an uint32_t (only valid bits set) */
+    tcg_gen_ld32u_i64(todpr, cpu_env, offsetof(CPUS390XState, todpr));

Any reason not to use a uint16_t and use tcg_gen_ld16u_i64 here?

Yes, KVM introduced and uses this field. As it is used for migration, we cannot easily change it. (any maybe it was designed that way for future changes)

Quoted 3 lines

r~

Signature · 3 lines
Thanks,

David / dhildenb
#11 lore raw

Re: [Qemu-devel] [PATCH v2 for-2.12 4/5] s390x/tcg: indicate value of TODPR in STCKE

Quoted 11 lines · David Hildenbrand
Quoted 6 lines · Richard Henderson

target/s390x/translate.c static ExitStatus op_stcke(DisasContext *s, DisasOps *o) · line 3903

Quoted 2 lines · David Hildenbrand
target/s390x/translate.c +2 -0
+    /* 16 bit value store in an uint32_t (only valid bits set) */
+    tcg_gen_ld32u_i64(todpr, cpu_env, offsetof(CPUS390XState, todpr));

Any reason not to use a uint16_t and use tcg_gen_ld16u_i64 here?

Yes, KVM introduced and uses this field. As it is used for migration, we cannot easily change it. (any maybe it was designed that way for future changes)

FWIW, if you look at the POP the TOD programmable register is 32bit (of which bits 0-15 in IBM speak) are 0.

author
#12 lore raw

PATCH v2 5/5 s390x/tcg: wire up STORE CHANNEL REPORT WORD

In reply to David Hildenbrand

CRW machine check handling requires STCRW. So let's wire it up.

Reviewed-by Thomas Huth <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/helper.h      | 1 +
 target/s390x/insn-data.def | 1 +
 target/s390x/misc_helper.c | 9 +++++++++
 target/s390x/translate.c   | 8 ++++++++
 4 files changed, 19 insertions(+)
target/s390x/helper.h +1 -0
index 0281c286b8..2ce57edc14 100644
@@ -166,6 +166,7 @@ DEF_HELPER_3(msch, void, env, i64, i64)
 DEF_HELPER_2(rchp, void, env, i64)
 DEF_HELPER_2(rsch, void, env, i64)
 DEF_HELPER_3(ssch, void, env, i64, i64)
+DEF_HELPER_2(stcrw, void, env, i64)
 DEF_HELPER_3(stsch, void, env, i64, i64)
 DEF_HELPER_3(tsch, void, env, i64, i64)
 DEF_HELPER_2(chsc, void, env, i64)
target/s390x/insn-data.def +1 -0
index 8c2541f545..43ab1963c8 100644
@@ -1055,6 +1055,7 @@
     C(0xb23b, RCHP,    S,     Z,   0, 0, 0, 0, rchp, 0)
     C(0xb238, RSCH,    S,     Z,   0, 0, 0, 0, rsch, 0)
     C(0xb233, SSCH,    S,     Z,   0, insn, 0, 0, ssch, 0)
+    C(0xb239, STCRW,   S,     Z,   0, insn, 0, 0, stcrw, 0)
     C(0xb234, STSCH,   S,     Z,   0, insn, 0, 0, stsch, 0)
     C(0xb235, TSCH,    S,     Z,   0, insn, 0, 0, tsch, 0)
     /* ??? Not listed in PoO ninth edition, but there's a linux driver that
target/s390x/misc_helper.c +9 -0
index 25b71f08ee..3541e47114 100644
@@ -385,6 +385,15 @@ void HELPER(ssch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
     qemu_mutex_unlock_iothread();
 }
 
+void HELPER(stcrw)(CPUS390XState *env, uint64_t inst)
+{
+    S390CPU *cpu = s390_env_get_cpu(env);
+
+    qemu_mutex_lock_iothread();
+    ioinst_handle_stcrw(cpu, inst >> 16, GETPC());
+    qemu_mutex_unlock_iothread();
+}
+
 void HELPER(stsch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
target/s390x/translate.c +8 -0
index 8da8610839..5e051fdd03 100644
@@ -4071,6 +4071,14 @@ static ExitStatus op_stsch(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_stcrw(DisasContext *s, DisasOps *o)
+{
+    check_privileged(s);
+    gen_helper_stcrw(cpu_env, o->in2);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_tsch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
Signature · 1 line
2.14.3
Reviewed-by on v2 5/5
#13 lore raw

Re: [Qemu-devel] [PATCH v2 for-2.12 5/5] s390x/tcg: wire up STORE CHANNEL REPORT WORD

Quoted 10 lines · David Hildenbrand

CRW machine check handling requires STCRW. So let's wire it up.

Reviewed-by Thomas Huth <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 target/s390x/helper.h      | 1 +
 target/s390x/insn-data.def | 1 +
 target/s390x/misc_helper.c | 9 +++++++++
 target/s390x/translate.c   | 8 ++++++++
 4 files changed, 19 insertions(+)
Reviewed-by Richard Henderson <[email protected]>

r~