The branch main has been updated by jaeyoon: URL: https://cgit.FreeBSD.org/src/commit/?id=c84c2bf78b39058a339551117323668c933f6162
commit c84c2bf78b39058a339551117323668c933f6162 Author: Jaeyoon Choi <[email protected]> AuthorDate: 2026-08-10 01:39:12 +0000 Commit: Jaeyoon Choi <[email protected]> CommitDate: 2026-08-10 02:28:49 +0000 ufshci: fix WLUN periph reference counting 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 --- sys/dev/ufshci/ufshci_ctrlr.c | 2 ++ sys/dev/ufshci/ufshci_dev.c | 13 ++++++++----- sys/dev/ufshci/ufshci_private.h | 1 + sys/dev/ufshci/ufshci_sim.c | 36 +++++++++++++++++++++++++++++++----- 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/sys/dev/ufshci/ufshci_ctrlr.c b/sys/dev/ufshci/ufshci_ctrlr.c index cb5549f8e7c9..421aa1e28124 100644 --- a/sys/dev/ufshci/ufshci_ctrlr.c +++ b/sys/dev/ufshci/ufshci_ctrlr.c @@ -448,6 +448,8 @@ ufshci_ctrlr_destruct(struct ufshci_controller *ctrlr, device_t dev) bus_release_resource(ctrlr->dev, SYS_RES_IRQ, rman_get_rid(ctrlr->res), ctrlr->res); + ufshci_sim_release_wlun_periph(ctrlr); + mtx_lock(&ctrlr->sc_mtx); ufshci_sim_detach(ctrlr); diff --git a/sys/dev/ufshci/ufshci_dev.c b/sys/dev/ufshci/ufshci_dev.c index c53d9b71ba2c..7113834db13a 100644 --- a/sys/dev/ufshci/ufshci_dev.c +++ b/sys/dev/ufshci/ufshci_dev.c @@ -508,12 +508,15 @@ ufshci_dev_init_ufs_power_mode(struct ufshci_controller *ctrlr) if (ctrlr->quirks & UFSHCI_QUIRK_SKIP_WELL_KNOWN_LUNS) return (0); - ctrlr->ufs_device_wlun_periph = ufshci_sim_find_periph(ctrlr, - UFSHCI_WLUN_UFS_DEVICE); if (ctrlr->ufs_device_wlun_periph == NULL) { - ufshci_printf(ctrlr, - "Well-known LUN `UFS Device (0x50)` not found\n"); - return (0); + /* The returned reference is kept by the cached pointer. */ + ctrlr->ufs_device_wlun_periph = ufshci_sim_find_periph(ctrlr, + UFSHCI_WLUN_UFS_DEVICE); + if (ctrlr->ufs_device_wlun_periph == NULL) { + ufshci_printf(ctrlr, + "Well-known LUN `UFS Device (0x50)` not found\n"); + return (0); + } } ctrlr->ufs_dev.power_mode_supported = true; diff --git a/sys/dev/ufshci/ufshci_private.h b/sys/dev/ufshci/ufshci_private.h index d4c4140f82ae..d87651a46a2a 100644 --- a/sys/dev/ufshci/ufshci_private.h +++ b/sys/dev/ufshci/ufshci_private.h @@ -434,6 +434,7 @@ uint8_t ufshci_sim_translate_scsi_to_ufs_lun(lun_id_t scsi_lun); uint64_t ufshci_sim_translate_ufs_to_scsi_lun(uint8_t ufs_lun); int ufshci_sim_attach(struct ufshci_controller *ctrlr); void ufshci_sim_detach(struct ufshci_controller *ctrlr); +void ufshci_sim_release_wlun_periph(struct ufshci_controller *ctrlr); struct cam_periph *ufshci_sim_find_periph(struct ufshci_controller *ctrlr, uint8_t wlun); int ufshci_sim_send_ssu(struct ufshci_controller *ctrlr, bool start, diff --git a/sys/dev/ufshci/ufshci_sim.c b/sys/dev/ufshci/ufshci_sim.c index 148840123d13..9d80dd195ad9 100644 --- a/sys/dev/ufshci/ufshci_sim.c +++ b/sys/dev/ufshci/ufshci_sim.c @@ -393,6 +393,20 @@ ufshci_sim_attach(struct ufshci_controller *ctrlr) return (0); } +/* + * Drop the cached WLUN periph reference. cam_periph_release() takes the + * CAM device lock itself, so call this without sc_mtx held: CAM takes + * the device lock before the SIM lock, not the other way around. + */ +void +ufshci_sim_release_wlun_periph(struct ufshci_controller *ctrlr) +{ + if (ctrlr->ufs_device_wlun_periph != NULL) { + cam_periph_release(ctrlr->ufs_device_wlun_periph); + ctrlr->ufs_device_wlun_periph = NULL; + } +} + void ufshci_sim_detach(struct ufshci_controller *ctrlr) { @@ -425,6 +439,10 @@ ufshci_sim_detach(struct ufshci_controller *ctrlr) } } +/* + * On success this returns a referenced periph; the caller is responsible + * for dropping the reference with cam_periph_release(). + */ struct cam_periph * ufshci_sim_find_periph(struct ufshci_controller *ctrlr, uint8_t wlun) { @@ -446,6 +464,8 @@ ufshci_sim_find_periph(struct ufshci_controller *ctrlr, uint8_t wlun) while (1) { xpt_path_lock(path); periph = cam_periph_find(path, "pass"); + if (periph != NULL && cam_periph_acquire(periph) != 0) + periph = NULL; xpt_path_unlock(path); if (periph) @@ -474,17 +494,23 @@ ufshci_sim_send_ssu(struct ufshci_controller *ctrlr, bool start, union ccb *ccb; int err; - /* Acquire periph reference */ - if (periph && cam_periph_acquire(periph) != 0) { + /* Acquire a periph reference for the duration of this call. */ + if (periph != NULL && cam_periph_acquire(periph) != 0) { + /* The cached periph is going away; drop its reference. */ + cam_periph_release(periph); + ctrlr->ufs_device_wlun_periph = NULL; periph = NULL; } if (periph == NULL) { - /* If the periph device does not exist, it will try to find it - * again */ + /* + * If the periph device does not exist, try to find it again. + * The reference returned by ufshci_sim_find_periph() is used + * for this call; take an extra one for the cached pointer. + */ periph = ufshci_sim_find_periph(ctrlr, (uint8_t)UFSHCI_WLUN_UFS_DEVICE); - if (periph) + if (periph != NULL && cam_periph_acquire(periph) == 0) ctrlr->ufs_device_wlun_periph = periph; }
