Re: [Qemu-devel] [PATCH] s390x/event-facility: variable-length event masks

[Jason J. Herne]

On 10/16/2017 09:44 AM, Cornelia Huck wrote:
> On Wed, 11 Oct 2017 09:39:53 -0400
> "Jason J. Herne" <address@hidden> wrote:
> > ...
>
> Out of curiousity: Do you have a guest that can verify this for mask
> lengths != 4? Given that the guest I wrote that one for back then is
> not publicly available...

I do have a guest OS that exercises larger mask lengths. Nothing 
publicly available however.

> > ...
> >   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;
> >   
> > -    /* Attention: We assume that Linux uses 4-byte masks, what it actually
> > -       does. Architecture allows for masks of variable size, though */
> > -    if (be16_to_cpu(we_mask->mask_length) != 4) {
> > +    if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX)) {
> >           sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
> >           goto out;
> >       }
> >   
> > +    /*
> > +     * Note: We currently only support masks up to 4 byte length;
> > +     *       the remainder is filled up with zeroes. Linux uses
> > +     *       a 4 byte mask length.
> > +     */
>
> Do you have any plans for extending this? Or is there no need?
>
> (I have to ask those questions, as the documentation is not publicly
> available...)

As of right now 4B is all that is actually needed for any of the masks. 
The reason to allow for 32 byte requests is for guest operating systems 
who are already planning for the future. Someday more than 4B may be 
used but we have no way to know if or when.

> > ...
>
> Patch looks reasonable as far as I can see (the documentation issue
> again...)
>
> Did you need to change much from the original patch?

Very little. Just a quick forward fit and test and all is well :)
Thanks for the original work. This saved me some time.

--
-- Jason J. Herne (address@hidden)