Skip to content

Add encoding for MI/NVMe status values#502

Merged
igaw merged 2 commits intolinux-nvme:masterfrom
CodeConstruct:pr/mi-status
Oct 24, 2022
Merged

Add encoding for MI/NVMe status values#502
igaw merged 2 commits intolinux-nvme:masterfrom
CodeConstruct:pr/mi-status

Conversation

@jk-ozlabs
Copy link
Copy Markdown
Collaborator

As a fix for #456, this PR adds a new encoding for the status values of the nvme_mi_* and nvme_* API, with the latter being completely unchanged.

To do this, we use the top set of bits in the int return value as an encoding of the "type" of status value; possible types are currently NVMe (as existing, from CDW3), and MI (from the status field of the MI header). The most-significant bit is already used for the sign bit (where negative values already represent internal errors, rather than NVMe status), so we use the next three significant bits to represent a type, and allow for future expansion of the possible types in future

For NVMe status, we only need the lower 15 bits of the int; for MI, we only need the lower 8.

So, an NVMe status value is encoded as

|s|typ| unused      | status        |
|0|000|0000000000000|xxxxxxxxxxxxxxx|

(ie, exactly matching the existing usage in the API)

and an MI status value encoded as:

|s|typ| unused             | status |
|0|001|00000000000000000000|xxxxxxxx|

(ie, indicating MI in the type bits)

Corresponding changes to nvme-cli coming soon in a draft PR.

As always: comments, questions and feedback are most welcome.

We curerently have some overloading in the status values returned from
the nvme_* API, as the NVMe CDW3 values overlap with the
recently-introduced NVMe-MI response header status.

This change introduces a new encoding for the return values of MI
functions, where we use a set of bits in the return value to encode
whether the value is either a MI status value or a NVMe status value. We
leave room for future expansion too, by defining three bits of possible
type values.

This has minimal change to the current API, as we're using 0 for the
current NVMe status codes, so they are all unchanged. Since the MI
values alised those, they will have high bits set now, but we couldn't
previously distinguish them from the NVMe values anyway.

Fixes: linux-nvme#456

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Add a function, similar to nvme_status_to_string(), that returns a
string value of the spec-defined wording for each status value.

We keep this in the mi object for now, to keep the existing division of
libnvme.so/libnvme-mi.so

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #502 (6cd5403) into master (b483bbd) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #502      +/-   ##
==========================================
- Coverage   24.20%   24.17%   -0.03%     
==========================================
  Files          31       31              
  Lines        5946     5953       +7     
  Branches     1225     1230       +5     
==========================================
  Hits         1439     1439              
- Misses       4028     4031       +3     
- Partials      479      483       +4     
Impacted Files Coverage Δ
src/nvme/mi.c 65.61% <0.00%> (-0.37%) ⬇️
src/nvme/mi.h 72.09% <ø> (ø)
src/nvme/types.h 0.00% <0.00%> (ø)
test/mi-mctp.c 83.81% <0.00%> (-0.61%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@igaw igaw merged commit a12f1dd into linux-nvme:master Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants