PCI_DEVICE(0x1217, 0x8760) (O2 Micro, Inc. FORESEE E2M2 NVMe SSD) is a NMVe to eMMC bridge, that can be used with different eMMC memory devices. The NVMe device name contains the eMMC device name, for instance: `BAYHUB SanDisk-DA4128-91904055-128GB`
The bridge is known to work with many eMMC devices, we need to limit the queue depth once we know which eMMC device is behind the bridge.
Fixes: commit 83bdfcbdbe5d ("nvme-pci: qdepth 1 quirk")
Cc: stable@vger.kernel.org Signed-off-by: Gwendal Grignou gwendal@chromium.org --- drivers/nvme/host/pci.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 4b9fda0b1d9a3..1c908e129fddf 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -3448,8 +3448,6 @@ static const struct pci_device_id nvme_id_table[] = { NVME_QUIRK_BOGUS_NID, }, { PCI_VDEVICE(REDHAT, 0x0010), /* Qemu emulated controller */ .driver_data = NVME_QUIRK_BOGUS_NID, }, - { PCI_DEVICE(0x1217, 0x8760), /* O2 Micro 64GB Steam Deck */ - .driver_data = NVME_QUIRK_QDEPTH_ONE }, { PCI_DEVICE(0x126f, 0x2262), /* Silicon Motion generic */ .driver_data = NVME_QUIRK_NO_DEEPEST_PS | NVME_QUIRK_BOGUS_NID, },
On Mon, Oct 28, 2024 at 07:42:36PM -0700, Gwendal Grignou wrote:
PCI_DEVICE(0x1217, 0x8760) (O2 Micro, Inc. FORESEE E2M2 NVMe SSD) is a NMVe to eMMC bridge, that can be used with different eMMC memory devices. The NVMe device name contains the eMMC device name, for instance: `BAYHUB SanDisk-DA4128-91904055-128GB`
The bridge is known to work with many eMMC devices, we need to limit the queue depth once we know which eMMC device is behind the bridge.
Fixes: commit 83bdfcbdbe5d ("nvme-pci: qdepth 1 quirk")
Nit, no need for "commit" here, and no blank line after this one.
thanks,
greg k-h
On Mon, Oct 28, 2024 at 07:42:36PM -0700, Gwendal Grignou wrote:
PCI_DEVICE(0x1217, 0x8760) (O2 Micro, Inc. FORESEE E2M2 NVMe SSD) is a NMVe to eMMC bridge, that can be used with different eMMC memory devices.
Holy f**k, what an awful idea..
The NVMe device name contains the eMMC device name, for instance: `BAYHUB SanDisk-DA4128-91904055-128GB`
The bridge is known to work with many eMMC devices, we need to limit the queue depth once we know which eMMC device is behind the bridge.
Please work with Tobert to quirk based on the identify data for "his" device to keep it quirked instead of regressing it.
On Tue, Oct 29, 2024 at 12:41 AM Christoph Hellwig hch@lst.de wrote:
On Mon, Oct 28, 2024 at 07:42:36PM -0700, Gwendal Grignou wrote:
PCI_DEVICE(0x1217, 0x8760) (O2 Micro, Inc. FORESEE E2M2 NVMe SSD) is a NMVe to eMMC bridge, that can be used with different eMMC memory devices.
Holy f**k, what an awful idea..
The NVMe device name contains the eMMC device name, for instance: `BAYHUB SanDisk-DA4128-91904055-128GB`
The bridge is known to work with many eMMC devices, we need to limit the queue depth once we know which eMMC device is behind the bridge.
Please work with Tobert to quirk based on the identify data for "his" device to keep it quirked instead of regressing it.
The issue is we would need to base the quirk on the model name (subsys->model) that is not available in `nvme_id_table`. Beside, `q_depth` is set in `nvme_pci_enable`, called at probe time before calling `nvme_init_ctrl_finish` that will indirectly populate `subsys`.
Bob, to address the data corruption problem from user space, adding a udev rule to set `queue/nr_requests` to 1 when `device/model` matches the device used in the Steam Deck would most likely be too late in the boot process, wouldn't it?
Gwendal.
On Tue, Oct 29, 2024 at 11:58:40AM -0700, Gwendal Grignou wrote:
On Tue, Oct 29, 2024 at 12:41 AM Christoph Hellwig hch@lst.de wrote:
On Mon, Oct 28, 2024 at 07:42:36PM -0700, Gwendal Grignou wrote:
PCI_DEVICE(0x1217, 0x8760) (O2 Micro, Inc. FORESEE E2M2 NVMe SSD) is a NMVe to eMMC bridge, that can be used with different eMMC memory devices.
Holy f**k, what an awful idea..
The NVMe device name contains the eMMC device name, for instance: `BAYHUB SanDisk-DA4128-91904055-128GB`
The bridge is known to work with many eMMC devices, we need to limit the queue depth once we know which eMMC device is behind the bridge.
Please work with Tobert to quirk based on the identify data for "his" device to keep it quirked instead of regressing it.
The issue is we would need to base the quirk on the model name (subsys->model) that is not available in `nvme_id_table`. Beside, `q_depth` is set in `nvme_pci_enable`, called at probe time before calling `nvme_init_ctrl_finish` that will indirectly populate `subsys`.
Bob, to address the data corruption problem from user space, adding a udev rule to set `queue/nr_requests` to 1 when `device/model` matches the device used in the Steam Deck would most likely be too late in the boot process, wouldn't it?
I think that is too late. There's the module parameter, 'nvme.io_queue_depth=2', that accomplishes the same thing as this quirk, if adding kernel parameters isn't too inconvenient to use here.
Alternatively, you could put the quirk in the core_quirks, which do take a model name. The pci driver would have to move this check until after init_ctrl_finish, as you noticed, but that looks okay to do. We just need to be careful to update both dev->q_depth and ctrl->sqsize after the "finish".
---- On Tue, 29 Oct 2024 18:58:40 +0000 Gwendal Grignou wrote ---
On Tue, Oct 29, 2024 at 12:41 AM Christoph Hellwig hch@lst.de> wrote:
On Mon, Oct 28, 2024 at 07:42:36PM -0700, Gwendal Grignou wrote:
PCI_DEVICE(0x1217, 0x8760) (O2 Micro, Inc. FORESEE E2M2 NVMe SSD) is a NMVe to eMMC bridge, that can be used with different eMMC memory devices.
Holy f**k, what an awful idea..
The NVMe device name contains the eMMC device name, for instance: `BAYHUB SanDisk-DA4128-91904055-128GB`
The bridge is known to work with many eMMC devices, we need to limit the queue depth once we know which eMMC device is behind the bridge.
Please work with Tobert to quirk based on the identify data for "his" device to keep it quirked instead of regressing it.
The issue is we would need to base the quirk on the model name (subsys->model) that is not available in `nvme_id_table`. Beside, `q_depth` is set in `nvme_pci_enable`, called at probe time before calling `nvme_init_ctrl_finish` that will indirectly populate `subsys`.
Bob, to address the data corruption problem from user space, adding a udev rule to set `queue/nr_requests` to 1 when `device/model` matches the device used in the Steam Deck would most likely be too late in the boot process, wouldn't it?
I've never seen the corruption outside of severe stress testing. In the field, people reported it during and after os image updates (more stress testing). However, as this concerns data integrity, it seems risky to me to rely on a fix after bootup.
Gwendal.
linux-stable-mirror@lists.linaro.org
-
Christoph Hellwig -
Greg KH -
Gwendal Grignou -
Keith Busch -
Robert Beckett