| Message ID | 20201119104318.79297-1-mika.westerberg@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | ahci: Add Intel Emmitsburg PCH RAID PCI IDs | expand |
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.
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.
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).
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".
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.
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
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.
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.
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
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).
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*/
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(+)