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: 28fefc441e ("ufshci: add a control device node")
Sponsored by: Samsung Electronics
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
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
UPIU header, and a header that declares more than was copied in would
reach past the descriptor. The ioctl layer copies output back only on
a zero return. So a command that reached the device is a success even
when it was refused. The caller reads the reason from the response
header, and an answerless failure comes back as EIO. Clear the
response before use so no stale bytes read as a device answer.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59559
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
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
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
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)
----+--------------+--------------+--------------+-------------
1 | 1357 | 1221 | 12.1 | 27.2
4 | 3103 | 3234 | 46.5 | 92.9
32 | 3508 | 3238 | 176.4 | 125.0
Sequential writes land in the WriteBooster buffer. Sustained
writes drop to 556 MiB/s once the buffer runs out.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59298
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
cam_sim_free() with free_devq set already frees the devq, so the
following cam_simq_free() call on the xpt_bus_register() and
xpt_create_path() failure paths was a double free. Also clear
ctrlr->ufshci_sim so a later ufshci_sim_detach() does not operate on
the freed SIM.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58662
The hardware queue and ucd_bus_addr allocations use M_NOWAIT but were
used without a NULL check, and the payload bus_dmamap_create() return
value was ignored, so a failed allocation was only discovered by
faulting on it later. Fail the construction instead. The teardown
path handles the partially constructed queue.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58661
When attach fails, ufshci_req_sdb_destroy() runs on a partially
constructed queue, and it runs twice: once from the construct error
path and once from the controller destructor.
Make that safe: NULL-check each resource before freeing it and clear
the pointer afterwards, so a second call finds nothing to do. The
construct error label no longer frees the command descriptors itself,
which fixes a double free of ucd_bus_addr. Also destroy the payload
DMA tag, which was previously leaked. Drop the mtx_initialized()
checks: the locks are always set up before any failure path can reach
the destroy.
Attach can also fail before the queues were constructed at all. The
destructor would then call a NULL qops.destroy pointer, so skip the
destroy when the queue was never set up.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58660
The bus_dmamap_load() error paths passed hwq->utrd and req_queue->ucd
to bus_dmamem_free(), but both pointers are only assigned after a
successful load and are still NULL at that point. The freshly
allocated memory was leaked. Free the local buffer instead.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58659
The driver stored the WLUN periph pointer without holding a reference,
so the pointer went stale when the pass(4) device went away. In
addition, ufshci_sim_send_ssu() released a reference that it had never
acquired.
Define a simple ownership rule. ufshci_sim_find_periph() acquires the
periph and returns it. The cache owns one reference. The controller
destructor drops it with cam_periph_release() before taking the SIM
lock, since the release takes the CAM device lock by itself.
ufshci_sim_send_ssu() acquires its own reference and releases it when
done. Reuse the cached periph instead of searching again, so the old
reference is not leaked.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58658
ufshci_sim_find_periph() freed the lookup path only when it found the
periph. The timeout path returned without freeing it and leaked the
path. Free the path at the single exit instead.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58657
ufshchi_sim_scsiio() did not check the M_NOWAIT request allocation
for NULL. The CDB validation and submit failure paths also returned
without freeing the request.
Fail the CCB when the allocation returns NULL. Free the request on
every failure path. Mark the CCB as queued right before the submit,
so the failure paths above do not need to touch that flag.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58656
Return submission errors from the controller command helpers and
propagate them to polled callers before waiting for completion. Free
requests that never enter a hardware queue so failure paths do not leak
or panic after the poll timeout.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58655
ufshci_uic_send_cmd() only logged the error code and returned success,
so a failed DME_GET gave its caller a stale value as if it were valid.
The gear and lane settings could then be programmed from that garbage.
Return ENXIO for reads instead. Writes keep logging and continuing,
because a device may reject an optional attribute and that must not
fail bring-up.
Sponsored by: Samsung Electronics
Reviewed by imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58654
When bus_dmamap_load_mem() failed, ufshci_req_queue_prepare_prdt()
manually completed and released the tracker, but its caller kept going:
it built the UTRD, set the slot back to SCHEDULED, and rang the
doorbell for a tracker whose request had already been freed. Return the
mapping error and stop the submission so the released tracker is not
resurrected.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58653
The data_direction field in the UTP Transfer Request Descriptor is only
2 bits wide ([26:25]). UFSHCI_DATA_DIRECTION_FROM_TGT_TO_SYS was defined
as 0x10, which truncates to 0b00 (No data transfer) when stored into the
2-bit field, so every read command was described to the controller as
having no data phase. Only writes (0b01) happened to be encoded
correctly.
Define all values as 2-bit binary literals, matching the existing
RESERVED = 0b11 entry, so read is encoded as 0b10 as required by the
specification.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D58652
Supports UFS host controller attachment via ACPI. Tested on the
Samsung Galaxy Book 4 Edge using Qualcomm Snapdragon X Elite.
Additionally, a quirk related to power mode change has been added.
For reference, it doesn't reach maximum speed yet. I plan to improve
it later.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D55986
Adds parameters related to the performance of the UFS device.
Also update man page for the missing sysctl entries.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D55985
When enabling the request queue, safely reset the list base address.
This was added due to a quirk in the Qualcomm UFS controller during
the process of activating it.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D55984
This patch removes the UIC error caused by QEMU not supporting certain
UIC command. Additionally, it removes the unused unipro_version.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D54513
Revert the argument change that broke libcam in 8c35de49 and move
power_condition support to scsi_start_stop_pc().
Reported by: imp
Reviewed By: #cam, imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D54822
Also, This patch fixes an issue where the admin command flag was not
passed through.
Sponsored by: Samsung Electronic
Reviewed by: imp
Differential Revision: https://reviews.freebsd.org/D54244
Fix a kernel panic caused by the task management queue using
the transfer request queue’s num_entries value.
Sponsored by: Samsung Electronic
Reviewed by: imp
Differential Revision: https://reviews.freebsd.org/D54243
Alder Lake-N supports Auto Hibernate, but it does not work.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D54006
Automatically transition the UniPro link to Hibernate when it is idle
for the duration configured by the Auto-Hibernate Idle Timer (AHIT).
This reduces link power while the device is inactive.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D54004
Handle system power events and issue START STOP UNIT (SSU) to the
UFS Device WLUN (0x50).
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D54002
This patch checks whether wlun is registered as a periph device.
It also implements a function to issue an SSU.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D53923
This patch adds a power_condition parameter to the
scsi_start_stop() function and sets the power condition via SSU.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronic
Differential Revision: https://reviews.freebsd.org/D53922
This patch adds an additional state to probe well-known logical units
before probing normal logical units.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D53920
This patch revisits the controller reset path and introduces timeout
handling for I/O commands.
To support controller reset during driver operation, the controller’s
construct, destruct, enable, and disable functions are clearly
separated in ufshci_ctrlr.c. ufshci_ctrlr_hw_reset() function is
added to leverage enable/disable.
After initialization, ufshci_ctrlr_reset_task() is also introduced to
ensure controller resets are performed via the task queue.
Timeout handling is designed in five steps. This patch implements
Step 1 and Step 5, while the remaining steps will be added later.
The timeout mechanism follows the same shared timeout model used in
the NVMe driver.
Test: Intentionally delayed UPIU I/O in QEMU to trigger a timeout and
verify timeout handling.
Sponsored by: Samsung Electronics
Reviewed by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D52440
Add myself (jaeyoon@) as a new src committer with imp@ as my mentor.
Reviewed by: imp (mentor)
Approved by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D52570
Jaeyoon Choi