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

Janosch Frank
2022-04-12T15:33:03Z

On 4/11/22 12:07, Nico Boehr wrote:
> Add a basic implementation for reading from the SCLP ACII console. The goal of
> this is to support migration tests on s390x. To know when the migration has been
> finished, we need to listen for a newline on our console.
>
> Hence, this implementation is focused on the SCLP ASCII console of QEMU and
> currently won't work under e.g. LPAR.

How much pain would it be to add the line mode read?

>
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
[..]
>
> +static void sclp_console_enable_read(void)
> +{
> + sclp_write_event_mask(SCLP_EVENT_MASK_MSG_ASCII, SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG);
> +}
> +
> +static void sclp_console_disable_read(void)
> +{
> + sclp_write_event_mask(0, SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG);
> +}
> +
> void sclp_console_setup(void)
> {
> - sclp_set_write_mask();
> + /* We send ASCII and line mode. */
> + sclp_write_event_mask(0, SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG);
> }
>
> void sclp_print(const char *str)
> @@ -227,3 +240,59 @@ void sclp_print(const char *str)
> sclp_print_ascii(str);
> sclp_print_lm(str);
> }
> +
> +#define SCLP_EVENT_ASCII_DATA_STREAM_FOLLOWS 0

-> sclp.h

> +
> +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;

Reverse Christmas tree
The const int variables are all caps because they are essentially constants?

> +
> + sclp_console_enable_read();
> +
> + sclp_mark_busy();
> + memset(sccb, 0, 4096);

sizeof(*sccb)

> + sccb->h.length = PAGE_SIZE;
> + sccb->h.function_code = SCLP_UNCONDITIONAL_READ;
> + sccb->h.control_mask[2] = 0x80;
> +
> + sclp_service_call(SCLP_CMD_READ_EVENT_DATA, sccb);
> +
> + if ((sccb->h.response_code == SCLP_RC_NO_EVENT_BUFFERS_STORED) ||
> + (sccb->ebh.type != SCLP_EVENT_ASCII_CONSOLE_DATA) ||
> + (sccb->type != SCLP_EVENT_ASCII_DATA_STREAM_FOLLOWS)) {
> + ret = -1;
> + goto out;
> + }
> +
> + assert(sccb->ebh.length <= MAX_EVENT_BUFFER_LEN);
> + assert(sccb->ebh.length > EVENT_BUFFER_ASCII_RECV_HEADER_LEN);
> +
> + 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;
> +
> +out:
> + sclp_console_disable_read();
> +
> + return ret;
> +}
> +
> +int __getchar(void)
> +{
> + int ret;
> +
> + if (read_index >= read_buf_end) {
> + ret = console_refill_read_buffer();
> + if (ret < 0) {
> + return ret;
> + }
> + }
> +
> + return read_buf[read_index++];
> +}
> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
> index fead007a6037..5bd1741d721d 100644
> --- a/lib/s390x/sclp.h
> +++ b/lib/s390x/sclp.h
> @@ -313,6 +313,13 @@ typedef struct ReadEventData {
> uint32_t mask;
> } __attribute__((packed)) ReadEventData;
>
> +typedef struct ReadEventDataAsciiConsole {
> + SCCBHeader h;
> + EventBufferHeader ebh;
> + uint8_t type;
> + char data[];
> +} __attribute__((packed)) ReadEventDataAsciiConsole;
> +
> extern char _sccb[];
> void sclp_setup_int(void);
> void sclp_handle_ext(void);
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 53b0fe044fe7..62e197cb93d7 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -71,6 +71,7 @@ cflatobjs += lib/alloc_phys.o
> cflatobjs += lib/alloc_page.o
> cflatobjs += lib/vmalloc.o
> cflatobjs += lib/alloc_phys.o
> +cflatobjs += lib/getchar.o
> cflatobjs += lib/s390x/io.o
> cflatobjs += lib/s390x/stack.o
> cflatobjs += lib/s390x/sclp.o