| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
commit af21c3fd5b3ec540a97b367a70b26616ff7e0c55 upstream.
Commit 57d104c153d3 ("ufs: add UFS power management support") made the UFS
driver submit a REQUEST SENSE command before submitting a power management
command to a WLUN to clear the POWER ON unit attention. Instead of
submitting a REQUEST SENSE command before submitting a power management
command, retry the power management command until it succeeds.
This is the preparation to get rid of all UNIT ATTENTION code which should
be handled by users.
Link: https://lore.kernel.org/r/20211001182015.1347587-2-jaegeuk@kernel.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
commit 7dc9fb47bc9a95f1cc6c5655341860c5e50f91d4 upstream.
Add PCI ID and callbacks to support Intel Alder Lake.
Link: https://lore.kernel.org/r/20211124204218.1784559-1-adrian.hunter@intel.com
Cc: stable@vger.kernel.org # v5.15+
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
[ Upstream commit 5cb37a26355d79ab290220677b1b57d28e99a895 ]
hba->outstanding_tasks, which is read under host_lock spinlock, tells the
interrupt handler what task management tags are in use by the driver. The
doorbell register bits indicate which tags are in use by the hardware. A
doorbell bit that is 0 is because the bit has yet to be set by the driver,
or because the task is complete. It is only possible to disambiguate the 2
cases, if reading/writing the doorbell register is synchronized with
reading/writing hba->outstanding_tasks.
For that reason, reading REG_UTP_TASK_REQ_DOOR_BELL must be done under
spinlock.
Link: https://lore.kernel.org/r/20211108064815.569494-3-adrian.hunter@intel.com
Fixes: f5ef336fd2e4 ("scsi: ufs: core: Fix task management completion")
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 886fe2915cce6658b0fc19e64b82879325de61ea ]
__ufshcd_issue_tm_cmd() clears req->end_io_data after timing out, which
races with the completion function ufshcd_tmc_handler() which expects
req->end_io_data to have a value.
Note __ufshcd_issue_tm_cmd() and ufshcd_tmc_handler() are already
synchronized using hba->tmf_rqs and hba->outstanding_tasks under the
host_lock spinlock.
It is also not necessary (nor typical) to clear req->end_io_data because
the block layer does it before allocating out requests e.g. via
blk_get_request().
So fix by not clearing it.
Link: https://lore.kernel.org/r/20211108064815.569494-2-adrian.hunter@intel.com
Fixes: f5ef336fd2e4 ("scsi: ufs: core: Fix task management completion")
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 3ff1f6b6ba6f97f50862aa50e79959cc8ddc2566 ]
The following has been observed on a test setup:
WARNING: CPU: 4 PID: 250 at drivers/scsi/ufs/ufshcd.c:2737 ufshcd_queuecommand+0x468/0x65c
Call trace:
ufshcd_queuecommand+0x468/0x65c
scsi_send_eh_cmnd+0x224/0x6a0
scsi_eh_test_devices+0x248/0x418
scsi_eh_ready_devs+0xc34/0xe58
scsi_error_handler+0x204/0x80c
kthread+0x150/0x1b4
ret_from_fork+0x10/0x30
That warning is triggered by the following statement:
WARN_ON(lrbp->cmd);
Fix this warning by clearing lrbp->cmd from the abort handler.
Link: https://lore.kernel.org/r/20211104181059.4129537-1-bvanassche@acm.org
Fixes: 7a3e97b0dc4b ("[SCSI] ufshcd: UFS Host controller driver")
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 9ec5128a8b5631d652ed06b37e0166f337802f90 ]
The spec recommends that for transfer length larger than the max-single-cmd
attribute (bMAX_DATA_SIZE_FOR_HPB_SINGLE_CMD) it is possible to couple
pre-requests with the HPB-READ command. Being a recommendation, using
pre-requests can be perceived merely as a means of optimization. A common
practice was to send pre-requests for chunks within some interval, and
leave the READ10 untouched if larger.
Now that the pre-request flows have been removed, all the commands are
single commands. Properly handle this attribute and do not send HPB-READ
for transfer lengths larger than max-single-cmd.
[mkp: resolve conflict]
Fixes: 09d9e4d04187 ("scsi: ufs: ufshpb: Remove HPB2.0 flows")
Link: https://lore.kernel.org/r/20211031123654.17719-1-avri.altman@wdc.com
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 1da3b0141e74c18c2377d4c2655406a90a87742f ]
Calling ufshcd_rpm_{get/put}_sync() prior to ufshcd_scsi_add_wlus() being
called will trigger a NULL pointer dereference. This is because
hba->sdev_ufs_device is initialized in ufshcd_scsi_add_wlus().
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000348
Mem abort info:
ESR = 0x96000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004
CM = 0, WnR = 0
[0000000000000348] user address but active_mm is swapper
Internal error: Oops: 96000004 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 91 Comm: kworker/u16:1 Not tainted 5.15.0-rc1-beanhuo-linaro-1423
Hardware name: MicronRB (DT)
Workqueue: events_unbound async_run_entry_fn
pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : pm_runtime_drop_link+0x128/0x338
lr : ufshpb_get_dev_info+0x8c/0x148
sp : ffff800012573c10
x29: ffff800012573c10 x28: 0000000000000000 x27: 0000000000000003
x26: ffff000001d21298 x25: 000000005abcea60 x24: ffff800011d89000
x23: 0000000000000001 x22: ffff000001d21880 x21: ffff000001ec9300
x20: 0000000000000004 x19: 0000000000000198 x18: ffffffffffffffff
x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000041400
x14: 5eee00201100200a x13: 000000000000bb03 x12: 0000000000000000
x11: 0000000000000100 x10: 0200000000000000 x9 : bb0000021a162c01
x8 : 0302010021021003 x7 : 0000000000000000 x6 : ffff800012573af0
x5 : 0000000000000001 x4 : 0000000000000001 x3 : 0000000000000200
x2 : 0000000000000348 x1 : 0000000000000348 x0 : ffff80001095308c
Call trace:
pm_runtime_drop_link+0x128/0x338
ufshpb_get_dev_info+0x8c/0x148
ufshcd_probe_hba+0xda0/0x11b8
ufshcd_async_scan+0x34/0x330
async_run_entry_fn+0x38/0x180
process_one_work+0x1f4/0x498
worker_thread+0x48/0x480
kthread+0x140/0x158
ret_from_fork+0x10/0x20
Code: 88027c01 35ffffa2 17fff6c4 f9800051 (885f7c40)
---[ end trace 2ba541335f595c95 ]
ufshpb_get_dev_info() is only called during asynchronous scanning and at
that time pm_runtime_get_sync() has been called:
...
/* Hold auto suspend until async scan completes */
pm_runtime_get_sync(dev);
atomic_set(&hba->scsi_block_reqs_cnt, 0);
...
ufshcd_async_scan()
ufshcd_probe_hba(hba, true);
ufshcd_device_params_init(hba);
ufshpb_get_dev_info();
...
pm_runtime_put_sync(hba->dev);
Remove ufshcd_rpm_{get/put}_sync() from ufshpb_get_dev_info() to fix this
problem.
Link: https://lore.kernel.org/r/20210929200640.828611-2-huobean@gmail.com
Fixes: 351b3a849ac7 ("scsi: ufs: ufshpb: Use proper power management API")
Signed-off-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 351b3a849ac7d92449dc75c43db8a857b38387ea ]
In ufshpb, pm_runtime_{get,put}_sync() are used to avoid unwanted runtime
suspend during query requests. Whereas commit b294ff3e3449 ("scsi: ufs:
core: Enable power management for wlun") modified the driver core to use
ufshcd_rpm_{get,put}_sync() APIs.
Switch to these APIs in HPB module as well.
Link: https://lore.kernel.org/r/20210902003534epcms2p1937a0f0eeb48a441cb69f5ef13ff8430@epcms2p1
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit b6ca770ae7f2c560a29bbd02c4e3d734fafaf804 ]
UFS drivers that probe defer will end up leaking memory allocated for clk
and regulator names via kstrdup() because the structure that is holding
this memory is allocated via devm_* variants which will be freed during
probe defer but the names are never freed.
Use same devm_* variant of kstrdup to free the memory allocated to name
when driver probe defers.
Kmemleak found around 11 leaks on Qualcomm Dragon Board RB5:
unreferenced object 0xffff66f243fb2c00 (size 128):
comm "kworker/u16:0", pid 7, jiffies 4294893319 (age 94.848s)
hex dump (first 32 bytes):
63 6f 72 65 5f 63 6c 6b 00 76 69 72 74 75 61 6c core_clk.virtual
2f 77 6f 72 6b 71 75 65 75 65 2f 73 63 73 69 5f /workqueue/scsi_
backtrace:
[<000000006f788cd1>] slab_post_alloc_hook+0x88/0x410
[<00000000cfd1372b>] __kmalloc_track_caller+0x138/0x230
[<00000000a92ab17b>] kstrdup+0xb0/0x110
[<0000000037263ab6>] ufshcd_pltfrm_init+0x1a8/0x500
[<00000000a20a5caa>] ufs_qcom_probe+0x20/0x58
[<00000000a5e43067>] platform_probe+0x6c/0x118
[<00000000ef686e3f>] really_probe+0xc4/0x330
[<000000005b18792c>] __driver_probe_device+0x88/0x118
[<00000000a5d295e8>] driver_probe_device+0x44/0x158
[<000000007e83f58d>] __device_attach_driver+0xb4/0x128
[<000000004bfa4470>] bus_for_each_drv+0x68/0xd0
[<00000000b89a83bc>] __device_attach+0xec/0x170
[<00000000ada2beea>] device_initial_probe+0x14/0x20
[<0000000079921612>] bus_probe_device+0x9c/0xa8
[<00000000d268bf7c>] deferred_probe_work_func+0x90/0xd0
[<000000009ef64bfa>] process_one_work+0x29c/0x788
unreferenced object 0xffff66f243fb2c80 (size 128):
comm "kworker/u16:0", pid 7, jiffies 4294893319 (age 94.848s)
hex dump (first 32 bytes):
62 75 73 5f 61 67 67 72 5f 63 6c 6b 00 00 00 00 bus_aggr_clk....
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
With this patch no memory leaks are reported.
Link: https://lore.kernel.org/r/20210914092214.6468-1-srinivas.kandagatla@linaro.org
Fixes: aa4976130934 ("ufs: Add regulator enable support")
Fixes: c6e79dacd86f ("ufs: Add clock initialization support")
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit edc0596cc04bf0ac3a69c66e994d3ff8b650ff71 ]
Commit aa53f580e67b ("scsi: ufs: Minor adjustments to error handling")
introduced a ufshcd_clear_ua_wluns() call in
ufshcd_err_handling_unprepare(). As explained in detail by Adrian Hunter,
this can trigger a deadlock. Avoid that deadlock by removing the code that
clears the unit attention. This is safe because the only software that
relies on clearing unit attentions is the Android Trusty software and
because support for handling unit attentions has been added in the Trusty
software.
See also https://lore.kernel.org/linux-scsi/20210930124224.114031-2-adrian.hunter@intel.com/
Note that "scsi: ufs: Retry START_STOP on UNIT_ATTENTION" is a prerequisite
for this commit.
Link: https://lore.kernel.org/r/20211001182015.1347587-3-jaegeuk@kernel.org
Fixes: aa53f580e67b ("scsi: ufs: Minor adjustments to error handling")
Cc: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 68444d73d6a5864ede12df6366bd6602e022ae5b ]
Since commit 568dd9959611 ("scsi: ufs: Rename the second ufshcd_probe_hba()
argument"), the second ufshcd_probe_hba() argument has been changed to
init_dev_params.
Link: https://lore.kernel.org/r/20210929200640.828611-3-huobean@gmail.com
Fixes: 568dd9959611 ("scsi: ufs: Rename the second ufshcd_probe_hba() argument")
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
The Host Performance Buffer feature allows UFS read commands to carry the
physical media addresses along with the LBAs, thus allowing less internal
L2P-table switches in the device. HPB1.0 allowed a single LBA, while
HPB2.0 increases this capacity up to 255 blocks.
Carrying more than a single record, the read operation is no longer purely
of type "read" but a "hybrid" command: Writing the physical address to the
device in one operation and reading back the required payload in another.
The JEDEC HPB spec defines two commands for this operation:
HPB-WRITE-BUFFER (0x2) to write the physical addresses to device, and
HPB-READ to read the payload.
With the current HPB design the UFS driver has no alternative but to divide
the READ request into 2 separate commands: HPB-WRITE-BUFFER and HPB-READ.
This causes a great deal of aggravation to the block layer guys who
demanded that we completely revert the entire HPB driver regardless of the
huge amount of corporate effort already invested in it.
As a compromise, remove only the pieces that implement the 2.0
specification. This is done as a matter of urgency for the final 5.15
release.
Link: https://lore.kernel.org/r/20211030062301.248-1-avri.altman@wdc.com
Tested-by: Avri Altman <avri.altman@wdc.com>
Tested-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
PA_PWRMODEUSERDATA0 -> DL_FC0PROTTIMEOUTVAL
PA_PWRMODEUSERDATA1 -> DL_TC0REPLAYTIMEOUTVAL
PA_PWRMODEUSERDATA2 -> DL_AFC0REQTIMEOUTVAL
Link: https://lore.kernel.org/r/20211018062841.18226-1-chanho61.park@samsung.com
Fixes: a967ddb22d94 ("scsi: ufs: ufs-exynos: Apply vendor-specific values for three timeouts")
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Kiwoong Kim <kwmad.kim@samsung.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Chanho Park <chanho61.park@samsung.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Implement the ->restore() PM operation and set the link to off, which will
force a full reset and restore. This ensures that Host Performance Booster
is reset after suspend-to-disk.
The Host Performance Booster feature caches logical-to-physical mapping
information in the host memory. After suspend-to-disk, such information is
not valid, so a full reset and restore is needed.
A full reset and restore is done if the SPM level is 5 or 6, but not for
other SPM levels, so this change fixes those cases.
A full reset and restore also restores base address registers, so that code
is removed.
Link: https://lore.kernel.org/r/20211018151004.284200-2-adrian.hunter@intel.com
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
The UFS driver uses blk_mq_tagset_busy_iter() when identifying task
management requests to complete, however blk_mq_tagset_busy_iter() doesn't
work.
blk_mq_tagset_busy_iter() only iterates requests dispatched by the block
layer. That appears as if it might have started since commit 37f4a24c2469
("blk-mq: centralise related handling into blk_mq_get_driver_tag") which
removed 'data->hctx->tags->rqs[rq->tag] = rq' from blk_mq_rq_ctx_init()
which gets called:
blk_get_request
blk_mq_alloc_request
__blk_mq_alloc_request
blk_mq_rq_ctx_init
Since UFS task management requests are not dispatched by the block layer,
hctx->tags->rqs[rq->tag] remains NULL, and since blk_mq_tagset_busy_iter()
relies on finding requests using hctx->tags->rqs[rq->tag], UFS task
management requests are never found by blk_mq_tagset_busy_iter().
By using blk_mq_tagset_busy_iter(), the UFS driver was relying on internal
details of the block layer, which was fragile and subsequently got
broken. Fix by removing the use of blk_mq_tagset_busy_iter() and having the
driver keep track of task management requests.
Link: https://lore.kernel.org/r/20210922091059.4040-1-adrian.hunter@intel.com
Fixes: 1235fc569e0b ("scsi: ufs: core: Fix task management request completion timeout")
Fixes: 69a6c269c097 ("scsi: ufs: Use blk_{get,put}_request() to allocate and free TMFs")
Cc: stable@vger.kernel.org
Tested-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Fix incorrect index for UTMRD reference in ufshcd_add_tm_upiu_trace().
Link: https://lore.kernel.org/r/20210924085848.25500-1-jonathan.hsu@mediatek.com
Fixes: 4b42d557a8ad ("scsi: ufs: core: Fix wrong Task Tag used in task management request UPIUs")
Cc: stable@vger.kernel.org
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jonathan Hsu <jonathan.hsu@mediatek.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
A command tag is passed as the second argument of the
__ufshcd_transfer_req_compl() call in ufshcd_eh_device_reset_handler()
instead of a bitmask. Fix this by passing a bitmask as argument instead of
a command tag.
Link: https://lore.kernel.org/r/20210916175408.2260084-1-bvanassche@acm.org
Fixes: a45f937110fa ("scsi: ufs: Optimize host lock on transfer requests send/compl paths")
Cc: Can Guo <cang@codeaurora.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
This reverts commit a113eaaf86373362b053279049907ff82b5df6c8.
There are a couple of issues with the commit:
1. It causes deadlocks.
2. It causes the shost->eh_cmd_q list of failed requests not to be
processed, ever.
So revert it.
1. Deadlocks
The SCSI error handler runs with requests blocked beginning when
scsi_schedule_eh() sets SHOST_RECOVERY state, continuing through
scsi_error_handler() callback ->eh_strategy_handler() until
scsi_restart_operations() is called. By setting eh_strategy_handler to
ufshcd_err_handler, the patch changed the UFS error handler to run with
requests blocked, including PM requests, for the entire run of the error
handler.
That conflicts with UFS error handler existing synchronization with UFS
device PM operations. The UFS error handler synchronizes with runtime PM
by doing pm_runtime_get_sync() prior to blocking requests itself. It
synchronizes with system PM by use of hba->host_sem, again before blocking
requests itself. However, if requests are already blocked, then PM
operations will block. So:
the UFS error handler blocks waiting on PM
+ PM blocks waiting on SCSI PM requests to process or fail
+ PM requests are blocked waiting on error handling to finish
= deadlock
This happens both for runtime PM and system PM.
Prior to the patch, these deadlocks could not happen even if SCSI error
handling was running, because the presence of requests in shost->eh_cmd_q
would mean the queues could not be suspended, which would mean that, should
the UFS error handler run at the same time, it would not need to wait for
PM or vice versa.
Please note these scenarios are not just theoretical, they were found
during testing on a Samsung Galaxy Book S.
2. ->eh_strategy_handler() must process shost->eh_cmd_q list of failed
requests, as all other eh_strategy_handler's do except UFS error handler.
Refer for example: scsi_unjam_host(), ata_scsi_error() and
sas_scsi_recover_host().
Link: https://lore.kernel.org/r/20210917144349.14058-1-adrian.hunter@intel.com
Fixes: a113eaaf8637 ("scsi: ufs: Synchronize SCSI and UFS error handling")
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Intel LKF can experience link errors. Make fixes to increase link
stability, especially when switching to high speed modes.
Link: https://lore.kernel.org/r/20210831145317.26306-1-adrian.hunter@intel.com
Fixes: b2c57925df1f ("scsi: ufs: ufs-pci: Add support for Intel LKF")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
The following parameters are not used in the function. Remove them.
*func(): ufshpb_set_hpb_read_to_upiu
-> struct ufshpb_lu *hpb
-> u32 lpn
Link: https://lore.kernel.org/r/20210901025617.31174-1-cw9316.lee@samsung.com
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: ChanWoo Lee <cw9316.lee@samsung.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Samsung KLUFG8RHDA-B2D1 does not clear the unit attention condition if the
length is zero. So go back to requesting all the sense data, as it was
before patch "scsi: ufs: Request sense data asynchronously". That is
simpler than creating and maintaining a quirk for affected devices.
Link: https://lore.kernel.org/r/20210824114150.2105-1-adrian.hunter@intel.com
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
clk_get_rate() returns unsigned long and currently this driver stores the
return value in u32 type, resulting the below warning:
Fixed smatch warnings:
drivers/scsi/ufs/ufs-exynos.c:286 exynos_ufs_get_clk_info()
warn: wrong type for 'ufs->mclk_rate' (should be 'ulong')
drivers/scsi/ufs/ufs-exynos.c:287 exynos_ufs_get_clk_info()
warn: wrong type for 'pclk_rate' (should be 'ulong')
Link: https://lore.kernel.org/r/20210819171131.55912-1-alim.akhtar@samsung.com
Fixes: 55f4b1f73631 ("scsi: ufs: ufs-exynos: Add UFS host support for Exynos SoCs")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Change "allcation" to "allocation".
Link: https://lore.kernel.org/r/1891546521.01629282781634.JavaMail.epsvc@epcpadp4
Signed-off-by: Keoseong Park <keosung.park@samsung.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
When HPB pinned region exists and mctx allocation for this region fails, a
memory leak is possible because memory is not released for the subregion
table of the current region.
Free memory for the subregion table of the current region.
Link: https://lore.kernel.org/r/1891546521.01629711601304.JavaMail.epsvc@epcpadp3
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Keoseong Park <keosung.park@samsung.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Prepare for removal of the request pointer by using scsi_cmd_to_rq()
instead. This patch does not change any functionality.
Link: https://lore.kernel.org/r/20210809230355.8186-1-bvanassche@acm.org
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Prepare for removal of the request pointer by using scsi_cmd_to_rq()
instead. This patch does not change any functionality.
Link: https://lore.kernel.org/r/20210809230355.8186-48-bvanassche@acm.org
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
In host control mode, eviction is perceived as an extreme measure. There
are several conditions that both the entering and exiting regions should
meet, so that eviction will take place.
The common case however, is that those conditions are rarely met, so it is
normal that the act of eviction fails. Therefore, do not report an error
in host control mode if eviction fails.
Link: https://lore.kernel.org/r/20210808090024.21721-5-avri.altman@wdc.com
Fixes: 6c59cb501b86 (scsi: ufs: ufshpb: Make eviction depend on region's reads)
Signed-off-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
'num_inflight_map_req' should not be negative. It is incremented and
decremented without any protection, allowing it theoretically to be
negative, should some weird unbalanced count occur.
Verify that the those calls are properly serialized.
Link: https://lore.kernel.org/r/20210808090024.21721-4-avri.altman@wdc.com
Fixes: 33845a2d844b (scsi: ufs: ufshpb: Limit the number of in-flight map requests)
Signed-off-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
In HPB2.0, if pre_req_min_tr_len < transfer_len < pre_req_max_tr_len, the
driver is expected to send a HPB-WRITE-BUFFER companion to HPB-READ.
The upper bound should fit into a single byte, regardless of bMAX_
DATA_SIZE_FOR_HPB_SINGLE_CMD which being an attribute (u32) can be
significantly larger.
To further illustrate the issue, consider the following scenario:
- SCSI_DEFAULT_MAX_SECTORS is 1024 limiting the I/O chunks to 512KB
- The OEM changes scsi_host_template .max_sectors to be 2048 which allows
for 1MB requests: transfer_len = 256
- pre_req_max_tr_len = HPB_MULTI_CHUNK_HIGH = 256
- ufshpb_is_supported_chunk() returns true (256 <= 256)
- WARN_ON_ONCE(256 > 256) doesn't warn
- ufshpb_set_hpb_read_to_upiu() casts transfer_len to u8: transfer_len = 0
- The command is failing with ILLEGAL REQUEST
Link: https://lore.kernel.org/r/20210808090024.21721-3-avri.altman@wdc.com
Fixes: 41d8a9333cc9 (scsi: ufs: ufshpb: Add HPB 2.0 support)
Signed-off-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
The purpose of the "cold"-timer is not to hang-on to active regions with no
reads. Therefore the read timeout should be rewound on every read, and not
just when the region is activated.
Link: https://lore.kernel.org/r/20210808090024.21721-2-avri.altman@wdc.com
Fixes: 13c044e91678 (scsi: ufs: ufshpb: Add "cold" regions timer)
Signed-off-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
The variable 'lba' is being initialized with a value that is never read, it
is being updated later on. The assignment is redundant and can be removed.
Link: https://lore.kernel.org/r/20210804133241.113509-1-colin.king@canonical.com
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Addresses-Coverity: ("Unused value")
|
|
Managed device links are deleted by device_del(). However it is possible to
add a device link to a consumer before device_add(), and then discovering
an error prevents the device from being used. In that case normally
references to the device would be dropped and the device would be deleted.
However the device link holds a reference to the device, so the device link
and device remain indefinitely (unless the supplier is deleted).
For UFSHCD, if a LUN fails to probe (e.g. absent BOOT WLUN), the device
will not have been registered but can still have a device link holding a
reference to the device. The unwanted device link will prevent runtime
suspend indefinitely.
Amend device link removal to accept removal of a link with an unregistered
consumer device (suggested by Rafael), and fix UFSHCD by explicitly
deleting the device link when SCSI destroys the SCSI device.
Link: https://lore.kernel.org/r/a1c9bac8-b560-b662-f0aa-58c7e000cbbd@intel.com
Fixes: b294ff3e3449 ("scsi: ufs: core: Enable power management for wlun")
Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Variable 'tag' is currently an unsigned int and is being compared to less
than zero, this check is always false. Fix this by making 'tag' an int.
Link: https://lore.kernel.org/r/20210806144301.19864-1-colin.king@canonical.com
Fixes: 4728ab4a8e64 ("scsi: ufs: Remove ufshcd_valid_tag()")
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Addresses-Coverity: ("Macro compares unsigned to 0")
|
|
We need to check whether HPB is enabled on a given LU from the userspace
tool. Add lu_enable sysfs node.
Link: https://lore.kernel.org/r/20210804182128.458356-3-huobean@gmail.com
Tested-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
For Micron UFS devices the L2P entry need to be byteswapped before sending
an HPB READ command to the UFS device. Add the quirk
UFS_DEVICE_QUIRK_SWAP_L2P_ENTRY_FOR_HPB_READ to address this.
Link: https://lore.kernel.org/r/20210804182128.458356-2-huobean@gmail.com
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
ufshcd_add_cmd_upiu_trace() will be called later anyway. Simplify code by
moving if-statement.
Link: https://lore.kernel.org/r/20210802180803.100033-1-huobean@gmail.com
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Make it easier to test the UFS error handler and abort handler.
Link: https://lore.kernel.org/r/20210722033439.26550-19-bvanassche@acm.org
Acked-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Neither SAM nor the UFS standard require that the UFS controller fills in
the completion status of commands that have been aborted (LUN RESET aborts
pending commands). Hence do not rely on the completion status provided by
the UFS controller for aborted commands but instead ask the SCSI core to
retry SCSI commands that have been aborted.
Link: https://lore.kernel.org/r/20210722033439.26550-18-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Use the SCSI error handler instead of a custom error handling strategy.
This change reduces the number of potential races in the UFS drivers since
the UFS error handler and the SCSI error handler no longer run
concurrently.
Link: https://lore.kernel.org/r/20210722033439.26550-17-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Tested-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Clearing a unit attention synchronously from inside the UFS error handler
may trigger the following deadlock:
- ufshcd_err_handler() calls ufshcd_err_handling_unprepare() and the
latter function calls ufshcd_clear_ua_wluns().
- ufshcd_clear_ua_wluns() submits a REQUEST SENSE command and that command
activates the SCSI error handler.
- The SCSI error handler calls ufshcd_host_reset_and_restore().
- ufshcd_host_reset_and_restore() executes the following code:
ufshcd_schedule_eh_work(hba); flush_work(&hba->eh_work);
This sequence results in a deadlock (circular wait). Fix this by requesting
sense data asynchronously.
Link: https://lore.kernel.org/r/20210722033439.26550-16-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Make the following changes in ufshcd_abort():
- Return FAILED instead of SUCCESS if the abort handler notices that a
SCSI command has already been completed. Returning SUCCESS in this case
triggers a use-after-free and may trigger a kernel crash.
- Fix the code for aborting SCSI commands submitted to a WLUN.
The current approach for aborting SCSI commands that have been submitted to
a WLUN and that timed out is as follows:
- Report to the SCSI core that the command has completed successfully.
Let the block layer free any data buffers associated with the command.
- Mark the command as outstanding in 'outstanding_reqs'.
- If the block layer tries to reuse the tag associated with the aborted
command, busy-wait until the tag is freed.
This approach can result in:
- Memory corruption if the controller accesses the data buffer after the
block layer has freed the associated data buffers.
- A race condition if ufshcd_queuecommand() or ufshcd_exec_dev_cmd()
checks the bit that corresponds to an aborted command in
'outstanding_reqs' after it has been cleared and before it is reset.
- High energy consumption if ufshcd_queuecommand() repeatedly returns
SCSI_MLQUEUE_HOST_BUSY.
Fix this by reporting to the SCSI error handler that aborting a SCSI
command failed if the SCSI command was submitted to a WLUN.
Link: https://lore.kernel.org/r/20210722033439.26550-15-bvanassche@acm.org
Fixes: 7a7e66c65d41 ("scsi: ufs: Fix a race condition between ufshcd_abort() and eh_work()")
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Use a spinlock to protect hba->outstanding_reqs instead of using atomic
operations to update this member variable.
This patch is a performance improvement because it reduces the number of
atomic operations in the hot path (test_and_clear_bit()) and because it
reduces the lock contention on the SCSI host lock. On my test setup this
patch improves IOPS by about 1%.
Link: https://lore.kernel.org/r/20210722033439.26550-14-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Reduce the number of times the host lock is taken in the hot path.
Additionally, inline ufshcd_vops_setup_xfer_req() because that function is
too short to keep it.
Link: https://lore.kernel.org/r/20210722033439.26550-13-bvanassche@acm.org
Fixes: a45f937110fa ("scsi: ufs: Optimize host lock on transfer requests send/compl paths")
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Bean Huo <beanhuo@micron.com>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Register"
Using the UTRLCNR register involves two MMIO accesses in the hot path while
using the doorbell register only involves a single MMIO access. Since MMIO
accesses take time, do not use the UTRLCNR register. The spinlock
contention on the SCSI host lock that is reintroduced by this commit will
be addressed later.
This reverts commit 6f7151729647e58ac7c522081255fd0c07b38105.
Link: https://lore.kernel.org/r/20210722033439.26550-12-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Tested-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Inline ufshcd_outstanding_req_clear() since it only has one caller and
since its body is only one line long.
Link: https://lore.kernel.org/r/20210722033439.26550-11-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
From arch/arm/include/asm/io.h
#define __iowmb() wmb()
[ ... ]
#define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); })
From Documentation/memory-barriers.txt: "Note that, when using writel(), a
prior wmb() is not needed to guarantee that the cache coherent memory
writes have completed before writing to the MMIO region."
In other words, calling wmb() before writel() is not necessary. Hence
remove the wmb() calls that precede a writel() call. Remove the wmb() calls
that precede a ufshcd_send_command() call since the latter function uses
writel(). Remove the wmb() call from ufshcd_wait_for_dev_cmd() since the
following chain of events guarantees that the CPU will see up-to-date LRB
values:
- UFS controller writes to host memory.
- UFS controller posts completion interrupt after the memory writes from
the previous step are visible to the CPU.
- complete(hba->dev_cmd.complete) is called from the UFS interrupt handler.
- The wait_for_completion(hba->dev_cmd.complete) call in
ufshcd_wait_for_dev_cmd() returns.
Link: https://lore.kernel.org/r/20210722033439.26550-10-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Tested-by: Avri altman <avri.altman@wdc.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Assign a name to the enumeration type for UFS host controller states and
remove the default clause from switch statements on this enumeration type
to make the compiler warn about unhandled enumeration labels.
Link: https://lore.kernel.org/r/20210722033439.26550-9-bvanassche@acm.org
Cc: Can Guo <cang@codeaurora.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Keoseong Park <keosung.park@samsung.com>
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
Instead of documenting the locking requirements of the UIC code as
comments, use lockdep_assert_held() such that lockdep verifies the lockdep
requirements at runtime if lockdep is enabled.
Link: https://lore.kernel.org/r/20210722033439.26550-8-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
scsi_add_host() allocates shost->can_queue tags. ufshcd_init() sets
shost->can_queue to hba->nutrs. In other words, we know that tag values
will less than hba->nutrs. Hence remove the checks that verify that
blk_get_request() returns a tag less than hba->nutrs. This check was
introduced by commit 14497328b6a6 ("scsi: ufs: verify command tag
validity").
Keep the tag >= 0 check because it helps to detect use-after-free issues.
Link: https://lore.kernel.org/r/20210722033439.26550-7-bvanassche@acm.org
CC: Avri Altman <avri.altman@wdc.com>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
|
From Documentation/scheduler/completion.rst: "When a completion is declared
as a local variable within a function, then the initialization should
always use DECLARE_COMPLETION_ONSTACK() explicitly, not just to make
lockdep happy, but also to make it clear that limited scope had been
considered and is intentional."
Link: https://lore.kernel.org/r/20210722033439.26550-6-bvanassche@acm.org
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
|
