| Message ID | 20231022200329.60844-1-marex@denx.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] scsi: mvsas: Try to enable MSI | expand |
On 22/10/2023 21:03, Marek Vasut wrote: > This seems to be needed on OCZ RevoDrive 3 X2 / RevoDrive 350 > OCZ Technology Group, Inc. RevoDrive 3 X2 PCIe SSD 240 GB (Marvell SAS Controller) [1b85:1021] (rev 02) > By chance do you have any documentation on this controller which tells us that it requires MSI? > Without MSI enabled, the controller fails as follows: > " > mvsas 0000:00:02.0: mvsas: PCI-E x0, Bandwidth Usage: UnKnown Gbps > scsi host0: mvsas > sas: Enter sas_scsi_recover_host busy: 0 failed: 0 > ata1.00: qc timeout after 5000 msecs (cmd 0xec) > ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) > ata1.00: qc timeout after 10000 msecs (cmd 0xec) > ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) > ata1.00: qc timeout after 30000 msecs (cmd 0xec) > ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) > sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 > sas: sas_probe_sata: for direct-attached device 0000000000000000 returned -19 > sas: Enter sas_scsi_recover_host busy: 0 failed: 0 > ata2.00: qc timeout after 5000 msecs (cmd 0xec) > ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) > ata2.00: qc timeout after 10000 msecs (cmd 0xec) > ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) > ata2.00: qc timeout after 30000 msecs (cmd 0xec) > ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) > sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 > sas: sas_probe_sata: for direct-attached device 0100000000000000 returned -19 > " > > With this patch, the controller detects the two SSD drives on it: > " > mvsas 0000:00:02.0: mvsas: PCI-E x0, Bandwidth Usage: UnKnown Gbps > scsi host0: mvsas > sas: Enter sas_scsi_recover_host busy: 0 failed: 0 > ata1.00: ATA-8: OCZ-REVODRIVE350, 2.50, max UDMA/133 > ata1.00: 234441648 sectors, multi 1: LBA48 NCQ (depth 32) > ata1.00: configured for UDMA/133 > sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 > scsi 0:0:0:0: Direct-Access ATA OCZ-REVODRIVE350 2.50 PQ: 0 ANSI: 5 > sas: Enter sas_scsi_recover_host busy: 0 failed: 0 > ata2.00: ATA-8: OCZ-REVODRIVE350, 2.50, max UDMA/133 > ata2.00: 234441648 sectors, multi 1: LBA48 NCQ (depth 32) > ata2.00: configured for UDMA/133 > sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 > scsi 0:0:1:0: Direct-Access ATA OCZ-REVODRIVE350 2.50 PQ: 0 ANSI: 5 > scsi 0:0:0:0: Attached scsi generic sg0 type 0 > sd 0:0:0:0: [sda] 234441648 512-byte logical blocks: (120 GB/112 GiB) > sd 0:0:1:0: [sdb] 234441648 512-byte logical blocks: (120 GB/112 GiB) > sd 0:0:1:0: Attached scsi generic sg1 type 0 > sd 0:0:0:0: [sda] Write Protect is off > sd 0:0:1:0: [sdb] Write Protect is off > sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA > sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA > sd 0:0:0:0: [sda] Preferred minimum I/O size 512 bytes > sd 0:0:1:0: [sdb] Preferred minimum I/O size 512 bytes > sd 0:0:0:0: [sda] Attached SCSI disk > sd 0:0:1:0: [sdb] Attached SCSI disk > " > > I am not sure whether this is the correct fix, or whether this should > be a controller specific quirk instead, considering how this is likely > a legacy controller driver. pci_enable_msi() switches from pin-based interrupts to MSI. So currently the driver relies on pin-based. As such, I would be more inclined to quirk the driver for this controller. > > The enablement of MSIs has been part of this driver before, but was > removed without any real explanation in commit: > 20b09c2992fe ("[SCSI] mvsas: add support for 94xx; layout change; bug fixes") > The enablement of MSIs is also part of the 'oczpcie' driver, which is > really an ancient fork of this driver with a lot of variable renames > and such. > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Note that the "PCI-E x0, Bandwidth Usage: UnKnown Gbps" is due to QEMU > vfio-pci VT-d passthrough, for some reason this is what it reports. > The issue with PCI MSI happens on real hardware too, this vfio/VT-d > is just debugging convenience. > Note that this would be nice to have in stable series, but I'm reluctant > to ask for that in order to avoid breaking other peoples' machines. > Maybe a default-off kernel parameter for the mvsas module would be > acceptable for stable? > --- > Cc: "James E.J. Bottomley" <jejb@linux.ibm.com> > Cc: "Martin K. Petersen" <martin.petersen@oracle.com> > Cc: Bart Van Assche <bvanassche@acm.org> > Cc: Damien Le Moal <dlemoal@kernel.org> > Cc: Jason Yan <yanaijie@huawei.com> > Cc: John Garry <john.g.garry@oracle.com> > Cc: linux-scsi@vger.kernel.org > --- > drivers/scsi/mvsas/mv_init.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c > index 43ebb331e2167..6850e39237d3e 100644 > --- a/drivers/scsi/mvsas/mv_init.c > +++ b/drivers/scsi/mvsas/mv_init.c > @@ -571,6 +571,8 @@ static int mvs_pci_init(struct pci_dev *pdev, const struct pci_device_id *ent) > rc = sas_register_ha(SHOST_TO_SAS_HA(shost)); > if (rc) > goto err_out_shost; > + /* Try to enable MSI, this is needed at least on OCZ RevoDrive 3 X2 */ > + pci_enable_msi(pdev); You should check the return code. Thanks, John
On 10/24/23 12:46, John Garry wrote: > On 22/10/2023 21:03, Marek Vasut wrote: >> This seems to be needed on OCZ RevoDrive 3 X2 / RevoDrive 350 >> OCZ Technology Group, Inc. RevoDrive 3 X2 PCIe SSD 240 GB (Marvell SAS >> Controller) [1b85:1021] (rev 02) >> > > By chance do you have any documentation on this controller None what-so-ever, only this ancient oczpcie mvsas driver fork, which I injected hunk by hunk into existing mvsas, tested, etc. until I got to this one PCIe MSI line thing which made that controller work. > which tells us that it requires MSI? The situation is even worse in that OCZ got assimilated by Toshiba and all information about this device disappeared from toshiba website long ago. It is still in archive.org, but whatever software they released is non-trivial to even get working at all, and tbh even this drive itself is weird piece of hardware. >> Without MSI enabled, the controller fails as follows: >> " >> mvsas 0000:00:02.0: mvsas: PCI-E x0, Bandwidth Usage: UnKnown Gbps >> scsi host0: mvsas >> sas: Enter sas_scsi_recover_host busy: 0 failed: 0 >> ata1.00: qc timeout after 5000 msecs (cmd 0xec) >> ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) >> ata1.00: qc timeout after 10000 msecs (cmd 0xec) >> ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) >> ata1.00: qc timeout after 30000 msecs (cmd 0xec) >> ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) >> sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 >> sas: sas_probe_sata: for direct-attached device 0000000000000000 >> returned -19 >> sas: Enter sas_scsi_recover_host busy: 0 failed: 0 >> ata2.00: qc timeout after 5000 msecs (cmd 0xec) >> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) >> ata2.00: qc timeout after 10000 msecs (cmd 0xec) >> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) >> ata2.00: qc timeout after 30000 msecs (cmd 0xec) >> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) >> sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 >> sas: sas_probe_sata: for direct-attached device 0100000000000000 >> returned -19 >> " >> >> With this patch, the controller detects the two SSD drives on it: >> " >> mvsas 0000:00:02.0: mvsas: PCI-E x0, Bandwidth Usage: UnKnown Gbps >> scsi host0: mvsas >> sas: Enter sas_scsi_recover_host busy: 0 failed: 0 >> ata1.00: ATA-8: OCZ-REVODRIVE350, 2.50, max UDMA/133 >> ata1.00: 234441648 sectors, multi 1: LBA48 NCQ (depth 32) >> ata1.00: configured for UDMA/133 >> sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 >> scsi 0:0:0:0: Direct-Access ATA OCZ-REVODRIVE350 2.50 PQ: 0 >> ANSI: 5 >> sas: Enter sas_scsi_recover_host busy: 0 failed: 0 >> ata2.00: ATA-8: OCZ-REVODRIVE350, 2.50, max UDMA/133 >> ata2.00: 234441648 sectors, multi 1: LBA48 NCQ (depth 32) >> ata2.00: configured for UDMA/133 >> sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 >> scsi 0:0:1:0: Direct-Access ATA OCZ-REVODRIVE350 2.50 PQ: 0 >> ANSI: 5 >> scsi 0:0:0:0: Attached scsi generic sg0 type 0 >> sd 0:0:0:0: [sda] 234441648 512-byte logical blocks: (120 GB/112 GiB) >> sd 0:0:1:0: [sdb] 234441648 512-byte logical blocks: (120 GB/112 GiB) >> sd 0:0:1:0: Attached scsi generic sg1 type 0 >> sd 0:0:0:0: [sda] Write Protect is off >> sd 0:0:1:0: [sdb] Write Protect is off >> sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't >> support DPO or FUA >> sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't >> support DPO or FUA >> sd 0:0:0:0: [sda] Preferred minimum I/O size 512 bytes >> sd 0:0:1:0: [sdb] Preferred minimum I/O size 512 bytes >> sd 0:0:0:0: [sda] Attached SCSI disk >> sd 0:0:1:0: [sdb] Attached SCSI disk >> " >> >> I am not sure whether this is the correct fix, or whether this should >> be a controller specific quirk instead, considering how this is likely >> a legacy controller driver. > > pci_enable_msi() switches from pin-based interrupts to MSI. So currently > the driver relies on pin-based. As such, I would be more inclined to > quirk the driver for this controller. Indeed. I'll send a V2 with the quirk and return value check when time permits, considering there seem to be few users, I don't think there is much urgency. Thanks !
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c index 43ebb331e2167..6850e39237d3e 100644 --- a/drivers/scsi/mvsas/mv_init.c +++ b/drivers/scsi/mvsas/mv_init.c @@ -571,6 +571,8 @@ static int mvs_pci_init(struct pci_dev *pdev, const struct pci_device_id *ent) rc = sas_register_ha(SHOST_TO_SAS_HA(shost)); if (rc) goto err_out_shost; + /* Try to enable MSI, this is needed at least on OCZ RevoDrive 3 X2 */ + pci_enable_msi(pdev); rc = request_irq(pdev->irq, irq_handler, IRQF_SHARED, DRV_NAME, SHOST_TO_SAS_HA(shost)); if (rc) @@ -583,6 +585,7 @@ static int mvs_pci_init(struct pci_dev *pdev, const struct pci_device_id *ent) return 0; err_not_sas: + pci_disable_msi(pdev); sas_unregister_ha(SHOST_TO_SAS_HA(shost)); err_out_shost: scsi_remove_host(mvi->shost); @@ -607,6 +610,7 @@ static void mvs_pci_remove(struct pci_dev *pdev) tasklet_kill(&((struct mvs_prv_info *)sha->lldd_ha)->mv_tasklet); #endif + pci_disable_msi(pdev); sas_unregister_ha(sha); sas_remove_host(mvi->shost);
This seems to be needed on OCZ RevoDrive 3 X2 / RevoDrive 350 OCZ Technology Group, Inc. RevoDrive 3 X2 PCIe SSD 240 GB (Marvell SAS Controller) [1b85:1021] (rev 02) Without MSI enabled, the controller fails as follows: " mvsas 0000:00:02.0: mvsas: PCI-E x0, Bandwidth Usage: UnKnown Gbps scsi host0: mvsas sas: Enter sas_scsi_recover_host busy: 0 failed: 0 ata1.00: qc timeout after 5000 msecs (cmd 0xec) ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) ata1.00: qc timeout after 10000 msecs (cmd 0xec) ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) ata1.00: qc timeout after 30000 msecs (cmd 0xec) ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 sas: sas_probe_sata: for direct-attached device 0000000000000000 returned -19 sas: Enter sas_scsi_recover_host busy: 0 failed: 0 ata2.00: qc timeout after 5000 msecs (cmd 0xec) ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) ata2.00: qc timeout after 10000 msecs (cmd 0xec) ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) ata2.00: qc timeout after 30000 msecs (cmd 0xec) ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 sas: sas_probe_sata: for direct-attached device 0100000000000000 returned -19 " With this patch, the controller detects the two SSD drives on it: " mvsas 0000:00:02.0: mvsas: PCI-E x0, Bandwidth Usage: UnKnown Gbps scsi host0: mvsas sas: Enter sas_scsi_recover_host busy: 0 failed: 0 ata1.00: ATA-8: OCZ-REVODRIVE350, 2.50, max UDMA/133 ata1.00: 234441648 sectors, multi 1: LBA48 NCQ (depth 32) ata1.00: configured for UDMA/133 sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 scsi 0:0:0:0: Direct-Access ATA OCZ-REVODRIVE350 2.50 PQ: 0 ANSI: 5 sas: Enter sas_scsi_recover_host busy: 0 failed: 0 ata2.00: ATA-8: OCZ-REVODRIVE350, 2.50, max UDMA/133 ata2.00: 234441648 sectors, multi 1: LBA48 NCQ (depth 32) ata2.00: configured for UDMA/133 sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 scsi 0:0:1:0: Direct-Access ATA OCZ-REVODRIVE350 2.50 PQ: 0 ANSI: 5 scsi 0:0:0:0: Attached scsi generic sg0 type 0 sd 0:0:0:0: [sda] 234441648 512-byte logical blocks: (120 GB/112 GiB) sd 0:0:1:0: [sdb] 234441648 512-byte logical blocks: (120 GB/112 GiB) sd 0:0:1:0: Attached scsi generic sg1 type 0 sd 0:0:0:0: [sda] Write Protect is off sd 0:0:1:0: [sdb] Write Protect is off sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sd 0:0:0:0: [sda] Preferred minimum I/O size 512 bytes sd 0:0:1:0: [sdb] Preferred minimum I/O size 512 bytes sd 0:0:0:0: [sda] Attached SCSI disk sd 0:0:1:0: [sdb] Attached SCSI disk " I am not sure whether this is the correct fix, or whether this should be a controller specific quirk instead, considering how this is likely a legacy controller driver. The enablement of MSIs has been part of this driver before, but was removed without any real explanation in commit: 20b09c2992fe ("[SCSI] mvsas: add support for 94xx; layout change; bug fixes") The enablement of MSIs is also part of the 'oczpcie' driver, which is really an ancient fork of this driver with a lot of variable renames and such. Signed-off-by: Marek Vasut <marex@denx.de> --- Note that the "PCI-E x0, Bandwidth Usage: UnKnown Gbps" is due to QEMU vfio-pci VT-d passthrough, for some reason this is what it reports. The issue with PCI MSI happens on real hardware too, this vfio/VT-d is just debugging convenience. Note that this would be nice to have in stable series, but I'm reluctant to ask for that in order to avoid breaking other peoples' machines. Maybe a default-off kernel parameter for the mvsas module would be acceptable for stable? --- Cc: "James E.J. Bottomley" <jejb@linux.ibm.com> Cc: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: Bart Van Assche <bvanassche@acm.org> Cc: Damien Le Moal <dlemoal@kernel.org> Cc: Jason Yan <yanaijie@huawei.com> Cc: John Garry <john.g.garry@oracle.com> Cc: linux-scsi@vger.kernel.org --- drivers/scsi/mvsas/mv_init.c | 4 ++++ 1 file changed, 4 insertions(+)