[v1,4/4] s390x: Testing the Subchannel I/O read

Message ID	1573647799-30584-5-git-send-email-pmorel@linux.ibm.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=b7qh=ZF=vger.kernel.org=kvm-owner@kernel.org> Gateway: Authorized Use Only! Violators will be prosecuted for <kvm@vger.kernel.org> from <pmorel@linux.ibm.com>; Wed, 13 Nov 2019 12:23:25 -0000 Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Wed, 13 Nov 2019 12:23:22 -0000 From: Pierre Morel <pmorel@linux.ibm.com> To: kvm@vger.kernel.org Cc: linux-s390@vger.kernel.org, frankja@linux.ibm.com, david@redhat.com, thuth@redhat.com Subject: [PATCH v1 4/4] s390x: Testing the Subchannel I/O read Date: Wed, 13 Nov 2019 13:23:19 +0100 In-Reply-To: <1573647799-30584-1-git-send-email-pmorel@linux.ibm.com> References: <1573647799-30584-1-git-send-email-pmorel@linux.ibm.com> Message-Id: <1573647799-30584-5-git-send-email-pmorel@linux.ibm.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk
Series	s390x: Testing the Subchannel I/O \| expand [v1,0/4] s390x: Testing the Subchannel I/O [v1,2/4] s390x: Define the PSW bits [v1,3/4] s390x:irq: make IRQ handler weak [v1,4/4] s390x: Testing the Subchannel I/O read

Pierre Morel Nov. 13, 2019, 12:23 p.m. UTC

This simple test test the I/O reading by the SUB Channel by:
- initializing the Channel SubSystem with predefined CSSID:
  0xfe000000 CSSID for a Virtual CCW
  0x00090000 SSID for CCW-PONG
- initializing the ORB pointing to a single READ CCW
- starts the STSH command with the ORB
- Expect an interrupt
- writes the read data to output

The test implements lots of traces when DEBUG is on and
tests if memory above the stack is corrupted.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/css.h      | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/css_dump.c | 141 +++++++++++++++++++++++++++++
 s390x/Makefile       |   2 +
 s390x/css.c          | 222 ++++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg  |   4 +
 5 files changed, 613 insertions(+)
 create mode 100644 lib/s390x/css.h
 create mode 100644 lib/s390x/css_dump.c
 create mode 100644 s390x/css.c

Cornelia Huck Nov. 13, 2019, 1:05 p.m. UTC | #1

On Wed, 13 Nov 2019 13:23:19 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> This simple test test the I/O reading by the SUB Channel by:
> - initializing the Channel SubSystem with predefined CSSID:
>   0xfe000000 CSSID for a Virtual CCW

0 should be fine with recent QEMU versions as well, I guess?

>   0x00090000 SSID for CCW-PONG

subchannel id, or subchannel set id?

> - initializing the ORB pointing to a single READ CCW

Out of curiosity: Would using a NOP also be an option?

> - starts the STSH command with the ORB

s/STSH/SSCH/ ?

> - Expect an interrupt
> - writes the read data to output
> 
> The test implements lots of traces when DEBUG is on and
> tests if memory above the stack is corrupted.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/css.h      | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/s390x/css_dump.c | 141 +++++++++++++++++++++++++++++
>  s390x/Makefile       |   2 +
>  s390x/css.c          | 222 ++++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg  |   4 +
>  5 files changed, 613 insertions(+)
>  create mode 100644 lib/s390x/css.h
>  create mode 100644 lib/s390x/css_dump.c
>  create mode 100644 s390x/css.c
> 
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> new file mode 100644
> index 0000000..a7c42fd
> --- /dev/null
> +++ b/lib/s390x/css.h

(...)

> +static inline int rsch(unsigned long schid)

I don't think anyone has tried rsch with QEMU before; sounds like a
good idea to test this :)

> +{
> +	register unsigned long reg1 asm("1") = schid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	rsch\n"
> +		"	ipm	%0\n"
> +		"	srl	%0,28"
> +		: "=d" (ccode)
> +		: "d" (reg1)
> +		: "cc");
> +	return ccode;
> +}
> +
> +static inline int rchp(unsigned long chpid)

Anything useful we can test here?

> +{
> +	register unsigned long reg1 asm("1") = chpid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	rchp\n"
> +		"	ipm	%0\n"
> +		"	srl	%0,28"
> +		: "=d" (ccode)
> +		: "d" (reg1)
> +		: "cc");
> +	return ccode;
> +}

(...)

> diff --git a/s390x/css.c b/s390x/css.c
> new file mode 100644
> index 0000000..6cdaf61
> --- /dev/null
> +++ b/s390x/css.c

(...)

> +static void set_io_irq_subclass_mask(uint64_t const new_mask)
> +{
> +	asm volatile (
> +		"lctlg %%c6, %%c6, %[source]\n"
> +		: /* No outputs */
> +		: [source] "R" (new_mask));
> +}
> +
> +static void set_system_mask(uint8_t new_mask)
> +{
> +	asm volatile (
> +		"ssm %[source]\n"
> +		: /* No outputs */
> +		: [source] "R" (new_mask));
> +}
> +
> +static void enable_io_irq(void)
> +{
> +	set_io_irq_subclass_mask(0x00000000ff000000);

So, you always enable all iscs? Maybe add a comment?

> +	set_system_mask(PSW_PRG_MASK >> 56);
> +}
> +
> +void handle_io_int(sregs_t *regs)
> +{
> +	int ret = 0;
> +
> +	DBG("IO IRQ: subsys_id_word=%08x", lowcore->subsys_id_word);
> +	DBG("......: io_int_parm   =%08x", lowcore->io_int_param);
> +	DBG("......: io_int_word   =%08x", lowcore->io_int_word);
> +	ret = tsch(lowcore->subsys_id_word, &irb);
> +	dump_irb(&irb);
> +	if (ret)
> +		DBG("......: tsch retval %d", ret);
> +	DBG("IO IRQ: END");
> +}
> +
> +static void set_schib(struct schib *sch)
> +{
> +	struct pmcw *p = &sch->pmcw;
> +
> +	p->intparm = 0xdeadbeef;
> +	p->devnum = 0xc0ca;
> +	p->lpm = 0x80;
> +	p->flags = 0x3081;

Use #defines instead of magic numbers?

> +	p->chpid[7] = 0x22;
> +	p->pim = 0x0f;
> +	p->pam = 0x0f;
> +	p->pom = 0x0f;
> +	p->lpm = 0x0f;
> +	p->lpum = 0xaa;
> +	p->pnom = 0xf0;
> +	p->mbi = 0xaa;
> +	p->mbi = 0xaaaa;

Many of these fields are not supposed to be modifiable by the program
-- do you want to check what you get back after msch?

Also, you set mbi twice ;) (And for it to actually have any effect,
you'd have to execute SET CHANNEL MONITOR, no?)


> +}
> +
> +static void css_enable(void)
> +{
> +	int ret;
> +
> +	ret = stsch(CSSID_PONG, &schib);
> +	if (ret)
> +		DBG("stsch: %x\n", ret);
> +	dump_schib(&schib);
> +	set_schib(&schib);
> +	dump_schib(&schib);
> +	ret = msch(CSSID_PONG, &schib);
> +	if (ret)
> +		DBG("msch : %x\n", ret);
> +}
> +
> +/* These two definitions are part of the QEMU PONG interface */
> +#define PONG_WRITE 0x21
> +#define PONG_READ  0x22

Ah, so it's not a plain read/write, but a specialized one? Mention that
in the patch description?

> +
> +static int css_run(int fake)
> +{
> +	struct orb *p = orb;

I'd maybe call that variable 'orb' instead; at a glance, I was confused
what you did with the pmcw below, until I realized that it's the orb :)

> +	int cc;
> +
> +	if (fake)
> +		return 0;
> +	css_enable();
> +
> +	enable_io_irq();
> +
> +	ccw[0].code = PONG_READ;
> +	ccw[0].flags = CCW_F_PCI;
> +	ccw[0].count = 80;
> +	ccw[0].data = (unsigned int)(unsigned long) &buffer;
> +
> +	p->intparm = 0xcafec0ca;
> +	p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
> +	p->cpa = (unsigned int) (unsigned long)&ccw[0];
> +
> +	printf("ORB AT %p\n", orb);
> +	dump_orb(p);
> +	cc = ssch(CSSID_PONG, p);
> +	if (cc) {
> +		DBG("cc: %x\n", cc);
> +		return cc;
> +	}
> +
> +	delay(1);
> +
> +	stsch(CSSID_PONG, &schib);
> +	dump_schib(&schib);
> +	DBG("got: %s\n", buffer);
> +
> +	return 0;
> +}
(...)

Janosch Frank Nov. 14, 2019, 9:15 a.m. UTC | #2

On 11/13/19 1:23 PM, Pierre Morel wrote:
> This simple test test the I/O reading by the SUB Channel by:
> - initializing the Channel SubSystem with predefined CSSID:
>   0xfe000000 CSSID for a Virtual CCW
>   0x00090000 SSID for CCW-PONG
> - initializing the ORB pointing to a single READ CCW
> - starts the STSH command with the ORB
> - Expect an interrupt
> - writes the read data to output
> 
> The test implements lots of traces when DEBUG is on and
> tests if memory above the stack is corrupted.

What happens if we do not habe the pong device?

> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/css.h      | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/s390x/css_dump.c | 141 +++++++++++++++++++++++++++++

Hmm, what about splitting the patch into css.h/css_dump.c and the actual
test in s390x/css.c?

>  s390x/Makefile       |   2 +
>  s390x/css.c          | 222 ++++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg  |   4 +
>  5 files changed, 613 insertions(+)
>  create mode 100644 lib/s390x/css.h
>  create mode 100644 lib/s390x/css_dump.c
>  create mode 100644 s390x/css.c
> 
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> new file mode 100644
> index 0000000..a7c42fd
> --- /dev/null
> +++ b/lib/s390x/css.h
> @@ -0,0 +1,244 @@
> +/*
> + * CSS definitions
> + *
> + * Copyright IBM, Corp. 2019
> + * Author: Pierre Morel <pmorel@linux.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#ifndef CSS_H
> +#define CSS_H
> +
> +#define CCW_F_CD	0x80
> +#define CCW_F_CC	0x40
> +#define CCW_F_SLI	0x20
> +#define CCW_F_SKP	0x10
> +#define CCW_F_PCI	0x08
> +#define CCW_F_IDA	0x04
> +#define CCW_F_S		0x02
> +#define CCW_F_MIDA	0x01
> +
> +#define CCW_C_NOP	0x03
> +#define CCW_C_TIC	0x08
> +
> +struct ccw {
> +	unsigned char code;
> +	unsigned char flags;
> +	unsigned short count;
> +	unsigned int data;
> +} __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

\n

> +struct orb {
> +	unsigned int	intparm;
> +	unsigned int	ctrl;
> +	unsigned int	cpa;
> +	unsigned int	prio;
> +	unsigned int	reserved[4];
> +} __attribute__ ((aligned(4)));;

Double ;;

> +
> +struct scsw {
> +	uint32_t ctrl;
> +	uint32_t addr;
> +	uint8_t  devs;
> +	uint8_t  schs;
> +	uint16_t count;
> +};
> +
> +struct pmcw {
> +	uint32_t intparm;
> +	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 flag2;
> +};
> +struct schib {
> +	struct pmcw pmcw;
> +	struct scsw scsw;
> +	uint32_t  md0;
> +	uint32_t  md1;
> +	uint32_t  md2;
> +} __attribute__ ((aligned(4)));
> +
> +struct irb {
> +	struct scsw scsw;
> +	uint32_t esw[5];
> +	uint32_t ecw[8];
> +	uint32_t emw[8];
> +} __attribute__ ((aligned(4)));;

Double ;;

> +
> +/* CSS low level access functions */
> +
> +static inline int ssch(unsigned long schid, struct orb *addr)
> +{
> +	register long long reg1 asm("1") = schid;
> +	int ccode = -1;

s/ccode/cc/

> +
> +	asm volatile(
> +		"	   ssch	0(%2)\n"
> +		"0:	 ipm	 %0\n"
> +		"	   srl	 %0,28\n"
> +		"1:\n"
> +		: "+d" (ccode)
> +		: "d" (reg1), "a" (addr), "m" (*addr)
> +		: "cc", "memory");
> +	return ccode;
> +}
> +
> +static inline int stsch(unsigned long schid, struct schib *addr)
> +{
> +	register unsigned long reg1 asm ("1") = schid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	   stsch	0(%3)\n"
> +		"	   ipm	 %0\n"
> +		"	   srl	 %0,28"
> +		: "=d" (ccode), "=m" (*addr)
> +		: "d" (reg1), "a" (addr)
> +		: "cc");
> +	return ccode;
> +}
> +
> +static inline int msch(unsigned long schid, struct schib *addr)
> +{
> +	register unsigned long reg1 asm ("1") = schid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	   msch	0(%3)\n"
> +		"	   ipm	 %0\n"
> +		"	   srl	 %0,28"
> +		: "=d" (ccode), "=m" (*addr)
> +		: "d" (reg1), "a" (addr)
> +		: "cc");
> +	return ccode;
> +}
> +
> +static inline int tsch(unsigned long schid, struct irb *addr)
> +{
> +	register unsigned long reg1 asm ("1") = schid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	   tsch	0(%3)\n"
> +		"	   ipm	 %0\n"
> +		"	   srl	 %0,28"
> +		: "=d" (ccode), "=m" (*addr)
> +		: "d" (reg1), "a" (addr)
> +		: "cc");
> +	return ccode;
> +}
> +
> +static inline int hsch(unsigned long schid)
> +{
> +	register unsigned long reg1 asm("1") = schid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	hsch\n"
> +		"	ipm	%0\n"
> +		"	srl	%0,28"
> +		: "=d" (ccode)
> +		: "d" (reg1)
> +		: "cc");
> +	return ccode;
> +}
> +
> +static inline int xsch(unsigned long schid)
> +{
> +	register unsigned long reg1 asm("1") = schid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	xsch\n"
> +		"	ipm	%0\n"
> +		"	srl	%0,28"
> +		: "=d" (ccode)
> +		: "d" (reg1)
> +		: "cc");
> +	return ccode;
> +}
> +
> +static inline int csch(unsigned long schid)
> +{
> +	register unsigned long reg1 asm("1") = schid;
> +	int ccode;> +
> +	asm volatile(
> +		"	csch\n"
> +		"	ipm	%0\n"
> +		"	srl	%0,28"
> +		: "=d" (ccode)
> +		: "d" (reg1)
> +		: "cc");
> +	return ccode;
> +}
> +
> +static inline int rsch(unsigned long schid)
> +{
> +	register unsigned long reg1 asm("1") = schid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	rsch\n"
> +		"	ipm	%0\n"
> +		"	srl	%0,28"
> +		: "=d" (ccode)
> +		: "d" (reg1)
> +		: "cc");
> +	return ccode;
> +}
> +
> +static inline int rchp(unsigned long chpid)
> +{
> +	register unsigned long reg1 asm("1") = chpid;
> +	int ccode;
> +
> +	asm volatile(
> +		"	rchp\n"
> +		"	ipm	%0\n"
> +		"	srl	%0,28"
> +		: "=d" (ccode)
> +		: "d" (reg1)
> +		: "cc");
> +	return ccode;
> +}
> +
> +void dump_scsw(struct scsw *);
> +void dump_irb(struct irb *irbp);
> +void dump_pmcw_flags(uint16_t f);
> +void dump_pmcw(struct pmcw *p);
> +void dump_schib(struct schib *sch);
> +struct ccw *dump_ccw(struct ccw *cp);
> +void dump_orb(struct orb *op);
> +
> +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..4f6b628
> --- /dev/null
> +++ b/lib/s390x/css_dump.c
> @@ -0,0 +1,141 @@
> +/*
> + * Channel Sub-System structures dumping
> + *
> + * Copyright (c) 2019 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 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
> + *          a CCW chain
> + * - CCW  : Channel Command Word, describe the data and flow control
> + * - IRB  : Interuption response Block, describe the result of an operation
> + *          hold a SCSW and several channel type dependent fields.
> + * - SCHIB: SubChannel Information Block composed of:
> + *   - SCSW: SubChannel Status Word, status of the channel as a result of an
> + *           operation when in IRB.
> + *   - 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";
> +
> +void dump_scsw(struct scsw *s)
> +{
> +	int i;
> +	uint32_t scsw = s->ctrl;
> +	char line[64] = {};
> +
> +	for (i = 0; i < 16; i++) {
> +		if ((scsw << i) & 0x80000000)
> +			line[i] = scsw_str[i];
> +		else
> +			line[i] = '_';
> +	}
> +	line[i] = ' ';
> +	for (; i < 32; i++) {
> +		if ((scsw << i) & 0x80000000)
> +			line[i + 1] = scsw_str2[i - 16];
> +		else
> +			line[i + 1] = '_';
> +	}
> +	printf("scsw->flags: %s\n", line);
> +	printf("scsw->addr : %08x\n", s->addr);
> +	printf("scsw->devs : %02x\n", s->devs);
> +	printf("scsw->schs : %02x\n", s->schs);
> +	printf("scsw->count: %04x\n", s->count);
> +

Stray \n

> +}
> +
> +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);
> +}
> +
> +static const char *pmcw_str = "11iii111ellmmdtv";
> +void dump_pmcw_flags(uint16_t f)
> +{
> +	int i;
> +	char line[32] = {};
> +
> +	for (i = 0; i < 16; i++) {
> +		if ((f << i) & 0x8000)
> +			line[i] = pmcw_str[i];
> +		else
> +			line[i] = '_';
> +	}
> +	printf("pmcw->pmcw flgs: %s\n", line);
> +}
> +
> +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->flag2);
> +}
> +
> +void dump_schib(struct schib *sch)
> +{
> +	struct pmcw *p = &sch->pmcw;
> +	struct scsw *s = &sch->scsw;
> +
> +	printf("--SCHIB--\n");
> +	dump_pmcw(p);
> +	dump_scsw(s);
> +}
> +
> +struct ccw *dump_ccw(struct ccw *cp)
> +{
> +	printf("CCW: code: %02x flags: %02x count: %04x data: %08x\n", cp->code,
> +	    cp->flags, cp->count, cp->data);
> +
> +	if (cp->code == CCW_C_TIC)
> +		return (struct ccw *)(long)cp->data;
> +
> +	return (cp->flags & CCW_F_CC) ? cp + 1 : NULL;
> +}
> +
> +void dump_orb(struct orb *op)
> +{
> +	struct ccw *cp;
> +
> +	printf("ORB: intparm : %08x\n", op->intparm);
> +	printf("ORB: ctrl    : %08x\n", op->ctrl);
> +	printf("ORB: prio    : %08x\n", op->prio);
> +	cp = (struct ccw *)(long) (op->cpa);
> +	while (cp)
> +		cp = dump_ccw(cp);
> +}
> +
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 3744372..9ebbb84 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -16,6 +16,7 @@ tests += $(TEST_DIR)/diag288.elf
>  tests += $(TEST_DIR)/stsi.elf
>  tests += $(TEST_DIR)/skrf.elf
>  tests += $(TEST_DIR)/smp.elf
> +tests += $(TEST_DIR)/css.elf
>  tests_binary = $(patsubst %.elf,%.bin,$(tests))
>  
>  all: directories test_cases test_cases_binary
> @@ -50,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
>  
> diff --git a/s390x/css.c b/s390x/css.c
> new file mode 100644
> index 0000000..6cdaf61
> --- /dev/null
> +++ b/s390x/css.c
> @@ -0,0 +1,222 @@
> +/*
> + * Channel Sub-System tests
> + *
> + * Copyright (c) 2019 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 <asm/interrupt.h>
> +#include <asm/arch_def.h>
> +
> +#include <css.h>
> +
> +#define PSW_PRG_MASK (PSW_MASK_IO | PSW_MASK_EA | PSW_MASK_BA)
> +
> +#define DEBUG 1
> +#ifdef DEBUG
> +#define DBG(format, arg...) \
> +	printf("KT_%s:%d " format "\n", __func__, __LINE__, ##arg)
> +#else
> +#define DBG(format, arg...) do {} while (0)
> +#endif
> +
> +#define CSSID_PONG	(0xfe000000 | 0x00090000)
> +
> +struct lowcore *lowcore = (void *)0x0;
> +
> +#define NB_CCW  100
> +static struct ccw ccw[NB_CCW];
> +
> +#define NB_ORB  100
> +static struct orb orb[NB_ORB];
> +
> +static struct irb irb;
> +static struct schib schib;
> +
> +static char buffer[4096];
> +
> +static void delay(int d)
> +{
> +	int i, j;
> +
> +	while (d--)
> +		for (i = 1000000; i; i--)
> +			for (j = 1000000; j; j--)
> +				;
> +}

You could set a timer.

> +
> +static void set_io_irq_subclass_mask(uint64_t const new_mask)
> +{
> +	asm volatile (
> +		"lctlg %%c6, %%c6, %[source]\n"
> +		: /* No outputs */
> +		: [source] "R" (new_mask));

arch_def.h has lctlg() and ctl_set/clear_bit

> +}
> +
> +static void set_system_mask(uint8_t new_mask)
> +{
> +	asm volatile (
> +		"ssm %[source]\n"
> +		: /* No outputs */
> +		: [source] "R" (new_mask));
> +}
> +
> +static void enable_io_irq(void)
> +{
> +	set_io_irq_subclass_mask(0x00000000ff000000);
> +	set_system_mask(PSW_PRG_MASK >> 56);

load_psw_mask(extract_psw_mask() | PSW_PRG_MASK); no need for another
inline asm function :)

Or add a psw_set/clear_bit function and fixup enter_pstate()

> +}
> +
> +void handle_io_int(sregs_t *regs)
> +{
> +	int ret = 0;
> +
> +	DBG("IO IRQ: subsys_id_word=%08x", lowcore->subsys_id_word);
> +	DBG("......: io_int_parm   =%08x", lowcore->io_int_param);
> +	DBG("......: io_int_word   =%08x", lowcore->io_int_word);
> +	ret = tsch(lowcore->subsys_id_word, &irb);
> +	dump_irb(&irb);
> +	if (ret)
> +		DBG("......: tsch retval %d", ret);
> +	DBG("IO IRQ: END");
> +}
> +
> +static void set_schib(struct schib *sch)
> +{
> +	struct pmcw *p = &sch->pmcw;
> +
> +	p->intparm = 0xdeadbeef;
> +	p->devnum = 0xc0ca;
> +	p->lpm = 0x80;
> +	p->flags = 0x3081;
> +	p->chpid[7] = 0x22;
> +	p->pim = 0x0f;
> +	p->pam = 0x0f;
> +	p->pom = 0x0f;
> +	p->lpm = 0x0f;
> +	p->lpum = 0xaa;
> +	p->pnom = 0xf0;
> +	p->mbi = 0xaa;
> +	p->mbi = 0xaaaa;
> +}
> +
> +static void css_enable(void)
> +{
> +	int ret;
> +
> +	ret = stsch(CSSID_PONG, &schib);
> +	if (ret)
> +		DBG("stsch: %x\n", ret);
> +	dump_schib(&schib);
> +	set_schib(&schib);
> +	dump_schib(&schib);
> +	ret = msch(CSSID_PONG, &schib);
> +	if (ret)
> +		DBG("msch : %x\n", ret);
> +}
> +
> +/* These two definitions are part of the QEMU PONG interface */
> +#define PONG_WRITE 0x21
> +#define PONG_READ  0x22
> +
> +static int css_run(int fake)
> +{
> +	struct orb *p = orb;
> +	int cc;
> +
> +	if (fake)
> +		return 0;
> +	css_enable();
> +
> +	enable_io_irq();
> +
> +	ccw[0].code = PONG_READ;
> +	ccw[0].flags = CCW_F_PCI;
> +	ccw[0].count = 80;
> +	ccw[0].data = (unsigned int)(unsigned long) &buffer;
> +
> +	p->intparm = 0xcafec0ca;
> +	p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
> +	p->cpa = (unsigned int) (unsigned long)&ccw[0];
> +
> +	printf("ORB AT %p\n", orb);
> +	dump_orb(p);
> +	cc = ssch(CSSID_PONG, p);
> +	if (cc) {
> +		DBG("cc: %x\n", cc);
> +		return cc;
> +	}
> +
> +	delay(1);
> +
> +	stsch(CSSID_PONG, &schib);
> +	dump_schib(&schib);

Is all that dumping necessary or just a dev remainder?

> +	DBG("got: %s\n", buffer);
> +
> +	return 0;
> +}
> +
> +#define MAX_ERRORS 10
> +static int checkmem(phys_addr_t start, phys_addr_t end)
> +{
> +	phys_addr_t curr;
> +	int err = 0;
> +
> +	for (curr = start; curr != end; curr += PAGE_SIZE)
> +		if (memcmp((void *)start, (void *)curr, PAGE_SIZE)) {
> +			report("memcmp failed %lx", true, curr);

How many errors do you normally run into (hopefully 0)?

> +			if (err++ > MAX_ERRORS)
> +				break;
> +		}
> +	return err;
> +}
> +
> +extern unsigned long bss_end;
> +
> +int main(int argc, char *argv[])
> +{
> +	phys_addr_t base, top;
> +	int check_mem = 0;
> +	int err = 0;
> +
> +	if (argc == 2 && !strcmp(argv[1], "-i"))
> +		check_mem = 1;
> +
> +	report_prefix_push("css");
> +	phys_alloc_get_unused(&base, &top);
> +
> +	top = 0x08000000; /* 128MB Need to be updated */
> +	base = (phys_addr_t)&stacktop;
> +
> +	if (check_mem)
> +		memset((void *)base, 0x00, top - base);
> +
> +	if (check_mem)
> +		err = checkmem(base, top);
> +	if (err)
> +		goto out;
> +
> +	err = css_run(0);
> +	if (err)
> +		goto out;
> +
> +	if (check_mem)
> +		err = checkmem(base, top);
> +
> +out:
> +	if (err)
> +		report("Tested", 0);
> +	else
> +		report("Tested", 1);

Normally we report the sucsess or failure of single actions and a
summary will tell us if the whole test ran into errors.

> +
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index f1b07cd..1755d9e 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -75,3 +75,7 @@ file = stsi.elf
>  [smp]
>  file = smp.elf
>  extra_params =-smp 2
> +
> +[css]
> +file = css.elf
> +extra_params =-device ccw-pong
>

Pierre Morel Nov. 14, 2019, 10:11 a.m. UTC | #3

On 2019-11-13 14:05, Cornelia Huck wrote:
> On Wed, 13 Nov 2019 13:23:19 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> This simple test test the I/O reading by the SUB Channel by:
>> - initializing the Channel SubSystem with predefined CSSID:
>>    0xfe000000 CSSID for a Virtual CCW
> 0 should be fine with recent QEMU versions as well, I guess?
Right


>
>>    0x00090000 SSID for CCW-PONG
> subchannel id, or subchannel set id?

hum, only part of, I had SSID (Subchannel Set ID) 4 (a.k.a m bit) + Bit 
47  =1

But as you said, I can use CSSID 0 and m = 0 which makes:

Subsystem Identification word = 0x00010000


>
>> - initializing the ORB pointing to a single READ CCW
> Out of curiosity: Would using a NOP also be an option?

It will work but will not be handled by this device, css.c intercept it 
in sch_handle_start_func_virtual.

AFAIU If we want to have a really good testing environment, for driver 
testing for exemple, then it would be interesting to add a new 
do_subchannel_work callback like do_subchannel_work_emulation along with 
the _virtual and _paththrough variantes.

Having a dedicated callback for emulation, we can answer to any CSS 
instructions and SSCH commands, including NOP and TIC.

My goal here was to quickly develop a device answering to some basic 
READ/WRITE command to start memory transfers from inside a guest without 
Linux and without implementing VIRTIO in KVM tests.



>
>> - starts the STSH command with the ORB
> s/STSH/SSCH/ ?

:) yes, thanks


>
>> - Expect an interrupt
>> - writes the read data to output
>>
>> The test implements lots of traces when DEBUG is on and
>> tests if memory above the stack is corrupted.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h      | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/s390x/css_dump.c | 141 +++++++++++++++++++++++++++++
>>   s390x/Makefile       |   2 +
>>   s390x/css.c          | 222 ++++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg  |   4 +
>>   5 files changed, 613 insertions(+)
>>   create mode 100644 lib/s390x/css.h
>>   create mode 100644 lib/s390x/css_dump.c
>>   create mode 100644 s390x/css.c
>>
>> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
>> new file mode 100644
>> index 0000000..a7c42fd
>> --- /dev/null
>> +++ b/lib/s390x/css.h
> (...)
>
>> +static inline int rsch(unsigned long schid)
> I don't think anyone has tried rsch with QEMU before; sounds like a
> good idea to test this :)

With an do_subchannel_work_emulation() callback?


>
>> +{
>> +	register unsigned long reg1 asm("1") = schid;
>> +	int ccode;
>> +
>> +	asm volatile(
>> +		"	rsch\n"
>> +		"	ipm	%0\n"
>> +		"	srl	%0,28"
>> +		: "=d" (ccode)
>> +		: "d" (reg1)
>> +		: "cc");
>> +	return ccode;
>> +}
>> +
>> +static inline int rchp(unsigned long chpid)
> Anything useful we can test here?

Not for now.

I certainly can reduce the size of the file by removing the unused CSS 
instructions calls.

...snip...
> +
>> +static void enable_io_irq(void)
>> +{
>> +	set_io_irq_subclass_mask(0x00000000ff000000);
> So, you always enable all iscs? Maybe add a comment?

OK, is just a lazy option to get IRQ for this test.

Right, I add a comment.


>
>> +	set_system_mask(PSW_PRG_MASK >> 56);
>> +}
>> +
>> +void handle_io_int(sregs_t *regs)
>> +{
>> +	int ret = 0;
>> +
>> +	DBG("IO IRQ: subsys_id_word=%08x", lowcore->subsys_id_word);
>> +	DBG("......: io_int_parm   =%08x", lowcore->io_int_param);
>> +	DBG("......: io_int_word   =%08x", lowcore->io_int_word);
>> +	ret = tsch(lowcore->subsys_id_word, &irb);
>> +	dump_irb(&irb);
>> +	if (ret)
>> +		DBG("......: tsch retval %d", ret);
>> +	DBG("IO IRQ: END");
>> +}
>> +
>> +static void set_schib(struct schib *sch)
>> +{
>> +	struct pmcw *p = &sch->pmcw;
>> +
>> +	p->intparm = 0xdeadbeef;
>> +	p->devnum = 0xc0ca;
>> +	p->lpm = 0x80;
>> +	p->flags = 0x3081;
> Use #defines instead of magic numbers?

OK


>
>> +	p->chpid[7] = 0x22;
>> +	p->pim = 0x0f;
>> +	p->pam = 0x0f;
>> +	p->pom = 0x0f;
>> +	p->lpm = 0x0f;
>> +	p->lpum = 0xaa;
>> +	p->pnom = 0xf0;
>> +	p->mbi = 0xaa;
>> +	p->mbi = 0xaaaa;
> Many of these fields are not supposed to be modifiable by the program
> -- do you want to check what you get back after msch?
>
> Also, you set mbi twice ;) (And for it to actually have any effect,
> you'd have to execute SET CHANNEL MONITOR, no?)

Yes, it was for me to check what happens but I should remove most of 
these field initialization.

As you said they are not modifiable by the program.

Will clean this.


>
>
>> +}
>> +
>> +static void css_enable(void)
>> +{
>> +	int ret;
>> +
>> +	ret = stsch(CSSID_PONG, &schib);
>> +	if (ret)
>> +		DBG("stsch: %x\n", ret);
>> +	dump_schib(&schib);
>> +	set_schib(&schib);
>> +	dump_schib(&schib);
>> +	ret = msch(CSSID_PONG, &schib);
>> +	if (ret)
>> +		DBG("msch : %x\n", ret);
>> +}
>> +
>> +/* These two definitions are part of the QEMU PONG interface */
>> +#define PONG_WRITE 0x21
>> +#define PONG_READ  0x22
> Ah, so it's not a plain read/write, but a specialized one? Mention that
> in the patch description?
>
>> +
>> +static int css_run(int fake)
>> +{
>> +	struct orb *p = orb;
> I'd maybe call that variable 'orb' instead; at a glance, I was confused
> what you did with the pmcw below, until I realized that it's the orb :)

OK, is clearly better to use orb.


Thanks,

Pierre

Pierre Morel Nov. 14, 2019, 4:38 p.m. UTC | #4

On 2019-11-14 10:15, Janosch Frank wrote:
> On 11/13/19 1:23 PM, Pierre Morel wrote:
>> This simple test test the I/O reading by the SUB Channel by:
>> - initializing the Channel SubSystem with predefined CSSID:
>>    0xfe000000 CSSID for a Virtual CCW
>>    0x00090000 SSID for CCW-PONG
>> - initializing the ORB pointing to a single READ CCW
>> - starts the STSH command with the ORB
>> - Expect an interrupt
>> - writes the read data to output
>>
>> The test implements lots of traces when DEBUG is on and
>> tests if memory above the stack is corrupted.
> What happens if we do not habe the pong device?

CC error on stsch() which is currently not cached (but will in the next 
version)

CC error on msch() and on ssch() which is cached and makes the test to fail.


>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h      | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/s390x/css_dump.c | 141 +++++++++++++++++++++++++++++
> Hmm, what about splitting the patch into css.h/css_dump.c and the actual
> test in s390x/css.c?

OK


>
>>   s390x/Makefile       |   2 +
>>   s390x/css.c          | 222 ++++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg  |   4 +
>>   5 files changed, 613 insertions(+)
>>   create mode 100644 lib/s390x/css.h
>>   create mode 100644 lib/s390x/css_dump.c
>>   create mode 100644 s390x/css.c
>>
>> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
>> new file mode 100644

OK to all comments...  (I sniped out for clarity)

...snip...


>> +static char buffer[4096];
>> +
>> +static void delay(int d)
>> +{
>> +	int i, j;
>> +
>> +	while (d--)
>> +		for (i = 1000000; i; i--)
>> +			for (j = 1000000; j; j--)
>> +				;
>> +}
> You could set a timer.


Hum, do we really want to do this?


>
>> +
>> +static void set_io_irq_subclass_mask(uint64_t const new_mask)
>> +{
>> +	asm volatile (
>> +		"lctlg %%c6, %%c6, %[source]\n"
>> +		: /* No outputs */
>> +		: [source] "R" (new_mask));
> arch_def.h has lctlg() and ctl_set/clear_bit


OK, thanks


>
>> +}
>> +
>> +static void set_system_mask(uint8_t new_mask)
>> +{
>> +	asm volatile (
>> +		"ssm %[source]\n"
>> +		: /* No outputs */
>> +		: [source] "R" (new_mask));
>> +}
>> +
>> +static void enable_io_irq(void)
>> +{
>> +	set_io_irq_subclass_mask(0x00000000ff000000);
>> +	set_system_mask(PSW_PRG_MASK >> 56);
> load_psw_mask(extract_psw_mask() | PSW_PRG_MASK); no need for another
> inline asm function :)
>
> Or add a psw_set/clear_bit function and fixup enter_pstate()

I look at this.


>
>> +}
>> +
>> +void handle_io_int(sregs_t *regs)
>> +{
,,,snip...
>> +
>> +	delay(1);
>> +
>> +	stsch(CSSID_PONG, &schib);
>> +	dump_schib(&schib);
> Is all that dumping necessary or just a dev remainder?


it goes in the logs, so I thought it could be interresting to keep it.


>
>> +	DBG("got: %s\n", buffer);
>> +
>> +	return 0;
>> +}
>> +
>> +#define MAX_ERRORS 10
>> +static int checkmem(phys_addr_t start, phys_addr_t end)
>> +{
>> +	phys_addr_t curr;
>> +	int err = 0;
>> +
>> +	for (curr = start; curr != end; curr += PAGE_SIZE)
>> +		if (memcmp((void *)start, (void *)curr, PAGE_SIZE)) {
>> +			report("memcmp failed %lx", true, curr);
> How many errors do you normally run into (hopefully 0)?


hopefully.

However I thought it could be interesting to know how many pages have 
been dirtied.


>
>> +			if (err++ > MAX_ERRORS)
>> +				break;
>> +		}
>> +	return err;
>> +}
>> +
>> +extern unsigned long bss_end;
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +	phys_addr_t base, top;
>> +	int check_mem = 0;
>> +	int err = 0;
>> +
>> +	if (argc == 2 && !strcmp(argv[1], "-i"))
>> +		check_mem = 1;
>> +
>> +	report_prefix_push("css");
>> +	phys_alloc_get_unused(&base, &top);
>> +
>> +	top = 0x08000000; /* 128MB Need to be updated */
>> +	base = (phys_addr_t)&stacktop;
>> +
>> +	if (check_mem)
>> +		memset((void *)base, 0x00, top - base);
>> +
>> +	if (check_mem)
>> +		err = checkmem(base, top);
>> +	if (err)
>> +		goto out;
>> +
>> +	err = css_run(0);
>> +	if (err)
>> +		goto out;
>> +
>> +	if (check_mem)
>> +		err = checkmem(base, top);
>> +
>> +out:
>> +	if (err)
>> +		report("Tested", 0);
>> +	else
>> +		report("Tested", 1);
> Normally we report the sucsess or failure of single actions and a
> summary will tell us if the whole test ran into errors.

Right, will be enhanced.

Thanks for the comments.

Regards,

Pierre

Thomas Huth Nov. 14, 2019, 4:51 p.m. UTC | #5

On 14/11/2019 17.38, Pierre Morel wrote:
[...]
>>> +static char buffer[4096];
>>> +
>>> +static void delay(int d)
>>> +{
>>> +    int i, j;
>>> +
>>> +    while (d--)
>>> +        for (i = 1000000; i; i--)
>>> +            for (j = 1000000; j; j--)
>>> +                ;
>>> +}
>> You could set a timer.
> 
> Hum, do we really want to do this?

I'm pretty sure that the compiler optimizes empty loops away. Maybe have
a look at the disassembly of your delay function...

Anyway, it's likely better to use STCK and friends to get a proper
timing. You could move get_clock_ms() from s390x/intercept.c to the
lib/s390x folder and then use that function here.

 Thomas

Janosch Frank Nov. 14, 2019, 5:09 p.m. UTC | #6

On 11/14/19 5:38 PM, Pierre Morel wrote:
> 
> On 2019-11-14 10:15, Janosch Frank wrote:
>> On 11/13/19 1:23 PM, Pierre Morel wrote:
>>> This simple test test the I/O reading by the SUB Channel by:
>>> - initializing the Channel SubSystem with predefined CSSID:
>>>    0xfe000000 CSSID for a Virtual CCW
>>>    0x00090000 SSID for CCW-PONG
>>> - initializing the ORB pointing to a single READ CCW
>>> - starts the STSH command with the ORB
>>> - Expect an interrupt
>>> - writes the read data to output
>>>
>>> The test implements lots of traces when DEBUG is on and
>>> tests if memory above the stack is corrupted.
>> What happens if we do not habe the pong device?
> 
> CC error on stsch() which is currently not cached (but will in the next 
> version)
> 
> CC error on msch() and on ssch() which is cached and makes the test to fail.
> 
> 
>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>   lib/s390x/css.h      | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   lib/s390x/css_dump.c | 141 +++++++++++++++++++++++++++++
>> Hmm, what about splitting the patch into css.h/css_dump.c and the actual
>> test in s390x/css.c?
> 
> OK
> 
> 
>>
>>>   s390x/Makefile       |   2 +
>>>   s390x/css.c          | 222 ++++++++++++++++++++++++++++++++++++++++++++++
>>>   s390x/unittests.cfg  |   4 +
>>>   5 files changed, 613 insertions(+)
>>>   create mode 100644 lib/s390x/css.h
>>>   create mode 100644 lib/s390x/css_dump.c
>>>   create mode 100644 s390x/css.c
>>>
>>> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
>>> new file mode 100644
> 
> OK to all comments...  (I sniped out for clarity)
> 
> ...snip...
> 
> 
>>> +static char buffer[4096];
>>> +
>>> +static void delay(int d)
>>> +{
>>> +	int i, j;
>>> +
>>> +	while (d--)
>>> +		for (i = 1000000; i; i--)
>>> +			for (j = 1000000; j; j--)
>>> +				;
>>> +}
>> You could set a timer.
> 
> 
> Hum, do we really want to do this?

Why exactly do you need it if you can't have an exact time to wait for?

> 
> 
>>
>>> +
>>> +static void set_io_irq_subclass_mask(uint64_t const new_mask)
>>> +{
>>> +	asm volatile (
>>> +		"lctlg %%c6, %%c6, %[source]\n"
>>> +		: /* No outputs */
>>> +		: [source] "R" (new_mask));
>> arch_def.h has lctlg() and ctl_set/clear_bit
> 
> 
> OK, thanks
> 
> 
>>
>>> +}
>>> +
>>> +static void set_system_mask(uint8_t new_mask)
>>> +{
>>> +	asm volatile (
>>> +		"ssm %[source]\n"
>>> +		: /* No outputs */
>>> +		: [source] "R" (new_mask));
>>> +}
>>> +
>>> +static void enable_io_irq(void)
>>> +{
>>> +	set_io_irq_subclass_mask(0x00000000ff000000);
>>> +	set_system_mask(PSW_PRG_MASK >> 56);
>> load_psw_mask(extract_psw_mask() | PSW_PRG_MASK); no need for another
>> inline asm function :)
>>
>> Or add a psw_set/clear_bit function and fixup enter_pstate()
> 
> I look at this.
> 
> 
>>
>>> +}
>>> +
>>> +void handle_io_int(sregs_t *regs)
>>> +{
> ,,,snip...
>>> +
>>> +	delay(1);
>>> +
>>> +	stsch(CSSID_PONG, &schib);
>>> +	dump_schib(&schib);
>> Is all that dumping necessary or just a dev remainder?
> 
> 
> it goes in the logs, so I thought it could be interresting to keep it.

Depends on how much output is produced.
If I have to scroll through your dumps to get to the ouptuts
 of the reports then they are .

See the answer below...

> 
> 
>>
>>> +	DBG("got: %s\n", buffer);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +#define MAX_ERRORS 10
>>> +static int checkmem(phys_addr_t start, phys_addr_t end)
>>> +{
>>> +	phys_addr_t curr;
>>> +	int err = 0;
>>> +
>>> +	for (curr = start; curr != end; curr += PAGE_SIZE)
>>> +		if (memcmp((void *)start, (void *)curr, PAGE_SIZE)) {
>>> +			report("memcmp failed %lx", true, curr);
>> How many errors do you normally run into (hopefully 0)?
> 
> 
> hopefully.
> 
> However I thought it could be interesting to know how many pages have 
> been dirtied.

Honestly, for debugging a failing test we would need to add prints or
attach gdb anyway. So I see no reason to not fail on the first occurrence.

> 
> 
>>
>>> +			if (err++ > MAX_ERRORS)
>>> +				break;
>>> +		}
>>> +	return err;
>>> +}
>>> +
>>> +extern unsigned long bss_end;
>>> +
>>> +int main(int argc, char *argv[])
>>> +{
>>> +	phys_addr_t base, top;
>>> +	int check_mem = 0;
>>> +	int err = 0;
>>> +
>>> +	if (argc == 2 && !strcmp(argv[1], "-i"))
>>> +		check_mem = 1;
>>> +
>>> +	report_prefix_push("css");
>>> +	phys_alloc_get_unused(&base, &top);
>>> +
>>> +	top = 0x08000000; /* 128MB Need to be updated */
>>> +	base = (phys_addr_t)&stacktop;
>>> +
>>> +	if (check_mem)
>>> +		memset((void *)base, 0x00, top - base);
>>> +
>>> +	if (check_mem)
>>> +		err = checkmem(base, top);
>>> +	if (err)
>>> +		goto out;
>>> +
>>> +	err = css_run(0);
>>> +	if (err)
>>> +		goto out;
>>> +
>>> +	if (check_mem)
>>> +		err = checkmem(base, top);
>>> +
>>> +out:
>>> +	if (err)
>>> +		report("Tested", 0);
>>> +	else
>>> +		report("Tested", 1);
>> Normally we report the sucsess or failure of single actions and a
>> summary will tell us if the whole test ran into errors.
> 
> Right, will be enhanced.
> 
> Thanks for the comments.
> 
> Regards,
> 
> Pierre
> 
>

Pierre Morel Nov. 14, 2019, 5:50 p.m. UTC | #7

On 2019-11-14 17:51, Thomas Huth wrote:
> On 14/11/2019 17.38, Pierre Morel wrote:
> [...]
>>>> +static char buffer[4096];
>>>> +
>>>> +static void delay(int d)
>>>> +{
>>>> +    int i, j;
>>>> +
>>>> +    while (d--)
>>>> +        for (i = 1000000; i; i--)
>>>> +            for (j = 1000000; j; j--)
>>>> +                ;
>>>> +}
>>> You could set a timer.
>> Hum, do we really want to do this?
> I'm pretty sure that the compiler optimizes empty loops away. Maybe have
> a look at the disassembly of your delay function...
>
> Anyway, it's likely better to use STCK and friends to get a proper
> timing. You could move get_clock_ms() from s390x/intercept.c to the
> lib/s390x folder and then use that function here.


Yes, I can, thanks.

Pierre



>
>   Thomas
>

Pierre Morel Nov. 14, 2019, 5:55 p.m. UTC | #8

On 2019-11-14 18:09, Janosch Frank wrote:
> On 11/14/19 5:38 PM, Pierre Morel wrote:
>> On 2019-11-14 10:15, Janosch Frank wrote:
>>> On 11/13/19 1:23 PM, Pierre Morel wrote:
>>>> This simple test test the I/O reading by the SUB Channel by:
>>>> - initializing the Channel SubSystem with predefined CSSID:
>>>>     0xfe000000 CSSID for a Virtual CCW
>>>>     0x00090000 SSID for CCW-PONG
>>>> - initializing the ORB pointing to a single READ CCW
>>>> - starts the STSH command with the ORB
>>>> - Expect an interrupt
>>>> - writes the read data to output
>>>>
>>>> The test implements lots of traces when DEBUG is on and
>>>> tests if memory above the stack is corrupted.
>>> What happens if we do not habe the pong device?
>> CC error on stsch() which is currently not cached (but will in the next
>> version)
>>
>> CC error on msch() and on ssch() which is cached and makes the test to fail.
>>
>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>    lib/s390x/css.h      | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    lib/s390x/css_dump.c | 141 +++++++++++++++++++++++++++++
>>> Hmm, what about splitting the patch into css.h/css_dump.c and the actual
>>> test in s390x/css.c?
>> OK
>>
>>
>>>>    s390x/Makefile       |   2 +
>>>>    s390x/css.c          | 222 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>    s390x/unittests.cfg  |   4 +
>>>>    5 files changed, 613 insertions(+)
>>>>    create mode 100644 lib/s390x/css.h
>>>>    create mode 100644 lib/s390x/css_dump.c
>>>>    create mode 100644 s390x/css.c
>>>>
>>>> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
>>>> new file mode 100644
>> OK to all comments...  (I sniped out for clarity)
>>
>> ...snip...
>>
>>
>>>> +static char buffer[4096];
>>>> +
>>>> +static void delay(int d)
>>>> +{
>>>> +	int i, j;
>>>> +
>>>> +	while (d--)
>>>> +		for (i = 1000000; i; i--)
>>>> +			for (j = 1000000; j; j--)
>>>> +				;
>>>> +}
>>> You could set a timer.
>>
>> Hum, do we really want to do this?
> Why exactly do you need it if you can't have an exact time to wait for?

In fact I do not need it since the CCW instructions are treated 
synchronously.

It was for the fact we speak to a real device or if QEMU uses iothreads 
for CCW handling.


...snip


>>>> +
>>>> +	delay(1);
>>>> +
>>>> +	stsch(CSSID_PONG, &schib);
>>>> +	dump_schib(&schib);
>>> Is all that dumping necessary or just a dev remainder?
>>
>> it goes in the logs, so I thought it could be interresting to keep it.
> Depends on how much output is produced.
> If I have to scroll through your dumps to get to the ouptuts
>   of the reports then they are .
>
> See the answer below...


OK


>
>>
>>>> +	DBG("got: %s\n", buffer);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +#define MAX_ERRORS 10
>>>> +static int checkmem(phys_addr_t start, phys_addr_t end)
>>>> +{
>>>> +	phys_addr_t curr;
>>>> +	int err = 0;
>>>> +
>>>> +	for (curr = start; curr != end; curr += PAGE_SIZE)
>>>> +		if (memcmp((void *)start, (void *)curr, PAGE_SIZE)) {
>>>> +			report("memcmp failed %lx", true, curr);
>>> How many errors do you normally run into (hopefully 0)?
>>
>> hopefully.
>>
>> However I thought it could be interesting to know how many pages have
>> been dirtied.
> Honestly, for debugging a failing test we would need to add prints or
> attach gdb anyway. So I see no reason to not fail on the first occurrence.


OK

I will make the second version more quiet and much better.

Thanks for the comments,

Regards,

Pierre

Cornelia Huck Nov. 21, 2019, 4:02 p.m. UTC | #9

On Thu, 14 Nov 2019 11:11:18 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 2019-11-13 14:05, Cornelia Huck wrote:
> > On Wed, 13 Nov 2019 13:23:19 +0100
> > Pierre Morel <pmorel@linux.ibm.com> wrote:
> >  
> >> This simple test test the I/O reading by the SUB Channel by:
> >> - initializing the Channel SubSystem with predefined CSSID:
> >>    0xfe000000 CSSID for a Virtual CCW  
> > 0 should be fine with recent QEMU versions as well, I guess?  
> Right
> 
> 
> >  
> >>    0x00090000 SSID for CCW-PONG  
> > subchannel id, or subchannel set id?  
> 
> hum, only part of, I had SSID (Subchannel Set ID) 4 (a.k.a m bit) + Bit 
> 47  =1
> 
> But as you said, I can use CSSID 0 and m = 0 which makes:
> 
> Subsystem Identification word = 0x00010000

Yeah, I was mainly confused by the name 'SSID'.

> >> - initializing the ORB pointing to a single READ CCW  
> > Out of curiosity: Would using a NOP also be an option?  
> 
> It will work but will not be handled by this device, css.c intercept it 
> in sch_handle_start_func_virtual.
> 
> AFAIU If we want to have a really good testing environment, for driver 
> testing for exemple, then it would be interesting to add a new 
> do_subchannel_work callback like do_subchannel_work_emulation along with 
> the _virtual and _paththrough variantes.
> 
> Having a dedicated callback for emulation, we can answer to any CSS 
> instructions and SSCH commands, including NOP and TIC.

I guess that depends on what you want to test; if you actually want to
test device emulation as used by virtio etc., you obviously want to go
through the existing _virtual callback :)

The actual motivation behind my question was:
Is it possible to e.g. throw NOP (or TIC, or something else not
device-specific) at a normal, existing virtio device for test purposes?
You'd end up testing the common emulation code without needing any
extra support in QEMU. No idea how useful that would be.

> 
> My goal here was to quickly develop a device answering to some basic 
> READ/WRITE command to start memory transfers from inside a guest without 
> Linux and without implementing VIRTIO in KVM tests.

Yes, if you want to do some simple memory transfers, virtio is probably
not the first choice. Would e.g. doing a SenseID or so actually be
useful in some way already? After all, it does transfer memory (but
only in one direction).

> >> +static inline int rsch(unsigned long schid)  
> > I don't think anyone has tried rsch with QEMU before; sounds like a
> > good idea to test this :)  
> 
> With an do_subchannel_work_emulation() callback?

You probably need to build a simple channel program that suspends
itself and can be resumed later.

Pierre Morel Nov. 22, 2019, 9:03 a.m. UTC | #10

On 2019-11-21 17:02, Cornelia Huck wrote:
> On Thu, 14 Nov 2019 11:11:18 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> On 2019-11-13 14:05, Cornelia Huck wrote:
>>> On Wed, 13 Nov 2019 13:23:19 +0100
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>   
>>>> This simple test test the I/O reading by the SUB Channel by:
>>>> - initializing the Channel SubSystem with predefined CSSID:
>>>>     0xfe000000 CSSID for a Virtual CCW
>>> 0 should be fine with recent QEMU versions as well, I guess?
>> Right
>>
>>
>>>   
>>>>     0x00090000 SSID for CCW-PONG
>>> subchannel id, or subchannel set id?
>> hum, only part of, I had SSID (Subchannel Set ID) 4 (a.k.a m bit) + Bit
>> 47  =1
>>
>> But as you said, I can use CSSID 0 and m = 0 which makes:
>>
>> Subsystem Identification word = 0x00010000
> Yeah, I was mainly confused by the name 'SSID'.

Hum, yes sorry, I posted this to give a response to the kvm-test-unit 
css test.

I should have a lot more rework this old device before to post this series.
In between I did, so I will post a v2 which will suppress all these 
approximations.

>
>>>> - initializing the ORB pointing to a single READ CCW
>>> Out of curiosity: Would using a NOP also be an option?
>> It will work but will not be handled by this device, css.c intercept it
>> in sch_handle_start_func_virtual.
>>
>> AFAIU If we want to have a really good testing environment, for driver
>> testing for exemple, then it would be interesting to add a new
>> do_subchannel_work callback like do_subchannel_work_emulation along with
>> the _virtual and _paththrough variantes.
>>
>> Having a dedicated callback for emulation, we can answer to any CSS
>> instructions and SSCH commands, including NOP and TIC.
> I guess that depends on what you want to test; if you actually want to
> test device emulation as used by virtio etc., you obviously want to go
> through the existing _virtual callback :)

The first goal is to test basic I/O from inside the kvm-unit-test, 
producing errors and see how the system respond to errors.

In a standard system errors will be generated by QEMU analysing the I/O 
instruction after interception.

In a secured guest, we expect the same errors, however we want to check 
this.

This PONG device is intended to be low level, no VIRTIO, and to allow 
basic I/O.

>
> The actual motivation behind my question was:
> Is it possible to e.g. throw NOP (or TIC, or something else not
> device-specific) at a normal, existing virtio device for test purposes?
> You'd end up testing the common emulation code without needing any
> extra support in QEMU. No idea how useful that would be.

Writing a VIRTIO driver inside the kvm-unit-test is something we can do 
in the future.

As you said, the common code already handle NOP and TIC, the 
interpretation of the
CCW chain, once the SSCH has been intercepted is done by QEMU.
I do not think it would be different with SE.

>
>> My goal here was to quickly develop a device answering to some basic
>> READ/WRITE command to start memory transfers from inside a guest without
>> Linux and without implementing VIRTIO in KVM tests.
> Yes, if you want to do some simple memory transfers, virtio is probably
> not the first choice. Would e.g. doing a SenseID or so actually be
> useful in some way already? After all, it does transfer memory (but
> only in one direction).

The kvm-unit-test part is in development too.

Doing a SenseID will be implemented to recognize the PONG device.

>
>>>> +static inline int rsch(unsigned long schid)
>>> I don't think anyone has tried rsch with QEMU before; sounds like a
>>> good idea to test this :)
>> With an do_subchannel_work_emulation() callback?
> You probably need to build a simple channel program that suspends
> itself and can be resumed later.

Yes, that is something I plan to do.

To sum-up:

in kvm-unit-test: implement all I/O instructions and force instructions 
errors, like memory error, operand etc. and expect the right reaction of 
the system.

in QEMU, add the necessary infrastructure to test this.

Regards,

Pierre

Cornelia Huck Nov. 22, 2019, 10:54 a.m. UTC | #11

On Fri, 22 Nov 2019 10:03:21 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 2019-11-21 17:02, Cornelia Huck wrote:
> > On Thu, 14 Nov 2019 11:11:18 +0100
> > Pierre Morel <pmorel@linux.ibm.com> wrote:
> >  
> >> On 2019-11-13 14:05, Cornelia Huck wrote:  
> >>> On Wed, 13 Nov 2019 13:23:19 +0100
> >>> Pierre Morel <pmorel@linux.ibm.com> wrote:

> >>>> - initializing the ORB pointing to a single READ CCW  
> >>> Out of curiosity: Would using a NOP also be an option?  
> >> It will work but will not be handled by this device, css.c intercept it
> >> in sch_handle_start_func_virtual.
> >>
> >> AFAIU If we want to have a really good testing environment, for driver
> >> testing for exemple, then it would be interesting to add a new
> >> do_subchannel_work callback like do_subchannel_work_emulation along with
> >> the _virtual and _paththrough variantes.
> >>
> >> Having a dedicated callback for emulation, we can answer to any CSS
> >> instructions and SSCH commands, including NOP and TIC.  
> > I guess that depends on what you want to test; if you actually want to
> > test device emulation as used by virtio etc., you obviously want to go
> > through the existing _virtual callback :)  
> 
> The first goal is to test basic I/O from inside the kvm-unit-test, 
> producing errors and see how the system respond to errors.
> 
> In a standard system errors will be generated by QEMU analysing the I/O 
> instruction after interception.
> 
> In a secured guest, we expect the same errors, however we want to check 
> this.

But we still get the intercepts for all I/O instructions, right? We
just get/inject the parameters in a slightly different way, IIUC.

Not that I disagree with wanting to check this :)

> This PONG device is intended to be low level, no VIRTIO, and to allow 
> basic I/O.

Ok, so this is designed to test basic channel I/O handling, not
necessarily if the guest has set up all its control structures
correctly?

> > The actual motivation behind my question was:
> > Is it possible to e.g. throw NOP (or TIC, or something else not
> > device-specific) at a normal, existing virtio device for test purposes?
> > You'd end up testing the common emulation code without needing any
> > extra support in QEMU. No idea how useful that would be.  
> 
> Writing a VIRTIO driver inside the kvm-unit-test is something we can do 
> in the future.
> 
> As you said, the common code already handle NOP and TIC, the 
> interpretation of the
> CCW chain, once the SSCH has been intercepted is done by QEMU.
> I do not think it would be different with SE.

Yes. You don't really need to get the virtio device up on the virtio
side; if recognizing the device correctly via senseID works and you
maybe can do some NOP/TIC commands, you might have a very basic test
without introducing a new device.

Testing virtio-ccw via kvm-unit-tests is probably a good idea for the
future.

> To sum-up:
> 
> in kvm-unit-test: implement all I/O instructions and force instructions 
> errors, like memory error, operand etc. and expect the right reaction of 
> the system.
> 
> in QEMU, add the necessary infrastructure to test this.

Sounds good to me.

Pierre Morel Nov. 22, 2019, 12:48 p.m. UTC | #12

On 2019-11-22 11:54, Cornelia Huck wrote:
> On Fri, 22 Nov 2019 10:03:21 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> On 2019-11-21 17:02, Cornelia Huck wrote:
>>> On Thu, 14 Nov 2019 11:11:18 +0100
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>   
>>>> On 2019-11-13 14:05, Cornelia Huck wrote:
>>>>> On Wed, 13 Nov 2019 13:23:19 +0100
>>>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>>>> - initializing the ORB pointing to a single READ CCW
>>>>> Out of curiosity: Would using a NOP also be an option?
>>>> It will work but will not be handled by this device, css.c intercept it
>>>> in sch_handle_start_func_virtual.
>>>>
>>>> AFAIU If we want to have a really good testing environment, for driver
>>>> testing for exemple, then it would be interesting to add a new
>>>> do_subchannel_work callback like do_subchannel_work_emulation along with
>>>> the _virtual and _paththrough variantes.
>>>>
>>>> Having a dedicated callback for emulation, we can answer to any CSS
>>>> instructions and SSCH commands, including NOP and TIC.
>>> I guess that depends on what you want to test; if you actually want to
>>> test device emulation as used by virtio etc., you obviously want to go
>>> through the existing _virtual callback :)
>> The first goal is to test basic I/O from inside the kvm-unit-test,
>> producing errors and see how the system respond to errors.
>>
>> In a standard system errors will be generated by QEMU analysing the I/O
>> instruction after interception.
>>
>> In a secured guest, we expect the same errors, however we want to check
>> this.
> But we still get the intercepts for all I/O instructions, right? We
> just get/inject the parameters in a slightly different way, IIUC.
>
> Not that I disagree with wanting to check this :)

AFAIU the SE firmware, the SIE and KVM first handle the instruction 
interception before it comes to the QEMU code.

There are two major changes with secure execution that we want to test, 
SE firmware and SIE modifications.
If the instruction is treated by QEMU, then hopefully we get the same 
answer as without SE.


>
>> This PONG device is intended to be low level, no VIRTIO, and to allow
>> basic I/O.
> Ok, so this is designed to test basic channel I/O handling, not
> necessarily if the guest has set up all its control structures
> correctly?

More than this it is intended, in the next version, to test answers to 
bad configurations and wrong instruction's arguments.


>
>>> The actual motivation behind my question was:
>>> Is it possible to e.g. throw NOP (or TIC, or something else not
>>> device-specific) at a normal, existing virtio device for test purposes?
>>> You'd end up testing the common emulation code without needing any
>>> extra support in QEMU. No idea how useful that would be.
>> Writing a VIRTIO driver inside the kvm-unit-test is something we can do
>> in the future.
>>
>> As you said, the common code already handle NOP and TIC, the
>> interpretation of the
>> CCW chain, once the SSCH has been intercepted is done by QEMU.
>> I do not think it would be different with SE.
> Yes. You don't really need to get the virtio device up on the virtio
> side; if recognizing the device correctly via senseID works and you
> maybe can do some NOP/TIC commands, you might have a very basic test
> without introducing a new device.

Right, but the test is incomplete, as you said before, no write 
operation with this procedure.


>
> Testing virtio-ccw via kvm-unit-tests is probably a good idea for the
> future.
>
>> To sum-up:
>>
>> in kvm-unit-test: implement all I/O instructions and force instructions
>> errors, like memory error, operand etc. and expect the right reaction of
>> the system.
>>
>> in QEMU, add the necessary infrastructure to test this.
> Sounds good to me.

Thanks,

I think the next version will make the purpose of all of it even more 
obvious,
and hopefully answers all your questions better.

Best regards,

Pierre

>

[v1,4/4] s390x: Testing the Subchannel I/O read

Commit Message

Comments

Patch