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.
On Tue, Oct 29, 2024 at 12:19 PM Robert Beckett bob.beckett@collabora.com wrote:
---- 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.
Since limiting the queue depth to 2 is only needed for a small subset of eMMC memory modules that can be connected behind the bridge, would it make sense to apply this patch, but add the kernel module parameter mentioned earlier for impacted devices?
Gwendal.
Gwendal.
On Thu, Dec 05, 2024 at 01:53:29PM -0800, Gwendal Grignou wrote:
Since limiting the queue depth to 2 is only needed for a small subset of eMMC memory modules that can be connected behind the bridge, would it make sense to apply this patch, but add the kernel module parameter mentioned earlier for impacted devices?
I don't think qd2 is needed for anyone, though. The problem sounds like it was root caused to a boundary condition, which is addressed in a different patch. But I am not 100% sure, as the thread was left hanging for some external confirmation:
https://lore.kernel.org/linux-nvme/Z0DdU9K9QMFxBIL8@kbusch-mbp.dhcp.thefaceb...
linux-stable-mirror@lists.linaro.org
-
Christoph Hellwig -
Greg KH -
Gwendal Grignou -
Keith Busch -
Robert Beckett