diff mbox

[SRU,Y/Z,1/1] nvme: Quirk APST on Intel 600P/P3100 devices

Message ID 20170602045502.18736-2-kai.heng.feng@canonical.com
State New
Headers show

Commit Message

Kai-Heng Feng June 2, 2017, 4:55 a.m. UTC
From: Andy Lutomirski <luto@kernel.org>

BugLink: https://bugs.launchpad.net/bugs/1686592

They have known firmware bugs.  A fix is apparently in the works --
once fixed firmware is available, someone from Intel (Hi, Keith!)
can adjust the quirk accordingly.

Cc: stable@vger.kernel.org # v4.11
Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: Mario Limonciello <mario_limonciello@dell.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
(backported from commit 50af47d04ca530544b27affffb0722f158e2bb9c)
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/nvme/host/pci.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Colin Ian King June 2, 2017, 8:06 a.m. UTC | #1
On 02/06/17 05:55, Kai-Heng Feng wrote:
> From: Andy Lutomirski <luto@kernel.org>
> 
> BugLink: https://bugs.launchpad.net/bugs/1686592
> 
> They have known firmware bugs.  A fix is apparently in the works --
> once fixed firmware is available, someone from Intel (Hi, Keith!)
> can adjust the quirk accordingly.
> 
> Cc: stable@vger.kernel.org # v4.11
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Cc: Mario Limonciello <mario_limonciello@dell.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> (backported from commit 50af47d04ca530544b27affffb0722f158e2bb9c)
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/nvme/host/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 394038608734..2e61d3047435 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2288,6 +2288,8 @@ static const struct pci_device_id nvme_id_table[] = {
>  	{ PCI_VDEVICE(INTEL, 0x0a54),
>  		.driver_data = NVME_QUIRK_STRIPE_SIZE |
>  				NVME_QUIRK_DISCARD_ZEROES, },
> +	{ PCI_VDEVICE(INTEL, 0xf1a5),	/* Intel 600P/P3100 */
> +		.driver_data = NVME_QUIRK_NO_DEEPEST_PS },
>  	{ PCI_VDEVICE(INTEL, 0x5845),	/* Qemu emulated controller */
>  		.driver_data = NVME_QUIRK_IDENTIFY_CNS, },
>  	{ PCI_DEVICE(0x1c58, 0x0003),	/* HGST adapter */
> 

I guess having it work and not quite as power efficient is preferable to
it be power efficient and not working correctly :-)

Just wondering how we deal with the case with users who have the updated
firmware since this quirk will leave them with a less power efficient
config.

Are there plans for this to be backported to previous releases?
Kai-Heng Feng June 2, 2017, 8:26 a.m. UTC | #2
On Fri, Jun 2, 2017 at 4:06 PM, Colin Ian King <colin.king@canonical.com> wrote:
> On 02/06/17 05:55, Kai-Heng Feng wrote:
>> From: Andy Lutomirski <luto@kernel.org>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1686592
>>
>> They have known firmware bugs.  A fix is apparently in the works --
>> once fixed firmware is available, someone from Intel (Hi, Keith!)
>> can adjust the quirk accordingly.
>>
>> Cc: stable@vger.kernel.org # v4.11
>> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> Cc: Mario Limonciello <mario_limonciello@dell.com>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> (backported from commit 50af47d04ca530544b27affffb0722f158e2bb9c)
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>>  drivers/nvme/host/pci.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>> index 394038608734..2e61d3047435 100644
>> --- a/drivers/nvme/host/pci.c
>> +++ b/drivers/nvme/host/pci.c
>> @@ -2288,6 +2288,8 @@ static const struct pci_device_id nvme_id_table[] = {
>>       { PCI_VDEVICE(INTEL, 0x0a54),
>>               .driver_data = NVME_QUIRK_STRIPE_SIZE |
>>                               NVME_QUIRK_DISCARD_ZEROES, },
>> +     { PCI_VDEVICE(INTEL, 0xf1a5),   /* Intel 600P/P3100 */
>> +             .driver_data = NVME_QUIRK_NO_DEEPEST_PS },
>>       { PCI_VDEVICE(INTEL, 0x5845),   /* Qemu emulated controller */
>>               .driver_data = NVME_QUIRK_IDENTIFY_CNS, },
>>       { PCI_DEVICE(0x1c58, 0x0003),   /* HGST adapter */
>>
>
> I guess having it work and not quite as power efficient is preferable to
> it be power efficient and not working correctly :-)
>
> Just wondering how we deal with the case with users who have the updated
> firmware since this quirk will leave them with a less power efficient
> config.

We can use table core_quirks[] in drivers/nvme/core.c to match firmware version.
The current version is to blacklist bad devices, it won't be too hard
too add another whitelist quirk table, if new firmware indeed fixes
the issue.

>
> Are there plans for this to be backported to previous releases?

I'll backport APST (including quirks) to Xenial after things settle down.

>
Stefan Bader June 2, 2017, 10:11 a.m. UTC | #3
On 02.06.2017 06:55, Kai-Heng Feng wrote:
> From: Andy Lutomirski <luto@kernel.org>
> 
> BugLink: https://bugs.launchpad.net/bugs/1686592
> 
> They have known firmware bugs.  A fix is apparently in the works --
> once fixed firmware is available, someone from Intel (Hi, Keith!)
> can adjust the quirk accordingly.
> 
> Cc: stable@vger.kernel.org # v4.11
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Cc: Mario Limonciello <mario_limonciello@dell.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> (backported from commit 50af47d04ca530544b27affffb0722f158e2bb9c)
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/nvme/host/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 394038608734..2e61d3047435 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2288,6 +2288,8 @@ static const struct pci_device_id nvme_id_table[] = {
>  	{ PCI_VDEVICE(INTEL, 0x0a54),
>  		.driver_data = NVME_QUIRK_STRIPE_SIZE |
>  				NVME_QUIRK_DISCARD_ZEROES, },
> +	{ PCI_VDEVICE(INTEL, 0xf1a5),	/* Intel 600P/P3100 */
> +		.driver_data = NVME_QUIRK_NO_DEEPEST_PS },
>  	{ PCI_VDEVICE(INTEL, 0x5845),	/* Qemu emulated controller */
>  		.driver_data = NVME_QUIRK_IDENTIFY_CNS, },
>  	{ PCI_DEVICE(0x1c58, 0x0003),	/* HGST adapter */
> 

Indeed better to quirk right now. Just wondering, the future work likely needs
to apply quirks not only by vendor/model but firmware, too. Right?

-Stefan
Colin Ian King June 2, 2017, 11:19 a.m. UTC | #4
On 02/06/17 09:26, Kai-Heng Feng wrote:
> On Fri, Jun 2, 2017 at 4:06 PM, Colin Ian King <colin.king@canonical.com> wrote:
>> On 02/06/17 05:55, Kai-Heng Feng wrote:
>>> From: Andy Lutomirski <luto@kernel.org>
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1686592
>>>
>>> They have known firmware bugs.  A fix is apparently in the works --
>>> once fixed firmware is available, someone from Intel (Hi, Keith!)
>>> can adjust the quirk accordingly.
>>>
>>> Cc: stable@vger.kernel.org # v4.11
>>> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> Cc: Mario Limonciello <mario_limonciello@dell.com>
>>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>> (backported from commit 50af47d04ca530544b27affffb0722f158e2bb9c)
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> ---
>>>  drivers/nvme/host/pci.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>>> index 394038608734..2e61d3047435 100644
>>> --- a/drivers/nvme/host/pci.c
>>> +++ b/drivers/nvme/host/pci.c
>>> @@ -2288,6 +2288,8 @@ static const struct pci_device_id nvme_id_table[] = {
>>>       { PCI_VDEVICE(INTEL, 0x0a54),
>>>               .driver_data = NVME_QUIRK_STRIPE_SIZE |
>>>                               NVME_QUIRK_DISCARD_ZEROES, },
>>> +     { PCI_VDEVICE(INTEL, 0xf1a5),   /* Intel 600P/P3100 */
>>> +             .driver_data = NVME_QUIRK_NO_DEEPEST_PS },
>>>       { PCI_VDEVICE(INTEL, 0x5845),   /* Qemu emulated controller */
>>>               .driver_data = NVME_QUIRK_IDENTIFY_CNS, },
>>>       { PCI_DEVICE(0x1c58, 0x0003),   /* HGST adapter */
>>>
>>
>> I guess having it work and not quite as power efficient is preferable to
>> it be power efficient and not working correctly :-)
>>
>> Just wondering how we deal with the case with users who have the updated
>> firmware since this quirk will leave them with a less power efficient
>> config.
> 
> We can use table core_quirks[] in drivers/nvme/core.c to match firmware version.
> The current version is to blacklist bad devices, it won't be too hard
> too add another whitelist quirk table, if new firmware indeed fixes
> the issue.
> 
>>
>> Are there plans for this to be backported to previous releases?
> 
> I'll backport APST (including quirks) to Xenial after things settle down.
> 

OK, thanks.

Acked-by: Colin Ian King <colin.king@canonical.com>
Seth Forshee June 5, 2017, 6:26 p.m. UTC | #5
On Fri, Jun 02, 2017 at 12:55:02PM +0800, Kai-Heng Feng wrote:
> From: Andy Lutomirski <luto@kernel.org>
> 
> BugLink: https://bugs.launchpad.net/bugs/1686592
> 
> They have known firmware bugs.  A fix is apparently in the works --
> once fixed firmware is available, someone from Intel (Hi, Keith!)
> can adjust the quirk accordingly.
> 
> Cc: stable@vger.kernel.org # v4.11
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Cc: Mario Limonciello <mario_limonciello@dell.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> (backported from commit 50af47d04ca530544b27affffb0722f158e2bb9c)
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Applied to artful/master-next, thanks.
Kleber Sacilotto de Souza June 6, 2017, 10:21 a.m. UTC | #6
Applied to yakkety and zesty master-next branches. Thanks.
diff mbox

Patch

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 394038608734..2e61d3047435 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2288,6 +2288,8 @@  static const struct pci_device_id nvme_id_table[] = {
 	{ PCI_VDEVICE(INTEL, 0x0a54),
 		.driver_data = NVME_QUIRK_STRIPE_SIZE |
 				NVME_QUIRK_DISCARD_ZEROES, },
+	{ PCI_VDEVICE(INTEL, 0xf1a5),	/* Intel 600P/P3100 */
+		.driver_data = NVME_QUIRK_NO_DEEPEST_PS },
 	{ PCI_VDEVICE(INTEL, 0x5845),	/* Qemu emulated controller */
 		.driver_data = NVME_QUIRK_IDENTIFY_CNS, },
 	{ PCI_DEVICE(0x1c58, 0x0003),	/* HGST adapter */