When ida_alloc() fails, "pi" is not freed although the misleading
comment says otherwise.
Move the ida_alloc() call up so we really don't have to free "pi" in
case of ida_alloc() failure.
Also move ida_free() call from pi_remove_one() to
pata_parport_dev_release(). It was dereferencing already freed dev
pointer.
Testing revealed leak even in non-failure case which was tracked down
to missing put_device() call after bus_find_device_by_name(). As a
result, pata_parport_dev_release() was never called.
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Link: https://lore.kernel.org/r/202303111822.IHNchbkp-lkp@intel.com/
Signed-off-by: Ondrej Zary <linux@zary.sk>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
When adapter is not found, pi->disconnect() is called without previous
pi->connect(). This results in error like this:
parport0: pata_parport tried to release parport when not owner
Add missing out_disconnect label and use it correctly.
Signed-off-by: Ondrej Zary <linux@zary.sk>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Commit 104ff59af7 ("ata: ahci: Add Tiger Lake UP{3,4} AHCI
controller") enabled low power mode for the Tiger Lake AHIC adapter in
the author system but created regressions for others. Revert this patch
for now until a better solution is found to make this adapter
eco-friendly.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217114
CC: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Updates to the usual drivers (ufs, lpfc, qla2xxx, libsas). The major
core change is a rework to remove the two helpers around
scsi_execute_cmd and use it as the only submission interface along
with other minor fixes and updates.
Signed-off-by: James E.J. Bottomley <jejb@linux.ibm.com>
-----BEGIN PGP SIGNATURE-----
iJwEABMIAEQWIQTnYEDbdso9F2cI+arnQslM7pishQUCY/Z7BSYcamFtZXMuYm90
dG9tbGV5QGhhbnNlbnBhcnRuZXJzaGlwLmNvbQAKCRDnQslM7pishfF9AP9HzZaZ
0XumuxjchcJHRntcIAzb9kE62SSWFxBrgZQCtAEAyhAO1zK284esgCa+arkzEC7p
uhxpufQSGnYvbasStsU=
=VsH1
-----END PGP SIGNATURE-----
Merge tag 'scsi-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi
Pull SCSI updates from James Bottomley:
"Updates to the usual drivers (ufs, lpfc, qla2xxx, libsas).
The major core change is a rework to remove the two helpers around
scsi_execute_cmd and use it as the only submission interface along
with other minor fixes and updates"
* tag 'scsi-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi: (142 commits)
scsi: ufs: core: Fix an error handling path in ufshcd_read_desc_param()
scsi: ufs: core: Fix device management cmd timeout flow
scsi: aic94xx: Add missing check for dma_map_single()
scsi: smartpqi: Replace one-element array with flexible-array member
scsi: mpt3sas: Fix a memory leak
scsi: qla2xxx: Remove the unused variable wwn
scsi: ufs: core: Fix kernel-doc syntax
scsi: ufs: core: Add hibernation callbacks
scsi: snic: Fix memory leak with using debugfs_lookup()
scsi: ufs: core: Limit DMA alignment check
scsi: Documentation: Correct spelling
scsi: Documentation: Correct spelling
scsi: target: Documentation: Correct spelling
scsi: aacraid: Allocate cmd_priv with scsicmd
scsi: ufs: qcom: dt-bindings: Add SM8550 compatible string
scsi: ufs: ufs-qcom: Clear qunipro_g4_sel for HW version major 5
scsi: ufs: qcom: fix platform_msi_domain_free_irqs() reference
scsi: ufs: core: Enable DMA clustering
scsi: ufs: exynos: Fix the maximum segment size
scsi: ufs: exynos: Fix DMA alignment for PAGE_SIZE != 4096
...
* Small cleanup of the pata_octeon driver to drop a useless platform
callback, from Uwe.
* Simplify ata_scsi_cmd_error_handler() code using the fact that
ap->ops->error_handler is NULL most of the time, from Wenchao.
* Several patches improving libata error handling. This is in
preparation for supporting the command duration limits (CDL)
feature. The changes allow handling corner cases of ATA NCQ errors
which do not happen with regular drives but will be triggered with
CDL drives. From Niklas.
* Simplify the qc_fill_rtf operation, from me.
* Improve SCSI command translation for the
REPORT_SUPPORTED_OPERATION_CODES command, from me.
* Cleanup of libata FUA handling. This falls short of enabling FUA for
ATA drives that support it by default as there were concerns that
old drives would break. The series howeverfixes several issues with
the FUA support to ensure that FUA is reported as being supported
only for drives that can handle all possible write cases (NCQ and
non-NCQ). A check in the block layer is also added to ensure that we
never see read FUA commands (current behavior). From me.
* Several patches to move the old PARIDE (parallel port IDE) driver to
libata as pata_parport. Given that this driver also needs protocol
modules, the driver code resides in its own pata_parport directoy
under drivers/ata. From Ondrej.
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQSRPv8tYSvhwAzJdzjdoc3SxdoYdgUCY/VTnQAKCRDdoc3SxdoY
dk77AQCA1frczKhcOFe2PK/FsFAiO9Nlx/snk7V95JdjVG8GlwEAkey7mvbXMfX0
fDbqpaCkWFb6SvwxdMSATlqUvwEpSQ8=
=tqQP
-----END PGP SIGNATURE-----
Merge tag 'ata-6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata
Pull ATA updates from Damien Le Moal:
- Small cleanup of the pata_octeon driver to drop a useless platform
callback (Uwe)
- Simplify ata_scsi_cmd_error_handler() code using the fact that
ap->ops->error_handler is NULL most of the time (Wenchao)
- Several patches improving libata error handling. This is in
preparation for supporting the command duration limits (CDL) feature.
The changes allow handling corner cases of ATA NCQ errors which do
not happen with regular drives but will be triggered with CDL drives
(Niklas)
- Simplify the qc_fill_rtf operation (me)
- Improve SCSI command translation for REPORT_SUPPORTED_OPERATION_CODES
command (me)
- Cleanup of libata FUA handling.
This falls short of enabling FUA for ATA drives that support it by
default as there were concerns that old drives would break. The
series however fixes several issues with the FUA support to ensure
that FUA is reported as being supported only for drives that can
handle all possible write cases (NCQ and non-NCQ). A check in the
block layer is also added to ensure that we never see read FUA
commands (current behavior) (me)
- Several patches to move the old PARIDE (parallel port IDE) driver to
libata as pata_parport. Given that this driver also needs protocol
modules, the driver code resides in its own pata_parport directoy
under drivers/ata (Ondrej)
* tag 'ata-6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata:
ata: pata_parport: Fix ida_alloc return value error check
drivers/block: Move PARIDE protocol modules to drivers/ata/pata_parport
drivers/block: Remove PARIDE core and high-level protocols
ata: pata_parport: add driver (PARIDE replacement)
ata: libata: exclude FUA support for known buggy drives
ata: libata: Fix FUA handling in ata_build_rw_tf()
ata: libata: cleanup fua support detection
ata: libata: Rename and cleanup ata_rwcmd_protocol()
ata: libata: Introduce ata_ncq_supported()
block: add a sanity check for non-write flush/fua bios
ata: libata-scsi: improve ata_scsiop_maint_in()
ata: libata-scsi: do not overwrite SCSI ML and status bytes
ata: libata: move NCQ related ATA_DFLAGs
ata: libata: respect successfully completed commands during errors
ata: libata: read the shared status for successful NCQ commands once
ata: libata: simplify qc_fill_rtf port operation interface
ata: scsi: rename flag ATA_QCFLAG_FAILED to ATA_QCFLAG_EH
ata: libata-eh: Cleanup ata_scsi_cmd_error_handler()
ata: octeon: Drop empty platform remove function
This is a follow-up to the deprecation of most of the old-style board
files that was merged in linux-6.0, removing them for good.
This branch is almost exclusively dead code removal based on those
annotations. Some device driver removals went through separate subsystem
trees, but the majority is in the same branch, in order to better handle
dependencies between the patches and avoid breaking bisection.
Unfortunately that leads to merge conflicts against other changes in the
subsystem trees, but they should all be trivial to resolve by removing
the files.
See commit 7d0d3fa733 ("Merge tag 'arm-boardfiles-6.0' of
git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc") for the
description of which machines were marked unused and are now removed. The
only removals that got postponed are Terastation WXL (mv78xx0) and
Jornada720 (StrongARM1100), which turned out to still have potential
users.
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEEo6/YBQwIrVS28WGKmmx57+YAGNkFAmPvuCEACgkQmmx57+YA
GNm04Q//Q1W+qDOpK09BPskn7sFrpo1OOt9C+qRmAOmqZ/qY8JNfoqOLWLjS12st
qaTcODuSooGfFclWHsN5gNqT6yNfs3d2rRQEAd5ka+vt2dgV3OignNu1iEvjJmtG
sDxLHu1XYlHETz3k3pBGVv22SyuZTRowj1bdlerEBfOXgvJsxg1LkZowU+ffEau5
7LJeHwEGoi3LdfW/pVeNRU6iLwiBThVIXq94ZrOXsw1WNy4Bz6kmHfhlMis7hbhk
6X3JJCpDbtJp/4jccZFC/+Cc5DxYc1nnvkWGdUSpZWq3liWaNI0AoKm40p0vwdKa
ozflhYjM9PpB3JibwdkvkOrPj4GWOEHojKP1agN0fPBxEaWppmDpi7rbDU8Jvfxj
AwBM60fblqn6E+1HbckNpgyFx7rldcipmgQLPo5/ZhUnvad8Os0GLxmrH8Nqcycx
LktPcwOPJxd0mtaboHWc9qfeb5jeKqyEfQdhIN7H+u5HDEYA7EbcrhYAdMdmkduw
9C8sfTXQaD9/3/XBaq3elvTEVqNF1iOVwkXpbFUPjBNq9gQ2jHe5gxMuyoZ6lFz2
SnYMBo8DF+3EP5+UR6MgpbVn4zntk6o5hwbb6CZZGp9KXXic4kohh58nv8aQOOvx
Iy0Xxr38eXINAn4vsro89pFDmulpP1m7MKC1Cfw/9RZl4s/r0hg=
=WejQ
-----END PGP SIGNATURE-----
Merge tag 'arm-boardfile-remove-6.3' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc
Pull ARM SoC boardfile updates from Arnd Bergmann
"Unused boardfile removal for 6.3
This is a follow-up to the deprecation of most of the old-style board
files that was merged in linux-6.0, removing them for good.
This branch is almost exclusively dead code removal based on those
annotations. Some device driver removals went through separate
subsystem trees, but the majority is in the same branch, in order to
better handle dependencies between the patches and avoid breaking
bisection.
Unfortunately that leads to merge conflicts against other changes in
the subsystem trees, but they should all be trivial to resolve by
removing the files.
See commit 7d0d3fa733 ("Merge tag 'arm-boardfiles-6.0' of
git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc") for the
description of which machines were marked unused and are now removed.
The only removals that got postponed are Terastation WXL (mv78xx0) and
Jornada720 (StrongARM1100), which turned out to still have potential
users"
* tag 'arm-boardfile-remove-6.3' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc: (91 commits)
mmc: omap: drop TPS65010 dependency
ARM: pxa: restore mfp-pxa320.h
usb: ohci-omap: avoid unused-variable warning
ARM: debug: remove references in DEBUG_UART_8250_SHIFT to removed configs
ARM: s3c: remove obsolete s3c-cpu-freq header
MAINTAINERS: adjust SAMSUNG SOC CLOCK DRIVERS after s3c24xx support removal
MAINTAINERS: update file entries after arm multi-platform rework and mach-pxa removal
ARM: remove CONFIG_UNUSED_BOARD_FILES
mfd: remove htc-pasic3 driver
w1: remove ds1wm driver
usb: remove ohci-tmio driver
fbdev: remove w100fb driver
fbdev: remove tmiofb driver
mmc: remove tmio_mmc driver
mfd: remove ucb1400 support
mfd: remove toshiba tmio drivers
rtc: remove v3020 driver
power: remove pda_power supply driver
ASoC: pxa: remove unused board support
pcmcia: remove unused pxa/sa1100 drivers
...
Fix a slew of kernel-doc warnings in pata_octeon_cf.c by changing
all "/**" comments to "/*" since they are not in kernel-doc format.
Fixes: 3c929c6f5a ("libata: New driver for OCTEON SOC Compact Flash interface (v7).")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/all/202302101722.5O56RClE-lkp@intel.com/
Cc: David Daney <ddaney@caviumnetworks.com>
Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-ide@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Mark the Tiger Lake UP{3,4} AHCI controller as "low_power". This enables
S0ix to work out of the box. Otherwise this isn't working unless the
user manually sets /sys/class/scsi_host/*/link_power_management_policy.
Intel lists a total of 4 SATA controller IDs in [1] for those mobile
PCHs. This commit just adds the "AHCI" variant since I only tested
those.
[1]: https://cdrdv2.intel.com/v1/dl/getContent/631119
Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
CC: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Samsung MZ7LH drives are spewing messages like this in to dmesg with AMD
SATA controllers:
ata1.00: exception Emask 0x0 SAct 0x7e0000 SErr 0x0 action 0x6 frozen
ata1.00: failed command: SEND FPDMA QUEUED
ata1.00: cmd 64/01:88:00:00:00/00:00:00:00:00/a0 tag 17 ncq dma 512 out
res 40/00:01:01:4f:c2/00:00:00:00:00/00 Emask
0x4 (timeout)
Since this was seen previously with SSD 840 EVO drives in
https://bugzilla.kernel.org/show_bug.cgi?id=203475 let's add the same
fix for these drives as the EVOs have, since they likely have very
similar firmwares.
Signed-off-by: Patrick McLean <chutzpah@gentoo.org>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
pi->dev.id is unsigned so error checking of ida_alloc return value does
not work. Fix it.
Signed-off-by: Ondrej Zary <linux@zary.sk>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Commit 2dc0b46b5e ("libata: sata_down_spd_limit should return if
driver has not recorded sstatus speed") changed the behavior of
sata_down_spd_limit() to return doing nothing if a drive does not report
a current link speed, to avoid reducing the link speed to the lowest 1.5
Gbps speed.
However, the change assumed that a speed was recorded before probing
(e.g. before a suspend/resume) and set in link->sata_spd. This causes
problems with adapters/drives combination failing to establish a link
speed during probe autonegotiation. One example reported of this problem
is an mvebu adapter with a 3Gbps port-multiplier box: autonegotiation
fails, leaving no recorded link speed and no reported current link
speed. Probe retries also fail as no action is taken by sata_set_spd()
after each retry.
Fix this by returning early in sata_down_spd_limit() only if we do have
a recorded link speed, that is, if link->sata_spd is not 0. With this
fix, a failed probe not leading to a recorded link speed is retried at
the lower 1.5 Gbps speed, with the link speed potentially increased
later on the second revalidate of the device if the device reports
that it supports higher link speeds.
Reported-by: Marius Dinu <marius@psihoexpert.ro>
Fixes: 2dc0b46b5e ("libata: sata_down_spd_limit should return if driver has not recorded sstatus speed")
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Tested-by: Marius Dinu <marius@psihoexpert.ro>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Move PARIDE protocol modules out of drivers/block into
drivers/ata/pata_parport and update the CONFIG_ symbol names to
PATA_PARPORT.
[Damien]
The pata_parport driver file itsef is also moved together with the
protocol modules in drivers/ata/pata_parport.
Signed-off-by: Ondrej Zary <linux@zary.sk>
Acked-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Remove PARIDE core and high level protocols, taking care not to break
low-level drivers (used by pata_parport). Also update documentation.
Signed-off-by: Ondrej Zary <linux@zary.sk>
Acked-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
The pata_parport is a libata-based replacement of the old PARIDE
subsystem - driver for parallel port IDE devices.
It uses the original paride low-level protocol drivers but does not
need the high-level drivers (pd, pcd, pf, pt, pg). The IDE devices
behind parallel port adapters are handled by the ATA layer.
This will allow paride and its high-level drivers to be removed.
Unfortunately, libata drivers cannot sleep so pata_parport claims
parport before activating the ata host and keeps it claimed (and
protocol connected) until the ata host is removed. This means that
no devices can be chained (neither other pata_parport devices nor
a printer).
paride and pata_parport are mutually exclusive because the compiled
protocol drivers are incompatible.
Tested with:
- Imation SuperDisk LS-120 and HP C4381A (EPAT)
- Freecom Parallel CD (FRPW)
- Toshiba Mobile CD-RW 2793008 w/Freecom Parallel Cable rev.903 (FRIQ)
- Backpack CD-RW 222011 and CD-RW 19350 (BPCK6)
The following bugs in low-level protocol drivers were found and will
be fixed later:
Note: EPP-32 mode is buggy in EPAT - and also in all other protocol
drivers - they don't handle non-multiple-of-4 block transfers
correctly. This causes problems with LS-120 drive.
There is also another bug in EPAT: EPP modes don't work unless a 4-bit
or 8-bit mode is used first (probably some initialization missing?).
Once the device is initialized, EPP works until power cycle.
So after device power on, you have to:
echo "parport0 epat 0" >/sys/bus/pata_parport/new_device
echo pata_parport.0 >/sys/bus/pata_parport/delete_device
echo "parport0 epat 4" >/sys/bus/pata_parport/new_device
(autoprobe will initialize correctly as it tries the slowest modes
first but you'll get the broken EPP-32 mode)
Note: EPP modes are buggy in FRPW, only modes 0 and 1 work.
Signed-off-by: Ondrej Zary <linux@zary.sk>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Acked-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
The PXA palmld machine was removed, so the pata driver is no
longer used and can be removed. There is a chance that some of
this code might be useful for turning some of the other PXA
PCMCIA host drivers into PATA drivers, but it's clear that
it would not work unmodified, and it seems unlikely that
someone would do this work.
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Marek Vasut <marek.vasut@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-ide@vger.kernel.org
Acked-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
This device was only used by the smdk6410 board file that is now
gone, so the driver can be removed as well.
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
scsi_execute_req() is going to be removed. Convert libata to
scsi_execute_cmd().
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Acked-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
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>
Thread [1] reported back in 2012 problems with enabling FUA for 3
different drives. Add these drives to ata_device_blacklist[] to mark
them with the ATA_HORKAGE_NO_FUA flag. To be conservative and avoid
problems on old systems, the model number for the three new entries
are defined as to widely match all drives in the same product line.
[1]: https://lore.kernel.org/lkml/CA+6av4=uxu_q5U_46HtpUt=FSgbh3pZuAEY54J5_xK=MKWq-YQ@mail.gmail.com/
Suggested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
If a user issues a write command with the FUA bit set for a device with
NCQ support disabled (that is, the device queue depth was set to 1), the
LBA 48 command WRITE DMA FUA EXT must be used. However,
ata_build_rw_tf() ignores this and first tests if LBA 28 can be used
based on the write command sector and number of blocks. That is, for
small FUA writes at low LBAs, ata_rwcmd_protocol() will cause the write
to fail.
Fix this by preventing the use of LBA 28 for any FUA write request.
Given that the WRITE MULTI FUA EXT command is marked as obsolete in the
ATA specification since ACS-3 (published in 2013), remove the
ATA_CMD_WRITE_MULTI_FUA_EXT command from the ata_rw_cmds array.
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Move the detection of a device FUA support from
ata_scsiop_mode_sense()/ata_dev_supports_fua() to device scan time in
ata_dev_configure().
The function ata_dev_config_fua() is introduced to detect if a device
supports FUA and this support is indicated using the new device flag
ATA_DFLAG_FUA.
In order to blacklist known buggy devices, the horkage flag
ATA_HORKAGE_NO_FUA is introduced. Similarly to other horkage flags, the
libata.force= arguments "fua" and "nofua" are also introduced to allow
a user to control this horkage flag through the "force" libata
module parameter.
The ATA_DFLAG_FUA device flag is set only and only if all the following
conditions are met:
* libata.fua module parameter is set to 1
* The device supports the WRITE DMA FUA EXT command,
* The device is not marked with the ATA_HORKAGE_NO_FUA flag, either from
the blacklist or set by the user with libata.force=nofua
* The device supports NCQ (while this is not mandated by the standards,
this restriction is introduced to avoid problems with older non-NCQ
devices).
Enabling or diabling libata FUA support for all devices can now also be
done using the "force=[no]fua" module parameter when libata.fua is set
to 1.
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Rename ata_rwcmd_protocol() to ata_set_rwcmd_protocol() to better
reflect the fact that this function sets a task file command and
protocol. The arguments order is also reversed and the function return
type changed to a bool to indicate if the command and protocol were set
correctly (instead of returning a completely arbitrary "-1" value.
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Allow translation of REPORT_SUPPORTED_OPERATION_CODES commands using
the command format 0x3, that is, checking support for commands that are
identified using an opcode and a service action.
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
For SCSI ML byte:
In the case where a command is completed via libata EH:
irq -> ata_qc_complete() -> ata_qc_schedule_eh()
irq done
... -> ata_do_eh() -> ata_eh_link_autopsy() -> ata_eh_finish() ->
ata_eh_qc_complete() -> __ata_eh_qc_complete() -> __ata_qc_complete() ->
qc->complete_fn() (ata_scsi_qc_complete()) -> ata_qc_done() ->
qc->scsidone() (empty stub)
... -> scsi_eh_finish_cmd() -> scsi_eh_flush_done_q() ->
scsi_finish_command()
ata_eh_link_autopsy() will call ata_eh_analyze_tf(), which calls
scsi_check_sense(), which sets the SCSI ML byte.
Since ata_scsi_qc_complete() is called after scsi_check_sense() when
a command is completed via libata EH, we cannot simply overwrite the
SCSI ML byte that was set earlier in the call chain.
For SCSI status byte:
When a SCSI command is prepared using scsi_prepare_cmd(), it sets
cmd->result to 0. (SAM_STAT_GOOD is defined as 0x0).
Likewise, when a command is requeued from SCSI EH, scsi_queue_insert()
is called, which sets cmd->result to 0.
A SCSI command thus always has a GOOD status by default when being
sent to libata.
If libata fetches sense data from the device, it will call
ata_scsi_set_sense(), which will set the status byte to
SAM_STAT_CHECK_CONDITION, if the caller deems that the status should be
a check condition.
ata_scsi_qc_complete() should therefore never overwrite the existing
status byte, because if it is != GOOD, it was set by libata itself,
for a reason.
For the host byte:
When libata abort commands, because of a NCQ error, it will schedule
SCSI EH for all QCs using blk_abort_request(), which will all end up in
scsi_timeout(), which will call scsi_abort_command(). scsi_timeout()
sets DID_TIME_OUT regardless if a command was aborted or timed out.
If we don't clear the DID_TIME_OUT byte for the QC that caused the
NCQ error, that QC will be reported as a timed out command, instead
of being reported as a NCQ error.
For a command that actually timed out, DID_TIME_OUT would be fine to
keep, but libata has its own way of detecting that a command timed out
(see ata_scsi_cmd_error_handler()), and sets AC_ERR_TIMEOUT if that is
the case. libata will retry timed out commands.
We could clear DID_TIME_OUT only for the QC that caused the NCQ error,
but since libata has its own way of detecting timeouts, simply clear it
always.
Note that the existing ata_scsi_qc_complete() code does:
cmd->result = SAM_STAT_CHECK_CONDITION or cmd->result = SAM_STAT_GOOD.
This WILL clear the host byte. So us clearing the host byte
unconditionally is in line with the existing libata behavior.
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
In AHCI specification 1.3.1:
"5.5.3 Processing Completed Commands"
"For each port that has an interrupt pending:
1. Software determines the cause of the interrupt by reading the PxIS
register. It is possible for multiple bits to be set.
2. Software clears appropriate bits in the PxIS register corresponding
to the cause of the interrupt.
3. Software clears the interrupt bit in IS.IPS corresponding to the port.
4. If executing non-queued commands, software reads the PxCI register,
and compares the current value to the list of commands previously
issued by software that are still outstanding. If executing native
queued commands, software reads the PxSACT register and compares the
current value to the list of commands previously issued by software.
Software completes with success any outstanding command whose
corresponding bit has been cleared in the respective register. PxCI
and PxSACT are volatile registers; software should only use their
values to determine commands that have completed, not to determine
which commands have previously been issued.
5. If there were errors, noted in the PxIS register, software performs
error recovery actions (see section 6.2.2)."
The documentation for the PxSACT shadow register in AHCI:
"The device clears bits in this field by sending a Set Device Bits FIS
to the host. The HBA clears bits in this field that are set to ‘1’ in
the SActive field of the Set Device Bits FIS. The HBA only clears bits
that correspond to native queued commands that have completed
successfully."
Additionally, in SATA specification 3.5a:
"11.15 FPDMA QUEUED command protocol"
"DFPDMAQ11: ERROR
Halt command processing and transmit Set Device Bits FIS to host
with the ERR bit in Status field set to one, Interrupt bit set to one,
ATA error code set to one in the ERROR field, bits in ACT field cleared
to zero for any outstanding queued commands, and bits set to one
for any successfully completed queued commands that completion
notification not yet delivered."
I.e. even when the HBA triggers an error interrupt, the HBA will still
clear successfully completed commands in PxSACT. Commands that did not
complete successfully will still have its bit set in PxSACT.
(Which means the command that caused the NCQ error and queued commands
that had not yet finished at the time when the NCQ error occurred.)
Additionally, for a HBA that does not have the libata flag
AHCI_HFLAG_MULTI_MSI set, all ap->locks will point to host->lock, which
means that IRQs will be disabled for one port while another port's IRQ
handler is running. The HBA will still receive FISes from the device,
even if IRQs on the HBA itself are disabled. What can thus e.g. receive
a FIS that completes several commands successfully, followed by a FIS
that does (or does not) complete additional commands with the error bit
set, to indicate that at least one command was aborted.
Therefore, modify ahci_handle_port_interrupt() using the new helper
ahci_qc_complete() to complete the commands that have already been
signaled as successfully through a regular completion SDB FIS, as not
doing so would simply cause successfully completed commands to be
retried for no good reason.
Co-developed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Currently, the status is being read for each QC, inside
ata_qc_complete(), which means that QCs being completed by
ata_qc_complete_multiple() (i.e. multiple QCs completed during a single
interrupt), can have different status and error bits set. This is
because the FIS Receive Area will get updated as soon as the HBA
receives a new FIS from the device in the NCQ case.
Here is an example of the problem:
ata14.00: ata_qc_complete_multiple: done_mask: 0x180000
qc tag: 19 cmd: 0x61 flags: 0x11b err_mask: 0x0 tf->status: 0x40
qc tag: 20 cmd: 0x61 flags: 0x11b err_mask: 0x0 tf->status: 0x43
A print in ata_qc_complete_multiple(), shows that done_mask is: 0x180000
which means that tag 19 and 20 were completed. Another print in
ata_qc_complete(), after the call to fill_result_tf(), shows that tag 19
and 20 have different status values, even though they were completed in
the same ata_qc_complete_multiple() call.
If PMP is not enabled, simply read the status and error once, before
calling ata_qc_complete() for each QC. Without PMP, we know that all QCs
must share the same status and error values.
If PMP is enabled, we also read the status before calling
ata_qc_complete(), however, we still read the status for each QC, since
the QCs can belong to different PMP links (which means that the QCs
does not necessarily share the same status and error values).
Do all this by introducing the new port operation .qc_ncq_fill_rtf. If
set, this operation is called in ata_qc_complete_multiple() to set the
result tf for all completed QCs signaled by the last SDB FIS received.
QCs that have their result tf filled are marked with the new flag
ATA_QCFLAG_RTF_FILLED so that any later execution of the qc_fill_rtf
port operation does nothing (e.g. when called from ata_qc_complete()).
Co-developed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
The boolean return value of the qc_fill_rtf operation is used nowhere.
Simplify this operation interface by making it a void function. All
drivers defining this operation are also updated.
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
The name ATA_QCFLAG_FAILED is misleading since it does not mean that a
QC completed in error, or that it didn't complete at all. It means that
libata decided to schedule EH for the QC, so the QC is now owned by the
libata error handler (EH).
The normal execution path is responsible for not accessing a QC owned
by EH. libata core enforces the rule by returning NULL from
ata_qc_from_tag() for QCs owned by EH.
It is quite easy to mistake that a QC marked with ATA_QCFLAG_FAILED was
an error. However, a QC that was actually an error is instead indicated
by having qc->err_mask set. E.g. when we have a NCQ error, we abort all
QCs, which currently will mark all QCs as ATA_QCFLAG_FAILED. However, it
will only be a single QC that is an error (i.e. has qc->err_mask set).
Rename ATA_QCFLAG_FAILED to ATA_QCFLAG_EH to more clearly highlight that
this flag simply means that a QC is now owned by EH. This new name will
not mislead to think that the QC was an error (which is instead
indicated by having qc->err_mask set).
This also makes it more obvious that the EH code skips all QCs that do
not have ATA_QCFLAG_EH set (rather than ATA_QCFLAG_FAILED), since the EH
code should simply only care about QCs that are owned by EH itself.
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
If ap->ops->error_handler is NULL just return. This patch also
fixes some comment style issue.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
A remove callback just returning 0 is equivalent to no remove callback
at all. So drop the useless function.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
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 c312ef1763 ("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
09d6ac8dc5 ("libata/ahci: Fix PCS quirk application") landed in 5.3.8.
So testing my patch anywhere between commits c312ef1763 and
09d6ac8dc5 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 c312ef1763 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: c312ef1763 ("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>
ata changes fro 6.2 include 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 (from 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 (from Niklas).
- Cleanup of the pata_mpc52xx and sata_dwc_460ex drivers to remove the
use of the deprecated NO_IRQ macro (from 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
(from me).
- Some libata cleanups using the new helper function
ata_port_is_frozen() (from Niklas).
- Improve internal command handling by not retrying commands that
failed with a timeout (from 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 (from 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
(from Thomas).
- Changes to libata enum constants definitions to avoid warnings with
gcc-13 (from Arnd).
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQSRPv8tYSvhwAzJdzjdoc3SxdoYdgUCY5aregAKCRDdoc3SxdoY
dlfSAQCeTLQP9qBrmSUZnP5G5XOHHcxp5maXKWBrPFVsOhTmLQD/WxEGDzgEnnPe
m8hKvBcqTQIn2QRGCiXRnYAiG9Om0Qo=
=bJZM
-----END PGP SIGNATURE-----
Merge tag 'ata-6.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata
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
...
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 4ba09d2026 ("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: 4ba09d2026 ("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 e494f6a728 ("[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 e494f6a728 ("[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 e494f6a728 ("[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: e494f6a728 ("[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>
In ata_tdev_add(), the return value of transport_add_device() is
not checked. As a result, it causes null-ptr-deref while removing
the module, because transport_remove_device() is called to remove
the device that was not added.
Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0
CPU: 13 PID: 13603 Comm: rmmod Kdump: loaded Tainted: G W 6.1.0-rc3+ #36
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : device_del+0x48/0x3a0
lr : device_del+0x44/0x3a0
Call trace:
device_del+0x48/0x3a0
attribute_container_class_device_del+0x28/0x40
transport_remove_classdev+0x60/0x7c
attribute_container_device_trigger+0x118/0x120
transport_remove_device+0x20/0x30
ata_tdev_delete+0x24/0x50 [libata]
ata_tlink_delete+0x40/0xa0 [libata]
ata_tport_delete+0x2c/0x60 [libata]
ata_port_detach+0x148/0x1b0 [libata]
ata_pci_remove_one+0x50/0x80 [libata]
ahci_remove_one+0x4c/0x8c [ahci]
Fix this by checking and handling return value of transport_add_device()
in ata_tdev_add(). In the error path, device_del() is called to delete
the device which was added earlier in this function, and ata_tdev_free()
is called to free ata_dev.
Fixes: d9027470b8 ("[libata] Add ATA transport class")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
In ata_tlink_add(), the return value of transport_add_device() is
not checked. As a result, it causes null-ptr-deref while removing
the module, because transport_remove_device() is called to remove
the device that was not added.
Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0
CPU: 33 PID: 13850 Comm: rmmod Kdump: loaded Tainted: G W 6.1.0-rc3+ #12
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : device_del+0x48/0x39c
lr : device_del+0x44/0x39c
Call trace:
device_del+0x48/0x39c
attribute_container_class_device_del+0x28/0x40
transport_remove_classdev+0x60/0x7c
attribute_container_device_trigger+0x118/0x120
transport_remove_device+0x20/0x30
ata_tlink_delete+0x88/0xb0 [libata]
ata_tport_delete+0x2c/0x60 [libata]
ata_port_detach+0x148/0x1b0 [libata]
ata_pci_remove_one+0x50/0x80 [libata]
ahci_remove_one+0x4c/0x8c [ahci]
Fix this by checking and handling return value of transport_add_device()
in ata_tlink_add().
Fixes: d9027470b8 ("[libata] Add ATA transport class")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
In ata_tport_add(), the return value of transport_add_device() is
not checked. As a result, it causes null-ptr-deref while removing
the module, because transport_remove_device() is called to remove
the device that was not added.
Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0
CPU: 12 PID: 13605 Comm: rmmod Kdump: loaded Tainted: G W 6.1.0-rc3+ #8
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : device_del+0x48/0x39c
lr : device_del+0x44/0x39c
Call trace:
device_del+0x48/0x39c
attribute_container_class_device_del+0x28/0x40
transport_remove_classdev+0x60/0x7c
attribute_container_device_trigger+0x118/0x120
transport_remove_device+0x20/0x30
ata_tport_delete+0x34/0x60 [libata]
ata_port_detach+0x148/0x1b0 [libata]
ata_pci_remove_one+0x50/0x80 [libata]
ahci_remove_one+0x4c/0x8c [ahci]
Fix this by checking and handling return value of transport_add_device()
in ata_tport_add().
Fixes: d9027470b8 ("[libata] Add ATA transport class")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
In the error path in ata_tport_add(), when calling put_device(),
ata_tport_release() is called, it will put the refcount of 'ap->host'.
And then ata_host_put() is called again, the refcount is decreased
to 0, ata_host_release() is called, all ports are freed and set to
null.
When unbinding the device after failure, ata_host_stop() is called
to release the resources, it leads a null-ptr-deref(), because all
the ports all freed and null.
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
CPU: 7 PID: 18671 Comm: modprobe Kdump: loaded Tainted: G E 6.1.0-rc3+ #8
pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : ata_host_stop+0x3c/0x84 [libata]
lr : release_nodes+0x64/0xd0
Call trace:
ata_host_stop+0x3c/0x84 [libata]
release_nodes+0x64/0xd0
devres_release_all+0xbc/0x1b0
device_unbind_cleanup+0x20/0x70
really_probe+0x158/0x320
__driver_probe_device+0x84/0x120
driver_probe_device+0x44/0x120
__driver_attach+0xb4/0x220
bus_for_each_dev+0x78/0xdc
driver_attach+0x2c/0x40
bus_add_driver+0x184/0x240
driver_register+0x80/0x13c
__pci_register_driver+0x4c/0x60
ahci_pci_driver_init+0x30/0x1000 [ahci]
Fix this by removing redundant ata_host_put() in the error path.
Fixes: 2623c7a5f2 ("libata: add refcounting to ata_host")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Nobody seems to call ata_sff_busy_sleep(), so we can get rid of it...
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
SAT SCSI/ATA Translation specification requires SCSI SYNCHRONIZE CACHE
(10) and (16) commands both shall be translated to ATA flush command.
Also, ZBC Zoned Block Commands specification mandates SYNCHRONIZE CACHE
(16) command support. However, libata translates only SYNCHRONIZE CACHE
(10). This results in SYNCHRONIZE CACHE (16) command failures on SATA
drives and then libata translation does not conform to ZBC. To avoid the
failure, add support for SYNCHRONIZE CACHE (16).
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
If devm_platform_ioremap_resource() fails, it never return
NULL pointer, replace the check with IS_ERR().
Fixes: 57bf0f5a16 ("ARM: pxa: use pdev resource for palmld mmio")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Clang gives a warning when compiling pata_legacy.c with 'make W=1' about
the 'rt' local variable in pdc20230_set_piomode() being set but unused.
Quite obviously, there is an outb() call missing to write back the updated
variable. Moreover, checking the docs by Petr Soucek revealed that bitwise
AND should have been done with a negated timing mask and the master/slave
timing masks were swapped while updating...
Fixes: 669a5db411 ("[libata] Add a bunch of PATA drivers.")
Reported-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Variable num_processed is just being incremented and it's never used
anywhere else. The variable and the increment are redundant so
remove it.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
This device was used only on the davinci dm644x platform that
is now gone, and no references to the device remain in the
kernel.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Marc Zyngier <maz@kernel.org>
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
ata_read_log_page() first tries to read the log using READ LOG DMA EXT.
If that fails it will instead try to read the log using READ LOG EXT.
ata_exec_internal_sg() is synchronous, so it will wait for the command to
finish. If we actually got an error back from the device, it is correct
to retry. However, if the command timed out, ata_exec_internal_sg() will
freeze the port.
There is no point in retrying if the port is frozen, as
ata_exec_internal_sg() will return AC_ERR_SYSTEM on a frozen port,
without ever sending the command down to the drive.
Therefore, avoid retrying if the first command froze the port, as that
will result in a misleading AC_ERR_SYSTEM error print, instead of printing
the error that actually caused the port to be frozen (AC_ERR_TIMEOUT).
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
