On Thu, 28 Nov 2019 13:46:06 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
> When a channel is enabled we can start a SENSE command using the SSCH
> instruction to recognize the control unit and device.
>
> This tests the success of SSCH, the I/O interruption and the TSCH
> instructions.
>
> The test expect a device with a control unit type of 0xC0CA.
s/expect/expects/
>
> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
> ---
> lib/s390x/css.h | 13 +++++
> s390x/css.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 150 insertions(+)
>
(...)
> diff --git a/s390x/css.c b/s390x/css.c
> index e42dc2f..534864f 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -11,12 +11,28 @@
> */
>
> #include <libcflat.h>
> +#include <alloc_phys.h>
> +#include <asm/page.h>
> +#include <string.h>
> +#include <asm/interrupt.h>
> +#include <asm/arch_def.h>
> +#include <asm/clock.h>
>
> #include <css.h>
>
> #define SID_ONE 0x00010000
> +#define PSW_PRG_MASK (PSW_MASK_IO | PSW_MASK_EA | PSW_MASK_BA)
> +
> +struct lowcore *lowcore = (void *)0x0;
>
> static struct schib schib;
> +#define NB_CCW 100
s/NB_CCW/NUM_CCWS/ ?
I was scratching my head a bit when I first saw that define.
> +static struct ccw ccw[NB_CCW];
> +#define NB_ORB 100
> +static struct orb orb[NB_ORB];
> +static struct irb irb;
> +static char buffer[0x1000] __attribute__ ((aligned(8)));
> +static struct senseid senseid;
>
> static const char *Channel_type[3] = {
> "I/O", "CHSC", "MSG"
> @@ -24,6 +40,34 @@ static const char *Channel_type[3] = {
>
> static int test_device_sid;
>
(...)
> +void handle_io_int(void)
> +{
> + int ret = 0;
> + char *flags;
> +
> + report_prefix_push("Interrupt");
> + if (lowcore->io_int_param != 0xcafec0ca) {
> + report("Bad io_int_param: %x", 0, lowcore->io_int_param);
> + report_prefix_pop();
> + return;
> + }
Should you accommodate unsolicited interrupts as well?
> + report("io_int_param: %x", 1, lowcore->io_int_param);
> + report_prefix_pop();
> +
> + ret = tsch(lowcore->subsys_id_word, &irb);
> + dump_irb(&irb);
> + flags = dump_scsw_flags(irb.scsw.ctrl);
> +
> + if (ret)
> + report("IRB scsw flags: %s", 0, flags);
> + else
> + report("IRB scsw flags: %s", 1, flags);
> + report_prefix_pop();
> +}
> +
> +static int start_subchannel(int code, char *data, int count)
> +{
> + int ret;
> + struct pmcw *p = &schib.pmcw;
> + struct orb *orb_p = &orb[0];
> +
> + if (!test_device_sid) {
> + report_skip("No device");
> + return 0;
> + }
> + ret = stsch(test_device_sid, &schib);
That schib is a global variable, isn't it? Why do you need to re-check?
> + if (ret) {
> + report("Err %d on stsch on sid %08x", 0, ret, test_device_sid);
> + return 0;
> + }
> + if (!(p->flags & PMCW_ENABLE)) {
> + report_skip("Device (sid %08x) not enabled", test_device_sid);
> + return 0;
> + }
> + ccw[0].code = code ;
Extra ' ' before ';'
> + ccw[0].flags = CCW_F_PCI;
Huh, what's that PCI for?
> + ccw[0].count = count;
> + ccw[0].data = (int)(unsigned long)data;
Can you be sure that data is always below 2G?
> + orb_p->intparm = 0xcafec0ca;
> + orb_p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
> + orb_p->cpa = (unsigned int) (unsigned long)&ccw[0];
> +
> + report_prefix_push("Start Subchannel");
> + ret = ssch(test_device_sid, orb_p);
> + if (ret) {
> + report("ssch cc=%d", 0, ret);
> + report_prefix_pop();
> + return 0;
> + }
> + report_prefix_pop();
> + return 1;
> +}
> +
> +static void test_sense(void)
> +{
> + int success;
> +
> + enable_io_irq();
> +
> + success = start_subchannel(CCW_CMD_SENSE_ID, buffer, sizeof(senseid));
> + if (!success) {
> + report("start_subchannel failed", 0);
> + return;
> + }
> +
> + senseid.cu_type = buffer[2] | (buffer[1] << 8);
> + delay(1000);
> +
> + /* Sense ID is non packed cut_type is at offset +1 byte */
> + if (senseid.cu_type == PONG_CU)
> + report("cu_type: expect c0ca, got %04x", 1, senseid.cu_type);
> + else
> + report("cu_type: expect c0ca, got %04x", 0, senseid.cu_type);
> +}
I'm not really convinced by that logic here. This will fall apart if
you don't have your pong device exactly in the right place, and it does
not make it easy to extend this for more devices in the future.
What about the following:
- do a stsch() loop (basically re-use your first patch)
- for each I/O subchannel with dnv=1, do SenseID
- use the first (?) device with a c0ca CU type as your test device
Or maybe I'm overthinking this? It just does not strike me as very
robust and reusable.
> +
> static struct {
> const char *name;
> void (*func)(void);
> } tests[] = {
> { "enumerate (stsch)", test_enumerate },
> { "enable (msch)", test_enable },
> + { "sense (ssch/tsch)", test_sense },
> { NULL, NULL }
> };
>
