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.
- XPT_NOTIFY_ACKNOWLEDGE was not handled, causing stuck requests.
- XPT_ABORT was not even trying to abort active ATIOs/INOTs.
- Initiator's tag was not stored and not used where needed.
- List of TM request types needed update.
- mpt_scsi_tgt_status() missed some useful debugging.
After this change global TM requests, such as reset, should work properly.
ABORT TASK (ABTS) requests are still not passes to CTL, that is not good
and should be fixed.
MFC after: 2 weeks
- Not set BufferLength caused receive of empty ATIOs.
- CDB length guessing was broken at least for RC16.
- mpt_req_untimeout() was called with wrong req parameter.
- Sense data reporting was broken in several ways.
With this change my LSI7204EP-LC can pass at least basic tests as target.
The code is still far from perfect, but finally I found second hw/driver
after isp(4) that really can work in CAM target mode.
MFC after: 2 weeks
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
Previously, any timeout value for which (timeout * hz) will overflow the
signed integer, will give weird results, since callout(9) routines will
convert negative values of ticks to '1'. For unsigned integer overflow we
will get sufficiently smaller timeout values than expected.
Switch from callout_reset, which requires conversion to int based ticks
to callout_reset_sbt to avoid this.
Also correct isci to correctly resolve ccb timeout.
This was based on the original work done by Eygene Ryabinkin
<rea@freebsd.org> back in 5 Aug 2011 which used a macro to help avoid
the overlow.
Differential Revision: https://reviews.freebsd.org/D1157
Reviewed by: mav, davide
MFC after: 1 month
Sponsored by: Multiplay
- #if 0 the currently unused paired port linking and unlinking of dual
adapters.
- Simplify MSI/MSI-X allocation and release. For a single one, we don't need
to fiddle with the MSI/MSI-X count and pci_release_msi(9) is smart enough
to just do nothing in case of INTx.
- Canonicalize actions taken on attach failure and detach.
- Remove the remainder of incomplete support for older FreeBSD versions.
MFC after: 1 week
shifts into the sign bit. Instead use (1U << 31) which gets the
expected result.
This fix is not ideal as it assumes a 32 bit int, but does fix the issue
for most cases.
A similar change was made in OpenBSD.
Discussed with: -arch, rdivacky
Reviewed by: cperciva
mostly by adjustments to debugging printf() format specifiers. For high
numbered LUNs, also switch to printing them in hex as per SAM-5.
MFC after: 2 weeks
Stop abusing xpt_periph in random plases that really have no periph related
to CCB, for example, bus scanning. NULL value is fine in such cases and it
is correctly logged in debug messages as "noperiph". If at some point we
need some real XPT periphs (alike to pmpX now), quite likely they will be
per-bus, and not a single global instance as xpt_periph now.
every architecture's busdma_machdep.c. It is done by unifying the
bus_dmamap_load_buffer() routines so that they may be called from MI
code. The MD busdma is then given a chance to do any final processing
in the complete() callback.
The cam changes unify the bus_dmamap_load* handling in cam drivers.
The arm and mips implementations are updated to track virtual
addresses for sync(). Previously this was done in a type specific
way. Now it is done in a generic way by recording the list of
virtuals in the map.
Submitted by: jeff (sponsored by EMC/Isilon)
Reviewed by: kan (previous version), scottl,
mjacob (isp(4), no objections for target mode changes)
Discussed with: ian (arm changes)
Tested by: marius (sparc64), mips (jmallet), isci(4) on x86 (jharris),
amd64 (Fabian Keil <freebsd-listen@fabiankeil.de>)
to attach to target capable HBAs that implement the old immediate
notify (XPT_IMMED_NOTIFY) and notify acknowledge (XPT_NOTIFY_ACK)
CCBs. The new API has been in place since SVN change 196008 in
2009.
The solution is two-fold: fix CTL to handle the responses from the
HBAs, and convert the HBA drivers in question to use the new API.
These drivers have not been tested with CTL, so how well they will
interoperate with CTL is unknown.
scsi_target.c: Update the userland target example code to use the
new immediate notify API.
scsi_ctl.c: Detect when an immediate notify CCB is returned
with CAM_REQ_INVALID or CAM_PROVIDE_FAIL status,
and just free it.
Fix a duplicate assignment.
aic79xx.c,
aic79xx_osm.c: Update the aic79xx driver to use the new API.
Target mode is not enabled on for this driver, so
the changes will have no practical effect.
aic7xxx.c,
aic7xxx_osm.c: Update the aic7xxx driver to use the new API.
sbp_targ.c: Update the firewire target code to work with the
new API.
mpt_cam.c: Update the mpt(4) driver to work with the new API.
Target mode is only enabled for Fibre Channel
mpt(4) devices.
MFC after: 3 days
be the same chip):
- The I/O port resource may not be available with these. However, given
that we actually only need this resource for some controllers that
require their firmware to be up- and downloaded (which excludes the
SAS1078{,DE}) just handle failure to allocate this resource gracefully
when possible. While at it, generally put non-fatal resource allocation
failures under bootverbose.
- SAS1078{,DE} use a different hard reset protocol.
- Add workarounds for the 36GB physical address limitation of scatter/
gather elements of these controllers.
Tested by: Slawa Olhovchenkov
PR: 149220 (remaining part)
Zero any sense not transferred by the device as the SCSI specification
mandates that any untransferred data should be assumed to be zero.
Reviewed by: ken
CAM.
Desriptor sense is a new sense data format that originated in SPC-3. Among
other things, it allows for an 8-byte info field, which is necessary to
pass back block numbers larger than 4 bytes.
This change adds a number of new functions to scsi_all.c (and therefore
libcam) that abstract out most access to sense data.
This includes a bump of CAM_VERSION, because the CCB ABI has changed.
Userland programs that use the CAM pass(4) driver will need to be
recompiled.
camcontrol.c: Change uses of scsi_extract_sense() to use
scsi_extract_sense_len().
Use scsi_get_sks() instead of accessing sense key specific
data directly.
scsi_modes: Update the control mode page to the latest version (SPC-4).
scsi_cmds.c,
scsi_target.c: Change references to struct scsi_sense_data to struct
scsi_sense_data_fixed. This should be changed to allow the
user to specify fixed or descriptor sense, and then use
scsi_set_sense_data() to build the sense data.
ps3cdrom.c: Use scsi_set_sense_data() instead of setting sense data
manually.
cam_periph.c: Use scsi_extract_sense_len() instead of using
scsi_extract_sense() or accessing sense data directly.
cam_ccb.h: Bump the CAM_VERSION from 0x15 to 0x16. The change of
struct scsi_sense_data from 32 to 252 bytes changes the
size of struct ccb_scsiio, but not the size of union ccb.
So the version must be bumped to prevent structure
mis-matches.
scsi_all.h: Lots of updated SCSI sense data and other structures.
Add function prototypes for the new sense data functions.
Take out the inline implementation of scsi_extract_sense().
It is now too large to put in a header file.
Add macros to calculate whether fields are present and
filled in fixed and descriptor sense data
scsi_all.c: In scsi_op_desc(), allow the user to pass in NULL inquiry
data, and we'll assume a direct access device in that case.
Changed the SCSI RESERVED sense key name and description
to COMPLETED, as it is now defined in the spec.
Change the error recovery action for a number of read errors
to prevent lots of retries when the drive has said that the
block isn't accessible. This speeds up reconstruction of
the block by any RAID software running on top of the drive
(e.g. ZFS).
In scsi_sense_desc(), allow for invalid sense key numbers.
This allows calling this routine without checking the input
values first.
Change scsi_error_action() to use scsi_extract_sense_len(),
and handle things when invalid asc/ascq values are
encountered.
Add a new routine, scsi_desc_iterate(), that will call the
supplied function for every descriptor in descriptor format
sense data.
Add scsi_set_sense_data(), and scsi_set_sense_data_va(),
which build descriptor and fixed format sense data. They
currently default to fixed format sense data.
Add a number of scsi_get_*() functions, which get different
types of sense data fields from either fixed or descriptor
format sense data, if the data is present.
Add a number of scsi_*_sbuf() functions, which print
formatted versions of various sense data fields. These
functions work for either fixed or descriptor sense.
Add a number of scsi_sense_*_sbuf() functions, which have a
standard calling interface and print the indicated field.
These functions take descriptors only.
Add scsi_sense_desc_sbuf(), which will print a formatted
version of the given sense descriptor.
Pull out a majority of the scsi_sense_sbuf() function and
put it into scsi_sense_only_sbuf(). This allows callers
that don't use struct ccb_scsiio to easily utilize the
printing routines. Revamp that function to handle
descriptor sense and use the new sense fetching and
printing routines.
Move scsi_extract_sense() into scsi_all.c, and implement it
in terms of the new function, scsi_extract_sense_len().
The _len() version takes a length (which should be the
sense length - residual) and can indicate which fields are
present and valid in the sense data.
Add a couple of new scsi_get_*() routines to get the sense
key, asc, and ascq only.
mly.c: Rename struct scsi_sense_data to struct
scsi_sense_data_fixed.
sbp_targ.c: Use the new sense fetching routines to get sense data
instead of accessing it directly.
sbp.c: Change the firewire/SCSI sense data transformation code to
use struct scsi_sense_data_fixed instead of struct
scsi_sense_data. This should be changed later to use
scsi_set_sense_data().
ciss.c: Calculate the sense residual properly. Use
scsi_get_sense_key() to fetch the sense key.
mps_sas.c,
mpt_cam.c: Set the sense residual properly.
iir.c: Use scsi_set_sense_data() instead of building sense data by
hand.
iscsi_subr.c: Use scsi_extract_sense_len() instead of grabbing sense data
directly.
umass.c: Use scsi_set_sense_data() to build sense data.
Grab the sense key using scsi_get_sense_key().
Calculate the sense residual properly.
isp_freebsd.h: Use scsi_get_*() routines to grab asc, ascq, and sense key
values.
Calculate and set the sense residual.
MFC after: 3 days
Sponsored by: Spectra Logic Corporation
- Ignore some more internal SAS device status change events.
- Correct inverted Bus and TargetID arguments in a warning.
o Add a warning for MPI_EVENT_SAS_DISCOVERY_ERROR events, which can help
identifying broken disks.
Submitted by: Andrew Boyer
Approved by: re (kib)
Committed from: Chaos Communication Camp 2011
the right SIM in case the HBA is RAID-capable but the target in question is
not a hot spare or member of a RAID volume.
- Report the loss and addition of SAS and SATA targets detected via PHY link
status changes and signalled by MPI_EVENT_SAS_DEVICE_STATUS_CHANGE to cam(4)
as lost devices and trigger rescans as appropriate. Without this it can take
quite some time until a lost device actually is no longer tried to be used,
if it ever stops. [1]
- Handle MPI_EVENT_IR2, MPI_EVENT_LOG_ENTRY_ADDED, MPI_EVENT_SAS_DISCOVERY
and MPI_EVENT_SAS_PHY_LINK_STATUS silently as these serve no additional
purpose beyond adding cryptic entries to logs.
Thanks to Hans-Joerg Sirtl for providing one of the HBAs these changes were
developed with and RIP to the mainboard that didn't survive testing them.
PR: 157534 [1]
Approved by: re (kib)
MFC after: 2 weeks
- Sprinkle some const where appropriate.
- Consistently use target_id_t for the target parameter of mpt_map_physdisk()
and mpt_is_raid_volume().
- Fix some whitespace bugs.
Approved by: re (kib)
coherent.
- Add some missing bus_dmamap_sync() calls. This includes putting such
calls before calling reply handlers instead of calling bus_dmamap_sync()
for the request queue from individual reply handlers as these handlers
generally read back updates by the controller.
Tested on amd64 and sparc64.
MFC after: 2 weeks
The mpt driver previously didn't report a 'maxio' size to CAM, and so the
da(4) driver limited I/O sizes to DFLTPHYS (64K) by default. The number
of scatter gather segments allowed, as reported to busdma, was
(128K / PAGE_SIZE) + 1, or 33 on architectures with 4K pages.
Change things around so that we wait until we've determined how many
segments the adapter can support before creating the busdma tag used for
buffers, so we can potentially support more S/G segments and therefore
larger I/O sizes.
Also, fix some things that were broken about the module unload path. It
still gets hung up inside CAM, though.
mpt.c: Move some busdma initialization calls in here, and call
them just after we've gotten the IOCFacts, and know how
many S/G segments this adapter can support.
mpt.h: Get rid of MPT_MAXPHYS, it is no longer used.
Add max_cam_seg_cnt, which is used to report our maximum
I/O size up to CAM.
mpt_cam.c: Use max_cam_seg_cnt to report our maximum I/O size to CAM.
Fix the locking in mpt_cam_detach().
mpt_pci.c: Pull some busdma initialization and teardown out and put
it in mpt.c. We now delay it until we know many scatter
gather segments the adapter can support, and therefore
how to setup our busdma tags.
mpt_raid.c: Make sure we wake up the right wait channel to get the
raid thread to wake up when we're trying to shut it down.
Reviewed by: gibbs, mjacob
MFC after: 2 weeks
the race where interrupt thread can complete the request for which
timeout has fired and while mpt_timeout has blocked on mpt_lock.
Do a best effort to keep 4.x ang Giant-locked configurartions
compiling still.
Reported by: ups
Reviewed by: scottl
specifically SPI controllers now also work in big-endian
machines and some conversions relevant for FC and SAS
controllers as well as support for ILP32 machines which all
were omitted in previous attempts are now also implemented.
The IOCTL-interface is intentionally left (and where needed
actually changed) to be completely little-endian as otherwise
we would have to add conversion code for every possible
configuration page to mpt(4), which didn't seem the right
thing to do, neither did converting only half of the user-
interface to the native byte order.
This change was tested on amd64 (SAS+SPI), i386 (SAS) and
sparc64 (SAS+SPI). Due to lack of the necessary hardware
the target mode code is still left to be made endian-clean.
Reviewed by: scottl
MFC after: 1 month
to kproc_xxx as they actually make whole processes.
Thos makes way for us to add REAL kthread_create() and friends
that actually make theads. it turns out that most of these
calls actually end up being moved back to the thread version
when it's added. but we need to make this cosmetic change first.
I'd LOVE to do this rename in 7.0 so that we can eventually MFC the
new kthread_xxx() calls.
