- 10 Aug, 2021 8 commits
-
-
Colin Ian King authored
The variable ret is being initialized with a value that is never read, the assignment is redundant and can be removed. Link: https://lore.kernel.org/r/20210806112313.12434-1-colin.king@canonical.comSigned-off-by:
Colin Ian King <colin.king@canonical.com> Signed-off-by:
Martin K. Petersen <martin.petersen@oracle.com> Addresses-Coverity: ("Unused value")
-
Adrian Hunter authored
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: b294ff3e ("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:
-
Colin Ian King authored
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: 4728ab4a ("scsi: ufs: Remove ufshcd_valid_tag()") Reviewed-by:
Bart Van Assche <bvanassche@acm.org> Signed-off-by:
-
Damien Le Moal authored
Similarly to AHCI, introduce the device sysfs attribute sas_ncq_prio_supported to advertise if a SATA device supports the NCQ priority feature. Without this new attribute, the user can only discover if a SATA device supports NCQ priority by trying to enable the feature use with the sas_ncq_prio_enable sysfs device attribute, which fails when the device does not support high prioity commands. Link: https://lore.kernel.org/r/20210807041859.579409-11-damien.lemoal@wdc.comReviewed-by:
Hannes Reinecke <hare@suse.de> Signed-off-by:
Damien Le Moal <damien.lemoal@wdc.com> Signed-off-by:
-
Suganath Prabu S authored
Update driver version to 39.100.00.00. Link: https://lore.kernel.org/r/20210809072639.21228-3-suganath-prabu.subramani@broadcom.comSigned-off-by:
Suganath Prabu S <suganath-prabu.subramani@broadcom.com> Signed-off-by:
-
Suganath Prabu S authored
Currently, the mpt3sas driver sets the default queue depth based on the physical interface of the attached device: - SAS : 254 - SATA: 32 - NVMe: 128 The IOC firmware provides a recommended queue depth for each device through SAS IO Unit Page1 for SAS/SATA and PCIe IO Unit Page 1 for NVMe devices. If the host sets the queue depth greater than the firmware recommended value, then the IOC places the I/Os above the recommended queue depth in an internal pending queue. This consumes outstanding host-credit/resources, thereby leading to potential starvation of other devices. To avoid this, use the device depth recommended by the IOC firmware. Link: https://lore.kernel.org/r/20210809072639.21228-2-suganath-prabu.subramani@broadcom.comSigned-off-by:
-
Sreekanth Reddy authored
Bump driver version to 38.100.00.00. Link: https://lore.kernel.org/r/20210803065134.19090-1-sreekanth.reddy@broadcom.comReviewed-by:
Lee Duncan <lduncan@suse.com> Signed-off-by:
Sreekanth Reddy <sreekanth.reddy@broadcom.com> Signed-off-by:
-
Sreekanth Reddy authored
Enable the driver to work in non-IRQ mode, i.e. there will not be any MSI-X vectors associated with queues dedicated to polling. The IOC hardware is single submission queue and multiple reply queue. However, using the shared host tagset support it is possible to simulate multiple hardware queues. When poll_queues are enabled through the module parameter, the driver will allocate extra reply queues without an MSI-X association. All I/O completion on these queues will be done through the iopoll interface. Link: https://lore.kernel.org/r/20210727081212.2742-1-sreekanth.reddy@broadcom.comSigned-off-by:
-
- 06 Aug, 2021 5 commits
-
-
Bean Huo authored
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.comTested-by:
Avri Altman <avri.altman@wdc.com> Reviewed-by:
Bean Huo <beanhuo@micron.com> Signed-off-by:
-
Bean Huo authored
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.comReviewed-by:
-
Bean Huo authored
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.comReviewed-by:
-
Colin Ian King authored
The variable num_cnt 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/20210804131344.112635-1-colin.king@canonical.comReviewed-by:
Himanshu Madhani <himanshu.madhani@oracle.com> Signed-off-by:
-
Dan Carpenter authored
The sp->free(sp); call frees "sp" and then the debug code dereferences it on the next line. Swap the order. Link: https://lore.kernel.org/r/20210803155625.GA22735@kili Fixes: 84318a9f ("scsi: qla2xxx: edif: Add send, receive, and accept for auth_els") Reviewed-by:
Ewan D. Milne <emilne@redhat.com> Reviewed-by:
Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by:
-
- 03 Aug, 2021 22 commits
-
-
David Disseldorp authored
These members are only used for ALUA sense detail propagation, which can just as easily be done via sense_reason_t. Link: https://lore.kernel.org/r/20210728115353.2396-4-ddiss@suse.deReviewed-by:
Mike Christie <michael.christie@oracle.com> Signed-off-by:
David Disseldorp <ddiss@suse.de> Signed-off-by:
-
David Disseldorp authored
The se_cmd scsi_asc and scsi_ascq members are only used for tracking ALUA SCSI sense detail between target_core_alua and translate_sense_reason(), so they're effectively always zero here. Link: https://lore.kernel.org/r/20210728115353.2396-3-ddiss@suse.de Cc: Chris Boot <bootc@bootc.net> Reviewed-by:
-
David Disseldorp authored
core_alua_state_lba_dependent() currently uses lun->lun_tg_pt_gp without holding the lun_tg_pt_gp_lock. The lock is taken in the caller, so obtain the needed tg_pt_gp_id there instead. Link: https://lore.kernel.org/r/20210728115353.2396-2-ddiss@suse.de Cc: Hannes Reinecke <hare@suse.de> Reviewed-by:
-
Bodo Stroesser authored
When running command pipelining for WRITE direction commands (e.g. tape device write), userspace sends cmd completion to cmd ring before processing write data. In that case userspace has to copy data before sending completion, because cmd completion also implicitly releases the data buffer in data area. The new feature KEEP_BUF allows userspace to optionally keep the buffer after completion by setting new bit TCMU_UFLAG_KEEP_BUF in tcmu_cmd_entry_hdr->uflags. In that case buffer has to be released explicitly by writing the cmd_id to new action item free_kept_buf. All kept buffers are released during reset_ring and if userspace closes uio device (tcmu_release). Link: https://lore.kernel.org/r/20210713175021.20103-1-bostroesser@gmail.comReviewed-by:
Bodo Stroesser <bostroesser@gmail.com> Signed-off-by:
-
Bart Van Assche authored
Make it easier to test the UFS error handler and abort handler. Link: https://lore.kernel.org/r/20210722033439.26550-19-bvanassche@acm.orgAcked-by:
-
Bart Van Assche authored
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:
-
Bart Van Assche authored
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:
-
Bart Van Assche authored
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:
-
Bart Van Assche authored
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: 7a7e66c6 ("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:
-
Bart Van Assche authored
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:
-
Bart Van Assche authored
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: a45f9371 ("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:
-
Bart Van Assche authored
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 6f715172. 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:
-
Bart Van Assche authored
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:
-
Bart Van Assche authored
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: -
Bart Van Assche authored
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:
Keoseong Park <keosung.park@samsung.com> Reviewed-by:
-
Bart Van Assche authored
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:
-
Bart Van Assche authored
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 14497328 ("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:
-
Bart Van Assche authored
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:
-
Bart Van Assche authored
Rename the second argument of ufshcd_probe_hba() such that the name of that argument reflects its purpose instead of how the function is called. See also commit 1b9e2141 ("scsi: ufs: Split ufshcd_probe_hba() based on its called flow"). Link: https://lore.kernel.org/r/20210722033439.26550-5-bvanassche@acm.org Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Can Guo <cang@codeaurora.org> Reviewed-by:
Stanley Chu <stanley.chu@mediatek.com> Signed-off-by:
-
Bart Van Assche authored
This patch slightly reduces the UFS driver size if built with power management support disabled. Link: https://lore.kernel.org/r/20210722033439.26550-4-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:
-
Bart Van Assche authored
Move the dev_get_drvdata() calls into the ufshcd_{system,runtime}_*() functions. Remove ufshcd_runtime_idle() since it is empty. This patch does not change any functionality. Link: https://lore.kernel.org/r/20210722033439.26550-3-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: -
Bart Van Assche authored
If param_offset > buff_len then the memcpy() statement in ufshcd_read_desc_param() corrupts memory since it copies 256 + buff_len - param_offset bytes into a buffer with size buff_len. Since param_offset < 256 this results in writing past the bound of the output buffer. Link: https://lore.kernel.org/r/20210722033439.26550-2-bvanassche@acm.org Fixes: cbe193f6 ("scsi: ufs: Fix potential NULL pointer access during memcpy") Reviewed-by:
-
- 01 Aug, 2021 5 commits
-
-
Avri Altman authored
Elaborate some more on the host control mode logic parameters, explaining what they do and how to configure them. Link: https://lore.kernel.org/r/20210712095039.8093-13-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
Support devices that report they are using host control mode. Link: https://lore.kernel.org/r/20210712095039.8093-12-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
HPB WRITE BUFFER with buffer-id = 0x3h is supported in device control mode only. Link: https://lore.kernel.org/r/20210712095039.8093-11-avri.altman@wdc.comSigned-off-by:
-
Avri Altman authored
In host control mode the host is the originator of map requests. To not flood the device with map requests, use a simple throttling mechanism that limits the number of in-flight map requests. Link: https://lore.kernel.org/r/20210712095039.8093-10-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
In order not to hang on to "cold" regions, we inactivate a region that has had no READ access for a predefined amount of time - READ_TO_MS. For that purpose monitor the active regions list, polling it on every POLLING_INTERVAL_MS. On timeout expiry add the region to the "to-be-inactivated" list unless it is clean and did not exhaust its READ_TO_EXPIRIES - another parameter. None of this applies to pinned regions. Link: https://lore.kernel.org/r/20210712095039.8093-9-avri.altman@wdc.comReviewed-by:
-
