I'm not sure this should be a quirk. It should be part of our normal escelation...

This needs a devclt event

This needs an event

This should be under bootverbose

I thinkthe oigina comment is better.

This is a stopgap to unbreak T1 NVMe right now without it, people are stuck either reverting 5e0ba47aa00e or manually cherry-picking this diff out-of-tree.

The quirk gate isn't just caution for its own sake making part of the default escalation for all NVMe controllers without more testing can cause chaos.

As an example: this same S3X disk works fine with MQS enabled, but crashed at 78% during install when a timeout triggered under heavy load.

I don't want to change fatal-status handling for every controller in the tree on that basis.

Once this lands I'll follow up with either MQS support or the full escalation flow you're describing.

the old comment described how sqe_shift used to be derived. This diff removes that derivation.

So this isn't a 'recovery from error' but a 'let's program the card' on start when it's needed?

OK. Fair enough. I'm good with it.

It a hook, not a start-time provisioning step. the FLR-on-fatal branch only triggers when CSTS.CFS is already set.

On a normal, healthy start csts != NVME_GONE is true but CFS is 0, so this code path is skipped.

It's recovery from a fatal condition i got, gated by the quirk so it doesn't change reset behavior for every other controller.

I can remove it now, or i will with the MQS.

So did you see this when you started up once (or occassionally) and this fixed it? Or did we drive the card into timeout, and during timeout process, when we reset the card, this fixed it? It would be good to include that, though I'm on the fence if that's a 'inline comment' detail or 'commit log' detail, but I'm leaning to the former.

I'd add these details to this, gated by quirk for now is fine, but include a line about 'in the future, this should be part of normal recovery' or similar so we can integrated it into the currently overly simplistic recovery. It's fine to have it here. I'd leave the printfs below not bootverbose, at least for the moment, but in the fullness of time we may want to print them once and keep a sysctl counter. For now, don't go ahead with that, since we should do that as part of integrating. Since the if is gated by the quirk, we're not going to spam a lot of people either.

Ah, one other thing, I'd make the body of the if a function, returning and errno, and put the bulk of the comment there. That would let us integrate it other places as needed or move where we use it around better.

Driven into timeout under load, not startup.
I hit CSTS.CFS set after the controller wedged mid-transfer (heavy write load), not on a cold boot.

I modified my kernel to allow this panic! in mainline & at release mode, this code is a no-op.

I would consider adding a KASSERT here checking that the value here is greater than 0 for the non-admin qpair.