On 02/23/2018 01:51 PM, Claudio Imbrenda wrote:
> Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
> we only supported sclp event masks with a size of exactly 4 bytes, even
> though the architecture allows the guests to set up sclp event masks
> from 1 to 1021 bytes in length.
> After that patch, the behaviour was almost compliant, but some issues
> were still remaining, in particular regarding the handling of selective
> reads and migration.
>
> When setting the sclp event mask, a mask size is also specified. Until
> now we only considered the size in order to decide which bits to save
> in the internal state. On the other hand, when a guest performs a
> selective read, it sends a mask, but it does not specify a size; the
> implied size is the size of the last mask that has been set.
>
> Specifying bits in the mask of selective read that are not available in
> the internal mask should return an error, and bits past the end of the
> mask should obviously be ignored. This can only be achieved by keeping
> track of the lenght of the mask.
>
> The mask length is thus now part of the internal state that needs to be
> migrated.
>
> This patch fixes the handling of selective reads, whose size will now
> match the length of the event mask, as per architecture.
>
> While the default behaviour is to be compliant with the architecture,
> when using older machine models the old broken behaviour is selected
> (allowing only masks of size exactly 4), in order to be able to migrate
> toward older versions.
>
> Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event
> masks")
> Signed-off-by: Claudio Imbrenda
> ---
> hw/s390x/event-facility.c | 91
> +++---
> hw/s390x/s390-virtio-ccw.c | 8 +++-
> 2 files changed, 85 insertions(+), 14 deletions(-)
>
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 155a694..b7cd075 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -31,6 +31,15 @@ struct SCLPEventFacility {
> SCLPEventsBus sbus;
> /* guest' receive mask */
> unsigned int receive_mask;
> +/*
> + * when false, we keep the same broken, backwards compatible behaviour as
> + * before, allowing only masks of size exactly 4; when true, we implement
> + * the architecture correctly, allowing all valid mask sizes. Needed for
> + * migration toward older versions.
> + */
> +bool allow_all_mask_sizes;
> +/* length of the receive mask */
> +uint16_t mask_length;
> };
>
> /* return true if any child has event pending set */
> @@ -220,6 +229,17 @@ static uint16_t
> handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb,
> return rc;
> }
>
> +/* copy up to dst_len bytes and fill the rest of dst with zeroes */
you just moved this function but shouldnt it be
src_len bytes and fill will zeroes until dst_len?
> +static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> + uint16_t src_len)
> +{
> +int i;
> +
> +for (i = 0; i < dst_len; i++) {
> +dst[i] = i < src_len ? src[i] : 0;
> +}
> +}
> +
> static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
> {
> unsigned int sclp_active_selection_mask;
> @@ -240,7 +260,9 @@ static void read_event_data(SCLPEventFacility *ef, SCCB
> *sccb)
> sclp_active_selection_mask = sclp_cp_receive_mask;
> break;
> case SCLP_SELECTIVE_READ:
> -sclp_active_selection_mask = be32_to_cpu(red->mask);
> +copy_mask((uint8_t *)_active_selection_mask, (uint8_t
> *)>mask,
> + sizeof(sclp_active_selection_mask), ef->mask_length);
> +sclp_active_selection_mask = be32_to_cpu(sclp_active_selection_mask);
> if (!sclp_cp_receive_mask ||
> (sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
> sccb->h.response_code =
> @@ -259,24 +281,14 @@ out:
> return;
> }
>
> -/* copy up to dst_len bytes and fill the rest of dst with zeroes */
> -static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> - uint16_t src_len)
> -{
> -int i;
> -
> -for (i = 0; i < dst_len; i++) {
> -dst[i] = i < src_len ? src[i] : 0;
> -}
> -}
> -
> 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;
>
> -if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX)) {
> +if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) ||
> +((mask_length != 4) && !ef->allow_all_mask_sizes)) {
> sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
> goto out;
> }
> @@ -301,6 +313,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
> *sccb)
>