Re: libata: Simulate REPORT LUNS for ATAPI devices when not supported

Re: libata: Simulate REPORT LUNS for ATAPI devices when not supported

[Jeff Garzik]

Darrick J. Wong wrote:
> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
> response to a REPORT LUNS packet.  If this happens to an ATAPI device
> that is attached to a SAS controller (this is the case with sas_ata),
> the device does not load because SCSI won't touch a "SCSI device"
> that won't report its LUNs.  If we see this command fail, we should
> simulate a response that indicates the presence of LUN 0.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>

I think the answer to this issue lies in the behavior of the majority of 
ATAPI devices when responding to REPORT LUNS.  Regardless of SAS or SATA 
or whatever bus the device is using.

ISTR that REPORT LUNS can make ATAPI devices croak, so it might be wise 
and more safe to simply simulate REPORT LUNS by default for all ATAPI 
devices.  Then readdress the issue if someone has a burning need to 
support the rare multi-LUN ATAPI devices.  I have one, but I'm not 
highly motivated to dig it out.

	Jeff

[PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Darrick J. Wong]

The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
response to a REPORT LUNS packet.  If this happens to an ATAPI device
that is attached to a SAS controller (this is the case with sas_ata),
the device does not load because SCSI won't touch a "SCSI device"
that won't report its LUNs.  Since most ATAPI devices don't support
multiple LUNs anyway, we might as well fake a response like we do for
ATA devices.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/ata/libata-scsi.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 47ea111..5ecf260 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2833,8 +2833,13 @@ static inline int __ata_scsi_queuecmd(st
 			rc = ata_scsi_translate(dev, cmd, done, xlat_func);
 		else
 			ata_scsi_simulate(dev, cmd, done);
-	} else
-		rc = ata_scsi_translate(dev, cmd, done, atapi_xlat);
+	} else {
+		/* Simulate REPORT LUNS for ATAPI devices */
+		if (cmd->cmnd[0] == REPORT_LUNS)
+			ata_scsi_simulate(dev, cmd, done);
+		else
+			rc = ata_scsi_translate(dev, cmd, done, atapi_xlat);
+	}
 
 	return rc;
 }

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Jeff Garzik]

Darrick J. Wong wrote:
> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
> response to a REPORT LUNS packet.  If this happens to an ATAPI device
> that is attached to a SAS controller (this is the case with sas_ata),
> the device does not load because SCSI won't touch a "SCSI device"
> that won't report its LUNs.  Since most ATAPI devices don't support
> multiple LUNs anyway, we might as well fake a response like we do for
> ATA devices.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>

Seems sane to me, but I would like additional comment/testing/etc. 
before applying...

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Douglas Gilbert]

Jeff Garzik wrote:
> Darrick J. Wong wrote:
>> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
>> response to a REPORT LUNS packet.  If this happens to an ATAPI device
>> that is attached to a SAS controller (this is the case with sas_ata),
>> the device does not load because SCSI won't touch a "SCSI device"
>> that won't report its LUNs.  Since most ATAPI devices don't support
>> multiple LUNs anyway, we might as well fake a response like we do for
>> ATA devices.
>>
>> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> 
> Seems sane to me, but I would like additional comment/testing/etc.
> before applying...

A SCSI target contains zero or more logical units. As
in this case, those logical units may use a different
transport. In such cases a SCSI target needs to emulate responses
to some SCSI commands (and modify the responses to others).
Here is a list that is probably not comprehensive:
  - INQUIRY  (peripheral qualifier in standard response)
  - INQUIRY, device identification VPD page (0x83)
       - obviously for the device name+identifier and port
         name+identifier
       - may need to concatenate those with the lu's
         name+identifier
  - INQUIRY, SCSI ports VPD page
  - INQUIRY, ATA Information VPD page (for SAT)
  - REPORT LUNS [mandatory in SPC-3 hence mandatory in SAT]
  - protocol specific port mode page (0x19)
  - protocol specific lu mode page (0x18) [could simulate]
  - PATA control mode page (0xa,0xf1) (for SAT)
  - protocol specific port _log_ page (0x18)

And for SAT you could add the ATA PASS-THROUGH
commands to that list. Those that are really ambitious
could implement well know logical units (wluns) which are
essentially a clean way to talk directly to the target
rather than a logical unit.


About the multi-lun ATAPI devices comment: how would libata
represent multiple S-ATAPI devices connected to a SATA
port multiplier?

Doug Gilbert

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[James Bottomley]

On Mon, 2006-12-04 at 15:32 -0800, Darrick J. Wong wrote:
> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
> response to a REPORT LUNS packet.  If this happens to an ATAPI device
> that is attached to a SAS controller (this is the case with sas_ata),
> the device does not load because SCSI won't touch a "SCSI device"
> that won't report its LUNs.  Since most ATAPI devices don't support
> multiple LUNs anyway, we might as well fake a response like we do for
> ATA devices.

Actually, there may be a standards conflict here.  SPC says that all
devices reporting compliance with this standard (as the inquiry data for
this device claims) shall support REPORT LUNS.  On the other hand, MMC
doesn't list REPORT LUNS in its table of mandatory commands.

I'm starting to think that even if they report a SCSI compliance level
of 3 or greater, we still shouldn't send REPORT LUNS to devices that
return MMC type unless we have a white list override.

James

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Douglas Gilbert]

James Bottomley wrote:
> On Mon, 2006-12-04 at 15:32 -0800, Darrick J. Wong wrote:
>> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
>> response to a REPORT LUNS packet.  If this happens to an ATAPI device
>> that is attached to a SAS controller (this is the case with sas_ata),
>> the device does not load because SCSI won't touch a "SCSI device"
>> that won't report its LUNs.  Since most ATAPI devices don't support
>> multiple LUNs anyway, we might as well fake a response like we do for
>> ATA devices.
> 
> Actually, there may be a standards conflict here.  SPC says that all
> devices reporting compliance with this standard (as the inquiry data for
> this device claims) shall support REPORT LUNS.  On the other hand, MMC
> doesn't list REPORT LUNS in its table of mandatory commands.

MMC-5 rev 4 section 7.1:
"Some commands that may be implemented by MM drives are
not described in this standard, but are found in other
SCSI standards. For a complete list of these commands
refer to [SPC-3]."

Hmm, "may be implemented" yet REPORT LUNS is mandatory
in SPC-3 (and SPC-3 is a normative reference for MMC-5).
I guess there is wriggle room there.
In practice, MMC diverges from SPC a lot more than other
SCSI device type command sets (e.g. SBC and SSC).

> I'm starting to think that even if they report a SCSI compliance level
> of 3 or greater, we still shouldn't send REPORT LUNS to devices that
> return MMC type unless we have a white list override.

There is also SAT compliance. For the ATA command set (i.e.
disks) sat-r09 lists REPORT LUNS and refers to SPC-3. For
ATAPI sat-r09 is far less clear. It does recommend, for
example, that the ATA Information VPD pages is implemented
in the SATL for ATAPI devices.

Doug Gilbert

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Jeff Garzik]

Darrick J. Wong wrote:
> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
> response to a REPORT LUNS packet.  If this happens to an ATAPI device
> that is attached to a SAS controller (this is the case with sas_ata),
> the device does not load because SCSI won't touch a "SCSI device"
> that won't report its LUNs.  Since most ATAPI devices don't support
> multiple LUNs anyway, we might as well fake a response like we do for
> ATA devices.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>

I'm leaning towards applying this, perhaps with a module option that 
allows experimenters to revert back to the older behavior.

Any chance you could be talked into tackling some of the SAT 
translation-related items Doug G mentioned?  I'm almost certain there 
are some info pages we should be returning, but are not, at the very least.

	Jeff

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[James Bottomley]

On Mon, 2006-12-11 at 11:24 -0500, Jeff Garzik wrote:
> Darrick J. Wong wrote:
> > The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
> > response to a REPORT LUNS packet.  If this happens to an ATAPI device
> > that is attached to a SAS controller (this is the case with sas_ata),
> > the device does not load because SCSI won't touch a "SCSI device"
> > that won't report its LUNs.  Since most ATAPI devices don't support
> > multiple LUNs anyway, we might as well fake a response like we do for
> > ATA devices.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> 
> I'm leaning towards applying this, perhaps with a module option that 
> allows experimenters to revert back to the older behavior.
> 
> Any chance you could be talked into tackling some of the SAT 
> translation-related items Doug G mentioned?  I'm almost certain there 
> are some info pages we should be returning, but are not, at the very least.

I thought we were closing in on agreeing that the SPC/MMC
inconsistencies made this the correct candidate fix.

James

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index ce63044..9d5e75b 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -7,6 +7,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 
+#include <scsi/scsi.h>
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_devinfo.h>
 
@@ -439,6 +440,11 @@ int scsi_get_device_flags(struct scsi_de
 				return devinfo->flags;
 		}
 	}
+	/* MMC devices can return SCSI-3 compliance and yet still not
+	 * support REPORT LUNS, so make them act as BLIST_NOREPORTLUN
+	 * unless BLIST_REPORTLUN2 is specifically set */
+	if (sdev->type == TYPE_ROM && (bflags & BLIST_REPORTLUN2) == 0)
+		bflags |= BLIST_NOREPORTLUN;
 	return bflags;
 }
 
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Jeff Garzik]

James Bottomley wrote:
> On Mon, 2006-12-11 at 11:24 -0500, Jeff Garzik wrote:
>> Darrick J. Wong wrote:
>>> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
>>> response to a REPORT LUNS packet.  If this happens to an ATAPI device
>>> that is attached to a SAS controller (this is the case with sas_ata),
>>> the device does not load because SCSI won't touch a "SCSI device"
>>> that won't report its LUNs.  Since most ATAPI devices don't support
>>> multiple LUNs anyway, we might as well fake a response like we do for
>>> ATA devices.
>>>
>>> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
>> I'm leaning towards applying this, perhaps with a module option that 
>> allows experimenters to revert back to the older behavior.
>>
>> Any chance you could be talked into tackling some of the SAT 
>> translation-related items Doug G mentioned?  I'm almost certain there 
>> are some info pages we should be returning, but are not, at the very least.
> 
> I thought we were closing in on agreeing that the SPC/MMC
> inconsistencies made this the correct candidate fix.

that works for me...

	Jeff

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Darrick J. Wong]

James Bottomley wrote:

> I thought we were closing in on agreeing that the SPC/MMC
> inconsistencies made this the correct candidate fix.

I tried out the patch below, but with it applied, SCSI still issues
REPORT LUNS to the device.  It seems that sdev->type = -1 and bflags = 0
when scsi_get_device_flags is called because the type code is not set up
until scsi_add_lun, which is called later.  In any case, the check
doesn't work for me because the SATAPI GoVault reports itself as a
Direct Access device, not a CD-ROM.

> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
<snip>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
<empty>

Was there supposed to be more to this patch?

--D

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[James Bottomley]

On Tue, 2006-12-12 at 14:24 -0800, Darrick J. Wong wrote:
> I tried out the patch below, but with it applied, SCSI still issues
> REPORT LUNS to the device.  It seems that sdev->type = -1 and bflags = 0

Yes, the inquiry scanning is being called too early ... largely so
BLIST_ROM can work, I suppose.

> when scsi_get_device_flags is called because the type code is not set up
> until scsi_add_lun, which is called later.  In any case, the check
> doesn't work for me because the SATAPI GoVault reports itself as a
> Direct Access device, not a CD-ROM.

Er, if it's really a CD-ROM, doesn't it need a blacklist entry with
BLIST_ROM then?  Regardless, MMC is the only standard that seems to be
inconsistent in this regard.  Anything claiming to conform to SBC will
need to be explicitly blacklisted if it claims SCSI-3 or higher and
doesn't support REPORT LUNS.

Does the attached patch fare better?

James

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 14e635a..92fb26b 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -752,8 +752,15 @@ static int scsi_add_lun(struct scsi_devi
 	case TYPE_RBC:
 		sdev->writeable = 1;
 		break;
-	case TYPE_WORM:
 	case TYPE_ROM:
+		/* MMC devices can return SCSI-3 compliance and yet
+		 * still not support REPORT LUNS, so make them act as
+		 * BLIST_NOREPORTLUN unless BLIST_REPORTLUN2 is
+		 * specifically set */
+		if ((*bflags & BLIST_REPORTLUN2) == 0)
+			*bflags |= BLIST_NOREPORTLUN;
+		/* fall through */
+	case TYPE_WORM:
 		sdev->writeable = 0;
 		break;
 	default:

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Darrick J. Wong]

James Bottomley wrote:

> Er, if it's really a CD-ROM, doesn't it need a blacklist entry with
> BLIST_ROM then?  Regardless, MMC is the only standard that seems to be
> inconsistent in this regard.  Anything claiming to conform to SBC will
> need to be explicitly blacklisted if it claims SCSI-3 or higher and
> doesn't support REPORT LUNS.

Sorry, I should have clarified this earlier--the Quantum GoVault is a
removable disk drive, not a CD-ROM.  The device is reminiscent of Zip
disks, but the cartridge is a sealed unit and contains a 2.5" SATA disk
inside; hence it's not a CD-ROM.  So the patch below won't work for me,
because sdev->type == TYPE_DISK.

I wonder if the same sort of REPORT LUNS screw-up might apply to other
ATAPI devices too, such as tape drives.  Since we've seen that
manufacturers of CD-ROMs and removable disks don't implement it, it
would not surprise me if the other ATAPI devices don't either.  But,
that's speculation on my part.

--D

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[James Bottomley]

On Wed, 2006-12-13 at 09:38 -0800, Darrick J. Wong wrote:
> Sorry, I should have clarified this earlier--the Quantum GoVault is a
> removable disk drive, not a CD-ROM.  The device is reminiscent of Zip
> disks, but the cartridge is a sealed unit and contains a 2.5" SATA disk
> inside; hence it's not a CD-ROM.  So the patch below won't work for me,
> because sdev->type == TYPE_DISK.

So it's a SATAPI device of type 0 ... that's fun ... but it still needs
a blacklist; it's governed by SBC which isn't ambiguous about the need
to support REPORT LUNS.  Now, if it supported RBC instead of SBC, that's
different; RBC also makes REPORT LUNS optional.  However, it is required
to indicate RBC support by being type 0xe.

> I wonder if the same sort of REPORT LUNS screw-up might apply to other
> ATAPI devices too, such as tape drives.  Since we've seen that
> manufacturers of CD-ROMs and removable disks don't implement it, it
> would not surprise me if the other ATAPI devices don't either.  But,
> that's speculation on my part.

James

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Patrick Mansfield]

On Mon, Dec 04, 2006 at 03:32:20PM -0800, Darrick J. Wong wrote:
> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
> response to a REPORT LUNS packet.  If this happens to an ATAPI device
> that is attached to a SAS controller (this is the case with sas_ata),
> the device does not load because SCSI won't touch a "SCSI device"
> that won't report its LUNs.  Since most ATAPI devices don't support
> multiple LUNs anyway, we might as well fake a response like we do for
> ATA devices.

If the REPORT LUNS fails, we should fall back to a sequential scan.

Is (or why isn't) the error propagated back to scsi?

> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 
>  drivers/ata/libata-scsi.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 47ea111..5ecf260 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2833,8 +2833,13 @@ static inline int __ata_scsi_queuecmd(st
>  			rc = ata_scsi_translate(dev, cmd, done, xlat_func);
>  		else
>  			ata_scsi_simulate(dev, cmd, done);
> -	} else
> -		rc = ata_scsi_translate(dev, cmd, done, atapi_xlat);
> +	} else {
> +		/* Simulate REPORT LUNS for ATAPI devices */
> +		if (cmd->cmnd[0] == REPORT_LUNS)
> +			ata_scsi_simulate(dev, cmd, done);
> +		else
> +			rc = ata_scsi_translate(dev, cmd, done, atapi_xlat);
> +	}
> 
>  	return rc;
>  }

-- Patrick Mansfield

Re: [PATCH v2] libata: Simulate REPORT LUNS for ATAPI devices

[Darrick J. Wong]

Patrick Mansfield wrote:
> On Mon, Dec 04, 2006 at 03:32:20PM -0800, Darrick J. Wong wrote:
>> The Quantum GoVault SATAPI removable disk device returns ATA_ERR in
>> response to a REPORT LUNS packet.  If this happens to an ATAPI device
>> that is attached to a SAS controller (this is the case with sas_ata),
>> the device does not load because SCSI won't touch a "SCSI device"
>> that won't report its LUNs.  Since most ATAPI devices don't support
>> multiple LUNs anyway, we might as well fake a response like we do for
>> ATA devices.
> 
> If the REPORT LUNS fails, we should fall back to a sequential scan.
> 
> Is (or why isn't) the error propagated back to scsi?

I believe the error is reported back to SCSI, which attempts to follow
up with TEST UNIT READY.  Unfortunately, for some reason the device then
gets dropped.  libata normally calls __scsi_add_device with lun=0, but
SAS calls scsi_scan_target with lun=SCAN_WILD_CARD, which is why SCSI
sends REPORT LUNs in the first place.

As an alternative I suppose we could detect ATA devices in sas_rphy_add
and change that SCAN_WILD_CARD to "0".

--D