From: Arnd Bergmann <[email protected]>
The virt_to_bus/bus_to_virt interface has been deprecated for
decades. After Jakub Kicinski put a lot of work into cleaning out the
network drivers using them, there are only a couple of other drivers
left, which can all be removed or otherwise cleaned up, to remove the
old interface for good.
Any out of tree drivers using virt_to_bus() should be converted to
using the dma-mapping interfaces, typically dma_alloc_coherent()
or dma_map_single()).
There are a few m68k and ppc32 specific drivers that keep using the
interfaces, but these are all guarded with architecture-specific
Kconfig dependencies, and are not actually broken.
There are still a number of drivers that are using virt_to_phys()
and phys_to_virt() in place of dma-mapping operations, and these
are often broken, but they are out of scope for this series.
I would like the first two patches to either get merged through
the SCSI tree, or get an Ack from the SCSI maintainers so I can
merge them through the asm-generic tree
Arnd
---
Changes since v1:
- dropped VME patches that are already in staging-next
- dropped media patch that gets merged independently
- added a networking patch and dropped it again after it got merged
- replace BusLogic removal with a workaround
Cc: Jakub Kicinski <[email protected]>
Cc: Christoph Hellwig <[email protected]> # dma-mapping
Cc: Marek Szyprowski <[email protected]> # dma-mapping
Cc: Robin Murphy <[email protected]> # dma-mapping
Cc: [email protected]
Cc: Khalid Aziz <[email protected]> # buslogic
Cc: Maciej W. Rozycki <[email protected]> # buslogic
Cc: Matt Wang <[email protected]> # buslogic
Cc: Miquel van Smoorenburg <[email protected]> # dpt_i2o
Cc: Mark Salyzyn <[email protected]> # dpt_i2o
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Denis Efremov <[email protected]> # floppy
Arnd Bergmann (3):
scsi: dpt_i2o: drop stale VIRT_TO_BUS dependency
scsi: BusLogic remove bus_to_virt
arch/*/: remove CONFIG_VIRT_TO_BUS
.../core-api/bus-virt-phys-mapping.rst | 220 ------------------
Documentation/core-api/dma-api-howto.rst | 14 --
Documentation/core-api/index.rst | 1 -
.../translations/zh_CN/core-api/index.rst | 1 -
arch/alpha/Kconfig | 1 -
arch/alpha/include/asm/floppy.h | 2 +-
arch/alpha/include/asm/io.h | 8 +-
arch/ia64/Kconfig | 1 -
arch/ia64/include/asm/io.h | 8 -
arch/m68k/Kconfig | 1 -
arch/m68k/include/asm/virtconvert.h | 4 +-
arch/microblaze/Kconfig | 1 -
arch/microblaze/include/asm/io.h | 2 -
arch/mips/Kconfig | 1 -
arch/mips/include/asm/io.h | 9 -
arch/parisc/Kconfig | 1 -
arch/parisc/include/asm/floppy.h | 4 +-
arch/parisc/include/asm/io.h | 2 -
arch/powerpc/Kconfig | 1 -
arch/powerpc/include/asm/io.h | 2 -
arch/riscv/include/asm/page.h | 1 -
arch/x86/Kconfig | 1 -
arch/x86/include/asm/io.h | 9 -
arch/xtensa/Kconfig | 1 -
arch/xtensa/include/asm/io.h | 3 -
drivers/scsi/BusLogic.c | 27 ++-
drivers/scsi/Kconfig | 4 +-
drivers/scsi/dpt_i2o.c | 4 +-
include/asm-generic/io.h | 14 --
mm/Kconfig | 8 -
30 files changed, 30 insertions(+), 326 deletions(-)
delete mode 100644 Documentation/core-api/bus-virt-phys-mapping.rst
--
2.29.2
From: Arnd Bergmann <[email protected]>
The BusLogic driver is the last remaining driver that relies on the
deprecated bus_to_virt() function, which in turn only works on a few
architectures, and is incompatible with both swiotlb and iommu support.
Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."),
the driver had a dependency on x86-32, presumably because of this
problem. However, the change introduced another bug that made it still
impossible to use the driver on any 64-bit machine.
This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix
64-bit system enumeration error for Buslogic"), 8 years later, which
shows that there are not a lot of users.
Maciej is still using the driver on 32-bit hardware, and Khalid mentioned
that the driver works with the device emulation used in VirtualBox
and VMware. Both of those only emulate it for Windows 2000 and older
operating systems that did not ship with the better LSI logic driver.
Do a minimum fix that searches through the list of descriptors to find
one that matches the bus address. This is clearly as inefficient as
was indicated in the code comment about the lack of a bus_to_virt()
replacement. A better fix would likely involve changing out the entire
descriptor allocation for a simpler one, but that would be much
more invasive.
Cc: Maciej W. Rozycki <[email protected]>
Cc: Matt Wang <[email protected]>
Cc: Khalid Aziz <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/scsi/BusLogic.c | 27 ++++++++++++++++-----------
drivers/scsi/Kconfig | 2 +-
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index a897c8f914cf..d057abfcdd5c 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
return (hoststatus << 16) | tgt_status;
}
+/*
+ * turn the dma address from an inbox into a ccb pointer
+ * This is rather inefficient.
+ */
+static struct blogic_ccb *
+blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox)
+{
+ struct blogic_ccb *ccb;
+
+ for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all)
+ if (inbox->ccb == ccb->dma_handle)
+ break;
+
+ return ccb;
+}
/*
blogic_scan_inbox scans the Incoming Mailboxes saving any
Incoming Mailbox entries for completion processing.
*/
-
static void blogic_scan_inbox(struct blogic_adapter *adapter)
{
/*
@@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter)
enum blogic_cmplt_code comp_code;
while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
- /*
- We are only allowed to do this because we limit our
- architectures we run on to machines where bus_to_virt(
- actually works. There *needs* to be a dma_addr_to_virt()
- in the new PCI DMA mapping interface to replace
- bus_to_virt() or else this code is going to become very
- innefficient.
- */
- struct blogic_ccb *ccb =
- (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
+ struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox);
if (comp_code != BLOGIC_CMD_NOTFOUND) {
if (ccb->status == BLOGIC_CCB_ACTIVE ||
ccb->status == BLOGIC_CCB_RESET) {
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index cf75588a2587..56bdc08d0b77 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -513,7 +513,7 @@ config SCSI_HPTIOP
config SCSI_BUSLOGIC
tristate "BusLogic SCSI support"
- depends on PCI && SCSI && VIRT_TO_BUS
+ depends on PCI && SCSI
help
This is support for BusLogic MultiMaster and FlashPoint SCSI Host
Adapters. Consult the SCSI-HOWTO, available from
--
2.29.2
On 2022-06-17 13:57, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The BusLogic driver is the last remaining driver that relies on the
> deprecated bus_to_virt() function, which in turn only works on a few
> architectures, and is incompatible with both swiotlb and iommu support.
>
> Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."),
> the driver had a dependency on x86-32, presumably because of this
> problem. However, the change introduced another bug that made it still
> impossible to use the driver on any 64-bit machine.
>
> This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix
> 64-bit system enumeration error for Buslogic"), 8 years later, which
> shows that there are not a lot of users.
>
> Maciej is still using the driver on 32-bit hardware, and Khalid mentioned
> that the driver works with the device emulation used in VirtualBox
> and VMware. Both of those only emulate it for Windows 2000 and older
> operating systems that did not ship with the better LSI logic driver.
>
> Do a minimum fix that searches through the list of descriptors to find
> one that matches the bus address. This is clearly as inefficient as
> was indicated in the code comment about the lack of a bus_to_virt()
> replacement. A better fix would likely involve changing out the entire
> descriptor allocation for a simpler one, but that would be much
> more invasive.
FWIW, if efficiency *is* a practical concern, even under the current
allocation scheme it looks like there are only 4 actual DMA allocations
to search through. From a quick scan (since it's too hot here not to get
distracted...), if I wanted to optimise this in future I'd probably
remove the semi-redundant allocgrp_* fields from struct blogic_ccb and
hang a dedicated list of the block allocations off the adapter - at that
point the lookup could likely already be more efficient than a
theoretical dma_to_virt() interface would be if it had to go off and
walk an IOMMU pagetable. Then the next question would be whether it's
viable to make a single 32KB allocation rather 4*8KB, so it's no longer
even a list.
For now, though, I agree with the simple change that's clear and easy to
reason about:
Reviewed-by: Robin Murphy <[email protected]>
Thanks,
Robin.
> Cc: Maciej W. Rozycki <[email protected]>
> Cc: Matt Wang <[email protected]>
> Cc: Khalid Aziz <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/scsi/BusLogic.c | 27 ++++++++++++++++-----------
> drivers/scsi/Kconfig | 2 +-
> 2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index a897c8f914cf..d057abfcdd5c 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
> return (hoststatus << 16) | tgt_status;
> }
>
> +/*
> + * turn the dma address from an inbox into a ccb pointer
> + * This is rather inefficient.
> + */
> +static struct blogic_ccb *
> +blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox)
> +{
> + struct blogic_ccb *ccb;
> +
> + for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all)
> + if (inbox->ccb == ccb->dma_handle)
> + break;
> +
> + return ccb;
> +}
>
> /*
> blogic_scan_inbox scans the Incoming Mailboxes saving any
> Incoming Mailbox entries for completion processing.
> */
> -
> static void blogic_scan_inbox(struct blogic_adapter *adapter)
> {
> /*
> @@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter)
> enum blogic_cmplt_code comp_code;
>
> while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
> - /*
> - We are only allowed to do this because we limit our
> - architectures we run on to machines where bus_to_virt(
> - actually works. There *needs* to be a dma_addr_to_virt()
> - in the new PCI DMA mapping interface to replace
> - bus_to_virt() or else this code is going to become very
> - innefficient.
> - */
> - struct blogic_ccb *ccb =
> - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
> + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox);
> if (comp_code != BLOGIC_CMD_NOTFOUND) {
> if (ccb->status == BLOGIC_CCB_ACTIVE ||
> ccb->status == BLOGIC_CCB_RESET) {
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index cf75588a2587..56bdc08d0b77 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -513,7 +513,7 @@ config SCSI_HPTIOP
>
> config SCSI_BUSLOGIC
> tristate "BusLogic SCSI support"
> - depends on PCI && SCSI && VIRT_TO_BUS
> + depends on PCI && SCSI
> help
> This is support for BusLogic MultiMaster and FlashPoint SCSI Host
> Adapters. Consult the SCSI-HOWTO, available from
On 6/17/22 14:57, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The BusLogic driver is the last remaining driver that relies on the
> deprecated bus_to_virt() function, which in turn only works on a few
> architectures, and is incompatible with both swiotlb and iommu support.
>
> Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."),
> the driver had a dependency on x86-32, presumably because of this
> problem. However, the change introduced another bug that made it still
> impossible to use the driver on any 64-bit machine.
>
> This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix
> 64-bit system enumeration error for Buslogic"), 8 years later, which
> shows that there are not a lot of users.
>
> Maciej is still using the driver on 32-bit hardware, and Khalid mentioned
> that the driver works with the device emulation used in VirtualBox
> and VMware. Both of those only emulate it for Windows 2000 and older
> operating systems that did not ship with the better LSI logic driver.
>
> Do a minimum fix that searches through the list of descriptors to find
> one that matches the bus address. This is clearly as inefficient as
> was indicated in the code comment about the lack of a bus_to_virt()
> replacement. A better fix would likely involve changing out the entire
> descriptor allocation for a simpler one, but that would be much
> more invasive.
>
> Cc: Maciej W. Rozycki <[email protected]>
> Cc: Matt Wang <[email protected]>
> Cc: Khalid Aziz <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/scsi/BusLogic.c | 27 ++++++++++++++++-----------
> drivers/scsi/Kconfig | 2 +-
> 2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index a897c8f914cf..d057abfcdd5c 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
> return (hoststatus << 16) | tgt_status;
> }
>
> +/*
> + * turn the dma address from an inbox into a ccb pointer
> + * This is rather inefficient.
> + */
> +static struct blogic_ccb *
> +blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox)
> +{
> + struct blogic_ccb *ccb;
> +
> + for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all)
> + if (inbox->ccb == ccb->dma_handle)
> + break;
> +
> + return ccb;
> +}
>
> /*
> blogic_scan_inbox scans the Incoming Mailboxes saving any
> Incoming Mailbox entries for completion processing.
> */
> -
> static void blogic_scan_inbox(struct blogic_adapter *adapter)
> {
> /*
> @@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter)
> enum blogic_cmplt_code comp_code;
>
> while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
> - /*
> - We are only allowed to do this because we limit our
> - architectures we run on to machines where bus_to_virt(
> - actually works. There *needs* to be a dma_addr_to_virt()
> - in the new PCI DMA mapping interface to replace
> - bus_to_virt() or else this code is going to become very
> - innefficient.
> - */
> - struct blogic_ccb *ccb =
> - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
> + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox);
> if (comp_code != BLOGIC_CMD_NOTFOUND) {
> if (ccb->status == BLOGIC_CCB_ACTIVE ||
> ccb->status == BLOGIC_CCB_RESET) {
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index cf75588a2587..56bdc08d0b77 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -513,7 +513,7 @@ config SCSI_HPTIOP
>
> config SCSI_BUSLOGIC
> tristate "BusLogic SCSI support"
> - depends on PCI && SCSI && VIRT_TO_BUS
> + depends on PCI && SCSI
> help
> This is support for BusLogic MultiMaster and FlashPoint SCSI Host
> Adapters. Consult the SCSI-HOWTO, available from
CCB handling in the driver is ugly anyway, so that'll be good enough.
Reviewed-by: Hannes Reinecke <[email protected]>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
[email protected] +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
On 6/17/22 06:57, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The BusLogic driver is the last remaining driver that relies on the
> deprecated bus_to_virt() function, which in turn only works on a few
> architectures, and is incompatible with both swiotlb and iommu support.
>
> Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."),
> the driver had a dependency on x86-32, presumably because of this
> problem. However, the change introduced another bug that made it still
> impossible to use the driver on any 64-bit machine.
>
> This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix
> 64-bit system enumeration error for Buslogic"), 8 years later, which
> shows that there are not a lot of users.
>
> Maciej is still using the driver on 32-bit hardware, and Khalid mentioned
> that the driver works with the device emulation used in VirtualBox
> and VMware. Both of those only emulate it for Windows 2000 and older
> operating systems that did not ship with the better LSI logic driver.
>
> Do a minimum fix that searches through the list of descriptors to find
> one that matches the bus address. This is clearly as inefficient as
> was indicated in the code comment about the lack of a bus_to_virt()
> replacement. A better fix would likely involve changing out the entire
> descriptor allocation for a simpler one, but that would be much
> more invasive.
>
> Cc: Maciej W. Rozycki <[email protected]>
> Cc: Matt Wang <[email protected]>
> Cc: Khalid Aziz <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/scsi/BusLogic.c | 27 ++++++++++++++++-----------
> drivers/scsi/Kconfig | 2 +-
> 2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index a897c8f914cf..d057abfcdd5c 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
> return (hoststatus << 16) | tgt_status;
> }
>
> +/*
> + * turn the dma address from an inbox into a ccb pointer
> + * This is rather inefficient.
> + */
> +static struct blogic_ccb *
> +blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox)
> +{
> + struct blogic_ccb *ccb;
> +
> + for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all)
> + if (inbox->ccb == ccb->dma_handle)
> + break;
> +
> + return ccb;
> +}
>
> /*
> blogic_scan_inbox scans the Incoming Mailboxes saving any
> Incoming Mailbox entries for completion processing.
> */
> -
> static void blogic_scan_inbox(struct blogic_adapter *adapter)
> {
> /*
> @@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter)
> enum blogic_cmplt_code comp_code;
>
> while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
> - /*
> - We are only allowed to do this because we limit our
> - architectures we run on to machines where bus_to_virt(
> - actually works. There *needs* to be a dma_addr_to_virt()
> - in the new PCI DMA mapping interface to replace
> - bus_to_virt() or else this code is going to become very
> - innefficient.
> - */
> - struct blogic_ccb *ccb =
> - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
> + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox);
This change looks good enough as workaround to not use bus_to_virt() for
now. There are two problems I see though. One, I do worry about
blogic_inbox_to_ccb() returning NULL for ccb which should not happen
unless the mailbox pointer was corrupted which would indicate a bigger
problem. Nevertheless a NULL pointer causing kernel panic concerns me.
How about adding a check before we dereference ccb?
Second, with this patch applied, I am seeing errors from the driver:
=====================
[ 1623.902685] sdb: sdb1 sdb2
[ 1623.903245] sd 2:0:0:0: [sdb] Attached SCSI disk
[ 1623.911000] scsi2: Illegal CCB #76 status 2 in Incoming Mailbox
[ 1623.911005] scsi2: Illegal CCB #76 status 2 in Incoming Mailbox
[ 1623.911070] scsi2: Illegal CCB #79 status 2 in Incoming Mailbox
[ 1651.458008] scsi2: Warning: Partition Table appears to have Geometry
256/63 which is
[ 1651.458013] scsi2: not compatible with current BusLogic Host Adapter
Geometry 255/63
[ 1658.797609] scsi2: Resetting BusLogic BT-958D Failed
[ 1659.533208] sd 2:0:0:0: Device offlined - not ready after error recovery
[ 1659.533331] sd 2:0:0:0: Device offlined - not ready after error recovery
[ 1659.533333] sd 2:0:0:0: Device offlined - not ready after error recovery
[ 1659.533342] sd 2:0:0:0: [sdb] tag#101 FAILED Result:
hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK cmd_age=35s
[ 1659.533345] sd 2:0:0:0: [sdb] tag#101 CDB: Read(10) 28 00 00 00 00 28
00 00 10 00
[ 1659.533346] I/O error, dev sdb, sector 40 op 0x0:(READ) flags 0x80700
phys_seg 1 prio class 0
=================
This is on VirtualBox using emulated BusLogic adapter.
This patch needs more refinement.
Thanks,
Khalid
> if (comp_code != BLOGIC_Cn erroneousMD_NOTFOUND) {
> if (ccb->status == BLOGIC_CCB_ACTIVE ||
> ccb->status == BLOGIC_CCB_RESET) {
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index cf75588a2587..56bdc08d0b77 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -513,7 +513,7 @@ config SCSI_HPTIOP
>
> config SCSI_BUSLOGIC
> tristate "BusLogic SCSI support"
> - depends on PCI && SCSI && VIRT_TO_BUS
> + depends on PCI && SCSI
> help
> This is support for BusLogic MultiMaster and FlashPoint SCSI Host
> Adapters. Consult the SCSI-HOWTO, available from
On Tue, Jun 21, 2022 at 11:56 PM Khalid Aziz <[email protected]> wrote:
> > while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
> > - /*
> > - We are only allowed to do this because we limit our
> > - architectures we run on to machines where bus_to_virt(
> > - actually works. There *needs* to be a dma_addr_to_virt()
> > - in the new PCI DMA mapping interface to replace
> > - bus_to_virt() or else this code is going to become very
> > - innefficient.
> > - */
> > - struct blogic_ccb *ccb =
> > - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
> > + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox);
>
> This change looks good enough as workaround to not use bus_to_virt() for
> now. There are two problems I see though. One, I do worry about
> blogic_inbox_to_ccb() returning NULL for ccb which should not happen
> unless the mailbox pointer was corrupted which would indicate a bigger
> problem. Nevertheless a NULL pointer causing kernel panic concerns me.
> How about adding a check before we dereference ccb?
Right, makes sense
> Second, with this patch applied, I am seeing errors from the driver:
>
> =====================
> [ 1623.902685] sdb: sdb1 sdb2
> [ 1623.903245] sd 2:0:0:0: [sdb] Attached SCSI disk
> [ 1623.911000] scsi2: Illegal CCB #76 status 2 in Incoming Mailbox
> [ 1623.911005] scsi2: Illegal CCB #76 status 2 in Incoming Mailbox
> [ 1623.911070] scsi2: Illegal CCB #79 status 2 in Incoming Mailbox
> [ 1651.458008] scsi2: Warning: Partition Table appears to have Geometry
> 256/63 which is
> [ 1651.458013] scsi2: not compatible with current BusLogic Host Adapter
> Geometry 255/63
> [ 1658.797609] scsi2: Resetting BusLogic BT-958D Failed
> [ 1659.533208] sd 2:0:0:0: Device offlined - not ready after error recovery
> [ 1659.533331] sd 2:0:0:0: Device offlined - not ready after error recovery
> [ 1659.533333] sd 2:0:0:0: Device offlined - not ready after error recovery
> [ 1659.533342] sd 2:0:0:0: [sdb] tag#101 FAILED Result:
> hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK cmd_age=35s
> [ 1659.533345] sd 2:0:0:0: [sdb] tag#101 CDB: Read(10) 28 00 00 00 00 28
> 00 00 10 00
> [ 1659.533346] I/O error, dev sdb, sector 40 op 0x0:(READ) flags 0x80700
> phys_seg 1 prio class 0
>
> =================
>
> This is on VirtualBox using emulated BusLogic adapter.
>
> This patch needs more refinement.
Thanks for testing the patch, too bad it didn't work. At least I quickly found
one stupid mistake on my end, hope it's the only one.
Can you test it again with this patch on top?
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index d057abfcdd5c..9e67f2ee25ee 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -2554,8 +2554,14 @@ static void blogic_scan_inbox(struct
blogic_adapter *adapter)
enum blogic_cmplt_code comp_code;
while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
- struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter,
adapter->next_inbox);
- if (comp_code != BLOGIC_CMD_NOTFOUND) {
+ struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter,
next_inbox);
+ if (!ccb) {
+ /*
+ * This should never happen, unless the CCB list is
+ * corrupted in memory.
+ */
+ blogic_warn("Could not find CCB for dma
address 0x%x\n", adapter, next_inbox->ccb);
+ } else if (comp_code != BLOGIC_CMD_NOTFOUND) {
if (ccb->status == BLOGIC_CCB_ACTIVE ||
ccb->status == BLOGIC_CCB_RESET) {
On Fri, Jun 24, 2022 at 5:38 PM Khalid Aziz <[email protected]> wrote:
> On 6/23/22 08:47, Arnd Bergmann wrote:
>
> Driver works with this change. next_inbox is the correct pointer to pass.
Ok, great! I'll add a 'Tested-by' line then. I was already in the process of
preparing a v3 patch set, will send out the fixed patch in a bit.
Arnd
On 6/23/22 08:47, Arnd Bergmann wrote:
>
>
> Can you test it again with this patch on top?
>
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index d057abfcdd5c..9e67f2ee25ee 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -2554,8 +2554,14 @@ static void blogic_scan_inbox(struct
> blogic_adapter *adapter)
> enum blogic_cmplt_code comp_code;
>
> while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
> - struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter,
> adapter->next_inbox);
> - if (comp_code != BLOGIC_CMD_NOTFOUND) {
> + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter,
> next_inbox);
> + if (!ccb) {
> + /*
> + * This should never happen, unless the CCB list is
> + * corrupted in memory.
> + */
> + blogic_warn("Could not find CCB for dma
> address 0x%x\n", adapter, next_inbox->ccb);
> + } else if (comp_code != BLOGIC_CMD_NOTFOUND) {
> if (ccb->status == BLOGIC_CCB_ACTIVE ||
> ccb->status == BLOGIC_CCB_RESET) {
Hi Arnd,
Driver works with this change. next_inbox is the correct pointer to pass.
Thanks,
Khalid