From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40020) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1etbQ4-0003t3-RN for qemu-devel@nongnu.org; Wed, 07 Mar 2018 10:55:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1etbQ0-0007qH-0D for qemu-devel@nongnu.org; Wed, 07 Mar 2018 10:55:56 -0500 Date: Wed, 7 Mar 2018 16:55:48 +0100 From: Cornelia Huck Message-ID: <20180307165548.29f7ec07.cohuck@redhat.com> In-Reply-To: <1520435434-24471-1-git-send-email-imbrenda@linux.vnet.ibm.com> References: <1520435434-24471-1-git-send-email-imbrenda@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Claudio Imbrenda Cc: qemu-s390x@nongnu.org, qemu-devel@nongnu.org, borntraeger@de.ibm.com On Wed, 7 Mar 2018 16:10:34 +0100 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 > --- > hw/s390x/event-facility.c | 49 ++++++++++++++++++++++++++++++++------- > include/hw/s390x/event-facility.h | 2 +- > 2 files changed, 41 insertions(+), 10 deletions(-) > > @@ -376,6 +387,21 @@ static bool vmstate_event_facility_mask_length_needed(void *opaque) > return ef->allow_all_mask_sizes; > } > > +static const VMStateDescription vmstate_event_facility_mask64 = { > + .name = "vmstate-event-facility/mask64", Should that really be called 'mask64' - you're just transferring the missing part of the mask here, aren't you? > + .version_id = 0, > + .minimum_version_id = 0, > + .needed = vmstate_event_facility_mask64_needed, > + .fields = (VMStateField[]) { > +#ifdef HOST_WORDS_BIGENDIAN > + VMSTATE_UINT32(receive_mask_pieces[1], SCLPEventFacility), > +#else > + VMSTATE_UINT32(receive_mask_pieces[0], SCLPEventFacility), > +#endif That #ifdef is kind of ugly, and a bit confusing, I think. What about doing something like /* we need to save 32 bit chunks for compatibility */ #ifdef HOST_WORDS_BIGENDIAN #define RECV_MASK_LOWER 0 #define RECV_MASK_UPPER 1 #else /* little endian host */ #define RECV_MASK_LOWER 1 #define RECV_MASK_UPPER 0 #endif and transfer receive_mask_pieces[RECV_MASK_UPPER] here... > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const VMStateDescription vmstate_event_facility_mask_length = { > .name = "vmstate-event-facility/mask_length", > .version_id = 0, > @@ -392,10 +418,15 @@ static const VMStateDescription vmstate_event_facility = { > .version_id = 0, > .minimum_version_id = 0, > .fields = (VMStateField[]) { > - VMSTATE_UINT32(receive_mask, SCLPEventFacility), > +#ifdef HOST_WORDS_BIGENDIAN > + VMSTATE_UINT32(receive_mask_pieces[0], SCLPEventFacility), > +#else > + VMSTATE_UINT32(receive_mask_pieces[1], SCLPEventFacility), > +#endif ...and receive_mask_pieces[REV_MASK_LOWER] here? (And I guess 64bit receive masks should be enough for everybody? IOW, we don't expect a similar dance in the near future?) > VMSTATE_END_OF_LIST() > }, > .subsections = (const VMStateDescription * []) { > + &vmstate_event_facility_mask64, > &vmstate_event_facility_mask_length, > NULL > }