Re: CAM Shingled Disk support patches available

2016-03-02 Thread Scott Long

> On Mar 2, 2016, at 2:25 PM, Kenneth D. Merry  wrote:
> 
>> 
>> 
>> void scsi_ata_pass(struct ccb_scsiio *csio, u_int32_t retries,
>>  void (*cbfcnp)(struct cam_periph *, union ccb *),
>>  u_int32_t flags, u_int8_t tag_action,
>>  struct ata_cmd *cmd, struct ata_res *res,
>>  u_int8_t *data_ptr, u_int32_t dxfer_len,
>>  u_int8_t *data_ptr, u_int16_t dxfer_len,
> 
> I assume you only intended one line there, not two. :)
> 

Indeed =-)

>>  u_int8_t sense_len, u_int32_t timeout);
>> 
>> To differentiate between the 12 and 16 byte variants, you???d look at the
>> AP_EXTEND flag in the protocol field.  Btw, the handling of that flag is
>> inconsistent in the implementation of the existing scsi_ata_pass_16().  If
>> the caller providse an ata_res pointer then it gets filled on completion,
>> otherwise the caller does its best to look at 12.2.2.6 and extract what it
>> can from the sense descriptor.
>> 
>> So my proposal is to create a new scsi_ata_pass and deprecate but not remove
>> scsi_ata_pass_16.  Tell people that if they need to use it, dxfer_len is 
>> going to
>> have lower priority than sector_count/sector_count_exp if the latter 
>> multiply to
>> more than 65535.
> 
> In general I think that's a reasonable idea, but we should probably go
> further.
> 
> While we're at it, we should figure out what we need to do to add the
> Auxiliary register to struct ata_cmd.  We'll need that to do the NCQ
> versions of the various SMR commands, as well as TRIM.
> 

Warner and I talked about this, and I thought that at one point we had a patch
that defined a ‘struct ata_cmd_aux’.  As with function signatures, I’m very
much against redefining structures, and I think it’s reasonable to create a
new structure for what’s basically a late addition to the specs.  I need to read
more of the draft ACS and SATA specs to see if there’s anything else on the
horizon that should also be included.  However, and I talk a little bit about
this below, I think the situation is a bit more complicated than just adding
a field to the structure.  The ata_cmd structure mostly models what’s in an
ATA taskfile, and the taskfile definition, whether from ACS or APT, has never
been expanded to include the aux (and aux_exp) registers.  They exist only
in SATA as part of the Device-to-Host (D2H) FIS.  However, I’m kinda back
and forth on this; maybe my interpretation of ata_cmd is too strict, and we
can stick in whatever is needed and let the SIM deal with it.

At one point I had some patches that defined the
various FIS packets and allowed the periphs to send in an XPT_SATA_FIS
instead of an XPT_ATA_IO, but it seemed to not provide much value since
most drivers (and hardware) still operate in terms of legacy ATA taskfiles.
It also wasn’t clear in the scheme of driving I/O via a FIS where the
responsibility of the periph stopped and the SIM started; If the periph
sends a H2D FIS to initiate an I/O, does it need to then drive the PIO
and/or DMA FIS’s as well?  The FIS is really in the realm of the transport,
not the protocol, and it’s a huge shame that ATA is starting to blur the lines
by having the FIS Aux registers be part of the protocol.

> The obvious challenge is that probably means changing the existing struct
> ccb_ataio CCB and bumping the CAM version.  At least that will be source
> compatible, but will require ifdefs if people want to compile on older
> versions of FreeBSD.  But in that case, they'll also be faced with no
> support for sending the NCQ versions of the commands, anyway.  No way
> around that, though, since we have to follow the changing specs.
> 

Again, not a fan of redefining the structures.

A couple of other thoughts here.  Steve Hartland was right about not abusing
the AP_EXTEND flag.  What are your thoughts on having a new flag
in the function to signal 12 vs 16 byte variants?  Also do you have any thoughts
on the existing arguments?  I’m not sure that tag_action has ever made any
sense for the ATA transport, there’s no such a thing as ordered tags in ATA
and we don’t expose the NCQ tag number outside of the SIM.
MSG_SIMPLE_Q_TAG definitely has no meaning in ATA.  I think the
argument could go away.

Second, I’m not sure that cam_ata_pass (or cam_ata_pass_16, for that matter)
is the right place to include the aux register.  My reading is that it’s 
implementing
the 12.2.2.x chapter of SAT-3, and that doesn’t have any allowance for the
aux register.  SAT-4 r.04a doesn’t seem to mention this register either.  The
register seems to only be exposed when you have access to creating a H2D FIS,
and SAT is pretty much silent on that.  IIRC, when Warner implemented
NCQ TRIM, which required the Aux register, it could only be used on devices
attached via AHCI; LSI controllers had no way of expressing the register.  
Still,
we could add this ad-hoc to cam_ata_pass().  Maybe instead of it taking an
ata_cmd 

Re: CAM Shingled Disk support patches available

2016-03-02 Thread Kenneth D. Merry
On Tue, Mar 01, 2016 at 20:07:19 -0700, Scott Long wrote:
> Hi Ken,
> 
> I???m against changing the function signature of scsi_ata_pass_16().  Even
> if you manage to get things right with symbol versioning, it still leads to
> problems of code compatibility.  Maybe pre-existing binaries will work, but
> source code will forever have to include an #if __FreeBSD_version <
> xx bit of nonsense.

Good point, that would be annoying.

> I agree that it was incorrect for dxferlen to be declared as a uint16_t.
> However, the function already contains a sector count argument pair.  In
> theory the sector count multiplied by the sector length, both of which the
> application should know in order to arrive at a sensible dxferlen, can
> substitute for the dxferlen argument.  If so, then we can just ignore that
> argument and declare that sector_count has logical priority.

Okay.  That will probably work for the most part.

> Really though, I think that scsi_ata_pass_16() is a crummy function.  If its
> purpose is to implement SAT-3 12.2.2, it does an incredibly poor job at it:
> 
> - By my count, it only covers 12 of the available 13 registers.
> 
> - It has no 12 byte, opcode 0xa1 variant.
> 
> - It doesn???t make any allowance for providing the response registers to the
> caller on completion.  Well, maybe it kinda does through a sense descriptor,
> but???. it???s kinda open to vague interpretation.
> 
> - Its use of the registers is clunky, assuming for example that you???ll only 
> want
> to fill the six LBA registers with a host-ordered 64-bit number.  There are
> plenty of commands that re-use sub-parts of the LBA, features, and/or sector
> count registers for different things.  
> 
> I know you stated that you didn???t want to do this, but I think it???s 
> better to start
> over with a better function that has a better signature and a new name.  In 
> fact,
> I think it???s better to use the existing ata_cmd and ata_res structures from 
> sys/cam/ata/ata_all.h, provide accessors for the multi-byte registers if 
> needed,
> provide a 12-byte compatibility, and simply the signature.  Something like 
> this:
> 
> void scsi_ata_pass(struct ccb_scsiio *csio, u_int32_t retries,
>   void (*cbfcnp)(struct cam_periph *, union ccb *),
>   u_int32_t flags, u_int8_t tag_action,
>   struct ata_cmd *cmd, struct ata_res *res,
>   u_int8_t *data_ptr, u_int32_t dxfer_len,
>   u_int8_t *data_ptr, u_int16_t dxfer_len,

I assume you only intended one line there, not two. :)

>   u_int8_t sense_len, u_int32_t timeout);
> 
> To differentiate between the 12 and 16 byte variants, you???d look at the
> AP_EXTEND flag in the protocol field.  Btw, the handling of that flag is
> inconsistent in the implementation of the existing scsi_ata_pass_16().  If
> the caller providse an ata_res pointer then it gets filled on completion,
> otherwise the caller does its best to look at 12.2.2.6 and extract what it
> can from the sense descriptor.
> 
> So my proposal is to create a new scsi_ata_pass and deprecate but not remove
> scsi_ata_pass_16.  Tell people that if they need to use it, dxfer_len is 
> going to
> have lower priority than sector_count/sector_count_exp if the latter multiply 
> to
> more than 65535.

In general I think that's a reasonable idea, but we should probably go
further.

While we're at it, we should figure out what we need to do to add the
Auxiliary register to struct ata_cmd.  We'll need that to do the NCQ
versions of the various SMR commands, as well as TRIM.

The obvious challenge is that probably means changing the existing struct
ccb_ataio CCB and bumping the CAM version.  At least that will be source
compatible, but will require ifdefs if people want to compile on older
versions of FreeBSD.  But in that case, they'll also be faced with no
support for sending the NCQ versions of the commands, anyway.  No way
around that, though, since we have to follow the changing specs.

Ken
-- 
Kenneth Merry
k...@freebsd.org
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: CAM Shingled Disk support patches available

2016-03-02 Thread Douglas Gilbert

On 16-03-01 10:07 PM, Scott Long via freebsd-scsi wrote:

Hi Ken,

I’m against changing the function signature of scsi_ata_pass_16().  Even
if you manage to get things right with symbol versioning, it still leads to
problems of code compatibility.  Maybe pre-existing binaries will work, but
source code will forever have to include an #if __FreeBSD_version <
xx bit of nonsense.

I agree that it was incorrect for dxferlen to be declared as a uint16_t.
However, the function already contains a sector count argument pair.  In
theory the sector count multiplied by the sector length, both of which the
application should know in order to arrive at a sensible dxferlen, can
substitute for the dxferlen argument.  If so, then we can just ignore that
argument and declare that sector_count has logical priority.

Really though, I think that scsi_ata_pass_16() is a crummy function.  If its
purpose is to implement SAT-3 12.2.2, it does an incredibly poor job at it:

- By my count, it only covers 12 of the available 13 registers.

- It has no 12 byte, opcode 0xa1 variant.

- It doesn’t make any allowance for providing the response registers to the
caller on completion.  Well, maybe it kinda does through a sense descriptor,
but…. it’s kinda open to vague interpretation.

- Its use of the registers is clunky, assuming for example that you’ll only want
to fill the six LBA registers with a host-ordered 64-bit number.  There are
plenty of commands that re-use sub-parts of the LBA, features, and/or sector
count registers for different things.

I know you stated that you didn’t want to do this, but I think it’s better to 
start
over with a better function that has a better signature and a new name.  In 
fact,
I think it’s better to use the existing ata_cmd and ata_res structures from
sys/cam/ata/ata_all.h, provide accessors for the multi-byte registers if needed,
provide a 12-byte compatibility, and simply the signature.  Something like this:

void scsi_ata_pass(struct ccb_scsiio *csio, u_int32_t retries,
   void (*cbfcnp)(struct cam_periph *, union ccb *),
   u_int32_t flags, u_int8_t tag_action,
   struct ata_cmd *cmd, struct ata_res *res,
   u_int8_t *data_ptr, u_int32_t dxfer_len,
   u_int8_t *data_ptr, u_int16_t dxfer_len,
   u_int8_t sense_len, u_int32_t timeout);


uint32_t and uint8_t please :-)

For the pendants:
  https://www.freebsd.org/cgi/man.cgi?query=style&sektion=9

"The project is slowly moving to use the ISO/IEC 9899:1999
(``ISO C99'') unsigned integer identifiers of the form uintXX_t
in preference to the older BSD-style integer identifiers of
the form u_intXX_t. New code should use the former, and old
code should be converted to the new form if other major work is
being done in that area and there is no overriding reason to
prefer the older BSD-style. Like white-space commits, care should
be taken in making uintXX_t only commits."

The Linux kernel ain't much better with u8, u16 and u32 typedefs
everywhere.

Doug Gilbert


To differentiate between the 12 and 16 byte variants, you’d look at the
AP_EXTEND flag in the protocol field.  Btw, the handling of that flag is
inconsistent in the implementation of the existing scsi_ata_pass_16().  If
the caller providse an ata_res pointer then it gets filled on completion,
otherwise the caller does its best to look at 12.2.2.6 and extract what it
can from the sense descriptor.

So my proposal is to create a new scsi_ata_pass and deprecate but not remove
scsi_ata_pass_16.  Tell people that if they need to use it, dxfer_len is going 
to
have lower priority than sector_count/sector_count_exp if the latter multiply to
more than 65535.

Scott




On Mar 1, 2016, at 3:47 PM, Kenneth D. Merry  wrote:

I have a new set of SMR patches available.  (See the original message below
for a more detailed description of what these patches do.)

The primary change is to add library versioning to libcam so that we can
change the function prototype of scsi_ata_pass_16() in a way that won't
break existing binaries.

If someone more familiar with library versioning wants to review this, I'd
appreciate it.

The patches are here:

FreeBSD/head, as of SVN revision 296278

https://people.freebsd.org/~ken/cam_smr.head.20160301.1.txt

stable/10, as of SVN revision 296248

https://people.freebsd.org/~ken/cam_smr.stable10.20160301.1.txt

(Note that although there is a stable/10 version of the patches, I'm not
planning to merge them to stable/10 because of the change to struct bio.  I
can't really figure out a good way to make that backward compatible.  If
there is consensus that breaking it is fine because it isn't a user API,
then that may be another story.)

The problem is that the existing, in-tree version of scsi_ata_pass_16() has
a dxfer_len argument that is a uint16_t.  That restricts transfer sizes to
64KB.  So, we need to update it to allow larger than 64K transfers.  I
co

Re: CAM Shingled Disk support patches available

2016-03-01 Thread Scott Long
Hi Ken,

I’m against changing the function signature of scsi_ata_pass_16().  Even
if you manage to get things right with symbol versioning, it still leads to
problems of code compatibility.  Maybe pre-existing binaries will work, but
source code will forever have to include an #if __FreeBSD_version <
xx bit of nonsense.

I agree that it was incorrect for dxferlen to be declared as a uint16_t.
However, the function already contains a sector count argument pair.  In
theory the sector count multiplied by the sector length, both of which the
application should know in order to arrive at a sensible dxferlen, can
substitute for the dxferlen argument.  If so, then we can just ignore that
argument and declare that sector_count has logical priority.

Really though, I think that scsi_ata_pass_16() is a crummy function.  If its
purpose is to implement SAT-3 12.2.2, it does an incredibly poor job at it:

- By my count, it only covers 12 of the available 13 registers.

- It has no 12 byte, opcode 0xa1 variant.

- It doesn’t make any allowance for providing the response registers to the
caller on completion.  Well, maybe it kinda does through a sense descriptor,
but…. it’s kinda open to vague interpretation.

- Its use of the registers is clunky, assuming for example that you’ll only want
to fill the six LBA registers with a host-ordered 64-bit number.  There are
plenty of commands that re-use sub-parts of the LBA, features, and/or sector
count registers for different things.  

I know you stated that you didn’t want to do this, but I think it’s better to 
start
over with a better function that has a better signature and a new name.  In 
fact,
I think it’s better to use the existing ata_cmd and ata_res structures from 
sys/cam/ata/ata_all.h, provide accessors for the multi-byte registers if needed,
provide a 12-byte compatibility, and simply the signature.  Something like this:

void scsi_ata_pass(struct ccb_scsiio *csio, u_int32_t retries,
  void (*cbfcnp)(struct cam_periph *, union ccb *),
  u_int32_t flags, u_int8_t tag_action,
  struct ata_cmd *cmd, struct ata_res *res,
  u_int8_t *data_ptr, u_int32_t dxfer_len,
  u_int8_t *data_ptr, u_int16_t dxfer_len,
  u_int8_t sense_len, u_int32_t timeout);

To differentiate between the 12 and 16 byte variants, you’d look at the
AP_EXTEND flag in the protocol field.  Btw, the handling of that flag is
inconsistent in the implementation of the existing scsi_ata_pass_16().  If
the caller providse an ata_res pointer then it gets filled on completion,
otherwise the caller does its best to look at 12.2.2.6 and extract what it
can from the sense descriptor.

So my proposal is to create a new scsi_ata_pass and deprecate but not remove
scsi_ata_pass_16.  Tell people that if they need to use it, dxfer_len is going 
to
have lower priority than sector_count/sector_count_exp if the latter multiply to
more than 65535.

Scott



> On Mar 1, 2016, at 3:47 PM, Kenneth D. Merry  wrote:
> 
> I have a new set of SMR patches available.  (See the original message below
> for a more detailed description of what these patches do.)
> 
> The primary change is to add library versioning to libcam so that we can
> change the function prototype of scsi_ata_pass_16() in a way that won't
> break existing binaries.
> 
> If someone more familiar with library versioning wants to review this, I'd
> appreciate it.
> 
> The patches are here:
> 
> FreeBSD/head, as of SVN revision 296278
> 
> https://people.freebsd.org/~ken/cam_smr.head.20160301.1.txt
> 
> stable/10, as of SVN revision 296248
> 
> https://people.freebsd.org/~ken/cam_smr.stable10.20160301.1.txt
> 
> (Note that although there is a stable/10 version of the patches, I'm not
> planning to merge them to stable/10 because of the change to struct bio.  I
> can't really figure out a good way to make that backward compatible.  If
> there is consensus that breaking it is fine because it isn't a user API,
> then that may be another story.)
> 
> The problem is that the existing, in-tree version of scsi_ata_pass_16() has
> a dxfer_len argument that is a uint16_t.  That restricts transfer sizes to
> 64KB.  So, we need to update it to allow larger than 64K transfers.  I
> could just create a new function, but I'd rather just retire the broken
> version.
> 
> The intent here is that:
> 
> 1. Binaries built against the old version of libcam, before versioning was
> turned on, will get the old version of the scsi_ata_pass_16() function with
> a uint16_t dxfer_len.
> 
> 2. Binaries built against the new version of libcam will get the new
> version of the scsi_ata_pass_16() function with a uint32_t dxfer_len.
> 
> I've tested this, and it appears to work, but I'm not 100% certain this is
> all correct.  I looked at Dan Eischen's description of symbol versioning
> here:
> 
> https://people.freebsd.org/~deischen/symver/freebsd_versioning.txt
> 
> And 

Re: CAM Shingled Disk support patches available

2016-03-01 Thread Kenneth D. Merry
I have a new set of SMR patches available.  (See the original message below
for a more detailed description of what these patches do.)

The primary change is to add library versioning to libcam so that we can
change the function prototype of scsi_ata_pass_16() in a way that won't
break existing binaries.

If someone more familiar with library versioning wants to review this, I'd
appreciate it.

The patches are here:

FreeBSD/head, as of SVN revision 296278

https://people.freebsd.org/~ken/cam_smr.head.20160301.1.txt

stable/10, as of SVN revision 296248

https://people.freebsd.org/~ken/cam_smr.stable10.20160301.1.txt

(Note that although there is a stable/10 version of the patches, I'm not
planning to merge them to stable/10 because of the change to struct bio.  I
can't really figure out a good way to make that backward compatible.  If
there is consensus that breaking it is fine because it isn't a user API,
then that may be another story.)

The problem is that the existing, in-tree version of scsi_ata_pass_16() has
a dxfer_len argument that is a uint16_t.  That restricts transfer sizes to
64KB.  So, we need to update it to allow larger than 64K transfers.  I
could just create a new function, but I'd rather just retire the broken
version.

The intent here is that:

1. Binaries built against the old version of libcam, before versioning was
turned on, will get the old version of the scsi_ata_pass_16() function with
a uint16_t dxfer_len.

2. Binaries built against the new version of libcam will get the new
version of the scsi_ata_pass_16() function with a uint32_t dxfer_len.

I've tested this, and it appears to work, but I'm not 100% certain this is
all correct.  I looked at Dan Eischen's description of symbol versioning
here:

https://people.freebsd.org/~deischen/symver/freebsd_versioning.txt

And it looks like the actual implementation is a little different than what
is described there.  I looked around the tree, and didn't see anything that
is obviously exactly like what I'm trying to do here.

So, what I did is as follows:

1. For the kernel, the only change is to switch the dxfer_len argument from
a uint16_t to a uint32_t.

2. For userland, in scsi_all.c, there are now two versions of
scsi_ata_pass_16 -- _ver1 and _ver2.  _ver1 is aliased to
scsi_ata_pass_16() for FBSD_1.3 using __sym_compat().  _ver2 is aliased to
scsi_ata_pass_16() for FBSD_1.4 using __sym_default().

3. In lib/libcam/Versions.def, I defined FBSD_1.3 and FBSD_1.4, which
depends on FBSD_1.3.

4. In lib/libcam/Symbol.map, I pulled out all of the functions defined in
libcam, sorted them, and defined them in FBSD_1.3.  I moved
scsi_ata_pass_16() to FBSD_1.4.  (According to the freebsd_versioning.txt
paper linked above, I should have been able to have scsi_ata_pass_16() in
both FBSD_1.3 and FBSD_1.4, but that isn't the case in practice.)

In testing an old binary (linked against libcam without symbol versioning)
against a new libcam (with symbol versioning), the old version of the
function appears to be used.  With a new binary, the new version of the
function appears to be used.

So it looks like things work as intended, but I don't fully trust my
understanding here.  So, if someone could take a look at the changes, I'd
appreciate it.

In particular, I have a few questions:

1. If this change to scsi_ata_pass_16() gets merged to stable/10 (apart
from the larger SMR changes), what should be done with the libcam library
version?

2. Are 1.3 and 1.4 the proper versions to use?

3. If we make additional CAM helper function library changes, when do the
versions get bumped?  i.e., is this an opportunity to look for other
library functions with issues and make changes if possible?

4. When you're going from an unversioned library to a versioned library,
which version of a function gets linked in to a binary linked to the
unversioned library when you run it against a versioned library?  In other
words, what is supposed to happen in the test scenario I tried above, and
am I really seeing what is supposed to happen?

Thanks,

Ken

On Mon, Jan 18, 2016 at 17:37:04 -0500, Kenneth D. Merry wrote:
> I have a new set of SMR patches available.  See below for the full
> explanation.
> 
> The primary change here is that I have added SMR support to the ada(4)
> driver.  I spent some time considering whether to try to make the da(4) and
> ada(4) probe infrastructure somewhat common, but in the end concluded it
> would be too involved with not enough code reduction (if any) in the end.
> 
> So, although the ideas are similar, the probe logic is separate.
> 
> Note that NCQ support for SMR commands (Report Zones, Reset Write Pointer,
> etc.) for SATA protocol shingled drives isn't active.  For both the da(4)
> and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
> register down to the drive.
> 
> In the ada(4) case, we need to add the register to struct ccb_ataio and
> add support in one or more of the underlying SATA drivers, e.g. ahci(4).
> 

Re: CAM Shingled Disk support patches available

2016-01-19 Thread Scott Long

> On Jan 19, 2016, at 8:25 AM, Kenneth D. Merry  wrote:
> 
> 
>>> In the ada(4) case, we need to add the register to struct ccb_ataio and
>>> add support in one or more of the underlying SATA drivers, e.g. ahci(4).
>> 
>> I believe that changes the size of the CCB, so I tried to avoid
>> that since I didn???t want to force a recompile of camcontrol(8).
>> Adding it to the atacmd structure wasn???t so bad, and the CCB size
>> didn???t completely change. The problem was that the atacmd changed
>> size and pushed all the other fields.
> 
> Yes.  In order to do it, we'll need to add it to struct atacmd, and add
> compatibility shims.  I don't see another way to do it unfortunately.
> 

No, I object to changing the structure sizes and contents.  It should be a
new CCB, XPT_ATA_IO_EXT, ccb_ataio_ext.  If you want to add more
future-looking fields than just the AUX register, maybe a way to define
un-typed command and response register objects, that’s fine and probably a
good idea.  The periph drivers can probe for the proper command to send
based on whether the SIM returning CAM_FUNC_NOTAVAIL or via a PIM
flag, or even better, via a KVP capability CCB.  However I’m firmly against
changing the existing data structures; compat shims have been a pain and
are a poor solution.

Warner and I are pounding out a proposal for this, will share a diff shortly.

Scott

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: CAM Shingled Disk support patches available

2016-01-19 Thread Slawa Olhovchenkov
On Tue, Jan 19, 2016 at 12:06:41PM -0500, Kenneth D. Merry wrote:

> On Tue, Jan 19, 2016 at 20:02:52 +0300, Slawa Olhovchenkov wrote:
> > On Tue, Jan 19, 2016 at 11:38:31AM -0500, Kenneth D. Merry wrote:
> > 
> > > On Tue, Jan 19, 2016 at 14:45:23 +0300, Slawa Olhovchenkov wrote:
> > > > On Mon, Jan 18, 2016 at 05:37:04PM -0500, Kenneth D. Merry wrote:
> > > > 
> > > > > I have a new set of SMR patches available.  See below for the full
> > > > > explanation.
> > > > > 
> > > > > The primary change here is that I have added SMR support to the ada(4)
> > > > > driver.  I spent some time considering whether to try to make the 
> > > > > da(4) and
> > > > > ada(4) probe infrastructure somewhat common, but in the end concluded 
> > > > > it
> > > > > would be too involved with not enough code reduction (if any) in the 
> > > > > end.
> > > > > 
> > > > > So, although the ideas are similar, the probe logic is separate.
> > > > > 
> > > > > Note that NCQ support for SMR commands (Report Zones, Reset Write 
> > > > > Pointer,
> > > > > etc.) for SATA protocol shingled drives isn't active.  For both the 
> > > > > da(4)
> > > > > and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
> > > > > register down to the drive.
> > > > > 
> > > > > In the ada(4) case, we need to add the register to struct ccb_ataio 
> > > > > and
> > > > > add support in one or more of the underlying SATA drivers, e.g. 
> > > > > ahci(4).
> > > > > 
> > > > > In the da(4) case, it will require an update of the T-10 SAT spec to
> > > > > provide a way to pass the Auxiliary register down via the SCSI ATA
> > > > > PASS-THROUGH command, and then a subsquent update of the SAT layer in
> > > > > various vendors' SAS controller firmware.  At that point, there may 
> > > > > be 
> > > > > an official mapping of the SCSI ZBC commands to the ATA ZAC commands, 
> > > > > and
> > > > > we may be able to just issue the SCSI version of the commands instead 
> > > > > of
> > > > > composing ATA commands in the da(4) driver.  (We'll still need to 
> > > > > keep the
> > > > > ATA passthrough version for a while at least to support controllers 
> > > > > that
> > > > > don't have the updated translation code.)
> > > > 
> > > > Please, check me: currenly SMR lack of support in SCSI devices? On
> > > > [hardvare] vendor level? Currenly only SATA controllers compatible
> > > > with SMR (on command level)? (I am don't talk about FreeBSD support,
> > > > question about common state).
> > > 
> > > No, there are SAS/SCSI SMR drives in development, and there is the SCSI 
> > > ZBC
> > > spec that defines the command set.  I don't know whether any vendors are
> > > shipping SAS/SCSI SMR drives yet.
> > > 
> > > You can use SATA drives (SMR or not) with either a SATA controller or a 
> > > SAS
> > > controller.  But the way you talk to a SATA drive through a SAS controller
> > > is with SCSI commands.  There is a SCSI spec (SAT) that defines the 
> > > mapping
> > > of SCSI commands to ATA commands.  It has recently been updated to support
> > > mapping SMR commands from SCSI to ATA, but most (all?) SAS controllers
> > > have not caught up with the spec.
> > > 
> > > So to use a SATA SMR drive with a SAS controller that doesn't know how to
> > > map SMR commands from SCSI to ATA, you have to send the ATA SMR commands
> > > through the SCSI ATA PASS-THROUGH command.  That just bypasses the usual
> > > translations, and allows sending ATA commands in something like their
> > > native form.
> > 
> > What in case of expanders an port replicatiors (SATA drives and HBA
> > SAS controllers, of course)? Need expander be compatible with SMR? Or
> > any expander with SATA support automaticly compatible?
> 
> Expanders and port replicators shouldn't matter.  The place where you need
> to know about SMR is the place where the native ATA or SCSI drive commands
> are generated.  Expanders and port replicators typically just pass commands
> through.

Thanks for clarification!
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: CAM Shingled Disk support patches available

2016-01-19 Thread Kenneth D. Merry
On Tue, Jan 19, 2016 at 20:02:52 +0300, Slawa Olhovchenkov wrote:
> On Tue, Jan 19, 2016 at 11:38:31AM -0500, Kenneth D. Merry wrote:
> 
> > On Tue, Jan 19, 2016 at 14:45:23 +0300, Slawa Olhovchenkov wrote:
> > > On Mon, Jan 18, 2016 at 05:37:04PM -0500, Kenneth D. Merry wrote:
> > > 
> > > > I have a new set of SMR patches available.  See below for the full
> > > > explanation.
> > > > 
> > > > The primary change here is that I have added SMR support to the ada(4)
> > > > driver.  I spent some time considering whether to try to make the da(4) 
> > > > and
> > > > ada(4) probe infrastructure somewhat common, but in the end concluded it
> > > > would be too involved with not enough code reduction (if any) in the 
> > > > end.
> > > > 
> > > > So, although the ideas are similar, the probe logic is separate.
> > > > 
> > > > Note that NCQ support for SMR commands (Report Zones, Reset Write 
> > > > Pointer,
> > > > etc.) for SATA protocol shingled drives isn't active.  For both the 
> > > > da(4)
> > > > and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
> > > > register down to the drive.
> > > > 
> > > > In the ada(4) case, we need to add the register to struct ccb_ataio and
> > > > add support in one or more of the underlying SATA drivers, e.g. ahci(4).
> > > > 
> > > > In the da(4) case, it will require an update of the T-10 SAT spec to
> > > > provide a way to pass the Auxiliary register down via the SCSI ATA
> > > > PASS-THROUGH command, and then a subsquent update of the SAT layer in
> > > > various vendors' SAS controller firmware.  At that point, there may be 
> > > > an official mapping of the SCSI ZBC commands to the ATA ZAC commands, 
> > > > and
> > > > we may be able to just issue the SCSI version of the commands instead of
> > > > composing ATA commands in the da(4) driver.  (We'll still need to keep 
> > > > the
> > > > ATA passthrough version for a while at least to support controllers that
> > > > don't have the updated translation code.)
> > > 
> > > Please, check me: currenly SMR lack of support in SCSI devices? On
> > > [hardvare] vendor level? Currenly only SATA controllers compatible
> > > with SMR (on command level)? (I am don't talk about FreeBSD support,
> > > question about common state).
> > 
> > No, there are SAS/SCSI SMR drives in development, and there is the SCSI ZBC
> > spec that defines the command set.  I don't know whether any vendors are
> > shipping SAS/SCSI SMR drives yet.
> > 
> > You can use SATA drives (SMR or not) with either a SATA controller or a SAS
> > controller.  But the way you talk to a SATA drive through a SAS controller
> > is with SCSI commands.  There is a SCSI spec (SAT) that defines the mapping
> > of SCSI commands to ATA commands.  It has recently been updated to support
> > mapping SMR commands from SCSI to ATA, but most (all?) SAS controllers
> > have not caught up with the spec.
> > 
> > So to use a SATA SMR drive with a SAS controller that doesn't know how to
> > map SMR commands from SCSI to ATA, you have to send the ATA SMR commands
> > through the SCSI ATA PASS-THROUGH command.  That just bypasses the usual
> > translations, and allows sending ATA commands in something like their
> > native form.
> 
> What in case of expanders an port replicatiors (SATA drives and HBA
> SAS controllers, of course)? Need expander be compatible with SMR? Or
> any expander with SATA support automaticly compatible?

Expanders and port replicators shouldn't matter.  The place where you need
to know about SMR is the place where the native ATA or SCSI drive commands
are generated.  Expanders and port replicators typically just pass commands
through.

Ken
-- 
Kenneth Merry
k...@freebsd.org
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: CAM Shingled Disk support patches available

2016-01-19 Thread Slawa Olhovchenkov
On Tue, Jan 19, 2016 at 11:38:31AM -0500, Kenneth D. Merry wrote:

> On Tue, Jan 19, 2016 at 14:45:23 +0300, Slawa Olhovchenkov wrote:
> > On Mon, Jan 18, 2016 at 05:37:04PM -0500, Kenneth D. Merry wrote:
> > 
> > > I have a new set of SMR patches available.  See below for the full
> > > explanation.
> > > 
> > > The primary change here is that I have added SMR support to the ada(4)
> > > driver.  I spent some time considering whether to try to make the da(4) 
> > > and
> > > ada(4) probe infrastructure somewhat common, but in the end concluded it
> > > would be too involved with not enough code reduction (if any) in the end.
> > > 
> > > So, although the ideas are similar, the probe logic is separate.
> > > 
> > > Note that NCQ support for SMR commands (Report Zones, Reset Write Pointer,
> > > etc.) for SATA protocol shingled drives isn't active.  For both the da(4)
> > > and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
> > > register down to the drive.
> > > 
> > > In the ada(4) case, we need to add the register to struct ccb_ataio and
> > > add support in one or more of the underlying SATA drivers, e.g. ahci(4).
> > > 
> > > In the da(4) case, it will require an update of the T-10 SAT spec to
> > > provide a way to pass the Auxiliary register down via the SCSI ATA
> > > PASS-THROUGH command, and then a subsquent update of the SAT layer in
> > > various vendors' SAS controller firmware.  At that point, there may be 
> > > an official mapping of the SCSI ZBC commands to the ATA ZAC commands, and
> > > we may be able to just issue the SCSI version of the commands instead of
> > > composing ATA commands in the da(4) driver.  (We'll still need to keep the
> > > ATA passthrough version for a while at least to support controllers that
> > > don't have the updated translation code.)
> > 
> > Please, check me: currenly SMR lack of support in SCSI devices? On
> > [hardvare] vendor level? Currenly only SATA controllers compatible
> > with SMR (on command level)? (I am don't talk about FreeBSD support,
> > question about common state).
> 
> No, there are SAS/SCSI SMR drives in development, and there is the SCSI ZBC
> spec that defines the command set.  I don't know whether any vendors are
> shipping SAS/SCSI SMR drives yet.
> 
> You can use SATA drives (SMR or not) with either a SATA controller or a SAS
> controller.  But the way you talk to a SATA drive through a SAS controller
> is with SCSI commands.  There is a SCSI spec (SAT) that defines the mapping
> of SCSI commands to ATA commands.  It has recently been updated to support
> mapping SMR commands from SCSI to ATA, but most (all?) SAS controllers
> have not caught up with the spec.
> 
> So to use a SATA SMR drive with a SAS controller that doesn't know how to
> map SMR commands from SCSI to ATA, you have to send the ATA SMR commands
> through the SCSI ATA PASS-THROUGH command.  That just bypasses the usual
> translations, and allows sending ATA commands in something like their
> native form.

What in case of expanders an port replicatiors (SATA drives and HBA
SAS controllers, of course)? Need expander be compatible with SMR? Or
any expander with SATA support automaticly compatible?
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: CAM Shingled Disk support patches available

2016-01-19 Thread Kenneth D. Merry
On Tue, Jan 19, 2016 at 14:45:23 +0300, Slawa Olhovchenkov wrote:
> On Mon, Jan 18, 2016 at 05:37:04PM -0500, Kenneth D. Merry wrote:
> 
> > I have a new set of SMR patches available.  See below for the full
> > explanation.
> > 
> > The primary change here is that I have added SMR support to the ada(4)
> > driver.  I spent some time considering whether to try to make the da(4) and
> > ada(4) probe infrastructure somewhat common, but in the end concluded it
> > would be too involved with not enough code reduction (if any) in the end.
> > 
> > So, although the ideas are similar, the probe logic is separate.
> > 
> > Note that NCQ support for SMR commands (Report Zones, Reset Write Pointer,
> > etc.) for SATA protocol shingled drives isn't active.  For both the da(4)
> > and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
> > register down to the drive.
> > 
> > In the ada(4) case, we need to add the register to struct ccb_ataio and
> > add support in one or more of the underlying SATA drivers, e.g. ahci(4).
> > 
> > In the da(4) case, it will require an update of the T-10 SAT spec to
> > provide a way to pass the Auxiliary register down via the SCSI ATA
> > PASS-THROUGH command, and then a subsquent update of the SAT layer in
> > various vendors' SAS controller firmware.  At that point, there may be 
> > an official mapping of the SCSI ZBC commands to the ATA ZAC commands, and
> > we may be able to just issue the SCSI version of the commands instead of
> > composing ATA commands in the da(4) driver.  (We'll still need to keep the
> > ATA passthrough version for a while at least to support controllers that
> > don't have the updated translation code.)
> 
> Please, check me: currenly SMR lack of support in SCSI devices? On
> [hardvare] vendor level? Currenly only SATA controllers compatible
> with SMR (on command level)? (I am don't talk about FreeBSD support,
> question about common state).

No, there are SAS/SCSI SMR drives in development, and there is the SCSI ZBC
spec that defines the command set.  I don't know whether any vendors are
shipping SAS/SCSI SMR drives yet.

You can use SATA drives (SMR or not) with either a SATA controller or a SAS
controller.  But the way you talk to a SATA drive through a SAS controller
is with SCSI commands.  There is a SCSI spec (SAT) that defines the mapping
of SCSI commands to ATA commands.  It has recently been updated to support
mapping SMR commands from SCSI to ATA, but most (all?) SAS controllers
have not caught up with the spec.

So to use a SATA SMR drive with a SAS controller that doesn't know how to
map SMR commands from SCSI to ATA, you have to send the ATA SMR commands
through the SCSI ATA PASS-THROUGH command.  That just bypasses the usual
translations, and allows sending ATA commands in something like their
native form.

Ken
-- 
Kenneth Merry
k...@freebsd.org
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: CAM Shingled Disk support patches available

2016-01-19 Thread Kenneth D. Merry
On Mon, Jan 18, 2016 at 16:50:34 -0800, Warner Losh wrote:
> 
> > On Jan 18, 2016, at 2:37 PM, Kenneth D. Merry  wrote:
> > 
> > I have a new set of SMR patches available.  See below for the full
> > explanation.
> > 
> > The primary change here is that I have added SMR support to the ada(4)
> > driver.  I spent some time considering whether to try to make the da(4) and
> > ada(4) probe infrastructure somewhat common, but in the end concluded it
> > would be too involved with not enough code reduction (if any) in the end.
> > 
> > So, although the ideas are similar, the probe logic is separate.
> > 
> > Note that NCQ support for SMR commands (Report Zones, Reset Write Pointer,
> > etc.) for SATA protocol shingled drives isn't active.  For both the da(4)
> > and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
> > register down to the drive.
> 
> I???ve plumbed it down, but in a gross, kludgy way to make NCQ Trim work
> where the only value in the Auxiliary register needs to be 1. It only takes
> up one bit, but it doesn???t change the size of the CCB. If the NCQ Trim
> work wasn???t based on the I/O scheduler, I???d have pushed it into head
> and would be happy to share code.

Yeah, for SMR, we'll need to pass the full register down.  That is how you
specify the service action (open, close, finish, reset write pointer,
report zones).

> AHCI can send it, but it turns out that LSI???s drivers (mpt, mps, etc)
> can???t do it due to firmware inadequacies. The ability to send a FIS
> in these firmwares looked promising, but it requires a full draining of
> other requests, which kind of defeats the purpose of NCQ.

Yeah, that would kinda defeat the purpose.  I'm sending a SCSI command
(ATA PASS-THROUGH) to get the SATA zone commands down there.  Those are
treated like an ordered tag by the LSI firmware as well.  Which is just as
well, since there is no way to specify the Auxiliary register via that SCSI
command, and so we can't do NCQ anyway.

LSI/Avago said they're planning to support the zone commands in the SAT
layer in the HBAs in the 12Gb boards.  Phase 10 doesn't have it from what I
understand, but hopefully that'll show up soon.  The translation is in the
latest SAT draft, and it is very straightforward to map from one to the
other, because the SCSI and ATA commands and semantics are pretty much
identical.

> > In the ada(4) case, we need to add the register to struct ccb_ataio and
> > add support in one or more of the underlying SATA drivers, e.g. ahci(4).
> 
> I believe that changes the size of the CCB, so I tried to avoid
> that since I didn???t want to force a recompile of camcontrol(8).
> Adding it to the atacmd structure wasn???t so bad, and the CCB size
> didn???t completely change. The problem was that the atacmd changed
> size and pushed all the other fields.

Yes.  In order to do it, we'll need to add it to struct atacmd, and add
compatibility shims.  I don't see another way to do it unfortunately.

> > In the da(4) case, it will require an update of the T-10 SAT spec to
> > provide a way to pass the Auxiliary register down via the SCSI ATA
> > PASS-THROUGH command, and then a subsquent update of the SAT layer in
> > various vendors' SAS controller firmware.  At that point, there may be
> > an official mapping of the SCSI ZBC commands to the ATA ZAC commands, and
> > we may be able to just issue the SCSI version of the commands instead of
> > composing ATA commands in the da(4) driver.  (We'll still need to keep the
> > ATA passthrough version for a while at least to support controllers that
> > don't have the updated translation code.)
> 
> I looked to implement things here, but didn???t want to invent something that
> the T-10 would later reinvent.

Yeah.  Is NCQ trim a new thing?  Is that why you were looking at sending it
down via a FIS?

If so, it is likely that LSI will add it to the SCSI Unmap translation in
the firmware.  Of course if it isn't already in there, they're only going
to put it in their 12Gb controllers and not in the 6Gb controllers at this
point.

Since the SAT spec has the mapping for the SCSI ZBC -> ZAC commands, it sounds
like that'll make it into the LSI 12Gb firmware at some point.

> > FreeBSD/head as of SVN revision 294105:
> > 
> > https://people.freebsd.org/~ken/cam_smr.head.20160118.1.txt
> > 
> > FreeBSD stable/10 as of SVN revision 294100:
> > 
> > https://people.freebsd.org/~ken/cam_smr.stable10.20160118.1.txt
> > 
> > Testing and comments are welcome.
> 
> So having said all that, I???m totally open to something better.

I think that for the ATA side, we'll just have to add the register to the
CCB, bump the version and add compatibility shims.

For the SCSI side, we just need a way to probe and see whether the
translation is supported in the SAT layer (at least for the ZBC commands, I
don't know about trim) and use the SCSI command if it is supported,
otherwise use the ATA PASS-THROUGH command if it is not.

Ken
-- 
Kenneth Merry
k...@freebs

Re: CAM Shingled Disk support patches available

2016-01-19 Thread Slawa Olhovchenkov
On Mon, Jan 18, 2016 at 05:37:04PM -0500, Kenneth D. Merry wrote:

> I have a new set of SMR patches available.  See below for the full
> explanation.
> 
> The primary change here is that I have added SMR support to the ada(4)
> driver.  I spent some time considering whether to try to make the da(4) and
> ada(4) probe infrastructure somewhat common, but in the end concluded it
> would be too involved with not enough code reduction (if any) in the end.
> 
> So, although the ideas are similar, the probe logic is separate.
> 
> Note that NCQ support for SMR commands (Report Zones, Reset Write Pointer,
> etc.) for SATA protocol shingled drives isn't active.  For both the da(4)
> and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
> register down to the drive.
> 
> In the ada(4) case, we need to add the register to struct ccb_ataio and
> add support in one or more of the underlying SATA drivers, e.g. ahci(4).
> 
> In the da(4) case, it will require an update of the T-10 SAT spec to
> provide a way to pass the Auxiliary register down via the SCSI ATA
> PASS-THROUGH command, and then a subsquent update of the SAT layer in
> various vendors' SAS controller firmware.  At that point, there may be 
> an official mapping of the SCSI ZBC commands to the ATA ZAC commands, and
> we may be able to just issue the SCSI version of the commands instead of
> composing ATA commands in the da(4) driver.  (We'll still need to keep the
> ATA passthrough version for a while at least to support controllers that
> don't have the updated translation code.)

Please, check me: currenly SMR lack of support in SCSI devices? On
[hardvare] vendor level? Currenly only SATA controllers compatible
with SMR (on command level)? (I am don't talk about FreeBSD support,
question about common state).
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: CAM Shingled Disk support patches available

2016-01-18 Thread Warner Losh

> On Jan 18, 2016, at 2:37 PM, Kenneth D. Merry  wrote:
> 
> I have a new set of SMR patches available.  See below for the full
> explanation.
> 
> The primary change here is that I have added SMR support to the ada(4)
> driver.  I spent some time considering whether to try to make the da(4) and
> ada(4) probe infrastructure somewhat common, but in the end concluded it
> would be too involved with not enough code reduction (if any) in the end.
> 
> So, although the ideas are similar, the probe logic is separate.
> 
> Note that NCQ support for SMR commands (Report Zones, Reset Write Pointer,
> etc.) for SATA protocol shingled drives isn't active.  For both the da(4)
> and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
> register down to the drive.

I’ve plumbed it down, but in a gross, kludgy way to make NCQ Trim work
where the only value in the Auxiliary register needs to be 1. It only takes
up one bit, but it doesn’t change the size of the CCB. If the NCQ Trim
work wasn’t based on the I/O scheduler, I’d have pushed it into head
and would be happy to share code.

AHCI can send it, but it turns out that LSI’s drivers (mpt, mps, etc)
can’t do it due to firmware inadequacies. The ability to send a FIS
in these firmwares looked promising, but it requires a full draining of
other requests, which kind of defeats the purpose of NCQ.

> In the ada(4) case, we need to add the register to struct ccb_ataio and
> add support in one or more of the underlying SATA drivers, e.g. ahci(4).

I believe that changes the size of the CCB, so I tried to avoid
that since I didn’t want to force a recompile of camcontrol(8).
Adding it to the atacmd structure wasn’t so bad, and the CCB size
didn’t completely change. The problem was that the atacmd changed
size and pushed all the other fields.

> In the da(4) case, it will require an update of the T-10 SAT spec to
> provide a way to pass the Auxiliary register down via the SCSI ATA
> PASS-THROUGH command, and then a subsquent update of the SAT layer in
> various vendors' SAS controller firmware.  At that point, there may be
> an official mapping of the SCSI ZBC commands to the ATA ZAC commands, and
> we may be able to just issue the SCSI version of the commands instead of
> composing ATA commands in the da(4) driver.  (We'll still need to keep the
> ATA passthrough version for a while at least to support controllers that
> don't have the updated translation code.)

I looked to implement things here, but didn’t want to invent something that
the T-10 would later reinvent.

> FreeBSD/head as of SVN revision 294105:
> 
> https://people.freebsd.org/~ken/cam_smr.head.20160118.1.txt
> 
> FreeBSD stable/10 as of SVN revision 294100:
> 
> https://people.freebsd.org/~ken/cam_smr.stable10.20160118.1.txt
> 
> Testing and comments are welcome.

So having said all that, I’m totally open to something better.

Warner



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: CAM Shingled Disk support patches available

2016-01-18 Thread Kenneth D. Merry
I have a new set of SMR patches available.  See below for the full
explanation.

The primary change here is that I have added SMR support to the ada(4)
driver.  I spent some time considering whether to try to make the da(4) and
ada(4) probe infrastructure somewhat common, but in the end concluded it
would be too involved with not enough code reduction (if any) in the end.

So, although the ideas are similar, the probe logic is separate.

Note that NCQ support for SMR commands (Report Zones, Reset Write Pointer,
etc.) for SATA protocol shingled drives isn't active.  For both the da(4)
and ada(4) driver this is for lack of a way to plumb the ATA Auxiliary
register down to the drive.

In the ada(4) case, we need to add the register to struct ccb_ataio and
add support in one or more of the underlying SATA drivers, e.g. ahci(4).

In the da(4) case, it will require an update of the T-10 SAT spec to
provide a way to pass the Auxiliary register down via the SCSI ATA
PASS-THROUGH command, and then a subsquent update of the SAT layer in
various vendors' SAS controller firmware.  At that point, there may be 
an official mapping of the SCSI ZBC commands to the ATA ZAC commands, and
we may be able to just issue the SCSI version of the commands instead of
composing ATA commands in the da(4) driver.  (We'll still need to keep the
ATA passthrough version for a while at least to support controllers that
don't have the updated translation code.)

FreeBSD/head as of SVN revision 294105:

https://people.freebsd.org/~ken/cam_smr.head.20160118.1.txt

FreeBSD stable/10 as of SVN revision 294100:

https://people.freebsd.org/~ken/cam_smr.stable10.20160118.1.txt

Testing and comments are welcome.

Ken

On Wed, Nov 18, 2015 at 12:13:09 -0500, Kenneth D. Merry wrote:
> 
> I have work in progress patches to add SMR (Shingled Magnetic Recording)
> support to CAM and GEOM here:
> 
> FreeBSD/head as of SVN revision 290997:
> 
> https://people.freebsd.org/~ken/cam_smr.head.20151117.1.txt
> 
> FreeBSD stable/10 as of SVN revision 290995:
> 
> https://people.freebsd.org/~ken/cam_smr.stable10.20151117.1.txt
> 
> This includes support for Host Managed, Host Aware and Drive Managed SMR
> drives that are either SCSI (ZBC) or ATA (ZAC) attached via a SAS
> controller.  This does not include support for SMR ATA drives attched via
> an ATA controller.  Also, I have not yet figured out how to properly detect
> a Host Managed ATA drive, so this code won't do that.
> 
> The big drive vendors are moving to SMR for at least some of their drives.
> The primary challenge with SMR is that it requires writing a relatively
> large zone sequentially starting at the beginning of the zone.  The usual
> zone size is 256MB.  It is conceptually almost like having a 256MB sector
> size.
> 
> We (Spectra Logic) are working on ZFS changes that will use this CAM and
> GEOM infrastructure to make ZFS play well with SMR drives.  Those changes
> aren't yet done.
> 
> The patches linked above include:
>  o A new 'camcontrol zone' command that allows displaying and managing
>drive zones via SCSI/ATA passthrough.
>  o A new zonectl(8) utility that uses the new DIOCZONECMD ioctl to display
>and manage zones via the da(4) (and later ada(4)) driver.
>  o Changes to diskinfo -v to display the zone mode of a drive.
>  o A new disk zone API, sys/sys/disk_zone.h.
>  o A new bio type, BIO_ZONE, and modifications to GEOM to support it.  This
>new bio will allow filesystems to query zone support in a drive and
>manage zoned drives.
>  o Extensive modifications to the da(4) driver to handle probing SCSI and
>SATA behind SAS SMR drives.
>  o Additional CAM CDB building functions for zone commands.
> 
> The current issues that need to be addressed are:
>  o The da(4) driver now has 6 additional probe states, 5 of which are
>needed for probing ATA drives behind SAS controllers.  I have not yet
>added support for BIO_ZONE bios to ada(4), but it will be very similar
>to the da(4) driver version.  The ATA probe code needs to be pulled
>out of the da(4) driver and changed into a form that will allow it to
>work for either the ada(4) or da(4) driver.  Otherwise we'll have a fair
>amount of code duplication between the two drivers.
> 
>  o There is a reasonable amount of code duplication between 'camcontrol zone'
>and zonectl(8).  This was done for speed / expediency's sake, but it may
>be possible to make more of the code common there.
> 
>  o In order to add the new BIO_ZONE bio command, I had to change the bio_cmd
>field in struct bio from a uint8_t to a uint16_t.  This will cause
>binary compatibility problems with any 3rd party loadable modules.
>Advice on how to handle this would be welcome.
> 
>  o In the process of developing these changes, I discovered that the
>dxfer_len paramter for scsi_ata_pass_16() was too small (uint16_t, and
>it needed to be uint32_t).  I increased it, but that will potentially
>cause 

Re: CAM Shingled Disk support patches available

2015-11-20 Thread Don Lewis
On 19 Nov, Kenneth D. Merry wrote:
> On Thu, Nov 19, 2015 at 12:48:41 -0600, Matthew D. Fuller wrote:
>> On Wed, Nov 18, 2015 at 12:13:09PM -0500 I heard the voice of
>> Kenneth D. Merry, and lo! it spake thus:
>> > 
>> > Testing and comments are welcome.
>> 
>> GELI does explicit handling of each BIO type, so will need to be
>> updated to pass it through (possibly in the form of inverting the
>> default handling?) or it'll just EOPNOTSUPP it, whether the underlying
>> layer does or not.  I wouldn't be surprised if there were other geom
>> layers that did similar things.
>> 
>> Not meant to be read as some kind of "you need to"; just a comment on
>> a possible [lack of] impact.
> 
> You're correct.  For GEOM classes like GELI that don't change the layout on
> disk, passing the BIO_ZONE bio through would be the right thing to do.
> 
> For those that change the layout (i.e. the lba you write on the virtual
> disk doesn't match what goes down to the physical disk), like graid or

partitioning or

> gstripe, I think all we really need to do is just make sure they return
> EOPNOTSUPP.  If someone wants to modify that code to handle shingled disks,
> they can certainly do that.
> 
> Ken

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: CAM Shingled Disk support patches available

2015-11-19 Thread Kenneth D. Merry
On Thu, Nov 19, 2015 at 12:48:41 -0600, Matthew D. Fuller wrote:
> On Wed, Nov 18, 2015 at 12:13:09PM -0500 I heard the voice of
> Kenneth D. Merry, and lo! it spake thus:
> > 
> > Testing and comments are welcome.
> 
> GELI does explicit handling of each BIO type, so will need to be
> updated to pass it through (possibly in the form of inverting the
> default handling?) or it'll just EOPNOTSUPP it, whether the underlying
> layer does or not.  I wouldn't be surprised if there were other geom
> layers that did similar things.
> 
> Not meant to be read as some kind of "you need to"; just a comment on
> a possible [lack of] impact.

You're correct.  For GEOM classes like GELI that don't change the layout on
disk, passing the BIO_ZONE bio through would be the right thing to do.

For those that change the layout (i.e. the lba you write on the virtual
disk doesn't match what goes down to the physical disk), like graid or
gstripe, I think all we really need to do is just make sure they return
EOPNOTSUPP.  If someone wants to modify that code to handle shingled disks,
they can certainly do that.

Ken
-- 
Kenneth Merry
k...@freebsd.org
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: CAM Shingled Disk support patches available

2015-11-19 Thread Matthew D. Fuller
On Wed, Nov 18, 2015 at 12:13:09PM -0500 I heard the voice of
Kenneth D. Merry, and lo! it spake thus:
> 
> Testing and comments are welcome.

GELI does explicit handling of each BIO type, so will need to be
updated to pass it through (possibly in the form of inverting the
default handling?) or it'll just EOPNOTSUPP it, whether the underlying
layer does or not.  I wouldn't be surprised if there were other geom
layers that did similar things.

Not meant to be read as some kind of "you need to"; just a comment on
a possible [lack of] impact.


-- 
Matthew Fuller (MF4839)   |  fulle...@over-yonder.net
Systems/Network Administrator |  http://www.over-yonder.net/~fullermd/
   On the Internet, nobody can hear you scream.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


CAM Shingled Disk support patches available

2015-11-18 Thread Kenneth D. Merry

I have work in progress patches to add SMR (Shingled Magnetic Recording)
support to CAM and GEOM here:

FreeBSD/head as of SVN revision 290997:

https://people.freebsd.org/~ken/cam_smr.head.20151117.1.txt

FreeBSD stable/10 as of SVN revision 290995:

https://people.freebsd.org/~ken/cam_smr.stable10.20151117.1.txt

This includes support for Host Managed, Host Aware and Drive Managed SMR
drives that are either SCSI (ZBC) or ATA (ZAC) attached via a SAS
controller.  This does not include support for SMR ATA drives attched via
an ATA controller.  Also, I have not yet figured out how to properly detect
a Host Managed ATA drive, so this code won't do that.

The big drive vendors are moving to SMR for at least some of their drives.
The primary challenge with SMR is that it requires writing a relatively
large zone sequentially starting at the beginning of the zone.  The usual
zone size is 256MB.  It is conceptually almost like having a 256MB sector
size.

We (Spectra Logic) are working on ZFS changes that will use this CAM and
GEOM infrastructure to make ZFS play well with SMR drives.  Those changes
aren't yet done.

The patches linked above include:
 o A new 'camcontrol zone' command that allows displaying and managing
   drive zones via SCSI/ATA passthrough.
 o A new zonectl(8) utility that uses the new DIOCZONECMD ioctl to display
   and manage zones via the da(4) (and later ada(4)) driver.
 o Changes to diskinfo -v to display the zone mode of a drive.
 o A new disk zone API, sys/sys/disk_zone.h.
 o A new bio type, BIO_ZONE, and modifications to GEOM to support it.  This
   new bio will allow filesystems to query zone support in a drive and
   manage zoned drives.
 o Extensive modifications to the da(4) driver to handle probing SCSI and
   SATA behind SAS SMR drives.
 o Additional CAM CDB building functions for zone commands.

The current issues that need to be addressed are:
 o The da(4) driver now has 6 additional probe states, 5 of which are
   needed for probing ATA drives behind SAS controllers.  I have not yet
   added support for BIO_ZONE bios to ada(4), but it will be very similar
   to the da(4) driver version.  The ATA probe code needs to be pulled
   out of the da(4) driver and changed into a form that will allow it to
   work for either the ada(4) or da(4) driver.  Otherwise we'll have a fair
   amount of code duplication between the two drivers.

 o There is a reasonable amount of code duplication between 'camcontrol zone'
   and zonectl(8).  This was done for speed / expediency's sake, but it may
   be possible to make more of the code common there.

 o In order to add the new BIO_ZONE bio command, I had to change the bio_cmd
   field in struct bio from a uint8_t to a uint16_t.  This will cause
   binary compatibility problems with any 3rd party loadable modules.
   Advice on how to handle this would be welcome.

 o In the process of developing these changes, I discovered that the
   dxfer_len paramter for scsi_ata_pass_16() was too small (uint16_t, and
   it needed to be uint32_t).  I increased it, but that will potentially
   cause a binary incompatibility problem with any existing applications
   that use the current API via libcam.  Advice on how to handle that
   would be welcome.

If you look through the code, you'll notice that the disk_zone.h API is
separate from the SCSI and ATA APIs.  The intent is to allow filesystems
and other consumers of the API to just talk to the disk zone API without
dealing with the SCSI and ATA specifics.  Another reason behind all of this
is that even though the SCSI ZBC and ATA ZAC specs were developed in
concert, and are intended to be functionally identical, they are still SCSI
and ATA.  As usual, SCSI is big endian and ATA is little endian.  So to
present a common API to the filesystem, we give all of the zone data back
in native byte order, regardless of the underlying device protocol.

Another thing to note is the extensive use of ATA passthrough in the da(4)
driver.  This is necessary because the SCSI SAT (SCSI to ATA Translation)
specification has not yet caught up with translating SCSI zone commands
(ZBC) to ATA zone commands (ZAC).  So, until the spec is updated and LSI
and other vendors update their SCSI to ATA translation layers, we'll have
to use the ATA version of the commands when talking to ATA drives via SAS
controllers.

I have only tested the code so far with Seagate SATA Drive Managed and Host
Aware drives.  I would appreciate testing with any drives.  (And testing to
make sure that the patches don't cause problems with existing hardware.)
Right now, all you can really do is manage the zones manually using
camcontrol(8) or zonectl(8).  Automatic management will come with the ZFS
changes.  (Or changes to other filesysems if people want to do it.)

If you have a SATA Host Aware drive, in theory camcontrol(8) should allow
you to manage the drive if you have it attached to a SATA controller.

Here is an example of some of the commands.

Get ge