Re: [Qemu-devel] [PATCH v1] s390x/kvm: fix and cleanup storing CPU status

[David Hildenbrand]

On 22.09.2017 15:12, Richard Henderson wrote:
> On 09/21/2017 04:30 PM, David Hildenbrand wrote:
>> +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_PACKED;
> 
> I don't believe you need to use QEMU_PACKED.  Unless the "addr" that's passed
> in can really be arbitrary?  (If so, the memory returned by
> cpu_physical_memory_map will be unaligned and so we do need the PACKED to 
> force
> the compiler to generate unaligned stores into the structure.)

The address is either 0x1200 or it is masked with 0x7ffffe00u.

So most probably this should be fine without QEMU_PACKED, right?

> 
>> QEMU_BUILD_BUG_ON(sizeof(*sa) != 512);
> 
> Put this right next to the structure?
> 

Wasn't aware that this can be moved outside of a function. Thanks!

Thanks!

> 
> r~
> 


-- 

Thanks,

David