On Wed, Sep 13, 2023 at 09:46:08AM -0700, Keith Busch wrote:
I guess that I just can't understand how a controller can set bit CAP.CRMS.CRWMS to 1, without setting a value in CRTO.CRWMT. Is it such a silly spec violation, that they seem to have not bothered to read what this bit means at all.
Yep, but broken controllers are broken. They used to work in Linux before the driver started preferring the recommended CRTO, so we gotta fix 'em.
I'm not saying that these controllers shouldn't work with Linux. However, these controller used to work with CC.CRIME == 0, so perhaps we should continue to use them that way?
My reasoning is that if the controller did not even manage to get the most fundamental part of this new feature correctly (i.e. defining the actual timeout values for this feature), then perhaps we shouldn't assume that the rest of the feature, setting the NRDY bit, sending the AER, etc., is implemented correctly either.
When CC.CRIME == 1, both fields in CRTO actually have a defined meaning: CRTO.CRIMT is the maximum time that host software should wait for the controller to become ready. CRTO.CRWMT is the maximum time that host software should wait for all attached namespaces to become ready.
So having both fields defined to zero, or rather, to have both fields defined to a value smaller than CAP.TO, regardless of CC.CRIME value, is quite bad.
So perhaps it is better to keep CC.CRIME == 0 for such controllers.
If we have a way to sanity check for spec non-compliance, I would prefer doing that generically rather than quirk specific devices.
It's not going to be beautiful, but one way could be to: -check CAP.CRMS.CRIMS, if it is set to 1: -write CC.CRIME == 1, -re-read CAP register, since it can change depending on CC.CRIME (urgh) -check if CRTO.CRIMT is less than CAP.TO, if so: -write CC.CRIME == 0 (disable the feature since it is obviously broken) -re-read CAP register, since it can change depending on CC.CRIME (urgh)
For the record, I'm obviously happy with whatever decision you make, I'm just giving my two cents...
Kind regards, Niklas