Re: [kvm-unit-tests PATCH v1 1/4] lib: s390x: add support for SCLP console read

Nico Boehr
2022-04-13T14:41:39Z

On Tue, 2022-04-12 at 10:02 +0200, Thomas Huth wrote:
> On 11/04/2022 12.07, Nico Boehr wrote:
> > Add a basic implementation for reading from the SCLP ACII console.
> > The goal of
>
> s/ACII/ASCII/

Thanks, fixed.

[...]
> > diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> > index fa36a6a42381..8e22660bf25d 100644
[...]
> > +static int console_refill_read_buffer(void)
> > +{
> > +       const int MAX_EVENT_BUFFER_LEN = SCCB_SIZE -
> > offsetof(ReadEventDataAsciiConsole, ebh);
> > +       ReadEventDataAsciiConsole *sccb = (void *)_sccb;
> > +       const int EVENT_BUFFER_ASCII_RECV_HEADER_LEN = sizeof(sccb-
> > >ebh) + sizeof(sccb->type);
> > +       int ret = -1;
> > +
> > +       sclp_console_enable_read();
> > +
> > +       sclp_mark_busy();
> > +       memset(sccb, 0, 4096);
> > +       sccb->h.length = PAGE_SIZE;
> > +       sccb->h.function_code = SCLP_UNCONDITIONAL_READ;
> > +       sccb->h.control_mask[2] = 0x80;
>
> Add at least a comment about what the 0x80 means, please?

Oh yes, thanks. We already have a define for it which I will use
instead of the comment: SCLP_CM2_VARIABLE_LENGTH_RESPONSE

[...]
> > +
> > +       read_buf_end = sccb->ebh.length -
> > EVENT_BUFFER_ASCII_RECV_HEADER_LEN;
> > +
> > +       assert(read_buf_end <= sizeof(read_buf));
> > +       memcpy(read_buf, sccb->data, read_buf_end);
> > +
> > +       read_index = 0;
>
> Set "ret = 0" here?

Oooops, excellent catch. Thanks, fixed.