On 2021/08/16 1:27, Reimar Döffinger wrote: > The ATA_CMD_READ_LOG_DMA_EXT can cause controller/device to > become unresponsive until the next power cycle. > This seems to particularly affect IDE to SATA adapters, > possibly in combination with certain SATA SSDs, though > there might be more/different cases. > Comment 5 of https://bugzilla.kernel.org/show_bug.cgi?id=195895 > is an example. > Disable it by default on Crucial MX500 devices and all > PATA controllers. > Also add an option to set the workaround state, which might > help with gathering more data on the exact devices/controllers > affected, and speed up narrowing down the cause for users that > are affect but not covered by the added quirks. > Existing workarounds like forcing PIO mode do not work > (in addition to the performance issues) because READ_LOG_DMA > is issued even if PIO mode is forced. > > Signed-off-by: Reimar Döffinger <Reimar.Doeffinger@xxxxxx> > --- > Documentation/admin-guide/kernel-parameters.txt | 2 ++ > drivers/ata/libata-core.c | 15 +++++++++++++++ > 2 files changed, 17 insertions(+) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index bdb22006f713..191502e8fa74 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -2551,6 +2551,8 @@ > > * [no]ncqtrim: Turn off queued DSM TRIM. > > + * [no]dmalog: Turn off use of ATA_CMD_READ_LOG_DMA_EXT (0x47) command > + > * nohrst, nosrst, norst: suppress hard, soft > and both resets. > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index 61c762961ca8..219fa92ffc06 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -2004,7 +2004,11 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log, > > retry: > ata_tf_init(dev, &tf); > + /* Do not even attempt DMA with PATA-SATA adapters, they seem likely to > + * hang, see https://bugzilla.kernel.org/show_bug.cgi?id=195895 > + */ This is not the standard kernel comment style. Multi-lines comment should start with a "/*" line: + /* + * Do not even attempt DMA with PATA-SATA adapters, they seem likely to + * hang, see https://bugzilla.kernel.org/show_bug.cgi?id=195895 + */ Also, I would remove the bugzilla reference since it is in the commit message. > if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) && > + (ap_flags & ATA_FLAG_SATA) && Why is this necessary since you have the ATA_HORKAGE_NO_DMA_LOG flag ? If this is really necessary, ATA_HORKAGE_NO_DMA_LOG should be set for all devices that have ATA_FLAG_SATA when the device is initialized. With that, this additional condition can go away. > !(dev->horkage & ATA_HORKAGE_NO_DMA_LOG)) { > tf.command = ATA_CMD_READ_LOG_DMA_EXT; > tf.protocol = ATA_PROT_DMA; > @@ -4000,6 +4004,15 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = { > { "WDC WD3000JD-*", NULL, ATA_HORKAGE_WD_BROKEN_LPM }, > { "WDC WD3200JD-*", NULL, ATA_HORKAGE_WD_BROKEN_LPM }, > > + /* Devices with observed READ LOG DMA issues - unclear if only > + * specific firmware versions or only in combination with specific > + * controllers. > + * Specifically broken combinations reported > + * CT1000MX500SSD4, M3CR020 with B350M-Mortar > + * CT500MX500SSD4, M3CR023 with PATA-SATA adapter > + * https://bugzilla.kernel.org/show_bug.cgi?id=195895 > + */ Same comment about the comment format. I would also remove the bugzilla reference as it is in the commit message. > + { "Crucial_CT*MX500*", NULL, ATA_HORKAGE_NO_DMA_LOG }, > /* End Marker */ > { } > }; > @@ -6104,6 +6117,8 @@ static int __init ata_parse_force_one(char **cur, > { "ncq", .horkage_off = ATA_HORKAGE_NONCQ }, > { "noncqtrim", .horkage_on = ATA_HORKAGE_NO_NCQ_TRIM }, > { "ncqtrim", .horkage_off = ATA_HORKAGE_NO_NCQ_TRIM }, > + { "nodmalog", .horkage_on = ATA_HORKAGE_NO_DMA_LOG }, > + { "dmalog", .horkage_off = ATA_HORKAGE_NO_DMA_LOG }, > { "dump_id", .horkage_on = ATA_HORKAGE_DUMP_ID }, > { "pio0", .xfer_mask = 1 << (ATA_SHIFT_PIO + 0) }, > { "pio1", .xfer_mask = 1 << (ATA_SHIFT_PIO + 1) }, > -- Damien Le Moal Western Digital Research
