nvme APST fixes/improvements for 4.11

PATCH In review Andy Lutomirski 5 participants 14 messages Apr 20, 2017 NVMe

Share & tools

Andy Lutomirski

author

2017-04-20 03:02 UTC #0 lore raw

cover nvme APST fixes/improvements for 4.11

Sorry for waiting so long for this. I was waiting for feedback from Samsung, but they haven't root-caused the issue yet, and I should have just done this from the beginning.

This series makes APST more debuggable and updates the quirk list. The quirks I'm aware of are:

- Samsung 950 series SSDs in Dell XPS 15 9550 and Precision 5510 laptops (which are essentially the same laptop) can lose their PCIe link if they're allowed to use the deepest APST state. Samsung engineers have an affected system and are working on it. The same exact SSDs in other machines (even an XPS 13) seem to work fine.

- One Toshiba device malfunctions if APST is used at all.

One thing that improves my confidence that there aren't too many more problems with APST is that Ubuntu has backported APST to Zesty, so it's already gotten a bit of testing in a widely used (if very new) release.

Andy Lutomirski (5):
nvme: Fix APST comment
nvme: Display raw APST configuration via DYNAMIC_DEBUG
nvme: Add nvme_core.force_apst to ignore the NO_APST quirk
nvme: Adjust the Samsung APST quirk
nvme: Quirk APST off on "THNSF5256GPUK TOSHIBA"

 drivers/nvme/host/core.c | 61 ++++++++++++++++++++++++++++++++++++++++--------
 drivers/nvme/host/nvme.h |  5 ++++
 drivers/nvme/host/pci.c  | 26 ++++++++++++++++++++-
 3 files changed, 81 insertions(+), 11 deletions(-)

Signature · 1 line

2.9.3

Andy Lutomirski

author

2017-04-20 03:02 UTC #1 lore raw

PATCH 1/5 nvme: Fix APST comment

There was a typo in the description of the timeout heuristic.

Signed-off-by Andy Lutomirski <luto at kernel.org>

---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/nvme/host/core.c +1 -1

index 9583a5f58a1d..4fc58866640e 100644
@@ -1267,7 +1267,7 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 	 * heuristic: we are willing to spend at most 2% of the time
 	 * transitioning between power states.  Therefore, when running
 	 * in any given state, we will enter the next lower-power
-	 * non-operational state after waiting 100 * (enlat + exlat)
+	 * non-operational state after waiting 50 * (enlat + exlat)
 	 * microseconds, as long as that state's total latency is under
 	 * the requested maximum latency.
 	 *

Signature · 1 line

2.9.3

Andy Lutomirski

author

2017-04-20 03:02 UTC #2 lore raw

PATCH 2/5 nvme: Display raw APST configuration via DYNAMIC_DEBUG

In reply to Andy Lutomirski

Debugging APST is currently a bit of a pain. This gives optional simple log messages that describe the APST state.

The easiest way to use this is probably with the nvme_core.dyndbg=+p module parameter.

Signed-off-by Andy Lutomirski <luto at kernel.org>

---
 drivers/nvme/host/core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

drivers/nvme/host/core.c +16 -0

index 4fc58866640e..52b4e52b85a2 100644
@@ -1278,6 +1278,8 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 
 	unsigned apste;
 	struct nvme_feat_auto_pst *table;
+	u64 max_lat_us = 0;
+	int max_ps = -1;
 	int ret;
 
 	/*

@@ -1299,6 +1301,7 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 	if (ctrl->ps_max_latency_us == 0) {
 		/* Turn off APST. */
 		apste = 0;
+		dev_dbg(ctrl->device, "APST disabled\n");
 	} else {
 		__le64 target = cpu_to_le64(0);
 		int state;

@@ -1340,9 +1343,22 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 
 			target = cpu_to_le64((state << 3) |
 					     (transition_ms << 8));
+
+			if (max_ps == -1)
+				max_ps = state;
+
+			if (total_latency_us > max_lat_us)
+				max_lat_us = total_latency_us;
 		}
 
 		apste = 1;
+
+		if (max_ps == -1) {
+			dev_dbg(ctrl->device, "APST enabled but no non-operational states are available\n");
+		} else {
+			dev_dbg(ctrl->device, "APST enabled: max PS = %d, max round-trip latency = %lluus, table = %*phN\n",
+				max_ps, max_lat_us, (int)sizeof(*table), table);
+		}
 	}
 
 	ret = nvme_set_features(ctrl, NVME_FEAT_AUTO_PST, apste,

Signature · 1 line

2.9.3

Andy Lutomirski

author

2017-04-20 03:02 UTC #3 lore raw

PATCH 3/5 nvme: Add nvme_core.force_apst to ignore the NO_APST quirk

In reply to Andy Lutomirski

We're probably going to be stuck quirking APST off on an over-broad range of devices for 4.11. Let's make it easy to override the quirk for testing.

Signed-off-by Andy Lutomirski <luto at kernel.org>

---
 drivers/nvme/host/core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

drivers/nvme/host/core.c +14 -1

index 52b4e52b85a2..b422996f88ed 100644
@@ -62,6 +62,10 @@ module_param(default_ps_max_latency_us, ulong, 0644);
 MODULE_PARM_DESC(default_ps_max_latency_us,
 		 "max power saving latency for new devices; use PM QOS to change per device");
 
+static bool force_apst;
+module_param(force_apst, bool, 0644);
+MODULE_PARM_DESC(force_apst, "allow APST for newly enumerated devices even if quirked off");
+
 static LIST_HEAD(nvme_ctrl_list);
 static DEFINE_SPINLOCK(dev_list_lock);

@@ -1519,7 +1523,16 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 
 	ctrl->npss = id->npss;
 	prev_apsta = ctrl->apsta;
-	ctrl->apsta = (ctrl->quirks & NVME_QUIRK_NO_APST) ? 0 : id->apsta;
+	if (ctrl->quirks & NVME_QUIRK_NO_APST) {
+		if (force_apst && id->apsta) {
+			dev_warn(ctrl->dev, "forcibly allowing APST due to nvme_core.force_apst -- use at your own risk\n");
+			ctrl->apsta = 1;
+		} else {
+			ctrl->apsta = 0;
+		}
+	} else {
+		ctrl->apsta = id->apsta;
+	}
 	memcpy(ctrl->psd, id->psd, sizeof(ctrl->psd));
 
 	if (ctrl->ops->is_fabrics) {

Signature · 1 line

2.9.3

Andy Lutomirski

author

2017-04-20 03:02 UTC #4 lore raw

PATCH 4/5 nvme: Adjust the Samsung APST quirk

In reply to Andy Lutomirski

I got a couple more reports: the Samsung APST issues appears to affect multiple 950-series devices in Dell XPS 15 9550 and Precision 5510 laptops. Change the quirk: rather than blacklisting the firmware on the first problematic SSD that was reported, disable APST on all 144d:a802 devices if they're installed in the two affected Dell models. While we're at it, disable only the deepest sleep state instead of all of them -- the reporters say that this is sufficient to fix the problem.

(I have a device that appears to be entirely identical to one of the affected devices, but I have a different Dell laptop, so it's not the case that all Samsung devices with firmware BXW75D0Q are broken under all circumstances.)

Samsung engineers have an affected system, and hopefully they'll give us a better workaround some time soon. In the mean time, this should minimize regressions.

See https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1678184

Cc Kai-Heng Feng <kai.heng.feng at canonical.com>

Cc Niranjan Sivakumar <ns253 at cornell.edu>

Signed-off-by Andy Lutomirski <luto at kernel.org>

---
 drivers/nvme/host/core.c | 23 +++++++++++++----------
 drivers/nvme/host/nvme.h |  5 +++++
 drivers/nvme/host/pci.c  | 26 +++++++++++++++++++++++++-
 3 files changed, 43 insertions(+), 11 deletions(-)

drivers/nvme/host/core.c +13 ~~-10~~

index b422996f88ed..bec8c6973ae5 100644
@@ -1323,6 +1323,14 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 				table->entries[state] = target;
 
 			/*
+			 * Don't allow transitions to the deepest state
+			 * if it's quirked off.
+			 */
+			if (state == ctrl->npss &&
+			    (ctrl->quirks & NVME_QUIRK_NO_DEEPEST_PS))
+				continue;
+
+			/*
 			 * Is this state a useful non-operational state for
 			 * higher-power states to autonomously transition to?
 			 */

@@ -1407,16 +1415,6 @@ struct nvme_core_quirk_entry {
 };
 
 static const struct nvme_core_quirk_entry core_quirks[] = {
-	/*
-	 * Seen on a Samsung "SM951 NVMe SAMSUNG 256GB": using APST causes
-	 * the controller to go out to lunch.  It dies when the watchdog
-	 * timer reads CSTS and gets 0xffffffff.
-	 */
-	{
-		.vid = 0x144d,
-		.fr = "BXW75D0Q",
-		.quirks = NVME_QUIRK_NO_APST,
-	},
 };
 
 /* match is null-terminated but idstr is space-padded. */

@@ -1501,6 +1499,11 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 		}
 	}
 
+	if (force_apst && (ctrl->quirks & NVME_QUIRK_NO_DEEPEST_PS)) {
+		dev_warn(ctrl->dev, "forcibly allowing all power states due to nvme_core.force_apst -- use at your own risk\n");
+		ctrl->quirks &= ~NVME_QUIRK_NO_DEEPEST_PS;
+	}
+
 	ctrl->oacs = le16_to_cpu(id->oacs);
 	ctrl->vid = le16_to_cpu(id->vid);
 	ctrl->oncs = le16_to_cpup(&id->oncs);

drivers/nvme/host/nvme.h +5 -0

index 2aa20e3e5675..ab2d6ec7eb5c 100644
@@ -83,6 +83,11 @@ enum nvme_quirks {
 	 * APST should not be used.
 	 */
 	NVME_QUIRK_NO_APST			= (1 << 4),
+
+	/*
+	 * The deepest sleep state should not be used.
+	 */
+	NVME_QUIRK_NO_DEEPEST_PS		= (1 << 5),
 };
 
 /*

drivers/nvme/host/pci.c +25 -1

index 26a5fd05fe88..5d309535abbd 100644
@@ -19,6 +19,7 @@
 #include <linux/blk-mq-pci.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
+#include <linux/dmi.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
 #include <linux/genhd.h>

@@ -1943,10 +1944,31 @@ static int nvme_dev_map(struct nvme_dev *dev)
 	return -ENODEV;
 }
 
+static unsigned long check_dell_samsung_bug(struct pci_dev *pdev)
+{
+	if (pdev->vendor == 0x144d && pdev->device == 0xa802) {
+		/*
+		 * Several Samsung devices seem to drop off the PCIe bus
+		 * randomly when APST is on and uses the deepest sleep state.
+		 * This has been observed on a Samsung "SM951 NVMe SAMSUNG
+		 * 256GB", a "PM951 NVMe SAMSUNG 512GB", and a "Samsung SSD
+		 * 950 PRO 256GB", but it seems to be restricted to two Dell
+		 * laptops.
+		 */
+		if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.") &&
+		    (dmi_match(DMI_PRODUCT_NAME, "XPS 15 9550") ||
+		     dmi_match(DMI_PRODUCT_NAME, "Precision 5510")))
+			return NVME_QUIRK_NO_DEEPEST_PS;
+	}
+
+	return 0;
+}
+
 static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	int node, result = -ENOMEM;
 	struct nvme_dev *dev;
+	unsigned long quirks = id->driver_data;
 
 	node = dev_to_node(&pdev->dev);
 	if (node == NUMA_NO_NODE)

@@ -1978,8 +2000,10 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (result)
 		goto put_pci;
 
+	quirks |= check_dell_samsung_bug(pdev);
+
 	result = nvme_init_ctrl(&dev->ctrl, &pdev->dev, &nvme_pci_ctrl_ops,
-			id->driver_data);
+			quirks);
 	if (result)
 		goto release_pools;

Signature · 1 line

2.9.3

Andy Lutomirski

author

2017-04-20 03:02 UTC #8 lore raw

PATCH 5/5 nvme: Quirk APST off on "THNSF5256GPUK TOSHIBA"

In reply to Andy Lutomirski

There's a report that it malfunctions with APST on.

See https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1678184

Cc Kai-Heng Feng <kai.heng.feng at canonical.com>

Signed-off-by Andy Lutomirski <luto at kernel.org>

---
 drivers/nvme/host/core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

drivers/nvme/host/core.c +9 -0

index bec8c6973ae5..5249027a76ca 100644
@@ -1415,6 +1415,15 @@ struct nvme_core_quirk_entry {
 };
 
 static const struct nvme_core_quirk_entry core_quirks[] = {
+	{
+		/*
+		 * This Toshiba device seems to die using any APST states.  See:
+		 * https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1678184/comments/11
+		 */
+		.vid = 0x1179,
+		.mn = "THNSF5256GPUK TOSHIBA",
+		.quirks = NVME_QUIRK_NO_APST,
+	}
 };
 
 /* match is null-terminated but idstr is space-padded. */

Signature · 1 line

2.9.3

Jens Axboe

maintainer

2017-04-20 03:07 UTC #5 lore raw

Re: [PATCH 4/5] nvme: Adjust the Samsung APST quirk

In reply to Andy Lutomirski

Quoted 9 lines · Andy Lutomirski

I got a couple more reports: the Samsung APST issues appears to affect multiple 950-series devices in Dell XPS 15 9550 and Precision 5510 laptops. Change the quirk: rather than blacklisting the firmware on the first problematic SSD that was reported, disable APST on all 144d:a802 devices if they're installed in the two affected Dell models. While we're at it, disable only the deepest sleep state instead of all of them -- the reporters say that this is sufficient to fix the problem.

Quoted 8 lines

(I have a device that appears to be entirely identical to one of the affected devices, but I have a different Dell laptop, so it's not the case that all Samsung devices with firmware BXW75D0Q are broken under all circumstances.)
Samsung engineers have an affected system, and hopefully they'll give us a better workaround some time soon. In the mean time, this should minimize regressions.

Do we know for a fact that it only happens on those systems, and isn't purely specific to the device?

At this point in time, I'd be much more comfortable completely disabling APST on Samsung, period.

Signature · 1 line

Jens Axboe

Jens Axboe

maintainer

2017-04-20 03:10 UTC #9 lore raw

In reply to Andy Lutomirski

Quoted 7 lines · Andy Lutomirski

Sorry for waiting so long for this. I was waiting for feedback from Samsung, but they haven't root-caused the issue yet, and I should have just done this from the beginning.
This series makes APST more debuggable and updates the quirk list. The quirks I'm aware of are:

Quoted 8 lines

- Samsung 950 series SSDs in Dell XPS 15 9550 and Precision 5510 laptops (which are essentially the same laptop) can lose their PCIe link if they're allowed to use the deepest APST state. Samsung engineers have an affected system and are working on it. The same exact SSDs in other machines (even an XPS 13) seem to work fine.
- One Toshiba device malfunctions if APST is used at all.

You need to split this series in two, patches 1-3 can wait. For 4.11, all we need to do is turn off APST on any device that potentially has this problem.

Quoted 4 lines

One thing that improves my confidence that there aren't too many more problems with APST is that Ubuntu has backported APST to Zesty, so it's already gotten a bit of testing in a widely used (if very new) release.

Honestly, I think the best path for 4.11 is to turn off APST by default, make it opt-in instead. I don't share your optimism here, as I made clear back from before we even merged this feature.

Signature · 1 line

Jens Axboe

Andy Lutomirski

author

2017-04-20 03:51 UTC #6 lore raw

Re: [PATCH 4/5] nvme: Adjust the Samsung APST quirk

In reply to Jens Axboe

Quoted 15 lines · Jens Axboe

Quoted 14 lines · Andy Lutomirski
I got a couple more reports: the Samsung APST issues appears to affect multiple 950-series devices in Dell XPS 15 9550 and Precision 5510 laptops. Change the quirk: rather than blacklisting the firmware on the first problematic SSD that was reported, disable APST on all 144d:a802 devices if they're installed in the two affected Dell models. While we're at it, disable only the deepest sleep state instead of all of them -- the reporters say that this is sufficient to fix the problem.
(I have a device that appears to be entirely identical to one of the affected devices, but I have a different Dell laptop, so it's not the case that all Samsung devices with firmware BXW75D0Q are broken under all circumstances.)

Quoted 6 lines

Quoted 3 lines
Samsung engineers have an affected system, and hopefully they'll give us a better workaround some time soon. In the mean time, this should minimize regressions.
Do we know for a fact that it only happens on those systems, and isn't purely specific to the device?

I have decent evidence. All of the reports are from XPS 15 9550 or Precision 5510, and Dell confirmed that they're basically the same machine and run literally the same BIOS. One of these reports is from a device with exactly the same model and firmware as my SSD, and mine is fine. (I have a different laptop.)

Quoted 4 lines

At this point in time, I'd be much more comfortable completely disabling APST on Samsung, period.

I'd be fine with doing that for 4.11 and then doing this for 4.12-rc1.

Andy Lutomirski

author

2017-04-20 03:55 UTC #10 lore raw

In reply to Jens Axboe

Quoted 21 lines · Jens Axboe

Quoted 7 lines · Andy Lutomirski
Sorry for waiting so long for this. I was waiting for feedback from Samsung, but they haven't root-caused the issue yet, and I should have just done this from the beginning.
This series makes APST more debuggable and updates the quirk list. The quirks I'm aware of are:
Quoted 8 lines
- Samsung 950 series SSDs in Dell XPS 15 9550 and Precision 5510 laptops (which are essentially the same laptop) can lose their PCIe link if they're allowed to use the deepest APST state. Samsung engineers have an affected system and are working on it. The same exact SSDs in other machines (even an XPS 13) seem to work fine.
- One Toshiba device malfunctions if APST is used at all.
You need to split this series in two, patches 1-3 can wait. For 4.11, all we need to do is turn off APST on any device that potentially has this problem.

Quoted 10 lines

Quoted 4 lines
One thing that improves my confidence that there aren't too many more problems with APST is that Ubuntu has backported APST to Zesty, so it's already gotten a bit of testing in a widely used (if very new) release.
Honestly, I think the best path for 4.11 is to turn off APST by default, make it opt-in instead. I don't share your optimism here, as I made clear back from before we even merged this feature.

I can make it so that force_apst=0 means no APST and force_apst=1 mean yes APST and we could try again with a quirk list for 4.12. There's a decent chance that a few more weeks with Ubuntu having APST on will shake out all the problems fairly quickly.

--Andy

Judy Brock

2017-04-20 04:33 UTC #7 lore raw

RE: [PATCH 4/5] nvme: Adjust the Samsung APST quirk

In reply to Andy Lutomirski

[Jens] Do we know for a fact that it only happens on those systems, and isn't

Quoted 1 line

purely specific to the device?

[Andy] I have decent evidence. All of the reports are from XPS 15 9550 or Precision 5510, and Dell confirmed that they're basically the same machine and run literally the same BIOS.

The answer as per the above as far as we know is "yes".

Quoted 4 lines

At this point in time, I'd be much more comfortable completely disabling APST on Samsung, period.

1) Why? The answer to the question above was "Yes". This has been reported exclusively on the two Dell models with that exact same BIOS. Additionally, there are reports of the device acting fine on other systems so it is not purely specific to the device.

We request that the quirk should be only on the affected Dell machines - there is no reason to completely disable APST on Samsung.

2) Samsung shared in the more private thread that we are seeing excessive recovery attempts on the PCIe bus - no PCIe TLPs seen, just ordered sets. This looks like a signal integrity problem to us on the Dell side. We shared excerpt of PCIe trace on the offline thread.

3) We also shared a more extensive report with Dell today. We've asked them to look into it.

4) There was at least one report of same symptom on a Toshiba device and Lenovo system that seemed to also disappear by avoiding PS4. So it seems it would be best to continue to try to get to the bottom of the problem (root cause) and quirk judiciously in the meantime.

Thanks,
Judy

-----Original Message-----
From: Linux-nvme [mailto:[email protected]] On Behalf Of Andy Lutomirski
Sent: Wednesday, April 19, 2017 8:51 PM
To: Jens Axboe
Cc: Sagi Grimberg; linux-kernel at vger.kernel.org; linux-nvme; Keith Busch; Kai-Heng Feng; Andy Lutomirski; Christoph Hellwig; Niranjan Sivakumar
Subject: Re: [PATCH 4/5] nvme: Adjust the Samsung APST quirk

Quoted 15 lines · Jens Axboe

Quoted 14 lines · Andy Lutomirski
I got a couple more reports: the Samsung APST issues appears to affect multiple 950-series devices in Dell XPS 15 9550 and Precision 5510 laptops. Change the quirk: rather than blacklisting the firmware on the first problematic SSD that was reported, disable APST on all 144d:a802 devices if they're installed in the two affected Dell models. While we're at it, disable only the deepest sleep state instead of all of them -- the reporters say that this is sufficient to fix the problem.
(I have a device that appears to be entirely identical to one of the affected devices, but I have a different Dell laptop, so it's not the case that all Samsung devices with firmware BXW75D0Q are broken under all circumstances.)

Quoted 6 lines

Quoted 3 lines
Samsung engineers have an affected system, and hopefully they'll give us a better workaround some time soon. In the mean time, this should minimize regressions.
Do we know for a fact that it only happens on those systems, and isn't purely specific to the device?

Quoted 4 lines

At this point in time, I'd be much more comfortable completely disabling APST on Samsung, period.

I'd be fine with doing that for 4.11 and then doing this for 4.12-rc1.

_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Andy Lutomirski

2017-04-20 04:52 UTC #11 lore raw

In reply to Andy Lutomirski

Quoted 33 lines · Andy Lutomirski

Quoted 21 lines · Jens Axboe
Quoted 7 lines · Andy Lutomirski
Sorry for waiting so long for this. I was waiting for feedback from Samsung, but they haven't root-caused the issue yet, and I should have just done this from the beginning.
This series makes APST more debuggable and updates the quirk list. The quirks I'm aware of are:
Quoted 8 lines
- Samsung 950 series SSDs in Dell XPS 15 9550 and Precision 5510 laptops (which are essentially the same laptop) can lose their PCIe link if they're allowed to use the deepest APST state. Samsung engineers have an affected system and are working on it. The same exact SSDs in other machines (even an XPS 13) seem to work fine.
- One Toshiba device malfunctions if APST is used at all.
You need to split this series in two, patches 1-3 can wait. For 4.11, all we need to do is turn off APST on any device that potentially has this problem.
Quoted 10 lines
Quoted 4 lines
One thing that improves my confidence that there aren't too many more problems with APST is that Ubuntu has backported APST to Zesty, so it's already gotten a bit of testing in a widely used (if very new) release.
Honestly, I think the best path for 4.11 is to turn off APST by default, make it opt-in instead. I don't share your optimism here, as I made clear back from before we even merged this feature.

Quoted 4 lines

I can make it so that force_apst=0 means no APST and force_apst=1 mean yes APST and we could try again with a quirk list for 4.12. There's a decent chance that a few more weeks with Ubuntu having APST on will shake out all the problems fairly quickly.

Here's a more concrete and more sensible proposal:

For 4.11:

force_apst=0: Default. APST off on all Samsung 950-like devices regardless of what laptop and on the Toshiba device. force_apst=1: Use APST except where known bad. APST deepest state disabled on Samsung 950-like devices on XPS 15 and Precision 5510. APST off on the Toshiba device.
force_apst=2: APST fully on regardless of any quirks.

For 4.12-rc1: force_apst=0 works like force_apst=1, but we keep both values for compatibility and in case we need to add another overly broad quirk some day.

Would something like this make sense?

Christoph Hellwig

maintainer

2017-04-20 05:19 UTC #12 lore raw

Quoted 6 lines · Andy Lutomirski

Quoted 4 lines
I can make it so that force_apst=0 means no APST and force_apst=1 mean yes APST and we could try again with a quirk list for 4.12. There's a decent chance that a few more weeks with Ubuntu having APST on will shake out all the problems fairly quickly.
Here's a more concrete and more sensible proposal:

Can we just have force_apst=on to force it on, force_apst=off to turn it off, and leave it with that? And yes, I mean the strings instead of the weird numbers.

Andy Lutomirski

author

2017-04-20 16:01 UTC #13 lore raw

Quoted 11 lines · Christoph Hellwig

Quoted 6 lines · Andy Lutomirski
Quoted 4 lines
I can make it so that force_apst=0 means no APST and force_apst=1 mean yes APST and we could try again with a quirk list for 4.12. There's a decent chance that a few more weeks with Ubuntu having APST on will shake out all the problems fairly quickly.
Here's a more concrete and more sensible proposal:
Can we just have force_apst=on to force it on, force_apst=off to turn it off, and leave it with that? And yes, I mean the strings instead of the weird numbers.

That's essentially what the patch I emailed here does -- it's a regular bool param, so force_apst=1, force_apst=Y, and force_apst=y all mean the same thing. I can just leave it like that.