Re: [Qemu-devel] [PATCH v3 11/16] s390-bios: cio error handling

[Cornelia Huck <cohuck@redhat.com>]

On Fri,  1 Mar 2019 13:59:31 -0500
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:

> Add verbose error output for when unexpected i/o errors happen. This eases the
> burden of debugging and reporting i/o errors. No error information is printed
> in the success case, here is an example of what is output on error:
> 
> cio device error
>   ssid  : 0x0000000000000000
>   cssid : 0x0000000000000000
>   sch_no: 0x0000000000000000
> 
> Interrupt Response Block Data:
>     Function Ctrl : [Start]
>     Activity Ctrl : [Start-Pending]
>     Status Ctrl : [Alert] [Primary] [Secondary] [Status-Pending]
>     Device Status : [Unit-Check]
>     Channel Status :
>     cpa=: 0x000000007f8d6038
>     prev_ccw=: 0x0000000000000000
>     this_ccw=: 0x0000000000000000
> Eckd Dasd Sense Data (fmt 32-bytes):
>     Sense Condition Flags :
>     Residual Count     =: 0x0000000000000000
>     Phys Drive ID      =: 0x000000000000009e
>     low cyl address    =: 0x0000000000000000
>     head addr & hi cyl =: 0x0000000000000000
>     format/message     =: 0x0000000000000008
>     fmt-dependent[0-7] =: 0x0000000000000004
>     fmt-dependent[8-15]=: 0xe561282305082fff
>     prog action code   =: 0x0000000000000016
>     Configuration info =: 0x00000000000040e0
>     mcode / hi-cyl     =: 0x0000000000000000
>     cyl & head addr [0]=: 0x0000000000000000
>     cyl & head addr [1]=: 0x0000000000000000
>     cyl & head addr [2]=: 0x0000000000000000
> 
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/cio.c  | 230 ++++++++++++++++++++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/libc.h |  11 +++
>  2 files changed, 241 insertions(+)
> 
> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
> index e61cfd3..c528bbf 100644
> --- a/pc-bios/s390-ccw/cio.c
> +++ b/pc-bios/s390-ccw/cio.c
> @@ -82,6 +82,228 @@ static bool irb_error(Irb *irb)
>      return irb->scsw.dstat != (SCSW_DSTAT_DEVEND | SCSW_DSTAT_CHEND);
>  }
>  
> +static void print_eckd_dasd_sense_data(SenseDataEckdDasd *sd)
> +{
> +    char msgline[512];
> +
> +    if (sd->config_info & 0x8000) {
> +        sclp_print("Eckd Dasd Sense Data (fmt 24-bytes):\n");
> +    } else {
> +        sclp_print("Eckd Dasd Sense Data (fmt 32-bytes):\n");
> +    }
> +
> +    strcat(msgline, "    Sense Condition Flags :");
> +    if (sd->status[0] & SNS_STAT0_CMD_REJECT) {
> +        strcat(msgline, " [Cmd-Reject]");
> +    }
> +    if (sd->status[0] & SNS_STAT0_INTERVENTION_REQ) {
> +        strcat(msgline, " [Intervention-Required]");
> +    }
> +    if (sd->status[0] & SNS_STAT0_BUS_OUT_CHECK) {
> +        strcat(msgline, " [Bus-Out-Parity-Check]");
> +    }
> +    if (sd->status[0] & SNS_STAT0_EQUIPMENT_CHECK) {
> +        strcat(msgline, " [Equipment-Check]");
> +    }
> +    if (sd->status[0] & SNS_STAT0_DATA_CHECK) {
> +        strcat(msgline, " [Data-Check]");

I'm wondering whether it would make sense to factor the common bits
out. Might be overkill, though.

> +    }
> +    if (sd->status[0] & SNS_STAT0_OVERRUN) {
> +        strcat(msgline, " [Overrun]");
> +    }
> +    if (sd->status[0] & SNS_STAT0_INCOMPL_DOMAIN) {
> +        strcat(msgline, " [Incomplete-Domain]");
> +    }
(...)
> +        sclp_print("cio device error\n");
> +        print_int("  ssid  ", schid.ssid);
> +        print_int("  cssid ", schid.cssid);
> +        print_int("  sch_no", schid.sch_no);
> +        sclp_print("\n");
> +        print_irb_err(&irb);

Maybe do basic_sense + print sense data only if there's actually a unit
check?

(Also, I'm not sure if you can even do a basic_sense in case e.g. of
unexpected busy.)

> +        basic_sense(schid, &sd, sizeof(sd));
> +        print_eckd_dasd_sense_data(&sd);
>          rc = -1;
>          break;
>      }