Workaround proposition for pre-2.0 controllers
Dmytro Bagrii <[email protected]>
| Newsgroups | org.infradead.lists.linux-nvme |
|---|---|
| Message-ID | <[email protected]> |
Hi All,
There is a known issue with pre-2.0 controllers that causes non-volatile
ErrCount to increment at every boot. An example of such controller is
144d:a808 SM981/PM981/PM983 NVMe SSD Controller found in Samsung SSD 970
PRO.
This is definitely a flaw of firmware implementations that treat Invalid
Field in Command as an error worth loging and incrementing the counter,
rather than simply return an error code. However, it is unlikely that
firmwares for existing devices will ever be updated, especially by
manufacturers that no longer active.
The issue confuses userspace disk health monitoring tools and make users
prone to overlooking real errors.
My proposition is to allow to workaround the issue with a quirk flag.
Since it is known that specific controller does not support
nvme_admin_identify opcode with cns=5 and cns=6, issuing these commands
can be omitted. Using quirk module option allows users to decide whether
to workaround the issue or not.
Whether to add a quirk to nvme_id_table[] for known affected controllers
is out of scope of this proposition.
Option 1: re-use NVME_QUIRK_IDENTIFY_CNS by extending its effect for
versions >= 1.2.0, not only for 1.1.0 <= version < 1.2.0.
Pros: Minor code changes.
Cons:
It's not yet clear to me whether the driver can issue
nvme_admin_identify with cns > 6 if controller already rejected cns=5
and cns=6.
The impact for devices that already have NVME_QUIRK_IDENTIFY_CNS set
but claim support for version > 1.1.0 is unclear, if any such devices exist.
By the way, the comment for NVME_QUIRK_IDENTIFY_CNS in
drivers/nvme/host/nvme.h is misleading: it states that "The controller
doesn't handle Identify value others than 0 or 1 correctly" but
nvme_id_cns_ok() actually allows cns values up to 3.
Option 2: introduce another quirk flag.
Pros: Independent control, with no devices potentially affected by default.
Cons: More code changes.
I would also prefer to leave ioctl() behavior as is, allowing userspace
to issue nvme_admin_identify with any cns value.
I could try to prepare a patch if the idea isn't rejected outright.
--
With best regards,
Dmytro Bagrii