On 2020-06-09 09:09, Thomas Huth wrote:
On 08/06/2020 10.12, Pierre Morel wrote:
Provide some definitions and library routines that can be used by
...snip...
+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)
Hmm... What's the "m" (*addr) here good for? %3 is not used in the
assembly code?
addr is %2
"m" (*addr) means memory pointed by addr is read
+ : "cc", "memory");
Why "memory" ? Can this instruction also change the orb?
The orb not but this instruction modifies memory as follow:
orb -> ccw -> data
The CCW can be a READ or a WRITE instruction and the data my be anywhere
in memory (<2G)
A compiler memory barrier is need to avoid write instructions started
before the SSCH instruction to occur after for a write
and memory read made after the instruction to be executed before for a read.
+ 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");
I'm surprised that this does not use "memory" in the clobber list ... I
guess that's what the "=m" (*addr) is good for?
Yes the "=m" (*addr) is there to specify that the SCHIB pointed to by
addr will be modified.
+ 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)
I'm not an expert with these IO instructions, but this looks wrong to me
... Is MSCH reading or writing the SCHIB data?
MSCH is reading the SCHIB data in memory.
...snip...
+/* Debug functions */
+char *dump_pmcw_flags(uint16_t f);
+char *dump_scsw_flags(uint32_t f);
+
+void dump_scsw(struct 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);
In the patch description, you said that DEBUG_CSS needs to be defined
for these - but now DEBUG_CSS is not used in this header... does the
patch description need to be changed?
Yes, thanks, will do.
I removed it because it seems not useful to me.
+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..0c2b64e
--- /dev/null
+++ b/lib/s390x/css_dump.c
@@ -0,0 +1,153 @@
+/*
+ * Channel subsystem structures dumping
+ *
+ * Copyright (c) 2020 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 General Public License version 2.
In the header you used "or any later version" - here it's version 2
only. Maybe you want to standardize one one of the two flavors?
Yes, I will choose the shortest one, v2 only, as it seems to be the most
used in kvm-unit-test.
Thanks for the review.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
