[kvm-unit-tests PATCH v3 0/9] s390x: Testing the Channel Subsystem I/O

[kvm-unit-tests PATCH v3 0/9] 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 (9):
  s390x: saving regs for interrupts
  s390x: Define the PSW bits
  s390: 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 | 127 ++++++++-------
 lib/s390x/asm/time.h     |  27 ++++
 lib/s390x/css.h          | 273 +++++++++++++++++++++++++++++++
 lib/s390x/css_dump.c     | 156 ++++++++++++++++++
 lib/s390x/interrupt.c    |  23 ++-
 lib/s390x/interrupt.h    |   7 +
 s390x/Makefile           |   2 +
 s390x/css.c              | 335 +++++++++++++++++++++++++++++++++++++++
 s390x/cstart64.S         |  38 ++++-
 s390x/intercept.c        |  11 +-
 s390x/unittests.cfg      |   4 +
 11 files changed, 924 insertions(+), 79 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/interrupt.h
 create mode 100644 s390x/css.c

-- 
2.17.0

Changelog:
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 v3 1/9] s390x: saving regs for interrupts

[Pierre Morel]

If we use multiple source of interrupts, for exemple, 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 | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 86dd4c4..ff05f9b 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -118,6 +118,25 @@ memsetxc:
 	lmg	%r0, %r15, GEN_LC_SW_INT_GRS
 	.endm
 
+	.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
+	lgr     %r2, %r15
+	.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:
@@ -154,6 +173,8 @@ diag308_load_reset:
 	lpswe	GEN_LC_SW_INT_PSW
 1:	br	%r14
 
+
+
 .globl smp_cpu_setup_state
 smp_cpu_setup_state:
 	xgr	%r1, %r1
@@ -180,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 v3 2/9] s390x: Define the PSW bits

[Pierre Morel]

Let's define the PSW bits explicitly, it will clarify their
usage.

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

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index cf6e1ca..1293640 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -10,20 +10,81 @@
 #ifndef _ASM_S390X_ARCH_DEF_H_
 #define _ASM_S390X_ARCH_DEF_H_
 
-struct psw {
-	uint64_t	mask;
-	uint64_t	addr;
-};
-
 #define PSW_MASK_EXT			0x0100000000000000UL
 #define PSW_MASK_DAT			0x0400000000000000UL
 #define PSW_MASK_PSTATE			0x0001000000000000UL
+#define PSW_MASK_IO			0x0200000000000000
+#define PSW_MASK_EA			0x0000000100000000
+#define PSW_MASK_BA			0x0000000080000000
+
+#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 PGM_INT_CODE_OPERATION			0x01
+#define PGM_INT_CODE_PRIVILEGED_OPERATION	0x02
+#define PGM_INT_CODE_EXECUTE			0x03
+#define PGM_INT_CODE_PROTECTION			0x04
+#define PGM_INT_CODE_ADDRESSING			0x05
+#define PGM_INT_CODE_SPECIFICATION		0x06
+#define PGM_INT_CODE_DATA			0x07
+#define PGM_INT_CODE_FIXED_POINT_OVERFLOW	0x08
+#define PGM_INT_CODE_FIXED_POINT_DIVIDE		0x09
+#define PGM_INT_CODE_DECIMAL_OVERFLOW		0x0a
+#define PGM_INT_CODE_DECIMAL_DIVIDE		0x0b
+#define PGM_INT_CODE_HFP_EXPONENT_OVERFLOW	0x0c
+#define PGM_INT_CODE_HFP_EXPONENT_UNDERFLOW	0x0d
+#define PGM_INT_CODE_HFP_SIGNIFICANCE		0x0e
+#define PGM_INT_CODE_HFP_DIVIDE			0x0f
+#define PGM_INT_CODE_SEGMENT_TRANSLATION	0x10
+#define PGM_INT_CODE_PAGE_TRANSLATION		0x11
+#define PGM_INT_CODE_TRANSLATION_SPEC		0x12
+#define PGM_INT_CODE_SPECIAL_OPERATION		0x13
+#define PGM_INT_CODE_OPERAND			0x15
+#define PGM_INT_CODE_TRACE_TABLE		0x16
+#define PGM_INT_CODE_VECTOR_PROCESSING		0x1b
+#define PGM_INT_CODE_SPACE_SWITCH_EVENT		0x1c
+#define PGM_INT_CODE_HFP_SQUARE_ROOT		0x1d
+#define PGM_INT_CODE_PC_TRANSLATION_SPEC	0x1f
+#define PGM_INT_CODE_AFX_TRANSLATION		0x20
+#define PGM_INT_CODE_ASX_TRANSLATION		0x21
+#define PGM_INT_CODE_LX_TRANSLATION		0x22
+#define PGM_INT_CODE_EX_TRANSLATION		0x23
+#define PGM_INT_CODE_PRIMARY_AUTHORITY		0x24
+#define PGM_INT_CODE_SECONDARY_AUTHORITY	0x25
+#define PGM_INT_CODE_LFX_TRANSLATION		0x26
+#define PGM_INT_CODE_LSX_TRANSLATION		0x27
+#define PGM_INT_CODE_ALET_SPECIFICATION		0x28
+#define PGM_INT_CODE_ALEN_TRANSLATION		0x29
+#define PGM_INT_CODE_ALE_SEQUENCE		0x2a
+#define PGM_INT_CODE_ASTE_VALIDITY		0x2b
+#define PGM_INT_CODE_ASTE_SEQUENCE		0x2c
+#define PGM_INT_CODE_EXTENDED_AUTHORITY		0x2d
+#define PGM_INT_CODE_LSTE_SEQUENCE		0x2e
+#define PGM_INT_CODE_ASTE_INSTANCE		0x2f
+#define PGM_INT_CODE_STACK_FULL			0x30
+#define PGM_INT_CODE_STACK_EMPTY		0x31
+#define PGM_INT_CODE_STACK_SPECIFICATION	0x32
+#define PGM_INT_CODE_STACK_TYPE			0x33
+#define PGM_INT_CODE_STACK_OPERATION		0x34
+#define PGM_INT_CODE_ASCE_TYPE			0x38
+#define PGM_INT_CODE_REGION_FIRST_TRANS		0x39
+#define PGM_INT_CODE_REGION_SECOND_TRANS	0x3a
+#define PGM_INT_CODE_REGION_THIRD_TRANS		0x3b
+#define PGM_INT_CODE_MONITOR_EVENT		0x40
+#define PGM_INT_CODE_PER			0x80
+#define PGM_INT_CODE_CRYPTO_OPERATION		0x119
+#define PGM_INT_CODE_TX_ABORTED_EVENT		0x200
+
+#ifndef __ASSEMBLER__
+struct psw {
+	uint64_t	mask;
+	uint64_t	addr;
+};
+
 struct lowcore {
 	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
 	uint32_t	ext_int_param;			/* 0x0080 */
@@ -101,61 +162,6 @@ struct lowcore {
 } __attribute__ ((__packed__));
 _Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");
 
-#define PGM_INT_CODE_OPERATION			0x01
-#define PGM_INT_CODE_PRIVILEGED_OPERATION	0x02
-#define PGM_INT_CODE_EXECUTE			0x03
-#define PGM_INT_CODE_PROTECTION			0x04
-#define PGM_INT_CODE_ADDRESSING			0x05
-#define PGM_INT_CODE_SPECIFICATION		0x06
-#define PGM_INT_CODE_DATA			0x07
-#define PGM_INT_CODE_FIXED_POINT_OVERFLOW	0x08
-#define PGM_INT_CODE_FIXED_POINT_DIVIDE		0x09
-#define PGM_INT_CODE_DECIMAL_OVERFLOW		0x0a
-#define PGM_INT_CODE_DECIMAL_DIVIDE		0x0b
-#define PGM_INT_CODE_HFP_EXPONENT_OVERFLOW	0x0c
-#define PGM_INT_CODE_HFP_EXPONENT_UNDERFLOW	0x0d
-#define PGM_INT_CODE_HFP_SIGNIFICANCE		0x0e
-#define PGM_INT_CODE_HFP_DIVIDE			0x0f
-#define PGM_INT_CODE_SEGMENT_TRANSLATION	0x10
-#define PGM_INT_CODE_PAGE_TRANSLATION		0x11
-#define PGM_INT_CODE_TRANSLATION_SPEC		0x12
-#define PGM_INT_CODE_SPECIAL_OPERATION		0x13
-#define PGM_INT_CODE_OPERAND			0x15
-#define PGM_INT_CODE_TRACE_TABLE		0x16
-#define PGM_INT_CODE_VECTOR_PROCESSING		0x1b
-#define PGM_INT_CODE_SPACE_SWITCH_EVENT		0x1c
-#define PGM_INT_CODE_HFP_SQUARE_ROOT		0x1d
-#define PGM_INT_CODE_PC_TRANSLATION_SPEC	0x1f
-#define PGM_INT_CODE_AFX_TRANSLATION		0x20
-#define PGM_INT_CODE_ASX_TRANSLATION		0x21
-#define PGM_INT_CODE_LX_TRANSLATION		0x22
-#define PGM_INT_CODE_EX_TRANSLATION		0x23
-#define PGM_INT_CODE_PRIMARY_AUTHORITY		0x24
-#define PGM_INT_CODE_SECONDARY_AUTHORITY	0x25
-#define PGM_INT_CODE_LFX_TRANSLATION		0x26
-#define PGM_INT_CODE_LSX_TRANSLATION		0x27
-#define PGM_INT_CODE_ALET_SPECIFICATION		0x28
-#define PGM_INT_CODE_ALEN_TRANSLATION		0x29
-#define PGM_INT_CODE_ALE_SEQUENCE		0x2a
-#define PGM_INT_CODE_ASTE_VALIDITY		0x2b
-#define PGM_INT_CODE_ASTE_SEQUENCE		0x2c
-#define PGM_INT_CODE_EXTENDED_AUTHORITY		0x2d
-#define PGM_INT_CODE_LSTE_SEQUENCE		0x2e
-#define PGM_INT_CODE_ASTE_INSTANCE		0x2f
-#define PGM_INT_CODE_STACK_FULL			0x30
-#define PGM_INT_CODE_STACK_EMPTY		0x31
-#define PGM_INT_CODE_STACK_SPECIFICATION	0x32
-#define PGM_INT_CODE_STACK_TYPE			0x33
-#define PGM_INT_CODE_STACK_OPERATION		0x34
-#define PGM_INT_CODE_ASCE_TYPE			0x38
-#define PGM_INT_CODE_REGION_FIRST_TRANS		0x39
-#define PGM_INT_CODE_REGION_SECOND_TRANS	0x3a
-#define PGM_INT_CODE_REGION_THIRD_TRANS		0x3b
-#define PGM_INT_CODE_MONITOR_EVENT		0x40
-#define PGM_INT_CODE_PER			0x80
-#define PGM_INT_CODE_CRYPTO_OPERATION		0x119
-#define PGM_INT_CODE_TX_ABORTED_EVENT		0x200
-
 struct cpuid {
 	uint64_t version : 8;
 	uint64_t id : 24;
@@ -271,4 +277,5 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2)
 	return cc;
 }
 
+#endif /* __ASSEMBLER__ */
 #endif
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index ff05f9b..f292ed6 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
 
@@ -216,17 +217,17 @@ svc_int:
 reset_psw:
 	.quad	0x0008000180000000
 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

Re: [kvm-unit-tests PATCH v3 2/9] s390x: Define the PSW bits

[Thomas Huth]

On 06/12/2019 17.26, Pierre Morel wrote:
> Let's define the PSW bits explicitly, it will clarify their
> usage.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_def.h | 127 +++++++++++++++++++++------------------
>  s390x/cstart64.S         |  13 ++--
>  2 files changed, 74 insertions(+), 66 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index cf6e1ca..1293640 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -10,20 +10,81 @@
>  #ifndef _ASM_S390X_ARCH_DEF_H_
>  #define _ASM_S390X_ARCH_DEF_H_
>  
> -struct psw {
> -	uint64_t	mask;
> -	uint64_t	addr;
> -};
> -
>  #define PSW_MASK_EXT			0x0100000000000000UL
>  #define PSW_MASK_DAT			0x0400000000000000UL
>  #define PSW_MASK_PSTATE			0x0001000000000000UL
> +#define PSW_MASK_IO			0x0200000000000000
> +#define PSW_MASK_EA			0x0000000100000000
> +#define PSW_MASK_BA			0x0000000080000000
> +
> +#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 PGM_INT_CODE_OPERATION			0x01
> +#define PGM_INT_CODE_PRIVILEGED_OPERATION	0x02
> +#define PGM_INT_CODE_EXECUTE			0x03
> +#define PGM_INT_CODE_PROTECTION			0x04
> +#define PGM_INT_CODE_ADDRESSING			0x05
> +#define PGM_INT_CODE_SPECIFICATION		0x06
> +#define PGM_INT_CODE_DATA			0x07
> +#define PGM_INT_CODE_FIXED_POINT_OVERFLOW	0x08
> +#define PGM_INT_CODE_FIXED_POINT_DIVIDE		0x09
> +#define PGM_INT_CODE_DECIMAL_OVERFLOW		0x0a
> +#define PGM_INT_CODE_DECIMAL_DIVIDE		0x0b
> +#define PGM_INT_CODE_HFP_EXPONENT_OVERFLOW	0x0c
> +#define PGM_INT_CODE_HFP_EXPONENT_UNDERFLOW	0x0d
> +#define PGM_INT_CODE_HFP_SIGNIFICANCE		0x0e
> +#define PGM_INT_CODE_HFP_DIVIDE			0x0f
> +#define PGM_INT_CODE_SEGMENT_TRANSLATION	0x10
> +#define PGM_INT_CODE_PAGE_TRANSLATION		0x11
> +#define PGM_INT_CODE_TRANSLATION_SPEC		0x12
> +#define PGM_INT_CODE_SPECIAL_OPERATION		0x13
> +#define PGM_INT_CODE_OPERAND			0x15
> +#define PGM_INT_CODE_TRACE_TABLE		0x16
> +#define PGM_INT_CODE_VECTOR_PROCESSING		0x1b
> +#define PGM_INT_CODE_SPACE_SWITCH_EVENT		0x1c
> +#define PGM_INT_CODE_HFP_SQUARE_ROOT		0x1d
> +#define PGM_INT_CODE_PC_TRANSLATION_SPEC	0x1f
> +#define PGM_INT_CODE_AFX_TRANSLATION		0x20
> +#define PGM_INT_CODE_ASX_TRANSLATION		0x21
> +#define PGM_INT_CODE_LX_TRANSLATION		0x22
> +#define PGM_INT_CODE_EX_TRANSLATION		0x23
> +#define PGM_INT_CODE_PRIMARY_AUTHORITY		0x24
> +#define PGM_INT_CODE_SECONDARY_AUTHORITY	0x25
> +#define PGM_INT_CODE_LFX_TRANSLATION		0x26
> +#define PGM_INT_CODE_LSX_TRANSLATION		0x27
> +#define PGM_INT_CODE_ALET_SPECIFICATION		0x28
> +#define PGM_INT_CODE_ALEN_TRANSLATION		0x29
> +#define PGM_INT_CODE_ALE_SEQUENCE		0x2a
> +#define PGM_INT_CODE_ASTE_VALIDITY		0x2b
> +#define PGM_INT_CODE_ASTE_SEQUENCE		0x2c
> +#define PGM_INT_CODE_EXTENDED_AUTHORITY		0x2d
> +#define PGM_INT_CODE_LSTE_SEQUENCE		0x2e
> +#define PGM_INT_CODE_ASTE_INSTANCE		0x2f
> +#define PGM_INT_CODE_STACK_FULL			0x30
> +#define PGM_INT_CODE_STACK_EMPTY		0x31
> +#define PGM_INT_CODE_STACK_SPECIFICATION	0x32
> +#define PGM_INT_CODE_STACK_TYPE			0x33
> +#define PGM_INT_CODE_STACK_OPERATION		0x34
> +#define PGM_INT_CODE_ASCE_TYPE			0x38
> +#define PGM_INT_CODE_REGION_FIRST_TRANS		0x39
> +#define PGM_INT_CODE_REGION_SECOND_TRANS	0x3a
> +#define PGM_INT_CODE_REGION_THIRD_TRANS		0x3b
> +#define PGM_INT_CODE_MONITOR_EVENT		0x40
> +#define PGM_INT_CODE_PER			0x80
> +#define PGM_INT_CODE_CRYPTO_OPERATION		0x119
> +#define PGM_INT_CODE_TX_ABORTED_EVENT		0x200
> +
> +#ifndef __ASSEMBLER__
> +struct psw {
> +	uint64_t	mask;
> +	uint64_t	addr;
> +};
> +
>  struct lowcore {
>  	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
>  	uint32_t	ext_int_param;			/* 0x0080 */
> @@ -101,61 +162,6 @@ struct lowcore {
>  } __attribute__ ((__packed__));
>  _Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");
>  
> -#define PGM_INT_CODE_OPERATION			0x01
> -#define PGM_INT_CODE_PRIVILEGED_OPERATION	0x02
> -#define PGM_INT_CODE_EXECUTE			0x03
> -#define PGM_INT_CODE_PROTECTION			0x04
> -#define PGM_INT_CODE_ADDRESSING			0x05
> -#define PGM_INT_CODE_SPECIFICATION		0x06
> -#define PGM_INT_CODE_DATA			0x07
> -#define PGM_INT_CODE_FIXED_POINT_OVERFLOW	0x08
> -#define PGM_INT_CODE_FIXED_POINT_DIVIDE		0x09
> -#define PGM_INT_CODE_DECIMAL_OVERFLOW		0x0a
> -#define PGM_INT_CODE_DECIMAL_DIVIDE		0x0b
> -#define PGM_INT_CODE_HFP_EXPONENT_OVERFLOW	0x0c
> -#define PGM_INT_CODE_HFP_EXPONENT_UNDERFLOW	0x0d
> -#define PGM_INT_CODE_HFP_SIGNIFICANCE		0x0e
> -#define PGM_INT_CODE_HFP_DIVIDE			0x0f
> -#define PGM_INT_CODE_SEGMENT_TRANSLATION	0x10
> -#define PGM_INT_CODE_PAGE_TRANSLATION		0x11
> -#define PGM_INT_CODE_TRANSLATION_SPEC		0x12
> -#define PGM_INT_CODE_SPECIAL_OPERATION		0x13
> -#define PGM_INT_CODE_OPERAND			0x15
> -#define PGM_INT_CODE_TRACE_TABLE		0x16
> -#define PGM_INT_CODE_VECTOR_PROCESSING		0x1b
> -#define PGM_INT_CODE_SPACE_SWITCH_EVENT		0x1c
> -#define PGM_INT_CODE_HFP_SQUARE_ROOT		0x1d
> -#define PGM_INT_CODE_PC_TRANSLATION_SPEC	0x1f
> -#define PGM_INT_CODE_AFX_TRANSLATION		0x20
> -#define PGM_INT_CODE_ASX_TRANSLATION		0x21
> -#define PGM_INT_CODE_LX_TRANSLATION		0x22
> -#define PGM_INT_CODE_EX_TRANSLATION		0x23
> -#define PGM_INT_CODE_PRIMARY_AUTHORITY		0x24
> -#define PGM_INT_CODE_SECONDARY_AUTHORITY	0x25
> -#define PGM_INT_CODE_LFX_TRANSLATION		0x26
> -#define PGM_INT_CODE_LSX_TRANSLATION		0x27
> -#define PGM_INT_CODE_ALET_SPECIFICATION		0x28
> -#define PGM_INT_CODE_ALEN_TRANSLATION		0x29
> -#define PGM_INT_CODE_ALE_SEQUENCE		0x2a
> -#define PGM_INT_CODE_ASTE_VALIDITY		0x2b
> -#define PGM_INT_CODE_ASTE_SEQUENCE		0x2c
> -#define PGM_INT_CODE_EXTENDED_AUTHORITY		0x2d
> -#define PGM_INT_CODE_LSTE_SEQUENCE		0x2e
> -#define PGM_INT_CODE_ASTE_INSTANCE		0x2f
> -#define PGM_INT_CODE_STACK_FULL			0x30
> -#define PGM_INT_CODE_STACK_EMPTY		0x31
> -#define PGM_INT_CODE_STACK_SPECIFICATION	0x32
> -#define PGM_INT_CODE_STACK_TYPE			0x33
> -#define PGM_INT_CODE_STACK_OPERATION		0x34
> -#define PGM_INT_CODE_ASCE_TYPE			0x38
> -#define PGM_INT_CODE_REGION_FIRST_TRANS		0x39
> -#define PGM_INT_CODE_REGION_SECOND_TRANS	0x3a
> -#define PGM_INT_CODE_REGION_THIRD_TRANS		0x3b
> -#define PGM_INT_CODE_MONITOR_EVENT		0x40
> -#define PGM_INT_CODE_PER			0x80
> -#define PGM_INT_CODE_CRYPTO_OPERATION		0x119
> -#define PGM_INT_CODE_TX_ABORTED_EVENT		0x200

This patch definitely does more than you've mentioned in the patch
description. Please avoid the movement of the PGM definitions here and
move that into a separate patch (if it is really necessary).

 Thomas

Re: [kvm-unit-tests PATCH v3 2/9] s390x: Define the PSW bits

[Pierre Morel]

On 2019-12-06 17:29, Thomas Huth wrote:
> On 06/12/2019 17.26, Pierre Morel wrote:
>> Let's define the PSW bits explicitly, it will clarify their
>> usage.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/asm/arch_def.h | 127 +++++++++++++++++++++------------------
>>   s390x/cstart64.S         |  13 ++--
>>   2 files changed, 74 insertions(+), 66 deletions(-)
>>
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index cf6e1ca..1293640 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -10,20 +10,81 @@
>>   #ifndef _ASM_S390X_ARCH_DEF_H_
>>   #define _ASM_S390X_ARCH_DEF_H_
>>   
>> -struct psw {
>> -	uint64_t	mask;
>> -	uint64_t	addr;
>> -};
>> -
>>   #define PSW_MASK_EXT			0x0100000000000000UL
>>   #define PSW_MASK_DAT			0x0400000000000000UL
>>   #define PSW_MASK_PSTATE			0x0001000000000000UL
>> +#define PSW_MASK_IO			0x0200000000000000
>> +#define PSW_MASK_EA			0x0000000100000000
>> +#define PSW_MASK_BA			0x0000000080000000
>> +
>> +#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 PGM_INT_CODE_OPERATION			0x01
>> +#define PGM_INT_CODE_PRIVILEGED_OPERATION	0x02
>> +#define PGM_INT_CODE_EXECUTE			0x03
>> +#define PGM_INT_CODE_PROTECTION			0x04
>> +#define PGM_INT_CODE_ADDRESSING			0x05
>> +#define PGM_INT_CODE_SPECIFICATION		0x06
>> +#define PGM_INT_CODE_DATA			0x07
>> +#define PGM_INT_CODE_FIXED_POINT_OVERFLOW	0x08
>> +#define PGM_INT_CODE_FIXED_POINT_DIVIDE		0x09
>> +#define PGM_INT_CODE_DECIMAL_OVERFLOW		0x0a
>> +#define PGM_INT_CODE_DECIMAL_DIVIDE		0x0b
>> +#define PGM_INT_CODE_HFP_EXPONENT_OVERFLOW	0x0c
>> +#define PGM_INT_CODE_HFP_EXPONENT_UNDERFLOW	0x0d
>> +#define PGM_INT_CODE_HFP_SIGNIFICANCE		0x0e
>> +#define PGM_INT_CODE_HFP_DIVIDE			0x0f
>> +#define PGM_INT_CODE_SEGMENT_TRANSLATION	0x10
>> +#define PGM_INT_CODE_PAGE_TRANSLATION		0x11
>> +#define PGM_INT_CODE_TRANSLATION_SPEC		0x12
>> +#define PGM_INT_CODE_SPECIAL_OPERATION		0x13
>> +#define PGM_INT_CODE_OPERAND			0x15
>> +#define PGM_INT_CODE_TRACE_TABLE		0x16
>> +#define PGM_INT_CODE_VECTOR_PROCESSING		0x1b
>> +#define PGM_INT_CODE_SPACE_SWITCH_EVENT		0x1c
>> +#define PGM_INT_CODE_HFP_SQUARE_ROOT		0x1d
>> +#define PGM_INT_CODE_PC_TRANSLATION_SPEC	0x1f
>> +#define PGM_INT_CODE_AFX_TRANSLATION		0x20
>> +#define PGM_INT_CODE_ASX_TRANSLATION		0x21
>> +#define PGM_INT_CODE_LX_TRANSLATION		0x22
>> +#define PGM_INT_CODE_EX_TRANSLATION		0x23
>> +#define PGM_INT_CODE_PRIMARY_AUTHORITY		0x24
>> +#define PGM_INT_CODE_SECONDARY_AUTHORITY	0x25
>> +#define PGM_INT_CODE_LFX_TRANSLATION		0x26
>> +#define PGM_INT_CODE_LSX_TRANSLATION		0x27
>> +#define PGM_INT_CODE_ALET_SPECIFICATION		0x28
>> +#define PGM_INT_CODE_ALEN_TRANSLATION		0x29
>> +#define PGM_INT_CODE_ALE_SEQUENCE		0x2a
>> +#define PGM_INT_CODE_ASTE_VALIDITY		0x2b
>> +#define PGM_INT_CODE_ASTE_SEQUENCE		0x2c
>> +#define PGM_INT_CODE_EXTENDED_AUTHORITY		0x2d
>> +#define PGM_INT_CODE_LSTE_SEQUENCE		0x2e
>> +#define PGM_INT_CODE_ASTE_INSTANCE		0x2f
>> +#define PGM_INT_CODE_STACK_FULL			0x30
>> +#define PGM_INT_CODE_STACK_EMPTY		0x31
>> +#define PGM_INT_CODE_STACK_SPECIFICATION	0x32
>> +#define PGM_INT_CODE_STACK_TYPE			0x33
>> +#define PGM_INT_CODE_STACK_OPERATION		0x34
>> +#define PGM_INT_CODE_ASCE_TYPE			0x38
>> +#define PGM_INT_CODE_REGION_FIRST_TRANS		0x39
>> +#define PGM_INT_CODE_REGION_SECOND_TRANS	0x3a
>> +#define PGM_INT_CODE_REGION_THIRD_TRANS		0x3b
>> +#define PGM_INT_CODE_MONITOR_EVENT		0x40
>> +#define PGM_INT_CODE_PER			0x80
>> +#define PGM_INT_CODE_CRYPTO_OPERATION		0x119
>> +#define PGM_INT_CODE_TX_ABORTED_EVENT		0x200
>> +
>> +#ifndef __ASSEMBLER__
>> +struct psw {
>> +	uint64_t	mask;
>> +	uint64_t	addr;
>> +};
>> +
>>   struct lowcore {
>>   	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
>>   	uint32_t	ext_int_param;			/* 0x0080 */
>> @@ -101,61 +162,6 @@ struct lowcore {
>>   } __attribute__ ((__packed__));
>>   _Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");
>>   
>> -#define PGM_INT_CODE_OPERATION			0x01
>> -#define PGM_INT_CODE_PRIVILEGED_OPERATION	0x02
>> -#define PGM_INT_CODE_EXECUTE			0x03
>> -#define PGM_INT_CODE_PROTECTION			0x04
>> -#define PGM_INT_CODE_ADDRESSING			0x05
>> -#define PGM_INT_CODE_SPECIFICATION		0x06
>> -#define PGM_INT_CODE_DATA			0x07
>> -#define PGM_INT_CODE_FIXED_POINT_OVERFLOW	0x08
>> -#define PGM_INT_CODE_FIXED_POINT_DIVIDE		0x09
>> -#define PGM_INT_CODE_DECIMAL_OVERFLOW		0x0a
>> -#define PGM_INT_CODE_DECIMAL_DIVIDE		0x0b
>> -#define PGM_INT_CODE_HFP_EXPONENT_OVERFLOW	0x0c
>> -#define PGM_INT_CODE_HFP_EXPONENT_UNDERFLOW	0x0d
>> -#define PGM_INT_CODE_HFP_SIGNIFICANCE		0x0e
>> -#define PGM_INT_CODE_HFP_DIVIDE			0x0f
>> -#define PGM_INT_CODE_SEGMENT_TRANSLATION	0x10
>> -#define PGM_INT_CODE_PAGE_TRANSLATION		0x11
>> -#define PGM_INT_CODE_TRANSLATION_SPEC		0x12
>> -#define PGM_INT_CODE_SPECIAL_OPERATION		0x13
>> -#define PGM_INT_CODE_OPERAND			0x15
>> -#define PGM_INT_CODE_TRACE_TABLE		0x16
>> -#define PGM_INT_CODE_VECTOR_PROCESSING		0x1b
>> -#define PGM_INT_CODE_SPACE_SWITCH_EVENT		0x1c
>> -#define PGM_INT_CODE_HFP_SQUARE_ROOT		0x1d
>> -#define PGM_INT_CODE_PC_TRANSLATION_SPEC	0x1f
>> -#define PGM_INT_CODE_AFX_TRANSLATION		0x20
>> -#define PGM_INT_CODE_ASX_TRANSLATION		0x21
>> -#define PGM_INT_CODE_LX_TRANSLATION		0x22
>> -#define PGM_INT_CODE_EX_TRANSLATION		0x23
>> -#define PGM_INT_CODE_PRIMARY_AUTHORITY		0x24
>> -#define PGM_INT_CODE_SECONDARY_AUTHORITY	0x25
>> -#define PGM_INT_CODE_LFX_TRANSLATION		0x26
>> -#define PGM_INT_CODE_LSX_TRANSLATION		0x27
>> -#define PGM_INT_CODE_ALET_SPECIFICATION		0x28
>> -#define PGM_INT_CODE_ALEN_TRANSLATION		0x29
>> -#define PGM_INT_CODE_ALE_SEQUENCE		0x2a
>> -#define PGM_INT_CODE_ASTE_VALIDITY		0x2b
>> -#define PGM_INT_CODE_ASTE_SEQUENCE		0x2c
>> -#define PGM_INT_CODE_EXTENDED_AUTHORITY		0x2d
>> -#define PGM_INT_CODE_LSTE_SEQUENCE		0x2e
>> -#define PGM_INT_CODE_ASTE_INSTANCE		0x2f
>> -#define PGM_INT_CODE_STACK_FULL			0x30
>> -#define PGM_INT_CODE_STACK_EMPTY		0x31
>> -#define PGM_INT_CODE_STACK_SPECIFICATION	0x32
>> -#define PGM_INT_CODE_STACK_TYPE			0x33
>> -#define PGM_INT_CODE_STACK_OPERATION		0x34
>> -#define PGM_INT_CODE_ASCE_TYPE			0x38
>> -#define PGM_INT_CODE_REGION_FIRST_TRANS		0x39
>> -#define PGM_INT_CODE_REGION_SECOND_TRANS	0x3a
>> -#define PGM_INT_CODE_REGION_THIRD_TRANS		0x3b
>> -#define PGM_INT_CODE_MONITOR_EVENT		0x40
>> -#define PGM_INT_CODE_PER			0x80
>> -#define PGM_INT_CODE_CRYPTO_OPERATION		0x119
>> -#define PGM_INT_CODE_TX_ABORTED_EVENT		0x200
> 
> This patch definitely does more than you've mentioned in the patch
> description. Please avoid the movement of the PGM definitions here and
> move that into a separate patch (if it is really necessary).
> 
>   Thomas
> 

OK, thanks,

Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 3/9] s390: interrupt registration

[Pierre Morel]

Define two functions to register and to unregister a call back for IO
Interrupt handling.

Per default we keep the old behavior, so does a successful unregister
of the callback.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/interrupt.c | 23 ++++++++++++++++++++++-
 lib/s390x/interrupt.h |  7 +++++++
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 lib/s390x/interrupt.h

diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 3e07867..e0eae4d 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;
@@ -140,12 +140,33 @@ void handle_mcck_int(void)
 		     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: at %#lx",
 		     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: 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

Re: [kvm-unit-tests PATCH v3 3/9] s390: interrupt registration

[Thomas Huth]

On 06/12/2019 17.26, Pierre Morel wrote:
> Define two functions to register and to unregister a call back for IO
> Interrupt handling.
> 
> Per default we keep the old behavior, so does a successful unregister
> of the callback.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/interrupt.c | 23 ++++++++++++++++++++++-
>  lib/s390x/interrupt.h |  7 +++++++
>  2 files changed, 29 insertions(+), 1 deletion(-)
>  create mode 100644 lib/s390x/interrupt.h
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 3e07867..e0eae4d 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;
> @@ -140,12 +140,33 @@ void handle_mcck_int(void)
>  		     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: at %#lx",
>  		     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: 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
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH v3 3/9] s390: interrupt registration

[Pierre Morel]

On 2019-12-09 12:40, Thomas Huth wrote:
> On 06/12/2019 17.26, Pierre Morel wrote:
>> Define two functions to register and to unregister a call back for IO
>> Interrupt handling.
>>
>> Per default we keep the old behavior, so does a successful unregister
>> of the callback.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/interrupt.c | 23 ++++++++++++++++++++++-
>>   lib/s390x/interrupt.h |  7 +++++++
>>   2 files changed, 29 insertions(+), 1 deletion(-)
>>   create mode 100644 lib/s390x/interrupt.h
>>
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 3e07867..e0eae4d 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;
>> @@ -140,12 +140,33 @@ void handle_mcck_int(void)
>>   		     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: at %#lx",
>>   		     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: 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
>>
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 3/9] s390: interrupt registration

[David Hildenbrand]

On 06.12.19 17:26, Pierre Morel wrote:
> Define two functions to register and to unregister a call back for IO
> Interrupt handling.
> 
> Per default we keep the old behavior, so does a successful unregister
> of the callback.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/interrupt.c | 23 ++++++++++++++++++++++-
>  lib/s390x/interrupt.h |  7 +++++++
>  2 files changed, 29 insertions(+), 1 deletion(-)
>  create mode 100644 lib/s390x/interrupt.h
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 3e07867..e0eae4d 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;
> @@ -140,12 +140,33 @@ void handle_mcck_int(void)
>  		     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: at %#lx",
>  		     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: 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
> 

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

-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v3 3/9] s390: interrupt registration

[Pierre Morel]

On 2019-12-09 12:48, David Hildenbrand wrote:
> On 06.12.19 17:26, Pierre Morel wrote:
>> Define two functions to register and to unregister a call back for IO
>> Interrupt handling.
>>
>> Per default we keep the old behavior, so does a successful unregister
>> of the callback.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/interrupt.c | 23 ++++++++++++++++++++++-
>>   lib/s390x/interrupt.h |  7 +++++++
>>   2 files changed, 29 insertions(+), 1 deletion(-)
>>   create mode 100644 lib/s390x/interrupt.h
>>
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 3e07867..e0eae4d 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;
>> @@ -140,12 +140,33 @@ void handle_mcck_int(void)
>>   		     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: at %#lx",
>>   		     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: 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
>>
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 4/9] s390x: export the clock get_clock_ms() utility

[Pierre Morel]

To serve multiple times, the function get_clock_ms() is moved
from intercept.c test to the new file asm/time.h.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 lib/s390x/asm/time.h | 27 +++++++++++++++++++++++++++
 s390x/intercept.c    | 11 +----------
 2 files changed, 28 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..b07ccbd
--- /dev/null
+++ b/lib/s390x/asm/time.h
@@ -0,0 +1,27 @@
+/*
+ * 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 404b4c6..18aa60d 100644
--- a/s390x/intercept.c
+++ b/s390x/intercept.c
@@ -13,6 +13,7 @@
 #include <asm/asm-offsets.h>
 #include <asm/interrupt.h>
 #include <asm/page.h>
+#include <asm/time.h>
 
 static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
 
@@ -159,16 +160,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

Re: [kvm-unit-tests PATCH v3 4/9] s390x: export the clock get_clock_ms() utility

[Thomas Huth]

On 06/12/2019 17.26, Pierre Morel wrote:
> To serve multiple times, the function get_clock_ms() is moved
> from intercept.c test to the new file asm/time.h.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
>  lib/s390x/asm/time.h | 27 +++++++++++++++++++++++++++
>  s390x/intercept.c    | 11 +----------
>  2 files changed, 28 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..b07ccbd
> --- /dev/null
> +++ b/lib/s390x/asm/time.h
> @@ -0,0 +1,27 @@
> +/*
> + * 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;
> +}
> +
> +

Please remove one of the two empty lines.

With that cosmetic nit fixed:
Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH v3 4/9] s390x: export the clock get_clock_ms() utility

[David Hildenbrand]

On 09.12.19 12:42, Thomas Huth wrote:
> On 06/12/2019 17.26, Pierre Morel wrote:
>> To serve multiple times, the function get_clock_ms() is moved
>> from intercept.c test to the new file asm/time.h.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> ---
>>  lib/s390x/asm/time.h | 27 +++++++++++++++++++++++++++
>>  s390x/intercept.c    | 11 +----------
>>  2 files changed, 28 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..b07ccbd
>> --- /dev/null
>> +++ b/lib/s390x/asm/time.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * 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;
>> +}
>> +
>> +
> 
> Please remove one of the two empty lines.
> 
> With that cosmetic nit fixed:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v3 4/9] s390x: export the clock get_clock_ms() utility

[Pierre Morel]

On 2019-12-09 12:49, David Hildenbrand wrote:
> On 09.12.19 12:42, Thomas Huth wrote:
>> On 06/12/2019 17.26, Pierre Morel wrote:
>>> To serve multiple times, the function get_clock_ms() is moved
>>> from intercept.c test to the new file asm/time.h.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> Reviewed-by: David Hildenbrand <david@redhat.com>
>>> ---
>>>   lib/s390x/asm/time.h | 27 +++++++++++++++++++++++++++
>>>   s390x/intercept.c    | 11 +----------
>>>   2 files changed, 28 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..b07ccbd
>>> --- /dev/null
>>> +++ b/lib/s390x/asm/time.h
>>> @@ -0,0 +1,27 @@
>>> +/*
>>> + * 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;
>>> +}
>>> +
>>> +
>>
>> Please remove one of the two empty lines.
>>
>> With that cosmetic nit fixed:
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 4/9] s390x: export the clock get_clock_ms() utility

[Pierre Morel]

On 2019-12-09 12:42, Thomas Huth wrote:
> On 06/12/2019 17.26, Pierre Morel wrote:
>> To serve multiple times, the function get_clock_ms() is moved
>> from intercept.c test to the new file asm/time.h.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> ---
>>   lib/s390x/asm/time.h | 27 +++++++++++++++++++++++++++
>>   s390x/intercept.c    | 11 +----------
>>   2 files changed, 28 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..b07ccbd
>> --- /dev/null
>> +++ b/lib/s390x/asm/time.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * 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;
>> +}
>> +
>> +
> 
> Please remove one of the two empty lines.

yes, of course, thanks.

> 
> With that cosmetic nit fixed:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests

[Pierre Morel]

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

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/css.h      | 259 +++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/css_dump.c | 156 ++++++++++++++++++++++++++
 2 files changed, 415 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..6f19bb5
--- /dev/null
+++ b/lib/s390x/css.h
@@ -0,0 +1,259 @@
+/*
+ * 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];
+	uint16_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 = -1;
+
+	asm volatile(
+		"	   ssch	0(%2)\n"
+		"0:	 ipm	 %0\n"
+		"	   srl	 %0,28\n"
+		"1:\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);
+#undef DEBUG
+#ifdef DEBUG
+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
+
+extern unsigned long stacktop;
+#endif
diff --git a/lib/s390x/css_dump.c b/lib/s390x/css_dump.c
new file mode 100644
index 0000000..651755d
--- /dev/null
+++ b/lib/s390x/css_dump.c
@@ -0,0 +1,156 @@
+/*
+ * 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>
+
+#include <css.h>
+
+/*
+ * Try to have a more human representation of the SCSW flags:
+ * each letter in the two strings he under represent the first
+ * letter of the associated bit in the flag.
+ */
+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
+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

Re: [kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests

[Thomas Huth]

On 06/12/2019 17.26, Pierre Morel wrote:
> These are the include and library utilities for the css tests patch
> series.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/css.h      | 259 +++++++++++++++++++++++++++++++++++++++++++
>  lib/s390x/css_dump.c | 156 ++++++++++++++++++++++++++
>  2 files changed, 415 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..6f19bb5
> --- /dev/null
> +++ b/lib/s390x/css.h
[...]
> +/* Debug functions */
> +char *dump_pmcw_flags(uint16_t f);
> +char *dump_scsw_flags(uint32_t f);
> +#undef DEBUG
> +#ifdef DEBUG
> +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

I'd prefer to not have a "#undef DEBUG" (or "#define DEBUG") statement
in the header here - it could trigger unexpected behavior with other
files that also use a DEBUG macro.

Could you please declare the prototypes here and move the "#else" part
to the .c file instead? Thanks!

 Thomas

Re: [kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests

[Pierre Morel]

On 2019-12-09 12:49, Thomas Huth wrote:
> On 06/12/2019 17.26, Pierre Morel wrote:
>> These are the include and library utilities for the css tests patch
>> series.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h      | 259 +++++++++++++++++++++++++++++++++++++++++++
>>   lib/s390x/css_dump.c | 156 ++++++++++++++++++++++++++
>>   2 files changed, 415 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..6f19bb5
>> --- /dev/null
>> +++ b/lib/s390x/css.h
> [...]
>> +/* Debug functions */
>> +char *dump_pmcw_flags(uint16_t f);
>> +char *dump_scsw_flags(uint32_t f);
>> +#undef DEBUG
>> +#ifdef DEBUG
>> +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
> 
> I'd prefer to not have a "#undef DEBUG" (or "#define DEBUG") statement
> in the header here - it could trigger unexpected behavior with other
> files that also use a DEBUG macro.
> 
> Could you please declare the prototypes here and move the "#else" part
> to the .c file instead? Thanks!
> 
>   Thomas
> 

Yes, I can do this.
Thanks
Pierre


-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests

[Pierre Morel]

On 2019-12-09 12:49, Thomas Huth wrote:
> On 06/12/2019 17.26, Pierre Morel wrote:
>> These are the include and library utilities for the css tests patch
>> series.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h      | 259 +++++++++++++++++++++++++++++++++++++++++++
>>   lib/s390x/css_dump.c | 156 ++++++++++++++++++++++++++
>>   2 files changed, 415 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..6f19bb5
>> --- /dev/null
>> +++ b/lib/s390x/css.h
> [...]
>> +/* Debug functions */
>> +char *dump_pmcw_flags(uint16_t f);
>> +char *dump_scsw_flags(uint32_t f);
>> +#undef DEBUG
>> +#ifdef DEBUG
>> +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
> 
> I'd prefer to not have a "#undef DEBUG" (or "#define DEBUG") statement

Anyway hawfull!

> in the header here - it could trigger unexpected behavior with other
> files that also use a DEBUG macro.
> 
> Could you please declare the prototypes here and move the "#else" part
> to the .c file instead? Thanks!

What if I use a CSS_DEBUG here instead of a simple DEBUG definition?

It can be enabled or not by defining CSS_ENABLED ahead of the include...?



-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests

[Thomas Huth]

On 10/12/2019 11.07, Pierre Morel wrote:
> 
> 
> On 2019-12-09 12:49, Thomas Huth wrote:
>> On 06/12/2019 17.26, Pierre Morel wrote:
>>> These are the include and library utilities for the css tests patch
>>> series.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>   lib/s390x/css.h      | 259 +++++++++++++++++++++++++++++++++++++++++++
>>>   lib/s390x/css_dump.c | 156 ++++++++++++++++++++++++++
>>>   2 files changed, 415 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..6f19bb5
>>> --- /dev/null
>>> +++ b/lib/s390x/css.h
>> [...]
>>> +/* Debug functions */
>>> +char *dump_pmcw_flags(uint16_t f);
>>> +char *dump_scsw_flags(uint32_t f);
>>> +#undef DEBUG
>>> +#ifdef DEBUG
>>> +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
>>
>> I'd prefer to not have a "#undef DEBUG" (or "#define DEBUG") statement
> 
> Anyway hawfull!
> 
>> in the header here - it could trigger unexpected behavior with other
>> files that also use a DEBUG macro.
>>
>> Could you please declare the prototypes here and move the "#else" part
>> to the .c file instead? Thanks!
> 
> What if I use a CSS_DEBUG here instead of a simple DEBUG definition?
> 
> It can be enabled or not by defining CSS_ENABLED ahead of the include...?

Why does it have to be in the header and not in the .c file?

 Thomas

Re: [kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests

[Pierre Morel]

On 2019-12-10 11:28, Thomas Huth wrote:
> On 10/12/2019 11.07, Pierre Morel wrote:
>>
>>
>> On 2019-12-09 12:49, Thomas Huth wrote:
>>> On 06/12/2019 17.26, Pierre Morel wrote:
>>>> These are the include and library utilities for the css tests patch
>>>> series.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>    lib/s390x/css.h      | 259 +++++++++++++++++++++++++++++++++++++++++++
>>>>    lib/s390x/css_dump.c | 156 ++++++++++++++++++++++++++
>>>>    2 files changed, 415 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..6f19bb5
>>>> --- /dev/null
>>>> +++ b/lib/s390x/css.h
>>> [...]
>>>> +/* Debug functions */
>>>> +char *dump_pmcw_flags(uint16_t f);
>>>> +char *dump_scsw_flags(uint32_t f);
>>>> +#undef DEBUG
>>>> +#ifdef DEBUG
>>>> +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
>>>
>>> I'd prefer to not have a "#undef DEBUG" (or "#define DEBUG") statement
>>
>> Anyway hawfull!
>>
>>> in the header here - it could trigger unexpected behavior with other
>>> files that also use a DEBUG macro.
>>>
>>> Could you please declare the prototypes here and move the "#else" part
>>> to the .c file instead? Thanks!
>>
>> What if I use a CSS_DEBUG here instead of a simple DEBUG definition?
>>
>> It can be enabled or not by defining CSS_ENABLED ahead of the include...?
> 
> Why does it have to be in the header and not in the .c file?

I too mean in the C file. :)
above the include.

> 
>   Thomas
> 

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests

[Thomas Huth]

On 10/12/2019 12.22, Pierre Morel wrote:
> 
> 
> On 2019-12-10 11:28, Thomas Huth wrote:
>> On 10/12/2019 11.07, Pierre Morel wrote:
>>>
>>>
>>> On 2019-12-09 12:49, Thomas Huth wrote:
>>>> On 06/12/2019 17.26, Pierre Morel wrote:
>>>>> These are the include and library utilities for the css tests patch
>>>>> series.
>>>>>
>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>>> ---
>>>>>    lib/s390x/css.h      | 259
>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>    lib/s390x/css_dump.c | 156 ++++++++++++++++++++++++++
>>>>>    2 files changed, 415 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..6f19bb5
>>>>> --- /dev/null
>>>>> +++ b/lib/s390x/css.h
>>>> [...]
>>>>> +/* Debug functions */
>>>>> +char *dump_pmcw_flags(uint16_t f);
>>>>> +char *dump_scsw_flags(uint32_t f);
>>>>> +#undef DEBUG
>>>>> +#ifdef DEBUG
>>>>> +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
>>>>
>>>> I'd prefer to not have a "#undef DEBUG" (or "#define DEBUG") statement
>>>
>>> Anyway hawfull!
>>>
>>>> in the header here - it could trigger unexpected behavior with other
>>>> files that also use a DEBUG macro.
>>>>
>>>> Could you please declare the prototypes here and move the "#else" part
>>>> to the .c file instead? Thanks!
>>>
>>> What if I use a CSS_DEBUG here instead of a simple DEBUG definition?
>>>
>>> It can be enabled or not by defining CSS_ENABLED ahead of the
>>> include...?
>>
>> Why does it have to be in the header and not in the .c file?
> 
> I too mean in the C file. :)
> above the include.

Well, as long as we don't have any generic "#undef DEBUG" statements in
the header anymore, I think I don't care too much either way.

 Thomas

Re: [kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests

[Pierre Morel]

On 2019-12-10 12:27, Thomas Huth wrote:
> On 10/12/2019 12.22, Pierre Morel wrote:
>>
>>
>> On 2019-12-10 11:28, Thomas Huth wrote:
>>> On 10/12/2019 11.07, Pierre Morel wrote:
>>>>
>>>>
>>>> On 2019-12-09 12:49, Thomas Huth wrote:
>>>>> On 06/12/2019 17.26, Pierre Morel wrote:
>>>>>> These are the include and library utilities for the css tests patch
>>>>>> series.
>>>>>>
>>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>>>> ---
>>>>>>     lib/s390x/css.h      | 259
>>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>>     lib/s390x/css_dump.c | 156 ++++++++++++++++++++++++++
>>>>>>     2 files changed, 415 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..6f19bb5
>>>>>> --- /dev/null
>>>>>> +++ b/lib/s390x/css.h
>>>>> [...]
>>>>>> +/* Debug functions */
>>>>>> +char *dump_pmcw_flags(uint16_t f);
>>>>>> +char *dump_scsw_flags(uint32_t f);
>>>>>> +#undef DEBUG
>>>>>> +#ifdef DEBUG
>>>>>> +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
>>>>>
>>>>> I'd prefer to not have a "#undef DEBUG" (or "#define DEBUG") statement
>>>>
>>>> Anyway hawfull!
>>>>
>>>>> in the header here - it could trigger unexpected behavior with other
>>>>> files that also use a DEBUG macro.
>>>>>
>>>>> Could you please declare the prototypes here and move the "#else" part
>>>>> to the .c file instead? Thanks!
>>>>
>>>> What if I use a CSS_DEBUG here instead of a simple DEBUG definition?
>>>>
>>>> It can be enabled or not by defining CSS_ENABLED ahead of the
>>>> include...?
>>>
>>> Why does it have to be in the header and not in the .c file?
>>
>> I too mean in the C file. :)
>> above the include.
> 
> Well, as long as we don't have any generic "#undef DEBUG" statements in
> the header anymore, I think I don't care too much either way.

Yes, it was not good, shoudn't have survive my first tests.

Thanks
Pierre

> 
>   Thomas
> 

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 6/9] 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.

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

diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 6f19bb5..d37227b 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -82,6 +82,7 @@ struct pmcw {
 	uint8_t  chpid[8];
 	uint16_t flags2;
 };
+#define PMCW_CHANNEL_TYPE(pmcw) (pmcw->flags >> 21)
 
 struct schib {
 	struct pmcw pmcw;
diff --git a/s390x/Makefile b/s390x/Makefile
index 3744372..9ebbb84 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -16,6 +16,7 @@ tests += $(TEST_DIR)/diag288.elf
 tests += $(TEST_DIR)/stsi.elf
 tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
+tests += $(TEST_DIR)/css.elf
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
 all: directories test_cases test_cases_binary
@@ -50,6 +51,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..3d4a986
--- /dev/null
+++ b/s390x/css.c
@@ -0,0 +1,82 @@
+/*
+ * 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 <css.h>
+
+#define SID_ONE		0x00010000
+
+static struct schib schib;
+
+static const char *Channel_type[4] = {
+	"I/O", "CHSC", "MSG", "EADM"
+};
+
+static int test_device_sid;
+
+static void test_enumerate(void)
+{
+	struct pmcw *pmcw = &schib.pmcw;
+	int scn;
+	int cc, i;
+	int found = 0;
+
+	for (scn = 0; scn < 0xffff; scn++) {
+		cc = stsch(scn|SID_ONE, &schib);
+		if (!cc && (pmcw->flags & PMCW_DNV)) {
+			report_info("SID %04x Type %s PIM %x", scn,
+				     Channel_type[PMCW_CHANNEL_TYPE(pmcw)],
+				     pmcw->pim);
+			for (i = 0; i < 8; i++)  {
+				if ((pmcw->pim << i) & 0x80) {
+					report_info("CHPID[%d]: %02x", i,
+						    pmcw->chpid[i]);
+					break;
+				}
+			}
+			found++;
+		}
+		if (cc == 3) /* cc = 3 means no more channel in CSS */
+			break;
+		if (found && !test_device_sid)
+			test_device_sid = scn|SID_ONE;
+	}
+	if (!found) {
+		report("Tested %d devices, none found", 0, scn);
+		return;
+	}
+	report("Tested %d devices, %d found", 1, scn, 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 Sub-System");
+	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 f1b07cd..1755d9e 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -75,3 +75,7 @@ file = stsi.elf
 [smp]
 file = smp.elf
 extra_params =-smp 2
+
+[css]
+file = css.elf
+extra_params =-device ccw-pong
-- 
2.17.0

Re: [kvm-unit-tests PATCH v3 6/9] s390x: css: stsch, enumeration test

[Thomas Huth]

On 06/12/2019 17.26, Pierre Morel wrote:
> 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.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/css.h     |  1 +
>  s390x/Makefile      |  2 ++
>  s390x/css.c         | 82 +++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg |  4 +++
>  4 files changed, 89 insertions(+)
>  create mode 100644 s390x/css.c
> 
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> index 6f19bb5..d37227b 100644
> --- a/lib/s390x/css.h
> +++ b/lib/s390x/css.h
> @@ -82,6 +82,7 @@ struct pmcw {
>  	uint8_t  chpid[8];
>  	uint16_t flags2;
>  };
> +#define PMCW_CHANNEL_TYPE(pmcw) (pmcw->flags >> 21)
>  
>  struct schib {
>  	struct pmcw pmcw;
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 3744372..9ebbb84 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -16,6 +16,7 @@ tests += $(TEST_DIR)/diag288.elf
>  tests += $(TEST_DIR)/stsi.elf
>  tests += $(TEST_DIR)/skrf.elf
>  tests += $(TEST_DIR)/smp.elf
> +tests += $(TEST_DIR)/css.elf
>  tests_binary = $(patsubst %.elf,%.bin,$(tests))
>  
>  all: directories test_cases test_cases_binary
> @@ -50,6 +51,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..3d4a986
> --- /dev/null
> +++ b/s390x/css.c
> @@ -0,0 +1,82 @@
> +/*
> + * 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 <css.h>
> +
> +#define SID_ONE		0x00010000
> +
> +static struct schib schib;
> +
> +static const char *Channel_type[4] = {
> +	"I/O", "CHSC", "MSG", "EADM"
> +};
> +
> +static int test_device_sid;
> +
> +static void test_enumerate(void)
> +{
> +	struct pmcw *pmcw = &schib.pmcw;
> +	int scn;
> +	int cc, i;
> +	int found = 0;
> +
> +	for (scn = 0; scn < 0xffff; scn++) {
> +		cc = stsch(scn|SID_ONE, &schib);
> +		if (!cc && (pmcw->flags & PMCW_DNV)) {
> +			report_info("SID %04x Type %s PIM %x", scn,
> +				     Channel_type[PMCW_CHANNEL_TYPE(pmcw)],
> +				     pmcw->pim);
> +			for (i = 0; i < 8; i++)  {
> +				if ((pmcw->pim << i) & 0x80) {
> +					report_info("CHPID[%d]: %02x", i,
> +						    pmcw->chpid[i]);
> +					break;
> +				}
> +			}
> +			found++;
> +		}
> +		if (cc == 3) /* cc = 3 means no more channel in CSS */
> +			break;
> +		if (found && !test_device_sid)
> +			test_device_sid = scn|SID_ONE;
> +	}
> +	if (!found) {
> +		report("Tested %d devices, none found", 0, scn);
> +		return;
> +	}
> +	report("Tested %d devices, %d found", 1, scn, found);

I'm sorry, but since last Friday, you now have to swap the first two
parameters of the report() function. (that was unfortunately necessary
to fix an issue with Clang)

 Thomas

Re: [kvm-unit-tests PATCH v3 6/9] s390x: css: stsch, enumeration test

[Pierre Morel]

On 2019-12-09 12:52, Thomas Huth wrote:
> On 06/12/2019 17.26, Pierre Morel wrote:
>> 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.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h     |  1 +
>>   s390x/Makefile      |  2 ++
>>   s390x/css.c         | 82 +++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg |  4 +++
>>   4 files changed, 89 insertions(+)
>>   create mode 100644 s390x/css.c
>>
>> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
>> index 6f19bb5..d37227b 100644
>> --- a/lib/s390x/css.h
>> +++ b/lib/s390x/css.h
>> @@ -82,6 +82,7 @@ struct pmcw {
>>   	uint8_t  chpid[8];
>>   	uint16_t flags2;
>>   };
>> +#define PMCW_CHANNEL_TYPE(pmcw) (pmcw->flags >> 21)
>>   
>>   struct schib {
>>   	struct pmcw pmcw;
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 3744372..9ebbb84 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -16,6 +16,7 @@ tests += $(TEST_DIR)/diag288.elf
>>   tests += $(TEST_DIR)/stsi.elf
>>   tests += $(TEST_DIR)/skrf.elf
>>   tests += $(TEST_DIR)/smp.elf
>> +tests += $(TEST_DIR)/css.elf
>>   tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>   
>>   all: directories test_cases test_cases_binary
>> @@ -50,6 +51,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..3d4a986
>> --- /dev/null
>> +++ b/s390x/css.c
>> @@ -0,0 +1,82 @@
>> +/*
>> + * 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 <css.h>
>> +
>> +#define SID_ONE		0x00010000
>> +
>> +static struct schib schib;
>> +
>> +static const char *Channel_type[4] = {
>> +	"I/O", "CHSC", "MSG", "EADM"
>> +};
>> +
>> +static int test_device_sid;
>> +
>> +static void test_enumerate(void)
>> +{
>> +	struct pmcw *pmcw = &schib.pmcw;
>> +	int scn;
>> +	int cc, i;
>> +	int found = 0;
>> +
>> +	for (scn = 0; scn < 0xffff; scn++) {
>> +		cc = stsch(scn|SID_ONE, &schib);
>> +		if (!cc && (pmcw->flags & PMCW_DNV)) {
>> +			report_info("SID %04x Type %s PIM %x", scn,
>> +				     Channel_type[PMCW_CHANNEL_TYPE(pmcw)],
>> +				     pmcw->pim);
>> +			for (i = 0; i < 8; i++)  {
>> +				if ((pmcw->pim << i) & 0x80) {
>> +					report_info("CHPID[%d]: %02x", i,
>> +						    pmcw->chpid[i]);
>> +					break;
>> +				}
>> +			}
>> +			found++;
>> +		}
>> +		if (cc == 3) /* cc = 3 means no more channel in CSS */
>> +			break;
>> +		if (found && !test_device_sid)
>> +			test_device_sid = scn|SID_ONE;
>> +	}
>> +	if (!found) {
>> +		report("Tested %d devices, none found", 0, scn);
>> +		return;
>> +	}
>> +	report("Tested %d devices, %d found", 1, scn, found);
> 
> I'm sorry, but since last Friday, you now have to swap the first two
> parameters of the report() function. (that was unfortunately necessary
> to fix an issue with Clang)
> 
>   Thomas
> 

grrr.   :)

OK, will do.

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 6/9] s390x: css: stsch, enumeration test

[Cornelia Huck]

On Fri,  6 Dec 2019 17:26:25 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

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

> +static void test_enumerate(void)
> +{
> +	struct pmcw *pmcw = &schib.pmcw;
> +	int scn;
> +	int cc, i;
> +	int found = 0;
> +
> +	for (scn = 0; scn < 0xffff; scn++) {
> +		cc = stsch(scn|SID_ONE, &schib);
> +		if (!cc && (pmcw->flags & PMCW_DNV)) {

Not sure when dnv is actually applicable... it is used for I/O
subchannels; chsc subchannels don't have a device; message subchannels
use a different bit IIRC; not sure about EADM subchannels.

[Not very relevant as long as we run under KVM, but should be
considered if you plan to run this test under z/VM or LPAR as well.]

> +			report_info("SID %04x Type %s PIM %x", scn,
> +				     Channel_type[PMCW_CHANNEL_TYPE(pmcw)],
> +				     pmcw->pim);
> +			for (i = 0; i < 8; i++)  {
> +				if ((pmcw->pim << i) & 0x80) {
> +					report_info("CHPID[%d]: %02x", i,
> +						    pmcw->chpid[i]);
> +					break;

That 'break;' seems odd -- won't you end up printing the first chpid in
the pim only?

Maybe modify this loop to print the chpid if the path is in the pim,
and 'n/a' or so if not?

> +				}
> +			}
> +			found++;
> +		}
> +		if (cc == 3) /* cc = 3 means no more channel in CSS */

s/channel/subchannels/

> +			break;
> +		if (found && !test_device_sid)
> +			test_device_sid = scn|SID_ONE;
> +	}
> +	if (!found) {
> +		report("Tested %d devices, none found", 0, scn);
> +		return;
> +	}
> +	report("Tested %d devices, %d found", 1, scn, found);
> +}

Re: [kvm-unit-tests PATCH v3 6/9] s390x: css: stsch, enumeration test

[Pierre Morel]

On 2019-12-09 17:49, Cornelia Huck wrote:
> On Fri,  6 Dec 2019 17:26:25 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> 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.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h     |  1 +
>>   s390x/Makefile      |  2 ++
>>   s390x/css.c         | 82 +++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg |  4 +++
>>   4 files changed, 89 insertions(+)
>>   create mode 100644 s390x/css.c
>>
> 
>> +static void test_enumerate(void)
>> +{
>> +	struct pmcw *pmcw = &schib.pmcw;
>> +	int scn;
>> +	int cc, i;
>> +	int found = 0;
>> +
>> +	for (scn = 0; scn < 0xffff; scn++) {
>> +		cc = stsch(scn|SID_ONE, &schib);
>> +		if (!cc && (pmcw->flags & PMCW_DNV)) {
> 
> Not sure when dnv is actually applicable... it is used for I/O
> subchannels; chsc subchannels don't have a device; message subchannels
> use a different bit IIRC; not sure about EADM subchannels.
> 
> [Not very relevant as long as we run under KVM, but should be
> considered if you plan to run this test under z/VM or LPAR as well.]

Hum, interresting, I will check and modify accordingly.

> 
>> +			report_info("SID %04x Type %s PIM %x", scn,
>> +				     Channel_type[PMCW_CHANNEL_TYPE(pmcw)],
>> +				     pmcw->pim);
>> +			for (i = 0; i < 8; i++)  {
>> +				if ((pmcw->pim << i) & 0x80) {
>> +					report_info("CHPID[%d]: %02x", i,
>> +						    pmcw->chpid[i]);
>> +					break;
> 
> That 'break;' seems odd -- won't you end up printing the first chpid in
> the pim only?
yes
> 
> Maybe modify this loop to print the chpid if the path is in the pim,
> and 'n/a' or so if not?

OK

> 
>> +				}
>> +			}
>> +			found++;
>> +		}
>> +		if (cc == 3) /* cc = 3 means no more channel in CSS */
> 
> s/channel/subchannels/

thanks

> 
>> +			break;
>> +		if (found && !test_device_sid)
>> +			test_device_sid = scn|SID_ONE;
>> +	}
>> +	if (!found) {
>> +		report("Tested %d devices, none found", 0, scn);
>> +		return;
>> +	}
>> +	report("Tested %d devices, %d found", 1, scn, found);
>> +}
> 

Thanks for the reviewing,
Regards,

Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 6/9] s390x: css: stsch, enumeration test

[Pierre Morel]

On 2019-12-06 17:26, Pierre Morel wrote:
> 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.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   lib/s390x/css.h     |  1 +
>   s390x/Makefile      |  2 ++
>   s390x/css.c         | 82 +++++++++++++++++++++++++++++++++++++++++++++
>   s390x/unittests.cfg |  4 +++
>   4 files changed, 89 insertions(+)
>   create mode 100644 s390x/css.c
> 
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> index 6f19bb5..d37227b 100644
> --- a/lib/s390x/css.h
> +++ b/lib/s390x/css.h
> @@ -82,6 +82,7 @@ struct pmcw {
>   	uint8_t  chpid[8];
>   	uint16_t flags2;
>   };
> +#define PMCW_CHANNEL_TYPE(pmcw) (pmcw->flags >> 21)

This is wrong.
In between I redefined flag2 as 32bits and type as (flags2>>21)


>   
>   struct schib {
>   	struct pmcw pmcw;
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 3744372..9ebbb84 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -16,6 +16,7 @@ tests += $(TEST_DIR)/diag288.elf
>   tests += $(TEST_DIR)/stsi.elf
>   tests += $(TEST_DIR)/skrf.elf
>   tests += $(TEST_DIR)/smp.elf
> +tests += $(TEST_DIR)/css.elf
>   tests_binary = $(patsubst %.elf,%.bin,$(tests))
>   
>   all: directories test_cases test_cases_binary
> @@ -50,6 +51,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..3d4a986
> --- /dev/null
> +++ b/s390x/css.c
> @@ -0,0 +1,82 @@
> +/*
> + * 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 <css.h>
> +
> +#define SID_ONE		0x00010000
> +
> +static struct schib schib;
> +
> +static const char *Channel_type[4] = {
> +	"I/O", "CHSC", "MSG", "EADM"
> +};
> +
> +static int test_device_sid;
> +
> +static void test_enumerate(void)
> +{
> +	struct pmcw *pmcw = &schib.pmcw;
> +	int scn;
> +	int cc, i;
> +	int found = 0;
> +
> +	for (scn = 0; scn < 0xffff; scn++) {
> +		cc = stsch(scn|SID_ONE, &schib);
> +		if (!cc && (pmcw->flags & PMCW_DNV)) {
> +			report_info("SID %04x Type %s PIM %x", scn,
> +				     Channel_type[PMCW_CHANNEL_TYPE(pmcw)],
> +				     pmcw->pim);
> +			for (i = 0; i < 8; i++)  {
> +				if ((pmcw->pim << i) & 0x80) {
> +					report_info("CHPID[%d]: %02x", i,
> +						    pmcw->chpid[i]);
> +					break;
> +				}
> +			}
> +			found++;
> +		}
> +		if (cc == 3) /* cc = 3 means no more channel in CSS */
> +			break;
> +		if (found && !test_device_sid)
> +			test_device_sid = scn|SID_ONE;
> +	}
> +	if (!found) {
> +		report("Tested %d devices, none found", 0, scn);
> +		return;
> +	}
> +	report("Tested %d devices, %d found", 1, scn, 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 Sub-System");
> +	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 f1b07cd..1755d9e 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -75,3 +75,7 @@ file = stsi.elf
>   [smp]
>   file = smp.elf
>   extra_params =-smp 2
> +
> +[css]
> +file = css.elf
> +extra_params =-device ccw-pong
> 

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 7/9] 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 success of the MSCH instruction by enabling a channel.

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

diff --git a/s390x/css.c b/s390x/css.c
index 3d4a986..4c0031c 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -58,11 +58,50 @@ static void test_enumerate(void)
 	report("Tested %d devices, %d found", 1, scn, found);
 }
 
+static void test_enable(void)
+{
+	struct pmcw *pmcw = &schib.pmcw;
+	int cc;
+
+	if (!test_device_sid) {
+		report_skip("No device");
+		return;
+	}
+	/* Read the SCIB for this subchannel */
+	cc = stsch(test_device_sid, &schib);
+	if (cc) {
+		report("stsch cc=%d", 0, 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) {
+		report("msch cc=%d", 0, cc);
+		return;
+	}
+
+	/* Read the SCHIB again to verify the enablement */
+	cc = stsch(test_device_sid, &schib);
+	if (cc) {
+		report("stsch cc=%d", 0, cc);
+		return;
+	}
+	if (!(pmcw->flags & PMCW_ENABLE)) {
+		report("Enable failed. pmcw: %x", 0, pmcw->flags);
+		return;
+	}
+	report("Tested", 1);
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
 } tests[] = {
 	{ "enumerate (stsch)", test_enumerate },
+	{ "enable (msch)", test_enable },
 	{ NULL, NULL }
 };
 
-- 
2.17.0

Re: [kvm-unit-tests PATCH v3 7/9] s390x: css: msch, enable test

[Cornelia Huck]

On Fri,  6 Dec 2019 17:26:26 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> 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 success of the MSCH instruction by enabling a channel.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/s390x/css.c b/s390x/css.c
> index 3d4a986..4c0031c 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -58,11 +58,50 @@ static void test_enumerate(void)
>  	report("Tested %d devices, %d found", 1, scn, found);
>  }
>  
> +static void test_enable(void)
> +{
> +	struct pmcw *pmcw = &schib.pmcw;
> +	int cc;
> +
> +	if (!test_device_sid) {
> +		report_skip("No device");
> +		return;
> +	}
> +	/* Read the SCIB for this subchannel */

s/SCIB/SCHIB/

> +	cc = stsch(test_device_sid, &schib);
> +	if (cc) {
> +		report("stsch cc=%d", 0, 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) {
> +		report("msch cc=%d", 0, cc);

So you expect the subchannel to be idle? Probably true, especially as
QEMU has no reason to post an unsolicited interrupt for a test device.

> +		return;
> +	}
> +
> +	/* Read the SCHIB again to verify the enablement */
> +	cc = stsch(test_device_sid, &schib);
> +	if (cc) {
> +		report("stsch cc=%d", 0, cc);
> +		return;
> +	}
> +	if (!(pmcw->flags & PMCW_ENABLE)) {
> +		report("Enable failed. pmcw: %x", 0, pmcw->flags);

This check is fine when running under KVM. If this test is modified to
run under z/VM in the future, you probably should retry here: I've seen
the enable bit 'stick' only after the second msch() there.

> +		return;
> +	}
> +	report("Tested", 1);
> +}
> +
>  static struct {
>  	const char *name;
>  	void (*func)(void);
>  } tests[] = {
>  	{ "enumerate (stsch)", test_enumerate },
> +	{ "enable (msch)", test_enable },
>  	{ NULL, NULL }
>  };
>  

Re: [kvm-unit-tests PATCH v3 7/9] s390x: css: msch, enable test

[Pierre Morel]

On 2019-12-09 17:54, Cornelia Huck wrote:
> On Fri,  6 Dec 2019 17:26:26 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> 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 success of the MSCH instruction by enabling a channel.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/s390x/css.c b/s390x/css.c
>> index 3d4a986..4c0031c 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -58,11 +58,50 @@ static void test_enumerate(void)
>>   	report("Tested %d devices, %d found", 1, scn, found);
>>   }
>>   
>> +static void test_enable(void)
>> +{
>> +	struct pmcw *pmcw = &schib.pmcw;
>> +	int cc;
>> +
>> +	if (!test_device_sid) {
>> +		report_skip("No device");
>> +		return;
>> +	}
>> +	/* Read the SCIB for this subchannel */
> 
> s/SCIB/SCHIB/

yes

> 
>> +	cc = stsch(test_device_sid, &schib);
>> +	if (cc) {
>> +		report("stsch cc=%d", 0, 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) {
>> +		report("msch cc=%d", 0, cc);
> 
> So you expect the subchannel to be idle? Probably true, especially as
> QEMU has no reason to post an unsolicited interrupt for a test device.
> 
>> +		return;
>> +	}
>> +
>> +	/* Read the SCHIB again to verify the enablement */
>> +	cc = stsch(test_device_sid, &schib);
>> +	if (cc) {
>> +		report("stsch cc=%d", 0, cc);
>> +		return;
>> +	}
>> +	if (!(pmcw->flags & PMCW_ENABLE)) {
>> +		report("Enable failed. pmcw: %x", 0, pmcw->flags);
> 
> This check is fine when running under KVM. If this test is modified to
> run under z/VM in the future, you probably should retry here: I've seen
> the enable bit 'stick' only after the second msch() there.

Oh. Thanks, may be I can loop with a delay and count.
If I need to do this may be I need to create dedicated sub-functions to 
include the sanity tests.

> 
>> +		return;
>> +	}
>> +	report("Tested", 1);
>> +}
>> +
>>   static struct {
>>   	const char *name;
>>   	void (*func)(void);
>>   } tests[] = {
>>   	{ "enumerate (stsch)", test_enumerate },
>> +	{ "enable (msch)", test_enable },
>>   	{ NULL, NULL }
>>   };
>>   
> 

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 7/9] s390x: css: msch, enable test

[Cornelia Huck]

On Tue, 10 Dec 2019 10:01:46 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 2019-12-09 17:54, Cornelia Huck wrote:
> > On Fri,  6 Dec 2019 17:26:26 +0100
> > Pierre Morel <pmorel@linux.ibm.com> wrote:
> >   
> >> 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 success of the MSCH instruction by enabling a channel.
> >>
> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> >> ---
> >>   s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 39 insertions(+)
> >>
> >> diff --git a/s390x/css.c b/s390x/css.c
> >> index 3d4a986..4c0031c 100644
> >> --- a/s390x/css.c
> >> +++ b/s390x/css.c
> >> @@ -58,11 +58,50 @@ static void test_enumerate(void)
> >>   	report("Tested %d devices, %d found", 1, scn, found);
> >>   }
> >>   
> >> +static void test_enable(void)
> >> +{
> >> +	struct pmcw *pmcw = &schib.pmcw;
> >> +	int cc;
> >> +
> >> +	if (!test_device_sid) {
> >> +		report_skip("No device");
> >> +		return;
> >> +	}
> >> +	/* Read the SCIB for this subchannel */  
> > 
> > s/SCIB/SCHIB/  
> 
> yes
> 
> >   
> >> +	cc = stsch(test_device_sid, &schib);
> >> +	if (cc) {
> >> +		report("stsch cc=%d", 0, 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) {
> >> +		report("msch cc=%d", 0, cc);  
> > 
> > So you expect the subchannel to be idle? Probably true, especially as
> > QEMU has no reason to post an unsolicited interrupt for a test device.
> >   
> >> +		return;
> >> +	}
> >> +
> >> +	/* Read the SCHIB again to verify the enablement */
> >> +	cc = stsch(test_device_sid, &schib);
> >> +	if (cc) {
> >> +		report("stsch cc=%d", 0, cc);
> >> +		return;
> >> +	}
> >> +	if (!(pmcw->flags & PMCW_ENABLE)) {
> >> +		report("Enable failed. pmcw: %x", 0, pmcw->flags);  
> > 
> > This check is fine when running under KVM. If this test is modified to
> > run under z/VM in the future, you probably should retry here: I've seen
> > the enable bit 'stick' only after the second msch() there.  
> 
> Oh. Thanks, may be I can loop with a delay and count.

FWIW, the Linux kernel code is trying 5 times.

> If I need to do this may be I need to create dedicated sub-functions to 
> include the sanity tests.

I'm not sure how worthwhile investing time here is, actually: If you
don't plan to run under anything but KVM, you won't need it. I'm not
sure if current versions of z/VM still display the same behaviour (it
has been some time...); on the other hand, it is compliant with the
architecture...

> 
> >   
> >> +		return;
> >> +	}
> >> +	report("Tested", 1);
> >> +}
> >> +
> >>   static struct {
> >>   	const char *name;
> >>   	void (*func)(void);
> >>   } tests[] = {
> >>   	{ "enumerate (stsch)", test_enumerate },
> >> +	{ "enable (msch)", test_enable },
> >>   	{ NULL, NULL }
> >>   };
> >>     
> >   
> 

Re: [kvm-unit-tests PATCH v3 7/9] s390x: css: msch, enable test

[Pierre Morel]

On 2019-12-10 10:09, Cornelia Huck wrote:
> On Tue, 10 Dec 2019 10:01:46 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> On 2019-12-09 17:54, Cornelia Huck wrote:
>>> On Fri,  6 Dec 2019 17:26:26 +0100
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>    
>>>> 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 success of the MSCH instruction by enabling a channel.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>    s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 39 insertions(+)
>>>>
>>>> diff --git a/s390x/css.c b/s390x/css.c
>>>> index 3d4a986..4c0031c 100644
>>>> --- a/s390x/css.c
>>>> +++ b/s390x/css.c
>>>> @@ -58,11 +58,50 @@ static void test_enumerate(void)
>>>>    	report("Tested %d devices, %d found", 1, scn, found);
>>>>    }
>>>>    
>>>> +static void test_enable(void)
>>>> +{
>>>> +	struct pmcw *pmcw = &schib.pmcw;
>>>> +	int cc;
>>>> +
>>>> +	if (!test_device_sid) {
>>>> +		report_skip("No device");
>>>> +		return;
>>>> +	}
>>>> +	/* Read the SCIB for this subchannel */
>>>
>>> s/SCIB/SCHIB/
>>
>> yes
>>
>>>    
>>>> +	cc = stsch(test_device_sid, &schib);
>>>> +	if (cc) {
>>>> +		report("stsch cc=%d", 0, 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) {
>>>> +		report("msch cc=%d", 0, cc);
>>>
>>> So you expect the subchannel to be idle? Probably true, especially as
>>> QEMU has no reason to post an unsolicited interrupt for a test device.
>>>    
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	/* Read the SCHIB again to verify the enablement */
>>>> +	cc = stsch(test_device_sid, &schib);
>>>> +	if (cc) {
>>>> +		report("stsch cc=%d", 0, cc);
>>>> +		return;
>>>> +	}
>>>> +	if (!(pmcw->flags & PMCW_ENABLE)) {
>>>> +		report("Enable failed. pmcw: %x", 0, pmcw->flags);
>>>
>>> This check is fine when running under KVM. If this test is modified to
>>> run under z/VM in the future, you probably should retry here: I've seen
>>> the enable bit 'stick' only after the second msch() there.
>>
>> Oh. Thanks, may be I can loop with a delay and count.
> 
> FWIW, the Linux kernel code is trying 5 times.
> 
>> If I need to do this may be I need to create dedicated sub-functions to
>> include the sanity tests.
> 
> I'm not sure how worthwhile investing time here is, actually: If you
> don't plan to run under anything but KVM, you won't need it. I'm not
> sure if current versions of z/VM still display the same behaviour (it
> has been some time...); on the other hand, it is compliant with the
> architecture...

OK, I just insert the retry.

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 8/9] s390x: css: ssch/tsch with sense and interrupt

[Pierre Morel]

When a channel is enabled we can start a SENSE 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/css.h |  13 ++++
 s390x/css.c     | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 177 insertions(+)

diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index d37227b..2ac8ad7 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(8)));
+
 /* CSS low level access functions */
 
 static inline int ssch(unsigned long schid, struct orb *addr)
diff --git a/s390x/css.c b/s390x/css.c
index 4c0031c..54a7b38 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -11,12 +11,29 @@
  */
 
 #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
+#define PSW_PRG_MASK (PSW_MASK_IO | PSW_MASK_EA | PSW_MASK_BA)
+
+struct lowcore *lowcore = (void *)0x0;
 
 static struct schib schib;
+#define NUM_CCW  100
+static struct ccw1 ccw[NUM_CCW];
+#define NUM_ORB  100
+static struct orb orb[NUM_ORB];
+static struct irb irb;
+#define BUF_SZ  0x1000
+static char buffer[BUF_SZ] __attribute__ ((aligned(8)));
+static struct senseid senseid;
 
 static const char *Channel_type[4] = {
 	"I/O", "CHSC", "MSG", "EADM"
@@ -24,6 +41,34 @@ static const char *Channel_type[4] = {
 
 static int test_device_sid;
 
+static void delay(unsigned long ms)
+{
+	unsigned long startclk;
+
+	startclk = get_clock_ms();
+	for (;;) {
+		if (get_clock_ms() - startclk > ms)
+			break;
+	}
+}
+
+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)
 {
 	struct pmcw *pmcw = &schib.pmcw;
@@ -96,12 +141,131 @@ static void test_enable(void)
 	report("Tested", 1);
 }
 
+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;
+
+	report_prefix_push("Interrupt");
+	if (lowcore->io_int_param != 0xcafec0ca) {
+		report("Bad io_int_param: %x", 0, lowcore->io_int_param);
+		report_prefix_pop();
+		return;
+	}
+	report("io_int_param: %x", 1, lowcore->io_int_param);
+	report_prefix_pop();
+
+	ret = tsch(lowcore->subsys_id_word, &irb);
+	dump_irb(&irb);
+	flags = dump_scsw_flags(irb.scsw.ctrl);
+
+	if (ret)
+		report("IRB scsw flags: %s", 0, flags);
+	else
+		report("IRB scsw flags: %s", 1, flags);
+	report_prefix_pop();
+}
+
+static int start_subchannel(int code, char *data, int count)
+{
+	int ret;
+	struct pmcw *p = &schib.pmcw;
+	struct orb *orb_p = &orb[0];
+
+	/* Verify that a test subchannel has been set */
+	if (!test_device_sid) {
+		report_skip("No device");
+		return 0;
+	}
+
+	/* Verify that the subchannel has been enabled */
+	ret = stsch(test_device_sid, &schib);
+	if (ret) {
+		report("Err %d on stsch on sid %08x", 0, ret, test_device_sid);
+		return 0;
+	}
+	if (!(p->flags & PMCW_ENABLE)) {
+		report_skip("Device (sid %08x) not enabled", test_device_sid);
+		return 0;
+	}
+
+	report_prefix_push("Start Subchannel");
+	/* 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;
+	orb_p->intparm = 0xcafec0ca;
+	orb_p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
+	if ((unsigned long)&ccw[0] >= 0x80000000UL) {
+		report("Data above 2G! %016lx", 0, (unsigned long)&ccw[0]);
+		report_prefix_pop();
+		return 0;
+	}
+	orb_p->cpa = (unsigned int) (unsigned long)&ccw[0];
+
+	ret = ssch(test_device_sid, orb_p);
+	if (ret) {
+		report("ssch cc=%d", 0, 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;
+
+	ret = register_io_int_func(irq_io);
+	if (ret) {
+		report("Could not register IRQ handler", 0);
+		goto unreg_cb;
+	}
+
+	enable_io_irq();
+
+	ret = start_subchannel(CCW_CMD_SENSE_ID, buffer, sizeof(senseid));
+	if (!ret) {
+		report("start_subchannel failed", 0);
+		goto unreg_cb;
+	}
+
+	senseid.cu_type = buffer[2] | (buffer[1] << 8);
+	delay(100);
+
+	/* Sense ID is non packed cut_type is at offset +1 byte */
+	if (senseid.cu_type == PONG_CU)
+		report("cu_type: expect c0ca, got %04x", 1, senseid.cu_type);
+	else
+		report("cu_type: expect c0ca, got %04x", 0, 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

Re: [kvm-unit-tests PATCH v3 8/9] s390x: css: ssch/tsch with sense and interrupt

[Cornelia Huck]

On Fri,  6 Dec 2019 17:26:27 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> When a channel is enabled we can start a SENSE 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/css.h |  13 ++++
>  s390x/css.c     | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 177 insertions(+)
> 

> +static void irq_io(void)
> +{
> +	int ret = 0;
> +	char *flags;
> +
> +	report_prefix_push("Interrupt");
> +	if (lowcore->io_int_param != 0xcafec0ca) {
> +		report("Bad io_int_param: %x", 0, lowcore->io_int_param);

Use a #define for the intparm and print got vs. expected on mismatch?

> +		report_prefix_pop();
> +		return;
> +	}
> +	report("io_int_param: %x", 1, lowcore->io_int_param);

Well, at that moment you already know what the intparm is, don't you? :)

> +	report_prefix_pop();
> +
> +	ret = tsch(lowcore->subsys_id_word, &irb);
> +	dump_irb(&irb);
> +	flags = dump_scsw_flags(irb.scsw.ctrl);
> +
> +	if (ret)
> +		report("IRB scsw flags: %s", 0, flags);

I think you should also distinguish between cc 1 (not status pending)
and cc 3 (not operational) here (or at least also print that info).

> +	else
> +		report("IRB scsw flags: %s", 1, flags);
> +	report_prefix_pop();
> +}
> +
> +static int start_subchannel(int code, char *data, int count)
> +{
> +	int ret;
> +	struct pmcw *p = &schib.pmcw;
> +	struct orb *orb_p = &orb[0];
> +
> +	/* Verify that a test subchannel has been set */
> +	if (!test_device_sid) {
> +		report_skip("No device");
> +		return 0;
> +	}
> +
> +	/* Verify that the subchannel has been enabled */
> +	ret = stsch(test_device_sid, &schib);
> +	if (ret) {
> +		report("Err %d on stsch on sid %08x", 0, ret, test_device_sid);
> +		return 0;
> +	}
> +	if (!(p->flags & PMCW_ENABLE)) {
> +		report_skip("Device (sid %08x) not enabled", test_device_sid);
> +		return 0;
> +	}
> +
> +	report_prefix_push("Start Subchannel");
> +	/* 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;
> +	orb_p->intparm = 0xcafec0ca;
> +	orb_p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
> +	if ((unsigned long)&ccw[0] >= 0x80000000UL) {
> +		report("Data above 2G! %016lx", 0, (unsigned long)&ccw[0]);

Check for data under 2G before you set up data_address as well?

> +		report_prefix_pop();
> +		return 0;
> +	}
> +	orb_p->cpa = (unsigned int) (unsigned long)&ccw[0];
> +
> +	ret = ssch(test_device_sid, orb_p);
> +	if (ret) {
> +		report("ssch cc=%d", 0, 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;
> +
> +	ret = register_io_int_func(irq_io);
> +	if (ret) {
> +		report("Could not register IRQ handler", 0);
> +		goto unreg_cb;
> +	}
> +
> +	enable_io_irq();
> +
> +	ret = start_subchannel(CCW_CMD_SENSE_ID, buffer, sizeof(senseid));
> +	if (!ret) {
> +		report("start_subchannel failed", 0);
> +		goto unreg_cb;
> +	}
> +
> +	senseid.cu_type = buffer[2] | (buffer[1] << 8);
> +	delay(100);

Hm... registering an interrupt handler and then doing a random delay
seems a bit odd. I'd rather expect something like

(a) check for an indication that an interrupt has arrived (global
    variable)
(b) wait for a bit
(c) if timeout has not yet been hit: goto (a)

Or do a tpi loop, if this can't be done fully asynchronous?

Also, I don't understand what you are doing with the buffer and
senseid: Can't you make senseid a pointer to buffer, so that it can
simply access the fields after they have been filled by sense id?

Lastly, it might make sense if the reserved field of senseid has been
filled with 0xff; that way you can easily distinguish 'device is not a
pong device' from 'senseid has not been filled out correctly'.

> +
> +	/* Sense ID is non packed cut_type is at offset +1 byte */
> +	if (senseid.cu_type == PONG_CU)
> +		report("cu_type: expect c0ca, got %04x", 1, senseid.cu_type);
> +	else
> +		report("cu_type: expect c0ca, got %04x", 0, 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 }
>  };
>  

Re: [kvm-unit-tests PATCH v3 8/9] s390x: css: ssch/tsch with sense and interrupt

[Pierre Morel]

On 2019-12-09 18:22, Cornelia Huck wrote:
> On Fri,  6 Dec 2019 17:26:27 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> When a channel is enabled we can start a SENSE 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/css.h |  13 ++++
>>   s390x/css.c     | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 177 insertions(+)
>>
> 
>> +static void irq_io(void)
>> +{
>> +	int ret = 0;
>> +	char *flags;
>> +
>> +	report_prefix_push("Interrupt");
>> +	if (lowcore->io_int_param != 0xcafec0ca) {
>> +		report("Bad io_int_param: %x", 0, lowcore->io_int_param);
> 
> Use a #define for the intparm and print got vs. expected on mismatch?

OK

> 
>> +		report_prefix_pop();
>> +		return;
>> +	}
>> +	report("io_int_param: %x", 1, lowcore->io_int_param);
> 
> Well, at that moment you already know what the intparm is, don't you? :)

Yes, I can remove this, was for debug purpose.

> 
>> +	report_prefix_pop();
>> +
>> +	ret = tsch(lowcore->subsys_id_word, &irb);
>> +	dump_irb(&irb);
>> +	flags = dump_scsw_flags(irb.scsw.ctrl);
>> +
>> +	if (ret)
>> +		report("IRB scsw flags: %s", 0, flags);
> 
> I think you should also distinguish between cc 1 (not status pending)
> and cc 3 (not operational) here (or at least also print that info).

Yes, right, thanks.

> 
>> +	else
>> +		report("IRB scsw flags: %s", 1, flags);
>> +	report_prefix_pop();
>> +}
>> +
>> +static int start_subchannel(int code, char *data, int count)
>> +{
>> +	int ret;
>> +	struct pmcw *p = &schib.pmcw;
>> +	struct orb *orb_p = &orb[0];
>> +
>> +	/* Verify that a test subchannel has been set */
>> +	if (!test_device_sid) {
>> +		report_skip("No device");
>> +		return 0;
>> +	}
>> +
>> +	/* Verify that the subchannel has been enabled */
>> +	ret = stsch(test_device_sid, &schib);
>> +	if (ret) {
>> +		report("Err %d on stsch on sid %08x", 0, ret, test_device_sid);
>> +		return 0;
>> +	}
>> +	if (!(p->flags & PMCW_ENABLE)) {
>> +		report_skip("Device (sid %08x) not enabled", test_device_sid);
>> +		return 0;
>> +	}
>> +
>> +	report_prefix_push("Start Subchannel");
>> +	/* 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;
>> +	orb_p->intparm = 0xcafec0ca;
>> +	orb_p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
>> +	if ((unsigned long)&ccw[0] >= 0x80000000UL) {
>> +		report("Data above 2G! %016lx", 0, (unsigned long)&ccw[0]);
> 
> Check for data under 2G before you set up data_address as well?

yes. Even more important since it is a parameter.

> 
>> +		report_prefix_pop();
>> +		return 0;
>> +	}
>> +	orb_p->cpa = (unsigned int) (unsigned long)&ccw[0];
>> +
>> +	ret = ssch(test_device_sid, orb_p);
>> +	if (ret) {
>> +		report("ssch cc=%d", 0, 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;
>> +
>> +	ret = register_io_int_func(irq_io);
>> +	if (ret) {
>> +		report("Could not register IRQ handler", 0);
>> +		goto unreg_cb;
>> +	}
>> +
>> +	enable_io_irq();
>> +
>> +	ret = start_subchannel(CCW_CMD_SENSE_ID, buffer, sizeof(senseid));
>> +	if (!ret) {
>> +		report("start_subchannel failed", 0);
>> +		goto unreg_cb;
>> +	}
>> +
>> +	senseid.cu_type = buffer[2] | (buffer[1] << 8);
>> +	delay(100);
> 
> Hm... registering an interrupt handler and then doing a random delay
> seems a bit odd. I'd rather expect something like
> 
> (a) check for an indication that an interrupt has arrived (global
>      variable)
> (b) wait for a bit
> (c) if timeout has not yet been hit: goto (a)
> 
> Or do a tpi loop, if this can't be done fully asynchronous?

Currently the test is done on the io_int_parameter.
And you are right there is a problem, if no interrupt happen the test is 
silently skipped

> 
> Also, I don't understand what you are doing with the buffer and
> senseid: Can't you make senseid a pointer to buffer, so that it can
> simply access the fields after they have been filled by sense id?
> 
> Lastly, it might make sense if the reserved field of senseid has been
> filled with 0xff; that way you can easily distinguish 'device is not a
> pong device' from 'senseid has not been filled out correctly'.

yes, thanks.


Thanks for comemnts,
Regards

Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 9/9] 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 | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/s390x/css.c b/s390x/css.c
index 54a7b38..3b5ce9d 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -23,6 +23,10 @@
 #define SID_ONE		0x00010000
 #define PSW_PRG_MASK (PSW_MASK_IO | PSW_MASK_EA | PSW_MASK_BA)
 
+/* Channel Commands for PONG device */
+#define PONG_WRITE	0x21 /* Write */
+#define PONG_READ	0x22 /* Read buffer */
+
 struct lowcore *lowcore = (void *)0x0;
 
 static struct schib schib;
@@ -259,6 +263,51 @@ 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("Could not register IRQ handler", 0);
+		return;
+	}
+
+	while (cnt++ < max) {
+		snprintf(buffer, BUF_SZ, "%08x\n", cnt);
+		success = start_subchannel(PONG_WRITE, buffer, 8);
+		if (!success) {
+			report("start_subchannel failed", 0);
+			goto unreg_cb;
+		}
+		delay(100);
+		success = start_subchannel(PONG_READ, buffer, 8);
+		if (!success) {
+			report("start_subchannel failed", 0);
+			goto unreg_cb;
+		}
+		result = atol(buffer);
+		if (result != (cnt + 1)) {
+			report("Bad answer from pong: %08x - %08x",
+				0, cnt, result);
+			goto unreg_cb;
+		} else
+			report_info("%08x - %08x", cnt, result);
+
+		delay(100);
+	}
+	report("ping-pong count 0x%08x", 1, cnt);
+
+unreg_cb:
+	unregister_io_int_func(irq_io);
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
@@ -266,6 +315,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