| Message ID | 1544623878-11248-11-git-send-email-jjherne@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390: vfio-ccw dasd ipl support | expand |
On 12/12/2018 09:11 AM, Jason J. Herne wrote: > Add struct for format-0 ccws. Support executing format-0 channel > programs and waiting for their completion before continuing execution. > This will be used for real dasd ipl. > > Add cu_type() to channel io library. This will be used to query control > unit type which is used to determine if we are booting a virtio device or a > real dasd device. > > Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com> > --- > pc-bios/s390-ccw/cio.c | 108 ++++++++++++++++++++++++++++++++++++++ > pc-bios/s390-ccw/cio.h | 124 ++++++++++++++++++++++++++++++++++++++++++-- > pc-bios/s390-ccw/s390-ccw.h | 1 + > pc-bios/s390-ccw/start.S | 33 +++++++++++- > 4 files changed, 261 insertions(+), 5 deletions(-) > > diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c > index 095f79b..9019250 100644 > --- a/pc-bios/s390-ccw/cio.c > +++ b/pc-bios/s390-ccw/cio.c > @@ -10,6 +10,7 @@ > > #include "libc.h" > #include "s390-ccw.h" > +#include "s390-arch.h" > #include "cio.h" > > static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE))); > @@ -39,3 +40,110 @@ void enable_subchannel(SubChannelId schid) > schib.pmcw.ena = 1; > msch(schid, &schib); > } > + > +uint16_t cu_type(SubChannelId schid) > +{ > + Ccw1 sense_id_ccw; > + SenseId sense_data; > + > + sense_id_ccw.cmd_code = CCW_CMD_SENSE_ID; > + sense_id_ccw.cda = ptr2u32(&sense_data); > + sense_id_ccw.count = sizeof(sense_data); > + > + if (do_cio(schid, ptr2u32(&sense_id_ccw), CCW_FMT1)) { > + panic("Failed to run SenseID CCw\n"); > + } > + > + return sense_data.cu_type; > +} > + > +void basic_sense(SubChannelId schid, SenseData *sd) > +{ > + Ccw1 senseCcw; > + > + senseCcw.cmd_code = CCW_CMD_BASIC_SENSE; > + senseCcw.cda = ptr2u32(sd); > + senseCcw.count = sizeof(*sd); > + > + if (do_cio(schid, ptr2u32(&senseCcw), CCW_FMT1)) { > + panic("Failed to run Basic Sense CCW\n"); > + } > +} > + > +static bool irb_error(Irb *irb) > +{ > + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because > + * real devices expect a 24 byte SenseID buffer, and virtio devices expect > + * a much larger buffer. Neither device type can tolerate a buffer size > + * different from what they expect so they set this indicator. > + */ > + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) { > + return true; > + } > + return irb->scsw.dstat != 0xc; > +} > + > +/* Executes a channel program at a given subchannel. The request to run the > + * channel program is sent to the subchannel, we then wait for the interrupt > + * singaling completion of the I/O operation(s) perfomed by the channel > + * program. Lastly we verify that the i/o operation completed without error and > + * that the interrupt we received was for the subchannel used to run the > + * channel program. > + * > + * Note: This function assumes it is running in an environment where no other > + * cpus are generating or receiving I/O interrupts. So either run it in a > + * single-cpu environment or make sure all other cpus are not doing I/O and > + * have I/O interrupts masked off. > + */ > +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt) > +{ > + CmdOrb orb = {}; > + Irb irb = {}; > + SenseData sd; > + int rc, retries = 0; > + > + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format"); > + > + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */ > + if (fmt == 0) { > + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address"); > + } > + > + orb.fmt = fmt ; > + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */ > + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */ > + orb.lpm = 0xFF; /* All paths allowed */ > + orb.cpa = ccw_addr; > + > + while (true) { > + rc = ssch(schid, &orb); > + if (rc) { > + print_int("ssch failed with rc=", rc); > + break; > + } > + > + consume_io_int(); > + > + /* Clear read */ > + rc = tsch(schid, &irb); > + if (rc) { > + print_int("tsch failed with rc=", rc); > + break; > + } > + > + if (!irb_error(&irb)) { > + break; > + } Now that we enable i/o interrupts, do we still need drain_irqs function anymore? Maybe we could refactor that. My question is more out of curiosity and I know it's out of the score for this patch series. > + > + /* Unexpected unit check. Use sense to clear unit check then retry. */ > + if (unit_check(&irb) && retries <= 2) { > + basic_sense(schid, &sd); > + retries++; > + continue; > + } > + > + break; > + } > + > + return rc; > +} > diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h > index 7b07d75..5c16635 100644 > --- a/pc-bios/s390-ccw/cio.h > +++ b/pc-bios/s390-ccw/cio.h > @@ -70,9 +70,46 @@ struct scsw { > __u16 count; > } __attribute__ ((packed)); > > -#define SCSW_FCTL_CLEAR_FUNC 0x1000 > -#define SCSW_FCTL_HALT_FUNC 0x2000 > +/* Function Control */ > #define SCSW_FCTL_START_FUNC 0x4000 > +#define SCSW_FCTL_HALT_FUNC 0x2000 > +#define SCSW_FCTL_CLEAR_FUNC 0x1000 > + > +/* Activity Control */ > +#define SCSW_ACTL_RESUME_PEND 0x0800 > +#define SCSW_ACTL_START_PEND 0x0400 > +#define SCSW_ACTL_HALT_PEND 0x0200 > +#define SCSW_ACTL_CLEAR_PEND 0x0100 > +#define SCSW_ACTL_CH_ACTIVE 0x0080 > +#define SCSW_ACTL_DEV_ACTIVE 0x0040 > +#define SCSW_ACTL_SUSPENDED 0x0020 > + > +/* Status Control */ > +#define SCSW_SCTL_ALERT 0x0010 > +#define SCSW_SCTL_INTERMED 0x0008 > +#define SCSW_SCTL_PRIMARY 0x0004 > +#define SCSW_SCTL_SECONDARY 0x0002 > +#define SCSW_SCTL_STATUS_PEND 0x0001 > + > +/* SCSW Device Status Flags */ > +#define SCSW_DSTAT_ATTN 0x80 > +#define SCSW_DSTAT_STATMOD 0x40 > +#define SCSW_DSTAT_CUEND 0x20 > +#define SCSW_DSTAT_BUSY 0x10 > +#define SCSW_DSTAT_CHEND 0x08 > +#define SCSW_DSTAT_DEVEND 0x04 > +#define SCSW_DSTAT_UCHK 0x02 > +#define SCSW_DSTAT_UEXCP 0x01 > + > +/* SCSW Subchannel Status Flags */ > +#define SCSW_CSTAT_PCINT 0x80 > +#define SCSW_CSTAT_BADLEN 0x40 > +#define SCSW_CSTAT_PROGCHK 0x20 > +#define SCSW_CSTAT_PROTCHK 0x10 > +#define SCSW_CSTAT_CHDCHK 0x08 > +#define SCSW_CSTAT_CHCCHK 0x04 > +#define SCSW_CSTAT_ICCHK 0x02 > +#define SCSW_CSTAT_CHAINCHK 0x01 > > /* > * subchannel information block > @@ -127,7 +164,23 @@ struct tpi_info { > __u32 reserved4 : 12; > } __attribute__ ((packed, aligned(4))); > > -/* channel command word (type 1) */ > +/* channel command word (format 0) */ > +typedef struct ccw0 { > + __u8 cmd_code; > + __u32 cda : 24; > + __u32 chainData : 1; > + __u32 chain : 1; > + __u32 sli : 1; > + __u32 skip : 1; > + __u32 pci : 1; > + __u32 ida : 1; > + __u32 suspend : 1; > + __u32 mida : 1; > + __u8 reserved; > + __u16 count; > +} __attribute__ ((packed, aligned(8))) Ccw0; > + > +/* channel command word (format 1) */ > typedef struct ccw1 { > __u8 cmd_code; > __u8 flags; > @@ -135,6 +188,10 @@ typedef struct ccw1 { > __u32 cda; > } __attribute__ ((packed, aligned(8))) Ccw1; > > +/* do_cio() CCW formats */ > +#define CCW_FMT0 0x00 > +#define CCW_FMT1 0x01 > + > #define CCW_FLAG_DC 0x80 > #define CCW_FLAG_CC 0x40 > #define CCW_FLAG_SLI 0x20 > @@ -190,6 +247,9 @@ struct ciw { > __u16 count; > }; > > +#define CU_TYPE_VIRTIO 0x3832 > +#define CU_TYPE_DASD 0x3990 > + > /* > * sense-id response buffer layout > */ > @@ -205,6 +265,61 @@ typedef struct senseid { > struct ciw ciw[62]; > } __attribute__ ((packed, aligned(4))) SenseId; > > +/* architected values for first sense byte */ > +#define SNS0_CMD_REJECT 0x80 > +#define SNS0_INTERVENTION_REQ 0x40 > +#define SNS0_BUS_OUT_CHECK 0x20 > +#define SNS0_EQUIPMENT_CHECK 0x10 > +#define SNS0_DATA_CHECK 0x08 > +#define SNS0_OVERRUN 0x04 > +#define SNS0_INCOMPL_DOMAIN 0x01 > + > +/* architectured values for second sense byte */ > +#define SNS1_PERM_ERR 0x80 > +#define SNS1_INV_TRACK_FORMAT 0x40 > +#define SNS1_EOC 0x20 > +#define SNS1_MESSAGE_TO_OPER 0x10 > +#define SNS1_NO_REC_FOUND 0x08 > +#define SNS1_FILE_PROTECTED 0x04 > +#define SNS1_WRITE_INHIBITED 0x02 > +#define SNS1_INPRECISE_END 0x01 > + > +/* architectured values for third sense byte */ > +#define SNS2_REQ_INH_WRITE 0x80 > +#define SNS2_CORRECTABLE 0x40 > +#define SNS2_FIRST_LOG_ERR 0x20 > +#define SNS2_ENV_DATA_PRESENT 0x10 > +#define SNS2_INPRECISE_END 0x04 > + > +/* 24-byte Sense fmt/msg codes */ > +#define SENSE24_FMT_PROG_SYS 0x0 > +#define SENSE24_FMT_EQUIPMENT 0x2 > +#define SENSE24_FMT_CONTROLLER 0x3 > +#define SENSE24_FMT_MISC 0xF > + > +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 > + > +/* basic sense response buffer layout */ > +typedef struct senseData { > + uint8_t status[3]; > + uint8_t res_count; > + uint8_t phys_drive_id; > + uint8_t low_cyl_addr; > + uint8_t head_high_cyl_addr; > + uint8_t fmt_msg; > + uint64_t fmt_dependent_info[2]; > + uint8_t reserved; > + uint8_t program_action_code; > + uint16_t config_info; > + uint8_t mcode_hicyl; > + uint8_t cyl_head_addr[3]; > +} __attribute__ ((packed, aligned(4))) SenseData; > + > +#define SENSE24_GET_FMT(sd) (sd->fmt_msg & 0xF0 >> 4) > +#define SENSE24_GET_MSG(sd) (sd->fmt_msg & 0x0F) > + > +#define unit_check(irb) ((irb)->scsw.dstat & SCSW_DSTAT_UCHK) > + > /* interruption response block */ > typedef struct irb { > struct scsw scsw; > @@ -215,6 +330,9 @@ typedef struct irb { > > int enable_mss_facility(void); > void enable_subchannel(SubChannelId schid); > +uint16_t cu_type(SubChannelId schid); > +void basic_sense(SubChannelId schid, SenseData *sd); > +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt); > > /* > * Some S390 specific IO instructions as inline > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h > index b39ee5d..11bce7d 100644 > --- a/pc-bios/s390-ccw/s390-ccw.h > +++ b/pc-bios/s390-ccw/s390-ccw.h > @@ -52,6 +52,7 @@ typedef unsigned long long __u64; > /* start.s */ > void disabled_wait(void); > void consume_sclp_int(void); > +void consume_io_int(void); > > /* main.c */ > void panic(const char *string); > diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S > index eb8d024..a48c38f 100644 > --- a/pc-bios/s390-ccw/start.S > +++ b/pc-bios/s390-ccw/start.S > @@ -65,12 +65,32 @@ consume_sclp_int: > /* prepare external call handler */ > larl %r1, external_new_code > stg %r1, 0x1b8 > - larl %r1, external_new_mask > + larl %r1, int_new_mask > mvc 0x1b0(8),0(%r1) > /* load enabled wait PSW */ > larl %r1, enabled_wait_psw > lpswe 0(%r1) > > +/* > + * void consume_io_int(void) > + * > + * eats one I/O interrupt > + */ > + .globl consume_io_int > +consume_io_int: > + /* enable I/O interrupts in cr0 */ > + stctg 6,6,0(15) > + oi 4(15), 0xff > + lctlg 6,6,0(15) > + /* prepare external call handler */ shouldn't this be i/o call handler? > + larl %r1, io_new_code > + stg %r1, 0x1f8 > + larl %r1, int_new_mask > + mvc 0x1f0(8),0(%r1) > + /* load enabled wait PSW */ > + larl %r1, enabled_wait_psw > + lpswe 0(%r1) > + > external_new_code: > /* disable service interrupts in cr0 */ > stctg 0,0,0(15) > @@ -78,10 +98,19 @@ external_new_code: > lctlg 0,0,0(15) > br 14 > > +io_new_code: > + /* disable I/O interrupts in cr6 */ > + stctg 6,6,0(15) > + ni 4(15), 0x00 > + lctlg 6,6,0(15) > + br 14 > + > + > + > .align 8 > disabled_wait_psw: > .quad 0x0002000180000000,0x0000000000000000 > enabled_wait_psw: > .quad 0x0302000180000000,0x0000000000000000 > -external_new_mask: > +int_new_mask: > .quad 0x0000000180000000 >
On Wed, 12 Dec 2018 09:11:13 -0500 "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > Add struct for format-0 ccws. Support executing format-0 channel > programs and waiting for their completion before continuing execution. > This will be used for real dasd ipl. > > Add cu_type() to channel io library. This will be used to query control > unit type which is used to determine if we are booting a virtio device or a > real dasd device. > > Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com> > --- > pc-bios/s390-ccw/cio.c | 108 ++++++++++++++++++++++++++++++++++++++ > pc-bios/s390-ccw/cio.h | 124 ++++++++++++++++++++++++++++++++++++++++++-- > pc-bios/s390-ccw/s390-ccw.h | 1 + > pc-bios/s390-ccw/start.S | 33 +++++++++++- > 4 files changed, 261 insertions(+), 5 deletions(-) > (...) > +static bool irb_error(Irb *irb) > +{ > + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because > + * real devices expect a 24 byte SenseID buffer, and virtio devices expect > + * a much larger buffer. Neither device type can tolerate a buffer size > + * different from what they expect so they set this indicator. Hm, can't you specify SLI for SenseID? > + */ > + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) { > + return true; > + } > + return irb->scsw.dstat != 0xc; Also, shouldn't you actually use the #defines you introduce further down? > +} > + > +/* Executes a channel program at a given subchannel. The request to run the > + * channel program is sent to the subchannel, we then wait for the interrupt > + * singaling completion of the I/O operation(s) perfomed by the channel > + * program. Lastly we verify that the i/o operation completed without error and > + * that the interrupt we received was for the subchannel used to run the > + * channel program. > + * > + * Note: This function assumes it is running in an environment where no other > + * cpus are generating or receiving I/O interrupts. So either run it in a > + * single-cpu environment or make sure all other cpus are not doing I/O and > + * have I/O interrupts masked off. Anything about iscs here (cr6)? > + */ > +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt) > +{ > + CmdOrb orb = {}; > + Irb irb = {}; > + SenseData sd; > + int rc, retries = 0; > + > + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format"); > + > + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */ > + if (fmt == 0) { > + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address"); > + } > + > + orb.fmt = fmt ; > + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */ > + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */ > + orb.lpm = 0xFF; /* All paths allowed */ > + orb.cpa = ccw_addr; > + > + while (true) { > + rc = ssch(schid, &orb); I think we can get here: - cc 0 -> all ok - cc 1 -> status pending; could that be an unsolicited interrupt from the device? or would we always get a deferred cc 1 in that case? - cc 2 -> another function pending; Should Not Happen - cc 3 -> it's dead, Jim So I'm wondering whether we should consume the status and retry for cc 1. The handling of the others is fine. > + if (rc) { > + print_int("ssch failed with rc=", rc); > + break; > + } > + > + consume_io_int(); > + > + /* Clear read */ I find that comment confusing. /* collect status */ maybe? > + rc = tsch(schid, &irb); Here we can get: - cc 0 -> status pending, all ok - cc 1 -> no status pending, Should Not Happen - cc 3 -> it's dead, Jim So this looks fine. > + if (rc) { > + print_int("tsch failed with rc=", rc); > + break; > + } > + > + if (!irb_error(&irb)) { > + break; > + } > + > + /* Unexpected unit check. Use sense to clear unit check then retry. */ The dasds still don't support concurrent sense, do they? Might also be worth investigating whether some unit checks are more "recoverable" than others. I expect we simply want to ignore IFCCs? IIRC, the strategy for those is "retry, in case it is transient"; but that may take some time. Or was there some path handling to be considered? (i.e., retrying may select another path, which may be fine.) > + if (unit_check(&irb) && retries <= 2) { > + basic_sense(schid, &sd); > + retries++; > + continue; > + } > + > + break; > + } > + > + return rc; > +} (...) > @@ -190,6 +247,9 @@ struct ciw { > __u16 count; > }; > > +#define CU_TYPE_VIRTIO 0x3832 > +#define CU_TYPE_DASD 0x3990 No other dasd types we want to support? :) (Not sure if others are out in the wild. Maybe FBA?) > + > /* > * sense-id response buffer layout > */ > @@ -205,6 +265,61 @@ typedef struct senseid { > struct ciw ciw[62]; > } __attribute__ ((packed, aligned(4))) SenseId; > > +/* architected values for first sense byte */ > +#define SNS0_CMD_REJECT 0x80 > +#define SNS0_INTERVENTION_REQ 0x40 > +#define SNS0_BUS_OUT_CHECK 0x20 > +#define SNS0_EQUIPMENT_CHECK 0x10 > +#define SNS0_DATA_CHECK 0x08 > +#define SNS0_OVERRUN 0x04 > +#define SNS0_INCOMPL_DOMAIN 0x01 IIRC, only byte 0 is device independent, and the others below are (ECKD) dasd specific? > + > +/* architectured values for second sense byte */ > +#define SNS1_PERM_ERR 0x80 > +#define SNS1_INV_TRACK_FORMAT 0x40 > +#define SNS1_EOC 0x20 > +#define SNS1_MESSAGE_TO_OPER 0x10 > +#define SNS1_NO_REC_FOUND 0x08 > +#define SNS1_FILE_PROTECTED 0x04 > +#define SNS1_WRITE_INHIBITED 0x02 > +#define SNS1_INPRECISE_END 0x01 > + > +/* architectured values for third sense byte */ > +#define SNS2_REQ_INH_WRITE 0x80 > +#define SNS2_CORRECTABLE 0x40 > +#define SNS2_FIRST_LOG_ERR 0x20 > +#define SNS2_ENV_DATA_PRESENT 0x10 > +#define SNS2_INPRECISE_END 0x04 > + > +/* 24-byte Sense fmt/msg codes */ > +#define SENSE24_FMT_PROG_SYS 0x0 > +#define SENSE24_FMT_EQUIPMENT 0x2 > +#define SENSE24_FMT_CONTROLLER 0x3 > +#define SENSE24_FMT_MISC 0xF > + > +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 > + > +/* basic sense response buffer layout */ > +typedef struct senseData { > + uint8_t status[3]; > + uint8_t res_count; > + uint8_t phys_drive_id; > + uint8_t low_cyl_addr; > + uint8_t head_high_cyl_addr; > + uint8_t fmt_msg; > + uint64_t fmt_dependent_info[2]; > + uint8_t reserved; > + uint8_t program_action_code; > + uint16_t config_info; > + uint8_t mcode_hicyl; > + uint8_t cyl_head_addr[3]; > +} __attribute__ ((packed, aligned(4))) SenseData; And this looks _really_ dasd specific. > + > +#define SENSE24_GET_FMT(sd) (sd->fmt_msg & 0xF0 >> 4) > +#define SENSE24_GET_MSG(sd) (sd->fmt_msg & 0x0F) > + > +#define unit_check(irb) ((irb)->scsw.dstat & SCSW_DSTAT_UCHK) > + > /* interruption response block */ > typedef struct irb { > struct scsw scsw; (...) > diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S > index eb8d024..a48c38f 100644 > --- a/pc-bios/s390-ccw/start.S > +++ b/pc-bios/s390-ccw/start.S > @@ -65,12 +65,32 @@ consume_sclp_int: > /* prepare external call handler */ > larl %r1, external_new_code > stg %r1, 0x1b8 > - larl %r1, external_new_mask > + larl %r1, int_new_mask > mvc 0x1b0(8),0(%r1) > /* load enabled wait PSW */ > larl %r1, enabled_wait_psw > lpswe 0(%r1) > > +/* > + * void consume_io_int(void) > + * > + * eats one I/O interrupt *nomnom* > + */ > + .globl consume_io_int > +consume_io_int: > + /* enable I/O interrupts in cr0 */ cr6? > + stctg 6,6,0(15) > + oi 4(15), 0xff > + lctlg 6,6,0(15) > + /* prepare external call handler */ I/O call handler? > + larl %r1, io_new_code > + stg %r1, 0x1f8 > + larl %r1, int_new_mask > + mvc 0x1f0(8),0(%r1) > + /* load enabled wait PSW */ > + larl %r1, enabled_wait_psw > + lpswe 0(%r1) > + > external_new_code: > /* disable service interrupts in cr0 */ > stctg 0,0,0(15) > @@ -78,10 +98,19 @@ external_new_code: > lctlg 0,0,0(15) > br 14 > > +io_new_code: > + /* disable I/O interrupts in cr6 */ > + stctg 6,6,0(15) I'm wondering why you are changing cr6 every time you wait for an I/O interrupt. Just enable the isc(s) you want once, and disable them again after you're done with all I/O? Simply disabling the I/O interrupts should be enough to prevent further interrupts popping up. You maybe want two enabled wait PSWs, one with I/O + external and one with external only? > + ni 4(15), 0x00 > + lctlg 6,6,0(15) > + br 14 > + > + > + > .align 8 > disabled_wait_psw: > .quad 0x0002000180000000,0x0000000000000000 > enabled_wait_psw: > .quad 0x0302000180000000,0x0000000000000000 > -external_new_mask: > +int_new_mask: > .quad 0x0000000180000000
On 12/13/18 12:21 PM, Cornelia Huck wrote: > On Wed, 12 Dec 2018 09:11:13 -0500 > "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > >> Add struct for format-0 ccws. Support executing format-0 channel >> programs and waiting for their completion before continuing execution. >> This will be used for real dasd ipl. >> >> Add cu_type() to channel io library. This will be used to query control >> unit type which is used to determine if we are booting a virtio device or a >> real dasd device. >> >> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com> >> --- >> pc-bios/s390-ccw/cio.c | 108 ++++++++++++++++++++++++++++++++++++++ >> pc-bios/s390-ccw/cio.h | 124 ++++++++++++++++++++++++++++++++++++++++++-- >> pc-bios/s390-ccw/s390-ccw.h | 1 + >> pc-bios/s390-ccw/start.S | 33 +++++++++++- >> 4 files changed, 261 insertions(+), 5 deletions(-) >> > > (...) > >> +static bool irb_error(Irb *irb) >> +{ >> + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because >> + * real devices expect a 24 byte SenseID buffer, and virtio devices expect >> + * a much larger buffer. Neither device type can tolerate a buffer size >> + * different from what they expect so they set this indicator. > > Hm, can't you specify SLI for SenseID? > Yes, but this requires modifying run_ccw() in virtio.c to always specify the SLI flag. I'm not sure that is the best choice? I suppose I could add an sli argument to run_ccw if you'd prefer that. >> + */ >> + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) { >> + return true; >> + } >> + return irb->scsw.dstat != 0xc; > > Also, shouldn't you actually use the #defines you introduce further > down? > Yep, I added the defines after I wrote this code. I'll fix that. >> +} >> + >> +/* Executes a channel program at a given subchannel. The request to run the >> + * channel program is sent to the subchannel, we then wait for the interrupt >> + * singaling completion of the I/O operation(s) perfomed by the channel >> + * program. Lastly we verify that the i/o operation completed without error and >> + * that the interrupt we received was for the subchannel used to run the >> + * channel program. >> + * >> + * Note: This function assumes it is running in an environment where no other >> + * cpus are generating or receiving I/O interrupts. So either run it in a >> + * single-cpu environment or make sure all other cpus are not doing I/O and >> + * have I/O interrupts masked off. > > Anything about iscs here (cr6)? > Those details are handled in the assembler code. Do you think I should mention something about cr6 here? >> + */ >> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt) >> +{ >> + CmdOrb orb = {}; >> + Irb irb = {}; >> + SenseData sd; >> + int rc, retries = 0; >> + >> + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format"); >> + >> + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */ >> + if (fmt == 0) { >> + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address"); >> + } >> + >> + orb.fmt = fmt ; >> + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */ >> + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */ >> + orb.lpm = 0xFF; /* All paths allowed */ >> + orb.cpa = ccw_addr; >> + >> + while (true) { >> + rc = ssch(schid, &orb); > > I think we can get here: > - cc 0 -> all ok > - cc 1 -> status pending; could that be an unsolicited interrupt from > the device? or would we always get a deferred cc 1 in that case? > - cc 2 -> another function pending; Should Not Happen > - cc 3 -> it's dead, Jim > > So I'm wondering whether we should consume the status and retry for cc > 1. The handling of the others is fine. > I took a look at css_do_ssch() in hw/s390x/css.c and it appears as though CC1 is a possibility here. I'm not against taking action, but I suspect we would have to clear the status with a basic sense (or something) before simply retrying... right? Is it safe for us to just assume we can clear it and move on? It seems like an edge case that we'd be better off failing on. Perhaps let the user try again which will redrive the process? >> + if (rc) { >> + print_int("ssch failed with rc=", rc); >> + break; >> + } >> + >> + consume_io_int(); >> + >> + /* Clear read */ > > I find that comment confusing. /* collect status */ maybe? > >> + rc = tsch(schid, &irb); > > Here we can get: > - cc 0 -> status pending, all ok > - cc 1 -> no status pending, Should Not Happen > - cc 3 -> it's dead, Jim > > So this looks fine. > >> + if (rc) { >> + print_int("tsch failed with rc=", rc); >> + break; >> + } >> + >> + if (!irb_error(&irb)) { >> + break; >> + } >> + >> + /* Unexpected unit check. Use sense to clear unit check then retry. */ > > The dasds still don't support concurrent sense, do they? Might also be > worth investigating whether some unit checks are more "recoverable" > than others. > I wasn't sure on concurrent sense. I'd bet there are situations or environments where it won't be supported so it seems safest to assume we don't have it. We already recover from the one unit check scenario I've discovered in practice (initial reset). And the algorithm I chose is to simply retry a few times whenever we're presented with unexpected unit check status. This is what the kernel does. It seems fairly robust. > I expect we simply want to ignore IFCCs? IIRC, the strategy for those > is "retry, in case it is transient"; but that may take some time. Or > was there some path handling to be considered? (i.e., retrying may > select another path, which may be fine.) > Currently we'll give up on IFCC. I think this is the right thing to do. A user can always retry if they want. But in reality an IFCC very likely means faulty hardware IIUC. I've not thought about path management much. I suspect paths changing isn't something we should realistically see in the bios. Even still, a retry is really all we can do, so assuming path changes result in a unit check then we should be okay there. >> + if (unit_check(&irb) && retries <= 2) { >> + basic_sense(schid, &sd); >> + retries++; >> + continue; >> + } >> + >> + break; >> + } >> + >> + return rc; >> +} > > (...) > >> @@ -190,6 +247,9 @@ struct ciw { >> __u16 count; >> }; >> >> +#define CU_TYPE_VIRTIO 0x3832 >> +#define CU_TYPE_DASD 0x3990 > > No other dasd types we want to support? :) (Not sure if others are out > in the wild. Maybe FBA?) > I have no idea. I assumed 3390 was the only thing we supported. Perhaps 3380? I'd need to find a test device, which I could probably do ... I'll look more into this. >> + >> /* >> * sense-id response buffer layout >> */ >> @@ -205,6 +265,61 @@ typedef struct senseid { >> struct ciw ciw[62]; >> } __attribute__ ((packed, aligned(4))) SenseId; >> >> +/* architected values for first sense byte */ >> +#define SNS0_CMD_REJECT 0x80 >> +#define SNS0_INTERVENTION_REQ 0x40 >> +#define SNS0_BUS_OUT_CHECK 0x20 >> +#define SNS0_EQUIPMENT_CHECK 0x10 >> +#define SNS0_DATA_CHECK 0x08 >> +#define SNS0_OVERRUN 0x04 >> +#define SNS0_INCOMPL_DOMAIN 0x01 > > IIRC, only byte 0 is device independent, and the others below are > (ECKD) dasd specific? > >> + >> +/* architectured values for second sense byte */ >> +#define SNS1_PERM_ERR 0x80 >> +#define SNS1_INV_TRACK_FORMAT 0x40 >> +#define SNS1_EOC 0x20 >> +#define SNS1_MESSAGE_TO_OPER 0x10 >> +#define SNS1_NO_REC_FOUND 0x08 >> +#define SNS1_FILE_PROTECTED 0x04 >> +#define SNS1_WRITE_INHIBITED 0x02 >> +#define SNS1_INPRECISE_END 0x01 >> + >> +/* architectured values for third sense byte */ >> +#define SNS2_REQ_INH_WRITE 0x80 >> +#define SNS2_CORRECTABLE 0x40 >> +#define SNS2_FIRST_LOG_ERR 0x20 >> +#define SNS2_ENV_DATA_PRESENT 0x10 >> +#define SNS2_INPRECISE_END 0x04 >> + >> +/* 24-byte Sense fmt/msg codes */ >> +#define SENSE24_FMT_PROG_SYS 0x0 >> +#define SENSE24_FMT_EQUIPMENT 0x2 >> +#define SENSE24_FMT_CONTROLLER 0x3 >> +#define SENSE24_FMT_MISC 0xF >> + >> +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 >> + >> +/* basic sense response buffer layout */ >> +typedef struct senseData { >> + uint8_t status[3]; >> + uint8_t res_count; >> + uint8_t phys_drive_id; >> + uint8_t low_cyl_addr; >> + uint8_t head_high_cyl_addr; >> + uint8_t fmt_msg; >> + uint64_t fmt_dependent_info[2]; >> + uint8_t reserved; >> + uint8_t program_action_code; >> + uint16_t config_info; >> + uint8_t mcode_hicyl; >> + uint8_t cyl_head_addr[3]; >> +} __attribute__ ((packed, aligned(4))) SenseData; > > And this looks _really_ dasd specific. > Yep, I glossed over those details while I was furiously tracking down the reset bug. I'll take a look at redesigning this. >> + >> +#define SENSE24_GET_FMT(sd) (sd->fmt_msg & 0xF0 >> 4) >> +#define SENSE24_GET_MSG(sd) (sd->fmt_msg & 0x0F) >> + >> +#define unit_check(irb) ((irb)->scsw.dstat & SCSW_DSTAT_UCHK) >> + >> /* interruption response block */ >> typedef struct irb { >> struct scsw scsw; > > (...) > >> diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S >> index eb8d024..a48c38f 100644 >> --- a/pc-bios/s390-ccw/start.S >> +++ b/pc-bios/s390-ccw/start.S >> @@ -65,12 +65,32 @@ consume_sclp_int: >> /* prepare external call handler */ >> larl %r1, external_new_code >> stg %r1, 0x1b8 >> - larl %r1, external_new_mask >> + larl %r1, int_new_mask >> mvc 0x1b0(8),0(%r1) >> /* load enabled wait PSW */ >> larl %r1, enabled_wait_psw >> lpswe 0(%r1) >> >> +/* >> + * void consume_io_int(void) >> + * >> + * eats one I/O interrupt > > *nomnom* > >> + */ >> + .globl consume_io_int >> +consume_io_int: >> + /* enable I/O interrupts in cr0 */ > > cr6? > >> + stctg 6,6,0(15) >> + oi 4(15), 0xff >> + lctlg 6,6,0(15) >> + /* prepare external call handler */ > > I/O call handler? > Both copy/paste errors. Thanks for catching these. :) >> + larl %r1, io_new_code >> + stg %r1, 0x1f8 >> + larl %r1, int_new_mask >> + mvc 0x1f0(8),0(%r1) >> + /* load enabled wait PSW */ >> + larl %r1, enabled_wait_psw >> + lpswe 0(%r1) >> + >> external_new_code: >> /* disable service interrupts in cr0 */ >> stctg 0,0,0(15) >> @@ -78,10 +98,19 @@ external_new_code: >> lctlg 0,0,0(15) >> br 14 >> >> +io_new_code: >> + /* disable I/O interrupts in cr6 */ >> + stctg 6,6,0(15) > > I'm wondering why you are changing cr6 every time you wait for an I/O > interrupt. Just enable the isc(s) you want once, and disable them again > after you're done with all I/O? Simply disabling the I/O interrupts > should be enough to prevent further interrupts popping up. You maybe > want two enabled wait PSWs, one with I/O + external and one with > external only? > No real reason. We only come through here a hand full of times so performance is not a consideration. I guess my thought process was probably to keep the system is as close to initial state as possible through the ipl process. Eventually when we hand control to the guest OS we want the system as close to undisturbed as possible. If you think I should only be setting cr-6 once, it sounds reasonable. >> + ni 4(15), 0x00 >> + lctlg 6,6,0(15) >> + br 14 >> + >> + >> + >> .align 8 >> disabled_wait_psw: >> .quad 0x0002000180000000,0x0000000000000000 >> enabled_wait_psw: >> .quad 0x0302000180000000,0x0000000000000000 >> -external_new_mask: >> +int_new_mask: >> .quad 0x0000000180000000 > >
On Mon, 7 Jan 2019 14:02:45 -0500 "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > On 12/13/18 12:21 PM, Cornelia Huck wrote: > > On Wed, 12 Dec 2018 09:11:13 -0500 > > "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > > > >> Add struct for format-0 ccws. Support executing format-0 channel > >> programs and waiting for their completion before continuing execution. > >> This will be used for real dasd ipl. > >> > >> Add cu_type() to channel io library. This will be used to query control > >> unit type which is used to determine if we are booting a virtio device or a > >> real dasd device. > >> > >> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com> > >> --- > >> pc-bios/s390-ccw/cio.c | 108 ++++++++++++++++++++++++++++++++++++++ > >> pc-bios/s390-ccw/cio.h | 124 ++++++++++++++++++++++++++++++++++++++++++-- > >> pc-bios/s390-ccw/s390-ccw.h | 1 + > >> pc-bios/s390-ccw/start.S | 33 +++++++++++- > >> 4 files changed, 261 insertions(+), 5 deletions(-) > >> > > > > (...) > > > >> +static bool irb_error(Irb *irb) > >> +{ > >> + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because > >> + * real devices expect a 24 byte SenseID buffer, and virtio devices expect > >> + * a much larger buffer. Neither device type can tolerate a buffer size > >> + * different from what they expect so they set this indicator. > > > > Hm, can't you specify SLI for SenseID? > > > > Yes, but this requires modifying run_ccw() in virtio.c to always specify the SLI flag. I'm > not sure that is the best choice? I suppose I could add an sli argument to run_ccw if > you'd prefer that. Ignoring an error feels wrong :) Telling the function "hey, I don't know what buffer size you expect, just give me what you have" feels better. If I read SA22-7204-01 correctly, there's always just a minimum of sense id data, and how much we get is device dependent. (FWIW, the Linux kernel does sense id with SLI as well.) So yes, +1 to adding a sli parameter to run_ccw(). > > >> + */ > >> + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) { > >> + return true; > >> + } > >> + return irb->scsw.dstat != 0xc; > > > > Also, shouldn't you actually use the #defines you introduce further > > down? > > > > Yep, I added the defines after I wrote this code. I'll fix that. > > >> +} > >> + > >> +/* Executes a channel program at a given subchannel. The request to run the > >> + * channel program is sent to the subchannel, we then wait for the interrupt > >> + * singaling completion of the I/O operation(s) perfomed by the channel s/perfomed/performed/ > >> + * program. Lastly we verify that the i/o operation completed without error and > >> + * that the interrupt we received was for the subchannel used to run the > >> + * channel program. > >> + * > >> + * Note: This function assumes it is running in an environment where no other > >> + * cpus are generating or receiving I/O interrupts. So either run it in a > >> + * single-cpu environment or make sure all other cpus are not doing I/O and > >> + * have I/O interrupts masked off. > > > > Anything about iscs here (cr6)? > > > > Those details are handled in the assembler code. Do you think I should mention something > about cr6 here? We can probably do without. > > >> + */ > >> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt) > >> +{ > >> + CmdOrb orb = {}; > >> + Irb irb = {}; > >> + SenseData sd; > >> + int rc, retries = 0; > >> + > >> + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format"); > >> + > >> + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */ > >> + if (fmt == 0) { > >> + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address"); > >> + } > >> + > >> + orb.fmt = fmt ; > >> + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */ > >> + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */ > >> + orb.lpm = 0xFF; /* All paths allowed */ > >> + orb.cpa = ccw_addr; > >> + > >> + while (true) { > >> + rc = ssch(schid, &orb); > > > > I think we can get here: > > - cc 0 -> all ok > > - cc 1 -> status pending; could that be an unsolicited interrupt from > > the device? or would we always get a deferred cc 1 in that case? > > - cc 2 -> another function pending; Should Not Happen > > - cc 3 -> it's dead, Jim > > > > So I'm wondering whether we should consume the status and retry for cc > > 1. The handling of the others is fine. > > > > I took a look at css_do_ssch() in hw/s390x/css.c and it appears as though CC1 is a > possibility here. I'm not against taking action, but I suspect we would have to clear the > status with a basic sense (or something) before simply retrying... right? It depends on the status. If you get an unit check, you'll probably need the basic sense; in other cases, you'll probably want to simply retry. > > Is it safe for us to just assume we can clear it and move on? It seems like an edge case > that we'd be better off failing on. Perhaps let the user try again which will redrive the > process? A very low amount of retries (2?) sounds reasonable: this would keep us going if there's a state from the device that will be ignored anyway, and won't get us stuck in a pointless retry loop if something more involved is going on. > > > >> + if (rc) { > >> + print_int("ssch failed with rc=", rc); > >> + break; > >> + } > >> + > >> + consume_io_int(); > >> + > >> + /* Clear read */ > > > > I find that comment confusing. /* collect status */ maybe? > > > >> + rc = tsch(schid, &irb); > > > > Here we can get: > > - cc 0 -> status pending, all ok > > - cc 1 -> no status pending, Should Not Happen > > - cc 3 -> it's dead, Jim > > > > So this looks fine. > > > >> + if (rc) { > >> + print_int("tsch failed with rc=", rc); > >> + break; > >> + } > >> + > >> + if (!irb_error(&irb)) { > >> + break; > >> + } > >> + > >> + /* Unexpected unit check. Use sense to clear unit check then retry. */ > > > > The dasds still don't support concurrent sense, do they? Might also be > > worth investigating whether some unit checks are more "recoverable" > > than others. > > > > I wasn't sure on concurrent sense. I'd bet there are situations or environments where it > won't be supported so it seems safest to assume we don't have it. Ok. > > We already recover from the one unit check scenario I've discovered in practice (initial > reset). And the algorithm I chose is to simply retry a few times whenever we're presented > with unexpected unit check status. This is what the kernel does. It seems fairly robust. Nod. > > I expect we simply want to ignore IFCCs? IIRC, the strategy for those > > is "retry, in case it is transient"; but that may take some time. Or > > was there some path handling to be considered? (i.e., retrying may > > select another path, which may be fine.) > > > > Currently we'll give up on IFCC. I think this is the right thing to do. A user can always > retry if they want. But in reality an IFCC very likely means faulty hardware IIUC. It could also be a transient link issue. Maybe retry twice, just to avoid the very tiny blips? > I've not thought about path management much. I suspect paths changing isn't something we > should realistically see in the bios. Even still, a retry is really all we can do, so > assuming path changes result in a unit check then we should be okay there. If you use a full path mask, the channel subsystem might try a different path (that is working correctly) the next time. I don't think you want to implement path grouping stuff in the bios, which would mean a lot of pain for very little gain :) Thinking about path groups: One scenario we might have is that another LPAR did a reserve on a dasd and then died. The dasd is then unaccessible by our LPAR until we do a steal lock. If the device is bound to the vfio-ccw subchannel driver, we don't have an interface for that, though (we would need to re-bind to the I/O subchannel driver and the dasd driver so we can invoke tunedasd). We could add an option to break the lock from the bios, although that's probably overkill for a real edge case. Just wanted to mention it :) > > > >> + if (unit_check(&irb) && retries <= 2) { > >> + basic_sense(schid, &sd); > >> + retries++; > >> + continue; > >> + } > >> + > >> + break; > >> + } > >> + > >> + return rc; > >> +} > > > > (...) > > > >> @@ -190,6 +247,9 @@ struct ciw { > >> __u16 count; > >> }; > >> > >> +#define CU_TYPE_VIRTIO 0x3832 > >> +#define CU_TYPE_DASD 0x3990 > > > > No other dasd types we want to support? :) (Not sure if others are out > > in the wild. Maybe FBA?) > > > > I have no idea. I assumed 3390 was the only thing we supported. Perhaps 3380? I'd need to > find a test device, which I could probably do ... I'll look more into this. IIRC, z/VM can hand out FBA devices. I'm not sure if current storage systems can emulate them. > > > >> + > >> /* > >> * sense-id response buffer layout > >> */ > >> @@ -205,6 +265,61 @@ typedef struct senseid { > >> struct ciw ciw[62]; > >> } __attribute__ ((packed, aligned(4))) SenseId; > >> > >> +/* architected values for first sense byte */ > >> +#define SNS0_CMD_REJECT 0x80 > >> +#define SNS0_INTERVENTION_REQ 0x40 > >> +#define SNS0_BUS_OUT_CHECK 0x20 > >> +#define SNS0_EQUIPMENT_CHECK 0x10 > >> +#define SNS0_DATA_CHECK 0x08 > >> +#define SNS0_OVERRUN 0x04 > >> +#define SNS0_INCOMPL_DOMAIN 0x01 > > > > IIRC, only byte 0 is device independent, and the others below are > > (ECKD) dasd specific? > > > >> + > >> +/* architectured values for second sense byte */ > >> +#define SNS1_PERM_ERR 0x80 > >> +#define SNS1_INV_TRACK_FORMAT 0x40 > >> +#define SNS1_EOC 0x20 > >> +#define SNS1_MESSAGE_TO_OPER 0x10 > >> +#define SNS1_NO_REC_FOUND 0x08 > >> +#define SNS1_FILE_PROTECTED 0x04 > >> +#define SNS1_WRITE_INHIBITED 0x02 > >> +#define SNS1_INPRECISE_END 0x01 > >> + > >> +/* architectured values for third sense byte */ > >> +#define SNS2_REQ_INH_WRITE 0x80 > >> +#define SNS2_CORRECTABLE 0x40 > >> +#define SNS2_FIRST_LOG_ERR 0x20 > >> +#define SNS2_ENV_DATA_PRESENT 0x10 > >> +#define SNS2_INPRECISE_END 0x04 > >> + > >> +/* 24-byte Sense fmt/msg codes */ > >> +#define SENSE24_FMT_PROG_SYS 0x0 > >> +#define SENSE24_FMT_EQUIPMENT 0x2 > >> +#define SENSE24_FMT_CONTROLLER 0x3 > >> +#define SENSE24_FMT_MISC 0xF > >> + > >> +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 > >> + > >> +/* basic sense response buffer layout */ > >> +typedef struct senseData { > >> + uint8_t status[3]; > >> + uint8_t res_count; > >> + uint8_t phys_drive_id; > >> + uint8_t low_cyl_addr; > >> + uint8_t head_high_cyl_addr; > >> + uint8_t fmt_msg; > >> + uint64_t fmt_dependent_info[2]; > >> + uint8_t reserved; > >> + uint8_t program_action_code; > >> + uint16_t config_info; > >> + uint8_t mcode_hicyl; > >> + uint8_t cyl_head_addr[3]; > >> +} __attribute__ ((packed, aligned(4))) SenseData; > > > > And this looks _really_ dasd specific. > > > > Yep, I glossed over those details while I was furiously tracking down the reset bug. I'll > take a look at redesigning this. Ok. > > >> + > >> +#define SENSE24_GET_FMT(sd) (sd->fmt_msg & 0xF0 >> 4) > >> +#define SENSE24_GET_MSG(sd) (sd->fmt_msg & 0x0F) > >> + > >> +#define unit_check(irb) ((irb)->scsw.dstat & SCSW_DSTAT_UCHK) > >> + > >> /* interruption response block */ > >> typedef struct irb { > >> struct scsw scsw; > > > > (...) > > > >> diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S > >> index eb8d024..a48c38f 100644 > >> --- a/pc-bios/s390-ccw/start.S > >> +++ b/pc-bios/s390-ccw/start.S > >> @@ -65,12 +65,32 @@ consume_sclp_int: > >> /* prepare external call handler */ > >> larl %r1, external_new_code > >> stg %r1, 0x1b8 > >> - larl %r1, external_new_mask > >> + larl %r1, int_new_mask > >> mvc 0x1b0(8),0(%r1) > >> /* load enabled wait PSW */ > >> larl %r1, enabled_wait_psw > >> lpswe 0(%r1) > >> > >> +/* > >> + * void consume_io_int(void) > >> + * > >> + * eats one I/O interrupt > > > > *nomnom* > > > >> + */ > >> + .globl consume_io_int > >> +consume_io_int: > >> + /* enable I/O interrupts in cr0 */ > > > > cr6? > > > >> + stctg 6,6,0(15) > >> + oi 4(15), 0xff > >> + lctlg 6,6,0(15) > >> + /* prepare external call handler */ > > > > I/O call handler? > > > > Both copy/paste errors. Thanks for catching these. :) > > >> + larl %r1, io_new_code > >> + stg %r1, 0x1f8 > >> + larl %r1, int_new_mask > >> + mvc 0x1f0(8),0(%r1) > >> + /* load enabled wait PSW */ > >> + larl %r1, enabled_wait_psw > >> + lpswe 0(%r1) > >> + > >> external_new_code: > >> /* disable service interrupts in cr0 */ > >> stctg 0,0,0(15) > >> @@ -78,10 +98,19 @@ external_new_code: > >> lctlg 0,0,0(15) > >> br 14 > >> > >> +io_new_code: > >> + /* disable I/O interrupts in cr6 */ > >> + stctg 6,6,0(15) > > > > I'm wondering why you are changing cr6 every time you wait for an I/O > > interrupt. Just enable the isc(s) you want once, and disable them again > > after you're done with all I/O? Simply disabling the I/O interrupts > > should be enough to prevent further interrupts popping up. You maybe > > want two enabled wait PSWs, one with I/O + external and one with > > external only? > > > > No real reason. We only come through here a hand full of times so performance is not a > consideration. I guess my thought process was probably to keep the system is as close to > initial state as possible through the ipl process. Eventually when we hand control to the > guest OS we want the system as close to undisturbed as possible. If you think I should > only be setting cr-6 once, it sounds reasonable. It just looked a bit odd to me. But I agree that this isn't performance-sensitive. > > > >> + ni 4(15), 0x00 > >> + lctlg 6,6,0(15) > >> + br 14 > >> + > >> + > >> + > >> .align 8 > >> disabled_wait_psw: > >> .quad 0x0002000180000000,0x0000000000000000 > >> enabled_wait_psw: > >> .quad 0x0302000180000000,0x0000000000000000 > >> -external_new_mask: > >> +int_new_mask: > >> .quad 0x0000000180000000 > > > > > >
On 1/7/19 2:02 PM, Jason J. Herne wrote: >>> + >>> /* >>> * sense-id response buffer layout >>> */ >>> @@ -205,6 +265,61 @@ typedef struct senseid { >>> struct ciw ciw[62]; >>> } __attribute__ ((packed, aligned(4))) SenseId; >>> +/* architected values for first sense byte */ >>> +#define SNS0_CMD_REJECT 0x80 >>> +#define SNS0_INTERVENTION_REQ 0x40 >>> +#define SNS0_BUS_OUT_CHECK 0x20 >>> +#define SNS0_EQUIPMENT_CHECK 0x10 >>> +#define SNS0_DATA_CHECK 0x08 >>> +#define SNS0_OVERRUN 0x04 >>> +#define SNS0_INCOMPL_DOMAIN 0x01 >> >> IIRC, only byte 0 is device independent, and the others below are >> (ECKD) dasd specific? >> >>> + >>> +/* architectured values for second sense byte */ >>> +#define SNS1_PERM_ERR 0x80 >>> +#define SNS1_INV_TRACK_FORMAT 0x40 >>> +#define SNS1_EOC 0x20 >>> +#define SNS1_MESSAGE_TO_OPER 0x10 >>> +#define SNS1_NO_REC_FOUND 0x08 >>> +#define SNS1_FILE_PROTECTED 0x04 >>> +#define SNS1_WRITE_INHIBITED 0x02 >>> +#define SNS1_INPRECISE_END 0x01 >>> + >>> +/* architectured values for third sense byte */ >>> +#define SNS2_REQ_INH_WRITE 0x80 >>> +#define SNS2_CORRECTABLE 0x40 >>> +#define SNS2_FIRST_LOG_ERR 0x20 >>> +#define SNS2_ENV_DATA_PRESENT 0x10 >>> +#define SNS2_INPRECISE_END 0x04 >>> + >>> +/* 24-byte Sense fmt/msg codes */ >>> +#define SENSE24_FMT_PROG_SYS 0x0 >>> +#define SENSE24_FMT_EQUIPMENT 0x2 >>> +#define SENSE24_FMT_CONTROLLER 0x3 >>> +#define SENSE24_FMT_MISC 0xF >>> + >>> +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 >>> + >>> +/* basic sense response buffer layout */ >>> +typedef struct senseData { >>> + uint8_t status[3]; >>> + uint8_t res_count; >>> + uint8_t phys_drive_id; >>> + uint8_t low_cyl_addr; >>> + uint8_t head_high_cyl_addr; >>> + uint8_t fmt_msg; >>> + uint64_t fmt_dependent_info[2]; >>> + uint8_t reserved; >>> + uint8_t program_action_code; >>> + uint16_t config_info; >>> + uint8_t mcode_hicyl; >>> + uint8_t cyl_head_addr[3]; >>> +} __attribute__ ((packed, aligned(4))) SenseData; >> >> And this looks _really_ dasd specific. >> > > Yep, I glossed over those details while I was furiously tracking down the reset bug. I'll > take a look at redesigning this. All of my information for creating these data structures came from an internal ECKD DASD reference. There are probably some things that could stand a bit of cleanup or renaming. Aside from that, considering this is in a DASD only (ECKD DASD only at the moment) code path are you okay with my renaming the struct to senseDataECKD or something similar? I'm not sure what value there is in abstracting sense at the moment. I'm not even sure what other device's sense data looks like. Since my description of the SENSE CCW comes from an ECKD reference I have not been able to verify any areas of the data that are common across device types. Thoughts?
On Wed, 9 Jan 2019 13:10:26 -0500 "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > On 1/7/19 2:02 PM, Jason J. Herne wrote: > >>> + > >>> /* > >>> * sense-id response buffer layout > >>> */ > >>> @@ -205,6 +265,61 @@ typedef struct senseid { > >>> struct ciw ciw[62]; > >>> } __attribute__ ((packed, aligned(4))) SenseId; > >>> +/* architected values for first sense byte */ > >>> +#define SNS0_CMD_REJECT 0x80 > >>> +#define SNS0_INTERVENTION_REQ 0x40 > >>> +#define SNS0_BUS_OUT_CHECK 0x20 > >>> +#define SNS0_EQUIPMENT_CHECK 0x10 > >>> +#define SNS0_DATA_CHECK 0x08 > >>> +#define SNS0_OVERRUN 0x04 > >>> +#define SNS0_INCOMPL_DOMAIN 0x01 > >> > >> IIRC, only byte 0 is device independent, and the others below are > >> (ECKD) dasd specific? > >> > >>> + > >>> +/* architectured values for second sense byte */ > >>> +#define SNS1_PERM_ERR 0x80 > >>> +#define SNS1_INV_TRACK_FORMAT 0x40 > >>> +#define SNS1_EOC 0x20 > >>> +#define SNS1_MESSAGE_TO_OPER 0x10 > >>> +#define SNS1_NO_REC_FOUND 0x08 > >>> +#define SNS1_FILE_PROTECTED 0x04 > >>> +#define SNS1_WRITE_INHIBITED 0x02 > >>> +#define SNS1_INPRECISE_END 0x01 > >>> + > >>> +/* architectured values for third sense byte */ > >>> +#define SNS2_REQ_INH_WRITE 0x80 > >>> +#define SNS2_CORRECTABLE 0x40 > >>> +#define SNS2_FIRST_LOG_ERR 0x20 > >>> +#define SNS2_ENV_DATA_PRESENT 0x10 > >>> +#define SNS2_INPRECISE_END 0x04 > >>> + > >>> +/* 24-byte Sense fmt/msg codes */ > >>> +#define SENSE24_FMT_PROG_SYS 0x0 > >>> +#define SENSE24_FMT_EQUIPMENT 0x2 > >>> +#define SENSE24_FMT_CONTROLLER 0x3 > >>> +#define SENSE24_FMT_MISC 0xF > >>> + > >>> +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 > >>> + > >>> +/* basic sense response buffer layout */ > >>> +typedef struct senseData { > >>> + uint8_t status[3]; > >>> + uint8_t res_count; > >>> + uint8_t phys_drive_id; > >>> + uint8_t low_cyl_addr; > >>> + uint8_t head_high_cyl_addr; > >>> + uint8_t fmt_msg; > >>> + uint64_t fmt_dependent_info[2]; > >>> + uint8_t reserved; > >>> + uint8_t program_action_code; > >>> + uint16_t config_info; > >>> + uint8_t mcode_hicyl; > >>> + uint8_t cyl_head_addr[3]; > >>> +} __attribute__ ((packed, aligned(4))) SenseData; > >> > >> And this looks _really_ dasd specific. > >> > > > > Yep, I glossed over those details while I was furiously tracking down the reset bug. I'll > > take a look at redesigning this. > > All of my information for creating these data structures came from an internal ECKD DASD > reference. There are probably some things that could stand a bit of cleanup or renaming. > Aside from that, considering this is in a DASD only (ECKD DASD only at the moment) code > path are you okay with my renaming the struct to senseDataECKD or something similar? Renaming this makes sense. > I'm not sure what value there is in abstracting sense at the moment. I'm not even sure > what other device's sense data looks like. Since my description of the SENSE CCW comes > from an ECKD reference I have not been able to verify any areas of the data that are > common across device types. Thoughts? There's SA22-7204-01 ("Common I/O Device Commands"), which is from 1992 (this is what I have on my disk -- is there anything newer?). It specifies what bits 0-5 of byte 0 mean and states that bytes 1-31 are optional and device-specific. Maybe some other bits have been specified after 1992, but I have not come across documentation for them.
On 1/9/19 1:34 PM, Cornelia Huck wrote: > On Wed, 9 Jan 2019 13:10:26 -0500 > "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > >> On 1/7/19 2:02 PM, Jason J. Herne wrote: >>>>> + >>>>> /* >>>>> * sense-id response buffer layout >>>>> */ >>>>> @@ -205,6 +265,61 @@ typedef struct senseid { >>>>> struct ciw ciw[62]; >>>>> } __attribute__ ((packed, aligned(4))) SenseId; >>>>> +/* architected values for first sense byte */ >>>>> +#define SNS0_CMD_REJECT 0x80 >>>>> +#define SNS0_INTERVENTION_REQ 0x40 >>>>> +#define SNS0_BUS_OUT_CHECK 0x20 >>>>> +#define SNS0_EQUIPMENT_CHECK 0x10 >>>>> +#define SNS0_DATA_CHECK 0x08 >>>>> +#define SNS0_OVERRUN 0x04 >>>>> +#define SNS0_INCOMPL_DOMAIN 0x01 >>>> >>>> IIRC, only byte 0 is device independent, and the others below are >>>> (ECKD) dasd specific? >>>> >>>>> + >>>>> +/* architectured values for second sense byte */ >>>>> +#define SNS1_PERM_ERR 0x80 >>>>> +#define SNS1_INV_TRACK_FORMAT 0x40 >>>>> +#define SNS1_EOC 0x20 >>>>> +#define SNS1_MESSAGE_TO_OPER 0x10 >>>>> +#define SNS1_NO_REC_FOUND 0x08 >>>>> +#define SNS1_FILE_PROTECTED 0x04 >>>>> +#define SNS1_WRITE_INHIBITED 0x02 >>>>> +#define SNS1_INPRECISE_END 0x01 >>>>> + >>>>> +/* architectured values for third sense byte */ >>>>> +#define SNS2_REQ_INH_WRITE 0x80 >>>>> +#define SNS2_CORRECTABLE 0x40 >>>>> +#define SNS2_FIRST_LOG_ERR 0x20 >>>>> +#define SNS2_ENV_DATA_PRESENT 0x10 >>>>> +#define SNS2_INPRECISE_END 0x04 >>>>> + >>>>> +/* 24-byte Sense fmt/msg codes */ >>>>> +#define SENSE24_FMT_PROG_SYS 0x0 >>>>> +#define SENSE24_FMT_EQUIPMENT 0x2 >>>>> +#define SENSE24_FMT_CONTROLLER 0x3 >>>>> +#define SENSE24_FMT_MISC 0xF >>>>> + >>>>> +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 >>>>> + >>>>> +/* basic sense response buffer layout */ >>>>> +typedef struct senseData { >>>>> + uint8_t status[3]; >>>>> + uint8_t res_count; >>>>> + uint8_t phys_drive_id; >>>>> + uint8_t low_cyl_addr; >>>>> + uint8_t head_high_cyl_addr; >>>>> + uint8_t fmt_msg; >>>>> + uint64_t fmt_dependent_info[2]; >>>>> + uint8_t reserved; >>>>> + uint8_t program_action_code; >>>>> + uint16_t config_info; >>>>> + uint8_t mcode_hicyl; >>>>> + uint8_t cyl_head_addr[3]; >>>>> +} __attribute__ ((packed, aligned(4))) SenseData; >>>> >>>> And this looks _really_ dasd specific. >>>> >>> >>> Yep, I glossed over those details while I was furiously tracking down the reset bug. I'll >>> take a look at redesigning this. >> >> All of my information for creating these data structures came from an internal ECKD DASD >> reference. There are probably some things that could stand a bit of cleanup or renaming. >> Aside from that, considering this is in a DASD only (ECKD DASD only at the moment) code >> path are you okay with my renaming the struct to senseDataECKD or something similar? > > Renaming this makes sense. > >> I'm not sure what value there is in abstracting sense at the moment. I'm not even sure >> what other device's sense data looks like. Since my description of the SENSE CCW comes >> from an ECKD reference I have not been able to verify any areas of the data that are >> common across device types. Thoughts? > > There's SA22-7204-01 ("Common I/O Device Commands"), which is from 1992 > (this is what I have on my disk -- is there anything newer?). It > specifies what bits 0-5 of byte 0 mean and states that bytes 1-31 are > optional and device-specific. > > Maybe some other bits have been specified after 1992, but I have not > come across documentation for them. That publication is no longer available. According to my quick research it has been replaced by an internal only publication. I'll see what I can find.
On Wed, 9 Jan 2019 15:01:19 -0500 "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > On 1/9/19 1:34 PM, Cornelia Huck wrote: > > On Wed, 9 Jan 2019 13:10:26 -0500 > > "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > >> I'm not sure what value there is in abstracting sense at the moment. I'm not even sure > >> what other device's sense data looks like. Since my description of the SENSE CCW comes > >> from an ECKD reference I have not been able to verify any areas of the data that are > >> common across device types. Thoughts? > > > > There's SA22-7204-01 ("Common I/O Device Commands"), which is from 1992 > > (this is what I have on my disk -- is there anything newer?). It > > specifies what bits 0-5 of byte 0 mean and states that bytes 1-31 are > > optional and device-specific. > > > > Maybe some other bits have been specified after 1992, but I have not > > come across documentation for them. > > That publication is no longer available. According to my quick research it has been > replaced by an internal only publication. I'll see what I can find. The publication no longer being available is not good, as it is a normative reference pointed to by the virtio standard (see http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-30001) :( Is there any chance that at least that old version can be made available again?
On 1/10/19 7:15 AM, Cornelia Huck wrote: > On Wed, 9 Jan 2019 15:01:19 -0500 > "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > >> On 1/9/19 1:34 PM, Cornelia Huck wrote: >>> On Wed, 9 Jan 2019 13:10:26 -0500 >>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > >>>> I'm not sure what value there is in abstracting sense at the moment. I'm not even sure >>>> what other device's sense data looks like. Since my description of the SENSE CCW comes >>>> from an ECKD reference I have not been able to verify any areas of the data that are >>>> common across device types. Thoughts? >>> >>> There's SA22-7204-01 ("Common I/O Device Commands"), which is from 1992 >>> (this is what I have on my disk -- is there anything newer?). It >>> specifies what bits 0-5 of byte 0 mean and states that bytes 1-31 are >>> optional and device-specific. >>> >>> Maybe some other bits have been specified after 1992, but I have not >>> come across documentation for them. >> >> That publication is no longer available. According to my quick research it has been >> replaced by an internal only publication. I'll see what I can find. > > The publication no longer being available is not good, as it is a > normative reference pointed to by the virtio standard (see > http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-30001) > :( > > Is there any chance that at least that old version can be made > available again? A quick web search turns up the following link: http://publibz.boulder.ibm.com/support/libraryserver/FRAMESET/DZ9AR501/CCONTENTS?D So I guess it is available, even if the document has been superseded internally.
On Thu, 10 Jan 2019 10:02:48 -0500 "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > On 1/10/19 7:15 AM, Cornelia Huck wrote: > > On Wed, 9 Jan 2019 15:01:19 -0500 > > "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > > > >> On 1/9/19 1:34 PM, Cornelia Huck wrote: > >>> On Wed, 9 Jan 2019 13:10:26 -0500 > >>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > > > >>>> I'm not sure what value there is in abstracting sense at the moment. I'm not even sure > >>>> what other device's sense data looks like. Since my description of the SENSE CCW comes > >>>> from an ECKD reference I have not been able to verify any areas of the data that are > >>>> common across device types. Thoughts? > >>> > >>> There's SA22-7204-01 ("Common I/O Device Commands"), which is from 1992 > >>> (this is what I have on my disk -- is there anything newer?). It > >>> specifies what bits 0-5 of byte 0 mean and states that bytes 1-31 are > >>> optional and device-specific. > >>> > >>> Maybe some other bits have been specified after 1992, but I have not > >>> come across documentation for them. > >> > >> That publication is no longer available. According to my quick research it has been > >> replaced by an internal only publication. I'll see what I can find. > > > > The publication no longer being available is not good, as it is a > > normative reference pointed to by the virtio standard (see > > http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-30001) > > :( > > > > Is there any chance that at least that old version can be made > > available again? > > A quick web search turns up the following link: > http://publibz.boulder.ibm.com/support/libraryserver/FRAMESET/DZ9AR501/CCONTENTS?D > > So I guess it is available, even if the document has been superseded internally. Cool, thanks for checking!
On 1/7/19 2:02 PM, Jason J. Herne wrote: >>> @@ -190,6 +247,9 @@ struct ciw { >>> __u16 count; >>> }; >>> +#define CU_TYPE_VIRTIO 0x3832 >>> +#define CU_TYPE_DASD 0x3990 >> >> No other dasd types we want to support? :) (Not sure if others are out >> in the wild. Maybe FBA?) >> > > I have no idea. I assumed 3390 was the only thing we supported. Perhaps 3380? I'd need to > find a test device, which I could probably do ... I'll look more into this. After a few discussions with folks in the lab we've decided that we don't see a ton of value in supporting anything other than 3990 at the moment. Anything else would be older (3380) and/or rare to see in the wild (and very difficult to test). As for emulated setups like z/VM, a user can just use 3390 instead of FBA. So I recommend we move forward with 3390/3990 support for now. We can always add in others types if/when we need them.
On Mon, 14 Jan 2019 13:44:12 -0500 "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > On 1/7/19 2:02 PM, Jason J. Herne wrote: > >>> @@ -190,6 +247,9 @@ struct ciw { > >>> __u16 count; > >>> }; > >>> +#define CU_TYPE_VIRTIO 0x3832 > >>> +#define CU_TYPE_DASD 0x3990 > >> > >> No other dasd types we want to support? :) (Not sure if others are out > >> in the wild. Maybe FBA?) > >> > > > > I have no idea. I assumed 3390 was the only thing we supported. Perhaps 3380? I'd need to > > find a test device, which I could probably do ... I'll look more into this. > > After a few discussions with folks in the lab we've decided that we don't see a ton of > value in supporting anything other than 3990 at the moment. Anything else would be older > (3380) and/or rare to see in the wild (and very difficult to test). As for emulated setups > like z/VM, a user can just use 3390 instead of FBA. So I recommend we move forward with > 3390/3990 support for now. We can always add in others types if/when we need them. Sounds reasonable. What about calling the #define above CU_TYPE_DASD_3990 instead? Just to make clear that there are other dasd types out there, but we only support that particular one (at least at the moment).
On 1/9/19 1:34 PM, Cornelia Huck wrote: > On Wed, 9 Jan 2019 13:10:26 -0500 > "Jason J. Herne" <jjherne@linux.ibm.com> wrote: > >> On 1/7/19 2:02 PM, Jason J. Herne wrote: >>>>> + >>>>> /* >>>>> * sense-id response buffer layout >>>>> */ >>>>> @@ -205,6 +265,61 @@ typedef struct senseid { >>>>> struct ciw ciw[62]; >>>>> } __attribute__ ((packed, aligned(4))) SenseId; >>>>> +/* architected values for first sense byte */ >>>>> +#define SNS0_CMD_REJECT 0x80 >>>>> +#define SNS0_INTERVENTION_REQ 0x40 >>>>> +#define SNS0_BUS_OUT_CHECK 0x20 >>>>> +#define SNS0_EQUIPMENT_CHECK 0x10 >>>>> +#define SNS0_DATA_CHECK 0x08 >>>>> +#define SNS0_OVERRUN 0x04 >>>>> +#define SNS0_INCOMPL_DOMAIN 0x01 >>>> >>>> IIRC, only byte 0 is device independent, and the others below are >>>> (ECKD) dasd specific? >>>> >>>>> + >>>>> +/* architectured values for second sense byte */ >>>>> +#define SNS1_PERM_ERR 0x80 >>>>> +#define SNS1_INV_TRACK_FORMAT 0x40 >>>>> +#define SNS1_EOC 0x20 >>>>> +#define SNS1_MESSAGE_TO_OPER 0x10 >>>>> +#define SNS1_NO_REC_FOUND 0x08 >>>>> +#define SNS1_FILE_PROTECTED 0x04 >>>>> +#define SNS1_WRITE_INHIBITED 0x02 >>>>> +#define SNS1_INPRECISE_END 0x01 >>>>> + >>>>> +/* architectured values for third sense byte */ >>>>> +#define SNS2_REQ_INH_WRITE 0x80 >>>>> +#define SNS2_CORRECTABLE 0x40 >>>>> +#define SNS2_FIRST_LOG_ERR 0x20 >>>>> +#define SNS2_ENV_DATA_PRESENT 0x10 >>>>> +#define SNS2_INPRECISE_END 0x04 >>>>> + >>>>> +/* 24-byte Sense fmt/msg codes */ >>>>> +#define SENSE24_FMT_PROG_SYS 0x0 >>>>> +#define SENSE24_FMT_EQUIPMENT 0x2 >>>>> +#define SENSE24_FMT_CONTROLLER 0x3 >>>>> +#define SENSE24_FMT_MISC 0xF >>>>> + >>>>> +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 >>>>> + >>>>> +/* basic sense response buffer layout */ >>>>> +typedef struct senseData { >>>>> + uint8_t status[3]; >>>>> + uint8_t res_count; >>>>> + uint8_t phys_drive_id; >>>>> + uint8_t low_cyl_addr; >>>>> + uint8_t head_high_cyl_addr; >>>>> + uint8_t fmt_msg; >>>>> + uint64_t fmt_dependent_info[2]; >>>>> + uint8_t reserved; >>>>> + uint8_t program_action_code; >>>>> + uint16_t config_info; >>>>> + uint8_t mcode_hicyl; >>>>> + uint8_t cyl_head_addr[3]; >>>>> +} __attribute__ ((packed, aligned(4))) SenseData; >>>> >>>> And this looks _really_ dasd specific. >>>> >>> >>> Yep, I glossed over those details while I was furiously tracking down the reset bug. I'll >>> take a look at redesigning this. >> >> All of my information for creating these data structures came from an internal ECKD DASD >> reference. There are probably some things that could stand a bit of cleanup or renaming. >> Aside from that, considering this is in a DASD only (ECKD DASD only at the moment) code >> path are you okay with my renaming the struct to senseDataECKD or something similar? > > Renaming this makes sense. > >> I'm not sure what value there is in abstracting sense at the moment. I'm not even sure >> what other device's sense data looks like. Since my description of the SENSE CCW comes >> from an ECKD reference I have not been able to verify any areas of the data that are >> common across device types. Thoughts? > > There's SA22-7204-01 ("Common I/O Device Commands"), which is from 1992 > (this is what I have on my disk -- is there anything newer?). It > specifies what bits 0-5 of byte 0 mean and states that bytes 1-31 are > optional and device-specific. > > Maybe some other bits have been specified after 1992, but I have not > come across documentation for them. > So everything I'm finding (including SA22-7204-01) only speaks of the first 6 bits being device agnostic. It is odd that the entire first byte would not have been reserved. Perhaps it is and I'm just not looking at the right documentation. My thought process is to have the basic_Sense function change to this: - void basic_sense(SubChannelId schid, SenseData *sd); + void basic_sense(SubChannelId schid, void *sense_data); The caller is free to map it however they see fit depending on the device type they are sensing. I'll rename struct senseData to struct senseDataEckdDasd, break out the first status byte and define common constants for the first 6 bits of that byte. This should clear up the confusion. I'll add a small comment to explain. Does this sound good?
diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c index 095f79b..9019250 100644 --- a/pc-bios/s390-ccw/cio.c +++ b/pc-bios/s390-ccw/cio.c @@ -10,6 +10,7 @@ #include "libc.h" #include "s390-ccw.h" +#include "s390-arch.h" #include "cio.h" static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE))); @@ -39,3 +40,110 @@ void enable_subchannel(SubChannelId schid) schib.pmcw.ena = 1; msch(schid, &schib); } + +uint16_t cu_type(SubChannelId schid) +{ + Ccw1 sense_id_ccw; + SenseId sense_data; + + sense_id_ccw.cmd_code = CCW_CMD_SENSE_ID; + sense_id_ccw.cda = ptr2u32(&sense_data); + sense_id_ccw.count = sizeof(sense_data); + + if (do_cio(schid, ptr2u32(&sense_id_ccw), CCW_FMT1)) { + panic("Failed to run SenseID CCw\n"); + } + + return sense_data.cu_type; +} + +void basic_sense(SubChannelId schid, SenseData *sd) +{ + Ccw1 senseCcw; + + senseCcw.cmd_code = CCW_CMD_BASIC_SENSE; + senseCcw.cda = ptr2u32(sd); + senseCcw.count = sizeof(*sd); + + if (do_cio(schid, ptr2u32(&senseCcw), CCW_FMT1)) { + panic("Failed to run Basic Sense CCW\n"); + } +} + +static bool irb_error(Irb *irb) +{ + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because + * real devices expect a 24 byte SenseID buffer, and virtio devices expect + * a much larger buffer. Neither device type can tolerate a buffer size + * different from what they expect so they set this indicator. + */ + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) { + return true; + } + return irb->scsw.dstat != 0xc; +} + +/* Executes a channel program at a given subchannel. The request to run the + * channel program is sent to the subchannel, we then wait for the interrupt + * singaling completion of the I/O operation(s) perfomed by the channel + * program. Lastly we verify that the i/o operation completed without error and + * that the interrupt we received was for the subchannel used to run the + * channel program. + * + * Note: This function assumes it is running in an environment where no other + * cpus are generating or receiving I/O interrupts. So either run it in a + * single-cpu environment or make sure all other cpus are not doing I/O and + * have I/O interrupts masked off. + */ +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt) +{ + CmdOrb orb = {}; + Irb irb = {}; + SenseData sd; + int rc, retries = 0; + + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format"); + + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */ + if (fmt == 0) { + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address"); + } + + orb.fmt = fmt ; + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */ + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */ + orb.lpm = 0xFF; /* All paths allowed */ + orb.cpa = ccw_addr; + + while (true) { + rc = ssch(schid, &orb); + if (rc) { + print_int("ssch failed with rc=", rc); + break; + } + + consume_io_int(); + + /* Clear read */ + rc = tsch(schid, &irb); + if (rc) { + print_int("tsch failed with rc=", rc); + break; + } + + if (!irb_error(&irb)) { + break; + } + + /* Unexpected unit check. Use sense to clear unit check then retry. */ + if (unit_check(&irb) && retries <= 2) { + basic_sense(schid, &sd); + retries++; + continue; + } + + break; + } + + return rc; +} diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h index 7b07d75..5c16635 100644 --- a/pc-bios/s390-ccw/cio.h +++ b/pc-bios/s390-ccw/cio.h @@ -70,9 +70,46 @@ struct scsw { __u16 count; } __attribute__ ((packed)); -#define SCSW_FCTL_CLEAR_FUNC 0x1000 -#define SCSW_FCTL_HALT_FUNC 0x2000 +/* Function Control */ #define SCSW_FCTL_START_FUNC 0x4000 +#define SCSW_FCTL_HALT_FUNC 0x2000 +#define SCSW_FCTL_CLEAR_FUNC 0x1000 + +/* Activity Control */ +#define SCSW_ACTL_RESUME_PEND 0x0800 +#define SCSW_ACTL_START_PEND 0x0400 +#define SCSW_ACTL_HALT_PEND 0x0200 +#define SCSW_ACTL_CLEAR_PEND 0x0100 +#define SCSW_ACTL_CH_ACTIVE 0x0080 +#define SCSW_ACTL_DEV_ACTIVE 0x0040 +#define SCSW_ACTL_SUSPENDED 0x0020 + +/* Status Control */ +#define SCSW_SCTL_ALERT 0x0010 +#define SCSW_SCTL_INTERMED 0x0008 +#define SCSW_SCTL_PRIMARY 0x0004 +#define SCSW_SCTL_SECONDARY 0x0002 +#define SCSW_SCTL_STATUS_PEND 0x0001 + +/* SCSW Device Status Flags */ +#define SCSW_DSTAT_ATTN 0x80 +#define SCSW_DSTAT_STATMOD 0x40 +#define SCSW_DSTAT_CUEND 0x20 +#define SCSW_DSTAT_BUSY 0x10 +#define SCSW_DSTAT_CHEND 0x08 +#define SCSW_DSTAT_DEVEND 0x04 +#define SCSW_DSTAT_UCHK 0x02 +#define SCSW_DSTAT_UEXCP 0x01 + +/* SCSW Subchannel Status Flags */ +#define SCSW_CSTAT_PCINT 0x80 +#define SCSW_CSTAT_BADLEN 0x40 +#define SCSW_CSTAT_PROGCHK 0x20 +#define SCSW_CSTAT_PROTCHK 0x10 +#define SCSW_CSTAT_CHDCHK 0x08 +#define SCSW_CSTAT_CHCCHK 0x04 +#define SCSW_CSTAT_ICCHK 0x02 +#define SCSW_CSTAT_CHAINCHK 0x01 /* * subchannel information block @@ -127,7 +164,23 @@ struct tpi_info { __u32 reserved4 : 12; } __attribute__ ((packed, aligned(4))); -/* channel command word (type 1) */ +/* channel command word (format 0) */ +typedef struct ccw0 { + __u8 cmd_code; + __u32 cda : 24; + __u32 chainData : 1; + __u32 chain : 1; + __u32 sli : 1; + __u32 skip : 1; + __u32 pci : 1; + __u32 ida : 1; + __u32 suspend : 1; + __u32 mida : 1; + __u8 reserved; + __u16 count; +} __attribute__ ((packed, aligned(8))) Ccw0; + +/* channel command word (format 1) */ typedef struct ccw1 { __u8 cmd_code; __u8 flags; @@ -135,6 +188,10 @@ typedef struct ccw1 { __u32 cda; } __attribute__ ((packed, aligned(8))) Ccw1; +/* do_cio() CCW formats */ +#define CCW_FMT0 0x00 +#define CCW_FMT1 0x01 + #define CCW_FLAG_DC 0x80 #define CCW_FLAG_CC 0x40 #define CCW_FLAG_SLI 0x20 @@ -190,6 +247,9 @@ struct ciw { __u16 count; }; +#define CU_TYPE_VIRTIO 0x3832 +#define CU_TYPE_DASD 0x3990 + /* * sense-id response buffer layout */ @@ -205,6 +265,61 @@ typedef struct senseid { struct ciw ciw[62]; } __attribute__ ((packed, aligned(4))) SenseId; +/* architected values for first sense byte */ +#define SNS0_CMD_REJECT 0x80 +#define SNS0_INTERVENTION_REQ 0x40 +#define SNS0_BUS_OUT_CHECK 0x20 +#define SNS0_EQUIPMENT_CHECK 0x10 +#define SNS0_DATA_CHECK 0x08 +#define SNS0_OVERRUN 0x04 +#define SNS0_INCOMPL_DOMAIN 0x01 + +/* architectured values for second sense byte */ +#define SNS1_PERM_ERR 0x80 +#define SNS1_INV_TRACK_FORMAT 0x40 +#define SNS1_EOC 0x20 +#define SNS1_MESSAGE_TO_OPER 0x10 +#define SNS1_NO_REC_FOUND 0x08 +#define SNS1_FILE_PROTECTED 0x04 +#define SNS1_WRITE_INHIBITED 0x02 +#define SNS1_INPRECISE_END 0x01 + +/* architectured values for third sense byte */ +#define SNS2_REQ_INH_WRITE 0x80 +#define SNS2_CORRECTABLE 0x40 +#define SNS2_FIRST_LOG_ERR 0x20 +#define SNS2_ENV_DATA_PRESENT 0x10 +#define SNS2_INPRECISE_END 0x04 + +/* 24-byte Sense fmt/msg codes */ +#define SENSE24_FMT_PROG_SYS 0x0 +#define SENSE24_FMT_EQUIPMENT 0x2 +#define SENSE24_FMT_CONTROLLER 0x3 +#define SENSE24_FMT_MISC 0xF + +#define SENSE24_FMT0_MSG_RESET_NOTIFICATION 0x16 + +/* basic sense response buffer layout */ +typedef struct senseData { + uint8_t status[3]; + uint8_t res_count; + uint8_t phys_drive_id; + uint8_t low_cyl_addr; + uint8_t head_high_cyl_addr; + uint8_t fmt_msg; + uint64_t fmt_dependent_info[2]; + uint8_t reserved; + uint8_t program_action_code; + uint16_t config_info; + uint8_t mcode_hicyl; + uint8_t cyl_head_addr[3]; +} __attribute__ ((packed, aligned(4))) SenseData; + +#define SENSE24_GET_FMT(sd) (sd->fmt_msg & 0xF0 >> 4) +#define SENSE24_GET_MSG(sd) (sd->fmt_msg & 0x0F) + +#define unit_check(irb) ((irb)->scsw.dstat & SCSW_DSTAT_UCHK) + /* interruption response block */ typedef struct irb { struct scsw scsw; @@ -215,6 +330,9 @@ typedef struct irb { int enable_mss_facility(void); void enable_subchannel(SubChannelId schid); +uint16_t cu_type(SubChannelId schid); +void basic_sense(SubChannelId schid, SenseData *sd); +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt); /* * Some S390 specific IO instructions as inline diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h index b39ee5d..11bce7d 100644 --- a/pc-bios/s390-ccw/s390-ccw.h +++ b/pc-bios/s390-ccw/s390-ccw.h @@ -52,6 +52,7 @@ typedef unsigned long long __u64; /* start.s */ void disabled_wait(void); void consume_sclp_int(void); +void consume_io_int(void); /* main.c */ void panic(const char *string); diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S index eb8d024..a48c38f 100644 --- a/pc-bios/s390-ccw/start.S +++ b/pc-bios/s390-ccw/start.S @@ -65,12 +65,32 @@ consume_sclp_int: /* prepare external call handler */ larl %r1, external_new_code stg %r1, 0x1b8 - larl %r1, external_new_mask + larl %r1, int_new_mask mvc 0x1b0(8),0(%r1) /* load enabled wait PSW */ larl %r1, enabled_wait_psw lpswe 0(%r1) +/* + * void consume_io_int(void) + * + * eats one I/O interrupt + */ + .globl consume_io_int +consume_io_int: + /* enable I/O interrupts in cr0 */ + stctg 6,6,0(15) + oi 4(15), 0xff + lctlg 6,6,0(15) + /* prepare external call handler */ + larl %r1, io_new_code + stg %r1, 0x1f8 + larl %r1, int_new_mask + mvc 0x1f0(8),0(%r1) + /* load enabled wait PSW */ + larl %r1, enabled_wait_psw + lpswe 0(%r1) + external_new_code: /* disable service interrupts in cr0 */ stctg 0,0,0(15) @@ -78,10 +98,19 @@ external_new_code: lctlg 0,0,0(15) br 14 +io_new_code: + /* disable I/O interrupts in cr6 */ + stctg 6,6,0(15) + ni 4(15), 0x00 + lctlg 6,6,0(15) + br 14 + + + .align 8 disabled_wait_psw: .quad 0x0002000180000000,0x0000000000000000 enabled_wait_psw: .quad 0x0302000180000000,0x0000000000000000 -external_new_mask: +int_new_mask: .quad 0x0000000180000000
Add struct for format-0 ccws. Support executing format-0 channel programs and waiting for their completion before continuing execution. This will be used for real dasd ipl. Add cu_type() to channel io library. This will be used to query control unit type which is used to determine if we are booting a virtio device or a real dasd device. Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com> --- pc-bios/s390-ccw/cio.c | 108 ++++++++++++++++++++++++++++++++++++++ pc-bios/s390-ccw/cio.h | 124 ++++++++++++++++++++++++++++++++++++++++++-- pc-bios/s390-ccw/s390-ccw.h | 1 + pc-bios/s390-ccw/start.S | 33 +++++++++++- 4 files changed, 261 insertions(+), 5 deletions(-)