2006-11-03 18:54:39

by luugi.marsan

[permalink] [raw]
Subject: [PATCH 1/2] Add Legacy IDE mode support for SB600 SATA

From: [email protected]

ATI SB600 SATA controller supports 4 modes: Legacy IDE, Native IDE, AHCI and RAID.IDE modes are used for compatibility with some old OS without AHCI driver,but now they are not necessary for Linux since the kernel has supported AHCI.Some BIOS set Legacy IDE as SB600 SATA's default mode, but the AHCI driver cannot run in Legacy IDE.So, we should set the controller back to AHCI mode if it has been set as IDE by BIOS.

Signed-off-by: Luugi Marsan <[email protected]>

--- linux-2.6.19-rc4-git5/drivers/pci/quirks.c.orig 2006-11-04 03:50:25.000000000 +0800
+++ linux-2.6.19-rc4-git5/drivers/pci/quirks.c 2006-11-04 03:53:56.000000000 +0800
@@ -796,6 +796,35 @@ static void __init quirk_mediagx_master(
}
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CYRIX, PCI_DEVICE_ID_CYRIX_PCI_MASTER, quirk_mediagx_master );

+
+/*
+ * ATI SB600 SATA controller supports 4 modes: Legacy IDE, Native IDE, AHCI
+ * and RAID.
+ * IDE modes are used for compatibility with some old OS without AHCI driver,
+ * but now they are not necessary for Linux since the kernel has supported
+ * AHCI.
+ * Some BIOS set Legacy IDE as SB600 SATA's default mode, but the AHCI driver
+ * cannot run in Legacy IDE.
+ * So, we should set the controller back to AHCI mode if it has been set as IDE
+ * by BIOS.
+ */
+static void __devinit quirk_sb600_sata(struct pci_dev *pdev)
+{
+ if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE)
+ {
+ u8 tmp;
+
+ pci_read_config_byte(pdev, 0x40, &tmp);
+ pci_write_config_byte(pdev, 0x40, tmp|1);
+ pci_write_config_byte(pdev, 0x9, 1);
+ pci_write_config_byte(pdev, 0xa, 6);
+ pci_write_config_byte(pdev, 0x40, tmp);
+
+ pdev->class = 0x010601;
+ }
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_sb600_sata);
+
/*
* As per PCI spec, ignore base address registers 0-3 of the IDE controllers
* running in Compatible mode (bits 0 and 2 in the ProgIf for primary and


2006-11-03 19:26:08

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add Legacy IDE mode support for SB600 SATA

Ar Gwe, 2006-11-03 am 13:54 -0500, ysgrifennodd Luugi Marsan:
> From: [email protected]
>
> ATI SB600 SATA controller supports 4 modes: Legacy IDE, Native IDE, AHCI and RAID.IDE modes are used for compatibility with some old OS without AHCI driver,but now they are not necessary for Linux since the kernel has supported AHCI.Some BIOS set Legacy IDE as SB600 SATA's default mode, but the AHCI driver cannot run in Legacy IDE.So, we should set the controller back to AHCI mode if it has been set as IDE by BIOS.
>
> Signed-off-by: Luugi Marsan <[email protected]>

NAK

This should only be done if AHCI is configured into the kernel, so wants
a #ifdef check adding. Otherwise people using SB600 via the legacy ide
layer will get burned.

2006-11-05 12:20:29

by Conke Hu

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add Legacy IDE mode support for SB600 SATA

On Fri, 2006-11-03 at 19:30 +0000, Alan Cox wrote:
> Ar Gwe, 2006-11-03 am 13:54 -0500, ysgrifennodd Luugi Marsan:
> > From: [email protected]
> >
> > ATI SB600 SATA controller supports 4 modes: Legacy IDE, Native IDE, AHCI and RAID.IDE modes are used for compatibility with some old OS without AHCI driver,but now they are not necessary for Linux since the kernel has supported AHCI.Some BIOS set Legacy IDE as SB600 SATA's default mode, but the AHCI driver cannot run in Legacy IDE.So, we should set the controller back to AHCI mode if it has been set as IDE by BIOS.
> >
> > Signed-off-by: Luugi Marsan <[email protected]>
>
> NAK
>
> This should only be done if AHCI is configured into the kernel, so wants
> a #ifdef check adding. Otherwise people using SB600 via the legacy ide
> layer will get burned.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>

Hi Alan,
Thank you for your consideration and reply. But there seems to be
some misunderstanding.

1. The SATA configuration option "Legacy IDE mode" (as well as
Native IDE mode) in SB600 BIOS is ONLY for old OS, and it is not useful
any longer for new Linux kernels.

2. We could consider:
original system BIOS + this patch == new BIOS for Linux.
(This will not effect any other OS installed or to be installed on
SB600.)

3. "This should only be done if AHCI is configured into the kernel,
so wants a #ifdef check adding".
Alan, this fix should always be done whether AHCI is configured into
kernel or not, even when AHCI is not configured at all. Because:
a). Without it, the SB600 SATA controller will appear as an IDE,
which may misguide user to try to load legacy IDE driver (or other IDE
driver). For example, if user run command "lspci" he will consider the
controller as an IDE by mistake and maybe he will try to load legacy IDE
driver (or other IDE driver).
b). We have a RAID driver (close source) for SB600 SATA which does
not depends on the open source AHCI driver in linux kernel and supports
both AHCI and RAID. But if the controller is configured as legacy IDE by
BIOS, the RAID driver cannot run at all because of the IRQ policy.

4. "Otherwise people using SB600 via the legacy ide layer will get
burned."
Why? After kernel is fixed by this patch, is there still any legacy
ide layer or ide driver that will touch SB600 SATA controller? (the
class code has been changed to 0x010601, which tells the kernel, "I am
an AHCI 1.0 controller, not an IDE".)



Best regards,
Conke Hu



2006-11-05 12:41:14

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add Legacy IDE mode support for SB600 SATA

Ar Sul, 2006-11-05 am 20:17 +0800, ysgrifennodd Conke Hu:
> 1. The SATA configuration option "Legacy IDE mode" (as well as
> Native IDE mode) in SB600 BIOS is ONLY for old OS, and it is not useful
> any longer for new Linux kernels.

Some users choose to use old drivers for the feeling of security and
reduction of change. Remember there are users out there who have to go
through a formal verification process to switch the driver they use.

> 3. "This should only be done if AHCI is configured into the kernel,
> so wants a #ifdef check adding".
> Alan, this fix should always be done whether AHCI is configured into
> kernel or not, even when AHCI is not configured at all. Because:
> a). Without it, the SB600 SATA controller will appear as an IDE,
> which may misguide user to try to load legacy IDE driver (or other IDE

This is not neccessarily misguided. They may want to do this.

> b). We have a RAID driver (close source) for SB600 SATA

Thats your problem. Some day the lawyers can find out just how legal
that is.

You are right that most users will want to use the AHCI layer and that
if AHCI is compiled in then we should switch to AHCI. In the case the
kernel has no AHCI support compiled in the legacy driver support should
continue to work for it. This is how other vendors products such as
Jmicron are handled.

NAK reasserted.

Alan

2006-11-05 13:10:16

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add Legacy IDE mode support for SB600 SATA

On Sun, 2006-11-05 at 20:17 +0800, Conke Hu wrote:
> b). We have a RAID driver (close source) for SB600 SATA which does
> not depends on the open source AHCI driver in linux kernel and supports
> both AHCI and RAID. But if the controller is configured as legacy IDE by
> BIOS, the RAID driver cannot run at all because of the IRQ policy.

that is your own problem and in fact a very good reason to reject your
change. In linux, device mapper deals with such "raid", and your change
will block that.
Reverse engineering such raid is usually a one day effort, and it will
be done.. and that is a good thing. Trying to make that impossible is
just even dishonest [1].


[1] See Greg Kroah's OLS keynote. It's one thing to think your "raid IP"
is worth more than the Linux kernels IP, it's another one to sabotage
the Linux kernel to protect your "raid IP".


--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

2006-11-05 13:41:16

by Conke Hu

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add Legacy IDE mode support for SB600 SATA

On Sun, 2006-11-05 at 12:45 +0000, Alan Cox wrote:
> Ar Sul, 2006-11-05 am 20:17 +0800, ysgrifennodd Conke Hu:
> > 1. The SATA configuration option "Legacy IDE mode" (as well as
> > Native IDE mode) in SB600 BIOS is ONLY for old OS, and it is not useful
> > any longer for new Linux kernels.
>
> Some users choose to use old drivers for the feeling of security and
> reduction of change. Remember there are users out there who have to go
> through a formal verification process to switch the driver they use.
>
Consider that the behavior itself of using the new motherboard, SB600,
means a lot of change, so even if the user still use IDE driver, he also
a need verification process.

> > 3. "This should only be done if AHCI is configured into the kernel,
> > so wants a #ifdef check adding".
> > Alan, this fix should always be done whether AHCI is configured into
> > kernel or not, even when AHCI is not configured at all. Because:
> > a). Without it, the SB600 SATA controller will appear as an IDE,
> > which may misguide user to try to load legacy IDE driver (or other IDE
>
> This is not neccessarily misguided. They may want to do this.
But the SB600 SATA controller is really an AHCI controller. And that
will lose high performance (and cannot use NCQ).

>
> > b). We have a RAID driver (close source) for SB600 SATA
>
> Thats your problem. Some day the lawyers can find out just how legal
> that is.
No, even if there would be a open source RAID driver, it still could
no run.

>
> You are right that most users will want to use the AHCI layer and that
> if AHCI is compiled in then we should switch to AHCI. In the case the
> kernel has no AHCI support compiled in the legacy driver support should
> continue to work for it. This is how other vendors products such as
> Jmicron are handled.
>
The case of Jmicron is not the same as ours. They use different pci
functions in the SATA controller.

> NAK reasserted.
OK, I am rewriting the patch based on the following considerations:
1. move these code to ahci driver, and add new code to PATA driver.
2. add new options to kernel configuration, and users can choose IDE
driver or AHCI driver to support SB600 SATA when it is in IDE mode.


> Alan
>
At last, thank you again for your opinions and suggestion. if any other
advice, please feel free to contact me.


Best regards,
Conke Hu


2006-11-05 15:04:36

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add Legacy IDE mode support for SB600 SATA

Ar Sul, 2006-11-05 am 21:38 +0800, ysgrifennodd Conke Hu:
> Consider that the behavior itself of using the new motherboard, SB600,
> means a lot of change, so even if the user still use IDE driver, he also
> a need verification process.

It depends on the person but yes I had considered that.

> > This is not neccessarily misguided. They may want to do this.
> But the SB600 SATA controller is really an AHCI controller. And that
> will lose high performance (and cannot use NCQ).

Maybe they want stability and certainty first.

> OK, I am rewriting the patch based on the following considerations:
> 1. move these code to ahci driver, and add new code to PATA driver.
> 2. add new options to kernel configuration, and users can choose IDE
> driver or AHCI driver to support SB600 SATA when it is in IDE mode.

That seems fine to me. I would have thought putting the code you have
into the quirks.c file as you proposed was the better way to do this,
but with the addition of the

#if defined (CONFIG_ATA_AHCI) || defined(CONFIG_ATA_AHCI_MODULE)

#endif

around it, was sufficient ?