* [kvm-unit-tests PATCH v11 1/9] s390x: saving regs for interrupts
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 2/9] s390x: I/O interrupt registration Pierre Morel
` (7 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
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>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Acked-by: Cornelia Huck <cohuck@redhat.com>
---
s390x/cstart64.S | 41 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index e084f13..4e51150 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 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 multiple 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 that we're 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:
@@ -185,9 +222,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] 22+ messages in thread* [kvm-unit-tests PATCH v11 2/9] s390x: I/O interrupt registration
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 1/9] s390x: saving regs for interrupts Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 3/9] s390x: export the clock get_clock_ms() utility Pierre Morel
` (6 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
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>
Acked-by: Cornelia Huck <cohuck@redhat.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..1973d26
--- /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] 22+ messages in thread* [kvm-unit-tests PATCH v11 3/9] s390x: export the clock get_clock_ms() utility
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 1/9] s390x: saving regs for interrupts Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 2/9] s390x: I/O interrupt registration Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 4/9] s390x: clock and delays calculations Pierre Morel
` (5 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
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..1791380
--- /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] 22+ messages in thread* [kvm-unit-tests PATCH v11 4/9] s390x: clock and delays calculations
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (2 preceding siblings ...)
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 3/9] s390x: export the clock get_clock_ms() utility Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 5/9] s390x: define function to wait for interrupt Pierre Morel
` (4 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
The hardware gives us a good definition of the microsecond,
let's keep this information and let the routine accessing
the hardware keep all the information and return microseconds.
Calculate delays in microseconds and take care about wrapping
around zero.
Define values with macros and use inlines to keep the
milliseconds interface.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
lib/s390x/asm/time.h | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h
index 1791380..7375aa2 100644
--- a/lib/s390x/asm/time.h
+++ b/lib/s390x/asm/time.h
@@ -13,14 +13,38 @@
#ifndef ASM_S390X_TIME_H
#define ASM_S390X_TIME_H
-static inline uint64_t get_clock_ms(void)
+#define STCK_SHIFT_US (63 - 51)
+#define STCK_MAX ((1UL << 52) - 1)
+
+static inline uint64_t get_clock_us(void)
{
uint64_t clk;
asm volatile(" stck %0 " : : "Q"(clk) : "memory");
- /* Bit 51 is incrememented each microsecond */
- return (clk >> (63 - 51)) / 1000;
+ return clk >> STCK_SHIFT_US;
+}
+
+static inline uint64_t get_clock_ms(void)
+{
+ return get_clock_us() / 1000;
+}
+
+static inline void udelay(unsigned long us)
+{
+ unsigned long startclk = get_clock_us();
+ unsigned long c;
+
+ do {
+ c = get_clock_us();
+ if (c < startclk)
+ c += STCK_MAX;
+ } while (c < startclk + us);
+}
+
+static inline void mdelay(unsigned long ms)
+{
+ udelay(ms * 1000);
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [kvm-unit-tests PATCH v11 5/9] s390x: define function to wait for interrupt
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (3 preceding siblings ...)
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 4/9] s390x: clock and delays calculations Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 6/9] s390x: Library resources for CSS tests Pierre Morel
` (3 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
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>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
lib/s390x/asm/arch_def.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 1b3bb0c..022a564 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -17,6 +17,7 @@ struct psw {
#define PSW_MASK_EXT 0x0100000000000000UL
#define PSW_MASK_DAT 0x0400000000000000UL
+#define PSW_MASK_WAIT 0x0002000000000000UL
#define PSW_MASK_PSTATE 0x0001000000000000UL
#define CR0_EXTM_SCLP 0x0000000000000200UL
@@ -246,6 +247,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] 22+ messages in thread* [kvm-unit-tests PATCH v11 6/9] s390x: Library resources for CSS tests
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (4 preceding siblings ...)
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 5/9] s390x: define function to wait for interrupt Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 7/9] s390x: css: stsch, enumeration test Pierre Morel
` (2 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
Provide some definitions and library routines that can be used by
tests targeting the channel subsystem.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Acked-by: Janosch Frank <frankja@de.ibm.com>
---
lib/s390x/css.h | 256 +++++++++++++++++++++++++++++++++++++++++++
lib/s390x/css_dump.c | 152 +++++++++++++++++++++++++
s390x/Makefile | 1 +
3 files changed, 409 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..0ddceb1
--- /dev/null
+++ b/lib/s390x/css.h
@@ -0,0 +1,256 @@
+/*
+ * CSS definitions
+ *
+ * Copyright IBM, Corp. 2020
+ * Author: 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 CSS_H
+#define CSS_H
+
+/* subchannel ID bit 16 must always be one */
+#define SCHID_ONE 0x00010000
+
+#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 {
+ uint8_t code;
+ uint8_t flags;
+ uint16_t count;
+ uint32_t data_address;
+} __attribute__ ((aligned(8)));
+
+#define ORB_CTRL_KEY 0xf0000000
+#define ORB_CTRL_SPND 0x08000000
+#define ORB_CTRL_STR 0x04000000
+#define ORB_CTRL_MOD 0x02000000
+#define ORB_CTRL_SYNC 0x01000000
+#define ORB_CTRL_FMT 0x00800000
+#define ORB_CTRL_PFCH 0x00400000
+#define ORB_CTRL_ISIC 0x00200000
+#define ORB_CTRL_ALCC 0x00100000
+#define ORB_CTRL_SSIC 0x00080000
+#define ORB_CTRL_CPTC 0x00040000
+#define ORB_CTRL_C64 0x00020000
+#define ORB_CTRL_I2K 0x00010000
+#define ORB_CTRL_LPM 0x0000ff00
+#define ORB_CTRL_ILS 0x00000080
+#define ORB_CTRL_MIDAW 0x00000040
+#define ORB_CTRL_ORBX 0x00000001
+
+#define ORB_LPM_DFLT 0x00008000
+
+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)
+ : "d" (reg1), "m" (*addr), "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);
+
+void dump_scsw(struct scsw *scsw);
+void dump_irb(struct irb *irbp);
+void dump_schib(struct schib *sch);
+struct ccw1 *dump_ccw(struct ccw1 *cp);
+void dump_irb(struct irb *irbp);
+void dump_pmcw(struct pmcw *p);
+void dump_orb(struct orb *op);
+
+int css_enumerate(void);
+#define MAX_ENABLE_RETRIES 5
+int css_enable(int schid);
+
+#endif
diff --git a/lib/s390x/css_dump.c b/lib/s390x/css_dump.c
new file mode 100644
index 0000000..1266f04
--- /dev/null
+++ b/lib/s390x/css_dump.c
@@ -0,0 +1,152 @@
+/*
+ * 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 <libcflat.h>
+#include <stdint.h>
+#include <string.h>
+
+#include <css.h>
+
+/*
+ * Try to have a more human representation of the SCSW flags:
+ * each letter in the two strings represents 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 to have a more human representation of the PMCW flags
+ * each letter in the string represents 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;
+}
+
+void dump_scsw(struct scsw *s)
+{
+ dump_scsw_flags(s->ctrl);
+ printf("scsw->flags: %s\n", scsw_line);
+ printf("scsw->addr : %08x\n", s->ccw_addr);
+ printf("scsw->devs : %02x\n", s->dev_stat);
+ printf("scsw->schs : %02x\n", s->sch_stat);
+ 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 ccw1 *dump_ccw(struct ccw1 *cp)
+{
+ printf("CCW: code: %02x flags: %02x count: %04x data: %08x\n", cp->code,
+ cp->flags, cp->count, cp->data_address);
+
+ if (cp->code == CCW_C_TIC)
+ return (struct ccw1 *)(long)cp->data_address;
+
+ return (cp->flags & CCW_F_CC) ? cp + 1 : NULL;
+}
+
+void dump_orb(struct orb *op)
+{
+ struct ccw1 *cp;
+
+ printf("ORB: intparm : %08x\n", op->intparm);
+ printf("ORB: ctrl : %08x\n", op->ctrl);
+ printf("ORB: prio : %08x\n", op->prio);
+ cp = (struct ccw1 *)(long) (op->cpa);
+ while (cp)
+ cp = dump_ccw(cp);
+}
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] 22+ messages in thread* [kvm-unit-tests PATCH v11 7/9] s390x: css: stsch, enumeration test
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (5 preceding siblings ...)
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 6/9] s390x: Library resources for CSS tests Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test Pierre Morel
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 9/9] s390x: css: ssch/tsch with sense and interrupt Pierre Morel
8 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
First step for testing the channel subsystem is to enumerate the css and
retrieve the css devices.
We currently don't enable multiple subchannel sets and therefore only
look in subchannel set 0
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>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
lib/s390x/css_lib.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
s390x/Makefile | 2 ++
s390x/css.c | 55 +++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 4 +++
4 files changed, 131 insertions(+)
create mode 100644 lib/s390x/css_lib.c
create mode 100644 s390x/css.c
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
new file mode 100644
index 0000000..fd087ce
--- /dev/null
+++ b/lib/s390x/css_lib.c
@@ -0,0 +1,70 @@
+/*
+ * Channel Subsystem tests library
+ *
+ * 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;
+
+/*
+ * css_enumerate:
+ * On success return the first subchannel ID found.
+ * On error return an invalid subchannel ID containing cc
+ */
+int css_enumerate(void)
+{
+ struct pmcw *pmcw = &schib.pmcw;
+ int scn_found = 0;
+ int dev_found = 0;
+ int schid = 0;
+ int cc;
+ int scn;
+
+ for (scn = 0; scn < 0xffff; scn++) {
+ cc = stsch(scn | SCHID_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 happen for STSCH */
+ report_abort("Unexpected error %d on subchannel %08x",
+ cc, scn | SCHID_ONE);
+ return cc;
+ }
+
+ /* 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 (!schid)
+ schid = scn | SCHID_ONE;
+ report_info("Found subchannel %08x", scn | SCHID_ONE);
+ dev_found++;
+ }
+
+out:
+ report_info("Tested subchannels: %d, I/O subchannels: %d, I/O devices: %d",
+ scn, scn_found, dev_found);
+ return schid;
+}
diff --git a/s390x/Makefile b/s390x/Makefile
index 050c40b..166cb5c 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
@@ -52,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
new file mode 100644
index 0000000..e19ffc8
--- /dev/null
+++ b/s390x/css.c
@@ -0,0 +1,55 @@
+/*
+ * 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 int test_device_sid;
+
+static void test_enumerate(void)
+{
+ test_device_sid = css_enumerate();
+ if (test_device_sid & SCHID_ONE) {
+ report(1, "Schid of first I/O device: 0x%08x", test_device_sid);
+ return;
+ }
+ report(0, "No I/O device 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 b307329..0f156af 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -84,3 +84,7 @@ extra_params = -m 1G
[sclp-3g]
file = sclp.elf
extra_params = -m 3G
+
+[css]
+file = css.elf
+extra_params = -device virtio-net-ccw
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (6 preceding siblings ...)
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 7/9] s390x: css: stsch, enumeration test Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 11:40 ` Cornelia Huck
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 9/9] s390x: css: ssch/tsch with sense and interrupt Pierre Morel
8 siblings, 1 reply; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
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 and the specified ISC
- 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 and uses the specified ISC.
- If the command succeeds but subchannel is not enabled or the ISC
field is not as expected, retry a predefined retries count.
- If the command fails, report the failure and do not retry, even
if cc indicates a busy/status pending as we do not expect this.
This tests the MSCH instruction to enable a channel successfully.
Retries are done and in case of error, and if the retries count
is exceeded, a report is made.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
lib/s390x/css.h | 8 +++--
lib/s390x/css_lib.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
s390x/css.c | 15 ++++++++++
3 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 0ddceb1..106479d 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -71,8 +71,9 @@ struct scsw {
struct pmcw {
uint32_t intparm;
-#define PMCW_DNV 0x0001
-#define PMCW_ENABLE 0x0080
+#define PMCW_DNV 0x0001
+#define PMCW_ENABLE 0x0080
+#define PMCW_ISC_SHIFT 11
uint16_t flags;
uint16_t devnum;
uint8_t lpm;
@@ -251,6 +252,7 @@ void dump_orb(struct orb *op);
int css_enumerate(void);
#define MAX_ENABLE_RETRIES 5
-int css_enable(int schid);
+#define IO_SCH_ISC 3
+int css_enable(int schid, int isc);
#endif
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index fd087ce..eda68a4 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -15,6 +15,7 @@
#include <string.h>
#include <interrupt.h>
#include <asm/arch_def.h>
+#include <asm/time.h>
#include <css.h>
@@ -68,3 +69,74 @@ out:
scn, scn_found, dev_found);
return schid;
}
+
+/*
+ * css_msch: enable subchannel and set with specified ISC
+ * @schid: Subchannel Identifier
+ * @isc : number of the interruption subclass to use
+ * Return value:
+ * On success: 0
+ * On error the CC of the faulty instruction
+ * or -1 if the retry count is exceeded.
+ */
+int css_enable(int schid, int isc)
+{
+ struct pmcw *pmcw = &schib.pmcw;
+ int retry_count = 0;
+ uint16_t flags;
+ int cc;
+
+ /* Read the SCHIB for this subchannel */
+ cc = stsch(schid, &schib);
+ if (cc) {
+ report_info("stsch: sch %08x failed with cc=%d", schid, cc);
+ return cc;
+ }
+
+ flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
+ if ((pmcw->flags & flags) == flags) {
+ report_info("stsch: sch %08x already enabled", schid);
+ return 0;
+ }
+
+retry:
+ /* Update the SCHIB to enable the channel and set the ISC */
+ pmcw->flags |= flags;
+
+ /* Tell the CSS we want to modify the subchannel */
+ cc = msch(schid, &schib);
+ if (cc) {
+ /*
+ * If the subchannel is status pending or
+ * if a function is in progress,
+ * we consider both cases as errors.
+ */
+ report_info("msch: sch %08x failed with cc=%d", schid, cc);
+ return cc;
+ }
+
+ /*
+ * Read the SCHIB again to verify the enablement
+ */
+ cc = stsch(schid, &schib);
+ if (cc) {
+ report_info("stsch: updating sch %08x failed with cc=%d",
+ schid, cc);
+ return cc;
+ }
+
+ if ((pmcw->flags & flags) == flags) {
+ report_info("stsch: sch %08x successfully modified after %d retries",
+ schid, retry_count);
+ return 0;
+ }
+
+ if (retry_count++ < MAX_ENABLE_RETRIES) {
+ mdelay(10); /* the hardware was not ready, give it some time */
+ goto retry;
+ }
+
+ report_info("msch: modifying sch %08x failed after %d retries. pmcw flags: %04x",
+ schid, retry_count, pmcw->flags);
+ return -1;
+}
diff --git a/s390x/css.c b/s390x/css.c
index e19ffc8..f314a0c 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -31,11 +31,26 @@ static void test_enumerate(void)
report(0, "No I/O device found");
}
+static void test_enable(void)
+{
+ int cc;
+
+ if (!test_device_sid) {
+ report_skip("No device");
+ return;
+ }
+
+ cc = css_enable(test_device_sid, IO_SCH_ISC);
+
+ report(cc == 0, "Enable subchannel %08x", test_device_sid);
+}
+
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] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test Pierre Morel
@ 2020-07-09 11:40 ` Cornelia Huck
2020-07-09 13:12 ` Pierre Morel
0 siblings, 1 reply; 22+ messages in thread
From: Cornelia Huck @ 2020-07-09 11:40 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On Thu, 9 Jul 2020 10:07:47 +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 and the specified ISC
> - 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 and uses the specified ISC.
> - If the command succeeds but subchannel is not enabled or the ISC
> field is not as expected, retry a predefined retries count.
> - If the command fails, report the failure and do not retry, even
> if cc indicates a busy/status pending as we do not expect this.
>
> This tests the MSCH instruction to enable a channel successfully.
> Retries are done and in case of error, and if the retries count
> is exceeded, a report is made.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Acked-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
> lib/s390x/css.h | 8 +++--
> lib/s390x/css_lib.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
> s390x/css.c | 15 ++++++++++
> 3 files changed, 92 insertions(+), 3 deletions(-)
(...)
> +/*
> + * css_msch: enable subchannel and set with specified ISC
"css_enable: enable the subchannel with the specified ISC"
?
> + * @schid: Subchannel Identifier
> + * @isc : number of the interruption subclass to use
> + * Return value:
> + * On success: 0
> + * On error the CC of the faulty instruction
> + * or -1 if the retry count is exceeded.
> + */
> +int css_enable(int schid, int isc)
> +{
> + struct pmcw *pmcw = &schib.pmcw;
> + int retry_count = 0;
> + uint16_t flags;
> + int cc;
> +
> + /* Read the SCHIB for this subchannel */
> + cc = stsch(schid, &schib);
> + if (cc) {
> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
> + return cc;
> + }
> +
> + flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
> + if ((pmcw->flags & flags) == flags) {
I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
catches the case of "subchannel has been enabled before, but with a
different isc".
> + report_info("stsch: sch %08x already enabled", schid);
> + return 0;
> + }
(...)
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 11:40 ` Cornelia Huck
@ 2020-07-09 13:12 ` Pierre Morel
2020-07-09 13:30 ` Cornelia Huck
0 siblings, 1 reply; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 13:12 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On 2020-07-09 13:40, Cornelia Huck wrote:
> On Thu, 9 Jul 2020 10:07:47 +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 and the specified ISC
>> - 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 and uses the specified ISC.
>> - If the command succeeds but subchannel is not enabled or the ISC
>> field is not as expected, retry a predefined retries count.
>> - If the command fails, report the failure and do not retry, even
>> if cc indicates a busy/status pending as we do not expect this.
>>
>> This tests the MSCH instruction to enable a channel successfully.
>> Retries are done and in case of error, and if the retries count
>> is exceeded, a report is made.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Acked-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>> lib/s390x/css.h | 8 +++--
>> lib/s390x/css_lib.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
>> s390x/css.c | 15 ++++++++++
>> 3 files changed, 92 insertions(+), 3 deletions(-)
>
> (...)
>
>> +/*
>> + * css_msch: enable subchannel and set with specified ISC
>
> "css_enable: enable the subchannel with the specified ISC"
>
> ?
>
>> + * @schid: Subchannel Identifier
>> + * @isc : number of the interruption subclass to use
>> + * Return value:
>> + * On success: 0
>> + * On error the CC of the faulty instruction
>> + * or -1 if the retry count is exceeded.
>> + */
>> +int css_enable(int schid, int isc)
>> +{
>> + struct pmcw *pmcw = &schib.pmcw;
>> + int retry_count = 0;
>> + uint16_t flags;
>> + int cc;
>> +
>> + /* Read the SCHIB for this subchannel */
>> + cc = stsch(schid, &schib);
>> + if (cc) {
>> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
>> + return cc;
>> + }
>> +
>> + flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
>> + if ((pmcw->flags & flags) == flags) {
>
> I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
> catches the case of "subchannel has been enabled before, but with a
> different isc".
If with a different ISC, we need to modify the ISC.
Don't we ?
>
>> + report_info("stsch: sch %08x already enabled", schid);
>> + return 0;
>> + }
>
> (...)
>
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 13:12 ` Pierre Morel
@ 2020-07-09 13:30 ` Cornelia Huck
2020-07-09 13:41 ` Pierre Morel
0 siblings, 1 reply; 22+ messages in thread
From: Cornelia Huck @ 2020-07-09 13:30 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On Thu, 9 Jul 2020 15:12:05 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> On 2020-07-09 13:40, Cornelia Huck wrote:
> > On Thu, 9 Jul 2020 10:07:47 +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 and the specified ISC
> >> - 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 and uses the specified ISC.
> >> - If the command succeeds but subchannel is not enabled or the ISC
> >> field is not as expected, retry a predefined retries count.
> >> - If the command fails, report the failure and do not retry, even
> >> if cc indicates a busy/status pending as we do not expect this.
> >>
> >> This tests the MSCH instruction to enable a channel successfully.
> >> Retries are done and in case of error, and if the retries count
> >> is exceeded, a report is made.
> >>
> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> >> Acked-by: Thomas Huth <thuth@redhat.com>
> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> >> ---
> >> lib/s390x/css.h | 8 +++--
> >> lib/s390x/css_lib.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
> >> s390x/css.c | 15 ++++++++++
> >> 3 files changed, 92 insertions(+), 3 deletions(-)
> >
> > (...)
> >
> >> +/*
> >> + * css_msch: enable subchannel and set with specified ISC
> >
> > "css_enable: enable the subchannel with the specified ISC"
> >
> > ?
> >
> >> + * @schid: Subchannel Identifier
> >> + * @isc : number of the interruption subclass to use
> >> + * Return value:
> >> + * On success: 0
> >> + * On error the CC of the faulty instruction
> >> + * or -1 if the retry count is exceeded.
> >> + */
> >> +int css_enable(int schid, int isc)
> >> +{
> >> + struct pmcw *pmcw = &schib.pmcw;
> >> + int retry_count = 0;
> >> + uint16_t flags;
> >> + int cc;
> >> +
> >> + /* Read the SCHIB for this subchannel */
> >> + cc = stsch(schid, &schib);
> >> + if (cc) {
> >> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
> >> + return cc;
> >> + }
> >> +
> >> + flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
> >> + if ((pmcw->flags & flags) == flags) {
> >
> > I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
> > catches the case of "subchannel has been enabled before, but with a
> > different isc".
>
> If with a different ISC, we need to modify the ISC.
> Don't we ?
I think that's a policy decision (I would probably fail and require a
disable before setting another isc, but that's a matter of taste).
Regardless, I think the current check doesn't even catch the 'different
isc' case?
>
> >
> >> + report_info("stsch: sch %08x already enabled", schid);
> >> + return 0;
> >> + }
>
>
>
> >
> > (...)
> >
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 13:30 ` Cornelia Huck
@ 2020-07-09 13:41 ` Pierre Morel
2020-07-09 13:52 ` Cornelia Huck
0 siblings, 1 reply; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 13:41 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On 2020-07-09 15:30, Cornelia Huck wrote:
> On Thu, 9 Jul 2020 15:12:05 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> On 2020-07-09 13:40, Cornelia Huck wrote:
>>> On Thu, 9 Jul 2020 10:07:47 +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 and the specified ISC
>>>> - 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 and uses the specified ISC.
>>>> - If the command succeeds but subchannel is not enabled or the ISC
>>>> field is not as expected, retry a predefined retries count.
>>>> - If the command fails, report the failure and do not retry, even
>>>> if cc indicates a busy/status pending as we do not expect this.
>>>>
>>>> This tests the MSCH instruction to enable a channel successfully.
>>>> Retries are done and in case of error, and if the retries count
>>>> is exceeded, a report is made.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> Acked-by: Thomas Huth <thuth@redhat.com>
>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>>>> ---
>>>> lib/s390x/css.h | 8 +++--
>>>> lib/s390x/css_lib.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
>>>> s390x/css.c | 15 ++++++++++
>>>> 3 files changed, 92 insertions(+), 3 deletions(-)
>>>
>>> (...)
>>>
>>>> +/*
>>>> + * css_msch: enable subchannel and set with specified ISC
>>>
>>> "css_enable: enable the subchannel with the specified ISC"
>>>
>>> ?
>>>
>>>> + * @schid: Subchannel Identifier
>>>> + * @isc : number of the interruption subclass to use
>>>> + * Return value:
>>>> + * On success: 0
>>>> + * On error the CC of the faulty instruction
>>>> + * or -1 if the retry count is exceeded.
>>>> + */
>>>> +int css_enable(int schid, int isc)
>>>> +{
>>>> + struct pmcw *pmcw = &schib.pmcw;
>>>> + int retry_count = 0;
>>>> + uint16_t flags;
>>>> + int cc;
>>>> +
>>>> + /* Read the SCHIB for this subchannel */
>>>> + cc = stsch(schid, &schib);
>>>> + if (cc) {
>>>> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
>>>> + return cc;
>>>> + }
>>>> +
>>>> + flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
>>>> + if ((pmcw->flags & flags) == flags) {
>>>
>>> I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
>>> catches the case of "subchannel has been enabled before, but with a
>>> different isc".
>>
>> If with a different ISC, we need to modify the ISC.
>> Don't we ?
>
> I think that's a policy decision (I would probably fail and require a
> disable before setting another isc, but that's a matter of taste).
>
> Regardless, I think the current check doesn't even catch the 'different
> isc' case?
hum, right.
If it is OK I remove this one.
And I must rework the same test I do later
in this patch.
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 13:41 ` Pierre Morel
@ 2020-07-09 13:52 ` Cornelia Huck
2020-07-09 13:58 ` Pierre Morel
0 siblings, 1 reply; 22+ messages in thread
From: Cornelia Huck @ 2020-07-09 13:52 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On Thu, 9 Jul 2020 15:41:56 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> On 2020-07-09 15:30, Cornelia Huck wrote:
> > On Thu, 9 Jul 2020 15:12:05 +0200
> > Pierre Morel <pmorel@linux.ibm.com> wrote:
> >
> >> On 2020-07-09 13:40, Cornelia Huck wrote:
> >>> On Thu, 9 Jul 2020 10:07:47 +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 and the specified ISC
> >>>> - 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 and uses the specified ISC.
> >>>> - If the command succeeds but subchannel is not enabled or the ISC
> >>>> field is not as expected, retry a predefined retries count.
> >>>> - If the command fails, report the failure and do not retry, even
> >>>> if cc indicates a busy/status pending as we do not expect this.
> >>>>
> >>>> This tests the MSCH instruction to enable a channel successfully.
> >>>> Retries are done and in case of error, and if the retries count
> >>>> is exceeded, a report is made.
> >>>>
> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> >>>> Acked-by: Thomas Huth <thuth@redhat.com>
> >>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> >>>> ---
> >>>> lib/s390x/css.h | 8 +++--
> >>>> lib/s390x/css_lib.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
> >>>> s390x/css.c | 15 ++++++++++
> >>>> 3 files changed, 92 insertions(+), 3 deletions(-)
> >>>
> >>> (...)
> >>>
> >>>> +/*
> >>>> + * css_msch: enable subchannel and set with specified ISC
> >>>
> >>> "css_enable: enable the subchannel with the specified ISC"
> >>>
> >>> ?
> >>>
> >>>> + * @schid: Subchannel Identifier
> >>>> + * @isc : number of the interruption subclass to use
> >>>> + * Return value:
> >>>> + * On success: 0
> >>>> + * On error the CC of the faulty instruction
> >>>> + * or -1 if the retry count is exceeded.
> >>>> + */
> >>>> +int css_enable(int schid, int isc)
> >>>> +{
> >>>> + struct pmcw *pmcw = &schib.pmcw;
> >>>> + int retry_count = 0;
> >>>> + uint16_t flags;
> >>>> + int cc;
> >>>> +
> >>>> + /* Read the SCHIB for this subchannel */
> >>>> + cc = stsch(schid, &schib);
> >>>> + if (cc) {
> >>>> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
> >>>> + return cc;
> >>>> + }
> >>>> +
> >>>> + flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
> >>>> + if ((pmcw->flags & flags) == flags) {
> >>>
> >>> I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
> >>> catches the case of "subchannel has been enabled before, but with a
> >>> different isc".
> >>
> >> If with a different ISC, we need to modify the ISC.
> >> Don't we ?
> >
> > I think that's a policy decision (I would probably fail and require a
> > disable before setting another isc, but that's a matter of taste).
> >
> > Regardless, I think the current check doesn't even catch the 'different
> > isc' case?
>
> hum, right.
> If it is OK I remove this one.
> And I must rework the same test I do later
> in this patch.
So, you mean checking for PMCW_ENABLE? Or not at all?
(I'd check for PMCW_ENABLE.)
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 13:52 ` Cornelia Huck
@ 2020-07-09 13:58 ` Pierre Morel
2020-07-09 14:22 ` Cornelia Huck
0 siblings, 1 reply; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 13:58 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On 2020-07-09 15:52, Cornelia Huck wrote:
> On Thu, 9 Jul 2020 15:41:56 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> On 2020-07-09 15:30, Cornelia Huck wrote:
>>> On Thu, 9 Jul 2020 15:12:05 +0200
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>
>>>> On 2020-07-09 13:40, Cornelia Huck wrote:
>>>>> On Thu, 9 Jul 2020 10:07:47 +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 and the specified ISC
>>>>>> - 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 and uses the specified ISC.
>>>>>> - If the command succeeds but subchannel is not enabled or the ISC
>>>>>> field is not as expected, retry a predefined retries count.
>>>>>> - If the command fails, report the failure and do not retry, even
>>>>>> if cc indicates a busy/status pending as we do not expect this.
>>>>>>
>>>>>> This tests the MSCH instruction to enable a channel successfully.
>>>>>> Retries are done and in case of error, and if the retries count
>>>>>> is exceeded, a report is made.
>>>>>>
>>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>>>> Acked-by: Thomas Huth <thuth@redhat.com>
>>>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>>>>>> ---
>>>>>> lib/s390x/css.h | 8 +++--
>>>>>> lib/s390x/css_lib.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>> s390x/css.c | 15 ++++++++++
>>>>>> 3 files changed, 92 insertions(+), 3 deletions(-)
>>>>>
>>>>> (...)
>>>>>
>>>>>> +/*
>>>>>> + * css_msch: enable subchannel and set with specified ISC
>>>>>
>>>>> "css_enable: enable the subchannel with the specified ISC"
>>>>>
>>>>> ?
>>>>>
>>>>>> + * @schid: Subchannel Identifier
>>>>>> + * @isc : number of the interruption subclass to use
>>>>>> + * Return value:
>>>>>> + * On success: 0
>>>>>> + * On error the CC of the faulty instruction
>>>>>> + * or -1 if the retry count is exceeded.
>>>>>> + */
>>>>>> +int css_enable(int schid, int isc)
>>>>>> +{
>>>>>> + struct pmcw *pmcw = &schib.pmcw;
>>>>>> + int retry_count = 0;
>>>>>> + uint16_t flags;
>>>>>> + int cc;
>>>>>> +
>>>>>> + /* Read the SCHIB for this subchannel */
>>>>>> + cc = stsch(schid, &schib);
>>>>>> + if (cc) {
>>>>>> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
>>>>>> + return cc;
>>>>>> + }
>>>>>> +
>>>>>> + flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
>>>>>> + if ((pmcw->flags & flags) == flags) {
>>>>>
>>>>> I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
>>>>> catches the case of "subchannel has been enabled before, but with a
>>>>> different isc".
>>>>
>>>> If with a different ISC, we need to modify the ISC.
>>>> Don't we ?
>>>
>>> I think that's a policy decision (I would probably fail and require a
>>> disable before setting another isc, but that's a matter of taste).
>>>
>>> Regardless, I think the current check doesn't even catch the 'different
>>> isc' case?
>>
>> hum, right.
>> If it is OK I remove this one.
>> And I must rework the same test I do later
>> in this patch.
>
> So, you mean checking for PMCW_ENABLE? Or not at all?
>
> (I'd check for PMCW_ENABLE.)
>
- if ((pmcw->flags & flags) == flags) {
+ if ((pmcw->flags & (PMCW_ISC_MASK | PMCW_ENABLE)) == flags) {
report_info("stsch: sch %08x already enabled", schid);
return 0;
}
I keep both, otherwise I return 0 without setting the ISC.
then I have another error:
retry:
/* Update the SCHIB to enable the channel and set the ISC */
+ pmcw->flags &= ~(PMCW_ISC_MASK | PMCW_ENABLE);
pmcw->flags |= flags;
and finaly the same as the first later...
- if ((pmcw->flags & flags) == flags) {
+ if ((pmcw->flags & (PMCW_ISC_MASK | PMCW_ENABLE)) == flags) {
report_info("stsch: sch %08x successfully modified
after %d retries",
schid, retry_count);
is better I think.
What do you think?
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 13:58 ` Pierre Morel
@ 2020-07-09 14:22 ` Cornelia Huck
2020-07-09 14:38 ` Pierre Morel
0 siblings, 1 reply; 22+ messages in thread
From: Cornelia Huck @ 2020-07-09 14:22 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On Thu, 9 Jul 2020 15:58:07 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> On 2020-07-09 15:52, Cornelia Huck wrote:
> > On Thu, 9 Jul 2020 15:41:56 +0200
> > Pierre Morel <pmorel@linux.ibm.com> wrote:
> >
> >> On 2020-07-09 15:30, Cornelia Huck wrote:
> >>> On Thu, 9 Jul 2020 15:12:05 +0200
> >>> Pierre Morel <pmorel@linux.ibm.com> wrote:
> >>>
> >>>> On 2020-07-09 13:40, Cornelia Huck wrote:
> >>>>> On Thu, 9 Jul 2020 10:07:47 +0200
> >>>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
> >>>>> (...)
> >>>>>
> >>>>>> +/*
> >>>>>> + * css_msch: enable subchannel and set with specified ISC
> >>>>>
> >>>>> "css_enable: enable the subchannel with the specified ISC"
> >>>>>
> >>>>> ?
> >>>>>
> >>>>>> + * @schid: Subchannel Identifier
> >>>>>> + * @isc : number of the interruption subclass to use
> >>>>>> + * Return value:
> >>>>>> + * On success: 0
> >>>>>> + * On error the CC of the faulty instruction
> >>>>>> + * or -1 if the retry count is exceeded.
> >>>>>> + */
> >>>>>> +int css_enable(int schid, int isc)
> >>>>>> +{
> >>>>>> + struct pmcw *pmcw = &schib.pmcw;
> >>>>>> + int retry_count = 0;
> >>>>>> + uint16_t flags;
> >>>>>> + int cc;
> >>>>>> +
> >>>>>> + /* Read the SCHIB for this subchannel */
> >>>>>> + cc = stsch(schid, &schib);
> >>>>>> + if (cc) {
> >>>>>> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
> >>>>>> + return cc;
> >>>>>> + }
> >>>>>> +
> >>>>>> + flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
> >>>>>> + if ((pmcw->flags & flags) == flags) {
> >>>>>
> >>>>> I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
> >>>>> catches the case of "subchannel has been enabled before, but with a
> >>>>> different isc".
> >>>>
> >>>> If with a different ISC, we need to modify the ISC.
> >>>> Don't we ?
> >>>
> >>> I think that's a policy decision (I would probably fail and require a
> >>> disable before setting another isc, but that's a matter of taste).
> >>>
> >>> Regardless, I think the current check doesn't even catch the 'different
> >>> isc' case?
> >>
> >> hum, right.
> >> If it is OK I remove this one.
> >> And I must rework the same test I do later
> >> in this patch.
> >
> > So, you mean checking for PMCW_ENABLE? Or not at all?
> >
> > (I'd check for PMCW_ENABLE.)
> >
>
> - if ((pmcw->flags & flags) == flags) {
> + if ((pmcw->flags & (PMCW_ISC_MASK | PMCW_ENABLE)) == flags) {
> report_info("stsch: sch %08x already enabled", schid);
> return 0;
> }
>
> I keep both, otherwise I return 0 without setting the ISC.
Ah, I missed the 'return 0'.
> then I have another error:
>
> retry:
> /* Update the SCHIB to enable the channel and set the ISC */
> + pmcw->flags &= ~(PMCW_ISC_MASK | PMCW_ENABLE);
Maybe ~PMCW_ISC_MASK is enough?
> pmcw->flags |= flags;
>
> and finaly the same as the first later...
>
> - if ((pmcw->flags & flags) == flags) {
> + if ((pmcw->flags & (PMCW_ISC_MASK | PMCW_ENABLE)) == flags) {
I think you can keep that as-is.
> report_info("stsch: sch %08x successfully modified
> after %d retries",
> schid, retry_count);
>
>
> is better I think.
> What do you think?
It's probably the right direction.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test
2020-07-09 14:22 ` Cornelia Huck
@ 2020-07-09 14:38 ` Pierre Morel
0 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 14:38 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On 2020-07-09 16:22, Cornelia Huck wrote:
> On Thu, 9 Jul 2020 15:58:07 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> On 2020-07-09 15:52, Cornelia Huck wrote:
>>> On Thu, 9 Jul 2020 15:41:56 +0200
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>
>>>> On 2020-07-09 15:30, Cornelia Huck wrote:
>>>>> On Thu, 9 Jul 2020 15:12:05 +0200
>>>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>>>
>>>>>> On 2020-07-09 13:40, Cornelia Huck wrote:
>>>>>>> On Thu, 9 Jul 2020 10:07:47 +0200
>>>>>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>>>>>>> (...)
>>>>>>>
>>>>>>>> +/*
>>>>>>>> + * css_msch: enable subchannel and set with specified ISC
>>>>>>>
>>>>>>> "css_enable: enable the subchannel with the specified ISC"
>>>>>>>
>>>>>>> ?
>>>>>>>
>>>>>>>> + * @schid: Subchannel Identifier
>>>>>>>> + * @isc : number of the interruption subclass to use
>>>>>>>> + * Return value:
>>>>>>>> + * On success: 0
>>>>>>>> + * On error the CC of the faulty instruction
>>>>>>>> + * or -1 if the retry count is exceeded.
>>>>>>>> + */
>>>>>>>> +int css_enable(int schid, int isc)
>>>>>>>> +{
>>>>>>>> + struct pmcw *pmcw = &schib.pmcw;
>>>>>>>> + int retry_count = 0;
>>>>>>>> + uint16_t flags;
>>>>>>>> + int cc;
>>>>>>>> +
>>>>>>>> + /* Read the SCHIB for this subchannel */
>>>>>>>> + cc = stsch(schid, &schib);
>>>>>>>> + if (cc) {
>>>>>>>> + report_info("stsch: sch %08x failed with cc=%d", schid, cc);
>>>>>>>> + return cc;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
>>>>>>>> + if ((pmcw->flags & flags) == flags) {
>>>>>>>
>>>>>>> I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
>>>>>>> catches the case of "subchannel has been enabled before, but with a
>>>>>>> different isc".
>>>>>>
>>>>>> If with a different ISC, we need to modify the ISC.
>>>>>> Don't we ?
>>>>>
>>>>> I think that's a policy decision (I would probably fail and require a
>>>>> disable before setting another isc, but that's a matter of taste).
>>>>>
>>>>> Regardless, I think the current check doesn't even catch the 'different
>>>>> isc' case?
>>>>
>>>> hum, right.
>>>> If it is OK I remove this one.
>>>> And I must rework the same test I do later
>>>> in this patch.
>>>
>>> So, you mean checking for PMCW_ENABLE? Or not at all?
>>>
>>> (I'd check for PMCW_ENABLE.)
>>>
>>
>> - if ((pmcw->flags & flags) == flags) {
>> + if ((pmcw->flags & (PMCW_ISC_MASK | PMCW_ENABLE)) == flags) {
>> report_info("stsch: sch %08x already enabled", schid);
>> return 0;
>> }
>>
>> I keep both, otherwise I return 0 without setting the ISC.
>
> Ah, I missed the 'return 0'.
>
>> then I have another error:
>>
>> retry:
>> /* Update the SCHIB to enable the channel and set the ISC */
>> + pmcw->flags &= ~(PMCW_ISC_MASK | PMCW_ENABLE);
>
> Maybe ~PMCW_ISC_MASK is enough?
yes
>
>> pmcw->flags |= flags;
>>
>> and finaly the same as the first later...
>>
>> - if ((pmcw->flags & flags) == flags) {
>> + if ((pmcw->flags & (PMCW_ISC_MASK | PMCW_ENABLE)) == flags) {
>
> I think you can keep that as-is.
I don't thing so, I just stored the pmcw.
if ISC is stored as 3 and I want 1 it is a false positive.
Same error as you showed me before.
?
>
>> report_info("stsch: sch %08x successfully modified
>> after %d retries",
>> schid, retry_count);
>>
>>
>> is better I think.
>> What do you think?
>
> It's probably the right direction.
>
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v11 9/9] s390x: css: ssch/tsch with sense and interrupt
2020-07-09 8:07 [kvm-unit-tests PATCH v11 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
` (7 preceding siblings ...)
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 8/9] s390x: css: msch, enable test Pierre Morel
@ 2020-07-09 8:07 ` Pierre Morel
2020-07-09 12:13 ` Cornelia Huck
8 siblings, 1 reply; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 8:07 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, frankja, david, thuth, cohuck, drjones
After a channel is enabled we 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 SENSE_ID command response is tested to report 0xff inside
its reserved field and to report the same control unit type
as the cu_type kernel argument.
Without the cu_type kernel argument, the test expects a device
with a default control unit type of 0x3832, a.k.a virtio-net-ccw.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
lib/s390x/asm/arch_def.h | 1 +
lib/s390x/css.h | 35 ++++++++
lib/s390x/css_lib.c | 183 +++++++++++++++++++++++++++++++++++++++
s390x/css.c | 80 +++++++++++++++++
4 files changed, 299 insertions(+)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 022a564..edc06ef 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -16,6 +16,7 @@ struct psw {
};
#define PSW_MASK_EXT 0x0100000000000000UL
+#define PSW_MASK_IO 0x0200000000000000UL
#define PSW_MASK_DAT 0x0400000000000000UL
#define PSW_MASK_WAIT 0x0002000000000000UL
#define PSW_MASK_PSTATE 0x0001000000000000UL
diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 106479d..e754f9f 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -11,6 +11,8 @@
#ifndef CSS_H
#define CSS_H
+#define lowcore_ptr ((struct lowcore *)0x0)
+
/* subchannel ID bit 16 must always be one */
#define SCHID_ONE 0x00010000
@@ -62,9 +64,18 @@ struct orb {
} __attribute__ ((aligned(4)));
struct scsw {
+#define SCSW_SC_PENDING 0x00000001
+#define SCSW_SC_SECONDARY 0x00000002
+#define SCSW_SC_PRIMARY 0x00000004
+#define SCSW_SC_INTERMEDIATE 0x00000008
+#define SCSW_SC_ALERT 0x00000010
uint32_t ctrl;
uint32_t ccw_addr;
+#define SCSW_DEVS_DEV_END 0x04
+#define SCSW_DEVS_SCH_END 0x08
uint8_t dev_stat;
+#define SCSW_SCHS_PCI 0x80
+#define SCSW_SCHS_IL 0x40
uint8_t sch_stat;
uint16_t count;
};
@@ -101,6 +112,19 @@ struct irb {
uint32_t emw[8];
} __attribute__ ((aligned(4)));
+#define CCW_CMD_SENSE_ID 0xe4
+#define CSS_SENSEID_COMMON_LEN 8
+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)
@@ -255,4 +279,15 @@ int css_enumerate(void);
#define IO_SCH_ISC 3
int css_enable(int schid, int isc);
+
+/* Library functions */
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw);
+int start_single_ccw(unsigned int sid, int code, void *data, int count,
+ unsigned char flags);
+void css_irq_io(void);
+int css_residual_count(unsigned int schid);
+
+void enable_io_isc(uint8_t isc);
+int wait_and_check_io_completion(int schid);
+
#endif
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index eda68a4..c64edd5 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -16,6 +16,7 @@
#include <interrupt.h>
#include <asm/arch_def.h>
#include <asm/time.h>
+#include <asm/arch_def.h>
#include <css.h>
@@ -103,6 +104,9 @@ retry:
/* Update the SCHIB to enable the channel and set the ISC */
pmcw->flags |= flags;
+ /* Set Interruption Subclass to IO_SCH_ISC */
+ pmcw->flags |= (isc << PMCW_ISC_SHIFT);
+
/* Tell the CSS we want to modify the subchannel */
cc = msch(schid, &schib);
if (cc) {
@@ -140,3 +144,182 @@ retry:
schid, retry_count, pmcw->flags);
return -1;
}
+
+static struct irb irb;
+
+void css_irq_io(void)
+{
+ int ret = 0;
+ char *flags;
+ int sid;
+
+ report_prefix_push("Interrupt");
+ sid = lowcore_ptr->subsys_id_word;
+ /* Lowlevel set the SID as interrupt parameter. */
+ if (lowcore_ptr->io_int_param != sid) {
+ report(0,
+ "io_int_param: %x differs from subsys_id_word: %x",
+ lowcore_ptr->io_int_param, sid);
+ goto pop;
+ }
+ report_info("subsys_id_word: %08x io_int_param %08x io_int_word %08x",
+ lowcore_ptr->subsys_id_word,
+ lowcore_ptr->io_int_param,
+ lowcore_ptr->io_int_word);
+ report_prefix_pop();
+
+ report_prefix_push("tsch");
+ ret = tsch(sid, &irb);
+ switch (ret) {
+ case 1:
+ dump_irb(&irb);
+ flags = dump_scsw_flags(irb.scsw.ctrl);
+ report(0,
+ "I/O interrupt, but tsch returns CC 1 for subchannel %08x. SCSW flags: %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_ptr->io_old_psw.mask &= ~PSW_MASK_WAIT;
+}
+
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw)
+{
+ struct orb orb = {
+ .intparm = sid,
+ .ctrl = ORB_CTRL_ISIC|ORB_CTRL_FMT|ORB_LPM_DFLT,
+ .cpa = (unsigned int) (unsigned long)ccw,
+ };
+
+ return ssch(sid, &orb);
+}
+
+/*
+ * In the future, we want to implement support for CCW chains;
+ * for that, we will need to work with ccw1 pointers.
+ */
+static struct ccw1 unique_ccw;
+
+int start_single_ccw(unsigned int sid, int code, void *data, int count,
+ unsigned char flags)
+{
+ int cc;
+ struct ccw1 *ccw = &unique_ccw;
+
+ report_prefix_push("start_subchannel");
+ /* Build the CCW chain with a single CCW */
+ ccw->code = code;
+ ccw->flags = flags;
+ ccw->count = count;
+ ccw->data_address = (int)(unsigned long)data;
+
+ cc = start_ccw1_chain(sid, ccw);
+ if (cc) {
+ report(0, "cc = %d", cc);
+ report_prefix_pop();
+ return cc;
+ }
+ report_prefix_pop();
+ return 0;
+}
+
+/* wait_and_check_io_completion:
+ * @schid: the subchannel ID
+ *
+ * Makes the most common check to validate a successful I/O
+ * completion.
+ * Only report failures.
+ */
+int wait_and_check_io_completion(int schid)
+{
+ int ret = 0;
+
+ wait_for_interrupt(PSW_MASK_IO);
+
+ report_prefix_push("check I/O completion");
+
+ if (lowcore_ptr->io_int_param != schid) {
+ report(0, "interrupt parameter: expected %08x got %08x",
+ schid, lowcore_ptr->io_int_param);
+ ret = -1;
+ goto end;
+ }
+
+ /* Verify that device status is valid */
+ if (!(irb.scsw.ctrl & SCSW_SC_PENDING)) {
+ report(0, "No status pending after interrupt. Subch Ctrl: %08x",
+ irb.scsw.ctrl);
+ ret = -1;
+ goto end;
+ }
+
+ if (!(irb.scsw.ctrl & (SCSW_SC_SECONDARY | SCSW_SC_PRIMARY))) {
+ report(0, "Primary or secondary status missing. Subch Ctrl: %08x",
+ irb.scsw.ctrl);
+ ret = -1;
+ goto end;
+ }
+
+ if (!(irb.scsw.dev_stat & (SCSW_DEVS_DEV_END | SCSW_DEVS_SCH_END))) {
+ report(0, "No device end nor sch end. Dev. status: %02x",
+ irb.scsw.dev_stat);
+ ret = -1;
+ goto end;
+ }
+
+ if (irb.scsw.sch_stat & !(SCSW_SCHS_PCI | SCSW_SCHS_IL)) {
+ report_info("Unexpected Subch. status %02x", irb.scsw.sch_stat);
+ ret = -1;
+ goto end;
+ }
+
+end:
+ report_prefix_pop();
+ return ret;
+}
+
+/*
+ * css_residual_count
+ * Return the residual count, if it is valid.
+ *
+ * Return value:
+ * Success: the residual count
+ * Not meaningful: -1 (-1 can not be a valid count)
+ */
+int css_residual_count(unsigned int schid)
+{
+
+ if (!(irb.scsw.ctrl & (SCSW_SC_PENDING | SCSW_SC_PRIMARY)))
+ goto invalid;
+
+ if (irb.scsw.dev_stat)
+ if (irb.scsw.sch_stat & ~(SCSW_SCHS_PCI | SCSW_SCHS_IL))
+ goto invalid;
+
+ return irb.scsw.count;
+
+invalid:
+ return -1;
+}
+
+/*
+ * enable_io_isc: setup ISC in Control Register 6
+ * @isc: The interruption Sub Class as a bitfield
+ */
+void enable_io_isc(uint8_t isc)
+{
+ uint64_t value;
+
+ value = (uint64_t)isc << 24;
+ lctlg(6, value);
+}
diff --git a/s390x/css.c b/s390x/css.c
index f314a0c..ee3bc83 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -19,7 +19,11 @@
#include <css.h>
+#define DEFAULT_CU_TYPE 0x3832 /* virtio-ccw */
+static unsigned long cu_type = DEFAULT_CU_TYPE;
+
static int test_device_sid;
+static struct senseid senseid;
static void test_enumerate(void)
{
@@ -45,12 +49,87 @@ static void test_enable(void)
report(cc == 0, "Enable subchannel %08x", test_device_sid);
}
+/*
+ * test_sense
+ * Pre-requisites:
+ * - We need the test device as the first recognized
+ * device by the enumeration.
+ */
+static void test_sense(void)
+{
+ int ret;
+ int len;
+
+ if (!test_device_sid) {
+ report_skip("No device");
+ return;
+ }
+
+ ret = css_enable(test_device_sid, IO_SCH_ISC);
+ if (ret) {
+ report(0, "Could not enable the subchannel: %08x",
+ test_device_sid);
+ return;
+ }
+
+ ret = register_io_int_func(css_irq_io);
+ if (ret) {
+ report(0, "Could not register IRQ handler");
+ return;
+ }
+
+ lowcore_ptr->io_int_param = 0;
+
+ memset(&senseid, 0, sizeof(senseid));
+ ret = start_single_ccw(test_device_sid, CCW_CMD_SENSE_ID,
+ &senseid, sizeof(senseid), CCW_F_SLI);
+ if (ret)
+ goto error;
+
+ if (wait_and_check_io_completion(test_device_sid) < 0)
+ goto error;
+
+ /* Test transfer completion */
+ report_prefix_push("ssch transfer completion");
+
+ ret = css_residual_count(test_device_sid);
+
+ if (ret < 0) {
+ report_info("no valid residual count");
+ } else if (ret != 0) {
+ len = sizeof(senseid) - ret;
+ if (ret && len < CSS_SENSEID_COMMON_LEN) {
+ report(0, "transferred a too short length: %d", ret);
+ goto error;
+ } else if (ret && len)
+ report_info("transferred a shorter length: %d", len);
+ }
+
+ if (senseid.reserved != 0xff) {
+ report(0, "transferred garbage: 0x%02x", senseid.reserved);
+ goto error;
+ }
+
+ report_prefix_pop();
+
+ report_info("reserved 0x%02x cu_type 0x%04x cu_model 0x%02x dev_type 0x%04x dev_model 0x%02x",
+ senseid.reserved, senseid.cu_type, senseid.cu_model,
+ senseid.dev_type, senseid.dev_model);
+
+ report(senseid.cu_type == cu_type, "cu_type expected 0x%04x got 0x%04x",
+ (uint16_t) cu_type, senseid.cu_type);
+
+error:
+ unregister_io_int_func(css_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 }
};
@@ -59,6 +138,7 @@ int main(int argc, char *argv[])
int i;
report_prefix_push("Channel Subsystem");
+ enable_io_isc(0x80 >> IO_SCH_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] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 9/9] s390x: css: ssch/tsch with sense and interrupt
2020-07-09 8:07 ` [kvm-unit-tests PATCH v11 9/9] s390x: css: ssch/tsch with sense and interrupt Pierre Morel
@ 2020-07-09 12:13 ` Cornelia Huck
2020-07-09 13:18 ` Pierre Morel
0 siblings, 1 reply; 22+ messages in thread
From: Cornelia Huck @ 2020-07-09 12:13 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On Thu, 9 Jul 2020 10:07:48 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> After a channel is enabled we 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 SENSE_ID command response is tested to report 0xff inside
> its reserved field and to report the same control unit type
> as the cu_type kernel argument.
>
> Without the cu_type kernel argument, the test expects a device
> with a default control unit type of 0x3832, a.k.a virtio-net-ccw.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> lib/s390x/asm/arch_def.h | 1 +
> lib/s390x/css.h | 35 ++++++++
> lib/s390x/css_lib.c | 183 +++++++++++++++++++++++++++++++++++++++
> s390x/css.c | 80 +++++++++++++++++
> 4 files changed, 299 insertions(+)
(...)
> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
> index eda68a4..c64edd5 100644
> --- a/lib/s390x/css_lib.c
> +++ b/lib/s390x/css_lib.c
> @@ -16,6 +16,7 @@
> #include <interrupt.h>
> #include <asm/arch_def.h>
> #include <asm/time.h>
> +#include <asm/arch_def.h>
>
> #include <css.h>
>
> @@ -103,6 +104,9 @@ retry:
> /* Update the SCHIB to enable the channel and set the ISC */
> pmcw->flags |= flags;
>
> + /* Set Interruption Subclass to IO_SCH_ISC */
> + pmcw->flags |= (isc << PMCW_ISC_SHIFT);
But isn't the isc already contained in 'flags'? I think you should just
delete these two lines.
> +
> /* Tell the CSS we want to modify the subchannel */
> cc = msch(schid, &schib);
> if (cc) {
(...)
> +/* wait_and_check_io_completion:
> + * @schid: the subchannel ID
> + *
> + * Makes the most common check to validate a successful I/O
> + * completion.
> + * Only report failures.
> + */
> +int wait_and_check_io_completion(int schid)
> +{
> + int ret = 0;
> +
> + wait_for_interrupt(PSW_MASK_IO);
> +
> + report_prefix_push("check I/O completion");
> +
> + if (lowcore_ptr->io_int_param != schid) {
> + report(0, "interrupt parameter: expected %08x got %08x",
> + schid, lowcore_ptr->io_int_param);
> + ret = -1;
> + goto end;
> + }
> +
> + /* Verify that device status is valid */
> + if (!(irb.scsw.ctrl & SCSW_SC_PENDING)) {
> + report(0, "No status pending after interrupt. Subch Ctrl: %08x",
> + irb.scsw.ctrl);
> + ret = -1;
> + goto end;
> + }
> +
> + if (!(irb.scsw.ctrl & (SCSW_SC_SECONDARY | SCSW_SC_PRIMARY))) {
> + report(0, "Primary or secondary status missing. Subch Ctrl: %08x",
> + irb.scsw.ctrl);
> + ret = -1;
> + goto end;
> + }
> +
> + if (!(irb.scsw.dev_stat & (SCSW_DEVS_DEV_END | SCSW_DEVS_SCH_END))) {
> + report(0, "No device end nor sch end. Dev. status: %02x",
s/nor/or/ ?
> + irb.scsw.dev_stat);
> + ret = -1;
> + goto end;
> + }
> +
> + if (irb.scsw.sch_stat & !(SCSW_SCHS_PCI | SCSW_SCHS_IL)) {
Did you mean ~(SCSW_SCHS_PCI | SCSW_SCHS_IL)?
If yes, why do think a PCI may show up?
> + report_info("Unexpected Subch. status %02x", irb.scsw.sch_stat);
> + ret = -1;
> + goto end;
> + }
> +
> +end:
> + report_prefix_pop();
> + return ret;
(...)
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 9/9] s390x: css: ssch/tsch with sense and interrupt
2020-07-09 12:13 ` Cornelia Huck
@ 2020-07-09 13:18 ` Pierre Morel
2020-07-09 13:33 ` Cornelia Huck
0 siblings, 1 reply; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 13:18 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On 2020-07-09 14:13, Cornelia Huck wrote:
> On Thu, 9 Jul 2020 10:07:48 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> After a channel is enabled we 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 SENSE_ID command response is tested to report 0xff inside
>> its reserved field and to report the same control unit type
>> as the cu_type kernel argument.
>>
>> Without the cu_type kernel argument, the test expects a device
>> with a default control unit type of 0x3832, a.k.a virtio-net-ccw.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/asm/arch_def.h | 1 +
>> lib/s390x/css.h | 35 ++++++++
>> lib/s390x/css_lib.c | 183 +++++++++++++++++++++++++++++++++++++++
>> s390x/css.c | 80 +++++++++++++++++
>> 4 files changed, 299 insertions(+)
>
> (...)
>
>> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
>> index eda68a4..c64edd5 100644
>> --- a/lib/s390x/css_lib.c
>> +++ b/lib/s390x/css_lib.c
>> @@ -16,6 +16,7 @@
>> #include <interrupt.h>
>> #include <asm/arch_def.h>
>> #include <asm/time.h>
>> +#include <asm/arch_def.h>
>>
>> #include <css.h>
>>
>> @@ -103,6 +104,9 @@ retry:
>> /* Update the SCHIB to enable the channel and set the ISC */
>> pmcw->flags |= flags;
>>
>> + /* Set Interruption Subclass to IO_SCH_ISC */
>> + pmcw->flags |= (isc << PMCW_ISC_SHIFT);
>
> But isn't the isc already contained in 'flags'? I think you should just
> delete these two lines.
right.
>
>> +
>> /* Tell the CSS we want to modify the subchannel */
>> cc = msch(schid, &schib);
>> if (cc) {
>
> (...)
>
>> +/* wait_and_check_io_completion:
>> + * @schid: the subchannel ID
>> + *
>> + * Makes the most common check to validate a successful I/O
>> + * completion.
>> + * Only report failures.
>> + */
>> +int wait_and_check_io_completion(int schid)
>> +{
>> + int ret = 0;
>> +
>> + wait_for_interrupt(PSW_MASK_IO);
>> +
>> + report_prefix_push("check I/O completion");
>> +
>> + if (lowcore_ptr->io_int_param != schid) {
>> + report(0, "interrupt parameter: expected %08x got %08x",
>> + schid, lowcore_ptr->io_int_param);
>> + ret = -1;
>> + goto end;
>> + }
>> +
>> + /* Verify that device status is valid */
>> + if (!(irb.scsw.ctrl & SCSW_SC_PENDING)) {
>> + report(0, "No status pending after interrupt. Subch Ctrl: %08x",
>> + irb.scsw.ctrl);
>> + ret = -1;
>> + goto end;
>> + }
>> +
>> + if (!(irb.scsw.ctrl & (SCSW_SC_SECONDARY | SCSW_SC_PRIMARY))) {
>> + report(0, "Primary or secondary status missing. Subch Ctrl: %08x",
>> + irb.scsw.ctrl);
>> + ret = -1;
>> + goto end;
>> + }
>> +
>> + if (!(irb.scsw.dev_stat & (SCSW_DEVS_DEV_END | SCSW_DEVS_SCH_END))) {
>> + report(0, "No device end nor sch end. Dev. status: %02x",
>
> s/nor/or/ ?
OK
>
>> + irb.scsw.dev_stat);
>> + ret = -1;
>> + goto end;
>> + }
>> +
>> + if (irb.scsw.sch_stat & !(SCSW_SCHS_PCI | SCSW_SCHS_IL)) {
>
> Did you mean ~(SCSW_SCHS_PCI | SCSW_SCHS_IL)?
grrr... yes, thanks.
>
> If yes, why do think a PCI may show up?
Should not in the current implementation.
I thought I can add it as a general test.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 9/9] s390x: css: ssch/tsch with sense and interrupt
2020-07-09 13:18 ` Pierre Morel
@ 2020-07-09 13:33 ` Cornelia Huck
2020-07-09 13:38 ` Pierre Morel
0 siblings, 1 reply; 22+ messages in thread
From: Cornelia Huck @ 2020-07-09 13:33 UTC (permalink / raw)
To: Pierre Morel; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On Thu, 9 Jul 2020 15:18:25 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> On 2020-07-09 14:13, Cornelia Huck wrote:
> > On Thu, 9 Jul 2020 10:07:48 +0200
> > Pierre Morel <pmorel@linux.ibm.com> wrote:
> >> + if (irb.scsw.sch_stat & !(SCSW_SCHS_PCI | SCSW_SCHS_IL)) {
> >
> > Did you mean ~(SCSW_SCHS_PCI | SCSW_SCHS_IL)?
>
> grrr... yes, thanks.
>
> >
> > If yes, why do think a PCI may show up?
>
> Should not in the current implementation.
> I thought I can add it as a general test.
Yeah, maybe in the future. But for now, I think it is a bit confusing.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [kvm-unit-tests PATCH v11 9/9] s390x: css: ssch/tsch with sense and interrupt
2020-07-09 13:33 ` Cornelia Huck
@ 2020-07-09 13:38 ` Pierre Morel
0 siblings, 0 replies; 22+ messages in thread
From: Pierre Morel @ 2020-07-09 13:38 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, frankja, david, thuth, drjones
On 2020-07-09 15:33, Cornelia Huck wrote:
> On Thu, 9 Jul 2020 15:18:25 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> On 2020-07-09 14:13, Cornelia Huck wrote:
>>> On Thu, 9 Jul 2020 10:07:48 +0200
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>>>> + if (irb.scsw.sch_stat & !(SCSW_SCHS_PCI | SCSW_SCHS_IL)) {
>>>
>>> Did you mean ~(SCSW_SCHS_PCI | SCSW_SCHS_IL)?
>>
>> grrr... yes, thanks.
>>
>>>
>>> If yes, why do think a PCI may show up?
>>
>> Should not in the current implementation.
>> I thought I can add it as a general test.
>
> Yeah, maybe in the future. But for now, I think it is a bit confusing.
>
OK then I just remove it.
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 22+ messages in thread