On Tue, 20 Feb 2018 19:45:01 +0100 Claudio Imbrenda <imbre...@linux.vnet.ibm.com> wrote:
> Clean up SCLP masks: introduce an sccb_mask_t to be used for SCLP event > masks instead of just unsigned int or uint32_t. This will allow later > to extend the mask with more ease. Looks mostly sane. > > Signed-off-by: Claudio Imbrenda <imbre...@linux.vnet.ibm.com> > --- > hw/char/sclpconsole-lm.c | 4 ++-- > hw/char/sclpconsole.c | 4 ++-- > hw/s390x/event-facility.c | 18 +++++++++--------- > hw/s390x/sclpcpu.c | 4 ++-- > hw/s390x/sclpquiesce.c | 4 ++-- > include/hw/s390x/event-facility.h | 22 +++++++++++++--------- > 6 files changed, 30 insertions(+), 26 deletions(-) > > diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c > index c500bda..cc4d70a 100644 > --- a/hw/char/sclpconsole-lm.c > +++ b/hw/char/sclpconsole-lm.c > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c > index 2414614..f6f28fd 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' receive mask */ Let's make this "guest's", as you're touching the line right below. > - unsigned int receive_mask; > + sccb_mask_t receive_mask; > /* > * when false, we keep the same broken, backwards compatible behaviour as > * before; when true, we implement the architecture correctly. Needed for (...) > @@ -241,8 +241,8 @@ static void copy_mask(uint8_t *dst, uint8_t *src, > uint16_t dst_len, > > static void read_event_data(SCLPEventFacility *ef, SCCB *sccb) > { > - unsigned int sclp_active_selection_mask; > - unsigned int sclp_cp_receive_mask; > + sccb_mask_t sclp_active_selection_mask = 0; Why do you need to initialize this now? > + sccb_mask_t sclp_cp_receive_mask; > > ReadEventData *red = (ReadEventData *) sccb; > > @@ -284,7 +284,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB > *sccb) > { > WriteEventMask *we_mask = (WriteEventMask *) sccb; > uint16_t mask_length = be16_to_cpu(we_mask->mask_length); > - uint32_t tmp_mask; > + sccb_mask_t tmp_mask = 0; Same here. > > if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) || > ((mask_length != 4) && !ef->allow_all_mask_sizes)) { (...) > diff --git a/include/hw/s390x/event-facility.h > b/include/hw/s390x/event-facility.h > index 5119b9b..0a8b47a 100644 > --- a/include/hw/s390x/event-facility.h > +++ b/include/hw/s390x/event-facility.h > @@ -28,12 +28,14 @@ > #define SCLP_EVENT_SIGNAL_QUIESCE 0x1d > > /* SCLP event masks */ > -#define SCLP_EVENT_MASK_SIGNAL_QUIESCE 0x00000008 > -#define SCLP_EVENT_MASK_MSG_ASCII 0x00000040 > -#define SCLP_EVENT_MASK_CONFIG_MGT_DATA 0x10000000 > -#define SCLP_EVENT_MASK_OP_CMD 0x80000000 > -#define SCLP_EVENT_MASK_MSG 0x40000000 > -#define SCLP_EVENT_MASK_PMSGCMD 0x00800000 > +#define SCLPEVMSK(T) (1ULL << (sizeof(sccb_mask_t) * 8 - (T))) SCLP_EVMASK() would be a bit more readable, I think. > + > +#define SCLP_EVENT_MASK_OP_CMD SCLPEVMSK(SCLP_EVENT_OPRTNS_COMMAND) > +#define SCLP_EVENT_MASK_MSG SCLPEVMSK(SCLP_EVENT_MESSAGE) > +#define SCLP_EVENT_MASK_CONFIG_MGT_DATA SCLPEVMSK(SCLP_EVENT_CONFIG_MGT_DATA) > +#define SCLP_EVENT_MASK_PMSGCMD SCLPEVMSK(SCLP_EVENT_PMSGCMD) > +#define SCLP_EVENT_MASK_MSG_ASCII > SCLPEVMSK(SCLP_EVENT_ASCII_CONSOLE_DATA) > +#define SCLP_EVENT_MASK_SIGNAL_QUIESCE SCLPEVMSK(SCLP_EVENT_SIGNAL_QUIESCE) > > #define SCLP_UNCONDITIONAL_READ 0x00 > #define SCLP_SELECTIVE_READ 0x01