r330951 by smh fixed the mps driver to avoid deadlocks when panicing.
The same code is needed for mpr, so port it here, along with the fix
which allows the CCBs scheduled to complete avoiding at least a scary
message and likely other unintended consequences.
Sponsored by: Netflix
Differential Review: https://reviews.freebsd.org/D16663
When we're shutting down, we send a number of start/stop commands to
the known targets. We have to wait for them to complete. During a
panic, the interrupts are off, and using pause to wait for them to
fire and complete won't work: we have to poll after pause returns so
the completion routines of the CCBs run so we decrement work
outstanding counts.
Sponsored by: Netflix
Differential Review: https://reviews.freebsd.org/D16663
opt_compat.h is mentioned in nearly 180 files. In-progress network
driver compabibility improvements may add over 100 more so this is
closer to "just about everywhere" than "only some files" per the
guidance in sys/conf/options.
Keep COMPAT_LINUX32 in opt_compat.h as it is confined to a subset of
sys/compat/linux/*.c. A fake _COMPAT_LINUX option ensure opt_compat.h
is created on all architectures.
Move COMPAT_LINUXKPI to opt_dontuse.h as it is only used to control the
set of compiled files.
Reviewed by: kib, cem, jhb, jtl
Sponsored by: DARPA, AFRL
Differential Revision: https://reviews.freebsd.org/D14941
The mps(4) and mpr(4) drivers and hardware handle T10 Protection
Information, which is a system of checksums and guard blocks to protect
data while it is being transferred and while it is on disk. It is also
known as T10 DIF. For more details, see section 4.22 of the SBC-4 spec.
Supporting Type 2 protection requires using 32 byte CDBs, and filling in
the fields in those CDBs. We don't yet support that in the da(4) driver.
Type 1 and Type 3 protection don't require that, and can be handled by
the mps(4)/mpr(4) driver's code and firmware without any additional
input from the da(4) driver.
If a drive has Type 2 protection enabled (you frequently see this with
SAS drives shipped from Dell), don't set the various EEDP fields in the
mps(4)/mpr(4) driver command fields. Otherwise, you wind up with errors
like this that would otherwise make no sense:
(da9:mpr0:0:18:0): READ(10). CDB: 28 00 00 00 00 00 00 02 00 00
(da9:mpr0:0:18:0): CAM status: SCSI Status Error
(da9:mpr0:0:18:0): SCSI status: Check Condition
(da9:mpr0:0:18:0): SCSI sense: ILLEGAL REQUEST asc:20,0 (Invalid command operation code)
(da9:mpr0:0:18:0):
(da9:mpr0:0:18:0): Field Replaceable Unit: 0
(da9:mpr0:0:18:0): Command Specific Info: 0
(da9:mpr0:0:18:0):
(da9:mpr0:0:18:0): Descriptor 0x80: f8 21
(da9:mpr0:0:18:0): Descriptor 0x81: 00 00 00 00 00 00
(da9:mpr0:0:18:0): Error 22, Unretryable error
In other words, what kind of strange SAS hard drive doesn't support a
standard 10 byte SCSI READ command? In this case, one that has Type 2
protection enabled.
We can revisit this when we put Type 2 protection support in the da(4)
driver, but for now this will help people who put Type 2 formatted drives
in a system and wonder what in the world is going on.
MFC after: 3 days
Sponsored by: Spectra Logic
During shutdown mps waits for its SSU requests to complete however when
performing a reboot after handling a panic the scheduler is stopped so
getmicrotime which is used can be non-functional.
Switch to using the same method as shutdown_panic to ensure we actually
complete.
In addition reduce the timeout when RB_NOSYNC is set in howto as we expect
this to fail.
Reviewed by: slm
MFC after: 1 week
Sponsored by: Multiplay
Differential Revision: https://reviews.freebsd.org/D12776
Chain frames required to satisfy all 2K of declared I/Os of 128KB each take
more then a megabyte of a physical memory, all of which existing code tries
allocate as physically contiguous. This patch removes that physical
contiguousness requirement, leaving only virtual contiguousness. I was
thinking about other ways of allocation, but the less granular allocation
becomes, the bigger is the overhead and/or complexity, reaching about 100%
overhead if allocate each frame separately.
The patch also bumps the chain frames hard limit from 2K to 16K. It is more
than enough for the case of default REQ_FRAMES and MAXPHYS (the drivers will
allocate less than that automatically), while in case of increased MAXPHYS
it will control maximal memory usage.
Sponsored by: iXsystems, Inc.
Differential Revision: https://reviews.freebsd.org/D14420
This is a first part of the change. It makes the drivers to calculate
the required number of chain frames to satisfy worst case scenarios, but
it does not change existing overly strict limits on them. The next step
will be to rewrite the allocator to not require megabytes of physically
contiguous address space, that may be problematic if done after boot,
after doing which the limits can be removed. Until that this code can
just correct user set limits, if they are set too high.
Sponsored by: iXsystems, Inc.
Differential Revision: https://reviews.freebsd.org/D14261
a bit in the normal operation of the driver. Covert it to represent bytes
instead of 32bit words. Fix what I believe to be is a bug in this respect
with the Tri-mode cards.
Sponsored by: Netflix
Both drivers were found to report CAM bigger queue depth then they really
can handle. It made them later under high load with many disks return
some of submitted requests back with CAM_REQUEUE_REQ status for later
resubmission.
Reviewed by: scottl
MFC after: 1 week
Sponsored by: iXsystems, Inc.
Differential Revision: https://reviews.freebsd.org/D14215
In mp{r,s}_diag_register(), which is used to register diagnostic
buffers with the mp{r,s}(4) firmware, we allocate DMAable memory.
There were several issues here:
o No checking of the bus_dmamap_load() return value. If the load
failed or got deferred, mp{r,s}_diag_register() continued on as if
nothing had happened. We now check the return value and bail
out if it fails.
o No waiting for a deferred load callback. bus_dmamap_load()
calls a supplied callback when the mapping is done. This is
generally done immediately, but it can be deferred.
mp{r,s}_diag_register() did not check to see whether the callback
was already done before proceeding on. We now sleep until the
callback is done if it is deferred.
o No call to bus_dmamap_sync(... BUS_DMASYNC_PREREAD) after the
memory is allocated and loaded. This is necessary on some
platforms to synchronize host memory that is going to be updated
by a device.
Both drivers would also panic if the firmware was reinitialized while
a diagnostic buffer operation was in progress. This fixes that problem
as well. (The driver will reinitialize the firmware in various
circumstances, but the problem I ran into was that the firmware would
generate an IOC Fault due to a PCIe error.)
mp{r,s}var.h:
Add a new structure, struct mpr_busdma_context, that is
used for deferred busdma load callbacks.
Add a prototype for mp{r,s}_memaddr_wait_cb().
mp{r,s}.c:
Add a new busdma callback function, mp{r,s}_memaddr_wait_cb().
This provides synchronization for callers that want to
wait on a deferred bus_dmamap_load() callback.
mp{r,s}_user.c:
In bus_dmamap_register(), add a call to bus_dmamap_sync()
with the BUS_DMASYNC_PREREAD flag set after an allocation
is loaded.
Also, check the return value of bus_dmamap_load(). If it
fails, bail out. If it is EINPROGRESS, wait for the
callback to happen. We use an interruptible sleep (msleep
with PCATCH) and let the callback clean things up if we get
interrupted.
In mpr_diag_read_buffer() and mps_diag_read_buffer(), call
bus_dmamap_sync(..., BUS_DMASYNC_POSTREAD) before copying
the data out to make sure the data is in stable storage.
In mp{r,s}_post_fw_diag_buffer() and
mp{r,s}_release_fw_diag_buffer(), check the reply to see
whether it is NULL. It can be NULL (and the command non-NULL)
if the controller gets reinitialized while we're waiting for
the command to complete but the driver structures aren't
reallocated. The driver structures generally won't be
reallocated unless there is a firmware upgrade that changes
one of the IOCFacts.
When freeing diagnostic buffers in mp{r,s}_diag_register()
and mp{r,s}_diag_unregister(), zero/NULL out the buffer after
freeing it. This will prevent a duplicate free in some
situations.
Sponsored by: Spectra Logic
Reviewed by: mav, scottl
MFC after: 1 week
Differential Revision: D13453
SGList elements, but there's only enough space in the request frame for
either 1 element or a chain frame pointer. Previously, the code would
hit the wrong case, add the SGList element, but then fail to add the
chain frame due to lack of space. Re-arrange the code to catch this case
earlier and handle it.
Sponsored by: Netflix
When allocating memory through malloc(9), we always expect the amount of
memory requested to be unsigned as a negative value would either stand for
an error or an overflow.
Unsign some values, found when considering the use of mallocarray(9), to
avoid unnecessary casting. Also consider that indexes should be of
at least the same size/type as the upper limit they pretend to index.
MFC after: 3 weeks
Uses of mallocarray(9).
The use of mallocarray(9) has rocketed the required swap to build FreeBSD.
This is likely caused by the allocation size attributes which put extra pressure
on the compiler.
Given that most of these checks are superfluous we have to choose better
where to use mallocarray(9). We still have more uses of mallocarray(9) but
hopefully this is enough to bring swap usage to a reasonable level.
Reported by: wosch
PR: 225197
Focus on code where we are doing multiplications within malloc(9). None of
these is likely to overflow, however the change is still useful as some
static checkers can benefit from the allocation attributes we use for
mallocarray.
This initial sweep only covers malloc(9) calls with M_NOWAIT. No good
reason but I started doing the changes before r327796 and at that time it
was convenient to make sure the sorrounding code could handle NULL values.
Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.
The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.
Mainly focus on files that use BSD 3-Clause license.
The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.
Special thanks to Wind River for providing access to "The Duke of
Highlander" tool: an older (2014) run over FreeBSD tree was useful as a
starting point.
would attempt to re-allocate interrupts during a chip reset without
first de-allocating them. Doing that right is going to be tricky, so
just band-aid it for now so that a re-init doesn't guarantee a failure
due to resource re-use.
Reported by: gallatin
Sponsored by: Netflix
needed, but it silences an erroneous Coverity warning and makes the code a
little more logically consistent. Also mark the sysctl as MPSAFE.
Sponsored by: Netflix
commit it to make initiazation less chatty in the normal case, and more useful
and informative when real debugging is turned on.
Reviewed by: ken (earlier version)
Sponsored by: Netflix
When the mps(4) and mpr(4) drivers need to reinitialize the
firmware, they sometimes need to reallocate all of the memory
allocated by the driver. The reallocation happens whenever the IOC
Facts change. That should only happen after a firmware upgrade.
If the reinitialization happens as a result of a timed out command
sent to the card, the command that timed out and triggered the
reinit may have been freed if iocfacts_allocate() reallocated all
memory. If the caller attempts to access the command after that,
the kernel will panic because the caller will be dereferencing
freed memory.
The solution is to set a flag in the softc when we reallocate,
and avoid dereferencing the command strucure if we've reallocated.
The changes are largely the same in both drivers, since mpr(4) is a
derivative of mps(4).
o In iocfacts_allocate(), if the IOC Facts have changed and we
need to reallocate, set the REALLOCATED flag in the softc.
o Change wait_command() to take a struct mps_command ** instead of
a struct mps_command *. This allows us to NULL out the caller's
command pointer if we have to reinit the controller and the data
structures get reallocated. (The REALLOCATED flag will be set
in the softc if that has happened.)
o In every place that calls wait_command(), make sure we handle
the case where the command is NULL after the call.
o The mpr(4) driver has mpr_request_polled() which can also
reinitialize the card. Also check for reallocation there.
Reviewed by: scottl, slm
MFC after: 1 week
Sponsored by: Spectra Logic
the informational print functions. Collapse the debug API a bit to be
more generic and not require as much code duplication. While here, fix
a bug in MPS that was already fixed in MPR.
Do the allocation before requesting the IOCFacts message. This triggers
the LSI firmware to recognize the multiqueue should be enabled if available.
Multiqueue isn't used by the driver yet, but this also fixes a problem with
the cached IOCFacts not matching latter checks, leading to potential problems
with error recovery.
As a side-effect, fetch the driver tunables as early as possible.
Reviewed by: slm
Obtained from: Netflix
Differential Revision: D9243
mps_wait_command() and mpr_wait_command() were using getmicrotime() to
determine elapsed time when checking for a timeout in polled mode.
getmicrotime() isn't guaranteed to monotonically increase, and that
caused spurious timeouts occasionally.
Switch to using getmicrouptime(), which does increase monotonically.
This fixes the spurious timeouts in my test case.
Reviewed by: slm, scottl
MFC after: 3 days
Sponsored by: Spectra Logic
All of the printing from the tables file now has wrappers so that the
handling is cleaner and it's possible to print something out (say, during
development) without having to fight the global debug flags. This re-org
will also make it easier to have the tables be compiled out at build time
if desired.
Other than fixing some minor bugs, there are no user-visible changes from
this change
Sponsored by: Netflix, Inc.
Differential Revision: D9238
The sim_vid, hba_vid, and dev_name fields of struct ccb_pathinq are
fixed-length strings. AFAICT the only place they're read is in
sbin/camcontrol/camcontrol.c, which assumes they'll be null-terminated.
However, the kernel doesn't null-terminate them. A bunch of copy-pasted code
uses strncpy to write them, and doesn't guarantee null-termination. For at
least 4 drivers (mpr, mps, ciss, and hyperv), the hba_vid field actually
overflows. You can see the result by doing "camcontrol negotiate da0 -v".
This change null-terminates those fields everywhere they're set in the
kernel. It also shortens a few strings to ensure they'll fit within the
16-character field.
PR: 215474
Reported by: Coverity
CID: 1009997 1010000 1010001 1010002 1010003 1010004 1010005
CID: 1331519 1010006 1215097 1010007 1288967 1010008 1306000
CID: 1211924 1010009 1010010 1010011 1010012 1010013 1010014
CID: 1147190 1010017 1010016 1010018 1216435 1010020 1010021
CID: 1010022 1009666 1018185 1010023 1010025 1010026 1010027
CID: 1010028 1010029 1010030 1010031 1010033 1018186 1018187
CID: 1010035 1010036 1010042 1010041 1010040 1010039
Reviewed by: imp, sephe, slm
MFC after: 4 weeks
Sponsored by: Spectra Logic Corp
Differential Revision: https://reviews.freebsd.org/D9037
Differential Revision: https://reviews.freebsd.org/D9038
that are apparently misconfigured by the manufacturer and cause the mapping
logic to fail. The fallback allows drive numbers to be assigned based on the
PHY number that they're attached to. Add sysctls and tunables to overrid
this new behavior, but they should be considered only necessary for debugging.
Reviewed by: imp, smh
Obtained from: Netflix
MFC after: 3 days
Sponsored by: D8403
Upstream the BUF_TRACKING and FULL_BUF_TRACKING buffer debugging code.
This can be handy in tracking down what code touched hung bios and bufs
last. The full history is especially useful, but adds enough bloat that
it shouldn't be enabled in release builds.
Function names (or arbitrary string constants) are tracked in a
fixed-size ring in bufs. Bios gain a pointer to the upper buf for
tracking. SCSI CCBs gain a pointer to the upper bio for tracking.
Reviewed by: markj
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D8366
