[ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo]

Hello, all.

It took a lot longer than I thought, but it's ready now.

http://htj.dyndns.org/git/?p=libata-tj.git
git://htj.dyndns.org/libata-tj

The git tree contains ~120 commits from #upstream[1] and tagged as follows.

#libata-eh-prep		various prep patches for things to follow
#libata-eh-fw		new EH framework
#libata-eh		new EH
#libata-eh-merge-irq-pio merge irq-pio
#libata-ncq		NCQ support
#libata-hp-prep		prep for hotplug
#libata-hp-LLDD-prep	prep LLDDs for hotplug
#libata-hp		hotplug
#libata-link		introduce ata_link
#libata-pm-prep		prep for Port Multiplier support
#libata-pm		Port Multiplier support

I'll post above as patchsets later today with descriptions about what 
has changed since last post.

The following drivers support new features.

ata_piix:		new EH, warmplug
sata_sil:		new EH, hotplug
ahci:			new EH, NCQ, hotplug
sata_sil24:		new EH, NCQ, hotplug, Port Multiplier

The only Port Multiplier I've got is sil4726 and thus it's the only one 
tested.  I implemented AHCI PM support half-way but then realized I 
don't have any AHCI controller which supports PM (I have ICH7R).  Is 
there any AHCI controller which reports PM support?

In general, EH, NCQ and hotplug work great with Port Multiplier. 
However, there are some hardware issues.

* sil3124/32 don't have SNotification, so if devices with active 
commands are yanked out.  There's no way to abort the commands directly. 
  We have to wait till timeout.  Not so great but not so bad either.

* some drives lock up completely if it gets reset while commands are in 
progress.  Due to PM DMA CS errata, sometimes the controller is forced 
to reset the whole controller/PM on error detection causing this 
problem.  Well, nothing much can be done about it and recent drives 
don't seem to have such problems.

* sil3124 doesn't seem to detect R_ERR on command issue as well as 
sil3132.  If somethings goes wrong during EH, 3124 tends to timeout 
rather than quickly fail as 3132 does resulting in much longer recovery 
time.  However, I think this can be resolved by tuning timeouts used 
during EH.

* sil3124/32 reacts weirdly on ATAPI CHECK SENSE if the device is 
attached via PM and commands are active to other devices on the PM.  It 
seems that PM DMA CS errata doesn't cover all.  Problems occur even if 
only one other device is active.  This is worked around by executing all 
ATAPI commands by themselves.  So, if you connect ATAPI device to PM and 
access it, other devices sharing the PM will suffer.

I complained a lot but, all in all, sil3124/32 + sil4726 work greatly.

Albert, can you please verify that I didn't screw up during irq-pio 
merge?  It's done in the following two commits.

516fb4d37fa8648d4f7c9358c7dca22758a49c55 (actual merge)
1bb4c2efb606a5b746997b0ceabbdadbda5d3f26 (followup fixes)

Thanks.

-- 
tejun

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[zhao, forrest]

On Wed, 2006-05-10 at 10:44 +0900, Tejun Heo wrote:

> The only Port Multiplier I've got is sil4726 and thus it's the only one 
> tested.  I implemented AHCI PM support half-way but then realized I 
> don't have any AHCI controller which supports PM (I have ICH7R).  Is 
> there any AHCI controller which reports PM support?
> 
Tejun,

There're some machines in our lab, which support PM. The IO controller
on these machines is ICH7 family. But we have no port multiplier 
hardware, which connect multiple SATA drives to a port on SATA
controller. Do you know where we can buy a port multiplier card?
Is it possible to conduct the PM test without this PM card?

Thanks,
Forrest

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo]

Hello, Zhao, Forrest.

zhao, forrest wrote:
> On Wed, 2006-05-10 at 10:44 +0900, Tejun Heo wrote:
> 
>> The only Port Multiplier I've got is sil4726 and thus it's the only one 
>> tested.  I implemented AHCI PM support half-way but then realized I 
>> don't have any AHCI controller which supports PM (I have ICH7R).  Is 
>> there any AHCI controller which reports PM support?
>>
> Tejun,
> 
> There're some machines in our lab, which support PM. The IO controller
> on these machines is ICH7 family. But we have no port multiplier 

Does your ICH7 report 1 for the SMP bit (bit 17 of the CAP register)?
Cool.  How come mine doesn't?  :/  Can you tell me the exact name of the
product?

> hardware, which connect multiple SATA drives to a port on SATA
> controller. Do you know where we can buy a port multiplier card?

I couldn't buy one either.  Silicon Image guys were nice enough to send
me a sample board.  So, that's what I've worked on.  There seems to be
some external storage products incorporating PM, but I'm not sure about
the availability.

> Is it possible to conduct the PM test without this PM card?

No, I don't think that's possible in any meaningful way.

Zhao, I think I've ironed out all the problems you've reported with the
previous revision of hotplug.  If you've got some time, can you please
check whether those problems are really gone?

Thanks.

-- 
tejun

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[zhao, forrest]

On Wed, 2006-05-10 at 14:17 +0900, Tejun Heo wrote:

> Does your ICH7 report 1 for the SMP bit (bit 17 of the CAP register)?
> Cool.  How come mine doesn't?  :/  Can you tell me the exact name of the
> product?
Yes, PMP bit is set. let me post the dmesg output here:

ahci 0000:00:1f.2: AHCI 0001.0000 32 slots 6 ports ? Gbps 0x3f impl SATA
mode
ahci 0000:00:1f.2: flags: 64bit stag led pmp
ata1: SATA max UDMA/133 cmd 0xFFFFC20000002100 ctl 0x0 bmdma 0x0 irq 201
ata2: SATA max UDMA/133 cmd 0xFFFFC20000002180 ctl 0x0 bmdma 0x0 irq 201
ata3: SATA max UDMA/133 cmd 0xFFFFC20000002200 ctl 0x0 bmdma 0x0 irq 201
ata4: SATA max UDMA/133 cmd 0xFFFFC20000002280 ctl 0x0 bmdma 0x0 irq 201
ata5: SATA max UDMA/133 cmd 0xFFFFC20000002300 ctl 0x0 bmdma 0x0 irq 201
ata6: SATA max UDMA/133 cmd 0xFFFFC20000002380 ctl 0x0 bmdma 0x0 irq 201
ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata1: dev 0 cfg 49:2f00 82:346b 83:7d01 84:4003 85:3469 86:3c01 87:4003
88:007f
ata1: dev 0 ATA-6, max UDMA/133, 234441648 sectors: LBA48 NCQ (depth
0/32)
ata1: dev 0 configured for UDMA/133

The platform name is bensley, and the chipset is ESB2 instead of ICH7
(sorry for my mistake in last mail), the output of lspci is:

00:1f.2 Class 0106: Intel Corporation Enterprise Southbridge SATA
cc=AHCI (rev 08)

> Zhao, I think I've ironed out all the problems you've reported with the
> previous revision of hotplug.  If you've got some time, can you please
> check whether those problems are really gone?
> 
Yes, I'm preparing for the next round of test for your updated patches.

Thanks,
Forrest

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Raz Ben-Jehuda(caro)]

tejun hello
can u estimate what mainstream kernel version ( kernel.org)  would
contain these commits ?

thank you
raz.

On 5/10/06, Tejun Heo <htejun@gmail.com> wrote:
> Hello, all.
>
> It took a lot longer than I thought, but it's ready now.
>
> http://htj.dyndns.org/git/?p=libata-tj.git
> git://htj.dyndns.org/libata-tj
>
> The git tree contains ~120 commits from #upstream[1] and tagged as follows.
>
> #libata-eh-prep         various prep patches for things to follow
> #libata-eh-fw           new EH framework
> #libata-eh              new EH
> #libata-eh-merge-irq-pio merge irq-pio
> #libata-ncq             NCQ support
> #libata-hp-prep         prep for hotplug
> #libata-hp-LLDD-prep    prep LLDDs for hotplug
> #libata-hp              hotplug
> #libata-link            introduce ata_link
> #libata-pm-prep         prep for Port Multiplier support
> #libata-pm              Port Multiplier support
>
> I'll post above as patchsets later today with descriptions about what
> has changed since last post.
>
> The following drivers support new features.
>
> ata_piix:               new EH, warmplug
> sata_sil:               new EH, hotplug
> ahci:                   new EH, NCQ, hotplug
> sata_sil24:             new EH, NCQ, hotplug, Port Multiplier
>
> The only Port Multiplier I've got is sil4726 and thus it's the only one
> tested.  I implemented AHCI PM support half-way but then realized I
> don't have any AHCI controller which supports PM (I have ICH7R).  Is
> there any AHCI controller which reports PM support?
>
> In general, EH, NCQ and hotplug work great with Port Multiplier.
> However, there are some hardware issues.
>
> * sil3124/32 don't have SNotification, so if devices with active
> commands are yanked out.  There's no way to abort the commands directly.
>   We have to wait till timeout.  Not so great but not so bad either.
>
> * some drives lock up completely if it gets reset while commands are in
> progress.  Due to PM DMA CS errata, sometimes the controller is forced
> to reset the whole controller/PM on error detection causing this
> problem.  Well, nothing much can be done about it and recent drives
> don't seem to have such problems.
>
> * sil3124 doesn't seem to detect R_ERR on command issue as well as
> sil3132.  If somethings goes wrong during EH, 3124 tends to timeout
> rather than quickly fail as 3132 does resulting in much longer recovery
> time.  However, I think this can be resolved by tuning timeouts used
> during EH.
>
> * sil3124/32 reacts weirdly on ATAPI CHECK SENSE if the device is
> attached via PM and commands are active to other devices on the PM.  It
> seems that PM DMA CS errata doesn't cover all.  Problems occur even if
> only one other device is active.  This is worked around by executing all
> ATAPI commands by themselves.  So, if you connect ATAPI device to PM and
> access it, other devices sharing the PM will suffer.
>
> I complained a lot but, all in all, sil3124/32 + sil4726 work greatly.
>
> Albert, can you please verify that I didn't screw up during irq-pio
> merge?  It's done in the following two commits.
>
> 516fb4d37fa8648d4f7c9358c7dca22758a49c55 (actual merge)
> 1bb4c2efb606a5b746997b0ceabbdadbda5d3f26 (followup fixes)
>
> Thanks.
>
> --
> tejun
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


--
Raz

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo]

Raz Ben-Jehuda(caro) wrote:
> tejun hello
> can u estimate what mainstream kernel version ( kernel.org)  would
> contain these commits ?
> 

Probably 2.6.18.  In the meantime, I'm thinking about making patches 
available for stable series.

Thanks.

-- 
tejun

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Shem Multinymous]

On 5/10/06, Tejun Heo <htejun@gmail.com> wrote:
> git://htj.dyndns.org/libata-tj

With this kernel tree, ata_piix warmplug works perfectly with ICH6 on
ThinkPad T43.
At long last, I can now switch between the UltraBay DVD drive and
UltraBay HDD without rebooting!

On this machine the command for disabling the port is
# echo 1 > /sys/class/scsi_device/1\:0\:0\:0/device/delete
and the command for enabling and re-scanning is
# echo 0 0 0 >  /sys/class/scsi_host/host1/scan

Thanks for the excellent work!

  Shem

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Shem Multinymous]

Hi,

For the record, lest anyone burns his laptop:

On 5/10/06, Shem Multinymous <multinymous@gmail.com> wrote:
> With this kernel tree, ata_piix warmplug works perfectly with ICH6 on
> ThinkPad T43.
> At long last, I can now switch between the UltraBay DVD drive and
> UltraBay HDD without rebooting!
>
> On this machine the command for disabling the port is
> # echo 1 > /sys/class/scsi_device/1\:0\:0\:0/device/delete

On ThinkPads, before pulling out the drive you also need to run
# echo eject >  /proc/acpi/ibm/bay
to electrically shut down the UltraBay PATA port. This interface is
provided by the ibm-acpi module.

  Shem

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[zhao, forrest]

On Wed, 2006-05-10 at 10:44 +0900, Tejun Heo wrote:
> Hello, all.
Tejun,

I have comments about definition of macro ata_link_for_each_dev(dev,
link) and struct ata_port{};.

In the definition of struct ata_port{}, there's
......
struct ata_link link;
struct ata_device __dev1;
......

Then macro ata_link_for_each_dev() assumes that the field 'device' in
struct ata_link is adjacent to field '__dev1' in struct ata_port.

I think this assumption is not correct in theory. Because the alignment
may make these two fields not adjacent in memory.

Although we haven't found the problem so far, it's very dangerous to
have such assumption in the code.

Does this make sense to you?

Thanks,
Forrest

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo]

Hi, Zhao.

zhao, forrest wrote:
> On Wed, 2006-05-10 at 10:44 +0900, Tejun Heo wrote:
>> Hello, all.
> Tejun,
> 
> I have comments about definition of macro ata_link_for_each_dev(dev,
> link) and struct ata_port{};.
> 
> In the definition of struct ata_port{}, there's
> ......
> struct ata_link link;
> struct ata_device __dev1;
> ......
> 
> Then macro ata_link_for_each_dev() assumes that the field 'device' in
> struct ata_link is adjacent to field '__dev1' in struct ata_port.
> 
> I think this assumption is not correct in theory. Because the alignment
> may make these two fields not adjacent in memory.
> 
> Although we haven't found the problem so far, it's very dangerous to
> have such assumption in the code.

Well, I think the technique is pretty widespread w/ flexible array
member, which is C99 standard and before C99 GCC had zero length array
for the same purpose.  e.g.

struct asdf {
	int nr_entries;
	void *ar[];
};

struct asdf *p;

p = kmalloc(sizeof(*p) + nr * sizeof(p->ar[0], GFP_KERNEL);
if (p)
	p->nr_entries = nr;

This works because alignof(outer struct) >= alignof(nested struct).  So,
we might end up allocating a few extra bytes due to alignment
requirements but that's perfectly okay.

__dev1 is the same.  It might or might not be allocated right after
link->dev[0] but it's guaranteed that &link->dev[1] <= &link->__dev1.
So, no problem there.  I'm pretty sure that this kind of technique is
used in other parts of the kernel too.

-- 
tejun

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Jeff Garzik]

Tejun Heo wrote:
> tested.  I implemented AHCI PM support half-way but then realized I 
> don't have any AHCI controller which supports PM (I have ICH7R).  Is 
> there any AHCI controller which reports PM support?

You can always use SCR-based PM support...

	Jeff

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> tested.  I implemented AHCI PM support half-way but then realized I 
>> don't have any AHCI controller which supports PM (I have ICH7R).  Is 
>> there any AHCI controller which reports PM support?
> 
> You can always use SCR-based PM support...
> 

Can you elaborate a bit?

@ welcome back.

-- 
tejun

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> Tejun Heo wrote:
>>> tested.  I implemented AHCI PM support half-way but then realized I 
>>> don't have any AHCI controller which supports PM (I have ICH7R).  Is 
>>> there any AHCI controller which reports PM support?
>>
>> You can always use SCR-based PM support...
>>
> 
> Can you elaborate a bit?

Sections A.1 and 6.1 of the PM spec.  You set the PMP field of the 
SControl register.  This has obvious parallelism limitations, which is 
why "hardware context switching support" aka native PM support is 
greatly preferred.


> @ welcome back.

Thanks!  :)

	Jeff, learning to type with a ring on the finger

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Jeff Garzik wrote:
>>> Tejun Heo wrote:
>>>> tested.  I implemented AHCI PM support half-way but then realized I 
>>>> don't have any AHCI controller which supports PM (I have ICH7R).  Is 
>>>> there any AHCI controller which reports PM support?
>>>
>>> You can always use SCR-based PM support...
>>>
>>
>> Can you elaborate a bit?
> 
> Sections A.1 and 6.1 of the PM spec.  You set the PMP field of the 
> SControl register.  This has obvious parallelism limitations, which is 
> why "hardware context switching support" aka native PM support is 
> greatly preferred.
> 

That cmd-switching PM support is what AHCI SPM bit indicates.  As of 
v1.1, AHCI doesn't define FIS-switching PM support yet.

Supports Port Multiplier (SPM): Indicates whether the HBA can support a 
Port Multiplier. When set, a Port Multiplier using command-based 
switching is supported. When cleared to ‘0’, a Port Multiplier is not 
supported, and a Port Multiplier may not be attached to this HBA.

And. the SControl section says...

19:16 RO 0h  Port Multiplier Port (PMP): This field is not used by AHCI.

Which is understandable considering AHCI doesn't construct FISes itself. 
  I don't think PM requires more hardware support than being capable for 
setting PMP in the FISes, which AHCI certainly can (standing down on 
collision for SDB notify aside).  I can give it a shot but it clearly 
violates the spec, and even if it works, we would need to print big fat 
warning message while attaching PM.

-- 
tejun

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Jeff Garzik]

Tejun Heo wrote:
> That cmd-switching PM support is what AHCI SPM bit indicates.  As of 
> v1.1, AHCI doesn't define FIS-switching PM support yet.

Right you are; see, told you my brain was frazzled.


> Which is understandable considering AHCI doesn't construct FISes itself. 
>  I don't think PM requires more hardware support than being capable for 
> setting PMP in the FISes, which AHCI certainly can (standing down on 
> collision for SDB notify aside).  I can give it a shot but it clearly 
> violates the spec, and even if it works, we would need to print big fat 
> warning message while attaching PM.

Setting the PMP field during FIS construction would work, I should 
think, for outgoing FIS's.  Not sure how incoming FISs would behave.

	Jeff

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> That cmd-switching PM support is what AHCI SPM bit indicates.  As of 
>> v1.1, AHCI doesn't define FIS-switching PM support yet.
> 
> Right you are; see, told you my brain was frazzled.
> 
> 
>> Which is understandable considering AHCI doesn't construct FISes 
>> itself.  I don't think PM requires more hardware support than being 
>> capable for setting PMP in the FISes, which AHCI certainly can 
>> (standing down on collision for SDB notify aside).  I can give it a 
>> shot but it clearly violates the spec, and even if it works, we would 
>> need to print big fat warning message while attaching PM.
> 
> Setting the PMP field during FIS construction would work, I should 
> think, for outgoing FIS's.  Not sure how incoming FISs would behave.
> 

I think the incoming part will work too.  All it has to do is ignoring 
it.  SDB notify can also implemented using Unknown FIS interrupt and FIS 
reception area.  I'll give it a shot.  After all, working with big fat 
warning is a lot better than not working, and a LOT of AHCIs are out there.

-- 
tejun

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Stefan Smietanowski]

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

Tejun Heo wrote:
> Jeff Garzik wrote:
> 
>> Tejun Heo wrote:
>>
>>> Jeff Garzik wrote:
>>>
>>>> Tejun Heo wrote:
>>>>
>>>>> tested.  I implemented AHCI PM support half-way but then realized I
>>>>> don't have any AHCI controller which supports PM (I have ICH7R). 
>>>>> Is there any AHCI controller which reports PM support?
>>>>
>>>>
>>>> You can always use SCR-based PM support...
>>>>
>>>
>>> Can you elaborate a bit?
>>
>>
>> Sections A.1 and 6.1 of the PM spec.  You set the PMP field of the
>> SControl register.  This has obvious parallelism limitations, which is
>> why "hardware context switching support" aka native PM support is
>> greatly preferred.
>>
> 
> That cmd-switching PM support is what AHCI SPM bit indicates.  As of
> v1.1, AHCI doesn't define FIS-switching PM support yet.
> 
> Supports Port Multiplier (SPM): Indicates whether the HBA can support a
> Port Multiplier. When set, a Port Multiplier using command-based
> switching is supported. When cleared to ‘0’, a Port Multiplier is not
> supported, and a Port Multiplier may not be attached to this HBA.
> 
> And. the SControl section says...
> 
> 19:16 RO 0h  Port Multiplier Port (PMP): This field is not used by AHCI.
> 
> Which is understandable considering AHCI doesn't construct FISes itself.
>  I don't think PM requires more hardware support than being capable for
> setting PMP in the FISes, which AHCI certainly can (standing down on
> collision for SDB notify aside).  I can give it a shot but it clearly
> violates the spec, and even if it works, we would need to print big fat
> warning message while attaching PM.
> 

Are you saying that the next big thing since sliced bread (AHCI) that's
supposed to unite the world and make everybody sing Kumbayah doesn't
support port multipliers (according to the spec) ?

Or am I totally misreading this.

// Stefan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]

Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo]

Stefan Smietanowski wrote:
>> Which is understandable considering AHCI doesn't construct FISes itself.
>>  I don't think PM requires more hardware support than being capable for
>> setting PMP in the FISes, which AHCI certainly can (standing down on
>> collision for SDB notify aside).  I can give it a shot but it clearly
>> violates the spec, and even if it works, we would need to print big fat
>> warning message while attaching PM.
>>
> 
> Are you saying that the next big thing since sliced bread (AHCI) that's
> supposed to unite the world and make everybody sing Kumbayah doesn't
> support port multipliers (according to the spec) ?
> 
> Or am I totally misreading this.

Heh heh.

I love AHCI too, but as far as PMS is concerned, AHCI is not saving any 
world.  There's a bit called SPM (supports Port Multiplier) in the 
capability register.  If the bit is set, AHCI support PM by 
command-based switching, which ummm... sucks - only one device can have 
active commands at any given time.  AHCI v1.1 (the current spec) lists 
FIS-based switching PMS as 'to be specified in a future AHCI specification'.

To make matters worse, not many AHCI controllers seem to have that bit 
set.  My ICH7R (P5LD2) doesn't and I would be surprised if any of 
ICH6R/M and ICH7M have that bit set.  Zhao reported that ESB2 (server 
chipset w/ 6 SATA ports) have that bit set, so there are some 
controllers supporting PM out there.

However, cmd-switching PMS doesn't require much support from HBA, and I 
don't see why my ICH7R cannot do PMS.  I'm currently giving it a try now.

So, ATM, if you want PMS, sil3124/3132 series is the best choice.

-- 
tejun