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

[Claudio Imbrenda]

On Tue, 6 Mar 2018 10:27:52 +0100
Cornelia Huck <address@hidden> wrote:

> On Tue, 6 Mar 2018 09:23:23 +0100
> Christian Borntraeger <address@hidden> wrote:
> 
> > On 03/05/2018 04:27 PM, Cornelia Huck wrote:  
> > > On Fri, 2 Mar 2018 10:44:46 +0100
> > > Christian Borntraeger <address@hidden> wrote:
> > >     
> > >> On 02/23/2018 06:42 PM, Claudio Imbrenda 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];      
> > >>
> > >>
> > >> Before the change, we basically use be32_to_cpu to transfer the
> > >> byte field into a cpu endianess value. In the end it is actually
> > >> a bitfield, but for compat we need to keep he reversal. So it
> > >> will be hard to get this fixed without some kind of ugliness.    
> > > 
> > > Could we also use a compat mask callback/handler for older
> > > machines and switch to 64 bit handlers for the default case?
> > > Probably would be even more ugly, though.    
> > 
> > Claudio had a version with a pre/post/load/save handler. Claudio
> > can you repost this version so that we can have a look what is
> > "less ugly"? 
> 
> Would that other version be independent of the first two patches
> (i.e., only replace this patch)?

yes

> I would like to apply patch 1 as it is a fix, and patch 2 seems
> uncontroversial as a cleanup.

yes please