[kvm-unit-tests PATCH v5 00/10] s390x: Testing the Channel Subsystem I/O

[kvm-unit-tests PATCH v5 00/10] s390x: Testing the Channel Subsystem I/O

[Pierre Morel]

Goal of the series is to have a framwork to test Channel-Subsystem I/O with
QEMU/KVM.
  
To be able to support interrupt for CSS I/O and for SCLP we need to modify
the interrupt framework to allow re-entrant interruptions.
  
We add a registration for IRQ callbacks to the test programm to define its own
interrupt handler. We need to do special work under interrupt like acknoledging
the interrupt.
  
Being working on PSW bits to allow I/O interrupt, we define new PSW bits
in arch_def.h and use __ASSEMBLER__ define to be able to include this header
in an assembler source file.

This series presents four major tests:
- Enumeration:
        The CSS is enumerated using the STSCH instruction recursively on all
        potentially existing channels.
        Keeping the first channel found as a reference for future use.
        Checks STSCH
 
- Enable:
        If the enumeration succeeded the tests enables the reference
        channel with MSCH and verifies with STSCH that the channel is
        effectively enabled  
        Checks MSCH       
 
- Sense:
        If the channel is enabled this test sends a SENSE_ID command
        to the reference channel, analysing the answer and expecting
        the Control unit type being 0xc0ca
        Checks SSCH(READ) and IO-IRQ

- ping-pong:
        If the reference channel leads to the PONG device (0xc0ca),
        the test exchanges a string containing a 9 digit number with
        the PONG device and expecting this number to be incremented
        by the PONG device.
        Checks SSCH(WRITE)


Pierre Morel (10):
  s390x: saving regs for interrupts
  s390x: Use PSW bits definitions in cstart
  s390x: cr0: adding AFP-register control bit
  s390x: interrupt registration
  s390x: export the clock get_clock_ms() utility
  s390x: Library resources for CSS tests
  s390x: css: stsch, enumeration test
  s390x: css: msch, enable test
  s390x: css: ssch/tsch with sense and interrupt
  s390x: css: ping pong

 lib/s390x/asm/arch_def.h |  19 ++-
 lib/s390x/asm/time.h     |  36 +++++
 lib/s390x/css.h          | 277 +++++++++++++++++++++++++++++++
 lib/s390x/css_dump.c     | 157 ++++++++++++++++++
 lib/s390x/css_lib.c      |  55 +++++++
 lib/s390x/interrupt.c    |  22 ++-
 lib/s390x/interrupt.h    |   7 +
 s390x/Makefile           |   3 +
 s390x/css.c              | 341 +++++++++++++++++++++++++++++++++++++++
 s390x/cstart64.S         |  40 +++--
 s390x/intercept.c        |  11 +-
 s390x/unittests.cfg      |   4 +
 12 files changed, 946 insertions(+), 26 deletions(-)
 create mode 100644 lib/s390x/asm/time.h
 create mode 100644 lib/s390x/css.h
 create mode 100644 lib/s390x/css_dump.c
 create mode 100644 lib/s390x/css_lib.c
 create mode 100644 lib/s390x/interrupt.h
 create mode 100644 s390x/css.c

-- 
2.17.0

Changelog:
from v4 to v5
- add a patch to explicitely define the initial_cr0
  value
  (Janosch)
- add RB from Janosh on interrupt registration
- several formating, typo correction and removing
  unnecessary initialization in "linrary resources..."
  (Janosch)
- several formating and typo corrections on
  "stsch enumeration test"
  (Connie)
- reworking the msch test
  (Connie)
- reworking of ssch test, pack the sense-id structure
  (Connie)

from v3 to v4
- add RB from David and Thomas for patchs 
  (3) irq registration and (4) clock export
- rework the PSW bit definitions
  (Thomas)
- Suppress undef DEBUG from css_dump
  (Thomas)
- rework report() functions using new scheme
  (Thomas)
- suppress un-necessary report_info()
- more spelling corrections
- add a loop around enable bit testing
  (Connie)
- rework IRQ testing
  (Connie)
- Test data addresses to be under 2G
  (Connie)

from v2 to v3:
- Rework spelling
  (Connie)
- More descriptions
  (Connie)
- use __ASSEMBLER__ preprocessing to keep
  bits definitions and C structures in the same file
  (David)
- rename the new file clock.h as time.h
  (Janosch, David?)
- use registration for the IO interruption
  (David, Thomas)
- test the SCHIB to verify it has really be modified
  (Connie)
- Lot of simplifications in the tests
  (Connie)

from v1 to v2:
- saving floating point registers (David, Janosh)
- suppress unused PSW bits defintions (Janosh)
- added Thomas reviewed-by
- style and comments modifications (Connie, Janosh)
- moved get_clock_ms() into headers and use it (Thomas)
- separate header and library utility from tests
- Suppress traces, separate tests, make better usage of reports

[kvm-unit-tests PATCH v5 01/10] s390x: saving regs for interrupts

[Pierre Morel]

If we use multiple source of interrupts, for example, using SCLP
console to print information while using I/O interrupts, we need
to have a re-entrant register saving interruption handling.

Instead of saving at a static memory address, let's save the base
registers and the floating point registers on the stack.

Note that we keep the static register saving to recover from the
RESET tests.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 s390x/cstart64.S | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 9af6bb3..45da523 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -118,6 +118,25 @@ memsetxc:
 	lmg	%r0, %r15, GEN_LC_SW_INT_GRS
 	.endm
 
+/* Save registers on the stack, so we can have stacked interrupts. */
+	.macro SAVE_IRQ_REGS
+	slgfi   %r15, 15 * 8
+	stmg    %r0, %r14, 0(%r15)
+	slgfi   %r15, 16 * 8
+	.irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+	std	\i, \i * 8(%r15)
+	.endr
+	.endm
+
+	.macro RESTORE_IRQ_REGS
+	.irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+	ld	\i, \i * 8(%r15)
+	.endr
+	algfi   %r15, 16 * 8
+	lmg     %r0, %r14, 0(%r15)
+	algfi   %r15, 15 * 8
+	.endm
+
 .section .text
 /*
  * load_reset calling convention:
@@ -182,9 +201,9 @@ mcck_int:
 	lpswe	GEN_LC_MCCK_OLD_PSW
 
 io_int:
-	SAVE_REGS
+	SAVE_IRQ_REGS
 	brasl	%r14, handle_io_int
-	RESTORE_REGS
+	RESTORE_IRQ_REGS
 	lpswe	GEN_LC_IO_OLD_PSW
 
 svc_int:
-- 
2.17.0

[kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[Pierre Morel]

This patch defines the PSW bits EA/BA used to initialize the PSW masks
for exceptions.

Since some PSW mask definitions exist already in arch_def.h we add these
definitions there.
We move all PSW definitions together and protect assembler code against
C syntax.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h | 15 +++++++++++----
 s390x/cstart64.S         | 15 ++++++++-------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 15a4d49..69a8256 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -10,15 +10,21 @@
 #ifndef _ASM_S390X_ARCH_DEF_H_
 #define _ASM_S390X_ARCH_DEF_H_
 
+#define PSW_MASK_EXT			0x0100000000000000UL
+#define PSW_MASK_DAT			0x0400000000000000UL
+#define PSW_MASK_PSTATE			0x0001000000000000UL
+#define PSW_MASK_BA			0x0000000080000000UL
+#define PSW_MASK_EA			0x0000000100000000UL
+
+#define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
+
+#ifndef __ASSEMBLER__
+
 struct psw {
 	uint64_t	mask;
 	uint64_t	addr;
 };
 
-#define PSW_MASK_EXT			0x0100000000000000UL
-#define PSW_MASK_DAT			0x0400000000000000UL
-#define PSW_MASK_PSTATE			0x0001000000000000UL
-
 #define CR0_EXTM_SCLP			0X0000000000000200UL
 #define CR0_EXTM_EXTC			0X0000000000002000UL
 #define CR0_EXTM_EMGC			0X0000000000004000UL
@@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void)
 	return current_prefix;
 }
 
+#endif /* not __ASSEMBLER__ */
 #endif
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 45da523..2885a36 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -12,6 +12,7 @@
  */
 #include <asm/asm-offsets.h>
 #include <asm/sigp.h>
+#include <asm/arch_def.h>
 
 .section .init
 
@@ -214,19 +215,19 @@ svc_int:
 
 	.align	8
 reset_psw:
-	.quad	0x0008000180000000
+	.quad	PSW_EXCEPTION_MASK
 initial_psw:
-	.quad	0x0000000180000000, clear_bss_start
+	.quad	PSW_EXCEPTION_MASK, clear_bss_start
 pgm_int_psw:
-	.quad	0x0000000180000000, pgm_int
+	.quad	PSW_EXCEPTION_MASK, pgm_int
 ext_int_psw:
-	.quad	0x0000000180000000, ext_int
+	.quad	PSW_EXCEPTION_MASK, ext_int
 mcck_int_psw:
-	.quad	0x0000000180000000, mcck_int
+	.quad	PSW_EXCEPTION_MASK, mcck_int
 io_int_psw:
-	.quad	0x0000000180000000, io_int
+	.quad	PSW_EXCEPTION_MASK, io_int
 svc_int_psw:
-	.quad	0x0000000180000000, svc_int
+	.quad	PSW_EXCEPTION_MASK, svc_int
 initial_cr0:
 	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
 	.quad	0x0000000000040000
-- 
2.17.0

[kvm-unit-tests PATCH v5 03/10] s390x: cr0: adding AFP-register control bit

[Pierre Morel]

While adding the definition for the AFP-Register control bit, move all
existing definitions for CR0 out of the C zone to the assmbler zone to
keep the definitions concerning CR0 together.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h | 11 ++++++-----
 s390x/cstart64.S         |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 69a8256..863c2bf 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -18,6 +18,12 @@
 
 #define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
 
+#define CR0_EXTM_SCLP			0X0000000000000200UL
+#define CR0_EXTM_EXTC			0X0000000000002000UL
+#define CR0_EXTM_EMGC			0X0000000000004000UL
+#define CR0_EXTM_MASK			0X0000000000006200UL
+#define CR0_AFP_REG_CRTL		0x0000000000040000UL
+
 #ifndef __ASSEMBLER__
 
 struct psw {
@@ -25,11 +31,6 @@ struct psw {
 	uint64_t	addr;
 };
 
-#define CR0_EXTM_SCLP			0X0000000000000200UL
-#define CR0_EXTM_EXTC			0X0000000000002000UL
-#define CR0_EXTM_EMGC			0X0000000000004000UL
-#define CR0_EXTM_MASK			0X0000000000006200UL
-
 struct lowcore {
 	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
 	uint32_t	ext_int_param;			/* 0x0080 */
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 2885a36..3b59bd1 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -230,4 +230,4 @@ svc_int_psw:
 	.quad	PSW_EXCEPTION_MASK, svc_int
 initial_cr0:
 	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
-	.quad	0x0000000000040000
+	.quad	CR0_AFP_REG_CRTL
-- 
2.17.0

[kvm-unit-tests PATCH v5 04/10] s390x: interrupt registration

[Pierre Morel]

Let's make it possible to add and remove a custom io interrupt handler,
that can be used instead of the normal one.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/interrupt.c | 22 +++++++++++++++++++++-
 lib/s390x/interrupt.h |  7 +++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 lib/s390x/interrupt.h

diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 3a40cac..f6f0665 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -10,9 +10,9 @@
  * under the terms of the GNU Library General Public License version 2.
  */
 #include <libcflat.h>
-#include <asm/interrupt.h>
 #include <asm/barrier.h>
 #include <sclp.h>
+#include <interrupt.h>
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
@@ -144,12 +144,32 @@ void handle_mcck_int(void)
 		     stap(), lc->mcck_old_psw.addr);
 }
 
+static void (*io_int_func)(void);
+
 void handle_io_int(void)
 {
+	if (*io_int_func)
+		return (*io_int_func)();
 	report_abort("Unexpected io interrupt: on cpu %d at %#lx",
 		     stap(), lc->io_old_psw.addr);
 }
 
+int register_io_int_func(void (*f)(void))
+{
+	if (io_int_func)
+		return -1;
+	io_int_func = f;
+	return 0;
+}
+
+int unregister_io_int_func(void (*f)(void))
+{
+	if (io_int_func != f)
+		return -1;
+	io_int_func = NULL;
+	return 0;
+}
+
 void handle_svc_int(void)
 {
 	report_abort("Unexpected supervisor call interrupt: on cpu %d at %#lx",
diff --git a/lib/s390x/interrupt.h b/lib/s390x/interrupt.h
new file mode 100644
index 0000000..e945ef7
--- /dev/null
+++ b/lib/s390x/interrupt.h
@@ -0,0 +1,7 @@
+#ifndef __INTERRUPT_H
+#include <asm/interrupt.h>
+
+int register_io_int_func(void (*f)(void));
+int unregister_io_int_func(void (*f)(void));
+
+#endif
-- 
2.17.0

[kvm-unit-tests PATCH v5 05/10] s390x: export the clock get_clock_ms() utility

[Pierre Morel]

Let's move get_clock_ms() to lib/s390/asm/time.h, so it can be used in
multiple places.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/asm/time.h | 26 ++++++++++++++++++++++++++
 s390x/intercept.c    | 11 +----------
 2 files changed, 27 insertions(+), 10 deletions(-)
 create mode 100644 lib/s390x/asm/time.h

diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
new file mode 100644
index 0000000..25c7a3c
--- /dev/null
+++ b/lib/s390x/asm/time.h
@@ -0,0 +1,26 @@
+/*
+ * Clock utilities for s390
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *
+ * Copied from the s390/intercept test by:
+ *  Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2.
+ */
+#ifndef _ASM_S390X_TIME_H_
+#define _ASM_S390X_TIME_H_
+
+static inline uint64_t get_clock_ms(void)
+{
+	uint64_t clk;
+
+	asm volatile(" stck %0 " : : "Q"(clk) : "memory");
+
+	/* Bit 51 is incrememented each microsecond */
+	return (clk >> (63 - 51)) / 1000;
+}
+
+#endif
diff --git a/s390x/intercept.c b/s390x/intercept.c
index 5f46b82..2e38257 100644
--- a/s390x/intercept.c
+++ b/s390x/intercept.c
@@ -14,6 +14,7 @@
 #include <asm/interrupt.h>
 #include <asm/page.h>
 #include <asm/facility.h>
+#include <asm/time.h>
 
 static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
 
@@ -153,16 +154,6 @@ static void test_testblock(void)
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
 
-static uint64_t get_clock_ms(void)
-{
-	uint64_t clk;
-
-	asm volatile(" stck %0 " : : "Q"(clk) : "memory");
-
-	/* Bit 51 is incrememented each microsecond */
-	return (clk >> (63 - 51)) / 1000;
-}
-
 struct {
 	const char *name;
 	void (*func)(void);
-- 
2.17.0

[kvm-unit-tests PATCH v5 06/10] s390x: Library resources for CSS tests

[Pierre Morel]

These are the include and library utilities for the css tests patch
series.

Debug function can be activated by defining DEBUG_CSS before the
inclusion of the css.h header file.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/css.h      | 256 +++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/css_dump.c | 157 ++++++++++++++++++++++++++
 2 files changed, 413 insertions(+)
 create mode 100644 lib/s390x/css.h
 create mode 100644 lib/s390x/css_dump.c

diff --git a/lib/s390x/css.h b/lib/s390x/css.h
new file mode 100644
index 0000000..8144a21
--- /dev/null
+++ b/lib/s390x/css.h
@@ -0,0 +1,256 @@
+/*
+ * CSS definitions
+ *
+ * Copyright IBM, Corp. 2019
+ * Author: Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#ifndef CSS_H
+#define CSS_H
+
+#define CCW_F_CD	0x80
+#define CCW_F_CC	0x40
+#define CCW_F_SLI	0x20
+#define CCW_F_SKP	0x10
+#define CCW_F_PCI	0x08
+#define CCW_F_IDA	0x04
+#define CCW_F_S		0x02
+#define CCW_F_MIDA	0x01
+
+#define CCW_C_NOP	0x03
+#define CCW_C_TIC	0x08
+
+struct ccw1 {
+	unsigned char code;
+	unsigned char flags;
+	unsigned short count;
+	uint32_t data_address;
+} __attribute__ ((aligned(4)));
+
+#define ORB_M_KEY	0xf0000000
+#define ORB_F_SUSPEND	0x08000000
+#define ORB_F_STREAMING	0x04000000
+#define ORB_F_MODIFCTRL	0x02000000
+#define ORB_F_SYNC	0x01000000
+#define ORB_F_FORMAT	0x00800000
+#define ORB_F_PREFETCH	0x00400000
+#define ORB_F_INIT_IRQ	0x00200000
+#define ORB_F_ADDRLIMIT	0x00100000
+#define ORB_F_SUSP_IRQ	0x00080000
+#define ORB_F_TRANSPORT	0x00040000
+#define ORB_F_IDAW2	0x00020000
+#define ORB_F_IDAW_2K	0x00010000
+#define ORB_M_LPM	0x0000ff00
+#define ORB_F_LPM_DFLT	0x00008000
+#define ORB_F_ILSM	0x00000080
+#define ORB_F_CCW_IND	0x00000040
+#define ORB_F_ORB_EXT	0x00000001
+
+struct orb {
+	uint32_t intparm;
+	uint32_t ctrl;
+	uint32_t cpa;
+	uint32_t prio;
+	uint32_t reserved[4];
+} __attribute__ ((aligned(4)));
+
+struct scsw {
+	uint32_t ctrl;
+	uint32_t ccw_addr;
+	uint8_t  dev_stat;
+	uint8_t  sch_stat;
+	uint16_t count;
+};
+
+struct pmcw {
+	uint32_t intparm;
+#define PMCW_DNV        0x0001
+#define PMCW_ENABLE     0x0080
+	uint16_t flags;
+	uint16_t devnum;
+	uint8_t  lpm;
+	uint8_t  pnom;
+	uint8_t  lpum;
+	uint8_t  pim;
+	uint16_t mbi;
+	uint8_t  pom;
+	uint8_t  pam;
+	uint8_t  chpid[8];
+	uint32_t flags2;
+};
+
+struct schib {
+	struct pmcw pmcw;
+	struct scsw scsw;
+	uint8_t  md[12];
+} __attribute__ ((aligned(4)));
+
+struct irb {
+	struct scsw scsw;
+	uint32_t esw[5];
+	uint32_t ecw[8];
+	uint32_t emw[8];
+} __attribute__ ((aligned(4)));
+
+/* CSS low level access functions */
+
+static inline int ssch(unsigned long schid, struct orb *addr)
+{
+	register long long reg1 asm("1") = schid;
+	int cc;
+
+	asm volatile(
+		"	ssch	0(%2)\n"
+		"	ipm	%0\n"
+		"	srl	%0,28\n"
+		: "=d" (cc)
+		: "d" (reg1), "a" (addr), "m" (*addr)
+		: "cc", "memory");
+	return cc;
+}
+
+static inline int stsch(unsigned long schid, struct schib *addr)
+{
+	register unsigned long reg1 asm ("1") = schid;
+	int cc;
+
+	asm volatile(
+		"	stsch	0(%3)\n"
+		"	ipm	%0\n"
+		"	srl	%0,28"
+		: "=d" (cc), "=m" (*addr)
+		: "d" (reg1), "a" (addr)
+		: "cc");
+	return cc;
+}
+
+static inline int msch(unsigned long schid, struct schib *addr)
+{
+	register unsigned long reg1 asm ("1") = schid;
+	int cc;
+
+	asm volatile(
+		"	msch	0(%3)\n"
+		"	ipm	%0\n"
+		"	srl	%0,28"
+		: "=d" (cc), "=m" (*addr)
+		: "d" (reg1), "a" (addr)
+		: "cc");
+	return cc;
+}
+
+static inline int tsch(unsigned long schid, struct irb *addr)
+{
+	register unsigned long reg1 asm ("1") = schid;
+	int cc;
+
+	asm volatile(
+		"	tsch	0(%3)\n"
+		"	ipm	%0\n"
+		"	srl	%0,28"
+		: "=d" (cc), "=m" (*addr)
+		: "d" (reg1), "a" (addr)
+		: "cc");
+	return cc;
+}
+
+static inline int hsch(unsigned long schid)
+{
+	register unsigned long reg1 asm("1") = schid;
+	int cc;
+
+	asm volatile(
+		"	hsch\n"
+		"	ipm	%0\n"
+		"	srl	%0,28"
+		: "=d" (cc)
+		: "d" (reg1)
+		: "cc");
+	return cc;
+}
+
+static inline int xsch(unsigned long schid)
+{
+	register unsigned long reg1 asm("1") = schid;
+	int cc;
+
+	asm volatile(
+		"	xsch\n"
+		"	ipm	%0\n"
+		"	srl	%0,28"
+		: "=d" (cc)
+		: "d" (reg1)
+		: "cc");
+	return cc;
+}
+
+static inline int csch(unsigned long schid)
+{
+	register unsigned long reg1 asm("1") = schid;
+	int cc;
+
+	asm volatile(
+		"	csch\n"
+		"	ipm	%0\n"
+		"	srl	%0,28"
+		: "=d" (cc)
+		: "d" (reg1)
+		: "cc");
+	return cc;
+}
+
+static inline int rsch(unsigned long schid)
+{
+	register unsigned long reg1 asm("1") = schid;
+	int cc;
+
+	asm volatile(
+		"	rsch\n"
+		"	ipm	%0\n"
+		"	srl	%0,28"
+		: "=d" (cc)
+		: "d" (reg1)
+		: "cc");
+	return cc;
+}
+
+static inline int rchp(unsigned long chpid)
+{
+	register unsigned long reg1 asm("1") = chpid;
+	int cc;
+
+	asm volatile(
+		"	rchp\n"
+		"	ipm	%0\n"
+		"	srl	%0,28"
+		: "=d" (cc)
+		: "d" (reg1)
+		: "cc");
+	return cc;
+}
+
+/* Debug functions */
+char *dump_pmcw_flags(uint16_t f);
+char *dump_scsw_flags(uint32_t f);
+
+#ifdef DEBUG_CSS
+void dump_scsw(struct scsw *);
+void dump_irb(struct irb *irbp);
+void dump_schib(struct schib *sch);
+struct ccw *dump_ccw(struct ccw *cp);
+#else
+static inline void dump_scsw(struct scsw *scsw) {}
+static inline void dump_irb(struct irb *irbp) {}
+static inline void dump_pmcw(struct pmcw *p) {}
+static inline void dump_schib(struct schib *sch) {}
+static inline void dump_orb(struct orb *op) {}
+static inline struct ccw *dump_ccw(struct ccw *cp)
+{
+	return NULL;
+}
+#endif /* DEBUG_CSS */
+#endif
diff --git a/lib/s390x/css_dump.c b/lib/s390x/css_dump.c
new file mode 100644
index 0000000..e34c391
--- /dev/null
+++ b/lib/s390x/css_dump.c
@@ -0,0 +1,157 @@
+/*
+ * Channel subsystem structures dumping
+ *
+ * Copyright (c) 2019 IBM Corp.
+ *
+ * Authors:
+ *  Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ *
+ * Description:
+ * Provides the dumping functions for various structures used by subchannels:
+ * - ORB  : Operation request block, describes the I/O operation and points to
+ *          a CCW chain
+ * - CCW  : Channel Command Word, describes the data and flow control
+ * - IRB  : Interuption response Block, describes the result of an operation
+ *          holds a SCSW and model-dependent data.
+ * - SCHIB: SubCHannel Information Block composed of:
+ *   - SCSW: SubChannel Status Word, status of the channel.
+ *   - PMCW: Path Management Control Word
+ * You need the QEMU ccw-pong device in QEMU to answer the I/O transfers.
+ */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+
+#undef DEBUG_CSS
+#include <css.h>
+
+/*
+ * Try to have a more human representation of the SCSW flags:
+ * each letter in the two strings represent the first
+ * letter of the associated bit in the flag fields.
+ */
+static const char *scsw_str = "kkkkslccfpixuzen";
+static const char *scsw_str2 = "1SHCrshcsdsAIPSs";
+static char scsw_line[64] = {};
+
+char *dump_scsw_flags(uint32_t f)
+{
+	int i;
+
+	for (i = 0; i < 16; i++) {
+		if ((f << i) & 0x80000000)
+			scsw_line[i] = scsw_str[i];
+		else
+			scsw_line[i] = '_';
+	}
+	scsw_line[i] = ' ';
+	for (; i < 32; i++) {
+		if ((f << i) & 0x80000000)
+			scsw_line[i + 1] = scsw_str2[i - 16];
+		else
+			scsw_line[i + 1] = '_';
+	}
+	return scsw_line;
+}
+
+/*
+ * Try o have a more human representation of the PMCW flags
+ * each letter in the two strings he under represent the first
+ * letter of the associated bit in the flag.
+ */
+static const char *pmcw_str = "11iii111ellmmdtv";
+static char pcmw_line[32] = {};
+char *dump_pmcw_flags(uint16_t f)
+{
+	int i;
+
+	for (i = 0; i < 16; i++) {
+		if ((f << i) & 0x8000)
+			pcmw_line[i] = pmcw_str[i];
+		else
+			pcmw_line[i] = '_';
+	}
+	return pcmw_line;
+}
+
+#ifdef DEBUG_CSS
+void dump_scsw(struct scsw *s)
+{
+	dump_scsw_flags(s->ctrl);
+	printf("scsw->flags: %s\n", line);
+	printf("scsw->addr : %08x\n", s->addr);
+	printf("scsw->devs : %02x\n", s->devs);
+	printf("scsw->schs : %02x\n", s->schs);
+	printf("scsw->count: %04x\n", s->count);
+}
+
+void dump_irb(struct irb *irbp)
+{
+	int i;
+	uint32_t *p = (uint32_t *)irbp;
+
+	dump_scsw(&irbp->scsw);
+	for (i = 0; i < sizeof(*irbp)/sizeof(*p); i++, p++)
+		printf("irb[%02x] : %08x\n", i, *p);
+}
+
+void dump_pmcw(struct pmcw *p)
+{
+	int i;
+
+	printf("pmcw->intparm  : %08x\n", p->intparm);
+	printf("pmcw->flags    : %04x\n", p->flags);
+	dump_pmcw_flags(p->flags);
+	printf("pmcw->devnum   : %04x\n", p->devnum);
+	printf("pmcw->lpm      : %02x\n", p->lpm);
+	printf("pmcw->pnom     : %02x\n", p->pnom);
+	printf("pmcw->lpum     : %02x\n", p->lpum);
+	printf("pmcw->pim      : %02x\n", p->pim);
+	printf("pmcw->mbi      : %04x\n", p->mbi);
+	printf("pmcw->pom      : %02x\n", p->pom);
+	printf("pmcw->pam      : %02x\n", p->pam);
+	printf("pmcw->mbi      : %04x\n", p->mbi);
+	for (i = 0; i < 8; i++)
+		printf("pmcw->chpid[%d]: %02x\n", i, p->chpid[i]);
+	printf("pmcw->flags2  : %08x\n", p->flags2);
+}
+
+void dump_schib(struct schib *sch)
+{
+	struct pmcw *p = &sch->pmcw;
+	struct scsw *s = &sch->scsw;
+
+	printf("--SCHIB--\n");
+	dump_pmcw(p);
+	dump_scsw(s);
+}
+
+struct ccw *dump_ccw(struct ccw *cp)
+{
+	printf("CCW: code: %02x flags: %02x count: %04x data: %08x\n", cp->code,
+	    cp->flags, cp->count, cp->data);
+
+	if (cp->code == CCW_C_TIC)
+		return (struct ccw *)(long)cp->data;
+
+	return (cp->flags & CCW_F_CC) ? cp + 1 : NULL;
+}
+
+void dump_orb(struct orb *op)
+{
+	struct ccw *cp;
+
+	printf("ORB: intparm : %08x\n", op->intparm);
+	printf("ORB: ctrl    : %08x\n", op->ctrl);
+	printf("ORB: prio    : %08x\n", op->prio);
+	cp = (struct ccw *)(long) (op->cpa);
+	while (cp)
+		cp = dump_ccw(cp);
+}
+
+#endif
-- 
2.17.0

[kvm-unit-tests PATCH v5 07/10] s390x: css: stsch, enumeration test

[Pierre Morel]

First step for testing the channel subsystem is to enumerate the css and
retrieve the css devices.

This tests the success of STSCH I/O instruction, we do not test the
reaction of the VM for an instruction with wrong parameters.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/css.h     |  1 +
 s390x/Makefile      |  2 +
 s390x/css.c         | 91 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  4 ++
 4 files changed, 98 insertions(+)
 create mode 100644 s390x/css.c

diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 8144a21..448e597 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -82,6 +82,7 @@ struct pmcw {
 	uint8_t  chpid[8];
 	uint32_t flags2;
 };
+#define PMCW_CHANNEL_TYPE(pmcw) (pmcw->flags2 >> 21)
 
 struct schib {
 	struct pmcw pmcw;
diff --git a/s390x/Makefile b/s390x/Makefile
index ddb4b48..baebf18 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -17,6 +17,7 @@ tests += $(TEST_DIR)/stsi.elf
 tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
+tests += $(TEST_DIR)/css.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
@@ -51,6 +52,7 @@ cflatobjs += lib/s390x/sclp-console.o
 cflatobjs += lib/s390x/interrupt.o
 cflatobjs += lib/s390x/mmu.o
 cflatobjs += lib/s390x/smp.o
+cflatobjs += lib/s390x/css_dump.o
 
 OBJDIRS += lib/s390x
 
diff --git a/s390x/css.c b/s390x/css.c
new file mode 100644
index 0000000..cb33e00
--- /dev/null
+++ b/s390x/css.c
@@ -0,0 +1,91 @@
+/*
+ * Channel Subsystem tests
+ *
+ * Copyright (c) 2019 IBM Corp
+ *
+ * Authors:
+ *  Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2.
+ */
+
+#include <libcflat.h>
+#include <alloc_phys.h>
+#include <asm/page.h>
+#include <string.h>
+#include <interrupt.h>
+#include <asm/arch_def.h>
+#include <asm/time.h>
+
+#include <css.h>
+
+#define SID_ONE		0x00010000
+
+static struct schib schib;
+static int test_device_sid;
+
+static void test_enumerate(void)
+{
+	struct pmcw *pmcw = &schib.pmcw;
+	int cc;
+	int scn;
+	int scn_found = 0;
+	int dev_found = 0;
+
+	for (scn = 0; scn < 0xffff; scn++) {
+		cc = stsch(scn|SID_ONE, &schib);
+		switch (cc) {
+		case 0:		/* 0 means SCHIB stored */
+			break;
+		case 3:		/* 3 means no more channels */
+			goto out;
+		default:	/* 1 or 2 should never happened for STSCH */
+			report(0, "Unexpected cc=%d on subchannel number 0x%x",
+			       cc, scn);
+			return;
+		}
+		/* We currently only support type 0, a.k.a. I/O channels */
+		if (PMCW_CHANNEL_TYPE(pmcw) != 0)
+			continue;
+		/* We ignore I/O channels without valid devices */
+		scn_found++;
+		if (!(pmcw->flags & PMCW_DNV))
+			continue;
+		/* We keep track of the first device as our test device */
+		if (!test_device_sid)
+			test_device_sid = scn|SID_ONE;
+		dev_found++;
+	}
+out:
+	if (!dev_found) {
+		report(0, "Tested subchannels: %d, I/O subchannels: %d, I/O devices: %d",
+		       scn, scn_found, dev_found);
+		return;
+	}
+	report(1, "Tested subchannels: %d, I/O subchannels: %d, I/O devices: %d",
+	       scn, scn_found, dev_found);
+}
+
+static struct {
+	const char *name;
+	void (*func)(void);
+} tests[] = {
+	{ "enumerate (stsch)", test_enumerate },
+	{ NULL, NULL }
+};
+
+int main(int argc, char *argv[])
+{
+	int i;
+
+	report_prefix_push("Channel Subsystem");
+	for (i = 0; tests[i].name; i++) {
+		report_prefix_push(tests[i].name);
+		tests[i].func();
+		report_prefix_pop();
+	}
+	report_prefix_pop();
+
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 07013b2..a436ec0 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -83,3 +83,7 @@ extra_params = -m 1G
 [sclp-3g]
 file = sclp.elf
 extra_params = -m 3G
+
+[css]
+file = css.elf
+extra_params =-device ccw-pong
-- 
2.17.0

[kvm-unit-tests PATCH v5 08/10] s390x: css: msch, enable test

[Pierre Morel]

A second step when testing the channel subsystem is to prepare a channel
for use.
This includes:
- Get the current SubCHannel Information Block (SCHIB) using STSCH
- Update it in memory to set the ENABLE bit
- Tell the CSS that the SCHIB has been modified using MSCH
- Get the SCHIB from the CSS again to verify that the subchannel is
  enabled.

This tests the MSCH instruction to enable a channel succesfuly.
This is NOT a routine to really enable the channel, no retry is done,
in case of error, a report is made.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 s390x/css.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/s390x/css.c b/s390x/css.c
index cb33e00..aeee951 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -19,6 +19,7 @@
 #include <asm/time.h>
 
 #include <css.h>
+#include <asm/time.h>
 
 #define SID_ONE		0x00010000
 
@@ -67,11 +68,59 @@ out:
 	       scn, scn_found, dev_found);
 }
 
+static void test_enable(void)
+{
+	struct pmcw *pmcw = &schib.pmcw;
+	int cc;
+
+	if (!test_device_sid) {
+		report_skip("No device");
+		return;
+	}
+	/* Read the SCHIB for this subchannel */
+	cc = stsch(test_device_sid, &schib);
+	if (cc) {
+		report(0, "stsch cc=%d", cc);
+		return;
+	}
+
+	/* Update the SCHIB to enable the channel */
+	pmcw->flags |= PMCW_ENABLE;
+
+	/* Tell the CSS we want to modify the subchannel */
+	cc = msch(test_device_sid, &schib);
+	if (cc) {
+		/*
+		 * If the subchannel is status pending or
+		 * if a function is in progress,
+		 * we consider both cases as errors.
+		 */
+		report(0, "msch cc=%d", cc);
+		return;
+	}
+
+	/*
+	 * Read the SCHIB again to verify the enablement
+	 */
+	cc = stsch(test_device_sid, &schib);
+	if (cc) {
+		report(0, "stsch cc=%d", cc);
+		return;
+	}
+
+	if (!(pmcw->flags & PMCW_ENABLE)) {
+		report(0, "Enable failed. pmcw: %x", pmcw->flags);
+		return;
+	}
+	report(1, "Tested");
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
 } tests[] = {
 	{ "enumerate (stsch)", test_enumerate },
+	{ "enable (msch)", test_enable },
 	{ NULL, NULL }
 };
 
-- 
2.17.0

[kvm-unit-tests PATCH v5 09/10] s390x: css: ssch/tsch with sense and interrupt

[Pierre Morel]

We add a new css_lib file to contain the I/O function we may
share with different tests.
First function is the subchannel_enable() function.

When a channel is enabled we can start a SENSE ID command using
the SSCH instruction to recognize the control unit and device.

This tests the success of SSCH, the I/O interruption and the TSCH
instructions.

The test expects a device with a control unit type of 0xC0CA as the
first subchannel of the CSS.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h |   1 +
 lib/s390x/asm/time.h     |  10 +++
 lib/s390x/css.h          |  20 ++++++
 lib/s390x/css_lib.c      |  55 ++++++++++++++
 s390x/Makefile           |   1 +
 s390x/css.c              | 152 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 239 insertions(+)
 create mode 100644 lib/s390x/css_lib.c

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 863c2bf..ab3fc9d 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -10,6 +10,7 @@
 #ifndef _ASM_S390X_ARCH_DEF_H_
 #define _ASM_S390X_ARCH_DEF_H_
 
+#define PSW_MASK_IO			0x0200000000000000UL
 #define PSW_MASK_EXT			0x0100000000000000UL
 #define PSW_MASK_DAT			0x0400000000000000UL
 #define PSW_MASK_PSTATE			0x0001000000000000UL
diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
index 25c7a3c..d3e4eab 100644
--- a/lib/s390x/asm/time.h
+++ b/lib/s390x/asm/time.h
@@ -23,4 +23,14 @@ static inline uint64_t get_clock_ms(void)
 	return (clk >> (63 - 51)) / 1000;
 }
 
+static inline void delay(unsigned long ms)
+{
+	unsigned long startclk;
+
+	startclk = get_clock_ms();
+	for (;;)
+		if (get_clock_ms() - startclk > ms)
+			break;
+}
+
 #endif
diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 448e597..b6ab0ba 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -97,6 +97,19 @@ struct irb {
 	uint32_t emw[8];
 } __attribute__ ((aligned(4)));
 
+#define CCW_CMD_SENSE_ID	0xe4
+#define PONG_CU			0xc0ca
+struct senseid {
+	/* common part */
+	uint8_t reserved;        /* always 0x'FF' */
+	uint16_t cu_type;        /* control unit type */
+	uint8_t cu_model;        /* control unit model */
+	uint16_t dev_type;       /* device type */
+	uint8_t dev_model;       /* device model */
+	uint8_t unused;          /* padding byte */
+	uint8_t padding[256 - 10]; /* Extra padding for CCW */
+} __attribute__ ((aligned(4))) __attribute__ ((packed));
+
 /* CSS low level access functions */
 
 static inline int ssch(unsigned long schid, struct orb *addr)
@@ -254,4 +267,11 @@ static inline struct ccw *dump_ccw(struct ccw *cp)
 	return NULL;
 }
 #endif /* DEBUG_CSS */
+
+#define SID_ONE         0x00010000
+
+/* Library functions */
+int enable_subchannel(unsigned int sid);
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw);
+
 #endif
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
new file mode 100644
index 0000000..15d767a
--- /dev/null
+++ b/lib/s390x/css_lib.c
@@ -0,0 +1,55 @@
+/*
+ * Channel subsystem library functions
+ *
+ * Copyright (c) 2019 IBM Corp.
+ *
+ * Authors:
+ *  Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#include <stdint.h>
+#include <stddef.h>
+#include <css.h>
+
+int enable_subchannel(unsigned int sid)
+{
+	struct schib schib;
+	struct pmcw *pmcw = &schib.pmcw;
+	int try_count = 5;
+	int cc;
+
+	if (!(sid & SID_ONE))
+		return -1;
+
+	cc = stsch(sid, &schib);
+	if (cc)
+		return -cc;
+
+	do {
+		pmcw->flags |= PMCW_ENABLE;
+
+		cc = msch(sid, &schib);
+		if (cc)
+			return -cc;
+
+		cc = stsch(sid, &schib);
+		if (cc)
+			return -cc;
+
+	} while (!(pmcw->flags & PMCW_ENABLE) && --try_count);
+
+	return try_count;
+}
+
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw)
+{
+	struct orb orb;
+
+	orb.intparm = sid;
+	orb.ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
+	orb.cpa = (unsigned int) (unsigned long)ccw;
+
+	return ssch(sid, &orb);
+}
diff --git a/s390x/Makefile b/s390x/Makefile
index baebf18..166cb5c 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -53,6 +53,7 @@ cflatobjs += lib/s390x/interrupt.o
 cflatobjs += lib/s390x/mmu.o
 cflatobjs += lib/s390x/smp.o
 cflatobjs += lib/s390x/css_dump.o
+cflatobjs += lib/s390x/css_lib.o
 
 OBJDIRS += lib/s390x
 
diff --git a/s390x/css.c b/s390x/css.c
index aeee951..b9805a9 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -22,9 +22,34 @@
 #include <asm/time.h>
 
 #define SID_ONE		0x00010000
+#define PSW_PRG_MASK (PSW_MASK_IO | PSW_MASK_EA | PSW_MASK_BA)
+
+#define PONG_CU_TYPE		0xc0ca
+
+struct lowcore *lowcore = (void *)0x0;
 
 static struct schib schib;
 static int test_device_sid;
+#define NUM_CCW  100
+static struct ccw1 ccw[NUM_CCW];
+static struct irb irb;
+static struct senseid senseid;
+
+static void set_io_irq_subclass_mask(uint64_t const new_mask)
+{
+	asm volatile (
+		"lctlg %%c6, %%c6, %[source]\n"
+		: /* No outputs */
+		: [source] "R" (new_mask));
+}
+
+static void set_system_mask(uint8_t new_mask)
+{
+	asm volatile (
+		"ssm %[source]\n"
+		: /* No outputs */
+		: [source] "R" (new_mask));
+}
 
 static void test_enumerate(void)
 {
@@ -115,12 +140,139 @@ static void test_enable(void)
 	report(1, "Tested");
 }
 
+static void enable_io_irq(void)
+{
+	/* Let's enable all ISCs for I/O interrupt */
+	set_io_irq_subclass_mask(0x00000000ff000000);
+	set_system_mask(PSW_PRG_MASK >> 56);
+}
+
+static void irq_io(void)
+{
+	int ret = 0;
+	char *flags;
+	int sid;
+
+	report_prefix_push("Interrupt");
+	/* Lowlevel set the SID as interrupt parameter. */
+	if (lowcore->io_int_param != test_device_sid) {
+		report(0, "Bad io_int_param: %x expected %x", lowcore->io_int_param, test_device_sid);
+		report_prefix_pop();
+		return;
+	}
+	report_prefix_pop();
+
+	report_prefix_push("tsch");
+	sid = lowcore->subsys_id_word;
+	ret = tsch(sid, &irb);
+	switch (ret) {
+	case 1:
+		dump_irb(&irb);
+		flags = dump_scsw_flags(irb.scsw.ctrl);
+		report(0, "I/O interrupt, but sch not status pending: %s", flags);
+		goto pop;
+	case 2:
+		report(0, "TSCH returns unexpected CC 2");
+		goto pop;
+	case 3:
+		report(0, "Subchannel %08x not operational", sid);
+		goto pop;
+	case 0:
+		/* Stay humble on success */
+		break;
+	}
+pop:
+	report_prefix_pop();
+}
+
+static int start_subchannel(int code, void *data, int count)
+{
+	int ret;
+
+	report_prefix_push("start_senseid");
+	/* Build the CCW chain with a single CCW */
+	ccw[0].code = code;
+	ccw[0].flags = 0; /* No flags need to be set */
+	ccw[0].count = count;
+	ccw[0].data_address = (int)(unsigned long)data;
+
+	ret = start_ccw1_chain(test_device_sid, ccw);
+	if (ret) {
+		report(0, "start_ccw_chain failed ret=%d", ret);
+		report_prefix_pop();
+		return 0;
+	}
+	report_prefix_pop();
+	return 1;
+}
+
+/*
+ * test_sense
+ * Pre-requisits:
+ * - We need the QEMU PONG device as the first recognized
+ *   device by the enumeration.
+ * - ./s390x-run s390x/css.elf -device ccw-pong,cu_type=0xc0ca
+ */
+static void test_sense(void)
+{
+	int ret;
+
+	if (!test_device_sid) {
+		report_skip("No device");
+		return;
+	}
+
+	ret = enable_subchannel(test_device_sid);
+	if (ret < 0) {
+		report(0, "Could not enable the subchannel: %08x", test_device_sid);
+		return;
+	}
+
+	ret = register_io_int_func(irq_io);
+	if (ret) {
+		report(0, "Could not register IRQ handler");
+		goto unreg_cb;
+	}
+
+	enable_io_irq();
+	lowcore->io_int_param = 0;
+
+	ret = start_subchannel(CCW_CMD_SENSE_ID, &senseid, sizeof(senseid));
+	if (!ret) {
+		report(0, "start_senseid failed");
+		goto unreg_cb;
+	}
+
+	/* 100ms should be enough for the interruption to fire */
+	delay(100);
+	if (lowcore->io_int_param != test_device_sid) {
+		report(0, "No interrupts. io_int_param: expect 0x%08x, got 0x%08x",
+		       test_device_sid, lowcore->io_int_param);
+		goto unreg_cb;
+	}
+
+	report_info("reserved %02x cu_type %04x cu_model %02x dev_type %04x dev_model %02x\n",
+		    senseid.reserved, senseid.cu_type, senseid.cu_model,
+		    senseid.dev_type, senseid.dev_model);
+
+	if (senseid.cu_type == PONG_CU)
+		report(1, "cu_type: expect 0x%04x got 0x%04x",
+		       PONG_CU_TYPE, senseid.cu_type);
+	else
+		report(0, "cu_type: expect 0x%04x got 0x%04x",
+		       PONG_CU_TYPE, senseid.cu_type);
+
+unreg_cb:
+	unregister_io_int_func(irq_io);
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
 } tests[] = {
 	{ "enumerate (stsch)", test_enumerate },
 	{ "enable (msch)", test_enable },
+	{ "sense (ssch/tsch)", test_sense },
 	{ NULL, NULL }
 };
 
-- 
2.17.0

[kvm-unit-tests PATCH v5 10/10] s390x: css: ping pong

[Pierre Morel]

To test a write command with the SSCH instruction we need a QEMU device,
with control unit type 0xC0CA. The PONG device is such a device.

This type of device responds to PONG_WRITE requests by incrementing an
integer, stored as a string at offset 0 of the CCW data.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 s390x/css.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/s390x/css.c b/s390x/css.c
index b9805a9..c1616d4 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -25,6 +25,12 @@
 #define PSW_PRG_MASK (PSW_MASK_IO | PSW_MASK_EA | PSW_MASK_BA)
 
 #define PONG_CU_TYPE		0xc0ca
+/* Channel Commands for PONG device */
+#define PONG_WRITE	0x21 /* Write */
+#define PONG_READ	0x22 /* Read buffer */
+
+#define BUFSZ	9
+static char buffer[BUFSZ];
 
 struct lowcore *lowcore = (void *)0x0;
 
@@ -266,6 +272,48 @@ unreg_cb:
 	unregister_io_int_func(irq_io);
 }
 
+static void test_ping(void)
+{
+	int success, result;
+	int cnt = 0, max = 4;
+
+	if (senseid.cu_type != PONG_CU) {
+		report_skip("No PONG, no ping-pong");
+		return;
+	}
+
+	result = register_io_int_func(irq_io);
+	if (result) {
+		report(0, "Could not register IRQ handler");
+		return;
+	}
+
+	while (cnt++ < max) {
+		snprintf(buffer, BUFSZ, "%08x\n", cnt);
+		success = start_subchannel(PONG_WRITE, buffer, BUFSZ);
+		if (!success) {
+			report(0, "start_subchannel failed");
+			goto unreg_cb;
+		}
+		delay(100);
+		success = start_subchannel(PONG_READ, buffer, BUFSZ);
+		if (!success) {
+			report(0, "start_subchannel failed");
+			goto unreg_cb;
+		}
+		result = atol(buffer);
+		if (result != (cnt + 1)) {
+			report(0, "Bad answer from pong: %08x - %08x",
+			       cnt, result);
+			goto unreg_cb;
+		}
+	}
+	report(1, "ping-pong count 0x%08x", cnt);
+
+unreg_cb:
+	unregister_io_int_func(irq_io);
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
@@ -273,6 +321,7 @@ static struct {
 	{ "enumerate (stsch)", test_enumerate },
 	{ "enable (msch)", test_enable },
 	{ "sense (ssch/tsch)", test_sense },
+	{ "ping-pong (ssch/tsch)", test_ping },
 	{ NULL, NULL }
 };
 
-- 
2.17.0

Re: [kvm-unit-tests PATCH v5 00/10] s390x: Testing the Channel Subsystem I/O

[Pierre Morel]

On 2020-02-20 13:00, Pierre Morel wrote:

...snip...

> 
> 
> Pierre Morel (10):
>    s390x: saving regs for interrupts
>    s390x: Use PSW bits definitions in cstart
>    s390x: cr0: adding AFP-register control bit
>    s390x: export the clock get_clock_ms() utility

Please can you consider applying these 4 patches only.
I will send some changes I made for the patches on css tests.

I did not change the interrupt registration but since it is introduced 
for the css test patches I resend it with them.

Thanks,
Pierre

 >    s390x: interrupt registration

>    s390x: Library resources for CSS tests
>    s390x: css: stsch, enumeration test
>    s390x: css: msch, enable test
>    s390x: css: ssch/tsch with sense and interrupt
>    s390x: css: ping pong
> 
>   lib/s390x/asm/arch_def.h |  19 ++-
>   lib/s390x/asm/time.h     |  36 +++++
>   lib/s390x/css.h          | 277 +++++++++++++++++++++++++++++++
>   lib/s390x/css_dump.c     | 157 ++++++++++++++++++
>   lib/s390x/css_lib.c      |  55 +++++++
>   lib/s390x/interrupt.c    |  22 ++-
>   lib/s390x/interrupt.h    |   7 +
>   s390x/Makefile           |   3 +
>   s390x/css.c              | 341 +++++++++++++++++++++++++++++++++++++++
>   s390x/cstart64.S         |  40 +++--
>   s390x/intercept.c        |  11 +-
>   s390x/unittests.cfg      |   4 +
>   12 files changed, 946 insertions(+), 26 deletions(-)
>   create mode 100644 lib/s390x/asm/time.h
>   create mode 100644 lib/s390x/css.h
>   create mode 100644 lib/s390x/css_dump.c
>   create mode 100644 lib/s390x/css_lib.c
>   create mode 100644 lib/s390x/interrupt.h
>   create mode 100644 s390x/css.c
> 

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 03/10] s390x: cr0: adding AFP-register control bit

[David Hildenbrand]

On 20.02.20 13:00, Pierre Morel wrote:
> While adding the definition for the AFP-Register control bit, move all
> existing definitions for CR0 out of the C zone to the assmbler zone to
> keep the definitions concerning CR0 together.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_def.h | 11 ++++++-----
>  s390x/cstart64.S         |  2 +-
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 69a8256..863c2bf 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -18,6 +18,12 @@
>  
>  #define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
>  
> +#define CR0_EXTM_SCLP			0X0000000000000200UL
> +#define CR0_EXTM_EXTC			0X0000000000002000UL
> +#define CR0_EXTM_EMGC			0X0000000000004000UL
> +#define CR0_EXTM_MASK			0X0000000000006200UL
> +#define CR0_AFP_REG_CRTL		0x0000000000040000UL
> +
>  #ifndef __ASSEMBLER__
>  
>  struct psw {
> @@ -25,11 +31,6 @@ struct psw {
>  	uint64_t	addr;
>  };
>  
> -#define CR0_EXTM_SCLP			0X0000000000000200UL
> -#define CR0_EXTM_EXTC			0X0000000000002000UL
> -#define CR0_EXTM_EMGC			0X0000000000004000UL
> -#define CR0_EXTM_MASK			0X0000000000006200UL
> -
>  struct lowcore {
>  	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
>  	uint32_t	ext_int_param;			/* 0x0080 */
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 2885a36..3b59bd1 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -230,4 +230,4 @@ svc_int_psw:
>  	.quad	PSW_EXCEPTION_MASK, svc_int
>  initial_cr0:
>  	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
> -	.quad	0x0000000000040000
> +	.quad	CR0_AFP_REG_CRTL
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[David Hildenbrand]

On 20.02.20 13:00, Pierre Morel wrote:
> This patch defines the PSW bits EA/BA used to initialize the PSW masks
> for exceptions.
> 
> Since some PSW mask definitions exist already in arch_def.h we add these
> definitions there.
> We move all PSW definitions together and protect assembler code against
> C syntax.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_def.h | 15 +++++++++++----
>  s390x/cstart64.S         | 15 ++++++++-------
>  2 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 15a4d49..69a8256 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -10,15 +10,21 @@
>  #ifndef _ASM_S390X_ARCH_DEF_H_
>  #define _ASM_S390X_ARCH_DEF_H_
>  
> +#define PSW_MASK_EXT			0x0100000000000000UL
> +#define PSW_MASK_DAT			0x0400000000000000UL
> +#define PSW_MASK_PSTATE			0x0001000000000000UL
> +#define PSW_MASK_BA			0x0000000080000000UL
> +#define PSW_MASK_EA			0x0000000100000000UL
> +
> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
> +
> +#ifndef __ASSEMBLER__
> +
>  struct psw {
>  	uint64_t	mask;
>  	uint64_t	addr;
>  };
>  
> -#define PSW_MASK_EXT			0x0100000000000000UL
> -#define PSW_MASK_DAT			0x0400000000000000UL
> -#define PSW_MASK_PSTATE			0x0001000000000000UL
> -
>  #define CR0_EXTM_SCLP			0X0000000000000200UL
>  #define CR0_EXTM_EXTC			0X0000000000002000UL
>  #define CR0_EXTM_EMGC			0X0000000000004000UL
> @@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void)
>  	return current_prefix;
>  }
>  
> +#endif /* not __ASSEMBLER__ */
>  #endif
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 45da523..2885a36 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -12,6 +12,7 @@
>   */
>  #include <asm/asm-offsets.h>
>  #include <asm/sigp.h>
> +#include <asm/arch_def.h>
>  
>  .section .init
>  
> @@ -214,19 +215,19 @@ svc_int:
>  
>  	.align	8
>  reset_psw:
> -	.quad	0x0008000180000000
> +	.quad	PSW_EXCEPTION_MASK
>  initial_psw:
> -	.quad	0x0000000180000000, clear_bss_start
> +	.quad	PSW_EXCEPTION_MASK, clear_bss_start
>  pgm_int_psw:
> -	.quad	0x0000000180000000, pgm_int
> +	.quad	PSW_EXCEPTION_MASK, pgm_int
>  ext_int_psw:
> -	.quad	0x0000000180000000, ext_int
> +	.quad	PSW_EXCEPTION_MASK, ext_int
>  mcck_int_psw:
> -	.quad	0x0000000180000000, mcck_int
> +	.quad	PSW_EXCEPTION_MASK, mcck_int
>  io_int_psw:
> -	.quad	0x0000000180000000, io_int
> +	.quad	PSW_EXCEPTION_MASK, io_int
>  svc_int_psw:
> -	.quad	0x0000000180000000, svc_int
> +	.quad	PSW_EXCEPTION_MASK, svc_int
>  initial_cr0:
>  	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
>  	.quad	0x0000000000040000
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v5 00/10] s390x: Testing the Channel Subsystem I/O

[David Hildenbrand]

On 21.04.20 18:13, Pierre Morel wrote:
> 
> 
> On 2020-02-20 13:00, Pierre Morel wrote:
> 
> ...snip...
> 
>>
>>
>> Pierre Morel (10):
>>    s390x: saving regs for interrupts
>>    s390x: Use PSW bits definitions in cstart
>>    s390x: cr0: adding AFP-register control bit
>>    s390x: export the clock get_clock_ms() utility
> 
> Please can you consider applying these 4 patches only.
> I will send some changes I made for the patches on css tests.
> 

The first one requires a little more brain power - can anybody at IBM
help reviewing that?


-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[Pierre Morel]

On 2020-04-21 18:16, David Hildenbrand wrote:
> On 20.02.20 13:00, Pierre Morel wrote:
>> This patch defines the PSW bits EA/BA used to initialize the PSW masks
>> for exceptions.
>>
>> Since some PSW mask definitions exist already in arch_def.h we add these
>> definitions there.
>> We move all PSW definitions together and protect assembler code against
>> C syntax.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/asm/arch_def.h | 15 +++++++++++----
>>   s390x/cstart64.S         | 15 ++++++++-------
>>   2 files changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index 15a4d49..69a8256 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -10,15 +10,21 @@
>>   #ifndef _ASM_S390X_ARCH_DEF_H_
>>   #define _ASM_S390X_ARCH_DEF_H_
>>   
>> +#define PSW_MASK_EXT			0x0100000000000000UL
>> +#define PSW_MASK_DAT			0x0400000000000000UL
>> +#define PSW_MASK_PSTATE			0x0001000000000000UL
>> +#define PSW_MASK_BA			0x0000000080000000UL
>> +#define PSW_MASK_EA			0x0000000100000000UL
>> +
>> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
>> +
>> +#ifndef __ASSEMBLER__
>> +
>>   struct psw {
>>   	uint64_t	mask;
>>   	uint64_t	addr;
>>   };
>>   
>> -#define PSW_MASK_EXT			0x0100000000000000UL
>> -#define PSW_MASK_DAT			0x0400000000000000UL
>> -#define PSW_MASK_PSTATE			0x0001000000000000UL
>> -
>>   #define CR0_EXTM_SCLP			0X0000000000000200UL
>>   #define CR0_EXTM_EXTC			0X0000000000002000UL
>>   #define CR0_EXTM_EMGC			0X0000000000004000UL
>> @@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void)
>>   	return current_prefix;
>>   }
>>   
>> +#endif /* not __ASSEMBLER__ */
>>   #endif
>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>> index 45da523..2885a36 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -12,6 +12,7 @@
>>    */
>>   #include <asm/asm-offsets.h>
>>   #include <asm/sigp.h>
>> +#include <asm/arch_def.h>
>>   
>>   .section .init
>>   
>> @@ -214,19 +215,19 @@ svc_int:
>>   
>>   	.align	8
>>   reset_psw:
>> -	.quad	0x0008000180000000
>> +	.quad	PSW_EXCEPTION_MASK
>>   initial_psw:
>> -	.quad	0x0000000180000000, clear_bss_start
>> +	.quad	PSW_EXCEPTION_MASK, clear_bss_start
>>   pgm_int_psw:
>> -	.quad	0x0000000180000000, pgm_int
>> +	.quad	PSW_EXCEPTION_MASK, pgm_int
>>   ext_int_psw:
>> -	.quad	0x0000000180000000, ext_int
>> +	.quad	PSW_EXCEPTION_MASK, ext_int
>>   mcck_int_psw:
>> -	.quad	0x0000000180000000, mcck_int
>> +	.quad	PSW_EXCEPTION_MASK, mcck_int
>>   io_int_psw:
>> -	.quad	0x0000000180000000, io_int
>> +	.quad	PSW_EXCEPTION_MASK, io_int
>>   svc_int_psw:
>> -	.quad	0x0000000180000000, svc_int
>> +	.quad	PSW_EXCEPTION_MASK, svc_int
>>   initial_cr0:
>>   	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
>>   	.quad	0x0000000000040000
>>
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 03/10] s390x: cr0: adding AFP-register control bit

[Pierre Morel]

On 2020-04-21 18:15, David Hildenbrand wrote:
> On 20.02.20 13:00, Pierre Morel wrote:
>> While adding the definition for the AFP-Register control bit, move all
>> existing definitions for CR0 out of the C zone to the assmbler zone to
>> keep the definitions concerning CR0 together.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
...snip...
>> -	.quad	0x0000000000040000
>> +	.quad	CR0_AFP_REG_CRTL
>>
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 3130 bytes --]

On 2/20/20 1:00 PM, Pierre Morel wrote:
> This patch defines the PSW bits EA/BA used to initialize the PSW masks
> for exceptions.
> 
> Since some PSW mask definitions exist already in arch_def.h we add these
> definitions there.
> We move all PSW definitions together and protect assembler code against
> C syntax.

Please fix the issue mentioned below and run *all* tests against your
new code to verify you didn't introduce regressions.

The rest looks good to me.

> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_def.h | 15 +++++++++++----
>  s390x/cstart64.S         | 15 ++++++++-------
>  2 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 15a4d49..69a8256 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -10,15 +10,21 @@
>  #ifndef _ASM_S390X_ARCH_DEF_H_
>  #define _ASM_S390X_ARCH_DEF_H_
>  
> +#define PSW_MASK_EXT			0x0100000000000000UL
> +#define PSW_MASK_DAT			0x0400000000000000UL
> +#define PSW_MASK_PSTATE			0x0001000000000000UL
> +#define PSW_MASK_BA			0x0000000080000000UL
> +#define PSW_MASK_EA			0x0000000100000000UL
> +
> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)

Could you add a space before and after the | ?

> +
> +#ifndef __ASSEMBLER__
> +
>  struct psw {
>  	uint64_t	mask;
>  	uint64_t	addr;
>  };
>  
> -#define PSW_MASK_EXT			0x0100000000000000UL
> -#define PSW_MASK_DAT			0x0400000000000000UL
> -#define PSW_MASK_PSTATE			0x0001000000000000UL
> -
>  #define CR0_EXTM_SCLP			0X0000000000000200UL
>  #define CR0_EXTM_EXTC			0X0000000000002000UL
>  #define CR0_EXTM_EMGC			0X0000000000004000UL
> @@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void)
>  	return current_prefix;
>  }
>  
> +#endif /* not __ASSEMBLER__ */
>  #endif
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 45da523..2885a36 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -12,6 +12,7 @@
>   */
>  #include <asm/asm-offsets.h>
>  #include <asm/sigp.h>
> +#include <asm/arch_def.h>
>  
>  .section .init
>  
> @@ -214,19 +215,19 @@ svc_int:
>  
>  	.align	8
>  reset_psw:
> -	.quad	0x0008000180000000
> +	.quad	PSW_EXCEPTION_MASK

That won't work, this is a short PSW and you're removing the short
indication here. Notice the 0008 at the front.

>  initial_psw:
> -	.quad	0x0000000180000000, clear_bss_start
> +	.quad	PSW_EXCEPTION_MASK, clear_bss_start
>  pgm_int_psw:
> -	.quad	0x0000000180000000, pgm_int
> +	.quad	PSW_EXCEPTION_MASK, pgm_int
>  ext_int_psw:
> -	.quad	0x0000000180000000, ext_int
> +	.quad	PSW_EXCEPTION_MASK, ext_int
>  mcck_int_psw:
> -	.quad	0x0000000180000000, mcck_int
> +	.quad	PSW_EXCEPTION_MASK, mcck_int
>  io_int_psw:
> -	.quad	0x0000000180000000, io_int
> +	.quad	PSW_EXCEPTION_MASK, io_int
>  svc_int_psw:
> -	.quad	0x0000000180000000, svc_int
> +	.quad	PSW_EXCEPTION_MASK, svc_int
>  initial_cr0:
>  	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
>  	.quad	0x0000000000040000
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [kvm-unit-tests PATCH v5 03/10] s390x: cr0: adding AFP-register control bit

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 1897 bytes --]

On 2/20/20 1:00 PM, Pierre Morel wrote:
> While adding the definition for the AFP-Register control bit, move all
> existing definitions for CR0 out of the C zone to the assmbler zone to
> keep the definitions concerning CR0 together.

How about:
s390x: Move control register bit definitions and add AFP to them

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_def.h | 11 ++++++-----
>  s390x/cstart64.S         |  2 +-
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 69a8256..863c2bf 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -18,6 +18,12 @@
>  
>  #define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
>  
> +#define CR0_EXTM_SCLP			0X0000000000000200UL
> +#define CR0_EXTM_EXTC			0X0000000000002000UL
> +#define CR0_EXTM_EMGC			0X0000000000004000UL
> +#define CR0_EXTM_MASK			0X0000000000006200UL
> +#define CR0_AFP_REG_CRTL		0x0000000000040000UL
> +
>  #ifndef __ASSEMBLER__
>  
>  struct psw {
> @@ -25,11 +31,6 @@ struct psw {
>  	uint64_t	addr;
>  };
>  
> -#define CR0_EXTM_SCLP			0X0000000000000200UL
> -#define CR0_EXTM_EXTC			0X0000000000002000UL
> -#define CR0_EXTM_EMGC			0X0000000000004000UL
> -#define CR0_EXTM_MASK			0X0000000000006200UL
> -
>  struct lowcore {
>  	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
>  	uint32_t	ext_int_param;			/* 0x0080 */
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 2885a36..3b59bd1 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -230,4 +230,4 @@ svc_int_psw:
>  	.quad	PSW_EXCEPTION_MASK, svc_int
>  initial_cr0:
>  	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
> -	.quad	0x0000000000040000
> +	.quad	CR0_AFP_REG_CRTL
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [kvm-unit-tests PATCH v5 00/10] s390x: Testing the Channel Subsystem I/O

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 953 bytes --]

On 4/21/20 6:18 PM, David Hildenbrand wrote:
> On 21.04.20 18:13, Pierre Morel wrote:
>>
>>
>> On 2020-02-20 13:00, Pierre Morel wrote:
>>
>> ...snip...
>>
>>>
>>>
>>> Pierre Morel (10):
>>>    s390x: saving regs for interrupts
>>>    s390x: Use PSW bits definitions in cstart
>>>    s390x: cr0: adding AFP-register control bit
>>>    s390x: export the clock get_clock_ms() utility
>>
>> Please can you consider applying these 4 patches only.
>> I will send some changes I made for the patches on css tests.
>>
> 
> The first one requires a little more brain power - can anybody at IBM
> help reviewing that?
> 

I'll try to understand it :)

But I think we need a new series anyway.
@Pierre: You told me, that you removed delay() and this series still has
it. With the changes needed to the second patch and the delay change we
need all information to make decisions, so a new version of the series
would make sense.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[David Hildenbrand]

>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>> index 45da523..2885a36 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -12,6 +12,7 @@
>>   */
>>  #include <asm/asm-offsets.h>
>>  #include <asm/sigp.h>
>> +#include <asm/arch_def.h>
>>  
>>  .section .init
>>  
>> @@ -214,19 +215,19 @@ svc_int:
>>  
>>  	.align	8
>>  reset_psw:
>> -	.quad	0x0008000180000000
>> +	.quad	PSW_EXCEPTION_MASK
> 
> That won't work, this is a short PSW and you're removing the short
> indication here. Notice the 0008 at the front.

Good catch! Guess it would have bailed out when testing.


-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v5 01/10] s390x: saving regs for interrupts

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 1903 bytes --]

On 2/20/20 1:00 PM, Pierre Morel wrote:
> If we use multiple source of interrupts, for example, using SCLP
> console to print information while using I/O interrupts, we need
> to have a re-entrant register saving interruption handling.
> 
> Instead of saving at a static memory address, let's save the base
> registers and the floating point registers on the stack.
> 
> Note that we keep the static register saving to recover from the
> RESET tests.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  s390x/cstart64.S | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 9af6bb3..45da523 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -118,6 +118,25 @@ memsetxc:
>  	lmg	%r0, %r15, GEN_LC_SW_INT_GRS
>  	.endm
>  
> +/* Save registers on the stack, so we can have stacked interrupts. */
> +	.macro SAVE_IRQ_REGS
> +	slgfi   %r15, 15 * 8
> +	stmg    %r0, %r14, 0(%r15)

So this is just making some space on the stack and storing r0 - r14, right?

Same is done for the floating point registers below.

Could you add a comment above slgfi and stmg and explain what they do?
A few words are enough.

> +	slgfi   %r15, 16 * 8
> +	.irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
> +	std	\i, \i * 8(%r15)
> +	.endr
> +	.endm
> +
> +	.macro RESTORE_IRQ_REGS
> +	.irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
> +	ld	\i, \i * 8(%r15)
> +	.endr
> +	algfi   %r15, 16 * 8
> +	lmg     %r0, %r14, 0(%r15)
> +	algfi   %r15, 15 * 8
> +	.endm
> +
>  .section .text
>  /*
>   * load_reset calling convention:
> @@ -182,9 +201,9 @@ mcck_int:
>  	lpswe	GEN_LC_MCCK_OLD_PSW
>  
>  io_int:
> -	SAVE_REGS
> +	SAVE_IRQ_REGS
>  	brasl	%r14, handle_io_int
> -	RESTORE_REGS
> +	RESTORE_IRQ_REGS
>  	lpswe	GEN_LC_IO_OLD_PSW
>  
>  svc_int:
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [kvm-unit-tests PATCH v5 03/10] s390x: cr0: adding AFP-register control bit

[Cornelia Huck]

On Thu, 20 Feb 2020 13:00:36 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> While adding the definition for the AFP-Register control bit, move all
> existing definitions for CR0 out of the C zone to the assmbler zone to
> keep the definitions concerning CR0 together.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_def.h | 11 ++++++-----
>  s390x/cstart64.S         |  2 +-
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [kvm-unit-tests PATCH v5 05/10] s390x: export the clock get_clock_ms() utility

[Cornelia Huck]

On Thu, 20 Feb 2020 13:00:38 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> Let's move get_clock_ms() to lib/s390/asm/time.h, so it can be used in
> multiple places.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/asm/time.h | 26 ++++++++++++++++++++++++++
>  s390x/intercept.c    | 11 +----------
>  2 files changed, 27 insertions(+), 10 deletions(-)
>  create mode 100644 lib/s390x/asm/time.h

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[Pierre Morel]

On 2020-04-22 09:44, David Hildenbrand wrote:
> 
>>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>>> index 45da523..2885a36 100644
>>> --- a/s390x/cstart64.S
>>> +++ b/s390x/cstart64.S
>>> @@ -12,6 +12,7 @@
>>>    */
>>>   #include <asm/asm-offsets.h>
>>>   #include <asm/sigp.h>
>>> +#include <asm/arch_def.h>
>>>   
>>>   .section .init
>>>   
>>> @@ -214,19 +215,19 @@ svc_int:
>>>   
>>>   	.align	8
>>>   reset_psw:
>>> -	.quad	0x0008000180000000
>>> +	.quad	PSW_EXCEPTION_MASK
>>
>> That won't work, this is a short PSW and you're removing the short
>> indication here. Notice the 0008 at the front.

hum... :(

> 
> Good catch! Guess it would have bailed out when testing.
> 
> 

Yes it does. Sorry.


-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[Pierre Morel]

On 2020-04-22 09:35, Janosch Frank wrote:
> On 2/20/20 1:00 PM, Pierre Morel wrote:
>> This patch defines the PSW bits EA/BA used to initialize the PSW masks
>> for exceptions.
>>
>> Since some PSW mask definitions exist already in arch_def.h we add these
>> definitions there.
>> We move all PSW definitions together and protect assembler code against
>> C syntax.
> 
> Please fix the issue mentioned below and run *all* tests against your
> new code to verify you didn't introduce regressions.
> 
> The rest looks good to me.

Thanks,

> 
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/asm/arch_def.h | 15 +++++++++++----
>>   s390x/cstart64.S         | 15 ++++++++-------
>>   2 files changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index 15a4d49..69a8256 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -10,15 +10,21 @@
>>   #ifndef _ASM_S390X_ARCH_DEF_H_
>>   #define _ASM_S390X_ARCH_DEF_H_
>>   
>> +#define PSW_MASK_EXT			0x0100000000000000UL
>> +#define PSW_MASK_DAT			0x0400000000000000UL
>> +#define PSW_MASK_PSTATE			0x0001000000000000UL
>> +#define PSW_MASK_BA			0x0000000080000000UL
>> +#define PSW_MASK_EA			0x0000000100000000UL
>> +
>> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
> 
> Could you add a space before and after the | ?
> 
>> +
>> +#ifndef __ASSEMBLER__
>> +
>>   struct psw {
>>   	uint64_t	mask;
>>   	uint64_t	addr;
>>   };
>>   
>> -#define PSW_MASK_EXT			0x0100000000000000UL
>> -#define PSW_MASK_DAT			0x0400000000000000UL
>> -#define PSW_MASK_PSTATE			0x0001000000000000UL
>> -
>>   #define CR0_EXTM_SCLP			0X0000000000000200UL
>>   #define CR0_EXTM_EXTC			0X0000000000002000UL
>>   #define CR0_EXTM_EMGC			0X0000000000004000UL
>> @@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void)
>>   	return current_prefix;
>>   }
>>   
>> +#endif /* not __ASSEMBLER__ */
>>   #endif
>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>> index 45da523..2885a36 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -12,6 +12,7 @@
>>    */
>>   #include <asm/asm-offsets.h>
>>   #include <asm/sigp.h>
>> +#include <asm/arch_def.h>
>>   
>>   .section .init
>>   
>> @@ -214,19 +215,19 @@ svc_int:
>>   
>>   	.align	8
>>   reset_psw:
>> -	.quad	0x0008000180000000
>> +	.quad	PSW_EXCEPTION_MASK
> 
> That won't work, this is a short PSW and you're removing the short
> indication here. Notice the 0008 at the front.
> 

Will change and define PSW_MASK_SHORT_PSW and PSW_RESET_MASK like:

#define PSW_MASK_SHORT_PSW              0x0008000000000000UL
...

#define PSW_EXCEPTION_MASK      (PSW_MASK_EA | PSW_MASK_BA)
#define PSW_RESET_MASK          (PSW_EXCEPTION_MASK | PSW_MASK_SHORT_PSW)

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[David Hildenbrand]

On 22.04.20 10:59, Pierre Morel wrote:
> 
> 
> On 2020-04-22 09:44, David Hildenbrand wrote:
>>
>>>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>>>> index 45da523..2885a36 100644
>>>> --- a/s390x/cstart64.S
>>>> +++ b/s390x/cstart64.S
>>>> @@ -12,6 +12,7 @@
>>>>    */
>>>>   #include <asm/asm-offsets.h>
>>>>   #include <asm/sigp.h>
>>>> +#include <asm/arch_def.h>
>>>>   
>>>>   .section .init
>>>>   
>>>> @@ -214,19 +215,19 @@ svc_int:
>>>>   
>>>>   	.align	8
>>>>   reset_psw:
>>>> -	.quad	0x0008000180000000
>>>> +	.quad	PSW_EXCEPTION_MASK
>>>
>>> That won't work, this is a short PSW and you're removing the short
>>> indication here. Notice the 0008 at the front.
> 
> hum... :(
> 
>>
>> Good catch! Guess it would have bailed out when testing.
>>
>>
> 
> Yes it does. Sorry.

No worries, I run everything through the machinery before picking
patches up.

Feel free to resend only the 4/5 patches you want to have in first.

-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[Pierre Morel]

On 2020-04-22 11:07, David Hildenbrand wrote:
> On 22.04.20 10:59, Pierre Morel wrote:
>>
>>
>> On 2020-04-22 09:44, David Hildenbrand wrote:
>>>
>>>>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>>>>> index 45da523..2885a36 100644
>>>>> --- a/s390x/cstart64.S
>>>>> +++ b/s390x/cstart64.S
>>>>> @@ -12,6 +12,7 @@
>>>>>     */
>>>>>    #include <asm/asm-offsets.h>
>>>>>    #include <asm/sigp.h>
>>>>> +#include <asm/arch_def.h>
>>>>>    
>>>>>    .section .init
>>>>>    
>>>>> @@ -214,19 +215,19 @@ svc_int:
>>>>>    
>>>>>    	.align	8
>>>>>    reset_psw:
>>>>> -	.quad	0x0008000180000000
>>>>> +	.quad	PSW_EXCEPTION_MASK
>>>>
>>>> That won't work, this is a short PSW and you're removing the short
>>>> indication here. Notice the 0008 at the front.
>>
>> hum... :(
>>
>>>
>>> Good catch! Guess it would have bailed out when testing.
>>>
>>>
>>
>> Yes it does. Sorry.
> 
> No worries, I run everything through the machinery before picking
> patches up.
> 
> Feel free to resend only the 4/5 patches you want to have in first.
> 

OK, thanks, I ll do

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 01/10] s390x: saving regs for interrupts

[Pierre Morel]

On 2020-04-22 09:47, Janosch Frank wrote:
> On 2/20/20 1:00 PM, Pierre Morel wrote:
>> If we use multiple source of interrupts, for example, using SCLP
>> console to print information while using I/O interrupts, we need
>> to have a re-entrant register saving interruption handling.
>>
>> Instead of saving at a static memory address, let's save the base
>> registers and the floating point registers on the stack.
>>
>> Note that we keep the static register saving to recover from the
>> RESET tests.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   s390x/cstart64.S | 23 +++++++++++++++++++++--
>>   1 file changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>> index 9af6bb3..45da523 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -118,6 +118,25 @@ memsetxc:
>>   	lmg	%r0, %r15, GEN_LC_SW_INT_GRS
>>   	.endm
>>   
>> +/* Save registers on the stack, so we can have stacked interrupts. */
>> +	.macro SAVE_IRQ_REGS
>> +	slgfi   %r15, 15 * 8
>> +	stmg    %r0, %r14, 0(%r15)
> 
> So this is just making some space on the stack and storing r0 - r14, right?
> 
> Same is done for the floating point registers below.
> 
> Could you add a comment above slgfi and stmg and explain what they do?
> A few words are enough.
OK, thanks

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 03/10] s390x: cr0: adding AFP-register control bit

[Pierre Morel]

On 2020-04-22 09:39, Janosch Frank wrote:
> On 2/20/20 1:00 PM, Pierre Morel wrote:
>> While adding the definition for the AFP-Register control bit, move all
>> existing definitions for CR0 out of the C zone to the assmbler zone to
>> keep the definitions concerning CR0 together.
> 
> How about:
> s390x: Move control register bit definitions and add AFP to them
> 
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> 
If no one disagree I'll pick it.

Thanks for the review
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 03/10] s390x: cr0: adding AFP-register control bit

[Pierre Morel]

On 2020-04-22 09:59, Cornelia Huck wrote:
> On Thu, 20 Feb 2020 13:00:36 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> While adding the definition for the AFP-Register control bit, move all
>> existing definitions for CR0 out of the C zone to the assmbler zone to
>> keep the definitions concerning CR0 together.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/asm/arch_def.h | 11 ++++++-----
>>   s390x/cstart64.S         |  2 +-
>>   2 files changed, 7 insertions(+), 6 deletions(-)
>>
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 

Thanks for the review,
Regards,

Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 05/10] s390x: export the clock get_clock_ms() utility

[Pierre Morel]

On 2020-04-22 10:05, Cornelia Huck wrote:
> On Thu, 20 Feb 2020 13:00:38 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> Let's move get_clock_ms() to lib/s390/asm/time.h, so it can be used in
>> multiple places.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   lib/s390x/asm/time.h | 26 ++++++++++++++++++++++++++
>>   s390x/intercept.c    | 11 +----------
>>   2 files changed, 27 insertions(+), 10 deletions(-)
>>   create mode 100644 lib/s390x/asm/time.h
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 

Thanks for the review,
Regards,

Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 02/10] s390x: Use PSW bits definitions in cstart

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 3077 bytes --]

On 4/22/20 11:06 AM, Pierre Morel wrote:
> 
> 
> On 2020-04-22 09:35, Janosch Frank wrote:
>> On 2/20/20 1:00 PM, Pierre Morel wrote:
>>> This patch defines the PSW bits EA/BA used to initialize the PSW masks
>>> for exceptions.
>>>
>>> Since some PSW mask definitions exist already in arch_def.h we add these
>>> definitions there.
>>> We move all PSW definitions together and protect assembler code against
>>> C syntax.
>>
>> Please fix the issue mentioned below and run *all* tests against your
>> new code to verify you didn't introduce regressions.
>>
>> The rest looks good to me.
> 
> Thanks,
> 
>>
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>   lib/s390x/asm/arch_def.h | 15 +++++++++++----
>>>   s390x/cstart64.S         | 15 ++++++++-------
>>>   2 files changed, 19 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>>> index 15a4d49..69a8256 100644
>>> --- a/lib/s390x/asm/arch_def.h
>>> +++ b/lib/s390x/asm/arch_def.h
>>> @@ -10,15 +10,21 @@
>>>   #ifndef _ASM_S390X_ARCH_DEF_H_
>>>   #define _ASM_S390X_ARCH_DEF_H_
>>>   
>>> +#define PSW_MASK_EXT			0x0100000000000000UL
>>> +#define PSW_MASK_DAT			0x0400000000000000UL
>>> +#define PSW_MASK_PSTATE			0x0001000000000000UL
>>> +#define PSW_MASK_BA			0x0000000080000000UL
>>> +#define PSW_MASK_EA			0x0000000100000000UL
>>> +
>>> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
>>
>> Could you add a space before and after the | ?
>>
>>> +
>>> +#ifndef __ASSEMBLER__
>>> +
>>>   struct psw {
>>>   	uint64_t	mask;
>>>   	uint64_t	addr;
>>>   };
>>>   
>>> -#define PSW_MASK_EXT			0x0100000000000000UL
>>> -#define PSW_MASK_DAT			0x0400000000000000UL
>>> -#define PSW_MASK_PSTATE			0x0001000000000000UL
>>> -
>>>   #define CR0_EXTM_SCLP			0X0000000000000200UL
>>>   #define CR0_EXTM_EXTC			0X0000000000002000UL
>>>   #define CR0_EXTM_EMGC			0X0000000000004000UL
>>> @@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void)
>>>   	return current_prefix;
>>>   }
>>>   
>>> +#endif /* not __ASSEMBLER__ */
>>>   #endif
>>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>>> index 45da523..2885a36 100644
>>> --- a/s390x/cstart64.S
>>> +++ b/s390x/cstart64.S
>>> @@ -12,6 +12,7 @@
>>>    */
>>>   #include <asm/asm-offsets.h>
>>>   #include <asm/sigp.h>
>>> +#include <asm/arch_def.h>
>>>   
>>>   .section .init
>>>   
>>> @@ -214,19 +215,19 @@ svc_int:
>>>   
>>>   	.align	8
>>>   reset_psw:
>>> -	.quad	0x0008000180000000
>>> +	.quad	PSW_EXCEPTION_MASK
>>
>> That won't work, this is a short PSW and you're removing the short
>> indication here. Notice the 0008 at the front.
>>
> 
> Will change and define PSW_MASK_SHORT_PSW and PSW_RESET_MASK like:
> 
> #define PSW_MASK_SHORT_PSW              0x0008000000000000UL
> ...
> 
> #define PSW_EXCEPTION_MASK      (PSW_MASK_EA | PSW_MASK_BA)
> #define PSW_RESET_MASK          (PSW_EXCEPTION_MASK | PSW_MASK_SHORT_PSW)


Looks good to me, but please send out a new version.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [kvm-unit-tests PATCH v5 00/10] s390x: Testing the Channel Subsystem I/O

[Pierre Morel]

On 2020-04-22 09:43, Janosch Frank wrote:
> On 4/21/20 6:18 PM, David Hildenbrand wrote:
>> On 21.04.20 18:13, Pierre Morel wrote:
>>>
>>>
>>> On 2020-02-20 13:00, Pierre Morel wrote:
>>>
>>> ...snip...
>>>
>>>>
>>>>
>>>> Pierre Morel (10):
>>>>     s390x: saving regs for interrupts
>>>>     s390x: Use PSW bits definitions in cstart
>>>>     s390x: cr0: adding AFP-register control bit
>>>>     s390x: export the clock get_clock_ms() utility
>>>
>>> Please can you consider applying these 4 patches only.
>>> I will send some changes I made for the patches on css tests.
>>>
>>
>> The first one requires a little more brain power - can anybody at IBM
>> help reviewing that?
>>
> 
> I'll try to understand it :)
> 
> But I think we need a new series anyway.
> @Pierre: You told me, that you removed delay() and this series still has
> it. With the changes needed to the second patch and the delay change we
> need all information to make decisions, so a new version of the series
> would make sense.
> 
> 

Yes, this is clear, the next series will have some modifications for the 
css part.
Also I will send two series, first the general patches with bug fixes 
and comments and in a separate series the css specific patches.

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 04/10] s390x: interrupt registration

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 2446 bytes --]

On 2/20/20 1:00 PM, Pierre Morel wrote:
> Let's make it possible to add and remove a custom io interrupt handler,
> that can be used instead of the normal one.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/interrupt.c | 22 +++++++++++++++++++++-
>  lib/s390x/interrupt.h |  7 +++++++
>  2 files changed, 28 insertions(+), 1 deletion(-)
>  create mode 100644 lib/s390x/interrupt.h
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 3a40cac..f6f0665 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -10,9 +10,9 @@
>   * under the terms of the GNU Library General Public License version 2.
>   */
>  #include <libcflat.h>
> -#include <asm/interrupt.h>
>  #include <asm/barrier.h>
>  #include <sclp.h>
> +#include <interrupt.h>

Hrm

>  
>  static bool pgm_int_expected;
>  static bool ext_int_expected;
> @@ -144,12 +144,32 @@ void handle_mcck_int(void)
>  		     stap(), lc->mcck_old_psw.addr);
>  }
>  
> +static void (*io_int_func)(void);
> +
>  void handle_io_int(void)
>  {
> +	if (*io_int_func)
> +		return (*io_int_func)();
>  	report_abort("Unexpected io interrupt: on cpu %d at %#lx",
>  		     stap(), lc->io_old_psw.addr);
>  }
>  
> +int register_io_int_func(void (*f)(void))
> +{
> +	if (io_int_func)
> +		return -1;
> +	io_int_func = f;
> +	return 0;
> +}
> +
> +int unregister_io_int_func(void (*f)(void))
> +{
> +	if (io_int_func != f)
> +		return -1;
> +	io_int_func = NULL;
> +	return 0;
> +}

I'm currently working on something similar for PGMs and I see no
additional value in two functions for this. Unregistering can be done by
doing register_io_int_func(NULL)

This should be enough:

int register_io_int_func(void (*f)(void))
{
	io_int_func = f;
}

> +
>  void handle_svc_int(void)
>  {
>  	report_abort("Unexpected supervisor call interrupt: on cpu %d at %#lx",
> diff --git a/lib/s390x/interrupt.h b/lib/s390x/interrupt.h
> new file mode 100644
> index 0000000..e945ef7
> --- /dev/null
> +++ b/lib/s390x/interrupt.h
> @@ -0,0 +1,7 @@
> +#ifndef __INTERRUPT_H
> +#include <asm/interrupt.h>
> +
> +int register_io_int_func(void (*f)(void));
> +int unregister_io_int_func(void (*f)(void));
> +
> +#endif
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [kvm-unit-tests PATCH v5 04/10] s390x: interrupt registration

[Pierre Morel]

On 2020-04-24 10:27, Janosch Frank wrote:
> On 2/20/20 1:00 PM, Pierre Morel wrote:
>> Let's make it possible to add and remove a custom io interrupt handler,
>> that can be used instead of the normal one.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   lib/s390x/interrupt.c | 22 +++++++++++++++++++++-
>>   lib/s390x/interrupt.h |  7 +++++++
>>   2 files changed, 28 insertions(+), 1 deletion(-)
>>   create mode 100644 lib/s390x/interrupt.h
>>
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 3a40cac..f6f0665 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -10,9 +10,9 @@
>>    * under the terms of the GNU Library General Public License version 2.
>>    */
>>   #include <libcflat.h>
>> -#include <asm/interrupt.h>
>>   #include <asm/barrier.h>
>>   #include <sclp.h>
>> +#include <interrupt.h>
> 
> Hrm
> 
>>   
>>   static bool pgm_int_expected;
>>   static bool ext_int_expected;
>> @@ -144,12 +144,32 @@ void handle_mcck_int(void)
>>   		     stap(), lc->mcck_old_psw.addr);
>>   }
>>   
>> +static void (*io_int_func)(void);
>> +
>>   void handle_io_int(void)
>>   {
>> +	if (*io_int_func)
>> +		return (*io_int_func)();
>>   	report_abort("Unexpected io interrupt: on cpu %d at %#lx",
>>   		     stap(), lc->io_old_psw.addr);
>>   }
>>   
>> +int register_io_int_func(void (*f)(void))
>> +{
>> +	if (io_int_func)
>> +		return -1;
>> +	io_int_func = f;
>> +	return 0;
>> +}
>> +
>> +int unregister_io_int_func(void (*f)(void))
>> +{
>> +	if (io_int_func != f)
>> +		return -1;
>> +	io_int_func = NULL;
>> +	return 0;
>> +}
> 
> I'm currently working on something similar for PGMs and I see no
> additional value in two functions for this. Unregistering can be done by
> doing register_io_int_func(NULL)
> 
> This should be enough:
> 
> int register_io_int_func(void (*f)(void))
> {
> 	io_int_func = f;
> }
> 
There are several ways to do this and I really don't mind how it is done.
Since it has been reviewed by, I would like to have the others reviewers 
opinion.

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v5 04/10] s390x: interrupt registration

[Cornelia Huck]

On Fri, 24 Apr 2020 12:44:16 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 2020-04-24 10:27, Janosch Frank wrote:
> > On 2/20/20 1:00 PM, Pierre Morel wrote:  
> >> Let's make it possible to add and remove a custom io interrupt handler,
> >> that can be used instead of the normal one.
> >>
> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> >> Reviewed-by: Thomas Huth <thuth@redhat.com>
> >> Reviewed-by: David Hildenbrand <david@redhat.com>
> >> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >>   lib/s390x/interrupt.c | 22 +++++++++++++++++++++-
> >>   lib/s390x/interrupt.h |  7 +++++++
> >>   2 files changed, 28 insertions(+), 1 deletion(-)
> >>   create mode 100644 lib/s390x/interrupt.h
> >>

> >> +static void (*io_int_func)(void);
> >> +
> >>   void handle_io_int(void)
> >>   {
> >> +	if (*io_int_func)
> >> +		return (*io_int_func)();
> >>   	report_abort("Unexpected io interrupt: on cpu %d at %#lx",
> >>   		     stap(), lc->io_old_psw.addr);
> >>   }
> >>   
> >> +int register_io_int_func(void (*f)(void))
> >> +{
> >> +	if (io_int_func)
> >> +		return -1;
> >> +	io_int_func = f;
> >> +	return 0;
> >> +}
> >> +
> >> +int unregister_io_int_func(void (*f)(void))
> >> +{
> >> +	if (io_int_func != f)
> >> +		return -1;
> >> +	io_int_func = NULL;
> >> +	return 0;
> >> +}  
> > 
> > I'm currently working on something similar for PGMs and I see no
> > additional value in two functions for this. Unregistering can be done by
> > doing register_io_int_func(NULL)
> > 
> > This should be enough:
> > 
> > int register_io_int_func(void (*f)(void))
> > {
> > 	io_int_func = f;
> > }

You can even make this void :)

> >   
> There are several ways to do this and I really don't mind how it is done.
> Since it has been reviewed by, I would like to have the others reviewers 
> opinion.

One version might make it easier to catch programming errors, while the
other one is more compact. I don't really have a preference on this,
either is fine with me.