#
872f86e1 |
| 01-Jul-2024 |
Igor Pylypiv <ipylypiv@google.com> |
ata: libata-scsi: Check ATA_QCFLAG_RTF_FILLED before using result_tf
[ Upstream commit 816be86c7993d3c5832c3017c0056297e86f978c ]
qc->result_tf contents are only valid when the ATA_QCFLAG_RTF_FILLE
ata: libata-scsi: Check ATA_QCFLAG_RTF_FILLED before using result_tf
[ Upstream commit 816be86c7993d3c5832c3017c0056297e86f978c ]
qc->result_tf contents are only valid when the ATA_QCFLAG_RTF_FILLED flag is set. The ATA_QCFLAG_RTF_FILLED flag should be always set for commands that failed or for commands that have the ATA_QCFLAG_RESULT_TF flag set.
Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Igor Pylypiv <ipylypiv@google.com> Link: https://lore.kernel.org/r/20240702024735.1152293-8-ipylypiv@google.com Signed-off-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
#
c8d4acb3 |
| 01-Jul-2024 |
Igor Pylypiv <ipylypiv@google.com> |
ata: libata-scsi: Remove redundant sense_buffer memsets
[ Upstream commit 3f6d903b54a137e9e438d9c3b774b5d0432917bc ]
SCSI layer clears sense_buffer in scsi_queue_rq() so there is no need for libata
ata: libata-scsi: Remove redundant sense_buffer memsets
[ Upstream commit 3f6d903b54a137e9e438d9c3b774b5d0432917bc ]
SCSI layer clears sense_buffer in scsi_queue_rq() so there is no need for libata to clear it again.
Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Igor Pylypiv <ipylypiv@google.com> Link: https://lore.kernel.org/r/20240702024735.1152293-5-ipylypiv@google.com Signed-off-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
#
88042e41 |
| 13-Aug-2024 |
Niklas Cassel <cassel@kernel.org> |
Revert "ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error"
commit fa0db8e568787c665384430eaf2221b299b85367 upstream.
This reverts commit 28ab9769117ca944cb6eb537af5599aa436287a4.
Revert "ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error"
commit fa0db8e568787c665384430eaf2221b299b85367 upstream.
This reverts commit 28ab9769117ca944cb6eb537af5599aa436287a4.
Sense data can be in either fixed format or descriptor format.
SAT-6 revision 1, "10.4.6 Control mode page", defines the D_SENSE bit: "The SATL shall support this bit as defined in SPC-5 with the following exception: if the D_ SENSE bit is set to zero (i.e., fixed format sense data), then the SATL should return fixed format sense data for ATA PASS-THROUGH commands."
The libata SATL has always kept D_SENSE set to zero by default. (It is however possible to change the value using a MODE SELECT SG_IO command.)
Failed ATA PASS-THROUGH commands correctly respected the D_SENSE bit, however, successful ATA PASS-THROUGH commands incorrectly returned the sense data in descriptor format (regardless of the D_SENSE bit).
Commit 28ab9769117c ("ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error") fixed this bug for successful ATA PASS-THROUGH commands.
However, after commit 28ab9769117c ("ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error"), there were bug reports that hdparm, hddtemp, and udisks were no longer working as expected.
These applications incorrectly assume the returned sense data is in descriptor format, without even looking at the RESPONSE CODE field in the returned sense data (to see which format the returned sense data is in).
Considering that there will be broken versions of these applications around roughly forever, we are stuck with being bug compatible with older kernels.
Cc: stable@vger.kernel.org # 4.19+ Reported-by: Stephan Eisvogel <eisvogel@seitics.de> Reported-by: Christian Heusel <christian@heusel.eu> Closes: https://lore.kernel.org/linux-ide/0bf3f2f0-0fc6-4ba5-a420-c0874ef82d64@heusel.eu/ Fixes: 28ab9769117c ("ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error") Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> Link: https://lore.kernel.org/r/20240813131900.1285842-2-cassel@kernel.org Signed-off-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
2c59cc61 |
| 01-Jul-2024 |
Igor Pylypiv <ipylypiv@google.com> |
ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error
commit 28ab9769117ca944cb6eb537af5599aa436287a4 upstream.
SAT-5 revision 8 specification removed the text about the ANSI INCITS 43
ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error
commit 28ab9769117ca944cb6eb537af5599aa436287a4 upstream.
SAT-5 revision 8 specification removed the text about the ANSI INCITS 431-2007 compliance which was requiring SCSI/ATA Translation (SAT) to return descriptor format sense data for the ATA PASS-THROUGH commands regardless of the setting of the D_SENSE bit.
Let's honor the D_SENSE bit for ATA PASS-THROUGH commands while generating the "ATA PASS-THROUGH INFORMATION AVAILABLE" sense data.
SAT-5 revision 7 ================
12.2.2.8 Fixed format sense data
Table 212 shows the fields returned in the fixed format sense data (see SPC-5) for ATA PASS-THROUGH commands. SATLs compliant with ANSI INCITS 431-2007, SCSI/ATA Translation (SAT) return descriptor format sense data for the ATA PASS-THROUGH commands regardless of the setting of the D_SENSE bit.
SAT-5 revision 8 ================
12.2.2.8 Fixed format sense data
Table 211 shows the fields returned in the fixed format sense data (see SPC-5) for ATA PASS-THROUGH commands.
Cc: stable@vger.kernel.org # 4.19+ Reported-by: Niklas Cassel <cassel@kernel.org> Closes: https://lore.kernel.org/linux-ide/Zn1WUhmLglM4iais@ryzen.lan Reviewed-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Igor Pylypiv <ipylypiv@google.com> Reviewed-by: Hannes Reinecke <hare@suse.de> Link: https://lore.kernel.org/r/20240702024735.1152293-4-ipylypiv@google.com Signed-off-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
ce477199 |
| 01-Jul-2024 |
Igor Pylypiv <ipylypiv@google.com> |
ata: libata-scsi: Do not overwrite valid sense data when CK_COND=1
commit 97981926224afe17ba3e22e0c2b7dd8b516ee574 upstream.
Current ata_gen_passthru_sense() code performs two actions: 1. Generates
ata: libata-scsi: Do not overwrite valid sense data when CK_COND=1
commit 97981926224afe17ba3e22e0c2b7dd8b516ee574 upstream.
Current ata_gen_passthru_sense() code performs two actions: 1. Generates sense data based on the ATA 'status' and ATA 'error' fields. 2. Populates "ATA Status Return sense data descriptor" / "Fixed format sense data" with ATA taskfile fields.
The problem is that #1 generates sense data even when a valid sense data is already present (ATA_QCFLAG_SENSE_VALID is set). Factoring out #2 into a separate function allows us to generate sense data only when there is no valid sense data (ATA_QCFLAG_SENSE_VALID is not set).
As a bonus, we can now delete a FIXME comment in atapi_qc_complete() which states that we don't want to translate taskfile registers into sense descriptors for ATAPI.
Additionally, always set SAM_STAT_CHECK_CONDITION when CK_COND=1 because SAT specification mandates that SATL shall return CHECK CONDITION if the CK_COND bit is set.
The ATA PASS-THROUGH handling logic in ata_scsi_qc_complete() is hard to read/understand. Improve the readability of the code by moving checks into self-explanatory boolean variables.
Cc: stable@vger.kernel.org # 4.19+ Co-developed-by: Niklas Cassel <cassel@kernel.org> Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Igor Pylypiv <ipylypiv@google.com> Link: https://lore.kernel.org/r/20240702024735.1152293-3-ipylypiv@google.com Signed-off-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
5a7c16a8 |
| 01-Jul-2024 |
Igor Pylypiv <ipylypiv@google.com> |
ata: libata-scsi: Fix offsets for the fixed format sense data
commit 38dab832c3f4154968f95b267a3bb789e87554b0 upstream.
Correct the ATA PASS-THROUGH fixed format sense data offsets to conform to SP
ata: libata-scsi: Fix offsets for the fixed format sense data
commit 38dab832c3f4154968f95b267a3bb789e87554b0 upstream.
Correct the ATA PASS-THROUGH fixed format sense data offsets to conform to SPC-6 and SAT-5 specifications. Additionally, set the VALID bit to indicate that the INFORMATION field contains valid information.
INFORMATION ===========
SAT-5 Table 212 — "Fixed format sense data INFORMATION field for the ATA PASS-THROUGH commands" defines the following format:
+------+------------+ | Byte | Field | +------+------------+ | 0 | ERROR | | 1 | STATUS | | 2 | DEVICE | | 3 | COUNT(7:0) | +------+------------+
SPC-6 Table 48 - "Fixed format sense data" specifies that the INFORMATION field starts at byte 3 in sense buffer resulting in the following offsets for the ATA PASS-THROUGH commands:
+------------+-------------------------+ | Field | Offset in sense buffer | +------------+-------------------------+ | ERROR | 3 | | STATUS | 4 | | DEVICE | 5 | | COUNT(7:0) | 6 | +------------+-------------------------+
COMMAND-SPECIFIC INFORMATION ============================
SAT-5 Table 213 - "Fixed format sense data COMMAND-SPECIFIC INFORMATION field for ATA PASS-THROUGH" defines the following format:
+------+-------------------+ | Byte | Field | +------+-------------------+ | 0 | FLAGS | LOG INDEX | | 1 | LBA (7:0) | | 2 | LBA (15:8) | | 3 | LBA (23:16) | +------+-------------------+
SPC-6 Table 48 - "Fixed format sense data" specifies that the COMMAND-SPECIFIC-INFORMATION field starts at byte 8 in sense buffer resulting in the following offsets for the ATA PASS-THROUGH commands:
Offsets of these fields in the fixed sense format are as follows:
+-------------------+-------------------------+ | Field | Offset in sense buffer | +-------------------+-------------------------+ | FLAGS | LOG INDEX | 8 | | LBA (7:0) | 9 | | LBA (15:8) | 10 | | LBA (23:16) | 11 | +-------------------+-------------------------+
Reported-by: Akshat Jain <akshatzen@google.com> Fixes: 11093cb1ef56 ("libata-scsi: generate correct ATA pass-through sense") Cc: stable@vger.kernel.org Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Igor Pylypiv <ipylypiv@google.com> Link: https://lore.kernel.org/r/20240702024735.1152293-2-ipylypiv@google.com Signed-off-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
341b5e10 |
| 11-Apr-2024 |
Damien Le Moal <dlemoal@kernel.org> |
ata: libata-scsi: Fix ata_scsi_dev_rescan() error path
commit 79336504781e7fee5ddaf046dcc186c8dfdf60b1 upstream.
Commit 0c76106cb975 ("scsi: sd: Fix TCG OPAL unlock on system resume") incorrectly h
ata: libata-scsi: Fix ata_scsi_dev_rescan() error path
commit 79336504781e7fee5ddaf046dcc186c8dfdf60b1 upstream.
Commit 0c76106cb975 ("scsi: sd: Fix TCG OPAL unlock on system resume") incorrectly handles failures of scsi_resume_device() in ata_scsi_dev_rescan(), leading to a double call to spin_unlock_irqrestore() to unlock a device port. Fix this by redefining the goto labels used in case of errors and only unlock the port scsi_scan_mutex when scsi_resume_device() fails.
Bug found with the Smatch static checker warning:
drivers/ata/libata-scsi.c:4774 ata_scsi_dev_rescan() error: double unlocked 'ap->lock' (orig line 4757)
Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Fixes: 0c76106cb975 ("scsi: sd: Fix TCG OPAL unlock on system resume") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
a1f506af |
| 19-Mar-2024 |
Damien Le Moal <dlemoal@kernel.org> |
scsi: sd: Fix TCG OPAL unlock on system resume
commit 0c76106cb97548810214def8ee22700bbbb90543 upstream.
Commit 3cc2ffe5c16d ("scsi: sd: Differentiate system and runtime start/stop management") int
scsi: sd: Fix TCG OPAL unlock on system resume
commit 0c76106cb97548810214def8ee22700bbbb90543 upstream.
Commit 3cc2ffe5c16d ("scsi: sd: Differentiate system and runtime start/stop management") introduced the manage_system_start_stop scsi_device flag to allow libata to indicate to the SCSI disk driver that nothing should be done when resuming a disk on system resume. This change turned the execution of sd_resume() into a no-op for ATA devices on system resume. While this solved deadlock issues during device resume, this change also wrongly removed the execution of opal_unlock_from_suspend(). As a result, devices with TCG OPAL locking enabled remain locked and inaccessible after a system resume from sleep.
To fix this issue, introduce the SCSI driver resume method and implement it with the sd_resume() function calling opal_unlock_from_suspend(). The former sd_resume() function is renamed to sd_resume_common() and modified to call the new sd_resume() function. For non-ATA devices, this result in no functional changes.
In order for libata to explicitly execute sd_resume() when a device is resumed during system restart, the function scsi_resume_device() is introduced. libata calls this function from the revalidation work executed on devie resume, a state that is indicated with the new device flag ATA_DFLAG_RESUMING. Doing so, locked TCG OPAL enabled devices are unlocked on resume, allowing normal operation.
Fixes: 3cc2ffe5c16d ("scsi: sd: Differentiate system and runtime start/stop management") Link: https://bugzilla.kernel.org/show_bug.cgi?id=218538 Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Link: https://lore.kernel.org/r/20240319071209.1179257-1-dlemoal@kernel.org Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
b32c0a7c |
| 20-Nov-2023 |
Damien Le Moal <dlemoal@kernel.org> |
scsi: sd: Fix system start for ATA devices
commit b09d7f8fd50f6e93cbadd8d27fde178f745b42a1 upstream.
It is not always possible to keep a device in the runtime suspended state when a system level su
scsi: sd: Fix system start for ATA devices
commit b09d7f8fd50f6e93cbadd8d27fde178f745b42a1 upstream.
It is not always possible to keep a device in the runtime suspended state when a system level suspend/resume cycle is executed. E.g. for ATA devices connected to AHCI adapters, system resume resets the ATA ports, which causes connected devices to spin up. In such case, a runtime suspended disk will incorrectly be seen with a suspended runtime state because the device is not resumed by sd_resume_system(). The power state seen by the user is different than the actual device physical power state.
Fix this issue by introducing the struct scsi_device flag force_runtime_start_on_system_start. When set, this flag causes sd_resume_system() to request a runtime resume operation for runtime suspended devices. This results in the user seeing the device runtime_state as active after a system resume, thus correctly reflecting the device physical power state.
Fixes: 9131bff6a9f1 ("scsi: core: pm: Only runtime resume if necessary") Cc: <stable@vger.kernel.org> Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Link: https://lore.kernel.org/r/20231120225631.37938-3-dlemoal@kernel.org Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
80fb1378 |
| 20-Nov-2023 |
Damien Le Moal <dlemoal@kernel.org> |
scsi: Change SCSI device boolean fields to single bit flags
commit 6371be7aeb986905bb60ec73d002fc02343393b4 upstream.
Commit 3cc2ffe5c16d ("scsi: sd: Differentiate system and runtime start/stop man
scsi: Change SCSI device boolean fields to single bit flags
commit 6371be7aeb986905bb60ec73d002fc02343393b4 upstream.
Commit 3cc2ffe5c16d ("scsi: sd: Differentiate system and runtime start/stop management") changed the single bit manage_start_stop flag into 2 boolean fields of the SCSI device structure. Commit 24eca2dce0f8 ("scsi: sd: Introduce manage_shutdown device flag") introduced the manage_shutdown boolean field for the same structure. Together, these 2 commits increase the size of struct scsi_device by 8 bytes by using booleans instead of defining the manage_xxx fields as single bit flags, similarly to other flags of this structure.
Avoid this unnecessary structure size increase and be consistent with the definition of other flags by reverting the definitions of the manage_xxx fields as single bit flags.
Fixes: 3cc2ffe5c16d ("scsi: sd: Differentiate system and runtime start/stop management") Fixes: 24eca2dce0f8 ("scsi: sd: Introduce manage_shutdown device flag") Cc: <stable@vger.kernel.org> Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Link: https://lore.kernel.org/r/20231120225631.37938-2-dlemoal@kernel.org Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
#
24eca2dc |
| 25-Oct-2023 |
Damien Le Moal <dlemoal@kernel.org> |
scsi: sd: Introduce manage_shutdown device flag
Commit aa3998dbeb3a ("ata: libata-scsi: Disable scsi device manage_system_start_stop") change setting the manage_system_start_stop flag to false for l
scsi: sd: Introduce manage_shutdown device flag
Commit aa3998dbeb3a ("ata: libata-scsi: Disable scsi device manage_system_start_stop") change setting the manage_system_start_stop flag to false for libata managed disks to enable libata internal management of disk suspend/resume. However, a side effect of this change is that on system shutdown, disks are no longer being stopped (set to standby mode with the heads unloaded). While this is not a critical issue, this unclean shutdown is not recommended and shows up with increased smart counters (e.g. the unexpected power loss counter "Unexpect_Power_Loss_Ct").
Instead of defining a shutdown driver method for all ATA adapter drivers (not all of them define that operation), this patch resolves this issue by further refining the sd driver start/stop control of disks using the new flag manage_shutdown. If this new flag is set to true by a low level driver, the function sd_shutdown() will issue a START STOP UNIT command with the start argument set to 0 when a disk needs to be powered off (suspended) on system power off, that is, when system_state is equal to SYSTEM_POWER_OFF.
Similarly to the other manage_xxx flags, the new manage_shutdown flag is exposed through sysfs as a read-write device attribute.
To avoid any confusion between manage_shutdown and manage_system_start_stop, the comments describing these flags in include/scsi/scsi.h are also improved.
Fixes: aa3998dbeb3a ("ata: libata-scsi: Disable scsi device manage_system_start_stop") Cc: stable@vger.kernel.org Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218038 Link: https://lore.kernel.org/all/cd397c88-bf53-4768-9ab8-9d107df9e613@gmail.com/ Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com> Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: James Bottomley <James.Bottomley@HansenPartnership.com> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
show more ...
|
#
8b4d9469 |
| 04-Sep-2023 |
Damien Le Moal <dlemoal@kernel.org> |
ata: libata-scsi: Fix delayed scsi_rescan_device() execution
Commit 6aa0365a3c85 ("ata: libata-scsi: Avoid deadlock on rescan after device resume") modified ata_scsi_dev_rescan() to check the scsi d
ata: libata-scsi: Fix delayed scsi_rescan_device() execution
Commit 6aa0365a3c85 ("ata: libata-scsi: Avoid deadlock on rescan after device resume") modified ata_scsi_dev_rescan() to check the scsi device "is_suspended" power field to ensure that the scsi device associated with an ATA device is fully resumed when scsi_rescan_device() is executed. However, this fix is problematic as: 1) It relies on a PM internal field that should not be used without PM device locking protection. 2) The check for is_suspended and the call to scsi_rescan_device() are not atomic and a suspend PM event may be triggered between them, casuing scsi_rescan_device() to be called on a suspended device and in that function blocking while holding the scsi device lock. This would deadlock a following resume operation. These problems can trigger PM deadlocks on resume, especially with resume operations triggered quickly after or during suspend operations. E.g., a simple bash script like:
for (( i=0; i<10; i++ )); do echo "+2 > /sys/class/rtc/rtc0/wakealarm echo mem > /sys/power/state done
that triggers a resume 2 seconds after starting suspending a system can quickly lead to a PM deadlock preventing the system from correctly resuming.
Fix this by replacing the check on is_suspended with a check on the return value given by scsi_rescan_device() as that function will fail if called against a suspended device. Also make sure rescan tasks already scheduled are first cancelled before suspending an ata port.
Fixes: 6aa0365a3c85 ("ata: libata-scsi: Avoid deadlock on rescan after device resume") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
show more ...
|
#
aa3998db |
| 25-Aug-2023 |
Damien Le Moal <dlemoal@kernel.org> |
ata: libata-scsi: Disable scsi device manage_system_start_stop
The introduction of a device link to create a consumer/supplier relationship between the scsi device of an ATA device and the ATA port
ata: libata-scsi: Disable scsi device manage_system_start_stop
The introduction of a device link to create a consumer/supplier relationship between the scsi device of an ATA device and the ATA port of that ATA device fixes the ordering of system suspend and resume operations. For suspend, the scsi device is suspended first and the ata port after it. This is fine as this allows the synchronize cache and START STOP UNIT commands issued by the scsi disk driver to be executed before the ata port is disabled.
For resume operations, the ata port is resumed first, followed by the scsi device. This allows having the request queue of the scsi device to be unfrozen after the ata port resume is scheduled in EH, thus avoiding to see new requests prematurely issued to the ATA device. Since libata sets manage_system_start_stop to 1, the scsi disk resume operation also results in issuing a START STOP UNIT command to the device being resumed so that the device exits standby power mode.
However, restoring the ATA device to the active power mode must be synchronized with libata EH processing of the port resume operation to avoid either 1) seeing the start stop unit command being received too early when the port is not yet resumed and ready to accept commands, or after the port resume process issues commands such as IDENTIFY to revalidate the device. In this last case, the risk is that the device revalidation fails with timeout errors as the drive is still spun down.
Commit 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume") disabled issuing the START STOP UNIT command to avoid issues with it. But this is incorrect as transitioning a device to the active power mode from the standby power mode set on suspend requires a media access command. The IDENTIFY, READ LOG and SET FEATURES commands executed in libata EH context triggered by the ata port resume operation may thus fail.
Fix these synchronization issues is by handling a device power mode transitions for system suspend and resume directly in libata EH context, without relying on the scsi disk driver management triggered with the manage_system_start_stop flag.
To do this, the following libata helper functions are introduced:
1) ata_dev_power_set_standby():
This function issues a STANDBY IMMEDIATE command to transitiom a device to the standby power mode. For HDDs, this spins down the disks. This function applies only to ATA and ZAC devices and does nothing otherwise. This function also does nothing for devices that have the ATA_FLAG_NO_POWEROFF_SPINDOWN or ATA_FLAG_NO_HIBERNATE_SPINDOWN flag set.
For suspend, call ata_dev_power_set_standby() in ata_eh_handle_port_suspend() before the port is disabled and frozen. ata_eh_unload() is also modified to transition all enabled devices to the standby power mode when the system is shutdown or devices removed.
2) ata_dev_power_set_active() and
This function applies to ATA or ZAC devices and issues a VERIFY command for 1 sector at LBA 0 to transition the device to the active power mode. For HDDs, since this function will complete only once the disk spin up. Its execution uses the same timeouts as for reset, to give the drive enough time to complete spinup without triggering a command timeout.
For resume, call ata_dev_power_set_active() in ata_eh_revalidate_and_attach() after the port has been enabled and before any other command is issued to the device.
With these changes, the manage_system_start_stop and no_start_on_resume scsi device flags do not need to be set in ata_scsi_dev_config(). The flag manage_runtime_start_stop is still set to allow the sd driver to spinup/spindown a disk through the sd runtime operations.
Fixes: 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Hannes Reinecke <hare@suse.de> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
show more ...
|
#
3cc2ffe5 |
| 14-Sep-2023 |
Damien Le Moal <dlemoal@kernel.org> |
scsi: sd: Differentiate system and runtime start/stop management
The underlying device and driver of a SCSI disk may have different system and runtime power mode control requirements. This is becaus
scsi: sd: Differentiate system and runtime start/stop management
The underlying device and driver of a SCSI disk may have different system and runtime power mode control requirements. This is because runtime power management affects only the SCSI disk, while system level power management affects all devices, including the controller for the SCSI disk.
For instance, issuing a START STOP UNIT command when a SCSI disk is runtime suspended and resumed is fine: the command is translated to a STANDBY IMMEDIATE command to spin down the ATA disk and to a VERIFY command to wake it up. The SCSI disk runtime operations have no effect on the ata port device used to connect the ATA disk. However, for system suspend/resume operations, the ATA port used to connect the device will also be suspended and resumed, with the resume operation requiring re-validating the device link and the device itself. In this case, issuing a VERIFY command to spinup the disk must be done before starting to revalidate the device, when the ata port is being resumed. In such case, we must not allow the SCSI disk driver to issue START STOP UNIT commands.
Allow a low level driver to refine the SCSI disk start/stop management by differentiating system and runtime cases with two new SCSI device flags: manage_system_start_stop and manage_runtime_start_stop. These new flags replace the current manage_start_stop flag. Drivers setting the manage_start_stop are modifed to set both new flags, thus preserving the existing start/stop management behavior. For backward compatibility, the old manage_start_stop sysfs device attribute is kept as a read-only attribute showing a value of 1 for devices enabling both new flags and 0 otherwise.
Fixes: 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Hannes Reinecke <hare@suse.de> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
show more ...
|
#
fb99ef17 |
| 25-Aug-2023 |
Damien Le Moal <dlemoal@kernel.org> |
ata: libata-scsi: link ata port and scsi device
There is no direct device ancestry defined between an ata_device and its scsi device which prevents the power management code from correctly ordering
ata: libata-scsi: link ata port and scsi device
There is no direct device ancestry defined between an ata_device and its scsi device which prevents the power management code from correctly ordering suspend and resume operations. Create such ancestry with the ata device as the parent to ensure that the scsi device (child) is suspended before the ata device and that resume handles the ata device before the scsi device.
The parent-child (supplier-consumer) relationship is established between the ata_port (parent) and the scsi device (child) with the function device_add_link(). The parent used is not the ata_device as the PM operations are defined per port and the status of all devices connected through that port is controlled from the port operations.
The device link is established with the new function ata_scsi_slave_alloc(), and this function is used to define the ->slave_alloc callback of the scsi host template of all ata drivers.
Fixes: a19a93e4c6a9 ("scsi: core: pm: Rely on the device driver core for async power management") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> Reviewed-by: John Garry <john.g.garry@oracle.com>
show more ...
|
#
3ef60092 |
| 18-Sep-2023 |
Niklas Cassel <niklas.cassel@wdc.com> |
ata: libata-scsi: ignore reserved bits for REPORT SUPPORTED OPERATION CODES
For REPORT SUPPORTED OPERATION CODES command, the service action field is defined as bits 0-4 in the second byte in the CD
ata: libata-scsi: ignore reserved bits for REPORT SUPPORTED OPERATION CODES
For REPORT SUPPORTED OPERATION CODES command, the service action field is defined as bits 0-4 in the second byte in the CDB. Bits 5-7 in the second byte are reserved.
Only look at the service action field in the second byte when determining if the MAINTENANCE IN opcode is a REPORT SUPPORTED OPERATION CODES command.
This matches how we only look at the service action field in the second byte when determining if the SERVICE ACTION IN(16) opcode is a READ CAPACITY(16) command (reserved bits 5-7 in the second byte are ignored).
Fixes: 7b2030942859 ("libata: Add support for SCT Write Same") Cc: stable@vger.kernel.org Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
show more ...
|
#
2132df16 |
| 14-Sep-2023 |
Damien Le Moal <dlemoal@kernel.org> |
scsi: core: ata: Do no try to probe for CDL on old drives
Some old drives (e.g. an Ultra320 SCSI disk as reported by John) do not seem to execute MAINTENANCE_IN / MI_REPORT_SUPPORTED_OPERATION_CODES
scsi: core: ata: Do no try to probe for CDL on old drives
Some old drives (e.g. an Ultra320 SCSI disk as reported by John) do not seem to execute MAINTENANCE_IN / MI_REPORT_SUPPORTED_OPERATION_CODES commands correctly and hang when a non-zero service action is specified (one command format with service action case in scsi_report_opcode()).
Currently, CDL probing with scsi_cdl_check_cmd() is the only caller using a non zero service action for scsi_report_opcode(). To avoid issues with these old drives, do not attempt CDL probe if the device reports support for an SPC version lower than 5 (CDL was introduced in SPC-5). To keep things working with ATA devices which probe for the CDL T2A and T2B pages introduced with SPC-6, modify ata_scsiop_inq_std() to claim SPC-6 version compatibility for ATA drives supporting CDL.
SPC-6 standard version number is defined as Dh (= 13) in SPC-6 r09. Fix scsi_probe_lun() to correctly capture this value by changing the bit mask for the second byte of the INQUIRY response from 0x7 to 0xf. include/scsi/scsi.h is modified to add the definition SCSI_SPC_6 with the value 14 (Dh + 1). The missing definitions for the SCSI_SPC_4 and SCSI_SPC_5 versions are also added.
Reported-by: John David Anglin <dave.anglin@bell.net> Fixes: 624885209f31 ("scsi: core: Detect support for command duration limits") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Link: https://lore.kernel.org/r/20230915022034.678121-1-dlemoal@kernel.org Tested-by: David Gow <david@davidgow.net> Reviewed-by: Bart Van Assche <bvanassche@acm.org> Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
show more ...
|
#
79519528 |
| 22-Aug-2023 |
Bart Van Assche <bvanassche@acm.org> |
scsi: core: Improve type safety of scsi_rescan_device()
Most callers of scsi_rescan_device() have the scsi_device pointer readily available. Pass a struct scsi_device pointer to scsi_rescan_device()
scsi: core: Improve type safety of scsi_rescan_device()
Most callers of scsi_rescan_device() have the scsi_device pointer readily available. Pass a struct scsi_device pointer to scsi_rescan_device() instead of a struct device pointer. This change prevents that a pointer to another struct device would be passed accidentally to scsi_rescan_device().
Remove the scsi_rescan_device() declaration from the scsi_priv.h header file since it duplicates the declaration in <scsi/scsi_host.h>.
Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Reviewed-by: John Garry <john.g.garry@oracle.com> Cc: Mike Christie <michael.christie@oracle.com> Cc: Ming Lei <ming.lei@redhat.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> Link: https://lore.kernel.org/r/20230822153043.4046244-1-bvanassche@acm.org Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
show more ...
|
#
ff8072d5 |
| 31-Jul-2023 |
Hannes Reinecke <hare@suse.de> |
ata: libata: remove references to non-existing error_handler()
With commit 65a15d6560df ("scsi: ipr: Remove SATA support") all libata drivers now have the error_handler() callback provided, so we ca
ata: libata: remove references to non-existing error_handler()
With commit 65a15d6560df ("scsi: ipr: Remove SATA support") all libata drivers now have the error_handler() callback provided, so we can stop checking for non-existing error_handler callback.
Signed-off-by: Hannes Reinecke <hare@suse.de> [niklas: fixed review comments, rebased, solved conflicts during rebase, fixed bug that unconditionally dumped all QCs, removed the now unused function ata_dump_status(), removed the now unreachable failure paths in atapi_qc_complete(), removed the non-EH function to request ATAPI sense] Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> Reviewed-by: John Garry <john.g.garry@oracle.com> Reviewed-by: Jason Yan <yanaijie@huawei.com> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
show more ...
|
#
8c125363 |
| 29-Jul-2023 |
Sergey Shtylyov <s.shtylyov@omp.ru> |
ata: libata-scsi: fix timeout type in ata_scsi_park_store()
ata_scsi_park_store() passes its 'long input' variable (if it's >= 0) to ata_deadline() that now takes 'unsigned int' -- eliminate unneede
ata: libata-scsi: fix timeout type in ata_scsi_park_store()
ata_scsi_park_store() passes its 'long input' variable (if it's >= 0) to ata_deadline() that now takes 'unsigned int' -- eliminate unneeded implicit cast...
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
show more ...
|
#
0a858905 |
| 23-Jul-2023 |
Damien Le Moal <dlemoal@kernel.org> |
ata,scsi: do not issue START STOP UNIT on resume
During system resume, ata_port_pm_resume() triggers ata EH to 1) Resume the controller 2) Reset and rescan the ports 3) Revalidate devices This EH ex
ata,scsi: do not issue START STOP UNIT on resume
During system resume, ata_port_pm_resume() triggers ata EH to 1) Resume the controller 2) Reset and rescan the ports 3) Revalidate devices This EH execution is started asynchronously from ata_port_pm_resume(), which means that when sd_resume() is executed, none or only part of the above processing may have been executed. However, sd_resume() issues a START STOP UNIT to wake up the drive from sleep mode. This command is translated to ATA with ata_scsi_start_stop_xlat() and issued to the device. However, depending on the state of execution of the EH process and revalidation triggerred by ata_port_pm_resume(), two things may happen: 1) The START STOP UNIT fails if it is received before the controller has been reenabled at the beginning of the EH execution. This is visible with error messages like:
ata10.00: device reported invalid CHS sector 0 sd 9:0:0:0: [sdc] Start/Stop Unit failed: Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 9:0:0:0: [sdc] Sense Key : Illegal Request [current] sd 9:0:0:0: [sdc] Add. Sense: Unaligned write command sd 9:0:0:0: PM: dpm_run_callback(): scsi_bus_resume+0x0/0x90 returns -5 sd 9:0:0:0: PM: failed to resume async: error -5
2) The START STOP UNIT command is received while the EH process is on-going, which mean that it is stopped and must wait for its completion, at which point the command is rather useless as the drive is already fully spun up already. This case results also in a significant delay in sd_resume() which is observable by users as the entire system resume completion is delayed.
Given that ATA devices will be woken up by libata activity on resume, sd_resume() has no need to issue a START STOP UNIT command, which solves the above mentioned problems. Do not issue this command by introducing the new scsi_device flag no_start_on_resume and setting this flag to 1 in ata_scsi_dev_config(). sd_resume() is modified to issue a START STOP UNIT command only if this flag is not set.
Reported-by: Paul Ausbeck <paula@soe.ucsc.edu> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=215880 Fixes: a19a93e4c6a9 ("scsi: core: pm: Rely on the device driver core for async power management") Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Tested-by: Tanner Watkins <dalzot@gmail.com> Tested-by: Paul Ausbeck <paula@soe.ucsc.edu> Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
show more ...
|
#
6aa0365a |
| 15-Jun-2023 |
Damien Le Moal <dlemoal@kernel.org> |
ata: libata-scsi: Avoid deadlock on rescan after device resume
When an ATA port is resumed from sleep, the port is reset and a power management request issued to libata EH to reset the port and resc
ata: libata-scsi: Avoid deadlock on rescan after device resume
When an ATA port is resumed from sleep, the port is reset and a power management request issued to libata EH to reset the port and rescanning the device(s) attached to the port. Device rescanning is done by scheduling an ata_scsi_dev_rescan() work, which will execute scsi_rescan_device().
However, scsi_rescan_device() takes the generic device lock, which is also taken by dpm_resume() when the SCSI device is resumed as well. If a device rescan execution starts before the completion of the SCSI device resume, the rcu locking used to refresh the cached VPD pages of the device, combined with the generic device locking from scsi_rescan_device() and from dpm_resume() can cause a deadlock.
Avoid this situation by changing struct ata_port scsi_rescan_task to be a delayed work instead of a simple work_struct. ata_scsi_dev_rescan() is modified to check if the SCSI device associated with the ATA device that must be rescanned is not suspended. If the SCSI device is still suspended, ata_scsi_dev_rescan() returns early and reschedule itself for execution after an arbitrary delay of 5ms.
Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Reported-by: Joe Breuer <linux-kernel@jmbreuer.net> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217530 Fixes: a19a93e4c6a9 ("scsi: core: pm: Rely on the device driver core for async power management") Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Hannes Reinecke <hare@suse.de> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Tested-by: Joe Breuer <linux-kernel@jmbreuer.net>
show more ...
|
#
43cff7d9 |
| 04-Jun-2023 |
Damien Le Moal <dlemoal@kernel.org> |
ata: libata-scsi: Use ata_ncq_supported in ata_scsi_dev_config()
In ata_scsi_dev_config(), instead of hard-coding the test to check if an ATA device supports NCQ by looking at the ATA_DFLAG_NCQ flag
ata: libata-scsi: Use ata_ncq_supported in ata_scsi_dev_config()
In ata_scsi_dev_config(), instead of hard-coding the test to check if an ATA device supports NCQ by looking at the ATA_DFLAG_NCQ flag, use ata_ncq_supported().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: John Garry <john.g.garry@oracle.com> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
show more ...
|
#
401f8ef3 |
| 23-May-2023 |
Damien Le Moal <dlemoal@kernel.org> |
scsi: ata: libata-scsi: Fix ata_msense_control kdoc comment
Add missing description of the spg argument of ata_msense_control().
Fixes: df60f9c64576 ("scsi: ata: libata: Add ATA feature control sub
scsi: ata: libata-scsi: Fix ata_msense_control kdoc comment
Add missing description of the spg argument of ata_msense_control().
Fixes: df60f9c64576 ("scsi: ata: libata: Add ATA feature control sub-page translation") Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Link: https://lore.kernel.org/r/20230523074701.293502-1-dlemoal@kernel.org Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
show more ...
|
#
7f875850 |
| 22-May-2023 |
Damien Le Moal <dlemoal@kernel.org> |
ata: libata-scsi: Use correct device no in ata_find_dev()
For devices not attached to a port multiplier and managed directly by libata, the device number passed to ata_find_dev() must always be lowe
ata: libata-scsi: Use correct device no in ata_find_dev()
For devices not attached to a port multiplier and managed directly by libata, the device number passed to ata_find_dev() must always be lower than the maximum number of devices returned by ata_link_max_devices(). That is 1 for SATA devices or 2 for an IDE link with master+slave devices. This device number is the SCSI device ID which matches these constraints as the IDs are generated per port and so never exceed the maximum number of devices for the link being used.
However, for libsas managed devices, SCSI device IDs are assigned per struct scsi_host, leading to device IDs for SATA devices that can be well in excess of libata per-link maximum number of devices. This results in ata_find_dev() to always return NULL for libsas managed devices except for the first device of the target scsi_host with ID (device number) equal to 0. This issue is visible by executing the hdparm utility, which fails. E.g.:
hdparm -i /dev/sdX /dev/sdX: HDIO_GET_IDENTITY failed: No message of desired type
Fix this by rewriting ata_find_dev() to ignore the device number for non-PMP attached devices with a link with at most 1 device, that is SATA devices. For these, the device number 0 is always used to return the correct pointer to the struct ata_device of the port link. This change excludes IDE master/slave setups (maximum number of devices per link is 2) and port-multiplier attached devices. Also, to be consistant with the fact that SCSI device IDs and channel numbers used as device numbers are both unsigned int, change the devno argument of ata_find_dev() to unsigned int.
Reported-by: Xingui Yang <yangxingui@huawei.com> Fixes: 41bda9c98035 ("libata-link: update hotplug to handle PMP links") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Jason Yan <yanaijie@huawei.com>
show more ...
|