[PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Mark Lord]

In an ideal world, we would use the existing ATA_12 opcode
to issue 12-byte ATA passthrough commands for libata ATAPI drives
from userspace.

But ATA_12 happens to have the same SCSI opcode value as the older
CD/RW "BLANK" command, widely used by cdrecord and friends.

So, to achieve ATA passthru capability for libata ATAPI,
we have to instead use the ATA_16 opcode: a 16-byte command.

SCSI normally disallows issuing 16-byte commands to 12-byte devices,
so special support has to be added for this.

Introduce an "allow_ata_16" boolean to the scsi_host struct.
This provides a means for libata to signal that 16-byte ATA_16
commands should be permitted even for 12-byte ATAPI devices.

Signed-off-by:  Mark Lord <mlord@pobox.com>

--- old/include/scsi/scsi_host.h	2007-01-02 19:06:45.000000000 -0500
+++ new/include/scsi/scsi_host.h	2007-01-02 19:09:06.000000000 -0500
@@ -608,6 +608,13 @@
 	unsigned async_scan:1;
 
 	/*
+	 * True for libata, to allow issuing ATA_16 16-byte CDBs
+	 * to (otherwise) 12-byte ATAPI drives.  ATAPI cannot use
+	 * the ATA_12 opcode, because it means "BLANK" to CDRW drives.
+	 */
+	unsigned allow_ata_16:1;
+
+	/*
 	 * Optional work queue to be utilized by the transport
 	 */
 	char work_q_name[KOBJ_NAME_LEN];
--- old/drivers/scsi/scsi.c	2007-01-02 19:06:45.000000000 -0500
+++ new/drivers/scsi/scsi.c	2007-01-02 19:09:06.000000000 -0500
@@ -567,12 +567,17 @@
 	 * length exceeds what the host adapter can handle.
 	 */
 	if (CDB_SIZE(cmd) > cmd->device->host->max_cmd_len) {
-		SCSI_LOG_MLQUEUE(3,
+		/* permit ATA_16 passthru to 12-byte ATAPI devices */
+		if (cmd->cmnd[0]  != ATA_16
+		 || CDB_SIZE(cmd) != 16
+		 || !cmd->device->host->allow_ata_16) {
+			SCSI_LOG_MLQUEUE(3,
 				printk("queuecommand : command too long.\n"));
-		cmd->result = (DID_ABORT << 16);
+			cmd->result = (DID_ABORT << 16);
 
-		scsi_done(cmd);
-		goto out;
+			scsi_done(cmd);
+			goto out;
+		}
 	}
 
 	spin_lock_irqsave(host->host_lock, flags);
--- old/drivers/ata/libata-core.c	2007-01-02 19:06:44.000000000 -0500
+++ new/drivers/ata/libata-core.c	2007-01-02 19:09:06.000000000 -0500
@@ -5686,6 +5686,7 @@
 	shost->max_lun = 1;
 	shost->max_channel = 1;
 	shost->max_cmd_len = 12;
+	shost->allow_ata_16 = 1;
 }
 
 /**

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[James Bottomley]

On Tue, 2007-01-02 at 19:35 -0500, Mark Lord wrote:
> In an ideal world, we would use the existing ATA_12 opcode
> to issue 12-byte ATA passthrough commands for libata ATAPI drives
> from userspace.
> 
> But ATA_12 happens to have the same SCSI opcode value as the older
> CD/RW "BLANK" command, widely used by cdrecord and friends.

It's not older; it's still current (at least as of MMC-5)

> So, to achieve ATA passthru capability for libata ATAPI,
> we have to instead use the ATA_16 opcode: a 16-byte command.
> 
> SCSI normally disallows issuing 16-byte commands to 12-byte devices,
> so special support has to be added for this.
> 
> Introduce an "allow_ata_16" boolean to the scsi_host struct.
> This provides a means for libata to signal that 16-byte ATA_16
> commands should be permitted even for 12-byte ATAPI devices.

I don't think I quite understand what you're trying to do here.  My
understanding is that ATA_12 and ATA_16 are part of the SAT layer. i.e.
they're used when we're speaking SCSI to an underlying ATA device to
send taskfiles.  However, ATAPI devices don't use SAT ... every SCSI
command you send to an ATAPI device goes out as an ATA PACKET command
without being translated.  You can see this in
libata-scsi.c:__ata_scsi_queuecommand() where commands for ATAPI devices
go globally through atapi_xlat().  So at the moment libata would send
ATA_12 or ATA_16 over to an ATAPI device also as an ATA PACKET command.

Is there a missing piece to this patch, where you scan the incoming
commands to ATAPI devices and actually do translation for ATA_16?

James

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Mark Lord]

James Bottomley wrote:
> ..
> I don't think I quite understand what you're trying to do here.  My
> understanding is that ATA_12 and ATA_16 are part of the SAT layer. i.e.
> they're used when we're speaking SCSI to an underlying ATA device to
> send taskfiles.  However, ATAPI devices don't use SAT ... every SCSI
> command you send to an ATAPI device goes out as an ATA PACKET command
> without being translated.

ATAPI devices also implement quite a few ATA (non-packet) commands.
This patch gives us a way to issue them in response to SG_IO ATA passthru
and the existing (non working) libata HDIO_DRIVE_CMD and HDIO_DRIVE_TASK
ioctl calls.

Without this patch, SCSI blocks the ATA_16 passthru attempts,
because it thinks the CDB is too large for the 12-byte packet protocol
that ATAPI devices use.  This limit (12 bytes) is totally non-applicable
to ATA protocol commands.

> Is there a missing piece to this patch, where you scan the incoming
> commands to ATAPI devices and actually do translation for ATA_16?

Look at the existing libata-core.c ioctl's for HDIO_DRIVE_CMD
and HDIO_DRIVE_TASK, used by hdparm and others.

Cheers

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[James Bottomley]

On Wed, 2007-01-03 at 00:42 -0500, Mark Lord wrote:
> James Bottomley wrote:
> > I don't think I quite understand what you're trying to do here.  My
> > understanding is that ATA_12 and ATA_16 are part of the SAT layer. i.e.
> > they're used when we're speaking SCSI to an underlying ATA device to
> > send taskfiles.  However, ATAPI devices don't use SAT ... every SCSI
> > command you send to an ATAPI device goes out as an ATA PACKET command
> > without being translated.
> 
> ATAPI devices also implement quite a few ATA (non-packet) commands.
> This patch gives us a way to issue them in response to SG_IO ATA passthru
> and the existing (non working) libata HDIO_DRIVE_CMD and HDIO_DRIVE_TASK
> ioctl calls.

I know that ... but when you send ATA_16 down to an ATAPI device via
SG_IO  the code paths in libata don't unwrap it and send it out as a
taskfile ... they put ATA_16 out as a packet command.

> Without this patch, SCSI blocks the ATA_16 passthru attempts,
> because it thinks the CDB is too large for the 12-byte packet protocol
> that ATAPI devices use.  This limit (12 bytes) is totally non-applicable
> to ATA protocol commands.
> 
> > Is there a missing piece to this patch, where you scan the incoming
> > commands to ATAPI devices and actually do translation for ATA_16?
> 
> Look at the existing libata-core.c ioctl's for HDIO_DRIVE_CMD
> and HDIO_DRIVE_TASK, used by hdparm and others.

Those send out taskfile commands ... my point is that the SAT SCSI
commands only come down the SCSI paths, so without additional processing
in the ATAPI path they're just packaged up as PACKET commands and never
unwrapped and sent out as taskfiles.   If you think about it, cdrecord
sending BLANK via SG_IO to an ATAPI device would never work if we
interpreted ATA_12 in the ATAPI path...

James

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jeff Garzik]

James Bottomley wrote:
> On Wed, 2007-01-03 at 00:42 -0500, Mark Lord wrote:
>> James Bottomley wrote:
>>> I don't think I quite understand what you're trying to do here.  My
>>> understanding is that ATA_12 and ATA_16 are part of the SAT layer. i.e.
>>> they're used when we're speaking SCSI to an underlying ATA device to
>>> send taskfiles.  However, ATAPI devices don't use SAT ... every SCSI
>>> command you send to an ATAPI device goes out as an ATA PACKET command
>>> without being translated.
>> ATAPI devices also implement quite a few ATA (non-packet) commands.
>> This patch gives us a way to issue them in response to SG_IO ATA passthru
>> and the existing (non working) libata HDIO_DRIVE_CMD and HDIO_DRIVE_TASK
>> ioctl calls.
> 
> I know that ... but when you send ATA_16 down to an ATAPI device via
> SG_IO  the code paths in libata don't unwrap it and send it out as a
> taskfile ... they put ATA_16 out as a packet command.

He is aware of that.  That's what his two patches address.


>> Without this patch, SCSI blocks the ATA_16 passthru attempts,
>> because it thinks the CDB is too large for the 12-byte packet protocol
>> that ATAPI devices use.  This limit (12 bytes) is totally non-applicable
>> to ATA protocol commands.
>>
>>> Is there a missing piece to this patch, where you scan the incoming
>>> commands to ATAPI devices and actually do translation for ATA_16?
>> Look at the existing libata-core.c ioctl's for HDIO_DRIVE_CMD
>> and HDIO_DRIVE_TASK, used by hdparm and others.
> 
> Those send out taskfile commands ... my point is that the SAT SCSI

No, those HDIO_xxx ioctls cause libata to send ATA_{12,16} SCSI commands 
to the SCSI simulator (which then unwraps them, and sends a taskfile).

	Jeff

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[James Bottomley]

On Wed, 2007-01-03 at 10:45 -0500, Jeff Garzik wrote:
> James Bottomley wrote:
> > On Wed, 2007-01-03 at 00:42 -0500, Mark Lord wrote:
> >> James Bottomley wrote:
> >>> I don't think I quite understand what you're trying to do here.  My
> >>> understanding is that ATA_12 and ATA_16 are part of the SAT layer. i.e.
> >>> they're used when we're speaking SCSI to an underlying ATA device to
> >>> send taskfiles.  However, ATAPI devices don't use SAT ... every SCSI
> >>> command you send to an ATAPI device goes out as an ATA PACKET command
> >>> without being translated.
> >> ATAPI devices also implement quite a few ATA (non-packet) commands.
> >> This patch gives us a way to issue them in response to SG_IO ATA passthru
> >> and the existing (non working) libata HDIO_DRIVE_CMD and HDIO_DRIVE_TASK
> >> ioctl calls.
> > 
> > I know that ... but when you send ATA_16 down to an ATAPI device via
> > SG_IO  the code paths in libata don't unwrap it and send it out as a
> > taskfile ... they put ATA_16 out as a packet command.
> 
> He is aware of that.  That's what his two patches address.

Er I've only seen one patch ... I did ask if there was another to add
the ATA_16 interpretation ... did that get lost by the SCSI reflector?

James

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Mark Lord]

James Bottomley wrote:
>
> Er I've only seen one patch ... I did ask if there was another to add
> the ATA_16 interpretation ... did that get lost by the SCSI reflector?

Ahh.. my apologies on that, James.

The two patches apply and build independently,
and the second patch was only for libata, so it was
sent only to the linux-ide list.

There's actually a third patch as well, to fix a one-line
bug in sr.c that prevents *any* SG_IO from ever working
on ATAPI as well.

I'll email you the full set.

Cheers!

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Douglas Gilbert]

James Bottomley wrote:
> On Wed, 2007-01-03 at 00:42 -0500, Mark Lord wrote:
>> James Bottomley wrote:
>>> I don't think I quite understand what you're trying to do here.  My
>>> understanding is that ATA_12 and ATA_16 are part of the SAT layer. i.e.
>>> they're used when we're speaking SCSI to an underlying ATA device to
>>> send taskfiles.  However, ATAPI devices don't use SAT ... every SCSI
>>> command you send to an ATAPI device goes out as an ATA PACKET command
>>> without being translated.
>> ATAPI devices also implement quite a few ATA (non-packet) commands.
>> This patch gives us a way to issue them in response to SG_IO ATA passthru
>> and the existing (non working) libata HDIO_DRIVE_CMD and HDIO_DRIVE_TASK
>> ioctl calls.
> 
> I know that ... but when you send ATA_16 down to an ATAPI device via
> SG_IO  the code paths in libata don't unwrap it and send it out as a
> taskfile ... they put ATA_16 out as a packet command.
> 
>> Without this patch, SCSI blocks the ATA_16 passthru attempts,
>> because it thinks the CDB is too large for the 12-byte packet protocol
>> that ATAPI devices use.  This limit (12 bytes) is totally non-applicable
>> to ATA protocol commands.
>>
>>> Is there a missing piece to this patch, where you scan the incoming
>>> commands to ATAPI devices and actually do translation for ATA_16?
>> Look at the existing libata-core.c ioctl's for HDIO_DRIVE_CMD
>> and HDIO_DRIVE_TASK, used by hdparm and others.
> 
> Those send out taskfile commands ... my point is that the SAT SCSI
> commands only come down the SCSI paths, so without additional processing
> in the ATAPI path they're just packaged up as PACKET commands and never
> unwrapped and sent out as taskfiles.   If you think about it, cdrecord
> sending BLANK via SG_IO to an ATAPI device would never work if we
> interpreted ATA_12 in the ATAPI path...

James,
This doesn't only affect libata. The MPT Fusion SAS HBAs
have a SATL in firmware. It also make sense to put SATLs
close to the end device; for example: in a USB mass storage
bridge **, dito sbp2, and in a SAS enclosure (in which case
really useful SCSI commands like PERSISTENT RESERVE could
be implemented in front of SATA disks).

IMO we need to have a concept of near and far transport.
The near transport is what the LLD or local bridge (e.g.
libata is a virtual target) sees. The far transport is
what the lu sees (e.g. for cd/dvd drives that is usually
(S)ATAPI). So if the near transport does not limit cdb
length then the command should be sent to the target. If
the target can't either:
  - send the command unaltered to the lu, or
  - bridge the command to the far transport, or
  - interpret the command itself (e.g. REPORT LUNS + ATA_16)
then it can report an error.


I seem to be fighting a losing battle against the mindset
of trying to process errors at the highest possible level
and the even worse Unix block layer habit of trying to
foresee errors a priori.
Taking this example: should the block layer reject pass-
through SCSI commands due to command length? Of
course not, it shouldn't even know about SCSI command sets.
Should the cdrom driver block 16 byte SCSI commands?
No, because MMC imposes not limit in the cdb length.
It is the _transport_ that should block the command,
if it so chooses. In this case the transport is a
virtual one between sr/sg and libata and libata should
recognize SCSI ATA PASS-THROUGH (12+16) cdbs as
transport related and not (directly) to be sent to
the logical unit. And it is at this level we should
add the wrinkle that if the pdt=5 (cd or dvd) then
don't translate ATA_12. [I have already been around this
loop with Luben and his SATL: sorted with the minimum
of fuss.]


We also have the concept of the block layer as a common
security filter and some folks want to merge (i.e.
read _eliminate_ the sg driver's filter) the block layer
and the sg filter. Well sg's filter is SPC based with
some SBC knowledge and the block layer's filter is
heavily MMC based ("designed" to trick/facilitate cdrecord).
How to treat the ATA_16 command? Well at the moment sg will
require read-write permissions whereas I think the block
layer will require root (or CAP_IO_RAW) permissions. As a
point of reference, Windows documentation only identifies
one class of commands that it will block in its pass through
(SPT) and those are of the EXTENDED COPY type. That one is
way too powerful for OS folks to allow the users to get
their hands on.


** I have been told that USB disk enclosures being designed
now will include at a SATL. This will mean smartmontools
(version 5.37 and later) and hdparm (assuming that Mark is
looking at adding SATL pass through support) will just work
seamlessly. SAT does not include any sub-addressing support
so it doesn't help in trying to obtain diagnostic access
to physical (SATA) disks behind a logical SCSI (RAID) disk
device.

Doug Gilbert

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[James Bottomley]

On Wed, 2007-01-03 at 14:39 -0500, Douglas Gilbert wrote:
> James Bottomley wrote:
> > Those send out taskfile commands ... my point is that the SAT SCSI
> > commands only come down the SCSI paths, so without additional processing
> > in the ATAPI path they're just packaged up as PACKET commands and never
> > unwrapped and sent out as taskfiles.   If you think about it, cdrecord
> > sending BLANK via SG_IO to an ATAPI device would never work if we
> > interpreted ATA_12 in the ATAPI path...
> 
> James,
> This doesn't only affect libata. The MPT Fusion SAS HBAs
> have a SATL in firmware. It also make sense to put SATLs
> close to the end device; for example: in a USB mass storage
> bridge **, dito sbp2, and in a SAS enclosure (in which case
> really useful SCSI commands like PERSISTENT RESERVE could
> be implemented in front of SATA disks).

Actually, I think we're making somewhat gentle movements in the opposite
direction: towards not having a translation layer at all.  There have
just been recent changes in the block layer that will facilitate our
labelling the request with a cmd_type field.  We can use this to signal
the type of command payload in the request and hopefully avoid having to
encapsulate it in SCSI.

> IMO we need to have a concept of near and far transport.
> The near transport is what the LLD or local bridge (e.g.
> libata is a virtual target) sees. The far transport is
> what the lu sees (e.g. for cd/dvd drives that is usually
> (S)ATAPI). So if the near transport does not limit cdb
> length then the command should be sent to the target. If
> the target can't either:
>   - send the command unaltered to the lu, or
>   - bridge the command to the far transport, or
>   - interpret the command itself (e.g. REPORT LUNS + ATA_16)
> then it can report an error.

Actually, for this particular issue, I'm going to suggest a variant of
this, which is to allow the transport class to vet the command (and
definitively allow it or deny it without having to go through the length
and other checks).   The max_cmd_length is almost a historical remnant
from the few intelligent SPI controllers that had an in-built maximum
CDB length (I suspect qla1280 is the best current example since it's
capped at 12).

> I seem to be fighting a losing battle against the mindset
> of trying to process errors at the highest possible level
> and the even worse Unix block layer habit of trying to
> foresee errors a priori.
> Taking this example: should the block layer reject pass-
> through SCSI commands due to command length? Of
> course not, it shouldn't even know about SCSI command sets.
> Should the cdrom driver block 16 byte SCSI commands?
> No, because MMC imposes not limit in the cdb length.
> It is the _transport_ that should block the command,
> if it so chooses. In this case the transport is a
> virtual one between sr/sg and libata and libata should
> recognize SCSI ATA PASS-THROUGH (12+16) cdbs as
> transport related and not (directly) to be sent to
> the logical unit. And it is at this level we should
> add the wrinkle that if the pdt=5 (cd or dvd) then
> don't translate ATA_12. [I have already been around this
> loop with Luben and his SATL: sorted with the minimum
> of fuss.]

As I read the code Mark sent me, current libata will only accept ATA_16
for ATAPI devices (whether type 5 or not) ... pehaps this needs to be
altered?

> We also have the concept of the block layer as a common
> security filter and some folks want to merge (i.e.
> read _eliminate_ the sg driver's filter) the block layer
> and the sg filter. Well sg's filter is SPC based with
> some SBC knowledge and the block layer's filter is
> heavily MMC based ("designed" to trick/facilitate cdrecord).
> How to treat the ATA_16 command? Well at the moment sg will
> require read-write permissions whereas I think the block
> layer will require root (or CAP_IO_RAW) permissions. As a
> point of reference, Windows documentation only identifies
> one class of commands that it will block in its pass through
> (SPT) and those are of the EXTENDED COPY type. That one is
> way too powerful for OS folks to allow the users to get
> their hands on.

Er, well, as you know, I've never been a fan of this static list.  I
thought Jens was going to put us all out of our misery by making the
list settable per device by root and thus shovel the problem off onto
the distros?

> ** I have been told that USB disk enclosures being designed
> now will include at a SATL. This will mean smartmontools
> (version 5.37 and later) and hdparm (assuming that Mark is
> looking at adding SATL pass through support) will just work
> seamlessly. SAT does not include any sub-addressing support
> so it doesn't help in trying to obtain diagnostic access
> to physical (SATA) disks behind a logical SCSI (RAID) disk
> device.

That doesn't surprise me.  One could argue that a lot of USB disk
devices already include a SATL since they're usually cheap ATA devices
under the covers ...

James

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Mark Lord]

James Bottomley wrote:
>
> As I read the code Mark sent me, current libata will only accept ATA_16
> for ATAPI devices (whether type 5 or not) ... pehaps this needs to be
> altered?

It could be.

Both ATA_12 and ATA_16 are just ways of getting a taskfile sent
down to the LLD, and ATA_12 is merely a restricted subset of ATA_16
so it is not really necessary here.

I think that given the opcode conflict with "BLANK", it may be best for
the moment to just not bother with ATA_12 for libata ATAPI.  We don't really
have a need for ATA_12 anyway, as ATA_16 is more flexible.

And any application code that uses these isn't going to want to have
to vary its commands between ATA_12 and ATA_16 depending upon the target.
I think they'll all just go straight for ATA_16 for uniformity and
to minimize testing drudgery.

hdparm will certainly use only ATA_16 once it becomes available in SCSI/libata.

Cheers

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jens Axboe]

On Wed, Jan 03 2007, James Bottomley wrote:
> Er, well, as you know, I've never been a fan of this static list.  I
> thought Jens was going to put us all out of our misery by making the
> list settable per device by root and thus shovel the problem off onto
> the distros?

The code is there, just haven't had the guts to merge it yet.

http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=bf5f922d167a5c5cf57132bbcaa1e0ddfd5c45f7

-- 
Jens Axboe

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Mark Lord]

Jens Axboe wrote:
> On Wed, Jan 03 2007, James Bottomley wrote:
>> Er, well, as you know, I've never been a fan of this static list.  I
>> thought Jens was going to put us all out of our misery by making the
>> list settable per device by root and thus shovel the problem off onto
>> the distros?
> 
> The code is there, just haven't had the guts to merge it yet.
> 
> http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=bf5f922d167a5c5cf57132bbcaa1e0ddfd5c45f7
> 

That's nice, but doesn't help with the case of trying to do ATA passthru
to ATAPI devices, the subject of the original patch here.

Cheers!

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jens Axboe]

On Thu, Jan 04 2007, Mark Lord wrote:
> Jens Axboe wrote:
> >On Wed, Jan 03 2007, James Bottomley wrote:
> >>Er, well, as you know, I've never been a fan of this static list.  I
> >>thought Jens was going to put us all out of our misery by making the
> >>list settable per device by root and thus shovel the problem off onto
> >>the distros?
> >
> >The code is there, just haven't had the guts to merge it yet.
> >
> >http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=bf5f922d167a5c5cf57132bbcaa1e0ddfd5c45f7
> >
> 
> That's nice, but doesn't help with the case of trying to do ATA passthru
> to ATAPI devices, the subject of the original patch here.

James brought up the filtering issue, my reply pertains to that alone.
Hence the snipping of unrelated contents in the original mail :-)

-- 
Jens Axboe

Re: [PATCH] SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Luben Tuikov]

--- Douglas Gilbert <dougg@torque.net> wrote:
> 
> I seem to be fighting a losing battle against the mindset

That may be the case.

> It is the _transport_ that should block the command,
> if it so chooses. In this case the transport is a
> virtual one between sr/sg and libata and libata should
> recognize SCSI ATA PASS-THROUGH (12+16) cdbs as
> transport related and not (directly) to be sent to
> the logical unit. And it is at this level we should
> add the wrinkle that if the pdt=5 (cd or dvd) then
> don't translate ATA_12. [I have already been around this
> loop with Luben and his SATL: sorted with the minimum
> of fuss.]

LOL, I don't think your mentioning my name in an email
to bottomley is going to convince him of anything, in
fact it may prompt him to do it the opposite way.

Good luck!

As to the "been around this loop", the fix was a two
liner, without checking the PDT, and ATA_16, ATA_12
and BLANK work as expected.

> ** I have been told that USB disk enclosures being designed
> now will include at a SATL. This will mean smartmontools

Absolutely.  Many reasons, one of which is having to deal
with battles like this one, probably the least one, but
still a reason which is sometimes mentioned.

    Luben

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Tejun Heo]

Mark Lord wrote:
> In an ideal world, we would use the existing ATA_12 opcode
> to issue 12-byte ATA passthrough commands for libata ATAPI drives
> from userspace.
> 
> But ATA_12 happens to have the same SCSI opcode value as the older
> CD/RW "BLANK" command, widely used by cdrecord and friends.
> 
> So, to achieve ATA passthru capability for libata ATAPI,
> we have to instead use the ATA_16 opcode: a 16-byte command.
> 
> SCSI normally disallows issuing 16-byte commands to 12-byte devices,
> so special support has to be added for this.
> 
> Introduce an "allow_ata_16" boolean to the scsi_host struct.
> This provides a means for libata to signal that 16-byte ATA_16
> commands should be permitted even for 12-byte ATAPI devices.
> 
> There are companion patches submitted separately
> to add ATAPI ATA_16 capability to libata.
> 
> Signed-off-by:  Mark Lord <mlord@pobox.com>

I might have missed the discussion but can't we just set
host->max_cmd_len to 16 unconditionally?  libata is emulating a SCSI
device anyway and, from SCSI's POV, the situation is that any libata
ATAPI device can do both 12 and 16 byte commands while some of them only
allow allow ATA_16 for 16 byte command.

Also, it's not like host->max_cmd_len gives any guaranteed protection
against CDB length.  Being a 'host' limit, it's set to the highest
number in the host.  In that respect, it should be set to 16 too.  All
ATA hosts can do 16 byte CDBs.

Thanks.

-- 
tejun

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Mark Lord]

Tejun Heo wrote:
> Mark Lord wrote:
>..
>> So, to achieve ATA passthru capability for libata ATAPI,
>> we have to instead use the ATA_16 opcode: a 16-byte command.
>>
>> SCSI normally disallows issuing 16-byte commands to 12-byte devices,
>> so special support has to be added for this.

> I might have missed the discussion but can't we just set
> host->max_cmd_len to 16 unconditionally?

Sure thing, if you and Jeff are happy with that, then lets do it.

I just kind of assumed that the complexity in ata_set_port_max_cmd_len()
was there for some kind of reason.

For example, I think all existing ATAPI drives only speak 12-byte packet
protocols, and so if we tell SCSI we're good for 16-byte, then won't the
SCSI layer suddenly start sending us READ_16 and the like?

Cheers

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[James Bottomley]

On Thu, 2007-02-01 at 09:33 +0900, Tejun Heo wrote:
> I might have missed the discussion but can't we just set
> host->max_cmd_len to 16 unconditionally?  libata is emulating a SCSI
> device anyway and, from SCSI's POV, the situation is that any libata
> ATAPI device can do both 12 and 16 byte commands while some of them only
> allow allow ATA_16 for 16 byte command.
> 
> Also, it's not like host->max_cmd_len gives any guaranteed protection
> against CDB length.  Being a 'host' limit, it's set to the highest
> number in the host.  In that respect, it should be set to 16 too.  All
> ATA hosts can do 16 byte CDBs.

This looks like a much better proposal to me.

James

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[James Bottomley]

On Wed, 2007-01-31 at 19:42 -0500, Mark Lord wrote:
> Sure thing, if you and Jeff are happy with that, then lets do it.
> 
> I just kind of assumed that the complexity in
> ata_set_port_max_cmd_len()
> was there for some kind of reason.
> 
> For example, I think all existing ATAPI drives only speak 12-byte
> packet
> protocols, and so if we tell SCSI we're good for 16-byte, then won't
> the
> SCSI layer suddenly start sending us READ_16 and the like?

The SCSI parameter max_cdb is supposed to reflect the host, not the
device.  This is for the nutcase of connecting a > 2TB array to say a
qla1280 ... the mid-layer and the device can both go beyond 2TB but the
HBA can't (being limited to 12 byte commands).

In SCSI, we're very careful only to use large commands where necessary,
so even for a >2TB array, you only see 16 byte commands for sectors
beyond 2TB.  This essentially means that the capacity (and we do a
careful READ_CAPACITY(10) and only move on to READ_CAPACITY(16) if we're
told to) limits the command size.

James

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Tejun Heo]

Mark Lord wrote:
> Tejun Heo wrote:
>> Mark Lord wrote:
>> ..
>>> So, to achieve ATA passthru capability for libata ATAPI,
>>> we have to instead use the ATA_16 opcode: a 16-byte command.
>>>
>>> SCSI normally disallows issuing 16-byte commands to 12-byte devices,
>>> so special support has to be added for this.
> 
>> I might have missed the discussion but can't we just set
>> host->max_cmd_len to 16 unconditionally?
> 
> Sure thing, if you and Jeff are happy with that, then lets do it.
> 
> I just kind of assumed that the complexity in ata_set_port_max_cmd_len()
> was there for some kind of reason.
> 
> For example, I think all existing ATAPI drives only speak 12-byte packet
> protocols, and so if we tell SCSI we're good for 16-byte, then won't the
> SCSI layer suddenly start sending us READ_16 and the like?

SCSI always uses the smallest command it can use, so we're safe.  Most
other commands are issued directly from the userland and it's their
responsibility not to feed disallowed commands to a device (or we need
more advanced filter).  Anyways, this has never been guaranteed because
the limit is host wide.

So, I'm for setting it to 16.  Jeff, what do you think?

-- 
tejun

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Mark Lord]

James Bottomley wrote:
>..
> In SCSI, we're very careful only to use large commands where necessary,
> so even for a >2TB array, you only see 16 byte commands for sectors
> beyond 2TB.  This essentially means that the capacity (and we do a
> careful READ_CAPACITY(10) and only move on to READ_CAPACITY(16) if we're
> told to) limits the command size.

Very cool, James.

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Mark Lord]

Tejun Heo wrote:
> Anyways, this has never been guaranteed because
> the limit is host wide.

But until very very recently, "host wide" meant just a single device
for libata.  I was just assuming we did all of the fiddling to ensure
a minimal value there for some real reason.

But, yes, now we have PATA (2 drives per host), and PMP (many more
drives per host), so just maxing out the limit seems sensible.
 
> So, I'm for setting it to 16.  Jeff, what do you think?

This patch sets libata to always accept CDB lengths up to 16 bytes.
With this change, ATA_16 passthrough commands can now be passed into
libata for ATAPI devices.  There is a separate related patch already
pending to actually implement ATA_16 for ATAPI inside libata.

Signed-off-by:  Mark Lord <mlord@pobox.com>
---
--- old/drivers/ata/libata-core.c	2007-01-31 19:55:53.000000000 -0500
+++ linux/drivers/ata/libata-core.c	2007-01-31 19:57:15.000000000 -0500
@@ -1577,20 +1577,6 @@
 		snprintf(desc, desc_sz, "NCQ (depth %d/%d)", hdepth, ddepth);
 }
 
-static void ata_set_port_max_cmd_len(struct ata_port *ap)
-{
-	int i;
-
-	if (ap->scsi_host) {
-		unsigned int len = 0;
-
-		for (i = 0; i < ATA_MAX_DEVICES; i++)
-			len = max(len, ap->device[i].cdb_len);
-
-		ap->scsi_host->max_cmd_len = len;
-	}
-}
-
 /**
  *	ata_dev_configure - Configure the specified ATA/ATAPI device
  *	@dev: Target device to configure
@@ -1771,8 +1757,6 @@
 		}
 	}
 
-	ata_set_port_max_cmd_len(ap);
-
 	/* limit bridge transfers to udma5, 200 sectors */
 	if (ata_dev_knobble(dev)) {
 		if (ata_msg_drv(ap) && print_info)
@@ -5689,7 +5673,7 @@
 	shost->max_id = 16;
 	shost->max_lun = 1;
 	shost->max_channel = 1;
-	shost->max_cmd_len = 12;
+	shost->max_cmd_len = 16;
 }
 
 /**

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jeff Garzik]

Mark Lord wrote:
> For example, I think all existing ATAPI drives only speak 12-byte packet
> protocols, and so if we tell SCSI we're good for 16-byte, then won't the
> SCSI layer suddenly start sending us READ_16 and the like?


Speaking strictly about the device, IDENTIFY PACKET DEVICE data page 
tells us whether the device supports 12-byte or 16-byte CDBs.  No need 
to guess.

Some host controllers only support 12-byte, but I think that most should 
support 16-byte.

	Jeff

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jeff Garzik]

Tejun Heo wrote:
> SCSI always uses the smallest command it can use, so we're safe.  Most
> other commands are issued directly from the userland and it's their
> responsibility not to feed disallowed commands to a device (or we need
> more advanced filter).  Anyways, this has never been guaranteed because
> the limit is host wide.
> 
> So, I'm for setting it to 16.  Jeff, what do you think?


Like I just noted in another email, the limit is really on the /device/ 
side.  In theory the user could plug in a 16-byte ATAPI device and a 
12-byte ATAPI device to the same host.

We should be able to safely raise the limit to 16-byte for most host 
controllers.  Note I said "most".  The bitch will be figuring out which 
host controllers do not like 16-byte CDBs.

	Jeff

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jeff Garzik]

Mark Lord wrote:
> Tejun Heo wrote:
>> Anyways, this has never been guaranteed because
>> the limit is host wide.
> 
> But until very very recently, "host wide" meant just a single device
> for libata.  I was just assuming we did all of the fiddling to ensure
> a minimal value there for some real reason.
> 
> But, yes, now we have PATA (2 drives per host), and PMP (many more
> drives per host), so just maxing out the limit seems sensible.

No, we can't just assume that all host controller CDB FIFOs (if indeed 
that's the implementation) support 16-byte CDBs.

Gotta do a controller-by-controller check.

There are both /host/ and /device/ limits.

	Jeff

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Tejun Heo]

Jeff Garzik wrote:
> Mark Lord wrote:
>> For example, I think all existing ATAPI drives only speak 12-byte packet
>> protocols, and so if we tell SCSI we're good for 16-byte, then won't the
>> SCSI layer suddenly start sending us READ_16 and the like?
> 
> Speaking strictly about the device, IDENTIFY PACKET DEVICE data page
> tells us whether the device supports 12-byte or 16-byte CDBs.  No need
> to guess.
> 
> Some host controllers only support 12-byte, but I think that most should
> support 16-byte.

Out of curiosity, does ATA controllers which don't allow 16byte CDB
actually exist?  It's transferred using PIO protocol anyway.

-- 
tejun

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> SCSI always uses the smallest command it can use, so we're safe.  Most
>> other commands are issued directly from the userland and it's their
>> responsibility not to feed disallowed commands to a device (or we need
>> more advanced filter).  Anyways, this has never been guaranteed because
>> the limit is host wide.
>>
>> So, I'm for setting it to 16.  Jeff, what do you think?
> 
> 
> Like I just noted in another email, the limit is really on the /device/
> side.  In theory the user could plug in a 16-byte ATAPI device and a
> 12-byte ATAPI device to the same host.
> 
> We should be able to safely raise the limit to 16-byte for most host
> controllers.  Note I said "most".  The bitch will be figuring out which
> host controllers do not like 16-byte CDBs.

Well, it's not any worse than what we're currently doing.  We don't set
host cdb len limit according to the host.  We set it to the largest
value among the attached devices.  So, if there is a 12 byte only
controller out there and if you connect 16 byte ATAPI device to it,
you're screwed already and will continue to be screwed after the change.

Note that raising host cdb limit to 16 doesn't make anybody issue 16
byte cdb to the device.  The only unconditionally allowed 16 byte cdb is
ATA_16 which is executed by libata SAT and thus doesn't pass through the
host.

-- 
tejun

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> Mark Lord wrote:
>>> For example, I think all existing ATAPI drives only speak 12-byte packet
>>> protocols, and so if we tell SCSI we're good for 16-byte, then won't the
>>> SCSI layer suddenly start sending us READ_16 and the like?
>> Speaking strictly about the device, IDENTIFY PACKET DEVICE data page
>> tells us whether the device supports 12-byte or 16-byte CDBs.  No need
>> to guess.
>>
>> Some host controllers only support 12-byte, but I think that most should
>> support 16-byte.
> 
> Out of curiosity, does ATA controllers which don't allow 16byte CDB
> actually exist?  It's transferred using PIO protocol anyway.

That's why I think that most PATA controllers are likely safe, since it 
is just more "twiddling signals" directly to the device.

We have to be more careful, ironically, with the "smart" controllers.

They are more likely to keep the CDB contents in a FIFO or other silicon 
buffer somewhere, as temporary storage during a DMA -> buffer -> device 
transfer of the CDB contents.

Any first-gen SATA-emulating-PATA controller is instantly under 
suspicion, because they are not truly "twiddling signals" but emulating 
such by building FIS's under the hood.

	Jeff

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jeff Garzik]

Tejun Heo wrote:
> Well, it's not any worse than what we're currently doing.  We don't set

Agreed... but that doesn't make it the /right/ thing to do ;-)

The logic behind the current code, which limits to the maximum size 
allowed by an attached device on the port, is mainly to leverage the 
SCSI layer as a filter for bad CDB lengths.

IOW, it's called "being lazy" ;-)

	Jeff

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[James Bottomley]

On Thu, 2007-02-01 at 04:54 -0500, Jeff Garzik wrote:
> Agreed... but that doesn't make it the /right/ thing to do ;-)
> 
> The logic behind the current code, which limits to the maximum size 
> allowed by an attached device on the port, is mainly to leverage the 
> SCSI layer as a filter for bad CDB lengths.
> 
> IOW, it's called "being lazy" ;-)

But you're requesting code changes in the SCSI layer because of this
incorrect usage.  max_cdb is supposed to be the *host* limit.  The mid
layer finds out and respects device limits separately from this.

James

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jeff Garzik]

James Bottomley wrote:
> But you're requesting code changes in the SCSI layer because of this
> incorrect usage.  max_cdb is supposed to be the *host* limit.  The mid
> layer finds out and respects device limits separately from this.

I never requested any such thing.

	Jeff

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Douglas Gilbert]

James Bottomley wrote:
> On Thu, 2007-02-01 at 04:54 -0500, Jeff Garzik wrote:
>> Agreed... but that doesn't make it the /right/ thing to do ;-)
>>
>> The logic behind the current code, which limits to the maximum size 
>> allowed by an attached device on the port, is mainly to leverage the 
>> SCSI layer as a filter for bad CDB lengths.
>>
>> IOW, it's called "being lazy" ;-)
> 
> But you're requesting code changes in the SCSI layer because of this
> incorrect usage.  max_cdb is supposed to be the *host* limit.  The mid
> layer finds out and respects device limits separately from this.

To be more pedantic:
  actual_max_cdb = min(MAX_COMMAND_SIZE, host_limit)

Since the host is a bridge, that could be a limit on
near side (i.e. PCI (unlikely)) or the outer side (i.e.
transport initiator (port)). In modern HBAs the
host_limit is likely to be greater than 16, to allow
for advanced SBC and OSD commands. However currently
MAX_COMMAND_SIZE (defined in scsi/scsi_cmnd.h) is 16.

It is the ATAPI _transport_ that has the 12 byte cdb
limit *** (at least according to MMC-5 rev Annex A;
is S-ATAPI any better?).
Other MMC transports referred to in MMC-5 are
SPI, SBP(IEEE 1394) and USB mass storage; and no mention
is made of cdb length limits for them. Since ATAPI is
the dominant transport for cd/dvd drives, MMC doesn't
define any commands over 12 bytes in length, but both
SPC (which MMC should honour) and SSC-3 (think tape
drives, ATAPI connected) do.

My point is that the linux block layer and scsi mid
level should get out of the business of putting hard
limits place. Why?
Since kernel limits are at best necessary but not
sufficient, the upper layers still need to be able to
cope with errors associated with that limit.
So why have the limit?
Does the kernel do analysis to find out whether a USB
connected DVD drive has a USB to ATAPI bridge externally?
I don't think so. There is a role to fetch information
that may act as a guide when a ULD has a choice of commands
to build (e.g. sd deciding between READ(10) and READ(16)).
Let the cdb size bottleneck (or whatever) report an error
and upper layers that are impacted, including user space
programs, can act accordingly.

If the kernel really wants to offload complexity to the
user space, the kernel needs to get out of the business
of trying to foresee errors. It needs to get better at
coping with errors and if possible adapting its behaviour.


*** not the host nor the device

Doug Gilbert

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Jeff Garzik]

Douglas Gilbert wrote:
> It is the ATAPI _transport_ that has the 12 byte cdb
> limit *** (at least according to MMC-5 rev Annex A;
> is S-ATAPI any better?).

Then the spec is wrong.

1) The transport does not limit the CDB size.

2) The ATAPI device limits the CDB size, and it exports this limit via 
IDENTIFY PACKET DEVICE.  It can be either 12 or 16 bytes.

3) In some rare cases, the host controller silicon may limit CDB size.

	Jeff

Re: [PATCH] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Christoph Hellwig]

On Thu, Feb 01, 2007 at 03:21:25PM -0500, Douglas Gilbert wrote:
> My point is that the linux block layer and scsi mid
> level should get out of the business of putting hard
> limits place. Why?

Both of them never have been in the business of putting hard
limits in place.  We currently have a hard limit in place
because the storage for the cdb is allocated statically.  This
is a desin decision done more than ten years ago that we're
slowly trying to fix.  Second we have a facility to allow
LLDDs limit the size of the cdb they receive.  It's not
mandatory in any way, but very useful for those "smart"
HBAs that can't deal with larger dbs due to hardware limitations.