[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [Qemu-devel] [PATCH] s390x/event-facility: variable-len
From: |
Cornelia Huck |
Subject: |
Re: [qemu-s390x] [Qemu-devel] [PATCH] s390x/event-facility: variable-length event masks |
Date: |
Tue, 17 Oct 2017 09:56:24 +0200 |
On Mon, 16 Oct 2017 13:11:19 -0400
"Jason J. Herne" <address@hidden> wrote:
> 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.
Great. I have verified that this works with Linux under tcg as well.
>
> >> ...
> >> 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.
It should not be hard to extend the masks on top of this patch, I
think. Let's see what comes.
>
> >> ...
> >
> > 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.
You're welcome :)