sata_nv,ahci: add the ahci legacy mode support to sata_nv

Add the ahci controller legacy mode support to sata_nv. Move the DIDs of legacy mode from ahci.c to sata_nv.c

The patch base on kernel 2.6.23-rc8

---

diff -uprN -X linux-2.6.23-rc8-vanilla/Documentation/dontdiff linux-2.6.23-rc8-vanilla/drivers/ata/ahci.c linux-2.6.23-rc8/drivers/ata/ahci.c
@@ -434,14 +434,6 @@ static const struct pci_device_id ahci_p
 	{ PCI_VDEVICE(NVIDIA, 0x044d), board_ahci },		/* MCP65 */
 	{ PCI_VDEVICE(NVIDIA, 0x044e), board_ahci },		/* MCP65 */
 	{ PCI_VDEVICE(NVIDIA, 0x044f), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x045c), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x045d), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x045e), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x045f), board_ahci },		/* MCP65 */
-	{ PCI_VDEVICE(NVIDIA, 0x0550), board_ahci },		/* MCP67 */
-	{ PCI_VDEVICE(NVIDIA, 0x0551), board_ahci },		/* MCP67 */
-	{ PCI_VDEVICE(NVIDIA, 0x0552), board_ahci },		/* MCP67 */
-	{ PCI_VDEVICE(NVIDIA, 0x0553), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x0554), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x0555), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x0556), board_ahci },		/* MCP67 */

@@ -450,10 +442,6 @@ static const struct pci_device_id ahci_p
 	{ PCI_VDEVICE(NVIDIA, 0x0559), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x055a), board_ahci },		/* MCP67 */
 	{ PCI_VDEVICE(NVIDIA, 0x055b), board_ahci },		/* MCP67 */
-	{ PCI_VDEVICE(NVIDIA, 0x07f0), board_ahci },		/* MCP73 */
-	{ PCI_VDEVICE(NVIDIA, 0x07f1), board_ahci },		/* MCP73 */
-	{ PCI_VDEVICE(NVIDIA, 0x07f2), board_ahci },		/* MCP73 */
-	{ PCI_VDEVICE(NVIDIA, 0x07f3), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07f4), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07f5), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07f6), board_ahci },		/* MCP73 */

@@ -462,10 +450,6 @@ static const struct pci_device_id ahci_p
 	{ PCI_VDEVICE(NVIDIA, 0x07f9), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07fa), board_ahci },		/* MCP73 */
 	{ PCI_VDEVICE(NVIDIA, 0x07fb), board_ahci },		/* MCP73 */
-	{ PCI_VDEVICE(NVIDIA, 0x0ad0), board_ahci },		/* MCP77 */
-	{ PCI_VDEVICE(NVIDIA, 0x0ad1), board_ahci },		/* MCP77 */
-	{ PCI_VDEVICE(NVIDIA, 0x0ad2), board_ahci },		/* MCP77 */
-	{ PCI_VDEVICE(NVIDIA, 0x0ad3), board_ahci },		/* MCP77 */
 	{ PCI_VDEVICE(NVIDIA, 0x0ad4), board_ahci },		/* MCP77 */
 	{ PCI_VDEVICE(NVIDIA, 0x0ad5), board_ahci },		/* MCP77 */
 	{ PCI_VDEVICE(NVIDIA, 0x0ad6), board_ahci },		/* MCP77 */

-

--------------
Peer Chen
2007-09-25

I don't understand why these are being moved?

If an interface can be driven via the AHCI driver, that is greatly preferred over the sata_nv driver.

Jeff

We have three mode for one controller - IDE/RAID/AHCI, we want sata_nv being load when user select the IDE mode in BIOS, load ahci driver if RAID/AHCI being selected, which will verify if our legacy mode work well and have additional option if there is any bug for the ahci mode.

----------------
Peer Chen
2007-09-25

-------------------------------------------------------------
·¢¼þÈË£ºJeff Garzik
·¢ËÍÈÕÆÚ£º2007-09-25 15:08:45
ÊÕ¼þÈË£ºPeer Chen
³­ËÍ£ºlinux-kernel; linux-ide; akpm
Ö÷Ì⣺Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

I don't understand why these are being moved?

If an interface can be driven via the AHCI driver, that is greatly preferred over the sata_nv driver.

Jeff

I understand that logic, but look at what happens in practice:

1) User installs new OS in AHCI mode. Distro updates initramfs (loaded at kernel boot time, with boot drivers) to include ahci driver.
2) User reboots into BIOS setup, and switches from AHCI mode to IDE mode.
3) BIOS setup reboots computer.
4) OS kernel and initramfs image are loaded. ahci driver load fails.
5) User is left without a bootable system.

The same situation happens in reverse, if you install in IDE mode (sata_nv in initramfs), and then switch to AHCI/RAID mode.

Additionally, AHCI provides better performance and more direct exposure to the SATA frames. This is key for supporting many modern SATA features that cannot be accessed via IDE legacy mode. AHCI lacks in-silicon simulation of an IDE interface, which time has shown is a less stable, edge-case-prone approach to SATA.

I do not find the "verify nvidia's legacy mode works" argument compelling; that is not the kernel's job, nor the user's. And if there is an AHCI silicon bug, let us deal with that when such a bug appears.

Overall, AFAICS this patch -introduces- new ways for the user to easily render their systems unbootable.

Jeff

Yes,I hear what you are saying but user should know what they are setting in BIOS,there are lots of ways to change the BIOS setting result in unbootable system not only change AHCI/IDE mode. If they encounter booting failure after changing the BIOS setting,they should restore it. Using legacy driver for legacy mode won't affect user to enjoy the feature of AHCI,just select AHCI/RAID mode will ok.
As I know, Intel did it in the same way,and I think it's reasonable.

------------------
Peer Chen
2007-09-25

-------------------------------------------------------------
·¢¼þÈË£ºJeff Garzik
·¢ËÍÈÕÆÚ£º2007-09-25 16:13:52
ÊÕ¼þÈË£ºPeer Chen
³­ËÍ£ºlinux-kernel; linux-ide; akpm
Ö÷Ì⣺Re: [PATCH] sata_nv,ahci: add the ahci legacy mode support to sata_nv

I understand that logic, but look at what happens in practice:

1) User installs new OS in AHCI mode. Distro updates initramfs (loaded at kernel boot time, with boot drivers) to include ahci driver.
2) User reboots into BIOS setup, and switches from AHCI mode to IDE mode.
3) BIOS setup reboots computer.
4) OS kernel and initramfs image are loaded. ahci driver load fails.
5) User is left without a bootable system.

The same situation happens in reverse, if you install in IDE mode (sata_nv in initramfs), and then switch to AHCI/RAID mode.

Additionally, AHCI provides better performance and more direct exposure to the SATA frames. This is key for supporting many modern SATA features that cannot be accessed via IDE legacy mode. AHCI lacks in-silicon simulation of an IDE interface, which time has shown is a less stable, edge-case-prone approach to SATA.

I do not find the "verify nvidia's legacy mode works" argument compelling; that is not the kernel's job, nor the user's. And if there is an AHCI silicon bug, let us deal with that when such a bug appears.

Overall, AFAICS this patch -introduces- new ways for the user to easily render their systems unbootable.

Jeff

Would you mind checking libata-dev.git#upstream, and make sure it has all the NV patches?

I'm thinking I should go ahead and push the 'nv-swncq' branch, which contains the sata_nv updates for swncq. They have been in -mm for a while. I am leaning towards leaving the default to 'off' for 2.6.24 though.

Comments welcome...

Jeff

I hope one of the following patches can be merged to 2.6.24.
==========================
http://lkml.org/lkml/2007/10/8/93
http://lkml.org/lkml/2007/9/25/20

Yes, I agree to set the 'swncq' as default for 2.6.24, after all, for our server customers, stability is far more important than the new feature no matter the problem is caused by drive or controller.

2007/10/13, Jeff Garzik <[email protected]>:

Unfortunately I do not feel like this is the right course of action.

Experience from Intel platforms tells us that our users get very unhappy when their silicon supports AHCI mode, but they are forced into using a less-performant mode. A popular example is an <unnamed> OEM whose BIOS had no method whatsoever for enabling AHCI -- didn't even program the PCI BAR -- even though tests showed the AHCI mode worked just fine when manually programmed.

AHCI is more likely to provide a /stable/ Serial ATA experience, because the silicon deals primarily with sending and receiving FIS's, and not much else. In constrast, experience has shown the legacy IDE interface to be a less reliable method of SATA support. And certainly AHCI is much, much faster with less per-command overhead.

Given that AHCI is both faster and more stable, I feel it is the best policy to enable AHCI when the hardware supports it, regardless of PCI class code (IDE, SATA, or RAID).

Agreed. Done!

Jeff

Ok,I agree to use AHCI driver for our AHCI controllers no matter their class codes are IDE/RAID/AHCI. But for those new or upcoming AHCI controller which DIDs are not included in ahci.c and also IDE/RAID mode being set in BIOS, no driver will be loaded currently, so I hope the first patch http://lkml.org/lkml/2007/10/8/93 can be appied for this case. Any comments?

2007/10/19, Jeff Garzik <[email protected]>:

hmmm is that the correct URL? That one updates sata_nv to support AHCI controllers?.

I would think you would want to add the RAID class code to ahci.c instead? And just some regular PCI IDs?

Jeff

Yes, link - http://lkml.org/lkml/2007/10/8/93 add the AHCI legacy support to sata_nv when IDE/RAID mode been set in SBIOS and Device IDs are not in ahci.c at this moment. To do so, when a new chipset come out and DIDs haven't been submited to LKML,user still can use ahci driver to handle it when setting AHCI mode in BIOS, using sata_nv when setting RAID/IDE in BIOS.
For example, ahci driver in Fedora 7 doesn't include the DIDs of MCP78, so if you set the IDE/RAID mode in BIOS, os installation onto SATA drive will fail. But if Fedora 7 include this patch, installation will be successful.

2007/10/19, Jeff Garzik <[email protected]>:

The problem with this change is that sata_nv picks it up through the default entry, and then later on, ahci picks it up as well.

Rather than the confusion of "which driver is the user running on this hardware?" it is easier to take steps to ensure the user is always running in AHCI mode.

Therefore, I think the best idea is to follow the same policy as drivers/net/forcedeth.c (Ayaz @ NVIDIA) and Intel AHCI, and simply provide the individual PCI IDs in advance of product release.

If you wish to avoid this, then take steps to make sure drivers/ata/ahci.c has the appropriate wildcard entry in its pci_device_id table.

Really, look at the sata_nv bug reports that keep popping up. We want to move away from that driver long term, and always drive the SATA interface in AHCI mode.

The proposed sata_nv patch does the opposite -- guarantees we must support the continually problematic legacy IDE interface ad infinitum. Such patches are OK for the test lab, but in this specific case users /suffer/ when not running AHCI mode.

Jeff

Just to reinforce...

sata_nv support and bug fixes are primarily done right now through the valiant efforts of Robert Hancock (with assists from Alan, Tejun, and others).

Robert's job is difficult, because he has no hardware documentation[1], and NVIDIA does not seem to be helping out much with driver bug reports on the lists or in bugzillas.

As far as I know, I am the only one in the universe outside of NVIDIA with any SATA docs at all, and those docs _only_ cover ADMA registers and DMA structures, no PCI config info, no errata, nothing on SWNCQ or legacy IDE (well, half a page).

NVIDIA has indeed become more engaged in sata_nv in recent times, and that's a positive sign. You, Kuon and Ayaz have all been noticeably more responsive in email. Thanks. Users have definitely benefited, particularly from your help addressing a couple SWNCQ issues.

But at this point in time, being asked to choose between sata_nv and ahci is no choice at all. One has public documentation, wide industry support and little-or-no bugs. The other has several open issues, no documentation, and support obstacles.

Jeff

[1] Robert, please correct me if I'm wrong...

Right, I don't have anything. Unless the original incomplete ADMA driver release from NVIDIA counts as documentation, lol.

And yes, I've CC'ed NVIDIA people about a few ADMA-related issues and been met with silence. It would be nice if they were as responsive about ADMA issues as I must say Kuan and Peer have been on the SWNCQ side of things..

They're not even equivalent interfaces in this case, in the proposed AHCI legacy mode patch these controllers are supported in the default SFF mode only, no ADMA or SWNCQ, so you don't get any NCQ support..

Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/

hi,
The below error happens when i rmmod sata_nv in adma mode on ck804 chipset with 2.6.24 kernel.
I traced the code and found that the driver attempts to write device mem that has been unmapped.

Only simply removing the code" writew(0, mmio + NV_ADMA_CTL);" in the nv_adma_port_stop function or remove .port_stop field in nv_adma_ops, rmmod is ok.

static void nv_adma_port_stop(struct ata_port *ap)

{
	struct nv_adma_port_priv *pp = ap->private_data;
	void __iomem *mmio = pp->ctl_block;

	VPRINTK("ENTER\n");

- writew(0, mmio + NV_ADMA_CTL);
}

Or
Place pcim_iomap_regions before ata_pci_prepare_native_host in
nv_init_one function.
This can guarantee that the code "writew(0, mmio + NV_ADMA_CTL) " write device mem before the device mem is unmapped.

messages:

Dec 19 02:23:24 localhost kernel: ata16.00: disabled
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Synchronizing SCSI
cache
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Result:
hostbyte=0x04 driverbyte=0x00
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Stopping disk
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] START_STOP FAILED
Dec 19 02:23:24 localhost kernel: sd 15:0:0:0: [sda] Result:
hostbyte=0x04 driverbyte=0x00
Dec 19 02:23:24 localhost kernel: BUG: unable to handle kernel paging
request at virtual address e881a4c0
Dec 19 02:23:24 localhost kernel: printing eip: e88224e9 *pde = 0180c067
*pte = 00000000 
Dec 19 02:23:24 localhost kernel: Oops: 0002 [#1] SMP 
Dec 19 02:23:24 localhost kernel: Modules linked in: sata_nv libata
Dec 19 02:23:24 localhost kernel: 
Dec 19 02:23:24 localhost kernel: Pid: 4915, comm: rmmod Not tainted
(2.6.24-rc1-ge2e031eb #2)
Dec 19 02:23:24 localhost kernel: EIP: 0060:[<e88224e9>] EFLAGS:
00210292 CPU: 1
Dec 19 02:23:24 localhost kernel: EIP is at nv_adma_port_stop+0x3d/0x53
[sata_nv]
Dec 19 02:23:24 localhost kernel: EAX: 00000015 EBX: e881a480 ECX:
00200046 EDX: 00200046
Dec 19 02:23:24 localhost kernel: ESI: c220b50c EDI: c1979154 EBP:
c197904c ESP: c2429ecc
Dec 19 02:23:24 localhost kernel:  DS: 007b ES: 007b FS: 00d8 GS: 0033
SS: 0068
Dec 19 02:23:24 localhost kernel: Process rmmod (pid: 4915, ti=c2428000
task=c25e5a40 task.ti=c2428000)
Dec 19 02:23:24 localhost kernel: Stack: e8823e1b e881a480 00000000
e884398e c220b500 c18a3ec0 c1979154 00000000 
Dec 19 02:23:24 localhost kernel:        c02818be c1979154 c197904c
0000000d c18a3ec0 c18a3a40 c197904c e88259d0 
Dec 19 02:23:24 localhost kernel:        e88259d0 c2428000 c0281987
00200286 c197904c c027f8bb c1834600 c197904c 
Dec 19 02:23:24 localhost kernel: Call Trace:
Dec 19 02:23:24 localhost kernel:  [<e884398e>]
ata_host_release+0x2f/0x92 [libata]
Dec 19 02:23:24 localhost kernel:  [<c02818be>]
release_nodes+0x10f/0x12f
Dec 19 02:23:24 localhost kernel:  [<c0281987>]
devres_release_all+0x27/0x2a
Dec 19 02:23:24 localhost kernel:  [<c027f8bb>]
__device_release_driver+0x72/0x88
Dec 19 02:23:24 localhost kernel:  [<c027fccb>] driver_detach+0x76/0xb3
Dec 19 02:23:24 localhost kernel:  [<c027f49f>]
bus_remove_driver+0x63/0x81
Dec 19 02:23:24 localhost kernel:  [<c0228b5a>]
pci_unregister_driver+0xc/0x57
Dec 19 02:23:24 localhost kernel:  [<c0143792>]
sys_delete_module+0x197/0x1bd
Dec 19 02:23:24 localhost kernel:  [<c0417673>]
do_page_fault+0x202/0x581
Dec 19 02:23:24 localhost kernel:  [<c01570ec>] do_munmap+0x193/0x1ac
Dec 19 02:23:24 localhost kernel:  [<c0104e4e>]
sysenter_past_esp+0x5f/0x85
Dec 19 02:23:24 localhost kernel:  =======================
Dec 19 02:23:24 localhost kernel: Code: 3e 82 e8 e8 b8 30 90 d7 89 5c 24
04 c7 04 24 0f 3e 82 e8 e8 a8 30 90 d7 8b 5b 10 c7 04 24 1b 3e 82 e8 89
5c 24 04 e8 95 30 90 d7 <66> c7 43 40 00 00 c7 04 24 29 3e 82 e8 e8 83
30 90 d7 5b 58 5b 
Dec 19 02:23:24 localhost kernel: EIP: [<e88224e9>]
nv_adma_port_stop+0x3d/0x53 [sata_nv] SS:ESP 0068:c2429ecc
-----------------------------------------------------------------------------------

This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Which kernel version are you using? The following commit should have fixed the problem. Please give a shot at 2.6.24-rc5. Thanks.

commit 32ebbc0c0d5d18c0135b55d1eb0029f48c54aff0

Author: Tejun Heo <[email protected]>
Date:   Thu Nov 8 13:09:00 2007 +0900

libata: port and host should be stopped before hardware resources are released

tejun

I have seen your commit. And i apologize for not updating the newest kernel.
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------