[PATCH] PCI: Apply Intel NVMe quirk to Solidigm P44 Pro

[PATCH] PCI: Apply Intel NVMe quirk to Solidigm P44 Pro

[Mike Pastore]

Prevent KVM hang when a Solidgm P44 Pro NVMe is passed through to a
guest via IOMMU and the guest is subsequently rebooted.

A similar issue was identified and patched in commit 51ba09452d11b
("PCI: Delay after FLR of Intel DC P3700 NVMe") and the same fix can be
aplied for this case. (Intel spun off their NAND and SSD business as
Solidigm and sold it to SK Hynix in late 2021.)

Signed-off-by: Mike Pastore <mike@oobak.org>
---
 drivers/pci/quirks.c    | 10 ++++++----
 include/linux/pci_ids.h |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 44cab813bf95..b47844d0e574 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3980,10 +3980,11 @@ static int nvme_disable_and_flr(struct pci_dev *dev, bool probe)
 }
 
 /*
- * Intel DC P3700 NVMe controller will timeout waiting for ready status
- * to change after NVMe enable if the driver starts interacting with the
- * device too soon after FLR.  A 250ms delay after FLR has heuristically
- * proven to produce reliably working results for device assignment cases.
+ * Some NVMe controllers such as Intel DC P3700 and Solidigm P44 Pro will
+ * timeout waiting for ready status to change after NVMe enable if the driver
+ * starts interacting with the device too soon after FLR.  A 250ms delay after
+ * FLR has heuristically proven to produce reliably working results for device
+ * assignment cases.
  */
 static int delay_250ms_after_flr(struct pci_dev *dev, bool probe)
 {
@@ -4070,6 +4071,7 @@ static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
 	{ PCI_VENDOR_ID_SAMSUNG, 0xa804, nvme_disable_and_flr },
 	{ PCI_VENDOR_ID_INTEL, 0x0953, delay_250ms_after_flr },
 	{ PCI_VENDOR_ID_INTEL, 0x0a54, delay_250ms_after_flr },
+	{ PCI_VENDOR_ID_SOLIDIGM, 0xf1ac, delay_250ms_after_flr },
 	{ PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
 		reset_chelsio_generic_dev },
 	{ PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HINIC_VF,
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 45c3d62e616d..6105eddf41bf 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3119,4 +3119,6 @@
 
 #define PCI_VENDOR_ID_NCUBE		0x10ff
 
+#define PCI_VENDOR_ID_SOLIDIGM		0x025e
+
 #endif /* _LINUX_PCI_IDS_H */

base-commit: 63355b9884b3d1677de6bd1517cd2b8a9bf53978
-- 
2.39.2

Re: [PATCH] PCI: Apply Intel NVMe quirk to Solidigm P44 Pro

[Bjorn Helgaas]

[+cc Alex, kvm list]

On Sun, May 07, 2023 at 02:35:19AM -0500, Mike Pastore wrote:
> Prevent KVM hang when a Solidgm P44 Pro NVMe is passed through to a
> guest via IOMMU and the guest is subsequently rebooted.
> 
> A similar issue was identified and patched in commit 51ba09452d11b
> ("PCI: Delay after FLR of Intel DC P3700 NVMe") and the same fix can be
> aplied for this case. (Intel spun off their NAND and SSD business as
> Solidigm and sold it to SK Hynix in late 2021.)
> 
> Signed-off-by: Mike Pastore <mike@oobak.org>

Applied with subject:

  PCI: Delay after FLR of Solidigm P44 Pro NVMe

to my virtualization branch for v6.5.  I also moved
PCI_VENDOR_ID_SOLIDIGM to keep pci_ids.h sorted.

> ---
>  drivers/pci/quirks.c    | 10 ++++++----
>  include/linux/pci_ids.h |  2 ++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 44cab813bf95..b47844d0e574 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3980,10 +3980,11 @@ static int nvme_disable_and_flr(struct pci_dev *dev, bool probe)
>  }
>  
>  /*
> - * Intel DC P3700 NVMe controller will timeout waiting for ready status
> - * to change after NVMe enable if the driver starts interacting with the
> - * device too soon after FLR.  A 250ms delay after FLR has heuristically
> - * proven to produce reliably working results for device assignment cases.
> + * Some NVMe controllers such as Intel DC P3700 and Solidigm P44 Pro will
> + * timeout waiting for ready status to change after NVMe enable if the driver
> + * starts interacting with the device too soon after FLR.  A 250ms delay after
> + * FLR has heuristically proven to produce reliably working results for device
> + * assignment cases.
>   */
>  static int delay_250ms_after_flr(struct pci_dev *dev, bool probe)
>  {
> @@ -4070,6 +4071,7 @@ static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
>  	{ PCI_VENDOR_ID_SAMSUNG, 0xa804, nvme_disable_and_flr },
>  	{ PCI_VENDOR_ID_INTEL, 0x0953, delay_250ms_after_flr },
>  	{ PCI_VENDOR_ID_INTEL, 0x0a54, delay_250ms_after_flr },
> +	{ PCI_VENDOR_ID_SOLIDIGM, 0xf1ac, delay_250ms_after_flr },
>  	{ PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
>  		reset_chelsio_generic_dev },
>  	{ PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HINIC_VF,
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 45c3d62e616d..6105eddf41bf 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -3119,4 +3119,6 @@
>  
>  #define PCI_VENDOR_ID_NCUBE		0x10ff
>  
> +#define PCI_VENDOR_ID_SOLIDIGM		0x025e
> +
>  #endif /* _LINUX_PCI_IDS_H */
> 
> base-commit: 63355b9884b3d1677de6bd1517cd2b8a9bf53978
> -- 
> 2.39.2
> 

Re: [PATCH] PCI: Apply Intel NVMe quirk to Solidigm P44 Pro

[Mike Pastore]

On Tue, May 9, 2023 at 4:48 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> Applied with subject:
>
>   PCI: Delay after FLR of Solidigm P44 Pro NVMe
>
> to my virtualization branch for v6.5.

Thank you for testing! Should I submit a new version of the patch with
this subject line?

> I also moved PCI_VENDOR_ID_SOLIDIGM to keep pci_ids.h sorted.

Ah, the base commit I'm referencing has some unsorted lines at the
bottom, which threw me off. I can move it as well if I submit a new
version. Thank you.

Mike

Re: [PATCH] PCI: Apply Intel NVMe quirk to Solidigm P44 Pro

[Bjorn Helgaas]

On Tue, May 09, 2023 at 09:23:20PM -0500, Mike Pastore wrote:
> On Tue, May 9, 2023 at 4:48 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > Applied with subject:
> >
> >   PCI: Delay after FLR of Solidigm P44 Pro NVMe
> >
> > to my virtualization branch for v6.5.
> 
> Thank you for testing! Should I submit a new version of the patch with
> this subject line?

I didn't actually *test* this; I assume you did, and I assume/hope I
didn't break anything when I moved PCI_VENDOR_ID_SOLIDIGM.

> > I also moved PCI_VENDOR_ID_SOLIDIGM to keep pci_ids.h sorted.
> 
> Ah, the base commit I'm referencing has some unsorted lines at the
> bottom, which threw me off. I can move it as well if I submit a new
> version. Thank you.

No, I made the tweaks in my local tree, and you don't need to do
anything more.  This on its way to v6.5.

You can check my work here to make sure I didn't break anything:

  https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=virtualization&id=0ac448e0d29d

Bjorn