Skip to content

Conversation

@ikegami-t
Copy link

Drop struct nvme_dsm_args.

igaw added 9 commits July 31, 2025 19:59
The next change will add a static inline implementation of nvme_get_log
which uses nvme_get_log_page. Thus move the nvme_get_log_page function
up so it can be used directly.

Signed-off-by: Daniel Wagner <[email protected]>
Refactor nvme_get_log and nvme_get_log_page to use use struct
nvme_passthru_cmd directly and drop the struct nvme_get_log_args.

The struct args have are ABI thus it's hard to change it after a
release. The nvme_passthru_cmd buffer is a generic buffer and it's
possible to use the setters directly on the buffer in an inline
function. Besides a reduced API to maintain it also generates better
code and avoids one call less when issue a command.

Signed-off-by: Daniel Wagner <[email protected]>
Drop struct nvme_format_nvm_args.

Signed-off-by: Daniel Wagner <[email protected]>
Drop struct nvme_ns_mgmt_args.

Signed-off-by: Daniel Wagner <[email protected]>
Drop struct nvme_ns_attach_args.

Signed-off-by: Daniel Wagner <[email protected]>
Drop struct nvme_fw_download_args.

Signed-off-by: Daniel Wagner <[email protected]>
Drop struct nvme_fw_commit_args.

Signed-off-by: Daniel Wagner <[email protected]>
Drop struct nvme_security_send_args.

Signed-off-by: Daniel Wagner <[email protected]>
Drop struct nvme_dsm_args.

Signed-off-by: Tokunori Ikegami <[email protected]>
Drop struct nvme_io_args.

Signed-off-by: Tokunori Ikegami <[email protected]>
To use the fields in nvme_io() and nvme_dsm() functions.

Signed-off-by: Tokunori Ikegami <[email protected]>
Drop struct nvme_copy_args.

Signed-off-by: Tokunori Ikegami <[email protected]>
src/nvme/ioctl.h Outdated
*/
int nvme_dsm(nvme_link_t l, struct nvme_dsm_args *args);
static inline int nvme_dsm(nvme_link_t l, struct nvme_dsm_range *dsm, __u16 nr_ranges, __u32 attrs,
__u32 nsid, __u32 *result)
Copy link

@igaw igaw Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I just figured out, the nsid is in cdw0, so it should be right after nvme_link_t l. Sorry for my mistake.

@igaw
Copy link

igaw commented Aug 5, 2025

Looks great! If you could reorder the nsid argument that would be awesome! Thanks!

Also order the pointer and its length member go to the end of the list.

Signed-off-by: Tokunori Ikegami <[email protected]>
@ikegami-t
Copy link
Author

Just fixed the review comments. Thank you.

@igaw igaw force-pushed the libnvme2 branch 2 times, most recently from 58a0811 to 5fa5c8a Compare August 7, 2025 07:49
@igaw
Copy link

igaw commented Aug 8, 2025

I'll pick those and rebase them on the libnvme2 branch. Let me figure out what is the simplest way.

@igaw
Copy link

igaw commented Aug 8, 2025

I've picked these directly. They uncovered a bunch of bugs in the handling of the control argument for nvme_dsm. Need to fix these in master first.

@igaw igaw closed this Aug 8, 2025
@igaw
Copy link

igaw commented Aug 8, 2025

Applied. I'll merge the nsid match into the other changes later. Thanks!

NVME_VAL(NVM_CDW12_STC) |
NVME_VAL(NVM_CDW12_PRINFO) |
NVME_VAL(NVM_CDW12_FUA) |
NVME_VAL(NVM_CDW12_LR),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, this masks is already shifted thus it doesn't work as expected. I've added the necessary >> NVME_NVM_CDW12_CONTROL_SHIFT so it works. Caught by meson test -C .build :)

@ikegami-t
Copy link
Author

Thank you. Noted.

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.

2 participants