This driver uses MSR functions that aren't implemented under UML. Avoid building it to prevent tripping up allyesconfig. e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: pata_cs5535.c:(.text+0x3a3): undefined reference to `__tracepoint_read_msr' /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: pata_cs5535.c:(.text+0x3d2): undefined reference to `__tracepoint_write_msr' /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: pata_cs5535.c:(.text+0x457): undefined reference to `__tracepoint_write_msr' /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: pata_cs5535.c:(.text+0x481): undefined reference to `do_trace_write_msr' /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: pata_cs5535.c:(.text+0x4d5): undefined reference to `do_trace_write_msr' /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: pata_cs5535.c:(.text+0x4f5): undefined reference to `do_trace_read_msr' /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: pata_cs5535.c:(.text+0x51c): undefined reference to `do_trace_write_msr' Signed-off-by: Peter Foley <pefoley2@pefoley.com> Reviewed-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Since kernel 5.3.4 my laptop (ICH8M controller) does not see Kingston
SV300S37A60G SSD disk connected into a SATA connector on wake from
suspend. The problem was introduced in c312ef176399 ("libata/ahci: Drop
PCS quirk for Denverton and beyond"): the quirk is not applied on wake
from suspend as it originally was.
It is worth to mention the commit contained another bug: the quirk is
not applied at all to controllers which require it. The fix commit
09d6ac8dc51a ("libata/ahci: Fix PCS quirk application") landed in 5.3.8.
So testing my patch anywhere between commits c312ef176399 and
09d6ac8dc51a is pointless.
Not all disks trigger the problem. For example nothing bad happens with
Western Digital WD5000LPCX HDD.
Test hardware:
- Acer 5920G with ICH8M SATA controller
- sda: some SATA HDD connnected into the DVD drive IDE port with a
SATA-IDE caddy. It is a boot disk
- sdb: Kingston SV300S37A60G SSD connected into the only SATA port
Sample "dmesg --notime | grep -E '^(sd |ata)'" output on wake:
sd 0:0:0:0: [sda] Starting disk
sd 2:0:0:0: [sdb] Starting disk
ata4: SATA link down (SStatus 4 SControl 300)
ata3: SATA link down (SStatus 4 SControl 300)
ata1.00: ACPI cmd ef/03:0c:00:00:00:a0 (SET FEATURES) filtered out
ata1.00: ACPI cmd ef/03:42:00:00:00:a0 (SET FEATURES) filtered out
ata1: FORCE: cable set to 80c
ata5: SATA link down (SStatus 0 SControl 300)
ata3: SATA link down (SStatus 4 SControl 300)
ata3: SATA link down (SStatus 4 SControl 300)
ata3.00: disabled
sd 2:0:0:0: rejecting I/O to offline device
ata3.00: detaching (SCSI 2:0:0:0)
sd 2:0:0:0: [sdb] Start/Stop Unit failed: Result: hostbyte=DID_NO_CONNECT
driverbyte=DRIVER_OK
sd 2:0:0:0: [sdb] Synchronizing SCSI cache
sd 2:0:0:0: [sdb] Synchronize Cache(10) failed: Result:
hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK
sd 2:0:0:0: [sdb] Stopping disk
sd 2:0:0:0: [sdb] Start/Stop Unit failed: Result: hostbyte=DID_BAD_TARGET
driverbyte=DRIVER_OK
Commit c312ef176399 dropped ahci_pci_reset_controller() which internally
calls ahci_reset_controller() and applies the PCS quirk if needed after
that. It was called each time a reset was required instead of just
ahci_reset_controller(). This patch puts the function back in place.
Fixes: c312ef176399 ("libata/ahci: Drop PCS quirk for Denverton and beyond")
Signed-off-by: Adam Vodopjan <grozzly@protonmail.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Pull ata updates from Damien Le Moal:
"The ususal set of driver fixes and improvements as well as several
patches improving libata core in preparation of the introduction of
the support for the command duration limits feature. In more details:
- Define the missing COMPLETED sense key in scsi header (me)
- Several patches to improve libata handling of the status of
completed commands and the retry and sense data reported to the
scsi layer for failed commands. In particular, this widen the
support for NCQ autosense to all drives that support this feature
instead of restricting this feature use to ZAC drives only (Niklas)
- Cleanup of the pata_mpc52xx and sata_dwc_460ex drivers to remove
the use of the deprecated NO_IRQ macro (Christophe)
- Fix build dedependency on OF vs use of the of_match_ptr() macro to
avoid build errors with the sata_gemini and pata_ftide010 drivers
(me)
- Some libata cleanups using the new helper function
ata_port_is_frozen() (Niklas)
- Improve internal command handling by not retrying commands that
failed with a timeout (Niklas)
- Remove code for several unused libata helper functions (from
Niklas)
- Remove the palmchip pata_bk3710 driver. A couple of other driver
removal should come in through the arm tree pull request (from
Arnd)
- Remove unused variable and function in the sata_dwc_460ex driver
and libata-sff code (Colin and Sergey)
- Minor cleanup of the pata_ep93xx driver platform code (from
Minghao)
- Remove the unnecessary linux/msi.h include from the ahci driver
(Thomas)
- Changes to libata enum constants definitions to avoid warnings with
gcc-13 (Arnd)"
* tag 'ata-6.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata: (24 commits)
ata: ahci: fix enum constants for gcc-13
ata: libata: fix commands incorrectly not getting retried during NCQ error
ata: ahci: Remove linux/msi.h include
ata: sata_dwc_460ex: Check !irq instead of irq == NO_IRQ
ata: pata_ep93xx: use devm_platform_get_and_ioremap_resource()
ata: libata-sff: kill unused ata_sff_busy_sleep()
ata: sata_dwc_460ex: remove variable num_processed
ata: remove palmchip pata_bk3710 driver
ata: remove unused helper ata_id_flush_ext_enabled()
ata: remove unused helper ata_id_flush_enabled()
ata: remove unused helper ata_id_lba48_enabled()
ata: libata-core: do not retry reading the log on timeout
scsi: libsas: make use of ata_port_is_frozen() helper
ata: make use of ata_port_is_frozen() helper
ata: add ata_port_is_frozen() helper
ata: pata_ftide010: Remove build dependency on OF
ata: sata_gemini: Remove dependency on OF for compile tests
ata: pata_mpc52xx: Replace NO_IRQ with 0
ata: libahci: read correct status and error field for NCQ commands
ata: libata: fetch sense data for ATA devices supporting sense reporting
...
When booting a arm 32-bit kernel with config CONFIG_AHCI_DWC enabled on
a am57xx-evm board. This happens when the clock references are unnamed
in DT, the strcmp() produces a NULL pointer dereference, see the
following oops, NULL pointer dereference:
[ 4.673950] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 4.682098] [00000000] *pgd=00000000
[ 4.685699] Internal error: Oops: 5 [#1] SMP ARM
[ 4.690338] Modules linked in:
[ 4.693420] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc7 #1
[ 4.699615] Hardware name: Generic DRA74X (Flattened Device Tree)
[ 4.705749] PC is at strcmp+0x0/0x34
[ 4.709350] LR is at ahci_platform_find_clk+0x3c/0x5c
[ 4.714416] pc : [<c130c494>] lr : [<c0c230e0>] psr: 20000013
[ 4.720703] sp : f000dda8 ip : 00000001 fp : c29b1840
[ 4.725952] r10: 00000020 r9 : c1b23380 r8 : c1b23368
[ 4.731201] r7 : c1ab4cc4 r6 : 00000001 r5 : c3c66040 r4 : 00000000
[ 4.737762] r3 : 00000080 r2 : 00000080 r1 : c1ab4cc4 r0 : 00000000
[...]
[ 4.998870] strcmp from ahci_platform_find_clk+0x3c/0x5c
[ 5.004302] ahci_platform_find_clk from ahci_dwc_probe+0x1f0/0x54c
[ 5.010589] ahci_dwc_probe from platform_probe+0x64/0xc0
[ 5.016021] platform_probe from really_probe+0xe8/0x41c
[ 5.021362] really_probe from __driver_probe_device+0xa4/0x204
[ 5.027313] __driver_probe_device from driver_probe_device+0x38/0xc8
[ 5.033782] driver_probe_device from __driver_attach+0xb4/0x1ec
[ 5.039825] __driver_attach from bus_for_each_dev+0x78/0xb8
[ 5.045532] bus_for_each_dev from bus_add_driver+0x17c/0x220
[ 5.051300] bus_add_driver from driver_register+0x90/0x124
[ 5.056915] driver_register from do_one_initcall+0x48/0x1e8
[ 5.062591] do_one_initcall from kernel_init_freeable+0x1cc/0x234
[ 5.068817] kernel_init_freeable from kernel_init+0x20/0x13c
[ 5.074584] kernel_init from ret_from_fork+0x14/0x2c
[ 5.079681] Exception stack(0xf000dfb0 to 0xf000dff8)
[ 5.084747] dfa0: 00000000 00000000 00000000 00000000
[ 5.092956] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 5.101165] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 5.107818] Code: e5e32001 e3520000 1afffffb e12fff1e (e4d03001)
[ 5.114013] ---[ end trace 0000000000000000 ]---
Add an extra check in the if-statement if hpriv-clks[i].id.
Fixes: 6ce73f3a6fc0 ("ata: libahci_platform: Add function returning a clock-handle by id")
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
gcc-13 slightly changes the type of constant expressions that are defined in an enum, which triggers a compile time sanity check in libata: linux/drivers/ata/libahci.c: In function 'ahci_led_store': linux/include/linux/compiler_types.h:357:45: error: call to '__compiletime_assert_302' declared with attribute error: BUILD_BUG_ON failed: sizeof(_s) > sizeof(long) 357 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) The new behavior is that sizeof() returns the same value for the constant as it does for the enum type, which is generally more sensible and consistent. The problem in libata is that it contains a single enum definition for lots of unrelated constants, some of which are large positive (unsigned) integers like 0xffffffff, while others like (1<<31) are interpreted as negative integers, and this forces the enum type to become 64 bit wide even though most constants would still fit into a signed 32-bit 'int'. Fix this by changing the entire enum definition to use BIT(x) in place of (1<<x), which results in all values being seen as 'unsigned' and fitting into an unsigned 32-bit type. Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107917 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107405 Reported-by: Luis Machado <luis.machado@arm.com> Cc: linux-ide@vger.kernel.org Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> Cc: stable@vger.kernel.org Cc: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Arnd Bergmann <arnd@arndb.de> Tested-by: Luis Machado <luis.machado@arm.com> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
A NCQ error means that the device has aborted processing of all active
commands.
To get the single NCQ command that caused the NCQ error, host software has
to read the NCQ error log, which also takes the device out of error state.
When the device encounters a NCQ error, we receive an error interrupt from
the HBA, and call ata_do_link_abort() to mark all outstanding commands on
the link as ATA_QCFLAG_FAILED (which means that these commands are owned
by libata EH), and then call ata_qc_complete() on them.
ata_qc_complete() will call fill_result_tf() for all commands marked as
ATA_QCFLAG_FAILED.
The taskfile is simply the latest status/error as seen from the device's
perspective. The taskfile will have ATA_ERR set in the status field and
ATA_ABORTED set in the error field.
When we fill the current taskfile values for all outstanding commands,
that means that qc->result_tf will have ATA_ERR set for all commands
owned by libata EH.
When ata_eh_link_autopsy() later analyzes all commands owned by libata EH,
it will call ata_eh_analyze_tf(), which will check if qc->result_tf has
ATA_ERR set, if it does, it will set qc->err_mask (which marks the command
as an error).
When ata_eh_finish() later calls __ata_qc_complete() on all commands owned
by libata EH, it will call qc->complete_fn() (ata_scsi_qc_complete()),
ata_scsi_qc_complete() will call ata_gen_ata_sense() to generate sense
data if qc->err_mask is set.
This means that we will generate sense data for commands that should not
have any sense data set. Having sense data set for the non-failed commands
will cause SCSI to finish these commands instead of retrying them.
While this incorrect behavior has existed for a long time, this first
became a problem once we started reading the correct taskfile register in
commit 4ba09d202657 ("ata: libahci: read correct status and error field
for NCQ commands").
Before this commit, NCQ commands would read the taskfile values received
from the last non-NCQ command completion, which most likely did not have
ATA_ERR set, since the last non-NCQ command was most likely not an error.
Fix this by changing ata_eh_analyze_ncq_error() to mark all non-failed
commands as ATA_QCFLAG_RETRY, and change the loop in ata_eh_link_autopsy()
to skip commands marked as ATA_QCFLAG_RETRY.
While at it, make sure that we clear ATA_ERR and any error bits for all
commands except the actual command that caused the NCQ error, so that no
other libata code will be able to misinterpret these commands as errors.
Fixes: 4ba09d202657 ("ata: libahci: read correct status and error field for NCQ commands")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Nothing in this file needs anything from linux/msi.h Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> Cc: linux-ide@vger.kernel.org Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
NO_IRQ is a relic from the old days. It is not used anymore in core functions. By the way, function irq_of_parse_and_map() returns value 0 on error. In some drivers, NO_IRQ is erroneously used to check the return of irq_of_parse_and_map(). It is not a real bug today because the only architectures using the drivers being fixed by this patch define NO_IRQ as 0, but there are architectures which define NO_IRQ as -1. If one day those architectures start using the non fixed drivers, there will be a problem. Long time ago Linus advocated for not using NO_IRQ, see https://lkml.org/lkml/2005/11/21/221 . He re-iterated the same view recently in https://lkml.org/lkml/2022/10/12/622 So test !irq instead of tesing irq == NO_IRQ. And remove the fallback definition of NO_IRQ at the top of the file. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Convert platform_get_resource(), devm_ioremap_resource() to a single call to devm_platform_get_and_ioremap_resource(), as this is exactly what this function does. Reported-by: Zeal Robot <zealci@zte.com.cn> Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
While the ATA specification states that a device should return command
aborted for all commands queued after the device has entered error state,
since ATA only keeps the sense data for the latest command (in non-NCQ
case), we really don't want to send block layer commands to the device
after it has entered error state. (Only ATA EH commands should be sent,
to read the sense data etc.)
Currently, scsi_queue_rq() will check if scsi_host_in_recovery()
(state is SHOST_RECOVERY), and if so, it will _not_ issue a command via:
scsi_dispatch_cmd() -> host->hostt->queuecommand() (ata_scsi_queuecmd())
-> __ata_scsi_queuecmd() -> ata_scsi_translate() -> ata_qc_issue()
Before commit e494f6a72839 ("[SCSI] improved eh timeout handler"),
when receiving a TFES error IRQ, the call chain looked like this:
ahci_error_intr() -> ata_port_abort() -> ata_do_link_abort() ->
ata_qc_complete() -> ata_qc_schedule_eh() -> blk_abort_request() ->
blk_rq_timed_out() -> q->rq_timed_out_fn() (scsi_times_out()) ->
scsi_eh_scmd_add() -> scsi_host_set_state(shost, SHOST_RECOVERY)
Which meant that as soon as an error IRQ was serviced, SHOST_RECOVERY
would be set.
However, after commit e494f6a72839 ("[SCSI] improved eh timeout handler"),
scsi_times_out() will instead call scsi_abort_command() which will queue
delayed work, and the worker function scmd_eh_abort_handler() will call
scsi_eh_scmd_add(), which calls scsi_host_set_state(shost, SHOST_RECOVERY).
So now, after the TFES error IRQ has been serviced, we need to wait for
the SCSI workqueue to run its work before SHOST_RECOVERY gets set.
It is worth noting that, even before commit e494f6a72839 ("[SCSI] improved
eh timeout handler"), we could receive an error IRQ from the time when
scsi_queue_rq() checks scsi_host_in_recovery(), to the time when
ata_scsi_queuecmd() is actually called.
In order to handle both the delayed setting of SHOST_RECOVERY and the
window where we can receive an error IRQ, add a check against
ATA_PFLAG_EH_PENDING (which gets set when servicing the error IRQ),
inside ata_scsi_queuecmd() itself, while holding the ap->lock.
(Since the ap->lock is held while servicing IRQs.)
Fixes: e494f6a72839 ("[SCSI] improved eh timeout handler")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Tested-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>