> > And with cdutils.c just accepting the blank sense data as if no error
> > has happend, hald seems to detect either an "cd-rom media" or an
> > "audio cd media", when no media in present in an atapi p-ata device.
> >
> > I think this needs to be fixed in the "ata" driver.
> > But the code in hal'd uscsi() function doesn't look quite right,
> > either...
> That sounds reasonable to me. HAL misdetecting media presence can be
> confusing to users, so a bug should be filed. It will take me some time
> to get to reproducing and analyzing this (since I've switched to other
> projects, Tamarack bugfixing lowered in priority somewhat, and its
> holidays). If you find out more about the ata issue or get a bug ID,
> please also let us know.
I've filed the (p-)ata/atapi bug as 6642656:
"atapi auto request sense broken for big request sense buffers (>243 bytes)"
When the user passes a request sense data buffer of more than
20 bytes to USCSICMD, the kernel will internally request the
maximum amount of sense data, MAX_SENSE_LENGTH == 252 bytes.
This was changed with the put back for:
PSARC 2001/252 Recovering SCSI Sense Data
4046204 USCSICMD fails to pass back more than 20 bytes ARQ Sense DATA;
6603211 the uscsi_cdb should not restrict bp->b_back in the st driver
http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/scsi/impl/scsi_subr.c#2065
2065 /*
2066 * Here uscmd->uscsi_rqbuf currently points to the
caller's
2067 * buffer, but we replace this with a kernel buffer that
2068 * we allocate to use with the sense data. The sense
data
2069 * (if present) gets copied into this new buffer before
the
2070 * command is completed. Then we copy the sense data
from
2071 * our allocated buf into the caller's buffer below.
Note
2072 * that uscmd->uscsi_rqbuf and uscmd->uscsi_rqlen are
used
2073 * below to perform the copy back to the caller's buf.
2074 */
2075 if (uicmd->uic_rqlen <= SENSE_LENGTH) {
2076 uscmd->uscsi_rqlen = SENSE_LENGTH;
2077 uscmd->uscsi_rqbuf = kmem_zalloc(SENSE_LENGTH,
2078 KM_SLEEP);
2079 } else {
2080 uscmd->uscsi_rqlen = MAX_SENSE_LENGTH;
2081 uscmd->uscsi_rqbuf =
kmem_zalloc(MAX_SENSE_LENGTH, <<<<<<<<
2082 KM_SLEEP);
2083 }
In sd.c, sd_initpkt_for_uscsi() a 12 byte "struct scsi_arq_status"
header is prepended before the 252 byte request sense buffer,
so that a total of 12+252==264 bytes of "statuslen" is passed to
atapi_tran_init_pkt:
http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/scsi/targets/sd.c#12199
12199 /*
12200 * Allocate the scsi_pkt for the command.
12201 * Note: If PKT_DMA_PARTIAL flag is set, scsi_vhci binds a path
12202 * during scsi_init_pkt time and will continue to use the
12203 * same path as long as the same scsi_pkt is used without
12204 * intervening scsi_dma_free(). Since uscsi command does
12205 * not call scsi_dmafree() before retry failed command, it
12206 * is necessary to make sure PKT_DMA_PARTIAL flag is NOT
12207 * set such that scsi_vhci can use other available path
for
12208 * retry. Besides, ucsci command does not allow DMA
breakup,
12209 * so there is no need to set PKT_DMA_PARTIAL flag.
12210 */
12211 if (uscmd->uscsi_rqlen > SENSE_LENGTH) {
12212 pktp = scsi_init_pkt(SD_ADDRESS(un), NULL,
12213 ((bp->b_bcount != 0) ? bp : NULL),
uscmd->uscsi_cdblen,
12214 ((int)(uscmd->uscsi_rqlen) + sizeof (struct
scsi_arq_status) <<<<<<<<<<<
12215 - sizeof (struct scsi_extended_sense)), 0,
12216 (un->un_pkt_flags & ~PKT_DMA_PARTIAL) | PKT_XARQ,
12217 sdrunout, (caddr_t)un);
12218 } else {
12219 pktp = scsi_init_pkt(SD_ADDRESS(un), NULL,
12220 ((bp->b_bcount != 0) ? bp : NULL),
uscmd->uscsi_cdblen,
12221 sizeof (struct scsi_arq_status), 0,
12222 (un->un_pkt_flags & ~PKT_DMA_PARTIAL),
12223 sdrunout, (caddr_t)un);
12224 }
Finally, the bug is in usr/src/uts/intel/io/dktp/controller/ata/atapi.c,
which stores the statuslen of 264 to an 8-bit uchar_t, truncating it
from 264 to 8. But 8 bytes of status buffer is too small for scsi
auto request sense, so that ata/atapi doesn't perform ARQ any more.
Truncation happens here, both due to a cast and because the
target variable is defines as uchar_t:
http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/intel/io/dktp/controller/ata/atapi.c#678
678 ata_pktp->ap_statuslen = (uchar_t)statuslen;
> BTW while coding cdutils.c, I was peeking at cdrw code a lot - so cdrw's
> version of uscsi() might have the same defect.
Yep, cdrw might be affected as well. I see that transport.h
defines the request sense buffer length as RQBUFLEN == 32
(so that internally MAX_SENSE_LENGTH == 252 will be used).
And the uscsi() function in usr/src/cmd/cdrw/transport.c looks
quite similar to the one in hald / cdutils.c...
--
This message posted from opensolaris.org
