On 2019-12-02 15:06, Cornelia Huck wrote:
On Thu, 28 Nov 2019 13:46:03 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
These are the include and library utilities for the css tests patch
series.
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
lib/s390x/css.h | 269 +++++++++++++++++++++++++++++++++++++++++++
lib/s390x/css_dump.c | 147 +++++++++++++++++++++++
2 files changed, 416 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..95dec72
--- /dev/null
+++ b/lib/s390x/css.h
@@ -0,0 +1,269 @@
+/*
+ * CSS definitions
+ *
+ * Copyright IBM, Corp. 2019
+ * Author: Pierre Morel <pmorel@xxxxxxxxxxxxx>
+ *
+ * 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 ccw {
ccw1, to make it clear that this is a format-1 ccw?
(Do you have any plans to test this with format-0 ccws as well?)
not in a near future
+ unsigned char code;
+ unsigned char flags;
+ unsigned short count;
+ unsigned int data;
data_address?
OK
+} __attribute__ ((aligned(4)));
+
+#define ORB_M_KEY 0xf0000000
+#define ORB_F_SUSPEND 0x08000000
+#define ORB_F_STREAMING 0x04000000
+#define ORB_F_MODIFCTRL 0x02000000
+#define ORB_F_SYNC 0x01000000
+#define ORB_F_FORMAT 0x00800000
+#define ORB_F_PREFETCH 0x00400000
+#define ORB_F_INIT_IRQ 0x00200000
+#define ORB_F_ADDRLIMIT 0x00100000
+#define ORB_F_SUSP_IRQ 0x00080000
+#define ORB_F_TRANSPORT 0x00040000
+#define ORB_F_IDAW2 0x00020000
+#define ORB_F_IDAW_2K 0x00010000
+#define ORB_M_LPM 0x0000ff00
+#define ORB_F_LPM_DFLT 0x00008000
+#define ORB_F_ILSM 0x00000080
+#define ORB_F_CCW_IND 0x00000040
+#define ORB_F_ORB_EXT 0x00000001
+
+struct orb {
+ unsigned int intparm;
+ unsigned int ctrl;
+ unsigned int cpa;
+ unsigned int prio;
+ unsigned int reserved[4];
+} __attribute__ ((aligned(4)));
+
+struct scsw {
+ uint32_t ctrl;
+ uint32_t addr;
ccw_addr?
OK
+ uint8_t devs;
+ uint8_t schs;
Maybe dev_stat/sch_stat?
OK
+ uint16_t count;
+};
Out of curiousity (I'm not familiar with the conventions in
kvm-unit-tests): You use the explicit uint32_t et al. types here, while
you used unsigned int et al. in the other definitions... maybe it would
be better to use one or the other?
OK, I suppose that if we want to be working with TCG uintxxx_t is better.
So I will take care to be coherent.
Also, you probably always want to test against a QEMU-provided device
anyway, so you can probably ignore transport mode and stick with
command mode only, right?
Yes
+
+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];
+ struct {
+ uint8_t res0;
+ uint8_t st:3;
+ uint8_t :5;
+ uint16_t :13;
+ uint16_t f:1;
+ uint16_t x:1;
+ uint16_t s:1;
+ };
Hm... you used masks for the other fields, any reason you didn't here?
Here too , I must be coherent.
Reason was that I use the flags differently, the test of the bit fields
is easier to read than shift & mask but ... anyway I prefer masks for
hardware close programing.
+};
+
+struct schib {
+ struct pmcw pmcw;
+ struct scsw scsw;
+ uint32_t md0;
+ uint32_t md1;
+ uint32_t md2;
Hm, both Linux and QEMU express the fields you called md<n> as a 64 bit
measurement-block address and a four bytes model-dependent area...
would it make sense to do so here as well? If the extended measurement
block facility is not installed, we'd get a 12 bytes model-dependent
area, which IMHO would also look better here.
OK, IMHO too
+} __attribute__ ((aligned(4)));
+
+struct irb {
+ struct scsw scsw;
+ uint32_t esw[5];
+ uint32_t ecw[8];
+ uint32_t emw[8];
If I read the PoP correctly, esw, ecw, and emw are defined bytewise,
not u32-wise.
Hum, I found them referenced u32-wise.
I intended to define them as a structure at the moment they are used.
If you really prefer I can use uint8_t esw[40] uint8_t ecw[64] etc.
but I like better word aligned structures.
+} __attribute__ ((aligned(4)));
+
+/* CSS low level access functions */
+
+static inline int ssch(unsigned long schid, struct orb *addr)
+{
+ register long long reg1 asm("1") = schid;
+ int cc = -1;
+
+ asm volatile(
+ " ssch 0(%2)\n"
+ "0: ipm %0\n"
+ " srl %0,28\n"
+ "1:\n"
+ : "+d" (cc)
+ : "d" (reg1), "a" (addr), "m" (*addr)
+ : "cc", "memory");
+ return cc;
+}
Looking at the Linux code, stsch, msch, and ssch all set up an
exception handler. IIRC, I had introduced that for stsch for multiple
subchannels sets, not sure about the others. Are we sure we never need
to catch exceptions here?
This is the next and important step.
I want to surround each call with expect_pgm_int() / check_pgm_int_code()
Also testing each error case.
So we do not need exception handler here, we will use the kvm-test ones.
+
+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 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;
+}
Does rchp actually do anything useful on QEMU? Or is this mainly for
completeness' sake?
It is for completness for this first series.
However I would like to do something with it in a following one.
to be ready for QEMU implementation or to test real hardware when the
kvm-unit-test run on LPAR.
+
+/* Debug functions */
+char *dump_pmcw_flags(uint16_t f);
+char *dump_scsw_flags(uint32_t f);
+#undef DEBUG
+#ifdef DEBUG
+void dump_scsw(struct scsw *);
+void dump_irb(struct irb *irbp);
+void dump_schib(struct schib *sch);
+struct ccw *dump_ccw(struct ccw *cp);
+#else
+static inline void dump_scsw(struct scsw *scsw){}
+static inline void dump_irb(struct irb *irbp){}
+static inline void dump_pmcw(struct pmcw *p){}
+static inline void dump_schib(struct schib *sch){}
+static inline void dump_orb(struct orb *op){}
+static inline struct ccw *dump_ccw(struct ccw *cp)
+{
+ return NULL;
+}
+#endif
+
+extern unsigned long stacktop;
+#endif
diff --git a/lib/s390x/css_dump.c b/lib/s390x/css_dump.c
new file mode 100644
index 0000000..5356df2
--- /dev/null
+++ b/lib/s390x/css_dump.c
@@ -0,0 +1,147 @@
+/*
+ * Channel Sub-System structures dumping
I think "subsystem" is the more usual spelling.
OK, I found it much fun so :)
+ *
+ * Copyright (c) 2019 IBM Corp.
+ *
+ * Authors:
+ * Pierre Morel <pmorel@xxxxxxxxxxxxx>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ *
+ * Description:
+ * Provides the dumping functions for various structures used by subchannels:
+ * - ORB : Operation request block, describe the I/O operation and point to
s/describe/describes/
s/point/points/
thanks
+ * a CCW chain
+ * - CCW : Channel Command Word, describe the data and flow control
s/describe/describes/
but maybe better:
"contains the command, parameters, and a pointer to data"
?
yes, thanks
Also this is format-1 only, isn't it?
yes, I add this
+ * - IRB : Interuption response Block, describe the result of an operation
s/describe/describes/
again!
thanks :)
+ * hold a SCSW and several channel type dependent fields.
s/hold/holds/
hum...
s/several channel type dependent fields/model-dependent data/ ?
yes
+ * - SCHIB: SubChannel Information Block composed of:
s/SubChannel/Subchannel/
+ * - SCSW: SubChannel Status Word, status of the channel as a result of an
s/SubChannel/Subchannel/
OK
+ * operation when in IRB.
I think that description is a bit confusing: An SCSW always contains
the subchannel status; it's just when it is contained in an IRB that
the status is associated to the last event on that subchannel (as the
result of an operation, or as an unsolicited status.)
yes, the "when in IRB has nothing to do there" IRB defintion is above.
I will make it clearer
+ * - PMCW: Path Management Control Word
+ * You need the QEMU ccw-pong device in QEMU to answer the I/O transfers.
+ */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+
+#include <css.h>
+
+static const char *scsw_str = "kkkkslccfpixuzen";
+static const char *scsw_str2 = "1SHCrshcsdsAIPSs";
Nice, random strings? :)
these are the bit definitions associated with the scsw flags.
Easier to read than a bit field ... as long as you have the
documentation (Pop)
Thanks a lot for the review.
Seems I have some work for the v3
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
