bhyveload: validate character disk devices
Currently, bhyveload(8) does not validate the supplied disk
image path. For example, it allows passing the /dev/null
device, which later fails in userboot because it does not
support DIOCGSECTORSIZE and DIOCGMEDIASIZE ioctls (see
userdisk_init() in stand/userboot/userboot/userboot_disk.c).
Fix that by checking DIOCGSECTORSIZE and DIOCGMEDIASIZE ioctls early.
A similar check already exists in bhyve(8). While here, make
cb_diskioctl() report the obtained sector size instead of
hard-coding 512.
Reviewed by: markj
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D59253
(cherry picked from commit 51234535ddd6ec0afe9dd4e3f34a31b92d5cdd78)
libcasper: tolerate kernels without PD_NOWAITPID
Commit 1a296762b3d0 made libcasper pass PD_NOWAITPID to pdfork(2).
Kernels predating the flag (15.1 and earlier, since the flag first
ships in 15.2) reject it with EINVAL, which makes cap_init() and every
service fork fail when a newer world runs on an older kernel, for
example in a poudriere jail.
Add casper_pdfork(), which retries without the flag on EINVAL, and use
it at both pdfork(2) call sites. The retry is safe because the kernel
validates pdfork flags before creating a child. On such kernels the
zombie must still be reaped with waitpid(2), as before the flag was
introduced.
The fallback is compiled out once __FreeBSD_version reaches 1700000,
so it disappears from main when stable/16 branches while remaining in
the stable/15 and stable/16 branches that need it.
Reviewed by: kib
[3 lines not shown]
ufshci: build the ioctl file into the kernel
The passthrough ioctl went into the module build only. A kernel with
device ufshci then failed to link, because ufshci_ctrlr.c calls
ufshci_ioctl_construct() and ufshci_ioctl_destruct() and neither was
compiled in.
Add the file to sys/conf/files.
Fixes: 28fefc441e3b ("ufshci: add a control device node")
Sponsored by: Samsung Electronics
(cherry picked from commit d435248a2196bc78f626850a7ae700aaeace1c13)
ufshci: add a passthrough ioctl
This ioctl is for a port of ufs-utils:
https://github.com/SanDisk-Open-Source/ufs-utils
The driver only exposed a CAM SIM. Reading a descriptor, an attribute
or a flag needs a query request, and a UniPro attribute needs a DME
command. The driver built both only for its own setup, so userland
could reach neither.
Add two ioctls on the control node. UFSHCI_PASSTHROUGH_CMD sends a
UPIU the caller built, sizes the request from its transaction code,
and copies the response UPIU back. UFSHCI_PASSTHROUGH_UIC carries the
four attribute commands and refuses the rest, which can drop the link
or power the device off. It keeps the raw argument2 so the caller can
read the result code the device reported, not just a failure.
Validate the input and bound it by what the controller can map. The
descriptor has no request length, so the controller reads it from the
[12 lines not shown]
ufshci: add a control device node
The driver only exposed a CAM SIM. Userland had no way to reach the
device for anything that is not a SCSI command, so reading a descriptor
or an attribute was impossible.
Add /dev/ufshci%d as a root only node and the ioctl ABI header for it.
The node answers no ioctl yet. The header pulls in ufshci.h, which
declares bool only under _KERNEL, so include stdbool.h for userland the
way nvme.h already does.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59558
(cherry picked from commit 28fefc441e3b701acc2888892a518774394255c7)
ufshci: install the ioctl header
The passthrough ioctl has a userland ABI header, but nothing installed
it. A program that wanted to use the ioctl had to copy the headers out
of the source tree by hand.
Install ufshci.h and ufshci_ioctl.h under /usr/include/dev/ufshci, the
way nvme installs nvme.h. The ioctl header pulls in ufshci.h, so both
go. Add the directory to the include mtree so installworld creates it.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59560
(cherry picked from commit 8895b1348f3d21e2db16a06f31d555be0471e36d)
ufshci: tell the controller how long the EHS is
The transfer request descriptor has a field for the total Extra Header
Segment length. The driver left it at zero. A request that carried an
EHS went out as the bare command UPIU, and the device answered a request
it had only seen part of.
Fill the field from the request UPIU header, which already carries the
same length. Every other path sets it to zero, so nothing else changes.
An EHS is the first thing that makes a request vary in size, so assert
that the request and the response still fit in the command descriptor.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59557
(cherry picked from commit d3e5082ce4dcb154cbf50cba05d8f1dbbd55a5fc)
ufshci: skip the reinit when the new link works
UFSHCI_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH always rebuilt the
link after the gear switch. It threw away a working HS link and
ended up in PWM. The reinit is only needed for a dead link.
There the local side reports HS and the peer never answers. A
local readback cannot tell the two apart. Peer traffic can.
Probe the peer with DME_PEER_GET after the switch. Skip the
reinit when the probe succeeds. Log it when the probe fails.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59299
(cherry picked from commit 9930150214d1ca4ad21561d2e0afce9ebf2cdf6c)
ufshci: set HS series per platform and adapt type per gear
The driver always asked for Rate-B. It never set the adaptation
type. The Snapdragon X Elite firmware tunes the PHY for Rate-A.
A Rate-B link dies at every gear there. HS-G4 and above need
initial adaptation. This is a UniPro rule. It applies to
every host.
Add an hs_series field to the device tables. Use Rate-A on the
Snapdragon X Elite. Keep Rate-B on the PCI hosts. A table entry
without an HS series fails to attach. Set PA_TxHsAdaptType to
initial adaptation at HS-G4 and above. Leave it alone below
that. Hosts before UniPro 1.8 do not have it. The Galaxy Book
4 Edge now links at HS-G5 Rate-A.
fio results (128k sequential, 4k random, posixaio):
QD | SEQ_R(MiB/s) | SEQ_W(MiB/s) | RND_R(kIOPS) | RND_W(kIOPS)
----+--------------+--------------+--------------+-------------
[12 lines not shown]
ufshci: fix the Snapdragon X Elite reference clock
The driver's ACPI table set bRefClkFreq to 19.2 MHz. The
Snapdragon X Elite feeds the device 38.4 MHz from its CXO. The
firmware has no property for it. The device ran its PLL from
the wrong base. Every HS mode failed. PWM still worked. The
attribute is persistent. The wrong value survived reboots.
Set 38.4 MHz in the table. Read the attribute first. Write it
only when the value differs or the read fails. Log a changed
value and a failed read. Verified on the Galaxy Book 4 Edge.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59297
(cherry picked from commit 973783515e7db6e19550c57c8f9d94d907e3bd0e)
ufshci: handle a recovery reset before the SIM attach
When the first start attempt fails early, the recovery reset
runs the start sequence again without a SIM. That pass still
looked up the WLUN, so it dereferenced a NULL SIM and panicked.
Attach the SIM whenever it does not exist yet. Also make the
WLUN lookup return NULL when there is no SIM.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58947
(cherry picked from commit e59d4ec66fcab3da813f593f52bb8f1577cbb403)
ufshci: reject new requests on a failed controller
A failed controller accepted new requests, but nothing ever
completed them, so the caller waited forever. The admin retry
path could also resubmit a request to a dead queue.
Reject new submits and admin retries on a failed controller.
The submit check runs under the queue lock, so it cannot race
with the queue walk in the fail path.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58948
(cherry picked from commit 3ecee9314d88e2bb277b365d9413e219fd9a1283)
ufshci: build valid fake responses for manual completion
The manual completion wrote the fake response to the wrong
descriptor for task management slots. It also left the task tag
at zero, which tripped the task tag check under INVARIANTS.
Write the fake response where the completion path reads it.
Copy the task tag from the request.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58946
(cherry picked from commit 746278a6ddc80a98001f875cd975283d7c99b960)
ufshci: claim trackers before failing them
ufshci_req_queue_fail() drops the queue lock to complete each
tracker. In that window the completion path could complete the
same tracker again.
Claim the slot before dropping the lock, so the completion scan
skips it. Reserved slots are left to their submit thread, which
completes them itself. The manual request completion helper
lost its only caller, so drop it.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58945
(cherry picked from commit 7682b8f37669671844fedd94a382ec0c459b9b2c)
ufshci: run the controller fail path only once
Two threads could run ufshci_ctrlr_fail() at the same time.
Each one walked the queues and completed the same trackers
again, which caused a double free and a panic.
Turn is_failed into an atomic gate, so only the first caller
walks the queues. The reset task now returns early on a failed
controller instead of re-enabling it.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58944
(cherry picked from commit 1258ae2bcb559854c966e425f4c5c48668d3cf7b)
ufshci: report the highest LUN number in the path inquiry
cpi->max_lun is an inclusive upper bound, but the driver reported the
LUN count (8 or 32), so CAM probed one nonexistent LUN past the end.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58820
(cherry picked from commit 68091ebf0a72b58424e6b57d6ff9687af9043003)
ufshci: correct the crypto/config register offsets and HCMID fields
The reserved array after CCAP must be 508, but it was 511.
This pushed the config, MCQ config, and ESI registers from 0x300 and
0x380 up to 0x900. None of these registers are used yet, so nothing
broke.
Also fix the HCMID bank index field. The spec places it at bits
[23:16], but it was defined on top of the manufacturer code at [15:0].
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58819
(cherry picked from commit 34518292a3e077efe239b61f64b9d860bbfb2960)
ufshci: consolidate the device query submit/poll pattern
The six query helpers duplicated the same submit, error check, poll,
and status check sequence. Move it into ufshci_dev_send_query() so
future changes to the query flow are made in one place. This also
unifies the failure log message format.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58818
(cherry picked from commit e1aa173f812010675005e2917d7b2b44ee4a226e)
ufshci: pass the queue being destroyed to the cmd descriptor teardown
ufshci_req_sdb_destroy() hardcoded &ctrlr->transfer_req_queue when
destroying command descriptors instead of using its req_queue argument.
No functional change: the branch only runs for the transfer queue, so
the two pointers are always the same today. Using the argument keeps
the function queue-agnostic for when more transfer queues exist (MCQ).
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58816
(cherry picked from commit dbcebc7c661e961ea6e72ef77e29675aca86ab4b)
ufshci: validate the CDB before allocating a request
The CDB pointer and length checks depend only on the CCB, so perform
them before allocating and initializing the request. This avoids a
wasted allocation for invalid CCBs on the I/O path and removes one
request-free error path.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58817
(cherry picked from commit f3c3db35d545bb14b96e4cb079312c377247540b)
ufshci: return the real errno from SDB queue construction
ufshci_req_sdb_cmd_desc_construct() and ufshci_req_sdb_construct()
returned ENOMEM for every failure, so an EINVAL from bus_dma_tag_create()
was reported as a memory shortage. Capture and return the real errno,
and drop the cmd descriptor construct's now pointless out label.
No functional change: no caller inspects the value beyond propagating
it, so this only improves the diagnostics on an attach failure.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D58815
(cherry picked from commit a508a9f7238828fb5fb0f68e9f5df7d24072d6c2)
ufshci: do not reset the device in the XPT_RESET_DEV handler
CAM calls the SIM action callback with the SIM lock and the CAM
device lock held. The XPT_RESET_DEV handler called
ufshci_dev_reset(), which sleeps on device commands. Sleeping there
panics when another thread contends for the lock: "panic: sleeping
thread holds CAM device lock".
Report success without touching the device, as nvme_sim(4) does.
A real device reset needs the controller reset path. That rework is
planned together with in-flight request recovery.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58671
(cherry picked from commit a33860b0a2b98caf32c2ff62707f254ca92773f4)
ufshci: free the taskqueue on detach
ufshci_ctrlr_destruct() never freed the taskqueue. Every load and
unload cycle leaked the taskqueue and its kernel thread. A task that
was still queued could also run after the module was gone.
Free the taskqueue in destruct. Do it after the interrupt teardown
so nothing enqueues new work. A reset task that is still queued at
this point races the queue teardown. That race is older than this
change. The planned in-flight recovery rework will close it.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58670
(cherry picked from commit 2d32961316248fdff54a2a9fc24ac8b712fee9b0)
ufshci: release the CCB after sending a start stop unit command
ufshci_sim_send_ssu() got a CCB from cam_periph_getccb() but never
returned it. Each call leaked the CCB and one slot of the device's
CCB allocation budget. When the budget runs out, the next
cam_periph_getccb() waits forever and the suspend path hangs.
Release the CCB while the periph lock is still held, as the other CAM
periph drivers do.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58669
(cherry picked from commit aaf0e80e684da35cd527491243583467ae7c75cb)
ufshci: check completions under the queue lock
The completion scan held only the recovery lock. The submit path sets
a slot to SCHEDULED and then rings the doorbell, both under the queue
lock. A scan running between those two steps saw a SCHEDULED slot with
a clear doorbell and completed a command the device had not started.
The command failed with OCS 0xf, and a reused slot could return wrong
read data.
Check the slot state and the doorbell under the queue lock. The submit
path holds it across both steps, so a half-submitted slot can no
longer be seen. Found with fio randrw verify on QEMU.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58668
(cherry picked from commit c686e7d3b0d315c358be81b4a1151711213d998d)
ufshci: read UIC command results while holding the lock
The UIC result registers (UICCMDARG2/3) are only valid between a
command's completion and the next command's submission. They were read
after uic_cmd_lock was dropped, so a concurrent UIC submitter could
overwrite them in between. Read them into locals before releasing the
lock.
Also mask the generic error code to its [7:0] field when checking it,
so unrelated bits in UICCMDARG2 (such as the attribute set type echoed
for DME_SET) cannot be mistaken for an error.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58667
(cherry picked from commit 266ce89fc231d58431293cf5b77e0854440aa13d)
ufshci: initialize desc_size for non-descriptor query requests
The flag and attribute query builders left param.desc_size
uninitialized, so stack garbage was sent as the query UPIU length
field. Devices generally ignore the length for these opcodes, which
hid the bug. Zero it explicitly.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58665
(cherry picked from commit af00e00f23a1d427d96fecbb39e276fc2d7dfde1)
ufshci: byte-swap big-endian UPIU fields
The UPIU wire fields are big-endian. The task management and query
builders wrote host-order values into them. The completion paths also
read the results back without conversion. On a little-endian host an
ABORT_TASK carried a swapped task tag and LUN, a query carried a
swapped length, and attribute reads returned swapped values. Tolerant
devices masked most of the damage.
Convert with htobe*/be*toh at the wire boundary, as ufshci_sim.c
already does for its fields.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58664
(cherry picked from commit 50a00f10d830119ed54f5bce1b34533f3a09325e)
ufshci: initialize alloc_units before the dedicated-buffer scan
If every unit descriptor read failed in the LU-dedicated WriteBooster
scan, alloc_units was used uninitialized. Start it at zero so that case
is treated as a zero-sized buffer and WriteBooster is disabled.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58663
(cherry picked from commit 456ab423dddc0ce07787b21fcb79426abc7b436e)
