[v2,for-2.12,4/5] s390x/tcg: indicate value of TODPR in STCKE

Message ID 20171207153141.15523-5-david@redhat.com
State New
Headers show
Series
  • s390x/tcg: CCW hotplug support
Related show

Commit Message

David Hildenbrand Dec. 7, 2017, 3:31 p.m.
We were not yet using the value of the TOD Programmable Register.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/translate.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Richard Henderson Dec. 8, 2017, 12:02 a.m. | #1
On 12/07/2017 07:31 AM, David Hildenbrand wrote:
> +    /* 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~
David Hildenbrand Dec. 8, 2017, 12:45 p.m. | #2
On 08.12.2017 01:02, Richard Henderson wrote:
> On 12/07/2017 07:31 AM, David Hildenbrand wrote:
>> +    /* 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)

> 
> r~
>
Christian Borntraeger Dec. 8, 2017, 1 p.m. | #3
On 12/08/2017 01:45 PM, David Hildenbrand wrote:
> On 08.12.2017 01:02, Richard Henderson wrote:
>> On 12/07/2017 07:31 AM, David Hildenbrand wrote:
>>> +    /* 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.

Patch

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 48b031894a..8da8610839 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -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;