[kvm-unit-tests PATCH v1 0/6] s390x: basic test infrastructure

[kvm-unit-tests PATCH v1 0/6] s390x: basic test infrastructure

[David Hildenbrand]

This adds the very basic infrastructure for s390x.

For now, only a basic self test is provided. This one will also run under
TCG.

The following, previously sent patches are required:

- [PATCH kvm-unit-tests] Makefile: drop "-lpthread -lrt"
- [kvm-unit-tests PATCH v1 0/3] provide asm-generic spinlock
- [kvm-unit-tests PATCH v1] lib: fix compilation warning

Parts taken from a prototype by Thomas Huth.


David Hildenbrand (6):
  s390x: initial infrastructure
  s390x: basic self test
  s390x: copy sclp.h and sclp-ascii.c from QEMU
  s390x: compile sclp-ascii.c
  s390x: wire up sclp console output
  MAINTAINERS: add s390x maintainers

 MAINTAINERS              |   7 +++
 configure                |   2 +-
 lib/s390x/asm/barrier.h  |  16 +++++++
 lib/s390x/asm/io.h       |  17 ++++++++
 lib/s390x/asm/page.h     |  16 +++++++
 lib/s390x/asm/spinlock.h |  16 +++++++
 lib/s390x/asm/stack.h    |  21 +++++++++
 lib/s390x/io.c           |  49 +++++++++++++++++++++
 lib/s390x/sclp-ascii.c   |  68 +++++++++++++++++++++++++++++
 lib/s390x/sclp.h         | 108 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/stack.c        |  28 ++++++++++++
 s390x/Makefile           |  35 +++++++++++++++
 s390x/cstart64.S         |  39 +++++++++++++++++
 s390x/flat.lds           |  42 ++++++++++++++++++
 s390x/run                |  56 ++++++++++++++++++++++++
 s390x/selftest.c         |  31 ++++++++++++++
 s390x/unittests.cfg      |  24 +++++++++++
 17 files changed, 574 insertions(+), 1 deletion(-)
 create mode 100644 lib/s390x/asm/barrier.h
 create mode 100644 lib/s390x/asm/io.h
 create mode 100644 lib/s390x/asm/page.h
 create mode 100644 lib/s390x/asm/spinlock.h
 create mode 100644 lib/s390x/asm/stack.h
 create mode 100644 lib/s390x/io.c
 create mode 100644 lib/s390x/sclp-ascii.c
 create mode 100644 lib/s390x/sclp.h
 create mode 100644 lib/s390x/stack.c
 create mode 100644 s390x/Makefile
 create mode 100644 s390x/cstart64.S
 create mode 100644 s390x/flat.lds
 create mode 100755 s390x/run
 create mode 100644 s390x/selftest.c
 create mode 100644 s390x/unittests.cfg

-- 
2.9.3

[kvm-unit-tests PATCH v1 1/6] s390x: initial infrastructure

[David Hildenbrand]

This adds the very basic infrastructure for s390x. As we compile for
z900, also QEMU tcg can be used.

To cross compile:

$./configure --arch=s390x --cross-prefix=/usr/bin/s390x-linux-gnu-
$ make

Please note that for now, nothing will be compiled, as there is no test
to compile. A basic self test will be added in the following patches.

smp and stack unwinding is not supported yet. sclp console output will
be added in the following patches.

Parts based on a prototype by Thomas Huth.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 configure                |  2 +-
 lib/s390x/asm/barrier.h  | 16 ++++++++++++++
 lib/s390x/asm/io.h       | 17 +++++++++++++++
 lib/s390x/asm/page.h     | 16 ++++++++++++++
 lib/s390x/asm/spinlock.h | 16 ++++++++++++++
 lib/s390x/asm/stack.h    | 21 ++++++++++++++++++
 lib/s390x/io.c           | 48 +++++++++++++++++++++++++++++++++++++++++
 lib/s390x/stack.c        | 28 ++++++++++++++++++++++++
 s390x/Makefile           | 32 +++++++++++++++++++++++++++
 s390x/cstart64.S         | 39 +++++++++++++++++++++++++++++++++
 s390x/flat.lds           | 42 ++++++++++++++++++++++++++++++++++++
 s390x/run                | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg      | 19 ++++++++++++++++
 13 files changed, 351 insertions(+), 1 deletion(-)
 create mode 100644 lib/s390x/asm/barrier.h
 create mode 100644 lib/s390x/asm/io.h
 create mode 100644 lib/s390x/asm/page.h
 create mode 100644 lib/s390x/asm/spinlock.h
 create mode 100644 lib/s390x/asm/stack.h
 create mode 100644 lib/s390x/io.c
 create mode 100644 lib/s390x/stack.c
 create mode 100644 s390x/Makefile
 create mode 100644 s390x/cstart64.S
 create mode 100644 s390x/flat.lds
 create mode 100755 s390x/run
 create mode 100644 s390x/unittests.cfg

diff --git a/configure b/configure
index d152414..3690041 100755
--- a/configure
+++ b/configure
@@ -7,7 +7,7 @@ objcopy=objcopy
 objdump=objdump
 ar=ar
 addr2line=addr2line
-arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
+arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/;s/s390x.*/s390x/'`
 host=$arch
 cross_prefix=
 endian=""
diff --git a/lib/s390x/asm/barrier.h b/lib/s390x/asm/barrier.h
new file mode 100644
index 0000000..5e77984
--- /dev/null
+++ b/lib/s390x/asm/barrier.h
@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#include <asm-generic/barrier.h>
+
+#ifndef _ASM_S390X_BARRIER_H_
+#define _ASM_S390X_BARRIER_H_
+
+#endif
diff --git a/lib/s390x/asm/io.h b/lib/s390x/asm/io.h
new file mode 100644
index 0000000..feb4115
--- /dev/null
+++ b/lib/s390x/asm/io.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#ifndef _ASMS390X_IO_H_
+#define _ASMS390X_IO_H_
+
+#define __iomem
+
+#include <asm-generic/io.h>
+#endif
diff --git a/lib/s390x/asm/page.h b/lib/s390x/asm/page.h
new file mode 100644
index 0000000..c67b251
--- /dev/null
+++ b/lib/s390x/asm/page.h
@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#include <asm-generic/page.h>
+
+#ifndef _ASMS390X_PAGE_H_
+#define _ASMS390X_PAGE_H_
+
+#endif
diff --git a/lib/s390x/asm/spinlock.h b/lib/s390x/asm/spinlock.h
new file mode 100644
index 0000000..9d40a94
--- /dev/null
+++ b/lib/s390x/asm/spinlock.h
@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#include <asm-generic/spinlock.h>
+
+#ifndef __ASMS390X_SPINLOCK_H
+#define __ASMS390X_SPINLOCK_H
+
+#endif
diff --git a/lib/s390x/asm/stack.h b/lib/s390x/asm/stack.h
new file mode 100644
index 0000000..e36d975
--- /dev/null
+++ b/lib/s390x/asm/stack.h
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#ifndef _ASMS390X_STACK_H_
+#define _ASMS390X_STACK_H_
+
+#ifndef _STACK_H_
+#error Do not directly include <asm/stack.h>. Just use <stack.h>.
+#endif
+
+#define HAVE_ARCH_BACKTRACE_FRAME
+#define HAVE_ARCH_BACKTRACE
+
+#endif
diff --git a/lib/s390x/io.c b/lib/s390x/io.c
new file mode 100644
index 0000000..a652124
--- /dev/null
+++ b/lib/s390x/io.c
@@ -0,0 +1,48 @@
+/*
+ * s390x io implementation
+ *
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#include <libcflat.h>
+#include <asm/spinlock.h>
+
+extern void setup_args_progname(const char *args);
+extern char ipl_args[];
+
+static struct spinlock lock;
+
+void puts(const char *s)
+{
+	spin_lock(&lock);
+	/* FIXME */
+	(void)s;
+	spin_unlock(&lock);
+}
+
+static void sigp_stop()
+{
+	register unsigned long status asm ("1") = 0;
+	register unsigned long cpu asm ("2") = 0;
+
+	asm volatile(
+		"	sigp %0,%1,0(%2)\n"
+		: "+d" (status)  : "d" (cpu), "d" (5) : "cc");
+}
+
+void setup()
+{
+	setup_args_progname(ipl_args);
+}
+
+void exit(int code)
+{
+	printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);
+	sigp_stop();
+}
diff --git a/lib/s390x/stack.c b/lib/s390x/stack.c
new file mode 100644
index 0000000..cd34b20
--- /dev/null
+++ b/lib/s390x/stack.c
@@ -0,0 +1,28 @@
+/*
+ * s390x stack implementation
+ *
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#include <libcflat.h>
+#include <stack.h>
+
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
+{
+	printf("TODO: Implement backtrace_frame(%p, %p, %d) function!\n",
+	       frame, return_addrs, max_depth);
+	return 0;
+}
+
+int backtrace(const void **return_addrs, int max_depth)
+{
+	printf("TODO: Implement backtrace(%p, %d) function!\n",
+	       return_addrs, max_depth);
+	return 0;
+}
diff --git a/s390x/Makefile b/s390x/Makefile
new file mode 100644
index 0000000..f9468bb
--- /dev/null
+++ b/s390x/Makefile
@@ -0,0 +1,32 @@
+all: test_cases
+
+test_cases: $(tests)
+
+CFLAGS += -std=gnu99
+CFLAGS += -ffreestanding
+CFLAGS += -Wextra
+CFLAGS += -I lib
+CFLAGS += -O2
+CFLAGS += -msoft-float
+CFLAGS += -march=z900
+LDFLAGS += -nostdlib
+
+# We want to keep intermediate files
+.PRECIOUS: %.o
+
+cflatobjs += lib/util.o
+cflatobjs += lib/alloc.o
+cflatobjs += lib/s390x/io.o
+cflatobjs += lib/s390x/stack.o
+
+cstart.o = $(TEST_DIR)/cstart64.o
+
+FLATLIBS = $(libcflat)
+%.elf: %.o $(FLATLIBS) s390x/flat.lds $(cstart.o)
+	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) lib/auxinfo.c -DPROGNAME=\"$@\"
+	$(CC) $(LDFLAGS) -o $@ -T s390x/flat.lds -Ttext=0x10000 \
+		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
+	$(RM) $(@:.elf=.aux.o)
+
+arch_clean:
+	$(RM) $(TEST_DIR)/*.{o,elf} $(TEST_DIR)/.*.d lib/s390x/.*.d
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
new file mode 100644
index 0000000..b5741c2
--- /dev/null
+++ b/s390x/cstart64.S
@@ -0,0 +1,39 @@
+/*
+ * s390x startup code
+ *
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+.section .init
+
+/* entry point - for KVM + TCG we directly start in 64 bit mode */
+	.globl start
+start:
+	/* setup stack */
+	larl	%r15, stacktop
+	/* setup initial PSW mask */
+	larl	%r1, initital_psw
+	lpswe	0(%r1)
+init_psw_cont:
+	/* call setup() */
+	brasl	%r14, setup
+	/* forward test parameter */
+	larl	%r2, __argc
+	llgf	%r2, 0(%r2)
+	larl	%r3, __argv
+	/* call to main() */
+	brasl	%r14, main
+	/* forward exit code */
+	lgr	%r3, %r2
+	/* call exit() */
+	j exit
+
+	.align	8
+initital_psw:
+	.quad	0x0000000180000000, init_psw_cont
diff --git a/s390x/flat.lds b/s390x/flat.lds
new file mode 100644
index 0000000..b6e2172
--- /dev/null
+++ b/s390x/flat.lds
@@ -0,0 +1,42 @@
+SECTIONS
+{
+	.text : {
+		*(.init)
+		. = 0x480;
+		ipl_args = .;
+		. = 0x600;
+		*(.text)
+		*(.text.*)
+	}
+	. = ALIGN(64K);
+	etext = .;
+	.opd : { *(.opd) }
+	. = ALIGN(16);
+	.dynamic : {
+		dynamic_start = .;
+		*(.dynamic)
+	}
+	.dynsym : {
+		dynsym_start = .;
+		*(.dynsym)
+	}
+	.rela.dyn : { *(.rela*) }
+	. = ALIGN(16);
+	.data : {
+		*(.data)
+		*(.data.rel*)
+	}
+	. = ALIGN(16);
+	.rodata : { *(.rodata) *(.rodata.*) }
+	. = ALIGN(16);
+	.bss : { *(.bss) }
+	. = ALIGN(64K);
+	edata = .;
+	. += 64K;
+	. = ALIGN(64K);
+	/*
+	 * stackptr set with initial stack frame preallocated
+	 */
+	stackptr = . - 160;
+	stacktop = .;
+}
diff --git a/s390x/run b/s390x/run
new file mode 100755
index 0000000..cf333de
--- /dev/null
+++ b/s390x/run
@@ -0,0 +1,56 @@
+#!/usr/bin/env bash
+
+if [ -z "$STANDALONE" ]; then
+	if [ ! -f config.mak ]; then
+		echo "run ./configure && make first. See ./configure -h"
+		exit 2
+	fi
+	source config.mak
+	source scripts/arch-run.bash
+fi
+
+if [ -c /dev/kvm ]; then
+	if [ "$HOST" = "s390x" ] && [ "$ARCH" = "s390x" ]; then
+		kvm_available=yes
+	fi
+fi
+
+if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
+	echo "KVM is needed, but not available on this host"
+	exit 2
+fi
+
+if [ -z "$ACCEL" ]; then
+	if [ "$kvm_available" = "yes" ]; then
+		ACCEL="kvm"
+	else
+		ACCEL="tcg"
+	fi
+fi
+
+qemu=$(search_qemu_binary)
+
+M='-machine s390-ccw-virtio'
+M+=",accel=$ACCEL"
+command="$qemu -nodefaults -nographic $M"
+command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"
+[ -f "$ENV" ] && command+=" -initrd $ENV"
+command+=" -kernel"
+command="$(timeout_cmd) $command"
+echo $command "$@"
+
+# We return the exit code via stdout, not via the QEMU return code
+lines=$(run_qemu $command "$@")
+ret=$?
+echo "$lines"
+if [ $ret -eq 1 ]; then
+	testret=$(grep '^EXIT: ' <<<"$lines" | sed 's/.*STATUS=\([0-9][0-9]*\).*/\1/')
+	if [ "$testret" ]; then
+		if [ $testret -eq 1 ]; then
+			ret=0
+		else
+			ret=$testret
+		fi
+	fi
+fi
+exit $ret
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
new file mode 100644
index 0000000..b1e0b1e
--- /dev/null
+++ b/s390x/unittests.cfg
@@ -0,0 +1,19 @@
+##############################################################################
+# unittest configuration
+#
+# [unittest_name]
+# file = <name>.elf		# Name of the elf file to be used.
+# extra_params = -append <params...>	# Additional parameters used.
+# groups = <group_name1> <group_name2> ... # Used to identify test cases
+#					   # with run_tests -g ...
+#					   # Specify group_name=nodefault
+#					   # to have test not run by default
+# accel = kvm|tcg		# Optionally specify if test must run with
+#				# kvm or tcg. If not specified, then kvm will
+#				# be used when available.
+# timeout = <duration>		# Optionally specify a timeout.
+# check = <path>=<value> # check a file for a particular value before running
+#			 # a test. The check line can contain multiple files
+#			 # to check separated by a space but each check
+#			 # parameter needs to be of the form <path>=<value>
+##############################################################################
-- 
2.9.3

[kvm-unit-tests PATCH v1 2/6] s390x: basic self test

[David Hildenbrand]

Test if the general infrastructure is working. The test will fail until
we have proper sclp console output.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 s390x/Makefile      |  2 ++
 s390x/selftest.c    | 31 +++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  5 +++++
 3 files changed, 38 insertions(+)
 create mode 100644 s390x/selftest.c

diff --git a/s390x/Makefile b/s390x/Makefile
index f9468bb..549c1c0 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -1,3 +1,5 @@
+tests = $(TEST_DIR)/selftest.elf
+
 all: test_cases
 
 test_cases: $(tests)
diff --git a/s390x/selftest.c b/s390x/selftest.c
new file mode 100644
index 0000000..827ef60
--- /dev/null
+++ b/s390x/selftest.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#include <libcflat.h>
+#include <util.h>
+
+int main(int argc, char**argv)
+{
+	report_prefix_push("selftest");
+
+	if (argc != 3)
+		report_abort("Wrong number of arguments");
+
+	if (strcmp(argv[0], "s390x/selftest.elf") != 0)
+		report_abort("wrong program name");
+	if (strcmp(argv[1], "test") != 0)
+		report_abort("wrong parameter value");
+	if (strcmp(argv[2], "123") != 0)
+		report_abort("wrong parameter value");
+
+	report("test true", true, 0);
+
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index b1e0b1e..92e01ab 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -17,3 +17,8 @@
 #			 # to check separated by a space but each check
 #			 # parameter needs to be of the form <path>=<value>
 ##############################################################################
+
+[selftest-setup]
+file = selftest.elf
+groups = selftest
+extra_params = -append 'test 123'
-- 
2.9.3

[kvm-unit-tests PATCH v1 3/6] s390x: copy sclp.h and sclp-ascii.c from QEMU

[David Hildenbrand]

The s390x-ccw pcbios already has what we need to print to the sclp
console. Let's copy the files and adjust them to our needs later.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 lib/s390x/sclp-ascii.c |  82 +++++++++++++++++++++++++++++++++++++
 lib/s390x/sclp.h       | 107 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 189 insertions(+)
 create mode 100644 lib/s390x/sclp-ascii.c
 create mode 100644 lib/s390x/sclp.h

diff --git a/lib/s390x/sclp-ascii.c b/lib/s390x/sclp-ascii.c
new file mode 100644
index 0000000..dc1c3e4
--- /dev/null
+++ b/lib/s390x/sclp-ascii.c
@@ -0,0 +1,82 @@
+/*
+ * SCLP ASCII access driver
+ *
+ * Copyright (c) 2013 Alexander Graf <agraf@suse.de>
+ *
+ * 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.
+ */
+
+#include "s390-ccw.h"
+#include "sclp.h"
+
+static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
+
+/* Perform service call. Return 0 on success, non-zero otherwise. */
+static int sclp_service_call(unsigned int command, void *sccb)
+{
+        int cc;
+
+        asm volatile(
+                "       .insn   rre,0xb2200000,%1,%2\n"  /* servc %1,%2 */
+                "       ipm     %0\n"
+                "       srl     %0,28"
+                : "=&d" (cc) : "d" (command), "a" (__pa(sccb))
+                : "cc", "memory");
+        consume_sclp_int();
+        if (cc == 3)
+                return -EIO;
+        if (cc == 2)
+                return -EBUSY;
+        return 0;
+}
+
+static void sclp_set_write_mask(void)
+{
+    WriteEventMask *sccb = (void *)_sccb;
+
+    sccb->h.length = sizeof(WriteEventMask);
+    sccb->mask_length = sizeof(unsigned int);
+    sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
+    sccb->cp_receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
+    sccb->send_mask = SCLP_EVENT_MASK_MSG_ASCII;
+    sccb->cp_send_mask = SCLP_EVENT_MASK_MSG_ASCII;
+
+    sclp_service_call(SCLP_CMD_WRITE_EVENT_MASK, sccb);
+}
+
+void sclp_setup(void)
+{
+    sclp_set_write_mask();
+}
+
+static int _strlen(const char *str)
+{
+    int i;
+    for (i = 0; *str; i++)
+        str++;
+    return i;
+}
+
+static void _memcpy(char *dest, const char *src, int len)
+{
+    int i;
+    for (i = 0; i < len; i++)
+        dest[i] = src[i];
+}
+
+void sclp_print(const char *str)
+{
+    int len = _strlen(str);
+    WriteEventData *sccb = (void *)_sccb;
+
+    sccb->h.length = sizeof(WriteEventData) + len;
+    sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
+    sccb->ebh.length = sizeof(EventBufferHeader) + len;
+    sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
+    sccb->ebh.flags = 0;
+    _memcpy(sccb->data, str, len);
+
+    sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
+}
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
new file mode 100644
index 0000000..3cbfb78
--- /dev/null
+++ b/lib/s390x/sclp.h
@@ -0,0 +1,107 @@
+/*
+ * SCLP ASCII access driver
+ *
+ * Copyright (c) 2013 Alexander Graf <agraf@suse.de>
+ *
+ * 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 SCLP_H
+#define SCLP_H
+
+/* SCLP command codes */
+#define SCLP_CMDW_READ_SCP_INFO                 0x00020001
+#define SCLP_CMDW_READ_SCP_INFO_FORCED          0x00120001
+#define SCLP_CMD_READ_EVENT_DATA                0x00770005
+#define SCLP_CMD_WRITE_EVENT_DATA               0x00760005
+#define SCLP_CMD_READ_EVENT_DATA                0x00770005
+#define SCLP_CMD_WRITE_EVENT_DATA               0x00760005
+#define SCLP_CMD_WRITE_EVENT_MASK               0x00780005
+
+/* SCLP response codes */
+#define SCLP_RC_NORMAL_READ_COMPLETION          0x0010
+#define SCLP_RC_NORMAL_COMPLETION               0x0020
+#define SCLP_RC_INVALID_SCLP_COMMAND            0x01f0
+#define SCLP_RC_CONTAINED_EQUIPMENT_CHECK       0x0340
+#define SCLP_RC_INSUFFICIENT_SCCB_LENGTH        0x0300
+#define SCLP_RC_INVALID_FUNCTION                0x40f0
+#define SCLP_RC_NO_EVENT_BUFFERS_STORED         0x60f0
+#define SCLP_RC_INVALID_SELECTION_MASK          0x70f0
+#define SCLP_RC_INCONSISTENT_LENGTHS            0x72f0
+#define SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR       0x73f0
+#define SCLP_RC_INVALID_MASK_LENGTH             0x74f0
+
+/* Service Call Control Block (SCCB) and its elements */
+
+#define SCCB_SIZE 4096
+
+#define SCLP_VARIABLE_LENGTH_RESPONSE           0x80
+#define SCLP_EVENT_BUFFER_ACCEPTED              0x80
+
+#define SCLP_FC_NORMAL_WRITE                    0
+
+typedef struct SCCBHeader {
+    uint16_t length;
+    uint8_t function_code;
+    uint8_t control_mask[3];
+    uint16_t response_code;
+} __attribute__((packed)) SCCBHeader;
+
+#define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader))
+
+typedef struct ReadInfo {
+    SCCBHeader h;
+    uint16_t rnmax;
+    uint8_t rnsize;
+} __attribute__((packed)) ReadInfo;
+
+typedef struct SCCB {
+    SCCBHeader h;
+    char data[SCCB_DATA_LEN];
+ } __attribute__((packed)) SCCB;
+
+/* SCLP event types */
+#define SCLP_EVENT_ASCII_CONSOLE_DATA           0x1a
+#define SCLP_EVENT_SIGNAL_QUIESCE               0x1d
+
+/* SCLP event masks */
+#define SCLP_EVENT_MASK_SIGNAL_QUIESCE          0x00000008
+#define SCLP_EVENT_MASK_MSG_ASCII               0x00000040
+
+#define SCLP_UNCONDITIONAL_READ                 0x00
+#define SCLP_SELECTIVE_READ                     0x01
+
+typedef struct WriteEventMask {
+    SCCBHeader h;
+    uint16_t _reserved;
+    uint16_t mask_length;
+    uint32_t cp_receive_mask;
+    uint32_t cp_send_mask;
+    uint32_t send_mask;
+    uint32_t receive_mask;
+} __attribute__((packed)) WriteEventMask;
+
+typedef struct EventBufferHeader {
+    uint16_t length;
+    uint8_t  type;
+    uint8_t  flags;
+    uint16_t _reserved;
+} __attribute__((packed)) EventBufferHeader;
+
+typedef struct WriteEventData {
+    SCCBHeader h;
+    EventBufferHeader ebh;
+    char data[0];
+} __attribute__((packed)) WriteEventData;
+
+typedef struct ReadEventData {
+    SCCBHeader h;
+    EventBufferHeader ebh;
+    uint32_t mask;
+} __attribute__((packed)) ReadEventData;
+
+#define __pa(x) (x)
+
+#endif /* SCLP_H */
-- 
2.9.3

[kvm-unit-tests PATCH v1 4/6] s390x: compile sclp-ascii.c

[David Hildenbrand]

We can reuse strlen and memcpy. __pa(x) is already defined in page.h
We don't have (and need) defines for error values and consuming sclp
irqs is not required, as we won't be passing control to some other OS.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 lib/s390x/sclp-ascii.c | 28 +++++++---------------------
 lib/s390x/sclp.h       |  2 --
 s390x/Makefile         |  1 +
 3 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/lib/s390x/sclp-ascii.c b/lib/s390x/sclp-ascii.c
index dc1c3e4..dfb9e68 100644
--- a/lib/s390x/sclp-ascii.c
+++ b/lib/s390x/sclp-ascii.c
@@ -8,7 +8,9 @@
  * directory.
  */
 
-#include "s390-ccw.h"
+#include <libcflat.h>
+#include <string.h>
+#include <asm/page.h>
 #include "sclp.h"
 
 static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
@@ -24,11 +26,10 @@ static int sclp_service_call(unsigned int command, void *sccb)
                 "       srl     %0,28"
                 : "=&d" (cc) : "d" (command), "a" (__pa(sccb))
                 : "cc", "memory");
-        consume_sclp_int();
         if (cc == 3)
-                return -EIO;
+                return -1;
         if (cc == 2)
-                return -EBUSY;
+                return -1;
         return 0;
 }
 
@@ -51,24 +52,9 @@ void sclp_setup(void)
     sclp_set_write_mask();
 }
 
-static int _strlen(const char *str)
-{
-    int i;
-    for (i = 0; *str; i++)
-        str++;
-    return i;
-}
-
-static void _memcpy(char *dest, const char *src, int len)
-{
-    int i;
-    for (i = 0; i < len; i++)
-        dest[i] = src[i];
-}
-
 void sclp_print(const char *str)
 {
-    int len = _strlen(str);
+    int len = strlen(str);
     WriteEventData *sccb = (void *)_sccb;
 
     sccb->h.length = sizeof(WriteEventData) + len;
@@ -76,7 +62,7 @@ void sclp_print(const char *str)
     sccb->ebh.length = sizeof(EventBufferHeader) + len;
     sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
     sccb->ebh.flags = 0;
-    _memcpy(sccb->data, str, len);
+    memcpy(sccb->data, str, len);
 
     sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
 }
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index 3cbfb78..c48cc21 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -102,6 +102,4 @@ typedef struct ReadEventData {
     uint32_t mask;
 } __attribute__((packed)) ReadEventData;
 
-#define __pa(x) (x)
-
 #endif /* SCLP_H */
diff --git a/s390x/Makefile b/s390x/Makefile
index 549c1c0..0f72c9e 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -20,6 +20,7 @@ cflatobjs += lib/util.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/s390x/io.o
 cflatobjs += lib/s390x/stack.o
+cflatobjs += lib/s390x/sclp-ascii.o
 
 cstart.o = $(TEST_DIR)/cstart64.o
 
-- 
2.9.3

[kvm-unit-tests PATCH v1 5/6] s390x: wire up sclp console output

[David Hildenbrand]

Now the basics should be working and therefore the self test should
pass.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 lib/s390x/io.c   | 5 +++--
 lib/s390x/sclp.h | 3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/s390x/io.c b/lib/s390x/io.c
index a652124..067ecf7 100644
--- a/lib/s390x/io.c
+++ b/lib/s390x/io.c
@@ -12,6 +12,7 @@
  */
 #include <libcflat.h>
 #include <asm/spinlock.h>
+#include "sclp.h"
 
 extern void setup_args_progname(const char *args);
 extern char ipl_args[];
@@ -21,8 +22,7 @@ static struct spinlock lock;
 void puts(const char *s)
 {
 	spin_lock(&lock);
-	/* FIXME */
-	(void)s;
+	sclp_print(s);
 	spin_unlock(&lock);
 }
 
@@ -39,6 +39,7 @@ static void sigp_stop()
 void setup()
 {
 	setup_args_progname(ipl_args);
+	sclp_setup();
 }
 
 void exit(int code)
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index c48cc21..3f4c138 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -102,4 +102,7 @@ typedef struct ReadEventData {
     uint32_t mask;
 } __attribute__((packed)) ReadEventData;
 
+void sclp_setup(void);
+void sclp_print(const char *str);
+
 #endif /* SCLP_H */
-- 
2.9.3

[kvm-unit-tests PATCH v1 6/6] MAINTAINERS: add s390x maintainers

[David Hildenbrand]

Add Thomas and myself as maintainers for now.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b86dea7..207bcd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -82,3 +82,10 @@ M: Radim Krčmář <rkrcmar@redhat.com>
 L: kvm@vger.kernel.org
 F: x86/*
 F: lib/x86/*
+
+S390X
+M: Thomas Huth <thuth@redhat.com>
+M: David Hildenbrand <david@redhat.com>
+L: kvm@vger.kernel.org
+F: s390x/*
+F: lib/s390x/*
-- 
2.9.3

Re: [kvm-unit-tests PATCH v1 1/6] s390x: initial infrastructure

[Christian Borntraeger]

Nice to see somebody doing this for s390x.


On 05/12/2017 12:58 PM, David Hildenbrand wrote:
[...]

> diff --git a/s390x/Makefile b/s390x/Makefile
> new file mode 100644
> index 0000000..f9468bb
> --- /dev/null
> +++ b/s390x/Makefile
> @@ -0,0 +1,32 @@
> +all: test_cases
> +
> +test_cases: $(tests)
> +
> +CFLAGS += -std=gnu99
> +CFLAGS += -ffreestanding
> +CFLAGS += -Wextra
> +CFLAGS += -I lib
> +CFLAGS += -O2
> +CFLAGS += -msoft-float


If you enable the AFP register control bit in your startup code, you can probably
avoid the soft-float thing.

Re: [kvm-unit-tests PATCH v1 1/6] s390x: initial infrastructure

[David Hildenbrand]

On 15.05.2017 11:19, Christian Borntraeger wrote:
> Nice to see somebody doing this for s390x.
> 
> 
> On 05/12/2017 12:58 PM, David Hildenbrand wrote:
> [...]
> 
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> new file mode 100644
>> index 0000000..f9468bb
>> --- /dev/null
>> +++ b/s390x/Makefile
>> @@ -0,0 +1,32 @@
>> +all: test_cases
>> +
>> +test_cases: $(tests)
>> +
>> +CFLAGS += -std=gnu99
>> +CFLAGS += -ffreestanding
>> +CFLAGS += -Wextra
>> +CFLAGS += -I lib
>> +CFLAGS += -O2
>> +CFLAGS += -msoft-float
> 
> 
> If you enable the AFP register control bit in your startup code, you can probably
> avoid the soft-float thing.
> 

Looks like we can drop that completely.

a) no floating point operations are used either way.

b) adding a very basic division results in

static void test_fp(void)
{
	double a, b;

	b = 6.5;
	a = 2.0;

	b = b / a;

	report("test fp", b == 3.25);
}

(compiled with -O0)

s390x/selftest.o: In function `test_fp':
/home/dhildenb/git/kvm-unit-tests/s390x/selftest.c:21: undefined
reference to `__divdf3'
/home/dhildenb/git/kvm-unit-tests/s390x/selftest.c:23: undefined
reference to `__eqdf2'

So we would need a soft float lib (as we're using -nostdlib)

c) dropping -msoft-float

A DDB (DIVIDE long BFP) is generated. That seems to work under TCG
without any exception. I assume the check for AFP-register control is
missing. I assume, that test would not pass under KVM.


For now I will simply drop -msoft-float.

Thanks!

-- 

Thanks,

David

Re: [kvm-unit-tests PATCH v1 1/6] s390x: initial infrastructure

[Christian Borntraeger]

On 05/16/2017 09:54 AM, David Hildenbrand wrote:
> On 15.05.2017 11:19, Christian Borntraeger wrote:
>> Nice to see somebody doing this for s390x.
>>
>>
>> On 05/12/2017 12:58 PM, David Hildenbrand wrote:
>> [...]
>>
>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>> new file mode 100644
>>> index 0000000..f9468bb
>>> --- /dev/null
>>> +++ b/s390x/Makefile
>>> @@ -0,0 +1,32 @@
>>> +all: test_cases
>>> +
>>> +test_cases: $(tests)
>>> +
>>> +CFLAGS += -std=gnu99
>>> +CFLAGS += -ffreestanding
>>> +CFLAGS += -Wextra
>>> +CFLAGS += -I lib
>>> +CFLAGS += -O2
>>> +CFLAGS += -msoft-float
>>
>>
>> If you enable the AFP register control bit in your startup code, you can probably
>> avoid the soft-float thing.
>>
> 
> Looks like we can drop that completely.
> 
> a) no floating point operations are used either way.
> 
> b) adding a very basic division results in
> 
> static void test_fp(void)
> {
> 	double a, b;
> 
> 	b = 6.5;
> 	a = 2.0;
> 
> 	b = b / a;
> 
> 	report("test fp", b == 3.25);
> }
> 
> (compiled with -O0)
> 
> s390x/selftest.o: In function `test_fp':
> /home/dhildenb/git/kvm-unit-tests/s390x/selftest.c:21: undefined
> reference to `__divdf3'
> /home/dhildenb/git/kvm-unit-tests/s390x/selftest.c:23: undefined
> reference to `__eqdf2'
> 
> So we would need a soft float lib (as we're using -nostdlib)
> 
> c) dropping -msoft-float
> 
> A DDB (DIVIDE long BFP) is generated. That seems to work under TCG
> without any exception. I assume the check for AFP-register control is
> missing. I assume, that test would not pass under KVM.
> 
> 
> For now I will simply drop -msoft-float.

This will allow gcc to spill/fill into floating point registers, which are
not all available without afp-register control. It will probably work because
the gr->fpr move will not be used for -march=z900, but I suggest to not rely
on that.

Re: [kvm-unit-tests PATCH v1 1/6] s390x: initial infrastructure

[David Hildenbrand]

On 16.05.2017 09:58, Christian Borntraeger wrote:
> On 05/16/2017 09:54 AM, David Hildenbrand wrote:
>> On 15.05.2017 11:19, Christian Borntraeger wrote:
>>> Nice to see somebody doing this for s390x.
>>>
>>>
>>> On 05/12/2017 12:58 PM, David Hildenbrand wrote:
>>> [...]
>>>
>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>> new file mode 100644
>>>> index 0000000..f9468bb
>>>> --- /dev/null
>>>> +++ b/s390x/Makefile
>>>> @@ -0,0 +1,32 @@
>>>> +all: test_cases
>>>> +
>>>> +test_cases: $(tests)
>>>> +
>>>> +CFLAGS += -std=gnu99
>>>> +CFLAGS += -ffreestanding
>>>> +CFLAGS += -Wextra
>>>> +CFLAGS += -I lib
>>>> +CFLAGS += -O2
>>>> +CFLAGS += -msoft-float
>>>
>>>
>>> If you enable the AFP register control bit in your startup code, you can probably
>>> avoid the soft-float thing.
>>>
>>
>> Looks like we can drop that completely.
>>
>> a) no floating point operations are used either way.
>>
>> b) adding a very basic division results in
>>
>> static void test_fp(void)
>> {
>> 	double a, b;
>>
>> 	b = 6.5;
>> 	a = 2.0;
>>
>> 	b = b / a;
>>
>> 	report("test fp", b == 3.25);
>> }
>>
>> (compiled with -O0)
>>
>> s390x/selftest.o: In function `test_fp':
>> /home/dhildenb/git/kvm-unit-tests/s390x/selftest.c:21: undefined
>> reference to `__divdf3'
>> /home/dhildenb/git/kvm-unit-tests/s390x/selftest.c:23: undefined
>> reference to `__eqdf2'
>>
>> So we would need a soft float lib (as we're using -nostdlib)
>>
>> c) dropping -msoft-float
>>
>> A DDB (DIVIDE long BFP) is generated. That seems to work under TCG
>> without any exception. I assume the check for AFP-register control is
>> missing. I assume, that test would not pass under KVM.
>>
>>
>> For now I will simply drop -msoft-float.
> 
> This will allow gcc to spill/fill into floating point registers, which are
> not all available without afp-register control. It will probably work because
> the gr->fpr move will not be used for -march=z900, but I suggest to not rely
> on that.
> 

Good point, so enabling AFP sounds right. And that should be valid on
any z/Architecture system (base feature set).

Thanks!

-- 

Thanks,

David

Re: [kvm-unit-tests PATCH v1 2/6] s390x: basic self test

[Radim Krčmář]

2017-05-12 12:58+0200, David Hildenbrand:
> Test if the general infrastructure is working. The test will fail until
> we have proper sclp console output.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> @@ -0,0 +1,31 @@
> +int main(int argc, char**argv)
> +{
> +	report_prefix_push("selftest");
> +
> +	if (argc != 3)
> +		report_abort("Wrong number of arguments");
> +
> +	if (strcmp(argv[0], "s390x/selftest.elf") != 0)
> +		report_abort("wrong program name");

This is going to fail when executed as a standalone test (argv[0] would
be a temp file name).  No point in checking, IMO.

> +	if (strcmp(argv[1], "test") != 0)
> +		report_abort("wrong parameter value");
> +	if (strcmp(argv[2], "123") != 0)
> +		report_abort("wrong parameter value");
> +
> +	report("test true", true, 0);
                                  ^
It seems you'll be doing v2 -- please remove this zero in it.

(I'll prepare patch that check format of report in order to have
 automatic notifications in the future. :])

Thanks.

Re: [kvm-unit-tests PATCH v1 2/6] s390x: basic self test

[David Hildenbrand]

On 16.05.2017 15:35, Radim Krčmář wrote:
> 2017-05-12 12:58+0200, David Hildenbrand:
>> Test if the general infrastructure is working. The test will fail until
>> we have proper sclp console output.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> diff --git a/s390x/selftest.c b/s390x/selftest.c
>> @@ -0,0 +1,31 @@
>> +int main(int argc, char**argv)
>> +{
>> +	report_prefix_push("selftest");
>> +
>> +	if (argc != 3)
>> +		report_abort("Wrong number of arguments");
>> +
>> +	if (strcmp(argv[0], "s390x/selftest.elf") != 0)
>> +		report_abort("wrong program name");
> 
> This is going to fail when executed as a standalone test (argv[0] would
> be a temp file name).  No point in checking, IMO.

That name is determined during compile time, so it shouldn't change, or
am i wrong? But if you prefer, I can drop it.

> 
>> +	if (strcmp(argv[1], "test") != 0)
>> +		report_abort("wrong parameter value");
>> +	if (strcmp(argv[2], "123") != 0)
>> +		report_abort("wrong parameter value");
>> +
>> +	report("test true", true, 0);
>                                   ^
> It seems you'll be doing v2 -- please remove this zero in it.

Thanks, if that would throw an error during compile time like printf, it
would be perfect.

Yes, I'll resend!

> 
> (I'll prepare patch that check format of report in order to have
>  automatic notifications in the future. :])
> 
> Thanks.
> 


-- 

Thanks,

David

Re: [kvm-unit-tests PATCH v1 2/6] s390x: basic self test

[Radim Krčmář]

2017-05-16 21:16+0200, David Hildenbrand:
> On 16.05.2017 15:35, Radim Krčmář wrote:
> > 2017-05-12 12:58+0200, David Hildenbrand:
> >> Test if the general infrastructure is working. The test will fail until
> >> we have proper sclp console output.
> >>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >> ---
> >> diff --git a/s390x/selftest.c b/s390x/selftest.c
> >> @@ -0,0 +1,31 @@
> >> +int main(int argc, char**argv)
> >> +{
> >> +	report_prefix_push("selftest");
> >> +
> >> +	if (argc != 3)
> >> +		report_abort("Wrong number of arguments");
> >> +
> >> +	if (strcmp(argv[0], "s390x/selftest.elf") != 0)
> >> +		report_abort("wrong program name");
> > 
> > This is going to fail when executed as a standalone test (argv[0] would
> > be a temp file name).  No point in checking, IMO.
> 
> That name is determined during compile time, so it shouldn't change, or
> am i wrong? But if you prefer, I can drop it.

You are right, only x86 uses the actual kernel file name, sorry.

>>> +	if (strcmp(argv[1], "test") != 0)
>>> +		report_abort("wrong parameter value");
>>> +	if (strcmp(argv[2], "123") != 0)
>>> +		report_abort("wrong parameter value");
>>> +
>>> +	report("test true", true, 0);
>>                                   ^
>> It seems you'll be doing v2 -- please remove this zero in it.
> 
> Thanks, if that would throw an error during compile time like printf, it
> would be perfect.
> 
> Yes, I'll resend!

Thinking more about it, the test would look better if we used report()
instead of if()+report_abort() and a final report, something like:

  report("argv[0] == PROGNAME", !strcmp(argv[0], "s390x/selftest.elf"));
  report("argv[1] == test",     !strcmp(argv[1], "test"));
  report("argv[2] == 123",      !strcmp(argv[2], "123"));

This is also a nitpick and I'll gladly accept the original version. :)

Re: [kvm-unit-tests PATCH v1 2/6] s390x: basic self test

[David Hildenbrand]

>>>> +	if (strcmp(argv[1], "test") != 0)
>>>> +		report_abort("wrong parameter value");
>>>> +	if (strcmp(argv[2], "123") != 0)
>>>> +		report_abort("wrong parameter value");
>>>> +
>>>> +	report("test true", true, 0);
>>>                                   ^
>>> It seems you'll be doing v2 -- please remove this zero in it.
>>
>> Thanks, if that would throw an error during compile time like printf, it
>> would be perfect.
>>
>> Yes, I'll resend!
> 
> Thinking more about it, the test would look better if we used report()
> instead of if()+report_abort() and a final report, something like:
> 
>   report("argv[0] == PROGNAME", !strcmp(argv[0], "s390x/selftest.elf"));
>   report("argv[1] == test",     !strcmp(argv[1], "test"));
>   report("argv[2] == 123",      !strcmp(argv[2], "123"));
> 
> This is also a nitpick and I'll gladly accept the original version. :)
> 

Thanks - I'll add that change and send a new version out tomorrow.

-- 

Thanks,

David