Re: [qemu-s390x] [Qemu-devel] [PATCH v2 2/3] hw/s390x/css: Remove QEMU_PACKED from struct SenseId

[Thomas Huth]

On 2018-09-26 13:21, David Hildenbrand wrote:
> 
>>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>>> index 5a9fe45ce8..db378f6238 100644
>>> --- a/hw/s390x/css.c
>>> +++ b/hw/s390x/css.c
>>> @@ -750,12 +750,13 @@ static void sch_handle_halt_func(SubchDev *sch)
>>>
>>>  }
>>>
>>> -static void copy_sense_id_to_guest(SenseId *dest, SenseId *src)
>>> +static void copy_sense_id_to_guest(SenseIdPacked *dest, SenseId *src)
>>>  {
>>>      int i;
>>>
>>>      dest->reserved = src->reserved;
>>> -    dest->cu_type = cpu_to_be16(src->cu_type);
>>> +    /* avoid unaligned accesses */
>>> +    dest->cu_type[0] = cpu_to_be16(src->cu_type) >> 8;
>>
>> The error Peter reported was "load of misaligned address", so the
>> culprit is the cpu_to_be16() call. See bswap16s() casting an unaligned u16.
>>
>> IMHO your patch should trigger the same issue.
> 
> No, src->cu_type would be aligned now ...
> 
>>
>> What about this instead?
>>
>>       dest->cu_type = lduw_be_p(&src->cu_type);
>>
> 
> ... however that certainly looks better ...
> 
> ... but looks like Thomas wants to stick to the handcrafted approach.

That would still does not fix the issue in the migration code
("VMSTATE_UINT16(cu_type, SenseId)").

 Thomas