s390x/kvm: fix and cleanup storing CPU status
PATCH v2 1/1 s390x/kvm: fix and cleanup storing CPU status
env->psa is a 64bit value, while we copy 4 bytes into the save area, resulting always in 0 getting stored.
Let's try to reduce such errors by using a proper structure. While at it, use correct cpu->be conversion (and get_psw_mask()), as we will be reusing this code for TCG soon.
---
v1 -> v2:
- dropped QEMU_PACKED
- Moved QEMU_BUILD_BUG_ON()
- Retested if it works now
target/s390x/kvm.c | 62 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 20 deletions(-)
target/s390x/kvm.c +42 -20
@@ -1553,22 +1553,37 @@ static int do_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len)return 0 ; } +struct sigp_save_area { +uint64_t fprs[16 ];/* 0x0000 */ +uint64_t grs[16 ];/* 0x0080 */ +PSW psw;/* 0x0100 */ +uint8_t pad_0x0110[0x0118 -0x0110 ];/* 0x0110 */ +uint32_t prefix;/* 0x0118 */ +uint32_t fpc;/* 0x011c */ +uint8_t pad_0x0120[0x0124 -0x0120 ];/* 0x0120 */ +uint32_t todpr;/* 0x0124 */ +uint64_t cputm;/* 0x0128 */ +uint64_t ckc;/* 0x0130 */ +uint8_t pad_0x0138[0x0140 -0x0138 ];/* 0x0138 */ +uint32_t ars[16 ];/* 0x0140 */ +uint64_t crs[16 ];/* 0x0384 */ +}; +QEMU_BUILD_BUG_ON (sizeof (struct sigp_save_area ) !=512 ); +#define KVM_S390_STORE_STATUS_DEF_ADDR offsetof(LowCore, floating_pt_save_area) -#define SAVE_AREA_SIZE 512static int kvm_s390_store_status (S390CPU *cpu,hwaddr addr,bool store_arch) {static const uint8_t ar_id =1 ; -uint64_t ckc = cpu->env.ckc >>8 ; -void *mem; +struct sigp_save_area *sa; +hwaddr len =sizeof (*sa);int i; -hwaddr len =SAVE_AREA_SIZE ; - mem =cpu_physical_memory_map (addr, &len,1 ); -if (!mem) { + sa =cpu_physical_memory_map (addr, &len,1 ); +if (!sa) {return -EFAULT ; } -if (len !=SAVE_AREA_SIZE ) { -cpu_physical_memory_unmap (mem, len,1 ,0 ); +if (len !=sizeof (*sa)) { +cpu_physical_memory_unmap (sa, len,1 ,0 );return -EFAULT ; }
@@ -1576,19 +1591,26 @@ static int kvm_s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)cpu_physical_memory_write (offsetof(LowCore , ar_access_id), &ar_id,1 ); }for (i =0 ; i <16 ; ++i) { - *((uint64_t *)mem + i) =get_freg (&cpu->env, i)->ll; - } -memcpy (mem +128 , &cpu->env.regs,128 ); -memcpy (mem +256 , &cpu->env.psw,16 ); -memcpy (mem +280 , &cpu->env.psa,4 ); -memcpy (mem +284 , &cpu->env.fpc,4 ); -memcpy (mem +292 , &cpu->env.todpr,4 ); -memcpy (mem +296 , &cpu->env.cputm,8 ); -memcpy (mem +304 , &ckc,8 ); -memcpy (mem +320 , &cpu->env.aregs,64 ); -memcpy (mem +384 , &cpu->env.cregs,128 ); + sa->fprs[i] =cpu_to_be64 (get_freg (&cpu->env, i)->ll); + } +for (i =0 ; i <16 ; ++i) { + sa->grs[i] =cpu_to_be64 (cpu->env.regs[i]); + } + sa->psw.addr =cpu_to_be64 (cpu->env.psw.addr); + sa->psw.mask =cpu_to_be64 (get_psw_mask (&cpu->env)); + sa->prefix =cpu_to_be32 (cpu->env.psa); + sa->fpc =cpu_to_be32 (cpu->env.fpc); + sa->todpr =cpu_to_be32 (cpu->env.todpr); + sa->cputm =cpu_to_be64 (cpu->env.cputm); + sa->ckc =cpu_to_be64 (cpu->env.ckc >>8 ); +for (i =0 ; i <16 ; ++i) { + sa->ars[i] =cpu_to_be32 (cpu->env.aregs[i]); + } +for (i =0 ; i <16 ; ++i) { + sa->ars[i] =cpu_to_be64 (cpu->env.cregs[i]); + } -cpu_physical_memory_unmap (mem, len,1 , len); +cpu_physical_memory_unmap (sa, len,1 , len);return 0 ; }
Signature · 1 line
2.13.5
Quoted 10 lines · David Hildenbrand
env->psais a 64bit value, while we copy 4 bytes into the save area, resulting always in 0 getting stored.Let's try to reduce such errors by using a proper structure. While at it, use correct
cpu->beconversion (andget_psw_mask()), as we will be reusing this code for TCG soon.Signed-off-by David Hildenbrand <[email protected]>---
Quoted 7 lines
v1 -> v2:
- droppedQEMU_PACKED
- MovedQEMU_BUILD_BUG_ON()
- Retested if it works nowtarget/s390x/kvm.c | 62 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 20 deletions(-)
r~
Quoted 97 lines · David Hildenbrand
env->psais a 64bit value, while we copy 4 bytes into the save area, resulting always in 0 getting stored.Let's try to reduce such errors by using a proper structure. While at it, use correct
cpu->beconversion (andget_psw_mask()), as we will be reusing this code for TCG soon.Signed-off-by David Hildenbrand <[email protected]>---
v1 -> v2:
- droppedQEMU_PACKED
- MovedQEMU_BUILD_BUG_ON()
- Retested if it works nowtarget/s390x/kvm.c | 62 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 20 deletions(-)target/s390x/kvm.c +37
-19@@ -1553,22 +1553,37 @@ static int do_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len)return 0 ; } +struct sigp_save_area { +uint64_t fprs[16 ];/* 0x0000 */ +uint64_t grs[16 ];/* 0x0080 */ +PSW psw;/* 0x0100 */ +uint8_t pad_0x0110[0x0118 -0x0110 ];/* 0x0110 */ +uint32_t prefix;/* 0x0118 */ +uint32_t fpc;/* 0x011c */ +uint8_t pad_0x0120[0x0124 -0x0120 ];/* 0x0120 */ +uint32_t todpr;/* 0x0124 */ +uint64_t cputm;/* 0x0128 */ +uint64_t ckc;/* 0x0130 */ +uint8_t pad_0x0138[0x0140 -0x0138 ];/* 0x0138 */ +uint32_t ars[16 ];/* 0x0140 */ +uint64_t crs[16 ];/* 0x0384 */ +}; +QEMU_BUILD_BUG_ON (sizeof (struct sigp_save_area ) !=512 ); +#define KVM_S390_STORE_STATUS_DEF_ADDR offsetof(LowCore, floating_pt_save_area) -#define SAVE_AREA_SIZE 512static int kvm_s390_store_status (S390CPU *cpu,hwaddr addr,bool store_arch) {static const uint8_t ar_id =1 ; -uint64_t ckc = cpu->env.ckc >>8 ; -void *mem; +struct sigp_save_area *sa; +hwaddr len =sizeof (*sa);int i; -hwaddr len =SAVE_AREA_SIZE ; - mem =cpu_physical_memory_map (addr, &len,1 ); -if (!mem) { + sa =cpu_physical_memory_map (addr, &len,1 ); +if (!sa) {return -EFAULT ; } -if (len !=SAVE_AREA_SIZE ) { -cpu_physical_memory_unmap (mem, len,1 ,0 ); +if (len !=sizeof (*sa)) { +cpu_physical_memory_unmap (sa, len,1 ,0 );return -EFAULT ; }@@ -1576,19 +1591,26 @@ static int kvm_s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)cpu_physical_memory_write (offsetof(LowCore , ar_access_id), &ar_id,1 ); }for (i =0 ; i <16 ; ++i) { - *((uint64_t *)mem + i) =get_freg (&cpu->env, i)->ll; - } -memcpy (mem +128 , &cpu->env.regs,128 ); -memcpy (mem +256 , &cpu->env.psw,16 ); -memcpy (mem +280 , &cpu->env.psa,4 ); -memcpy (mem +284 , &cpu->env.fpc,4 ); -memcpy (mem +292 , &cpu->env.todpr,4 ); -memcpy (mem +296 , &cpu->env.cputm,8 ); -memcpy (mem +304 , &ckc,8 ); -memcpy (mem +320 , &cpu->env.aregs,64 ); -memcpy (mem +384 , &cpu->env.cregs,128 ); + sa->fprs[i] =cpu_to_be64 (get_freg (&cpu->env, i)->ll); + } +for (i =0 ; i <16 ; ++i) { + sa->grs[i] =cpu_to_be64 (cpu->env.regs[i]); + } + sa->psw.addr =cpu_to_be64 (cpu->env.psw.addr); + sa->psw.mask =cpu_to_be64 (get_psw_mask (&cpu->env)); + sa->prefix =cpu_to_be32 (cpu->env.psa); + sa->fpc =cpu_to_be32 (cpu->env.fpc); + sa->todpr =cpu_to_be32 (cpu->env.todpr); + sa->cputm =cpu_to_be64 (cpu->env.cputm); + sa->ckc =cpu_to_be64 (cpu->env.ckc >>8 ); +for (i =0 ; i <16 ; ++i) { + sa->ars[i] =cpu_to_be32 (cpu->env.aregs[i]);
target/s390x/kvm.c static int kvm_s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch) · line 1616
Quoted 10 lines
target/s390x/kvm.c +5
-1+ } +for (i =0 ; i <16 ; ++i) { + sa->ars[i] =cpu_to_be64 (cpu->env.cregs[i]); + } -cpu_physical_memory_unmap (mem, len,1 , len); +cpu_physical_memory_unmap (sa, len,1 , len);return 0 ; }
Cornelia Huck
AppliedQuoted 7 lines · On Fri, 22 Sep 2017 16:03:38 +0200 David Hildenbrand <[email protected]> wrote:
env->psais a 64bit value, while we copy 4 bytes into the save area, resulting always in 0 getting stored.Let's try to reduce such errors by using a proper structure. While at it, use correct
cpu->beconversion (andget_psw_mask()), as we will be reusing this code for TCG soon.
Quoted 7 lines
Signed-off-by David Hildenbrand <[email protected]>---
v1 -> v2:
- droppedQEMU_PACKED
- MovedQEMU_BUILD_BUG_ON()
- Retested if it works now
Testing? What's going on here? :)
Quoted 3 lines
target/s390x/kvm.c | 62 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 20 deletions(-)
Thanks, applied.