Skip to content

Conversation

@ikegami-t
Copy link

Move nvme_security_receive_args to this repository as dropped in libnvme.

@ikegami-t
Copy link
Author

Changes for the issue comment linux-nvme/libnvme#495 (comment).

Copy link

@MaisenbacherD MaisenbacherD left a comment

Choose a reason for hiding this comment

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

Maybe the commit messages could be adjusted as the nvme_*_args are completely dropped :)

nvme-rpmb.c Outdated
};

return nvme_security_receive(l, &args);
return nvme_security_receive(l, 0, 0, tgt, RPMB_NVME_SPSP, 0, RPMB_NVME_SECP, rsp, size,

Choose a reason for hiding this comment

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

The argument order got mangled up:

Suggested change
return nvme_security_receive(l, 0, 0, tgt, RPMB_NVME_SPSP, 0, RPMB_NVME_SECP, rsp, size,
return nvme_security_receive(l, 0, tgt, RPMB_NVME_SPSP, 0, RPMB_NVME_SECP, 0, (void *)rsp, size,

spsp0 and spsp1 will be merged into one as per nvme-experiments/libnvme#31

nvme.c Outdated

err = nvme_security_receive(l, &args);
err = nvme_security_receive(l, cfg.namespace_id, cfg.al, cfg.nssf, cfg.spsp & 0xff,
cfg.spsp >> 8, cfg.secp, sec_buf, cfg.size, NULL);

Choose a reason for hiding this comment

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

Argument ordering is also off here.

nvme.c Outdated
.result = NULL,
};
err = nvme_get_lba_status(l, &args);
err = nvme_get_lba_status(l, cfg.namespace_id, cfg.slba, cfg.atype, cfg.rl, buf, cfg.mndw,

Choose a reason for hiding this comment

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

Argument ordering is a bit off:

Suggested change
err = nvme_get_lba_status(l, cfg.namespace_id, cfg.slba, cfg.atype, cfg.rl, buf, cfg.mndw,
err = nvme_get_lba_status(l, cfg.namespace_id, cfg.slba, cfg.mndw, cfg.atype, cfg.rl, buf,

nvme.c Outdated
.result = &result,
};
err = nvme_directive_send(l, &args);
err = nvme_directive_send(l, cfg.namespace_id, cfg.doper, cfg.dtype, dw12, cfg.dspec, buf,

Choose a reason for hiding this comment

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

Argument ordering is a bit off:

Suggested change
err = nvme_directive_send(l, cfg.namespace_id, cfg.doper, cfg.dtype, dw12, cfg.dspec, buf,
err = nvme_directive_send(l, cfg.namespace_id, cfg.doper, cfg.dtype, cfg.dspec, dw12, buf,

nvme.c Outdated
.result = &result,
};
err = nvme_directive_recv(l, &args);
err = nvme_directive_recv(l, cfg.namespace_id, cfg.doper, cfg.dtype, dw12, cfg.dspec, buf,

Choose a reason for hiding this comment

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

Argument ordering is a bit off:

Suggested change
err = nvme_directive_recv(l, cfg.namespace_id, cfg.doper, cfg.dtype, dw12, cfg.dspec, buf,
err = nvme_directive_recv(l, cfg.namespace_id, cfg.doper, cfg.dtype, cfg.dspec, dw12, buf,```

nvme.c Outdated
.result = &result,
};
err = nvme_capacity_mgmt(l, &args);
err = nvme_capacity_mgmt(l, cfg.dw11, cfg.dw12, cfg.element_id, cfg.operation, &result);

Choose a reason for hiding this comment

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

Argument ordering is a bit off:

Suggested change
err = nvme_capacity_mgmt(l, cfg.dw11, cfg.dw12, cfg.element_id, cfg.operation, &result);
err = nvme_capacity_mgmt(l, cfg.operation, cfg.element_id, cfg.dw11, cfg.dw12, &result);

nvme.c Outdated
.result = NULL,
};
err = nvme_lockdown(l, &args);
err = nvme_lockdown(l, cfg.scp, cfg.prhbt, cfg.ifc, cfg.ofi, cfg.uuid, NULL);

Choose a reason for hiding this comment

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

Nit: cfg.uuid could be renamed as uidx to align with the specs (this also needs to be adjusted in libnvme)

nvme.c Outdated
};

err = nvme_sanitize_nvm(l, &args);
err = nvme_sanitize_nvm(l, cfg.sanact, cfg.ovrpat, cfg.ause, cfg.owpass, cfg.oipbp,

Choose a reason for hiding this comment

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

Argument ordering is a bit off:

	err = nvme_sanitize_nvm(l, cfg.sanact, cfg.ause, cfg.owpass, cfg.oipbp, cfg.no_dealloc, cfg.emvs, cfg.ovrpat, NULL);

@MaisenbacherD
Copy link

I hope that I didn't miss any libnvme patch which reorders the ioctl function arguments on the positions where I commented on :)

nvme_security_receive_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_get_lba_status_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_directive_send_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_directive_recv_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_capacity_mgmt_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_lockdown_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_set_property_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_get_property_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_sanitize_nvm_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_dev_self_test_args dropped in libnvme.

Signed-off-by: Tokunori Ikegami <[email protected]>
nvme_virtual_mgmt_args dropped in libnvme.

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

Just fixed as the review comments. Thank you.

@igaw
Copy link

igaw commented Sep 19, 2025

I've added these patches to the nvme-cli3 branch. Thanks.

Anyway, I think I pulled now all patches but there are still some fallouts. So I might have missed something. If there are patches missing please create a new PR against the current nvme-cli3 branch. It was a bit tricky to get all sorted out, especially because we were still in the process to update libnvme2.

@igaw igaw closed this Sep 19, 2025
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