Skip to content

Commit 15ea942

Browse files
igawigaw
committed
ioctl: nvme_get_log use nvme_passthru_cmd directly
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 <wagi@kernel.org> merge with: 532c209 ("ioctl: nvme_get_log use nvme_passthru_cmd directly")
1 parent 022e829 commit 15ea942

File tree

12 files changed

+465
-805
lines changed

12 files changed

+465
-805
lines changed

‎examples/mi-mctp-csi-test.c‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ void hexdump(const unsigned char *buf, int len)
5656

5757
int do_get_log_page(nvme_mi_ep_t ep, int argc, char **argv)
5858
{
59-
struct nvme_get_log_args args = { 0 };
59+
enum nvme_cmd_get_log_lid lid;
6060
struct nvme_link *link;
6161
uint8_t buf[4096];
6262
uint16_t ctrl_id;
@@ -75,15 +75,11 @@ int do_get_log_page(nvme_mi_ep_t ep, int argc, char **argv)
7575

7676
ctrl_id = tmp & 0xffff;
7777

78-
args.args_size = sizeof(args);
79-
args.log = buf;
80-
args.len = sizeof(buf);
81-
8278
if (argc > 2) {
8379
tmp = atoi(argv[2]);
84-
args.lid = tmp & 0xff;
80+
lid = tmp & 0xff;
8581
} else {
86-
args.lid = 0x1;
82+
lid = 0x1;
8783
}
8884

8985
link = nvme_mi_init_link(ep, ctrl_id);
@@ -92,14 +88,18 @@ int do_get_log_page(nvme_mi_ep_t ep, int argc, char **argv)
9288
return -1;
9389
}
9490

95-
rc = nvme_get_log(link, &args);
91+
rc = nvme_get_log(link, NVME_NSID_NONE, false, NVME_LOG_LSP_NONE,
92+
lid, NVME_LOG_LSI_NONE, NVME_CSI_NVM,
93+
false, NVME_UUID_NONE,
94+
0, buf, sizeof(buf),
95+
sizeof(buf), NULL);
9696
if (rc) {
9797
warn("can't perform Get Log page command");
9898
return -1;
9999
}
100100

101-
printf("Get log page (log id = 0x%02x) data:\n", args.lid);
102-
hexdump(buf, args.len);
101+
printf("Get log page (log id = 0x%02x) data:\n", lid);
102+
hexdump(buf, sizeof(buf));
103103

104104
return 0;
105105
}

‎examples/mi-mctp.c‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ void hexdump(const unsigned char *buf, int len)
369369

370370
int do_get_log_page(nvme_mi_ep_t ep, int argc, char **argv)
371371
{
372-
struct nvme_get_log_args args = { 0 };
372+
enum nvme_cmd_get_log_lid lid;
373373
struct nvme_link *link;
374374
uint8_t buf[512];
375375
uint16_t ctrl_id;
@@ -388,15 +388,11 @@ int do_get_log_page(nvme_mi_ep_t ep, int argc, char **argv)
388388

389389
ctrl_id = tmp & 0xffff;
390390

391-
args.args_size = sizeof(args);
392-
args.log = buf;
393-
args.len = sizeof(buf);
394-
395391
if (argc > 2) {
396392
tmp = atoi(argv[2]);
397-
args.lid = tmp & 0xff;
393+
lid = tmp & 0xff;
398394
} else {
399-
args.lid = 0x1;
395+
lid = 0x1;
400396
}
401397

402398
link = nvme_mi_init_link(ep, ctrl_id);
@@ -405,14 +401,18 @@ int do_get_log_page(nvme_mi_ep_t ep, int argc, char **argv)
405401
return -1;
406402
}
407403

408-
rc = nvme_get_log(link, &args);
404+
rc = nvme_get_log(link, NVME_NSID_NONE, false, NVME_LOG_LSP_NONE,
405+
lid, NVME_LOG_LSI_NONE, NVME_CSI_NVM,
406+
false, NVME_UUID_NONE,
407+
0, buf, sizeof(buf),
408+
sizeof(buf),NULL);
409409
if (rc) {
410410
warn("can't perform Get Log page command");
411411
return -1;
412412
}
413413

414-
printf("Get log page (log id = 0x%02x) data:\n", args.lid);
415-
hexdump(buf, args.len);
414+
printf("Get log page (log id = 0x%02x) data:\n", lid);
415+
hexdump(buf, sizeof(buf));
416416

417417
return 0;
418418
}

‎src/libnvme.map‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,7 @@ LIBNVME_2_0 {
129129
nvme_get_features_write_protect;
130130
nvme_get_host_telemetry;
131131
nvme_get_lba_status;
132-
nvme_get_log;
133-
nvme_get_log_page;
132+
nvme_get_log_partial;
134133
nvme_get_logging_level;
135134
nvme_get_logical_block_size;
136135
nvme_get_new_host_telemetry;

‎src/nvme/api-types.h‎

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -49,43 +49,6 @@ void nvme_free_root(nvme_root_t r);
4949
* be set to zero.
5050
*/
5151

52-
/**
53-
* struct nvme_get_log_args - Arguments for the NVMe Admin Get Log command
54-
* @lpo: Log page offset for partial log transfers
55-
* @result: The command completion result from CQE dword0
56-
* @log: User space destination address to transfer the data
57-
* @args_size: Length of the structure
58-
* @timeout: Timeout in ms
59-
* @lid: Log page identifier, see &enum nvme_cmd_get_log_lid for known
60-
* values
61-
* @len: Length of provided user buffer to hold the log data in bytes
62-
* @nsid: Namespace identifier, if applicable
63-
* @csi: Command set identifier, see &enum nvme_csi for known values
64-
* @lsi: Log Specific Identifier
65-
* @lsp: Log specific field
66-
* @uuidx: UUID selection, if supported
67-
* @rae: Retain asynchronous events
68-
* @ot: Offset Type; if set @lpo specifies the index into the list
69-
* of data structures, otherwise @lpo specifies the byte offset
70-
* into the log page.
71-
*/
72-
struct nvme_get_log_args {
73-
__u64 lpo;
74-
__u32 *result;
75-
void *log;
76-
int args_size;
77-
__u32 timeout;
78-
enum nvme_cmd_get_log_lid lid;
79-
__u32 len;
80-
__u32 nsid;
81-
enum nvme_csi csi;
82-
__u16 lsi;
83-
__u8 lsp;
84-
__u8 uuidx;
85-
bool rae;
86-
bool ot;
87-
};
88-
8952
/**
9053
* struct nvme_set_features_args - Arguments for the NVMe Admin Set Feature command
9154
* @result: The command completion result from CQE dword0

‎src/nvme/fabrics.c‎

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,17 +1118,6 @@ static int nvme_discovery_log(const struct nvme_get_discovery_args *args,
11181118
const char *name = nvme_ctrl_get_name(args->c);
11191119
uint64_t genctr, numrec;
11201120
nvme_link_t l = nvme_ctrl_get_link(args->c);
1121-
struct nvme_get_log_args log_args = {
1122-
.result = args->result,
1123-
.args_size = sizeof(log_args),
1124-
.timeout = args->timeout,
1125-
.lid = NVME_LOG_LID_DISCOVER,
1126-
.nsid = NVME_NSID_NONE,
1127-
.csi = NVME_CSI_NVM,
1128-
.lsi = NVME_LOG_LSI_NONE,
1129-
.lsp = args->lsp,
1130-
.uuidx = NVME_UUID_NONE,
1131-
};
11321121

11331122
log = __nvme_alloc(sizeof(*log));
11341123
if (!log) {
@@ -1139,9 +1128,11 @@ static int nvme_discovery_log(const struct nvme_get_discovery_args *args,
11391128

11401129
nvme_msg(l->root, LOG_DEBUG, "%s: get header (try %d/%d)\n",
11411130
name, retries, args->max_retries);
1142-
log_args.log = log;
1143-
log_args.len = DISCOVERY_HEADER_LEN;
1144-
err = nvme_get_log_page(l, NVME_LOG_PAGE_PDU_SIZE, &log_args);
1131+
err = nvme_get_log(l, NVME_NSID_NONE, false, args->lsp,
1132+
NVME_LOG_LID_DISCOVER, NVME_LOG_LSI_NONE, NVME_CSI_NVM,
1133+
false, NVME_UUID_NONE,
1134+
0, log, DISCOVERY_HEADER_LEN,
1135+
NVME_LOG_PAGE_PDU_SIZE, NULL);
11451136
if (err) {
11461137
nvme_msg(l->root, LOG_INFO,
11471138
"%s: discover try %d/%d failed, error %d\n",
@@ -1171,10 +1162,11 @@ static int nvme_discovery_log(const struct nvme_get_discovery_args *args,
11711162
"%s: get %" PRIu64 " records (genctr %" PRIu64 ")\n",
11721163
name, numrec, genctr);
11731164

1174-
log_args.lpo = sizeof(*log);
1175-
log_args.log = log->entries;
1176-
log_args.len = entries_size;
1177-
err = nvme_get_log_page(l, NVME_LOG_PAGE_PDU_SIZE, &log_args);
1165+
err = nvme_get_log(l, NVME_NSID_NONE, false, args->lsp,
1166+
NVME_LOG_LID_DISCOVER, NVME_LOG_LSI_NONE, NVME_CSI_NVM,
1167+
false, NVME_UUID_NONE,
1168+
sizeof(*log), log->entries, entries_size,
1169+
NVME_LOG_PAGE_PDU_SIZE, NULL);
11781170
if (err) {
11791171
nvme_msg(l->root, LOG_INFO,
11801172
"%s: discover try %d/%d failed, error %d\n",
@@ -1188,10 +1180,11 @@ static int nvme_discovery_log(const struct nvme_get_discovery_args *args,
11881180
*/
11891181
nvme_msg(l->root, LOG_DEBUG, "%s: get header again\n", name);
11901182

1191-
log_args.lpo = 0;
1192-
log_args.log = log;
1193-
log_args.len = DISCOVERY_HEADER_LEN;
1194-
err = nvme_get_log_page(l, NVME_LOG_PAGE_PDU_SIZE, &log_args);
1183+
err = nvme_get_log(l, NVME_NSID_NONE, false, args->lsp,
1184+
NVME_LOG_LID_DISCOVER, NVME_LOG_LSI_NONE, NVME_CSI_NVM,
1185+
false, NVME_UUID_NONE,
1186+
0, log, DISCOVERY_HEADER_LEN,
1187+
NVME_LOG_PAGE_PDU_SIZE, NULL);
11951188
if (err) {
11961189
nvme_msg(l->root, LOG_INFO,
11971190
"%s: discover try %d/%d failed, error %d\n",

‎src/nvme/ioctl.c‎

Lines changed: 33 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -282,42 +282,6 @@ enum features {
282282
NVME_FEATURES_IOCSP_IOCSCI_MASK = 0xff,
283283
};
284284

285-
int nvme_get_log(nvme_link_t l, struct nvme_get_log_args *args)
286-
{
287-
__u32 numd = (args->len >> 2) - 1;
288-
__u16 numdu = numd >> 16, numdl = numd & 0xffff;
289-
290-
__u32 cdw10 = NVME_SET(args->lid, LOG_CDW10_LID) |
291-
NVME_SET(args->lsp, LOG_CDW10_LSP) |
292-
NVME_SET(!!args->rae, LOG_CDW10_RAE) |
293-
NVME_SET(numdl, LOG_CDW10_NUMDL);
294-
__u32 cdw11 = NVME_SET(numdu, LOG_CDW11_NUMDU) |
295-
NVME_SET(args->lsi, LOG_CDW11_LSI);
296-
__u32 cdw12 = args->lpo & 0xffffffff;
297-
__u32 cdw13 = args->lpo >> 32;
298-
__u32 cdw14 = NVME_SET(args->uuidx, LOG_CDW14_UUID) |
299-
NVME_SET(!!args->ot, LOG_CDW14_OT) |
300-
NVME_SET(args->csi, LOG_CDW14_CSI);
301-
302-
struct nvme_passthru_cmd cmd = {
303-
.opcode = nvme_admin_get_log_page,
304-
.nsid = args->nsid,
305-
.addr = (__u64)(uintptr_t)args->log,
306-
.data_len = args->len,
307-
.cdw10 = cdw10,
308-
.cdw11 = cdw11,
309-
.cdw12 = cdw12,
310-
.cdw13 = cdw13,
311-
.cdw14 = cdw14,
312-
.timeout_ms = args->timeout,
313-
};
314-
315-
if (args->args_size < sizeof(struct nvme_get_log_args))
316-
return -EINVAL;
317-
318-
return nvme_submit_admin_passthru(l, &cmd, args->result);
319-
}
320-
321285
static bool force_4k;
322286

323287
__attribute__((constructor))
@@ -450,13 +414,21 @@ static int nvme_uring_cmd_wait_complete(struct io_uring *ring, int n)
450414
}
451415
#endif
452416

453-
int nvme_get_log_page(nvme_link_t l, __u32 xfer_len, struct nvme_get_log_args *args)
417+
int nvme_get_log_partial(nvme_link_t l, struct nvme_passthru_cmd *cmd,
418+
__u64 lpo, void *log, __u32 len,
419+
__u32 xfer_len, __u32 *result)
454420
{
455-
__u64 offset = 0, xfer, data_len = args->len;
456-
__u64 start = args->lpo;
457-
bool retain = args->rae;
458-
void *ptr = args->log;
421+
__u64 offset = 0, xfer, data_len = len;
422+
__u64 start = lpo;
423+
void *ptr = log;
459424
int ret;
425+
bool rae;
426+
__u32 numd;
427+
__u16 numdu, numdl;
428+
bool retain = NVME_GET(cmd->cdw10, LOG_CDW10_RAE);
429+
__u32 cdw10 = cmd->cdw10 & (NVME_VAL(LOG_CDW10_LID) |
430+
NVME_VAL(LOG_CDW10_LSP));
431+
__u32 cdw11 = cmd->cdw11 & NVME_VAL(LOG_CDW11_LSI);
460432

461433
if (force_4k)
462434
xfer_len = NVME_LOG_PAGE_PDU_SIZE;
@@ -495,10 +467,21 @@ int nvme_get_log_page(nvme_link_t l, __u32 xfer_len, struct nvme_get_log_args *a
495467
* last portion of this log page so the data remains latched
496468
* during the fetch sequence.
497469
*/
498-
args->lpo = start + offset;
499-
args->len = xfer;
500-
args->log = ptr;
501-
args->rae = offset + xfer < data_len || retain;
470+
lpo = start + offset;
471+
numd = (xfer >> 2) - 1;
472+
numdu = numd >> 16;
473+
numdl = numd & 0xffff;
474+
rae = offset + xfer < data_len || retain;
475+
476+
cmd->cdw10 = cdw10 |
477+
NVME_SET(!!rae, LOG_CDW10_RAE) |
478+
NVME_SET(numdl, LOG_CDW10_NUMDL);
479+
cmd->cdw11 = cdw11 |
480+
NVME_SET(numdu, LOG_CDW11_NUMDU);
481+
cmd->cdw12 = lpo & 0xffffffff;
482+
cmd->cdw13 = lpo >> 32;
483+
cmd->data_len = xfer;
484+
cmd->addr = (__u64)(uintptr_t)ptr;
502485
#ifdef CONFIG_LIBURING
503486
if (io_uring_kernel_support == IO_URING_AVAILABLE && use_uring) {
504487
if (n >= NVME_URING_ENTRIES) {
@@ -512,7 +495,7 @@ int nvme_get_log_page(nvme_link_t l, __u32 xfer_len, struct nvme_get_log_args *a
512495
nvme_uring_cmd_exit(&ring);
513496
} else
514497
#endif
515-
ret = nvme_get_log(l, args);
498+
ret = nvme_submit_admin_passthru(l, cmd, result);
516499
if (ret)
517500
return ret;
518501

@@ -541,7 +524,7 @@ static int read_ana_chunk(nvme_link_t l, enum nvme_log_ana_lsp lsp, bool rae,
541524
__u32 len = min_t(__u32, log_end - *read, NVME_LOG_PAGE_PDU_SIZE);
542525
int ret;
543526

544-
ret = nvme_get_log_ana(l, lsp, rae, *read - log, len, *read);
527+
ret = nvme_get_log_ana(l, rae, lsp, *read - log, *read, len);
545528
if (ret)
546529
return ret;
547530

@@ -596,8 +579,9 @@ static int try_read_ana(nvme_link_t l, enum nvme_log_ana_lsp lsp, bool rae,
596579
return 0;
597580
}
598581

599-
int nvme_get_ana_log_atomic(nvme_link_t l, bool rgo, bool rae, unsigned int retries,
600-
struct nvme_ana_log *log, __u32 *len)
582+
int nvme_get_ana_log_atomic(nvme_link_t l, bool rae, bool rgo,
583+
struct nvme_ana_log *log, __u32 *len,
584+
unsigned int retries)
601585
{
602586
const enum nvme_log_ana_lsp lsp =
603587
rgo ? NVME_LOG_ANA_LSP_RGO_GROUPS_ONLY : 0;

0 commit comments

Comments
 (0)