[PATCH 0/2] fusion SAS support

[PATCH 0/2] fusion SAS support

[Christoph Hellwig]

This patch implements minimal SAS support for fusion.  It needs the
previous set of fusion updates and the minimal SAS transport class to
work.  I'll probably revisit the transport class later to introduce a
rport object, but I'll need some real enclosures with extenders and
SMP ports to experiment with first.  So my suggestion is to put the
current state of the art into scsi-misc so it gets exposure in -mm
and people can start reviwing the design and send patches.  (Lots
of thanks to James Smart btw, I had hoped to get more of this
constructive critizism).

Re: [PATCH 0/2] fusion SAS support

[Jeremy Higdon]

On Fri, Aug 19, 2005 at 06:26:34PM +0200, Christoph Hellwig wrote:
> This patch implements minimal SAS support for fusion.  It needs the
> previous set of fusion updates and the minimal SAS transport class to
> work.  I'll probably revisit the transport class later to introduce a
> rport object, but I'll need some real enclosures with extenders and
> SMP ports to experiment with first.  So my suggestion is to put the
> current state of the art into scsi-misc so it gets exposure in -mm
> and people can start reviwing the design and send patches.  (Lots
> of thanks to James Smart btw, I had hoped to get more of this
> constructive critizism).


Hi Christoph,

I applied your four (sets of) patches, and they all seem to work
fine.  When connected through an expander, the data is a little light,
but direct connect works great.  Below, targets 0 and 1 are direct
connect to a 1064 chip, while targets 21-29 are through an expander.

pc-mdr5:/sys/class/sas_transport/target0:0:1 # lsscsi
[0:0:0:0]    disk    SEAGATE  ST373454SS       0002  /dev/sda
[0:0:1:0]    disk    SEAGATE  ST3146854SS      0002  /dev/sdb
[0:0:21:0]   enclosu LSILOGIC SASX12 SEP       v0.1  -       
[0:0:22:0]   disk    SEAGATE  ST373454SS       0002  /dev/sdk
[0:0:23:0]   disk    SEAGATE  ST373454SS       0002  /dev/sdl
[0:0:24:0]   disk    SEAGATE  ST3146854SS      0002  /dev/sdm
[0:0:25:0]   disk    SEAGATE  ST3146854SS      0002  /dev/sdn
[0:0:26:0]   disk    SEAGATE  ST3146854SS      0002  /dev/sdo
[0:0:27:0]   disk    SEAGATE  ST3146854SS      0002  /dev/sdp
[0:0:28:0]   disk    SEAGATE  ST373454SS       0002  /dev/sdq
[0:0:29:0]   disk    SEAGATE  ST373454SS       0002  /dev/sdr

pc-mdr5:/sys/class/sas_transport/target0:0:1 # more /sys/class/sas_transport/target0\:0\:[01]/*

*** /sys/class/sas_transport/target0:0:0/device: directory ***

::::::::::::::
/sys/class/sas_transport/target0:0:0/device_type
::::::::::::::
end device
::::::::::::::
/sys/class/sas_transport/target0:0:0/initiator_port_protocols
::::::::::::::
none
::::::::::::::
/sys/class/sas_transport/target0:0:0/phy_identifier
::::::::::::::
1
::::::::::::::
/sys/class/sas_transport/target0:0:0/sas_address
::::::::::::::
0x5000c500004015fd
::::::::::::::
/sys/class/sas_transport/target0:0:0/target_port_protocols
::::::::::::::
ssp

*** /sys/class/sas_transport/target0:0:1/device: directory ***

::::::::::::::
/sys/class/sas_transport/target0:0:1/device_type
::::::::::::::
end device
::::::::::::::
/sys/class/sas_transport/target0:0:1/initiator_port_protocols
::::::::::::::
none
::::::::::::::
/sys/class/sas_transport/target0:0:1/phy_identifier
::::::::::::::
0
::::::::::::::
/sys/class/sas_transport/target0:0:1/sas_address
::::::::::::::
0x5000c50000000ef5
::::::::::::::
/sys/class/sas_transport/target0:0:1/target_port_protocols
::::::::::::::
ssp

*** /sys/class/sas_transport/target0:0:24/device: directory ***

::::::::::::::
/sys/class/sas_transport/target0:0:24/device_type
::::::::::::::
none
::::::::::::::
/sys/class/sas_transport/target0:0:24/initiator_port_protocols
::::::::::::::
none
::::::::::::::
/sys/class/sas_transport/target0:0:24/phy_identifier
::::::::::::::
0
::::::::::::::
/sys/class/sas_transport/target0:0:24/sas_address
::::::::::::::
0x0
::::::::::::::
/sys/class/sas_transport/target0:0:24/target_port_protocols
::::::::::::::
none

Re: [PATCH 0/2] fusion SAS support

[Christoph Hellwig]

On Wed, Aug 24, 2005 at 12:08:40AM -0700, Jeremy Higdon wrote:
> Hi Christoph,
> 
> I applied your four (sets of) patches, and they all seem to work
> fine.  When connected through an expander, the data is a little light,
> but direct connect works great.  Below, targets 0 and 1 are direct
> connect to a 1064 chip, while targets 21-29 are through an expander.

The device_type = none for the target connected through the expander
looks wrong.  I'll hopefully have an enclosure with an expander soon
and will fix this.

Re: [PATCH 0/2] fusion SAS support

[Jeremy Higdon]

On Wed, Aug 24, 2005 at 10:57:56AM +0200, Christoph Hellwig wrote:
> On Wed, Aug 24, 2005 at 12:08:40AM -0700, Jeremy Higdon wrote:
> > Hi Christoph,
> > 
> > I applied your four (sets of) patches, and they all seem to work
> > fine.  When connected through an expander, the data is a little light,
> > but direct connect works great.  Below, targets 0 and 1 are direct
> > connect to a 1064 chip, while targets 21-29 are through an expander.
> 
> The device_type = none for the target connected through the expander
> looks wrong.  I'll hopefully have an enclosure with an expander soon
> and will fix this.

I'm also having a couple of problems problem with discovery.

One is that devices connected to the expander are not discovered
at driver init time.  It requires a manual probe after driver init.

The other is that there's something that the firmware doesn't like
about the probe.  I got these sorts of errors.

mptscsih: ioc0: >> Attempting task abort! (sc=e000003014da9c80)
mptscsih: ioc0: >> Attempting task abort! (sc=e000003014da9c80)
mptbase: Initiating ioc0 recovery
mptscsih: ioc0: >> Attempting task abort! (sc=e000003014da9c80)
mptbase: Initiating ioc0 recovery
mptscsih: ioc0: >> Attempting target reset! (sc=e000003014da9c80)
mptscsih: ioc0: >> Attempting bus reset! (sc=e000003014da9c80)
mptbase: ioc0: WARNING - IOC is in FAULT state!!!
           FAULT code = 1600h
mptbase: ioc0: Recovered from IOC FAULT

However, after updating firmware on the 1064, this latter problem
seems to be fixed (still doesn't discover devices on the expander
at driver init).

jeremy

Re: [PATCH 0/2] fusion SAS support

[Christoph Hellwig]

On Wed, Aug 24, 2005 at 02:10:14AM -0700, Jeremy Higdon wrote:
> However, after updating firmware on the 1064, this latter problem
> seems to be fixed (still doesn't discover devices on the expander
> at driver init).

As mentioned expanders aren't tested at all yet, I'll fix it as soon
as I have an expander and hopefully soon the fusion specs (still waiting
for lsi to contact me about the nda)

RE: [PATCH 0/2] fusion SAS support

[Moore, Eric Dean]

On Wednesday, August 24, 2005 3:14 AM, Christoph Hellwig wrote:
> On Wed, Aug 24, 2005 at 02:10:14AM -0700, Jeremy Higdon wrote:
> > However, after updating firmware on the 1064, this latter problem
> > seems to be fixed (still doesn't discover devices on the expander
> > at driver init).
> 
> As mentioned expanders aren't tested at all yet, I'll fix it as soon
> as I have an expander and hopefully soon the fusion specs 
> (still waiting
> for lsi to contact me about the nda)
>

I will ship you a SASx12 Expander on Monday. My manager is working
on NDA's for both you and James, and hopefully they will be completed
very soon.  I sent you two SAS HBA, perhaps one of those you could
send to James Bottomley.

I apologize its taking so long to review the patches you sent last
week, but I have been swamped working issues for many different customers.
I only began looking at your patches last night.   Here are some comments:

+		ioc->sas_ports[i] = sas_port_add(ioc->sh, i, attrs);
+		if (IS_ERR(ioc->sas_ports[i])) {
+			error = PTR_ERR(ioc->sas_ports[i]);
+			goto out_free_ports;
+		}

This code is only attaching ports for the direct ports.  I have
a LSI SAS1064 - 4 ports - so it only reports four ports.  

I see:

/sys/class/sas_port/port-4:0
/sys/class/sas_port/port-4:1
/sys/class/sas_port/port-4:2
/sys/class/sas_port/port-4:3

In my configuration I have SASx12 sitting on the 3rd port.
Expander support is not there in your version of the driver.
How do you propose to report the expanders 12 ports that is attached to 
/sys/class/sas_port/port-4:3 ??  Will I see all the 12 ports added
to /sys/class/sas_port/ or should these ports be inside 
/sys/class/sas_port/port-4:3 ??  Such as

/sys/class/sas_port/port-4:3/port-12:0
/sys/class/sas_port/port-4:3/port-12:1
/sys/class/sas_port/port-4:3/port-12:2
/sys/class/sas_port/port-4:3/port-12:3

If so, then the sas_transport code needs to change.


+	if (*phy_counter >= ioc->num_sas_ports) {
+		sas_add_target(ioc->sas_ports[pg0->PhysicalPort],
+				&ioc->sas_attached[pg0->PhysicalPort],
+				pg0->Bus, pg0->TargetID)

My expander has 7 devices attached. There is 5 SATA and 2 SAS devices.
>From the function mptsas_probe_attached_device - this will
call Sas Device Page 0, this containing all the devices seen in
the domain.  The function sas_add_target() will be called
for all seven devices, to include the SMP expander; e.g. 8 times.
During the time it called sas_add_target() 8 times,  its sending the same
ioc->sas_ports and ioc->sas_attached data pointers because all 8
devices are at pg0->PhysicalPort=3.  Thus how will setup the 
12 ports for expander? The information regarding
these 12 ports can be obtained via Sas Expander Page 1. 
I guess we can add this to the function mptsas_probe_phy() so ports for
direct attached and expanders are reported? What do you think, or 
did you have another idea bout this?  I'm not sure about how
calling sas_port_add() 4 times for the HBA, and 12 times
for the exander, how that would be populating the tree.  I guess you want
to wait till you recieve the expander to comment on this?

Additional comments:

Loading the driver as modules - will not work because its missing
EXPORT_SYMBOL(mptbase_sas_persist_operation) in mptbase.c

We have Integrated RAID support for SAS - thus in my code(3.02.55) I have 
broken out the RAID related parameters from typedef _ScsiCfgData
into another structure called _RaidCfgData - I would suggest
picking up these changes.

Hot plug support - how is this going to be done. Take a look
at mptscsih_hot_plug_worker_thread, that I sent in 3.02.55.

There are other minor items left out in your port, which I would like
added.  I will go ahead make those changes and send across a patch for that.


Eric Moore
LSI Logic

Re: [PATCH 0/2] fusion SAS support

[Christoph Hellwig]

On Fri, Aug 26, 2005 at 05:15:11PM -0600, Moore, Eric Dean wrote:
> I apologize its taking so long to review the patches you sent last
> week, but I have been swamped working issues for many different customers.
> I only began looking at your patches last night.   Here are some comments:
> 
> +		ioc->sas_ports[i] = sas_port_add(ioc->sh, i, attrs);
> +		if (IS_ERR(ioc->sas_ports[i])) {
> +			error = PTR_ERR(ioc->sas_ports[i]);
> +			goto out_free_ports;
> +		}
> 
> This code is only attaching ports for the direct ports.  I have
> a LSI SAS1064 - 4 ports - so it only reports four ports.  
> 
> I see:
> 
> /sys/class/sas_port/port-4:0
> /sys/class/sas_port/port-4:1
> /sys/class/sas_port/port-4:2
> /sys/class/sas_port/port-4:3
> 
> In my configuration I have SASx12 sitting on the 3rd port.
> Expander support is not there in your version of the driver.

Yes, as noted in the transport class announcement this doesn't
deal with expanders yet at all.

> How do you propose to report the expanders 12 ports that is attached to 
> /sys/class/sas_port/port-4:3 ??  Will I see all the 12 ports added
> to /sys/class/sas_port/ or should these ports be inside 
> /sys/class/sas_port/port-4:3 ??  Such as
> 
> /sys/class/sas_port/port-4:3/port-12:0
> /sys/class/sas_port/port-4:3/port-12:1
> /sys/class/sas_port/port-4:3/port-12:2
> /sys/class/sas_port/port-4:3/port-12:3
> 
> If so, then the sas_transport code needs to change.

In the /sys/class/ hierachy they will have to be flat due to the
way the generic class_device code works.  No it's still open whether
we will have a flat hierachy of class_devices or whether we'll attach
extender ports to the struct device of the "parent" port.  I'll prefer
the former because it keeps the hierachy simpler and mirrors what is
done in FibreChannel, James prefers the latter.  I'll probably prototype
both.

> +	if (*phy_counter >= ioc->num_sas_ports) {
> +		sas_add_target(ioc->sas_ports[pg0->PhysicalPort],
> +				&ioc->sas_attached[pg0->PhysicalPort],
> +				pg0->Bus, pg0->TargetID)
> 
> My expander has 7 devices attached. There is 5 SATA and 2 SAS devices.
> >From the function mptsas_probe_attached_device - this will
> call Sas Device Page 0, this containing all the devices seen in
> the domain.  The function sas_add_target() will be called
> for all seven devices, to include the SMP expander; e.g. 8 times.
> During the time it called sas_add_target() 8 times,  its sending the same
> ioc->sas_ports and ioc->sas_attached data pointers because all 8
> devices are at pg0->PhysicalPort=3.  Thus how will setup the 
> 12 ports for expander? The information regarding
> these 12 ports can be obtained via Sas Expander Page 1. 
> I guess we can add this to the function mptsas_probe_phy() so ports for
> direct attached and expanders are reported? What do you think, or 
> did you have another idea bout this?  I'm not sure about how
> calling sas_port_add() 4 times for the HBA, and 12 times
> for the exander, how that would be populating the tree.  I guess you want
> to wait till you recieve the expander to comment on this?

I think getting the NDA and docs is more important.  What I really
want to do is to call scsi_scan_target for every sas device that's a scsi
target, so that we can support multi-lun devices easily and coherently with
other SCSI transports.  Now I need to figure out how to properly get
information out of the Fusion firmware to only report attached sas devices,
not individual luns, and I need to make sure different sas devices never
get the same target id, so scsi_scan_device does the right thing.  This
could mean I need to the work I suggested to Luben in a previous mail to
make ->id meaningless for SAS and similar transports, or a scsi midlayer
to fusion target id mapping.


> Additional comments:
> 
> Loading the driver as modules - will not work because its missing
> EXPORT_SYMBOL(mptbase_sas_persist_operation) in mptbase.c

ok.

> We have Integrated RAID support for SAS - thus in my code(3.02.55) I have 
> broken out the RAID related parameters from typedef _ScsiCfgData
> into another structure called _RaidCfgData - I would suggest
> picking up these changes.

The changes have conflict with James' RAID and DV changes, so I only want
to merge them over after James' chanhes are in.

> Hot plug support - how is this going to be done. Take a look
> at mptscsih_hot_plug_worker_thread, that I sent in 3.02.55.

We'll probably have a workqueue for it in the transport class, but hotplug
support isn't on the top of my TODO list, so it'll have to wait a little.

RE: [PATCH 0/2] fusion SAS support

[Moore, Eric Dean]

On Monday, August 29, 2005 2:01 AM, Christoph Hellwig wrote:

The expander with one crossover cable has been shipped to you
this morning. The crossover should be connected to phy0 on the expander.

> > 
> > /sys/class/sas_port/port-4:3/port-12:0
> > /sys/class/sas_port/port-4:3/port-12:1
> > /sys/class/sas_port/port-4:3/port-12:2
> > /sys/class/sas_port/port-4:3/port-12:3
> > 
> > If so, then the sas_transport code needs to change.
> 
> In the /sys/class/ hierachy they will have to be flat due to the
> way the generic class_device code works.  No it's still open whether
> we will have a flat hierachy of class_devices or whether we'll attach
> extender ports to the struct device of the "parent" port.  I'll prefer
> the former because it keeps the hierachy simpler and mirrors what is
> done in FibreChannel, James prefers the latter.  I'll 
> probably prototype
> both.
> 


Does flat means all sas hbas and expanders would reside in
/sys/class/sas_port ??
How would one figure out the topology parent-child relationship
between hbas-expanders?  I think with large topologies having everything
on the same level will be very messy.   


> I think getting the NDA and docs is more important.  

That is on the way being worked. Hopefully very soon.
I have the spec sitting here next to me.
Ask if you have doubts on anything, and I will explain.


> What I really
> want to do is to call scsi_scan_target for every sas device 
> that's a scsi
> target, so that we can support multi-lun devices easily and 
> coherently with
> other SCSI transports.  Now I need to figure out how to properly get
> information out of the Fusion firmware to only report 
> attached sas devices,
> not individual luns, and I need to make sure different sas 
> devices never
> get the same target id, so scsi_scan_device does the right 
> thing.  This
> could mean I need to the work I suggested to Luben in a 
> previous mail to
> make ->id meaningless for SAS and similar transports, or a 
> scsi midlayer
> to fusion target id mapping.
> 

I will explain - in this loop below in your sas patch:

+	if (*phy_counter >= ioc->num_sas_ports) {
+		sas_add_target(ioc->sas_ports[pg0->PhysicalPort],
+				&ioc->sas_attached[pg0->PhysicalPort],
+				pg0->Bus, pg0->TargetID);
+	} else


add this check below - before calling sas_add_target, 
so you will get the unique scans for each valid scsi target. 
The SMP devices and phys entries for the direct hba phys will be ignored: 
 
			if (le32_to_cpu(sasDevicePg0->DeviceInfo) &
			    (MPI_SAS_DEVICE_INFO_SSP_TARGET |
			     MPI_SAS_DEVICE_INFO_STP_TARGET |
			     MPI_SAS_DEVICE_INFO_SATA_DEVICE )) {


Also - I suggest maintaining a link list of devices in 
the driver when sas_add_target is called. 
This having the sas address, and respective HCTL mapping, and 
other properties. Something similar to the function called 
mpt_sas_get_info() in the 3.02.55 driver.

Thus if you decide to make ->id meaningless for SAS(Lubens thread),
the scsi core could merely send sas address, or maybe an object, or handle, 
or what ever you decide.
Thus the driver could map back to the created object/handle when
sas_add_target occured.
Then for queue command, it can take the object/handle, then translate
to internal bus/target mapping so SCSI_IO request can be sent to 
the firmware as its is today.

> 
> 
> > Hot plug support - how is this going to be done. Take a look
> > at mptscsih_hot_plug_worker_thread, that I sent in 3.02.55.
> 
> We'll probably have a workqueue for it in the transport 
> class, but hotplug
> support isn't on the top of my TODO list, so it'll have to 
> wait a little.

What ever you decide.  Just to let you know the sas firmware send
"device_added" and "device_not_responding" events, so we know when
when devices come and go. We also have events for raid volumes
being added and removed on the fly, and the corresponding hidden phys disk
coming and going as well.  The current hotplug workqueue in the driver is
handling
raid and non-raid events for devices coming and going.
Something for you to consider when you get around to looking at hot plug.

Eric Moore