mi: Add ISH bit (admin command cflags)#3086
Conversation
sc108-lee
force-pushed
the
mi_ish
branch
2 times, most recently
from
dce9947 to
c56413e
Compare
|
I am not really happy to pass this flag through all helpers. This makes the number of arguments per function even bigger. Also the true/false as argument is not self explaining. What about introducing a dedicated helper just to set the flag? Something like: static inline void
nvme_init_cmd_flags(struct nvme_passthru_cmd *cmd, bool ish)
{
cmd->flags = NVME_FIELD_ENCODE(ish,
NVME_MI_ADMIN_CFLAGS_ISH_SHIFT,
NVME_MI_ADMIN_CFLAGS_ISH_MASK);
}And then nvme_init_format_nvm(&cmd, nsid, lbaf, mset, pi, pil, ses);
if (ish) {
if (nvme_transport_is_mi_handle(hdl))
nvme_init_cmd_flags(&cmd, ish);
else
printf("no ISH not supported\n");
}just as rough idea. naming could be improved. |
Add ish bit for mi admin commands that access media
Spec NVM-Express-Management-Interface-Specification
-Revision-2.1-2025.08.01-Ratified
Figure 136: NVMe Admin Command Request Description
Command Flags (CFLGS): Bit 2 : Ignore Shutdown (ISH)
This bit shall have no effect on the value of the CSTS.SHST field.
(193 page from spec)
If an NVMe Admin Command does not require access to media, then the ISH
bit shall have no effect on the processing of that NVMe Admin Command.
spec NVM-Express-Base-Specification-Revision-2.3-2025.08.01-Ratified
Figure 84: Admin Commands Permitted to Return a Status Code of Admin
Command Media Not Ready
From Figure 84, we can assume that below Admin commands access media
Capacity Management, Device Self-test, Firmware Commit,
Firmware Image Download, Get LBA Status, Get Log Page,
Namespace Attachment, Namespace Management, Format NVM, Sanitize,
Sanitize Namespace, Security Receive, Security Send, Vendor Specific
Signed-off-by: Steven Seungcheol Lee <sc108.lee@samsung.com>
Reported-by: Hojin Ahn <hojin82.ahn@samsung.com>
|
@igaw Thanks for good opinion, Updated based on that :) |
| int nvme_fw_download_seq(struct nvme_transport_handle *hdl, __u32 size, __u32 xfer, __u32 offset, | ||
| void *buf); | ||
| int nvme_fw_download_seq(struct nvme_transport_handle *hdl, bool ish, | ||
| __u32 size, __u32 xfer, __u32 offset, void *buf); |
There was a problem hiding this comment.
Ah, I see we still have to modify the linux.h API for this, because this is API is wrapping the command initialization and the command submission into one function. There are more of these function left in the code base. Ideally we could convert them somehow into the other style (splitting command initialization and submission). Though this is a different task. So for this here it's fine. I am glad we don't have to update all the ioctl.h APIs with this approach :)
|
Nice, way better! BTW, could you update the documentation for the commands as well? Thanks! |
2 participants
Add ish bit for mi admin commands that access media
Spec NVM-Express-Management-Interface-Specification
-Revision-2.1-2025.08.01-Ratified
Figure 136: NVMe Admin Command Request Description Command Flags (CFLGS): Bit 2 : Ignore Shutdown (ISH) This bit shall have no effect on the value of the CSTS.SHST field.
(193 page from spec)
If an NVMe Admin Command does not require access to media, then the ISH bit shall have no effect on the processing of that NVMe Admin Command.
spec NVM-Express-Base-Specification-Revision-2.3-2025.08.01-Ratified Figure 84: Admin Commands Permitted to Return a Status Code of Admin
Command Media Not Ready
From Figure 84, we can assume that below Admin commands access media Capacity Management, Device Self-test, Firmware Commit, Firmware Image Download, Get LBA Status, Get Log Page, Namespace Attachment, Namespace Management, Format NVM, Sanitize, Sanitize Namespace, Security Receive, Security Send, Vendor Specific
Reported-by: Hojin Ahn hojin82.ahn@samsung.com