Re: [qemu-s390x] [PATCH v4 3/3] s390x/sclp: extend SCLP event masks to 64 bits

[Christian Borntraeger]

h

On 02/26/2018 05:57 PM, Cornelia Huck wrote:
> On Fri, 23 Feb 2018 18:42:58 +0100
> Claudio Imbrenda <address@hidden> wrote:
> 
>> Extend the SCLP event masks to 64 bits.
>>
>> Notice that using any of the new bits results in a state that cannot be
>> migrated to an older version.
>>
>> Signed-off-by: Claudio Imbrenda <address@hidden>
>> ---
>>  hw/s390x/event-facility.c         | 56 
>> ++++++++++++++++++++++++++++++---------
>>  include/hw/s390x/event-facility.h |  2 +-
>>  2 files changed, 45 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
>> index e04ed9f..c3e39ee 100644
>> --- a/hw/s390x/event-facility.c
>> +++ b/hw/s390x/event-facility.c
>> @@ -30,7 +30,7 @@ struct SCLPEventFacility {
>>      SysBusDevice parent_obj;
>>      SCLPEventsBus sbus;
>>      /* guest's receive mask */
>> -    sccb_mask_t receive_mask;
>> +    uint32_t receive_mask_pieces[2];
>>      /*
>>       * when false, we keep the same broken, backwards compatible behaviour 
>> as
>>       * before, allowing only masks of size exactly 4; when true, we 
>> implement
>> @@ -42,6 +42,18 @@ struct SCLPEventFacility {
>>      uint16_t mask_length;
>>  };
>>  
>> +static inline sccb_mask_t make_receive_mask(SCLPEventFacility *ef)
>> +{
>> +    return ((sccb_mask_t)ef->receive_mask_pieces[0]) << 32 |
>> +                         ef->receive_mask_pieces[1];
>> +}
>> +
>> +static inline void store_receive_mask(SCLPEventFacility *ef, sccb_mask_t 
>> val)
>> +{
>> +    ef->receive_mask_pieces[1] = val;
>> +    ef->receive_mask_pieces[0] = val >> 32;
>> +}
>> +
> 
> Hm... how are all those values actually defined in the architecture?
> You pass around some values internally (which are supposedly in host
> endian) and then and/or them with the receive mask here. Are they
> compared byte-for-byte? 32-bit-for-32-bit?

Its a bitfield in ibm byte order. the length is a multitude of bytes.


> 
> I'm also not a fan of the _pieces suffix - reminds me of Dwarf pieces :)
>