[PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Mika Westerberg]

Add Intel Emmitsburg PCH RAID PCI IDs to the list of supported
controllers.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/ata/ahci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 00ba8e5a1ccc..0b39f0e7fd8f 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -398,6 +398,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, 0x2823), board_ahci }, /* Lewisburg AHCI*/
 	{ PCI_VDEVICE(INTEL, 0x2826), board_ahci }, /* Lewisburg RAID*/
 	{ PCI_VDEVICE(INTEL, 0x2827), board_ahci }, /* Lewisburg RAID*/
+	{ PCI_VDEVICE(INTEL, 0x282b), board_ahci }, /* Emmitsburg RAID */
+	{ PCI_VDEVICE(INTEL, 0x282f), board_ahci }, /* Emmitsburg RAID */
 	{ PCI_VDEVICE(INTEL, 0xa182), board_ahci }, /* Lewisburg AHCI*/
 	{ PCI_VDEVICE(INTEL, 0xa186), board_ahci }, /* Lewisburg RAID*/
 	{ PCI_VDEVICE(INTEL, 0xa1d2), board_ahci }, /* Lewisburg RAID*/
-- 
2.29.2

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Christoph Hellwig]

On Thu, Nov 19, 2020 at 01:43:18PM +0300, Mika Westerberg wrote:
> Add Intel Emmitsburg PCH RAID PCI IDs to the list of supported
> controllers.

Stupid question: what would it to get Intel to finally report the
correct classcode after all the time?  The amount of IDs we need to list
is getting ridiculous.

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Mika Westerberg]

On Thu, Nov 19, 2020 at 04:50:22PM +0000, Christoph Hellwig wrote:
> On Thu, Nov 19, 2020 at 01:43:18PM +0300, Mika Westerberg wrote:
> > Add Intel Emmitsburg PCH RAID PCI IDs to the list of supported
> > controllers.
> 
> Stupid question: what would it to get Intel to finally report the
> correct classcode after all the time?  The amount of IDs we need to list
> is getting ridiculous.

What is the correct class code in this case that it works with the AHCI
driver?

I think (not 100% sure) it reports standard AHCI class code when it is
not in RAID mode but these PCI IDs are for the RAID mode.

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Christoph Hellwig]

On Fri, Nov 20, 2020 at 12:53:09PM +0200, Mika Westerberg wrote:
> On Thu, Nov 19, 2020 at 04:50:22PM +0000, Christoph Hellwig wrote:
> > On Thu, Nov 19, 2020 at 01:43:18PM +0300, Mika Westerberg wrote:
> > > Add Intel Emmitsburg PCH RAID PCI IDs to the list of supported
> > > controllers.
> > 
> > Stupid question: what would it to get Intel to finally report the
> > correct classcode after all the time?  The amount of IDs we need to list
> > is getting ridiculous.
> 
> What is the correct class code in this case that it works with the AHCI
> driver?
> 
> I think (not 100% sure) it reports standard AHCI class code when it is
> not in RAID mode but these PCI IDs are for the RAID mode.

The right class code is the AHCI one.  The so called RAID mode doesn't
change the operation of the device at all (except for sometimes hiding
NVMe devices that are a different PCIe function to start with).

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Mika Westerberg]

On Mon, Nov 23, 2020 at 10:09:17AM +0000, Christoph Hellwig wrote:
> On Fri, Nov 20, 2020 at 12:53:09PM +0200, Mika Westerberg wrote:
> > On Thu, Nov 19, 2020 at 04:50:22PM +0000, Christoph Hellwig wrote:
> > > On Thu, Nov 19, 2020 at 01:43:18PM +0300, Mika Westerberg wrote:
> > > > Add Intel Emmitsburg PCH RAID PCI IDs to the list of supported
> > > > controllers.
> > > 
> > > Stupid question: what would it to get Intel to finally report the
> > > correct classcode after all the time?  The amount of IDs we need to list
> > > is getting ridiculous.
> > 
> > What is the correct class code in this case that it works with the AHCI
> > driver?
> > 
> > I think (not 100% sure) it reports standard AHCI class code when it is
> > not in RAID mode but these PCI IDs are for the RAID mode.
> 
> The right class code is the AHCI one.  The so called RAID mode doesn't
> change the operation of the device at all (except for sometimes hiding
> NVMe devices that are a different PCIe function to start with).

Thanks. I looked at the AHCI spec (1.3.1) and it says this regarding the
class code (CC) field:

  Informative Note: For HBAs that support RAID, the Sub Class Code reset
  value should be 04h and the Programming Interface reset value should be
  00h.

I think this is what the controller is doing when in "RAID mode".

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Christoph Hellwig]

On Mon, Nov 23, 2020 at 01:26:22PM +0200, Mika Westerberg wrote:
> On Mon, Nov 23, 2020 at 10:09:17AM +0000, Christoph Hellwig wrote:
> > On Fri, Nov 20, 2020 at 12:53:09PM +0200, Mika Westerberg wrote:
> > > On Thu, Nov 19, 2020 at 04:50:22PM +0000, Christoph Hellwig wrote:
> > > > On Thu, Nov 19, 2020 at 01:43:18PM +0300, Mika Westerberg wrote:
> > > > > Add Intel Emmitsburg PCH RAID PCI IDs to the list of supported
> > > > > controllers.
> > > > 
> > > > Stupid question: what would it to get Intel to finally report the
> > > > correct classcode after all the time?  The amount of IDs we need to list
> > > > is getting ridiculous.
> > > 
> > > What is the correct class code in this case that it works with the AHCI
> > > driver?
> > > 
> > > I think (not 100% sure) it reports standard AHCI class code when it is
> > > not in RAID mode but these PCI IDs are for the RAID mode.
> > 
> > The right class code is the AHCI one.  The so called RAID mode doesn't
> > change the operation of the device at all (except for sometimes hiding
> > NVMe devices that are a different PCIe function to start with).
> 
> Thanks. I looked at the AHCI spec (1.3.1) and it says this regarding the
> class code (CC) field:
> 
>   Informative Note: For HBAs that support RAID, the Sub Class Code reset
>   value should be 04h and the Programming Interface reset value should be
>   00h.
> 
> I think this is what the controller is doing when in "RAID mode".

The point is: these AHCI controllers do not support RAID in form
despite the confusing naming.

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Mika Westerberg]

On Mon, Nov 23, 2020 at 11:38:01AM +0000, Christoph Hellwig wrote:
> The point is: these AHCI controllers do not support RAID in form
> despite the confusing naming.

Are you sure?

I looked at the Emmitsburg data sheet and it actually seems to support
some sort of RAID but it requires some special Rapid Storage Technology
[1] drivers from Intel. Probably the idea is that when in "RAID mode"
the thing, as AHCI spec says, does not announce itself to be AHCI
compliant which makes Windows inbox AHCI driver to not to load, and this
allows the Intel propriatery driver to load that then takes advantage of
this somehow.

I'm not too familiar with these technologies here so I may be missing
something.

[1] https://www.intel.com/content/www/us/en/architecture-and-technology/rapid-storage-technology.html

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Christoph Hellwig]

On Mon, Nov 23, 2020 at 02:28:20PM +0200, Mika Westerberg wrote:
> On Mon, Nov 23, 2020 at 11:38:01AM +0000, Christoph Hellwig wrote:
> > The point is: these AHCI controllers do not support RAID in form
> > despite the confusing naming.
> 
> Are you sure?
> 
> I looked at the Emmitsburg data sheet and it actually seems to support
> some sort of RAID but it requires some special Rapid Storage Technology
> [1] drivers from Intel. Probably the idea is that when in "RAID mode"
> the thing, as AHCI spec says, does not announce itself to be AHCI
> compliant which makes Windows inbox AHCI driver to not to load, and this
> allows the Intel propriatery driver to load that then takes advantage of
> this somehow.

Yes, and that is purely (badly implemented) software RAID.

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Mika Westerberg]

On Mon, Nov 23, 2020 at 12:51:14PM +0000, Christoph Hellwig wrote:
> On Mon, Nov 23, 2020 at 02:28:20PM +0200, Mika Westerberg wrote:
> > On Mon, Nov 23, 2020 at 11:38:01AM +0000, Christoph Hellwig wrote:
> > > The point is: these AHCI controllers do not support RAID in form
> > > despite the confusing naming.
> > 
> > Are you sure?
> > 
> > I looked at the Emmitsburg data sheet and it actually seems to support
> > some sort of RAID but it requires some special Rapid Storage Technology
> > [1] drivers from Intel. Probably the idea is that when in "RAID mode"
> > the thing, as AHCI spec says, does not announce itself to be AHCI
> > compliant which makes Windows inbox AHCI driver to not to load, and this
> > allows the Intel propriatery driver to load that then takes advantage of
> > this somehow.
> 
> Yes, and that is purely (badly implemented) software RAID.

Sorry for the delay. I'm trying to find the AHCI owners at Intel and
forward this request to them. I would not hold my breath though that
anything will change but I'll try :)

Even if that would change, Emmitsburg still needs these PCI IDs so hope
this patch if fine for you.

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Williams, Dan J]

On Mon, 2020-11-23 at 11:38 +0000, Christoph Hellwig wrote:
> On Mon, Nov 23, 2020 at 01:26:22PM +0200, Mika Westerberg wrote:
> > On Mon, Nov 23, 2020 at 10:09:17AM +0000, Christoph Hellwig wrote:
> > > On Fri, Nov 20, 2020 at 12:53:09PM +0200, Mika Westerberg wrote:
> > > > On Thu, Nov 19, 2020 at 04:50:22PM +0000, Christoph Hellwig wrote:
> > > > > On Thu, Nov 19, 2020 at 01:43:18PM +0300, Mika Westerberg wrote:
> > > > > > Add Intel Emmitsburg PCH RAID PCI IDs to the list of supported
> > > > > > controllers.
> > > > > 
> > > > > Stupid question: what would it to get Intel to finally report the
> > > > > correct classcode after all the time?  The amount of IDs we need to list
> > > > > is getting ridiculous.

It turns out the problem is $OTHER_OS wants to load an AHCI driver
based on class code and if RAID mode reused the class code it would
break legacy there, but see below, there is a path to stem the tide of
new ids flowing into this file...

> > > > 
> > > > What is the correct class code in this case that it works with the AHCI
> > > > driver?
> > > > 
> > > > I think (not 100% sure) it reports standard AHCI class code when it is
> > > > not in RAID mode but these PCI IDs are for the RAID mode.
> > > 
> > > The right class code is the AHCI one.  The so called RAID mode doesn't
> > > change the operation of the device at all (except for sometimes hiding
> > > NVMe devices that are a different PCIe function to start with).
> > 
> > Thanks. I looked at the AHCI spec (1.3.1) and it says this regarding the
> > class code (CC) field:
> > 
> >   Informative Note: For HBAs that support RAID, the Sub Class Code reset
> >   value should be 04h and the Programming Interface reset value should be
> >   00h.
> > 
> > I think this is what the controller is doing when in "RAID mode".

Mika, "RAID mode" is an unfortunate misnomer because the hardware is
just standard AHCI programming model, always.

> The point is: these AHCI controllers do not support RAID in form
> despite the confusing naming.

I did some digging on this to both get the history and to push for a
concession that Intel will fix the hardware development pipeline to
stop the pointless churn of "RAID" ids going forward. The history is
equal parts encouraging and unfortunate. The encouraging finding is
that the "RAID" device-ids on "server" platforms has been stable for
several generations and will continue to be stable:

{ PCI_VDEVICE(INTEL, 0x2826), board_ahci }, /* PBG/Lewisburg RAID*/
...
{ PCI_VDEVICE(INTEL, 0x2827), board_ahci }, /* Wellsburg/Lewisburg RAID */

I.e. those RAID-ids also work for Emmitsburg. The unfortunate part of
the story is that those ids only correspond to "SATA0" and "SATA1"
instances of AHCI controllers in "RAID" mode on those platforms. Yes,
the exact same hardware duplicated at different PCI DEVFNs has a
different PCI device-id. Why? Apparently, $OTHER_OS wanted per
controller instance identification. I can only guess to apply per
controller id quirks. However, and this is a subtle detail, that
legacy-requirement only applies to the native "AHCI" mode case. I.e. in
AHCI mode, $OTHER_OS expects per-controller ids in addition to the
class code. For RAID mode, that per-controller id policy was carried
over on server platforms, but that appears to be due only to inertia,
not a requirement that matters in practice.

So what this patch is actually asking is to add another generic server
controller RAID-id but for a third instance of a
 controller in RAID mode. I.e. something like this diff:

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ab5811ef5a53..039f3211eddc 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -324,7 +324,6 @@ static const struct pci_device_id ahci_pci_tbl[] = {
        { PCI_VDEVICE(INTEL, 0x1d02), board_ahci }, /* PBG AHCI */
        { PCI_VDEVICE(INTEL, 0x1d04), board_ahci }, /* PBG RAID */
        { PCI_VDEVICE(INTEL, 0x1d06), board_ahci }, /* PBG RAID */
-       { PCI_VDEVICE(INTEL, 0x2826), board_ahci }, /* PBG/Lewisburg RAID*/
        { PCI_VDEVICE(INTEL, 0x2323), board_ahci }, /* DH89xxCC AHCI */
        { PCI_VDEVICE(INTEL, 0x1e02), board_ahci }, /* Panther Point AHCI */
        { PCI_VDEVICE(INTEL, 0x1e03), board_ahci_mobile }, /* Panther M AHCI */
@@ -367,7 +366,9 @@ static const struct pci_device_id ahci_pci_tbl[] = {
        { PCI_VDEVICE(INTEL, 0x1f3e), board_ahci_avn }, /* Avoton RAID */
        { PCI_VDEVICE(INTEL, 0x1f3f), board_ahci_avn }, /* Avoton RAID */
        { PCI_VDEVICE(INTEL, 0x2823), board_ahci }, /* Wellsburg/Lewisburg AHCI*/
-       { PCI_VDEVICE(INTEL, 0x2827), board_ahci }, /* Wellsburg/Lewisburg RAID*/
+       { PCI_VDEVICE(INTEL, 0x2826), board_ahci }, /* Server SATA0 "RAID" */
+       { PCI_VDEVICE(INTEL, 0x2827), board_ahci }, /* Server SATA1 "RAID" */
+       { PCI_VDEVICE(INTEL, 0x282f), board_ahci }, /* Server SATA2 "RAID" */
        { PCI_VDEVICE(INTEL, 0x43d4), board_ahci }, /* Rocket Lake PCH-H RAID */
        { PCI_VDEVICE(INTEL, 0x43d5), board_ahci }, /* Rocket Lake PCH-H RAID */
        { PCI_VDEVICE(INTEL, 0x43d6), board_ahci }, /* Rocket Lake PCH-H RAID */

I.e. ids that are compatible from one generation to the next.

While it is too late to intercept hardware shipping with this new SATA2
generic-id, the purely theoretical future SATA3+ instances are on
notice to avoid the need to come back and touch this file. I.e. any
additional controller(s) can just be 0x282f from here on out... for
server, for client platforms where there has not been a commitment to a
stable "RAID" id, the recommendation continues to be switch to AHCI
mode in the BIOS.

I hope this is sufficient to clarify that this SATA2 id should be the
last time a new RAID id needs to be added to this file, save for cases
where a quirk set beyond board_ahci needs to be applied (unlikely).

Re: [PATCH] ahci: Add Intel Emmitsburg PCH RAID PCI IDs

[Mika Westerberg]

Hi Jens,

On Thu, Nov 19, 2020 at 01:43:18PM +0300, Mika Westerberg wrote:
> Add Intel Emmitsburg PCH RAID PCI IDs to the list of supported
> controllers.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Would it be possible to get this one merged for v5.11?

Thanks!

> ---
>  drivers/ata/ahci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 00ba8e5a1ccc..0b39f0e7fd8f 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -398,6 +398,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
>  	{ PCI_VDEVICE(INTEL, 0x2823), board_ahci }, /* Lewisburg AHCI*/
>  	{ PCI_VDEVICE(INTEL, 0x2826), board_ahci }, /* Lewisburg RAID*/
>  	{ PCI_VDEVICE(INTEL, 0x2827), board_ahci }, /* Lewisburg RAID*/
> +	{ PCI_VDEVICE(INTEL, 0x282b), board_ahci }, /* Emmitsburg RAID */
> +	{ PCI_VDEVICE(INTEL, 0x282f), board_ahci }, /* Emmitsburg RAID */
>  	{ PCI_VDEVICE(INTEL, 0xa182), board_ahci }, /* Lewisburg AHCI*/
>  	{ PCI_VDEVICE(INTEL, 0xa186), board_ahci }, /* Lewisburg RAID*/
>  	{ PCI_VDEVICE(INTEL, 0xa1d2), board_ahci }, /* Lewisburg RAID*/
> -- 
> 2.29.2