* [kvm-unit-tests PATCH v7 01/12] s390x: saving regs for interrupts
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-26 10:30 ` Janosch Frank
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 02/12] s390x: Use PSW bits definitions in cstart Pierre Morel
` (10 subsequent siblings)
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
If we use multiple source of interrupts, for example, using SCLP
console to print information while using I/O interrupts, we need
to have a re-entrant register saving interruption handling.
Instead of saving at a static memory address, let's save the base
registers, the floating point registers and the floating point
control register on the stack in case of I/O interrupts
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 | 41 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 9af6bb3..3c7d8a9 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -118,6 +118,43 @@ memsetxc:
lmg %r0, %r15, GEN_LC_SW_INT_GRS
.endm
+/* Save registers on the stack (r15), so we can have stacked interrupts. */
+ .macro SAVE_REGS_STACK
+ /* Allocate a stack frame for 15 general registers */
+ slgfi %r15, 15 * 8
+ /* Store all registers from r0 to r14 on the stack */
+ stmg %r0, %r14, 0(%r15)
+ /* Allocate a stack frame for 16 floating point registers */
+ /* The size of a FP register is the size of an double word */
+ slgfi %r15, 16 * 8
+ /* Save fp register on stack: offset to SP is multiple of reg number */
+ .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+ std \i, \i * 8(%r15)
+ .endr
+ /* Save fpc, but keep stack aligned on 64bits */
+ slgfi %r15, 8
+ efpc %r0
+ stg %r0, 0(%r15)
+ .endm
+
+/* Restore the register in reverse order */
+ .macro RESTORE_REGS_STACK
+ /* Restore fpc */
+ lfpc 0(%r15)
+ algfi %r15, 8
+ /* Restore fp register from stack: SP still where it was left */
+ /* and offset to SP is a multile of reg number */
+ .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+ ld \i, \i * 8(%r15)
+ .endr
+ /* Now it is done, rewind the stack pointer by 16 double word */
+ algfi %r15, 16 * 8
+ /* Load the registers from stack */
+ lmg %r0, %r14, 0(%r15)
+ /* Rewind the stack by 15 double word */
+ algfi %r15, 15 * 8
+ .endm
+
.section .text
/*
* load_reset calling convention:
@@ -182,9 +219,9 @@ mcck_int:
lpswe GEN_LC_MCCK_OLD_PSW
io_int:
- SAVE_REGS
+ SAVE_REGS_STACK
brasl %r14, handle_io_int
- RESTORE_REGS
+ RESTORE_REGS_STACK
lpswe GEN_LC_IO_OLD_PSW
svc_int:
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 01/12] s390x: saving regs for interrupts
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 01/12] s390x: saving regs for interrupts Pierre Morel
@ 2020-05-26 10:30 ` Janosch Frank
2020-05-26 11:39 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Janosch Frank @ 2020-05-26 10:30 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: linux-s390, david, thuth, cohuck
[-- Attachment #1.1: Type: text/plain, Size: 2812 bytes --]
On 5/18/20 6:07 PM, Pierre Morel wrote:
> If we use multiple source of interrupts, for example, using SCLP
> console to print information while using I/O interrupts, we need
> to have a re-entrant register saving interruption handling.
>
> Instead of saving at a static memory address, let's save the base
> registers, the floating point registers and the floating point
> control register on the stack in case of I/O interrupts
>
> Note that we keep the static register saving to recover from the
> RESET tests.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Some nits below
Acked-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/cstart64.S | 41 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 9af6bb3..3c7d8a9 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -118,6 +118,43 @@ memsetxc:
> lmg %r0, %r15, GEN_LC_SW_INT_GRS
> .endm
>
> +/* Save registers on the stack (r15), so we can have stacked interrupts. */
> + .macro SAVE_REGS_STACK
> + /* Allocate a stack frame for 15 general registers */
> + slgfi %r15, 15 * 8
> + /* Store all registers from r0 to r14 on the stack */
/* Store registers r0 to r14 on the stack */
> + stmg %r0, %r14, 0(%r15)
> + /* Allocate a stack frame for 16 floating point registers */
> + /* The size of a FP register is the size of an double word */
> + slgfi %r15, 16 * 8
> + /* Save fp register on stack: offset to SP is multiple of reg number */
> + .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
> + std \i, \i * 8(%r15)
> + .endr
> + /* Save fpc, but keep stack aligned on 64bits */
> + slgfi %r15, 8
> + efpc %r0
> + stg %r0, 0(%r15)
> + .endm
> +
> +/* Restore the register in reverse order */
> + .macro RESTORE_REGS_STACK
> + /* Restore fpc */
> + lfpc 0(%r15)
> + algfi %r15, 8
> + /* Restore fp register from stack: SP still where it was left */
> + /* and offset to SP is a multile of reg number */
multile?
Multiple?
> + .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
> + ld \i, \i * 8(%r15)
> + .endr
> + /* Now it is done, rewind the stack pointer by 16 double word */
Now that we're done, rewind the stack pointer by 16 double words
> + algfi %r15, 16 * 8
> + /* Load the registers from stack */
> + lmg %r0, %r14, 0(%r15)
> + /* Rewind the stack by 15 double word */
> + algfi %r15, 15 * 8
> + .endm
> +
> .section .text
> /*
> * load_reset calling convention:
> @@ -182,9 +219,9 @@ mcck_int:
> lpswe GEN_LC_MCCK_OLD_PSW
>
> io_int:
> - SAVE_REGS
> + SAVE_REGS_STACK
> brasl %r14, handle_io_int
> - RESTORE_REGS
> + RESTORE_REGS_STACK
> lpswe GEN_LC_IO_OLD_PSW
>
> svc_int:
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 01/12] s390x: saving regs for interrupts
2020-05-26 10:30 ` Janosch Frank
@ 2020-05-26 11:39 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-26 11:39 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, david, thuth, cohuck
On 2020-05-26 12:30, Janosch Frank wrote:
> On 5/18/20 6:07 PM, Pierre Morel wrote:
>> If we use multiple source of interrupts, for example, using SCLP
>> console to print information while using I/O interrupts, we need
>> to have a re-entrant register saving interruption handling.
>>
>> Instead of saving at a static memory address, let's save the base
>> registers, the floating point registers and the floating point
>> control register on the stack in case of I/O interrupts
>>
>> Note that we keep the static register saving to recover from the
>> RESET tests.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>
> Some nits below
>
> Acked-by: Janosch Frank <frankja@linux.ibm.com>
>
>
Thanks, I will modify the nits as you proposed.
regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 02/12] s390x: Use PSW bits definitions in cstart
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 01/12] s390x: saving regs for interrupts Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-26 10:17 ` Janosch Frank
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 03/12] s390x: Move control register bit definitions and add AFP to them Pierre Morel
` (9 subsequent siblings)
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
This patch defines the PSW bits EA/BA used to initialize the PSW masks
for exceptions.
Since some PSW mask definitions exist already in arch_def.h we add these
definitions there.
We move all PSW definitions together and protect assembler code against
C syntax.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
lib/s390x/asm/arch_def.h | 15 +++++++++++----
s390x/cstart64.S | 15 ++++++++-------
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 15a4d49..820af93 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -10,15 +10,21 @@
#ifndef _ASM_S390X_ARCH_DEF_H_
#define _ASM_S390X_ARCH_DEF_H_
+#define PSW_MASK_EXT 0x0100000000000000UL
+#define PSW_MASK_DAT 0x0400000000000000UL
+#define PSW_MASK_SHORT_PSW 0x0008000000000000UL
+#define PSW_MASK_PSTATE 0x0001000000000000UL
+#define PSW_MASK_BA 0x0000000080000000UL
+#define PSW_MASK_EA 0x0000000100000000UL
+
+#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA)
+
+#ifndef __ASSEMBLER__
struct psw {
uint64_t mask;
uint64_t addr;
};
-#define PSW_MASK_EXT 0x0100000000000000UL
-#define PSW_MASK_DAT 0x0400000000000000UL
-#define PSW_MASK_PSTATE 0x0001000000000000UL
-
#define CR0_EXTM_SCLP 0X0000000000000200UL
#define CR0_EXTM_EXTC 0X0000000000002000UL
#define CR0_EXTM_EMGC 0X0000000000004000UL
@@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void)
return current_prefix;
}
+#endif /* __ASSEMBLER */
#endif
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 3c7d8a9..e890568 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
@@ -232,19 +233,19 @@ svc_int:
.align 8
reset_psw:
- .quad 0x0008000180000000
+ .quad PSW_EXCEPTION_MASK | PSW_MASK_SHORT_PSW
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.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 02/12] s390x: Use PSW bits definitions in cstart
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 02/12] s390x: Use PSW bits definitions in cstart Pierre Morel
@ 2020-05-26 10:17 ` Janosch Frank
2020-05-26 11:40 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Janosch Frank @ 2020-05-26 10:17 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: linux-s390, david, thuth, cohuck
[-- Attachment #1.1: Type: text/plain, Size: 2928 bytes --]
On 5/18/20 6:07 PM, Pierre Morel wrote:
> This patch defines the PSW bits EA/BA used to initialize the PSW masks
> for exceptions.
>
> Since some PSW mask definitions exist already in arch_def.h we add these
> definitions there.
> We move all PSW definitions together and protect assembler code against
> C syntax.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Nice
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/asm/arch_def.h | 15 +++++++++++----
> s390x/cstart64.S | 15 ++++++++-------
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 15a4d49..820af93 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -10,15 +10,21 @@
> #ifndef _ASM_S390X_ARCH_DEF_H_
> #define _ASM_S390X_ARCH_DEF_H_
>
> +#define PSW_MASK_EXT 0x0100000000000000UL
> +#define PSW_MASK_DAT 0x0400000000000000UL
> +#define PSW_MASK_SHORT_PSW 0x0008000000000000UL
> +#define PSW_MASK_PSTATE 0x0001000000000000UL
> +#define PSW_MASK_BA 0x0000000080000000UL
> +#define PSW_MASK_EA 0x0000000100000000UL
> +
> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA)
> +
> +#ifndef __ASSEMBLER__
> struct psw {
> uint64_t mask;
> uint64_t addr;
> };
>
> -#define PSW_MASK_EXT 0x0100000000000000UL
> -#define PSW_MASK_DAT 0x0400000000000000UL
> -#define PSW_MASK_PSTATE 0x0001000000000000UL
> -
> #define CR0_EXTM_SCLP 0X0000000000000200UL
> #define CR0_EXTM_EXTC 0X0000000000002000UL
> #define CR0_EXTM_EMGC 0X0000000000004000UL
> @@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void)
> return current_prefix;
> }
>
> +#endif /* __ASSEMBLER */
> #endif
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 3c7d8a9..e890568 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
>
> @@ -232,19 +233,19 @@ svc_int:
>
> .align 8
> reset_psw:
> - .quad 0x0008000180000000
> + .quad PSW_EXCEPTION_MASK | PSW_MASK_SHORT_PSW
> initial_psw:
> - .quad 0x0000000180000000, clear_bss_start
> + .quad PSW_EXCEPTION_MASK, clear_bss_start
> pgm_int_psw:
> - .quad 0x0000000180000000, pgm_int
> + .quad PSW_EXCEPTION_MASK, pgm_int
> ext_int_psw:
> - .quad 0x0000000180000000, ext_int
> + .quad PSW_EXCEPTION_MASK, ext_int
> mcck_int_psw:
> - .quad 0x0000000180000000, mcck_int
> + .quad PSW_EXCEPTION_MASK, mcck_int
> io_int_psw:
> - .quad 0x0000000180000000, io_int
> + .quad PSW_EXCEPTION_MASK, io_int
> svc_int_psw:
> - .quad 0x0000000180000000, svc_int
> + .quad PSW_EXCEPTION_MASK, svc_int
> initial_cr0:
> /* enable AFP-register control, so FP regs (+BFP instr) can be used */
> .quad 0x0000000000040000
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 02/12] s390x: Use PSW bits definitions in cstart
2020-05-26 10:17 ` Janosch Frank
@ 2020-05-26 11:40 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-26 11:40 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, david, thuth, cohuck
On 2020-05-26 12:17, Janosch Frank wrote:
> On 5/18/20 6:07 PM, Pierre Morel wrote:
>> This patch defines the PSW bits EA/BA used to initialize the PSW masks
>> for exceptions.
>>
>> Since some PSW mask definitions exist already in arch_def.h we add these
>> definitions there.
>> We move all PSW definitions together and protect assembler code against
>> C syntax.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>
> Nice
>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 03/12] s390x: Move control register bit definitions and add AFP to them
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 01/12] s390x: saving regs for interrupts Pierre Morel
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 02/12] s390x: Use PSW bits definitions in cstart Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-25 18:57 ` Thomas Huth
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 04/12] s390x: interrupt registration Pierre Morel
` (8 subsequent siblings)
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
While adding the definition for the AFP-Register control bit, move all
existing definitions for CR0 out of the C zone to the assmbler zone to
keep the definitions concerning CR0 together.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
lib/s390x/asm/arch_def.h | 11 ++++++-----
s390x/cstart64.S | 2 +-
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 820af93..54ffd0b 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -19,17 +19,18 @@
#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA)
+#define CR0_EXTM_SCLP 0X0000000000000200UL
+#define CR0_EXTM_EXTC 0X0000000000002000UL
+#define CR0_EXTM_EMGC 0X0000000000004000UL
+#define CR0_EXTM_MASK 0X0000000000006200UL
+#define CR0_AFP_REG_CRTL 0x0000000000040000UL
+
#ifndef __ASSEMBLER__
struct psw {
uint64_t mask;
uint64_t addr;
};
-#define CR0_EXTM_SCLP 0X0000000000000200UL
-#define CR0_EXTM_EXTC 0X0000000000002000UL
-#define CR0_EXTM_EMGC 0X0000000000004000UL
-#define CR0_EXTM_MASK 0X0000000000006200UL
-
struct lowcore {
uint8_t pad_0x0000[0x0080 - 0x0000]; /* 0x0000 */
uint32_t ext_int_param; /* 0x0080 */
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index e890568..0679fce 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -248,4 +248,4 @@ svc_int_psw:
.quad PSW_EXCEPTION_MASK, svc_int
initial_cr0:
/* enable AFP-register control, so FP regs (+BFP instr) can be used */
- .quad 0x0000000000040000
+ .quad CR0_AFP_REG_CRTL
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 03/12] s390x: Move control register bit definitions and add AFP to them
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 03/12] s390x: Move control register bit definitions and add AFP to them Pierre Morel
@ 2020-05-25 18:57 ` Thomas Huth
2020-05-26 11:51 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Thomas Huth @ 2020-05-25 18:57 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: linux-s390, frankja, david, cohuck
On 18/05/2020 18.07, Pierre Morel wrote:
> While adding the definition for the AFP-Register control bit, move all
> existing definitions for CR0 out of the C zone to the assmbler zone to
> keep the definitions concerning CR0 together.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
> lib/s390x/asm/arch_def.h | 11 ++++++-----
> s390x/cstart64.S | 2 +-
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 820af93..54ffd0b 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -19,17 +19,18 @@
>
> #define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA)
>
> +#define CR0_EXTM_SCLP 0X0000000000000200UL
> +#define CR0_EXTM_EXTC 0X0000000000002000UL
> +#define CR0_EXTM_EMGC 0X0000000000004000UL
> +#define CR0_EXTM_MASK 0X0000000000006200UL
> +#define CR0_AFP_REG_CRTL 0x0000000000040000UL
> +
> #ifndef __ASSEMBLER__
> struct psw {
> uint64_t mask;
> uint64_t addr;
> };
>
> -#define CR0_EXTM_SCLP 0X0000000000000200UL
> -#define CR0_EXTM_EXTC 0X0000000000002000UL
> -#define CR0_EXTM_EMGC 0X0000000000004000UL
> -#define CR0_EXTM_MASK 0X0000000000006200UL
This patch does not apply anymore due to commit f7df29115f736b ...
please switch to lower-case "0x"s in the next version.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 03/12] s390x: Move control register bit definitions and add AFP to them
2020-05-25 18:57 ` Thomas Huth
@ 2020-05-26 11:51 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-26 11:51 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, frankja, david, cohuck
On 2020-05-25 20:57, Thomas Huth wrote:
> On 18/05/2020 18.07, Pierre Morel wrote:
>> While adding the definition for the AFP-Register control bit, move all
>> existing definitions for CR0 out of the C zone to the assmbler zone to
>> keep the definitions concerning CR0 together.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>> lib/s390x/asm/arch_def.h | 11 ++++++-----
>> s390x/cstart64.S | 2 +-
>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index 820af93..54ffd0b 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -19,17 +19,18 @@
>>
>> #define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA)
>>
>> +#define CR0_EXTM_SCLP 0X0000000000000200UL
>> +#define CR0_EXTM_EXTC 0X0000000000002000UL
>> +#define CR0_EXTM_EMGC 0X0000000000004000UL
>> +#define CR0_EXTM_MASK 0X0000000000006200UL
>> +#define CR0_AFP_REG_CRTL 0x0000000000040000UL
>> +
>> #ifndef __ASSEMBLER__
>> struct psw {
>> uint64_t mask;
>> uint64_t addr;
>> };
>>
>> -#define CR0_EXTM_SCLP 0X0000000000000200UL
>> -#define CR0_EXTM_EXTC 0X0000000000002000UL
>> -#define CR0_EXTM_EMGC 0X0000000000004000UL
>> -#define CR0_EXTM_MASK 0X0000000000006200UL
>
> This patch does not apply anymore due to commit f7df29115f736b ...
> please switch to lower-case "0x"s in the next version.
>
> Thanks,
> Thomas
>
OK, I will rebase.
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 04/12] s390x: interrupt registration
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (2 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 03/12] s390x: Move control register bit definitions and add AFP to them Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-26 18:08 ` Thomas Huth
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 05/12] s390x: export the clock get_clock_ms() utility Pierre Morel
` (7 subsequent siblings)
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
Let's make it possible to add and remove a custom io interrupt handler,
that can be used instead of the normal one.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
lib/s390x/interrupt.c | 23 ++++++++++++++++++++++-
lib/s390x/interrupt.h | 8 ++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
create mode 100644 lib/s390x/interrupt.h
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 3a40cac..243b9c2 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -10,9 +10,9 @@
* under the terms of the GNU Library General Public License version 2.
*/
#include <libcflat.h>
-#include <asm/interrupt.h>
#include <asm/barrier.h>
#include <sclp.h>
+#include <interrupt.h>
static bool pgm_int_expected;
static bool ext_int_expected;
@@ -144,12 +144,33 @@ void handle_mcck_int(void)
stap(), lc->mcck_old_psw.addr);
}
+static void (*io_int_func)(void);
+
void handle_io_int(void)
{
+ if (io_int_func)
+ return io_int_func();
+
report_abort("Unexpected io interrupt: on cpu %d at %#lx",
stap(), lc->io_old_psw.addr);
}
+int register_io_int_func(void (*f)(void))
+{
+ if (io_int_func)
+ return -1;
+ io_int_func = f;
+ return 0;
+}
+
+int unregister_io_int_func(void (*f)(void))
+{
+ if (io_int_func != f)
+ return -1;
+ io_int_func = NULL;
+ return 0;
+}
+
void handle_svc_int(void)
{
report_abort("Unexpected supervisor call interrupt: on cpu %d at %#lx",
diff --git a/lib/s390x/interrupt.h b/lib/s390x/interrupt.h
new file mode 100644
index 0000000..323258d
--- /dev/null
+++ b/lib/s390x/interrupt.h
@@ -0,0 +1,8 @@
+#ifndef __INTERRUPT_H
+#define __INTERRUPT_H
+#include <asm/interrupt.h>
+
+int register_io_int_func(void (*f)(void));
+int unregister_io_int_func(void (*f)(void));
+
+#endif /* __INTERRUPT_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 04/12] s390x: interrupt registration
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 04/12] s390x: interrupt registration Pierre Morel
@ 2020-05-26 18:08 ` Thomas Huth
2020-05-27 15:54 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Thomas Huth @ 2020-05-26 18:08 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: linux-s390, frankja, david, cohuck
On 18/05/2020 18.07, Pierre Morel wrote:
> Let's make it possible to add and remove a custom io interrupt handler,
> that can be used instead of the normal one.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/interrupt.c | 23 ++++++++++++++++++++++-
> lib/s390x/interrupt.h | 8 ++++++++
> 2 files changed, 30 insertions(+), 1 deletion(-)
> create mode 100644 lib/s390x/interrupt.h
...
> diff --git a/lib/s390x/interrupt.h b/lib/s390x/interrupt.h
> new file mode 100644
> index 0000000..323258d
> --- /dev/null
> +++ b/lib/s390x/interrupt.h
> @@ -0,0 +1,8 @@
> +#ifndef __INTERRUPT_H
> +#define __INTERRUPT_H
Looking at this patch again, I noticed another nit: No double
underscores at the beginning of header guards, please! That's reserved
namespace. Simply use INTERRUPT_H or S390X_INTERRUPT_H or something
similar instead.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 04/12] s390x: interrupt registration
2020-05-26 18:08 ` Thomas Huth
@ 2020-05-27 15:54 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-27 15:54 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, frankja, david, cohuck
On 2020-05-26 20:08, Thomas Huth wrote:
> On 18/05/2020 18.07, Pierre Morel wrote:
>> Let's make it possible to add and remove a custom io interrupt handler,
>> that can be used instead of the normal one.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> lib/s390x/interrupt.c | 23 ++++++++++++++++++++++-
>> lib/s390x/interrupt.h | 8 ++++++++
>> 2 files changed, 30 insertions(+), 1 deletion(-)
>> create mode 100644 lib/s390x/interrupt.h
> ...
>> diff --git a/lib/s390x/interrupt.h b/lib/s390x/interrupt.h
>> new file mode 100644
>> index 0000000..323258d
>> --- /dev/null
>> +++ b/lib/s390x/interrupt.h
>> @@ -0,0 +1,8 @@
>> +#ifndef __INTERRUPT_H
>> +#define __INTERRUPT_H
>
> Looking at this patch again, I noticed another nit: No double
> underscores at the beginning of header guards, please! That's reserved
> namespace. Simply use INTERRUPT_H or S390X_INTERRUPT_H or something
> similar instead.
>
> Thanks,
> Thomas
>
OK, thanks
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 05/12] s390x: export the clock get_clock_ms() utility
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (3 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 04/12] s390x: interrupt registration Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-26 18:10 ` Thomas Huth
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 06/12] s390x: use get_clock_ms() to calculate a delay in ms Pierre Morel
` (6 subsequent siblings)
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
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>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
lib/s390x/asm/time.h | 26 ++++++++++++++++++++++++++
s390x/intercept.c | 11 +----------
2 files changed, 27 insertions(+), 10 deletions(-)
create mode 100644 lib/s390x/asm/time.h
diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
new file mode 100644
index 0000000..25c7a3c
--- /dev/null
+++ b/lib/s390x/asm/time.h
@@ -0,0 +1,26 @@
+/*
+ * Clock utilities for s390
+ *
+ * Authors:
+ * Thomas Huth <thuth@redhat.com>
+ *
+ * Copied from the s390/intercept test by:
+ * Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2.
+ */
+#ifndef _ASM_S390X_TIME_H_
+#define _ASM_S390X_TIME_H_
+
+static inline uint64_t get_clock_ms(void)
+{
+ uint64_t clk;
+
+ asm volatile(" stck %0 " : : "Q"(clk) : "memory");
+
+ /* Bit 51 is incrememented each microsecond */
+ return (clk >> (63 - 51)) / 1000;
+}
+
+#endif
diff --git a/s390x/intercept.c b/s390x/intercept.c
index 5f46b82..2e38257 100644
--- a/s390x/intercept.c
+++ b/s390x/intercept.c
@@ -14,6 +14,7 @@
#include <asm/interrupt.h>
#include <asm/page.h>
#include <asm/facility.h>
+#include <asm/time.h>
static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
@@ -153,16 +154,6 @@ static void test_testblock(void)
check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
}
-static uint64_t get_clock_ms(void)
-{
- uint64_t clk;
-
- asm volatile(" stck %0 " : : "Q"(clk) : "memory");
-
- /* Bit 51 is incrememented each microsecond */
- return (clk >> (63 - 51)) / 1000;
-}
-
struct {
const char *name;
void (*func)(void);
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 05/12] s390x: export the clock get_clock_ms() utility
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 05/12] s390x: export the clock get_clock_ms() utility Pierre Morel
@ 2020-05-26 18:10 ` Thomas Huth
2020-06-04 6:49 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Thomas Huth @ 2020-05-26 18:10 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: linux-s390, frankja, david, cohuck
On 18/05/2020 18.07, 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>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
> lib/s390x/asm/time.h | 26 ++++++++++++++++++++++++++
> s390x/intercept.c | 11 +----------
> 2 files changed, 27 insertions(+), 10 deletions(-)
> create mode 100644 lib/s390x/asm/time.h
>
> diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
> new file mode 100644
> index 0000000..25c7a3c
> --- /dev/null
> +++ b/lib/s390x/asm/time.h
> @@ -0,0 +1,26 @@
> +/*
> + * Clock utilities for s390
> + *
> + * Authors:
> + * Thomas Huth <thuth@redhat.com>
> + *
> + * Copied from the s390/intercept test by:
> + * Pierre Morel <pmorel@linux.ibm.com>
> + *
> + * This code is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2.
> + */
> +#ifndef _ASM_S390X_TIME_H_
> +#define _ASM_S390X_TIME_H_
Please also remove the underscores at the beginning (and preferably also
at the end) here.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 05/12] s390x: export the clock get_clock_ms() utility
2020-05-26 18:10 ` Thomas Huth
@ 2020-06-04 6:49 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-06-04 6:49 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, frankja, david, cohuck
On 2020-05-26 20:10, Thomas Huth wrote:
> On 18/05/2020 18.07, 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.
...snip...
>> index 0000000..25c7a3c
>> --- /dev/null
>> +++ b/lib/s390x/asm/time.h
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Clock utilities for s390
>> + *
>> + * Authors:
>> + * Thomas Huth <thuth@redhat.com>
>> + *
>> + * Copied from the s390/intercept test by:
>> + * Pierre Morel <pmorel@linux.ibm.com>
>> + *
>> + * This code is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2.
>> + */
>> +#ifndef _ASM_S390X_TIME_H_
>> +#define _ASM_S390X_TIME_H_
>
> Please also remove the underscores at the beginning (and preferably also
> at the end) here.
OK,
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 06/12] s390x: use get_clock_ms() to calculate a delay in ms
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (4 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 05/12] s390x: export the clock get_clock_ms() utility Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-26 18:16 ` Thomas Huth
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 07/12] s390x: Library resources for CSS tests Pierre Morel
` (5 subsequent siblings)
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
use get_clock_ms() to calculate a delay in ms
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
lib/s390x/asm/time.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
index 25c7a3c..931a119 100644
--- a/lib/s390x/asm/time.h
+++ b/lib/s390x/asm/time.h
@@ -23,4 +23,14 @@ static inline uint64_t get_clock_ms(void)
return (clk >> (63 - 51)) / 1000;
}
+static inline void mdelay(unsigned long ms)
+{
+ unsigned long startclk;
+
+ startclk = get_clock_ms();
+ for (;;)
+ if (get_clock_ms() - startclk > ms)
+ break;
+}
+
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 06/12] s390x: use get_clock_ms() to calculate a delay in ms
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 06/12] s390x: use get_clock_ms() to calculate a delay in ms Pierre Morel
@ 2020-05-26 18:16 ` Thomas Huth
2020-06-04 7:21 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Thomas Huth @ 2020-05-26 18:16 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: linux-s390, frankja, david, cohuck
On 18/05/2020 18.07, Pierre Morel wrote:
> use get_clock_ms() to calculate a delay in ms
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> lib/s390x/asm/time.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
> index 25c7a3c..931a119 100644
> --- a/lib/s390x/asm/time.h
> +++ b/lib/s390x/asm/time.h
> @@ -23,4 +23,14 @@ static inline uint64_t get_clock_ms(void)
> return (clk >> (63 - 51)) / 1000;
> }
>
> +static inline void mdelay(unsigned long ms)
> +{
> + unsigned long startclk;
> +
> + startclk = get_clock_ms();
> + for (;;)
> + if (get_clock_ms() - startclk > ms)
> + break;
Maybe rather:
for (;get_clock_ms() - startclk <= ms;)
;
?
Or:
while (get_clock_ms() - startclk <= ms)
;
?
Thomas
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 06/12] s390x: use get_clock_ms() to calculate a delay in ms
2020-05-26 18:16 ` Thomas Huth
@ 2020-06-04 7:21 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-06-04 7:21 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, frankja, david, cohuck
On 2020-05-26 20:16, Thomas Huth wrote:
> On 18/05/2020 18.07, Pierre Morel wrote:
>> use get_clock_ms() to calculate a delay in ms
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/asm/time.h | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
>> index 25c7a3c..931a119 100644
>> --- a/lib/s390x/asm/time.h
>> +++ b/lib/s390x/asm/time.h
>> @@ -23,4 +23,14 @@ static inline uint64_t get_clock_ms(void)
>> return (clk >> (63 - 51)) / 1000;
>> }
>>
>> +static inline void mdelay(unsigned long ms)
>> +{
>> + unsigned long startclk;
>> +
>> + startclk = get_clock_ms();
>> + for (;;)
>> + if (get_clock_ms() - startclk > ms)
>> + break;
>
> Maybe rather:
>
> for (;get_clock_ms() - startclk <= ms;)
> ;
>
> ?
> Or:
>
> while (get_clock_ms() - startclk <= ms)
> ;
> ?
>
> Thomas
>
Hi,
your comment made me realize I did not take care on the wrapping.
I will rework this.
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 07/12] s390x: Library resources for CSS tests
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (5 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 06/12] s390x: use get_clock_ms() to calculate a delay in ms Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-26 16:30 ` Cornelia Huck
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test Pierre Morel
` (4 subsequent siblings)
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
Provide some definitions and library routines that can be used by
tests targeting the channel subsystem.
Debug function can be activated by defining DEBUG_CSS before the
inclusion of the css.h header file.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
lib/s390x/css.h | 259 +++++++++++++++++++++++++++++++++++++++++++
lib/s390x/css_dump.c | 157 ++++++++++++++++++++++++++
s390x/Makefile | 1 +
3 files changed, 417 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..e0d3a98
--- /dev/null
+++ b/lib/s390x/css.h
@@ -0,0 +1,259 @@
+/*
+ * CSS definitions
+ *
+ * Copyright IBM, Corp. 2020
+ * 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 SID_ONE 0x00010000
+
+#define ORB_M_KEY 0xf0000000
+#define ORB_F_SUSPEND 0x08000000
+#define ORB_F_STREAMING 0x04000000
+#define ORB_F_MODIFCTRL 0x02000000
+#define ORB_F_SYNC 0x01000000
+#define ORB_F_FORMAT 0x00800000
+#define ORB_F_PREFETCH 0x00400000
+#define ORB_F_INIT_IRQ 0x00200000
+#define ORB_F_ADDRLIMIT 0x00100000
+#define ORB_F_SUSP_IRQ 0x00080000
+#define ORB_F_TRANSPORT 0x00040000
+#define ORB_F_IDAW2 0x00020000
+#define ORB_F_IDAW_2K 0x00010000
+#define ORB_M_LPM 0x0000ff00
+#define ORB_F_LPM_DFLT 0x00008000
+#define ORB_F_ILSM 0x00000080
+#define ORB_F_CCW_IND 0x00000040
+#define ORB_F_ORB_EXT 0x00000001
+
+struct orb {
+ uint32_t intparm;
+ uint32_t ctrl;
+ uint32_t cpa;
+ uint32_t prio;
+ uint32_t reserved[4];
+} __attribute__ ((aligned(4)));
+
+struct scsw {
+ uint32_t ctrl;
+ uint32_t ccw_addr;
+ uint8_t dev_stat;
+ uint8_t sch_stat;
+ uint16_t count;
+};
+
+struct pmcw {
+ uint32_t intparm;
+#define PMCW_DNV 0x0001
+#define PMCW_ENABLE 0x0080
+ uint16_t flags;
+ uint16_t devnum;
+ uint8_t lpm;
+ uint8_t pnom;
+ uint8_t lpum;
+ uint8_t pim;
+ uint16_t mbi;
+ uint8_t pom;
+ uint8_t pam;
+ uint8_t chpid[8];
+ uint32_t flags2;
+};
+#define PMCW_CHANNEL_TYPE(pmcw) (pmcw->flags2 >> 21)
+
+struct schib {
+ struct pmcw pmcw;
+ struct scsw scsw;
+ uint8_t md[12];
+} __attribute__ ((aligned(4)));
+
+struct irb {
+ struct scsw scsw;
+ uint32_t esw[5];
+ uint32_t ecw[8];
+ uint32_t emw[8];
+} __attribute__ ((aligned(4)));
+
+/* CSS low level access functions */
+
+static inline int ssch(unsigned long schid, struct orb *addr)
+{
+ register long long reg1 asm("1") = schid;
+ int cc;
+
+ asm volatile(
+ " ssch 0(%2)\n"
+ " ipm %0\n"
+ " srl %0,28\n"
+ : "=d" (cc)
+ : "d" (reg1), "a" (addr), "m" (*addr)
+ : "cc", "memory");
+ return cc;
+}
+
+static inline int stsch(unsigned long schid, struct schib *addr)
+{
+ register unsigned long reg1 asm ("1") = schid;
+ int cc;
+
+ asm volatile(
+ " stsch 0(%3)\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=d" (cc), "=m" (*addr)
+ : "d" (reg1), "a" (addr)
+ : "cc");
+ return cc;
+}
+
+static inline int msch(unsigned long schid, struct schib *addr)
+{
+ register unsigned long reg1 asm ("1") = schid;
+ int cc;
+
+ asm volatile(
+ " msch 0(%3)\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=d" (cc), "=m" (*addr)
+ : "d" (reg1), "a" (addr)
+ : "cc");
+ return cc;
+}
+
+static inline int tsch(unsigned long schid, struct irb *addr)
+{
+ register unsigned long reg1 asm ("1") = schid;
+ int cc;
+
+ asm volatile(
+ " tsch 0(%3)\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=d" (cc), "=m" (*addr)
+ : "d" (reg1), "a" (addr)
+ : "cc");
+ return cc;
+}
+
+static inline int hsch(unsigned long schid)
+{
+ register unsigned long reg1 asm("1") = schid;
+ int cc;
+
+ asm volatile(
+ " hsch\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=d" (cc)
+ : "d" (reg1)
+ : "cc");
+ return cc;
+}
+
+static inline int xsch(unsigned long schid)
+{
+ register unsigned long reg1 asm("1") = schid;
+ int cc;
+
+ asm volatile(
+ " xsch\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=d" (cc)
+ : "d" (reg1)
+ : "cc");
+ return cc;
+}
+
+static inline int csch(unsigned long schid)
+{
+ register unsigned long reg1 asm("1") = schid;
+ int cc;
+
+ asm volatile(
+ " csch\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=d" (cc)
+ : "d" (reg1)
+ : "cc");
+ return cc;
+}
+
+static inline int rsch(unsigned long schid)
+{
+ register unsigned long reg1 asm("1") = schid;
+ int cc;
+
+ asm volatile(
+ " rsch\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=d" (cc)
+ : "d" (reg1)
+ : "cc");
+ return cc;
+}
+
+static inline int rchp(unsigned long chpid)
+{
+ register unsigned long reg1 asm("1") = chpid;
+ int cc;
+
+ asm volatile(
+ " rchp\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=d" (cc)
+ : "d" (reg1)
+ : "cc");
+ return cc;
+}
+
+/* Debug functions */
+char *dump_pmcw_flags(uint16_t f);
+char *dump_scsw_flags(uint32_t f);
+
+#ifdef DEBUG_CSS
+void dump_scsw(struct scsw *);
+void dump_irb(struct irb *irbp);
+void dump_schib(struct schib *sch);
+struct ccw *dump_ccw(struct ccw *cp);
+#else
+static inline void dump_scsw(struct scsw *scsw) {}
+static inline void dump_irb(struct irb *irbp) {}
+static inline void dump_pmcw(struct pmcw *p) {}
+static inline void dump_schib(struct schib *sch) {}
+static inline void dump_orb(struct orb *op) {}
+static inline struct ccw *dump_ccw(struct ccw *cp)
+{
+ return NULL;
+}
+#endif /* DEBUG_CSS */
+#endif
diff --git a/lib/s390x/css_dump.c b/lib/s390x/css_dump.c
new file mode 100644
index 0000000..c1e8a53
--- /dev/null
+++ b/lib/s390x/css_dump.c
@@ -0,0 +1,157 @@
+/*
+ * Channel subsystem structures dumping
+ *
+ * Copyright (c) 2020 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.
+ *
+ * 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 command, data and flow control
+ * - IRB : Interuption response Block, describes the result of an operation;
+ * holds a SCSW and model-dependent data.
+ * - SCHIB: SubCHannel Information Block composed of:
+ * - SCSW: SubChannel Status Word, status of the channel.
+ * - PMCW: Path Management Control Word
+ * You need the QEMU ccw-pong device in QEMU to answer the I/O transfers.
+ */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+
+#undef DEBUG_CSS
+#include <css.h>
+
+/*
+ * Try to have a more human representation of the SCSW flags:
+ * each letter in the two strings represent the first
+ * letter of the associated bit in the flag fields.
+ */
+static const char *scsw_str = "kkkkslccfpixuzen";
+static const char *scsw_str2 = "1SHCrshcsdsAIPSs";
+static char scsw_line[64] = {};
+
+char *dump_scsw_flags(uint32_t f)
+{
+ int i;
+
+ for (i = 0; i < 16; i++) {
+ if ((f << i) & 0x80000000)
+ scsw_line[i] = scsw_str[i];
+ else
+ scsw_line[i] = '_';
+ }
+ scsw_line[i] = ' ';
+ for (; i < 32; i++) {
+ if ((f << i) & 0x80000000)
+ scsw_line[i + 1] = scsw_str2[i - 16];
+ else
+ scsw_line[i + 1] = '_';
+ }
+ return scsw_line;
+}
+
+/*
+ * Try o have a more human representation of the PMCW flags
+ * each letter in the string represent the first
+ * letter of the associated bit in the flag fields.
+ */
+static const char *pmcw_str = "11iii111ellmmdtv";
+static char pcmw_line[32] = {};
+char *dump_pmcw_flags(uint16_t f)
+{
+ int i;
+
+ for (i = 0; i < 16; i++) {
+ if ((f << i) & 0x8000)
+ pcmw_line[i] = pmcw_str[i];
+ else
+ pcmw_line[i] = '_';
+ }
+ return pcmw_line;
+}
+
+#ifdef DEBUG_CSS
+void dump_scsw(struct scsw *s)
+{
+ dump_scsw_flags(s->ctrl);
+ printf("scsw->flags: %s\n", line);
+ printf("scsw->addr : %08x\n", s->addr);
+ printf("scsw->devs : %02x\n", s->devs);
+ printf("scsw->schs : %02x\n", s->schs);
+ printf("scsw->count: %04x\n", s->count);
+}
+
+void dump_irb(struct irb *irbp)
+{
+ int i;
+ uint32_t *p = (uint32_t *)irbp;
+
+ dump_scsw(&irbp->scsw);
+ for (i = 0; i < sizeof(*irbp)/sizeof(*p); i++, p++)
+ printf("irb[%02x] : %08x\n", i, *p);
+}
+
+void dump_pmcw(struct pmcw *p)
+{
+ int i;
+
+ printf("pmcw->intparm : %08x\n", p->intparm);
+ printf("pmcw->flags : %04x\n", p->flags);
+ dump_pmcw_flags(p->flags);
+ printf("pmcw->devnum : %04x\n", p->devnum);
+ printf("pmcw->lpm : %02x\n", p->lpm);
+ printf("pmcw->pnom : %02x\n", p->pnom);
+ printf("pmcw->lpum : %02x\n", p->lpum);
+ printf("pmcw->pim : %02x\n", p->pim);
+ printf("pmcw->mbi : %04x\n", p->mbi);
+ printf("pmcw->pom : %02x\n", p->pom);
+ printf("pmcw->pam : %02x\n", p->pam);
+ printf("pmcw->mbi : %04x\n", p->mbi);
+ for (i = 0; i < 8; i++)
+ printf("pmcw->chpid[%d]: %02x\n", i, p->chpid[i]);
+ printf("pmcw->flags2 : %08x\n", p->flags2);
+}
+
+void dump_schib(struct schib *sch)
+{
+ struct pmcw *p = &sch->pmcw;
+ struct scsw *s = &sch->scsw;
+
+ printf("--SCHIB--\n");
+ dump_pmcw(p);
+ dump_scsw(s);
+}
+
+struct ccw *dump_ccw(struct ccw *cp)
+{
+ printf("CCW: code: %02x flags: %02x count: %04x data: %08x\n", cp->code,
+ cp->flags, cp->count, cp->data);
+
+ if (cp->code == CCW_C_TIC)
+ return (struct ccw *)(long)cp->data;
+
+ return (cp->flags & CCW_F_CC) ? cp + 1 : NULL;
+}
+
+void dump_orb(struct orb *op)
+{
+ struct ccw *cp;
+
+ printf("ORB: intparm : %08x\n", op->intparm);
+ printf("ORB: ctrl : %08x\n", op->ctrl);
+ printf("ORB: prio : %08x\n", op->prio);
+ cp = (struct ccw *)(long) (op->cpa);
+ while (cp)
+ cp = dump_ccw(cp);
+}
+
+#endif
diff --git a/s390x/Makefile b/s390x/Makefile
index ddb4b48..050c40b 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -51,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
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 07/12] s390x: Library resources for CSS tests
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 07/12] s390x: Library resources for CSS tests Pierre Morel
@ 2020-05-26 16:30 ` Cornelia Huck
2020-06-04 7:42 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Cornelia Huck @ 2020-05-26 16:30 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth
On Mon, 18 May 2020 18:07:26 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> Provide some definitions and library routines that can be used by
> tests targeting the channel subsystem.
>
> Debug function can be activated by defining DEBUG_CSS before the
> inclusion of the css.h header file.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> lib/s390x/css.h | 259 +++++++++++++++++++++++++++++++++++++++++++
> lib/s390x/css_dump.c | 157 ++++++++++++++++++++++++++
> s390x/Makefile | 1 +
> 3 files changed, 417 insertions(+)
> create mode 100644 lib/s390x/css.h
> create mode 100644 lib/s390x/css_dump.c
>
(...)
> +struct ccw1 {
> + unsigned char code;
> + unsigned char flags;
> + unsigned short count;
I'm wondering why you're using unsigned {char,short} here, instead of
the uint*_t types everywhere else? It's not wrong, but probably better
to be consistent?
> + uint32_t data_address;
> +} __attribute__ ((aligned(4)));
> +
> +#define SID_ONE 0x00010000
> +
I think it would be beneficial for the names to somewhat match the
naming in Linux and/or QEMU -- or more speaking names (as you do for
some), which is also good.
> +#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
ORB_F_ISIC? (As it does not refer to 'initialization', but 'initial'.)
> +#define ORB_F_ADDRLIMIT 0x00100000
> +#define ORB_F_SUSP_IRQ 0x00080000
ORB_F_SSIC? (As it deals with suppression.)
> +#define ORB_F_TRANSPORT 0x00040000
> +#define ORB_F_IDAW2 0x00020000
ORB_F_IDAW_FMT2?
Or following Linux/QEMU, use ORB_F_C64 for a certain retro appeal :)
> +#define ORB_F_IDAW_2K 0x00010000
> +#define ORB_M_LPM 0x0000ff00
> +#define ORB_F_LPM_DFLT 0x00008000
That's a default lpm of 0x80, right? It's a bit buried between the orb
definitions, and it also seems to be more of a implementation choice --
move it out from the flags here?
> +#define ORB_F_ILSM 0x00000080
ORB_F_ILS?
> +#define ORB_F_CCW_IND 0x00000040
ORB_F_MIDAW? I had a hard time figuring out that one :)
> +#define ORB_F_ORB_EXT 0x00000001
(...)
> +/*
> + * Try o have a more human representation of the PMCW flags
s/o/to/
> + * each letter in the string represent the first
s/represent/represents/
> + * letter of the associated bit in the flag fields.
> + */
(...)
Generally, looks good to me.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 07/12] s390x: Library resources for CSS tests
2020-05-26 16:30 ` Cornelia Huck
@ 2020-06-04 7:42 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-06-04 7:42 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth
On 2020-05-26 18:30, Cornelia Huck wrote:
> On Mon, 18 May 2020 18:07:26 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> Provide some definitions and library routines that can be used by
>> tests targeting the channel subsystem.
>>
>> Debug function can be activated by defining DEBUG_CSS before the
>> inclusion of the css.h header file.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/css.h | 259 +++++++++++++++++++++++++++++++++++++++++++
>> lib/s390x/css_dump.c | 157 ++++++++++++++++++++++++++
>> s390x/Makefile | 1 +
>> 3 files changed, 417 insertions(+)
>> create mode 100644 lib/s390x/css.h
>> create mode 100644 lib/s390x/css_dump.c
>>
>
> (...)
>
>> +struct ccw1 {
>> + unsigned char code;
>> + unsigned char flags;
>> + unsigned short count;
>
> I'm wondering why you're using unsigned {char,short} here, instead of
> the uint*_t types everywhere else? It's not wrong, but probably better
> to be consistent?
>
>> + uint32_t data_address;
>> +} __attribute__ ((aligned(4)));
>> +
>> +#define SID_ONE 0x00010000
>> +
>
> I think it would be beneficial for the names to somewhat match the
> naming in Linux and/or QEMU -- or more speaking names (as you do for
> some), which is also good.
>
>> +#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
>
> ORB_F_ISIC? (As it does not refer to 'initialization', but 'initial'.)
>
>> +#define ORB_F_ADDRLIMIT 0x00100000
>> +#define ORB_F_SUSP_IRQ 0x00080000
>
> ORB_F_SSIC? (As it deals with suppression.)
>
>> +#define ORB_F_TRANSPORT 0x00040000
>> +#define ORB_F_IDAW2 0x00020000
>
> ORB_F_IDAW_FMT2?
>
> Or following Linux/QEMU, use ORB_F_C64 for a certain retro appeal :)
>
>> +#define ORB_F_IDAW_2K 0x00010000
>> +#define ORB_M_LPM 0x0000ff00
>> +#define ORB_F_LPM_DFLT 0x00008000
>
> That's a default lpm of 0x80, right? It's a bit buried between the orb
> definitions, and it also seems to be more of a implementation choice --
> move it out from the flags here?
>
>> +#define ORB_F_ILSM 0x00000080
>
> ORB_F_ILS?
>
>> +#define ORB_F_CCW_IND 0x00000040
>
> ORB_F_MIDAW? I had a hard time figuring out that one :)
>
>> +#define ORB_F_ORB_EXT 0x00000001
>
> (...)
>
>> +/*
>> + * Try o have a more human representation of the PMCW flags
>
> s/o/to/
>
>> + * each letter in the string represent the first
>
> s/represent/represents/
>
>> + * letter of the associated bit in the flag fields.
>> + */
>
> (...)
>
> Generally, looks good to me.
>
Thanks a lot,
I take all your remarks and will update the names for better ones as you
suggest.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (6 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 07/12] s390x: Library resources for CSS tests Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-25 19:12 ` Thomas Huth
2020-05-27 8:55 ` Cornelia Huck
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test Pierre Morel
` (3 subsequent siblings)
11 siblings, 2 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
First step for testing the channel subsystem is to enumerate the css and
retrieve the css devices.
This tests the success of STSCH I/O instruction, we do not test the
reaction of the VM for an instruction with wrong parameters.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
s390x/Makefile | 1 +
s390x/css.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 4 ++
3 files changed, 94 insertions(+)
create mode 100644 s390x/css.c
diff --git a/s390x/Makefile b/s390x/Makefile
index 050c40b..baebf18 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -17,6 +17,7 @@ tests += $(TEST_DIR)/stsi.elf
tests += $(TEST_DIR)/skrf.elf
tests += $(TEST_DIR)/smp.elf
tests += $(TEST_DIR)/sclp.elf
+tests += $(TEST_DIR)/css.elf
tests_binary = $(patsubst %.elf,%.bin,$(tests))
all: directories test_cases test_cases_binary
diff --git a/s390x/css.c b/s390x/css.c
new file mode 100644
index 0000000..d7989d8
--- /dev/null
+++ b/s390x/css.c
@@ -0,0 +1,89 @@
+/*
+ * Channel Subsystem tests
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ * Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2.
+ */
+
+#include <libcflat.h>
+#include <alloc_phys.h>
+#include <asm/page.h>
+#include <string.h>
+#include <interrupt.h>
+#include <asm/arch_def.h>
+
+#include <css.h>
+
+static struct schib schib;
+static int test_device_sid;
+
+static void test_enumerate(void)
+{
+ struct pmcw *pmcw = &schib.pmcw;
+ int cc;
+ int scn;
+ int scn_found = 0;
+ int dev_found = 0;
+
+ for (scn = 0; scn < 0xffff; scn++) {
+ cc = stsch(scn|SID_ONE, &schib);
+ switch (cc) {
+ case 0: /* 0 means SCHIB stored */
+ break;
+ case 3: /* 3 means no more channels */
+ goto out;
+ default: /* 1 or 2 should never happened for STSCH */
+ report(0, "Unexpected cc=%d on subchannel number 0x%x",
+ cc, scn);
+ return;
+ }
+
+ /* We currently only support type 0, a.k.a. I/O channels */
+ if (PMCW_CHANNEL_TYPE(pmcw) != 0)
+ continue;
+
+ /* We ignore I/O channels without valid devices */
+ scn_found++;
+ if (!(pmcw->flags & PMCW_DNV))
+ continue;
+
+ /* We keep track of the first device as our test device */
+ if (!test_device_sid)
+ test_device_sid = scn | SID_ONE;
+
+ dev_found++;
+ }
+
+out:
+ report(dev_found,
+ "Tested subchannels: %d, I/O subchannels: %d, I/O devices: %d",
+ scn, scn_found, dev_found);
+}
+
+static struct {
+ const char *name;
+ void (*func)(void);
+} tests[] = {
+ { "enumerate (stsch)", test_enumerate },
+ { NULL, NULL }
+};
+
+int main(int argc, char *argv[])
+{
+ int i;
+
+ report_prefix_push("Channel Subsystem");
+ for (i = 0; tests[i].name; i++) {
+ report_prefix_push(tests[i].name);
+ tests[i].func();
+ report_prefix_pop();
+ }
+ report_prefix_pop();
+
+ return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 07013b2..a436ec0 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -83,3 +83,7 @@ extra_params = -m 1G
[sclp-3g]
file = sclp.elf
extra_params = -m 3G
+
+[css]
+file = css.elf
+extra_params =-device ccw-pong
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test Pierre Morel
@ 2020-05-25 19:12 ` Thomas Huth
2020-05-26 10:41 ` Janosch Frank
2020-05-27 8:55 ` Cornelia Huck
1 sibling, 1 reply; 48+ messages in thread
From: Thomas Huth @ 2020-05-25 19:12 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: linux-s390, frankja, david, cohuck
On 18/05/2020 18.07, 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, we do not test the
> reaction of the VM for an instruction with wrong parameters.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> s390x/Makefile | 1 +
> s390x/css.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
> s390x/unittests.cfg | 4 ++
> 3 files changed, 94 insertions(+)
> create mode 100644 s390x/css.c
[...]
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 07013b2..a436ec0 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -83,3 +83,7 @@ extra_params = -m 1G
> [sclp-3g]
> file = sclp.elf
> extra_params = -m 3G
> +
> +[css]
> +file = css.elf
> +extra_params =-device ccw-pong
I gave your patch series a try on a normal upstream QEMU (that does not
have the ccw-pong device yet), and the css test of course fails there,
since QEMU bails out with:
-device ccw-pong: 'ccw-pong' is not a valid device model name
This is unfortunate - I think we likely have to deal with QEMUs for
quite a while that do not have this device enabled. Could you maybe add
some kind of check to the kvm-unit-tests scripts that only run a test if
a given device is available, and skip the test otherwise?
Thomas
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-05-25 19:12 ` Thomas Huth
@ 2020-05-26 10:41 ` Janosch Frank
2020-05-26 10:49 ` Thomas Huth
0 siblings, 1 reply; 48+ messages in thread
From: Janosch Frank @ 2020-05-26 10:41 UTC (permalink / raw)
To: Thomas Huth, Pierre Morel, kvm; +Cc: linux-s390, david, cohuck
[-- Attachment #1.1: Type: text/plain, Size: 1714 bytes --]
On 5/25/20 9:12 PM, Thomas Huth wrote:
> On 18/05/2020 18.07, 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, we do not test the
>> reaction of the VM for an instruction with wrong parameters.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> s390x/Makefile | 1 +
>> s390x/css.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
>> s390x/unittests.cfg | 4 ++
>> 3 files changed, 94 insertions(+)
>> create mode 100644 s390x/css.c
> [...]
>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>> index 07013b2..a436ec0 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -83,3 +83,7 @@ extra_params = -m 1G
>> [sclp-3g]
>> file = sclp.elf
>> extra_params = -m 3G
>> +
>> +[css]
>> +file = css.elf
>> +extra_params =-device ccw-pong
>
> I gave your patch series a try on a normal upstream QEMU (that does not
> have the ccw-pong device yet), and the css test of course fails there,
> since QEMU bails out with:
>
> -device ccw-pong: 'ccw-pong' is not a valid device model name
>
> This is unfortunate - I think we likely have to deal with QEMUs for
> quite a while that do not have this device enabled. Could you maybe add
> some kind of check to the kvm-unit-tests scripts that only run a test if
> a given device is available, and skip the test otherwise?
>
> Thomas
>
Could we for now remove it from unittests.cfg and let Pierre come up
with a solution without delaying this whole series? I expect changes to
run_tests.sh to attract a rather long discussion...
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-05-26 10:41 ` Janosch Frank
@ 2020-05-26 10:49 ` Thomas Huth
2020-05-26 11:38 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Thomas Huth @ 2020-05-26 10:49 UTC (permalink / raw)
To: Janosch Frank, Pierre Morel, kvm; +Cc: linux-s390, david, cohuck
On 26/05/2020 12.41, Janosch Frank wrote:
> On 5/25/20 9:12 PM, Thomas Huth wrote:
>> On 18/05/2020 18.07, 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, we do not test the
>>> reaction of the VM for an instruction with wrong parameters.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>> s390x/Makefile | 1 +
>>> s390x/css.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
>>> s390x/unittests.cfg | 4 ++
>>> 3 files changed, 94 insertions(+)
>>> create mode 100644 s390x/css.c
>> [...]
>>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>>> index 07013b2..a436ec0 100644
>>> --- a/s390x/unittests.cfg
>>> +++ b/s390x/unittests.cfg
>>> @@ -83,3 +83,7 @@ extra_params = -m 1G
>>> [sclp-3g]
>>> file = sclp.elf
>>> extra_params = -m 3G
>>> +
>>> +[css]
>>> +file = css.elf
>>> +extra_params =-device ccw-pong
>>
>> I gave your patch series a try on a normal upstream QEMU (that does not
>> have the ccw-pong device yet), and the css test of course fails there,
>> since QEMU bails out with:
>>
>> -device ccw-pong: 'ccw-pong' is not a valid device model name
>>
>> This is unfortunate - I think we likely have to deal with QEMUs for
>> quite a while that do not have this device enabled. Could you maybe add
>> some kind of check to the kvm-unit-tests scripts that only run a test if
>> a given device is available, and skip the test otherwise?
>>
>> Thomas
>>
>
> Could we for now remove it from unittests.cfg and let Pierre come up
> with a solution without delaying this whole series? I expect changes to
> run_tests.sh to attract a rather long discussion...
Fine for me, too.
Thomas
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-05-26 10:49 ` Thomas Huth
@ 2020-05-26 11:38 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-26 11:38 UTC (permalink / raw)
To: Thomas Huth, Janosch Frank, kvm; +Cc: linux-s390, david, cohuck
On 2020-05-26 12:49, Thomas Huth wrote:
> On 26/05/2020 12.41, Janosch Frank wrote:
>> On 5/25/20 9:12 PM, Thomas Huth wrote:
>>> On 18/05/2020 18.07, Pierre Morel wrote:
>>>> First step for testing the channel subsystem is to enumerate the css and
>>>> retrieve the css devices.
...snip...
>>> I gave your patch series a try on a normal upstream QEMU (that does not
>>> have the ccw-pong device yet), and the css test of course fails there,
>>> since QEMU bails out with:
>>>
>>> -device ccw-pong: 'ccw-pong' is not a valid device model name
>>>
>>> This is unfortunate - I think we likely have to deal with QEMUs for
>>> quite a while that do not have this device enabled. Could you maybe add
>>> some kind of check to the kvm-unit-tests scripts that only run a test if
>>> a given device is available, and skip the test otherwise?
>>>
>>> Thomas
>>>
>>
>> Could we for now remove it from unittests.cfg and let Pierre come up
>> with a solution without delaying this whole series? I expect changes to
>> run_tests.sh to attract a rather long discussion...
>
> Fine for me, too.
>
> Thomas
>
OK, thanks you both, I do so, take the unittest.cfg out from this series
and come back later with a proposition for unittest.cfg.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test Pierre Morel
2020-05-25 19:12 ` Thomas Huth
@ 2020-05-27 8:55 ` Cornelia Huck
2020-06-04 11:35 ` Pierre Morel
1 sibling, 1 reply; 48+ messages in thread
From: Cornelia Huck @ 2020-05-27 8:55 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth
On Mon, 18 May 2020 18:07:27 +0200
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, we do not test the
> reaction of the VM for an instruction with wrong parameters.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> s390x/Makefile | 1 +
> s390x/css.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
> s390x/unittests.cfg | 4 ++
> 3 files changed, 94 insertions(+)
> create mode 100644 s390x/css.c
(...)
> +static void test_enumerate(void)
> +{
> + struct pmcw *pmcw = &schib.pmcw;
> + int cc;
> + int scn;
> + int scn_found = 0;
> + int dev_found = 0;
> +
> + for (scn = 0; scn < 0xffff; scn++) {
> + cc = stsch(scn|SID_ONE, &schib);
> + switch (cc) {
> + case 0: /* 0 means SCHIB stored */
> + break;
> + case 3: /* 3 means no more channels */
> + goto out;
> + default: /* 1 or 2 should never happened for STSCH */
> + report(0, "Unexpected cc=%d on subchannel number 0x%x",
> + cc, scn);
> + return;
> + }
> +
> + /* We currently only support type 0, a.k.a. I/O channels */
> + if (PMCW_CHANNEL_TYPE(pmcw) != 0)
> + continue;
> +
> + /* We ignore I/O channels without valid devices */
> + scn_found++;
> + if (!(pmcw->flags & PMCW_DNV))
> + continue;
> +
> + /* We keep track of the first device as our test device */
> + if (!test_device_sid)
> + test_device_sid = scn | SID_ONE;
> +
> + dev_found++;
> + }
> +
> +out:
> + report(dev_found,
> + "Tested subchannels: %d, I/O subchannels: %d, I/O devices: %d",
> + scn, scn_found, dev_found);
Just wondering: with the current invocation, you expect to find exactly
one subchannel with a valid device, right?
> +}
> +
> +static struct {
> + const char *name;
> + void (*func)(void);
> +} tests[] = {
> + { "enumerate (stsch)", test_enumerate },
> + { NULL, NULL }
> +};
> +
> +int main(int argc, char *argv[])
> +{
> + int i;
> +
> + report_prefix_push("Channel Subsystem");
> + for (i = 0; tests[i].name; i++) {
> + report_prefix_push(tests[i].name);
> + tests[i].func();
> + report_prefix_pop();
> + }
> + report_prefix_pop();
> +
> + return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 07013b2..a436ec0 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -83,3 +83,7 @@ extra_params = -m 1G
> [sclp-3g]
> file = sclp.elf
> extra_params = -m 3G
> +
> +[css]
> +file = css.elf
> +extra_params =-device ccw-pong
Hm... you could test enumeration even with a QEMU that does not include
support for the pong device, right? Would it be worthwhile to split out
a set of css tests that use e.g. a virtio-net-ccw device, and have a
css-pong set of tests that require the pong device?
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-05-27 8:55 ` Cornelia Huck
@ 2020-06-04 11:35 ` Pierre Morel
2020-06-04 11:45 ` Thomas Huth
0 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-06-04 11:35 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth
On 2020-05-27 10:55, Cornelia Huck wrote:
> On Mon, 18 May 2020 18:07:27 +0200
> 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, we do not test the
>> reaction of the VM for an instruction with wrong parameters.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> s390x/Makefile | 1 +
>> s390x/css.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
>> s390x/unittests.cfg | 4 ++
>> 3 files changed, 94 insertions(+)
>> create mode 100644 s390x/css.c
>
> (...)
>
>> +static void test_enumerate(void)
>> +{
>> + struct pmcw *pmcw = &schib.pmcw;
>> + int cc;
>> + int scn;
>> + int scn_found = 0;
>> + int dev_found = 0;
>> +
>> + for (scn = 0; scn < 0xffff; scn++) {
>> + cc = stsch(scn|SID_ONE, &schib);
>> + switch (cc) {
>> + case 0: /* 0 means SCHIB stored */
>> + break;
>> + case 3: /* 3 means no more channels */
>> + goto out;
>> + default: /* 1 or 2 should never happened for STSCH */
>> + report(0, "Unexpected cc=%d on subchannel number 0x%x",
>> + cc, scn);
>> + return;
>> + }
>> +
>> + /* We currently only support type 0, a.k.a. I/O channels */
>> + if (PMCW_CHANNEL_TYPE(pmcw) != 0)
>> + continue;
>> +
>> + /* We ignore I/O channels without valid devices */
>> + scn_found++;
>> + if (!(pmcw->flags & PMCW_DNV))
>> + continue;
>> +
>> + /* We keep track of the first device as our test device */
>> + if (!test_device_sid)
>> + test_device_sid = scn | SID_ONE;
>> +
>> + dev_found++;
>> + }
>> +
>> +out:
>> + report(dev_found,
>> + "Tested subchannels: %d, I/O subchannels: %d, I/O devices: %d",
>> + scn, scn_found, dev_found);
>
> Just wondering: with the current invocation, you expect to find exactly
> one subchannel with a valid device, right?
...snip...
>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>> index 07013b2..a436ec0 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -83,3 +83,7 @@ extra_params = -m 1G
>> [sclp-3g]
>> file = sclp.elf
>> extra_params = -m 3G
>> +
>> +[css]
>> +file = css.elf
>> +extra_params =-device ccw-pong
>
> Hm... you could test enumeration even with a QEMU that does not include
> support for the pong device, right? Would it be worthwhile to split out
> a set of css tests that use e.g. a virtio-net-ccw device, and have a
> css-pong set of tests that require the pong device?
>
Yes, you are right, using a virtio-net-ccw will allow to keep this test
without waiting for the PONG device to exist.
@Thomas, what do you think? I will still have to figure something out
for PONG tests but here, it should be OK with virtio-net-ccw.
Thanks a lot for the solution, :)
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-06-04 11:35 ` Pierre Morel
@ 2020-06-04 11:45 ` Thomas Huth
2020-06-04 12:27 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Thomas Huth @ 2020-06-04 11:45 UTC (permalink / raw)
To: Pierre Morel, Cornelia Huck; +Cc: kvm, linux-s390, frankja, david
On 04/06/2020 13.35, Pierre Morel wrote:
>
>
> On 2020-05-27 10:55, Cornelia Huck wrote:
>> On Mon, 18 May 2020 18:07:27 +0200
>> 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, we do not test the
>>> reaction of the VM for an instruction with wrong parameters.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>> s390x/Makefile | 1 +
>>> s390x/css.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
>>> s390x/unittests.cfg | 4 ++
>>> 3 files changed, 94 insertions(+)
>>> create mode 100644 s390x/css.c
>>
>> (...)
>>
>>> +static void test_enumerate(void)
>>> +{
>>> + struct pmcw *pmcw = &schib.pmcw;
>>> + int cc;
>>> + int scn;
>>> + int scn_found = 0;
>>> + int dev_found = 0;
>>> +
>>> + for (scn = 0; scn < 0xffff; scn++) {
>>> + cc = stsch(scn|SID_ONE, &schib);
>>> + switch (cc) {
>>> + case 0: /* 0 means SCHIB stored */
>>> + break;
>>> + case 3: /* 3 means no more channels */
>>> + goto out;
>>> + default: /* 1 or 2 should never happened for STSCH */
>>> + report(0, "Unexpected cc=%d on subchannel number 0x%x",
>>> + cc, scn);
>>> + return;
>>> + }
>>> +
>>> + /* We currently only support type 0, a.k.a. I/O channels */
>>> + if (PMCW_CHANNEL_TYPE(pmcw) != 0)
>>> + continue;
>>> +
>>> + /* We ignore I/O channels without valid devices */
>>> + scn_found++;
>>> + if (!(pmcw->flags & PMCW_DNV))
>>> + continue;
>>> +
>>> + /* We keep track of the first device as our test device */
>>> + if (!test_device_sid)
>>> + test_device_sid = scn | SID_ONE;
>>> +
>>> + dev_found++;
>>> + }
>>> +
>>> +out:
>>> + report(dev_found,
>>> + "Tested subchannels: %d, I/O subchannels: %d, I/O
>>> devices: %d",
>>> + scn, scn_found, dev_found);
>>
>> Just wondering: with the current invocation, you expect to find exactly
>> one subchannel with a valid device, right?
> ...snip...
>
>>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>>> index 07013b2..a436ec0 100644
>>> --- a/s390x/unittests.cfg
>>> +++ b/s390x/unittests.cfg
>>> @@ -83,3 +83,7 @@ extra_params = -m 1G
>>> [sclp-3g]
>>> file = sclp.elf
>>> extra_params = -m 3G
>>> +
>>> +[css]
>>> +file = css.elf
>>> +extra_params =-device ccw-pong
>>
>> Hm... you could test enumeration even with a QEMU that does not include
>> support for the pong device, right? Would it be worthwhile to split out
>> a set of css tests that use e.g. a virtio-net-ccw device, and have a
>> css-pong set of tests that require the pong device?
>>
>
> Yes, you are right, using a virtio-net-ccw will allow to keep this test
> without waiting for the PONG device to exist.
>
> @Thomas, what do you think? I will still have to figure something out
> for PONG tests but here, it should be OK with virtio-net-ccw.
Sure, sounds good. We can go with -device virtio-net-ccw for now, and
then later add an additional entry a la:
[css-pong]
file = css.elf
device = ccw-pong
... where the test scripts then check for the availability of the device
first before starting the test?
Thomas
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test
2020-06-04 11:45 ` Thomas Huth
@ 2020-06-04 12:27 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-06-04 12:27 UTC (permalink / raw)
To: Thomas Huth, Cornelia Huck; +Cc: kvm, linux-s390, frankja, david
On 2020-06-04 13:45, Thomas Huth wrote:
> On 04/06/2020 13.35, Pierre Morel wrote:
>>
>>
>> On 2020-05-27 10:55, Cornelia Huck wrote:
>>> On Mon, 18 May 2020 18:07:27 +0200
>>> 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, we do not test the
>>>> reaction of the VM for an instruction with wrong parameters.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>> s390x/Makefile | 1 +
>>>> s390x/css.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
>>>> s390x/unittests.cfg | 4 ++
>>>> 3 files changed, 94 insertions(+)
>>>> create mode 100644 s390x/css.c
>>>
>>> (...)
>>>
>>>> +static void test_enumerate(void)
>>>> +{
>>>> + struct pmcw *pmcw = &schib.pmcw;
>>>> + int cc;
>>>> + int scn;
>>>> + int scn_found = 0;
>>>> + int dev_found = 0;
>>>> +
>>>> + for (scn = 0; scn < 0xffff; scn++) {
>>>> + cc = stsch(scn|SID_ONE, &schib);
>>>> + switch (cc) {
>>>> + case 0: /* 0 means SCHIB stored */
>>>> + break;
>>>> + case 3: /* 3 means no more channels */
>>>> + goto out;
>>>> + default: /* 1 or 2 should never happened for STSCH */
>>>> + report(0, "Unexpected cc=%d on subchannel number 0x%x",
>>>> + cc, scn);
>>>> + return;
>>>> + }
>>>> +
>>>> + /* We currently only support type 0, a.k.a. I/O channels */
>>>> + if (PMCW_CHANNEL_TYPE(pmcw) != 0)
>>>> + continue;
>>>> +
>>>> + /* We ignore I/O channels without valid devices */
>>>> + scn_found++;
>>>> + if (!(pmcw->flags & PMCW_DNV))
>>>> + continue;
>>>> +
>>>> + /* We keep track of the first device as our test device */
>>>> + if (!test_device_sid)
>>>> + test_device_sid = scn | SID_ONE;
>>>> +
>>>> + dev_found++;
>>>> + }
>>>> +
>>>> +out:
>>>> + report(dev_found,
>>>> + "Tested subchannels: %d, I/O subchannels: %d, I/O
>>>> devices: %d",
>>>> + scn, scn_found, dev_found);
>>>
>>> Just wondering: with the current invocation, you expect to find exactly
>>> one subchannel with a valid device, right?
>> ...snip...
>>
>>>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>>>> index 07013b2..a436ec0 100644
>>>> --- a/s390x/unittests.cfg
>>>> +++ b/s390x/unittests.cfg
>>>> @@ -83,3 +83,7 @@ extra_params = -m 1G
>>>> [sclp-3g]
>>>> file = sclp.elf
>>>> extra_params = -m 3G
>>>> +
>>>> +[css]
>>>> +file = css.elf
>>>> +extra_params =-device ccw-pong
>>>
>>> Hm... you could test enumeration even with a QEMU that does not include
>>> support for the pong device, right? Would it be worthwhile to split out
>>> a set of css tests that use e.g. a virtio-net-ccw device, and have a
>>> css-pong set of tests that require the pong device?
>>>
>>
>> Yes, you are right, using a virtio-net-ccw will allow to keep this test
>> without waiting for the PONG device to exist.
>>
>> @Thomas, what do you think? I will still have to figure something out
>> for PONG tests but here, it should be OK with virtio-net-ccw.
>
> Sure, sounds good. We can go with -device virtio-net-ccw for now, and
> then later add an additional entry a la:
>
> [css-pong]
> file = css.elf
> device = ccw-pong
>
> ... where the test scripts then check for the availability of the device
> first before starting the test?
>
> Thomas
>
yes,
thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (7 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 08/12] s390x: css: stsch, enumeration test Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-27 9:42 ` Cornelia Huck
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt Pierre Morel
` (2 subsequent siblings)
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
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.
- If the subchannel is not enabled retry a predefined retries count.
This tests the MSCH instruction to enable a channel succesfuly.
This is NOT a routine to really enable the channel, no retry is done,
in case of error, a report is made.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
s390x/css.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/s390x/css.c b/s390x/css.c
index d7989d8..1b60a47 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -16,6 +16,7 @@
#include <string.h>
#include <interrupt.h>
#include <asm/arch_def.h>
+#include <asm/time.h>
#include <css.h>
@@ -65,11 +66,77 @@ out:
scn, scn_found, dev_found);
}
+#define MAX_ENABLE_RETRIES 5
+static void test_enable(void)
+{
+ struct pmcw *pmcw = &schib.pmcw;
+ int retry_count = 0;
+ int cc;
+
+ if (!test_device_sid) {
+ report_skip("No device");
+ return;
+ }
+
+ /* Read the SCHIB for this subchannel */
+ cc = stsch(test_device_sid, &schib);
+ if (cc) {
+ report(0, "stsch cc=%d", cc);
+ return;
+ }
+
+ if (pmcw->flags & PMCW_ENABLE) {
+ report(1, "stsch: sch %08x already enabled", test_device_sid);
+ return;
+ }
+
+retry:
+ /* Update the SCHIB to enable the channel */
+ pmcw->flags |= PMCW_ENABLE;
+
+ /* Tell the CSS we want to modify the subchannel */
+ cc = msch(test_device_sid, &schib);
+ if (cc) {
+ /*
+ * If the subchannel is status pending or
+ * if a function is in progress,
+ * we consider both cases as errors.
+ */
+ report(0, "msch cc=%d", cc);
+ return;
+ }
+
+ /*
+ * Read the SCHIB again to verify the enablement
+ */
+ cc = stsch(test_device_sid, &schib);
+ if (cc) {
+ report(0, "stsch cc=%d", cc);
+ return;
+ }
+
+ if (pmcw->flags & PMCW_ENABLE) {
+ report(1, "msch: sch %08x enabled after %d retries",
+ test_device_sid, retry_count);
+ return;
+ }
+
+ if (retry_count++ < MAX_ENABLE_RETRIES) {
+ mdelay(10); /* the hardware was not ready, let it some time */
+ goto retry;
+ }
+
+ report(0,
+ "msch: enabling sch %08x failed after %d retries. pmcw: %x",
+ test_device_sid, retry_count, pmcw->flags);
+}
+
static struct {
const char *name;
void (*func)(void);
} tests[] = {
{ "enumerate (stsch)", test_enumerate },
+ { "enable (msch)", test_enable },
{ NULL, NULL }
};
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test Pierre Morel
@ 2020-05-27 9:42 ` Cornelia Huck
2020-06-04 12:46 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Cornelia Huck @ 2020-05-27 9:42 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth
On Mon, 18 May 2020 18:07:28 +0200
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.
> - If the subchannel is not enabled retry a predefined retries count.
>
> This tests the MSCH instruction to enable a channel succesfuly.
> This is NOT a routine to really enable the channel, no retry is done,
> in case of error, a report is made.
Hm... so you retry if the subchannel is not enabled after cc 0, but you
don't retry if the cc indicates busy/status pending? Makes sense, as we
don't expect the subchannel to be busy, but a more precise note in the
patch description would be good :)
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> s390x/css.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/s390x/css.c b/s390x/css.c
> index d7989d8..1b60a47 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -16,6 +16,7 @@
> #include <string.h>
> #include <interrupt.h>
> #include <asm/arch_def.h>
> +#include <asm/time.h>
>
> #include <css.h>
>
> @@ -65,11 +66,77 @@ out:
> scn, scn_found, dev_found);
> }
>
> +#define MAX_ENABLE_RETRIES 5
> +static void test_enable(void)
> +{
> + struct pmcw *pmcw = &schib.pmcw;
> + int retry_count = 0;
> + int cc;
> +
> + if (!test_device_sid) {
> + report_skip("No device");
> + return;
> + }
> +
> + /* Read the SCHIB for this subchannel */
> + cc = stsch(test_device_sid, &schib);
> + if (cc) {
> + report(0, "stsch cc=%d", cc);
> + return;
> + }
> +
> + if (pmcw->flags & PMCW_ENABLE) {
> + report(1, "stsch: sch %08x already enabled", test_device_sid);
> + return;
> + }
> +
> +retry:
> + /* Update the SCHIB to enable the channel */
> + pmcw->flags |= PMCW_ENABLE;
> +
> + /* Tell the CSS we want to modify the subchannel */
> + cc = msch(test_device_sid, &schib);
> + if (cc) {
> + /*
> + * If the subchannel is status pending or
> + * if a function is in progress,
> + * we consider both cases as errors.
Could also be cc 3, but that would be even more weird. Just logging the
cc seems fine, though.
> + */
> + report(0, "msch cc=%d", cc);
> + return;
> + }
> +
> + /*
> + * Read the SCHIB again to verify the enablement
> + */
> + cc = stsch(test_device_sid, &schib);
> + if (cc) {
> + report(0, "stsch cc=%d", cc);
> + return;
> + }
> +
> + if (pmcw->flags & PMCW_ENABLE) {
> + report(1, "msch: sch %08x enabled after %d retries",
> + test_device_sid, retry_count);
> + return;
> + }
> +
> + if (retry_count++ < MAX_ENABLE_RETRIES) {
> + mdelay(10); /* the hardware was not ready, let it some time */
s/let/give/
> + goto retry;
> + }
> +
> + report(0,
> + "msch: enabling sch %08x failed after %d retries. pmcw: %x",
> + test_device_sid, retry_count, pmcw->flags);
> +}
> +
> static struct {
> const char *name;
> void (*func)(void);
> } tests[] = {
> { "enumerate (stsch)", test_enumerate },
> + { "enable (msch)", test_enable },
> { NULL, NULL }
> };
>
Otherwise,
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test
2020-05-27 9:42 ` Cornelia Huck
@ 2020-06-04 12:46 ` Pierre Morel
2020-06-04 13:29 ` Cornelia Huck
0 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-06-04 12:46 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth
On 2020-05-27 11:42, Cornelia Huck wrote:
> On Mon, 18 May 2020 18:07:28 +0200
> 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.
>> - If the subchannel is not enabled retry a predefined retries count.
>>
>> This tests the MSCH instruction to enable a channel succesfuly.
>> This is NOT a routine to really enable the channel, no retry is done,
>> in case of error, a report is made.
>
> Hm... so you retry if the subchannel is not enabled after cc 0, but you
> don't retry if the cc indicates busy/status pending? Makes sense, as we
> don't expect the subchannel to be busy, but a more precise note in the
> patch description would be good :)
OK, I add something like
"
- If the command succeed but subchannel is not enabled retry a
predefined retries count.
- If the command fails, report the failure and do not retry, even
if cc indicates a busy/status as we do not expect this.
"
>
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> s390x/css.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 67 insertions(+)
>>
>> diff --git a/s390x/css.c b/s390x/css.c
>> index d7989d8..1b60a47 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -16,6 +16,7 @@
>> #include <string.h>
>> #include <interrupt.h>
>> #include <asm/arch_def.h>
>> +#include <asm/time.h>
>>
>> #include <css.h>
>>
>> @@ -65,11 +66,77 @@ out:
>> scn, scn_found, dev_found);
>> }
>>
>> +#define MAX_ENABLE_RETRIES 5
>> +static void test_enable(void)
>> +{
>> + struct pmcw *pmcw = &schib.pmcw;
>> + int retry_count = 0;
>> + int cc;
>> +
>> + if (!test_device_sid) {
>> + report_skip("No device");
>> + return;
>> + }
>> +
>> + /* Read the SCHIB for this subchannel */
>> + cc = stsch(test_device_sid, &schib);
>> + if (cc) {
>> + report(0, "stsch cc=%d", cc);
>> + return;
>> + }
>> +
>> + if (pmcw->flags & PMCW_ENABLE) {
>> + report(1, "stsch: sch %08x already enabled", test_device_sid);
>> + return;
>> + }
>> +
>> +retry:
>> + /* Update the SCHIB to enable the channel */
>> + pmcw->flags |= PMCW_ENABLE;
>> +
>> + /* Tell the CSS we want to modify the subchannel */
>> + cc = msch(test_device_sid, &schib);
>> + if (cc) {
>> + /*
>> + * If the subchannel is status pending or
>> + * if a function is in progress,
>> + * we consider both cases as errors.
>
> Could also be cc 3, but that would be even more weird. Just logging the
> cc seems fine, though.
>
>> + */
>> + report(0, "msch cc=%d", cc);
>> + return;
>> + }
>> +
>> + /*
>> + * Read the SCHIB again to verify the enablement
>> + */
>> + cc = stsch(test_device_sid, &schib);
>> + if (cc) {
>> + report(0, "stsch cc=%d", cc);
>> + return;
>> + }
>> +
>> + if (pmcw->flags & PMCW_ENABLE) {
>> + report(1, "msch: sch %08x enabled after %d retries",
>> + test_device_sid, retry_count);
>> + return;
>> + }
>> +
>> + if (retry_count++ < MAX_ENABLE_RETRIES) {
>> + mdelay(10); /* the hardware was not ready, let it some time */
>
> s/let/give/
>
>> + goto retry;
>> + }
>> +
>> + report(0,
>> + "msch: enabling sch %08x failed after %d retries. pmcw: %x",
>> + test_device_sid, retry_count, pmcw->flags);
>> +}
>> +
>> static struct {
>> const char *name;
>> void (*func)(void);
>> } tests[] = {
>> { "enumerate (stsch)", test_enumerate },
>> + { "enable (msch)", test_enable },
>> { NULL, NULL }
>> };
>>
>
> Otherwise,
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>
Thanks a lot.
I make the corrections.
regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test
2020-06-04 12:46 ` Pierre Morel
@ 2020-06-04 13:29 ` Cornelia Huck
2020-06-05 7:37 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Cornelia Huck @ 2020-06-04 13:29 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth
On Thu, 4 Jun 2020 14:46:05 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> On 2020-05-27 11:42, Cornelia Huck wrote:
> > On Mon, 18 May 2020 18:07:28 +0200
> > 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.
> >> - If the subchannel is not enabled retry a predefined retries count.
> >>
> >> This tests the MSCH instruction to enable a channel succesfuly.
> >> This is NOT a routine to really enable the channel, no retry is done,
> >> in case of error, a report is made.
> >
> > Hm... so you retry if the subchannel is not enabled after cc 0, but you
> > don't retry if the cc indicates busy/status pending? Makes sense, as we
> > don't expect the subchannel to be busy, but a more precise note in the
> > patch description would be good :)
>
> OK, I add something like
> "
> - If the command succeed but subchannel is not enabled retry a
s/succeed/succeeds/ :)
> predefined retries count.
> - If the command fails, report the failure and do not retry, even
> if cc indicates a busy/status as we do not expect this.
"indicates busy/status pending" ?
> "
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test
2020-06-04 13:29 ` Cornelia Huck
@ 2020-06-05 7:37 ` Pierre Morel
2020-06-05 8:24 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-06-05 7:37 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth
On 2020-06-04 15:29, Cornelia Huck wrote:
> On Thu, 4 Jun 2020 14:46:05 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> On 2020-05-27 11:42, Cornelia Huck wrote:
>>> On Mon, 18 May 2020 18:07:28 +0200
>>> 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.
>>>> - If the subchannel is not enabled retry a predefined retries count.
>>>>
>>>> This tests the MSCH instruction to enable a channel succesfuly.
>>>> This is NOT a routine to really enable the channel, no retry is done,
>>>> in case of error, a report is made.
>>>
>>> Hm... so you retry if the subchannel is not enabled after cc 0, but you
>>> don't retry if the cc indicates busy/status pending? Makes sense, as we
>>> don't expect the subchannel to be busy, but a more precise note in the
>>> patch description would be good :)
>>
>> OK, I add something like
>> "
>> - If the command succeed but subchannel is not enabled retry a
>
> s/succeed/succeeds/ :)
>
>> predefined retries count.
>> - If the command fails, report the failure and do not retry, even
>> if cc indicates a busy/status as we do not expect this.
>
> "indicates busy/status pending" ?
>
>> "
>
done, thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test
2020-06-05 7:37 ` Pierre Morel
@ 2020-06-05 8:24 ` Pierre Morel
2020-06-05 9:00 ` Cornelia Huck
0 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-06-05 8:24 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth
Hi Connie,
for the next series, I will move more code of the css.c to the css_lib.c
to be able to reuse it for other tests.
One of your suggestions IIRC.
So I will not be able to keep your RB until you have a look at the changes.
I will keep the same algorithms but still...
regards,
Pierre
On 2020-06-05 09:37, Pierre Morel wrote:
>
>
> On 2020-06-04 15:29, Cornelia Huck wrote:
>> On Thu, 4 Jun 2020 14:46:05 +0200
>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>
>>> On 2020-05-27 11:42, Cornelia Huck wrote:
>>>> On Mon, 18 May 2020 18:07:28 +0200
>>>> 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.
>>>>> - If the subchannel is not enabled retry a predefined retries count.
>>>>>
>>>>> This tests the MSCH instruction to enable a channel succesfuly.
>>>>> This is NOT a routine to really enable the channel, no retry is done,
>>>>> in case of error, a report is made.
>>>>
>>>> Hm... so you retry if the subchannel is not enabled after cc 0, but you
>>>> don't retry if the cc indicates busy/status pending? Makes sense, as we
>>>> don't expect the subchannel to be busy, but a more precise note in the
>>>> patch description would be good :)
>>>
>>> OK, I add something like
>>> "
>>> - If the command succeed but subchannel is not enabled retry a
>>
>> s/succeed/succeeds/ :)
>>
>>> predefined retries count.
>>> - If the command fails, report the failure and do not retry, even
>>> if cc indicates a busy/status as we do not expect this.
>>
>> "indicates busy/status pending" ?
>>
>>> "
>>
> done, thanks,
>
> Pierre
>
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test
2020-06-05 8:24 ` Pierre Morel
@ 2020-06-05 9:00 ` Cornelia Huck
0 siblings, 0 replies; 48+ messages in thread
From: Cornelia Huck @ 2020-06-05 9:00 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth
On Fri, 5 Jun 2020 10:24:56 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> Hi Connie,
>
> for the next series, I will move more code of the css.c to the css_lib.c
> to be able to reuse it for other tests.
> One of your suggestions IIRC.
> So I will not be able to keep your RB until you have a look at the changes.
> I will keep the same algorithms but still...
np, I'll look at it again.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (8 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 09/12] s390x: css: msch, enable test Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-26 10:42 ` Janosch Frank
2020-05-27 9:45 ` Cornelia Huck
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 11/12] s390x: css: ssch/tsch with sense and interrupt Pierre Morel
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 12/12] s390x: css: ping pong Pierre Morel
11 siblings, 2 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
Allow the program to wait for an interrupt.
The interrupt handler is in charge to remove the WAIT bit
when it finished handling the interrupt.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
lib/s390x/asm/arch_def.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 54ffd0b..f200e28 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -10,9 +10,11 @@
#ifndef _ASM_S390X_ARCH_DEF_H_
#define _ASM_S390X_ARCH_DEF_H_
+#define PSW_MASK_IO 0x0200000000000000UL
#define PSW_MASK_EXT 0x0100000000000000UL
#define PSW_MASK_DAT 0x0400000000000000UL
#define PSW_MASK_SHORT_PSW 0x0008000000000000UL
+#define PSW_MASK_WAIT 0x0002000000000000UL
#define PSW_MASK_PSTATE 0x0001000000000000UL
#define PSW_MASK_BA 0x0000000080000000UL
#define PSW_MASK_EA 0x0000000100000000UL
@@ -253,6 +255,18 @@ static inline void load_psw_mask(uint64_t mask)
: "+r" (tmp) : "a" (&psw) : "memory", "cc" );
}
+static inline void wait_for_interrupt(uint64_t irq_mask)
+{
+ uint64_t psw_mask = extract_psw_mask();
+
+ load_psw_mask(psw_mask | irq_mask | PSW_MASK_WAIT);
+ /*
+ * After being woken and having processed the interrupt, let's restore
+ * the PSW mask.
+ */
+ load_psw_mask(psw_mask);
+}
+
static inline void enter_pstate(void)
{
uint64_t mask;
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt Pierre Morel
@ 2020-05-26 10:42 ` Janosch Frank
2020-05-26 11:40 ` Pierre Morel
2020-05-27 9:45 ` Cornelia Huck
1 sibling, 1 reply; 48+ messages in thread
From: Janosch Frank @ 2020-05-26 10:42 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: linux-s390, david, thuth, cohuck
[-- Attachment #1.1: Type: text/plain, Size: 1606 bytes --]
On 5/18/20 6:07 PM, Pierre Morel wrote:
> Allow the program to wait for an interrupt.
>
> The interrupt handler is in charge to remove the WAIT bit
> when it finished handling the interrupt.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/asm/arch_def.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 54ffd0b..f200e28 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -10,9 +10,11 @@
> #ifndef _ASM_S390X_ARCH_DEF_H_
> #define _ASM_S390X_ARCH_DEF_H_
>
> +#define PSW_MASK_IO 0x0200000000000000UL
> #define PSW_MASK_EXT 0x0100000000000000UL
> #define PSW_MASK_DAT 0x0400000000000000UL
> #define PSW_MASK_SHORT_PSW 0x0008000000000000UL
> +#define PSW_MASK_WAIT 0x0002000000000000UL
> #define PSW_MASK_PSTATE 0x0001000000000000UL
> #define PSW_MASK_BA 0x0000000080000000UL
> #define PSW_MASK_EA 0x0000000100000000UL
> @@ -253,6 +255,18 @@ static inline void load_psw_mask(uint64_t mask)
> : "+r" (tmp) : "a" (&psw) : "memory", "cc" );
> }
>
> +static inline void wait_for_interrupt(uint64_t irq_mask)
> +{
> + uint64_t psw_mask = extract_psw_mask();
> +
> + load_psw_mask(psw_mask | irq_mask | PSW_MASK_WAIT);
> + /*
> + * After being woken and having processed the interrupt, let's restore
> + * the PSW mask.
> + */
> + load_psw_mask(psw_mask);
> +}
> +
> static inline void enter_pstate(void)
> {
> uint64_t mask;
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt
2020-05-26 10:42 ` Janosch Frank
@ 2020-05-26 11:40 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-26 11:40 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, david, thuth, cohuck
On 2020-05-26 12:42, Janosch Frank wrote:
> On 5/18/20 6:07 PM, Pierre Morel wrote:
>> Allow the program to wait for an interrupt.
>>
>> The interrupt handler is in charge to remove the WAIT bit
>> when it finished handling the interrupt.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt Pierre Morel
2020-05-26 10:42 ` Janosch Frank
@ 2020-05-27 9:45 ` Cornelia Huck
2020-06-04 12:47 ` Pierre Morel
1 sibling, 1 reply; 48+ messages in thread
From: Cornelia Huck @ 2020-05-27 9:45 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth
On Mon, 18 May 2020 18:07:29 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> Allow the program to wait for an interrupt.
>
> The interrupt handler is in charge to remove the WAIT bit
> when it finished handling the interrupt.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> lib/s390x/asm/arch_def.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt
2020-05-27 9:45 ` Cornelia Huck
@ 2020-06-04 12:47 ` Pierre Morel
0 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-06-04 12:47 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth
On 2020-05-27 11:45, Cornelia Huck wrote:
> On Mon, 18 May 2020 18:07:29 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> Allow the program to wait for an interrupt.
>>
>> The interrupt handler is in charge to remove the WAIT bit
>> when it finished handling the interrupt.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/asm/arch_def.h | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 11/12] s390x: css: ssch/tsch with sense and interrupt
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (9 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 10/12] s390x: define function to wait for interrupt Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
2020-05-27 10:09 ` Cornelia Huck
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 12/12] s390x: css: ping pong Pierre Morel
11 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
We add a new css_lib file to contain the I/O functions we may
share with different tests.
First function is the subchannel_enable() function.
When a channel is enabled we can start a SENSE_ID command using
the SSCH instruction to recognize the control unit and device.
This tests the success of SSCH, the I/O interruption and the TSCH
instructions.
The test expects a device with a control unit type of 0xC0CA as the
first subchannel of the CSS.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
lib/s390x/css.h | 20 ++++++
lib/s390x/css_lib.c | 55 +++++++++++++++++
s390x/Makefile | 1 +
s390x/css.c | 145 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 221 insertions(+)
create mode 100644 lib/s390x/css_lib.c
diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index e0d3a98..f4df7bc 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -99,6 +99,19 @@ struct irb {
uint32_t emw[8];
} __attribute__ ((aligned(4)));
+#define CCW_CMD_SENSE_ID 0xe4
+#define PONG_CU 0xc0ca
+struct senseid {
+ /* common part */
+ uint8_t reserved; /* always 0x'FF' */
+ uint16_t cu_type; /* control unit type */
+ uint8_t cu_model; /* control unit model */
+ uint16_t dev_type; /* device type */
+ uint8_t dev_model; /* device model */
+ uint8_t unused; /* padding byte */
+ uint8_t padding[256 - 10]; /* Extra padding for CCW */
+} __attribute__ ((aligned(4))) __attribute__ ((packed));
+
/* CSS low level access functions */
static inline int ssch(unsigned long schid, struct orb *addr)
@@ -256,4 +269,11 @@ static inline struct ccw *dump_ccw(struct ccw *cp)
return NULL;
}
#endif /* DEBUG_CSS */
+
+#define SID_ONE 0x00010000
+
+/* Library functions */
+int enable_subchannel(unsigned int sid);
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw);
+
#endif
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
new file mode 100644
index 0000000..80b9359
--- /dev/null
+++ b/lib/s390x/css_lib.c
@@ -0,0 +1,55 @@
+/*
+ * Channel subsystem library functions
+ *
+ * Copyright (c) 2020 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 <stdint.h>
+#include <stddef.h>
+#include <css.h>
+
+int enable_subchannel(unsigned int sid)
+{
+ struct schib schib;
+ struct pmcw *pmcw = &schib.pmcw;
+ int try_count = 5;
+ int cc;
+
+ if (!(sid & SID_ONE))
+ return -1;
+
+ cc = stsch(sid, &schib);
+ if (cc)
+ return -cc;
+
+ do {
+ pmcw->flags |= PMCW_ENABLE;
+
+ cc = msch(sid, &schib);
+ if (cc)
+ return -cc;
+
+ cc = stsch(sid, &schib);
+ if (cc)
+ return -cc;
+
+ } while (!(pmcw->flags & PMCW_ENABLE) && --try_count);
+
+ return try_count;
+}
+
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw)
+{
+ struct orb orb;
+
+ orb.intparm = sid;
+ orb.ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
+ orb.cpa = (unsigned int) (unsigned long)ccw;
+
+ return ssch(sid, &orb);
+}
diff --git a/s390x/Makefile b/s390x/Makefile
index baebf18..166cb5c 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -53,6 +53,7 @@ cflatobjs += lib/s390x/interrupt.o
cflatobjs += lib/s390x/mmu.o
cflatobjs += lib/s390x/smp.o
cflatobjs += lib/s390x/css_dump.o
+cflatobjs += lib/s390x/css_lib.o
OBJDIRS += lib/s390x
diff --git a/s390x/css.c b/s390x/css.c
index 1b60a47..c94a916 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -20,8 +20,24 @@
#include <css.h>
+#define PONG_CU_TYPE 0xc0ca
+
+struct lowcore *lowcore = (void *)0x0;
+
static struct schib schib;
static int test_device_sid;
+#define NUM_CCW 100
+static struct ccw1 ccw[NUM_CCW];
+static struct irb irb;
+static struct senseid senseid;
+
+static void set_io_irq_subclass_mask(uint64_t const new_mask)
+{
+ asm volatile (
+ "lctlg %%c6, %%c6, %[source]\n"
+ : /* No outputs */
+ : [source] "R" (new_mask));
+}
static void test_enumerate(void)
{
@@ -131,12 +147,140 @@ retry:
test_device_sid, retry_count, pmcw->flags);
}
+static void enable_io_isc(void)
+{
+ /* Let's enable all ISCs for I/O interrupt */
+ set_io_irq_subclass_mask(0x00000000ff000000);
+}
+
+static void irq_io(void)
+{
+ int ret = 0;
+ char *flags;
+ int sid;
+
+ report_prefix_push("Interrupt");
+ /* Lowlevel set the SID as interrupt parameter. */
+ if (lowcore->io_int_param != test_device_sid) {
+ report(0,
+ "Bad io_int_param: %x expected %x",
+ lowcore->io_int_param, test_device_sid);
+ goto pop;
+ }
+ report_prefix_pop();
+
+ report_prefix_push("tsch");
+ sid = lowcore->subsys_id_word;
+ ret = tsch(sid, &irb);
+ switch (ret) {
+ case 1:
+ dump_irb(&irb);
+ flags = dump_scsw_flags(irb.scsw.ctrl);
+ report(0,
+ "I/O interrupt, CC 1 but tsch reporting sch %08x as not status pending: %s",
+ sid, flags);
+ break;
+ case 2:
+ report(0, "tsch returns unexpected CC 2");
+ break;
+ case 3:
+ report(0, "tsch reporting sch %08x as not operational", sid);
+ break;
+ case 0:
+ /* Stay humble on success */
+ break;
+ }
+pop:
+ report_prefix_pop();
+ lowcore->io_old_psw.mask &= ~PSW_MASK_WAIT;
+}
+
+static int start_subchannel(int code, void *data, int count,
+ unsigned char flags)
+{
+ int ret;
+
+ report_prefix_push("start_senseid");
+ /* Build the CCW chain with a single CCW */
+ ccw[0].code = code;
+ ccw[0].flags = flags; /* No flags need to be set */
+ ccw[0].count = count;
+ ccw[0].data_address = (int)(unsigned long)data;
+
+ ret = start_ccw1_chain(test_device_sid, ccw);
+ if (ret) {
+ report(0, "start_ccw_chain failed ret=%d", ret);
+ report_prefix_pop();
+ return 0;
+ }
+ report_prefix_pop();
+ return 1;
+}
+
+/*
+ * test_sense
+ * Pre-requisits:
+ * - We need the QEMU PONG device as the first recognized
+ * device by the enumeration.
+ * - ./s390x-run s390x/css.elf -device ccw-pong,cu_type=0xc0ca
+ */
+static void test_sense(void)
+{
+ int ret;
+
+ if (!test_device_sid) {
+ report_skip("No device");
+ return;
+ }
+
+ ret = enable_subchannel(test_device_sid);
+ if (ret < 0) {
+ report(0,
+ "Could not enable the subchannel: %08x",
+ test_device_sid);
+ return;
+ }
+
+ ret = register_io_int_func(irq_io);
+ if (ret) {
+ report(0, "Could not register IRQ handler");
+ goto unreg_cb;
+ }
+
+ lowcore->io_int_param = 0;
+
+ ret = start_subchannel(CCW_CMD_SENSE_ID, &senseid, sizeof(senseid),
+ CCW_F_SLI);
+ if (!ret) {
+ report(0, "ssch failed for SENSE ID on sch %08x",
+ test_device_sid);
+ goto unreg_cb;
+ }
+
+ wait_for_interrupt(PSW_MASK_IO);
+
+ if (lowcore->io_int_param != test_device_sid)
+ goto unreg_cb;
+
+ report_info("reserved %02x cu_type %04x cu_model %02x dev_type %04x dev_model %02x",
+ senseid.reserved, senseid.cu_type, senseid.cu_model,
+ senseid.dev_type, senseid.dev_model);
+
+ report((senseid.cu_type == PONG_CU),
+ "cu_type: expect 0x%04x got 0x%04x",
+ PONG_CU_TYPE, senseid.cu_type);
+
+unreg_cb:
+ unregister_io_int_func(irq_io);
+}
+
static struct {
const char *name;
void (*func)(void);
} tests[] = {
{ "enumerate (stsch)", test_enumerate },
{ "enable (msch)", test_enable },
+ { "sense (ssch/tsch)", test_sense },
{ NULL, NULL }
};
@@ -145,6 +289,7 @@ int main(int argc, char *argv[])
int i;
report_prefix_push("Channel Subsystem");
+ enable_io_isc();
for (i = 0; tests[i].name; i++) {
report_prefix_push(tests[i].name);
tests[i].func();
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 11/12] s390x: css: ssch/tsch with sense and interrupt
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 11/12] s390x: css: ssch/tsch with sense and interrupt Pierre Morel
@ 2020-05-27 10:09 ` Cornelia Huck
2020-06-05 7:37 ` Pierre Morel
0 siblings, 1 reply; 48+ messages in thread
From: Cornelia Huck @ 2020-05-27 10:09 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth
On Mon, 18 May 2020 18:07:30 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> We add a new css_lib file to contain the I/O functions we may
> share with different tests.
> First function is the subchannel_enable() function.
>
> When a channel is enabled we can start a SENSE_ID command using
> the SSCH instruction to recognize the control unit and device.
>
> This tests the success of SSCH, the I/O interruption and the TSCH
> instructions.
>
> The test expects a device with a control unit type of 0xC0CA as the
> first subchannel of the CSS.
It might make sense to extend this to be able to check for any expected
type (e.g. 0x3832, should my suggestion to split css tests and css-pong
tests make sense.)
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> lib/s390x/css.h | 20 ++++++
> lib/s390x/css_lib.c | 55 +++++++++++++++++
> s390x/Makefile | 1 +
> s390x/css.c | 145 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 221 insertions(+)
> create mode 100644 lib/s390x/css_lib.c
(...)
> +int enable_subchannel(unsigned int sid)
> +{
> + struct schib schib;
> + struct pmcw *pmcw = &schib.pmcw;
> + int try_count = 5;
> + int cc;
> +
> + if (!(sid & SID_ONE))
> + return -1;
Hm... this error is indistinguishable for the caller from a cc 1 for
the msch. Use something else (as this is a coding error)?
> +
> + cc = stsch(sid, &schib);
> + if (cc)
> + return -cc;
> +
> + do {
> + pmcw->flags |= PMCW_ENABLE;
> +
> + cc = msch(sid, &schib);
> + if (cc)
> + return -cc;
> +
> + cc = stsch(sid, &schib);
> + if (cc)
> + return -cc;
> +
> + } while (!(pmcw->flags & PMCW_ENABLE) && --try_count);
> +
> + return try_count;
How useful is that information for the caller? I don't see the code
below making use of it.
> +}
> +
> +int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw)
> +{
> + struct orb orb;
> +
> + orb.intparm = sid;
Just an idea: If you use something else here (maybe the cpa), and set
the intparm to the sid in msch, you can test something else: Does msch
properly set the intparm, and is that intparm overwritten by a
successful ssch, until the next ssch or msch comes around?
> + orb.ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
> + orb.cpa = (unsigned int) (unsigned long)ccw;
Use a struct initializer, so that unset fields are 0?
> +
> + return ssch(sid, &orb);
> +}
(...)
> +/*
> + * test_sense
> + * Pre-requisits:
> + * - We need the QEMU PONG device as the first recognized
> + * device by the enumeration.
> + * - ./s390x-run s390x/css.elf -device ccw-pong,cu_type=0xc0ca
> + */
> +static void test_sense(void)
> +{
> + int ret;
> +
> + if (!test_device_sid) {
> + report_skip("No device");
> + return;
> + }
> +
> + ret = enable_subchannel(test_device_sid);
> + if (ret < 0) {
> + report(0,
> + "Could not enable the subchannel: %08x",
> + test_device_sid);
> + return;
> + }
> +
> + ret = register_io_int_func(irq_io);
> + if (ret) {
> + report(0, "Could not register IRQ handler");
> + goto unreg_cb;
> + }
> +
> + lowcore->io_int_param = 0;
> +
> + ret = start_subchannel(CCW_CMD_SENSE_ID, &senseid, sizeof(senseid),
> + CCW_F_SLI);
Clear senseid, before actually sending the program?
> + if (!ret) {
> + report(0, "ssch failed for SENSE ID on sch %08x",
> + test_device_sid);
> + goto unreg_cb;
> + }
> +
> + wait_for_interrupt(PSW_MASK_IO);
> +
> + if (lowcore->io_int_param != test_device_sid)
> + goto unreg_cb;
> +
> + report_info("reserved %02x cu_type %04x cu_model %02x dev_type %04x dev_model %02x",
> + senseid.reserved, senseid.cu_type, senseid.cu_model,
> + senseid.dev_type, senseid.dev_model);
> +
I'd also recommend checking that senseid.reserved is indeed 0xff -- in
combination with senseid clearing before the ssch, that ensures that
the senseid structure has actually been written to and is not pure
garbage. (It's also a cu type agnostic test :)
It also might make sense to check how much data you actually got, as
you set SLI.
> + report((senseid.cu_type == PONG_CU),
> + "cu_type: expect 0x%04x got 0x%04x",
> + PONG_CU_TYPE, senseid.cu_type);
> +
> +unreg_cb:
> + unregister_io_int_func(irq_io);
> +}
> +
> static struct {
> const char *name;
> void (*func)(void);
> } tests[] = {
> { "enumerate (stsch)", test_enumerate },
> { "enable (msch)", test_enable },
> + { "sense (ssch/tsch)", test_sense },
> { NULL, NULL }
> };
>
> @@ -145,6 +289,7 @@ int main(int argc, char *argv[])
> int i;
>
> report_prefix_push("Channel Subsystem");
> + enable_io_isc();
> for (i = 0; tests[i].name; i++) {
> report_prefix_push(tests[i].name);
> tests[i].func();
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 11/12] s390x: css: ssch/tsch with sense and interrupt
2020-05-27 10:09 ` Cornelia Huck
@ 2020-06-05 7:37 ` Pierre Morel
2020-06-05 9:02 ` Cornelia Huck
0 siblings, 1 reply; 48+ messages in thread
From: Pierre Morel @ 2020-06-05 7:37 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth
On 2020-05-27 12:09, Cornelia Huck wrote:
> On Mon, 18 May 2020 18:07:30 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> We add a new css_lib file to contain the I/O functions we may
>> share with different tests.
>> First function is the subchannel_enable() function.
>>
>> When a channel is enabled we can start a SENSE_ID command using
>> the SSCH instruction to recognize the control unit and device.
>>
>> This tests the success of SSCH, the I/O interruption and the TSCH
>> instructions.
>>
>> The test expects a device with a control unit type of 0xC0CA as the
>> first subchannel of the CSS.
>
> It might make sense to extend this to be able to check for any expected
> type (e.g. 0x3832, should my suggestion to split css tests and css-pong
> tests make sense.)
right.
>
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/css.h | 20 ++++++
>> lib/s390x/css_lib.c | 55 +++++++++++++++++
>> s390x/Makefile | 1 +
>> s390x/css.c | 145 ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 221 insertions(+)
>> create mode 100644 lib/s390x/css_lib.c
>
> (...)
>
>> +int enable_subchannel(unsigned int sid)
>> +{
>> + struct schib schib;
>> + struct pmcw *pmcw = &schib.pmcw;
>> + int try_count = 5;
>> + int cc;
>> +
>> + if (!(sid & SID_ONE))
>> + return -1;
>
> Hm... this error is indistinguishable for the caller from a cc 1 for
> the msch. Use something else (as this is a coding error)?
right it is a coding error -> assert ?
>
>> +
>> + cc = stsch(sid, &schib);
>> + if (cc)
>> + return -cc;
>> +
>> + do {
>> + pmcw->flags |= PMCW_ENABLE;
>> +
>> + cc = msch(sid, &schib);
>> + if (cc)
>> + return -cc;
>> +
>> + cc = stsch(sid, &schib);
>> + if (cc)
>> + return -cc;
>> +
>> + } while (!(pmcw->flags & PMCW_ENABLE) && --try_count);
>> +
>> + return try_count;
>
> How useful is that information for the caller? I don't see the code
> below making use of it.
right,
I will change the fail cases to a report_info and return 0 in case of
success.
>
>> +}
>> +
>> +int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw)
>> +{
>> + struct orb orb;
>> +
>> + orb.intparm = sid;
>
> Just an idea: If you use something else here (maybe the cpa), and set
> the intparm to the sid in msch, you can test something else: Does msch
> properly set the intparm, and is that intparm overwritten by a
> successful ssch, until the next ssch or msch comes around?
good idea.
Using cpa is all what we need at the current development.
>
>> + orb.ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
>> + orb.cpa = (unsigned int) (unsigned long)ccw;
>
> Use a struct initializer, so that unset fields are 0?
not only more beautifull but effective. thanks!
>
>> +
>> + return ssch(sid, &orb);
>> +}
>
> (...)
>
>> +/*
>> + * test_sense
>> + * Pre-requisits:
>> + * - We need the QEMU PONG device as the first recognized
>> + * device by the enumeration.
>> + * - ./s390x-run s390x/css.elf -device ccw-pong,cu_type=0xc0ca
>> + */
>> +static void test_sense(void)
>> +{
>> + int ret;
>> +
>> + if (!test_device_sid) {
>> + report_skip("No device");
>> + return;
>> + }
>> +
>> + ret = enable_subchannel(test_device_sid);
>> + if (ret < 0) {
>> + report(0,
>> + "Could not enable the subchannel: %08x",
>> + test_device_sid);
>> + return;
>> + }
>> +
>> + ret = register_io_int_func(irq_io);
>> + if (ret) {
>> + report(0, "Could not register IRQ handler");
>> + goto unreg_cb;
>> + }
>> +
>> + lowcore->io_int_param = 0;
>> +
>> + ret = start_subchannel(CCW_CMD_SENSE_ID, &senseid, sizeof(senseid),
>> + CCW_F_SLI);
>
> Clear senseid, before actually sending the program?
yes.
>
>> + if (!ret) {
>> + report(0, "ssch failed for SENSE ID on sch %08x",
>> + test_device_sid);
>> + goto unreg_cb;
>> + }
>> +
>> + wait_for_interrupt(PSW_MASK_IO);
>> +
>> + if (lowcore->io_int_param != test_device_sid)
>> + goto unreg_cb;
>> +
>> + report_info("reserved %02x cu_type %04x cu_model %02x dev_type %04x dev_model %02x",
>> + senseid.reserved, senseid.cu_type, senseid.cu_model,
>> + senseid.dev_type, senseid.dev_model);
>> +
>
> I'd also recommend checking that senseid.reserved is indeed 0xff -- in
> combination with senseid clearing before the ssch, that ensures that
> the senseid structure has actually been written to and is not pure
> garbage. (It's also a cu type agnostic test :)
good idea, thanks.
>
> It also might make sense to check how much data you actually got, as
> you set SLI.
>
>
Yes, will do.
Thanks for the comments, I make the changes for the next revision.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [kvm-unit-tests PATCH v7 11/12] s390x: css: ssch/tsch with sense and interrupt
2020-06-05 7:37 ` Pierre Morel
@ 2020-06-05 9:02 ` Cornelia Huck
0 siblings, 0 replies; 48+ messages in thread
From: Cornelia Huck @ 2020-06-05 9:02 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth
On Fri, 5 Jun 2020 09:37:39 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> On 2020-05-27 12:09, Cornelia Huck wrote:
> > On Mon, 18 May 2020 18:07:30 +0200
> > Pierre Morel <pmorel@linux.ibm.com> wrote:
> >
> >> We add a new css_lib file to contain the I/O functions we may
> >> share with different tests.
> >> First function is the subchannel_enable() function.
> >>
> >> When a channel is enabled we can start a SENSE_ID command using
> >> the SSCH instruction to recognize the control unit and device.
> >>
> >> This tests the success of SSCH, the I/O interruption and the TSCH
> >> instructions.
> >>
> >> The test expects a device with a control unit type of 0xC0CA as the
> >> first subchannel of the CSS.
> >
> > It might make sense to extend this to be able to check for any expected
> > type (e.g. 0x3832, should my suggestion to split css tests and css-pong
> > tests make sense.)
>
> right.
>
> >
> >>
> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> >> ---
> >> lib/s390x/css.h | 20 ++++++
> >> lib/s390x/css_lib.c | 55 +++++++++++++++++
> >> s390x/Makefile | 1 +
> >> s390x/css.c | 145 ++++++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 221 insertions(+)
> >> create mode 100644 lib/s390x/css_lib.c
> >
> > (...)
> >
> >> +int enable_subchannel(unsigned int sid)
> >> +{
> >> + struct schib schib;
> >> + struct pmcw *pmcw = &schib.pmcw;
> >> + int try_count = 5;
> >> + int cc;
> >> +
> >> + if (!(sid & SID_ONE))
> >> + return -1;
> >
> > Hm... this error is indistinguishable for the caller from a cc 1 for
> > the msch. Use something else (as this is a coding error)?
>
> right it is a coding error -> assert ?
Sounds good to me.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [kvm-unit-tests PATCH v7 12/12] s390x: css: ping pong
2020-05-18 16:07 [kvm-unit-tests PATCH v7 00/12] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (10 preceding siblings ...)
2020-05-18 16:07 ` [kvm-unit-tests PATCH v7 11/12] s390x: css: ssch/tsch with sense and interrupt Pierre Morel
@ 2020-05-18 16:07 ` Pierre Morel
11 siblings, 0 replies; 48+ messages in thread
From: Pierre Morel @ 2020-05-18 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck
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 | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/s390x/css.c b/s390x/css.c
index c94a916..da80574 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -21,6 +21,12 @@
#include <css.h>
#define PONG_CU_TYPE 0xc0ca
+/* Channel Commands for PONG device */
+#define PONG_WRITE 0x21 /* Write */
+#define PONG_READ 0x22 /* Read buffer */
+
+#define BUFSZ 9
+static char buffer[BUFSZ];
struct lowcore *lowcore = (void *)0x0;
@@ -274,6 +280,53 @@ 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("Device is not a pong device.");
+ return;
+ }
+
+ result = register_io_int_func(irq_io);
+ if (result) {
+ report(0, "Could not register IRQ handler");
+ return;
+ }
+
+ while (cnt++ < max) {
+ snprintf(buffer, BUFSZ, "%08x\n", cnt);
+ success = start_subchannel(PONG_WRITE, buffer, BUFSZ, 0);
+ if (!success) {
+ report(0, "start_subchannel failed");
+ goto unreg_cb;
+ }
+
+ wait_for_interrupt(PSW_MASK_IO);
+
+ success = start_subchannel(PONG_READ, buffer, BUFSZ, 0);
+ if (!success) {
+ report(0, "start_subchannel failed");
+ goto unreg_cb;
+ }
+
+ wait_for_interrupt(PSW_MASK_IO);
+
+ result = atol(buffer);
+ if (result != (cnt + 1)) {
+ report(0, "Bad answer from pong: %08x - %08x",
+ cnt, result);
+ goto unreg_cb;
+ }
+ }
+ report(1, "ping-pong count 0x%08x", cnt);
+
+unreg_cb:
+ unregister_io_int_func(irq_io);
+}
+
static struct {
const char *name;
void (*func)(void);
@@ -281,6 +334,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.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread