Re: [qemu-s390x] [PATCH 10/15] s390-bios: Support for running format-0/1 channel programs

[Cornelia Huck]

On Wed, 9 Jan 2019 13:10:26 -0500
"Jason J. Herne" <address@hidden> 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.