Hi,
First of all, does anyone have a copy of: "MultiMaster UltraSCSI Host
Adapters for PCI Systems: Technical Reference Manual" (pub. 3002493-E)?
It used to live in the "Mylex Manuals and Documentation Archives" section
of the Mylex web site <http://www.mylex.com/pub/manuals/index.htm>,
specifically at: <http://www.mylex.com/pub/manuals/mmultra.pdf>.
Another useful document might be: "Wide SCSI Host Adapters for PCI and
EISA Systems: Technical Reference Manual" (pub. 3000763-A), which used to
live at: <http://www.mylex.com/pub/manuals/widescsi.pdf>, linked from the
same place.
Sadly I didn't get to these resources while they were still there, and
neither did archive.org, and now they not appear available from anywhere
online. I'm sure Leonard had this all, but, alas, he is long gone too.
It looks to me like either or both documents would help understanding how
the BusLogic devices (are supposed to) work and possibly deal with issues
in a better way.
So we are here owing to Christoph's recent ISA bounce buffering sweep:
<https://lore.kernel.org/linux-scsi/[email protected]/T/#m981284e74e93216626a0728ce1601ca18fca92e8>
which has prompted me to verify the current version of Linux with my old
server, which has been long equipped with venerable Linux 2.6.18 and which
I now have available for general experimenting, and the BusLogic BT-958
PCI SCSI host bus adapter the server has used for 20-something years now.
This revealed numerous issues with the BusLogic driver.
Firstly (1/5) it has suffered from some bitrot and messages produced have
become messy from the lack of update for proper `pr_cont' support.
Secondly (2/5) there has been a potential buffer overrun/stack corruption
security issue from using an unbounded `vsprintf' call.
Thirdly (3/5) it has become obvious the BusLogic driver would have been
non-functional, should I have upgraded the kernel, at least with this
configuration for some 8 years now, and the underlying cause has been a
long-known issue with the MultiMaster firmware I have dealt with already,
back in 2003. To put it short the firmware cannot cope with commands that
request an allocation length exceeding the length of actual data returned.
I have originally observed it with a LOG SENSE command in the course of
investigating why smartmontools bring the system to a death, and worked it
around: <https://sourceforge.net/p/smartmontools/mailman/message/4993087/>
by issuing the command twice, first just to obtain the allocation length
required. As it turns out we need a similar workaround in the kernel now.
But in the course of investigating this issue I have discovered there is
a second bottom to it and hence I have prepared follow-up changes (4-5/5)
to address problems with our handling of Vital Product Data INQUIRY pages.
See individual change descriptions for further details.
Questions, comments, concerns? Otherwise please apply.
Maciej
Existing `blogic_msg' invocations do not appear to overrun its internal
buffer of a fixed length of 100, which would cause stack corruption, but
it's easy to miss with possible further updates and a fix is cheap in
performance terms, so limit the output produced into the buffer by using
`vsnprintf' rather than `vsprintf'.
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
drivers/scsi/BusLogic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
linux-buslogic-vsnprintf.diff
Index: linux-macro-ide/drivers/scsi/BusLogic.c
===================================================================
--- linux-macro-ide.orig/drivers/scsi/BusLogic.c
+++ linux-macro-ide/drivers/scsi/BusLogic.c
@@ -3588,7 +3588,7 @@ static void blogic_msg(enum blogic_msgle
int len = 0;
va_start(args, adapter);
- len = vsprintf(buf, fmt, args);
+ len = vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
if (msglevel == BLOGIC_ANNOUNCE_LEVEL) {
static int msglines = 0;
Set the allocation length to 255 for the ATA Information VPD page
requested in the WRITE SAME handler, so as not to limit information
examined by `scsi_get_vpd_page' in the supported vital product data
pages unnecessarily.
Originally it was thought that Areca hardware may have issues with a
valid allocation length supplied for a VPD inquiry, however older SCSI
standard revisions[1] consider 255 the maximum length allowed and what
has later become the high order byte is considered reserved and must be
zero with the INQUIRY command. Therefore it was unnecessary to reduce
the amount of data requested from 512 as far down as to 64, arbitrarily
chosen, and 255 would as well do.
With commit b3ae8780b429 ("[SCSI] Add EVPD page 0x83 and 0x80 to sysfs")
we have since got the SCSI_VPD_PG_LEN macro, so use that instead.
References:
[1] "Information technology - Small Computer System Interface - 2",
WORKING DRAFT, X3T9.2, Project 375D, Revision 10L, 7-SEP-93, Section
8.2.5 "INQUIRY command", pp.104-108
Signed-off-by: Maciej W. Rozycki <[email protected]>
Fixes: af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request")
---
Nix,
I can see you're still around. Would you therefore please be so kind
as to verify this change with your Areca hardware if you still have it?
It looks to me like you were thinking in the right direction with:
<https://lore.kernel.org/linux-scsi/[email protected]/>.
Sadly nobody seemed to have paid attention to your observation and neither
were different buffer sizes considered (or at least it wasn't mentioned in
the discussion).
Maciej
---
drivers/scsi/sd.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
linux-scsi-write-same-vpd-buffer.diff
Index: linux-macro-ide/drivers/scsi/sd.c
===================================================================
--- linux-macro-ide.orig/drivers/scsi/sd.c
+++ linux-macro-ide/drivers/scsi/sd.c
@@ -3076,16 +3076,13 @@ static void sd_read_write_same(struct sc
}
if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) {
- /* too large values might cause issues with arcmsr */
- int vpd_buf_len = 64;
-
sdev->no_report_opcodes = 1;
/* Disable WRITE SAME if REPORT SUPPORTED OPERATION
* CODES is unsupported and the device has an ATA
* Information VPD page (SAT).
*/
- if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len))
+ if (!scsi_get_vpd_page(sdev, 0x89, buffer, SCSI_VPD_PG_LEN))
sdev->no_write_same = 1;
}
Update BusLogic driver's messaging system to use `pr_cont' for
continuation lines, bringing messy output:
pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
scsi0: Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
scsi0: PCI Bus: 0, Device: 19, Address:
0xE0012000,
Host Adapter SCSI ID: 7
scsi0: Parity Checking: Enabled, Extended Translation: Enabled
scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192
scsi0: Tagged Queue Depth:
Automatic
, Untagged Queue Depth: 3
scsi0: SCSI Bus Termination: Both Enabled
, SCAM: Disabled
scsi0: *** BusLogic BT-958 Initialized Successfully ***
scsi host0: BusLogic BT-958
back to order:
pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
scsi0: Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
scsi0: PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7
scsi0: Parity Checking: Enabled, Extended Translation: Enabled
scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192
scsi0: Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
scsi0: SCSI Bus Termination: Both Enabled, SCAM: Disabled
scsi0: *** BusLogic BT-958 Initialized Successfully ***
scsi host0: BusLogic BT-958
Also diagnostic output such as with the `BusLogic=TraceConfiguration'
parameter is affected and becomes vertical and therefore hard to read.
This has now been corrected, e.g.:
pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
blogic_cmd(86) Status = 30: 4 ==> 4: FF 05 93 00
blogic_cmd(95) Status = 28: (Modify I/O Address)
blogic_cmd(91) Status = 30: 1 ==> 1: 01
blogic_cmd(04) Status = 30: 4 ==> 4: 41 41 35 30
blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
blogic_cmd(04) Status = 30: 4 ==> 4: 41 41 35 30
blogic_cmd(0B) Status = 30: 3 ==> 3: 00 08 07
blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00
blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
blogic_cmd(84) Status = 30: 1 ==> 1: 37
blogic_cmd(8B) Status = 30: 5 ==> 5: 39 35 38 20 20
blogic_cmd(85) Status = 30: 1 ==> 1: 42
blogic_cmd(86) Status = 30: 4 ==> 4: FF 05 93 00
blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC
scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
etc.
Signed-off-by: Maciej W. Rozycki <[email protected]>
Fixes: 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation lines")
Cc: [email protected] # v4.9+
---
drivers/scsi/BusLogic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
linux-buslogic-pr-cont.diff
Index: linux-macro-ide/drivers/scsi/BusLogic.c
===================================================================
--- linux-macro-ide.orig/drivers/scsi/BusLogic.c
+++ linux-macro-ide/drivers/scsi/BusLogic.c
@@ -3603,7 +3603,7 @@ static void blogic_msg(enum blogic_msgle
if (buf[0] != '\n' || len > 1)
printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
} else
- printk("%s", buf);
+ pr_cont("%s", buf);
} else {
if (begin) {
if (adapter != NULL && adapter->adapter_initd)
@@ -3611,7 +3611,7 @@ static void blogic_msg(enum blogic_msgle
else
printk("%s%s", blogic_msglevelmap[msglevel], buf);
} else
- printk("%s", buf);
+ pr_cont("%s", buf);
}
begin = (buf[len - 1] == '\n');
}
Allow SCSI hosts to request avoiding trailing allocation length with VPD
inquiries, and use the mechanism to work around an issue with at least
some BusLogic MultiMaster host bus adapters and observed with the BT-958
model specifically where issuing commands that return less data than
provided for causes fatal failures:
scsi host0: BusLogic BT-958
scsi 0:0:0:0: Direct-Access IBM DDYS-T18350M SA5A PQ: 0 ANSI: 3
scsi 0:0:1:0: Direct-Access SEAGATE ST336607LW 0006 PQ: 0 ANSI: 3
scsi 0:0:5:0: Direct-Access IOMEGA ZIP 100 E.08 PQ: 0 ANSI: 2
sd 0:0:1:0: [sdb] 71687372 512-byte logical blocks: (36.7 GB/34.2 GiB)
sd 0:0:0:0: [sda] 35843670 512-byte logical blocks: (18.4 GB/17.1 GiB)
sd 0:0:1:0: [sdb] Write Protect is off
sd 0:0:5:0: [sdc] Attached SCSI removable disk
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, supports DPO and FUA
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
scsi0: *** BusLogic BT-958 Initialized Successfully ***
scsi0: *** BusLogic BT-958 Initialized Successfully ***
scsi0: *** BusLogic BT-958 Initialized Successfully ***
scsi0: *** BusLogic BT-958 Initialized Successfully ***
sd 0:0:0:0: Device offlined - not ready after error recovery
sd 0:0:1:0: Device offlined - not ready after error recovery
sd 0:0:0:0: scsi_vpd_inquiry(0): buf[64] => -5
sd 0:0:1:0: scsi_vpd_inquiry(0): buf[64] => -5
sd 0:0:0:0: [sda] Attached SCSI disk
sd 0:0:1:0: [sdb] Attached SCSI disk
VFS: Cannot open root device "802" or unknown-block(8,2): error -6
(here and elsewhere reported with some instrumentation added so as to
show the causing requests and with irrelevant messages filtered out).
As already observed back in 2003 and worked around in smartmontools at
least some versions of BusLogic firmware such as 5.07B are unable to
handle such commands, but it is possible to request enough data first
for the length of the data response to be determined and then reissue
the same command with the allocation length matching the response
expected.
It is what this change does on a host-by-host basis, by providing a flag
for individual HBA drivers to enable this workaround, currently set by
the BusLogic driver, and then issuing these double calls as requested,
which then produce results as expected:
sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, supports DPO and FUA
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 0:0:1:0: scsi_vpd_inquiry(0): buf[64] => 13
sd 0:0:0:0: scsi_vpd_inquiry(0): buf[64] => 7
sdb: sdb1 sdb2
sd 0:0:1:0: scsi_vpd_inquiry(0): buf[64] => 13
sd 0:0:1:0: [sdb] Attached SCSI disk
sda: sda1 sda2 sda3 sda4 < sda5 sda6 sda7 sda8 sda9 sda10 >
sd 0:0:0:0: scsi_vpd_inquiry(0): buf[64] => 7
sd 0:0:0:0: [sda] Attached SCSI disk
EXT4-fs (sda2): mounting ext2 file system using the ext4 subsystem
EXT4-fs (sda2): mounted filesystem without journal. Opts: (null). Quota mode: disabled.
VFS: Mounted root (ext2 filesystem) readonly on device 8:2.
The minimum request size of 4 for the repeated call has been chosen to
match one required for a successful return from `scsi_vpd_inquiry'.
Interestingly enough it has only started triggering with not so recent
commit af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request")
that decreased the allocation length for the originating request from
512 down to 64. Previously the request was rejected outright by the
respective targets as invalid and therefore did not trigger the issue
with MultiMaster firmware as that would only happen for a command that
succeeded but produced less data than provided for:
scsi0: CCB #36 Target 0: Result 2 Host Adapter Status 00 Target Status 02
scsi0: CDB 12 01 00 02 00 00
scsi0: Sense 70 00 05 00 00 00 00 18 00 00 00 00 24 00 00 C0 00 03 00 [...]
sd 0:0:0:0: scsi_vpd_inquiry(0): buf[512] => -5
scsi0: CCB #37 Target 1: Result 2 Host Adapter Status 00 Target Status 02
scsi0: CDB 12 01 00 02 00 00
scsi0: Sense 70 00 05 00 00 00 00 0A 00 00 00 00 24 00 01 C9 00 03 00 [...]
sd 0:0:1:0: scsi_vpd_inquiry(0): buf[512] => -5
(here with the buffer size set back to 512, the `BusLogic=TraceErrors'
parameter and trailing sense data zeros trimmed for brevity). Note the
sense key of 0x5 returned denoting an illegal request even for page 0.
Signed-off-by: Maciej W. Rozycki <[email protected]>
Fixes: 881a256d84e6 ("[SCSI] Add VPD helper")
Cc: [email protected] # v2.6.30+
---
drivers/scsi/BusLogic.c | 1 +
drivers/scsi/scsi.c | 24 +++++++++++++++++++++---
include/scsi/scsi_host.h | 3 +++
3 files changed, 25 insertions(+), 3 deletions(-)
linux-buslogic-get-vpd-page-buffer.diff
Index: linux-macro-ide/drivers/scsi/BusLogic.c
===================================================================
--- linux-macro-ide.orig/drivers/scsi/BusLogic.c
+++ linux-macro-ide/drivers/scsi/BusLogic.c
@@ -2301,6 +2301,7 @@ static void __init blogic_inithoststruct
host->sg_tablesize = adapter->drvr_sglimit;
host->unchecked_isa_dma = adapter->need_bouncebuf;
host->cmd_per_lun = adapter->untag_qdepth;
+ host->no_trailing_allocation_length = true;
}
/*
Index: linux-macro-ide/drivers/scsi/scsi.c
===================================================================
--- linux-macro-ide.orig/drivers/scsi/scsi.c
+++ linux-macro-ide/drivers/scsi/scsi.c
@@ -346,8 +346,19 @@ int scsi_get_vpd_page(struct scsi_device
if (sdev->skip_vpd_pages)
goto fail;
- /* Ask for all the pages supported by this device */
- result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
+ /*
+ * Ask for all the pages supported by this device. Determine the
+ * actual data length first if so required by the host, e.g.
+ * BusLogic BT-958.
+ */
+ if (sdev->host->no_trailing_allocation_length) {
+ result = scsi_vpd_inquiry(sdev, buf, 0, min(4, buf_len));
+ if (result < 4)
+ goto fail;
+ } else {
+ result = buf_len;
+ }
+ result = scsi_vpd_inquiry(sdev, buf, 0, min(result, buf_len));
if (result < 4)
goto fail;
@@ -366,7 +377,14 @@ int scsi_get_vpd_page(struct scsi_device
goto fail;
found:
- result = scsi_vpd_inquiry(sdev, buf, page, buf_len);
+ if (sdev->host->no_trailing_allocation_length) {
+ result = scsi_vpd_inquiry(sdev, buf, page, min(4, buf_len));
+ if (result < 4)
+ goto fail;
+ } else {
+ result = buf_len;
+ }
+ result = scsi_vpd_inquiry(sdev, buf, page, min(result, buf_len));
if (result < 0)
goto fail;
Index: linux-macro-ide/include/scsi/scsi_host.h
===================================================================
--- linux-macro-ide.orig/include/scsi/scsi_host.h
+++ linux-macro-ide/include/scsi/scsi_host.h
@@ -653,6 +653,9 @@ struct Scsi_Host {
/* The transport requires the LUN bits NOT to be stored in CDB[1] */
unsigned no_scsi2_lun_in_cdb:1;
+ /* Allocation length must not exceed actual data length. */
+ unsigned no_trailing_allocation_length:1;
+
/*
* Optional work queue to be utilized by the transport
*/
As discussed in a previous workaround for a BusLogic BT-958 problem with
VPD inquiries with an allocation length of 512 bytes as requested before
commit af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request")
are rejected outright as invalid at least by some SCSI target devices as
are any requests with a non-zero value in byte #3:
scsi host0: BusLogic BT-958
scsi 0:0:0:0: Direct-Access IBM DDYS-T18350M SA5A PQ: 0 ANSI: 3
scsi 0:0:1:0: Direct-Access SEAGATE ST336607LW 0006 PQ: 0 ANSI: 3
scsi 0:0:5:0: Direct-Access IOMEGA ZIP 100 E.08 PQ: 0 ANSI: 2
[...]
scsi0: CCB #36 Target 0: Result 2 Host Adapter Status 00 Target Status 02
scsi0: CDB 12 01 00 01 06 00
scsi0: Sense 70 00 05 00 00 00 00 18 00 00 00 00 24 00 00 C0 00 03 00 [...]
sd 0:0:0:0: scsi_vpd_inquiry(0): buf[262] => -5
scsi0: CCB #37 Target 1: Result 2 Host Adapter Status 00 Target Status 02
scsi0: CDB 12 01 00 01 06 00
scsi0: Sense 70 00 05 00 00 00 00 0A 00 00 00 00 24 00 01 C8 00 03 00 [...]
sd 0:0:1:0: scsi_vpd_inquiry(0): buf[262] => -5
(here with the buffer size tweaked to 262 so as to verify if a bit in
byte #3 of the INQUIRY command is ignored and the length of 6 assumed or
tripped over, the `BusLogic=TraceErrors' parameter and trailing sense
data zeros trimmed for brevity). Note the sense key of 0x5 denoting an
illegal request.
For the record with the buffer size of 6 requests for page 0 complete
successfully and due to page truncation `scsi_get_vpd_page' proceeds
with an attempt to get inexistent page 0x89:
sd 0:0:0:0: scsi_vpd_inquiry(0): buf[6] => 7
sd 0:0:1:0: scsi_vpd_inquiry(0): buf[6] => 13
sd 0:0:0:0: scsi_vpd_inquiry(137): buf[6] => -5
sd 0:0:1:0: scsi_vpd_inquiry(137): buf[6] => -5
Upon a further investigation it has turned out at least SCSI-2 considers
byte #3 of the INQUIRY command[1] as well as byte #2 of vital product
data pages[2] reserved and expects a value of zero there. The response
from SCSI-3 devices shown above indicates the same expectation.
Therefore it is unsafe to issue INQUIRY requests unconditionally with
the allocation length beyond 255, as they may fail with an otherwise
supported request or cause undefined behaviour with some hardware.
Now we actually never do that as all our callers of `scsi_get_vpd_page'
either hardcode the buffer size to a value between 8 and 255 or
calculate it from a structure size, of which the largest is:
struct c2_inquiry {
u8 peripheral_info; /* 0 1 */
u8 page_code; /* 1 1 */
u8 reserved1; /* 2 1 */
u8 page_len; /* 3 1 */
u8 page_id[4]; /* 4 4 */
u8 sw_version[3]; /* 8 3 */
u8 sw_date[3]; /* 11 3 */
u8 features_enabled; /* 14 1 */
u8 max_lun_supported; /* 15 1 */
u8 partitions[239]; /* 16 239 */
/* size: 255, cachelines: 2, members: 10 */
/* last cacheline: 127 bytes */
};
As from commit b3ae8780b429 ("[SCSI] Add EVPD page 0x83 and 0x80 to sysfs")
we now also have the SCSI_VPD_PG_LEN macro that reflects the limitation.
However for the sake of a possible future requirement to support VPD
pages that do have a length exceeding 255 bytes and now that the danger
of using the formerly reserved byte #3 of the INQUIRY command has been
identified execute calls to `scsi_get_vpd_page' with a request size
exceeding 255 bytes in two stages, by determining the actual length of
data to be returned first and only then issuing the intended request for
full data.
References:
[1] "Information technology - Small Computer System Interface - 2",
WORKING DRAFT, X3T9.2, Project 375D, Revision 10L, 7-SEP-93, Section
8.2.5 "INQUIRY command", pp.104-108
[2] same, Section 8.3.4 "Vital product data parameters", pp.154-159
Signed-off-by: Maciej W. Rozycki <[email protected]>
Fixes: 881a256d84e6 ("[SCSI] Add VPD helper")
---
Hi,
NB the SCSI-2 working draft is the only normative reference I have access
to, downloaded many years ago and not online anymore. I have more recent
vendor documents that do indicate that bytes #3 & #2 respectively are a
part of the length field, but based on empirical evidence presented here
it is unsafe to unconditionally assume that the bytes can be set to a
non-zero value. So I think it will be safest long-term if we handle it
correctly right away now that the knowledge is fresh, as past experience
with commit af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request")
indicates the circumstances are not always correctly understood.
Maciej
---
drivers/scsi/scsi.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
linux-scsi-vpd-inquiry-buffer.diff
Index: linux-macro-ide/drivers/scsi/scsi.c
===================================================================
--- linux-macro-ide.orig/drivers/scsi/scsi.c
+++ linux-macro-ide/drivers/scsi/scsi.c
@@ -348,10 +348,15 @@ int scsi_get_vpd_page(struct scsi_device
/*
* Ask for all the pages supported by this device. Determine the
- * actual data length first if so required by the host, e.g.
- * BusLogic BT-958.
+ * actual data length first if the length requested is beyond 255
+ * bytes as the high order length byte used to be reserved with
+ * older SCSI standard revisions and a non-zero value there may
+ * cause either such an INQUIRY command to be rejected by a target
+ * or undefined behaviour to occur. Also do so if so required by
+ * the host, e.g. BusLogic BT-958.
*/
- if (sdev->host->no_trailing_allocation_length) {
+ if (buf_len > SCSI_VPD_PG_LEN ||
+ sdev->host->no_trailing_allocation_length) {
result = scsi_vpd_inquiry(sdev, buf, 0, min(4, buf_len));
if (result < 4)
goto fail;
@@ -377,7 +382,8 @@ int scsi_get_vpd_page(struct scsi_device
goto fail;
found:
- if (sdev->host->no_trailing_allocation_length) {
+ if (buf_len > SCSI_VPD_PG_LEN ||
+ sdev->host->no_trailing_allocation_length) {
result = scsi_vpd_inquiry(sdev, buf, page, min(4, buf_len));
if (result < 4)
goto fail;
On 14 Apr 2021, Maciej W. Rozycki stated:
> Set the allocation length to 255 for the ATA Information VPD page
> requested in the WRITE SAME handler, so as not to limit information
> examined by `scsi_get_vpd_page' in the supported vital product data
> pages unnecessarily.
>
> Originally it was thought that Areca hardware may have issues with a
> valid allocation length supplied for a VPD inquiry, however older SCSI
> standard revisions[1] consider 255 the maximum length allowed and what
Aaaah. That explains a lot! (Not that I can remember what SCSI standard
rev that Areca firmware claimed to implement. I know I never updated the
firmware, so it's going to be something no newer than mid-2009 and
probably quite a bit older.)
> Nix,
>
> I can see you're still around. Would you therefore please be so kind
> as to verify this change with your Areca hardware if you still have it?
It's been up in the loft for years, but I'll get it out this weekend and
give it a spin :) this'll let me make sure the disks still spin as well,
which matters for an in-case-of-lightning-strike disaster-recovery
backup box.
(I just hope this kernel boots on it at all. It's about three years
since I retired it... let's see!)
> It looks to me like you were thinking in the right direction with:
> <https://lore.kernel.org/linux-scsi/[email protected]/>.
It's the sort of mistake I could see myself making: an easy mistake to
make when so many things in C require buffer size - 1 or you get a
disastrous security hole...
--
NULL && (void)
On Thu, 2021-04-15 at 00:39 +0200, Maciej W. Rozycki wrote:
> Update BusLogic driver's messaging system to use `pr_cont' for
> continuation lines, bringing messy output:
>
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0: Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0: PCI Bus: 0, Device: 19, Address:
> 0xE0012000,
> Host Adapter SCSI ID: 7
> scsi0: Parity Checking: Enabled, Extended Translation: Enabled
> scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0: Tagged Queue Depth:
> Automatic
> , Untagged Queue Depth: 3
> scsi0: SCSI Bus Termination: Both Enabled
> , SCAM: Disabled
>
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
>
> back to order:
>
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0: Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0: PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7
> scsi0: Parity Checking: Enabled, Extended Translation: Enabled
> scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0: Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
> scsi0: SCSI Bus Termination: Both Enabled, SCAM: Disabled
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
>
> Also diagnostic output such as with the `BusLogic=TraceConfiguration'
> parameter is affected and becomes vertical and therefore hard to read.
> This has now been corrected, e.g.:
>
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> blogic_cmd(86) Status = 30: 4 ==> 4: FF 05 93 00
> blogic_cmd(95) Status = 28: (Modify I/O Address)
> blogic_cmd(91) Status = 30: 1 ==> 1: 01
> blogic_cmd(04) Status = 30: 4 ==> 4: 41 41 35 30
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
> blogic_cmd(04) Status = 30: 4 ==> 4: 41 41 35 30
> blogic_cmd(0B) Status = 30: 3 ==> 3: 00 08 07
> blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
> blogic_cmd(84) Status = 30: 1 ==> 1: 37
> blogic_cmd(8B) Status = 30: 5 ==> 5: 39 35 38 20 20
> blogic_cmd(85) Status = 30: 1 ==> 1: 42
> blogic_cmd(86) Status = 30: 4 ==> 4: FF 05 93 00
> blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
>
> etc.
In patch 2, vscnprintf should probably be used to make sure it's
0 terminated.
And while it's a lot more code, I'd prefer a solution that looks more
like the other commonly used kernel logging extension mechanisms
where adapter is placed before the format, ... in the argument list.
Today it's:
void blogic_msg(enum, fmt, adapter, ...);
without the __printf marking so there is one format/arg mismatch.
fyi: in the suggested patch below it's
- blogic_info("BIOS Address: 0x%lX, ", adapter,
- adapter->bios_addr);
+ blogic_cont(adapter, "BIOS Address: 0x%X, ",
+ adapter->bios_addr);
I'd prefer
__printf(3, 4)
void blogic_msg(enum, adapter, fmt, ...)
(or maybe void blogic_msg(adapter, enum, fmt, ...))
And there's a simple addition of a blogic_cont macro and extension
to blogic_msg to simplify the logic and obviousness of the logging
extension lines too.
I suggest this done with coccinelle and a little typing:
---
drivers/scsi/BusLogic.c | 496 +++++++++++++++++++++++++++++++-----------------
drivers/scsi/BusLogic.h | 32 ++--
2 files changed, 341 insertions(+), 187 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index ccb061ab0a0a..7a52371b5ab6 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -134,8 +134,10 @@ static char *blogic_cmd_failure_reason;
static void blogic_announce_drvr(struct blogic_adapter *adapter)
{
- blogic_announce("***** BusLogic SCSI Driver Version " blogic_drvr_version " of " blogic_drvr_date " *****\n", adapter);
- blogic_announce("Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>\n", adapter);
+ blogic_announce(adapter,
+ "***** BusLogic SCSI Driver Version " blogic_drvr_version " of " blogic_drvr_date " *****\n");
+ blogic_announce(adapter,
+ "Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>\n");
}
@@ -198,8 +200,7 @@ static bool __init blogic_create_initccbs(struct blogic_adapter *adapter)
blk_pointer = dma_alloc_coherent(&adapter->pci_device->dev,
blk_size, &blkp, GFP_KERNEL);
if (blk_pointer == NULL) {
- blogic_err("UNABLE TO ALLOCATE CCB GROUP - DETACHING\n",
- adapter);
+ blogic_err(adapter, "UNABLE TO ALLOCATE CCB GROUP - DETACHING\n");
return false;
}
blogic_init_ccbs(adapter, blk_pointer, blk_size, blkp);
@@ -259,10 +260,13 @@ static void blogic_create_addlccbs(struct blogic_adapter *adapter,
}
if (adapter->alloc_ccbs > prev_alloc) {
if (print_success)
- blogic_notice("Allocated %d additional CCBs (total now %d)\n", adapter, adapter->alloc_ccbs - prev_alloc, adapter->alloc_ccbs);
+ blogic_notice(adapter,
+ "Allocated %d additional CCBs (total now %d)\n",
+ adapter->alloc_ccbs - prev_alloc,
+ adapter->alloc_ccbs);
return;
}
- blogic_notice("Failed to allocate additional CCBs\n", adapter);
+ blogic_notice(adapter, "Failed to allocate additional CCBs\n");
if (adapter->drvr_qdepth > adapter->alloc_ccbs - adapter->tgt_count) {
adapter->drvr_qdepth = adapter->alloc_ccbs - adapter->tgt_count;
adapter->scsi_host->can_queue = adapter->drvr_qdepth;
@@ -441,7 +445,9 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum blogic_opcode opcode,
goto done;
}
if (blogic_global_options.trace_config)
- blogic_notice("blogic_cmd(%02X) Status = %02X: (Modify I/O Address)\n", adapter, opcode, statusreg.all);
+ blogic_notice(adapter,
+ "blogic_cmd(%02X) Status = %02X: (Modify I/O Address)\n",
+ opcode, statusreg.all);
result = 0;
goto done;
}
@@ -499,15 +505,16 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum blogic_opcode opcode,
*/
if (blogic_global_options.trace_config) {
int i;
- blogic_notice("blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
- adapter, opcode, statusreg.all, replylen,
+ blogic_notice(adapter,
+ "blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
+ opcode, statusreg.all, replylen,
reply_b);
if (replylen > reply_b)
replylen = reply_b;
for (i = 0; i < replylen; i++)
- blogic_notice(" %02X", adapter,
- ((unsigned char *) reply)[i]);
- blogic_notice("\n", adapter);
+ blogic_cont(adapter, " %02X",
+ ((unsigned char *)reply)[i]);
+ blogic_cont(adapter, "\n");
}
/*
Process Command Invalid conditions.
@@ -717,23 +724,37 @@ static int __init blogic_init_mm_probeinfo(struct blogic_adapter *adapter)
pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
if (pci_resource_flags(pci_device, 0) & IORESOURCE_MEM) {
- blogic_err("BusLogic: Base Address0 0x%lX not I/O for MultiMaster Host Adapter\n", NULL, base_addr0);
- blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
+ blogic_err(NULL,
+ "BusLogic: Base Address0 0x%lX not I/O for MultiMaster Host Adapter\n",
+ base_addr0);
+ blogic_err(NULL,
+ "at PCI Bus %d Device %d I/O Address 0x%lX\n",
+ bus, device, io_addr);
continue;
}
if (pci_resource_flags(pci_device, 1) & IORESOURCE_IO) {
- blogic_err("BusLogic: Base Address1 0x%lX not Memory for MultiMaster Host Adapter\n", NULL, base_addr1);
- blogic_err("at PCI Bus %d Device %d PCI Address 0x%lX\n", NULL, bus, device, pci_addr);
+ blogic_err(NULL,
+ "BusLogic: Base Address1 0x%lX not Memory for MultiMaster Host Adapter\n",
+ base_addr1);
+ blogic_err(NULL,
+ "at PCI Bus %d Device %d PCI Address 0x%lX\n",
+ bus, device, pci_addr);
continue;
}
if (irq_ch == 0) {
- blogic_err("BusLogic: IRQ Channel %d invalid for MultiMaster Host Adapter\n", NULL, irq_ch);
- blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
+ blogic_err(NULL,
+ "BusLogic: IRQ Channel %d invalid for MultiMaster Host Adapter\n",
+ irq_ch);
+ blogic_err(NULL,
+ "at PCI Bus %d Device %d I/O Address 0x%lX\n",
+ bus, device, io_addr);
continue;
}
if (blogic_global_options.trace_probe) {
- blogic_notice("BusLogic: PCI MultiMaster Host Adapter detected at\n", NULL);
- blogic_notice("BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n", NULL, bus, device, io_addr, pci_addr);
+ blogic_notice(NULL, "BusLogic: PCI MultiMaster Host Adapter detected at\n");
+ blogic_notice(NULL,
+ "BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n",
+ bus, device, io_addr, pci_addr);
}
/*
Issue the Inquire PCI Host Adapter Information command to determine
@@ -819,7 +840,7 @@ static int __init blogic_init_mm_probeinfo(struct blogic_adapter *adapter)
nonpr_mmcount++;
mmcount++;
} else
- blogic_warn("BusLogic: Too many Host Adapters detected\n", NULL);
+ blogic_warn(NULL, "BusLogic: Too many Host Adapters detected\n");
}
/*
If the AutoSCSI "Use Bus And Device # For PCI Scanning Seq."
@@ -957,23 +978,37 @@ static int __init blogic_init_fp_probeinfo(struct blogic_adapter *adapter)
pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
#ifdef CONFIG_SCSI_FLASHPOINT
if (pci_resource_flags(pci_device, 0) & IORESOURCE_MEM) {
- blogic_err("BusLogic: Base Address0 0x%lX not I/O for FlashPoint Host Adapter\n", NULL, base_addr0);
- blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
+ blogic_err(NULL,
+ "BusLogic: Base Address0 0x%lX not I/O for FlashPoint Host Adapter\n",
+ base_addr0);
+ blogic_err(NULL,
+ "at PCI Bus %d Device %d I/O Address 0x%lX\n",
+ bus, device, io_addr);
continue;
}
if (pci_resource_flags(pci_device, 1) & IORESOURCE_IO) {
- blogic_err("BusLogic: Base Address1 0x%lX not Memory for FlashPoint Host Adapter\n", NULL, base_addr1);
- blogic_err("at PCI Bus %d Device %d PCI Address 0x%lX\n", NULL, bus, device, pci_addr);
+ blogic_err(NULL,
+ "BusLogic: Base Address1 0x%lX not Memory for FlashPoint Host Adapter\n",
+ base_addr1);
+ blogic_err(NULL,
+ "at PCI Bus %d Device %d PCI Address 0x%lX\n",
+ bus, device, pci_addr);
continue;
}
if (irq_ch == 0) {
- blogic_err("BusLogic: IRQ Channel %d invalid for FlashPoint Host Adapter\n", NULL, irq_ch);
- blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
+ blogic_err(NULL,
+ "BusLogic: IRQ Channel %d invalid for FlashPoint Host Adapter\n",
+ irq_ch);
+ blogic_err(NULL,
+ "at PCI Bus %d Device %d I/O Address 0x%lX\n",
+ bus, device, io_addr);
continue;
}
if (blogic_global_options.trace_probe) {
- blogic_notice("BusLogic: FlashPoint Host Adapter detected at\n", NULL);
- blogic_notice("BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n", NULL, bus, device, io_addr, pci_addr);
+ blogic_notice(NULL, "BusLogic: FlashPoint Host Adapter detected at\n");
+ blogic_notice(NULL,
+ "BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n",
+ bus, device, io_addr, pci_addr);
}
if (blogic_probeinfo_count < BLOGIC_MAX_ADAPTERS) {
struct blogic_probeinfo *probeinfo =
@@ -988,11 +1023,15 @@ static int __init blogic_init_fp_probeinfo(struct blogic_adapter *adapter)
probeinfo->pci_device = pci_dev_get(pci_device);
fpcount++;
} else
- blogic_warn("BusLogic: Too many Host Adapters detected\n", NULL);
+ blogic_warn(NULL, "BusLogic: Too many Host Adapters detected\n");
#else
- blogic_err("BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n", NULL, bus, device);
- blogic_err("BusLogic: I/O Address 0x%lX PCI Address 0x%lX, irq %d, but FlashPoint\n", NULL, io_addr, pci_addr, irq_ch);
- blogic_err("BusLogic: support was omitted in this kernel configuration.\n", NULL);
+ blogic_err(NULL,
+ "BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n",
+ bus, device);
+ blogic_err(NULL,
+ "BusLogic: I/O Address 0x%lX PCI Address 0x%lX, irq %d, but FlashPoint\n",
+ io_addr, pci_addr, irq_ch);
+ blogic_err("BusLogic: support was omitted in this kernel configuration\n", NULL);
#endif
}
/*
@@ -1098,15 +1137,19 @@ static bool blogic_failure(struct blogic_adapter *adapter, char *msg)
{
blogic_announce_drvr(adapter);
if (adapter->adapter_bus_type == BLOGIC_PCI_BUS) {
- blogic_err("While configuring BusLogic PCI Host Adapter at\n",
- adapter);
- blogic_err("Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX:\n", adapter, adapter->bus, adapter->dev, adapter->io_addr, adapter->pci_addr);
+ blogic_err(adapter, "While configuring BusLogic PCI Host Adapter at\n");
+ blogic_err(adapter,
+ "Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX:\n",
+ adapter->bus, adapter->dev, adapter->io_addr,
+ adapter->pci_addr);
} else
- blogic_err("While configuring BusLogic Host Adapter at I/O Address 0x%lX:\n", adapter, adapter->io_addr);
- blogic_err("%s FAILED - DETACHING\n", adapter, msg);
+ blogic_err(adapter,
+ "While configuring BusLogic Host Adapter at I/O Address 0x%lX:\n",
+ adapter->io_addr);
+ blogic_err(adapter, "%s FAILED - DETACHING\n", msg);
if (blogic_cmd_failure_reason != NULL)
- blogic_err("ADDITIONAL FAILURE INFO - %s\n", adapter,
- blogic_cmd_failure_reason);
+ blogic_err(adapter, "ADDITIONAL FAILURE INFO - %s\n",
+ blogic_cmd_failure_reason);
return false;
}
@@ -1130,13 +1173,20 @@ static bool __init blogic_probe(struct blogic_adapter *adapter)
fpinfo->present = false;
if (!(FlashPoint_ProbeHostAdapter(fpinfo) == 0 &&
fpinfo->present)) {
- blogic_err("BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n", adapter, adapter->bus, adapter->dev);
- blogic_err("BusLogic: I/O Address 0x%lX PCI Address 0x%lX, but FlashPoint\n", adapter, adapter->io_addr, adapter->pci_addr);
- blogic_err("BusLogic: Probe Function failed to validate it.\n", adapter);
+ blogic_err(adapter,
+ "BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n",
+ adapter->bus, adapter->dev);
+ blogic_err(adapter,
+ "BusLogic: I/O Address 0x%lX PCI Address 0x%lX, but FlashPoint\n",
+ adapter->io_addr, adapter->pci_addr);
+ blogic_err(adapter,
+ "BusLogic: Probe Function failed to validate it\n");
return false;
}
if (blogic_global_options.trace_probe)
- blogic_notice("BusLogic_Probe(0x%lX): FlashPoint Found\n", adapter, adapter->io_addr);
+ blogic_notice(adapter,
+ "BusLogic_Probe(0x%lX): FlashPoint Found\n",
+ adapter->io_addr);
/*
Indicate the Host Adapter Probe completed successfully.
*/
@@ -1153,7 +1203,10 @@ static bool __init blogic_probe(struct blogic_adapter *adapter)
intreg.all = blogic_rdint(adapter);
georeg.all = blogic_rdgeom(adapter);
if (blogic_global_options.trace_probe)
- blogic_notice("BusLogic_Probe(0x%lX): Status 0x%02X, Interrupt 0x%02X, Geometry 0x%02X\n", adapter, adapter->io_addr, statusreg.all, intreg.all, georeg.all);
+ blogic_notice(adapter,
+ "BusLogic_Probe(0x%lX): Status 0x%02X, Interrupt 0x%02X, Geometry 0x%02X\n",
+ adapter->io_addr, statusreg.all, intreg.all,
+ georeg.all);
if (statusreg.all == 0 || statusreg.sr.diag_active ||
statusreg.sr.cmd_param_busy || statusreg.sr.rsvd ||
statusreg.sr.cmd_invalid || intreg.ir.rsvd != 0)
@@ -1232,7 +1285,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
udelay(100);
}
if (blogic_global_options.trace_hw_reset)
- blogic_notice("BusLogic_HardwareReset(0x%lX): Diagnostic Active, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
+ blogic_notice(adapter,
+ "BusLogic_HardwareReset(0x%lX): Diagnostic Active, Status 0x%02X\n",
+ adapter->io_addr, statusreg.all);
if (timeout < 0)
return false;
/*
@@ -1252,7 +1307,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
udelay(100);
}
if (blogic_global_options.trace_hw_reset)
- blogic_notice("BusLogic_HardwareReset(0x%lX): Diagnostic Completed, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
+ blogic_notice(adapter,
+ "BusLogic_HardwareReset(0x%lX): Diagnostic Completed, Status 0x%02X\n",
+ adapter->io_addr, statusreg.all);
if (timeout < 0)
return false;
/*
@@ -1268,7 +1325,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
udelay(100);
}
if (blogic_global_options.trace_hw_reset)
- blogic_notice("BusLogic_HardwareReset(0x%lX): Host Adapter Ready, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
+ blogic_notice(adapter,
+ "BusLogic_HardwareReset(0x%lX): Host Adapter Ready, Status 0x%02X\n",
+ adapter->io_addr, statusreg.all);
if (timeout < 0)
return false;
/*
@@ -1280,11 +1339,11 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
if (statusreg.sr.diag_failed || !statusreg.sr.adapter_ready) {
blogic_cmd_failure_reason = NULL;
blogic_failure(adapter, "HARD RESET DIAGNOSTICS");
- blogic_err("HOST ADAPTER STATUS REGISTER = %02X\n", adapter,
- statusreg.all);
+ blogic_err(adapter, "HOST ADAPTER STATUS REGISTER = %02X\n",
+ statusreg.all);
if (statusreg.sr.datain_ready)
- blogic_err("HOST ADAPTER ERROR CODE = %d\n", adapter,
- blogic_rddatain(adapter));
+ blogic_err(adapter, "HOST ADAPTER ERROR CODE = %d\n",
+ blogic_rddatain(adapter));
return false;
}
/*
@@ -1324,9 +1383,10 @@ static bool __init blogic_checkadapter(struct blogic_adapter *adapter)
Provide tracing information if requested and return.
*/
if (blogic_global_options.trace_probe)
- blogic_notice("BusLogic_Check(0x%lX): MultiMaster %s\n", adapter,
- adapter->io_addr,
- (result ? "Found" : "Not Found"));
+ blogic_notice(adapter,
+ "BusLogic_Check(0x%lX): MultiMaster %s\n",
+ adapter->io_addr,
+ (result ? "Found" : "Not Found"));
return result;
}
@@ -1836,30 +1896,40 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
char *tagq_msg = tagq_str;
int tgt_id;
- blogic_info("Configuring BusLogic Model %s %s%s%s%s SCSI Host Adapter\n", adapter, adapter->model, blogic_adapter_busnames[adapter->adapter_bus_type], (adapter->wide ? " Wide" : ""), (adapter->differential ? " Differential" : ""), (adapter->ultra ? " Ultra" : ""));
- blogic_info(" Firmware Version: %s, I/O Address: 0x%lX, IRQ Channel: %d/%s\n", adapter, adapter->fw_ver, adapter->io_addr, adapter->irq_ch, (adapter->level_int ? "Level" : "Edge"));
+ blogic_info(adapter,
+ "Configuring BusLogic Model %s %s%s%s%s SCSI Host Adapter\n",
+ adapter->model,
+ blogic_adapter_busnames[adapter->adapter_bus_type],
+ (adapter->wide ? " Wide" : ""),
+ (adapter->differential ? " Differential" : ""),
+ (adapter->ultra ? " Ultra" : ""));
+ blogic_info(adapter,
+ " Firmware Version: %s, I/O Address: 0x%lX, IRQ Channel: %d/%s\n",
+ adapter->fw_ver, adapter->io_addr, adapter->irq_ch,
+ (adapter->level_int ? "Level" : "Edge"));
if (adapter->adapter_bus_type != BLOGIC_PCI_BUS) {
- blogic_info(" DMA Channel: ", adapter);
+ blogic_info(adapter, " DMA Channel: ");
if (adapter->dma_ch > 0)
- blogic_info("%d, ", adapter, adapter->dma_ch);
+ blogic_cont(adapter, "%d, ", adapter->dma_ch);
else
- blogic_info("None, ", adapter);
+ blogic_cont(adapter, "None, ");
if (adapter->bios_addr > 0)
- blogic_info("BIOS Address: 0x%lX, ", adapter,
- adapter->bios_addr);
+ blogic_cont(adapter, "BIOS Address: 0x%X, ",
+ adapter->bios_addr);
else
- blogic_info("BIOS Address: None, ", adapter);
+ blogic_cont(adapter, "BIOS Address: None, ");
} else {
- blogic_info(" PCI Bus: %d, Device: %d, Address: ", adapter,
- adapter->bus, adapter->dev);
+ blogic_cont(adapter, " PCI Bus: %d, Device: %d, Address: ",
+ adapter->bus, adapter->dev);
if (adapter->pci_addr > 0)
- blogic_info("0x%lX, ", adapter, adapter->pci_addr);
+ blogic_cont(adapter, "0x%lX, ", adapter->pci_addr);
else
- blogic_info("Unassigned, ", adapter);
+ blogic_cont(adapter, "Unassigned, ");
}
- blogic_info("Host Adapter SCSI ID: %d\n", adapter, adapter->scsi_id);
- blogic_info(" Parity Checking: %s, Extended Translation: %s\n",
- adapter, (adapter->parity ? "Enabled" : "Disabled"),
+ blogic_cont(adapter, "Host Adapter SCSI ID: %d\n", adapter->scsi_id);
+ blogic_info(adapter,
+ " Parity Checking: %s, Extended Translation: %s\n",
+ (adapter->parity ? "Enabled" : "Disabled"),
(adapter->ext_trans_enable ? "Enabled" : "Disabled"));
alltgt_mask &= ~(1 << adapter->scsi_id);
sync_ok = adapter->sync_ok & alltgt_mask;
@@ -1928,16 +1998,25 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
tagq_str[adapter->scsi_id] = '#';
tagq_str[adapter->maxdev] = '\0';
}
- blogic_info(" Synchronous Negotiation: %s, Wide Negotiation: %s\n",
- adapter, syncmsg, widemsg);
- blogic_info(" Disconnect/Reconnect: %s, Tagged Queuing: %s\n", adapter,
- discon_msg, tagq_msg);
+ blogic_info(adapter,
+ " Synchronous Negotiation: %s, Wide Negotiation: %s\n",
+ syncmsg, widemsg);
+ blogic_info(adapter,
+ " Disconnect/Reconnect: %s, Tagged Queuing: %s\n",
+ discon_msg, tagq_msg);
if (blogic_multimaster_type(adapter)) {
- blogic_info(" Scatter/Gather Limit: %d of %d segments, Mailboxes: %d\n", adapter, adapter->drvr_sglimit, adapter->adapter_sglimit, adapter->mbox_count);
- blogic_info(" Driver Queue Depth: %d, Host Adapter Queue Depth: %d\n", adapter, adapter->drvr_qdepth, adapter->adapter_qdepth);
+ blogic_info(adapter,
+ " Scatter/Gather Limit: %d of %d segments, Mailboxes: %d\n",
+ adapter->drvr_sglimit, adapter->adapter_sglimit,
+ adapter->mbox_count);
+ blogic_info(adapter,
+ " Driver Queue Depth: %d, Host Adapter Queue Depth: %d\n",
+ adapter->drvr_qdepth, adapter->adapter_qdepth);
} else
- blogic_info(" Driver Queue Depth: %d, Scatter/Gather Limit: %d segments\n", adapter, adapter->drvr_qdepth, adapter->drvr_sglimit);
- blogic_info(" Tagged Queue Depth: ", adapter);
+ blogic_info(adapter,
+ " Driver Queue Depth: %d, Scatter/Gather Limit: %d segments\n",
+ adapter->drvr_qdepth, adapter->drvr_sglimit);
+ blogic_info(adapter, " Tagged Queue Depth: ");
common_tagq_depth = true;
for (tgt_id = 1; tgt_id < adapter->maxdev; tgt_id++)
if (adapter->qdepth[tgt_id] != adapter->qdepth[0]) {
@@ -1946,24 +2025,28 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
}
if (common_tagq_depth) {
if (adapter->qdepth[0] > 0)
- blogic_info("%d", adapter, adapter->qdepth[0]);
+ blogic_cont(adapter, "%d", adapter->qdepth[0]);
else
- blogic_info("Automatic", adapter);
+ blogic_cont(adapter, "Automatic");
} else
- blogic_info("Individual", adapter);
- blogic_info(", Untagged Queue Depth: %d\n", adapter,
- adapter->untag_qdepth);
+ blogic_cont(adapter, "Individual");
+ blogic_cont(adapter, ", Untagged Queue Depth: %d\n",
+ adapter->untag_qdepth);
if (adapter->terminfo_valid) {
if (adapter->wide)
- blogic_info(" SCSI Bus Termination: %s", adapter,
- (adapter->low_term ? (adapter->high_term ? "Both Enabled" : "Low Enabled") : (adapter->high_term ? "High Enabled" : "Both Disabled")));
+ blogic_info(adapter, " SCSI Bus Termination: %s",
+ (adapter->low_term
+ ? (adapter->high_term ? "Both Enabled" : "Low Enabled")
+ : (adapter->high_term ? "High Enabled" : "Both Disabled")));
else
- blogic_info(" SCSI Bus Termination: %s", adapter,
- (adapter->low_term ? "Enabled" : "Disabled"));
+ blogic_info(adapter, " SCSI Bus Termination: %s",
+ (adapter->low_term ? "Enabled" : "Disabled"));
if (adapter->scam)
- blogic_info(", SCAM: %s", adapter,
- (adapter->scam_enabled ? (adapter->scam_lev2 ? "Enabled, Level 2" : "Enabled, Level 1") : "Disabled"));
- blogic_info("\n", adapter);
+ blogic_cont(adapter, ", SCAM: %s",
+ (adapter->scam_enabled
+ ? (adapter->scam_lev2 ? "Enabled, Level 2" : "Enabled, Level 1")
+ : "Disabled"));
+ blogic_cont(adapter, "\n");
}
/*
Indicate reporting the Host Adapter configuration completed
@@ -1981,8 +2064,8 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
static bool __init blogic_getres(struct blogic_adapter *adapter)
{
if (adapter->irq_ch == 0) {
- blogic_err("NO LEGAL INTERRUPT CHANNEL ASSIGNED - DETACHING\n",
- adapter);
+ blogic_err(adapter,
+ "NO LEGAL INTERRUPT CHANNEL ASSIGNED - DETACHING\n");
return false;
}
/*
@@ -1990,8 +2073,9 @@ static bool __init blogic_getres(struct blogic_adapter *adapter)
*/
if (request_irq(adapter->irq_ch, blogic_inthandler, IRQF_SHARED,
adapter->full_model, adapter) < 0) {
- blogic_err("UNABLE TO ACQUIRE IRQ CHANNEL %d - DETACHING\n",
- adapter, adapter->irq_ch);
+ blogic_err(adapter,
+ "UNABLE TO ACQUIRE IRQ CHANNEL %d - DETACHING\n",
+ adapter->irq_ch);
return false;
}
adapter->irq_acquired = true;
@@ -2000,7 +2084,9 @@ static bool __init blogic_getres(struct blogic_adapter *adapter)
*/
if (adapter->dma_ch > 0) {
if (request_dma(adapter->dma_ch, adapter->full_model) < 0) {
- blogic_err("UNABLE TO ACQUIRE DMA CHANNEL %d - DETACHING\n", adapter, adapter->dma_ch);
+ blogic_err(adapter,
+ "UNABLE TO ACQUIRE DMA CHANNEL %d - DETACHING\n",
+ adapter->dma_ch);
return false;
}
set_dma_mode(adapter->dma_ch, DMA_MODE_CASCADE);
@@ -2148,12 +2234,12 @@ static bool blogic_initadapter(struct blogic_adapter *adapter)
*/
done:
if (!adapter->adapter_initd) {
- blogic_info("*** %s Initialized Successfully ***\n", adapter,
- adapter->full_model);
- blogic_info("\n", adapter);
+ blogic_info(adapter, "*** %s Initialized Successfully ***\n",
+ adapter->full_model);
+ blogic_info(adapter, "\n");
} else
- blogic_warn("*** %s Initialized Successfully ***\n", adapter,
- adapter->full_model);
+ blogic_warn(adapter, "*** %s Initialized Successfully ***\n",
+ adapter->full_model);
adapter->adapter_initd = true;
/*
@@ -2365,15 +2451,16 @@ static int __init blogic_init(void)
kcalloc(BLOGIC_MAX_ADAPTERS, sizeof(struct blogic_probeinfo),
GFP_KERNEL);
if (blogic_probeinfo_list == NULL) {
- blogic_err("BusLogic: Unable to allocate Probe Info List\n",
- NULL);
+ blogic_err(NULL,
+ "BusLogic: Unable to allocate Probe Info List\n");
return -ENOMEM;
}
adapter = kzalloc(sizeof(struct blogic_adapter), GFP_KERNEL);
if (adapter == NULL) {
kfree(blogic_probeinfo_list);
- blogic_err("BusLogic: Unable to allocate Prototype Host Adapter\n", NULL);
+ blogic_err(NULL,
+ "BusLogic: Unable to allocate Prototype Host Adapter\n");
return -ENOMEM;
}
@@ -2633,8 +2720,9 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
case BLOGIC_INVALID_OUTBOX_CODE:
case BLOGIC_INVALID_CMD_CODE:
case BLOGIC_BAD_CMD_PARAM:
- blogic_warn("BusLogic Driver Protocol Error 0x%02X\n",
- adapter, adapter_status);
+ blogic_warn(adapter,
+ "BusLogic Driver Protocol Error 0x%02X\n",
+ adapter_status);
fallthrough;
case BLOGIC_DATA_UNDERRUN:
case BLOGIC_DATA_OVERRUN:
@@ -2659,8 +2747,8 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
hoststatus = DID_RESET;
break;
default:
- blogic_warn("Unknown Host Adapter Status 0x%02X\n", adapter,
- adapter_status);
+ blogic_warn(adapter, "Unknown Host Adapter Status 0x%02X\n",
+ adapter_status);
hoststatus = DID_ERROR;
break;
}
@@ -2718,7 +2806,9 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter)
then there is most likely a bug in
the Host Adapter firmware.
*/
- blogic_warn("Illegal CCB #%ld status %d in Incoming Mailbox\n", adapter, ccb->serial, ccb->status);
+ blogic_warn(adapter,
+ "Illegal CCB #%ld status %d in Incoming Mailbox\n",
+ ccb->serial, ccb->status);
}
}
next_inbox->comp_code = BLOGIC_INBOX_FREE;
@@ -2753,7 +2843,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
if (ccb->opcode == BLOGIC_BDR) {
int tgt_id = ccb->tgt_id;
- blogic_warn("Bus Device Reset CCB #%ld to Target %d Completed\n", adapter, ccb->serial, tgt_id);
+ blogic_warn(adapter,
+ "Bus Device Reset CCB #%ld to Target %d Completed\n",
+ ccb->serial, tgt_id);
blogic_inc_count(&adapter->tgt_stats[tgt_id].bdr_done);
adapter->tgt_flags[tgt_id].tagq_active = false;
adapter->cmds_since_rst[tgt_id] = 0;
@@ -2806,7 +2898,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
case BLOGIC_INBOX_FREE:
case BLOGIC_CMD_NOTFOUND:
case BLOGIC_INVALID_CCB:
- blogic_warn("CCB #%ld to Target %d Impossible State\n", adapter, ccb->serial, ccb->tgt_id);
+ blogic_warn(adapter,
+ "CCB #%ld to Target %d Impossible State\n",
+ ccb->serial, ccb->tgt_id);
break;
case BLOGIC_CMD_COMPLETE_GOOD:
adapter->tgt_stats[ccb->tgt_id]
@@ -2816,8 +2910,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
command->result = DID_OK << 16;
break;
case BLOGIC_CMD_ABORT_BY_HOST:
- blogic_warn("CCB #%ld to Target %d Aborted\n",
- adapter, ccb->serial, ccb->tgt_id);
+ blogic_warn(adapter,
+ "CCB #%ld to Target %d Aborted\n",
+ ccb->serial, ccb->tgt_id);
blogic_inc_count(&adapter->tgt_stats[ccb->tgt_id].aborts_done);
command->result = DID_ABORT << 16;
break;
@@ -2829,16 +2924,26 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
.cmds_complete++;
if (blogic_global_options.trace_err) {
int i;
- blogic_notice("CCB #%ld Target %d: Result %X Host "
- "Adapter Status %02X Target Status %02X\n", adapter, ccb->serial, ccb->tgt_id, command->result, ccb->adapter_status, ccb->tgt_status);
- blogic_notice("CDB ", adapter);
+ blogic_notice(adapter,
+ "CCB #%ld Target %d: Result %X Host Adapter Status %02X Target Status %02X\n",
+ ccb->serial,
+ ccb->tgt_id,
+ command->result,
+ ccb->adapter_status,
+ ccb->tgt_status);
+ blogic_notice(adapter,
+ "CDB ");
for (i = 0; i < ccb->cdblen; i++)
- blogic_notice(" %02X", adapter, ccb->cdb[i]);
- blogic_notice("\n", adapter);
- blogic_notice("Sense ", adapter);
+ blogic_cont(adapter,
+ " %02X",
+ ccb->cdb[i]);
+ blogic_cont(adapter, "\n");
+ blogic_notice(adapter, "Sense ");
for (i = 0; i < ccb->sense_datalen; i++)
- blogic_notice(" %02X", adapter, command->sense_buffer[i]);
- blogic_notice("\n", adapter);
+ blogic_cont(adapter,
+ " %02X",
+ command->sense_buffer[i]);
+ blogic_cont(adapter, "\n");
}
}
break;
@@ -2925,7 +3030,8 @@ static irqreturn_t blogic_inthandler(int irq_ch, void *devid)
adapter->adapter_extreset = true;
break;
case FPOINT_INTERN_ERR:
- blogic_warn("Internal FlashPoint Error detected - Resetting Host Adapter\n", adapter);
+ blogic_warn(adapter,
+ "Internal FlashPoint Error detected - Resetting Host Adapter\n");
adapter->adapter_intern_err = true;
break;
}
@@ -2939,12 +3045,16 @@ static irqreturn_t blogic_inthandler(int irq_ch, void *devid)
Reset the Host Adapter if requested.
*/
if (adapter->adapter_extreset) {
- blogic_warn("Resetting %s due to External SCSI Bus Reset\n", adapter, adapter->full_model);
+ blogic_warn(adapter,
+ "Resetting %s due to External SCSI Bus Reset\n",
+ adapter->full_model);
blogic_inc_count(&adapter->ext_resets);
blogic_resetadapter(adapter, false);
adapter->adapter_extreset = false;
} else if (adapter->adapter_intern_err) {
- blogic_warn("Resetting %s due to Host Adapter Internal Error\n", adapter, adapter->full_model);
+ blogic_warn(adapter,
+ "Resetting %s due to Host Adapter Internal Error\n",
+ adapter->full_model);
blogic_inc_count(&adapter->adapter_intern_errors);
blogic_resetadapter(adapter, true);
adapter->adapter_intern_err = false;
@@ -3142,8 +3252,9 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
&& tgt_flags->tagq_ok &&
(adapter->tagq_ok & (1 << tgt_id))) {
tgt_flags->tagq_active = true;
- blogic_notice("Tagged Queuing now active for Target %d\n",
- adapter, tgt_id);
+ blogic_notice(adapter,
+ "Tagged Queuing now active for Target %d\n",
+ tgt_id);
}
if (tgt_flags->tagq_active) {
enum blogic_queuetag queuetag = BLOGIC_SIMPLETAG;
@@ -3184,8 +3295,8 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
command->sense_buffer, ccb->sense_datalen,
DMA_FROM_DEVICE);
if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
- blogic_err("DMA mapping for sense data buffer failed\n",
- adapter);
+ blogic_err(adapter,
+ "DMA mapping for sense data buffer failed\n");
blogic_dealloc_ccb(ccb, 0);
return SCSI_MLQUEUE_HOST_BUSY;
}
@@ -3204,12 +3315,12 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
*/
if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START, ccb)) {
spin_unlock_irq(adapter->scsi_host->host_lock);
- blogic_warn("Unable to write Outgoing Mailbox - Pausing for 1 second\n", adapter);
+ blogic_warn(adapter, "Unable to write Outgoing Mailbox - Pausing for 1 second\n");
blogic_delay(1);
spin_lock_irq(adapter->scsi_host->host_lock);
if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START,
ccb)) {
- blogic_warn("Still unable to write Outgoing Mailbox - Host Adapter Dead?\n", adapter);
+ blogic_warn(adapter, "Still unable to write Outgoing Mailbox - Host Adapter Dead?\n");
blogic_dealloc_ccb(ccb, 1);
command->result = DID_ERROR << 16;
command->scsi_done(command);
@@ -3259,13 +3370,16 @@ static int blogic_abort(struct scsi_cmnd *command)
if (ccb->command == command)
break;
if (ccb == NULL) {
- blogic_warn("Unable to Abort Command to Target %d - No CCB Found\n", adapter, tgt_id);
+ blogic_warn(adapter, "Unable to Abort Command to Target %d - No CCB Found\n",
+ tgt_id);
return SUCCESS;
} else if (ccb->status == BLOGIC_CCB_COMPLETE) {
- blogic_warn("Unable to Abort Command to Target %d - CCB Completed\n", adapter, tgt_id);
+ blogic_warn(adapter, "Unable to Abort Command to Target %d - CCB Completed\n",
+ tgt_id);
return SUCCESS;
} else if (ccb->status == BLOGIC_CCB_RESET) {
- blogic_warn("Unable to Abort Command to Target %d - CCB Reset\n", adapter, tgt_id);
+ blogic_warn(adapter, "Unable to Abort Command to Target %d - CCB Reset\n",
+ tgt_id);
return SUCCESS;
}
if (blogic_multimaster_type(adapter)) {
@@ -3283,16 +3397,18 @@ static int blogic_abort(struct scsi_cmnd *command)
*/
if (adapter->tgt_flags[tgt_id].tagq_active &&
adapter->fw_ver[0] < '5') {
- blogic_warn("Unable to Abort CCB #%ld to Target %d - Abort Tag Not Supported\n", adapter, ccb->serial, tgt_id);
+ blogic_warn(adapter, "Unable to Abort CCB #%ld to Target %d - Abort Tag Not Supported\n",
+ ccb->serial, tgt_id);
return FAILURE;
} else if (blogic_write_outbox(adapter, BLOGIC_MBOX_ABORT,
ccb)) {
- blogic_warn("Aborting CCB #%ld to Target %d\n",
- adapter, ccb->serial, tgt_id);
+ blogic_warn(adapter, "Aborting CCB #%ld to Target %d\n",
+ ccb->serial, tgt_id);
blogic_inc_count(&adapter->tgt_stats[tgt_id].aborts_tried);
return SUCCESS;
} else {
- blogic_warn("Unable to Abort CCB #%ld to Target %d - No Outgoing Mailboxes\n", adapter, ccb->serial, tgt_id);
+ blogic_warn(adapter, "Unable to Abort CCB #%ld to Target %d - No Outgoing Mailboxes\n",
+ ccb->serial, tgt_id);
return FAILURE;
}
} else {
@@ -3300,8 +3416,8 @@ static int blogic_abort(struct scsi_cmnd *command)
Call the FlashPoint SCCB Manager to abort execution of
the CCB.
*/
- blogic_warn("Aborting CCB #%ld to Target %d\n", adapter,
- ccb->serial, tgt_id);
+ blogic_warn(adapter, "Aborting CCB #%ld to Target %d\n",
+ ccb->serial, tgt_id);
blogic_inc_count(&adapter->tgt_stats[tgt_id].aborts_tried);
FlashPoint_AbortCCB(adapter->cardhandle, ccb);
/*
@@ -3333,8 +3449,8 @@ static int blogic_resetadapter(struct blogic_adapter *adapter, bool hard_reset)
if (!(blogic_hwreset(adapter, hard_reset) &&
blogic_initadapter(adapter))) {
- blogic_err("Resetting %s Failed\n", adapter,
- adapter->full_model);
+ blogic_err(adapter, "Resetting %s Failed\n",
+ adapter->full_model);
return FAILURE;
}
@@ -3443,10 +3559,15 @@ static int blogic_diskparam(struct scsi_device *sdev, struct block_device *dev,
diskparam->cylinders = (unsigned long) capacity / (diskparam->heads * diskparam->sectors);
if (part_no < 4 && part_end_sector == diskparam->sectors) {
if (diskparam->cylinders != saved_cyl)
- blogic_warn("Adopting Geometry %d/%d from Partition Table\n", adapter, diskparam->heads, diskparam->sectors);
+ blogic_warn(adapter,
+ "Adopting Geometry %d/%d from Partition Table\n",
+ diskparam->heads,
+ diskparam->sectors);
} else if (part_end_head > 0 || part_end_sector > 0) {
- blogic_warn("Warning: Partition Table appears to have Geometry %d/%d which is\n", adapter, part_end_head + 1, part_end_sector);
- blogic_warn("not compatible with current BusLogic Host Adapter Geometry %d/%d\n", adapter, diskparam->heads, diskparam->sectors);
+ blogic_warn(adapter,
+ "Warning: Partition Table appears to have Geometry %d/%d which is not compatible with current BusLogic Host Adapter Geometry %d/%d\n",
+ part_end_head + 1, part_end_sector,
+ diskparam->heads, diskparam->sectors);
}
}
kfree(buf);
@@ -3571,6 +3692,7 @@ Target Requested Completed Requested Completed Requested Completed\n\
}
seq_printf(m, "\nExternal Host Adapter Resets: %d\n", adapter->ext_resets);
seq_printf(m, "Host Adapter Internal Errors: %d\n", adapter->adapter_intern_errors);
+
return 0;
}
@@ -3579,41 +3701,51 @@ Target Requested Completed Requested Completed Requested Completed\n\
blogic_msg prints Driver Messages.
*/
-static void blogic_msg(enum blogic_msglevel msglevel, char *fmt,
- struct blogic_adapter *adapter, ...)
+static void blogic_msg(enum blogic_msglevel msglevel,
+ struct blogic_adapter *adapter, const char *fmt, ...)
{
static char buf[BLOGIC_LINEBUF_SIZE];
- static bool begin = true;
va_list args;
int len = 0;
+ const char *kern_level = blogic_msglevelmap[msglevel];
- va_start(args, adapter);
- len = vsprintf(buf, fmt, args);
+ va_start(args, fmt);
+ len = vscnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
- if (msglevel == BLOGIC_ANNOUNCE_LEVEL) {
+
+ switch (msglevel) {
+ case BLOGIC_ANNOUNCE_LEVEL: {
static int msglines = 0;
+
strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
adapter->msgbuflen += len;
if (++msglines <= 2)
- printk("%sscsi: %s", blogic_msglevelmap[msglevel], buf);
- } else if (msglevel == BLOGIC_INFO_LEVEL) {
+ printk("%sscsi: %s", kern_level, buf);
+ break;
+ }
+
+ case BLOGIC_INFO_LEVEL:
strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
adapter->msgbuflen += len;
- if (begin) {
- if (buf[0] != '\n' || len > 1)
- printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
- } else
- printk("%s", buf);
- } else {
- if (begin) {
- if (adapter != NULL && adapter->adapter_initd)
- printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
- else
- printk("%s%s", blogic_msglevelmap[msglevel], buf);
- } else
- printk("%s", buf);
+ if (buf[0] != '\n' || len > 1)
+ printk("%sscsi%d: %s",
+ kern_level, adapter->host_no, buf);
+ break;
+
+ case BLOGIC_CONT_LEVEL:
+ strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
+ adapter->msgbuflen += len;
+ printk("%s%s", kern_level, buf);
+ break;
+
+ default:
+ if (adapter && adapter->adapter_initd)
+ printk("%sscsi%d: %s",
+ kern_level, adapter->host_no, buf);
+ else
+ printk("%s%s", kern_level, buf);
+ break;
}
- begin = (buf[len - 1] == '\n');
}
@@ -3691,7 +3823,9 @@ static int __init blogic_parseopts(char *options)
blogic_probe_options.probe134 = true;
break;
default:
- blogic_err("BusLogic: Invalid Driver Options (invalid I/O Address 0x%lX)\n", NULL, io_addr);
+ blogic_err(NULL,
+ "BusLogic: Invalid Driver Options (invalid I/O Address 0x%lX)\n",
+ io_addr);
return 0;
}
} else if (blogic_parse(&options, "NoProbeISA"))
@@ -3712,7 +3846,9 @@ static int __init blogic_parseopts(char *options)
for (tgt_id = 0; tgt_id < BLOGIC_MAXDEV; tgt_id++) {
unsigned short qdepth = simple_strtoul(options, &options, 0);
if (qdepth > BLOGIC_MAX_TAG_DEPTH) {
- blogic_err("BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n", NULL, qdepth);
+ blogic_err(NULL,
+ "BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n",
+ qdepth);
return 0;
}
drvr_opts->qdepth[tgt_id] = qdepth;
@@ -3721,12 +3857,16 @@ static int __init blogic_parseopts(char *options)
else if (*options == ']')
break;
else {
- blogic_err("BusLogic: Invalid Driver Options (',' or ']' expected at '%s')\n", NULL, options);
+ blogic_err(NULL,
+ "BusLogic: Invalid Driver Options (',' or ']' expected at '%s')\n",
+ options);
return 0;
}
}
if (*options != ']') {
- blogic_err("BusLogic: Invalid Driver Options (']' expected at '%s')\n", NULL, options);
+ blogic_err(NULL,
+ "BusLogic: Invalid Driver Options (']' expected at '%s')\n",
+ options);
return 0;
} else
options++;
@@ -3734,7 +3874,9 @@ static int __init blogic_parseopts(char *options)
unsigned short qdepth = simple_strtoul(options, &options, 0);
if (qdepth == 0 ||
qdepth > BLOGIC_MAX_TAG_DEPTH) {
- blogic_err("BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n", NULL, qdepth);
+ blogic_err(NULL,
+ "BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n",
+ qdepth);
return 0;
}
drvr_opts->common_qdepth = qdepth;
@@ -3780,7 +3922,9 @@ static int __init blogic_parseopts(char *options)
unsigned short bus_settle_time =
simple_strtoul(options, &options, 0);
if (bus_settle_time > 5 * 60) {
- blogic_err("BusLogic: Invalid Driver Options (invalid Bus Settle Time %d)\n", NULL, bus_settle_time);
+ blogic_err(NULL,
+ "BusLogic: Invalid Driver Options (invalid Bus Settle Time %d)\n",
+ bus_settle_time);
return 0;
}
drvr_opts->bus_settle_time = bus_settle_time;
@@ -3805,14 +3949,17 @@ static int __init blogic_parseopts(char *options)
if (*options == ',')
options++;
else if (*options != ';' && *options != '\0') {
- blogic_err("BusLogic: Unexpected Driver Option '%s' ignored\n", NULL, options);
+ blogic_err(NULL,
+ "BusLogic: Unexpected Driver Option '%s' ignored\n",
+ options);
*options = '\0';
}
}
if (!(blogic_drvr_options_count == 0 ||
blogic_probeinfo_count == 0 ||
blogic_drvr_options_count == blogic_probeinfo_count)) {
- blogic_err("BusLogic: Invalid Driver Options (all or no I/O Addresses must be specified)\n", NULL);
+ blogic_err(NULL,
+ "BusLogic: Invalid Driver Options (all or no I/O Addresses must be specified)\n");
return 0;
}
/*
@@ -3866,7 +4013,8 @@ static int __init blogic_setup(char *str)
(void) get_options(str, ARRAY_SIZE(ints), ints);
if (ints[0] != 0) {
- blogic_err("BusLogic: Obsolete Command Line Entry Format Ignored\n", NULL);
+ blogic_err(NULL,
+ "BusLogic: Obsolete Command Line Entry Format Ignored\n");
return 0;
}
if (str == NULL || *str == '\0')
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 6182cc8a0344..42333fba6005 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -108,30 +108,34 @@ enum blogic_msglevel {
BLOGIC_INFO_LEVEL = 1,
BLOGIC_NOTICE_LEVEL = 2,
BLOGIC_WARN_LEVEL = 3,
- BLOGIC_ERR_LEVEL = 4
+ BLOGIC_ERR_LEVEL = 4,
+ BLOGIC_CONT_LEVEL = 5,
};
-static char *blogic_msglevelmap[] = { KERN_NOTICE, KERN_NOTICE, KERN_NOTICE, KERN_WARNING, KERN_ERR };
+static char *blogic_msglevelmap[] = { KERN_NOTICE, KERN_NOTICE, KERN_NOTICE, KERN_WARNING, KERN_ERR, KERN_CONT };
/*
Define Driver Message macros.
*/
-#define blogic_announce(format, args...) \
- blogic_msg(BLOGIC_ANNOUNCE_LEVEL, format, ##args)
+#define blogic_announce(adapter, fmt, ...) \
+ blogic_msg(BLOGIC_ANNOUNCE_LEVEL, adapter, fmt, ##__VA_ARGS__)
-#define blogic_info(format, args...) \
- blogic_msg(BLOGIC_INFO_LEVEL, format, ##args)
+#define blogic_cont(adapter, fmt, ...) \
+ blogic_msg(BLOGIC_CONT_LEVEL, adapter, fmt, ##__VA_ARGS__)
-#define blogic_notice(format, args...) \
- blogic_msg(BLOGIC_NOTICE_LEVEL, format, ##args)
+#define blogic_info(adapter, fmt, ...) \
+ blogic_msg(BLOGIC_INFO_LEVEL, adapter, fmt, ##__VA_ARGS__)
-#define blogic_warn(format, args...) \
- blogic_msg(BLOGIC_WARN_LEVEL, format, ##args)
+#define blogic_notice(adapter, fmt, ...) \
+ blogic_msg(BLOGIC_NOTICE_LEVEL, adapter, fmt, ##__VA_ARGS__)
-#define blogic_err(format, args...) \
- blogic_msg(BLOGIC_ERR_LEVEL, format, ##args)
+#define blogic_warn(adapter, fmt, ...) \
+ blogic_msg(BLOGIC_WARN_LEVEL, adapter, fmt, ##__VA_ARGS__)
+
+#define blogic_err(adapter, fmt, ...) \
+ blogic_msg(BLOGIC_ERR_LEVEL, adapter, fmt, ##__VA_ARGS__)
/*
@@ -1289,7 +1293,9 @@ static int blogic_slaveconfig(struct scsi_device *);
static void blogic_qcompleted_ccb(struct blogic_ccb *);
static irqreturn_t blogic_inthandler(int, void *);
static int blogic_resetadapter(struct blogic_adapter *, bool hard_reset);
-static void blogic_msg(enum blogic_msglevel, char *, struct blogic_adapter *, ...);
+__printf(3, 4)
+static void blogic_msg(enum blogic_msglevel, struct blogic_adapter *adapter,
+ const char *fmt, ...);
static int __init blogic_setup(char *);
#endif /* _BUSLOGIC_H */
On Thu, 15 Apr 2021, Joe Perches wrote:
> In patch 2, vscnprintf should probably be used to make sure it's
> 0 terminated.
Why? C99 has this[1]:
"The vsnprintf function is equivalent to snprintf, with the variable
argument list replaced by arg, which shall have been initialized by the
va_start macro (and possibly subsequent va_arg calls)."
and then[2]:
"The snprintf function is equivalent to fprintf, except that the output
is written into an array (specified by argument s) rather than to a
stream. If n is zero, nothing is written, and s may be a null pointer.
Otherwise, output characters beyond the n-1st are discarded rather than
being written to the array, and a null character is written at the end
of the characters actually written into the array."
therefore output from `vsnprintf' is always null-terminated.
> And while it's a lot more code, I'd prefer a solution that looks more
> like the other commonly used kernel logging extension mechanisms
> where adapter is placed before the format, ... in the argument list.
I agree having `adapter' as the second argument seems weird, so that is
fine with me as a follow-up cleanup. However as a user-visible change I
think the fix I propose here ought to be applied first (and backported
as suitable). Then any internal clean-ups can follow, applied to trunk
only.
> And there's a simple addition of a blogic_cont macro and extension
> to blogic_msg to simplify the logic and obviousness of the logging
> extension lines too.
I did this first actually, before I realised a simpler change suitable
for backporting could be done. I'm not sure if that complex message
routing via `blogic_msg' is worth having even, rather than calling
`printk' or suitable variants directly.
References:
[1] "Programming languages -- C", INTERNATIONAL STANDARD, ISO/IEC 9899,
Second edition, 1999-12-01, Section 7.19.6.12 "The vsnprintf
function", p.293
[2] same, 7.19.6.5 "The snprintf function", p.289
Maciej
On Fri, 2021-04-16 at 12:48 +0200, Maciej W. Rozycki wrote:
> I'm not sure if that complex message
> routing via `blogic_msg' is worth having even, rather than calling
> `printk' or suitable variants directly.
It's to allow the message content to be added to the internal
&adapter->msgbuf[adapter->msgbuflen]
with strcpy for later use with blogic_show_info()/seq_write.
On Fri, 16 Apr 2021, Joe Perches wrote:
> > I'm not sure if that complex message
> > routing via `blogic_msg' is worth having even, rather than calling
> > `printk' or suitable variants directly.
>
> It's to allow the message content to be added to the internal
> &adapter->msgbuf[adapter->msgbuflen]
> with strcpy for later use with blogic_show_info()/seq_write.
I know, but it's not clear to me if it's worth it (a potential buffer
overrun there too, BTW).
Maciej
On Fri, 2021-04-16 at 16:28 +0200, Maciej W. Rozycki wrote:
> On Fri, 16 Apr 2021, Joe Perches wrote:
>
> > > I'm not sure if that complex message
> > > routing via `blogic_msg' is worth having even, rather than calling
> > > `printk' or suitable variants directly.
> >
> > It's to allow the message content to be added to the internal
> > &adapter->msgbuf[adapter->msgbuflen]
> > with strcpy for later use with blogic_show_info()/seq_write.
>
> ?I know, but it's not clear to me if it's worth it (a potential buffer
> overrun there too, BTW).
It's seq_ output so it's nominally an ABI.
But then again, I don't use this at all so I don't care much either.
It's also odd/bad form that one output KERN_<level> does not match
its blogic_<level> (blogic_info is emitted at KERN_NOTICE)
On Thu, 15 Apr 2021, Nix wrote:
> > Set the allocation length to 255 for the ATA Information VPD page
> > requested in the WRITE SAME handler, so as not to limit information
> > examined by `scsi_get_vpd_page' in the supported vital product data
> > pages unnecessarily.
> >
> > Originally it was thought that Areca hardware may have issues with a
> > valid allocation length supplied for a VPD inquiry, however older SCSI
> > standard revisions[1] consider 255 the maximum length allowed and what
>
> Aaaah. That explains a lot! (Not that I can remember what SCSI standard
> rev that Areca firmware claimed to implement. I know I never updated the
> firmware, so it's going to be something no newer than mid-2009 and
> probably quite a bit older.)
From the original discussion I gather Areca sometimes acts as a
pass-through device to actual storage hardware, so it may well have been
decided for the firmware to take a conservative approach and interpret
the low order byte only. A genuine bug cannot be ruled out either of
course, which I why I will appreciate your testing.
> > I can see you're still around. Would you therefore please be so kind
> > as to verify this change with your Areca hardware if you still have it?
>
> It's been up in the loft for years, but I'll get it out this weekend and
> give it a spin :) this'll let me make sure the disks still spin as well,
> which matters for an in-case-of-lightning-strike disaster-recovery
> backup box.
>
> (I just hope this kernel boots on it at all. It's about three years
> since I retired it... let's see!)
FWIW if all else fails you can try this patch with the original kernel
you used with the box. This piece of code hasn't changed, so until I
came up with the complete five-part solution proposed here I merely had
the original commit reverted as it is so as to allow forward progress.
In any case, as per the cover letter, I have upgraded from 2.6.18, much
older, and this was the sole show-stopper for the machine, running SMP
even, so chances are 5.11+ will work with your system as well. The
other plain 486/EISA/ATA box, similarly upgraded (now that I got its
faulty odd industrial PSU finally replaced) works just fine with vanilla
5.11.
OTOH versions ~3.15 through to ~4.5 I have tried while bisecting this
issue mostly failed to even start booting due to what looks like a
heisenbug to me (e.g. switching from XZ to gzip for compression would
make some, but not all versions/configurations boot occasionally), so
YMMV.
Overall we're not that bad with keeping stuff working, it's more new
use that causes troubles sometimes.
> > It looks to me like you were thinking in the right direction with:
> > <https://lore.kernel.org/linux-scsi/[email protected]/>.
>
> It's the sort of mistake I could see myself making: an easy mistake to
> make when so many things in C require buffer size - 1 or you get a
> disastrous security hole...
And here it's masking, except that with (256 - 1) rather than (512 - 1)
as you suggested.
Thank you for your input!
Maciej
On 4/14/21 4:39 PM, Maciej W. Rozycki wrote:
> Update BusLogic driver's messaging system to use `pr_cont' for
> continuation lines, bringing messy output:
>
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0: Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0: PCI Bus: 0, Device: 19, Address:
> 0xE0012000,
> Host Adapter SCSI ID: 7
> scsi0: Parity Checking: Enabled, Extended Translation: Enabled
> scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0: Tagged Queue Depth:
> Automatic
> , Untagged Queue Depth: 3
> scsi0: SCSI Bus Termination: Both Enabled
> , SCAM: Disabled
>
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
>
> back to order:
>
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0: Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0: PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7
> scsi0: Parity Checking: Enabled, Extended Translation: Enabled
> scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0: Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
> scsi0: SCSI Bus Termination: Both Enabled, SCAM: Disabled
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
>
> Also diagnostic output such as with the `BusLogic=TraceConfiguration'
> parameter is affected and becomes vertical and therefore hard to read.
> This has now been corrected, e.g.:
>
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> blogic_cmd(86) Status = 30: 4 ==> 4: FF 05 93 00
> blogic_cmd(95) Status = 28: (Modify I/O Address)
> blogic_cmd(91) Status = 30: 1 ==> 1: 01
> blogic_cmd(04) Status = 30: 4 ==> 4: 41 41 35 30
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>
> blogic_cmd(04) Status = 30: 4 ==> 4: 41 41 35 30
> blogic_cmd(0B) Status = 30: 3 ==> 3: 00 08 07
> blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
> blogic_cmd(84) Status = 30: 1 ==> 1: 37
> blogic_cmd(8B) Status = 30: 5 ==> 5: 39 35 38 20 20
> blogic_cmd(85) Status = 30: 1 ==> 1: 42
> blogic_cmd(86) Status = 30: 4 ==> 4: FF 05 93 00
> blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
>
> etc.
>
> Signed-off-by: Maciej W. Rozycki <[email protected]>
> Fixes: 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation lines")
> Cc: [email protected] # v4.9+
> ---
> drivers/scsi/BusLogic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> linux-buslogic-pr-cont.diff
> Index: linux-macro-ide/drivers/scsi/BusLogic.c
> ===================================================================
> --- linux-macro-ide.orig/drivers/scsi/BusLogic.c
> +++ linux-macro-ide/drivers/scsi/BusLogic.c
> @@ -3603,7 +3603,7 @@ static void blogic_msg(enum blogic_msgle
> if (buf[0] != '\n' || len > 1)
> printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
> } else
> - printk("%s", buf);
> + pr_cont("%s", buf);
> } else {
> if (begin) {
> if (adapter != NULL && adapter->adapter_initd)
> @@ -3611,7 +3611,7 @@ static void blogic_msg(enum blogic_msgle
> else
> printk("%s%s", blogic_msglevelmap[msglevel], buf);
> } else
> - printk("%s", buf);
> + pr_cont("%s", buf);
> }
> begin = (buf[len - 1] == '\n');
> }
>
Looks good.
Acked-by: Khalid Aziz <[email protected]>
On 4/14/21 4:38 PM, Maciej W. Rozycki wrote:
> Hi,
>
> First of all, does anyone have a copy of: "MultiMaster UltraSCSI Host
> Adapters for PCI Systems: Technical Reference Manual" (pub. 3002493-E)?
> It used to live in the "Mylex Manuals and Documentation Archives" section
> of the Mylex web site <http://www.mylex.com/pub/manuals/index.htm>,
> specifically at: <http://www.mylex.com/pub/manuals/mmultra.pdf>.
>
> Another useful document might be: "Wide SCSI Host Adapters for PCI and
> EISA Systems: Technical Reference Manual" (pub. 3000763-A), which used to
> live at: <http://www.mylex.com/pub/manuals/widescsi.pdf>, linked from the
> same place.
>
> Sadly I didn't get to these resources while they were still there, and
> neither did archive.org, and now they not appear available from anywhere
> online. I'm sure Leonard had this all, but, alas, he is long gone too.
These documents were all gone by the time I started working on this
driver in 2013.
--
Khalid
On 4/14/21 4:39 PM, Maciej W. Rozycki wrote:
> Existing `blogic_msg' invocations do not appear to overrun its internal
> buffer of a fixed length of 100, which would cause stack corruption, but
> it's easy to miss with possible further updates and a fix is cheap in
> performance terms, so limit the output produced into the buffer by using
> `vsnprintf' rather than `vsprintf'.
>
> Signed-off-by: Maciej W. Rozycki <[email protected]>
> ---
> drivers/scsi/BusLogic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> linux-buslogic-vsnprintf.diff
> Index: linux-macro-ide/drivers/scsi/BusLogic.c
> ===================================================================
> --- linux-macro-ide.orig/drivers/scsi/BusLogic.c
> +++ linux-macro-ide/drivers/scsi/BusLogic.c
> @@ -3588,7 +3588,7 @@ static void blogic_msg(enum blogic_msgle
> int len = 0;
>
> va_start(args, adapter);
> - len = vsprintf(buf, fmt, args);
> + len = vsnprintf(buf, sizeof(buf), fmt, args);
> va_end(args);
> if (msglevel == BLOGIC_ANNOUNCE_LEVEL) {
> static int msglines = 0;
>
As Maciej explained in other email that snprintf() does null-terminate
the string, I think this change is fine.
Acked-by: Khalid Aziz <[email protected]>
On 4/15/21 8:08 PM, Joe Perches wrote:
> And while it's a lot more code, I'd prefer a solution that looks more
> like the other commonly used kernel logging extension mechanisms
> where adapter is placed before the format, ... in the argument list.
Hi Joe,
I don't mind making these changes. It is quite a bit of code but
consistency with other kernel code is useful. Would you like to finalize
this patch, or would you prefer that I take this patch as starting point
and finalize it?
Thanks,
Khalid
>
> Today it's:
>
> void blogic_msg(enum, fmt, adapter, ...);
>
> without the __printf marking so there is one format/arg mismatch.
>
> fyi: in the suggested patch below it's
> - blogic_info("BIOS Address: 0x%lX, ", adapter,
> - adapter->bios_addr);
> + blogic_cont(adapter, "BIOS Address: 0x%X, ",
> + adapter->bios_addr);
>
> I'd prefer
> __printf(3, 4)
> void blogic_msg(enum, adapter, fmt, ...)
>
> (or maybe void blogic_msg(adapter, enum, fmt, ...))
>
> And there's a simple addition of a blogic_cont macro and extension
> to blogic_msg to simplify the logic and obviousness of the logging
> extension lines too.
>
> I suggest this done with coccinelle and a little typing:
> ---
> drivers/scsi/BusLogic.c | 496 +++++++++++++++++++++++++++++++-----------------
> drivers/scsi/BusLogic.h | 32 ++--
> 2 files changed, 341 insertions(+), 187 deletions(-)
>
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index ccb061ab0a0a..7a52371b5ab6 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -134,8 +134,10 @@ static char *blogic_cmd_failure_reason;
>
> static void blogic_announce_drvr(struct blogic_adapter *adapter)
> {
> - blogic_announce("***** BusLogic SCSI Driver Version " blogic_drvr_version " of " blogic_drvr_date " *****\n", adapter);
> - blogic_announce("Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>\n", adapter);
> + blogic_announce(adapter,
> + "***** BusLogic SCSI Driver Version " blogic_drvr_version " of " blogic_drvr_date " *****\n");
> + blogic_announce(adapter,
> + "Copyright 1995-1998 by Leonard N. Zubkoff <[email protected]>\n");
> }
>
>
> @@ -198,8 +200,7 @@ static bool __init blogic_create_initccbs(struct blogic_adapter *adapter)
> blk_pointer = dma_alloc_coherent(&adapter->pci_device->dev,
> blk_size, &blkp, GFP_KERNEL);
> if (blk_pointer == NULL) {
> - blogic_err("UNABLE TO ALLOCATE CCB GROUP - DETACHING\n",
> - adapter);
> + blogic_err(adapter, "UNABLE TO ALLOCATE CCB GROUP - DETACHING\n");
> return false;
> }
> blogic_init_ccbs(adapter, blk_pointer, blk_size, blkp);
> @@ -259,10 +260,13 @@ static void blogic_create_addlccbs(struct blogic_adapter *adapter,
> }
> if (adapter->alloc_ccbs > prev_alloc) {
> if (print_success)
> - blogic_notice("Allocated %d additional CCBs (total now %d)\n", adapter, adapter->alloc_ccbs - prev_alloc, adapter->alloc_ccbs);
> + blogic_notice(adapter,
> + "Allocated %d additional CCBs (total now %d)\n",
> + adapter->alloc_ccbs - prev_alloc,
> + adapter->alloc_ccbs);
> return;
> }
> - blogic_notice("Failed to allocate additional CCBs\n", adapter);
> + blogic_notice(adapter, "Failed to allocate additional CCBs\n");
> if (adapter->drvr_qdepth > adapter->alloc_ccbs - adapter->tgt_count) {
> adapter->drvr_qdepth = adapter->alloc_ccbs - adapter->tgt_count;
> adapter->scsi_host->can_queue = adapter->drvr_qdepth;
> @@ -441,7 +445,9 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum blogic_opcode opcode,
> goto done;
> }
> if (blogic_global_options.trace_config)
> - blogic_notice("blogic_cmd(%02X) Status = %02X: (Modify I/O Address)\n", adapter, opcode, statusreg.all);
> + blogic_notice(adapter,
> + "blogic_cmd(%02X) Status = %02X: (Modify I/O Address)\n",
> + opcode, statusreg.all);
> result = 0;
> goto done;
> }
> @@ -499,15 +505,16 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum blogic_opcode opcode,
> */
> if (blogic_global_options.trace_config) {
> int i;
> - blogic_notice("blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
> - adapter, opcode, statusreg.all, replylen,
> + blogic_notice(adapter,
> + "blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
> + opcode, statusreg.all, replylen,
> reply_b);
> if (replylen > reply_b)
> replylen = reply_b;
> for (i = 0; i < replylen; i++)
> - blogic_notice(" %02X", adapter,
> - ((unsigned char *) reply)[i]);
> - blogic_notice("\n", adapter);
> + blogic_cont(adapter, " %02X",
> + ((unsigned char *)reply)[i]);
> + blogic_cont(adapter, "\n");
> }
> /*
> Process Command Invalid conditions.
> @@ -717,23 +724,37 @@ static int __init blogic_init_mm_probeinfo(struct blogic_adapter *adapter)
> pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
>
> if (pci_resource_flags(pci_device, 0) & IORESOURCE_MEM) {
> - blogic_err("BusLogic: Base Address0 0x%lX not I/O for MultiMaster Host Adapter\n", NULL, base_addr0);
> - blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
> + blogic_err(NULL,
> + "BusLogic: Base Address0 0x%lX not I/O for MultiMaster Host Adapter\n",
> + base_addr0);
> + blogic_err(NULL,
> + "at PCI Bus %d Device %d I/O Address 0x%lX\n",
> + bus, device, io_addr);
> continue;
> }
> if (pci_resource_flags(pci_device, 1) & IORESOURCE_IO) {
> - blogic_err("BusLogic: Base Address1 0x%lX not Memory for MultiMaster Host Adapter\n", NULL, base_addr1);
> - blogic_err("at PCI Bus %d Device %d PCI Address 0x%lX\n", NULL, bus, device, pci_addr);
> + blogic_err(NULL,
> + "BusLogic: Base Address1 0x%lX not Memory for MultiMaster Host Adapter\n",
> + base_addr1);
> + blogic_err(NULL,
> + "at PCI Bus %d Device %d PCI Address 0x%lX\n",
> + bus, device, pci_addr);
> continue;
> }
> if (irq_ch == 0) {
> - blogic_err("BusLogic: IRQ Channel %d invalid for MultiMaster Host Adapter\n", NULL, irq_ch);
> - blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
> + blogic_err(NULL,
> + "BusLogic: IRQ Channel %d invalid for MultiMaster Host Adapter\n",
> + irq_ch);
> + blogic_err(NULL,
> + "at PCI Bus %d Device %d I/O Address 0x%lX\n",
> + bus, device, io_addr);
> continue;
> }
> if (blogic_global_options.trace_probe) {
> - blogic_notice("BusLogic: PCI MultiMaster Host Adapter detected at\n", NULL);
> - blogic_notice("BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n", NULL, bus, device, io_addr, pci_addr);
> + blogic_notice(NULL, "BusLogic: PCI MultiMaster Host Adapter detected at\n");
> + blogic_notice(NULL,
> + "BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n",
> + bus, device, io_addr, pci_addr);
> }
> /*
> Issue the Inquire PCI Host Adapter Information command to determine
> @@ -819,7 +840,7 @@ static int __init blogic_init_mm_probeinfo(struct blogic_adapter *adapter)
> nonpr_mmcount++;
> mmcount++;
> } else
> - blogic_warn("BusLogic: Too many Host Adapters detected\n", NULL);
> + blogic_warn(NULL, "BusLogic: Too many Host Adapters detected\n");
> }
> /*
> If the AutoSCSI "Use Bus And Device # For PCI Scanning Seq."
> @@ -957,23 +978,37 @@ static int __init blogic_init_fp_probeinfo(struct blogic_adapter *adapter)
> pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
> #ifdef CONFIG_SCSI_FLASHPOINT
> if (pci_resource_flags(pci_device, 0) & IORESOURCE_MEM) {
> - blogic_err("BusLogic: Base Address0 0x%lX not I/O for FlashPoint Host Adapter\n", NULL, base_addr0);
> - blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
> + blogic_err(NULL,
> + "BusLogic: Base Address0 0x%lX not I/O for FlashPoint Host Adapter\n",
> + base_addr0);
> + blogic_err(NULL,
> + "at PCI Bus %d Device %d I/O Address 0x%lX\n",
> + bus, device, io_addr);
> continue;
> }
> if (pci_resource_flags(pci_device, 1) & IORESOURCE_IO) {
> - blogic_err("BusLogic: Base Address1 0x%lX not Memory for FlashPoint Host Adapter\n", NULL, base_addr1);
> - blogic_err("at PCI Bus %d Device %d PCI Address 0x%lX\n", NULL, bus, device, pci_addr);
> + blogic_err(NULL,
> + "BusLogic: Base Address1 0x%lX not Memory for FlashPoint Host Adapter\n",
> + base_addr1);
> + blogic_err(NULL,
> + "at PCI Bus %d Device %d PCI Address 0x%lX\n",
> + bus, device, pci_addr);
> continue;
> }
> if (irq_ch == 0) {
> - blogic_err("BusLogic: IRQ Channel %d invalid for FlashPoint Host Adapter\n", NULL, irq_ch);
> - blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
> + blogic_err(NULL,
> + "BusLogic: IRQ Channel %d invalid for FlashPoint Host Adapter\n",
> + irq_ch);
> + blogic_err(NULL,
> + "at PCI Bus %d Device %d I/O Address 0x%lX\n",
> + bus, device, io_addr);
> continue;
> }
> if (blogic_global_options.trace_probe) {
> - blogic_notice("BusLogic: FlashPoint Host Adapter detected at\n", NULL);
> - blogic_notice("BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n", NULL, bus, device, io_addr, pci_addr);
> + blogic_notice(NULL, "BusLogic: FlashPoint Host Adapter detected at\n");
> + blogic_notice(NULL,
> + "BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n",
> + bus, device, io_addr, pci_addr);
> }
> if (blogic_probeinfo_count < BLOGIC_MAX_ADAPTERS) {
> struct blogic_probeinfo *probeinfo =
> @@ -988,11 +1023,15 @@ static int __init blogic_init_fp_probeinfo(struct blogic_adapter *adapter)
> probeinfo->pci_device = pci_dev_get(pci_device);
> fpcount++;
> } else
> - blogic_warn("BusLogic: Too many Host Adapters detected\n", NULL);
> + blogic_warn(NULL, "BusLogic: Too many Host Adapters detected\n");
> #else
> - blogic_err("BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n", NULL, bus, device);
> - blogic_err("BusLogic: I/O Address 0x%lX PCI Address 0x%lX, irq %d, but FlashPoint\n", NULL, io_addr, pci_addr, irq_ch);
> - blogic_err("BusLogic: support was omitted in this kernel configuration.\n", NULL);
> + blogic_err(NULL,
> + "BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n",
> + bus, device);
> + blogic_err(NULL,
> + "BusLogic: I/O Address 0x%lX PCI Address 0x%lX, irq %d, but FlashPoint\n",
> + io_addr, pci_addr, irq_ch);
> + blogic_err("BusLogic: support was omitted in this kernel configuration\n", NULL);
> #endif
> }
> /*
> @@ -1098,15 +1137,19 @@ static bool blogic_failure(struct blogic_adapter *adapter, char *msg)
> {
> blogic_announce_drvr(adapter);
> if (adapter->adapter_bus_type == BLOGIC_PCI_BUS) {
> - blogic_err("While configuring BusLogic PCI Host Adapter at\n",
> - adapter);
> - blogic_err("Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX:\n", adapter, adapter->bus, adapter->dev, adapter->io_addr, adapter->pci_addr);
> + blogic_err(adapter, "While configuring BusLogic PCI Host Adapter at\n");
> + blogic_err(adapter,
> + "Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX:\n",
> + adapter->bus, adapter->dev, adapter->io_addr,
> + adapter->pci_addr);
> } else
> - blogic_err("While configuring BusLogic Host Adapter at I/O Address 0x%lX:\n", adapter, adapter->io_addr);
> - blogic_err("%s FAILED - DETACHING\n", adapter, msg);
> + blogic_err(adapter,
> + "While configuring BusLogic Host Adapter at I/O Address 0x%lX:\n",
> + adapter->io_addr);
> + blogic_err(adapter, "%s FAILED - DETACHING\n", msg);
> if (blogic_cmd_failure_reason != NULL)
> - blogic_err("ADDITIONAL FAILURE INFO - %s\n", adapter,
> - blogic_cmd_failure_reason);
> + blogic_err(adapter, "ADDITIONAL FAILURE INFO - %s\n",
> + blogic_cmd_failure_reason);
> return false;
> }
>
> @@ -1130,13 +1173,20 @@ static bool __init blogic_probe(struct blogic_adapter *adapter)
> fpinfo->present = false;
> if (!(FlashPoint_ProbeHostAdapter(fpinfo) == 0 &&
> fpinfo->present)) {
> - blogic_err("BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n", adapter, adapter->bus, adapter->dev);
> - blogic_err("BusLogic: I/O Address 0x%lX PCI Address 0x%lX, but FlashPoint\n", adapter, adapter->io_addr, adapter->pci_addr);
> - blogic_err("BusLogic: Probe Function failed to validate it.\n", adapter);
> + blogic_err(adapter,
> + "BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n",
> + adapter->bus, adapter->dev);
> + blogic_err(adapter,
> + "BusLogic: I/O Address 0x%lX PCI Address 0x%lX, but FlashPoint\n",
> + adapter->io_addr, adapter->pci_addr);
> + blogic_err(adapter,
> + "BusLogic: Probe Function failed to validate it\n");
> return false;
> }
> if (blogic_global_options.trace_probe)
> - blogic_notice("BusLogic_Probe(0x%lX): FlashPoint Found\n", adapter, adapter->io_addr);
> + blogic_notice(adapter,
> + "BusLogic_Probe(0x%lX): FlashPoint Found\n",
> + adapter->io_addr);
> /*
> Indicate the Host Adapter Probe completed successfully.
> */
> @@ -1153,7 +1203,10 @@ static bool __init blogic_probe(struct blogic_adapter *adapter)
> intreg.all = blogic_rdint(adapter);
> georeg.all = blogic_rdgeom(adapter);
> if (blogic_global_options.trace_probe)
> - blogic_notice("BusLogic_Probe(0x%lX): Status 0x%02X, Interrupt 0x%02X, Geometry 0x%02X\n", adapter, adapter->io_addr, statusreg.all, intreg.all, georeg.all);
> + blogic_notice(adapter,
> + "BusLogic_Probe(0x%lX): Status 0x%02X, Interrupt 0x%02X, Geometry 0x%02X\n",
> + adapter->io_addr, statusreg.all, intreg.all,
> + georeg.all);
> if (statusreg.all == 0 || statusreg.sr.diag_active ||
> statusreg.sr.cmd_param_busy || statusreg.sr.rsvd ||
> statusreg.sr.cmd_invalid || intreg.ir.rsvd != 0)
> @@ -1232,7 +1285,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
> udelay(100);
> }
> if (blogic_global_options.trace_hw_reset)
> - blogic_notice("BusLogic_HardwareReset(0x%lX): Diagnostic Active, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
> + blogic_notice(adapter,
> + "BusLogic_HardwareReset(0x%lX): Diagnostic Active, Status 0x%02X\n",
> + adapter->io_addr, statusreg.all);
> if (timeout < 0)
> return false;
> /*
> @@ -1252,7 +1307,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
> udelay(100);
> }
> if (blogic_global_options.trace_hw_reset)
> - blogic_notice("BusLogic_HardwareReset(0x%lX): Diagnostic Completed, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
> + blogic_notice(adapter,
> + "BusLogic_HardwareReset(0x%lX): Diagnostic Completed, Status 0x%02X\n",
> + adapter->io_addr, statusreg.all);
> if (timeout < 0)
> return false;
> /*
> @@ -1268,7 +1325,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
> udelay(100);
> }
> if (blogic_global_options.trace_hw_reset)
> - blogic_notice("BusLogic_HardwareReset(0x%lX): Host Adapter Ready, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
> + blogic_notice(adapter,
> + "BusLogic_HardwareReset(0x%lX): Host Adapter Ready, Status 0x%02X\n",
> + adapter->io_addr, statusreg.all);
> if (timeout < 0)
> return false;
> /*
> @@ -1280,11 +1339,11 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
> if (statusreg.sr.diag_failed || !statusreg.sr.adapter_ready) {
> blogic_cmd_failure_reason = NULL;
> blogic_failure(adapter, "HARD RESET DIAGNOSTICS");
> - blogic_err("HOST ADAPTER STATUS REGISTER = %02X\n", adapter,
> - statusreg.all);
> + blogic_err(adapter, "HOST ADAPTER STATUS REGISTER = %02X\n",
> + statusreg.all);
> if (statusreg.sr.datain_ready)
> - blogic_err("HOST ADAPTER ERROR CODE = %d\n", adapter,
> - blogic_rddatain(adapter));
> + blogic_err(adapter, "HOST ADAPTER ERROR CODE = %d\n",
> + blogic_rddatain(adapter));
> return false;
> }
> /*
> @@ -1324,9 +1383,10 @@ static bool __init blogic_checkadapter(struct blogic_adapter *adapter)
> Provide tracing information if requested and return.
> */
> if (blogic_global_options.trace_probe)
> - blogic_notice("BusLogic_Check(0x%lX): MultiMaster %s\n", adapter,
> - adapter->io_addr,
> - (result ? "Found" : "Not Found"));
> + blogic_notice(adapter,
> + "BusLogic_Check(0x%lX): MultiMaster %s\n",
> + adapter->io_addr,
> + (result ? "Found" : "Not Found"));
> return result;
> }
>
> @@ -1836,30 +1896,40 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
> char *tagq_msg = tagq_str;
> int tgt_id;
>
> - blogic_info("Configuring BusLogic Model %s %s%s%s%s SCSI Host Adapter\n", adapter, adapter->model, blogic_adapter_busnames[adapter->adapter_bus_type], (adapter->wide ? " Wide" : ""), (adapter->differential ? " Differential" : ""), (adapter->ultra ? " Ultra" : ""));
> - blogic_info(" Firmware Version: %s, I/O Address: 0x%lX, IRQ Channel: %d/%s\n", adapter, adapter->fw_ver, adapter->io_addr, adapter->irq_ch, (adapter->level_int ? "Level" : "Edge"));
> + blogic_info(adapter,
> + "Configuring BusLogic Model %s %s%s%s%s SCSI Host Adapter\n",
> + adapter->model,
> + blogic_adapter_busnames[adapter->adapter_bus_type],
> + (adapter->wide ? " Wide" : ""),
> + (adapter->differential ? " Differential" : ""),
> + (adapter->ultra ? " Ultra" : ""));
> + blogic_info(adapter,
> + " Firmware Version: %s, I/O Address: 0x%lX, IRQ Channel: %d/%s\n",
> + adapter->fw_ver, adapter->io_addr, adapter->irq_ch,
> + (adapter->level_int ? "Level" : "Edge"));
> if (adapter->adapter_bus_type != BLOGIC_PCI_BUS) {
> - blogic_info(" DMA Channel: ", adapter);
> + blogic_info(adapter, " DMA Channel: ");
> if (adapter->dma_ch > 0)
> - blogic_info("%d, ", adapter, adapter->dma_ch);
> + blogic_cont(adapter, "%d, ", adapter->dma_ch);
> else
> - blogic_info("None, ", adapter);
> + blogic_cont(adapter, "None, ");
> if (adapter->bios_addr > 0)
> - blogic_info("BIOS Address: 0x%lX, ", adapter,
> - adapter->bios_addr);
> + blogic_cont(adapter, "BIOS Address: 0x%X, ",
> + adapter->bios_addr);
> else
> - blogic_info("BIOS Address: None, ", adapter);
> + blogic_cont(adapter, "BIOS Address: None, ");
> } else {
> - blogic_info(" PCI Bus: %d, Device: %d, Address: ", adapter,
> - adapter->bus, adapter->dev);
> + blogic_cont(adapter, " PCI Bus: %d, Device: %d, Address: ",
> + adapter->bus, adapter->dev);
> if (adapter->pci_addr > 0)
> - blogic_info("0x%lX, ", adapter, adapter->pci_addr);
> + blogic_cont(adapter, "0x%lX, ", adapter->pci_addr);
> else
> - blogic_info("Unassigned, ", adapter);
> + blogic_cont(adapter, "Unassigned, ");
> }
> - blogic_info("Host Adapter SCSI ID: %d\n", adapter, adapter->scsi_id);
> - blogic_info(" Parity Checking: %s, Extended Translation: %s\n",
> - adapter, (adapter->parity ? "Enabled" : "Disabled"),
> + blogic_cont(adapter, "Host Adapter SCSI ID: %d\n", adapter->scsi_id);
> + blogic_info(adapter,
> + " Parity Checking: %s, Extended Translation: %s\n",
> + (adapter->parity ? "Enabled" : "Disabled"),
> (adapter->ext_trans_enable ? "Enabled" : "Disabled"));
> alltgt_mask &= ~(1 << adapter->scsi_id);
> sync_ok = adapter->sync_ok & alltgt_mask;
> @@ -1928,16 +1998,25 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
> tagq_str[adapter->scsi_id] = '#';
> tagq_str[adapter->maxdev] = '\0';
> }
> - blogic_info(" Synchronous Negotiation: %s, Wide Negotiation: %s\n",
> - adapter, syncmsg, widemsg);
> - blogic_info(" Disconnect/Reconnect: %s, Tagged Queuing: %s\n", adapter,
> - discon_msg, tagq_msg);
> + blogic_info(adapter,
> + " Synchronous Negotiation: %s, Wide Negotiation: %s\n",
> + syncmsg, widemsg);
> + blogic_info(adapter,
> + " Disconnect/Reconnect: %s, Tagged Queuing: %s\n",
> + discon_msg, tagq_msg);
> if (blogic_multimaster_type(adapter)) {
> - blogic_info(" Scatter/Gather Limit: %d of %d segments, Mailboxes: %d\n", adapter, adapter->drvr_sglimit, adapter->adapter_sglimit, adapter->mbox_count);
> - blogic_info(" Driver Queue Depth: %d, Host Adapter Queue Depth: %d\n", adapter, adapter->drvr_qdepth, adapter->adapter_qdepth);
> + blogic_info(adapter,
> + " Scatter/Gather Limit: %d of %d segments, Mailboxes: %d\n",
> + adapter->drvr_sglimit, adapter->adapter_sglimit,
> + adapter->mbox_count);
> + blogic_info(adapter,
> + " Driver Queue Depth: %d, Host Adapter Queue Depth: %d\n",
> + adapter->drvr_qdepth, adapter->adapter_qdepth);
> } else
> - blogic_info(" Driver Queue Depth: %d, Scatter/Gather Limit: %d segments\n", adapter, adapter->drvr_qdepth, adapter->drvr_sglimit);
> - blogic_info(" Tagged Queue Depth: ", adapter);
> + blogic_info(adapter,
> + " Driver Queue Depth: %d, Scatter/Gather Limit: %d segments\n",
> + adapter->drvr_qdepth, adapter->drvr_sglimit);
> + blogic_info(adapter, " Tagged Queue Depth: ");
> common_tagq_depth = true;
> for (tgt_id = 1; tgt_id < adapter->maxdev; tgt_id++)
> if (adapter->qdepth[tgt_id] != adapter->qdepth[0]) {
> @@ -1946,24 +2025,28 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
> }
> if (common_tagq_depth) {
> if (adapter->qdepth[0] > 0)
> - blogic_info("%d", adapter, adapter->qdepth[0]);
> + blogic_cont(adapter, "%d", adapter->qdepth[0]);
> else
> - blogic_info("Automatic", adapter);
> + blogic_cont(adapter, "Automatic");
> } else
> - blogic_info("Individual", adapter);
> - blogic_info(", Untagged Queue Depth: %d\n", adapter,
> - adapter->untag_qdepth);
> + blogic_cont(adapter, "Individual");
> + blogic_cont(adapter, ", Untagged Queue Depth: %d\n",
> + adapter->untag_qdepth);
> if (adapter->terminfo_valid) {
> if (adapter->wide)
> - blogic_info(" SCSI Bus Termination: %s", adapter,
> - (adapter->low_term ? (adapter->high_term ? "Both Enabled" : "Low Enabled") : (adapter->high_term ? "High Enabled" : "Both Disabled")));
> + blogic_info(adapter, " SCSI Bus Termination: %s",
> + (adapter->low_term
> + ? (adapter->high_term ? "Both Enabled" : "Low Enabled")
> + : (adapter->high_term ? "High Enabled" : "Both Disabled")));
> else
> - blogic_info(" SCSI Bus Termination: %s", adapter,
> - (adapter->low_term ? "Enabled" : "Disabled"));
> + blogic_info(adapter, " SCSI Bus Termination: %s",
> + (adapter->low_term ? "Enabled" : "Disabled"));
> if (adapter->scam)
> - blogic_info(", SCAM: %s", adapter,
> - (adapter->scam_enabled ? (adapter->scam_lev2 ? "Enabled, Level 2" : "Enabled, Level 1") : "Disabled"));
> - blogic_info("\n", adapter);
> + blogic_cont(adapter, ", SCAM: %s",
> + (adapter->scam_enabled
> + ? (adapter->scam_lev2 ? "Enabled, Level 2" : "Enabled, Level 1")
> + : "Disabled"));
> + blogic_cont(adapter, "\n");
> }
> /*
> Indicate reporting the Host Adapter configuration completed
> @@ -1981,8 +2064,8 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
> static bool __init blogic_getres(struct blogic_adapter *adapter)
> {
> if (adapter->irq_ch == 0) {
> - blogic_err("NO LEGAL INTERRUPT CHANNEL ASSIGNED - DETACHING\n",
> - adapter);
> + blogic_err(adapter,
> + "NO LEGAL INTERRUPT CHANNEL ASSIGNED - DETACHING\n");
> return false;
> }
> /*
> @@ -1990,8 +2073,9 @@ static bool __init blogic_getres(struct blogic_adapter *adapter)
> */
> if (request_irq(adapter->irq_ch, blogic_inthandler, IRQF_SHARED,
> adapter->full_model, adapter) < 0) {
> - blogic_err("UNABLE TO ACQUIRE IRQ CHANNEL %d - DETACHING\n",
> - adapter, adapter->irq_ch);
> + blogic_err(adapter,
> + "UNABLE TO ACQUIRE IRQ CHANNEL %d - DETACHING\n",
> + adapter->irq_ch);
> return false;
> }
> adapter->irq_acquired = true;
> @@ -2000,7 +2084,9 @@ static bool __init blogic_getres(struct blogic_adapter *adapter)
> */
> if (adapter->dma_ch > 0) {
> if (request_dma(adapter->dma_ch, adapter->full_model) < 0) {
> - blogic_err("UNABLE TO ACQUIRE DMA CHANNEL %d - DETACHING\n", adapter, adapter->dma_ch);
> + blogic_err(adapter,
> + "UNABLE TO ACQUIRE DMA CHANNEL %d - DETACHING\n",
> + adapter->dma_ch);
> return false;
> }
> set_dma_mode(adapter->dma_ch, DMA_MODE_CASCADE);
> @@ -2148,12 +2234,12 @@ static bool blogic_initadapter(struct blogic_adapter *adapter)
> */
> done:
> if (!adapter->adapter_initd) {
> - blogic_info("*** %s Initialized Successfully ***\n", adapter,
> - adapter->full_model);
> - blogic_info("\n", adapter);
> + blogic_info(adapter, "*** %s Initialized Successfully ***\n",
> + adapter->full_model);
> + blogic_info(adapter, "\n");
> } else
> - blogic_warn("*** %s Initialized Successfully ***\n", adapter,
> - adapter->full_model);
> + blogic_warn(adapter, "*** %s Initialized Successfully ***\n",
> + adapter->full_model);
> adapter->adapter_initd = true;
>
> /*
> @@ -2365,15 +2451,16 @@ static int __init blogic_init(void)
> kcalloc(BLOGIC_MAX_ADAPTERS, sizeof(struct blogic_probeinfo),
> GFP_KERNEL);
> if (blogic_probeinfo_list == NULL) {
> - blogic_err("BusLogic: Unable to allocate Probe Info List\n",
> - NULL);
> + blogic_err(NULL,
> + "BusLogic: Unable to allocate Probe Info List\n");
> return -ENOMEM;
> }
>
> adapter = kzalloc(sizeof(struct blogic_adapter), GFP_KERNEL);
> if (adapter == NULL) {
> kfree(blogic_probeinfo_list);
> - blogic_err("BusLogic: Unable to allocate Prototype Host Adapter\n", NULL);
> + blogic_err(NULL,
> + "BusLogic: Unable to allocate Prototype Host Adapter\n");
> return -ENOMEM;
> }
>
> @@ -2633,8 +2720,9 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
> case BLOGIC_INVALID_OUTBOX_CODE:
> case BLOGIC_INVALID_CMD_CODE:
> case BLOGIC_BAD_CMD_PARAM:
> - blogic_warn("BusLogic Driver Protocol Error 0x%02X\n",
> - adapter, adapter_status);
> + blogic_warn(adapter,
> + "BusLogic Driver Protocol Error 0x%02X\n",
> + adapter_status);
> fallthrough;
> case BLOGIC_DATA_UNDERRUN:
> case BLOGIC_DATA_OVERRUN:
> @@ -2659,8 +2747,8 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
> hoststatus = DID_RESET;
> break;
> default:
> - blogic_warn("Unknown Host Adapter Status 0x%02X\n", adapter,
> - adapter_status);
> + blogic_warn(adapter, "Unknown Host Adapter Status 0x%02X\n",
> + adapter_status);
> hoststatus = DID_ERROR;
> break;
> }
> @@ -2718,7 +2806,9 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter)
> then there is most likely a bug in
> the Host Adapter firmware.
> */
> - blogic_warn("Illegal CCB #%ld status %d in Incoming Mailbox\n", adapter, ccb->serial, ccb->status);
> + blogic_warn(adapter,
> + "Illegal CCB #%ld status %d in Incoming Mailbox\n",
> + ccb->serial, ccb->status);
> }
> }
> next_inbox->comp_code = BLOGIC_INBOX_FREE;
> @@ -2753,7 +2843,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
> if (ccb->opcode == BLOGIC_BDR) {
> int tgt_id = ccb->tgt_id;
>
> - blogic_warn("Bus Device Reset CCB #%ld to Target %d Completed\n", adapter, ccb->serial, tgt_id);
> + blogic_warn(adapter,
> + "Bus Device Reset CCB #%ld to Target %d Completed\n",
> + ccb->serial, tgt_id);
> blogic_inc_count(&adapter->tgt_stats[tgt_id].bdr_done);
> adapter->tgt_flags[tgt_id].tagq_active = false;
> adapter->cmds_since_rst[tgt_id] = 0;
> @@ -2806,7 +2898,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
> case BLOGIC_INBOX_FREE:
> case BLOGIC_CMD_NOTFOUND:
> case BLOGIC_INVALID_CCB:
> - blogic_warn("CCB #%ld to Target %d Impossible State\n", adapter, ccb->serial, ccb->tgt_id);
> + blogic_warn(adapter,
> + "CCB #%ld to Target %d Impossible State\n",
> + ccb->serial, ccb->tgt_id);
> break;
> case BLOGIC_CMD_COMPLETE_GOOD:
> adapter->tgt_stats[ccb->tgt_id]
> @@ -2816,8 +2910,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
> command->result = DID_OK << 16;
> break;
> case BLOGIC_CMD_ABORT_BY_HOST:
> - blogic_warn("CCB #%ld to Target %d Aborted\n",
> - adapter, ccb->serial, ccb->tgt_id);
> + blogic_warn(adapter,
> + "CCB #%ld to Target %d Aborted\n",
> + ccb->serial, ccb->tgt_id);
> blogic_inc_count(&adapter->tgt_stats[ccb->tgt_id].aborts_done);
> command->result = DID_ABORT << 16;
> break;
> @@ -2829,16 +2924,26 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
> .cmds_complete++;
> if (blogic_global_options.trace_err) {
> int i;
> - blogic_notice("CCB #%ld Target %d: Result %X Host "
> - "Adapter Status %02X Target Status %02X\n", adapter, ccb->serial, ccb->tgt_id, command->result, ccb->adapter_status, ccb->tgt_status);
> - blogic_notice("CDB ", adapter);
> + blogic_notice(adapter,
> + "CCB #%ld Target %d: Result %X Host Adapter Status %02X Target Status %02X\n",
> + ccb->serial,
> + ccb->tgt_id,
> + command->result,
> + ccb->adapter_status,
> + ccb->tgt_status);
> + blogic_notice(adapter,
> + "CDB ");
> for (i = 0; i < ccb->cdblen; i++)
> - blogic_notice(" %02X", adapter, ccb->cdb[i]);
> - blogic_notice("\n", adapter);
> - blogic_notice("Sense ", adapter);
> + blogic_cont(adapter,
> + " %02X",
> + ccb->cdb[i]);
> + blogic_cont(adapter, "\n");
> + blogic_notice(adapter, "Sense ");
> for (i = 0; i < ccb->sense_datalen; i++)
> - blogic_notice(" %02X", adapter, command->sense_buffer[i]);
> - blogic_notice("\n", adapter);
> + blogic_cont(adapter,
> + " %02X",
> + command->sense_buffer[i]);
> + blogic_cont(adapter, "\n");
> }
> }
> break;
> @@ -2925,7 +3030,8 @@ static irqreturn_t blogic_inthandler(int irq_ch, void *devid)
> adapter->adapter_extreset = true;
> break;
> case FPOINT_INTERN_ERR:
> - blogic_warn("Internal FlashPoint Error detected - Resetting Host Adapter\n", adapter);
> + blogic_warn(adapter,
> + "Internal FlashPoint Error detected - Resetting Host Adapter\n");
> adapter->adapter_intern_err = true;
> break;
> }
> @@ -2939,12 +3045,16 @@ static irqreturn_t blogic_inthandler(int irq_ch, void *devid)
> Reset the Host Adapter if requested.
> */
> if (adapter->adapter_extreset) {
> - blogic_warn("Resetting %s due to External SCSI Bus Reset\n", adapter, adapter->full_model);
> + blogic_warn(adapter,
> + "Resetting %s due to External SCSI Bus Reset\n",
> + adapter->full_model);
> blogic_inc_count(&adapter->ext_resets);
> blogic_resetadapter(adapter, false);
> adapter->adapter_extreset = false;
> } else if (adapter->adapter_intern_err) {
> - blogic_warn("Resetting %s due to Host Adapter Internal Error\n", adapter, adapter->full_model);
> + blogic_warn(adapter,
> + "Resetting %s due to Host Adapter Internal Error\n",
> + adapter->full_model);
> blogic_inc_count(&adapter->adapter_intern_errors);
> blogic_resetadapter(adapter, true);
> adapter->adapter_intern_err = false;
> @@ -3142,8 +3252,9 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
> && tgt_flags->tagq_ok &&
> (adapter->tagq_ok & (1 << tgt_id))) {
> tgt_flags->tagq_active = true;
> - blogic_notice("Tagged Queuing now active for Target %d\n",
> - adapter, tgt_id);
> + blogic_notice(adapter,
> + "Tagged Queuing now active for Target %d\n",
> + tgt_id);
> }
> if (tgt_flags->tagq_active) {
> enum blogic_queuetag queuetag = BLOGIC_SIMPLETAG;
> @@ -3184,8 +3295,8 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
> command->sense_buffer, ccb->sense_datalen,
> DMA_FROM_DEVICE);
> if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
> - blogic_err("DMA mapping for sense data buffer failed\n",
> - adapter);
> + blogic_err(adapter,
> + "DMA mapping for sense data buffer failed\n");
> blogic_dealloc_ccb(ccb, 0);
> return SCSI_MLQUEUE_HOST_BUSY;
> }
> @@ -3204,12 +3315,12 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
> */
> if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START, ccb)) {
> spin_unlock_irq(adapter->scsi_host->host_lock);
> - blogic_warn("Unable to write Outgoing Mailbox - Pausing for 1 second\n", adapter);
> + blogic_warn(adapter, "Unable to write Outgoing Mailbox - Pausing for 1 second\n");
> blogic_delay(1);
> spin_lock_irq(adapter->scsi_host->host_lock);
> if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START,
> ccb)) {
> - blogic_warn("Still unable to write Outgoing Mailbox - Host Adapter Dead?\n", adapter);
> + blogic_warn(adapter, "Still unable to write Outgoing Mailbox - Host Adapter Dead?\n");
> blogic_dealloc_ccb(ccb, 1);
> command->result = DID_ERROR << 16;
> command->scsi_done(command);
> @@ -3259,13 +3370,16 @@ static int blogic_abort(struct scsi_cmnd *command)
> if (ccb->command == command)
> break;
> if (ccb == NULL) {
> - blogic_warn("Unable to Abort Command to Target %d - No CCB Found\n", adapter, tgt_id);
> + blogic_warn(adapter, "Unable to Abort Command to Target %d - No CCB Found\n",
> + tgt_id);
> return SUCCESS;
> } else if (ccb->status == BLOGIC_CCB_COMPLETE) {
> - blogic_warn("Unable to Abort Command to Target %d - CCB Completed\n", adapter, tgt_id);
> + blogic_warn(adapter, "Unable to Abort Command to Target %d - CCB Completed\n",
> + tgt_id);
> return SUCCESS;
> } else if (ccb->status == BLOGIC_CCB_RESET) {
> - blogic_warn("Unable to Abort Command to Target %d - CCB Reset\n", adapter, tgt_id);
> + blogic_warn(adapter, "Unable to Abort Command to Target %d - CCB Reset\n",
> + tgt_id);
> return SUCCESS;
> }
> if (blogic_multimaster_type(adapter)) {
> @@ -3283,16 +3397,18 @@ static int blogic_abort(struct scsi_cmnd *command)
> */
> if (adapter->tgt_flags[tgt_id].tagq_active &&
> adapter->fw_ver[0] < '5') {
> - blogic_warn("Unable to Abort CCB #%ld to Target %d - Abort Tag Not Supported\n", adapter, ccb->serial, tgt_id);
> + blogic_warn(adapter, "Unable to Abort CCB #%ld to Target %d - Abort Tag Not Supported\n",
> + ccb->serial, tgt_id);
> return FAILURE;
> } else if (blogic_write_outbox(adapter, BLOGIC_MBOX_ABORT,
> ccb)) {
> - blogic_warn("Aborting CCB #%ld to Target %d\n",
> - adapter, ccb->serial, tgt_id);
> + blogic_warn(adapter, "Aborting CCB #%ld to Target %d\n",
> + ccb->serial, tgt_id);
> blogic_inc_count(&adapter->tgt_stats[tgt_id].aborts_tried);
> return SUCCESS;
> } else {
> - blogic_warn("Unable to Abort CCB #%ld to Target %d - No Outgoing Mailboxes\n", adapter, ccb->serial, tgt_id);
> + blogic_warn(adapter, "Unable to Abort CCB #%ld to Target %d - No Outgoing Mailboxes\n",
> + ccb->serial, tgt_id);
> return FAILURE;
> }
> } else {
> @@ -3300,8 +3416,8 @@ static int blogic_abort(struct scsi_cmnd *command)
> Call the FlashPoint SCCB Manager to abort execution of
> the CCB.
> */
> - blogic_warn("Aborting CCB #%ld to Target %d\n", adapter,
> - ccb->serial, tgt_id);
> + blogic_warn(adapter, "Aborting CCB #%ld to Target %d\n",
> + ccb->serial, tgt_id);
> blogic_inc_count(&adapter->tgt_stats[tgt_id].aborts_tried);
> FlashPoint_AbortCCB(adapter->cardhandle, ccb);
> /*
> @@ -3333,8 +3449,8 @@ static int blogic_resetadapter(struct blogic_adapter *adapter, bool hard_reset)
>
> if (!(blogic_hwreset(adapter, hard_reset) &&
> blogic_initadapter(adapter))) {
> - blogic_err("Resetting %s Failed\n", adapter,
> - adapter->full_model);
> + blogic_err(adapter, "Resetting %s Failed\n",
> + adapter->full_model);
> return FAILURE;
> }
>
> @@ -3443,10 +3559,15 @@ static int blogic_diskparam(struct scsi_device *sdev, struct block_device *dev,
> diskparam->cylinders = (unsigned long) capacity / (diskparam->heads * diskparam->sectors);
> if (part_no < 4 && part_end_sector == diskparam->sectors) {
> if (diskparam->cylinders != saved_cyl)
> - blogic_warn("Adopting Geometry %d/%d from Partition Table\n", adapter, diskparam->heads, diskparam->sectors);
> + blogic_warn(adapter,
> + "Adopting Geometry %d/%d from Partition Table\n",
> + diskparam->heads,
> + diskparam->sectors);
> } else if (part_end_head > 0 || part_end_sector > 0) {
> - blogic_warn("Warning: Partition Table appears to have Geometry %d/%d which is\n", adapter, part_end_head + 1, part_end_sector);
> - blogic_warn("not compatible with current BusLogic Host Adapter Geometry %d/%d\n", adapter, diskparam->heads, diskparam->sectors);
> + blogic_warn(adapter,
> + "Warning: Partition Table appears to have Geometry %d/%d which is not compatible with current BusLogic Host Adapter Geometry %d/%d\n",
> + part_end_head + 1, part_end_sector,
> + diskparam->heads, diskparam->sectors);
> }
> }
> kfree(buf);
> @@ -3571,6 +3692,7 @@ Target Requested Completed Requested Completed Requested Completed\n\
> }
> seq_printf(m, "\nExternal Host Adapter Resets: %d\n", adapter->ext_resets);
> seq_printf(m, "Host Adapter Internal Errors: %d\n", adapter->adapter_intern_errors);
> +
> return 0;
> }
>
> @@ -3579,41 +3701,51 @@ Target Requested Completed Requested Completed Requested Completed\n\
> blogic_msg prints Driver Messages.
> */
>
> -static void blogic_msg(enum blogic_msglevel msglevel, char *fmt,
> - struct blogic_adapter *adapter, ...)
> +static void blogic_msg(enum blogic_msglevel msglevel,
> + struct blogic_adapter *adapter, const char *fmt, ...)
> {
> static char buf[BLOGIC_LINEBUF_SIZE];
> - static bool begin = true;
> va_list args;
> int len = 0;
> + const char *kern_level = blogic_msglevelmap[msglevel];
>
> - va_start(args, adapter);
> - len = vsprintf(buf, fmt, args);
> + va_start(args, fmt);
> + len = vscnprintf(buf, sizeof(buf), fmt, args);
> va_end(args);
> - if (msglevel == BLOGIC_ANNOUNCE_LEVEL) {
> +
> + switch (msglevel) {
> + case BLOGIC_ANNOUNCE_LEVEL: {
> static int msglines = 0;
> +
> strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
> adapter->msgbuflen += len;
> if (++msglines <= 2)
> - printk("%sscsi: %s", blogic_msglevelmap[msglevel], buf);
> - } else if (msglevel == BLOGIC_INFO_LEVEL) {
> + printk("%sscsi: %s", kern_level, buf);
> + break;
> + }
> +
> + case BLOGIC_INFO_LEVEL:
> strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
> adapter->msgbuflen += len;
> - if (begin) {
> - if (buf[0] != '\n' || len > 1)
> - printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
> - } else
> - printk("%s", buf);
> - } else {
> - if (begin) {
> - if (adapter != NULL && adapter->adapter_initd)
> - printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
> - else
> - printk("%s%s", blogic_msglevelmap[msglevel], buf);
> - } else
> - printk("%s", buf);
> + if (buf[0] != '\n' || len > 1)
> + printk("%sscsi%d: %s",
> + kern_level, adapter->host_no, buf);
> + break;
> +
> + case BLOGIC_CONT_LEVEL:
> + strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
> + adapter->msgbuflen += len;
> + printk("%s%s", kern_level, buf);
> + break;
> +
> + default:
> + if (adapter && adapter->adapter_initd)
> + printk("%sscsi%d: %s",
> + kern_level, adapter->host_no, buf);
> + else
> + printk("%s%s", kern_level, buf);
> + break;
> }
> - begin = (buf[len - 1] == '\n');
> }
>
>
> @@ -3691,7 +3823,9 @@ static int __init blogic_parseopts(char *options)
> blogic_probe_options.probe134 = true;
> break;
> default:
> - blogic_err("BusLogic: Invalid Driver Options (invalid I/O Address 0x%lX)\n", NULL, io_addr);
> + blogic_err(NULL,
> + "BusLogic: Invalid Driver Options (invalid I/O Address 0x%lX)\n",
> + io_addr);
> return 0;
> }
> } else if (blogic_parse(&options, "NoProbeISA"))
> @@ -3712,7 +3846,9 @@ static int __init blogic_parseopts(char *options)
> for (tgt_id = 0; tgt_id < BLOGIC_MAXDEV; tgt_id++) {
> unsigned short qdepth = simple_strtoul(options, &options, 0);
> if (qdepth > BLOGIC_MAX_TAG_DEPTH) {
> - blogic_err("BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n", NULL, qdepth);
> + blogic_err(NULL,
> + "BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n",
> + qdepth);
> return 0;
> }
> drvr_opts->qdepth[tgt_id] = qdepth;
> @@ -3721,12 +3857,16 @@ static int __init blogic_parseopts(char *options)
> else if (*options == ']')
> break;
> else {
> - blogic_err("BusLogic: Invalid Driver Options (',' or ']' expected at '%s')\n", NULL, options);
> + blogic_err(NULL,
> + "BusLogic: Invalid Driver Options (',' or ']' expected at '%s')\n",
> + options);
> return 0;
> }
> }
> if (*options != ']') {
> - blogic_err("BusLogic: Invalid Driver Options (']' expected at '%s')\n", NULL, options);
> + blogic_err(NULL,
> + "BusLogic: Invalid Driver Options (']' expected at '%s')\n",
> + options);
> return 0;
> } else
> options++;
> @@ -3734,7 +3874,9 @@ static int __init blogic_parseopts(char *options)
> unsigned short qdepth = simple_strtoul(options, &options, 0);
> if (qdepth == 0 ||
> qdepth > BLOGIC_MAX_TAG_DEPTH) {
> - blogic_err("BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n", NULL, qdepth);
> + blogic_err(NULL,
> + "BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n",
> + qdepth);
> return 0;
> }
> drvr_opts->common_qdepth = qdepth;
> @@ -3780,7 +3922,9 @@ static int __init blogic_parseopts(char *options)
> unsigned short bus_settle_time =
> simple_strtoul(options, &options, 0);
> if (bus_settle_time > 5 * 60) {
> - blogic_err("BusLogic: Invalid Driver Options (invalid Bus Settle Time %d)\n", NULL, bus_settle_time);
> + blogic_err(NULL,
> + "BusLogic: Invalid Driver Options (invalid Bus Settle Time %d)\n",
> + bus_settle_time);
> return 0;
> }
> drvr_opts->bus_settle_time = bus_settle_time;
> @@ -3805,14 +3949,17 @@ static int __init blogic_parseopts(char *options)
> if (*options == ',')
> options++;
> else if (*options != ';' && *options != '\0') {
> - blogic_err("BusLogic: Unexpected Driver Option '%s' ignored\n", NULL, options);
> + blogic_err(NULL,
> + "BusLogic: Unexpected Driver Option '%s' ignored\n",
> + options);
> *options = '\0';
> }
> }
> if (!(blogic_drvr_options_count == 0 ||
> blogic_probeinfo_count == 0 ||
> blogic_drvr_options_count == blogic_probeinfo_count)) {
> - blogic_err("BusLogic: Invalid Driver Options (all or no I/O Addresses must be specified)\n", NULL);
> + blogic_err(NULL,
> + "BusLogic: Invalid Driver Options (all or no I/O Addresses must be specified)\n");
> return 0;
> }
> /*
> @@ -3866,7 +4013,8 @@ static int __init blogic_setup(char *str)
> (void) get_options(str, ARRAY_SIZE(ints), ints);
>
> if (ints[0] != 0) {
> - blogic_err("BusLogic: Obsolete Command Line Entry Format Ignored\n", NULL);
> + blogic_err(NULL,
> + "BusLogic: Obsolete Command Line Entry Format Ignored\n");
> return 0;
> }
> if (str == NULL || *str == '\0')
> diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
> index 6182cc8a0344..42333fba6005 100644
> --- a/drivers/scsi/BusLogic.h
> +++ b/drivers/scsi/BusLogic.h
> @@ -108,30 +108,34 @@ enum blogic_msglevel {
> BLOGIC_INFO_LEVEL = 1,
> BLOGIC_NOTICE_LEVEL = 2,
> BLOGIC_WARN_LEVEL = 3,
> - BLOGIC_ERR_LEVEL = 4
> + BLOGIC_ERR_LEVEL = 4,
> + BLOGIC_CONT_LEVEL = 5,
> };
>
> -static char *blogic_msglevelmap[] = { KERN_NOTICE, KERN_NOTICE, KERN_NOTICE, KERN_WARNING, KERN_ERR };
> +static char *blogic_msglevelmap[] = { KERN_NOTICE, KERN_NOTICE, KERN_NOTICE, KERN_WARNING, KERN_ERR, KERN_CONT };
>
>
> /*
> Define Driver Message macros.
> */
>
> -#define blogic_announce(format, args...) \
> - blogic_msg(BLOGIC_ANNOUNCE_LEVEL, format, ##args)
> +#define blogic_announce(adapter, fmt, ...) \
> + blogic_msg(BLOGIC_ANNOUNCE_LEVEL, adapter, fmt, ##__VA_ARGS__)
>
> -#define blogic_info(format, args...) \
> - blogic_msg(BLOGIC_INFO_LEVEL, format, ##args)
> +#define blogic_cont(adapter, fmt, ...) \
> + blogic_msg(BLOGIC_CONT_LEVEL, adapter, fmt, ##__VA_ARGS__)
>
> -#define blogic_notice(format, args...) \
> - blogic_msg(BLOGIC_NOTICE_LEVEL, format, ##args)
> +#define blogic_info(adapter, fmt, ...) \
> + blogic_msg(BLOGIC_INFO_LEVEL, adapter, fmt, ##__VA_ARGS__)
>
> -#define blogic_warn(format, args...) \
> - blogic_msg(BLOGIC_WARN_LEVEL, format, ##args)
> +#define blogic_notice(adapter, fmt, ...) \
> + blogic_msg(BLOGIC_NOTICE_LEVEL, adapter, fmt, ##__VA_ARGS__)
>
> -#define blogic_err(format, args...) \
> - blogic_msg(BLOGIC_ERR_LEVEL, format, ##args)
> +#define blogic_warn(adapter, fmt, ...) \
> + blogic_msg(BLOGIC_WARN_LEVEL, adapter, fmt, ##__VA_ARGS__)
> +
> +#define blogic_err(adapter, fmt, ...) \
> + blogic_msg(BLOGIC_ERR_LEVEL, adapter, fmt, ##__VA_ARGS__)
>
>
> /*
> @@ -1289,7 +1293,9 @@ static int blogic_slaveconfig(struct scsi_device *);
> static void blogic_qcompleted_ccb(struct blogic_ccb *);
> static irqreturn_t blogic_inthandler(int, void *);
> static int blogic_resetadapter(struct blogic_adapter *, bool hard_reset);
> -static void blogic_msg(enum blogic_msglevel, char *, struct blogic_adapter *, ...);
> +__printf(3, 4)
> +static void blogic_msg(enum blogic_msglevel, struct blogic_adapter *adapter,
> + const char *fmt, ...);
> static int __init blogic_setup(char *);
>
> #endif /* _BUSLOGIC_H */
>
>
On Fri, 16 Apr 2021, Khalid Aziz wrote:
> > Sadly I didn't get to these resources while they were still there, and
> > neither did archive.org, and now they not appear available from anywhere
> > online. I'm sure Leonard had this all, but, alas, he is long gone too.
>
> These documents were all gone by the time I started working on this
> driver in 2013.
According to my e-mail archives I got my BT-958 directly from Mylex brand
new as KT-958 back in early 1998 (the rest of the system is a bit older).
It wasn't up until 2003 when I was caught by the issue with the LOG SENSE
command that I got interested in the programming details of the adapter.
At that time Mylex was in flux already, having been bought by LSI shortly
before. Support advised me what was there at Leonard's http://www.dandelion.com
site was all that was available (I have a personal copy of the site) and
they would suggest to switch to their current products. So it was too
late already ten years before you got at the driver.
I'll yet double-check the contents of the KT-958 kit which I have kept,
but if there was any technical documentation supplied there on a CD (which
I doubt), I would have surely discovered it earlier. It's away along with
the server, remotely managed, ~160km/100mi from here, so it'll be some
time before I get at it though.
Still, maybe one of the SCSI old-timers has that stuff stashed somewhere.
I have plenty of technical documentation going back to early to mid 1990s
(some in the hard copy form), not necessarily readily available nowadays.
Sadly lots of such stuff goes offline or is completely lost to the mist of
time.
Maciej
On Fri, 2021-04-16 at 14:41 -0600, Khalid Aziz wrote:
> On 4/15/21 8:08 PM, Joe Perches wrote:
> > And while it's a lot more code, I'd prefer a solution that looks more
> > like the other commonly used kernel logging extension mechanisms
> > where adapter is placed before the format, ... in the argument list.
>
> Hi Joe,
>
> I don't mind making these changes. It is quite a bit of code but
> consistency with other kernel code is useful. Would you like to finalize
> this patch, or would you prefer that I take this patch as starting point
> and finalize it?
Probably better to apply Maciej's patches first and then something
like this.
btw: the coccinelle script was
@@
expression a, b;
@@
\(blogic_announce\|blogic_info\|blogic_notice\|blogic_warn\|blogic_err\)(
- a, b
+ b, a
, ...)
From: Maciej W. Rozycki
> Sent: 16 April 2021 11:49
>
> On Thu, 15 Apr 2021, Joe Perches wrote:
>
> > In patch 2, vscnprintf should probably be used to make sure it's
> > 0 terminated.
>
> Why? C99 has this[1]:
>
> "The vsnprintf function is equivalent to snprintf, with the variable
> argument list replaced by arg, which shall have been initialized by the
> va_start macro (and possibly subsequent va_arg calls)."
vscnprintf() is normally the function you want (not vsnprintf())
because the return value is the number of characters actually
put into the buffer, not the number that would have been written
had the buffer been long enough.
Return values larger than the buffer size are almost never
allowed for - and are probably a set of 'buffer overflow' bugs.
While probably justified by saying that it lets you malloc()
a big enough buffer and try again, the return value is almost
certainly just historic.
The original sprintf() libc code allocated a FILE structure on
stack set to fully-buffered with the current buffer pointer set
to the caller's buffer and a buffer length of MAXINT.
It then just called vprintf() to do the work.
snprintf() was done the same way, except the buffer length was
set and the 'write character' (or 'flush buffer') function
intercepted to avoid writes beyond the buffer end.
(Possibly by re-routing the writes to a global buffer.)
The return value from vprintf() gets returned to the user.
The Unix versions have always '\0' terminated the buffer.
Only Microsoft has ever released an snprintf() that doesn't
'\0' terminate the output - another source of bugs.
Personally I think bounded string functions should
return the buffer size on overflow.
This means sequences of:
offset += xxx(buf + offset, sizeof buf - offset, ...);
are safe and the overflow can be detected right at the end.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Sat, 17 Apr 2021, David Laight wrote:
> > > In patch 2, vscnprintf should probably be used to make sure it's
> > > 0 terminated.
> >
> > Why? C99 has this[1]:
> >
> > "The vsnprintf function is equivalent to snprintf, with the variable
> > argument list replaced by arg, which shall have been initialized by the
> > va_start macro (and possibly subsequent va_arg calls)."
>
> vscnprintf() is normally the function you want (not vsnprintf())
> because the return value is the number of characters actually
> put into the buffer, not the number that would have been written
> had the buffer been long enough.
Good catch, thank you! I'll respin the series then. Thank you for the
background story too!
Maciej
On Friday 16 April 2021 23:25:18 Maciej W. Rozycki wrote:
> On Fri, 16 Apr 2021, Khalid Aziz wrote:
>
> > > Sadly I didn't get to these resources while they were still there, and
> > > neither did archive.org, and now they not appear available from anywhere
> > > online. I'm sure Leonard had this all, but, alas, he is long gone too.
> >
> > These documents were all gone by the time I started working on this
> > driver in 2013.
>
> According to my e-mail archives I got my BT-958 directly from Mylex brand
> new as KT-958 back in early 1998 (the rest of the system is a bit older).
> It wasn't up until 2003 when I was caught by the issue with the LOG SENSE
> command that I got interested in the programming details of the adapter.
>
> At that time Mylex was in flux already, having been bought by LSI shortly
> before. Support advised me what was there at Leonard's http://www.dandelion.com
> site was all that was available (I have a personal copy of the site) and
> they would suggest to switch to their current products. So it was too
> late already ten years before you got at the driver.
>
> I'll yet double-check the contents of the KT-958 kit which I have kept,
> but if there was any technical documentation supplied there on a CD (which
> I doubt), I would have surely discovered it earlier. It's away along with
> the server, remotely managed, ~160km/100mi from here, so it'll be some
> time before I get at it though.
>
> Still, maybe one of the SCSI old-timers has that stuff stashed somewhere.
> I have plenty of technical documentation going back to early to mid 1990s
> (some in the hard copy form), not necessarily readily available nowadays.
> Sadly lots of such stuff goes offline or is completely lost to the mist of
> time.
>
> Maciej
>
Found the 3000763 document here:
https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/3000763_PCI_EISA_Wide_SCSI_Tech_Ref_Dec94.pdf
There's also 3002593 there:
https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/
--
Ondrej Zary
On Mon, 19 Apr 2021, Khalid Aziz wrote:
> On 4/18/21 2:21 PM, Ondrej Zary wrote:
> >
> > Found the 3000763 document here:
> > https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/3000763_PCI_EISA_Wide_SCSI_Tech_Ref_Dec94.pdf
> >
> > There's also 3002593 there:
> > https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/
> >
>
> Thanks!!!
Ondrej: Thanks a lot indeed! These documents seem to have the essential
interface details all covered, except for Fast-20 SCSI adapters, which I
guess are a minor modification from the software's point of view.
Khalid: I have skimmed over these documents and I infer 24-bit addressing
can be verified with any MultiMaster adapter, including ones that do have
32-bit addressing implemented, by using the legacy Initialize Mailbox HBA
command. That could be used to stop Christoph's recent changes for older
adapter support removal and replace them with proper fixes for whatever
has become broken. Is that something you'd be willing as the driver's
maintainer to look into, or shall I?
Maciej
On 4/18/21 2:21 PM, Ondrej Zary wrote:
> On Friday 16 April 2021 23:25:18 Maciej W. Rozycki wrote:
>> On Fri, 16 Apr 2021, Khalid Aziz wrote:
>>
>>>> Sadly I didn't get to these resources while they were still there, and
>>>> neither did archive.org, and now they not appear available from anywhere
>>>> online. I'm sure Leonard had this all, but, alas, he is long gone too.
>>>
>>> These documents were all gone by the time I started working on this
>>> driver in 2013.
>>
>> According to my e-mail archives I got my BT-958 directly from Mylex brand
>> new as KT-958 back in early 1998 (the rest of the system is a bit older).
>> It wasn't up until 2003 when I was caught by the issue with the LOG SENSE
>> command that I got interested in the programming details of the adapter.
>>
>> At that time Mylex was in flux already, having been bought by LSI shortly
>> before. Support advised me what was there at Leonard's http://www.dandelion.com
>> site was all that was available (I have a personal copy of the site) and
>> they would suggest to switch to their current products. So it was too
>> late already ten years before you got at the driver.
>>
>> I'll yet double-check the contents of the KT-958 kit which I have kept,
>> but if there was any technical documentation supplied there on a CD (which
>> I doubt), I would have surely discovered it earlier. It's away along with
>> the server, remotely managed, ~160km/100mi from here, so it'll be some
>> time before I get at it though.
>>
>> Still, maybe one of the SCSI old-timers has that stuff stashed somewhere.
>> I have plenty of technical documentation going back to early to mid 1990s
>> (some in the hard copy form), not necessarily readily available nowadays.
>> Sadly lots of such stuff goes offline or is completely lost to the mist of
>> time.
>>
>> Maciej
>>
>
> Found the 3000763 document here:
> https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/3000763_PCI_EISA_Wide_SCSI_Tech_Ref_Dec94.pdf
>
> There's also 3002593 there:
> https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/
>
Thanks!!!
--
Khalid
On 4/19/21 10:01 AM, Maciej W. Rozycki wrote:
> On Mon, 19 Apr 2021, Khalid Aziz wrote:
>
>> On 4/18/21 2:21 PM, Ondrej Zary wrote:
>>>
>>> Found the 3000763 document here:
>>> https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/3000763_PCI_EISA_Wide_SCSI_Tech_Ref_Dec94.pdf
>>>
>>> There's also 3002593 there:
>>> https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/
>>>
>>
>> Thanks!!!
>
> Ondrej: Thanks a lot indeed! These documents seem to have the essential
> interface details all covered, except for Fast-20 SCSI adapters, which I
> guess are a minor modification from the software's point of view.
>
> Khalid: I have skimmed over these documents and I infer 24-bit addressing
> can be verified with any MultiMaster adapter, including ones that do have
> 32-bit addressing implemented, by using the legacy Initialize Mailbox HBA
> command. That could be used to stop Christoph's recent changes for older
> adapter support removal and replace them with proper fixes for whatever
> has become broken. Is that something you'd be willing as the driver's
> maintainer to look into, or shall I?
>
Hi Maciej,
Do you mean use OpCode 01 (INITIALIZE MAILBOX) to set a 24-bit address
for mailbox in place of OpCode 81? Verifying the change would be a
challenge. Do you have an old adapter to test it with? If you do, go
ahead and make the changes. I will be happy to review. I have only a
BT-757 adapter.
Thanks,
Khalid
On Mon, 19 Apr 2021, Khalid Aziz wrote:
> > Khalid: I have skimmed over these documents and I infer 24-bit addressing
> > can be verified with any MultiMaster adapter, including ones that do have
> > 32-bit addressing implemented, by using the legacy Initialize Mailbox HBA
> > command. That could be used to stop Christoph's recent changes for older
> > adapter support removal and replace them with proper fixes for whatever
> > has become broken. Is that something you'd be willing as the driver's
> > maintainer to look into, or shall I?
>
> Do you mean use OpCode 01 (INITIALIZE MAILBOX) to set a 24-bit address
> for mailbox in place of OpCode 81? Verifying the change would be a
> challenge. Do you have an old adapter to test it with? If you do, go
> ahead and make the changes. I will be happy to review. I have only a
> BT-757 adapter.
Yes, but upon inspection it looks like our driver doesn't use that opcode
and relies solely on 32-bit Mode Initialize Mailbox (0x81) even with ISA
devices. That makes sense as documentation indicates the firmware has
been designed to be unified so that the same binary microcontroller code
runs across all BusLogic MultiMaster devices.
Anyway given the unified API it should be straightforward to simulate an
older adapter with a newer one, except for host bus protocol differences.
So verifying the workaround for broken BT-445S adapters continues to work
once modernised is not going to be a problem as it can be unconditionally
activated in a debug environment. That would verify correct DMA bounce
buffer operation under the new scheme.
Verifying actual ISA operations (third-party DMA, etc.) cannot be made
this way, but as I understand the issue there is merely with passing data
structures around and that may not require too much attention beyond
getting things syntactically correct, which I gather someone forgot to do
with a change made a while ago. So that should be doable as well.
NB as noted before I only have a BT-958 readily wired for operation. I
don't expect I have any other BusLogic hardware, but I may yet have to
double-check a stash of hardware I have accumulated over the years. But
that is overseas, so I won't be able to get at it before we're at least
somewhat closer to normality. If all else fails I could possibly buy one.
I have respun the series now as promised. Does your BT-757 adapter avoid
the issue with trailing allocation somehow?
Maciej
On 4/20/21 12:02 PM, Maciej W. Rozycki wrote:
> On Mon, 19 Apr 2021, Khalid Aziz wrote:
>
>>> Khalid: I have skimmed over these documents and I infer 24-bit addressing
>>> can be verified with any MultiMaster adapter, including ones that do have
>>> 32-bit addressing implemented, by using the legacy Initialize Mailbox HBA
>>> command. That could be used to stop Christoph's recent changes for older
>>> adapter support removal and replace them with proper fixes for whatever
>>> has become broken. Is that something you'd be willing as the driver's
>>> maintainer to look into, or shall I?
>>
>> Do you mean use OpCode 01 (INITIALIZE MAILBOX) to set a 24-bit address
>> for mailbox in place of OpCode 81? Verifying the change would be a
>> challenge. Do you have an old adapter to test it with? If you do, go
>> ahead and make the changes. I will be happy to review. I have only a
>> BT-757 adapter.
>
> Yes, but upon inspection it looks like our driver doesn't use that opcode
> and relies solely on 32-bit Mode Initialize Mailbox (0x81) even with ISA
> devices. That makes sense as documentation indicates the firmware has
> been designed to be unified so that the same binary microcontroller code
> runs across all BusLogic MultiMaster devices.
>
> Anyway given the unified API it should be straightforward to simulate an
> older adapter with a newer one, except for host bus protocol differences.
> So verifying the workaround for broken BT-445S adapters continues to work
> once modernised is not going to be a problem as it can be unconditionally
> activated in a debug environment. That would verify correct DMA bounce
> buffer operation under the new scheme.
>
> Verifying actual ISA operations (third-party DMA, etc.) cannot be made
> this way, but as I understand the issue there is merely with passing data
> structures around and that may not require too much attention beyond
> getting things syntactically correct, which I gather someone forgot to do
> with a change made a while ago. So that should be doable as well.
In theory this sounds reasonable, but without being able to test with a
real hardware I would be concerned about making this change.
>
> NB as noted before I only have a BT-958 readily wired for operation. I
> don't expect I have any other BusLogic hardware, but I may yet have to
> double-check a stash of hardware I have accumulated over the years. But
> that is overseas, so I won't be able to get at it before we're at least
> somewhat closer to normality. If all else fails I could possibly buy one.
>
> I have respun the series now as promised. Does your BT-757 adapter avoid
> the issue with trailing allocation somehow?
>
Well, my only test machine with a legacy PCI slot died some time back. I
have been working on putting together a replacement and have now been
able to get a working machine with a BT-950 adapter. I have not seen
issue with trailing allocation upto 5.12-rc8. I am going to try the top
of tree as well to make sure I do not run into this issue.
--
Khalid
On Wed, 21 Apr 2021, Khalid Aziz wrote:
> > Verifying actual ISA operations (third-party DMA, etc.) cannot be made
> > this way, but as I understand the issue there is merely with passing data
> > structures around and that may not require too much attention beyond
> > getting things syntactically correct, which I gather someone forgot to do
> > with a change made a while ago. So that should be doable as well.
>
> In theory this sounds reasonable, but without being able to test with a
> real hardware I would be concerned about making this change.
Sometimes you have little choice really and that would be less disruptive
than dropping support altogether. Even if there's a small issue somewhere
it's easier to fix by a competent developer who actually gets the hands on
a piece of hardware than bringing back old code that has been removed and
consequently not updated according to internal API evolution, etc.
I had this issue with the defxx driver with which for years I didn't have
a specimen of the EISA variant of the hardware handled. I did my best to
maintain EISA support however and while indeed I made a few of mistakes on
the way, they were easy to straighten out once I finally did get my hands
on an EISA piece.
> > NB as noted before I only have a BT-958 readily wired for operation. I
> > don't expect I have any other BusLogic hardware, but I may yet have to
> > double-check a stash of hardware I have accumulated over the years. But
> > that is overseas, so I won't be able to get at it before we're at least
> > somewhat closer to normality. If all else fails I could possibly buy one.
> >
> > I have respun the series now as promised. Does your BT-757 adapter avoid
> > the issue with trailing allocation somehow?
>
> Well, my only test machine with a legacy PCI slot died some time back. I
> have been working on putting together a replacement and have now been
> able to get a working machine with a BT-950 adapter. I have not seen
> issue with trailing allocation upto 5.12-rc8. I am going to try the top
> of tree as well to make sure I do not run into this issue.
I guess you won't see the issue with a FlashPoint adapter as they work in
a different manner. I think your EISA MultiMaster device is more likely
to have a problem here.
And AFAICT most SCSI commands (or at least the older ones which used to
be there when development was still active with the MultiMaster devices)
return exactly as much data as requested, so I guess the issue may have
gone unnoticed. I'll see if I can find some time to investigate this
further now that we have proper documentation available, but meanwhile I
do hope the workaround I have come up with 18 years ago already is good
enough to keep it.
Maciej
On 4/22/21 10:27 AM, Maciej W. Rozycki wrote:
> On Wed, 21 Apr 2021, Khalid Aziz wrote:
>
>>> Verifying actual ISA operations (third-party DMA, etc.) cannot be made
>>> this way, but as I understand the issue there is merely with passing data
>>> structures around and that may not require too much attention beyond
>>> getting things syntactically correct, which I gather someone forgot to do
>>> with a change made a while ago. So that should be doable as well.
>>
>> In theory this sounds reasonable, but without being able to test with a
>> real hardware I would be concerned about making this change.
>
> Sometimes you have little choice really and that would be less disruptive
> than dropping support altogether. Even if there's a small issue somewhere
> it's easier to fix by a competent developer who actually gets the hands on
> a piece of hardware than bringing back old code that has been removed and
> consequently not updated according to internal API evolution, etc.
We are talking about removing support for BT-445S with firmware version
older than 3.37. That is a very specific case. To continue support for
this very specific case, we have to add new code to use local bounce
buffer and we have no hardware to verify this new code. This will be new
code whether we add it now or later after we find someone even has this
very old card with old firmware. I would prefer to remove support for
now and add new code to add support for firmware version older than 3.37
back only if there is a need later. For now anyone who is using a
BT-445S and has updated firmware on their card will not see a change.
--
Khalid
On Thu, 22 Apr 2021, Khalid Aziz wrote:
> >>> Verifying actual ISA operations (third-party DMA, etc.) cannot be made
> >>> this way, but as I understand the issue there is merely with passing data
> >>> structures around and that may not require too much attention beyond
> >>> getting things syntactically correct, which I gather someone forgot to do
> >>> with a change made a while ago. So that should be doable as well.
> >>
> >> In theory this sounds reasonable, but without being able to test with a
> >> real hardware I would be concerned about making this change.
> >
> > Sometimes you have little choice really and that would be less disruptive
> > than dropping support altogether. Even if there's a small issue somewhere
> > it's easier to fix by a competent developer who actually gets the hands on
> > a piece of hardware than bringing back old code that has been removed and
> > consequently not updated according to internal API evolution, etc.
>
> We are talking about removing support for BT-445S with firmware version
> older than 3.37.
Umm, no. As still quoted above I referred to ISA devices, such as the
BT-545C. ISA only has 24 address lines so no firmware change can make
these devices address memory beyond 16MiB (whether as a bus master or with
the aid of an 8237 DMA controller).
> That is a very specific case. To continue support for
> this very specific case, we have to add new code to use local bounce
> buffer and we have no hardware to verify this new code. This will be new
> code whether we add it now or later after we find someone even has this
> very old card with old firmware. I would prefer to remove support for
> now and add new code to add support for firmware version older than 3.37
> back only if there is a need later. For now anyone who is using a
> BT-445S and has updated firmware on their card will not see a change.
As long as ISA support has been retained the BT-445S can just reuse the
logic.
I'm not strongly attached to ISA support though, and we continue
supporting other SCSI HBAs for ISA. But we do that even though they
require a dedicated driver while with the unified MultiMaster architecture
it would seem support for another host bus should be low-hanging fruit.
Maciej