RE: Adaptec SAS integration notes

RE: Adaptec SAS integration notes

[James.Smart]

Luben,

> > * Adaptec SAS abuses Scsi_Host_Template.  The host template 
> is a set of
> > ops for various layers, and it is fine as an interface.  No need to
> > avoid it.
> 
> Neither the Host Adapter, nor the FW, implements anything which has
> to do with the scsi host template.  What the Host Adapter 
> (the hardware) and
> the FW implement is access to the transport -- that' all.
> 
> So in effect the LLDD is _access to the (SAS) transport_. Cf. 
> the USB Storage
> code and SBP code.
> 
> Putting the host template back into the LLDD is a step 
> _backwards_, i.e.
> it's just a workaround, not a step forward.

This statement absolutely amazes me....

Your first comment "Host Adapter, nor the FW ..." is true of every
adapter.

The job of a Device Driver is to map between the interface presented
by the Host Adapter/FW and the interfaces of the host os. This is true
of whatever the technology is - SCSI, Ethernet, rs232, whatever.

As the current design of the scsi subsystem has SCSI-hba's attaching
via the host template - of course the LLDD will contain the host
template.

So - if you want to change this there are only 2 options:
A) Change the scsi subsystem LLDD interface
B) Work within the existing scsi subsystem hba interface

As this list has repeatedly stated:

If you choose to do A :
  As you are the one motivated for change, you better be able to 
    back it up by effort (aka the coding).

  The resulting work had better:
    Support the midlayer and above to the same extent that
      the existing LLDD interface does.
    Be extensible to other LLDDs.
      Minimally this means at least 2 LLDDs.
      In the best world, this means any LLDD, supporting any
        type of transport.
      And what better way to prove extensible than by providing
        an example!  (a 2nd LLDD)

  As this is "new" and unproven, expect this work to evolve 
    outside of the upstream kernel (in a parallel git tree).
    It integrates upstream only when it is generally accepted by
    the community that it meets the requirements of midlayer 
    support and extensibility.

If you choose B :
  You are free to partition the implementation of your driver
    in any way you want, which can include an upper "layer" that
    contains the host template, but no hardware/fw knowledge,
    leaving that for lower "layer".

  And if other drivers can reuse those layers - great.
  
  Two thoughts to remember though:
    This really is a driver implementation choice, not a mandated
      "Specification" choice.  It may or may not be, for various
      reasons, be based on an example in an external specification.

    Every layer in the driver makes an expectation on what can
      and cannot be done at that level. It is truth that
      different adapters will have hardware/FW interfaces with
      different capabilities. Not all adapters may align well with
      all layers. Thus, the best re-use scenario is one
      in which the layers are selectable, not all mandated.
    
    For it to become part of the upstream kernel, it must meet a
      "style" that is acceptable to the community, as you will not
      be the only one that will attempt to support the driver.


> Before we start messing with what you suggest above, SCSI Core has a
> _long_ way to go.
> 
> (BTW, we're already doing this.)

I assume from this statement, you are creating an implementation for A.


> You cannot care about "remote" ports -- some types of device have _no_
> port layer, as well you only care about the port "half" on your end.
> SAS is quite clear on that.

Not quite true - you actually care about the other "half" as well. In other
words, is it a SSP Target, etc.  If you start interacting (sending i/o)
with that other half - of course you care about it.  There are functional
levels that will care about phy's, and other functional levels that care
about ports.


> > * duplicates HCIL mapping code
> 
> True.  I've been meaning to remove that somehow, but calling
> scsi_scan_target() and passing HCIL to _it_ is also ugly.
> Ideally I'd like to see struct scsi_domain_device { ... }; in place.
> My comments in sas_discover.c and sas_scsi_host.c are pretty 
> clear on that.

Agreed. But this is another example of chosing option A or option B.


> Yeah, I agree with you: the political BS is just overflowing 
> and hitting
> the fan all the time.

IMHO - if you're going to ask for "specific" answers, replies, etc,
the best way to obtain that is to lead by example....  These words
provide no value.

> A true SAM layer would be *much* _smaller_ and would have a more
> straightforward path for tasks. (as does the SAS layer)

IMHO - rarely does expectations meet reality when you go to truly
make it work. Consider CAM and CAM-3, both are SAM-based scsi
core implementations. [granted, it all depends on where you set
your boundaries]

> > * Using scsi_scan_target() would allow us to use the normal LUN scan
> 
> Again not my code.  Normal LUN "scan" is done through REPORT LUNS
> as shown in sas_discover.c as I posted both functions to this list
> in a recent email.

FYI - in looking at SAS-1.1, I see nothing that describes LUN scanning.
What I do see are things that state "may be" a SCSI Domain or SCSI
Device - and use a general "see SAM-3" as a reference. No where
do I see the spec stating that a SCSI Device "shall" be a SCSI Device
as defined by SAM-3.

Example: please look at the definition of "SCSI Target Device" and
see if that definition could not apply to a SAM-1, or even a SCSI-2
device.

It should be evident what I'm highlighting. The spec is not as literal
as your convictions. It may not be appropriate to base things on the
literal assumptions.

> You don't see the _big_ picture: ...
> ...
> "Helper libs" makes the code ugly and spaghetti like.
> ...
> Look at the big picture and how the layers interact with each other.
> You need an _overall_ picture.

Initially, I wanted to reitterate the "lead by example" statement.
But quite frankly, the language used repeatedly in these threads with
people that do not have the same opinion as you is repulsive. In this
case it's a bit subtle, in others it has not been.

Jeff is a very smart individual, as are you, and many others on this list.
Please try to treat everyone with dignity and respect. There are many
many ways to phrase things.

-- james s

Re: Adaptec SAS integration notes

[Stefan Richter]

James.Smart@Emulex.Com wrote:
> Luben,
[Jeff Garzik wrote]
>>> * Adaptec SAS abuses Scsi_Host_Template.  The host template 
>>> is a set of ops for various layers, and it is fine as an
>>> interface.  No need to avoid it.

The scsi_host_template is a set of ops for (and interface between) 
exactly two layers. What is the abuse?

>>Neither the Host Adapter, nor the FW, implements anything which has
>>to do with the scsi host template.  What the Host Adapter 
>>(the hardware) and
>>the FW implement is access to the transport -- that' all.
>>
>>So in effect the LLDD is _access to the (SAS) transport_. Cf. 
>>the USB Storage code and SBP code.
>>
>>Putting the host template back into the LLDD is a step 
>>_backwards_, i.e. it's just a workaround, not a step forward.
> 
> This statement absolutely amazes me....
> 
> Your first comment "Host Adapter, nor the FW ..." is true of every
> adapter.
> 
> The job of a Device Driver is to map between the interface presented
> by the Host Adapter/FW and the interfaces of the host os. This is true
> of whatever the technology is - SCSI, Ethernet, rs232, whatever.
> 
> As the current design of the scsi subsystem has SCSI-hba's attaching
> via the host template - of course the LLDD will contain the host
> template.

The current SCSI subsystem has "low level drivers" attaching via the 
scsi_host_template ("low level" from the perspective of the SCSI 
subsystem). The SCSI low level drivers
     (1)  provide access to a transport
and either
     (2a) drive interconnect hardware
or
     (2b) rely on another driver or entire subsystem which provides
          access to the interconnect.
In addition, it may
     (3)  expose means to manage the transport and/or interconnect
          infrastructure.

The scsi_host_template only fulfills a role between the SCSI subsystem's 
core driver and function (1) of the SCSI low level driver. As far as I 
can tell, the scsi_host_template is irrelevant to any interface used for 
(2b).

In http://marc.theaimsgroup.com/?l=linux-scsi&m=112629423714248 , Luben 
used the following terms:
| The infrastructure is broken into
| 	* SAS LLDD,
| 	* SAS Layer.

The SAS LLDD is not the SCSI low level driver. The SAS Layer is the SCSI 
low level driver.

Similarly, usbcore and ieee1394 (together with [eou]hci_hcd, ohci1394, 
pcilynx) are not SCSI low level drivers. Usb-storage and sbp2 are SCSI 
low level drivers.

Whenever "LLDDs" are discussed, it should be clarified whether SCSI 
LLDDs (like the SAS Layer) or other LLDDs (like the SAS LLDD) are meant, 
if not apparent from context.

> - of course the LLDD will contain the host template.

Yes, a SCSI LLDD does contain it. A driver of interconnect hardware does 
not necessarily contain it.
-- 
Stefan Richter
-=====-=-=-= =-=- -====
http://arcgraph.de/sr/

Re: Adaptec SAS integration notes

[Luben Tuikov]

On 10/15/05 10:21, James.Smart@Emulex.Com wrote:
> Initially, I wanted to reitterate the "lead by example" statement.
> But quite frankly, the language used repeatedly in these threads with
> people that do not have the same opinion as you is repulsive. In this
> case it's a bit subtle, in others it has not been.
> 
> Jeff is a very smart individual, as are you, and many others on this list.
> Please try to treat everyone with dignity and respect. There are many
> many ways to phrase things.

Ok, sorry, you're right.

	Luben

Re: Adaptec SAS integration notes

[Luben Tuikov]

On 10/15/05 10:21, James.Smart@Emulex.Com wrote:
>>>* Adaptec SAS abuses Scsi_Host_Template.  The host template 
>>
>>is a set of
>>
>>>ops for various layers, and it is fine as an interface.  No need to
>>>avoid it.
>>
>>Neither the Host Adapter, nor the FW, implements anything which has
>>to do with the scsi host template.  What the Host Adapter 
>>(the hardware) and
>>the FW implement is access to the transport -- that' all.
>>
>>So in effect the LLDD is _access to the (SAS) transport_. Cf. 
>>the USB Storage
>>code and SBP code.
>>
>>Putting the host template back into the LLDD is a step 
>>_backwards_, i.e.
>>it's just a workaround, not a step forward.
> 
> 
> This statement absolutely amazes me....
> 
> Your first comment "Host Adapter, nor the FW ..." is true of every
> adapter.
> 
> The job of a Device Driver is to map between the interface presented
> by the Host Adapter/FW and the interfaces of the host os. This is true
> of whatever the technology is - SCSI, Ethernet, rs232, whatever.

I understand that a lot of vendors have legacy drivers which they'll
try to reuse for SAS.  This is fine -- they'll fit in as they've
always had.

But for newer hardware, you cannot muzzle it up as has been done,
until now.  We need to use its potential fully, as is shown in the
SAS Transport Layer code: domain discovery, domain presentation,
addressablity, etc.

USB Storage, SBP, etc. transport layers have shown the way to follow.
The reason for this is that the Storage Protocol and the interconnect
can be completely non-related: e.g. USB Storage.

> As the current design of the scsi subsystem has SCSI-hba's attaching
> via the host template - of course the LLDD will contain the host
> template.

Other members of the community have addressed this too and shown
that it is not necessary for this to be the case.

Separating those layers: SCSI Core's host template and the
interconnect driver gives you, I've said this before,
_layer intersection_.  This is most apparent for USB, as
well as for SAS, whereby you can communicate with an expander
accessing the interconnect driver (layer), or you can communicate
with a SCSI device/LU at SCSI Core level (layer).

> So - if you want to change this there are only 2 options:
> A) Change the scsi subsystem LLDD interface
> B) Work within the existing scsi subsystem hba interface

Yes, I understand the need to support legacy.  Although
A) would give some LLDDs heart attack and others would be
impossible to convert.
B) gives us the same old infrastructure, which to current
technology is incomplete.

An option C would be to create scsi_domain_device { ... };
and to start using this and its interface so that newer
Layers and LLDD are smaller and more straightforward.

> As this list has repeatedly stated:
> 
> If you choose to do A :
>   As you are the one motivated for change, you better be able to 
>     back it up by effort (aka the coding).

I don't mind coding.  The problem is that it would not necessarily
be accepted into the kernel.

adp94xx didn't get accepted because Discovery was in the LLDD,
and the community wanted it common.  It is common now, but in
due time MPT-like drivers came out with SAS, which has different
layering, and all of a sudden there "Discovery common" is no longer
a criterium for kernel acceptance.

>   The resulting work had better:
>     Support the midlayer and above to the same extent that
>       the existing LLDD interface does.

So then why do you need the "motivated change"?

>     Be extensible to other LLDDs.
>       Minimally this means at least 2 LLDDs.
>       In the best world, this means any LLDD, supporting any
>         type of transport.
>       And what better way to prove extensible than by providing
>         an example!  (a 2nd LLDD)

USB/SBP/SAS are layers.  Their LLDDs are the interconnect drivers.
Does this satisfy?

If Jeff starts work on BCM8603, that interconnect LLDD can also
use the SAS Layer.

USB as SBP as SAS is just a transport/interconnect -- there is
nothing about LUs in them or about host templates.

>   As this is "new" and unproven, expect this work to evolve 

"unproven" -- I see you've already made up your mind.
"expect this to work" -- you can find a working
driver here: http://linux.adaptec.com/sas/ .

>     outside of the upstream kernel (in a parallel git tree).
>     It integrates upstream only when it is generally accepted by
>     the community that it meets the requirements of midlayer 
>     support and extensibility.

IOW, nothing constructive.

BTW, Christoph and Jeff have mentioned that the code is good
and easy to follow, etc.

> If you choose B :
>   You are free to partition the implementation of your driver
>     in any way you want, which can include an upper "layer" that
>     contains the host template, but no hardware/fw knowledge,
>     leaving that for lower "layer".

Pure hw knowlege can be exported in a hw only struct.
Similarly how this is done by the struct hw_prof { ... };
which is embedded into struct asd_ha_struct { ... };
as is shown in the aic94xx LLDD.

Similarly, hw only knowlege can be exported to the
upper layer in struct hw_profile { ... }; (non-existing)
to be embedded into struct sas_ha_struct { ... } (existing).

The reason I didn't include it is because our controller
doesn't quite have hw limitations, as the common sg limits,
or DMA limits, etc.

>   And if other drivers can reuse those layers - great.

Yes they can.

As history shows, implementations start with a layer
and one driver.  As time goes by, more and more interconnect
LLDDs would be written to use the interface, domain discovery
etc.  Like say the BCM8603 chip.

>   Two thoughts to remember though:
>     This really is a driver implementation choice, not a mandated
>       "Specification" choice.  It may or may not be, for various
>       reasons, be based on an example in an external specification.
> 
>     Every layer in the driver makes an expectation on what can
>       and cannot be done at that level. It is truth that
>       different adapters will have hardware/FW interfaces with
>       different capabilities. Not all adapters may align well with
>       all layers. Thus, the best re-use scenario is one
>       in which the layers are selectable, not all mandated.

I completely agree with selectable layers.   As you can see
I do _not_ try to push the SAS Transport Layer on other SAS
implementations, e.g. MPT.  One of the reasons is the difference
in technology and layering and the other is the freedom you get
with open source.

>     For it to become part of the upstream kernel, it must meet a
>       "style" that is acceptable to the community, as you will not
>       be the only one that will attempt to support the driver.

Such stipulation has to be more constructive and specific.  For
example, when adp94xx was posted, the community wanted common
tasks to go to a common place.  SAS Transport layer + aic94xx
does this, but the community "style" has changed over the course
of the 8 months, so we have this situation now.

As Christoph and Jeff have already mentioned, the code is clean,
straightforward, easy to follow.

The best bet is for you to read the code itself.

Just think about this this: All of the SAS Layer exports _only two_
functions into the kernel -- only TWO.  All the while it implements
domain discovery, port management, etc, etc.

>>Before we start messing with what you suggest above, SCSI Core has a
>>_long_ way to go.
>>
>>(BTW, we're already doing this.)
> 
> I assume from this statement, you are creating an implementation for A.

No.  Jeff was mentioning something about addressability of HAs, I guess
from Doug's mentioning of OBDs.  Either way none of those are related.

What I meant to say is that HA are addressable out on the domain, say
if you want to be a target, you are addressable and initators on the
domain can address you.

Internally (in the kernel) HA are addressable by the devices they
give to the kernel, not by an opaque token.  That is, read from
device X, and the kernel knows how to do it.

A SAS HA is not a network device after all.

> Not quite true - you actually care about the other "half" as well. In other
> words, is it a SSP Target, etc.  If you start interacting (sending i/o)
> with that other half - of course you care about it.  There are functional
> levels that will care about phy's, and other functional levels that care
> about ports.

James you can access devices with which you *do not* form a port.
Think about that.

> IMHO - if you're going to ask for "specific" answers, replies, etc,
> the best way to obtain that is to lead by example....  These words
> provide no value.

Indeed they do not provide any value.

Jeff's email didn't even specify which SAS code was being addressed,
in his bulletpoints.  Was it the "transport attributes" or was it the
SAS Transport layer -- in this respect it was extremely vague
email.

>>A true SAM layer would be *much* _smaller_ and would have a more
>>straightforward path for tasks. (as does the SAS layer)
> 
> IMHO - rarely does expectations meet reality when you go to truly
> make it work. Consider CAM and CAM-3, both are SAM-based scsi

Have you taken a look at the SAS Transport Layer code yet?

> core implementations. [granted, it all depends on where you set
> your boundaries]

I agree with this statement about where one should set their
boundaries.  

Leonardo Da Vinci said that they should be set slightly higher
than one thinks attainable.

> FYI - in looking at SAS-1.1, I see nothing that describes LUN scanning.

Indeed -- it is completely not the job of SAS (or USB or SBP for that
matter) to specify LUN scanning -- this is SAM/SPC issue.

I never claimed that SAS specifies how to do it.  The reason I do it
in sas_discover.c is so that I can register LUs with SCSI Core,
since SCSI Core has no concept of SCSI domain devices as
is shown in my comment in sas_discover.c:

/**
 * sas_do_lu_discovery -- Discover LUs of a SCSI device
 * @dev: pointer to a domain device of interest
 *
 * Discover logical units present in the SCSI device.  I'd like this
 * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
 * device with a SCSI Target port".  A SCSI device with a SCSI Target
 * port is a device which the _transport_ found, but other than that,
 * the transport has little or _no_ knowledge about the device.
 * Ideally, a LLDD would register a "SCSI device with a SCSI Target
 * port" with SCSI Core and then SCSI Core would do LU discovery of
 * that device.
 *
 * REPORT LUNS is mandatory.  If a device doesn't support it,
 * it is broken and you should return it.  Nevertheless, we
 * assume (optimistically) that the link hasn't been severed and
 * that maybe we can get to the device anyhow.
 */
static int sas_do_lu_discovery(struct domain_device *dev)
{
	...

> What I do see are things that state "may be" a SCSI Domain or SCSI
> Device - and use a general "see SAM-3" as a reference. No where
> do I see the spec stating that a SCSI Device "shall" be a SCSI Device
> as defined by SAM-3.

Nowhere do I claim any such things.

> Example: please look at the definition of "SCSI Target Device" and
> see if that definition could not apply to a SAM-1, or even a SCSI-2
> device.
> 
> It should be evident what I'm highlighting. The spec is not as literal
> as your convictions. It may not be appropriate to base things on the
> literal assumptions.

No assumption here.

It is all about insight: where is storage moving to and where it
is going.  Thus, what changes are needed in Linux SCSI Core in
order to make it a Storage OS of choice.

Customers want to use Linux for their storage needs.
Vendors try to provide this.  Linux needs some long-needed
advancements.  Again, such advancements are not pushed
for everyone to use -- it is at the discretion of the
other LLDD writer to decide -- freedom of open source
as I mentioned above.

	Luben
-- 
http://linux.adaptec.com/sas/

Adaptec SAS integration notes

[Jeff Garzik]

So, I've been looking through Adaptec's SAS stuff, trying to figure
out how to best match it up with where the SCSI core currently is.
Here's the issues as best I can describe them, and proposed solutions.
Pending discussion of course, I hope to have a first cut of the
solutions sometime early next week.

Problems (with both Adaptec SAS and existing kernel)
--------
* Adaptec SAS abuses Scsi_Host_Template.  The host template is a set of
ops for various layers, and it is fine as an interface.  No need to
avoid it.

* Way too much glue between scsi_host and the rest of SAS.
sas_scsi_host.c is just the beginning.

* struct domain_device ultimately means "an RPC message destination",
and I'm not convinced it is a useful abstraction.  Further, if people
choose to introduce this abstraction, the implementation of such should
be far more widespread.

* Adaptec SAS could certainly integrate a bit with scsi_transport_sas...
not sure if full integration will work:

* scsi_transport_sas's sas_rphy_add() digs a bit too deep into
discovery.  I would prefer that libsas call scsi_scan_target() as it
makes connections.

* Given how low-level SAS wide ports are, it really feels like we should
be dealing with things at the SAS port level, not SAS phy level.

* Thought should be given to HBAs as RPC targets, since they have WWNs etc.

* very poor SATA interface

* not sure if SMP really needs a full blown block driver abstraction.
Christoph did some experiments and wasn't terribly pleased with what he
saw.  I can see his point.

* duplicates HCIL mapping code

* existing LUN scan should be fine, with maybe a few tiny tweaks

* Consider the problem of SAS vs. SATA, and both Adaptec SAS and libata
fighting over who's host struct to use.


Nice stuff (with both Adaptec SAS and existing kernel)
----------
* Overall, the Adaptec SAS code is pretty clean and easy to read

* Having ->lldd_execute_task() and similar hooks match closely with
the hardware behavior, so those are nice.

* Adaptec SAS transport is cleanly separated from the rest of things

* SCSI core already pretty much equates to SAM, even if there is plenty
of HCIL dependency in various places (HCIL should get marginalized,
as all agree)


Solutions
---------
* Similar to libata drivers, scsi-host-template should be in the LLDD,
even if it is filled with mostly generic helper calls.

* I would tend to prefer calling scsi_scan_target() from SAS discovery
code, rather than sas_rhy_add()

* Using scsi_scan_target() would allow us to use the normal LUN scan

* Avoid writing a separate SMP driver for now, and see how things
shake out with future SAS+SATA hardware.

* Put HCIL mapping into top-level helper code, for sharing between FC
and SAS (hopefully!)

* use existing struct scsi_lun where feasible

* LONG TERM: I wouldn't mind a small "scsi_rpc" helper lib
which provided execute_task() hook and other task mgmt functions.
Authors could choose to use that interface -- sitting just underneath
Scsi_Host_Template -- if their hardware closely matches the pure SAM
TMF model.  If hardware doesn't closely match, using this interface
will simply lead to a lot of glue/simulation code.  This gets the
hardware a bit closer to scsi_execute_req() etc.



Overall
-------
Not a _ton_ of work to integrate as I have described, but I may be
missing some pertinent details.



Comments welcome -- this is basically where I plan to start coding.

Re: Adaptec SAS integration notes

[Jeff Garzik]

oh, and I forgot:  bundle up the generic SAS code nicely in a lib (easy)

Re: Adaptec SAS integration notes

[Luben Tuikov]

[-- Attachment #1: Type: text/plain, Size: 11839 bytes --]

--- Jeff Garzik <jgarzik@pobox.com> wrote:

> Problems (with both Adaptec SAS and existing kernel)

Jeff, you've got to be _specific_ in your specifying to _which_ group
those problems pertain to.  Saying "both Adaptec SAS and existing kernel"
is very vague.

I'll try to stay on technical matter, being _specific_ as well as
cutting and pasting code and function declarations where appropriate.

I'll be able to be even more specific when I come back.

> * Adaptec SAS abuses Scsi_Host_Template.  The host template is a set of
> ops for various layers, and it is fine as an interface.  No need to
> avoid it.

Neither the Host Adapter, nor the FW, implements anything which has
to do with the scsi host template.  What the Host Adapter (the hardware) and
the FW implement is access to the transport -- that' all.

So in effect the LLDD is _access to the (SAS) transport_. Cf. the USB Storage
code and SBP code.

Putting the host template back into the LLDD is a step _backwards_, i.e.
it's just a workaround, not a step forward.

The _reason_ the host template exists is that 10 years ago, there was
_only_ Parallel SCSI around and there was simply no need for layer
abstraction, so the host template was declared in the Parallel SCSI LLDD.

That is, SPI LLDDs from 10 years ago until now, implemented _both_
the layer _and_ the transport interface.  This (wrong) assumption has
been shattered with USB/SBP/SAS.

> * Way too much glue between scsi_host and the rest of SAS.
> sas_scsi_host.c is just the beginning.

sas_scsi_host.c is only 996 lines comments, GPL blurb and all.  And what
it implements is _essential_ -- there is no bloat.

sas_scsi_host.c has been attached at the end of this message for everyone
to see.
 
> * struct domain_device ultimately means "an RPC message destination",

Depends on your technial background.

What it actually means, as the name _also_ suggest is a device on
the domain, which is transport addressible.  See SAM chapter 4 and 5.

> and I'm not convinced it is a useful abstraction.  Further, if people
> choose to introduce this abstraction, the implementation of such should
> be far more widespread.

You must've seen latest emails between me and James B on linux-scsi
mailing list.  We both seem to agree (James correct me if I'm wrong)
that a struct scsi_domain_device { ... }; would be the proper
way to go next.  It doesn't exist yet, but it would be really easy to
create it.  It would also eliminate HCIL at the _core_.

SAM tells you what goes in struct scsi_domain_device { ... };.

Will the community be accepting patches?

> * Adaptec SAS could certainly integrate a bit with scsi_transport_sas...
> not sure if full integration will work:
> 
> * scsi_transport_sas's sas_rphy_add() digs a bit too deep into
> discovery.  I would prefer that libsas call scsi_scan_target() as it
> makes connections.

This concept was all taken from FC and "implanted" into "scsi_transport_sas".
And it is wrong.  For proper implmentation see the SAS code:
http://linux.adaptec.com/sas/

You cannot care about "remote" ports -- some types of device have _no_
port layer, as well you only care about the port "half" on your end.
SAS is quite clear on that.

> * Given how low-level SAS wide ports are, it really feels like we should
> be dealing with things at the SAS port level, not SAS phy level.

I see this is not directed at the SAS code, but at the "SAS transport
attributes" code not written by me.

http://linux.adaptec.com/sas/ clearly shows you the proper
abstraction.  E.g. see line 524 of sas_discover.c:

 res = task->dev->port->ha->lldd_execute_task(task, 1, GFP_KERNEL);

A task "belongs" to a device,
  A device is addressible from a port,
    A port "belongs"/was formed to/at a Host Adapter,
      A host adapter implmements SAM's Execute Command SCSI RPC to provide
      access to the transport.

> * Thought should be given to HBAs as RPC targets, since they have WWNs etc.

It is an interesting concept, except you should ask yourself what _services_
will they provide: SMP/SSP/STP?  Initiator/target, etc.

Before we start messing with what you suggest above, SCSI Core has a
_long_ way to go.

(BTW, we're already doing this.)

> * very poor SATA interface

Hmm, no sorry, I'm not going to accept the BS FUD, generalized
comment.

SATA interface is just STP.  There is one thing I'd like to remove/improve
from the ata task struct as shown in the SAS code, but other than that
this is how you access STP targets.

Look, I've even included comments on how to integrate the code with
a SATL (which libata is _not_, but it does implement some SATL tasks).

I know that you're pushing for libata-scsi to become SATL, but this
is hard when your devices belong to a port in an _array_.

What you need is this: any entity from anywhere in the kernel
can fill out a struct and _register_ it with SATL for SATL to do
translation and bookkeeping -- all this at the _device_ level, not
port level as in libata, since ports may not be HW accessible or
of the same transport.

In effect, SATL would export only 2 functions: satl_register_device(),
and satl_unregister_device().  The rest of the stubs are filled out
at registration time (satl_register_device()) inside the struct
being passed to SATL.  Similarly to how it is done with the SAS code:
it exports only 2 functions: sas_register_ha() and sas_unregister_ha().
All the funcitionality is filled in by the LLDD and the SAS layer at
registration time.

> * not sure if SMP really needs a full blown block driver abstraction.
> Christoph did some experiments and wasn't terribly pleased with what he
> saw.  I can see his point.

Apparently this applies to his code not mine.

This link show how to use SMP:
http://marc.theaimsgroup.com/?l=linux-scsi&m=112629509318354&w=2
(this is also xlinked from the Adaptec SAS page)

SMP is NOT block device and will NEVER BE.

It is simply a _management_ protocol at a _transport_ level.  SCSI Core
should NOT know or be aware of it.  Reasons: expanders! (what are they?)

The readers of this thread can follow how user space interfaces with
SMP targets starting at sas_expander.c, line 1758, smp_portal_write()
and smp_portal_read() -- the functions are pretty small and easy
to read and follow.  There is information in drivers/scsi/sas/README
and in the link above (from the Announcements texts to linux-kernel
and linux-scsi).

> * duplicates HCIL mapping code

True.  I've been meaning to remove that somehow, but calling
scsi_scan_target() and passing HCIL to _it_ is also ugly.
Ideally I'd like to see struct scsi_domain_device { ... }; in place.
My comments in sas_discover.c and sas_scsi_host.c are pretty clear on that.

> * existing LUN scan should be fine, with maybe a few tiny tweaks
> 
> * Consider the problem of SAS vs. SATA, and both Adaptec SAS and libata
> fighting over who's host struct to use.

Yeah, I agree with you: the political BS is just overflowing and hitting
the fan all the time.

How about a drivers/scsi/satl/satl.c which Doug can start writing
and then we can contribute?

If there is proper SATL implemented, then you _do not_ need this
"fighting over a whose host struct to use", because:

1. host struct is higher abstraction,
2. Transport could be _different_ for the SATA device: this means that
   error handling could be different.

Will the community be accepting patches?

> Nice stuff (with both Adaptec SAS and existing kernel)
> ----------
> * Overall, the Adaptec SAS code is pretty clean and easy to read

Thank you!
 
> * Having ->lldd_execute_task() and similar hooks match closely with
> the hardware behavior, so those are nice.

The hardware was designed and implmented with SAS spec in mind, which
in turn mirrors SAM.  Many reasons but most notably, it is just easier
to go this way as opposed to having the HW do it one way and then needing
a translation to SAM behaviour.

You will notice that all SAS solutions on the market go this way: it's
just easier.

> * Adaptec SAS transport is cleanly separated from the rest of things

Yes, it is.  It is a Transport _Layer_ sitting between the (SAS) Transport
interface to the interconnect and SCSI Core (SAM to be).  Designing it
this way was the whole point.

Thank you for the nice words. 
 
> * SCSI core already pretty much equates to SAM, even if there is plenty
> of HCIL dependency in various places (HCIL should get marginalized,
> as all agree)

At a concept level yes, at implementation level absolutely no.

A true SAM layer would be *much* _smaller_ and would have a more
straightforward path for tasks. (as does the SAS layer)

> Solutions
> ---------
> * Similar to libata drivers, scsi-host-template should be in the LLDD,
> even if it is filled with mostly generic helper calls.

Again, see SBP and USB storage code (as well as the SAS code): this would be
a step backwards not forwards.  A true SAS LLLD has nothing to do with
the scsi host template.  This abstraction happens at the (SAS/SBP/USB Storage)
level layer.
 
> * I would tend to prefer calling scsi_scan_target() from SAS discovery
> code, rather than sas_rhy_add()

This is apparently not my code.

The SAS transport _layer_ does complete domain discovery as stipulated in
SAS 1.1.  (2.0 is out -- I'll be updating things when I come back).

> * Using scsi_scan_target() would allow us to use the normal LUN scan

Again not my code.  Normal LUN "scan" is done through REPORT LUNS
as shown in sas_discover.c as I posted both functions to this list
in a recent email.
 
> * Avoid writing a separate SMP driver for now, and see how things
> shake out with future SAS+SATA hardware.

No such intentions.  SMP should not be in a separate driver.
For SMP you need _addressing_ and the solution to this problem
has been showin in driver/scsi/sas/README and in the announcement
emails to linux-kernel and linux-scsi here:
http://linux.adaptec.com/sas/
 
> * Put HCIL mapping into top-level helper code, for sharing between FC
> and SAS (hopefully!)

One step further: eliminate the ugly legacy SPI-centric HCIL from
SCSI Core.  Minimise and streamline SCSI Core.

> * use existing struct scsi_lun where feasible
> 
> * LONG TERM: I wouldn't mind a small "scsi_rpc" helper lib
> which provided execute_task() hook and other task mgmt functions.
> Authors could choose to use that interface -- sitting just underneath
> Scsi_Host_Template -- if their hardware closely matches the pure SAM
> TMF model.  If hardware doesn't closely match, using this interface
> will simply lead to a lot of glue/simulation code.  This gets the
> hardware a bit closer to scsi_execute_req() etc.

You don't see the _big_ picture: you need a _layer_, not "helper libs".
This is what Object Oriented Deisign is all about.  SAM is object
oriented, as is SAS as is iSCSI as is SBP, etc.

"Helper libs" makes the code ugly and spaghetti like.

A transport LLDD implments access to the _transport_: take USB, SBP and SAS --
they all do it.  They need a management transport layer as showin in
those implementations.

I know that this is how libata works: "helper libs" but you have to think
on a larger scale:

libata is two layers: SATL and HW interface -- given ata_port and how
devices are array elemets in ata_port, I'd say let Doug write
drivers/scsi/satl/satl.c (if he accepts) and let libata become
smaller, using SATL at a _device_ level, not port level.

This would make it possible for any entity to register a device
with SATL for SAT translation.  Plus such layers export only 2 functions
and the rest are in the stubs of the structures as showin in the SAS
Transport layer code.

Look at the big picture and how the layers interact with each other.
You need an _overall_ picture.

    Luben
-- 
http://linux.adaptec.com/sas/
Opinion expressed are my own, not my empoyer's or otherwise.
For inquiries write to ltuikov@yahoo.com.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 4006696493-sas_scsi_host.c --]
[-- Type: text/x-csrc; name="sas_scsi_host.c", Size: 25967 bytes --]

/*
 * Serial Attached SCSI (SAS) class SCSI Host glue.
 *
 * Copyright (C) 2005 Adaptec, Inc.  All rights reserved.
 * Copyright (C) 2005 Luben Tuikov <luben_tuikov@adaptec.com>
 *
 * This file is licensed under GPLv2.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License as
 * published by the Free Software Foundation; either version 2 of the
 * License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful, but
 * WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
 * USA
 *
 * $Id: //depot/sas-class/sas_scsi_host.c#60 $
 */

#include "sas_internal.h"
#include <scsi/sas/sas_discover.h>
#include <scsi/sas/sas_task.h>

#include <scsi/scsi_host.h>
#include <scsi/scsi_device.h>
#include <scsi/scsi_tcq.h>
#include <scsi/scsi.h>

#include <linux/err.h>
#include <linux/blkdev.h>
#include <linux/kobject.h>
#include <linux/scatterlist.h>

/* The SAM LUN structure should be _completely_ opaque to SCSI Core.
 * This is why this macro here, and not using the broken
 * scsilun_to_int().  Ideally, a SCSI LUN should be communicated in
 * its entirety, and not as an integer.  For some unknown to myself
 * reason, SCSI Core thinks that SCSI LUNs can be interpreted as
 * integers.
 */
#define SCSI_LUN(_sam_lun)   ((unsigned int)be32_to_cpu(*(__be32 *)_sam_lun))

/* ---------- SCSI Core device registration ---------- */

int  sas_register_with_scsi(struct LU *lu)
{
	int err;

	lu->map.channel = lu->parent->port->id;
	lu->map.id      = sas_reserve_free_id(lu->parent->port);
	if (lu->map.id == -ENOMEM)
		return -ENOMEM;

	err = scsi_add_device(lu->parent->port->ha->core.shost,
			      lu->map.channel, lu->map.id,
			      SCSI_LUN(lu->LUN));
	return err;
}

void sas_unregister_with_scsi(struct LU *lu)
{
	if (lu->uldd_dev) {
		struct scsi_device *scsi_dev = lu->uldd_dev;
		scsi_remove_device(scsi_dev);
	}
}

/* ---------- SCSI Host glue ---------- */

#define TO_SAS_TASK(_scsi_cmd)  ((void *)(_scsi_cmd)->host_scribble)
#define ASSIGN_SAS_TASK(_sc, _t) do { (_sc)->host_scribble = (void *) _t; } while (0)

static void sas_scsi_task_done(struct sas_task *task)
{
	struct task_status_struct *ts = &task->task_status;
	struct scsi_cmnd *sc = task->uldd_task;
	unsigned ts_flags = task->task_state_flags;
	int hs = 0, stat = 0;

	if (unlikely(!sc)) {
		SAS_DPRINTK("task_done called with non existing SCSI cmnd!\n");
		list_del_init(&task->list);
		sas_free_task(task);
		return;
	}

	if (ts->resp == SAS_TASK_UNDELIVERED) {
		/* transport error */
		hs = DID_NO_CONNECT;
	} else { /* ts->resp == SAS_TASK_COMPLETE */
		/* task delivered, what happened afterwards? */
		switch (ts->stat) {
		case SAS_DEV_NO_RESPONSE:
		case SAS_INTERRUPTED:
		case SAS_PHY_DOWN:
		case SAS_NAK_R_ERR:
		case SAS_OPEN_TO:
			hs = DID_NO_CONNECT;
			break;
		case SAS_DATA_UNDERRUN:
			sc->resid = ts->residual;
			if (sc->request_bufflen - sc->resid < sc->underflow)
				hs = DID_ERROR;
			break;
		case SAS_DATA_OVERRUN:
			hs = DID_ERROR;
			break;
		case SAS_QUEUE_FULL:
			hs = DID_SOFT_ERROR; /* retry */
			break;
		case SAS_DEVICE_UNKNOWN:
			hs = DID_BAD_TARGET;
			break;
		case SAS_SG_ERR:
			hs = DID_PARITY;
			break;
		case SAS_OPEN_REJECT:
			if (ts->open_rej_reason == SAS_OREJ_RSVD_RETRY)
				hs = DID_SOFT_ERROR; /* retry */
			else
				hs = DID_ERROR;
			break;
		case SAS_PROTO_RESPONSE:
			SAS_DPRINTK("LLDD:%s sent SAS_PROTO_RESP for an SSP "
				    "task; please report this\n",
				    task->dev->port->ha->sas_ha_name);
			break;
		case SAS_ABORTED_TASK:
			hs = DID_ABORT;
			break;
		case SAM_CHECK_COND:
			memcpy(sc->sense_buffer, ts->buf,
			       max(SCSI_SENSE_BUFFERSIZE, ts->buf_valid_size));
			stat = SAM_CHECK_COND;
			break;
		default:
			stat = ts->stat;
			break;
		}
	}
	ASSIGN_SAS_TASK(sc, NULL);
	sc->result = (hs << 16) | stat;
	list_del_init(&task->list);
	sas_free_task(task);
	/* This is very ugly but this is how SCSI Core works. */
	if (ts_flags & SAS_TASK_STATE_ABORTED)
		scsi_finish_command(sc);
	else
		sc->scsi_done(sc);
}

static inline enum task_attribute sas_scsi_get_task_attr(struct scsi_cmnd *cmd)
{
	enum task_attribute ta = TASK_ATTR_SIMPLE;
	if (cmd->request && blk_rq_tagged(cmd->request)) {
		if (cmd->device->ordered_tags &&
		    (cmd->request->flags & REQ_HARDBARRIER))
			ta = TASK_ATTR_HOQ;
	}
	return ta;
}

static inline struct sas_task *sas_create_task(struct scsi_cmnd *cmd,
					       struct LU *lu,
					       unsigned long gfp_flags)
{
	struct sas_task *task = sas_alloc_task(gfp_flags);

	if (!task)
		return NULL;

	*(u32 *)cmd->sense_buffer = 0;
	task->uldd_task = cmd;
	ASSIGN_SAS_TASK(cmd, task);

	task->dev = lu->parent;
	task->task_proto = task->dev->tproto; /* BUG_ON(!SSP) */

	task->ssp_task.retry_count = 1;
	memcpy(task->ssp_task.LUN, lu->LUN, 8);
	task->ssp_task.task_attr = sas_scsi_get_task_attr(cmd);
	memcpy(task->ssp_task.cdb, cmd->cmnd, 16);

	task->scatter = cmd->request_buffer;
	task->num_scatter = cmd->use_sg;
	task->total_xfer_len = cmd->request_bufflen;
	task->data_dir = cmd->sc_data_direction;

	task->task_done = sas_scsi_task_done;

	return task;
}

static inline int sas_queue_up(struct sas_task *task)
{
	struct sas_ha_struct *sas_ha = task->dev->port->ha;
	struct scsi_core *core = &sas_ha->core;
	unsigned long flags;
	LIST_HEAD(list);

	spin_lock_irqsave(&core->task_queue_lock, flags);
	if (sas_ha->lldd_queue_size < core->task_queue_size + 1) {
		spin_unlock_irqrestore(&core->task_queue_lock, flags);
		return -SAS_QUEUE_FULL;
	}
	list_add_tail(&task->list, &core->task_queue);
	core->task_queue_size += 1;
	spin_unlock_irqrestore(&core->task_queue_lock, flags);
	up(&core->queue_thread_sema);

	return 0;
}

/**
 * sas_queuecommand -- Enqueue a command for processing
 * @parameters: See SCSI Core documentation
 *
 * Note: XXX: Remove the host unlock/lock pair when SCSI Core can
 * call us without holding an IRQ spinlock...
 */
static int sas_queuecommand(struct scsi_cmnd *cmd,
			    void (*scsi_done)(struct scsi_cmnd *))
{
	int res = 0;
	struct LU *lu = cmd->device->hostdata;
	struct Scsi_Host *host = cmd->device->host;

	spin_unlock_irq(host->host_lock);
	if (!lu) {
		SAS_DPRINTK("scsi cmd 0x%p sent to non existing LU\n",
			    cmd);
		cmd->result = DID_BAD_TARGET << 16;
		scsi_done(cmd);
		goto out;
	} else {
		struct sas_ha_struct *sas_ha = lu->parent->port->ha;
		struct sas_task *task;

		res = -ENOMEM;
		task = sas_create_task(cmd, lu, GFP_ATOMIC);
		if (!task)
			goto out;

		cmd->scsi_done = scsi_done;
		/* Queue up, Direct Mode or Task Collector Mode. */
		if (sas_ha->lldd_max_execute_num < 2)
			res = sas_ha->lldd_execute_task(task, 1, GFP_ATOMIC);
		else
			res = sas_queue_up(task);

		/* Examine */
		if (res) {
			SAS_DPRINTK("lldd_execute_task returned: %d\n", res);
			ASSIGN_SAS_TASK(cmd, NULL);
			sas_free_task(task);
			if (res == -SAS_QUEUE_FULL) {
				cmd->result = DID_SOFT_ERROR << 16; /* retry */
				res = 0;
				scsi_done(cmd);
			}
			goto out;
		}
	}
out:
	spin_lock_irq(host->host_lock);
	return res;
}

static void sas_scsi_clear_queue_lu(struct list_head *error_q, struct LU *lu)
{
	struct scsi_cmnd *cmd, *n;

	list_for_each_entry_safe(cmd, n, error_q, eh_entry) {
		struct LU *x = cmd->device->hostdata;

		if (x == lu)
			list_del_init(&cmd->eh_entry);
	}
}

static void sas_scsi_clear_queue_I_T(struct list_head *error_q,
				     struct domain_device *dev)
{
	struct scsi_cmnd *cmd, *n;

	list_for_each_entry_safe(cmd, n, error_q, eh_entry) {
		struct LU *y = cmd->device->hostdata;
		struct domain_device *x = y->parent;

		if (x == dev)
			list_del_init(&cmd->eh_entry);
	}
}

static void sas_scsi_clear_queue_port(struct list_head *error_q,
				      struct sas_port *port)
{
	struct scsi_cmnd *cmd, *n;

	list_for_each_entry_safe(cmd, n, error_q, eh_entry) {
		struct LU *y = cmd->device->hostdata;
		struct sas_port *x = y->parent->port;

		if (x == port)
			list_del_init(&cmd->eh_entry);
	}
}

enum task_disposition {
	TASK_IS_DONE,
	TASK_IS_ABORTED,
	TASK_IS_AT_LU,
	TASK_IS_NOT_AT_LU,
};

static enum task_disposition sas_scsi_find_task(struct sas_task *task)
{
	struct sas_ha_struct *ha = task->dev->port->ha;
	unsigned long flags;
	int i, res;

	if (ha->lldd_max_execute_num > 1) {
		struct scsi_core *core = &ha->core;
		struct sas_task *t, *n;

		spin_lock_irqsave(&core->task_queue_lock, flags);
		list_for_each_entry_safe(t, n, &core->task_queue, list) {
			if (task == t) {
				list_del_init(&t->list);
				spin_unlock_irqrestore(&core->task_queue_lock,
						       flags);
				SAS_DPRINTK("%s: task 0x%p aborted from "
					    "task_queue\n",
					    __FUNCTION__, task);
				return TASK_IS_ABORTED;
			}
		}
		spin_unlock_irqrestore(&core->task_queue_lock, flags);
	}

	for (i = 0; i < 5; i++) {
		SAS_DPRINTK("%s: aborting task 0x%p\n", __FUNCTION__, task);
		res = task->dev->port->ha->lldd_abort_task(task);

		spin_lock_irqsave(&task->task_state_lock, flags);
		if (task->task_state_flags & SAS_TASK_STATE_DONE) {
			spin_unlock_irqrestore(&task->task_state_lock, flags);
			SAS_DPRINTK("%s: task 0x%p is done\n", __FUNCTION__,
				    task);
			return TASK_IS_DONE;
		}
		spin_unlock_irqrestore(&task->task_state_lock, flags);

		if (res == TMF_RESP_FUNC_COMPLETE) {
			SAS_DPRINTK("%s: task 0x%p is aborted\n",
				    __FUNCTION__, task);
			return TASK_IS_ABORTED;
		} else if (ha->lldd_query_task) {
			SAS_DPRINTK("%s: querying task 0x%p\n",
				    __FUNCTION__, task);
			res = ha->lldd_query_task(task);
			if (res == TMF_RESP_FUNC_SUCC) {
				SAS_DPRINTK("%s: task 0x%p at LU\n",
					    __FUNCTION__, task);
				return TASK_IS_AT_LU;
			} else if (res == TMF_RESP_FUNC_COMPLETE) {
				SAS_DPRINTK("%s: task 0x%p not at LU\n",
					    __FUNCTION__, task);
				return TASK_IS_NOT_AT_LU;
			}
		}
	}
	return res;
}

static int sas_recover_lu(struct domain_device *dev, struct LU *lu)
{
	struct sas_ha_struct *ha = dev->port->ha;
	int res = TMF_RESP_FUNC_FAILED;

	SAS_DPRINTK("eh: device %llx LUN %llx has the task\n",
		    SAS_ADDR(dev->sas_addr),
		    SAS_ADDR(lu->LUN));

	if (ha->lldd_abort_task_set)
		res = ha->lldd_abort_task_set(dev, lu->LUN);

	if (res == TMF_RESP_FUNC_FAILED) {
		if (ha->lldd_clear_task_set)
			res = ha->lldd_clear_task_set(dev, lu->LUN);
	}

	if (res == TMF_RESP_FUNC_FAILED) {
		if (ha->lldd_lu_reset)
			res = ha->lldd_lu_reset(dev, lu->LUN);
	}

	return res;
}

static int sas_recover_I_T(struct domain_device *dev)
{
	struct sas_ha_struct *ha = dev->port->ha;
	int res = TMF_RESP_FUNC_FAILED;

	SAS_DPRINTK("I_T nexus reset for dev %016llx\n",
		    SAS_ADDR(dev->sas_addr));

	if (ha->lldd_I_T_nexus_reset)
		res = ha->lldd_I_T_nexus_reset(dev);

	return res;
}

static int sas_scsi_recover_host(struct Scsi_Host *shost)
{
	struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
	unsigned long flags;
	LIST_HEAD(error_q);
	struct scsi_cmnd *cmd, *n;
	enum task_disposition res = TASK_IS_DONE;
	int tmf_resp;

	spin_lock_irqsave(shost->host_lock, flags);
	list_splice_init(&shost->eh_cmd_q, &error_q);
	spin_unlock_irqrestore(shost->host_lock, flags);

	SAS_DPRINTK("Enter %s\n", __FUNCTION__);

	/* All tasks on this list were marked SAS_TASK_STATE_ABORTED
	 * by sas_scsi_timed_out() callback.
	 */
Again:
	SAS_DPRINTK("going over list...\n");
	list_for_each_entry_safe(cmd, n, &error_q, eh_entry) {
		struct sas_task *task = TO_SAS_TASK(cmd);
		struct LU *lu = cmd->device->hostdata;

		SAS_DPRINTK("trying to find task 0x%p\n", task);
		list_del_init(&cmd->eh_entry);
		res = sas_scsi_find_task(task);

		cmd->eh_eflags = 0;
		shost->host_failed--;

		switch (res) {
		case TASK_IS_DONE:
			SAS_DPRINTK("%s: task 0x%p is done\n", __FUNCTION__,
				    task);
			task->task_done(task);
			continue;
		case TASK_IS_ABORTED:
			SAS_DPRINTK("%s: task 0x%p is aborted\n",
				    __FUNCTION__, task);
			task->task_done(task);
			continue;
		case TASK_IS_AT_LU:
			SAS_DPRINTK("task 0x%p is at LU: lu recover\n", task);
			tmf_resp = sas_recover_lu(task->dev, lu);
			if (tmf_resp == TMF_RESP_FUNC_COMPLETE) {
				SAS_DPRINTK("dev %016llx LU %016llx is "
					    "recovered\n",
					    SAS_ADDR(task->dev),
					    SAS_ADDR(lu->LUN));
				task->task_done(task);
				sas_scsi_clear_queue_lu(&error_q, lu);
				goto Again;
			}
			/* fallthrough */
		case TASK_IS_NOT_AT_LU:
			SAS_DPRINTK("task 0x%p is not at LU: I_T recover\n",
				    task);
			tmf_resp = sas_recover_I_T(task->dev);
			if (tmf_resp == TMF_RESP_FUNC_COMPLETE) {
				SAS_DPRINTK("I_T %016llx recovered\n",
					    SAS_ADDR(task->dev->sas_addr));
				task->task_done(task);
				sas_scsi_clear_queue_I_T(&error_q, task->dev);
				goto Again;
			}
			/* Hammer time :-) */
			if (ha->lldd_clear_nexus_port) {
				struct sas_port *port = task->dev->port;
				SAS_DPRINTK("clearing nexus for port:%d\n",
					    port->id);
				res = ha->lldd_clear_nexus_port(port);
				if (res == TMF_RESP_FUNC_COMPLETE) {
					SAS_DPRINTK("clear nexus port:%d "
						    "succeeded\n", port->id);
					task->task_done(task);
					sas_scsi_clear_queue_port(&error_q,
								  port);
					goto Again;
				}
			}
			if (ha->lldd_clear_nexus_ha) {
				SAS_DPRINTK("clear nexus ha\n");
				res = ha->lldd_clear_nexus_ha(ha);
				if (res == TMF_RESP_FUNC_COMPLETE) {
					SAS_DPRINTK("clear nexus ha "
						    "succeeded\n");
					task->task_done(task);
					goto out;
				}
			}
			/* If we are here -- this means that no amount
			 * of effort could recover from errors.  Quite
			 * possibly the HA just disappeared.
			 */
			SAS_DPRINTK("error from  device %llx, LUN %llx "
				    "couldn't be recovered in any way\n",
				    SAS_ADDR(task->dev->sas_addr),
				    SAS_ADDR(lu->LUN));

			task->task_done(task);
			goto clear_q;
		}
	}
out:
	SAS_DPRINTK("--- Exit %s\n", __FUNCTION__);
	return 0;
clear_q:
	SAS_DPRINTK("--- Exit %s -- clear_q\n", __FUNCTION__);
	list_for_each_entry_safe(cmd, n, &error_q, eh_entry) {
		struct sas_task *task = TO_SAS_TASK(cmd);
		list_del_init(&cmd->eh_entry);
		task->task_done(task);
	}
	return 0;
}

static enum scsi_eh_timer_return sas_scsi_timed_out(struct scsi_cmnd *cmd)
{
	struct sas_task *task = TO_SAS_TASK(cmd);
	unsigned long flags;

	if (!task) {
		SAS_DPRINTK("command 0x%p, task 0x%p, timed out: EH_HANDLED\n",
			    cmd, task);
		return EH_HANDLED;
	}

	spin_lock_irqsave(&task->task_state_lock, flags);
	if (task->task_state_flags & SAS_TASK_STATE_DONE) {
		spin_unlock_irqrestore(&task->task_state_lock, flags);
		SAS_DPRINTK("command 0x%p, task 0x%p, timed out: EH_HANDLED\n",
			    cmd, task);
		return EH_HANDLED;
	}
	task->task_state_flags |= SAS_TASK_STATE_ABORTED;
	spin_unlock_irqrestore(&task->task_state_lock, flags);

	SAS_DPRINTK("command 0x%p, task 0x%p, timed out: EH_NOT_HANDLED\n",
		    cmd, task);

	return EH_NOT_HANDLED;
}

/**
 * sas_slave_alloc -- configure an LU which SCSI Core wants to poke at
 * @scsi_dev: pointer to scsi device
 *
 * The kludge here is that the only token we have to go by in order to
 * identify which device SCSI Core has just found about, is channel,
 * id and lun/2.  Of course this is 1) incredibly broken and 2)
 * leftover from when SCSI Core was SPI-centric.  A solution would be
 * to pass an opaque token to scsi_add_device, which SCSI Core treats
 * as that, an opaque token, which it sets inside scsi_dev, so we can
 * find out which device SCSI Core is talking about.  That is, how
 * SCSI Core is _addressing_ the device is not the business of LLDD
 * and vice versa.  An even _better_ solution is if SCSI Core knew
 * about a "SCSI device with Target ports" so we can register only the
 * targets, and then it would do its own LU discovery...  See comment
 * in sas_do_lu_discovery().
 */
static int sas_slave_alloc(struct scsi_device *scsi_dev)
{
	struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(scsi_dev->host);
	struct sas_port *port = sas_ha->sas_port[scsi_dev->channel];
        unsigned id = scsi_dev->id;
	unsigned lun = scsi_dev->lun;

	struct domain_device *dev = NULL;
	struct LU *lu = NULL;

	scsi_dev->hostdata = NULL;

	list_for_each_entry(dev, &port->dev_list, dev_list_node) {
		if (dev->dev_type == SAS_END_DEV) {
			list_for_each_entry(lu, &dev->end_dev.LU_list, list) {
				if (lu->map.id == id &&
				    SCSI_LUN(lu->LUN) == lun) {
					scsi_dev->hostdata = lu;
					lu->uldd_dev = scsi_dev;
					goto out_loop;
				}
			}
		}
	}
out_loop:
	if (!scsi_dev->hostdata) {
		SAS_DPRINTK("sas device not found! How is this possible?\n");
		return -ENODEV;
	}
	kobject_get(&lu->lu_obj);
	return 0;
}

#define SAS_DEF_QD 32
#define SAS_MAX_QD 64

static int sas_slave_configure(struct scsi_device *scsi_dev)
{
	struct LU *lu = scsi_dev->hostdata;
	struct domain_device *dev;
	struct sas_ha_struct *sas_ha;

	if (!lu) {
		SAS_DPRINTK("slave configure and no LU?!\n");
		return -ENODEV;
	}

	dev = lu->parent;
	sas_ha = dev->port->ha;

	if (scsi_dev->inquiry_len > 7) {
		u8 bq = (scsi_dev->inquiry[6] & 0x80) ? 1 : 0;
		u8 cq = (scsi_dev->inquiry[7] & 0x02) ? 1 : 0;

		if (bq ^ cq) {
			lu->tm_type = (bq<<1) | cq;
			scsi_dev->tagged_supported = 1;
			if (cq)
				scsi_set_tag_type(scsi_dev, MSG_ORDERED_TAG);
			else
				scsi_set_tag_type(scsi_dev, MSG_SIMPLE_TAG);
			scsi_activate_tcq(scsi_dev, SAS_DEF_QD);
		} else {
			SAS_DPRINTK("device %llx, LUN %llx doesn't support "
				    "TCQ\n", SAS_ADDR(dev->sas_addr),
				    SAS_ADDR(lu->LUN));
			scsi_dev->tagged_supported = 0;
			scsi_set_tag_type(scsi_dev, 0);
			scsi_deactivate_tcq(scsi_dev, 1);
		}
	}

	if (dev->end_dev.itnl_timeout > 0)
		scsi_dev->timeout = HZ +
			msecs_to_jiffies(dev->end_dev.itnl_timeout);

	return 0;
}

static void sas_slave_destroy(struct scsi_device *scsi_dev)
{
	struct LU *lu = scsi_dev->hostdata;

	if (lu) {
		scsi_dev->hostdata = NULL;
		lu->uldd_dev = NULL;
		kobject_put(&lu->lu_obj);
	}
}

static int sas_change_queue_depth(struct scsi_device *scsi_dev, int new_depth)
{
	int res = min(new_depth, SAS_MAX_QD);

	if (scsi_dev->tagged_supported)
		scsi_adjust_queue_depth(scsi_dev, scsi_get_tag_type(scsi_dev),
					res);
	else {
		struct LU *lu = scsi_dev->hostdata;
		sas_printk("device %llx LUN %llx queue depth changed to 1\n",
			   SAS_ADDR(lu->parent->sas_addr),
			   SAS_ADDR(lu->LUN));
		scsi_adjust_queue_depth(scsi_dev, 0, 1);
		res = 1;
	}

	return res;
}

static int sas_change_queue_type(struct scsi_device *scsi_dev, int qt)
{
	struct LU *lu = scsi_dev->hostdata;

	if (!scsi_dev->tagged_supported)
		return 0;

	scsi_deactivate_tcq(scsi_dev, 1);

	switch (qt) {
	case MSG_ORDERED_TAG:
		if (lu->tm_type != TASK_MANAGEMENT_FULL)
			qt = MSG_SIMPLE_TAG;
		break;
	case MSG_SIMPLE_TAG:
	default:
		;
	}

	scsi_set_tag_type(scsi_dev, qt);
	scsi_activate_tcq(scsi_dev, scsi_dev->queue_depth);

	return qt;
}

static int sas_bios_param(struct scsi_device *scsi_dev,
			  struct block_device *bdev,
			  sector_t capacity, int *hsc)
{
	hsc[0] = 255;
	hsc[1] = 63;
	sector_div(capacity, 255*63);
	hsc[2] = capacity;

	return 0;
}

static const struct scsi_host_template sas_host_template = {
	.module = THIS_MODULE,
	/* .name is initialized */
	.name = "",
	.queuecommand = sas_queuecommand,
	.eh_strategy_handler = sas_scsi_recover_host,
	.eh_timed_out = sas_scsi_timed_out,
	.slave_alloc = sas_slave_alloc,
	.slave_configure = sas_slave_configure,
	.slave_destroy = sas_slave_destroy,
	.change_queue_depth = sas_change_queue_depth,
	.change_queue_type = sas_change_queue_type,
	.bios_param = sas_bios_param,
	/* .can_queue is initialized */
	.this_id = -1,
	.sg_tablesize = SG_ALL,
	.max_sectors = SCSI_DEFAULT_MAX_SECTORS,
	/* .cmd_per_lun is initilized to .can_queue */
	.use_clustering = ENABLE_CLUSTERING,
};

static inline void sas_init_host_template(struct sas_ha_struct *sas_ha)
{
	struct scsi_host_template *sht = sas_ha->core.sht;

	*sht = sas_host_template;

	sht->name = sas_ha->sas_ha_name;
	sht->can_queue = sas_ha->lldd_queue_size;
	sht->cmd_per_lun = sht->can_queue;
}

int sas_register_scsi_host(struct sas_ha_struct *sas_ha)
{
	int err = -ENOMEM;

	sas_ha->core.sht = kzalloc(sizeof(*sas_ha->core.sht), GFP_KERNEL);
	if (!sas_ha->core.sht)
		return -ENOMEM;

	sas_init_host_template(sas_ha);

	sas_ha->core.shost = scsi_host_alloc(sas_ha->core.sht, sizeof(void *));
	if (!sas_ha->core.shost) {
		printk(KERN_NOTICE "couldn't allocate scsi host\n");
		goto out_err;
	}
	SHOST_TO_SAS_HA(sas_ha->core.shost) = sas_ha;

	/* XXX: SCSI Core should really fix this (max vs. num of) */
	sas_ha->core.shost->max_channel = sas_ha->num_phys - 1;
	sas_ha->core.shost->max_id = ~0 - 1;
	sas_ha->core.shost->max_lun = ~0 - 1;

	sas_ha->core.shost->max_cmd_len = 16;

	err = scsi_add_host(sas_ha->core.shost, &sas_ha->pcidev->dev);
	if (err) {
		scsi_host_put(sas_ha->core.shost);
		sas_ha->core.shost = NULL;
		goto out_err;
	}
	return 0;

out_err:
	kfree(sas_ha->core.sht);
	sas_ha->core.sht = NULL;
	return err;
}

void sas_unregister_scsi_host(struct sas_ha_struct *sas_ha)
{
	scsi_remove_host(sas_ha->core.shost);
	scsi_host_put(sas_ha->core.shost);
	sas_ha->core.shost = NULL;
	kfree(sas_ha->core.sht);
	sas_ha->core.sht = NULL;
}

/* ---------- Task Collector Thread implementation ---------- */

static void sas_queue(struct sas_ha_struct *sas_ha)
{
	struct scsi_core *core = &sas_ha->core;
	unsigned long flags;
	LIST_HEAD(q);
	int can_queue;
	int res;

	spin_lock_irqsave(&core->task_queue_lock, flags);
	while (!core->queue_thread_kill &&
	       !list_empty(&core->task_queue)) {

		can_queue = sas_ha->lldd_queue_size - core->task_queue_size;
		if (can_queue >= 0) {
			can_queue = core->task_queue_size;
			list_splice_init(&core->task_queue, &q);
		} else {
			struct list_head *a, *n;

			can_queue = sas_ha->lldd_queue_size;
			list_for_each_safe(a, n, &core->task_queue) {
				list_move_tail(a, &q);
				if (--can_queue == 0)
					break;
			}
			can_queue = sas_ha->lldd_queue_size;
		}
		core->task_queue_size -= can_queue;
		spin_unlock_irqrestore(&core->task_queue_lock, flags);
		{
			struct sas_task *task = list_entry(q.next,
							   struct sas_task,
							   list);
			list_del_init(&q);
			res = sas_ha->lldd_execute_task(task, can_queue,
							GFP_KERNEL);
			if (unlikely(res))
				__list_add(&q, task->list.prev, &task->list);
		}
		spin_lock_irqsave(&core->task_queue_lock, flags);
		if (res) {
			list_splice_init(&q, &core->task_queue); /*at head*/
			core->task_queue_size += can_queue;
		}
	}
	spin_unlock_irqrestore(&core->task_queue_lock, flags);
}

static DECLARE_COMPLETION(queue_th_comp);

/**
 * sas_queue_thread -- The Task Collector thread
 * @_sas_ha: pointer to struct sas_ha
 */
static int sas_queue_thread(void *_sas_ha)
{
	struct sas_ha_struct *sas_ha = _sas_ha;
	struct scsi_core *core = &sas_ha->core;

	daemonize("sas_queue_%d", core->shost->host_no);
	current->flags |= PF_NOFREEZE;

	complete(&queue_th_comp);

	while (1) {
		down_interruptible(&core->queue_thread_sema);
		sas_queue(sas_ha);
		if (core->queue_thread_kill)
			break;
	}

	complete(&queue_th_comp);

	return 0;
}

/* ---------- SCSI Core struct attributes ---------- */

static ssize_t show_task_queue_size(struct scsi_core *core, char *page)
{
	return sprintf(page, "%d\n", core->task_queue_size);
}

struct scsi_core_attribute {
	struct attribute attr;
	ssize_t (*show)(struct scsi_core *, char *);
	ssize_t (*store)(struct scsi_core *, const char *, size_t len);
};

#define to_scsi_core(_obj) container_of((_obj), struct scsi_core, \
					scsi_core_obj)
#define to_sc_attr(_attr) container_of((_attr), struct scsi_core_attribute,\
				       attr)

static ssize_t sc_show_attr(struct kobject *kobj, struct attribute *attr,
			    char *page)
{
	ssize_t ret = 0;
	struct scsi_core *core = to_scsi_core(kobj);
	struct scsi_core_attribute *sc_attr = to_sc_attr(attr);

	if (sc_attr->show)
		ret = sc_attr->show(core, page);
	return ret;
}

static struct scsi_core_attribute sc_attrs[] = {
	__ATTR(task_queue_size, 0444, show_task_queue_size, NULL),
	__ATTR_NULL,
};

static struct attribute *sc_def_attrs[ARRAY_SIZE(sc_attrs)];

static struct sysfs_ops sc_sysfs_ops = {
	.show = sc_show_attr,
};

static struct kobj_type scsi_core_ktype = {
	.sysfs_ops = &sc_sysfs_ops,
	.default_attrs = sc_def_attrs,
};

int sas_init_queue(struct sas_ha_struct *sas_ha)
{
	int res;
	struct scsi_core *core = &sas_ha->core;

	spin_lock_init(&core->task_queue_lock);
	core->task_queue_size = 0;
	INIT_LIST_HEAD(&core->task_queue);
	init_MUTEX_LOCKED(&core->queue_thread_sema);

	res = kernel_thread(sas_queue_thread, sas_ha, 0);
	if (res >= 0) {
		int i;
		wait_for_completion(&queue_th_comp);

		for (i = 0; i < ARRAY_SIZE(sc_attrs)-1; i++)
			sc_def_attrs[i] = &sc_attrs[i].attr;
		sc_def_attrs[i] = NULL;

		core->scsi_core_obj.kset = &sas_ha->ha_kset;
		kobject_set_name(&core->scsi_core_obj, "%s", "scsi_core");
		core->scsi_core_obj.ktype = &scsi_core_ktype;
	}

	return res < 0 ? res : 0;
}

void sas_shutdown_queue(struct sas_ha_struct *sas_ha)
{
	unsigned long flags;
	struct scsi_core *core = &sas_ha->core;
	struct sas_task *task, *n;

	init_completion(&queue_th_comp);
	core->queue_thread_kill = 1;
	up(&core->queue_thread_sema);
	wait_for_completion(&queue_th_comp);

	if (!list_empty(&core->task_queue))
		SAS_DPRINTK("HA: %llx: scsi core task queue is NOT empty!?\n",
			    SAS_ADDR(sas_ha->sas_addr));

	spin_lock_irqsave(&core->task_queue_lock, flags);
	list_for_each_entry_safe(task, n, &core->task_queue, list) {
		struct scsi_cmnd *cmd = task->uldd_task;

		list_del_init(&task->list);

		ASSIGN_SAS_TASK(cmd, NULL);
		sas_free_task(task);
		cmd->result = DID_ABORT << 16;
		cmd->scsi_done(cmd);
	}
	spin_unlock_irqrestore(&core->task_queue_lock, flags);
}

Re: Adaptec SAS integration notes

[Stefan Richter]

Luben Tuikov wrote:
> --- Jeff Garzik <jgarzik@pobox.com> wrote:
>>(HCIL should get marginalized, as all agree)

"Marginalized" was not the term that I remember.

>>* Put HCIL mapping into top-level helper code, for sharing between FC
>>and SAS (hopefully!)
> 
> One step further: eliminate the ugly legacy SPI-centric HCIL from
> SCSI Core.  Minimise and streamline SCSI Core.

I agree. FC, SAS, USB, SBP et al do not need to _share_ some HCIL 
mapping helpers --- they and the core need to get rid of HCIL mapping 
entirely. Why spend energy on optimization of existing HCIL mappings?

usb-storage and sbp2 already implement the leanest possible HCIL mapping 
(1 logical unit to 1 H). There is only one way to make this even more 
leaner and, at the same time, cleaner.
-- 
Stefan Richter
-=====-=-=-= =-=- -====
http://arcgraph.de/sr/

Re: Adaptec SAS integration notes

[Douglas Gilbert]

Stefan Richter wrote:
> Luben Tuikov wrote:
> 
>> --- Jeff Garzik <jgarzik@pobox.com> wrote:
>>
>>> (HCIL should get marginalized, as all agree)
> 
> 
> "Marginalized" was not the term that I remember.
> 
>>> * Put HCIL mapping into top-level helper code, for sharing between FC
>>> and SAS (hopefully!)
>>
>>
>> One step further: eliminate the ugly legacy SPI-centric HCIL from
>> SCSI Core.  Minimise and streamline SCSI Core.
> 
> 
> I agree. FC, SAS, USB, SBP et al do not need to _share_ some HCIL
> mapping helpers --- they and the core need to get rid of HCIL mapping
> entirely. Why spend energy on optimization of existing HCIL mappings?
> 
> usb-storage and sbp2 already implement the leanest possible HCIL mapping
> (1 logical unit to 1 H).

Stefan,
Here is a counter example from a USB multicard reader:

$ lsscsi -v
[0:0:0:0]    disk    Generic  USB SD Reader    1.00  /dev/sda
  dir: /sys/bus/scsi/devices/0:0:0:0
[/sys/devices/pci0000:00/0000:00:07.2/usb1/1-1/1-1:1.0/host0/target0:0:0/0:0:0:0]
[0:0:0:1]    disk    Generic  USB CF Reader    1.01  /dev/sdb
  dir: /sys/bus/scsi/devices/0:0:0:1
[/sys/devices/pci0000:00/0000:00:07.2/usb1/1-1/1-1:1.0/host0/target0:0:0/0:0:0:1]
[0:0:0:2]    disk    Generic  USB SM Reader    1.02  /dev/sdc
  dir: /sys/bus/scsi/devices/0:0:0:2
[/sys/devices/pci0000:00/0000:00:07.2/usb1/1-1/1-1:1.0/host0/target0:0:0/0:0:0:2]
[0:0:0:3]    disk    Generic  USB MS Reader    1.03  /dev/sdd
  dir: /sys/bus/scsi/devices/0:0:0:3
[/sys/devices/pci0000:00/0000:00:07.2/usb1/1-1/1-1:1.0/host0/target0:0:0/0:0:0:3]

That is 4 logical units to 1 host. [Since I'm often
critical of USB mass storage device SCSI support, in
this case it is good to see some thought went into the
INQUIRY string namings.]

There is an argument for the SCSI subsystem to keep
at least the "l" part of the maligned <h:c:i:l> tuple
because the "l" part really is SCSI (and should be u8[8]).
SATA-II is beginning to struggle along that path as well
with port multipliers of up to 15 devices. SCSI luns
can easily cope with that.

Doug Gilbert

Re: Adaptec SAS integration notes

[Stefan Richter]

Douglas Gilbert wrote:
> Stefan Richter wrote:
>>usb-storage and sbp2 already implement the leanest possible HCIL mapping
>>(1 logical unit to 1 H).
> 
> Stefan,
> Here is a counter example from a USB multicard reader:
[...]
> That is 4 logical units to 1 host. [Since I'm often
> critical of USB mass storage device SCSI support, in
> this case it is good to see some thought went into the
> INQUIRY string namings.]

Indeed. Thanks for the correction. It is 1 target = 1 H then. BTW I 
prefer to call it "H" or "scsi_host", not "host". :-)

Sbp2 currently transforms the logical unit number into scsi core's "L" 
too but allocates a separate H for each logical unit anyway AFAICS. That 
is certainly not very logical. But on the other hand, real target names 
and logical unit numbers are not exposed directly to userspace yet (or 
is there a direct, transport-independent way?), so why bother with more 
refined mapping for now.

> There is an argument for the SCSI subsystem to keep
> at least the "l" part of the maligned <h:c:i:l> tuple
> because the "l" part really is SCSI (and should be u8[8]).
> SATA-II is beginning to struggle along that path as well
> with port multipliers of up to 15 devices. SCSI luns
> can easily cope with that.

Absolutely. There should be "logical units" (with proper LUNs) which 
belong to "targets" (with proper target names exposed to userspace).

The concepts of "host adapter" and "channel" may exist at the level of 
transport/ interconnect drivers, but they may be absent on any higher level.
-- 
Stefan Richter
-=====-=-=-= =-=- =---=
http://arcgraph.de/sr/

Re: Adaptec SAS integration notes

[Jeff Garzik]

Luben Tuikov wrote:
> --- Jeff Garzik <jgarzik@pobox.com> wrote:
> 
> 
>>Problems (with both Adaptec SAS and existing kernel)
> 
> 
> Jeff, you've got to be _specific_ in your specifying to _which_ group
> those problems pertain to.  Saying "both Adaptec SAS and existing kernel"
> is very vague.

You are supposed to know when I am referring to the upstream kernel, and 
when I am referring to your code :)  Logically, if I'm not talking about 
updates your code needs, I am talking about updates the core kernel needs.


>>* Adaptec SAS abuses Scsi_Host_Template.  The host template is a set of
>>ops for various layers, and it is fine as an interface.  No need to
>>avoid it.
> 
> 
> Neither the Host Adapter, nor the FW, implements anything which has
> to do with the scsi host template.

Wrong.  DMA boundary is a trivial counterexample.


>>* struct domain_device ultimately means "an RPC message destination",
> 
> 
> Depends on your technial background.
> 
> What it actually means, as the name _also_ suggest is a device on
> the domain, which is transport addressible.  See SAM chapter 4 and 5.

I'm discussing the topic at hand at a higher level than SAM.


>>and I'm not convinced it is a useful abstraction.  Further, if people
>>choose to introduce this abstraction, the implementation of such should
>>be far more widespread.
> 
> 
> You must've seen latest emails between me and James B on linux-scsi
> mailing list.  We both seem to agree (James correct me if I'm wrong)
> that a struct scsi_domain_device { ... }; would be the proper
> way to go next.  It doesn't exist yet, but it would be really easy to
> create it.  It would also eliminate HCIL at the _core_.
> 
> SAM tells you what goes in struct scsi_domain_device { ... };.

My overall point is that we would need to convert the device classes and 
SPI transport in order for this to happen.  And I'm still not convinced 
that scsi_domain_device as you have implemented it is a useful abstraction.

There is interface stuff in struct sas_ha_struct that really should move 
to a higher level -- its not SAS-only, and that may be the better interface.


>>* Adaptec SAS could certainly integrate a bit with scsi_transport_sas...
>>not sure if full integration will work:
>>
>>* scsi_transport_sas's sas_rphy_add() digs a bit too deep into
>>discovery.  I would prefer that libsas call scsi_scan_target() as it
>>makes connections.
> 
> 
> This concept was all taken from FC and "implanted" into "scsi_transport_sas".
> And it is wrong.  For proper implmentation see the SAS code:
> http://linux.adaptec.com/sas/

This entire thread is talking about the code at that URL.  No need to be 
self-referential.


> You cannot care about "remote" ports -- some types of device have _no_
> port layer, as well you only care about the port "half" on your end.
> SAS is quite clear on that.

Remote ports deals with the other half of the connection.  You gotta 
deal with it somewhere.


>>* very poor SATA interface
> 
> 
> Hmm, no sorry, I'm not going to accept the BS FUD, generalized
> comment.

If it doesn't interface with libata, the current SATA interface, then 
the code needs improvement.  Its a statement of fact.


> Look, I've even included comments on how to integrate the code with
> a SATL (which libata is _not_, but it does implement some SATL tasks).
> 
> I know that you're pushing for libata-scsi to become SATL, but this
> is hard when your devices belong to a port in an _array_.

I've simply stated that there will not be more than one SCSI<->ATA 
simulator in the kernel.

All implementation decisions trickle down from that overall path.


>>* not sure if SMP really needs a full blown block driver abstraction.
>>Christoph did some experiments and wasn't terribly pleased with what he
>>saw.  I can see his point.
> 
> 
> Apparently this applies to his code not mine.
> 
> This link show how to use SMP:
> http://marc.theaimsgroup.com/?l=linux-scsi&m=112629509318354&w=2
> (this is also xlinked from the Adaptec SAS page)
> 
> SMP is NOT block device and will NEVER BE.

In this context, a block device is simply a messaging queue.


>>* existing LUN scan should be fine, with maybe a few tiny tweaks
>>
>>* Consider the problem of SAS vs. SATA, and both Adaptec SAS and libata
>>fighting over who's host struct to use.
> 
> 
> Yeah, I agree with you: the political BS is just overflowing and hitting
> the fan all the time.

Sigh.  I'm talking about implementation details, here.


> How about a drivers/scsi/satl/satl.c which Doug can start writing
> and then we can contribute?

Absolutely not.  Update libata-scsi.


>>* SCSI core already pretty much equates to SAM, even if there is plenty
>>of HCIL dependency in various places (HCIL should get marginalized,
>>as all agree)
> 
> 
> At a concept level yes, at implementation level absolutely no.
> 
> A true SAM layer would be *much* _smaller_ and would have a more
> straightforward path for tasks. (as does the SAS layer)

Important concept:  struct naming and explicit layering based on 
pictures in a spec is not how we want to do development.  We want to 
-think- about the overall issues, and implement in the best manner while 
ensuring the hardware state machine isn't violated.

SAM is ultimately message passing RPC over a network.


>>Solutions
>>---------
>>* Similar to libata drivers, scsi-host-template should be in the LLDD,
>>even if it is filled with mostly generic helper calls.
> 
> 
> Again, see SBP and USB storage code (as well as the SAS code): this would be
> a step backwards not forwards.  A true SAS LLLD has nothing to do with
> the scsi host template.  This abstraction happens at the (SAS/SBP/USB Storage)
> level layer.

A SCSI domain and Linux SCSI host are already pretty close.  There's no 
need to rewrite the world, just because they don't match 100%.


>>* I would tend to prefer calling scsi_scan_target() from SAS discovery
>>code, rather than sas_rhy_add()
> 
> 
> This is apparently not my code.
> 
> The SAS transport _layer_ does complete domain discovery as stipulated in
> SAS 1.1.  (2.0 is out -- I'll be updating things when I come back).

I'm trying to move forward by talking about how your code will integrate 
with the kernel.

Pointing out "that's not my code" is completely irrelevant, and unhelpful.


>>* Using scsi_scan_target() would allow us to use the normal LUN scan
> 
> 
> Again not my code.  Normal LUN "scan" is done through REPORT LUNS
> as shown in sas_discover.c as I posted both functions to this list
> in a recent email.

SCSI core does REPORT LUNS, so we should use that.


>>* Avoid writing a separate SMP driver for now, and see how things
>>shake out with future SAS+SATA hardware.
> 
> 
> No such intentions.  SMP should not be in a separate driver.
> For SMP you need _addressing_ and the solution to this problem
> has been showin in driver/scsi/sas/README and in the announcement
> emails to linux-kernel and linux-scsi here:
> http://linux.adaptec.com/sas/

As I said above, this entire thread is talking about your code, and how 
it will integrate into the upstream kernel.  There is little value in 
repeatedly posting a URL to code we're already discussing.


>>* Put HCIL mapping into top-level helper code, for sharing between FC
>>and SAS (hopefully!)
> 
> 
> One step further: eliminate the ugly legacy SPI-centric HCIL from
> SCSI Core.  Minimise and streamline SCSI Core.

How many times must we repeat:  that's the long term goal.

Rome wasn't built in a day.


>>* use existing struct scsi_lun where feasible
>>
>>* LONG TERM: I wouldn't mind a small "scsi_rpc" helper lib
>>which provided execute_task() hook and other task mgmt functions.
>>Authors could choose to use that interface -- sitting just underneath
>>Scsi_Host_Template -- if their hardware closely matches the pure SAM
>>TMF model.  If hardware doesn't closely match, using this interface
>>will simply lead to a lot of glue/simulation code.  This gets the
>>hardware a bit closer to scsi_execute_req() etc.
> 
> 
> You don't see the _big_ picture: you need a _layer_, not "helper libs".
> This is what Object Oriented Deisign is all about.  SAM is object
> oriented, as is SAS as is iSCSI as is SBP, etc.
> 
> "Helper libs" makes the code ugly and spaghetti like.
> 
> A transport LLDD implments access to the _transport_: take USB, SBP and SAS --
> they all do it.  They need a management transport layer as showin in
> those implementations.
> 
> I know that this is how libata works: "helper libs" but you have to think
> on a larger scale:

Helper lib is another way of saying object-oriented design.  Look closer.


> libata is two layers: SATL and HW interface -- given ata_port and how

Obviously.  ls(1) would have told you that:

[jgarzik@pretzel sas-2.6]$ ls drivers/scsi/libata*.c
drivers/scsi/libata-core.c  drivers/scsi/libata-scsi.c

The hardware interface is separate from the SCSI simulator.

	Jeff

Re: Adaptec SAS integration notes

[Luben Tuikov]

On 10/18/05 12:44, Jeff Garzik wrote:
>>Jeff, you've got to be _specific_ in your specifying to _which_ group
>>those problems pertain to.  Saying "both Adaptec SAS and existing kernel"
>>is very vague.
> 
> 
> You are supposed to know when I am referring to the upstream kernel, and 
> when I am referring to your code :)  Logically, if I'm not talking about 
> updates your code needs, I am talking about updates the core kernel needs.

Yes, reading your post it was really unclear which is which -- hopefully
not intentional.

It would've been much better if you had:

Kernel needed updates:
 1)
 2)
 3)
 ... etc.

SAS Transport Layer updates:
 1)
 2)
 ... etc

etc.

Unless of course your message wasn't addressed to engineers of this list.

>>Neither the Host Adapter, nor the FW, implements anything which has
>>to do with the scsi host template.
> 
> 
> Wrong.  DMA boundary is a trivial counterexample.

I've addressed DMA boundary in another email which I posted 2 hours ago
to this list.

Our controller has no limitation as to DMA boundaries.  All 64 bit space
is fair game.  There is also no sg list size limitation.

For this reason I did not overengineer the code.

Adding HW specific stuff is trivial.  In fact I'll add it to my code
today and update the code.

The aic94xx code does include it: struct hw_profile { ... }; embedded in
struct asd_ha_struct { ... };  See drivers/scsi/aic94xx/aic94xx_hwi.h.

Exactly the same concept applies to struct sas_ha_struct { ... };.
I'll embed struct hw_profile { ... }; into it later today.  sg lists
max lengths, DMA maps, would be there (all ~0).

>>What it actually means, as the name _also_ suggest is a device on
>>the domain, which is transport addressible.  See SAM chapter 4 and 5.
> 
> 
> I'm discussing the topic at hand at a higher level than SAM.

Can you give us a reference to this "higher level than SAM"?
I'd like to read about it.

>>SAM tells you what goes in struct scsi_domain_device { ... };.
> 
> 
> My overall point is that we would need to convert the device classes and 
> SPI transport in order for this to happen.  And I'm still not convinced 
> that scsi_domain_device as you have implemented it is a useful abstraction.

I've never implemented struct scsi_domain_device.  Only struct domain_device { }
exists in the SAS code, to represent a domain device on a SAS domain.

BTW, SAM tells you clearly whether "scsi_domain_device" is a "useful abstraction."

> There is interface stuff in struct sas_ha_struct that really should move 
> to a higher level -- its not SAS-only, and that may be the better interface.

Well, you see, as _you_ said: the SAS code is pretty self contained.
I couldn't go and rip the whole SCSI Core apart.

But you are right: TMFs should've been part of SCSI Core long ago.
James B remembers when I was asking for TMF integration into
SCSI Core 3 years ago.

>>http://linux.adaptec.com/sas/
> 
> 
> This entire thread is talking about the code at that URL.  No need to be 
> self-referential.

http://linux.adaptec.com/sas/ ;-)

>>You cannot care about "remote" ports -- some types of device have _no_
>>port layer, as well you only care about the port "half" on your end.
>>SAS is quite clear on that.
> 
> Remote ports deals with the other half of the connection.  You gotta 
> deal with it somewhere.

I do deal with it.  Just look at the code and how it shows you the
ports that have been formed (from Announcement 1 at 
http://marc.theaimsgroup.com/?l=linux-scsi&m=112629509826900&w=2)

Example of a domain:

tree -d /sys/devices/pci0000\:01/0000\:01\:04.0/host11/sas/ha 

/sys/devices/pci0000:01/0000:01:04.0/host11/sas/ha
|-- phys
|   |-- 0
|   |   `-- port -> ../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/ports/0
|   |-- 1
|   |-- 2
|   |-- 3
|   |-- 4
|   |   `-- port -> ../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/ports/1
|   |-- 5
|   |   `-- port -> ../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/ports/1
|   |-- 6
|   |   `-- port -> ../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/ports/1
|   `-- 7
|       `-- port -> ../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/ports/1
`-- ports
    |-- 0
    |   |-- domain
    |   |   `-- 500000e000031c12
    |   |       `-- LUNS
    |   |           `-- 0000000000000000
    |   `-- phys
    |       `-- 0 -> ../../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/phys/0
    `-- 1
        |-- domain
        |   `-- 50001c1716010600
        |       |-- 50001c1071609c00
        |       |   |-- 50001c1071609c02
        |       |   |-- 5000c50000102a65
        |       |   |   `-- LUNS
        |       |   |       `-- 0000000000000000
        |       |   `-- 5005076a000001e0
        |       |       |-- 5000c50000513385
        |       |       |   `-- LUNS
        |       |       |       `-- 0000000000000000
        |       |       |-- 5005076a000001ed
        |       |       |   `-- LUNS
        |       |       |       `-- 0000000000000000
        |       |       `-- 5c50000000409c11
        |       |           `-- LUNS
        |       |               `-- 0000000000000000
        |       |-- 50001c1716010603
        |       |-- 50001c171601060d
        |       |   `-- LUNS
        |       |       `-- 0000000000000000
        |       `-- 5000c50000513329
        |           `-- LUNS
        |               `-- 0000000000000000
        `-- phys
            |-- 4 -> ../../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/phys/4
            |-- 5 -> ../../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/phys/5
            |-- 6 -> ../../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/phys/6
            `-- 7 -> ../../../../../../../../../devices/pci0000:01/0000:01:04.0/host11/sas/ha/phys/7

All three announcements can be found here:
http://linux.adaptec.com/sas/

>>>* very poor SATA interface
>>
>>
>>Hmm, no sorry, I'm not going to accept the BS FUD, generalized
>>comment.
> 
> 
> If it doesn't interface with libata, the current SATA interface, then 
> the code needs improvement.  Its a statement of fact.

Aaah, _that's_ what you meant. "If it doesn't use my code, it bad code."

>>Look, I've even included comments on how to integrate the code with
>>a SATL (which libata is _not_, but it does implement some SATL tasks).
>>
>>I know that you're pushing for libata-scsi to become SATL, but this
>>is hard when your devices belong to a port in an _array_.
> 
> 
> I've simply stated that there will not be more than one SCSI<->ATA 
> simulator in the kernel.
> 
> All implementation decisions trickle down from that overall path.

And you're forcing libata-scsi to be SATL which it is currently not.

The problem is that libata-scsi wasn't written with SATL in mind.
It doesn't present the translation at _device_ level, but at
port level, but the port in libata-core and libata-scsi is a
hw extension, QED.

E.g: int ata_scsi_queuecmd(...) references ata_port which clearly doesn't
exist as far as SATL is concerned.  Furthermore ata_port is a hw extension
which is used in libata-core.c.

Let me repeat: you need to have code which, say, exports only 2 functions.
One to register the SATA device to be translated for, the other
to unregister.  Thus any entity could register a translation service.
All other functions are filled in from both layers (registrant and registrar)
depending on their capabilities and the type of SATA device.

If you take a look at the design of the SAS Transport layer it does
exactly this: only 2 functions are exported to the kernel and everything
else is filled in the struct sas_ha_struct at the time of registering
the host adapter -- filled in by the LLDD and by the SAS Transport Layer.

This design is coherent.

Exporting 1e9 functions into the kernel is questionable.

>>SMP is NOT block device and will NEVER BE.
> 
> In this context, a block device is simply a messaging queue.

You are forgetting the definition of a block device.

SMP is not a block device, because (please fill in):
	1) ...
	2) ...

The SMP access implementation in the SAS Transport layer
gives you most of what you need, it is short and sweet:


/* ---------- SMP portal ---------- */

static ssize_t smp_portal_write(struct kobject *kobj, char *buf, loff_t offs,
				size_t size)
{
	struct domain_device *dev = to_dom_device(kobj);
	struct expander_device *ex = &dev->ex_dev;

	if (offs != 0)
		return -EFBIG;
	else if (size == 0)
		return 0;

	down_interruptible(&ex->smp_sema);
	if (ex->smp_req)
		kfree(ex->smp_req);
	ex->smp_req = kzalloc(size, GFP_USER);
	if (!ex->smp_req) {
		up(&ex->smp_sema);
		return -ENOMEM;
	}
	memcpy(ex->smp_req, buf, size);
	ex->smp_req_size = size;
	ex->smp_portal_pid = current->pid;
	up(&ex->smp_sema);

	return size;
}

static ssize_t smp_portal_read(struct kobject *kobj, char *buf, loff_t offs,
			       size_t size)
{
	struct domain_device *dev = to_dom_device(kobj);
	struct expander_device *ex = &dev->ex_dev;
	u8 *smp_resp;
	int res = -EINVAL;

	/* XXX: sysfs gives us an offset of 0x10 or 0x8 while in fact
	 *  it should be 0.
	 */

	down_interruptible(&ex->smp_sema);
	if (!ex->smp_req || ex->smp_portal_pid != current->pid)
		goto out;

	res = 0;
	if (size == 0)
		goto out;

	res = -ENOMEM;
	smp_resp = alloc_smp_resp(size);
	if (!smp_resp)
		goto out;
	res = smp_execute_task(dev, ex->smp_req, ex->smp_req_size,
			       smp_resp, size);
	if (!res) {
		memcpy(buf, smp_resp, size);
		res = size;
	}

	kfree(smp_resp);
out:
	kfree(ex->smp_req);
	ex->smp_req = NULL;
	ex->smp_req_size = 0;
	ex->smp_portal_pid = -1;
	up(&ex->smp_sema);
	return res;
}


A user space program is also provided in drivers/scsi/sas/expander_conf.c
and how to use it in drivers/scsi/sas/README.

There is no reason to make something be something it is not.

> Sigh.  I'm talking about implementation details, here.

Me too -- more than ever.  My code shows it, the comments there in.
And emails.  I've tried to cut and paste code as much as possible.

>>How about a drivers/scsi/satl/satl.c which Doug can start writing
>>and then we can contribute?
> 
> Absolutely not.  Update libata-scsi.

Jeff, the _design_ of libata-scsi doesn't satisfy for SATL.

Such a transformation would _completely_ change libata-scsi.c
other than the comment at the very top.  Is this what is desired?

What is needed by a SATL complient code is OOD: for any entity
to call in to register a device abstraction for translation,
at _device_ level.

>>>* SCSI core already pretty much equates to SAM, even if there is plenty
>>>of HCIL dependency in various places (HCIL should get marginalized,
>>>as all agree)
>>
>>
>>At a concept level yes, at implementation level absolutely no.
>>
>>A true SAM layer would be *much* _smaller_ and would have a more
>>straightforward path for tasks. (as does the SAS layer)
> 
> 
> Important concept:  struct naming and explicit layering based on 
> pictures in a spec is not how we want to do development.  We want to 
> -think- about the overall issues, and implement in the best manner while 
> ensuring the hardware state machine isn't violated.

Well, the hw state machine is _reflected_ in the SAS Transport layer
and aic94xx code.  It is exactly what it is, without any "impurities".

> SAM is ultimately message passing RPC over a network.

While I understand what you mean, according to the Abstract it is:

	"This standard specifies the SCSI Architecture Model.
	The purpose of the architecture is to provide a common
	basis for the coordination of SCSI standards and to specify
	those aspects of SCSI I/O system behavior that are independent
	of a particular technology and common to all implementations."

Not all transports would be a "network".  Anyway this isn't important
to the tasks at hand.

>>>Solutions
>>>---------
>>>* Similar to libata drivers, scsi-host-template should be in the LLDD,
>>>even if it is filled with mostly generic helper calls.
>>
>>
>>Again, see SBP and USB storage code (as well as the SAS code): this would be
>>a step backwards not forwards.  A true SAS LLLD has nothing to do with
>>the scsi host template.  This abstraction happens at the (SAS/SBP/USB Storage)
>>level layer.
> 
> 
> A SCSI domain and Linux SCSI host are already pretty close.  There's no 
> need to rewrite the world, just because they don't match 100%.

Not sure how your response relates to the quoted above.

But yes, you do not need to rewrite the word, or even one letter,
to make it work.  It works already here:
http://linux.adaptec.com/sas/

There is no SCSI Domain representation in SCSI Core.

The Linux SCSI Host structure isn't quite relevant -- it is shielded
from lower layers.

But above you talk about how there is relevant things in 
sas_ha_struct { ... }; which you want to pull into upper layers.

My suggestion is: One step at a time.

>>>* I would tend to prefer calling scsi_scan_target() from SAS discovery
>>>code, rather than sas_rhy_add()
>>
>>
>>This is apparently not my code.
>>
>>The SAS transport _layer_ does complete domain discovery as stipulated in
>>SAS 1.1.  (2.0 is out -- I'll be updating things when I come back).
> 
> 
> I'm trying to move forward by talking about how your code will integrate 
> with the kernel.

Ok, good.

> Pointing out "that's not my code" is completely irrelevant, and unhelpful.

I was merely stating that I wouldn't have been able to comment on
the code side.

>>>* Using scsi_scan_target() would allow us to use the normal LUN scan
>>
>>
>>Again not my code.  Normal LUN "scan" is done through REPORT LUNS
>>as shown in sas_discover.c as I posted both functions to this list
>>in a recent email.
> 
> 
> SCSI core does REPORT LUNS, so we should use that.

HCIL.


>>>* Avoid writing a separate SMP driver for now, and see how things
>>>shake out with future SAS+SATA hardware.
>>
>>
>>No such intentions.  SMP should not be in a separate driver.
>>For SMP you need _addressing_ and the solution to this problem
>>has been showin in driver/scsi/sas/README and in the announcement
>>emails to linux-kernel and linux-scsi here:
>>http://linux.adaptec.com/sas/
> 
> 
> As I said above, this entire thread is talking about your code, and how 
> it will integrate into the upstream kernel.  There is little value in 
> repeatedly posting a URL to code we're already discussing.

Well, in all honesty and actuallity the code has already been integrated
and it works.  The link is here:
http://linux.adaptec.com/sas/

Plus there are people using this code for their storage needs every day.

>>>* Put HCIL mapping into top-level helper code, for sharing between FC
>>>and SAS (hopefully!)
>>
>>
>>One step further: eliminate the ugly legacy SPI-centric HCIL from
>>SCSI Core.  Minimise and streamline SCSI Core.
> 
> 
> How many times must we repeat:  that's the long term goal.
> 
> Rome wasn't built in a day.

Indeed Rome wasn't built in a day.

As to SCSI Core, if you conjure up struct scsi_domain_device { ... };
you rid yourself of HCIL immediately.

>>You don't see the _big_ picture: you need a _layer_, not "helper libs".
>>This is what Object Oriented Deisign is all about.  SAM is object
>>oriented, as is SAS as is iSCSI as is SBP, etc.
>>
>>"Helper libs" makes the code ugly and spaghetti like.
>>
>>A transport LLDD implments access to the _transport_: take USB, SBP and SAS --
>>they all do it.  They need a management transport layer as showin in
>>those implementations.
>>
>>I know that this is how libata works: "helper libs" but you have to think
>>on a larger scale:
> 
> 
> Helper lib is another way of saying object-oriented design.  Look closer.

Jeff, can I quote you on this and use it as my sig? ;-)

>>libata is two layers: SATL and HW interface -- given ata_port and how
> 
> Obviously.  ls(1) would have told you that:
> 
> [jgarzik@pretzel sas-2.6]$ ls drivers/scsi/libata*.c
> drivers/scsi/libata-core.c  drivers/scsi/libata-scsi.c
> 
> The hardware interface is separate from the SCSI simulator.

The exported to the kernel ata_scsi_queuecommand() defined in
libata-scsi.c dereferences struct ata_port.

That is, if, say SAS Transport Layer wanted translation for
a SATA device it would have to conjure up ata_port, which
would need to be made up.

	Luben
-- 
http://linux.adaptec.com/sas/

Re: Adaptec SAS integration notes

[Jeff Garzik]

Luben Tuikov wrote:
> On 10/18/05 12:44, Jeff Garzik wrote:
>>Wrong.  DMA boundary is a trivial counterexample.
> 
> 
> I've addressed DMA boundary in another email which I posted 2 hours ago
> to this list.
> 
> Our controller has no limitation as to DMA boundaries.  All 64 bit space
> is fair game.  There is also no sg list size limitation.

These are an Adaptec-only assumptions, at the present time.

The DMA boundary, s/g stuff etc. are there for the LLDD to communicate 
that to the SCSI and block layers that need that info.

Once you encounter non-Adaptec hardware with limitations on these 
details, you'll either be forced to reinvent the wheel, duplicating this 
stuff, or backtrack and use the scsi-host-template as it was intended to 
be used.


> For this reason I did not overengineer the code.
> 
> Adding HW specific stuff is trivial.  In fact I'll add it to my code
> today and update the code.
> 
> The aic94xx code does include it: struct hw_profile { ... }; embedded in
> struct asd_ha_struct { ... };  See drivers/scsi/aic94xx/aic94xx_hwi.h.
> 
> Exactly the same concept applies to struct sas_ha_struct { ... };.
> I'll embed struct hw_profile { ... }; into it later today.  sg lists
> max lengths, DMA maps, would be there (all ~0).

No.  This would simply be duplicating more of the SCSI core, because you 
don't like scsi-host-template.


>>>What it actually means, as the name _also_ suggest is a device on
>>>the domain, which is transport addressible.  See SAM chapter 4 and 5.
>>
>>
>>I'm discussing the topic at hand at a higher level than SAM.
> 
> 
> Can you give us a reference to this "higher level than SAM"?
> I'd like to read about it.

<shrug>  Any literature on messaging, networks, and RPC.

Block layer, and Dragonfly BSD concepts are along these lines.


>>>You cannot care about "remote" ports -- some types of device have _no_
>>>port layer, as well you only care about the port "half" on your end.
>>>SAS is quite clear on that.
>>
>>Remote ports deals with the other half of the connection.  You gotta 
>>deal with it somewhere.
> 
> 
> I do deal with it.  Just look at the code and how it shows you the
> ports that have been formed (from Announcement 1 at 
> http://marc.theaimsgroup.com/?l=linux-scsi&m=112629509826900&w=2)

The quoted part you deleted was referring to the existing SAS transport 
class, unless I'm completely lost in this thread.


>>>>* very poor SATA interface
>>>
>>>
>>>Hmm, no sorry, I'm not going to accept the BS FUD, generalized
>>>comment.
>>
>>
>>If it doesn't interface with libata, the current SATA interface, then 
>>the code needs improvement.  Its a statement of fact.
> 
> 
> Aaah, _that's_ what you meant. "If it doesn't use my code, it bad code."

If it duplicates SCSI<->ATA translation, and ignores existing kernel 
facilities...


>>>Look, I've even included comments on how to integrate the code with
>>>a SATL (which libata is _not_, but it does implement some SATL tasks).
>>>
>>>I know that you're pushing for libata-scsi to become SATL, but this
>>>is hard when your devices belong to a port in an _array_.
>>
>>
>>I've simply stated that there will not be more than one SCSI<->ATA 
>>simulator in the kernel.
>>
>>All implementation decisions trickle down from that overall path.
> 
> 
> And you're forcing libata-scsi to be SATL which it is currently not.

I'm describing the path that needs to be taken, moving forward.


>>SAM is ultimately message passing RPC over a network.
> 
> 
> While I understand what you mean, according to the Abstract it is:
> 
> 	"This standard specifies the SCSI Architecture Model.
> 	The purpose of the architecture is to provide a common
> 	basis for the coordination of SCSI standards and to specify
> 	those aspects of SCSI I/O system behavior that are independent
> 	of a particular technology and common to all implementations."
> 
> Not all transports would be a "network".  Anyway this isn't important
> to the tasks at hand.

It's quite important.

SAS is networking -- we have packets, we have routing.


>>>>* Using scsi_scan_target() would allow us to use the normal LUN scan
>>>
>>>
>>>Again not my code.  Normal LUN "scan" is done through REPORT LUNS
>>>as shown in sas_discover.c as I posted both functions to this list
>>>in a recent email.
>>
>>
>>SCSI core does REPORT LUNS, so we should use that.
> 
> 
> HCIL.

That's a reason to work with the SCSI layer to update it, not work 
around it.


>>>>* Avoid writing a separate SMP driver for now, and see how things
>>>>shake out with future SAS+SATA hardware.
>>>
>>>
>>>No such intentions.  SMP should not be in a separate driver.
>>>For SMP you need _addressing_ and the solution to this problem
>>>has been showin in driver/scsi/sas/README and in the announcement
>>>emails to linux-kernel and linux-scsi here:
>>>http://linux.adaptec.com/sas/
>>
>>
>>As I said above, this entire thread is talking about your code, and how 
>>it will integrate into the upstream kernel.  There is little value in 
>>repeatedly posting a URL to code we're already discussing.
> 
> 
> Well, in all honesty and actuallity the code has already been integrated
> and it works.  The link is here:
> http://linux.adaptec.com/sas/

How many times are you going to post this link in each message?


>>>libata is two layers: SATL and HW interface -- given ata_port and how
>>
>>Obviously.  ls(1) would have told you that:
>>
>>[jgarzik@pretzel sas-2.6]$ ls drivers/scsi/libata*.c
>>drivers/scsi/libata-core.c  drivers/scsi/libata-scsi.c
>>
>>The hardware interface is separate from the SCSI simulator.
> 
> 
> The exported to the kernel ata_scsi_queuecommand() defined in
> libata-scsi.c dereferences struct ata_port.
> 
> That is, if, say SAS Transport Layer wanted translation for
> a SATA device it would have to conjure up ata_port, which
> would need to be made up.

Again, I'm stating the path to move forward.  Obviously, code changes 
are needed.  Rather than continually pointing to a URL, I'm trying to 
point out where those code changes should occur.

	Jeff

Re: Adaptec SAS integration notes

[Luben Tuikov]

On 10/18/05 16:16, Jeff Garzik wrote:
>>Our controller has no limitation as to DMA boundaries.  All 64 bit space
>>is fair game.  There is also no sg list size limitation.
> 
> 
> These are an Adaptec-only assumptions, at the present time.
> 
> The DMA boundary, s/g stuff etc. are there for the LLDD to communicate 
> that to the SCSI and block layers that need that info.
> 
> Once you encounter non-Adaptec hardware with limitations on these 
> details, you'll either be forced to reinvent the wheel, duplicating this 
> stuff, or backtrack and use the scsi-host-template as it was intended to 
> be used.

Not so.  Since the architecture is cleanly _layered_, it is very
simple to add struct hw_profile { ... } to struct sas_ha_struct
and "move forward".

As I said I didn't want to overengineer.

The SCSI Host template is Parallel SCSI structure invented
when there was a need for it, over 10 years ago.  It (host
template) completely mixes up hw limitations, SCSI Core
concepts and Protocol (Parallel SCSI) into a single
list of variables and function stubs.

struct sas_ha_struct and struct asd_ha_struct show how
you can have a separate structure (object?) to express
what is needed: hw limitations, software presentations,
etc.  See how struct hw_profile is embedded into
asd_ha_struct ?  Similarly for struct sas_ha_struct.

Again, look at USB storage and SBP code to see how the concept
is the same.

In open source there is no such thing as "xyz was intended to
be used".  This is all about innovation, and "moving forward"
as far as technology is concerned.

>>Exactly the same concept applies to struct sas_ha_struct { ... };.
>>I'll embed struct hw_profile { ... }; into it later today.  sg lists
>>max lengths, DMA maps, would be there (all ~0).
> 
> 
> No.  This would simply be duplicating more of the SCSI core, because you 
> don't like scsi-host-template.

It has nothing to do with "like".  It has to do with what is the
host template and what is the interconnect driver.

The interconnect driver, aic94xx, is not a SCSI host, thus
it cannot be represented as struct scsi_host_template.

The SCSI Host abstraction is achieved by and in the SAS
transport Layer, in exactly the same way it is done
in USB Storage and in the SBP code.

Such concepts are not new Jeff, and there is no reason to
force struct scsi_host_template into aic94xx, while it has
been shown that it doesn't need it since it is not
a SCSI Host to begin with.  This is clearly shown
in the code, currently working just fine:
http://linux.adaptec.com/sas/

aic94xx is a PCI driver providing access to certain
type of interconnect.

The SAS Transport Layer provides the SCSI Host abstraction.

What all this accomplishes is clearly exposed and separated
layer: an interconnect and Transport.

That is, it is possible for another entity to want to
control the Host Adapter (aic94xx) using the struct sas_ha_struct 
_directly_.

Currrently the only other such entitty is SCSI Core via
sas_scsi_host.c file glue (996 lines), it (SCSI Core) doesn't
necessarily have to be the only one.

There is a clear example of another entity which may want
such low level control, but this a different topic.

All in all, if you would like to put the SCSI Host template
in aic94xx, so that James B officially accepts the code into
scsi-misc-2.6, so be it.  I just want to make the technical
objection known into this list and the archives.

>>>>What it actually means, as the name _also_ suggest is a device on
>>>>the domain, which is transport addressible.  See SAM chapter 4 and 5.
>>>
>>>
>>>I'm discussing the topic at hand at a higher level than SAM.
>>
>>
>>Can you give us a reference to this "higher level than SAM"?
>>I'd like to read about it.
> 
> 
> <shrug>  Any literature on messaging, networks, and RPC.
> 
> Block layer, and Dragonfly BSD concepts are along these lines.

You seem to talk about "higher level than SAM" and then
you mention that getting rid of HCIL is long term.

While getting rid of HCIL is straightforward and we
can postpone those "higher level than SAM" concepts
for later.

>>>>You cannot care about "remote" ports -- some types of device have _no_
>>>>port layer, as well you only care about the port "half" on your end.
>>>>SAS is quite clear on that.
>>>
>>>Remote ports deals with the other half of the connection.  You gotta 
>>>deal with it somewhere.
>>
>>
>>I do deal with it.  Just look at the code and how it shows you the
>>ports that have been formed (from Announcement 1 at 
>>http://marc.theaimsgroup.com/?l=linux-scsi&m=112629509826900&w=2)
> 
> 
> The quoted part you deleted was referring to the existing SAS transport 
> class, unless I'm completely lost in this thread.

In this case I must've assumed that you were talking about my code,
while in fact you were talking about the "SAS Transport attributes",
which do not follow SAS but are FC-like.

Unless I am also completely lost in this thread. ;-)

>>>If it doesn't interface with libata, the current SATA interface, then 
>>>the code needs improvement.  Its a statement of fact.
>>
>>
>>Aaah, _that's_ what you meant. "If it doesn't use my code, it bad code."
> 
> 
> If it duplicates SCSI<->ATA translation, and ignores existing kernel 
> facilities...

Well, I've pointed out numerous times why libata-scsi.c isn't
quite SATL.  I'm not sure if I should repeat myself for a
5th time.

It would take an insurmountable effort to change libata-scsi.c
to become SATL, after which only the top comment would be
left.  Judging from your comments, am I assuming incorrectly that
this is the intention?

Also wouldn't it be more logical to call the file "satl.c" since
this is exactly what it provides?

>>>All implementation decisions trickle down from that overall path.
>>
>>
>>And you're forcing libata-scsi to be SATL which it is currently not.
> 
> 
> I'm describing the path that needs to be taken, moving forward.

Again higly subjectively, Jeff.

>>>SAM is ultimately message passing RPC over a network.
>>
>>
>>While I understand what you mean, according to the Abstract it is:
>>
>>	"This standard specifies the SCSI Architecture Model.
>>	The purpose of the architecture is to provide a common
>>	basis for the coordination of SCSI standards and to specify
>>	those aspects of SCSI I/O system behavior that are independent
>>	of a particular technology and common to all implementations."
>>
>>Not all transports would be a "network".  Anyway this isn't important
>>to the tasks at hand.
> 
> 
> It's quite important.
> 
> SAS is networking -- we have packets, we have routing.

Not all transports would be a "network".  Again this isn't important
to the tasks at hand.

I've numerous times explained why a SAS Domain isn't quite a
"network" a lot less "Ethernet-like".

>>>>>* Using scsi_scan_target() would allow us to use the normal LUN scan
>>>>
>>>>
>>>>Again not my code.  Normal LUN "scan" is done through REPORT LUNS
>>>>as shown in sas_discover.c as I posted both functions to this list
>>>>in a recent email.
>>>
>>>
>>>SCSI core does REPORT LUNS, so we should use that.
>>
>>
>>HCIL.
> 
> 
> That's a reason to work with the SCSI layer to update it, not work 
> around it.

Create scsi_domain_device { ... }; ?

>>Well, in all honesty and actuallity the code has already been integrated
>>and it works.  The link is here:
>>http://linux.adaptec.com/sas/
> 
> 
> How many times are you going to post this link in each message?

Not sure.  Jeff, do you personally object to my postig this link, which
contains a working kernel with SAS Transport Layer and aic94xx?

>>>>libata is two layers: SATL and HW interface -- given ata_port and how
>>>
>>>Obviously.  ls(1) would have told you that:
>>>
>>>[jgarzik@pretzel sas-2.6]$ ls drivers/scsi/libata*.c
>>>drivers/scsi/libata-core.c  drivers/scsi/libata-scsi.c
>>>
>>>The hardware interface is separate from the SCSI simulator.
>>
>>
>>The exported to the kernel ata_scsi_queuecommand() defined in
>>libata-scsi.c dereferences struct ata_port.
>>
>>That is, if, say SAS Transport Layer wanted translation for
>>a SATA device it would have to conjure up ata_port, which
>>would need to be made up.
> 
> 
> Again, I'm stating the path to move forward.  Obviously, code changes 
> are needed.  Rather than continually pointing to a URL, I'm trying to 
> point out where those code changes should occur.

Ok, so how do you think libata-scsi.c should change to support SATL?

I think that you need to have code which, say, exports only 2 functions.
One to register the SATA device to be translated for, the other
to unregister.  Thus any entity could register a translation service.
All other functions are filled in from both layers (registrant and registrar)
depending on their capabilities and the type of SATA device.

If you take a look at the design of the SAS Transport layer it does
exactly this: only 2 functions are exported to the kernel and everything
else is filled in the struct sas_ha_struct at the time of registering
the host adapter -- filled in by the LLDD and by the SAS Transport Layer.

This design is coherent.

Exporting 1e9 functions into the kernel is questionable.

What do you think?

	Luben
-- 
http://linux.adaptec.com/sas/

Re: Adaptec SAS integration notes

[Stefan Richter]

Jeff Garzik wrote:
> Luben Tuikov wrote:
[Jeff Garzik wrote:]
>>> * Put HCIL mapping into top-level helper code, for sharing between FC
>>> and SAS (hopefully!)
>>
>> One step further: eliminate the ugly legacy SPI-centric HCIL from
>> SCSI Core.  Minimise and streamline SCSI Core.
> 
> How many times must we repeat:  that's the long term goal.
> 
> Rome wasn't built in a day.

All transports implement HCIL mappings. (SPI drivers enjoy the easiest 
mapping, hcil<->HCIL.) They are all faced with the necessity of mapping 
and solve it in different ways, for different reasons. Why stop at FC + 
SAS and try to find a unified solution for just those two? Also, when 
you are working out how FC and SAS could interface to a common HCIL 
mapping library (or HCIL mapping layer?), you are effectively designing 
aspects of how an interface between FC|SAS and SCSI core should look 
like. Or, why not provide the HCIL mapping helpers in the core and 
subsequently make users of this new interface completely unaware of the 
existence of HCIL...?

Moreover, HCIL mapping is but one aspect of a common problem. As an 
example, I think yet another aspect of the same problem is that device 
properties like target names and LUNs get lost below the 
transport<->core interface. We want to present these device properties 
to userspace in a unified manner. Here it becomes even more clear: 
Neither is it a problem for FC and SAS alone, nor are transport-level 
helpers a good solution to the problem.

I freely admit that I don't have a picture of where the real 
difficulties lie; I am far too unfamiliar with the innards of scsi core. 
But time spent on transport-level helpers which are meant to ease 
symptoms of the transport<->core difficulties seems to be spent rather 
ineffectively to me. More concrete, what is the ratio of benefit to cost 
of HCIL mapping code sharing between SAS and FC? Especially since that 
mapping is deemed obsolete already (in the long term or perhaps even mid 
term). And to completely go back to the subject: Will attempts on code 
sharing between FC layer and SAS layer bring things forward in the 
short-term goal of SAS integration?
-- 
Stefan Richter
-=====-=-=-= =-=- =--=-
http://arcgraph.de/sr/

Re: Adaptec SAS integration notes

[Jeff Garzik]

Stefan Richter wrote:
> All transports implement HCIL mappings. (SPI drivers enjoy the easiest 
> mapping, hcil<->HCIL.) They are all faced with the necessity of mapping 
> and solve it in different ways, for different reasons. Why stop at FC + 
> SAS and try to find a unified solution for just those two? Also, when 
> you are working out how FC and SAS could interface to a common HCIL 
> mapping library (or HCIL mapping layer?), you are effectively designing 
> aspects of how an interface between FC|SAS and SCSI core should look 
> like. Or, why not provide the HCIL mapping helpers in the core and 
> subsequently make users of this new interface completely unaware of the 
> existence of HCIL...?

Ignoring some legacy ioctls and bits of existing code...   The only 
mapping that's _really_ required is an internal C pointer, used by the 
device class to send messages to the transport class.

Adaptec's code does a bit of that on the transport side, but we'd need 
to update all the existing device classes (sd, sr, st, ...) before they 
had been converted to a purely using pointers to connect the transport 
to the device.

Getting to the point where device classes need only to do

	transport_instance->send_message(scsi command | TMF)

and where the transport class replies with

	device_instance->send_message(scsi response | TMF response)

may be a long term goal.  Depends on where evolution takes us...


> Moreover, HCIL mapping is but one aspect of a common problem. As an 
> example, I think yet another aspect of the same problem is that device 
> properties like target names and LUNs get lost below the 
> transport<->core interface. We want to present these device properties 
> to userspace in a unified manner. Here it becomes even more clear: 
> Neither is it a problem for FC and SAS alone, nor are transport-level 
> helpers a good solution to the problem.

Target names are already in the generic device.  Luben has a good point, 
though, that there may be multiple "labels" for the same addressible object.


> I freely admit that I don't have a picture of where the real 
> difficulties lie; I am far too unfamiliar with the innards of scsi core. 
> But time spent on transport-level helpers which are meant to ease 
> symptoms of the transport<->core difficulties seems to be spent rather 
> ineffectively to me. More concrete, what is the ratio of benefit to cost 
> of HCIL mapping code sharing between SAS and FC? Especially since that 
> mapping is deemed obsolete already (in the long term or perhaps even mid 
> term). And to completely go back to the subject: Will attempts on code 
> sharing between FC layer and SAS layer bring things forward in the 
> short-term goal of SAS integration?

HCIL mapping is a simple and very small part of this task :)  There are 
a bunch of line items to tackle, including SMP and SDI details, all 
bundled up in this.

	Jeff