Commit Graph

2368 Commits

Author SHA1 Message Date
Alexander Motin
ec18cf79e6 Remove session locking from cfiscsi_pdu_update_cmdsn().
cs_cmdsn can be incremented with single atomic.  expcmdsn/maxcmdsn set in
cfiscsi_pdu_prepare() based on cs_cmdsn are not required to be updated
synchronously, only monotonically, that is achieved with lock there.

MFC after:	2 weeks
Sponsored by:	iXsystems, Inc.
2020-05-29 17:52:20 +00:00
Alexander Motin
dbcf7598b0 Report STATUS_QUEUED/SENT in ctladm dumpooa output.
MFC after:	2 weeks
Sponsored by:	iXsystems, Inc.
2020-05-29 13:07:52 +00:00
Alexander Motin
353c460050 Move EXPDATASN/R2TSN from PDU to CTL_PRIV_FRONTEND.
We any way have per-I/O space in CTL_PRIV_FRONTEND, while for PDU private
fields I have better use ideas.  Plus to me such use of PDU fields looked
a layering violation.

MFC after:	2 weeks
Sponsored by:	iXsystems, Inc.
2020-05-29 02:32:48 +00:00
Alexander Motin
30a31f6c71 Remove PDU_TOTAL_TRANSFER_LEN() macro.
I don't see a point to copy io->scsiio.kern_total_len into the request
PDU private field.  The io is going to stay with us till the end, and
kern_total_len field is not changed after being first initialized.

MFC after:	2 weeks
Sponsored by:	iXsystems, Inc.
2020-05-28 23:55:46 +00:00
Alexander Motin
767300e87a Make struct ctl_be_lun first element of struct ctl_be_*_lun.
It allows to remove some extra pointer dereferences and slightly tightens
up the code by unification.

MFC after:	2 weeks
Sponsored by:	iXsystems, Inc.
2020-05-28 21:30:29 +00:00
Alexander Motin
0d7fed74c7 Remove ctl_free_beio() LUN and ctl_io dependencies.
This slightly simplifies the code, plus may be a ground for asynchronous
buffer free.

MFC after:	2 weeks
Sponsored by:	iXsystems, Inc.
2020-05-28 18:12:05 +00:00
Adrian Chadd
69c41b7071 [ata_da] remove duplicate definition; it trips up ye olde gcc-6 on mips32
Checked first with: irc
2020-05-27 02:10:09 +00:00
Alexander Motin
2bbcd07e39 Properly check kern_sg_entries for S/G list.
ctl_data_print() is called in core context, so does not even know meaning
of ext_sg_entries.

MFC after:	1 week
Sponsored by:	iXsystems, Inc.
2020-05-26 19:09:19 +00:00
Alexander Motin
3873b14991 Fix fallout of r319722 in CTL HA.
ha_lso is a listening socket (unless bind() has failed), so should use
solisten_upcall_set(NULL, NULL), not soupcall_clear().

MFC after:	1 week
Sponsored by:	iXsystems, Inc.
2020-05-26 15:08:35 +00:00
Alexander Motin
fd10265cd2 Do not remove upcall if we haven't yet.
This fixes assertion if we failed to bind listening HA socket.

MFC after:	1 week
Sponsored by:	iXsystems, Inc.
2020-05-26 13:57:14 +00:00
Conrad Meyer
9982b3ee29 cam: ANSIfy 0-argument function definitions
No functional change.

Reviewed by:	imp
Differential Revision:	https://reviews.freebsd.org/D24854
2020-05-16 14:33:08 +00:00
Warner Losh
fd26063f4a Add nvd alias back to nda now that it actually works. 2020-05-13 19:17:35 +00:00
Warner Losh
0f280cbd0a Make the ata probe* and xpt* routines aprobe* and axpt* respectively.
Often, in traiging core files, one only has a traceback of where a
panic occurred. We have probe* and xpt* routines that live in both the
scsi and ata layers with identical names. To make one or the other
stand out, prefix all the probe and xpt routines in ata with an
'a'. I've left the scsi ones alone since they were there first and are
more numerous. I also rejected using #define to do this as being too
confusing. I chose this method because the CAM name for the probe
device was already 'aprobe'.

Normally, this doesn't matter because file scope protects one from
interfering with the other. However, due to the indirect nature of
CAM's state machine, you don't know if the following traceback is
SCSI or ATA:
	xpt_done
	probedone
	xpt_done_process
	xpt_done_td
	fork_exit

nvme and mmc already have unique names.

MFC: 1 week
Differential revision: https://reviews.freebsd.org/D24825
2020-05-13 00:18:44 +00:00
John Baldwin
f721fcbccb Fix a memory leak for ENCIOC_GETSTRING I introduced in r360171.
MFC after:	1 week
Sponsored by:	DARPA
2020-05-08 16:41:23 +00:00
Alexander Motin
8a1cd3cee3 Add session locking in cfiscsi_ioctl_handoff().
While there, remove ifdef around cs_target check in cfiscsi_ioctl_list().
I am not sure why this ifdef was added, but without this check code will
crash below on NULL dereference.

Submitted by:	Aleksandr Fedorov <aleksandr.fedorov@itglobal.com>
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D24587
2020-05-03 16:14:55 +00:00
Warner Losh
4f397ed21e We need to hold the periph lock when we release the ccb (and when we
run it). Make sure that we do. Simplify the flow a bit, and fix a
comment since we do need to do these things.

Noticed by: cperciva (not sure why my invariants kernel didn't trigger)
2020-05-03 04:22:27 +00:00
Alexander Motin
34144c2c71 Cleanup LUN addition/removal.
- Make ctl_add_lun() synchronous.  Asynchronous addition was used by
Copan's proprietary code long ago and never for upstream FreeBSD.
 - Move LUN enable/disable calls from backends to CTL core.
 - Serialize LUN modification and partially removal to avoid double frees.
 - Slightly unify backends code.

MFC after:	2 weeks
Sponsored by:	iXsystems, Inc.
2020-05-02 16:54:59 +00:00
Warner Losh
2d76f4aad4 Fix three bugs highlighted by review:
- maxio should be dp->d_maxsize. This is often MAXPHYS, but not always
  (especially if MAXPHYS is > 1MB).
- Unlock the periph before returning. We don't need to relock it to
  release the ccb.
- Make sure we release the ccb in error paths.

Reviewed by: cperciva
2020-04-30 21:08:59 +00:00
Warner Losh
e8a7873da5 Remove stray blank line.
This was accidentally added in r360483.
2020-04-30 19:48:55 +00:00
Warner Losh
2446ce7a63 Implement the NVME_GET_NSID and NVME_PASSTHROUGH_CMD ioctls
With these two ioctls implemented in the nda driver, nvmecontrol now
works with nda just like it does with nvd. It eliminates the need to
jump through odd hoops to get this data.
2020-04-30 00:43:07 +00:00
Warner Losh
027d061296 Return the nvmeX device associated with the ndaX device.
Add the nvmeX device to the XPT_PATH_INQ nvme specific
information. while one could figure this out by looking up the
domain🚌slot:function, it's a lot easier to have the SIM set it
directly since the sim knows this.
2020-04-30 00:43:02 +00:00
Warner Losh
75ce42271a Export the nda device's flags as a sysctl. 2020-04-27 23:43:17 +00:00
Warner Losh
96eb32bf0f Convert rotating to a flag bit.
Move rotating to a flag bit. Add bit definitions for it. Create a
compat sysctl for it.
2020-04-27 23:43:12 +00:00
Warner Losh
cf3ff63e55 Convert unmappedio over to a flag.
Make unmappedio a flag. Move it to the flags definition. Add compat
sysctl for it.
2020-04-27 23:43:08 +00:00
Warner Losh
aeab0812e6 Add flags sysctl to ada
Report the ada device flags like we do the da devices. No booleans
have (yet) been converted, but iomapped and rotating are planned.
2020-04-27 23:43:04 +00:00
Warner Losh
72f8ed61a7 Change the flags back to an enum
This was changed in the review process for the flags sysctl. The
reasons for the change are no longer valid as the code changed after
that. Cast the one place where it might make a difference (but I don't
think it does).  This restores the ability to see flags for softc in
gdb.
2020-04-27 23:39:32 +00:00
John Baldwin
81b62a7663 Don't pass a user buffer pointer as the data pointer in a CCB.
Allocate a temporary buffer in the kernel to serve as the CCB data
pointer for a pass-through transaction and use copyin/copyout to
shuffle the data to/from the user buffer.

Reviewed by:	scottl, brooks
Obtained from:	CheriBSD
MFC after:	2 weeks
Sponsored by:	DARPA
Differential Revision:	https://reviews.freebsd.org/D24489
2020-04-21 23:38:54 +00:00
John Baldwin
47e735aded Don't access a user buffer directly from the kernel.
The handle_string callback for the ENCIOC_SETSTRING ioctl was passing
a user pointer to memcpy().  Fix by using copyin() instead.

For ENCIOC_GETSTRING ioctls, the handler was storing the user pointer
in a CCB's data_ptr field where it was indirected by other code.  Fix
this by allocating a temporary buffer (which ENCIOC_SETSTRING already
did) and copying the result out to the user buffer after the CCB has
been processed.

Reviewed by:	kib
Obtained from:	CheriBSD
MFC after:	1 week
Sponsored by:	DARPA
Differential Revision:	https://reviews.freebsd.org/D24487
2020-04-21 17:47:05 +00:00
John Baldwin
5326289d93 Don't try to copyout() to a kernel buffer.
The handle_string callback for the ENCIOC_GET_ENCNAME and
ENCIOC_GETENCID ioctls tries to copy the size of the generated string
out to userland.  However, the callback only has access to the kernel
copy of the structure populated by copyin().  The copyout() call
simply overwrites the value in the kernel's copy preventing the
subsequent overflow prevention logic from working.

Fix this by instead doing a copyout() of the updated length in the
caller after the callback returns.

Reviewed by:	kib
Obtained from:	CheriBSD
Sponsored by:	DARPA
Differential Revision:	https://reviews.freebsd.org/D24456
2020-04-17 18:19:13 +00:00
Warner Losh
ab485b018a Checks here against useracc are not useful and are racy.
copyin/copyout are sufficient to guard against bad addresses. They will return
EFAULT if the user is up to no good (by choice or ignorance). There's no point
in checking, since it doesn't even improve the error messages.

Noticed by: jhb
Reviewed by: brooks, jhb
2020-04-13 21:04:33 +00:00
Warner Losh
450a2e2a12 Remove stale comment
There's no useracc here, and even if there was it shouldn't be here. vmapbuf is
sufficient and as the comment says, useracc is racy.
2020-04-13 21:03:30 +00:00
Warner Losh
9cf738228d Now that we don't have special-case geom hacking defined in md_var.h, stop
including it. sparc64 was the last straggler here, but these weren't removed at
the time.
2020-04-07 22:23:22 +00:00
Alexander Motin
efeedddcb5 Fix panic on kern.cam.ctl.ha_role change after r333446.
MFC after:	1 week
Sponsored by:	iXsystems, Inc.
2020-04-07 03:19:00 +00:00
Alexander Motin
12fec4c35d Relax too strict SES element descriptors check in r355430.
SES specifications allows the string to be NULL-terminated, while previous
code was considering it as invalid due to incorrectly ordered conditions.

MFC after:	 1 week
Sponsored by:	iXsystem, Inc.
2020-04-06 18:42:01 +00:00
Ed Maste
3709674072 sys/cam: remove doubled ;s 2020-03-20 16:15:45 +00:00
Alexander Motin
e4ee6c5444 Fix SES on device slots without phys after r349321.
Broadcom 9400-8i8e HBAs report virtual SES device, where slots representing
external connectors are reported having no phys.  Since sasdev_phys is NULL
there and proto_hdr is a union, ses_paths_iter() misinterpreted them as ATA.
Add explicit protocol check to properly differentiate them.

MFC after:	1 week
Sponsored by:	iXsystems, Inc.
2020-03-19 17:20:50 +00:00
David Bright
35c5ccf66d Fix parameter reversal potentially causing buffer overrun.
Two arguments were reversed in calls to cam_strvis() in
nvme_da.c. This was found by a Coverity scan of this code within Dell
(Isilon). These are also marked in the FreeBSD Coverity scan as CIDs
1400526 & 1400531.

Submitted by:	robert.herndon@dell.com
Reviewed by:	vangyzen@, imp@
MFC after:	3 days
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D24117
2020-03-19 12:22:20 +00:00
Warner Losh
81490eda60 Add comment about how the deferred callback for AC_FOUND_DEVICE we
generate for a race where a device goes away, we start to tear down
the periph state for the device, and then the device suddently
reappears. The key that makes it work is removal of periph from the
drv list before calling the deferred callback.

Hat tip to: mav@
2020-03-14 02:36:45 +00:00
Warner Losh
7defd7582e Give some indication of the CCB that's in flight when we panic.
Print the pointer to ccb so we can find it (for what good it does)
as well as the type of operation in flight when the cam_path has
been freed out from under us. This helps both core analysis as well
as automated systems that collect panic strings but little else.
2020-03-13 18:45:36 +00:00
Warner Losh
e6db0f0409 Remove redundantly repetitive static __inline forward function
declarations.

We typically don't use them elsewhere in the kernel, and they aren't
needed here: the actual functions are a few lines away and aren't
mutually recursive.
2020-03-11 15:12:31 +00:00
Warner Losh
5870223ddc Eliminate camq_alloc() and camq_free()
These are no longer needed now that it's embedded in cam_ccbq. They are also
unused.

Reviewed by: ken, chuck
Differential Revision:	https://reviews.freebsd.org/D24008
2020-03-11 00:00:03 +00:00
Warner Losh
3dfb13c1cd Eliminate xpt_copy_path.
It's used in exactly one place. In that place it's used so we can hold the lock
on the device associated with the path (since we do a xpt_path_lock and unlock
pair around the callback). Instead, inline taking and dropping the reference to
the device so we can ensure we can unlock the mutex after the callback finishes
if the path in the ccb that's queued to be processed by xpt_scanner_thread is
destroyed while being processed. We don't actually need the path itself for
anything other than dereferencing it to get the device to do the lock and
unlock.

This also makes the locking / use model for cam_path a little cleaner by
eliminating a case where we needlessly copy the object.

Reviewed by: chuck, chs, ken
Differential Revision:	https://reviews.freebsd.org/D24008
2020-03-10 23:59:58 +00:00
Warner Losh
9ac30e0b66 Remove unused cam ccb flags
These flags have been unused for some time. Some of them were in the
CAM2 specification, but CAM has moved on a bit from that. Some were
used in the old Pluto VideoSpace (and AirSpace) systems which had the
video playback I/O scheduler in userspace, but have been unused since
then.

Reviewed by: chuck, ken
Differential Revision:  https://reviews.freebsd.org/D24008
2020-03-10 23:58:41 +00:00
Warner Losh
6fda2c54da Reword a comment to describe what's actually going on. We can call invalidate
several times potentially. We just don't do anything on the second and
subsequent calls.
2020-03-07 00:29:12 +00:00
Warner Losh
1247272e41 All paths lead to xpt_done, so move it after the switch. 2020-03-05 06:21:00 +00:00
Warner Losh
56eccd2d06 xpt_async is submitting a CCB, not finishing it up, so use xpt_action() instead
of xpt_done(). Add the missing XPT_ASYNC case to xpt_action_default. xpt_async
wants to use the side-effect of the xpt_done() routine to queue this to the
camisr thread so it can be done in that context. However, this breaks the
symmetry that you create a ccb and call xpt_action() for it to be
dispatched. Restore that symmetry by having it go through that path. As far as I
can tell, this is the only CCB that we create and call xpt_done() on directly.
2020-03-05 06:20:17 +00:00
Warner Losh
91b685872c Get rid of silly /* FALLTHROUGH */ lines
Consistently omit /* FALLTHROUGH */ when we have a case statement that does
nothing. Since compilers don't warn about stacked case statements, and we were
inconsistent, resolve by removing extras.
2020-03-03 17:40:29 +00:00
Warner Losh
51447e4962 Remove pre-FreeBSD 11 compat code. 2020-03-01 23:01:47 +00:00
Warner Losh
a248d7c2f5 This is a FALLTHROUGH for sure. no need for xxx comment. 2020-02-28 01:18:11 +00:00
Scott Long
ecca2aa545 Add a quirk for the WDC Green series of SSDs to disable NCQ TRIM, as this
avoids silent data corruption.

PR:		225666
Submitted by:	anders lundgren
MFC after:	3 days
2020-02-27 05:00:21 +00:00