[kvm-unit-tests PATCH v3 0/4] s390x: add migration test support

[kvm-unit-tests PATCH v3 0/4] s390x: add migration test support

[Nico Boehr]

Changelog from v2:
---
- Cosmetic fixes, typo in comments (thanks Thomas)
- Improve migration prompt when tests are run manually (thanks Thomas)
- Whitespace fixes

This series depends on my SIGP store additional status series to have access to
the guarded-storage and vector related defines
("[kvm-unit-tests PATCH v3 0/2] s390x: Add tests for SIGP store adtl status").

Add migration test support for s390x.

arm and powerpc already support basic migration tests.

If a test is in the migration group, it can print "migrate" on its console. This
will cause it to be migrated to a new QEMU instance. When migration is finished,
the test will be able to read a newline from its standard input.

We need the following pieces for this to work under s390x:

* read support for the sclp console. This can be very basic, it doesn't even
  have to read anything useful, we just need to know something happened on
  the console.
* s390/run adjustments to call the migration helper script.

This series adds basic migration tests for s390x, which I plan to extend
further.

Nico Boehr (4):
  lib: s390x: add support for SCLP console read
  s390x: add support for migration tests
  s390x: don't run migration tests under PV
  s390x: add basic migration test

 lib/s390x/sclp-console.c |  79 ++++++++++++++++--
 lib/s390x/sclp.h         |   8 ++
 s390x/Makefile           |   2 +
 s390x/migration.c        | 172 +++++++++++++++++++++++++++++++++++++++
 s390x/run                |   7 +-
 s390x/unittests.cfg      |   5 ++
 scripts/s390x/func.bash  |   2 +-
 7 files changed, 267 insertions(+), 8 deletions(-)
 create mode 100644 s390x/migration.c

-- 
2.31.1

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

[Nico Boehr]

Add a basic implementation for reading from the SCLP ASCII 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.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 lib/s390x/sclp-console.c | 79 +++++++++++++++++++++++++++++++++++++---
 lib/s390x/sclp.h         |  8 ++++
 s390x/Makefile           |  1 +
 3 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
index fa36a6a42381..8c4bf68cbbab 100644
--- a/lib/s390x/sclp-console.c
+++ b/lib/s390x/sclp-console.c
@@ -89,6 +89,10 @@ static char lm_buff[120];
 static unsigned char lm_buff_off;
 static struct spinlock lm_buff_lock;
 
+static char read_buf[4096];
+static int read_index = sizeof(read_buf) - 1;
+static int read_buf_end = 0;
+
 static void sclp_print_ascii(const char *str)
 {
 	int len = strlen(str);
@@ -185,7 +189,7 @@ static void sclp_print_lm(const char *str)
  * indicating which messages the control program (we) want(s) to
  * send/receive.
  */
-static void sclp_set_write_mask(void)
+static void sclp_write_event_mask(int receive_mask, int send_mask)
 {
 	WriteEventMask *sccb = (void *)_sccb;
 
@@ -195,18 +199,27 @@ static void sclp_set_write_mask(void)
 	sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
 	sccb->mask_length = sizeof(sccb_mask_t);
 
-	/* For now we don't process sclp input. */
-	sccb->cp_receive_mask = 0;
-	/* We send ASCII and line mode. */
-	sccb->cp_send_mask = SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG;
+	sccb->cp_receive_mask = receive_mask;
+	sccb->cp_send_mask = send_mask;
 
 	sclp_service_call(SCLP_CMD_WRITE_EVENT_MASK, sccb);
 	assert(sccb->h.response_code == SCLP_RC_NORMAL_COMPLETION);
 }
 
+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,57 @@ void sclp_print(const char *str)
 	sclp_print_ascii(str);
 	sclp_print_lm(str);
 }
+
+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, SCCB_SIZE);
+	sccb->h.length = PAGE_SIZE;
+	sccb->h.function_code = SCLP_UNCONDITIONAL_READ;
+	sccb->h.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;
+		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;
+	ret = 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..e48a5a3df20b 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -313,6 +313,14 @@ typedef struct ReadEventData {
 	uint32_t mask;
 } __attribute__((packed)) 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;
+
 extern char _sccb[];
 void sclp_setup_int(void);
 void sclp_handle_ext(void);
diff --git a/s390x/Makefile b/s390x/Makefile
index c11f6efbd767..f38f442b9cb1 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -75,6 +75,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
-- 
2.31.1

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

[Claudio Imbrenda]

On Wed, 20 Apr 2022 15:45:54 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> Add a basic implementation for reading from the SCLP ASCII 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.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  lib/s390x/sclp-console.c | 79 +++++++++++++++++++++++++++++++++++++---
>  lib/s390x/sclp.h         |  8 ++++
>  s390x/Makefile           |  1 +
>  3 files changed, 82 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> index fa36a6a42381..8c4bf68cbbab 100644
> --- a/lib/s390x/sclp-console.c
> +++ b/lib/s390x/sclp-console.c
> @@ -89,6 +89,10 @@ static char lm_buff[120];
>  static unsigned char lm_buff_off;
>  static struct spinlock lm_buff_lock;
>  
> +static char read_buf[4096];
> +static int read_index = sizeof(read_buf) - 1;
> +static int read_buf_end = 0;
> +
>  static void sclp_print_ascii(const char *str)
>  {
>  	int len = strlen(str);
> @@ -185,7 +189,7 @@ static void sclp_print_lm(const char *str)
>   * indicating which messages the control program (we) want(s) to
>   * send/receive.
>   */
> -static void sclp_set_write_mask(void)
> +static void sclp_write_event_mask(int receive_mask, int send_mask)
>  {
>  	WriteEventMask *sccb = (void *)_sccb;
>  
> @@ -195,18 +199,27 @@ static void sclp_set_write_mask(void)
>  	sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
>  	sccb->mask_length = sizeof(sccb_mask_t);
>  
> -	/* For now we don't process sclp input. */
> -	sccb->cp_receive_mask = 0;
> -	/* We send ASCII and line mode. */
> -	sccb->cp_send_mask = SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG;
> +	sccb->cp_receive_mask = receive_mask;
> +	sccb->cp_send_mask = send_mask;
>  
>  	sclp_service_call(SCLP_CMD_WRITE_EVENT_MASK, sccb);
>  	assert(sccb->h.response_code == SCLP_RC_NORMAL_COMPLETION);
>  }
>  
> +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,57 @@ void sclp_print(const char *str)
>  	sclp_print_ascii(str);
>  	sclp_print_lm(str);
>  }
> +
> +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, SCCB_SIZE);
> +	sccb->h.length = PAGE_SIZE;
> +	sccb->h.function_code = SCLP_UNCONDITIONAL_READ;
> +	sccb->h.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;
> +		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;
> +	ret = 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..e48a5a3df20b 100644
> --- a/lib/s390x/sclp.h
> +++ b/lib/s390x/sclp.h
> @@ -313,6 +313,14 @@ typedef struct ReadEventData {
>  	uint32_t mask;
>  } __attribute__((packed)) 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;
> +
>  extern char _sccb[];
>  void sclp_setup_int(void);
>  void sclp_handle_ext(void);
> diff --git a/s390x/Makefile b/s390x/Makefile
> index c11f6efbd767..f38f442b9cb1 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -75,6 +75,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

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

[Janosch Frank]

On 4/20/22 15:45, Nico Boehr wrote:
> Add a basic implementation for reading from the SCLP ASCII 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.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>   lib/s390x/sclp-console.c | 79 +++++++++++++++++++++++++++++++++++++---
>   lib/s390x/sclp.h         |  8 ++++
>   s390x/Makefile           |  1 +
>   3 files changed, 82 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> index fa36a6a42381..8c4bf68cbbab 100644
> --- a/lib/s390x/sclp-console.c
> +++ b/lib/s390x/sclp-console.c
> @@ -89,6 +89,10 @@ static char lm_buff[120];
>   static unsigned char lm_buff_off;
>   static struct spinlock lm_buff_lock;
>   
> +static char read_buf[4096];
> +static int read_index = sizeof(read_buf) - 1;
> +static int read_buf_end = 0;
> +
>   static void sclp_print_ascii(const char *str)
>   {
>   	int len = strlen(str);
> @@ -185,7 +189,7 @@ static void sclp_print_lm(const char *str)
>    * indicating which messages the control program (we) want(s) to
>    * send/receive.
>    */
> -static void sclp_set_write_mask(void)
> +static void sclp_write_event_mask(int receive_mask, int send_mask)
>   {
>   	WriteEventMask *sccb = (void *)_sccb;
>   
> @@ -195,18 +199,27 @@ static void sclp_set_write_mask(void)
>   	sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
>   	sccb->mask_length = sizeof(sccb_mask_t);
>   
> -	/* For now we don't process sclp input. */
> -	sccb->cp_receive_mask = 0;
> -	/* We send ASCII and line mode. */
> -	sccb->cp_send_mask = SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG;
> +	sccb->cp_receive_mask = receive_mask;
> +	sccb->cp_send_mask = send_mask;
>   
>   	sclp_service_call(SCLP_CMD_WRITE_EVENT_MASK, sccb);
>   	assert(sccb->h.response_code == SCLP_RC_NORMAL_COMPLETION);
>   }
>   
> +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,57 @@ void sclp_print(const char *str)
>   	sclp_print_ascii(str);
>   	sclp_print_lm(str);
>   }
> +
> +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, SCCB_SIZE);
> +	sccb->h.length = PAGE_SIZE;
> +	sccb->h.function_code = SCLP_UNCONDITIONAL_READ;
> +	sccb->h.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;
> +		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;

Isn't this more like a length of the current read buffer contents?

> +
> +	assert(read_buf_end <= sizeof(read_buf));
> +	memcpy(read_buf, sccb->data, read_buf_end);
> +
> +	read_index = 0;
> +	ret = 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..e48a5a3df20b 100644
> --- a/lib/s390x/sclp.h
> +++ b/lib/s390x/sclp.h
> @@ -313,6 +313,14 @@ typedef struct ReadEventData {
>   	uint32_t mask;
>   } __attribute__((packed)) ReadEventData;
>   
> +#define SCLP_EVENT_ASCII_TYPE_DATA_STREAM_FOLLOWS 0

Hrm, I'm not completely happy with the naming here since I confused it 
to the ebh->type when looking up the constants. But now I understand why 
you chose it.

> +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 c11f6efbd767..f38f442b9cb1 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -75,6 +75,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

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

[Nico Boehr]

On Thu, 2022-04-21 at 16:29 +0200, Janosch Frank wrote:
> 
[...]
> > diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> > index fa36a6a42381..8c4bf68cbbab 100644
> > --- a/lib/s390x/sclp-console.c
> > +++ b/lib/s390x/sclp-console.c
[...]
> > +       read_buf_end = sccb->ebh.length -
> > event_buffer_ascii_recv_header_len;
> 
> Isn't this more like a length of the current read buffer contents?

Right, thanks, length is a much better name. 

[...]
> > diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
> > index fead007a6037..e48a5a3df20b 100644
> > --- a/lib/s390x/sclp.h
> > +++ b/lib/s390x/sclp.h
> > @@ -313,6 +313,14 @@ typedef struct ReadEventData {
> >         uint32_t mask;
> >   } __attribute__((packed)) ReadEventData;
> >   
> > +#define SCLP_EVENT_ASCII_TYPE_DATA_STREAM_FOLLOWS 0
> 
> Hrm, I'm not completely happy with the naming here since I confused
> it 
> to the ebh->type when looking up the constants. But now I understand
> why 
> you chose it.

Yeah, it sure is confusing.

Maybe it is better if we leave out the "type" entirely, but this might
make it harder to understand where it's coming from:
SCLP_ASCII_RECEIVE_DATA_STREAM_FOLLOWS

Another alternative I thought about is using enums, it won't fix the
naming, but at least it might be clearer to which type it belongs.

Let me know what you think.

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

[Janosch Frank]

On 4/22/22 09:50, Nico Boehr wrote:
> On Thu, 2022-04-21 at 16:29 +0200, Janosch Frank wrote:
>>
> [...]
>>> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
>>> index fa36a6a42381..8c4bf68cbbab 100644
>>> --- a/lib/s390x/sclp-console.c
>>> +++ b/lib/s390x/sclp-console.c
> [...]
>>> +       read_buf_end = sccb->ebh.length -
>>> event_buffer_ascii_recv_header_len;
>>
>> Isn't this more like a length of the current read buffer contents?
> 
> Right, thanks, length is a much better name.
> 
> [...]
>>> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
>>> index fead007a6037..e48a5a3df20b 100644
>>> --- a/lib/s390x/sclp.h
>>> +++ b/lib/s390x/sclp.h
>>> @@ -313,6 +313,14 @@ typedef struct ReadEventData {
>>>          uint32_t mask;
>>>    } __attribute__((packed)) ReadEventData;
>>>    
>>> +#define SCLP_EVENT_ASCII_TYPE_DATA_STREAM_FOLLOWS 0
>>
>> Hrm, I'm not completely happy with the naming here since I confused
>> it
>> to the ebh->type when looking up the constants. But now I understand
>> why
>> you chose it.
> 
> Yeah, it sure is confusing.
> 
> Maybe it is better if we leave out the "type" entirely, but this might
> make it harder to understand where it's coming from:
> SCLP_ASCII_RECEIVE_DATA_STREAM_FOLLOWS
> 
> Another alternative I thought about is using enums, it won't fix the
> naming, but at least it might be clearer to which type it belongs.
> 
> Let me know what you think.

It should be fine as is. I don't expect that we have to touch this file 
very often.

[kvm-unit-tests PATCH v3 2/4] s390x: add support for migration tests

[Nico Boehr]

Now that we have SCLP console read support, run our tests with migration_cmd, so
we can get migration support on s390x.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 s390x/run | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/s390x/run b/s390x/run
index 064ecd1b337a..2bcdabbaa14f 100755
--- a/s390x/run
+++ b/s390x/run
@@ -25,7 +25,7 @@ M+=",accel=$ACCEL"
 command="$qemu -nodefaults -nographic $M"
 command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"
 command+=" -kernel"
-command="$(timeout_cmd) $command"
+command="$(migration_cmd) $(timeout_cmd) $command"
 
 # We return the exit code via stdout, not via the QEMU return code
 run_qemu_status $command "$@"
-- 
2.31.1

[kvm-unit-tests PATCH v3 3/4] s390x: don't run migration tests under PV

[Nico Boehr]

PV doesn't support migration, so don't run the migration tests there.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Acked-by: Thomas Huth <thuth@redhat.com>
---
 s390x/run               | 5 +++++
 scripts/s390x/func.bash | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/s390x/run b/s390x/run
index 2bcdabbaa14f..24138f6803be 100755
--- a/s390x/run
+++ b/s390x/run
@@ -20,6 +20,11 @@ if [ "${1: -7}" = ".pv.bin" ] || [ "${TESTNAME: -3}" = "_PV" ] && [ "$ACCEL" = "
 	exit 2
 fi
 
+if [ "${1: -7}" = ".pv.bin" ] || [ "${TESTNAME: -3}" = "_PV" ] && [ "$MIGRATION" = "yes" ]; then
+	echo "Migration isn't supported under Protected Virtualization"
+	exit 2
+fi
+
 M='-machine s390-ccw-virtio'
 M+=",accel=$ACCEL"
 command="$qemu -nodefaults -nographic $M"
diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
index bf799a567c3b..2a941bbb0794 100644
--- a/scripts/s390x/func.bash
+++ b/scripts/s390x/func.bash
@@ -21,7 +21,7 @@ function arch_cmd_s390x()
 	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 
 	# run PV test case
-	if [ "$ACCEL" = 'tcg' ]; then
+	if [ "$ACCEL" = 'tcg' ] || find_word "migration" "$groups"; then
 		return
 	fi
 	kernel=${kernel%.elf}.pv.bin
-- 
2.31.1

[kvm-unit-tests PATCH v3 4/4] s390x: add basic migration test

[Nico Boehr]

Add a basic migration test for s390x. This tests the guarded-storage registers
and vector registers are preserved on migration.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 s390x/Makefile      |   1 +
 s390x/migration.c   | 172 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   5 ++
 3 files changed, 178 insertions(+)
 create mode 100644 s390x/migration.c

diff --git a/s390x/Makefile b/s390x/Makefile
index f38f442b9cb1..5336ed8ae0b4 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -30,6 +30,7 @@ tests += $(TEST_DIR)/spec_ex-sie.elf
 tests += $(TEST_DIR)/firq.elf
 tests += $(TEST_DIR)/epsw.elf
 tests += $(TEST_DIR)/adtl-status.elf
+tests += $(TEST_DIR)/migration.elf
 
 pv-tests += $(TEST_DIR)/pv-diags.elf
 
diff --git a/s390x/migration.c b/s390x/migration.c
new file mode 100644
index 000000000000..2f31fc53bf68
--- /dev/null
+++ b/s390x/migration.c
@@ -0,0 +1,172 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Migration Test for s390x
+ *
+ * Copyright IBM Corp. 2022
+ *
+ * Authors:
+ *  Nico Boehr <nrb@linux.ibm.com>
+ */
+#include <libcflat.h>
+#include <asm/arch_def.h>
+#include <asm/vector.h>
+#include <asm/barrier.h>
+#include <gs.h>
+#include <bitops.h>
+#include <smp.h>
+
+static struct gs_cb gs_cb;
+static struct gs_epl gs_epl;
+
+/* set by CPU1 to signal it has completed */
+static int flag_thread_complete;
+/* set by CPU0 to signal migration has completed */
+static int flag_migration_complete;
+
+static void write_gs_regs(void)
+{
+	const unsigned long gs_area = 0x2000000;
+	const unsigned long gsc = 25; /* align = 32 M, section size = 512K */
+
+	gs_cb.gsd = gs_area | gsc;
+	gs_cb.gssm = 0xfeedc0ffe;
+	gs_cb.gs_epl_a = (uint64_t) &gs_epl;
+
+	load_gs_cb(&gs_cb);
+}
+
+static void check_gs_regs(void)
+{
+	struct gs_cb gs_cb_after_migration;
+
+	store_gs_cb(&gs_cb_after_migration);
+
+	report_prefix_push("guarded-storage registers");
+
+	report(gs_cb_after_migration.gsd == gs_cb.gsd, "gsd matches");
+	report(gs_cb_after_migration.gssm == gs_cb.gssm, "gssm matches");
+	report(gs_cb_after_migration.gs_epl_a == gs_cb.gs_epl_a, "gs_epl_a matches");
+
+	report_prefix_pop();
+}
+
+static void test_func(void)
+{
+	uint8_t expected_vec_contents[VEC_REGISTER_NUM][VEC_REGISTER_SIZE];
+	uint8_t actual_vec_contents[VEC_REGISTER_NUM][VEC_REGISTER_SIZE];
+	uint8_t *vec_reg;
+	int i;
+	int vec_result = 0;
+
+	for (i = 0; i < VEC_REGISTER_NUM; i++) {
+		vec_reg = &expected_vec_contents[i][0];
+		/* i+1 to avoid zero content */
+		memset(vec_reg, i + 1, VEC_REGISTER_SIZE);
+	}
+
+	ctl_set_bit(0, CTL0_VECTOR);
+	ctl_set_bit(2, CTL2_GUARDED_STORAGE);
+
+	write_gs_regs();
+
+	/*
+	 * It is important loading the vector/floating point registers and
+	 * comparing their contents occurs in the same inline assembly block.
+	 * Otherwise, the compiler is allowed to re-use the registers for
+	 * something else in between.
+	 * For this very reason, this also runs on a second CPU, so all the
+	 * complex console stuff can be done in C on the first CPU and here we
+	 * just need to wait for it to set the flag.
+	 */
+	asm inline(
+		"	.machine z13\n"
+		/* load vector registers: vlm handles at most 16 registers at a time */
+		"	vlm 0,15, 0(%[expected_vec_reg])\n"
+		"	vlm 16,31, 256(%[expected_vec_reg])\n"
+		/* inform CPU0 we are done, it will request migration */
+		"	mvhi %[flag_thread_complete], 1\n"
+		/* wait for migration to finish */
+		"0:	clfhsi %[flag_migration_complete], 1\n"
+		"	jnz 0b\n"
+		/*
+		 * store vector register contents in actual_vec_reg: vstm
+		 * handles at most 16 registers at a time
+		 */
+		"	vstm 0,15, 0(%[actual_vec_reg])\n"
+		"	vstm 16,31, 256(%[actual_vec_reg])\n"
+		/*
+		 * compare the contents in expected_vec_reg with actual_vec_reg:
+		 * clc handles at most 256 bytes at a time
+		 */
+		"	clc 0(256, %[expected_vec_reg]), 0(%[actual_vec_reg])\n"
+		"	jnz 1f\n"
+		"	clc 256(256, %[expected_vec_reg]), 256(%[actual_vec_reg])\n"
+		"	jnz 1f\n"
+		/* success */
+		"	mvhi %[vec_result], 1\n"
+		"1:"
+		:
+		: [expected_vec_reg] "a"(expected_vec_contents),
+		  [actual_vec_reg] "a"(actual_vec_contents),
+		  [flag_thread_complete] "Q"(flag_thread_complete),
+		  [flag_migration_complete] "Q"(flag_migration_complete),
+		  [vec_result] "Q"(vec_result)
+		: "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9",
+		  "v10", "v11", "v12", "v13", "v14", "v15", "v16", "v17", "v18",
+		  "v19", "v20", "v21", "v22", "v23", "v24", "v25", "v26", "v27",
+		  "v28", "v29", "v30", "v31", "cc", "memory"
+	);
+
+	report(vec_result, "vector contents match");
+
+	check_gs_regs();
+
+	report(stctg(0) & BIT(CTL0_VECTOR), "ctl0 vector bit set");
+	report(stctg(2) & BIT(CTL2_GUARDED_STORAGE), "ctl2 guarded-storage bit set");
+
+	ctl_clear_bit(0, CTL0_VECTOR);
+	ctl_clear_bit(2, CTL2_GUARDED_STORAGE);
+
+	flag_thread_complete = 1;
+}
+
+int main(void)
+{
+	struct psw psw;
+
+	/* don't say migrate here otherwise we will migrate right away */
+	report_prefix_push("migration");
+
+	if (smp_query_num_cpus() == 1) {
+		report_skip("need at least 2 cpus for this test");
+		goto done;
+	}
+
+	/* Second CPU does the actual tests */
+	psw.mask = extract_psw_mask();
+	psw.addr = (unsigned long)test_func;
+	smp_cpu_setup(1, psw);
+
+	/* wait for thread setup */
+	while(!flag_thread_complete)
+		mb();
+	flag_thread_complete = 0;
+
+	/* ask migrate_cmd to migrate (it listens for 'migrate') */
+	puts("Please migrate me, then press return\n");
+
+	/* wait for migration to finish, we will read a newline */
+	(void)getchar();
+
+	flag_migration_complete = 1;
+
+	/* wait for thread to complete assertions */
+	while(!flag_thread_complete)
+		mb();
+
+	smp_cpu_destroy(1);
+
+done:
+	report_prefix_pop();
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 256c71691adc..b456b2881448 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -171,3 +171,8 @@ file = adtl-status.elf
 smp = 2
 accel = tcg
 extra_params = -cpu qemu,gs=off,vx=off
+
+[migration]
+file = migration.elf
+groups = migration
+smp = 2
-- 
2.31.1

Re: [kvm-unit-tests PATCH v3 4/4] s390x: add basic migration test

[Claudio Imbrenda]

On Wed, 20 Apr 2022 15:45:57 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> Add a basic migration test for s390x. This tests the guarded-storage registers
> and vector registers are preserved on migration.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  s390x/Makefile      |   1 +
>  s390x/migration.c   | 172 ++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg |   5 ++
>  3 files changed, 178 insertions(+)
>  create mode 100644 s390x/migration.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index f38f442b9cb1..5336ed8ae0b4 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -30,6 +30,7 @@ tests += $(TEST_DIR)/spec_ex-sie.elf
>  tests += $(TEST_DIR)/firq.elf
>  tests += $(TEST_DIR)/epsw.elf
>  tests += $(TEST_DIR)/adtl-status.elf
> +tests += $(TEST_DIR)/migration.elf
>  
>  pv-tests += $(TEST_DIR)/pv-diags.elf
>  
> diff --git a/s390x/migration.c b/s390x/migration.c
> new file mode 100644
> index 000000000000..2f31fc53bf68
> --- /dev/null
> +++ b/s390x/migration.c
> @@ -0,0 +1,172 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Migration Test for s390x
> + *
> + * Copyright IBM Corp. 2022
> + *
> + * Authors:
> + *  Nico Boehr <nrb@linux.ibm.com>
> + */
> +#include <libcflat.h>
> +#include <asm/arch_def.h>
> +#include <asm/vector.h>
> +#include <asm/barrier.h>
> +#include <gs.h>
> +#include <bitops.h>
> +#include <smp.h>
> +
> +static struct gs_cb gs_cb;
> +static struct gs_epl gs_epl;
> +
> +/* set by CPU1 to signal it has completed */
> +static int flag_thread_complete;
> +/* set by CPU0 to signal migration has completed */
> +static int flag_migration_complete;
> +
> +static void write_gs_regs(void)
> +{
> +	const unsigned long gs_area = 0x2000000;
> +	const unsigned long gsc = 25; /* align = 32 M, section size = 512K */
> +
> +	gs_cb.gsd = gs_area | gsc;
> +	gs_cb.gssm = 0xfeedc0ffe;
> +	gs_cb.gs_epl_a = (uint64_t) &gs_epl;
> +
> +	load_gs_cb(&gs_cb);
> +}
> +
> +static void check_gs_regs(void)
> +{
> +	struct gs_cb gs_cb_after_migration;
> +
> +	store_gs_cb(&gs_cb_after_migration);
> +
> +	report_prefix_push("guarded-storage registers");
> +
> +	report(gs_cb_after_migration.gsd == gs_cb.gsd, "gsd matches");
> +	report(gs_cb_after_migration.gssm == gs_cb.gssm, "gssm matches");
> +	report(gs_cb_after_migration.gs_epl_a == gs_cb.gs_epl_a, "gs_epl_a matches");
> +
> +	report_prefix_pop();
> +}
> +
> +static void test_func(void)
> +{
> +	uint8_t expected_vec_contents[VEC_REGISTER_NUM][VEC_REGISTER_SIZE];
> +	uint8_t actual_vec_contents[VEC_REGISTER_NUM][VEC_REGISTER_SIZE];
> +	uint8_t *vec_reg;
> +	int i;
> +	int vec_result = 0;
> +
> +	for (i = 0; i < VEC_REGISTER_NUM; i++) {
> +		vec_reg = &expected_vec_contents[i][0];
> +		/* i+1 to avoid zero content */
> +		memset(vec_reg, i + 1, VEC_REGISTER_SIZE);
> +	}
> +
> +	ctl_set_bit(0, CTL0_VECTOR);
> +	ctl_set_bit(2, CTL2_GUARDED_STORAGE);
> +
> +	write_gs_regs();
> +
> +	/*
> +	 * It is important loading the vector/floating point registers and
> +	 * comparing their contents occurs in the same inline assembly block.
> +	 * Otherwise, the compiler is allowed to re-use the registers for
> +	 * something else in between.
> +	 * For this very reason, this also runs on a second CPU, so all the
> +	 * complex console stuff can be done in C on the first CPU and here we
> +	 * just need to wait for it to set the flag.
> +	 */
> +	asm inline(
> +		"	.machine z13\n"
> +		/* load vector registers: vlm handles at most 16 registers at a time */
> +		"	vlm 0,15, 0(%[expected_vec_reg])\n"
> +		"	vlm 16,31, 256(%[expected_vec_reg])\n"
> +		/* inform CPU0 we are done, it will request migration */
> +		"	mvhi %[flag_thread_complete], 1\n"
> +		/* wait for migration to finish */
> +		"0:	clfhsi %[flag_migration_complete], 1\n"
> +		"	jnz 0b\n"
> +		/*
> +		 * store vector register contents in actual_vec_reg: vstm
> +		 * handles at most 16 registers at a time
> +		 */
> +		"	vstm 0,15, 0(%[actual_vec_reg])\n"
> +		"	vstm 16,31, 256(%[actual_vec_reg])\n"
> +		/*
> +		 * compare the contents in expected_vec_reg with actual_vec_reg:
> +		 * clc handles at most 256 bytes at a time
> +		 */
> +		"	clc 0(256, %[expected_vec_reg]), 0(%[actual_vec_reg])\n"
> +		"	jnz 1f\n"
> +		"	clc 256(256, %[expected_vec_reg]), 256(%[actual_vec_reg])\n"
> +		"	jnz 1f\n"
> +		/* success */
> +		"	mvhi %[vec_result], 1\n"
> +		"1:"
> +		:
> +		: [expected_vec_reg] "a"(expected_vec_contents),
> +		  [actual_vec_reg] "a"(actual_vec_contents),
> +		  [flag_thread_complete] "Q"(flag_thread_complete),
> +		  [flag_migration_complete] "Q"(flag_migration_complete),
> +		  [vec_result] "Q"(vec_result)
> +		: "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9",
> +		  "v10", "v11", "v12", "v13", "v14", "v15", "v16", "v17", "v18",
> +		  "v19", "v20", "v21", "v22", "v23", "v24", "v25", "v26", "v27",
> +		  "v28", "v29", "v30", "v31", "cc", "memory"
> +	);
> +
> +	report(vec_result, "vector contents match");
> +
> +	check_gs_regs();
> +
> +	report(stctg(0) & BIT(CTL0_VECTOR), "ctl0 vector bit set");
> +	report(stctg(2) & BIT(CTL2_GUARDED_STORAGE), "ctl2 guarded-storage bit set");
> +
> +	ctl_clear_bit(0, CTL0_VECTOR);
> +	ctl_clear_bit(2, CTL2_GUARDED_STORAGE);
> +
> +	flag_thread_complete = 1;
> +}
> +
> +int main(void)
> +{
> +	struct psw psw;
> +
> +	/* don't say migrate here otherwise we will migrate right away */
> +	report_prefix_push("migration");
> +
> +	if (smp_query_num_cpus() == 1) {
> +		report_skip("need at least 2 cpus for this test");
> +		goto done;
> +	}
> +
> +	/* Second CPU does the actual tests */
> +	psw.mask = extract_psw_mask();
> +	psw.addr = (unsigned long)test_func;
> +	smp_cpu_setup(1, psw);
> +
> +	/* wait for thread setup */
> +	while(!flag_thread_complete)
> +		mb();
> +	flag_thread_complete = 0;
> +
> +	/* ask migrate_cmd to migrate (it listens for 'migrate') */
> +	puts("Please migrate me, then press return\n");
> +
> +	/* wait for migration to finish, we will read a newline */
> +	(void)getchar();
> +
> +	flag_migration_complete = 1;
> +
> +	/* wait for thread to complete assertions */
> +	while(!flag_thread_complete)
> +		mb();
> +
> +	smp_cpu_destroy(1);
> +
> +done:
> +	report_prefix_pop();
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 256c71691adc..b456b2881448 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -171,3 +171,8 @@ file = adtl-status.elf
>  smp = 2
>  accel = tcg
>  extra_params = -cpu qemu,gs=off,vx=off
> +
> +[migration]
> +file = migration.elf
> +groups = migration
> +smp = 2