libata EH/NCQ/hotplug/PM git tree

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

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

Hello, Zhao, Forrest.

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?

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.

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

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)

Yes, I'm preparing for the next round of test for your updated patches.

Thanks,
Forrest

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

thank you
raz.

--
Raz

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

Thanks.

tejun

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

Hi,

For the record, lest anyone burns his laptop:

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

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

Hi, Zhao.

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

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

Jeff

Can you elaborate a bit?

@ welcome back.

tejun

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.

Thanks! :)

Jeff, learning to type with a ring on the finger

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

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

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

Jeff

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

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

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