[kvm-unit-tests RFC 0/3] s390x: Improve console handling

[kvm-unit-tests RFC 0/3] s390x: Improve console handling

[Janosch Frank]

Console IO is and has been in a state of "works for me". I don't think
that will change soon since there's no need for a proper console
driver when all we want is the ability to print or read a line at a
time.

However since input is only supported on the ASCII console I was
forced to use it on the HMC. The HMC generally does not add a \r on a
\n so each line doesn't start at column 0. It's time to finally fix
that.

Also, since there are environments that only provide the line-mode
console it's time to add line-mode input to properly support them.

Janosch Frank (3):
  lib: s390x: sclp: Add carriage return to line feed
  lib: s390x: sclp: Clear ASCII screen on setup
  lib: s390x: sclp: Add line mode input handling

 lib/s390x/sclp-console.c | 191 +++++++++++++++++++++++++++++++++++----
 lib/s390x/sclp.h         |  26 +++++-
 s390x/run                |   2 +-
 3 files changed, 197 insertions(+), 22 deletions(-)

-- 
2.34.1

[kvm-unit-tests RFC 1/3] lib: s390x: sclp: Add carriage return to line feed

[Janosch Frank]

Without the \r the output of the ASCII console takes a lot of
additional effort to read in comparison to the line mode console.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/sclp-console.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
index 19c74e46..384080b0 100644
--- a/lib/s390x/sclp-console.c
+++ b/lib/s390x/sclp-console.c
@@ -97,14 +97,27 @@ static void sclp_print_ascii(const char *str)
 {
 	int len = strlen(str);
 	WriteEventData *sccb = (void *)_sccb;
+	char *str_dest = (char *)&sccb->msg;
+	int i = 0;
 
 	sclp_mark_busy();
 	memset(sccb, 0, sizeof(*sccb));
+
+	for (; i < len; i++) {
+		*str_dest = str[i];
+		str_dest++;
+		/* Add a \r to the \n */
+		if (str[i] == '\n') {
+			*str_dest = '\r';
+			str_dest++;
+		}
+	}
+
+	len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;
 	sccb->h.length = offsetof(WriteEventData, msg) + len;
 	sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
 	sccb->ebh.length = sizeof(EventBufferHeader) + len;
 	sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
-	memcpy(&sccb->msg, str, len);
 
 	sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
 }
-- 
2.34.1

[kvm-unit-tests RFC 2/3] lib: s390x: sclp: Clear ASCII screen on setup

[Janosch Frank]

In contrast to the line-mode console the ASCII console will retain
previously written text on a reboot. So let's clear the console on
setup so only our text will be displayed. To not clear the whole
screen when running under QEMU we switch the run command to the line
mode console.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/sclp-console.c | 2 ++
 s390x/run                | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
index 384080b0..534d3443 100644
--- a/lib/s390x/sclp-console.c
+++ b/lib/s390x/sclp-console.c
@@ -233,6 +233,8 @@ void sclp_console_setup(void)
 {
 	/* We send ASCII and line mode. */
 	sclp_write_event_mask(0, SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG);
+	/* Hard terminal reset to clear screen */
+	sclp_print_ascii("\ec");
 }
 
 void sclp_print(const char *str)
diff --git a/s390x/run b/s390x/run
index f1111dbd..68f8e733 100755
--- a/s390x/run
+++ b/s390x/run
@@ -28,7 +28,7 @@ fi
 M='-machine s390-ccw-virtio'
 M+=",accel=$ACCEL"
 command="$qemu -nodefaults -nographic $M"
-command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"
+command+=" -chardev stdio,id=con0 -device sclplmconsole,chardev=con0"
 command+=" -kernel"
 command="$(panic_cmd) $(migration_cmd) $(timeout_cmd) $command"
 
-- 
2.34.1

[kvm-unit-tests RFC 3/3] lib: s390x: sclp: Add line mode input handling

[Janosch Frank]

Time to add line-mode input so we can use input handling under LPAR if
there's no access to a ASCII console.

Line-mode IO is pretty wild and the documentation could be improved a
lot. Hence I've copied the input parsing functions from the s390-tools
zipl code.

For some reason output is a type 2 event but input is a type 1
event. This also means that the input and output structures are
different from each other.

The input can consist of multiple structures which don't contain text
data before the input text data is reached. Hence we need a bunch of
search functions to retrieve a pointer to the text data.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/sclp-console.c | 174 ++++++++++++++++++++++++++++++++++-----
 lib/s390x/sclp.h         |  26 +++++-
 2 files changed, 180 insertions(+), 20 deletions(-)

diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
index 534d3443..2149098e 100644
--- a/lib/s390x/sclp-console.c
+++ b/lib/s390x/sclp-console.c
@@ -85,6 +85,41 @@ static uint8_t _ascebc[256] = {
      0x90, 0x3F, 0x3F, 0x3F, 0x3F, 0xEA, 0x3F, 0xFF
 };
 
+static const uint8_t _ebcasc[] = {
+    0x00, 0x01, 0x02, 0x03, 0x07, 0x09, 0x07, 0x7F,
+    0x07, 0x07, 0x07, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+    0x10, 0x11, 0x12, 0x13, 0x07, 0x0A, 0x08, 0x07,
+    0x18, 0x19, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07,
+    0x07, 0x07, 0x1C, 0x07, 0x07, 0x0A, 0x17, 0x1B,
+    0x07, 0x07, 0x07, 0x07, 0x07, 0x05, 0x06, 0x07,
+    0x07, 0x07, 0x16, 0x07, 0x07, 0x07, 0x07, 0x04,
+    0x07, 0x07, 0x07, 0x07, 0x14, 0x15, 0x07, 0x1A,
+    0x20, 0xFF, 0x83, 0x84, 0x85, 0xA0, 0x07, 0x86,
+    0x87, 0xA4, 0x5B, 0x2E, 0x3C, 0x28, 0x2B, 0x21,
+    0x26, 0x82, 0x88, 0x89, 0x8A, 0xA1, 0x8C, 0x07,
+    0x8D, 0xE1, 0x5D, 0x24, 0x2A, 0x29, 0x3B, 0x5E,
+    0x2D, 0x2F, 0x07, 0x8E, 0x07, 0x07, 0x07, 0x8F,
+    0x80, 0xA5, 0x07, 0x2C, 0x25, 0x5F, 0x3E, 0x3F,
+    0x07, 0x90, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07,
+    0x70, 0x60, 0x3A, 0x23, 0x40, 0x27, 0x3D, 0x22,
+    0x07, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67,
+    0x68, 0x69, 0xAE, 0xAF, 0x07, 0x07, 0x07, 0xF1,
+    0xF8, 0x6A, 0x6B, 0x6C, 0x6D, 0x6E, 0x6F, 0x70,
+    0x71, 0x72, 0xA6, 0xA7, 0x91, 0x07, 0x92, 0x07,
+    0xE6, 0x7E, 0x73, 0x74, 0x75, 0x76, 0x77, 0x78,
+    0x79, 0x7A, 0xAD, 0xAB, 0x07, 0x07, 0x07, 0x07,
+    0x9B, 0x9C, 0x9D, 0xFA, 0x07, 0x07, 0x07, 0xAC,
+    0xAB, 0x07, 0xAA, 0x7C, 0x07, 0x07, 0x07, 0x07,
+    0x7B, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47,
+    0x48, 0x49, 0x07, 0x93, 0x94, 0x95, 0xA2, 0x07,
+    0x7D, 0x4A, 0x4B, 0x4C, 0x4D, 0x4E, 0x4F, 0x50,
+    0x51, 0x52, 0x07, 0x96, 0x81, 0x97, 0xA3, 0x98,
+    0x5C, 0xF6, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58,
+    0x59, 0x5A, 0xFD, 0x07, 0x99, 0x07, 0x07, 0x07,
+    0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+    0x38, 0x39, 0x07, 0x07, 0x9A, 0x07, 0x07, 0x07,
+};
+
 static char lm_buff[120];
 static unsigned char lm_buff_off;
 static struct spinlock lm_buff_lock;
@@ -221,7 +256,8 @@ static void sclp_write_event_mask(int receive_mask, int send_mask)
 
 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);
+	sclp_write_event_mask(SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_OPCMD,
+			      SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG);
 }
 
 static void sclp_console_disable_read(void)
@@ -256,37 +292,137 @@ void sclp_print(const char *str)
 	sclp_print_lm(str);
 }
 
+static char *console_read_ascii(struct EventBufferHeader *ebh, int *len)
+{
+	struct ReadEventDataAsciiConsole *evdata = (void *)ebh;
+	const int max_event_buffer_len = SCCB_SIZE - offsetof(ReadEventDataAsciiConsole, ebh);
+	const int event_buffer_ascii_recv_header_len = offsetof(ReadEventDataAsciiConsole, data);
+
+	assert(ebh->length <= max_event_buffer_len);
+	assert(ebh->length > event_buffer_ascii_recv_header_len);
+
+	*len = ebh->length - event_buffer_ascii_recv_header_len;
+	return evdata->data;
+}
+
+
+static struct gds_vector *sclp_find_gds_vector(void *start, void *end, uint16_t id)
+{
+	struct gds_vector *v;
+
+	for (v = start; (void *)v < end; v = (void *)v + v->length)
+		if (v->gds_id == id)
+			return v;
+	return NULL;
+}
+
+static struct gds_subvector *sclp_eval_selfdeftextmsg(struct gds_subvector *sv)
+{
+	void *end;
+
+	end = (void *)sv + sv->length;
+	for (sv = sv + 1; (void *)sv < end; sv = (void *)sv + sv->length)
+		if (sv->key == 0x30)
+			return sv;
+	return NULL;
+}
+
+static struct gds_subvector *sclp_eval_textcmd(struct gds_vector *v)
+{
+	struct gds_subvector *sv;
+	void *end;
+
+	end = (void *)v + v->length;
+	for (sv = (struct gds_subvector *)(v + 1); (void *)sv < end;
+	     sv = (void *)sv + sv->length)
+		if (sv->key == GDS_KEY_SELFDEFTEXTMSG)
+			return sclp_eval_selfdeftextmsg(sv);
+	return NULL;
+}
+
+static struct gds_subvector *sclp_eval_cpmsu(struct gds_vector *v)
+{
+	void *end;
+
+	end = (void *)v + v->length;
+	for (v = v + 1; (void *)v < end; v = (void *)v + v->length)
+		if (v->gds_id == GDS_ID_TEXTCMD)
+			return sclp_eval_textcmd(v);
+	return NULL;
+}
+
+static struct gds_subvector *sclp_eval_mdsmu(struct gds_vector *v)
+{
+	v = sclp_find_gds_vector(v + 1, (void *)v + v->length, GDS_ID_CPMSU);
+	if (v)
+		return sclp_eval_cpmsu(v);
+	return NULL;
+}
+
+static char *console_read_lm(struct EventBufferHeader *ebh, int *len)
+{
+	struct gds_vector *v = (void *)ebh + sizeof(*ebh);
+	struct gds_subvector *sv;
+
+	v = sclp_find_gds_vector(v, (void *)ebh + ebh->length,
+				 GDS_ID_MDSMU);
+	if (!v)
+		return NULL;
+
+	sv = sclp_eval_mdsmu(v);
+	if (!sv)
+		return NULL;
+
+	*len = sv->length - (sizeof(*sv));
+	return (char *)(sv + 1);
+}
+
+static void ebc_to_asc(char *data, int len)
+{
+	int i;
+
+	 for (i = 0; i < len; i++)
+		 data[i] = _ebcasc[(uint8_t)data[i]];
+}
+
 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;
+	struct SCCBHeader *sccb = (struct SCCBHeader *)_sccb;
+	struct EventBufferHeader *ebh = (void *)_sccb + sizeof(struct SCCBHeader);
+	char *data;
+	int ret = -1, len;
 
 	sclp_console_enable_read();
 
 	sclp_mark_busy();
-	memset(sccb, 0, SCCB_SIZE);
-	sccb->h.length = PAGE_SIZE;
-	sccb->h.function_code = SCLP_UNCONDITIONAL_READ;
-	sccb->h.control_mask[2] = SCLP_CM2_VARIABLE_LENGTH_RESPONSE;
+	memset(_sccb, 0, SCCB_SIZE);
+	sccb->length = PAGE_SIZE;
+	sccb->function_code = SCLP_UNCONDITIONAL_READ;
+	sccb->control_mask[2] = SCLP_CM2_VARIABLE_LENGTH_RESPONSE;
 
 	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_TYPE_DATA_STREAM_FOLLOWS) {
-		ret = -1;
+	if (sccb->response_code == SCLP_RC_NO_EVENT_BUFFERS_STORED)
+		goto out;
+
+	switch (ebh->type) {
+	case SCLP_EVENT_OP_CMD:
+		data = console_read_lm(ebh, &len);
+		if (data)
+			ebc_to_asc(data, len);
+		break;
+	case SCLP_EVENT_ASCII_CONSOLE_DATA:
+		data = console_read_ascii(ebh, &len);
+		break;
+	default:
 		goto out;
 	}
 
-	assert(sccb->ebh.length <= max_event_buffer_len);
-	assert(sccb->ebh.length > event_buffer_ascii_recv_header_len);
+	if (!data)
+		goto out;
 
-	read_buf_length = sccb->ebh.length - event_buffer_ascii_recv_header_len;
-
-	assert(read_buf_length <= sizeof(read_buf));
-	memcpy(read_buf, sccb->data, read_buf_length);
+	assert(len <= sizeof(read_buf));
+	memcpy(read_buf, data, len);
 
 	read_index = 0;
 	ret = 0;
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index 853529bf..9f026648 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -225,6 +225,7 @@ typedef struct SCCB {
 } __attribute__((packed)) SCCB;
 
 /* SCLP event types */
+#define SCLP_EVENT_OP_CMD			0x01
 #define SCLP_EVENT_ASCII_CONSOLE_DATA           0x1a
 #define SCLP_EVENT_SIGNAL_QUIESCE               0x1d
 
@@ -232,6 +233,7 @@ typedef struct SCCB {
 #define SCLP_EVENT_MASK_SIGNAL_QUIESCE          0x00000008
 #define SCLP_EVENT_MASK_MSG_ASCII               0x00000040
 #define SCLP_EVENT_MASK_MSG          		0x40000000
+#define SCLP_EVENT_MASK_OPCMD          		0x80000000
 
 #define SCLP_UNCONDITIONAL_READ                 0x00
 #define SCLP_SELECTIVE_READ                     0x01
@@ -295,6 +297,23 @@ struct mdb {
 	struct mto mto;
 } __attribute__((packed));
 
+/* vector keys and ids */
+#define GDS_ID_MDSMU		0x1310
+#define GDS_ID_CPMSU		0x1212
+#define GDS_ID_TEXTCMD		0x1320
+#define GDS_KEY_SELFDEFTEXTMSG	0x31
+#define EBC_MDB                 0xd4c4c240
+
+struct gds_vector {
+	uint16_t     length;
+	uint16_t     gds_id;
+} __attribute__((packed));
+
+struct gds_subvector {
+	uint8_t      length;
+	uint8_t      key;
+} __attribute__((packed));
+
 typedef struct EventBufferHeader {
 	uint16_t length;
 	uint8_t  type;
@@ -319,12 +338,17 @@ typedef struct ReadEventData {
 
 #define SCLP_EVENT_ASCII_TYPE_DATA_STREAM_FOLLOWS 0
 typedef struct ReadEventDataAsciiConsole {
-	SCCBHeader h;
 	EventBufferHeader ebh;
 	uint8_t type;
 	char data[];
 } __attribute__((packed)) ReadEventDataAsciiConsole;
 
+struct ReadEventDataLMConsole {
+	SCCBHeader h;
+	EventBufferHeader ebh;
+	struct gds_vector v[];
+};
+
 extern char _sccb[];
 void sclp_setup_int(void);
 void sclp_handle_ext(void);
-- 
2.34.1

Re: [kvm-unit-tests RFC 1/3] lib: s390x: sclp: Add carriage return to line feed

[Claudio Imbrenda]

On Fri, 30 Jun 2023 14:54:47 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Without the \r the output of the ASCII console takes a lot of
> additional effort to read in comparison to the line mode console.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/sclp-console.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> index 19c74e46..384080b0 100644
> --- a/lib/s390x/sclp-console.c
> +++ b/lib/s390x/sclp-console.c
> @@ -97,14 +97,27 @@ static void sclp_print_ascii(const char *str)
>  {
>  	int len = strlen(str);
>  	WriteEventData *sccb = (void *)_sccb;
> +	char *str_dest = (char *)&sccb->msg;
> +	int i = 0;
>  
>  	sclp_mark_busy();
>  	memset(sccb, 0, sizeof(*sccb));
> +
> +	for (; i < len; i++) {
> +		*str_dest = str[i];
> +		str_dest++;
> +		/* Add a \r to the \n */
> +		if (str[i] == '\n') {
> +			*str_dest = '\r';
> +			str_dest++;
> +		}
> +	}
> +
> +	len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;

some strings will therefore potentially overflow the SCCB

sclp_print() refuses to print more than 2kB, with this patch that limit
could potentially be crossed

can you please briefly explain in a comment why that is ok? (or maybe
that is not ok? then fix it somehow :) )

>  	sccb->h.length = offsetof(WriteEventData, msg) + len;
>  	sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
>  	sccb->ebh.length = sizeof(EventBufferHeader) + len;
>  	sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
> -	memcpy(&sccb->msg, str, len);
>  
>  	sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
>  }

Re: [kvm-unit-tests RFC 2/3] lib: s390x: sclp: Clear ASCII screen on setup

[Claudio Imbrenda]

On Fri, 30 Jun 2023 14:54:48 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> In contrast to the line-mode console the ASCII console will retain

what's the problem with that?

> previously written text on a reboot. So let's clear the console on
> setup so only our text will be displayed. To not clear the whole
> screen when running under QEMU we switch the run command to the line
> mode console.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/sclp-console.c | 2 ++
>  s390x/run                | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> index 384080b0..534d3443 100644
> --- a/lib/s390x/sclp-console.c
> +++ b/lib/s390x/sclp-console.c
> @@ -233,6 +233,8 @@ void sclp_console_setup(void)
>  {
>  	/* We send ASCII and line mode. */
>  	sclp_write_event_mask(0, SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG);
> +	/* Hard terminal reset to clear screen */
> +	sclp_print_ascii("\ec");
>  }
>  
>  void sclp_print(const char *str)
> diff --git a/s390x/run b/s390x/run
> index f1111dbd..68f8e733 100755
> --- a/s390x/run
> +++ b/s390x/run
> @@ -28,7 +28,7 @@ fi
>  M='-machine s390-ccw-virtio'
>  M+=",accel=$ACCEL"
>  command="$qemu -nodefaults -nographic $M"
> -command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"
> +command+=" -chardev stdio,id=con0 -device sclplmconsole,chardev=con0"
>  command+=" -kernel"
>  command="$(panic_cmd) $(migration_cmd) $(timeout_cmd) $command"
>  

Re: [kvm-unit-tests RFC 2/3] lib: s390x: sclp: Clear ASCII screen on setup

[Janosch Frank]

On 6/30/23 17:25, Claudio Imbrenda wrote:
> On Fri, 30 Jun 2023 14:54:48 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> In contrast to the line-mode console the ASCII console will retain
> 
> what's the problem with that?

It can be a bit hard to read since you need to find the line where the 
old output ends and the new one starts.


I don't insist on this patch being included, the \r and sclp line mode 
input patches give me enough usability.

Re: [kvm-unit-tests RFC 1/3] lib: s390x: sclp: Add carriage return to line feed

[Janosch Frank]

On 6/30/23 17:12, Claudio Imbrenda wrote:
> On Fri, 30 Jun 2023 14:54:47 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Without the \r the output of the ASCII console takes a lot of
>> additional effort to read in comparison to the line mode console.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   lib/s390x/sclp-console.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
>> index 19c74e46..384080b0 100644
>> --- a/lib/s390x/sclp-console.c
>> +++ b/lib/s390x/sclp-console.c
>> @@ -97,14 +97,27 @@ static void sclp_print_ascii(const char *str)
>>   {
>>   	int len = strlen(str);
>>   	WriteEventData *sccb = (void *)_sccb;
>> +	char *str_dest = (char *)&sccb->msg;
>> +	int i = 0;
>>   
>>   	sclp_mark_busy();
>>   	memset(sccb, 0, sizeof(*sccb));
>> +
>> +	for (; i < len; i++) {
>> +		*str_dest = str[i];
>> +		str_dest++;
>> +		/* Add a \r to the \n */
>> +		if (str[i] == '\n') {
>> +			*str_dest = '\r';
>> +			str_dest++;
>> +		}
>> +	}
>> +
>> +	len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;
> 
> some strings will therefore potentially overflow the SCCB
> 
> sclp_print() refuses to print more than 2kB, with this patch that limit
> could potentially be crossed
> 
> can you please briefly explain in a comment why that is ok? (or maybe
> that is not ok? then fix it somehow :) )

I'd like to see someone find a useful application for printing 2kb in a 
single printf() call.

Anyway, I could truncate the ASCII after the 2KB limit when adding the \r.

I'm wondering how the line-mode console interprets the \r. If it ignores 
it, then we could also convert to \n\r for both consoles and check for 
2kb when converting.

Re: [kvm-unit-tests RFC 2/3] lib: s390x: sclp: Clear ASCII screen on setup

[Claudio Imbrenda]

On Mon, 3 Jul 2023 13:36:05 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 6/30/23 17:25, Claudio Imbrenda wrote:
> > On Fri, 30 Jun 2023 14:54:48 +0000
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> In contrast to the line-mode console the ASCII console will retain  
> > 
> > what's the problem with that?  
> 
> It can be a bit hard to read since you need to find the line where the 
> old output ends and the new one starts.
> 
> 
> I don't insist on this patch being included, the \r and sclp line mode 
> input patches give me enough usability.

make it a compile-time option? (default off)

then you won't need to change the run script, and you can still clear
the console when you need it (HMC)

Re: [kvm-unit-tests RFC 1/3] lib: s390x: sclp: Add carriage return to line feed

[Claudio Imbrenda]

On Mon, 3 Jul 2023 13:46:29 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 6/30/23 17:12, Claudio Imbrenda wrote:
> > On Fri, 30 Jun 2023 14:54:47 +0000
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> Without the \r the output of the ASCII console takes a lot of
> >> additional effort to read in comparison to the line mode console.
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >>   lib/s390x/sclp-console.c | 15 ++++++++++++++-
> >>   1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> >> index 19c74e46..384080b0 100644
> >> --- a/lib/s390x/sclp-console.c
> >> +++ b/lib/s390x/sclp-console.c
> >> @@ -97,14 +97,27 @@ static void sclp_print_ascii(const char *str)
> >>   {
> >>   	int len = strlen(str);
> >>   	WriteEventData *sccb = (void *)_sccb;
> >> +	char *str_dest = (char *)&sccb->msg;
> >> +	int i = 0;
> >>   
> >>   	sclp_mark_busy();
> >>   	memset(sccb, 0, sizeof(*sccb));
> >> +
> >> +	for (; i < len; i++) {
> >> +		*str_dest = str[i];
> >> +		str_dest++;
> >> +		/* Add a \r to the \n */
> >> +		if (str[i] == '\n') {
> >> +			*str_dest = '\r';
> >> +			str_dest++;
> >> +		}
> >> +	}
> >> +
> >> +	len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;  
> > 
> > some strings will therefore potentially overflow the SCCB
> > 
> > sclp_print() refuses to print more than 2kB, with this patch that limit
> > could potentially be crossed
> > 
> > can you please briefly explain in a comment why that is ok? (or maybe
> > that is not ok? then fix it somehow :) )  
> 
> I'd like to see someone find a useful application for printing 2kb in a 
> single printf() call.

I'm with you there

> 
> Anyway, I could truncate the ASCII after the 2KB limit when adding the \r.

sounds like a plan

> 
> I'm wondering how the line-mode console interprets the \r. If it ignores 
> it, then we could also convert to \n\r for both consoles and check for 
> 2kb when converting.

is \n\r the right order? what about \r\n ?

Re: [kvm-unit-tests RFC 2/3] lib: s390x: sclp: Clear ASCII screen on setup

[Janosch Frank]

On 7/3/23 13:57, Claudio Imbrenda wrote:
> On Mon, 3 Jul 2023 13:36:05 +0200
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 6/30/23 17:25, Claudio Imbrenda wrote:
>>> On Fri, 30 Jun 2023 14:54:48 +0000
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>    
>>>> In contrast to the line-mode console the ASCII console will retain
>>>
>>> what's the problem with that?
>>
>> It can be a bit hard to read since you need to find the line where the
>> old output ends and the new one starts.
>>
>>
>> I don't insist on this patch being included, the \r and sclp line mode
>> input patches give me enough usability.
> 
> make it a compile-time option? (default off)
> 
> then you won't need to change the run script, and you can still clear
> the console when you need it (HMC)

Well, the compile-time check made me remember that we have 
lib/s390x/hardware.c. We can simply check for HOST_IS_LPAR and clear 
accordingly.